1 2016-07-28 04:42:23	0|GitHub179|[13bitcoin] 15shinradev opened pull request #8415: Update tor.md (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/8415
  2 2016-07-28 06:49:52	0|jonasschnelli|wumpus: I guess this is now safe: https://github.com/bitcoin/bitcoin/pull/8407/files#diff-bd81cc35fa9249ec9e6a2ace6d3a4d59R446
  3 2016-07-28 07:31:37	0|wumpus|jonasschnelli: I don't think that will work; you just moved settings.value(strSettingsVersionKey)  to the end
  4 2016-07-28 07:32:08	0|wumpus|jonasschnelli: what do you have against what I suggested there? settingsVersion = settings.contains(strSettingsVersionKey) ? settings.value(strSettingsVersionKey) : 0;   upfront, then use then from there on
  5 2016-07-28 07:32:20	0|wumpus|also saves calls to settings.value()
  6 2016-07-28 07:35:12	0|GitHub154|13bitcoin/06master 14d3af342 15Kaz Wesley: prepend license statement to indirectmap...
  7 2016-07-28 07:35:12	0|GitHub154|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/4d4970fe530a...c24b50ec168e
  8 2016-07-28 07:35:13	0|GitHub154|13bitcoin/06master 14c24b50e 15Wladimir J. van der Laan: Merge #8414: prepend license statement to indirectmap.h...
  9 2016-07-28 07:35:25	0|GitHub100|[13bitcoin] 15laanwj closed pull request #8414: prepend license statement to indirectmap.h (06master...06indirectmap-license) 02https://github.com/bitcoin/bitcoin/pull/8414
 10 2016-07-28 07:35:57	0|wumpus|by regarding the absence of strSettingsVersionKey as settings version 0, it will do exactly what you want
 11 2016-07-28 07:51:28	0|GitHub90|13bitcoin/06master 1438c4c8b 15Jorge Timón: Trivial: Segwit: Don't call IsWitnessEnabled from ContextualCheckBlock
 12 2016-07-28 07:51:28	0|GitHub90|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/c24b50ec168e...64d660a43fb8
 13 2016-07-28 07:51:29	0|GitHub90|13bitcoin/06master 1464d660a 15Wladimir J. van der Laan: Merge #8348: Trivial: Segwit: Don't call IsWitnessEnabled from ContextualCheckBlock...
 14 2016-07-28 07:51:36	0|GitHub120|[13bitcoin] 15laanwj closed pull request #8348: Trivial: Segwit: Don't call IsWitnessEnabled from ContextualCheckBlock (06master...060.12.99-consensus-segwit) 02https://github.com/bitcoin/bitcoin/pull/8348
 15 2016-07-28 08:11:43	0|GitHub159|[13bitcoin] 15shinradev closed pull request #8415: Update tor.md (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/8415
 16 2016-07-28 08:11:55	0|jonasschnelli|wumpus: Ah. Sorry. Didn't got your suggestion. But right, makes more sense.
 17 2016-07-28 08:11:59	0|jonasschnelli|Let me change it
 18 2016-07-28 08:12:12	0|wumpus|thanks :)
 19 2016-07-28 08:13:25	0|jonasschnelli|Yes. Much cleaner...
 20 2016-07-28 08:19:07	0|jonasschnelli|wumpus: updated and tested. https://github.com/bitcoin/bitcoin/pull/8407/files#diff-bd81cc35fa9249ec9e6a2ace6d3a4d59R446
 21 2016-07-28 08:39:22	0|wumpus|jonasschnelli: you removed the check for nDatabaseCache==100 itself :)
 22 2016-07-28 08:39:36	0|jonasschnelli|looking...
 23 2016-07-28 08:39:37	0|wumpus|jonasschnelli: now it will always change the value to the default on an upgrade even if the user changed it
 24 2016-07-28 08:41:03	0|jonasschnelli|damit. I should focus on one thing at the time. Thanks wumpus. What would we (I!) do without you...
 25 2016-07-28 08:41:04	0|wumpus|if (settingsVersion < 130000 && settings.contains("nDatabaseCache")  && settings.value(strSettingsVersionKey).toInt() == 100)
 26 2016-07-28 08:42:26	0|wumpus|eh that's wrong
 27 2016-07-28 08:42:39	0|wumpus|if (settingsVersion < 130000 && settings.contains("nDatabaseCache")  && settings.value("nDatabaseCache").toInt() == 100)
 28 2016-07-28 08:42:48	0|jonasschnelli|wumpus: that should be better: https://github.com/bitcoin/bitcoin/pull/8407/files#diff-bd81cc35fa9249ec9e6a2ace6d3a4d59R447 right?
 29 2016-07-28 08:43:01	0|wumpus|now I was confusing the settings names... this seems so trivial, but actually it's trivial to accidentally mess up
 30 2016-07-28 08:43:16	0|jonasschnelli|yes. Needs to focus to get this right...
 31 2016-07-28 08:45:12	0|wumpus|which is difficult with the heat
 32 2016-07-28 08:46:25	0|wumpus|my plan is to at least test 8371 today
 33 2016-07-28 08:47:18	0|wumpus|jonasschnelli: yes so if (settingsVersion < 130000 && settingsVersion == 100) also doesn't work, it makes the same confusion I did. We need to look at nDatabaseCache here :)
 34 2016-07-28 08:47:52	0|jonasschnelli|hah. To dump...
 35 2016-07-28 08:48:38	0|jonasschnelli|if (settingsVersion < 130000 && settings.contains("nDatabaseCache") && settings.value("nDatabaseCache"))?
 36 2016-07-28 08:48:55	0|jonasschnelli|.toInt() == 100 at the end
 37 2016-07-28 08:48:59	0|wumpus|if (settingsVersion < 130000 && settings.contains("nDatabaseCache") && settings.value("nDatabaseCache").toInt()==100)
 38 2016-07-28 08:49:01	0|wumpus|yes I think so
 39 2016-07-28 08:52:00	0|wumpus|or toLongLong() as we seem to provide it as qint64
 40 2016-07-28 08:56:51	0|jonasschnelli|Yes. Probably better,... though I don't expect machines with that much RAM. :)
 41 2016-07-28 08:57:21	0|wumpus|I don't either, but I'm not certain how qt handled type conflicts there. If it works with toInt() that's good enough :)
 42 2016-07-28 08:57:58	0|jonasschnelli|Changed it to toLongLong() https://github.com/bitcoin/bitcoin/pull/8407/files#diff-bd81cc35fa9249ec9e6a2ace6d3a4d59R447
 43 2016-07-28 09:09:49	0|wumpus|awesome
 44 2016-07-28 09:14:59	0|wumpus|looks good to me now
 45 2016-07-28 09:15:51	0|wumpus|going to test
 46 2016-07-28 09:29:00	0|GitHub189|13bitcoin/06master 14893f379 15Jonas Schnelli: [Qt] Add dbcache migration path
 47 2016-07-28 09:29:00	0|GitHub189|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/64d660a43fb8...30a87c0747a1
 48 2016-07-28 09:29:01	0|GitHub189|13bitcoin/06master 1430a87c0 15Wladimir J. van der Laan: Merge #8407: [Qt] Add dbcache migration path...
 49 2016-07-28 09:29:16	0|GitHub169|[13bitcoin] 15laanwj closed pull request #8407: [Qt] Add dbcache migration path (06master...062016/07/qt_dbcache) 02https://github.com/bitcoin/bitcoin/pull/8407
 50 2016-07-28 09:30:01	0|jonasschnelli|Should probably merge fine into 0.13
 51 2016-07-28 09:30:20	0|wumpus|yep
 52 2016-07-28 09:30:50	0|GitHub153|13bitcoin/060.13 1445eba4b 15Jonas Schnelli: [Qt] Add dbcache migration path...
 53 2016-07-28 09:30:50	0|GitHub153|[13bitcoin] 15laanwj pushed 1 new commit to 060.13: 02https://github.com/bitcoin/bitcoin/commit/45eba4b1e0658639bb92730723b987f05e171529
 54 2016-07-28 09:45:26	0|jonasschnelli|wumpus: thanks for testing, just changed the font and added the 2nd info text: https://github.com/bitcoin/bitcoin/pull/8371
 55 2016-07-28 09:48:15	0|wumpus|okay thanks for updating, will re-test again soon, now looking at 8389
 56 2016-07-28 09:49:08	0|wumpus|you've certainly been solving many 0.13.0rc1 issues quickly, where would we be without you
 57 2016-07-28 09:53:14	0|wumpus|looking for more review for: Prevent fingerprinting, disk-DoS with compact blocks #8408
 58 2016-07-28 09:54:47	0|jonasschnelli|Yes. 8389 is more important
 59 2016-07-28 09:54:54	0|jonasschnelli|Looking at 8408
 60 2016-07-28 09:59:30	0|jonasschnelli|8408 makes sense. Props to sdaftuar! It needs extrem focus to find these issues...
 61 2016-07-28 10:00:00	0|wumpus|yes these are very sneaky, good to have them discovered and fixed before the release
 62 2016-07-28 10:12:15	0|wumpus|ooh, regtest has the same RPC default port as testnet. I only discover this now
 63 2016-07-28 10:13:01	0|jonasschnelli|wasn't aware of that...
 64 2016-07-28 10:13:29	0|jonasschnelli|Ah. Just the p2p port is differnt...
 65 2016-07-28 10:13:57	0|wumpus|me neither. Never tried to start a regtest node on the same host that was running a testnet node before
 66 2016-07-28 10:14:24	0|wumpus|it gives you: "Error: Unable to start HTTP server. See debug log for details." (where the log indeed contains details about failed binding)
 67 2016-07-28 10:14:50	0|wumpus|not a big issue just passing rpcport now, but I think it would make sense to bump that port number
 68 2016-07-28 10:16:52	0|wumpus|I get a "Warning: Error reading wallet.dat! All keys read correctly, but transaction data or address book entries might be missing or incorrect." with #8389, not sure why
 69 2016-07-28 10:17:49	0|wumpus|after that it just continues
 70 2016-07-28 10:18:01	0|wumpus|this is an old-fashioned, pre-HD wallet
 71 2016-07-28 10:18:15	0|jonasschnelli|let me have a look
 72 2016-07-28 10:19:02	0|jonasschnelli|wumpus: hmm.. that error should not be related to 8389 i guess
 73 2016-07-28 10:19:12	0|wumpus|let me see if it happens without
 74 2016-07-28 10:19:24	0|wumpus|maybe it's just a corrupted wallet, that's possible
 75 2016-07-28 10:20:12	0|wumpus|oops happens without #8408 too
 76 2016-07-28 10:20:22	0|wumpus|eh without 8389 I mean
 77 2016-07-28 10:20:54	0|jonasschnelli|hmm... this error means you had an error during ReadKeyValue() but not for a key
 78 2016-07-28 10:21:18	0|wumpus|moving wallet out of the way, creating a new non-HD wallet
 79 2016-07-28 10:21:22	0|jonasschnelli|I think your wallet.dat has a key that your current code cannot understand
 80 2016-07-28 10:21:34	0|jonasschnelli|yes. Maybe do a clean start
 81 2016-07-28 10:22:33	0|wumpus|that works fine, also after restarting bitcoind with that wallet
 82 2016-07-28 10:22:56	0|wumpus|so it was something specific to the previous wallet, maybe it was corrupted in some older testing. Ok, phew
 83 2016-07-28 10:23:35	0|wumpus|I've saved that wallet, might take a look at what key confused it later
 84 2016-07-28 10:27:33	0|wumpus|huh: dumpwallet result: cTjHTNHECpYNvFsJ2QXGVaDqbX8gX625S45Errqjy44oRHxPEzMc 2011-02-02T21:16:42Z hdmaster=1 # addr=mzUm95ZQPCVhGyPcJ3CvYwARBBQKX7efdR
 85 2016-07-28 10:28:17	0|wumpus|the date is wrong, and the key stayed the same before and after encryption
 86 2016-07-28 10:29:00	0|jonasschnelli|hmm... that's wrong. Let me retest. Maybe we have a rebase issue here...
 87 2016-07-28 10:29:07	0|jonasschnelli|Did you test with master? or 0.13?
 88 2016-07-28 10:29:17	0|wumpus|0.13 (as that's what the pull is against)
 89 2016-07-28 10:29:25	0|jonasschnelli|Okay. Will do also.
 90 2016-07-28 10:30:28	0|jonasschnelli|We should implement the dumpwallet test as you mentioned in that issue yesterday.
 91 2016-07-28 10:35:53	0|wumpus|jonasschnelli: see https://github.com/bitcoin/bitcoin/pull/8389 for details on what I did
 92 2016-07-28 10:36:56	0|jonasschnelli|there is on inactivehdmaster....
 93 2016-07-28 10:36:59	0|jonasschnelli|s/on/no
 94 2016-07-28 10:37:03	0|jonasschnelli|hmm...
 95 2016-07-28 10:37:12	0|wumpus|indeed, there is no inactivehdmaster, and the hdmaster didn't change
 96 2016-07-28 10:44:34	0|jonasschnelli|wumpus: I'm running the same steps but get a clean/correct 2nd dump
 97 2016-07-28 10:45:00	0|jonasschnelli|Do you have the "Warning: Error reading wallet.dat! " in your log at the 2nd start of bitcoind?
 98 2016-07-28 10:45:22	0|jonasschnelli|We definitively need a RPC test for this...
 99 2016-07-28 10:45:26	0|jonasschnelli|going to write one now
100 2016-07-28 10:45:50	0|wumpus|oh shit - I've been testing without #8389 merged
101 2016-07-28 10:46:17	0|wumpus|I unmerged it because of testing the error message, then forgot to apply it again
102 2016-07-28 10:46:36	0|wumpus|is this the expected output for pre-8389?
103 2016-07-28 10:47:13	0|wumpus|LOL yes it is, look at http://www.hastebin.com/rugovalaqi.diff - it generates the same key again for the same hdkeypath
104 2016-07-28 10:47:44	0|wumpus|going to retry with actually the right code, sorry
105 2016-07-28 10:48:56	0|jonasschnelli|okay.. no problem. Better an mistake during testing then a mistake in the code.
106 2016-07-28 10:49:09	0|jonasschnelli|But anyways,.. going to write a dumpwallet test
107 2016-07-28 10:49:22	0|wumpus|yes, we need one anyway
108 2016-07-28 10:55:22	0|wumpus|trying to update my post to a tested ACK but I get pink unicorns
109 2016-07-28 10:56:04	0|wumpus|BTW: unrelated to any of the HD changes, but I wonder why is a key generated with label="" at initial wallet initialization?
110 2016-07-28 10:56:21	0|wumpus|I'm fairly sure it's always been the case, but it doesn't seem necessary
111 2016-07-28 10:58:26	0|jonasschnelli|wumpus: During initial wallet created it always generate a first address... luke-jr once explained to my why this is necessary..
112 2016-07-28 11:00:31	0|GitHub72|13bitcoin/06master 14e37b16a 15Daniel Cousens: transaction: clarify witness branches
113 2016-07-28 11:00:31	0|GitHub72|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/30a87c0747a1...806b9e7570a2
114 2016-07-28 11:00:32	0|GitHub72|13bitcoin/06master 14806b9e7 15Wladimir J. van der Laan: Merge #8332: semi trivial: clarify witness branches in transaction.h serialization...
115 2016-07-28 11:00:41	0|GitHub10|[13bitcoin] 15laanwj closed pull request #8332: semi trivial: clarify witness branches in transaction.h serialization (06master...06patch-1) 02https://github.com/bitcoin/bitcoin/pull/8332
116 2016-07-28 11:00:52	0|wumpus|curious
117 2016-07-28 11:03:47	0|wumpus|created an issue https://github.com/bitcoin/bitcoin/issues/8416 (not relevant for 0.13, but now that we're slowly starting to commit to removing old wallet cruft)
118 2016-07-28 11:04:28	0|jonasschnelli|I could imaging some function might run into problems if there are 0 receiving addresses...
119 2016-07-28 11:04:38	0|jonasschnelli|Need to remove the default key and run all the RPC tests
120 2016-07-28 11:04:43	0|wumpus|yes it's definitely something that requires extensive testing
121 2016-07-28 11:04:50	0|wumpus|which is probably why no one ever dared remove that code :)
122 2016-07-28 11:06:05	0|jonasschnelli|heh. Indeed
123 2016-07-28 11:10:51	0|GitHub76|[13bitcoin] 15laanwj closed pull request #8389: [0.13] Create a new HD seed after encrypting the wallet (060.13...062016/07/hd_enc) 02https://github.com/bitcoin/bitcoin/pull/8389
124 2016-07-28 11:10:51	0|GitHub88|[13bitcoin] 15laanwj pushed 3 new commits to 060.13: 02https://github.com/bitcoin/bitcoin/compare/45eba4b1e065...c3c82c48d9d7
125 2016-07-28 11:10:52	0|GitHub88|13bitcoin/060.13 14de45c06 15Jonas Schnelli: [Wallet] Add CKeyMetadata record for HDMasterKey(s), factor out HD key generation
126 2016-07-28 11:10:52	0|GitHub88|13bitcoin/060.13 14f142c11 15Jonas Schnelli: [0.13] Create a new HD seed after encrypting the wallet
127 2016-07-28 11:10:53	0|GitHub88|13bitcoin/060.13 14c3c82c4 15Wladimir J. van der Laan: Merge #8389: [0.13] Create a new HD seed after encrypting the wallet...
128 2016-07-28 11:11:17	0|jonasschnelli|reminder: 8389 needs an "up-port" (my mistake)
129 2016-07-28 11:13:05	0|wumpus|seems to apply cleanly to master, apart from the release-notes change which we can ignore there
130 2016-07-28 11:23:33	0|GitHub54|13bitcoin/06master 142266b43 15Jonas Schnelli: Port from 0.13: Create a new HD seed after encrypting the wallet...
131 2016-07-28 11:23:33	0|GitHub54|[13bitcoin] 15laanwj pushed 1 new commit to 06master: 02https://github.com/bitcoin/bitcoin/commit/2266b43e3317a889b9150e614169acda50383bf5
132 2016-07-28 11:28:05	0|wumpus|ok that leaves https://github.com/bitcoin/bitcoin/pull/8408 and we can roll rc2!
133 2016-07-28 11:54:21	0|instagibbs|\o/
134 2016-07-28 11:54:28	0|GitHub124|13bitcoin/06master 14fbc6070 15Thomas Snider: [trivial] Switched constants to sizeof()
135 2016-07-28 11:54:28	0|GitHub124|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/2266b43e3317...133c727cc4f7
136 2016-07-28 11:54:29	0|GitHub124|13bitcoin/06master 14133c727 15Wladimir J. van der Laan: Merge #8321: [trivial] Switched constants to sizeof()...
137 2016-07-28 11:54:40	0|GitHub32|[13bitcoin] 15laanwj closed pull request #8321: [trivial] Switched constants to sizeof() (06master...06tjps_cleanups) 02https://github.com/bitcoin/bitcoin/pull/8321
138 2016-07-28 13:07:28	0|GitHub44|[13bitcoin] 15jonasschnelli opened pull request #8417: [QA] Add walletdump RPC test (including HD- & encryption-tests) (06master...062016/07/dump_test) 02https://github.com/bitcoin/bitcoin/pull/8417
139 2016-07-28 16:44:24	0|jonasschnelli|cfields: https://github.com/bitcoin/bitcoin/pull/8135
140 2016-07-28 16:44:39	0|jonasschnelli|Is this still required? Lost track of the osx based deployment process...
141 2016-07-28 16:44:44	0|jonasschnelli|If not, I can close the PR
142 2016-07-28 16:45:24	0|cfields|jonasschnelli: let me take another look. pretty sure i walked away from that one to think on it a bit. all solutions are ugly :\
143 2016-07-28 16:45:56	0|jonasschnelli|heh.. sure.. I don't really care. I think deploying (dmging) on OSX is not something we need to support.
144 2016-07-28 16:46:08	0|jonasschnelli|Depends/gitian based deployment should be sufficient IMO
145 2016-07-28 16:46:33	0|cfields|jonasschnelli: iirc macports/brew use it though
146 2016-07-28 16:46:44	0|jonasschnelli|Ah. Okay. that could be...
147 2016-07-28 16:47:12	0|jonasschnelli|Yes. Not sure if 8135 fixes it completly... I know it worked on my local mac when I tested it a couple of weeks ago
148 2016-07-28 16:47:39	0|cfields|jonasschnelli: what do you think of just pulling the ~3 python libs directly into support/macdeploy?
149 2016-07-28 16:47:48	0|cfields|i believe we patch them all already
150 2016-07-28 16:48:03	0|cfields|(until upstream merges the fixes, that is)
151 2016-07-28 16:48:20	0|jonasschnelli|Or pull in a script that downloads and patches them?
152 2016-07-28 16:48:31	0|jonasschnelli|I don't want to blow up our code base...
153 2016-07-28 16:50:23	0|cfields|ok. thinking on it. i'll ack/pr something either way so we can move on
154 2016-07-28 16:52:52	0|cfields|jonasschnelli: hmm, now that i think about it, macports/brew only use the .app and not the dmg. how about for osx we stop there, and only linux/gitian creates the dmg?
155 2016-07-28 16:56:20	0|cfields|yes, they just use 'make appbundle'. so that works for me if it's ok by you
156 2016-07-28 17:01:01	0|jonasschnelli|cfields: yes. I think this would make sense.
157 2016-07-28 17:01:13	0|jonasschnelli|You don't need a dmg if you selfcompile
158 2016-07-28 17:01:37	0|jonasschnelli|Only if you want to distribute... Which we should not encourage over non gitian
159 2016-07-28 17:10:55	0|cfields|right
160 2016-07-28 17:37:56	0|jtimon|is there any tutorial for monkies like me to do the gitian builds for a given release?
161 2016-07-28 17:40:29	0|sipa|jtimon: https://github.com/bitcoin/bitcoin/blob/0.13/doc/gitian-building.md
162 2016-07-28 17:47:53	0|jtimon|sipa: awesome, thanks
163 2016-07-28 17:48:45	0|jtimon|looks very complete
164 2016-07-28 18:10:14	0|GitHub99|[13bitcoin] 15sdaftuar opened pull request #8418: Add tests for compact blocks (06master...06cb-testing) 02https://github.com/bitcoin/bitcoin/pull/8418
165 2016-07-28 18:19:52	0|instagibbs|jtimon, I was able to walk through it fine.
166 2016-07-28 18:23:23	0|jtimon|instagibbs: cool, I'm not planning on doing it today, but I would like to start doing it. Hopefully starting with 0.13's release
167 2016-07-28 18:26:39	0|sipa|it takes a while :)
168 2016-07-28 18:26:54	0|sipa|i went through it again for 0.13.0rc1
169 2016-07-28 18:34:21	0|GitHub152|[13bitcoin] 15sdaftuar opened pull request #8419: Enable size accounting in mining unit tests (06master...06fix-mining-tests) 02https://github.com/bitcoin/bitcoin/pull/8419
170 2016-07-28 18:35:45	0|sdaftuar|sipa: i am having a hard time parsing your comment in #8408
171 2016-07-28 18:39:32	0|sipa|sdaftuar: in general, peers should not send getblocktxn for anything near the tip
172 2016-07-28 18:39:41	0|sipa|*anything not near the tip
173 2016-07-28 18:39:51	0|sdaftuar|right, agreed
174 2016-07-28 18:40:00	0|sdaftuar|are you suggesting that we disconnect for old blocks as wel?
175 2016-07-28 18:40:09	0|sdaftuar|rather than not disconnect for unknown blocks?
176 2016-07-28 18:40:37	0|sipa|if they do, they're either confused (and they're better off with us disconnecting them, or they'll wait long before timing out) or trying to do something nasty (in which case we're better off disconnecting them)
177 2016-07-28 18:40:59	0|sipa|the question is whether there are no cases under which this can happen accidentally
178 2016-07-28 18:41:12	0|sdaftuar|i assumed naively that this could happen on testnet
179 2016-07-28 18:41:32	0|sdaftuar|ie without much rigorous thought
180 2016-07-28 18:41:55	0|sdaftuar|mostly i guess i was comparing the handling of getblocktxn to how we handle getdata requests
181 2016-07-28 18:42:33	0|sdaftuar|where we silently ignore unknown, and non-main-chain old blocks
182 2016-07-28 18:43:03	0|sipa|ok, in that case let's follow the same approach
183 2016-07-28 18:43:40	0|sipa|i think there is a lot of behaviour that's ad hoc and hard to reason about there, but at least that follows existing tested code
184 2016-07-28 18:44:00	0|sipa|but perhaps we should rethink this in a bigger rework-block-sync-logic in the future...
185 2016-07-28 18:44:03	0|sdaftuar|fyi i did add a test in #8418 that catches the behaviors corrected in #8408
186 2016-07-28 18:44:21	0|sdaftuar|but 8418 is so much code that i didn't think it necessarily made sense as a bugfix PR for 0.13.0
187 2016-07-28 18:44:40	0|sipa|agree
188 2016-07-28 18:45:05	0|sipa|wait; 8418 has no changed behaviour, apart from what is already in 8408?
189 2016-07-28 18:45:09	0|sipa|i assume
190 2016-07-28 18:45:16	0|sdaftuar|it has one changed behavior, the new command line option -bip9params
191 2016-07-28 18:45:26	0|sdaftuar|i needed a way to turn off segwit on regtest
192 2016-07-28 18:45:33	0|sipa|sure sure
193 2016-07-28 18:45:42	0|sipa|i mean it should no impact on anything in mainnet
194 2016-07-28 18:46:03	0|sdaftuar|yes, agreed.  it's a lot of testing code, plus something that should be regtest-only
195 2016-07-28 18:46:13	0|sdaftuar|assuming i didn't screw up :)
196 2016-07-28 19:00:13	0|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
197 2016-07-28 19:00:14	0|jtimon|ready
198 2016-07-28 19:00:21	0|cfields|thanks, here
199 2016-07-28 19:00:29	0|sipa|present
200 2016-07-28 19:00:46	0|instagibbs|pre-zent
201 2016-07-28 19:01:04	0|lightningbot|Meeting started Thu Jul 28 19:01:03 2016 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
202 2016-07-28 19:01:04	0|lightningbot|Useful Commands: #action #agreed #help #info #idea #link #topic.
203 2016-07-28 19:01:04	0|wumpus|#startmeeting
204 2016-07-28 19:01:05	0|kanzure|greetings
205 2016-07-28 19:01:43	0|wumpus|topc suggestions?
206 2016-07-28 19:01:50	0|jtimon|proposed topic eliminate ISM (NicolasDorier's #8391)
207 2016-07-28 19:02:10	0|luke-jr|about to board flight..
208 2016-07-28 19:02:16	0|wumpus|rc2 is very near being tagged, https://github.com/bitcoin/bitcoin/milestone/20, could use some extra review for https://github.com/bitcoin/bitcoin/pull/8408
209 2016-07-28 19:02:41	0|wumpus|but that's the only remaining pull tagged 0.13 left
210 2016-07-28 19:02:48	0|luke-jr|wumpus: releadse notes still need to not recommend/opressure bad policy
211 2016-07-28 19:03:08	0|wumpus|luke-jr: ok
212 2016-07-28 19:03:19	0|luke-jr|tag applicable pr pls
213 2016-07-28 19:03:21	0|jtimon|wumpus: shouldn't #8412 be in 0.13?
214 2016-07-28 19:03:25	0|wumpus|but I guess people don't agree on what 'bad policy' is
215 2016-07-28 19:03:38	0|kanzure|also about to board a flight in a bit
216 2016-07-28 19:03:41	0|luke-jr|wumpus: …
217 2016-07-28 19:03:51	0|gmaxwell|luke-jr: don't elipises wumpus.
218 2016-07-28 19:03:51	0|jonasschnelli|I think it should
219 2016-07-28 19:03:57	0|jonasschnelli|(8412)
220 2016-07-28 19:04:20	0|jonasschnelli|I'll tag it
221 2016-07-28 19:04:20	0|jtimon|in fact, I think it should go in 0.12.2 if there's going to be one
222 2016-07-28 19:04:27	0|wumpus|I'm fine with 8412, hadn't heard of it before
223 2016-07-28 19:04:40	0|jtimon|wumpus: yeah, just created it yesterday
224 2016-07-28 19:04:48	0|sipa|8412 seems harmless and silly not to include
225 2016-07-28 19:04:54	0|jonasschnelli|It's a inconsequence with no risk attached to it (IMO)
226 2016-07-28 19:05:00	0|wumpus|seems we agree
227 2016-07-28 19:05:08	0|jtimon|great
228 2016-07-28 19:05:18	0|luke-jr|the current release nites recommends and undisputably bad policy. if there is controversy it shouldnt recommend anything
229 2016-07-28 19:05:33	0|wumpus|luke-jr: do you have a proposed update to the release notes?
230 2016-07-28 19:05:38	0|gmaxwell|RE 8412: why are we adding flags for long locked in network features?
231 2016-07-28 19:05:59	0|cfields|sdaftuar: any clues where to poke at #8096? Do you still think that's a blocker for release, or calling it a fluke?
232 2016-07-28 19:06:23	0|luke-jr|wumpus: yes, the pr is open for a week now
233 2016-07-28 19:06:34	0|cfields|err.. sorry for topic drift. we can pick that up later.
234 2016-07-28 19:06:36	0|jtimon|what is the "bad policy"? what am I missing?
235 2016-07-28 19:06:44	0|gmaxwell|okay never mind, I didn't look at the patch initially, I thought it was GBT rather than libconsensus.
236 2016-07-28 19:06:53	0|wumpus|luke-jr: the only PR I know of also changes quite some code
237 2016-07-28 19:06:56	0|jonasschnelli|https://github.com/bitcoin/bitcoin/pull/8388/
238 2016-07-28 19:07:05	0|jonasschnelli|Yes. Its not an release not only PR
239 2016-07-28 19:07:13	0|morcos|luke-jr: i don't understand why we need to change anything now.  segwit is not released.  using blockmaxcost is fine for 0.13
240 2016-07-28 19:07:18	0|gmaxwell|s/not/note/
241 2016-07-28 19:07:27	0|luke-jr|jtimon: blockmaxweight rsther than blockmaxsize
242 2016-07-28 19:07:27	0|morcos|uh, or weight
243 2016-07-28 19:07:44	0|jonasschnelli|gmaxwell: thanks. :)
244 2016-07-28 19:07:58	0|luke-jr|wumpus: because people were insisting the code had to change to remove the recommencaation
245 2016-07-28 19:08:14	0|gmaxwell|I think it's foolish that we're still releasing with settings that don't reflect the near ubiquitious network usage.
246 2016-07-28 19:08:33	0|sipa|luke-jr: we shouldn't be relying on miners picking safe policy that only harms themselves
247 2016-07-28 19:08:37	0|jtimon|gmaxwell: on that note, agree with sipa that we should decouple the consensus flags bit poisitions from the script flags', but that's only for master
248 2016-07-28 19:08:49	0|sipa|luke-jr: if we're not fine with larger blocks, we shouldn't do segwit as-is
249 2016-07-28 19:09:02	0|morcos|sipa: agreed
250 2016-07-28 19:09:15	0|morcos|where we is the big we
251 2016-07-28 19:09:16	0|sipa|in practice, nearly every miner will set the max block size and weight to the maximum allowed value
252 2016-07-28 19:09:23	0|sipa|morcos: yes!
253 2016-07-28 19:09:40	0|luke-jr|sipa: we cannot stop segwit for better or worse
254 2016-07-28 19:09:46	0|gmaxwell|Asking miners to produce blocks smaller than the network technically allows is a failed policy, in nay case, as we've seen. The default has been 750k-- but go look at the chain, no 750k blocks to be seen. :P
255 2016-07-28 19:09:49	0|luke-jr|and hopefully in some monthhs larger blocks WILL vbe fine
256 2016-07-28 19:10:00	0|luke-jr|sipa: prqactice is tbd
257 2016-07-28 19:10:01	0|sipa|luke-jr: 'fine' is not a boolean
258 2016-07-28 19:10:28	0|luke-jr|i need to board…
259 2016-07-28 19:10:41	0|wumpus|gmaxwell: on the other hand, miners not dumbly using the default settings is great, it's exactly what should be happening
260 2016-07-28 19:10:44	0|gmaxwell|luke-jr: well nothing special will happen because you aren't here. :P go board, we can talk later.
261 2016-07-28 19:10:45	0|morcos|i think it would be fine to include an explanation of the difference in the release notes without making a judgement about whether or not you should want to have blocks with size smaller than what might be created if you only limited by weight
262 2016-07-28 19:10:54	0|luke-jr|bbl
263 2016-07-28 19:11:50	0|jtimon|the way I see it, the default is stupid to kind of force miners not to use the default (and to avoid being changing some defaults all the time)
264 2016-07-28 19:11:58	0|morcos|if we don't recommend using size but explain why the option exists, then i also see no reason to cache size, if some people want to use it, its ok if its calc'ed on the fly (as current code does)_
265 2016-07-28 19:12:07	0|jtimon|so the issue is that weight has a bigger default than size had?
266 2016-07-28 19:12:11	0|wumpus|jtimon: yes, I thought that was the point
267 2016-07-28 19:12:30	0|wumpus|jtimon: otherwise it's back to 'devs decide the values'
268 2016-07-28 19:12:46	0|btcdrak|sorry I am late. #8412 should be included.
269 2016-07-28 19:12:52	0|jtimon|yeah, and people asking to increase op_return size and shit
270 2016-07-28 19:13:20	0|gmaxwell|IMO we should just make the defaults the maximums. Doing otherwise is thereatics, we know that users will immediately set them to the largest allowed values. And the few that wouldn't will happily manually reduce them.
271 2016-07-28 19:13:52	0|jtimon|re #8412 should I open the PR to 0.13 then?
272 2016-07-28 19:13:55	0|gmaxwell|this argument specifically applies to the 'max weight/size' things because we have expirence there with how they will be set in practice.
273 2016-07-28 19:14:21	0|sipa|jtimon: 8412 will backport trivially, no need for backport PR
274 2016-07-28 19:14:35	0|gmaxwell|(also, FWIW, the income difference between 750k size and maximum is non-trivial).
275 2016-07-28 19:15:29	0|jtimon|no strong opinion, it seems natural to take the oportunity of moving from size to weight to change the default to something more used, but I will complain when somebody asks to change it later
276 2016-07-28 19:15:34	0|wumpus|I really don't like to go back to discussing what defaults to use, e.g. we've had tons of pull requests to change the default block size and we've always used the reasoning explained by jtimon to hold them off
277 2016-07-28 19:15:40	0|wumpus|but indeed, no strong opinion either
278 2016-07-28 19:16:10	0|jtimon|if we want to set a lower default to "force people" not to use defaults there, that's fine with me as well
279 2016-07-28 19:16:32	0|sipa|the current 0.13 code does not change any defaults
280 2016-07-28 19:16:37	0|morcos|wumpus: i suppose the confusion is that now that we have 2 options, its totally non-intuitive how to se tthem given they have non max defaults
281 2016-07-28 19:16:46	0|jtimon|sipa: then what's the problem again?
282 2016-07-28 19:16:49	0|sipa|and the release notes explain the difference, and that only using weight is faster
283 2016-07-28 19:16:54	0|wumpus|yes, what is the problem then?
284 2016-07-28 19:16:59	0|sipa|i think the code and notes in 0.13 are fine
285 2016-07-28 19:17:46	0|gmaxwell|The notes don't seem to inform me how to set it to the maximum.
286 2016-07-28 19:18:07	0|gmaxwell|which is what 99% of the users are going to want to do, as it's what they currently do.
287 2016-07-28 19:18:21	0|sipa|ok, let's fix that in the --help message?
288 2016-07-28 19:18:47	0|gmaxwell|They won't read that. They'll read the release notes with greater odds than --help.
289 2016-07-28 19:18:55	0|wumpus|if it's what they currently do ,then why would the default even matter? they override it already.
290 2016-07-28 19:18:58	0|gmaxwell|(otherwise they wouldn't know of the maxweight option at all)
291 2016-07-28 19:18:58	0|sipa|i assume they'll read neither
292 2016-07-28 19:19:13	0|sdaftuar|(Note that for mainnet, in this release, the equivalent parameter for -blockmaxweight would be four times the desired -blockmaxsize. See BIP 141 for additional details.)
293 2016-07-28 19:19:17	0|morcos|you need to know how to set the limits to max, and also not have the code to unnecessary extra serialization
294 2016-07-28 19:19:28	0|gmaxwell|The default doesn't matter, except it forces people to override, which they'll likely get wrong now that it's been made more complicated.
295 2016-07-28 19:20:00	0|wumpus|in any case the help message and release notes should be clear, if they are missing information that needs to be added
296 2016-07-28 19:20:22	0|gmaxwell|E.g. you'll read that release note and then change your maxsize to maxweight, and then be surprised when their blocks are 250k. :P
297 2016-07-28 19:20:43	0|achow101|why not just set it to the max? I can't see any reason why users would complain about that given that that is what they are going to do anyways
298 2016-07-28 19:21:14	0|gmaxwell|achow101: that was the argument I was making before. wumpus correctly notes that going and debating defaults again isn't great.
299 2016-07-28 19:21:18	0|jtimon|gmaxwell: ok, so you want to just clarify how to select the max block size in the --help because now it's more complicated than before and add something to the release notes?
300 2016-07-28 19:21:30	0|gmaxwell|In particular luke will go on a public attack campaign, and some are trying to avoid it.
301 2016-07-28 19:21:43	0|wumpus|because we should not be in the business of determining what values miners should use, we've always tried to avoid that, for good reason
302 2016-07-28 19:21:47	0|gmaxwell|(because he sees a kind of principled position arising from setting a more correct default which no one uses)
303 2016-07-28 19:22:34	0|gmaxwell|wumpus: not at question of should, it's a question of 'does', and forcing people to set settings which are now somewhat complex due to the new option is a bit unfortunate.
304 2016-07-28 19:22:38	0|wumpus|if you want to inform people how to set the maximum, then adding how to do so in the release notes makes sense
305 2016-07-28 19:22:52	0|gmaxwell|yes, I can make a PR that explains how to do that.
306 2016-07-28 19:22:53	0|wumpus|gmaxwell: we've been there; this really feels like deja vu
307 2016-07-28 19:23:34	0|wumpus|so to be clear: the current 0.13 defaults will cause miners to do something else than create 750kb blocks, as was the case for 0.12?
308 2016-07-28 19:23:57	0|gmaxwell|No, the defaults are functionally equal to 0.12's.
309 2016-07-28 19:23:59	0|sdaftuar|wumpus: no, that is not the case.
310 2016-07-28 19:24:07	0|wumpus|ok, good then
311 2016-07-28 19:24:18	0|morcos|i think the release notes are accurate and relatively comprehensive enough right now.  unfortunately, i think the logic that exists is complicated enough that miners will make mistakes and either create smaller blocks than they meant to or slow down their mining computation
312 2016-07-28 19:24:33	0|gmaxwell|the default maxweight is 3 million, which, with no witness txn is exactly equal to 750k maxsize.
313 2016-07-28 19:24:43	0|jtimon|I really don't understand luke-jr's issue if the defaults are untouched
314 2016-07-28 19:25:03	0|morcos|i'm not sure what the solution to that is other than explaining exactly what you should do if you want to mine max size blocks, which might be seen as a recommendation
315 2016-07-28 19:25:25	0|wumpus|explaining in the release notes and documentation is exactly the right thing to do
316 2016-07-28 19:25:29	0|gmaxwell|jtimon: because a maxweight of 3m allows for a 3mb block in future versions with segwit activated and he doesn't want miners being instructed to use maxweight.
317 2016-07-28 19:25:31	0|instagibbs|jtimon, theoretical size max would be 3MB I think
318 2016-07-28 19:25:35	0|wumpus|so people understand what is being done
319 2016-07-28 19:25:50	0|wumpus|instead of saying 'this is too complicated for you, just follow the defaults'
320 2016-07-28 19:26:10	0|gmaxwell|wumpus: 'just follow the defaults' was never my position.
321 2016-07-28 19:26:32	0|jtimon|gmaxwell: why do miners need to be instructed about maxweight before activation? (not that I'm against informing people beforehand)
322 2016-07-28 19:26:43	0|gmaxwell|Rather, we have defaults that match AFAICT what _no one_ wants, meaning that there is no one who can go "oh what the defaults want is already what I want, so I don't have to screw with anything."
323 2016-07-28 19:26:50	0|wumpus|gmaxwell: I know, but that's what would effectively happen if the defaults are just the maximum which everyone uses
324 2016-07-28 19:26:52	0|morcos|gmaxwell: exactly
325 2016-07-28 19:27:42	0|wumpus|gmaxwell: people will start using the defaults, and be recommended using the defaults, which makes us in charge of determining the values. Next release there will be 20 pulls again of people that want to change the default to their favourite value, to lead others into using it
326 2016-07-28 19:27:47	0|morcos|jtimon: they need to be informed because we kept blockmaxsize as an option, but it is slower to calculate (by unknown amount)
327 2016-07-28 19:27:59	0|gmaxwell|jtimon: because he believes its establishing a bad set of practices which will hold over. I think he's right that it will hold over.. but only slightly, people are also going to immediately crank to maximum. I think luke realizes this too, but is adopting a principled position of not supporting something he thinks is bad even if it is inevitable.
328 2016-07-28 19:28:01	0|jtimon|ok, so imagining what most are doing and assuming they don't touch their configuration for now and don't do anything about weigh (ie use the default), everything will be fine, right?
329 2016-07-28 19:28:16	0|gmaxwell|And he think it is bad until compactblock deployment has proven that miners producing larger blocks is okay.
330 2016-07-28 19:28:17	0|jtimon|morcos: I see
331 2016-07-28 19:28:19	0|wumpus|hence, I really think defaults discussion is a waste of time, it just creates more of the same
332 2016-07-28 19:29:08	0|gmaxwell|instead we get worse discussions because we have setting which actively trip up users.
333 2016-07-28 19:29:09	0|sipa|i think we can just leave defaults as they are now
334 2016-07-28 19:29:21	0|gmaxwell|Sorry, wumpus, the software doesn't exist to save us from discussion, it exists to serve its users.
335 2016-07-28 19:29:28	0|wumpus|that's what we've learned from previous messing around with defaults, I don't understand why you've certainly changed your mind about this
336 2016-07-28 19:29:29	0|gmaxwell|I agree that we can leave them.
337 2016-07-28 19:29:31	0|instagibbs|ack for leaving as-is
338 2016-07-28 19:29:45	0|jtimon|mhmm, if they were creating bigger blocks, they will keep doing so, and if they were using the default, they will keep doing so
339 2016-07-28 19:29:57	0|gmaxwell|But we need instructions for setting them to maximum, because as is people will screw it up and produce 250k blocks (and if you care about complaining: they'll accuse us of being sneaky)
340 2016-07-28 19:30:09	0|wumpus|gmaxwell: where did I claim that the software exists to save us from discussion!??
341 2016-07-28 19:30:14	0|wumpus|gmaxwell: wtf is this
342 2016-07-28 19:30:16	0|jtimon|if they move to weigh because it's cheaper but still use the default, the size is the same
343 2016-07-28 19:30:19	0|morcos|I think the lesson to be learned here is we make the code too complicated by trying to be all things to all people.  For policy (not consensus) related decisions, i don't think we need to be able to support every possible thing.   We waste so much time trying to shoehorn in things like priority and maxsize, while simultaneously trying to make the code more efficient
344 2016-07-28 19:30:32	0|sipa|agree
345 2016-07-28 19:31:34	0|gmaxwell|wumpus: I'm sorry, thats what I'm reading out of your position on the defaults. I feel like you're basically arguing that we should have setting we know are bad and don't match usage, because it allows to refuse to discuss the defaults.
346 2016-07-28 19:31:53	0|Eliel_|Well, you could always just make the mining part refuse to function unless the user manually sets the required config values :P
347 2016-07-28 19:31:55	0|gmaxwell|I think thats an argument that we should make the software objectively worse in order to avoid debates over what is better.
348 2016-07-28 19:32:05	0|gmaxwell|I apologize for reading so much into it.
349 2016-07-28 19:32:07	0|Eliel_|as in, no dfaults
350 2016-07-28 19:32:17	0|jtimon|anyway, it seems this is mostly an issue for luke-jr, so I don't see the point in keeping discussing it while he is on a plane
351 2016-07-28 19:32:25	0|achow101|Eliel_: it's not very user friendly though
352 2016-07-28 19:32:31	0|sipa|how about we leave things as is, and in the future (whenever that may be) we find a way to simplify the options (back to one argument, for example)
353 2016-07-28 19:32:40	0|sdaftuar|ACK simplifying in the future
354 2016-07-28 19:32:46	0|morcos|sipa: i think thats the best we're going to get at this point
355 2016-07-28 19:32:48	0|sipa|that can 1) solve the problem of inadvisable defaults
356 2016-07-28 19:32:49	0|gmaxwell|But we really do need to have instructions on doing what people will do. And when we write those luke will complain because it will be arguable that we are endorsing those settings. Cannot escape the drama. :)
357 2016-07-28 19:33:08	0|wumpus|Eliel_: yes, that has been suggested before
358 2016-07-28 19:33:19	0|sipa|and 2) by changing things at the same time as simplifying the logic, it is not necessarily a recommendation
359 2016-07-28 19:33:22	0|achow101|gmaxwell: just tell him to suck it up  and deal with it
360 2016-07-28 19:33:22	0|Eliel_|achow101: that depends... if the error message comes with a link to a good instructional document, it might actually be a pretty good result.
361 2016-07-28 19:33:36	0|gmaxwell|luke only insisted we have the size option because compactblocks is not yet deployed, and if relay network isn't working, orphaning is highly related to size.
362 2016-07-28 19:33:47	0|gmaxwell|It didn't seem like it was worth debating.
363 2016-07-28 19:33:53	0|morcos|gmaxwell: i'd argue we can leave the release notes as is (which luke might still object to) and then just use other channels to explain to people very simply how they can create max blocks if they want.  simple.  set -blockmaxweight=4000000 and dont' set blockmaxsize at all
364 2016-07-28 19:33:58	0|wumpus|Eliel_: it may be better than setting silly defaults, but there was never agreement to do it
365 2016-07-28 19:34:10	0|gmaxwell|morcos: I can agree with that too.
366 2016-07-28 19:34:24	0|jtimon|gmaxwell: maybe overkill instructions with all the options (keep using the default in a more efficient way (ie not using maxsize) and do what most poeple do) is enough for luke-jr?
367 2016-07-28 19:34:26	0|Eliel_|wumpus: that's pretty much my reasoning behind the suggestion.
368 2016-07-28 19:34:41	0|Eliel_|if you can't set defaults the way most people want them, don't set any.
369 2016-07-28 19:34:51	0|morcos|I'm also fine with that suggestion
370 2016-07-28 19:35:05	0|gmaxwell|luke argued for that for years.
371 2016-07-28 19:35:11	0|Eliel_|and about user friendliness... do you really want to allow people to mine without understanding what they're doing?
372 2016-07-28 19:35:15	0|wumpus|Eliel_: the thing is that people disagree deeply over these setings
373 2016-07-28 19:35:26	0|jtimon|other topics?
374 2016-07-28 19:36:02	0|wumpus|#topic eliminate ISM (NicolasDorier's #8391)
375 2016-07-28 19:36:12	0|gmaxwell|Eliel_: absolutely.
376 2016-07-28 19:36:52	0|gmaxwell|Eliel_: participants in the network should be avoiding judgement, they're not in charge of the system because they participate.. and not because they have some theoretical technical ability to censor varrious behaviors.
377 2016-07-28 19:37:04	0|kanzure|i need to leave. i would like to request that folks send me topic suggestions for my wishlist for the upcoming gathering. not end of the world if it doesn't happen, but having a few thoughts prepared is worthwhile.
378 2016-07-28 19:37:37	0|wumpus|ping jtimon
379 2016-07-28 19:37:43	0|jtimon|this is quite important for other libconsensus refactors, the sooner it is merged the easy it will be to PR other changes, I would really appreciate fast review, test and merge of #8391
380 2016-07-28 19:38:18	0|jtimon|I would say it's the most important libconsensus' PR opened right now
381 2016-07-28 19:38:32	0|wumpus|I think we already agreed removing ISM was a good thing lastm eeting, is there more to discuss about it?
382 2016-07-28 19:39:04	0|jtimon|wumpus: well, I hope not, but that's why I bring it up (apart from asking for review)
383 2016-07-28 19:39:22	0|wumpus|yes it does need more review/testing
384 2016-07-28 19:39:28	0|wumpus|#action review https://github.com/bitcoin/bitcoin/pull/8391
385 2016-07-28 19:39:40	0|jtimon|thanks
386 2016-07-28 19:39:42	0|cfields|next topic suggestion: boost threads/sync replacement for master
387 2016-07-28 19:40:01	0|wumpus|#topic boost threads/sync replacement for master
388 2016-07-28 19:40:11	0|jtimon|and of course nacks and nits always the sooner the better
389 2016-07-28 19:40:16	0|cfields|https://github.com/bitcoin/bitcoin/pull/8023
390 2016-07-28 19:40:28	0|cfields|is nowish a good time to start pushing for that?
391 2016-07-28 19:40:38	0|wumpus|sounds good to me
392 2016-07-28 19:41:19	0|gmaxwell|neat.
393 2016-07-28 19:41:20	0|cfields|ok. it's a drop-in replacement for interruptible boost threads/condvars. preferred to switch chunks at a time, or all in one go?
394 2016-07-28 19:41:26	0|jtimon|cfields: since it's disruptive, the "refactor window" seems like the perfect time for it, no?
395 2016-07-28 19:41:31	0|wumpus|what about your network refactor? seems we should start merging for that as well?
396 2016-07-28 19:42:11	0|cfields|wumpus: yea, review/acks of 2 outstanding PRs would be a big help...
397 2016-07-28 19:42:11	0|wumpus|cfields: what would 'chunks at a time' mean, using two potentially conflicting thread libraries?
398 2016-07-28 19:42:18	0|jtimon|oh, #8023 is not disruptive at all
399 2016-07-28 19:42:24	0|cfields|sec for numbers
400 2016-07-28 19:42:29	0|wumpus|cfields: can you post the links? ok
401 2016-07-28 19:42:59	0|NicolasDorier|I'm back.
402 2016-07-28 19:43:03	0|NicolasDorier|for removing ISM
403 2016-07-28 19:43:05	0|wumpus|welcome back
404 2016-07-28 19:43:11	0|NicolasDorier|I synched nodes successfully
405 2016-07-28 19:43:21	0|NicolasDorier|so basically, it needs some review from other people, but should be good
406 2016-07-28 19:43:27	0|cfields|#8128 and #8085
407 2016-07-28 19:43:52	0|btcdrak|NicolasDorier: ok I'll run a full sync this weekend
408 2016-07-28 19:43:53	0|wumpus|#action review+test https://github.com/bitcoin/bitcoin/pull/8128 Net: Turn net structures into dumb storage classes
409 2016-07-28 19:44:03	0|cfields|#8085 is a beast to rebase. I have it done locally, but I'm waiting for #8128 to go in before rebasing again and pushing
410 2016-07-28 19:44:10	0|sipa|i tried switching some subset of the code to std::thread before, and it was impossible... you need to change it everywhere at once
411 2016-07-28 19:44:24	0|wumpus|#action review+test https://github.com/bitcoin/bitcoin/pull/8085 p2p: Begin encapsulation
412 2016-07-28 19:44:54	0|cfields|ok, I'll approach it as one big change then
413 2016-07-28 19:45:11	0|wumpus|yes it probably makes most sense to do it all as once, it's painful but at least it should be a one time pain then
414 2016-07-28 19:46:24	0|NicolasDorier|question: what you call "refactor windows" what is it ?
415 2016-07-28 19:46:31	0|cfields|ok. iirc there are 1/2 pre-reqs left for boost-specific usage, then basically a search/replace. I'll go back through my notes and do pulls for the pre-reqs.
416 2016-07-28 19:47:52	0|wumpus|NicolasDorier: well the best time for larger changes in 0.14 is now, as there's still some months to go before the release and there is enough time to address problems that come up
417 2016-07-28 19:48:17	0|wumpus|e.g. you wouldn't want to switch the thread library a few days before release
418 2016-07-28 19:48:26	0|NicolasDorier|ok nice to know, as I'm working around libconsensus... will go a bit quicker so
419 2016-07-28 19:48:49	0|NicolasDorier|oh
420 2016-07-28 19:49:29	0|NicolasDorier|about https://github.com/bitcoin/bitcoin/pull/8259 when will it be merged for 0.13 ?
421 2016-07-28 19:49:47	0|jtimon|re ISM (sorry for not saying it earlier), thoughts on https://github.com/jtimon/bitcoin/commit/273e27bd0c086f5ba20cebc2f5ec9e24f9d79015 ? independently on adding it to the PR or leave it for later
422 2016-07-28 19:49:51	0|sipa|NicolasDorier: we'll need to backport that to 0.13 before segwit
423 2016-07-28 19:50:03	0|NicolasDorier|ok
424 2016-07-28 19:51:02	0|wumpus|NicolasDorier: it's not even merged for master yet, after it is it can be backported
425 2016-07-28 19:51:38	0|wumpus|I'll add a "needs backport" label
426 2016-07-28 19:51:51	0|NicolasDorier|ok no prob
427 2016-07-28 19:52:03	0|sipa|NicolasDorier: thanks for the reminder on that btw
428 2016-07-28 19:52:14	0|wumpus|seems to need some more review too
429 2016-07-28 19:52:29	0|NicolasDorier|jtimon: I'd like to see it go personally, but I don't know the reason why bip34Hash was here in the first place
430 2016-07-28 19:52:51	0|sipa|jtimon: i vaguely remember there were ugly interactions between the cache code that avoids some database operation, bip30 and bip34... but i don't remember the details
431 2016-07-28 19:52:53	0|wumpus|#action review https://github.com/bitcoin/bitcoin/pull/8259 Hash Cache
432 2016-07-28 19:53:14	0|jtimon|uff, #8259 is ultra disruptive to consensus branch, should review...
433 2016-07-28 19:53:46	0|NicolasDorier|jtimon: yes, I was asking because as part of our work on libconsensus, we'll need to be fixed on that
434 2016-07-28 19:54:22	0|jtimon|yep, you agreed on #8346 first, right?
435 2016-07-28 19:54:46	0|sipa|jtimon: 8259 will need backport to 0.13
436 2016-07-28 19:55:03	0|NicolasDorier|yes
437 2016-07-28 19:56:08	0|wumpus|ok, any final topics?
438 2016-07-28 19:57:01	0|jtimon|sipa: damm, and #8346 cannot be backported because it's a refactor, "refactor winwow interlocking", before a release it's really the worse time to make refactors, but I repeated this many times...
439 2016-07-28 19:57:19	0|wumpus|no, refactors will not be backported, that is too risky
440 2016-07-28 19:58:03	0|jtimon|wumpus: then having the "refactor window" precisely when we're backporting more stuff is a bad idea
441 2016-07-28 19:58:03	0|sipa|it's fine, we'll find a solution for that
442 2016-07-28 19:58:06	0|wumpus|well wait until after the release then jtimon
443 2016-07-28 19:58:19	0|wumpus|hopefully rc2 will be the last
444 2016-07-28 19:58:35	0|wumpus|that'd mean the release can be next week or so
445 2016-07-28 19:58:48	0|jtimon|wumpus: if that still counts as "refactor window" I'm happy, I know when they start but I don't have a clear idea on when they finish
446 2016-07-28 19:59:00	0|sipa|wumpus: the issue is that 8346 won't be backported to 0.13, but 8259 will be, thus 8259 can't be based on that refactor
447 2016-07-28 19:59:31	0|sipa|but i don't think it's a big issue
448 2016-07-28 19:59:33	0|wumpus|sipa: yes, moving the refactor window to after the 0.13.0 release won't help for that
449 2016-07-28 19:59:53	0|sipa|sometimes code needs non-trivial backports
450 2016-07-28 19:59:55	0|sipa|so be it
451 2016-07-28 19:59:58	0|wumpus|yes
452 2016-07-28 20:00:09	0|wumpus|it's still trivial compared to backporting to 0.12
453 2016-07-28 20:00:24	0|sipa|DOING
454 2016-07-28 20:00:26	0|wumpus|#endmeeting
455 2016-07-28 20:00:27	0|lightningbot|Log:            http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-07-28-19.01.log.html
456 2016-07-28 20:00:27	0|lightningbot|Meeting ended Thu Jul 28 20:00:26 2016 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
457 2016-07-28 20:00:27	0|lightningbot|Minutes:        http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-07-28-19.01.html
458 2016-07-28 20:00:27	0|lightningbot|Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-07-28-19.01.txt
459 2016-07-28 20:00:58	0|sipa|DOH-ING
460 2016-07-28 20:01:03	0|jtimon|but in this case means that my alternative refactor to checkinputs will never get reviewed or merged...(even if it's way way older than #8259), so my best option is to rewrite it as nits ot #8259 I guess...
461 2016-07-28 20:01:06	0|gmaxwell|DOOIINNGGG
462 2016-07-28 20:01:30	0|jonasschnelli|sipa: time for your Pokemon walk. :P
463 2016-07-28 20:01:37	0|sdaftuar|cfields: regarding #8096, i don't think i've seen that error since i first reported it.
464 2016-07-28 20:01:38	0|jtimon|haha
465 2016-07-28 20:01:59	0|wumpus|:P
466 2016-07-28 20:02:12	0|cfields|sdaftuar: ok. I tried to investigate, but I couldn't even find a good starting point
467 2016-07-28 20:02:12	0|NicolasDorier|jtimon: my 8259 in fact date back in march :D
468 2016-07-28 20:02:24	0|NicolasDorier|I needed to refactor and reopen the issue several time
469 2016-07-28 20:02:31	0|NicolasDorier|I mean rebase
470 2016-07-28 20:02:37	0|sdaftuar|i recall having no idea what could have happened :)  i just fired up a script to run that test until failure, hopefully i'll get something i can debug
471 2016-07-28 20:02:49	0|NicolasDorier|(the first time 8259 was created was on sipa's branch before the merge :p)
472 2016-07-28 20:03:27	0|jtimon|well, my approach is as old as #6061
473 2016-07-28 20:03:30	0|cfields|sdaftuar: only thing I could think of is to add an assert(!empty()) at the beginning so at least we'll be able to tell if it was already empty, if we end up seeing another crash
474 2016-07-28 20:03:36	0|wumpus|sdaftuar: cfields: in any case, the question was whether that should be a blocker; I don't think a rare test failure should qualify as a release blocker unless you strongly suspect it to be a deeper problem
475 2016-07-28 20:03:53	0|sdaftuar|wumpus: i agree that it shouldn't be a blocker
476 2016-07-28 20:03:55	0|jtimon|that was a "please don't do too much stuff at a time" PR for my approach
477 2016-07-28 20:04:25	0|cfields|wumpus: i have no reason to suspect it's a major problem, i just don't like that we can't explain it :)
478 2016-07-28 20:04:53	0|wumpus|sdaftuar: thanks, removing the milestone then
479 2016-07-28 20:05:31	0|jtimon|NicolasDorier: last attempt to move in my direction: https://github.com/bitcoin/bitcoin/pull/6445
480 2016-07-28 20:05:35	0|wumpus|cfields: yes I agree an unexplainable problem is slightly worrying
481 2016-07-28 20:07:02	0|jtimon|it seems it was never the time until now, that it's too late because 8259 needs to be backported but #8346 can't, not your fault...
482 2016-07-28 20:07:20	0|jtimon|should I close #8346 then?
483 2016-07-28 20:07:27	0|jtimon|ping btcdrak
484 2016-07-28 20:08:04	0|NicolasDorier|I don't think it should be closed, we'll just merge later
485 2016-07-28 20:08:11	0|wumpus|jtimon: I agree that it's annoying
486 2016-07-28 20:08:19	0|btcdrak|jtimon: leave it open
487 2016-07-28 20:08:28	0|sipa|jtimon: leave it open
488 2016-07-28 20:08:33	0|jtimon|NicolasDorier: oh, it's not disruptive to your approach, cool
489 2016-07-28 20:08:56	0|wumpus|it has quite a lot of utACKs so just should be merged?
490 2016-07-28 20:09:14	0|NicolasDorier|fine for me
491 2016-07-28 20:09:26	0|jtimon|I thought it would interfere with #8259, sorry
492 2016-07-28 20:09:35	0|sipa|it would, but so be it
493 2016-07-28 20:09:40	0|sipa|sorry for bringing it uo
494 2016-07-28 20:10:05	0|jtimon|never be sorry for bringing things up, that's what I need
495 2016-07-28 20:11:03	0|NicolasDorier|I've rebased 8259 10 times, I guess I'll support an 11 th time :D
496 2016-07-28 20:11:10	0|NicolasDorier|I've nothing to do today anyway :^p
497 2016-07-28 20:11:21	0|jtimon|sipa: if you allow me to be honest, I feel you're some times "too polite" with me as a reviewer. Nits and nacks the sooner the better!
498 2016-07-28 20:11:58	0|jtimon|anyway, I should review 8259 before continuing talking about it, sorry
499 2016-07-28 20:19:15	0|jtimon|NicolasDorier: no harm in squashing #8259 at this point I think?
500 2016-07-28 20:41:37	0|NicolasDorier|jtimon: yes I'll squash. Will do it after changing stuff that need to change because of #8346.
501 2016-07-28 20:41:56	0|NicolasDorier|actually I think that 8346 will make it shorter to review in fact
502 2016-07-28 20:42:31	0|NicolasDorier|because a method which needed the hash cache is not called anymore.
503 2016-07-28 20:45:16	0|jtimon|cool
504 2016-07-28 20:45:47	0|jtimon|keep the old version for the backport though
505 2016-07-28 20:46:48	0|jtimon|rebase -i addict suggestion: and then try to rebase the old version on top of the new one and see what happens
506 2016-07-28 20:47:30	0|jtimon|nothing to commit is complete success
507 2016-07-28 20:48:03	0|jtimon|no, wait, forget the defintion of success, that's wrong
508 2016-07-28 20:48:59	0|NicolasDorier|In fact in https://github.com/bitcoin/bitcoin/pull/8259/files#diff-ca74c4b28865382863b8fe7633a85cd6L740 the hashCacheMap was not even use...
509 2016-07-28 20:49:10	0|NicolasDorier|used
510 2016-07-28 20:49:17	0|NicolasDorier|because it does not run the scripts
511 2016-07-28 20:50:17	0|jtimon|not sure I understand what you just said
512 2016-07-28 20:54:29	0|NicolasDorier|jtimon: in the common piece of code we are touching between #8259 and #8346, I was passing a hashCacheMap as parameter to CheckInput. But it was not even used inside CheckInput because fCheckScript is false.
513 2016-07-28 20:55:20	0|jtimon|oh, yeah, I see, was extending on the comment that it will be shorter for master than for 0.13
514 2016-07-28 20:56:40	0|jtimon|in fact, you can remove the fCheckScript parameter "for free" in the same commit, as now all callers use false. That's for master, not for 0.13
515 2016-07-28 20:57:01	0|jtimon|sorry, now all callers use true
516 2016-07-28 20:58:30	0|NicolasDorier|oh that's cool
517 2016-07-28 20:59:48	0|jtimon|or your life would be easier if you just backported both #8259 and #8346 instead of only one plus the differences of doing it in a different order, and would make people "forwardporting" stuff easier too, but that would be "cheating" by some defintion I am yet to unterstand
518 2016-07-28 21:02:47	0|jtimon|that's ancient coolness that's been ready to pick up for long
519 2016-07-28 21:04:39	0|jtimon|but what I really want to do is always call checkTxInputs way before checkInputs (before noone else calculates txFees in that function [ie ConnectBlock and AcceptToMemoryPool])
520 2016-07-28 21:05:14	0|jtimon|and remove some redundant calculations while helping with encapsulation
521 2016-07-28 21:05:51	0|jtimon|then divide the remaining checkinputs, one multi-thread and one plain simple
522 2016-07-28 21:06:18	0|jtimon|of course there's many ways to do this
523 2016-07-28 21:08:07	0|jtimon|the first version of verifyblock can be single threaded, we don't even have to expose it in libconsensus yet, just having it would be great (we can also expose it in rpc with the storage dependencies I guess, but not sure how useful that would be)
524 2016-07-28 21:10:13	0|jtimon|when we figure out how to expose multi-threading, storage and caches, we can expose verifyblock in libconsensus, but that sounds too long term and that's why I was focusing on maybe exposing verifyheader or verifytx or getflags or whatever was ready first
525 2016-07-28 21:10:30	0|jtimon|but I agree that just complete an internal verifyblock would be great
526 2016-07-28 21:12:26	0|jtimon|and I'm focusing on that now, like you, there's still some stuff I need to move up in my branch because it was about completing verifytx, but it's not needed for completing verifyblock (ie moving things out of contextualcheckblock() )
527 2016-07-28 21:13:50	0|jtimon|btw, for trivial consensus stuff #8413 is also opened
528 2016-07-28 21:17:07	0|bsm117532|ls
529 2016-07-28 21:21:09	0|jtimon|bsm117532:
530 2016-07-28 21:21:11	0|jtimon|drwxrwxr-x 23 jt jt     12288 jul 28 05:27 .
531 2016-07-28 21:21:13	0|jtimon|drwxrwxr-x 12 jt jt      4096 jul 28 05:24 ..
532 2016-07-28 21:28:49	0|bsm117532|[ whereis my brain?
533 2016-07-28 21:44:17	0|jtimon|bsm117532: in your brain and some other parts of your body, but wrong channel again
534 2016-07-28 21:45:21	0|bsm117532|bash: [: missing `]'
535 2016-07-28 22:53:00	0|jtimon|updated #8346
536 2016-07-28 22:58:13	0|cfields|sdaftuar: heh: i just forced a crash in PruneBlockIndexCandidates
537 2016-07-28 22:58:41	0|cfields|not the same crash, but I suspect it's something similar
538 2016-07-28 22:59:24	0|GitHub106|13bitcoin/06master 14d12b732 15Jorge Timón: libconsensus: Expose a flag for BIP112...
539 2016-07-28 22:59:24	0|GitHub106|[13bitcoin] 15sipa pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/133c727cc4f7...ad087638ee48
540 2016-07-28 22:59:25	0|GitHub106|13bitcoin/06master 14ad08763 15Pieter Wuille: Merge #8412: libconsensus: Expose a flag for BIP112...
541 2016-07-28 22:59:34	0|GitHub127|[13bitcoin] 15sipa closed pull request #8412: libconsensus: Expose a flag for BIP112 (06master...060.13-consensus-bip112-flag) 02https://github.com/bitcoin/bitcoin/pull/8412
542 2016-07-28 23:00:51	0|cfields|sdaftuar: http://pastebin.com/raw/CnV5HSUP. ctrl+c during startup.
543 2016-07-28 23:06:51	0|jtimon|illumination moment: there was a translation error in the way I was saying "for free": some times I meant "gratiously" I think
544 2016-07-28 23:14:21	0|jtimon|I wonder why native english speaker don't have some achronym for G.R.A.T.I.S. meaning something completely different yet...
545 2016-07-28 23:14:57	0|GitHub140|[13bitcoin] 15theuni opened pull request #8421: httpserver: drop boost (#8023 dependency) (06master...06http-thread) 02https://github.com/bitcoin/bitcoin/pull/8421
546 2016-07-28 23:19:43	0|GitHub133|13bitcoin/060.13 148360d5b 15Jorge Timón: libconsensus: Expose a flag for BIP112...
547 2016-07-28 23:19:43	0|GitHub133|[13bitcoin] 15sipa pushed 1 new commit to 060.13: 02https://github.com/bitcoin/bitcoin/commit/8360d5b37dd4d8248da0552de40e5ea1d17f51eb
548 2016-07-28 23:20:32	0|sipa|NicolasDorier: i think you misunderstand my suggestion
549 2016-07-28 23:20:48	0|NicolasDorier|mh maybe
550 2016-07-28 23:22:19	0|NicolasDorier|sipa: my point is that lazily calculating the mid state hashes is not worth the added complexity. The only saving is for transaction having no SIGHASH_ALL.
551 2016-07-28 23:22:43	0|NicolasDorier|if a CachedHashes are not read only, then I need to protect read and write with a lock
552 2016-07-28 23:23:02	0|sipa|NicolasDorier: but you solve that by having two CachedHashes
553 2016-07-28 23:23:51	0|sipa|one that's being used inside the computation and modified, and then only copying from the locked map beforehand, and merging back afterwards
554 2016-07-28 23:24:14	0|sipa|your code already does this correctly
555 2016-07-28 23:24:15	0|NicolasDorier|oh I see as part of the TrySet ?
556 2016-07-28 23:24:21	0|sipa|yes
557 2016-07-28 23:24:26	0|sipa|TrySet would merge
558 2016-07-28 23:24:53	0|NicolasDorier|ah ok yes indeed this is simple
559 2016-07-28 23:25:03	0|NicolasDorier|will do that thanks
560 2016-07-28 23:25:31	0|NicolasDorier|I'm a bit too used to everything is a pointer from C# world I forgot I was just using a copy
561 2016-07-28 23:25:53	0|NicolasDorier|for the computation
562 2016-07-28 23:25:57	0|sipa|it also has the advantage of making signature agregation easier later :)
563 2016-07-28 23:27:24	0|NicolasDorier|got it
564 2016-07-28 23:27:38	0|sipa|also, i'd not touch sigcache.h
565 2016-07-28 23:27:44	0|sipa|this is not signature caching
566 2016-07-28 23:29:29	0|NicolasDorier|ok will create a new file for it
567 2016-07-28 23:29:36	0|sipa|thanks
568 2016-07-28 23:47:39	0|jtimon|SIGHASH_ALL is the main.cpp of sighashes
569 2016-07-28 23:49:27	0|jtimon|at some point SIGHASH_ALL should be deprecated, and at some point main.cpp should be non-existent (not probably most users won't agree with this [yet])
570 2016-07-28 23:51:37	0|jtimon|btw, when I talk about "users", the non-existing CPolicy interface is the way I would like to talk with them, with CRandomPolicy on my side to defent some concerns future users may have
571 2016-07-28 23:54:11	0|jtimon|but to be completely honest, when I think about "users" I'm just thinking about me-and-or-potential-future-me's, at least r/btc got that right