12016-10-31T00:21:01 *** dgenr8 has quit IRC
22016-10-31T00:34:47 <BlueMatt> does anyone care about a 0.5ms speedup to CheckBlock?
32016-10-31T00:35:31 <gmaxwell> is it a simple clean change?
42016-10-31T00:35:46 <gmaxwell> I had speedups at that levle (somewhat faster) by merging loops over all the txn.
52016-10-31T00:36:11 * gmaxwell goes looking for those patches that were held off waiting for segwit.
62016-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
72016-10-31T00:36:43 <BlueMatt> i think it might actually be the first one, which is 2 line diff
82016-10-31T00:37:17 *** goatpig has quit IRC
92016-10-31T00:37:32 <sipa> do we even need that check?
102016-10-31T00:37:52 <BlueMatt> which? the transaction-length one? no, it is 100% redundant
112016-10-31T00:38:03 <BlueMatt> the duplicate-inputs one? unclear, probably not but we added it for a reason
122016-10-31T00:38:07 <BlueMatt> dont recall what it was
132016-10-31T00:38:24 <BlueMatt> i do remember that we had a reason, however
142016-10-31T00:38:25 <gmaxwell> was it related to bip30?
152016-10-31T00:39:33 *** AaronvanW has quit IRC
162016-10-31T00:40:22 <BlueMatt> hum...i dont see why? :/
172016-10-31T00:40:57 <BlueMatt> lol, all these half-empty blocks are fucking with my benchmarking :/
182016-10-31T00:42:00 <sipa> https://github.com/bitcoin/bitcoin/pull/443
192016-10-31T00:42:03 <sipa> you added this.
202016-10-31T00:42:38 <BlueMatt> yes, and i recall having a reason :(
212016-10-31T00:43:02 <gmaxwell> the pr explains, they were getting relayed.
222016-10-31T00:43:25 <gmaxwell> (or so says the pr)
232016-10-31T00:43:54 <BlueMatt> hum, that seems strange to me?
242016-10-31T00:44:20 <BlueMatt> i mean that was a long time ago, though, i guess before CCoins
252016-10-31T00:44:26 <BlueMatt> CCoinsViewCache stuff should have fixed this?
262016-10-31T00:44:59 <sipa> yes
272016-10-31T00:45:11 <sipa> i believe the ConnectInputs code at the time may not have been able to catch this
282016-10-31T00:45:21 <BlueMatt> yes, this sounds correct to me
292016-10-31T00:46:37 <sipa> it had an fBlock which was false for mempool txn, and it disabled part of the checks
302016-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.
312016-10-31T00:52:26 <BlueMatt> you mean in CheckBlock?
322016-10-31T00:52:38 <BlueMatt> theres only two over-tx loops
332016-10-31T00:52:52 <BlueMatt> no, 3
342016-10-31T00:53:18 <gmaxwell> there are three, coinbase check, checktransaction and sigops check.
352016-10-31T00:53:46 <BlueMatt> yea, should merge that, though the sigops loop is ~free
362016-10-31T00:54:21 <BlueMatt> well, its well under 1ms
372016-10-31T00:54:23 <BlueMatt> well, well under
382016-10-31T00:54:33 *** aalex has quit IRC
392016-10-31T00:54:58 <gmaxwell> IIRC I benchmarked it an it was about a 1ms change.
402016-10-31T00:55:01 * BlueMatt -> dinner
412016-10-31T00:55:12 <BlueMatt> hum, that would surprise me (or your system was slower than mine :p)
422016-10-31T00:55:39 <gmaxwell> well slower probably, -- probably benchmarked it on my laptop.
432016-10-31T00:58:30 *** aalex has joined #bitcoin-core-dev
442016-10-31T00:58:50 <gmaxwell> BlueMatt: or I flubbed the meaurement.
452016-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.
462016-10-31T01:06:03 *** BashCo has quit IRC
472016-10-31T01:15:47 *** aalex has quit IRC
482016-10-31T01:18:26 *** aalex has joined #bitcoin-core-dev
492016-10-31T01:19:33 *** CubicEarth has joined #bitcoin-core-dev
502016-10-31T01:20:10 *** tulip has quit IRC
512016-10-31T01:27:16 *** CubicEarth has quit IRC
522016-10-31T01:40:00 *** rebroad has joined #bitcoin-core-dev
532016-10-31T02:07:51 *** btcdrak has quit IRC
542016-10-31T02:20:33 *** rebroad has quit IRC
552016-10-31T02:50:08 *** Ylbam has quit IRC
562016-10-31T02:56:56 *** rebroad has joined #bitcoin-core-dev
572016-10-31T03:04:14 *** aalex has quit IRC
582016-10-31T03:08:23 *** aalex has joined #bitcoin-core-dev
592016-10-31T03:15:38 <gmaxwell> BlueMatt: in 9045 what initilizes data_hash?
602016-10-31T03:16:03 <BlueMatt> gmaxwell: default-initialized to null
612016-10-31T03:16:09 <BlueMatt> its a class
622016-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
632016-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)
642016-10-31T03:27:53 <BlueMatt> ofc actual block deserialization is really the largest time sink
652016-10-31T03:29:52 <gmaxwell> I don't believe you. http://0bin.net/paste/KLQCR8JpdMu5i-x2#VitVZ6nLLdJSu+EnHlCy70e95NLkWxxGgoeNtjY7JmZ
662016-10-31T03:36:29 *** DigiByteDev has joined #bitcoin-core-dev
672016-10-31T03:36:46 <gmaxwell> BlueMatt: ^
682016-10-31T03:37:41 *** dgenr8 has joined #bitcoin-core-dev
692016-10-31T03:37:46 <sipa> gmaxwell: ints are default not initialized
702016-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.
712016-10-31T03:40:23 <sipa> yes, it is
722016-10-31T03:40:46 <sipa> CNetMessage is a class, so all its members have their default constructor called
732016-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. :) )
742016-10-31T03:41:52 <sipa> we could in theory not initialize the member chars in a uint5
752016-10-31T03:41:57 <sipa> *uintw56
762016-10-31T03:42:06 <sipa> **uint256
772016-10-31T03:42:16 <sipa> but i think that would surprise people
782016-10-31T03:59:15 *** aalex has quit IRC
792016-10-31T03:59:44 *** rebroad has quit IRC
802016-10-31T04:01:18 *** rebroad has joined #bitcoin-core-dev
812016-10-31T04:03:30 *** aalex has joined #bitcoin-core-dev
822016-10-31T04:23:11 *** Alopex has quit IRC
832016-10-31T04:24:16 *** Alopex has joined #bitcoin-core-dev
842016-10-31T04:48:56 *** rebroad has quit IRC
852016-10-31T04:51:11 *** Alopex has quit IRC
862016-10-31T04:52:16 *** Alopex has joined #bitcoin-core-dev
872016-10-31T04:53:29 *** rebroad has joined #bitcoin-core-dev
882016-10-31T05:04:11 *** Alopex has quit IRC
892016-10-31T05:05:16 *** Alopex has joined #bitcoin-core-dev
902016-10-31T06:23:15 *** btcdrak has joined #bitcoin-core-dev
912016-10-31T06:43:53 *** echonaut has quit IRC
922016-10-31T06:44:49 *** echonaut has joined #bitcoin-core-dev
932016-10-31T06:58:04 *** rebroad has quit IRC
942016-10-31T06:58:44 *** wasi has quit IRC
952016-10-31T07:01:16 *** rebroad has joined #bitcoin-core-dev
962016-10-31T07:29:28 *** fengling has joined #bitcoin-core-dev
972016-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
982016-10-31T07:35:20 *** kadoban has quit IRC
992016-10-31T08:03:53 *** rebroad has quit IRC
1002016-10-31T08:12:15 <jonasschnelli> sipa: https://github.com/bitcoin/bitcoin/pull/8977/files fixes BlueMatt's #ifdef ENABLE_WALLET
1012016-10-31T08:17:51 *** rebroad has joined #bitcoin-core-dev
1022016-10-31T08:22:36 *** Alopex has quit IRC
1032016-10-31T08:23:42 *** Alopex has joined #bitcoin-core-dev
1042016-10-31T08:48:02 *** DigiByteDev has quit IRC
1052016-10-31T08:58:37 *** AaronvanW has joined #bitcoin-core-dev
1062016-10-31T08:58:37 *** AaronvanW has quit IRC
1072016-10-31T08:58:37 *** AaronvanW has joined #bitcoin-core-dev
1082016-10-31T09:06:19 *** cdecker has joined #bitcoin-core-dev
1092016-10-31T09:17:27 *** rebroad has quit IRC
1102016-10-31T09:21:09 *** jannes has joined #bitcoin-core-dev
1112016-10-31T09:24:11 *** DigiByteDev has joined #bitcoin-core-dev
1122016-10-31T09:26:16 *** justan0theruser has quit IRC
1132016-10-31T09:26:42 *** Ylbam has joined #bitcoin-core-dev
1142016-10-31T09:28:00 *** rebroad has joined #bitcoin-core-dev
1152016-10-31T09:34:04 *** rebroad has quit IRC
1162016-10-31T09:44:30 *** fengling has quit IRC
1172016-10-31T09:48:05 *** fengling has joined #bitcoin-core-dev
1182016-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)
1192016-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
1202016-10-31T10:06:42 *** fengling has quit IRC
1212016-10-31T10:07:51 *** fengling has joined #bitcoin-core-dev
1222016-10-31T10:50:29 *** cdecker has quit IRC
1232016-10-31T10:59:19 *** tonikt has quit IRC
1242016-10-31T11:01:57 *** laurentmt has joined #bitcoin-core-dev
1252016-10-31T11:02:23 *** laurentmt has quit IRC
1262016-10-31T11:05:11 *** Guyver2 has joined #bitcoin-core-dev
1272016-10-31T11:35:37 *** paveljanik has quit IRC
1282016-10-31T11:43:46 *** DigiByteDev has quit IRC
1292016-10-31T12:15:25 <achow101> ping gmaxwell
1302016-10-31T12:18:09 <Victorsueca> try to CTCP ping him? that usually shows on other tabs
1312016-10-31T12:32:37 <rabidus_> try ddos
1322016-10-31T12:32:49 <rabidus_> ehm... i didn't really say that
1332016-10-31T12:32:53 <rabidus_> that's not an advice
1342016-10-31T12:34:35 <luke-jr> â¦
1352016-10-31T12:34:52 <rabidus_> :'(
1362016-10-31T12:50:27 *** Chris_Stewart_5 has joined #bitcoin-core-dev
1372016-10-31T13:03:17 <luke-jr> https://www.reddit.com/r/Bitcoin/comments/5ac1a4/error_in_bitcoind_munmap_chunk_invalid_pointer/
1382016-10-31T13:05:26 *** tulip has joined #bitcoin-core-dev
1392016-10-31T13:08:09 *** atroxes has quit IRC
1402016-10-31T13:13:46 *** atroxes has joined #bitcoin-core-dev
1412016-10-31T13:14:54 *** Chris_Stewart_5 has quit IRC
1422016-10-31T13:31:02 *** Chris_Stewart_5 has joined #bitcoin-core-dev
1432016-10-31T13:31:27 <BlueMatt> sipa: I prefer adding an ifdef than having a segfault (also that was already merged in another pr)
1442016-10-31T13:38:36 *** Chris_Stewart_5 has quit IRC
1452016-10-31T13:48:26 *** face has joined #bitcoin-core-dev
1462016-10-31T13:50:13 *** rebroad has joined #bitcoin-core-dev
1472016-10-31T13:51:05 *** kadoban has joined #bitcoin-core-dev
1482016-10-31T13:51:48 *** ryanofsky has joined #bitcoin-core-dev
1492016-10-31T13:53:38 *** fengling has quit IRC
1502016-10-31T14:40:03 *** f0g has joined #bitcoin-core-dev
1512016-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
1522016-10-31T14:50:44 *** Victor_sueca has joined #bitcoin-core-dev
1532016-10-31T14:51:04 *** Victorsueca has quit IRC
1542016-10-31T14:51:23 *** Victorsueca has joined #bitcoin-core-dev
1552016-10-31T14:56:40 *** otium has joined #bitcoin-core-dev
1562016-10-31T15:10:48 *** tonikt has joined #bitcoin-core-dev
1572016-10-31T15:11:46 *** tonikt has quit IRC
1582016-10-31T15:13:22 *** tonikt has joined #bitcoin-core-dev
1592016-10-31T15:20:21 <sipa> BlueMatt: 8977 it seems
1602016-10-31T15:26:34 *** paveljanik has joined #bitcoin-core-dev
1612016-10-31T15:26:35 *** paveljanik has joined #bitcoin-core-dev
1622016-10-31T15:34:55 *** berndj has quit IRC
1632016-10-31T15:35:13 *** berndj has joined #bitcoin-core-dev
1642016-10-31T15:38:34 *** berndj has quit IRC
1652016-10-31T15:43:33 *** laurentmt has joined #bitcoin-core-dev
1662016-10-31T15:43:34 *** laurentmt has quit IRC
1672016-10-31T15:50:19 *** Chris_Stewart_5 has joined #bitcoin-core-dev
1682016-10-31T15:54:00 *** rebroad has quit IRC
1692016-10-31T16:02:52 *** kadoban has quit IRC
1702016-10-31T16:08:14 *** kadoban has joined #bitcoin-core-dev
1712016-10-31T16:16:23 *** tonikt has quit IRC
1722016-10-31T16:20:11 *** tulip has quit IRC
1732016-10-31T16:32:21 *** kadoban has quit IRC
1742016-10-31T16:32:49 *** kadoban has joined #bitcoin-core-dev
1752016-10-31T16:46:01 *** jannes has quit IRC
1762016-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
1772016-10-31T16:50:53 <BlueMatt> increase (of around 1.5ms in the 3-4ms function...)
1782016-10-31T16:51:38 *** jannes has joined #bitcoin-core-dev
1792016-10-31T16:53:54 *** Chris_Stewart_5 has quit IRC
1802016-10-31T16:54:47 <BlueMatt> nvm, 0.7ms
1812016-10-31T16:55:08 <BlueMatt> on my laptop: 8ms to deserialize, 11.1 -> 10.5ms for deserialize + checkblock
1822016-10-31T16:55:38 *** fengling has joined #bitcoin-core-dev
1832016-10-31T17:05:23 *** wasi has joined #bitcoin-core-dev
1842016-10-31T17:14:03 *** fengling has quit IRC
1852016-10-31T17:14:35 *** Chris_Stewart_5 has joined #bitcoin-core-dev
1862016-10-31T17:16:52 <sipa> BlueMatt: cool, so let's just get rid of the duplicates check
1872016-10-31T17:17:08 <BlueMatt> sipa: not so easy due to network partition risk, as sdaftuar and I just found out :(
1882016-10-31T17:18:02 <sipa> uh?
1892016-10-31T17:18:17 <sipa> ah.
1902016-10-31T17:18:25 <BlueMatt> oops, no, nvm, just makes it tricky-er to do pre-relay of compact blocks
1912016-10-31T17:18:40 <BlueMatt> since then CheckBlock becomes network-ban-partition-consensus
1922016-10-31T17:19:11 <BlueMatt> (0.13.1 will already ban you for this, so it cant get worse...)
1932016-10-31T17:20:53 <BlueMatt> sipa: re: #9026
1942016-10-31T17:22:02 <gmaxwell> bump the protocol version number. then you can decide on your behavior based on it.?
1952016-10-31T17:26:25 <BlueMatt> true, probably need to do that, though still really want to do for 0.13.2
1962016-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
1972016-10-31T17:33:52 *** Chris_Stewart_5 has quit IRC
1982016-10-31T17:50:45 *** molz has joined #bitcoin-core-dev
1992016-10-31T17:53:48 *** moli has quit IRC
2002016-10-31T18:01:09 *** CubicEarth has joined #bitcoin-core-dev
2012016-10-31T18:01:32 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2022016-10-31T18:06:33 *** Chris_Stewart_5 has quit IRC
2032016-10-31T18:10:13 <sipa> BlueMatt: i'm really curious what typo you made that got corrected into "I did not Rome the hashing"
2042016-10-31T18:10:40 <BlueMatt> dunno how my phone converted benchmark to rome, but it appears to have
2052016-10-31T18:11:25 <gmaxwell> .query eragmus
2062016-10-31T18:11:27 <gmaxwell> oops
2072016-10-31T18:11:49 *** Giszmo has quit IRC
2082016-10-31T18:12:01 *** Giszmo has joined #bitcoin-core-dev
2092016-10-31T18:16:01 *** laurentmt has joined #bitcoin-core-dev
2102016-10-31T18:16:15 *** laurentmt has quit IRC
2112016-10-31T18:19:21 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2122016-10-31T18:19:26 *** wolfspra1l has quit IRC
2132016-10-31T18:19:36 *** wolfspraul has joined #bitcoin-core-dev
2142016-10-31T18:28:20 *** echonaut has quit IRC
2152016-10-31T18:29:28 *** echonaut has joined #bitcoin-core-dev
2162016-10-31T18:29:48 *** Chris_Stewart_5 has quit IRC
2172016-10-31T18:38:51 *** dermoth_ has joined #bitcoin-core-dev
2182016-10-31T18:39:19 *** dermoth has quit IRC
2192016-10-31T18:39:21 *** dermoth_ is now known as dermoth
2202016-10-31T18:42:38 *** CubicEarth has quit IRC
2212016-10-31T18:45:18 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2222016-10-31T18:49:09 *** CubicEarth has joined #bitcoin-core-dev
2232016-10-31T18:49:41 *** MarcoFalke has joined #bitcoin-core-dev
2242016-10-31T18:49:59 <GitHub133> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d2143dc937e3...3d69ecb4edeb
2252016-10-31T18:50:00 <GitHub133> bitcoin/master 7f61b49 matthias: Change all instance of 'GMT epoch' to 'Unix epoch'
2262016-10-31T18:50:00 <GitHub133> bitcoin/master 3d69ecb MarcoFalke: Merge #9041: keypoololdest denote Unix epoch, not GMT...
2272016-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
2282016-10-31T18:51:37 *** CubicEarth has quit IRC
2292016-10-31T18:51:50 *** CubicEarth has joined #bitcoin-core-dev
2302016-10-31T18:52:33 *** Chris_Stewart_5 has quit IRC
2312016-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
2322016-10-31T18:56:05 *** CubicEarth has quit IRC
2332016-10-31T18:57:39 <sipa> duplicatble duplicate-input ?
2342016-10-31T18:58:44 *** berndj-blackout has joined #bitcoin-core-dev
2352016-10-31T19:02:38 *** berndj-blackout is now known as berndj
2362016-10-31T19:08:50 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2372016-10-31T19:33:01 *** MarcoFalke has left #bitcoin-core-dev
2382016-10-31T19:43:08 *** berndj has quit IRC
2392016-10-31T19:43:59 *** wasi has quit IRC
2402016-10-31T19:46:41 *** berndj has joined #bitcoin-core-dev
2412016-10-31T19:47:02 *** Chris_Stewart_5 has quit IRC
2422016-10-31T19:49:06 *** Chris_Stewart_5 has joined #bitcoin-core-dev
2432016-10-31T19:56:03 *** CubicEarth has joined #bitcoin-core-dev
2442016-10-31T20:08:18 *** Guyver2 has quit IRC
2452016-10-31T20:10:25 *** Guyver2 has joined #bitcoin-core-dev
2462016-10-31T20:29:01 *** dermoth_ has joined #bitcoin-core-dev
2472016-10-31T20:29:29 *** dermoth has quit IRC
2482016-10-31T20:29:31 *** dermoth_ is now known as dermoth
2492016-10-31T20:40:07 *** jannes has quit IRC
2502016-10-31T20:48:16 *** owowo has quit IRC
2512016-10-31T20:52:22 *** CubicEarth has quit IRC
2522016-10-31T20:53:42 *** owowo has joined #bitcoin-core-dev
2532016-10-31T20:54:21 *** Giszmo has quit IRC
2542016-10-31T20:57:29 *** owowo has quit IRC
2552016-10-31T21:04:13 *** owowo has joined #bitcoin-core-dev
2562016-10-31T21:22:51 *** droark has quit IRC
2572016-10-31T21:51:32 *** Guyver2 has quit IRC
2582016-10-31T21:53:20 *** silversword has joined #bitcoin-core-dev
2592016-10-31T21:54:29 *** mol has joined #bitcoin-core-dev
2602016-10-31T21:54:51 *** silversword has left #bitcoin-core-dev
2612016-10-31T21:58:17 *** molz has quit IRC
2622016-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
2632016-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?
2642016-10-31T22:10:36 <cfields_> gmaxwell: i believe that was fixed a while back with the initial CConnman merge.
2652016-10-31T22:10:43 <gmaxwell> Ah. good
2662016-10-31T22:11:11 <sipa> yes, that was fixed afaik
2672016-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
2682016-10-31T22:13:43 <gmaxwell> yup! that looks good. Okay, I'd missed that.
2692016-10-31T22:13:51 *** justanotheruser has joined #bitcoin-core-dev
2702016-10-31T22:16:35 <gmaxwell> cfields_: whats the motivation for making the nonce determinstic?
2712016-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.
2722016-10-31T22:19:04 <gmaxwell> uh. wait... you mean this is completely determinstic?
2732016-10-31T22:19:08 * gmaxwell goes to read the patch.
2742016-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
2752016-10-31T22:19:20 <cfields_> gmaxwell: heh, no. It's seeded at startup
2762016-10-31T22:19:41 <cfields_> wait, i hope...
2772016-10-31T22:19:59 <cfields_> yes, sorry. It's a per-connman seed.
2782016-10-31T22:20:13 <cfields_> you made me doubt myself for a sec :)
2792016-10-31T22:23:41 <gmaxwell> K. yes, I see that. Just the 'replay for testing' made me wonder!
2802016-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
2812016-10-31T22:27:07 <cfields_> heh, yes. NodeId is signed, so things could go wonky way before that
2822016-10-31T22:31:42 <cfields_> err
2832016-10-31T22:35:10 *** cryptapus has joined #bitcoin-core-dev
2842016-10-31T22:35:11 *** cryptapus has joined #bitcoin-core-dev
2852016-10-31T22:36:17 <gmaxwell> sipa: cfields_ just had a spurrious travis failure caused by the overly jumpy rand_bits test in libsecp256k1.
2862016-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.
2872016-10-31T22:39:01 <sipa> iirc i started a replacement for that at some point, but it remained too spurious
2882016-10-31T22:40:23 *** cryptapus has quit IRC
2892016-10-31T22:40:29 *** BonyM1 has quit IRC
2902016-10-31T22:41:41 <BlueMatt> cfields_: hum, hashing isnt much of a bottleneck here...
2912016-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.
2922016-10-31T22:43:30 <sipa> right, but it should be doable to bring that down to 1 in a billion runs or so
2932016-10-31T22:43:50 <BlueMatt> well, the compact blocks stuff is somewhat spurious
2942016-10-31T22:44:14 <BlueMatt> though i suppose it could effect regtest during semi-normal usage
2952016-10-31T22:44:55 <sipa> BlueMatt: this is about libsecp256k1
2962016-10-31T22:45:01 <BlueMatt> oh, sorry
2972016-10-31T22:45:04 <BlueMatt> get your own channel!
2982016-10-31T22:45:05 <BlueMatt> (jk)
2992016-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
3002016-10-31T22:48:15 <BlueMatt> cfields_: oops, see my respond on github
3012016-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
3022016-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.
3032016-10-31T22:52:38 <cfields_> hmm
3042016-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
3052016-10-31T22:54:24 *** BonyM1 has joined #bitcoin-core-dev
3062016-10-31T22:55:11 <sipa> and the bip151 hasher should be a few times faster
3072016-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.
3082016-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
3092016-10-31T22:56:23 * BlueMatt realizes he hasnt re-reviewed 8708, oops
3102016-10-31T22:56:24 <cfields_> BlueMatt: you can save the poke, i owe you an ack on that one already :)
3112016-10-31T22:56:30 <BlueMatt> heh, ok
3122016-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
3132016-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 :)
3142016-10-31T22:59:02 <BlueMatt> lol, so much to review
3152016-10-31T22:59:38 <sipa> 9039 grew a bit beyond what i originally planned to address, but i think the result is worth it
3162016-10-31T23:00:23 <BlueMatt> yea, i saw there was some back-and-forth, but, indeed, I'll bet thats worth it
3172016-10-31T23:00:33 <BlueMatt> serialization can always use cleaning
3182016-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.
3192016-10-31T23:01:07 <BlueMatt> we need an irc bot that posts pr titles after numbers are mentioned :p
3202016-10-31T23:01:32 <sipa> BlueMatt: yes!
3212016-10-31T23:01:41 <BlueMatt> where is nanotube when you need him?
3222016-10-31T23:01:43 <cfields_> BlueMatt: heh, yes, and links to commit ids
3232016-10-31T23:01:45 <BlueMatt> gribble: help me out bro
3242016-10-31T23:02:10 <cfields_> BlueMatt: i used that in another channel, iirc it was easy enough to setup
3252016-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
3262016-10-31T23:02:49 <BlueMatt> I mean its easy...4 digit number starting with a # and link to github.com/bitcoin/bitcoin/issue/#
3272016-10-31T23:03:11 <BlueMatt> cfields_: I'll try to get you another review on 8708 tonight, otherwise tomorrow morning
3282016-10-31T23:03:34 <sipa> BlueMatt, cfields_: can you two fight over the order in which 8708 and 8969?
3292016-10-31T23:03:53 <BlueMatt> should be minimal conflicts?
3302016-10-31T23:03:57 <sipa> ok
3312016-10-31T23:03:57 <BlueMatt> or at least easy-to-resolve ones?
3322016-10-31T23:04:02 <sipa> so much easier :)
3332016-10-31T23:04:08 <sipa> i assumed they'd conflict a lot
3342016-10-31T23:04:27 <BlueMatt> 8969 is some random trivial cleanups in preparation for the big split (tm)
3352016-10-31T23:04:34 <BlueMatt> so souldnt conflict with much of anything
3362016-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
3372016-10-31T23:06:25 <BlueMatt> sipa: I think 8708 is further along?
3382016-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
3392016-10-31T23:06:46 <BlueMatt> I think cfields_ and I probably agree on it now, just need to review
3402016-10-31T23:06:59 <BlueMatt> heh, ok
3412016-10-31T23:07:04 <sipa> ok
3422016-10-31T23:07:21 <sipa> i'll review 8708 in more detail too hen
3432016-10-31T23:07:23 <sipa> then
3442016-10-31T23:07:23 <cfields_> yea, it should be ready, i think, but it'll touch PushMessage again.
3452016-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
3462016-10-31T23:08:49 <cfields_> either way, doesn't matter at all to me.
3472016-10-31T23:09:09 <sipa> cfields_: i think it'll just boil down to removing "nType, nVersion, " everywhere in your patch
3482016-10-31T23:09:36 <cfields_> sipa: ah ok, that's trivial then.
3492016-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 :)
3502016-10-31T23:12:55 <BlueMatt> cfields_: cool
3512016-10-31T23:13:10 <BlueMatt> if we get all this shit in 0.14 I'm gonna be really impressed
3522016-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
3532016-10-31T23:20:33 <BlueMatt> cfields_: while you're at it, just change it to assert size >= HEADER_SIZE
3542016-10-31T23:23:18 <cfields_> BlueMatt: hm, thought I moved it. sorry about that. will make that change.
3552016-10-31T23:23:55 <BlueMatt> cfields_: while you're at it, you changed the LogPrint on sending messages
3562016-10-31T23:24:04 <BlueMatt> it used to read "sending: ..." you removed the sending
3572016-10-31T23:24:14 <BlueMatt> and you got rid of the SanitizeString() call there, though i doubt that matters
3582016-10-31T23:26:11 <cfields_> BlueMatt: grr, not sure why those got dropped. will fix.
3592016-10-31T23:26:25 <cfields_> crap, got to go help with dinner. bbl.
3602016-10-31T23:26:46 <BlueMatt> cfields_: ok, I'll queue them in review...gotta run as well, so might have to finish tomorrow
3612016-10-31T23:42:49 *** belcher has quit IRC
3622016-10-31T23:55:44 *** belcher has joined #bitcoin-core-dev