1 2016-08-10 01:59:49 0|Chris_Stewart_5|How do you distinguish between a txid node and non txid node in a MerkleBlock message? Is it simply the fact that that we have hit the maximum height on the binary tree?
2 2016-08-10 01:59:51 0|Chris_Stewart_5|https://bitcoin.org/en/developer-reference#parsing-a-merkleblock-message
3 2016-08-10 02:10:28 0|achow101|sipa: well clearly it doesn't work
4 2016-08-10 02:11:11 0|sipa|achow101: sorry, can you give a link to your compile output again?
5 2016-08-10 02:11:15 0|sipa|Chris_Stewart_5: eh...
6 2016-08-10 02:12:31 0|sipa|Chris_Stewart_5: right, you do by knowing the transaction count
7 2016-08-10 02:14:21 0|Chris_Stewart_5|sipa: Is there any other way with just a merkle block message? The docs distinguish between txids nodes (leaves in the full merkle tree) and non txid nodes
8 2016-08-10 02:14:55 0|Chris_Stewart_5|but, from what I can tell, you still need to compute the full tree with empty nodes to make that distinguishment? Am I missing something?
9 2016-08-10 02:20:35 0|achow101|sipa: https://gist.github.com/achow101/841c0c5bedaa3e0a6f641fa94b8e8c67
10 2016-08-10 02:26:21 0|sipa|Chris_Stewart_5: no you don't
11 2016-08-10 02:28:24 0|sipa|achow101: that really looks like you're compiling without c++11
12 2016-08-10 02:29:59 0|achow101|aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
13 2016-08-10 02:29:59 0|achow101|any idea on how to fix it?I'm using the lates versions I caaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
14 2016-08-10 02:30:00 0|achow101|aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaan get
15 2016-08-10 02:30:53 0|sipa|achow101: what are you doing exactly?
16 2016-08-10 02:30:56 0|sipa|achow101: is this gitian?
17 2016-08-10 02:31:39 0|achow101_|sorry the keyboard on my other computer spazzed out
18 2016-08-10 02:31:50 0|achow101_|it isn't gitian
19 2016-08-10 02:32:05 0|achow101_|gitian works fine
20 2016-08-10 02:32:11 0|sipa|so what are you doing?
21 2016-08-10 02:32:44 0|achow101_|cross compiling master for windows on Ubuntu 15.10
22 2016-08-10 02:35:39 0|sipa|using depends?
23 2016-08-10 02:36:11 0|achow101_|yes
24 2016-08-10 02:37:29 0|Chris_Stewart_5|sipa: What is the trick then to distinguish, the docs say:
25 2016-08-10 02:37:41 0|sipa|you just walk the tree
26 2016-08-10 02:37:52 0|sipa|you know the depth of each node you traverse
27 2016-08-10 02:38:03 0|sipa|you know the total depth of the tree at every point
28 2016-08-10 02:38:19 0|sipa|the rules tell you whether to descend or not
29 2016-08-10 02:38:48 0|Chris_Stewart_5|ugh thank you for the help -- been talking (or thinking) myself in circles all day. That makes sense
30 2016-08-10 02:39:04 0|Chris_Stewart_5|wait -- you derive the height from the tx count right?
31 2016-08-10 02:39:15 0|Chris_Stewart_5|2 ^ n?
32 2016-08-10 02:39:51 0|sipa|yes
33 2016-08-10 02:40:54 0|Chris_Stewart_5|much appreciated. Did you have a chance to look at my pull request for property based testing? Thoughts if so?
34 2016-08-10 02:44:44 0|sipa|Chris_Stewart_5: i haven't looked at it, but no concerns about adding such a framework
35 2016-08-10 05:49:38 0|GitHub128|[13bitcoin] 15sipa pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/6e6ab2c32382...484312bda2d4
36 2016-08-10 05:49:39 0|GitHub128|13bitcoin/06master 14484312b 15Pieter Wuille: Merge #8467: [Trivial] Do not shadow members in dbwrapper...
37 2016-08-10 05:49:39 0|GitHub128|13bitcoin/06master 144a35e0f 15Pavel JanÃÂk: Do not shadow members in dbwrapper
38 2016-08-10 05:49:49 0|GitHub86|[13bitcoin] 15sipa closed pull request #8467: [Trivial] Do not shadow members in dbwrapper (06master...0620160805_Wshadow_dbwrapper) 02https://github.com/bitcoin/bitcoin/pull/8467
39 2016-08-10 06:43:34 0|GitHub189|[13bitcoin] 15MarcoFalke opened pull request #8494: [init, wallet] ParameterInteraction() iff wallet enabled (06master...06Mf1608-initWalletParamInt) 02https://github.com/bitcoin/bitcoin/pull/8494
40 2016-08-10 07:03:03 0|jonasschnelli|Does bitcoin-cores UPNP (or UPNP in general) uses nat traversal or ICE? Or is the only way to get connection from the outside if your router supports uPNP and opens the port?
41 2016-08-10 07:19:56 0|gmaxwell|jonasschnelli: only opening the port.
42 2016-08-10 07:20:08 0|jonasschnelli|gmaxwell: Okay. Thanks.
43 2016-08-10 07:20:24 0|sipa|and for discovering external ip
44 2016-08-10 07:20:50 0|jonasschnelli|Would ICE works for Bitcoin? Or would that require UDP? (maybe off-topic here)
45 2016-08-10 07:22:05 0|GitHub4|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/484312bda2d4...edebf425a2df
46 2016-08-10 07:22:06 0|GitHub4|13bitcoin/06master 14160f895 15Luke Dashjr: Bugfix: Use pre-BIP141 sigops until segwit activates
47 2016-08-10 07:22:06 0|GitHub4|13bitcoin/06master 14239cbd2 15Luke Dashjr: qa/rpc-tests/segwit: Test GBT sigops before and after activation
48 2016-08-10 07:22:07 0|GitHub4|13bitcoin/06master 14edebf42 15Wladimir J. van der Laan: Merge #8489: Bugfix: Use pre-BIP141 sigops until segwit activates (GBT)...
49 2016-08-10 07:22:20 0|GitHub114|[13bitcoin] 15laanwj closed pull request #8489: Bugfix: Use pre-BIP141 sigops until segwit activates (GBT) (06master...06bugfix_gbt_sigops_presegwit) 02https://github.com/bitcoin/bitcoin/pull/8489
50 2016-08-10 07:23:11 0|GitHub28|13bitcoin/060.13 143f65ba2 15Pieter Wuille: Treat high-sigop transactions as larger rather than rejecting them
51 2016-08-10 07:23:11 0|GitHub28|[13bitcoin] 15laanwj pushed 2 new commits to 060.13: 02https://github.com/bitcoin/bitcoin/compare/114f7e944b1c...edc2c700a75c
52 2016-08-10 07:23:11 0|GitHub79|[13bitcoin] 15laanwj closed pull request #8438: [0.13] backport: Treat high-sigop transactions as larger rather than rejecting them (060.13...06btc-13-sigops) 02https://github.com/bitcoin/bitcoin/pull/8438
53 2016-08-10 07:23:12 0|GitHub28|13bitcoin/060.13 14edc2c70 15Wladimir J. van der Laan: Merge #8438: [0.13] backport: Treat high-sigop transactions as larger rather than rejecting them...
54 2016-08-10 07:23:42 0|gmaxwell|jonasschnelli: general traversal requires third party introduction, and no one technique generally works (and traversal for TCP almost never works)... I think firefox has a significant fraction of a million lines of code for nat traversal w/ rtcweb.
55 2016-08-10 07:24:06 0|jonasschnelli|heh.. okay. I see.
56 2016-08-10 07:24:49 0|gmaxwell|IMO, using tor hidden services is a /simpler/ way to bypass nat, as crazy as that sounds.
57 2016-08-10 07:27:42 0|jonasschnelli|gmaxwell: So using tor hidden service over 443 would probably allow bypassing firewalls (assume some large company firewalls where only 80,443 is open) to connect to the bitcoin network?
58 2016-08-10 07:30:21 0|gmaxwell|jonasschnelli: normally tor connections are on 443 and look superficially like regular https connections.
59 2016-08-10 07:30:38 0|jonasschnelli|gmaxwell: thanks.
60 2016-08-10 07:30:45 0|gmaxwell|and as long as you can connect out to the tor network you can run a hidden service where people can connect in.
61 2016-08-10 07:32:43 0|gmaxwell|sipa: we need to think about a replacement for addr messages at some point.
62 2016-08-10 07:32:57 0|gmaxwell|In particular, I2P and tor NG hidden services need more than 128 bits.
63 2016-08-10 07:33:14 0|gmaxwell|and it would be good to be able to carry a bit more metadata.
64 2016-08-10 07:35:50 0|sipa|gmaxwell: well in light of bip151... should they also carry a host pubkey?
65 2016-08-10 07:36:15 0|jonasschnelli|sipa: that would open the trust network problem?
66 2016-08-10 07:36:28 0|jonasschnelli|I think only pre-sharing makes more sense?
67 2016-08-10 07:37:36 0|jonasschnelli|Or what if malicious peers change the host in the addr messages?
68 2016-08-10 07:39:20 0|gmaxwell|sipa: I don't think thats very useful.
69 2016-08-10 07:39:43 0|GitHub30|[13bitcoin] 15laanwj closed pull request #8465: [0.13] Document reindexing changes (060.13...06docreindex) 02https://github.com/bitcoin/bitcoin/pull/8465
70 2016-08-10 07:39:45 0|GitHub185|13bitcoin/060.13 14b49d963 15Pieter Wuille: Document reindexing changes
71 2016-08-10 07:39:45 0|GitHub185|[13bitcoin] 15laanwj pushed 2 new commits to 060.13: 02https://github.com/bitcoin/bitcoin/compare/edc2c700a75c...45c656b914f0
72 2016-08-10 07:39:46 0|GitHub185|13bitcoin/060.13 1445c656b 15Wladimir J. van der Laan: Merge #8465: [0.13] Document reindexing changes...
73 2016-08-10 07:40:13 0|gmaxwell|largely unrelated to BIP151 there is a question of it you'd like to have long lived node identities.
74 2016-08-10 07:40:31 0|gmaxwell|and then support a 'connect to identity'
75 2016-08-10 07:40:53 0|gmaxwell|allowing you to, e.g. maintain connections to known well performing peers.
76 2016-08-10 07:41:16 0|gmaxwell|But that is basically 180degrees off of all the fingerprinting resistance we've worked on in the past.
77 2016-08-10 07:41:57 0|jonasschnelli|gmaxwell: I guess theres nothing holding back power users of doing this with the current bip151 (+auth) specification..
78 2016-08-10 07:42:05 0|gmaxwell|in that model you wouldn't have a 'addr' message, you'd have a 'nodeid' message, which is announcing a pubkey and one or more addresses it can be reached on, signed by that key.
79 2016-08-10 07:42:20 0|jonasschnelli|ah
80 2016-08-10 07:42:57 0|gmaxwell|so then there is a 'what if peers change the host' -- nothing can be changed because it's signed. And thing addrman would track is not addresses but IDs.. (and then when it wants to connect to an ID it would take that ID's latest addr announcement).
81 2016-08-10 07:43:37 0|sipa|gmaxwell: hmm, that's not what i'm thinking of
82 2016-08-10 07:43:51 0|gmaxwell|I know, but what you were thinking of is not useful.
83 2016-08-10 07:44:01 0|sipa|gmaxwell: more: we currently treat IP addresses as identities
84 2016-08-10 07:44:12 0|sipa|when you connect to the same IP again, you expect it to be the same identity
85 2016-08-10 07:44:35 0|gmaxwell|just stapling a pubkey to an addr message accomplishes nothing useful, anyone can and would change it in flight. If it doesn't match 'oh well, someone changed it in flight or the host changed'.
86 2016-08-10 07:44:57 0|gmaxwell|you might as well have just asked the host for a pubkey when you connected to it. :)
87 2016-08-10 07:44:58 0|jonasschnelli|Only if there would be a signature of the ip/port?
88 2016-08-10 07:45:29 0|sipa|when a peer tells you about a particular IP, you want to make sure that what you're connecting to is actually who they meant
89 2016-08-10 07:46:10 0|gmaxwell|but thats not really part of the addr message, its non-transitive.
90 2016-08-10 07:46:37 0|sipa|agree
91 2016-08-10 07:46:47 0|sipa|and i don't think we want a web-of-trust between nodes :)
92 2016-08-10 07:47:25 0|gmaxwell|I don't think thats very useful. If nodes really did have a persistant tracable identity, "this node was good in the past, I want to find it again" would be a useful service.
93 2016-08-10 07:47:36 0|gmaxwell|But it's in conflict with avoiding fingerprinting.
94 2016-08-10 07:49:50 0|GitHub120|[13bitcoin] 15luke-jr opened pull request #8495: [0.13] Bugfix: Use pre-BIP141 sigops until segwit activates (GBT) (060.13...06bugfix_gbt_sigops_presegwit-0.13.x) 02https://github.com/bitcoin/bitcoin/pull/8495
95 2016-08-10 07:49:52 0|gmaxwell|I suppose there is a middle ground where you can relay messages signed by P + H(P||blockhash[height//144*144]) or the like, and so if you know P (e.g. having previously been configured to authenticate towards that node), you can locate its updated addr messages.
96 2016-08-10 07:50:08 0|gmaxwell|But to everyone else host with key P does not have a long lived identity.
97 2016-08-10 09:17:40 0|GitHub177|[13bitcoin] 15MarcoFalke closed pull request #8477: [qa] Temporarily disable ipv6 in rpcbind test (06master...06Mf1608-qaIpv6) 02https://github.com/bitcoin/bitcoin/pull/8477
98 2016-08-10 13:04:52 0|GitHub52|13bitcoin/060.13 148b0eee6 15Luke Dashjr: Bugfix: Use pre-BIP141 sigops until segwit activates...
99 2016-08-10 13:04:52 0|GitHub52|[13bitcoin] 15laanwj pushed 1 new commit to 060.13: 02https://github.com/bitcoin/bitcoin/commit/8b0eee66e9c9057b6e53bb1f4a0a3821083e5df0
100 2016-08-10 13:05:22 0|GitHub7|[13bitcoin] 15laanwj closed pull request #8495: [0.13] Bugfix: Use pre-BIP141 sigops until segwit activates (GBT) (060.13...06bugfix_gbt_sigops_presegwit-0.13.x) 02https://github.com/bitcoin/bitcoin/pull/8495
101 2016-08-10 18:35:36 0|cfields|luke-jr: ping. gbt sends out the !segwit rule even if there are no segwit transactions included. that goes against my reading of the spec.
102 2016-08-10 18:35:53 0|cfields|luke-jr: (i'm implementing the bip9 gbt changes in ckpool)
103 2016-08-10 18:36:35 0|luke-jr|MAY != MUST
104 2016-08-10 18:36:57 0|gmaxwell|cfields: hm. It would be pretty surprising if the behavior modulates based on tx in mempool. I.e. your stuff catches fire when you're not looking.
105 2016-08-10 18:37:41 0|sipa|cfields: agree with luke-jr and gmaxwell here: you'd rather have mining infrastructure break at upgrade time than at segwit activation time
106 2016-08-10 18:37:51 0|sipa|where it potentially happen across all miners simultaneously
107 2016-08-10 18:37:53 0|luke-jr|'!' is required if there are such transactions, but optional (for non-segwit miners only) if there are not.
108 2016-08-10 18:38:08 0|gmaxwell|!segwit is super confusing. :(
109 2016-08-10 18:38:08 0|gribble|Error: "segwit" is not a valid command.
110 2016-08-10 18:38:21 0|gmaxwell|it even confuses gribble.
111 2016-08-10 18:38:23 0|luke-jr|lol
112 2016-08-10 18:38:25 0|cfields|heh, has not activated on gribble yet
113 2016-08-10 18:38:28 0|sipa|haha
114 2016-08-10 18:38:57 0|luke-jr|'*' would have probably made more sense, but I think it's too late to change it now?
115 2016-08-10 18:39:24 0|sipa|i never considered that '!' as part of a name would be interpreted as "not"
116 2016-08-10 18:39:52 0|sipa|and agree - i think it's not worth changing at this point
117 2016-08-10 18:40:02 0|cfields|heh. I read it as "segwit!" rather than "!segwit" and it's more clear :)
118 2016-08-10 18:40:18 0|sipa|yeah. it's "segwit! be aware!"
119 2016-08-10 18:40:56 0|cfields|either way, i'd say it's not entirely clear in the spec. I (wrongly) first implemented coinbase insertion based on the "!segwit" flag's presence.
120 2016-08-10 18:41:11 0|sipa|cfields: that's fine
121 2016-08-10 18:41:19 0|cfields|"On the other hand, when this prefix is used, it indicates a more subtle change to the block structure or generation transaction; examples of this would be BIP 34 (because it modifies coinbase construction)..."
122 2016-08-10 18:41:42 0|sipa|cfields: ah, no
123 2016-08-10 18:42:06 0|sipa|it means "you must understand why segwit means before you can mine on top of this, because things may have changed beyond normal transaction inclusion selection"
124 2016-08-10 18:43:18 0|cfields|got it. Ok, I'll PR a few clarifications there. Was trying to implement soley based on reading of bip9 changes.
125 2016-08-10 18:43:20 0|luke-jr|cfields: what's wrong with how you implemented it?
126 2016-08-10 18:43:40 0|sipa|but i don't see why "!segwit" -> "make coinbase commitment" would be a problem
127 2016-08-10 18:43:48 0|cfields|luke-jr: causes cb insertion even with no witness data
128 2016-08-10 18:43:55 0|sipa|cfields: which is fine
129 2016-08-10 18:43:59 0|sipa|just slightly wasteful
130 2016-08-10 18:44:05 0|gmaxwell|s/fine/desirable as far as I know.
131 2016-08-10 18:44:37 0|cfields|mm, i assumed the opposite. why desirable?
132 2016-08-10 18:44:52 0|sipa|because you'd test your infrastructures' compatibility ahead of time
133 2016-08-10 18:45:02 0|gmaxwell|Then the miner (and the network) can see their system correctly functioning.
134 2016-08-10 18:45:13 0|sipa|so things don't break all over the network when segwit activates
135 2016-08-10 18:45:16 0|gmaxwell|Otherwise you get the 'segwit tx shows up, now all the hashpower drops offline'
136 2016-08-10 18:45:46 0|cfields|ok, sure.
137 2016-08-10 18:59:20 0|cfields|sipa: in that case, can we append default_witness_commitment based on activation status rather than witness data presence?
138 2016-08-10 19:00:06 0|sipa|cfields: sure
139 2016-08-10 19:00:18 0|sipa|it's optional even when there are no witnesses in any transaction
140 2016-08-10 19:00:53 0|sipa|the rule is (after activation): 1) if the witness commitment is present, it must be correct 2) if there are any witnesses in any transaction, the witness commitment must be present
141 2016-08-10 19:02:47 0|gmaxwell|I think it's a good idea to have it present as soon as software is updated.
142 2016-08-10 19:03:00 0|gmaxwell|otherwise there is a behavior change at activation that might go wrong.
143 2016-08-10 19:03:45 0|sipa|it even gives us a way to observe miner adoption before the start date
144 2016-08-10 19:04:21 0|luke-jr|hmm, that's a point
145 2016-08-10 19:04:48 0|cfields|sure, i can do it unconditionally if desired
146 2016-08-10 19:06:01 0|cfields|we could also have 13.0 warn on 1), though it's a bit late for that
147 2016-08-10 21:19:33 0|alfas|luke
148 2016-08-10 22:44:57 0|jtimon|kanzure: https://github.com/bitcoin/bitcoin/pull/8493 looks like a long branch (because it has many tiny steps commits that could be squashed) but it's only +373 âËâ94 , please check it out
149 2016-08-10 22:45:41 0|kanzure|i have no bandwidth to physically test this
150 2016-08-10 22:46:08 0|kanzure|oh it looks like nothing added
151 2016-08-10 22:46:23 0|jtimon|BlueMatt: adviced to just move to the next simplest things to expose instead of trying to encapsulate verifyBlock without exposing anything new
152 2016-08-10 22:46:26 0|sipa|i now envision kanzure implementing this patch on a babbage machine, and then furiously turning the handles until it works
153 2016-08-10 22:46:58 0|kanzure|sipa: yeah i can be fairly serious about my esoteric architectures
154 2016-08-10 22:47:34 0|jtimon|kanzure: alternative APIs very welcomed
155 2016-08-10 22:48:12 0|kanzure|btw there is a typo in your pull request title
156 2016-08-10 22:48:38 0|jtimon|or alternative refactors, whatever, let's just try to force that PR to rebase and become smaller than +373 âËâ94
157 2016-08-10 22:49:50 0|jtimon|kanzure: mhmm, it seems I wrote Expose correctly and I made up the other 3 words, can you be more specific?
158 2016-08-10 22:50:04 0|kanzure|'libcosnensus'
159 2016-08-10 22:50:11 0|jtimon|oh, right
160 2016-08-10 22:54:38 0|kanzure|small concerns about name of 'ContextualCheckHeader' but this is only a nit
161 2016-08-10 22:55:12 0|kanzure|and i think this name is inherited from old code anyway, so it's a tough nit to do anything about
162 2016-08-10 22:57:50 0|jtimon|BlueMatt: also adviced not to be afraid of creating ugly wrappers or duplicating code if that was simpler to expose something
163 2016-08-10 22:58:58 0|jtimon|in this case I'm renaming main::ContextualCheckBlockHeader to Consensus::ContextualCheckHeader, but I'm keeping one with the original name in cppwrappers.o to avoid disruption in main
164 2016-08-10 22:59:42 0|jtimon|but yeah "Consensus::ContextualCheckHeader" it's a new name and it's open for bike-shedding, just like Consensus::VerifyHeader
165 2016-08-10 23:01:10 0|jtimon|I added Pow as a prefix for the 2 pow.o functions that needed a wrapper, also open for bikeshedding (but ideally the necessary preparations for not needing the wrappers should be done beforehand IMO)
166 2016-08-10 23:03:11 0|jtimon|if we do that, we don't even need to rename ContextualCheckBlockHeader,I don't care either way, for me it's just two paths to the same place
167 2016-08-10 23:04:19 0|jtimon|but yeah please give your opinion
168 2016-08-10 23:05:58 0|jtimon|I'm sure that some early nits can save me some work while turning jtimon/0.13-consensus-flags into another PR (WIP)
169 2016-08-10 23:06:41 0|jtimon|I want to expose get_flags too, but that sounds more crazy than verifyHeaders I think
170 2016-08-10 23:09:37 0|jtimon|where I would like more input is in https://github.com/bitcoin/bitcoin/pull/8493/files#diff-c2a099d775bac1dccc5f146a3cda81b8R11
171 2016-08-10 23:10:28 0|jtimon|or alternatives
172 2016-08-10 23:13:10 0|jtimon|and of course tests and testing
173 2016-08-10 23:14:34 0|jtimon|anyway, spammed the channel enough already...just please review