12017-06-08T00:06:55 *** chjj has quit IRC
22017-06-08T00:10:13 *** jamesob has joined #bitcoin-core-dev
32017-06-08T00:14:32 *** jamesob has quit IRC
42017-06-08T00:14:38 <achow101> is there any particular reason to be gathering coverage data for leveldb?
52017-06-08T00:14:54 <achow101> cfields_: ^^ (you added it 4 years ago)
62017-06-08T00:18:23 <gmaxwell> achow101: when reporting you can easily filter it out by passing through lcov -r
72017-06-08T00:18:53 <gmaxwell> I think it might be of interest at some point to know figures for how much of leveldb that our tests cover.
82017-06-08T00:19:32 <gmaxwell> though kinda odd to do it by default
92017-06-08T00:19:58 <achow101> lcov -r is really slow, but I have a workaround for that :)
102017-06-08T00:20:58 *** chjj has joined #bitcoin-core-dev
112017-06-08T00:21:01 <achow101> I don't think having it is necessary so we can just skip gathering data for it altogether
122017-06-08T00:27:59 *** coredump__ has joined #bitcoin-core-dev
132017-06-08T00:31:35 *** Chris_Stewart_5 has quit IRC
142017-06-08T00:32:07 <gmaxwell> faster computer. :P
152017-06-08T00:32:47 <gmaxwell> the way the lcov stuff is setup in our build seems kinda convoluted to me, but I'm not maintaining it. I normally just do it with CFLAGS/CCFLAGS overriding and buildings.
162017-06-08T00:34:22 <sipa> gmaxwell: achow101 reimplemented lcov -r in python, making it run 60x faster
172017-06-08T00:34:29 <sipa> (20 minutes -> 20 sec)
182017-06-08T00:36:39 <gmaxwell> lol
192017-06-08T00:40:43 <bitcoin-git> [bitcoin] jtimon closed pull request #9579: Net: Trivial-review: Make SendMessages easier to review (master...0.15-split-sendmessages) https://github.com/bitcoin/bitcoin/pull/9579
202017-06-08T00:42:53 <jtimon> can https://travis-ci.org/bitcoin/bitcoin/jobs/239728492 be restarted ?
212017-06-08T00:48:49 *** justan0theruser has joined #bitcoin-core-dev
222017-06-08T00:51:56 *** justanotheruser has quit IRC
232017-06-08T00:58:16 *** dabura667 has joined #bitcoin-core-dev
242017-06-08T00:59:12 *** Ylbam has quit IRC
252017-06-08T01:03:12 <bitcoin-git> [bitcoin] achow101 opened pull request #10552: [Test] Tests for zmqpubrawtx and zmqpubrawblock (master...zmq-raw-tests) https://github.com/bitcoin/bitcoin/pull/10552
262017-06-08T01:16:59 *** dabura667 has quit IRC
272017-06-08T01:21:32 *** dabura667 has joined #bitcoin-core-dev
282017-06-08T01:30:03 *** dermoth has quit IRC
292017-06-08T01:31:02 *** dermoth has joined #bitcoin-core-dev
302017-06-08T01:34:03 *** dgenr8 has quit IRC
312017-06-08T01:34:28 *** dgenr8 has joined #bitcoin-core-dev
322017-06-08T01:45:32 *** Dyaheon has quit IRC
332017-06-08T01:46:48 *** Dyaheon has joined #bitcoin-core-dev
342017-06-08T02:11:45 *** jamesob has joined #bitcoin-core-dev
352017-06-08T02:15:57 *** jamesob has quit IRC
362017-06-08T02:31:45 *** jamesob has joined #bitcoin-core-dev
372017-06-08T02:41:23 *** RubenSomsen has joined #bitcoin-core-dev
382017-06-08T02:42:50 *** RubenSomsen has quit IRC
392017-06-08T02:43:43 *** RubenSomsen has joined #bitcoin-core-dev
402017-06-08T02:46:13 *** RubenSomsen has quit IRC
412017-06-08T02:47:21 *** RubenSomsen has joined #bitcoin-core-dev
422017-06-08T02:47:57 *** RubenSomsen has quit IRC
432017-06-08T02:48:17 *** RubenSomsen has joined #bitcoin-core-dev
442017-06-08T02:50:00 *** RubenSomsen has quit IRC
452017-06-08T02:51:11 *** RubenSomsen has joined #bitcoin-core-dev
462017-06-08T02:54:19 *** RubenSomsen has quit IRC
472017-06-08T03:22:32 *** Chris_Stewart_5 has joined #bitcoin-core-dev
482017-06-08T03:24:39 <Chris_Stewart_5> bitcoind in -regtest activates all soft forks (+ segwit) and enforces all Policy flags after generating ~500 blocks right?
492017-06-08T03:35:08 *** justan0theruser is now known as justanotheruser
502017-06-08T03:41:46 *** shesek has joined #bitcoin-core-dev
512017-06-08T03:49:40 *** beatrootfarmer has joined #bitcoin-core-dev
522017-06-08T03:53:31 *** goatturneer has quit IRC
532017-06-08T04:06:49 *** beatrootfarmer has quit IRC
542017-06-08T04:08:19 *** beatrootfarmer has joined #bitcoin-core-dev
552017-06-08T04:13:57 *** chjj has quit IRC
562017-06-08T04:16:45 *** jtimon has quit IRC
572017-06-08T04:17:45 *** chjj has joined #bitcoin-core-dev
582017-06-08T04:41:08 *** Chris_Stewart_5 has quit IRC
592017-06-08T04:51:49 *** goatturneer has joined #bitcoin-core-dev
602017-06-08T04:55:35 *** beatrootfarmer has quit IRC
612017-06-08T05:03:42 *** beatrootfarmer has joined #bitcoin-core-dev
622017-06-08T05:07:32 *** goatturneer has quit IRC
632017-06-08T05:30:16 *** fanquake has joined #bitcoin-core-dev
642017-06-08T05:35:36 *** goatturneer has joined #bitcoin-core-dev
652017-06-08T05:38:59 *** beatrootfarmer has quit IRC
662017-06-08T05:51:25 *** goatturner has joined #bitcoin-core-dev
672017-06-08T05:54:44 *** goatturneer has quit IRC
682017-06-08T06:05:07 *** Deadhand has quit IRC
692017-06-08T06:12:47 *** laurentmt has joined #bitcoin-core-dev
702017-06-08T06:13:19 *** laurentmt has quit IRC
712017-06-08T06:13:20 *** justan0theruser has joined #bitcoin-core-dev
722017-06-08T06:13:50 *** justanotheruser has quit IRC
732017-06-08T06:25:09 *** afk11 has quit IRC
742017-06-08T06:26:07 *** afk11 has joined #bitcoin-core-dev
752017-06-08T06:26:16 *** jamesob has quit IRC
762017-06-08T06:27:43 *** Deadhand has joined #bitcoin-core-dev
772017-06-08T06:30:26 *** Dyaheon has quit IRC
782017-06-08T06:32:39 *** Dyaheon has joined #bitcoin-core-dev
792017-06-08T06:41:08 *** Ylbam has joined #bitcoin-core-dev
802017-06-08T06:46:25 <bitcoin-git> [bitcoin] fanquake closed pull request #10477: Use C++ initializer to initialze map and implement map comparator as const (master...master) https://github.com/bitcoin/bitcoin/pull/10477
812017-06-08T06:48:22 *** hephaestus has quit IRC
822017-06-08T06:49:53 *** afk11 has quit IRC
832017-06-08T06:52:25 *** afk11 has joined #bitcoin-core-dev
842017-06-08T06:53:11 *** BashCo has quit IRC
852017-06-08T06:59:58 <kallewoof> fanquake: I think you got the @name wrong in the comment on #10477 btw. (cg31 opened, you said @benma)
862017-06-08T07:00:00 <gribble> https://github.com/bitcoin/bitcoin/issues/10477 | Use C++ initializer to initialze map and implement map comparator as const by cg31 · Pull Request #10477 · bitcoin/bitcoin · GitHub
872017-06-08T07:07:08 *** SopaXorzTaker has joined #bitcoin-core-dev
882017-06-08T07:10:00 <wumpus> hhm thinking whether to add statistics such as number of incoming/outgoing connections, connections per bind point, to getnetworkinfo or just make a statistics script based on getpeerinfo on top for myself
892017-06-08T07:10:25 <wumpus> the # in/out connections is in the GUI, so in principle it's make sense to add it to RPC
902017-06-08T07:13:43 *** btcdrak has quit IRC
912017-06-08T07:14:48 *** dabura667 has quit IRC
922017-06-08T07:15:16 *** riemann has joined #bitcoin-core-dev
932017-06-08T07:15:16 *** dabura667 has joined #bitcoin-core-dev
942017-06-08T07:17:15 *** BashCo has joined #bitcoin-core-dev
952017-06-08T07:17:54 *** fanquake has joined #bitcoin-core-dev
962017-06-08T07:18:36 <fanquake> kallewoof oops, corrected.
972017-06-08T07:37:59 *** coredump__ has quit IRC
982017-06-08T07:39:18 *** riemann has quit IRC
992017-06-08T07:39:37 *** riemann has joined #bitcoin-core-dev
1002017-06-08T07:43:32 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/e801084decf4...6c2d81f34dc4
1012017-06-08T07:43:32 <bitcoin-git> bitcoin/master 0abc588 practicalswift: [tests] Remove printf(...)
1022017-06-08T07:43:33 <bitcoin-git> bitcoin/master 6c2d81f Wladimir J. van der Laan: Merge #10524: [tests] Remove printf(...)...
1032017-06-08T07:44:06 <bitcoin-git> [bitcoin] laanwj closed pull request #10524: [tests] Remove printf(...) (master...u-for-unsigned-int) https://github.com/bitcoin/bitcoin/pull/10524
1042017-06-08T08:19:25 *** Giszmo has quit IRC
1052017-06-08T08:21:09 *** timothy has joined #bitcoin-core-dev
1062017-06-08T08:28:02 *** cryptapus_afk has quit IRC
1072017-06-08T08:38:03 *** comboy has quit IRC
1082017-06-08T08:38:08 *** comboy_ has joined #bitcoin-core-dev
1092017-06-08T08:41:22 *** justan0theruser is now known as justanotheruser
1102017-06-08T08:49:14 <bitcoin-git> [bitcoin] practicalswift opened pull request #10553: Simplify "bool x = y ? true : false". Remove unused function and trailing semicolon. (master...minor-cleanups) https://github.com/bitcoin/bitcoin/pull/10553
1112017-06-08T09:08:57 *** Ylbam has quit IRC
1122017-06-08T09:23:49 *** fanquake has quit IRC
1132017-06-08T09:25:15 *** btcdrak has joined #bitcoin-core-dev
1142017-06-08T09:51:20 *** tunafizz has quit IRC
1152017-06-08T09:51:27 *** tunafizz has joined #bitcoin-core-dev
1162017-06-08T10:18:17 *** coredump_ has joined #bitcoin-core-dev
1172017-06-08T10:25:06 *** cryptapus_afk has joined #bitcoin-core-dev
1182017-06-08T10:29:41 *** goatturneer has joined #bitcoin-core-dev
1192017-06-08T10:33:15 *** jtimon has joined #bitcoin-core-dev
1202017-06-08T10:33:44 *** goatturner has quit IRC
1212017-06-08T10:35:26 *** cryptapus_afk has quit IRC
1222017-06-08T10:40:06 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/6c2d81f34dc4...71ab6e553856
1232017-06-08T10:40:06 <bitcoin-git> bitcoin/master 227ae9b practicalswift: [tests] Use FastRandomContext instead of boost::random::{mt19937,uniform_int_distribution}
1242017-06-08T10:40:07 <bitcoin-git> bitcoin/master 71ab6e5 Wladimir J. van der Laan: Merge #10547: [tests] Use FastRandomContext instead of boost::random::{mt19937,uniform_int_distribution}...
1252017-06-08T10:40:48 <bitcoin-git> [bitcoin] laanwj closed pull request #10547: [tests] Use FastRandomContext instead of boost::random::{mt19937,uniform_int_distribution} (master...remove-boost-random-dependency) https://github.com/bitcoin/bitcoin/pull/10547
1262017-06-08T10:41:02 *** coredump_ has quit IRC
1272017-06-08T10:45:57 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/71ab6e553856...35e7f13f68f9
1282017-06-08T10:45:57 <bitcoin-git> bitcoin/master 246a02f practicalswift: Use std::unordered_{map,set} (C++11) instead of boost::unordered_{map,set}
1292017-06-08T10:45:58 <bitcoin-git> bitcoin/master 35e7f13 Wladimir J. van der Laan: Merge #10548: Use std::unordered_{map,set} (C++11) instead of boost::unordered_{map,set}...
1302017-06-08T10:46:35 <bitcoin-git> [bitcoin] laanwj closed pull request #10548: Use std::unordered_{map,set} (C++11) instead of boost::unordered_{map,set} (master...unordered_map) https://github.com/bitcoin/bitcoin/pull/10548
1312017-06-08T11:17:34 *** cryptapus_afk has joined #bitcoin-core-dev
1322017-06-08T11:17:34 *** cryptapus_afk has joined #bitcoin-core-dev
1332017-06-08T11:27:35 *** cryptapus_afk has quit IRC
1342017-06-08T11:29:23 *** cryptapus_afk has joined #bitcoin-core-dev
1352017-06-08T11:31:29 *** JackH has joined #bitcoin-core-dev
1362017-06-08T11:33:24 *** cryptapus_afk has quit IRC
1372017-06-08T11:37:08 <bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/35e7f13f68f9...9c248e39f280
1382017-06-08T11:37:09 <bitcoin-git> bitcoin/master 5b75c47 Andrew Chow: Add a valid opcode sanity check to CScript...
1392017-06-08T11:37:09 <bitcoin-git> bitcoin/master ac4e438 Andrew Chow: Sanity check transaction scripts in DecodeHexTx...
1402017-06-08T11:37:10 <bitcoin-git> bitcoin/master 9c248e3 Wladimir J. van der Laan: Merge #10481: Decodehextx scripts sanity check...
1412017-06-08T11:37:38 <bitcoin-git> [bitcoin] laanwj closed pull request #10481: Decodehextx scripts sanity check (master...decodehextx-sanity) https://github.com/bitcoin/bitcoin/pull/10481
1422017-06-08T11:47:22 *** dermoth has quit IRC
1432017-06-08T11:48:03 *** dermoth has joined #bitcoin-core-dev
1442017-06-08T11:58:44 *** btcdrak has quit IRC
1452017-06-08T12:13:57 *** beatrootfarmer has joined #bitcoin-core-dev
1462017-06-08T12:17:38 *** goatturneer has quit IRC
1472017-06-08T12:18:56 *** dabura667 has quit IRC
1482017-06-08T12:23:08 *** dabura667 has joined #bitcoin-core-dev
1492017-06-08T12:30:35 *** dabura667 has quit IRC
1502017-06-08T12:34:20 *** AaronvanW has joined #bitcoin-core-dev
1512017-06-08T12:37:09 *** Aaronvan_ has joined #bitcoin-core-dev
1522017-06-08T12:40:53 *** AaronvanW has quit IRC
1532017-06-08T13:03:23 *** nickler_ is now known as nickler
1542017-06-08T13:03:31 *** btcdrak has joined #bitcoin-core-dev
1552017-06-08T13:17:29 *** Chris_Stewart_5 has joined #bitcoin-core-dev
1562017-06-08T13:22:12 <bitcoin-git> [bitcoin] somdoron opened pull request #10554: ZMQ: add publishers for wallet transactions. (master...zmq_wallet_tx) https://github.com/bitcoin/bitcoin/pull/10554
1572017-06-08T13:31:00 *** cryptapus_afk has joined #bitcoin-core-dev
1582017-06-08T13:31:00 *** cryptapus_afk has joined #bitcoin-core-dev
1592017-06-08T13:48:22 *** Dyaheon has quit IRC
1602017-06-08T13:50:46 *** Dyaheon has joined #bitcoin-core-dev
1612017-06-08T13:55:02 *** GAit has quit IRC
1622017-06-08T13:58:37 *** GAit has joined #bitcoin-core-dev
1632017-06-08T14:05:23 *** Guyver2 has joined #bitcoin-core-dev
1642017-06-08T14:08:27 *** tunafizz has quit IRC
1652017-06-08T14:08:54 *** tunafizz has joined #bitcoin-core-dev
1662017-06-08T14:46:10 *** cryptapus_afk has quit IRC
1672017-06-08T14:47:44 <bitcoin-git> [bitcoin] jnewbery opened pull request #10555: [tests] various improvements to zmq_test.py (master...zmqtestimprovements) https://github.com/bitcoin/bitcoin/pull/10555
1682017-06-08T15:01:01 *** midnightmagic has quit IRC
1692017-06-08T15:02:19 *** riemann has quit IRC
1702017-06-08T15:04:47 <bitcoin-git> [bitcoin] jnewbery opened pull request #10556: Move stop/start functions from utils.py into BitcoinTestFramework (master...testframeworkstopstart) https://github.com/bitcoin/bitcoin/pull/10556
1712017-06-08T15:08:26 *** cryptapus_afk has joined #bitcoin-core-dev
1722017-06-08T15:08:27 *** cryptapus_afk has joined #bitcoin-core-dev
1732017-06-08T15:24:30 *** cryptapus_afk has quit IRC
1742017-06-08T15:24:53 *** cryptapus_afk has joined #bitcoin-core-dev
1752017-06-08T15:24:54 *** cryptapus_afk has joined #bitcoin-core-dev
1762017-06-08T15:28:55 *** cryptapus_afk has quit IRC
1772017-06-08T15:29:18 *** cryptapus_afk has joined #bitcoin-core-dev
1782017-06-08T15:29:18 *** cryptapus_afk has joined #bitcoin-core-dev
1792017-06-08T15:32:12 <bitcoin-git> [bitcoin] morcos opened pull request #10557: Make check to distinguish between orphan txs and old txs more efficient. (master...dontcheckoutputs) https://github.com/bitcoin/bitcoin/pull/10557
1802017-06-08T15:42:42 *** Gnof has joined #bitcoin-core-dev
1812017-06-08T15:48:19 *** abpa has joined #bitcoin-core-dev
1822017-06-08T15:49:46 *** Aaronvan_ has quit IRC
1832017-06-08T15:56:23 *** BashCo has quit IRC
1842017-06-08T16:16:02 *** midnightmagic has joined #bitcoin-core-dev
1852017-06-08T16:16:42 <bitcoin-git> [bitcoin] morcos opened pull request #10558: Address nits from per-utxo change (master...10195nits) https://github.com/bitcoin/bitcoin/pull/10558
1862017-06-08T16:19:26 *** BashCo has joined #bitcoin-core-dev
1872017-06-08T16:22:44 <jtimon> travis didn't run for https://github.com/bitcoin/bitcoin/pull/9176
1882017-06-08T16:31:16 *** timothy has quit IRC
1892017-06-08T16:37:00 *** cryptapus_afk has quit IRC
1902017-06-08T16:51:55 *** jtimon has quit IRC
1912017-06-08T17:14:05 *** chjj has quit IRC
1922017-06-08T17:18:52 *** Gnof has quit IRC
1932017-06-08T17:19:37 *** cryptapus_afk has joined #bitcoin-core-dev
1942017-06-08T17:19:37 *** cryptapus_afk has joined #bitcoin-core-dev
1952017-06-08T17:25:05 *** Giszmo has joined #bitcoin-core-dev
1962017-06-08T17:28:32 *** chjj has joined #bitcoin-core-dev
1972017-06-08T17:45:15 *** Dyaheon has quit IRC
1982017-06-08T17:45:58 *** Dyaheon has joined #bitcoin-core-dev
1992017-06-08T17:52:34 *** ula has quit IRC
2002017-06-08T17:53:38 *** nakaluna has joined #bitcoin-core-dev
2012017-06-08T17:56:50 *** ula has joined #bitcoin-core-dev
2022017-06-08T17:57:56 <bitcoin-git> [bitcoin] achow101 closed pull request #9522: [RPC] Fix decoderawtransaction decoding of segwit txs (master...fix-decoderawtx) https://github.com/bitcoin/bitcoin/pull/9522
2032017-06-08T18:02:05 *** laurentmt has joined #bitcoin-core-dev
2042017-06-08T18:02:42 *** ndr76 has joined #bitcoin-core-dev
2052017-06-08T18:03:59 *** laurentmt has quit IRC
2062017-06-08T18:06:51 *** nakaluna has quit IRC
2072017-06-08T18:07:57 *** nakaluna has joined #bitcoin-core-dev
2082017-06-08T18:09:15 <arubi> vindicated at last :) https://github.com/bitcoin/bitcoin/pull/8837#issuecomment-250542708
2092017-06-08T18:11:18 <arubi> luke-jr made a comment about changing that 0x00 to 0xff. in case segwit is retried, this is very good input imo..
2102017-06-08T18:12:27 *** goatturner has joined #bitcoin-core-dev
2112017-06-08T18:13:00 *** goatturneer has joined #bitcoin-core-dev
2122017-06-08T18:13:41 <sipa> yeah we should have used 0xff
2132017-06-08T18:13:42 *** sdfkjs23 has quit IRC
2142017-06-08T18:13:54 <sipa> or even better... we should never have used normal transaction encoding for partial transactions
2152017-06-08T18:15:08 <arubi> right, I'd love for a blob of tx data to tell me what it wants to me to do with it
2162017-06-08T18:15:53 * sipa proposes ASN.1
2172017-06-08T18:15:55 *** beatrootfarmer has quit IRC
2182017-06-08T18:16:02 * sipa hides
2192017-06-08T18:16:03 <arubi> haha, make the wallet even fatter
2202017-06-08T18:16:16 <Chris_Stewart_5> arubi: so some sort of marker that says 'needs signature'?
2212017-06-08T18:16:49 <arubi> the scriptsig area itself can describe what it wants without hurting current layout
2222017-06-08T18:17:11 <arubi> "oops, this is unsigned input" or maybe a prevtxid "oops, not funded..."
2232017-06-08T18:17:19 *** goatturner has quit IRC
2242017-06-08T18:17:28 <arubi> I mean, it depends at what stage it's parsed.. hard to tell for "decoderawtransaction" :\
2252017-06-08T18:18:23 *** ajd_ has joined #bitcoin-core-dev
2262017-06-08T18:19:10 *** Ylbam has joined #bitcoin-core-dev
2272017-06-08T18:23:20 *** jtimon has joined #bitcoin-core-dev
2282017-06-08T18:27:05 <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/9c248e39f280...29f80cd230c3
2292017-06-08T18:27:05 <bitcoin-git> bitcoin/master 3fb81a8 practicalswift: Use list initialization (C++11) for maps/vectors instead of boost::assign::map_list_of/list_of
2302017-06-08T18:27:06 <bitcoin-git> bitcoin/master 29f80cd Wladimir J. van der Laan: Merge #10545: Use list initialization (C++11) for maps/vectors instead of boost::assign::map_list_of/list_of...
2312017-06-08T18:27:49 <bitcoin-git> [bitcoin] laanwj closed pull request #10545: Use list initialization (C++11) for maps/vectors instead of boost::assign::map_list_of/list_of (master...list_of) https://github.com/bitcoin/bitcoin/pull/10545
2322017-06-08T18:30:44 *** achow101 has left #bitcoin-core-dev
2332017-06-08T18:30:52 *** achow101 has joined #bitcoin-core-dev
2342017-06-08T18:50:46 <bitcoin-git> [bitcoin] sdaftuar reopened pull request #10357: Allow setting nMinimumChainWork on command line (master...2017-05-chainwork-commandline) https://github.com/bitcoin/bitcoin/pull/10357
2352017-06-08T18:50:50 <gmaxwell> Meeting in ten minutes.
2362017-06-08T19:00:00 <sipa> DOINK
2372017-06-08T19:00:04 <wumpus> #startmeeting
2382017-06-08T19:00:04 <lightningbot> Meeting started Thu Jun 8 19:00:04 2017 UTC. The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
2392017-06-08T19:00:04 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
2402017-06-08T19:00:07 <wumpus> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier jl2012 instagibbs
2412017-06-08T19:00:10 <jonasschnelli> hi
2422017-06-08T19:00:12 <sdaftuar> hi
2432017-06-08T19:00:14 <sipa> hi
2442017-06-08T19:00:15 <achow101> hi
2452017-06-08T19:00:29 <wumpus> proposed topics?
2462017-06-08T19:00:49 <kanzure> hi.
2472017-06-08T19:00:51 <sipa> UI interaction with pertxout upgrade?
2482017-06-08T19:00:57 <gmaxwell> #bitcoin-core-dev Meeting: wumpus sipa gmaxwell jonasschnelli morcos luke-jr btcdrak sdaftuar jtimon cfields petertodd kanzure bluematt instagibbs phantomcircuit codeshark michagogo marcofalke paveljanik NicolasDorier
2492017-06-08T19:01:07 <jonasschnelli> sipa: ack
2502017-06-08T19:01:33 <wumpus> ok, let's do high priority for review first, then that
2512017-06-08T19:01:37 <wumpus> #topic high priority for review
2522017-06-08T19:01:54 <sipa> #10148
2532017-06-08T19:01:56 <gribble> https://github.com/bitcoin/bitcoin/issues/10148 | Use non-atomic flushing with block replay by sipa · Pull Request #10148 · bitcoin/bitcoin · GitHub
2542017-06-08T19:01:59 <luke-jr> I've rebased multiwallet etc
2552017-06-08T19:02:08 <wumpus> luke-jr: great!
2562017-06-08T19:02:12 <sipa> (^ will double our effectively available dbcache)
2572017-06-08T19:02:30 <wumpus> luke-jr: sae there were some new review comments, did you address them yet?
2582017-06-08T19:03:00 <luke-jr> wumpus: I believe all review comments are addressed or answered
2592017-06-08T19:03:12 <wumpus> added 10148
2602017-06-08T19:03:15 <sipa> thanks!
2612017-06-08T19:03:51 <jtimon> still waiting on my current one, thanks wumpus for benchmarking
2622017-06-08T19:03:58 <jonasschnelli> #8694 now passes gitian, will re-utack soon
2632017-06-08T19:04:01 <gribble> https://github.com/bitcoin/bitcoin/issues/8694 | Basic multiwallet support by luke-jr · Pull Request #8694 · bitcoin/bitcoin · GitHub
2642017-06-08T19:04:13 <wumpus> jtimon: a very nice gain!
2652017-06-08T19:04:38 <luke-jr> oh, there was a comment asking for tests..
2662017-06-08T19:04:52 <luke-jr> I didn't write any, although I agree they would be nice
2672017-06-08T19:04:55 <wumpus> jtimon: unfortunately bitcoind nuked my log so I can't get the timings, but 26% savings in block hash operations is noce
2682017-06-08T19:05:06 <jtimon> wumpus: nice
2692017-06-08T19:05:37 <jtimon> we're talking about #10339 in case someone is lost
2702017-06-08T19:05:40 <gribble> https://github.com/bitcoin/bitcoin/issues/10339 | Optimization: Calculate block hash less times by jtimon · Pull Request #10339 · bitcoin/bitcoin · GitHub
2712017-06-08T19:05:43 <wumpus> luke-jr: well I'd say add those later, this is only the first in a series towards multiwallet after all
2722017-06-08T19:06:02 <sipa> wumpus: status on account to label conversion?
2732017-06-08T19:06:02 <luke-jr> indeed, it might not be possible to test right now since it's not exposed to RPC
2742017-06-08T19:06:06 <BlueMatt> wumpus: "in block hash operations" hmm? can you be more specific?
2752017-06-08T19:06:08 <gmaxwell> Related to hashing, does anyone here have AMD ryzen yet?
2762017-06-08T19:06:12 <BlueMatt> whats that compared to total runtime?
2772017-06-08T19:06:32 <wumpus> BlueMatt: I don't know about time, I just instrumented the block header hash function
2782017-06-08T19:06:34 <gmaxwell> BlueMatt: it's tiny vs total runtime, I assume-- but it should impact latency on block handling somewhat.
2792017-06-08T19:06:38 <luke-jr> gmaxwell: no, but I'd been holding out for SHA2 acceleration before upgrading, so I might get one if AMD is competing with Intel again
2802017-06-08T19:06:55 <sipa> related topic: sha2/crc32 hw accel?
2812017-06-08T19:07:08 <wumpus> as I said, I lost the timings (blame bitcoind nuking the log if it's too large)
2822017-06-08T19:07:11 <gmaxwell> luke-jr: Ryzen has the sha2 instructions... so I'm asking to find out if anyone will be able to test support for them. :)
2832017-06-08T19:07:23 <luke-jr> gmaxwell: right, that's why ;)
2842017-06-08T19:07:48 <luke-jr> I just haven't had time to investigate the pros/cons otherwise yet
2852017-06-08T19:07:58 <wumpus> sipa: no progress on that yet
2862017-06-08T19:08:02 <gmaxwell> In any case, 98% of the work needed to support sha2 instructions is the same as using SSE4 SHA2 which will itself be a non-trival speedup.
2872017-06-08T19:08:29 <wumpus> crc speedup should be possible after the leveldb upgrade that sipa PRed
2882017-06-08T19:08:39 <luke-jr> gmaxwell: we removed SSE4 stuff years ago, but I'm not sure if it was used for non-mining
2892017-06-08T19:08:49 <sipa> luke-jr: it was only used for mining at the time
2902017-06-08T19:08:50 <gmaxwell> BlueMatt: IIRC I instrumented and measured accepting a block at tip hashed the header ~6 times or so.
2912017-06-08T19:08:53 <gmaxwell> luke-jr: it was mining only.
2922017-06-08T19:09:00 <wumpus> luke-jr: that was a different one, the N-way-parallel
2932017-06-08T19:09:11 <sipa> yup 4-way parallel SSE2, iirc
2942017-06-08T19:09:11 <gmaxwell> and it was a vector SHA2 that had to work on 4 elements at a time.
2952017-06-08T19:09:36 <jtimon> gmaxwell: that's before or after the patch?
2962017-06-08T19:09:37 <gmaxwell> What we should be implementing now is the one at a time SIMD sha2. I believe matt uses it in fibre but without autodetection.
2972017-06-08T19:09:38 <sipa> it seems we've strayed from the topic?
2982017-06-08T19:09:41 <gmaxwell> jtimon: before.
2992017-06-08T19:09:41 <luke-jr> I wonder if it'd be worth using 4-way for merkle trees etc
3002017-06-08T19:09:42 <BlueMatt> gmaxwell: yea, but if its not measurable on the total, unlikely to be worth the effort and complexity.....an alternative - do we have CBlockIndex*es in most of those places? pblockindex->GetBlockHash() is free and simpler than passing hashes around
3012017-06-08T19:09:45 <jonasschnelli> Will have access to a Ryzen in a couple of days via Hetzner
3022017-06-08T19:09:47 *** Giszmo has quit IRC
3032017-06-08T19:10:13 * luke-jr wonders if the GCC compile farm has a Ryzen
3042017-06-08T19:10:37 <morcos> Without doing a thorough review of 10339, is the speedup worth it as opposed to the tradeoff on making the code a little more complicated/involved. Who was it that said the primary point of source code is to communicate between developers
3052017-06-08T19:10:51 <jonasschnelli> #10339
3062017-06-08T19:10:53 <morcos> It just seems weird to pass a block and separately it's hash into a bunch of functions
3072017-06-08T19:10:53 <gribble> https://github.com/bitcoin/bitcoin/issues/10339 | Optimization: Calculate block hash less times by jtimon · Pull Request #10339 · bitcoin/bitcoin · GitHub
3082017-06-08T19:11:07 <jtimon> BlueMatt: what's wrong with passing hashes around?
3092017-06-08T19:11:26 <BlueMatt> DoPartOfConnectBlock(42 args)
3102017-06-08T19:11:27 <wumpus> #topic Optimization: Calculate block hash less times by jtimon
3112017-06-08T19:11:35 <BlueMatt> we already have too many args in many of those functions
3122017-06-08T19:11:38 <instagibbs> "fewer" :P
3132017-06-08T19:11:54 <wumpus> well if hashing is a bottleneck, the obvious optimization is to just to it less
3142017-06-08T19:11:56 <sipa> instagibbs: i didn't want to say anything :)
3152017-06-08T19:12:04 <BlueMatt> wumpus: is it?
3162017-06-08T19:12:08 <wumpus> if hashing is not a bottleneck, then going after SSE and such makes no sense either
3172017-06-08T19:12:26 <sipa> BlueMatt: i believe that in all places where we already have CBlockIndex, no new block hashes are computed
3182017-06-08T19:12:34 <sipa> all of the cases in 10339 are before having a CBlockIndex
3192017-06-08T19:12:39 <wumpus> so if #10339 is not an improvement then we can leave hashing alone
3202017-06-08T19:12:41 <gribble> https://github.com/bitcoin/bitcoin/issues/10339 | Optimization: Calculate block hash less times by jtimon · Pull Request #10339 · bitcoin/bitcoin · GitHub
3212017-06-08T19:12:44 <gmaxwell> Well my suggestion on that hash thing was just to cache the hash in the block object, but Pieter said he prefered this.
3222017-06-08T19:12:46 <jtimon> instagibbs: calculate fewer times? thanks
3232017-06-08T19:12:59 <gmaxwell> (I also implemented that, though in a way that wasn't correct for the mining code.)
3242017-06-08T19:13:06 <BlueMatt> gmaxwell: I highly prefer that, though making CBlock const ala CTransaction is a bit more involved
3252017-06-08T19:13:09 <sipa> gmaxwell: if we create another CBlock for the cases where that's not needed
3262017-06-08T19:13:18 <sipa> (like GetTransaction and mining code)
3272017-06-08T19:13:26 <jtimon> Node also that we're sometimes getting the hash from the index and sometimes from the CBlock
3282017-06-08T19:13:38 <wumpus> right, calculating it eagerly can also result in overhead
3292017-06-08T19:13:43 <morcos> I'd just like to see some evidence, even a little bit, that this optimization is worth it. 26% fewer hashing operations results in what savings? Or do people not agree it makes the code a bit more cumbersome
3302017-06-08T19:14:07 <sipa> i don't see much problem in passing a hash along with a block if you know it
3312017-06-08T19:14:21 <BlueMatt> passing in the hash as a part of ProcessNewBlock? yuck
3322017-06-08T19:14:22 <sipa> it's certainly a bit of complication, but a trivial one
3332017-06-08T19:14:22 <wumpus> morcos: sure, my point is just that if 26% fewer hashing operations isn't worth it, then using special intstructions to gain a few % is netiher worth it
3342017-06-08T19:14:25 <CodeShark> seems like there are probably lower hanging fruit in optimizations but meh
3352017-06-08T19:14:28 <BlueMatt> I just spent a bunch of time getting fewer args there
3362017-06-08T19:14:45 <morcos> wumpus: well hashing occurs a lot more often than in the calculations of these block hashes
3372017-06-08T19:14:52 <BlueMatt> to separate the consensus code from net_processing, now we're adding more coupling :(
3382017-06-08T19:14:53 <jtimon> it seems to me that the reserve is mostly a question of style
3392017-06-08T19:14:54 <gmaxwell> morcos: My reason for bringing it up is that I believe the repeated hashing is on the latency critical path for block propagation to the tune of maybe a millisecond-ish.
3402017-06-08T19:15:08 <jtimon> I mean calculating it eagerly can penalize in some cases, right
3412017-06-08T19:15:22 <gmaxwell> wumpus: these optimizations just prevent repeated hashing of the headers, in terms of total bytes hashed while syncing thats pretty small even though we do have a high repetition factor.
3422017-06-08T19:15:30 *** Giszmo has joined #bitcoin-core-dev
3432017-06-08T19:15:36 <gmaxwell> jtimon: what case would computing it early peanalize?
3442017-06-08T19:15:37 <wumpus> gmaxwell: ok...
3452017-06-08T19:15:38 <CodeShark> BlueMatt: my preference is to sacrifice a little performance for better architecture ;)
3462017-06-08T19:15:42 <sdaftuar> i will try to benchmark the effect on validation speed for this
3472017-06-08T19:15:52 <wumpus> CodeShark: I think that's a fair argument
3482017-06-08T19:15:56 <sipa> wumpus: transaction hashing accounts for far more than block header hashing, but not on the critical path
3492017-06-08T19:16:02 <jtimon> gmaxwell: when the validation fails for some other reason before you need the hash?
3502017-06-08T19:16:04 * BlueMatt would be more ok if we calculated it at the top of ProcessNewBlock and pass that one around, but still not ideal
3512017-06-08T19:16:11 <BlueMatt> passing it in to ProcessNewBlock is.....ugh
3522017-06-08T19:16:36 <gmaxwell> jtimon: I don't think there is any validation failure path where we don't hash... after all, we'll want to know _what_ block hash failed to validate.
3532017-06-08T19:16:38 <jtimon> BlueMatt: you could have said that when each function had a separated commit, but I can leave that out
3542017-06-08T19:16:46 <BlueMatt> or, really, chnging the signatures of stuff in validation.h for this without a benchmark showing its a real win :(
3552017-06-08T19:16:58 <jtimon> gmaxwell: yeah, fair enough
3562017-06-08T19:17:16 * morcos has said his piece and is willing to let the people who spent more time thinking about this decide
3572017-06-08T19:17:22 <wumpus> ok, clear, better to close it I guess, would have made sense to have this discussion earlier
3582017-06-08T19:17:24 <jtimon> so we come back to only the "ugh" argument
3592017-06-08T19:17:40 <gmaxwell> But I still don't understand why sipa prefered to do this jtimon patch instead of making the object cache it. FWIW, just monkey patching it 'works' and the only obvious issue is mining.
3602017-06-08T19:17:57 <sipa> gmaxwell: and every other read from disk
3612017-06-08T19:18:46 <jtimon> I preferred this because I think it's simpler than the cache, even if it's "ugh"
3622017-06-08T19:18:49 <gmaxwell> sipa: I don't understand your comment.
3632017-06-08T19:18:54 <sipa> i consider adding more arguments to validation-specific functions less invasively than changing a primitive datastructure
3642017-06-08T19:18:58 <sipa> we can do both, if needed
3652017-06-08T19:19:16 <wumpus> sipa: I tend to agree
3662017-06-08T19:19:26 <sipa> gmaxwell: i believe we often read blocks from disk without caring about its hash, because we already know it
3672017-06-08T19:19:35 <wumpus> sipa: just passing extra arguments is easier to reason about than extending primitive datastructures
3682017-06-08T19:19:39 <gmaxwell> sipa: the read funcion computes it!
3692017-06-08T19:19:46 <sipa> gmaxwell: ?
3702017-06-08T19:20:01 <sipa> currently, yes, as a checksum
3712017-06-08T19:20:17 <sipa> if we care about the checksum functionality, we should add a crc
3722017-06-08T19:20:20 <gmaxwell> // Check the header
3732017-06-08T19:20:20 <gmaxwell> if (!CheckProofOfWork(block.GetHash(), block.nBits, consensusParams))
3742017-06-08T19:20:20 <wumpus> jtimon: I agree, caching is always somewhat risky and error prone, this is more explicit and clear
3752017-06-08T19:20:23 <gmaxwell> return error("ReadBlockFromDisk: Errors in block header at %s", pos.ToString());
3762017-06-08T19:20:48 <wumpus> then again if it's not worth it, performanec wise, we shouldn't do it at all
3772017-06-08T19:20:52 <wumpus> not in another way either
3782017-06-08T19:21:02 <sipa> making the block hash part of CBlock makes us unable to skip computing that hash
3792017-06-08T19:21:09 <sipa> even in places where we don't care about it
3802017-06-08T19:21:13 <gmaxwell> I think I previously gave figures on the latency impact of caching.
3812017-06-08T19:21:17 <jtimon> how can I benchmark this to convince morcos?
3822017-06-08T19:21:34 <gmaxwell> sipa: Where are those places?
3832017-06-08T19:21:34 <wumpus> I don't know, I tried
3842017-06-08T19:22:09 <sipa> gmaxwell: every time we read a block from disk, we already have its hash (because that's how we look it up!)
3852017-06-08T19:22:26 <sdaftuar> sipa: don't we also hash all the transactions when we read a block from disk?
3862017-06-08T19:22:28 <sipa> gmaxwell: recomputing it there serves as nothing more than a checksum
3872017-06-08T19:22:30 <sdaftuar> surely that dominates all this
3882017-06-08T19:22:34 <morcos> sipa: i know i said i'd shut up, but you just read from DISK, who cares if you hash 80 bytes?
3892017-06-08T19:22:41 <wumpus> might be time for next topic, if this is not a perf issue, might be better to discuss it after meeting
3902017-06-08T19:22:57 <sipa> sdaftuar: good point.
3912017-06-08T19:23:02 <gmaxwell> sdaftuar: we don't. I used to think we did but we do not.
3922017-06-08T19:23:15 <sipa> yes, we do
3932017-06-08T19:23:23 <gmaxwell> sipa: where?
3942017-06-08T19:23:32 <sipa> deserializing a transaction computes its hash
3952017-06-08T19:23:50 <gmaxwell> oh right, what we don't do is validate the hashroot.
3962017-06-08T19:23:53 <sipa> let's discuss this after the meeting
3972017-06-08T19:23:59 <gmaxwell> k
3982017-06-08T19:24:45 <sipa> next topic?
3992017-06-08T19:24:56 <wumpus> #topic UI interaction with pertxout upgrade
4002017-06-08T19:25:16 <jtimon> in any case, BlueMatt morcos if you already knew you didn't liked this, I would have appreaciated knowing it earlier, maybe a provisional concept NACK or something
4012017-06-08T19:25:45 <sipa> so, since 10195, we'll have a possibly lengthy (minutes on decent hardware, possibly much longer elsewhere) upgrade process from the old per-txid utxo db to the per-txout one
4022017-06-08T19:25:52 <morcos> jtimon: yes sorry, i only looked at the PR during todays meeting, also why i said i'd stop arguing about it
4032017-06-08T19:26:04 <sipa> that needs some GUI interaction, i believe
4042017-06-08T19:26:08 <wumpus> jtimon: same here, this should have been clear sooner, otherwise I wouldn't have spent as much time on it
4052017-06-08T19:26:19 <sipa> or perhaps even in bitcoind
4062017-06-08T19:26:36 <morcos> wumpus: in general perf improvements shoudl include data in the PR request when possible I think
4072017-06-08T19:26:40 <wumpus> jtimon: it's kind of disappointing to discover this does what it says on the tin, save ~1/4th of block hash operations, just to discover that's not even important
4082017-06-08T19:26:44 <jonasschnelli> Can you not just use uiInterface.Progress() like in VerifyDB?
4092017-06-08T19:26:45 <wumpus> jtimon: that kind of annoys me
4102017-06-08T19:26:57 <sipa> jonasschnelli: that doesn't let you interrupt
4112017-06-08T19:27:07 <morcos> sipa: yeah i noticed that it doesn't even output to debug log that its doing an upgrade right?
4122017-06-08T19:27:14 <sipa> it should log
4132017-06-08T19:27:19 <sipa> "Upgrading database..."
4142017-06-08T19:27:20 <luke-jr> jonasschnelli: I'd think users should be able to send txs during the upgrade.
4152017-06-08T19:27:21 <morcos> oh
4162017-06-08T19:27:40 <jonasschnelli> luke-jr: but RPC is also not ready in this case? RIght?
4172017-06-08T19:27:44 <gmaxwell> morcos: it logs, but you won't see it if you have leveldb logging turned on. :P
4182017-06-08T19:27:46 <sipa> nothing works during the upgrade
4192017-06-08T19:27:51 <sipa> there is no full node available
4202017-06-08T19:28:05 <jonasschnelli> why would you then need interruption?
4212017-06-08T19:28:10 <wumpus> it should at least show a progress indicator and be interruptible indeed, ideally
4222017-06-08T19:28:13 <sipa> because maybe it takes too long
4232017-06-08T19:28:20 <sipa> and they want to continue another time
4242017-06-08T19:28:36 <wumpus> yes, because the user may want to do something else with the computer, having not expected to have to spend a long time upgrading
4252017-06-08T19:28:44 <gmaxwell> by interruption this means 'crash', not continue running without the upgrade.
4262017-06-08T19:28:44 <jtimon> anyway, I would still like to benchmark it if anyone can help me (not sure what to do)
4272017-06-08T19:28:50 <jonasschnelli> Ah.. the update can be done in multiple steps..
4282017-06-08T19:28:57 <sipa> yes
4292017-06-08T19:28:58 <jonasschnelli> well that would also need communication then
4302017-06-08T19:28:59 <wumpus> gmaxwell: well 'exit' not crash
4312017-06-08T19:29:07 <gmaxwell> same difference
4322017-06-08T19:29:10 <morcos> sipa: yeah i missed that somehow, sorry
4332017-06-08T19:29:13 <sipa> crashing "should" be fine
4342017-06-08T19:29:29 <gmaxwell> it better be! :) (also, I've tested it a fair bit)
4352017-06-08T19:29:30 <jonasschnelli> [Updating 0%] \n You can shutdown and continue later [Shutdown-Button] <-- something like that?
4362017-06-08T19:29:37 <wumpus> jtimon: benchmarking the reindex chainstate maybe?
4372017-06-08T19:29:39 <gmaxwell> jonasschnelli: like that.
4382017-06-08T19:29:39 <luke-jr> what if you crash, run the old version, and then the new one again?
4392017-06-08T19:29:47 <wumpus> jtimon: I still wonder why -stopatblock doesn't work
4402017-06-08T19:30:18 <gmaxwell> luke-jr: you get to keep both pieces? (I believe the old version will tell you the database is corrupt and stop there.... but I haven't tested that, so it's a good question.)
4412017-06-08T19:30:21 <sipa> wumpus: no; that's dominated by tx validation/deserialization... a good benchmark tests block validation given fully populated mempool and sigcache etc
4422017-06-08T19:30:29 <jonasschnelli> How about logging? Can we log similar to the VerifyDB [the non-newline % steps]?
4432017-06-08T19:30:38 <sipa> jonasschnelli: sure
4442017-06-08T19:30:40 <wumpus> sipa: ok...
4452017-06-08T19:30:48 <jonasschnelli> I can work on that. Seems to fit my skillset. :)
4462017-06-08T19:30:51 <luke-jr> gmaxwell: IMO users may get tired of waiting and prefer to start the upgrade over in exchange for being able to use the old version in the meantime
4472017-06-08T19:30:51 <jtimon> sipa: perhaps it could be as simple as using -upgradeutxodb=1 once or something of the sort?
4482017-06-08T19:30:52 <wumpus> sipa: good luck doing that in a deterministic way, though
4492017-06-08T19:30:53 <sipa> maybe VerifyDB or something like that can pass a bool saying "interruptible" ?
4502017-06-08T19:31:04 <gmaxwell> luke-jr: sounds like a case we should handle.
4512017-06-08T19:31:05 <sipa> jtimon: you can't not do it
4522017-06-08T19:31:29 <sipa> jtimon: i mean, -upgradeutxodb=0 would mean just exiting immediately :)
4532017-06-08T19:31:37 <wumpus> no, the upgrade needs to be done
4542017-06-08T19:31:42 <luke-jr> we could prompt before starting to warn the user, but IMO we'd probably want to begin ASAP, even if we hold off destroying backward compat until the user OKs it
4552017-06-08T19:31:51 <gmaxwell> meh.
4562017-06-08T19:32:01 <wumpus> sure, you could warn...
4572017-06-08T19:32:07 <gmaxwell> Keep in mind that on a reasonably fast computer the upgrade does not take long.
4582017-06-08T19:32:07 <jtimon> no, I mean once you set -upgradeutxodb=1 once it doesn't matter what you set anymore
4592017-06-08T19:32:09 <sipa> it's a new major release, i don't care about backward compatibility; the user can always reindex if they really need to
4602017-06-08T19:32:13 <morcos> so just to be clear, right now if someone went back to an old version, there is some chance they'd screw themselves right?
4612017-06-08T19:32:23 <morcos> (and need to reindex)
4622017-06-08T19:32:32 <wumpus> morcos: yes, it simply doesn't work. We should warn in the release notes.
4632017-06-08T19:32:37 <sipa> morcos: i believe that with very high probability the startup sanity check will fail
4642017-06-08T19:32:38 <luke-jr> pruned nodes cant reindex
4652017-06-08T19:32:44 <gmaxwell> morcos: old version rejects with a "your database is corrupted" though luke was asking an interesting question about what happens if you do this mid-conversion.
4662017-06-08T19:33:02 <wumpus> we coudl have added logic to 0.14.2 to detect the new format and give a more specific error
4672017-06-08T19:33:03 <morcos> sipa: yeah that was my question, how far do you have to get along in the upgrade before your sanity check would fail with high probability
4682017-06-08T19:33:07 <gmaxwell> sipa: I did test that case, how could it not fail?
4692017-06-08T19:33:25 <sdaftuar> we only go back 6 blocks now in the startup sanity check i think
4702017-06-08T19:33:26 <luke-jr> wumpus: IMO it'd be better to destroy the upgrade so it starts over, and work (for incomplete upgrades)
4712017-06-08T19:33:31 <sdaftuar> isn't it possible you don't come across any bad entries?
4722017-06-08T19:33:35 <gmaxwell> wumpus: hm? I don't think that would be the right way, the right way would be to make the new version more incompatible.
4732017-06-08T19:33:53 <wumpus> gmaxwell: ok...
4742017-06-08T19:34:02 <sipa> there is a trivial change we can make that guarantees old versions will treat it as an empty db
4752017-06-08T19:34:24 <luke-jr> change the dir name?
4762017-06-08T19:34:28 <sipa> haha
4772017-06-08T19:34:31 <wumpus> gmaxwell: I just mean a specific error like 'the utxo database is in a newer format than supported by this version' would be clearer than a general sanity check error
4782017-06-08T19:34:33 <sipa> yes, i've considered that too
4792017-06-08T19:34:34 <morcos> empty db == corrupt and leave it so the new version can continue
4802017-06-08T19:34:39 <gmaxwell> no there is just the height index entry.
4812017-06-08T19:34:39 <morcos> wumpus +1
4822017-06-08T19:34:45 <wumpus> but I don't know ,sorry for suggesting it
4832017-06-08T19:34:46 <gmaxwell> wumpus: oh, sure that would have been okay!
4842017-06-08T19:34:51 <morcos> thats at least somethign we shoudl do for future changes
4852017-06-08T19:34:51 <sipa> wumpus: unfortunately it doesn't have a version number
4862017-06-08T19:34:55 <jtimon> luke-jr: we could move the main datadir to ./bitcoin/main maybe
4872017-06-08T19:34:55 <sipa> agree
4882017-06-08T19:34:57 <luke-jr> using a new db would also mean users from pre-obfuscation get that enabled..
4892017-06-08T19:35:10 <sipa> a new db is a possibility
4902017-06-08T19:35:18 <wumpus> sipa: we could have added one! but yes, too late.
4912017-06-08T19:35:19 <sipa> though i'd prefer to not need 2x disk storage during the upgrade
4922017-06-08T19:35:25 <sipa> not too late
4932017-06-08T19:35:27 <morcos> sipa: what was the trivial change you were referring to
4942017-06-08T19:35:45 <sipa> morcos: change the record name for the 'best block hash' entry
4952017-06-08T19:35:52 <gmaxwell> the database stores a record to indicate the current tip; we can just change that.
4962017-06-08T19:35:56 <sipa> and set the old one to something invalid
4972017-06-08T19:36:23 <luke-jr> [19:35:19] <sipa> though i'd prefer to not need 2x disk storage during the upgrade <-- IMO unavoidable if the "user gets tired of waiting and runs old version" is desired to work okay
4982017-06-08T19:36:32 <gmaxwell> FWIW the non-atomic flushing change already changes those records around.
4992017-06-08T19:36:39 <sipa> yup
5002017-06-08T19:36:46 <sipa> well, in a backward compatible way
5012017-06-08T19:36:47 <gmaxwell> I don't think we should support user gets tired of waiting.
5022017-06-08T19:36:59 <gmaxwell> too high a cost for too low a benefit.
5032017-06-08T19:36:59 <sipa> it's even easier if it doesn't need to be backward compatible
5042017-06-08T19:37:15 <morcos> gmaxwell: yes we shouldn't support it, but it might be nice to support they tried to do it and didn't corrupt their database by trying so they now have to reindex
5052017-06-08T19:37:41 <gmaxwell> morcos: yes, in practice that is the case but it isn't guarenteed. We could make it guarenteed.
5062017-06-08T19:37:44 <wumpus> gmaxwell: well just exiting and continuing the upgrade later would be ok
5072017-06-08T19:38:02 <gmaxwell> wumpus: yea, that works except we don't have an exit button other than kill -9 :)
5082017-06-08T19:38:03 <sipa> wumpus: also, i don't understand why the CompactRange doesn't work for you
5092017-06-08T19:38:21 <sipa> if it's not guaranteed (or even not likely) to work, maybe the 2x disk storage is inevitable
5102017-06-08T19:38:27 <sipa> and we're better off explicitly creating a new db
5112017-06-08T19:38:36 <wumpus> sipa: I don't know either, could try it again if you think it's a fluke...
5122017-06-08T19:38:48 <sipa> i'll test it myself again too
5132017-06-08T19:38:51 <gmaxwell> I tested an earlier range compacting patch and it did work.
5142017-06-08T19:39:00 <gmaxwell> I'll also test.
5152017-06-08T19:39:21 <sipa> monitor disk usage during upgrade, with and without compactrange patch
5162017-06-08T19:39:23 <sipa> would be nice
5172017-06-08T19:39:37 <sipa> and maybe this discussion depends on the outcome there
5182017-06-08T19:39:58 <gmaxwell> if indeed compacting isn't reliable we should change to create a new database. IMO
5192017-06-08T19:40:05 <sipa> agree
5202017-06-08T19:40:30 <wumpus> it might have to do with my test framework, I'll copy the database directory next time instead of using my usual hardlink hack
5212017-06-08T19:41:02 <gmaxwell> (FWIW, the reason I checked compacting before is because I was going to suggest changing to a new database if it didn't work.)
5222017-06-08T19:41:06 <wumpus> other topics?
5232017-06-08T19:41:27 <sipa> crc32 leveldb 1.20?
5242017-06-08T19:41:32 <gmaxwell> ^
5252017-06-08T19:41:41 <wumpus> #topic crc32 leveldb 1.20
5262017-06-08T19:41:41 <jtimon> gmaxwell: agreed on cost/benefit for supporting interruption, much easier to require confirmation with a warning "this may take a while and cannot be interrupted" or something
5272017-06-08T19:42:07 <gmaxwell> sipa: Why is travis failing?
5282017-06-08T19:42:09 <sipa> i personally really dislike the approach they're using (which seems to be the advised way even), which requires a separate object compiled with different flags
5292017-06-08T19:42:25 <sipa> gmaxwell: i assume incompatibility with our own build system integration of leveldb
5302017-06-08T19:42:35 <wumpus> compiling a separate object with special flags is pretty much the standard way of doing it
5312017-06-08T19:42:44 *** chjj has quit IRC
5322017-06-08T19:42:57 <sipa> they're even abusing it... they're calling the new object without knowing that the CPU supports it
5332017-06-08T19:43:25 <wumpus> ok, that's not good
5342017-06-08T19:43:27 <gmaxwell> Compiling a new object with different flags is standard and correct, calling it without knowing, is wrong.
5352017-06-08T19:43:33 <wumpus> yes
5362017-06-08T19:43:45 <wumpus> the detection function should not be in that object
5372017-06-08T19:43:46 <sipa> the approach in #10377 is so much easier
5382017-06-08T19:43:48 <gribble> https://github.com/bitcoin/bitcoin/issues/10377 | Use rdrand as entropy source on supported platforms by sipa · Pull Request #10377 · bitcoin/bitcoin · GitHub
5392017-06-08T19:43:49 <wumpus> simple as that
5402017-06-08T19:44:34 <wumpus> that only works because you don't spell out the instruction but put it in binary, if you spelled out the instruction it would have to be compiled with a special flag
5412017-06-08T19:44:56 <wumpus> you can't use e.g. sse4.2 instructions if the object isn't compiled with that
5422017-06-08T19:45:00 <gmaxwell> IIRC MSVC has banned inline ASM, if we don't care about MSVC then we're free to do other things. However, you need to do the object thing if you want to use code that accesses simd via intrensics, for something like rdrand or clmul it's no big deal to use asm.
5432017-06-08T19:45:07 <sipa> ok ok
5442017-06-08T19:45:36 <wumpus> gmaxwell: yes, intrinsics are more compatible
5452017-06-08T19:45:57 <cfields_> here!
5462017-06-08T19:46:31 <gmaxwell> so we need to fix leveldb to now call into that object without having done the detection. Anything else needed there?
5472017-06-08T19:46:41 <wumpus> I don't think leveldb's way is wrong, except for putting the detection function in the instruction-set specific object
5482017-06-08T19:47:14 <BlueMatt> oh, i was supposed to mention cfields_ would be late
5492017-06-08T19:47:25 <cfields_> heh, thanks
5502017-06-08T19:47:34 <jtimon> maybe we can open an issue on their repo or something?
5512017-06-08T19:47:38 <BlueMatt> you're welcome :)
5522017-06-08T19:47:48 <sipa> jtimon: the issue is in our own build system integration, i'm sure
5532017-06-08T19:47:51 <sipa> not upstream
5542017-06-08T19:48:05 <sipa> oh, you mean for calling the machine specific object? yeah
5552017-06-08T19:48:06 <wumpus> wouldn't upstream have the same problem then?
5562017-06-08T19:48:08 <gmaxwell> jtimon might have been referring to the detection in the wrong object, thats just wrong.
5572017-06-08T19:48:13 <sipa> yeah, agree
5582017-06-08T19:48:18 <sipa> if they ever look at issues...
5592017-06-08T19:48:18 <jtimon> gmaxwell: sipa right
5602017-06-08T19:48:21 <gmaxwell> we should submit a fix, it should be trivial.
5612017-06-08T19:48:27 <cfields_> that's done already: https://github.com/theuni/bitcoin/commit/2cb7dda13884e44105f33c16e7e2c1a9aed46295
5622017-06-08T19:48:33 <wumpus> yes better to submit a fix, submitting an issue likely won't help
5632017-06-08T19:48:35 <sipa> cfields_: oh!
5642017-06-08T19:48:36 <cfields_> or are you guys talking about something else?
5652017-06-08T19:48:56 <sipa> probably not
5662017-06-08T19:48:58 <sipa> let's try
5672017-06-08T19:49:09 <gmaxwell> okay, we have actions here.
5682017-06-08T19:49:33 <wumpus> lol <long discussion> oh, cfields did it already
5692017-06-08T19:49:37 <cfields_> that enables the runtime detection and separate object, but iirc there's still a little generic code that may end up with the wrong instructions
5702017-06-08T19:49:45 <sipa> cfields_: yup...
5712017-06-08T19:50:13 *** Dyaheon has quit IRC
5722017-06-08T19:50:15 <gmaxwell> that just needs to be moved into another file, I expect.
5732017-06-08T19:50:22 <sipa> yup
5742017-06-08T19:50:26 <sipa> let's do that independently
5752017-06-08T19:50:28 <wumpus> yes, move all the code out of it except for the function itself
5762017-06-08T19:50:42 <gmaxwell> should be a trivial patch we can take locally and send upstream.
5772017-06-08T19:50:51 <cfields_> i even linked it on the PR! :)
5782017-06-08T19:50:51 <cfields_> heh, sorry I was late. Movers showed up at 3:00 exactly :\
5792017-06-08T19:51:06 *** Dyaheon has joined #bitcoin-core-dev
5802017-06-08T19:51:19 <gmaxwell> We'll need to do the same thing for SSE4 (and sha2 instruction) sha2.
5812017-06-08T19:51:40 <jtimon> cfields_: that doesn't seem related the meeting started at 21:00 :p
5822017-06-08T19:51:59 <wumpus> from what I remember that's actual assembly code from intel, in .s format, not using intrinsics
5832017-06-08T19:52:07 <gmaxwell> ah. okay!
5842017-06-08T19:52:18 <luke-jr> jtimon: 3:00 is meeting time for east USA
5852017-06-08T19:52:29 <jtimon> luke-jr: sure, bad joke, sorry
5862017-06-08T19:52:38 <cfields_> i suspect I missed this discussion too, but I have intrinsics done for sha2
5872017-06-08T19:52:43 <gmaxwell> I usually expect corporate code to be intrinsics because having good performance is not enterprise enough. :P
5882017-06-08T19:52:55 <wumpus> https://github.com/laanwj/bitcoin/commit/7e3e717892d96cae7f05dd1b6742425a298cc12e
5892017-06-08T19:53:05 <wumpus> it's even in yasm format
5902017-06-08T19:53:05 *** ajd_ has quit IRC
5912017-06-08T19:53:19 <cfields_> wumpus: ah, i'll shut up :)
5922017-06-08T19:53:36 <gmaxwell> I'm very exicited about getting the faster hash in, with all our other improvements, I'm now expecting a 8% IBD speedup.
5932017-06-08T19:53:37 *** SopaXorzTaker has quit IRC
5942017-06-08T19:53:46 <gmaxwell> if not more.
5952017-06-08T19:53:55 <wumpus> cfields_: you mean sha-specific instructions? the stuff I quote is from before that
5962017-06-08T19:54:12 * jtimon was used to the other notation and used assemble with nasm, but it's pretty sure we don't want that
5972017-06-08T19:54:15 <cfields_> wumpus: yea, intrinsics for intel sha1/2
5982017-06-08T19:54:27 <wumpus> it just implements a SHA using SSE4/AVX, so it works on older architectures..
5992017-06-08T19:54:29 <sipa> jtimon: ha, cool
6002017-06-08T19:54:44 <sipa> wumpus: no license issues?
6012017-06-08T19:54:59 <jtimon> sipa: not cool, I should have learned with the at & t notation, no?
6022017-06-08T19:55:01 <gmaxwell> sipa: it's permissively licensed IIRC.
6032017-06-08T19:55:09 <cfields_> wumpus: ah, nice. I suspect that's where the leveldb discussion came from. We can re-use the build-time logic for sure.
6042017-06-08T19:55:10 <wumpus> sipa: it's either MIT or BSD
6052017-06-08T19:55:20 <sipa> jtimon: just swap the argument order :)
6062017-06-08T19:55:52 <wumpus> spot-the-license https://github.com/laanwj/bitcoin/blob/7e3e717892d96cae7f05dd1b6742425a298cc12e/src/crypto/sha256_avx1.asm#L10
6072017-06-08T19:55:55 <gmaxwell> Before the metting closes, I'd like to remind people that Matt's caching change #10192 is a 31% block connection speedup. It's done, rebased current, and needs review.
6082017-06-08T19:55:59 <gribble> https://github.com/bitcoin/bitcoin/issues/10192 | Cache full script execution results in addition to signatures by TheBlueMatt · Pull Request #10192 · bitcoin/bitcoin · GitHub
6092017-06-08T19:56:15 *** chjj has joined #bitcoin-core-dev
6102017-06-08T19:56:24 <wumpus> #topic cache full script execution
6112017-06-08T19:56:24 <jtimon> sipa: yeah, I think that's mostly it, and some minor things I think, but I was too lazzy to translate the programs I had
6122017-06-08T19:57:41 <gmaxwell> I don't know that there is too much to say about 10192. It saves a lot of operations when checking if an already validated tx is cached. It's fairly straight forward.
6132017-06-08T19:58:14 <sipa> yeah, let's do it
6142017-06-08T19:58:28 <wumpus> seems it had a lot of review, no (ut)ACKs though
6152017-06-08T19:58:57 <gmaxwell> I'm working on an ACK now. Just want other people to look, it sat for a long time needing rebase.
6162017-06-08T19:59:16 <wumpus> right
6172017-06-08T19:59:18 <morcos> on the list
6182017-06-08T19:59:20 <sdaftuar> +1
6192017-06-08T19:59:48 <wumpus> it's already on the high priority for review list
6202017-06-08T19:59:57 <gmaxwell> The non-atomic flush stuff is also still outstanding, but I think it might get slightly changed based on todays per txo discussion.
6212017-06-08T19:59:58 <sipa> cfields_: yours commit (after trivial cherry pick) works
6222017-06-08T20:00:33 <wumpus> #endmeeting
6232017-06-08T20:00:33 <lightningbot> Meeting ended Thu Jun 8 20:00:33 2017 UTC. Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
6242017-06-08T20:00:33 <lightningbot> Minutes: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-06-08-19.00.html
6252017-06-08T20:00:33 <lightningbot> Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-06-08-19.00.txt
6262017-06-08T20:00:33 <lightningbot> Log: http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-06-08-19.00.log.html
6272017-06-08T20:00:37 <gmaxwell> wumpus: thanks!
6282017-06-08T20:01:27 <luke-jr> #10512's travis failure is being weird :/
6292017-06-08T20:01:28 <gribble> https://github.com/bitcoin/bitcoin/issues/10512 | Rework same-chain from abusing DoS banning, to explicit checks by luke-jr · Pull Request #10512 · bitcoin/bitcoin · GitHub
6302017-06-08T20:01:42 <luke-jr> it seems like adding debugging logging is fixing the test
6312017-06-08T20:02:01 <sipa> ah, a heisenbu
6322017-06-08T20:02:10 <sipa> +g
6332017-06-08T20:02:21 <kanzure> heisentypu
6342017-06-08T20:02:24 <gmaxwell> aside, I have a fair amount of distaste for intrensics; they're slightly more portable but for a long time they had poor performance because they left register allocation up to the compliler and it usually did it wrong. But newer compilers have been much better about register allocation around simd code.
6352017-06-08T20:02:24 <cfields_> luke-jr: double the debug logging, close as fixed.
6362017-06-08T20:04:36 <luke-jr> oh wait, I'm wrong. *rubs eyes*
6372017-06-08T20:04:41 <luke-jr> forgot I had started a bisect last night
6382017-06-08T20:04:50 <jtimon> I have the feeling that some who didn't like #10339 may not like #8498 either for similar reasons
6392017-06-08T20:04:52 <gribble> https://github.com/bitcoin/bitcoin/issues/10339 | Optimization: Calculate block hash less times by jtimon · Pull Request #10339 · bitcoin/bitcoin · GitHub
6402017-06-08T20:04:54 <wumpus> for ARM32, intrinsics were pretty bad, apparently there were some changes in ARM64 arch making them better suited for optimization by compilers
6412017-06-08T20:04:55 <gribble> https://github.com/bitcoin/bitcoin/issues/8498 | Optimization: Minimize the number of times it is checked that no money... by jtimon · Pull Request #8498 · bitcoin/bitcoin · GitHub
6422017-06-08T20:05:23 <jtimon> BlueMatt: morcos ^^
6432017-06-08T20:05:52 <gmaxwell> Sorry about earlier with jtimon caching patch, I thought I had expressed the view that I thought the change was adding complexity-- on reread of my comment I can see that my effort to be more constructive (in the form of offering "why not just cache") backfired.
6442017-06-08T20:06:11 <gmaxwell> I don't really think the complexity it adds is that big a deal.
6452017-06-08T20:06:38 <wumpus> but if reducing the number of header hashing operations isn't worth it in the bigger scheme of things, it's a boondoggle
6462017-06-08T20:06:49 <jtimon> gmaxwell: I think you were clear: you preferred a cache, but since there were more comments and you didn't say anything else, I assumed it wasn't a strong preference
6472017-06-08T20:06:50 <sipa> wumpus: i'm not convinced it's not worth it
6482017-06-08T20:07:08 <cfields_> sipa: i can patch up leveldb to move all ops to that file, if you're not already working on it
6492017-06-08T20:07:40 <jtimon> is there a benchmarking tutorial somewhere ?
6502017-06-08T20:07:41 <sipa> wumpus: but it will (if at all) only be relevant for blocks with pre-cached utxos, pre-checked sigs, already loaded...
6512017-06-08T20:07:56 <sipa> which is what matters for relay, not reindex
6522017-06-08T20:08:26 <wumpus> sipa: well for what it's worth it also helped with the number of hashes in reindex
6532017-06-08T20:08:52 <sipa> wumpus: i very much doubt it will be significant if you could all SHA256 operations (as opposed to just block header hashes)
6542017-06-08T20:08:57 <sipa> *count
6552017-06-08T20:08:58 <jtimon> yeah, it should reduce the number of hashes when validating a block
6562017-06-08T20:09:07 <jnewbery> luke-jr: I've noticed an issue with zmq_test.py not exiting correctly. #10555 fixes that and also adds a timeout for individual tests (which means travis should show more useful output if one of the tests hangs). Try rebasing your PR on that and see if it helps.
6572017-06-08T20:09:09 <gribble> https://github.com/bitcoin/bitcoin/issues/10555 | [tests] various improvements to zmq_test.py by jnewbery · Pull Request #10555 · bitcoin/bitcoin · GitHub
6582017-06-08T20:09:19 <wumpus> well the point of the pull was to reduce header hashes, so that's what I counted
6592017-06-08T20:09:48 <morcos> jtimon: I
6602017-06-08T20:09:48 <sipa> i'm sure it reduces header hashes (and thanks for the concrete evidence for that)... but there are probably 2 orders of magnitude more hashes in computing merkle trees
6612017-06-08T20:09:49 <wumpus> but I don't care, won't spend any more time on it at least
6622017-06-08T20:10:09 <sipa> :)
6632017-06-08T20:10:21 <sipa> cfields_: that should go in our leveldb repo then
6642017-06-08T20:10:26 <sipa> ... or upstream
6652017-06-08T20:10:31 <morcos> jtimon: I'd need to look at 8498 more closely, but that one doesn't not seem to me to add that much complication.. it might be a nicer layout.. but i need to spend more time thinking it through
6662017-06-08T20:10:43 <cfields_> sipa: sure, first one, then the other
6672017-06-08T20:11:02 <gmaxwell> jtimon: it wasn't a strong preference, I'm okay with that PR if pieter is okay with it. Though morcos seems to have stronger views.
6682017-06-08T20:11:04 <jtimon> morcos: thank you for the fast look, that already helps
6692017-06-08T20:12:17 <jtimon> gmaxwell: yeah, that's why I should learn to benchmark these things, I'm sure it isn't hard and it's just that I've never tried
6702017-06-08T20:12:19 <morcos> like i said, i personally don't love the direction that 10339 goes, but if other people have already done the review and like it, i don't think its worth further debate to stop it
6712017-06-08T20:13:11 <morcos> it seems like premature optimization that makes the code less easy to use to me, but not something terrible
6722017-06-08T20:13:34 <sipa> right - perhaps there is no noticable performance gain, in which case it's obviously not worth it
6732017-06-08T20:14:08 <sipa> i soft of assumed there would be, and if that's the case, i think passing along a hash of an object with the object itself is a very reasonable thing to do
6742017-06-08T20:14:18 <sipa> but perhaps all of this is premature
6752017-06-08T20:14:51 <wumpus> right
6762017-06-08T20:14:59 <wumpus> it's all up to finding a way to benchmark it then
6772017-06-08T20:15:26 <sipa> i believe we should perhaps postpone this until after #10192
6782017-06-08T20:15:28 <gribble> https://github.com/bitcoin/bitcoin/issues/10192 | Cache full script execution results in addition to signatures by TheBlueMatt · Pull Request #10192 · bitcoin/bitcoin · GitHub
6792017-06-08T20:15:37 <sipa> and then use the benchmark strategy that BlueMatt used there
6802017-06-08T20:16:24 <sipa> or use some replay that feeds transactions and blocks into bitcoind over p2p...
6812017-06-08T20:18:20 <jtimon> I'll go look that benchmarking stratedy
6822017-06-08T20:25:32 <gmaxwell> I think there is a simpler way to argue the performance: benchmark hashing a header, and then count how many hashes the change saves. Total saved time is no more than the product.
6832017-06-08T20:25:51 <wumpus> the playback would work using morcos's data
6842017-06-08T20:26:22 <jtimon> so how did you counted it was about 6 ?
6852017-06-08T20:27:56 <wumpus> jtimon: https://github.com/laanwj/bitcoin/commit/fcfd20ea01f413dd2aa19b0013fe2b7aa2e47ad8
6862017-06-08T20:28:20 *** echonaut1 has quit IRC
6872017-06-08T20:32:11 *** trippysa1mon has joined #bitcoin-core-dev
6882017-06-08T20:33:11 *** trippysalmon has quit IRC
6892017-06-08T20:33:25 *** echonaut has joined #bitcoin-core-dev
6902017-06-08T20:40:51 *** nakaluna has quit IRC
6912017-06-08T20:45:38 *** Giszmo has quit IRC
6922017-06-08T20:45:56 <jtimon> wumpus: thanks
6932017-06-08T20:51:02 *** cryptapus_afk has quit IRC
6942017-06-08T20:51:30 *** cryptapus_afk has joined #bitcoin-core-dev
6952017-06-08T20:55:57 *** cryptapus_afk has quit IRC
6962017-06-08T21:05:34 <bitcoin-git> [bitcoin] morcos opened pull request #10559: Change semantics of HaveCoinInCache to match HaveCoin (master...haveunspentincache) https://github.com/bitcoin/bitcoin/pull/10559
6972017-06-08T21:10:57 *** owowo has quit IRC
6982017-06-08T21:16:03 *** owowo has joined #bitcoin-core-dev
6992017-06-08T21:16:03 *** owowo has joined #bitcoin-core-dev
7002017-06-08T21:18:45 *** Chris_Stewart_5 has quit IRC
7012017-06-08T21:21:35 *** beatrootfarmer has joined #bitcoin-core-dev
7022017-06-08T21:25:23 *** goatturneer has quit IRC
7032017-06-08T21:26:34 <gmaxwell> Am I alone in thinking we should have more comments in the code and commit message vs PRs? E.g. when I wanted to cite the multiply/shift hash trick for the mailing list, I first looked at the source-- no explination of what its doing, then the commit message that added the change, all it said was allow non-power-of-two cache sizes, -- the PR text had the citation I wanted https://github.com/
7042017-06-08T21:26:40 <gmaxwell> bitcoin/bitcoin/pull/9533
7052017-06-08T21:28:35 <wumpus> no, more (useful) source code comments would be nice
7062017-06-08T21:28:59 <wumpus> I'm always happy if people add them, or other documentation for that matter
7072017-06-08T21:29:04 <morcos> agreed, i've at least started to incororate most of my PR messages in the commits
7082017-06-08T21:29:37 <morcos> but more source code commentary would be good too, although it sometimes gets a bit stale when a major PR changes a lot of pieces
7092017-06-08T21:30:16 <wumpus> yes that's the drawback, keeping comments up to date is extra maintenance work
7102017-06-08T21:30:22 <wumpus> ideal would be self-illustrating code
7112017-06-08T21:30:27 <wumpus> but uh, yeah
7122017-06-08T21:30:36 <luke-jr> gmaxwell: I agree
7132017-06-08T21:31:16 <luke-jr> instead of answering questions on PRs, we should interpret it as a bug that it's underdocumented ;)
7142017-06-08T21:31:23 <wumpus> I've tried, there's many times I've encouraged people to add a code comment instead of just mentioning the rationale in a PR/commit message
7152017-06-08T21:32:57 <wumpus> in any case being able to find it in the commit message or even PR is still better than if it was some discussion on IRC :)
7162017-06-08T21:34:40 <luke-jr> heh
7172017-06-08T22:03:45 *** so has quit IRC
7182017-06-08T22:04:11 *** so has joined #bitcoin-core-dev
7192017-06-08T22:04:23 *** haakonn has quit IRC
7202017-06-08T22:04:29 *** haakonn has joined #bitcoin-core-dev
7212017-06-08T22:04:52 *** haakonn is now known as Guest56170
7222017-06-08T22:05:32 *** vicenteH has quit IRC
7232017-06-08T22:08:21 <bitcoin-git> [bitcoin] practicalswift closed pull request #10343: Remove redundant on-the-same-line-repetition of type names (DRY): RepeatedTypeName foo = static_cast<RepeatedTypeName>(bar) (master...auto) https://github.com/bitcoin/bitcoin/pull/10343
7242017-06-08T22:09:32 *** Guyver2 has quit IRC
7252017-06-08T22:54:37 *** Dyaheon has quit IRC
7262017-06-08T22:57:21 *** Dyaheon has joined #bitcoin-core-dev
7272017-06-08T23:03:01 *** ndr76 has quit IRC
7282017-06-08T23:11:19 *** owowo has quit IRC
7292017-06-08T23:21:52 <bitcoin-git> [bitcoin] practicalswift opened pull request #10560: Remove non-existing "force safe mode" (-testsafemode). Remove unused constants. (master...noop-modes) https://github.com/bitcoin/bitcoin/pull/10560
7302017-06-08T23:27:05 *** goatturneer has joined #bitcoin-core-dev
7312017-06-08T23:30:30 *** coredump_ has joined #bitcoin-core-dev
7322017-06-08T23:30:55 *** beatrootfarmer has quit IRC
7332017-06-08T23:35:04 <bitcoin-git> [bitcoin] practicalswift opened pull request #10561: Remove duplicate includes (master...remove-duplicate-includes) https://github.com/bitcoin/bitcoin/pull/10561
7342017-06-08T23:50:24 *** echonaut has quit IRC
7352017-06-08T23:50:32 *** echonaut2 has joined #bitcoin-core-dev
7362017-06-08T23:53:34 *** beatrootfarmer has joined #bitcoin-core-dev
7372017-06-08T23:57:31 *** goatturneer has quit IRC
7382017-06-08T23:58:50 *** cryptapus has joined #bitcoin-core-dev
7392017-06-08T23:58:52 *** cryptapus is now known as cryptapus_afk
7402017-06-08T23:59:29 *** abpa has quit IRC