1 2016-10-31T00:21:01  *** dgenr8 has quit IRC
  2 2016-10-31T00:34:47  <BlueMatt> does anyone care about a 0.5ms speedup to CheckBlock?
  3 2016-10-31T00:35:31  <gmaxwell> is it a simple clean change?
  4 2016-10-31T00:35:46  <gmaxwell> I had speedups at that levle (somewhat faster) by merging loops over all the txn.
  5 2016-10-31T00:36:11  * gmaxwell goes looking for those patches that were held off waiting for segwit.
  6 2016-10-31T00:36:32  <BlueMatt> well half of it is (https://github.com/bitcoinfibre/bitcoinfibre/commit/e4884c9f706881fb33ed49f7098e1cea58807e87) the other half is only bareless less so (https://github.com/bitcoinfibre/bitcoinfibre/commit/8bd42b8f763fb736bfde4fba7390d49b9f3cccc6) though i havent benchmarked individually so not sure which is helping
  7 2016-10-31T00:36:43  <BlueMatt> i think it might actually be the first one, which is 2 line diff
  8 2016-10-31T00:37:17  *** goatpig has quit IRC
  9 2016-10-31T00:37:32  <sipa> do we even need that check?
 10 2016-10-31T00:37:52  <BlueMatt> which? the transaction-length one? no, it is 100% redundant
 11 2016-10-31T00:38:03  <BlueMatt> the duplicate-inputs one? unclear, probably not but we added it for a reason
 12 2016-10-31T00:38:07  <BlueMatt> dont recall what it was
 13 2016-10-31T00:38:24  <BlueMatt> i do remember that we had a reason, however
 14 2016-10-31T00:38:25  <gmaxwell> was it related to bip30?
 15 2016-10-31T00:39:33  *** AaronvanW has quit IRC
 16 2016-10-31T00:40:22  <BlueMatt> hum...i dont see why? :/
 17 2016-10-31T00:40:57  <BlueMatt> lol, all these half-empty blocks are fucking with my benchmarking :/
 18 2016-10-31T00:42:00  <sipa> https://github.com/bitcoin/bitcoin/pull/443
 19 2016-10-31T00:42:03  <sipa> you added this.
 20 2016-10-31T00:42:38  <BlueMatt> yes, and i recall having a reason :(
 21 2016-10-31T00:43:02  <gmaxwell> the pr explains, they were getting relayed.
 22 2016-10-31T00:43:25  <gmaxwell> (or so says the pr)
 23 2016-10-31T00:43:54  <BlueMatt> hum, that seems strange to me?
 24 2016-10-31T00:44:20  <BlueMatt> i mean that was a long time ago, though, i guess before CCoins
 25 2016-10-31T00:44:26  <BlueMatt> CCoinsViewCache stuff should have fixed this?
 26 2016-10-31T00:44:59  <sipa> yes
 27 2016-10-31T00:45:11  <sipa> i believe the ConnectInputs code at the time may not have been able to catch this
 28 2016-10-31T00:45:21  <BlueMatt> yes, this sounds correct to me
 29 2016-10-31T00:46:37  <sipa> it had an fBlock which was false for mempool txn, and it disabled part of the checks
 30 2016-10-31T00:49:53  <gmaxwell> removal trumps optimizing, though if you want to optimize there, I think I found a 1ms-scale speedup by fusing varrious loops over every transaction in the block into a single loop.
 31 2016-10-31T00:52:26  <BlueMatt> you mean in CheckBlock?
 32 2016-10-31T00:52:38  <BlueMatt> theres only two over-tx loops
 33 2016-10-31T00:52:52  <BlueMatt> no, 3
 34 2016-10-31T00:53:18  <gmaxwell> there are three, coinbase check, checktransaction and sigops check.
 35 2016-10-31T00:53:46  <BlueMatt> yea, should merge that, though the sigops loop is ~free
 36 2016-10-31T00:54:21  <BlueMatt> well, its well under 1ms
 37 2016-10-31T00:54:23  <BlueMatt> well, well under
 38 2016-10-31T00:54:33  *** aalex has quit IRC
 39 2016-10-31T00:54:58  <gmaxwell> IIRC I benchmarked it an it was about a 1ms change.
 40 2016-10-31T00:55:01  * BlueMatt -> dinner
 41 2016-10-31T00:55:12  <BlueMatt> hum, that would surprise me (or your system was slower than mine :p)
 42 2016-10-31T00:55:39  <gmaxwell> well slower probably, -- probably benchmarked it on my laptop.
 43 2016-10-31T00:58:30  *** aalex has joined #bitcoin-core-dev
 44 2016-10-31T00:58:50  <gmaxwell> BlueMatt: or I flubbed the meaurement.
 45 2016-10-31T00:59:30  <gmaxwell> But I think the opterations can basically be fused into CheckTransaction... e.g. takes a flag that indicates if its allowed to be a coinbase, and returns the sigops count.
 46 2016-10-31T01:06:03  *** BashCo has quit IRC
 47 2016-10-31T01:15:47  *** aalex has quit IRC
 48 2016-10-31T01:18:26  *** aalex has joined #bitcoin-core-dev
 49 2016-10-31T01:19:33  *** CubicEarth has joined #bitcoin-core-dev
 50 2016-10-31T01:20:10  *** tulip has quit IRC
 51 2016-10-31T01:27:16  *** CubicEarth has quit IRC
 52 2016-10-31T01:40:00  *** rebroad has joined #bitcoin-core-dev
 53 2016-10-31T02:07:51  *** btcdrak has quit IRC
 54 2016-10-31T02:20:33  *** rebroad has quit IRC
 55 2016-10-31T02:50:08  *** Ylbam has quit IRC
 56 2016-10-31T02:56:56  *** rebroad has joined #bitcoin-core-dev
 57 2016-10-31T03:04:14  *** aalex has quit IRC
 58 2016-10-31T03:08:23  *** aalex has joined #bitcoin-core-dev
 59 2016-10-31T03:15:38  <gmaxwell> BlueMatt: in 9045 what initilizes data_hash?
 60 2016-10-31T03:16:03  <BlueMatt> gmaxwell: default-initialized to null
 61 2016-10-31T03:16:09  <BlueMatt> its a class
 62 2016-10-31T03:26:31  <BlueMatt> gmaxwell: anyway, i'll look at optimizing it a bit tomorrow...see if i can write a bench_bitcoin thinggy for it
 63 2016-10-31T03:27:09  <BlueMatt> CheckBlock is not-insignificant in terms of time from wire to udp broadcast (which is right before disk write in AcceptBlock - same place compact block announcement will end up)
 64 2016-10-31T03:27:53  <BlueMatt> ofc actual block deserialization is really the largest time sink
 65 2016-10-31T03:29:52  <gmaxwell> I don't believe you. http://0bin.net/paste/KLQCR8JpdMu5i-x2#VitVZ6nLLdJSu+EnHlCy70e95NLkWxxGgoeNtjY7JmZ
 66 2016-10-31T03:36:29  *** DigiByteDev has joined #bitcoin-core-dev
 67 2016-10-31T03:36:46  <gmaxwell> BlueMatt: ^
 68 2016-10-31T03:37:41  *** dgenr8 has joined #bitcoin-core-dev
 69 2016-10-31T03:37:46  <sipa> gmaxwell: ints are default not initialized
 70 2016-10-31T03:40:00  <gmaxwell> oh, matt was saying the uint256 constructor is initing it. I thought he was saying because CNetMessage is a class all its members are initilized.
 71 2016-10-31T03:40:23  <sipa> yes, it is
 72 2016-10-31T03:40:46  <sipa> CNetMessage is a class, so all its members have their default constructor called
 73 2016-10-31T03:41:31  <gmaxwell> Yes, I'd just missed that the hash was a uint256. (or rather that a uint256 behaves differently from a primitive type. :) )
 74 2016-10-31T03:41:52  <sipa> we could in theory not initialize the member chars in a uint5
 75 2016-10-31T03:41:57  <sipa> *uintw56
 76 2016-10-31T03:42:06  <sipa> **uint256
 77 2016-10-31T03:42:16  <sipa> but i think that would surprise people
 78 2016-10-31T03:59:15  *** aalex has quit IRC
 79 2016-10-31T03:59:44  *** rebroad has quit IRC
 80 2016-10-31T04:01:18  *** rebroad has joined #bitcoin-core-dev
 81 2016-10-31T04:03:30  *** aalex has joined #bitcoin-core-dev
 82 2016-10-31T04:23:11  *** Alopex has quit IRC
 83 2016-10-31T04:24:16  *** Alopex has joined #bitcoin-core-dev
 84 2016-10-31T04:48:56  *** rebroad has quit IRC
 85 2016-10-31T04:51:11  *** Alopex has quit IRC
 86 2016-10-31T04:52:16  *** Alopex has joined #bitcoin-core-dev
 87 2016-10-31T04:53:29  *** rebroad has joined #bitcoin-core-dev
 88 2016-10-31T05:04:11  *** Alopex has quit IRC
 89 2016-10-31T05:05:16  *** Alopex has joined #bitcoin-core-dev
 90 2016-10-31T06:23:15  *** btcdrak has joined #bitcoin-core-dev
 91 2016-10-31T06:43:53  *** echonaut has quit IRC
 92 2016-10-31T06:44:49  *** echonaut has joined #bitcoin-core-dev
 93 2016-10-31T06:58:04  *** rebroad has quit IRC
 94 2016-10-31T06:58:44  *** wasi has quit IRC
 95 2016-10-31T07:01:16  *** rebroad has joined #bitcoin-core-dev
 96 2016-10-31T07:29:28  *** fengling has joined #bitcoin-core-dev
 97 2016-10-31T07:31:21  <sipa> BlueMatt: https://github.com/bitcoin/bitcoin/pull/8930/commits/37aefff5fcf7169a1b07ff8939850f630640f7e7 <- i remember there was some discussion about the reintroduction of #ifdef ENABLE_WALLET there, but i don't know where it was or what was said
 98 2016-10-31T07:35:20  *** kadoban has quit IRC
 99 2016-10-31T08:03:53  *** rebroad has quit IRC
100 2016-10-31T08:12:15  <jonasschnelli> sipa: https://github.com/bitcoin/bitcoin/pull/8977/files fixes BlueMatt's #ifdef ENABLE_WALLET
101 2016-10-31T08:17:51  *** rebroad has joined #bitcoin-core-dev
102 2016-10-31T08:22:36  *** Alopex has quit IRC
103 2016-10-31T08:23:42  *** Alopex has joined #bitcoin-core-dev
104 2016-10-31T08:48:02  *** DigiByteDev has quit IRC
105 2016-10-31T08:58:37  *** AaronvanW has joined #bitcoin-core-dev
106 2016-10-31T08:58:37  *** AaronvanW has quit IRC
107 2016-10-31T08:58:37  *** AaronvanW has joined #bitcoin-core-dev
108 2016-10-31T09:06:19  *** cdecker has joined #bitcoin-core-dev
109 2016-10-31T09:17:27  *** rebroad has quit IRC
110 2016-10-31T09:21:09  *** jannes has joined #bitcoin-core-dev
111 2016-10-31T09:24:11  *** DigiByteDev has joined #bitcoin-core-dev
112 2016-10-31T09:26:16  *** justan0theruser has quit IRC
113 2016-10-31T09:26:42  *** Ylbam has joined #bitcoin-core-dev
114 2016-10-31T09:28:00  *** rebroad has joined #bitcoin-core-dev
115 2016-10-31T09:34:04  *** rebroad has quit IRC
116 2016-10-31T09:44:30  *** fengling has quit IRC
117 2016-10-31T09:48:05  *** fengling has joined #bitcoin-core-dev
118 2016-10-31T10:00:11  <wumpus> gmaxwell: can you take a look here? https://github.com/bitcoin-dot-org/bitcoin.org/pull/1401 (re: final alert stuff)
119 2016-10-31T10:01:14  <wumpus> I think it can be merged but as you're going to send it, it makes sense if you sign off on it
120 2016-10-31T10:06:42  *** fengling has quit IRC
121 2016-10-31T10:07:51  *** fengling has joined #bitcoin-core-dev
122 2016-10-31T10:50:29  *** cdecker has quit IRC
123 2016-10-31T10:59:19  *** tonikt has quit IRC
124 2016-10-31T11:01:57  *** laurentmt has joined #bitcoin-core-dev
125 2016-10-31T11:02:23  *** laurentmt has quit IRC
126 2016-10-31T11:05:11  *** Guyver2 has joined #bitcoin-core-dev
127 2016-10-31T11:35:37  *** paveljanik has quit IRC
128 2016-10-31T11:43:46  *** DigiByteDev has quit IRC
129 2016-10-31T12:15:25  <achow101> ping gmaxwell
130 2016-10-31T12:18:09  <Victorsueca> try to CTCP ping him? that usually shows on other tabs
131 2016-10-31T12:32:37  <rabidus_> try ddos
132 2016-10-31T12:32:49  <rabidus_> ehm... i didn't really say that
133 2016-10-31T12:32:53  <rabidus_> that's not an advice
134 2016-10-31T12:34:35  <luke-jr> …
135 2016-10-31T12:34:52  <rabidus_> :'(
136 2016-10-31T12:50:27  *** Chris_Stewart_5 has joined #bitcoin-core-dev
137 2016-10-31T13:03:17  <luke-jr> https://www.reddit.com/r/Bitcoin/comments/5ac1a4/error_in_bitcoind_munmap_chunk_invalid_pointer/
138 2016-10-31T13:05:26  *** tulip has joined #bitcoin-core-dev
139 2016-10-31T13:08:09  *** atroxes has quit IRC
140 2016-10-31T13:13:46  *** atroxes has joined #bitcoin-core-dev
141 2016-10-31T13:14:54  *** Chris_Stewart_5 has quit IRC
142 2016-10-31T13:31:02  *** Chris_Stewart_5 has joined #bitcoin-core-dev
143 2016-10-31T13:31:27  <BlueMatt> sipa: I prefer adding an ifdef than having a segfault (also that was already merged in another pr)
144 2016-10-31T13:38:36  *** Chris_Stewart_5 has quit IRC
145 2016-10-31T13:48:26  *** face has joined #bitcoin-core-dev
146 2016-10-31T13:50:13  *** rebroad has joined #bitcoin-core-dev
147 2016-10-31T13:51:05  *** kadoban has joined #bitcoin-core-dev
148 2016-10-31T13:51:48  *** ryanofsky has joined #bitcoin-core-dev
149 2016-10-31T13:53:38  *** fengling has quit IRC
150 2016-10-31T14:40:03  *** f0g has joined #bitcoin-core-dev
151 2016-10-31T14:47:01  <GitHub117> [bitcoin] sdaftuar opened pull request #9048: [0.13 backport] Fix handling of invalid compact blocks (0.13...fix-invalid-cb-ban-0.13) https://github.com/bitcoin/bitcoin/pull/9048
152 2016-10-31T14:50:44  *** Victor_sueca has joined #bitcoin-core-dev
153 2016-10-31T14:51:04  *** Victorsueca has quit IRC
154 2016-10-31T14:51:23  *** Victorsueca has joined #bitcoin-core-dev
155 2016-10-31T14:56:40  *** otium has joined #bitcoin-core-dev
156 2016-10-31T15:10:48  *** tonikt has joined #bitcoin-core-dev
157 2016-10-31T15:11:46  *** tonikt has quit IRC
158 2016-10-31T15:13:22  *** tonikt has joined #bitcoin-core-dev
159 2016-10-31T15:20:21  <sipa> BlueMatt: 8977 it seems
160 2016-10-31T15:26:34  *** paveljanik has joined #bitcoin-core-dev
161 2016-10-31T15:26:35  *** paveljanik has joined #bitcoin-core-dev
162 2016-10-31T15:34:55  *** berndj has quit IRC
163 2016-10-31T15:35:13  *** berndj has joined #bitcoin-core-dev
164 2016-10-31T15:38:34  *** berndj has quit IRC
165 2016-10-31T15:43:33  *** laurentmt has joined #bitcoin-core-dev
166 2016-10-31T15:43:34  *** laurentmt has quit IRC
167 2016-10-31T15:50:19  *** Chris_Stewart_5 has joined #bitcoin-core-dev
168 2016-10-31T15:54:00  *** rebroad has quit IRC
169 2016-10-31T16:02:52  *** kadoban has quit IRC
170 2016-10-31T16:08:14  *** kadoban has joined #bitcoin-core-dev
171 2016-10-31T16:16:23  *** tonikt has quit IRC
172 2016-10-31T16:20:11  *** tulip has quit IRC
173 2016-10-31T16:32:21  *** kadoban has quit IRC
174 2016-10-31T16:32:49  *** kadoban has joined #bitcoin-core-dev
175 2016-10-31T16:46:01  *** jannes has quit IRC
176 2016-10-31T16:50:53  <BlueMatt> gmaxwell: so i went and combined everything into one loop of txn in a block (and even combined the sigops count loops pushed into checktransaction, etc...also removed the duplicative GetSerializeSize check in CheckTransaction (which is duplicative of CheckBlock) and the only part of that whole excersize that made any noticeable performance difference was removing the duplicate-input check, which made for a significant performance
177 2016-10-31T16:50:53  <BlueMatt> increase (of around 1.5ms in the 3-4ms function...)
178 2016-10-31T16:51:38  *** jannes has joined #bitcoin-core-dev
179 2016-10-31T16:53:54  *** Chris_Stewart_5 has quit IRC
180 2016-10-31T16:54:47  <BlueMatt> nvm, 0.7ms
181 2016-10-31T16:55:08  <BlueMatt> on my laptop: 8ms to deserialize, 11.1 -> 10.5ms for deserialize + checkblock
182 2016-10-31T16:55:38  *** fengling has joined #bitcoin-core-dev
183 2016-10-31T17:05:23  *** wasi has joined #bitcoin-core-dev
184 2016-10-31T17:14:03  *** fengling has quit IRC
185 2016-10-31T17:14:35  *** Chris_Stewart_5 has joined #bitcoin-core-dev
186 2016-10-31T17:16:52  <sipa> BlueMatt: cool, so let's just get rid of the duplicates check
187 2016-10-31T17:17:08  <BlueMatt> sipa: not so easy due to network partition risk, as sdaftuar and I just found out :(
188 2016-10-31T17:18:02  <sipa> uh?
189 2016-10-31T17:18:17  <sipa> ah.
190 2016-10-31T17:18:25  <BlueMatt> oops, no, nvm, just makes it tricky-er to do pre-relay of compact blocks
191 2016-10-31T17:18:40  <BlueMatt> since then CheckBlock becomes network-ban-partition-consensus
192 2016-10-31T17:19:11  <BlueMatt> (0.13.1 will already ban you for this, so it cant get worse...)
193 2016-10-31T17:20:53  <BlueMatt> sipa: re: #9026
194 2016-10-31T17:22:02  <gmaxwell> bump the protocol version number. then you can decide on your behavior based on it.?
195 2016-10-31T17:26:25  <BlueMatt> true, probably need to do that, though still really want to do for 0.13.2
196 2016-10-31T17:26:53  <BlueMatt> and then the problem is we dont want to change too often because you would just skip pre-relay for people not on your protocol version
197 2016-10-31T17:33:52  *** Chris_Stewart_5 has quit IRC
198 2016-10-31T17:50:45  *** molz has joined #bitcoin-core-dev
199 2016-10-31T17:53:48  *** moli has quit IRC
200 2016-10-31T18:01:09  *** CubicEarth has joined #bitcoin-core-dev
201 2016-10-31T18:01:32  *** Chris_Stewart_5 has joined #bitcoin-core-dev
202 2016-10-31T18:06:33  *** Chris_Stewart_5 has quit IRC
203 2016-10-31T18:10:13  <sipa> BlueMatt: i'm really curious what typo you made that got corrected into "I did not Rome the hashing"
204 2016-10-31T18:10:40  <BlueMatt> dunno how my phone converted benchmark to rome, but it appears to have
205 2016-10-31T18:11:25  <gmaxwell> .query eragmus
206 2016-10-31T18:11:27  <gmaxwell> oops
207 2016-10-31T18:11:49  *** Giszmo has quit IRC
208 2016-10-31T18:12:01  *** Giszmo has joined #bitcoin-core-dev
209 2016-10-31T18:16:01  *** laurentmt has joined #bitcoin-core-dev
210 2016-10-31T18:16:15  *** laurentmt has quit IRC
211 2016-10-31T18:19:21  *** Chris_Stewart_5 has joined #bitcoin-core-dev
212 2016-10-31T18:19:26  *** wolfspra1l has quit IRC
213 2016-10-31T18:19:36  *** wolfspraul has joined #bitcoin-core-dev
214 2016-10-31T18:28:20  *** echonaut has quit IRC
215 2016-10-31T18:29:28  *** echonaut has joined #bitcoin-core-dev
216 2016-10-31T18:29:48  *** Chris_Stewart_5 has quit IRC
217 2016-10-31T18:38:51  *** dermoth_ has joined #bitcoin-core-dev
218 2016-10-31T18:39:19  *** dermoth has quit IRC
219 2016-10-31T18:39:21  *** dermoth_ is now known as dermoth
220 2016-10-31T18:42:38  *** CubicEarth has quit IRC
221 2016-10-31T18:45:18  *** Chris_Stewart_5 has joined #bitcoin-core-dev
222 2016-10-31T18:49:09  *** CubicEarth has joined #bitcoin-core-dev
223 2016-10-31T18:49:41  *** MarcoFalke has joined #bitcoin-core-dev
224 2016-10-31T18:49:59  <GitHub133> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d2143dc937e3...3d69ecb4edeb
225 2016-10-31T18:50:00  <GitHub133> bitcoin/master 7f61b49 matthias: Change all instance of 'GMT epoch' to 'Unix epoch'
226 2016-10-31T18:50:00  <GitHub133> bitcoin/master 3d69ecb MarcoFalke: Merge #9041: keypoololdest denote Unix epoch, not GMT...
227 2016-10-31T18:50:14  <GitHub16> [bitcoin] MarcoFalke closed pull request #9041: keypoololdest denote Unix epoch, not GMT (master...patch-8) https://github.com/bitcoin/bitcoin/pull/9041
228 2016-10-31T18:51:37  *** CubicEarth has quit IRC
229 2016-10-31T18:51:50  *** CubicEarth has joined #bitcoin-core-dev
230 2016-10-31T18:52:33  *** Chris_Stewart_5 has quit IRC
231 2016-10-31T18:52:49  <GitHub112> [bitcoin] TheBlueMatt opened pull request #9049: Remove duplicatble duplicate-input check from CheckTransaction (master...2016-10-bench-checkblock) https://github.com/bitcoin/bitcoin/pull/9049
232 2016-10-31T18:56:05  *** CubicEarth has quit IRC
233 2016-10-31T18:57:39  <sipa> duplicatble duplicate-input ?
234 2016-10-31T18:58:44  *** berndj-blackout has joined #bitcoin-core-dev
235 2016-10-31T19:02:38  *** berndj-blackout is now known as berndj
236 2016-10-31T19:08:50  *** Chris_Stewart_5 has joined #bitcoin-core-dev
237 2016-10-31T19:33:01  *** MarcoFalke has left #bitcoin-core-dev
238 2016-10-31T19:43:08  *** berndj has quit IRC
239 2016-10-31T19:43:59  *** wasi has quit IRC
240 2016-10-31T19:46:41  *** berndj has joined #bitcoin-core-dev
241 2016-10-31T19:47:02  *** Chris_Stewart_5 has quit IRC
242 2016-10-31T19:49:06  *** Chris_Stewart_5 has joined #bitcoin-core-dev
243 2016-10-31T19:56:03  *** CubicEarth has joined #bitcoin-core-dev
244 2016-10-31T20:08:18  *** Guyver2 has quit IRC
245 2016-10-31T20:10:25  *** Guyver2 has joined #bitcoin-core-dev
246 2016-10-31T20:29:01  *** dermoth_ has joined #bitcoin-core-dev
247 2016-10-31T20:29:29  *** dermoth has quit IRC
248 2016-10-31T20:29:31  *** dermoth_ is now known as dermoth
249 2016-10-31T20:40:07  *** jannes has quit IRC
250 2016-10-31T20:48:16  *** owowo has quit IRC
251 2016-10-31T20:52:22  *** CubicEarth has quit IRC
252 2016-10-31T20:53:42  *** owowo has joined #bitcoin-core-dev
253 2016-10-31T20:54:21  *** Giszmo has quit IRC
254 2016-10-31T20:57:29  *** owowo has quit IRC
255 2016-10-31T21:04:13  *** owowo has joined #bitcoin-core-dev
256 2016-10-31T21:22:51  *** droark has quit IRC
257 2016-10-31T21:51:32  *** Guyver2 has quit IRC
258 2016-10-31T21:53:20  *** silversword has joined #bitcoin-core-dev
259 2016-10-31T21:54:29  *** mol has joined #bitcoin-core-dev
260 2016-10-31T21:54:51  *** silversword has left #bitcoin-core-dev
261 2016-10-31T21:58:17  *** molz has quit IRC
262 2016-10-31T22:03:01  <GitHub106> [bitcoin] theuni opened pull request #9050: net: make a few values immutible, and use deterministic randomness for the localnonce (master...connman-const) https://github.com/bitcoin/bitcoin/pull/9050
263 2016-10-31T22:09:40  <gmaxwell> cfields_: I haven't looked yet, but does that fix the current race condition, where if you connect to yourself then someone else connects to you then you get your own nonce, you'll not detect the connect to self?
264 2016-10-31T22:10:36  <cfields_> gmaxwell: i believe that was fixed a while back with the initial CConnman merge.
265 2016-10-31T22:10:43  <gmaxwell> Ah. good
266 2016-10-31T22:11:11  <sipa> yes, that was fixed afaik
267 2016-10-31T22:12:38  <cfields_> gmaxwell: if we're speaking of the same race, this should've fixed it: https://github.com/bitcoin/bitcoin/commit/960cf2e4058a9c195bf64e1aecb46024f9ef022a
268 2016-10-31T22:13:43  <gmaxwell> yup! that looks good. Okay, I'd missed that.
269 2016-10-31T22:13:51  *** justanotheruser has joined #bitcoin-core-dev
270 2016-10-31T22:16:35  <gmaxwell> cfields_: whats the motivation for making the nonce determinstic?
271 2016-10-31T22:18:16  <cfields_> gmaxwell: none really, other than it's one of the few (the only?) users of Rand in there. Eliminating them could ease replay for testing.
272 2016-10-31T22:19:04  <gmaxwell> uh. wait... you mean this is completely determinstic?
273 2016-10-31T22:19:08  * gmaxwell goes to read the patch.
274 2016-10-31T22:19:12  <cfields_> gmaxwell: really not much though, i just figured if it was immutible and per-connection, may as well make it deterministic if possible
275 2016-10-31T22:19:20  <cfields_> gmaxwell: heh, no. It's seeded at startup
276 2016-10-31T22:19:41  <cfields_> wait, i hope...
277 2016-10-31T22:19:59  <cfields_> yes, sorry. It's a per-connman seed.
278 2016-10-31T22:20:13  <cfields_> you made me doubt myself for a sec :)
279 2016-10-31T22:23:41  <gmaxwell> K. yes, I see that.  Just the 'replay for testing' made me wonder!
280 2016-10-31T22:24:31  <gmaxwell> so if someone manages to make enough connections to wrap the peer_id ... it'll repeat. OTOH if someone manages to do that lots of other things will probably break. :P
281 2016-10-31T22:27:07  <cfields_> heh, yes. NodeId is signed, so things could go wonky way before that
282 2016-10-31T22:31:42  <cfields_> err
283 2016-10-31T22:35:10  *** cryptapus has joined #bitcoin-core-dev
284 2016-10-31T22:35:11  *** cryptapus has joined #bitcoin-core-dev
285 2016-10-31T22:36:17  <gmaxwell> sipa: cfields_ just had a spurrious travis failure caused by the overly jumpy rand_bits test in libsecp256k1.
286 2016-10-31T22:37:42  <gmaxwell> sipa: IMO we should ifdef out the RNG tests and just run them once per release as part of a prerelease checklist or something.
287 2016-10-31T22:39:01  <sipa> iirc i started a replacement for that at some point, but it remained too spurious
288 2016-10-31T22:40:23  *** cryptapus has quit IRC
289 2016-10-31T22:40:29  *** BonyM1 has quit IRC
290 2016-10-31T22:41:41  <BlueMatt> cfields_: hum, hashing isnt much of a bottleneck here...
291 2016-10-31T22:42:39  <gmaxwell> sipa: I believe its the only rest with spurrious failures, and the rng tests are the only ones which _must_ have spurrious failures.
292 2016-10-31T22:43:30  <sipa> right, but it should be doable to bring that down to 1 in a billion runs or so
293 2016-10-31T22:43:50  <BlueMatt> well, the compact blocks stuff is somewhat spurious
294 2016-10-31T22:44:14  <BlueMatt> though i suppose it could effect regtest during semi-normal usage
295 2016-10-31T22:44:55  <sipa> BlueMatt: this is about libsecp256k1
296 2016-10-31T22:45:01  <BlueMatt> oh, sorry
297 2016-10-31T22:45:04  <BlueMatt> get your own channel!
298 2016-10-31T22:45:05  <BlueMatt> (jk)
299 2016-10-31T22:47:59  <cfields_> BlueMatt: i just can't think of any good reason for handling it on the socket thread, which lags more with each added connection, as opposed to on the message thread(/pool), potentially lazily, and only as needed
300 2016-10-31T22:48:15  <BlueMatt> cfields_: oops, see my respond on github
301 2016-10-31T22:49:12  <BlueMatt> its a latency question...our current hasher should (mostly) be able to keep up with 1Gbps of shit (ok, maybe more like 500Mbps), but putting it in the processing logic adds latency to block processing
302 2016-10-31T22:51:42  <gmaxwell> also we do no other computation in the thread that handles the incoming data... so this should better let it run concurrently with processing given the current setup.
303 2016-10-31T22:52:38  <cfields_> hmm
304 2016-10-31T22:53:11  <sipa> i would expect that in the future it's also easier to parallellize networking than it is to parallellize processing
305 2016-10-31T22:54:24  *** BonyM1 has joined #bitcoin-core-dev
306 2016-10-31T22:55:11  <sipa> and the bip151 hasher should be a few times faster
307 2016-10-31T22:55:12  <cfields_> sipa: heh, i argued the exact opposite in the PR. At the select/epoll/etc. level anyway. Though I suppose you could split the nodes into batches.
308 2016-10-31T22:55:15  <BlueMatt> cfields_: if you're bored, #8969 can probably get another ack and a merge, then we're only about 3 commits from splitting main, altough the next one will probably have to wait until the compact block fix goes in
309 2016-10-31T22:56:23  * BlueMatt realizes he hasnt re-reviewed 8708, oops
310 2016-10-31T22:56:24  <cfields_> BlueMatt: you can save the poke, i owe you an ack on that one already :)
311 2016-10-31T22:56:30  <BlueMatt> heh, ok
312 2016-10-31T22:56:47  <BlueMatt> just trying to move things in since its gonna be a painful merge cycle to get this all in for 0.4
313 2016-10-31T22:58:48  <sipa> cfields_: also, 8708 mentions constness... i'm addressing that to some extent in #8580, which now is queued after #9039 :)
314 2016-10-31T22:59:02  <BlueMatt> lol, so much to review
315 2016-10-31T22:59:38  <sipa> 9039 grew a bit beyond what i originally planned to address, but i think the result is worth it
316 2016-10-31T23:00:23  <BlueMatt> yea, i saw there was some back-and-forth, but, indeed, I'll bet thats worth it
317 2016-10-31T23:00:33  <BlueMatt> serialization can always use cleaning
318 2016-10-31T23:00:44  <cfields_> sipa: ack. Your serialization changes are next on my list to review. As it relates to 8708, i removed the const hack that i justified with "but we already do this elsewhere!", so my changes won't undo your fixes.
319 2016-10-31T23:01:07  <BlueMatt> we need an irc bot that posts pr titles after numbers are mentioned :p
320 2016-10-31T23:01:32  <sipa> BlueMatt: yes!
321 2016-10-31T23:01:41  <BlueMatt> where is nanotube when you need him?
322 2016-10-31T23:01:43  <cfields_> BlueMatt: heh, yes, and links to commit ids
323 2016-10-31T23:01:45  <BlueMatt> gribble: help me out bro
324 2016-10-31T23:02:10  <cfields_> BlueMatt: i used that in another channel, iirc it was easy enough to setup
325 2016-10-31T23:02:29  <BlueMatt> I'm sure, but I'm lazy...prefer if someone who's already running a bot in here does it :p
326 2016-10-31T23:02:49  <BlueMatt> I mean its easy...4 digit number starting with a # and link to github.com/bitcoin/bitcoin/issue/#
327 2016-10-31T23:03:11  <BlueMatt> cfields_: I'll try to get you another review on 8708 tonight, otherwise tomorrow morning
328 2016-10-31T23:03:34  <sipa> BlueMatt, cfields_: can you two fight over the order in which 8708 and 8969?
329 2016-10-31T23:03:53  <BlueMatt> should be minimal conflicts?
330 2016-10-31T23:03:57  <sipa> ok
331 2016-10-31T23:03:57  <BlueMatt> or at least easy-to-resolve ones?
332 2016-10-31T23:04:02  <sipa> so much easier :)
333 2016-10-31T23:04:08  <sipa> i assumed they'd conflict a lot
334 2016-10-31T23:04:27  <BlueMatt> 8969 is some random trivial cleanups in preparation for the big split (tm)
335 2016-10-31T23:04:34  <BlueMatt> so souldnt conflict with much of anything
336 2016-10-31T23:05:21  <sipa> cfields_: 9039 touches pretty much every line that deals with serialization, so it'll conflict with the beginning of 8708
337 2016-10-31T23:06:25  <BlueMatt> sipa: I think 8708 is further along?
338 2016-10-31T23:06:36  <cfields_> sipa: np, i don't mind rebasing that one. It'll need one more set of changes on top, in fact, so i'll take the burden there
339 2016-10-31T23:06:46  <BlueMatt> I think cfields_ and I probably agree on it now, just need to review
340 2016-10-31T23:06:59  <BlueMatt> heh, ok
341 2016-10-31T23:07:04  <sipa> ok
342 2016-10-31T23:07:21  <sipa> i'll review 8708 in more detail too hen
343 2016-10-31T23:07:23  <sipa> then
344 2016-10-31T23:07:23  <cfields_> yea, it should be ready, i think, but it'll touch PushMessage again.
345 2016-10-31T23:08:36  <cfields_> sipa: if it makes more sense to get the serialization changes in first, I could go ahead and make the other change in 8708, rather than breaking it out, that way it can be reviewed in parallel, then just rebase on the serialization changes
346 2016-10-31T23:08:49  <cfields_> either way, doesn't matter at all to me.
347 2016-10-31T23:09:09  <sipa> cfields_: i think it'll just boil down to removing "nType, nVersion, " everywhere in your patch
348 2016-10-31T23:09:36  <cfields_> sipa: ah ok, that's trivial then.
349 2016-10-31T23:12:45  <cfields_> BlueMatt: btw, i'm just a few steps away from splitting CConnman out of net, but I'll wait for the main split :)
350 2016-10-31T23:12:55  <BlueMatt> cfields_: cool
351 2016-10-31T23:13:10  <BlueMatt> if we get all this shit in 0.14 I'm gonna be really impressed
352 2016-10-31T23:19:27  <BlueMatt> cfields_: you missed one of my nits - you need to move the assert in EndMessage up above the first WriteLE32
353 2016-10-31T23:20:33  <BlueMatt> cfields_: while you're at it, just change it to assert size >= HEADER_SIZE
354 2016-10-31T23:23:18  <cfields_> BlueMatt: hm, thought I moved it. sorry about that. will make that change.
355 2016-10-31T23:23:55  <BlueMatt> cfields_: while you're at it, you changed the LogPrint on sending messages
356 2016-10-31T23:24:04  <BlueMatt> it used to read "sending: ..." you removed the sending
357 2016-10-31T23:24:14  <BlueMatt> and you got rid of the SanitizeString() call there, though i doubt that matters
358 2016-10-31T23:26:11  <cfields_> BlueMatt: grr, not sure why those got dropped. will fix.
359 2016-10-31T23:26:25  <cfields_> crap, got to go help with dinner. bbl.
360 2016-10-31T23:26:46  <BlueMatt> cfields_: ok, I'll queue them in review...gotta run as well, so might have to finish tomorrow
361 2016-10-31T23:42:49  *** belcher has quit IRC
362 2016-10-31T23:55:44  *** belcher has joined #bitcoin-core-dev