1 2016-08-04 01:16:57	0|GitHub184|[13bitcoin] 15sipa opened pull request #8451: Get rid of the const field in CTransaction (06master...06noconsttx) 02https://github.com/bitcoin/bitcoin/pull/8451
  2 2016-08-04 02:23:53	0|GitHub106|[13bitcoin] 15sipa opened pull request #8452: Code simplification: inline CTxInWitness inside CTxIn (06master...06segwitinline) 02https://github.com/bitcoin/bitcoin/pull/8452
  3 2016-08-04 02:59:18	0|phantomcircuit|wumpus, replaced that wallet test with a python one
  4 2016-08-04 02:59:32	0|phantomcircuit|i think it actually covers more of the behavior than it did before but im going to go and check
  5 2016-08-04 03:00:22	0|phantomcircuit|hmm im not testing addmultisigaddress
  6 2016-08-04 03:00:45	0|phantomcircuit|but that's already being tested in other places
  7 2016-08-04 03:01:12	0|phantomcircuit|although not that it works with accounts
  8 2016-08-04 08:50:46	0|wumpus|phantomcircuit: nice!
  9 2016-08-04 08:53:42	0|wumpus|sipa: the last update of secp256k1 has been ~9 months ago (6c527ec), is it maybe time to update the subtree again? At least the (experimental) ARM assembly has been merged since
 10 2016-08-04 08:54:50	0|sipa|wumpus: good idea
 11 2016-08-04 08:55:26	0|sipa|there are certainly no regressions known that would make the current master less appropriate than the subtree in bitcoin currently
 12 2016-08-04 08:56:07	0|wumpus|exactly, I did mean for master, not 0.13
 13 2016-08-04 08:57:06	0|sipa|sure
 14 2016-08-04 09:25:46	0|GitHub30|[13bitcoin] 15laanwj opened pull request #8453: Bring secp256k1 subtree up to date with master (06master...062016_08_update_secp256k1) 02https://github.com/bitcoin/bitcoin/pull/8453
 15 2016-08-04 10:21:19	0|GitHub18|13bitcoin/06master 14122786d 15NicolasDorier: Consensus: Remove ISM
 16 2016-08-04 10:21:19	0|GitHub18|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/5c7a5e1f66d8...37d83bb0a980
 17 2016-08-04 10:21:20	0|GitHub18|13bitcoin/06master 1437d83bb 15Wladimir J. van der Laan: Merge #8391: Consensus: Remove ISM...
 18 2016-08-04 10:21:26	0|GitHub105|[13bitcoin] 15laanwj closed pull request #8391: Consensus: Remove ISM (06master...06remove-ism) 02https://github.com/bitcoin/bitcoin/pull/8391
 19 2016-08-04 10:34:02	0|GitHub46|[13bitcoin] 15laanwj pushed 4 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/37d83bb0a980...f97d335942ae
 20 2016-08-04 10:34:03	0|GitHub46|13bitcoin/06master 140fd2a33 15Pieter Wuille: Use a signal to continue init after genesis activation
 21 2016-08-04 10:34:03	0|GitHub46|13bitcoin/06master 14aa59f2e 15Pieter Wuille: Add extra message to avoid a long 'Loading banlist'
 22 2016-08-04 10:34:04	0|GitHub46|13bitcoin/06master 149d4eb9a 15Pieter Wuille: Do diskspace check before import thread is started
 23 2016-08-04 10:34:11	0|GitHub154|[13bitcoin] 15laanwj closed pull request #8392: Fix several node initialization issues (06master...06fixactivatewait) 02https://github.com/bitcoin/bitcoin/pull/8392
 24 2016-08-04 10:53:32	0|paveljanik|wumpus, do you use gmp? string.h is included in num_gmp_impl.h... This could be difference.
 25 2016-08-04 10:54:58	0|wumpus|yes, that could be it
 26 2016-08-04 10:55:45	0|wumpus|will try with bignum=no
 27 2016-08-04 10:55:59	0|wumpus|yes, that does it
 28 2016-08-04 11:02:14	0|wumpus|paveljanik: https://github.com/bitcoin-core/secp256k1/pull/410
 29 2016-08-04 11:17:21	0|paveljanik|thanks!
 30 2016-08-04 11:31:31	0|GitHub189|13bitcoin/06master 142c517b3 15Suhas Daftuar: Fix p2p-feefilter.py for changed tx relay behavior
 31 2016-08-04 11:31:31	0|GitHub189|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/f97d335942ae...6e6ab2c32382
 32 2016-08-04 11:31:32	0|GitHub189|13bitcoin/06master 146e6ab2c 15Wladimir J. van der Laan: Merge #8444: Fix p2p-feefilter.py for changed tx relay behavior...
 33 2016-08-04 11:31:41	0|GitHub87|[13bitcoin] 15laanwj closed pull request #8444: Fix p2p-feefilter.py for changed tx relay behavior (06master...06fix-feefilter-test) 02https://github.com/bitcoin/bitcoin/pull/8444
 34 2016-08-04 11:35:06	0|paveljanik|wumpus, in fact, the fix is not upstream, but side-stream ;-)
 35 2016-08-04 11:35:43	0|wumpus|well I hope it can be merged soon, I'll update #8453 after that
 36 2016-08-04 11:36:20	0|wumpus|bah I'd really feel better without expat in the depends :)
 37 2016-08-04 12:26:08	0|GitHub129|[13bitcoin] 15MarcoFalke opened pull request #8454: [0.13.1] Fix p2p-feefilter.py for changed tx relay behavior (060.13...06Mf1608-qaFeeFilter013) 02https://github.com/bitcoin/bitcoin/pull/8454
 38 2016-08-04 12:40:45	0|GitHub71|13bitcoin/060.13 14d485a6c 15Wladimir J. van der Laan: doc: Add list of new and removed RPC commands to release notes...
 39 2016-08-04 12:40:45	0|GitHub71|[13bitcoin] 15laanwj pushed 1 new commit to 060.13: 02https://github.com/bitcoin/bitcoin/commit/d485a6c5a8d238cac5ad732ce9e744f4b121143c
 40 2016-08-04 12:41:43	0|wumpus|sipa: do you still plan on writing something for the release notes about -reindex-chainstate?
 41 2016-08-04 12:45:55	0|wumpus|I've added a list of added and removed and changed RPC calls, which means that #7678 can almost be closed, leaving -maxtimeadjust and -reindex-chainstate, of which I think reindex-chainstate (and the changed reindexing in general) is most relevant
 42 2016-08-04 13:52:39	0|GitHub198|[13bitcoin] 15mrbandrews opened pull request #8456: [RPC] Simplified bumpfee command. (06master...06ba-rpcbumpfee) 02https://github.com/bitcoin/bitcoin/pull/8456
 43 2016-08-04 16:24:53	0|GitHub160|[13bitcoin] 15pedrobranco opened pull request #8457: Add block height support in rpc call getblock (06master...06feature/add-get-block-by-number) 02https://github.com/bitcoin/bitcoin/pull/8457
 44 2016-08-04 16:45:55	0|sipa|wumpus: ack, will write
 45 2016-08-04 18:08:13	0|instagibbs|during salvage on an unbroken wallet.dat file I'm seeing keys getting skipped
 46 2016-08-04 18:08:17	0|instagibbs|on master
 47 2016-08-04 18:08:49	0|instagibbs|0-4 keys on a fresh wallet with keypool 100, randomly
 48 2016-08-04 18:09:21	0|gmaxwell|I've said before that salvagewallet should be called "savagewallet".
 49 2016-08-04 18:09:29	0|gmaxwell|instagibbs: can you figure out why?
 50 2016-08-04 18:09:40	0|instagibbs|no ive been trying to chase it down all day
 51 2016-08-04 18:10:19	0|instagibbs|it's very random, and only happens once every 25+ keys
 52 2016-08-04 18:10:52	0|gmaxwell|presumably you've filled up the salvage code with instrumentation to see where they're getting dropped? (e.g. does bdb just never hand them to us?)
 53 2016-08-04 18:11:11	0|instagibbs|it's an error thrown when doing >> for private key
 54 2016-08-04 18:11:23	0|instagibbs|"key" type, trying to deserialize the key
 55 2016-08-04 18:11:27	0|instagibbs|priv*
 56 2016-08-04 18:11:33	0|sipa|is it trying to deserialize master key data as the wrong type?
 57 2016-08-04 18:13:48	0|instagibbs|I don't think so, as keypool=1 almost never has it happen
 58 2016-08-04 18:14:07	0|instagibbs|I've never had it happen with keypool 1, that is to say
 59 2016-08-04 18:22:07	0|wumpus|instagibbs: can you upload the wallet somewhere that it happens with?
 60 2016-08-04 18:23:15	0|instagibbs|it's literally a fresh wallet, start up bitcoind in regtest, stop, start back up with salvagewallet. Some not small % of time i get those messages
 61 2016-08-04 18:23:25	0|instagibbs|but yes I can upload one
 62 2016-08-04 18:23:28	0|wumpus|it's a known issue btw: salvagewallet drops valid keys #7379   but I've never had it happen to me
 63 2016-08-04 18:24:10	0|wumpus|another similar problem is salvagewallet fails verification #7463 , salvagewallet on an uncorrupted wallet sometimes makes bdb return errors
 64 2016-08-04 18:24:35	0|wumpus|so what you are seeing may be similar, I noticed that problem, just never saw keys being dropped
 65 2016-08-04 18:25:04	0|instagibbs|2016-08-04 18:07:42 Salvage(aggressive) found 439 records
 66 2016-08-04 18:25:04	0|instagibbs|2016-08-04 18:07:42 WARNING: CWalletDB::Recover skipping key:
 67 2016-08-04 18:25:08	0|wumpus|I would not be surprised if bdb's "aggressive" salvage sometimes returns crap or misses records
 68 2016-08-04 18:25:35	0|instagibbs|oh is it an actual bdb mode? That might explain the difference
 69 2016-08-04 18:27:03	0|gmaxwell|maybe it should run twice, once normally without any bdb magig, and then again in agressive mode?
 70 2016-08-04 18:27:25	0|gmaxwell|"never do worse than simply reading it."
 71 2016-08-04 18:27:28	0|wumpus|I don't know if that makes a difference, but worth a try
 72 2016-08-04 18:27:44	0|instagibbs|i can try that, if i figure out the settings
 73 2016-08-04 18:27:56	0|gmaxwell|well presumably it must, after all we don't miss those keys during normal reads without salvaging.
 74 2016-08-04 18:29:09	0|wumpus|ooh! those may be ghost keys instagibbs
 75 2016-08-04 18:29:13	0|wumpus|if the file being salvaged is in no way corrupt), and the output will almost certainly require editing before being loaded into a database."
 76 2016-08-04 18:29:13	0|wumpus|"Output all the key/data pairs in the file that can be found. By default, DB->verify() does not assume corruption. For example, if a key/data pair on a page is marked as deleted, it is not then written to the output file. When DB_AGGRESSIVE is specified, corruption is assumed, and any key/data pair that can be found is written. In this case, key/data pairs that are corrupted or have been deleted may appear in the output (even
 77 2016-08-04 18:29:34	0|instagibbs|i aint afraid of no ghost
 78 2016-08-04 18:29:35	0|wumpus|aggressive should return *more* records, but some of them may be deleted, or not really exist at all
 79 2016-08-04 18:29:55	0|cfields|sipa: mm, I just noticed that 0.13 doesn't enable asm for osx due to secp256k1 #373 :\
 80 2016-08-04 18:30:45	0|wumpus|*even if the file being salvaged is in no way corrupt* is the important part there
 81 2016-08-04 18:30:56	0|instagibbs|so how can we test that
 82 2016-08-04 18:31:04	0|wumpus|so yes, trying to salvage without aggressive makes sense first
 83 2016-08-04 18:32:21	0|wumpus|but how can we detect if keys have been dropped due to corruption, which would appear with salvage aggressive? I don't know
 84 2016-08-04 18:32:42	0|wumpus|yes you don't really know, that's the problem
 85 2016-08-04 18:32:54	0|wumpus|was it a real key or just a corrupted echo?
 86 2016-08-04 18:33:47	0|wumpus|cfields: maybe do the secp256k1 update for 0.13.1 too then
 87 2016-08-04 18:34:03	0|instagibbs|so worst case with "ghost" keys would be having keys you didn't actually generate? or?
 88 2016-08-04 18:34:17	0|wumpus|instagibbs: or partial old records
 89 2016-08-04 18:34:26	0|cfields|wumpus: probably makes sense, yes. I'm running a bench with/without atm.
 90 2016-08-04 18:34:53	0|wumpus|instagibbs: it does a linear scan over the file and everything that looks like a key/value is returned
 91 2016-08-04 18:35:18	0|instagibbs|so aggressive will indeed return more, but we toss anything that looks wrong
 92 2016-08-04 18:35:21	0|paveljanik|Trivial -Wshadow PRs for review: 8109, 8163, 8191, 8449. I'd be glad to get some ACKs. Thank you.
 93 2016-08-04 18:35:59	0|wumpus|instagibbs: yes, we do, we're fairly robust there. But it means possible false positives inthe log
 94 2016-08-04 18:36:13	0|instagibbs|wumpus, ok, well that's less scary, thanks
 95 2016-08-04 18:36:47	0|wumpus|instagibbs: I hope that's it! but you can only find out by analysing that wallet\
 96 2016-08-04 18:37:08	0|wumpus|see if the keys that are in the original wallet and post-salvage wallet are the same, despite the WARNING
 97 2016-08-04 18:37:50	0|sipa|wumpus: i'll merge the strings include in secp
 98 2016-08-04 18:37:59	0|wumpus|sipa: thanks
 99 2016-08-04 18:38:08	0|sipa|so that can be included in the subtree
100 2016-08-04 18:38:14	0|wumpus|right, I'll update the pr
101 2016-08-04 18:38:38	0|cfields|116us vs 124us. not the end of the world.
102 2016-08-04 18:39:01	0|wumpus|don't think that'd be worth it for any random warning, but missing prototypes are pretty serious business
103 2016-08-04 18:40:02	0|sipa|cfields: that's the output of bench_verify ?
104 2016-08-04 18:40:22	0|gmaxwell|cfields: I'm surprised by that.
105 2016-08-04 18:40:30	0|gmaxwell|the difference is smaller than I expected.
106 2016-08-04 18:40:30	0|sipa|likewise
107 2016-08-04 18:40:31	0|cfields|sipa: yes
108 2016-08-04 18:41:44	0|sipa|what compilation flags?
109 2016-08-04 18:41:58	0|cfields|sipa: default passed down from bitcoin
110 2016-08-04 18:42:38	0|wumpus|116 versus 124 seems in the error margin, are you sure something changed at all?
111 2016-08-04 18:42:42	0|cfields|sipa: http://pastebin.com/raw/VbmBERA8
112 2016-08-04 18:43:19	0|cfields|ran a few times of each, didn't deviate much. double-checking.
113 2016-08-04 18:43:23	0|wumpus|okay
114 2016-08-04 18:44:54	0|gmaxwell|cfields: what cpu is this?
115 2016-08-04 18:46:38	0|cfields|gmaxwell: i5, 2.4ghz. there's no /proc/cpuinfo in osx, i can dig around and find it if you'd like the gen
116 2016-08-04 18:48:04	0|sipa|cfields: reproduced
117 2016-08-04 18:48:13	0|cfields|there we go. 4258U. haswell.
118 2016-08-04 18:48:36	0|sipa|112us vs 117us here
119 2016-08-04 18:48:47	0|sipa|ecdsa_verify: min 117us / avg 117us / max 117us
120 2016-08-04 18:48:55	0|sipa|ecdsa_verify: min 112us / avg 113us / max 113us
121 2016-08-04 18:49:11	0|jonasschnelli|cfields: OSX: use "sysctl -n machdep.cpu.brand_string" instead cat /proc/cpuinfo
122 2016-08-04 18:49:15	0|cfields|sipa: in linux? or reproduced on mac?
123 2016-08-04 18:49:24	0|sipa|linux
124 2016-08-04 18:49:36	0|sipa|with cpu clock pinned to a single frequency
125 2016-08-04 18:49:45	0|GitHub127|13bitcoin/060.13 14cd0910b 15Suhas Daftuar: Fix p2p-feefilter.py for changed tx relay behavior...
126 2016-08-04 18:49:45	0|GitHub127|[13bitcoin] 15MarcoFalke pushed 2 new commits to 060.13: 02https://github.com/bitcoin/bitcoin/compare/d485a6c5a8d2...114f7e944b1c
127 2016-08-04 18:49:46	0|GitHub127|13bitcoin/060.13 14114f7e9 15MarcoFalke: Merge #8454: [0.13.1] Fix p2p-feefilter.py for changed tx relay behavior...
128 2016-08-04 18:49:46	0|GitHub182|[13bitcoin] 15MarcoFalke closed pull request #8454: [0.13.1] Fix p2p-feefilter.py for changed tx relay behavior (060.13...06Mf1608-qaFeeFilter013) 02https://github.com/bitcoin/bitcoin/pull/8454
129 2016-08-04 18:50:01	0|cfields|huh.
130 2016-08-04 18:50:02	0|gmaxwell|sipa: that looks broken, those numbers are much higher than I expect.
131 2016-08-04 18:50:09	0|gmaxwell|(for both of you)
132 2016-08-04 18:50:28	0|sipa|that's without gmp or endomorphism, at 2.6 GHz
133 2016-08-04 18:50:33	0|cfields|gmaxwell: remember no bignum or endo
134 2016-08-04 18:50:37	0|gmaxwell|I normally expect a verify to be more like 70us. but maybe thats all gmp.
135 2016-08-04 18:51:43	0|cfields|jonasschnelli: thanks
136 2016-08-04 18:52:03	0|wumpus|MarcoFalke: please don't merge anything for 0.13 now, we have to decide on the meeting whether to release final, this is a test change but I don't want to have to trigger a new rc without good reason
137 2016-08-04 18:52:29	0|MarcoFalke|This should not be enough for a new rc
138 2016-08-04 18:52:57	0|wumpus|I know, but you named it 0.13.1, and it's not time to merge for 0.13.1 yet, just clarifying
139 2016-08-04 18:53:10	0|MarcoFalke|oops
140 2016-08-04 18:53:35	0|gmaxwell|(in libsecp256k1)
141 2016-08-04 18:53:37	0|wumpus|(people may still want to do e.g. changelog changes before tagging final)
142 2016-08-04 18:53:44	0|gmaxwell|cd ~
143 2016-08-04 18:54:07	0|cfields|gmaxwell: for which?
144 2016-08-04 18:54:11	0|wumpus|bitcoin is always without gmp
145 2016-08-04 18:54:20	0|sipa|gmaxwell: sounds likely... with endo and gmp enabled is see no statistically significant performance difference between asm enabled and disabled
146 2016-08-04 18:54:44	0|sipa|71.2us vs 71.4us
147 2016-08-04 18:55:02	0|sipa|(with variance between tests exceeding 0.2us)
148 2016-08-04 18:57:32	0|gmaxwell|Aside, I think in the GUI and logs we should display the verification performance... so people will better realize how radically difference in speed different machines are. :)
149 2016-08-04 18:58:22	0|wumpus|yes, a graph of verification performance in the gui would be nice
150 2016-08-04 18:59:02	0|wumpus|similar to the bandwidth one
151 2016-08-04 18:59:19	0|wumpus|with part spent in i/o secp256k1 etc
152 2016-08-04 18:59:20	0|jonasschnelli|heh.. currently working on a mempool graph
153 2016-08-04 18:59:45	0|wumpus|nice
154 2016-08-04 19:00:10	0|jonasschnelli|http://i.imgur.com/G6yi9UR.png (very basic atm)
155 2016-08-04 19:00:14	0|gmaxwell|wumpus: Well I meant as a static this computer x faster than that computer. not usage.
156 2016-08-04 19:00:24	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
157 2016-08-04 19:00:43	0|lightningbot|Meeting started Thu Aug  4 19:00:43 2016 UTC.  The chair is wumpus. Information about MeetBot at http://wiki.debian.org/MeetBot.
158 2016-08-04 19:00:43	0|lightningbot|Useful Commands: #action #agreed #help #info #idea #link #topic.
159 2016-08-04 19:00:43	0|wumpus|#startmeeting
160 2016-08-04 19:01:01	0|cfields|gmaxwell: mm, i just stuck some "#error foo" in the asm paths, it blows up as expected.
161 2016-08-04 19:01:06	0|cfields|whoops, will continue after meeting
162 2016-08-04 19:01:16	0|MarcoFalke|topic 0.13.0, I guess?
163 2016-08-04 19:01:23	0|wumpus|#topic 0.13.0 final?
164 2016-08-04 19:01:34	0|wumpus|did anyone hear of any critical problems with rc2?
165 2016-08-04 19:01:55	0|gmaxwell|I haven't heard much on it. Been quiet.
166 2016-08-04 19:02:03	0|sipa|we forgot to backport 8438/8365 into 0.13...
167 2016-08-04 19:02:16	0|wumpus|sipa something for 0.13.1?
168 2016-08-04 19:02:29	0|kanzure|hi. greetings.
169 2016-08-04 19:02:39	0|luke-jr|wumpus: the release notes are still inappropriate AFAIK
170 2016-08-04 19:03:08	0|luke-jr|also gmaxwell was going to add a section I believe, covering the new relay policy influences (maybe I missed that being added)
171 2016-08-04 19:03:12	0|gmaxwell|I think I owed some release note text which I haven't done yet re max blocksize settings.
172 2016-08-04 19:03:12	0|jeremyrubin|hi
173 2016-08-04 19:03:12	0|kanzure|this isn't short-term development related but it's high signal-noise crypto content that a few developers recently participated in, http://diyhpl.us/wiki/transcripts/2016-july-bitcoin-developers-miners-meeting/dan-boneh/
174 2016-08-04 19:03:16	0|MarcoFalke|Wouldn't 8438 require rc3?
175 2016-08-04 19:03:17	0|wumpus|luke-jr: then update them, sipa also still wants to add something
176 2016-08-04 19:03:22	0|wumpus|MarcoFalke: yes
177 2016-08-04 19:03:29	0|luke-jr|wumpus: my PR to update them is still open.
178 2016-08-04 19:03:35	0|wumpus|luke-jr: it also updates code
179 2016-08-04 19:03:43	0|jonasschnelli|luke-jr: does the PR still contain code changes?
180 2016-08-04 19:03:47	0|luke-jr|so you want a version without code changes?
181 2016-08-04 19:04:06	0|wumpus|luke-jr: if you make a PR that just updates the changelog, and it's accepted by others, it would have been merged a long time ago
182 2016-08-04 19:04:48	0|luke-jr|wumpus: the current language was not accepted.
183 2016-08-04 19:05:00	0|jonasschnelli|sipa: do you think https://github.com/bitcoin/bitcoin/pull/8438 can wait for 0.13.1?
184 2016-08-04 19:05:06	0|luke-jr|I find this double-standard somewhat annoying.
185 2016-08-04 19:05:12	0|morcos|sigh, if this is another meeting of us all arguing against luke-jr then i'm going to find something else to do
186 2016-08-04 19:05:29	0|jonasschnelli|heh. Yes. Finish this. luke-jr can open a pr (action)
187 2016-08-04 19:05:40	0|wumpus|I'm not arguing against luke-jr
188 2016-08-04 19:06:09	0|wumpus|anyhow that derailed my question - so no one had any critical problems with 0.13.0rc2?
189 2016-08-04 19:06:22	0|wumpus|and I don't mean with the release notes
190 2016-08-04 19:06:31	0|sipa|is there any fear of instagibbs' problem being due to a code error?
191 2016-08-04 19:06:38	0|wumpus|what problem?
192 2016-08-04 19:06:42	0|instagibbs|sipa, fwiw it's showing up on older fork...
193 2016-08-04 19:06:58	0|instagibbs|it's not HD-related, or anything fairly recent
194 2016-08-04 19:07:00	0|gmaxwell|instagibbs: so without the bip32 changes?
195 2016-08-04 19:07:04	0|instagibbs|correct
196 2016-08-04 19:07:05	0|wumpus|you mean the salvagewallet problem? that's old hat, there's two issues open with salvagewallet problems
197 2016-08-04 19:07:10	0|sipa|instagibbs: ok, good to know
198 2016-08-04 19:07:11	0|wumpus|as I mentioned
199 2016-08-04 19:07:19	0|sipa|sorry, i didn't follow the whole discussion
200 2016-08-04 19:07:20	0|wumpus|this is not a last minute problem
201 2016-08-04 19:07:21	0|luke-jr|wumpus: earlier we were discussing 0.13.0's failure mode downgrading from 0.13.1; I am not clear what the status of that is, or if we need changes for it
202 2016-08-04 19:07:23	0|instagibbs|yes I'll double-check that keys aren't actually going away
203 2016-08-04 19:07:26	0|instagibbs|but not worried now
204 2016-08-04 19:07:31	0|gmaxwell|OK.
205 2016-08-04 19:07:36	0|instagibbs|LoadWallet works anyhoo :P
206 2016-08-04 19:07:40	0|wumpus|yes
207 2016-08-04 19:08:27	0|wumpus|so, all agree that it is time to release 0.13.0 final?
208 2016-08-04 19:08:35	0|wumpus|(after say, a day of updating the release notes)
209 2016-08-04 19:09:07	0|sipa|do we care about what happens when someone downgrades 0.13.1 to 0.13.0 after segwit activates?
210 2016-08-04 19:09:09	0|jonasschnelli|ACK, I can live with https://github.com/bitcoin/bitcoin/pull/8438  for 0.13.1 but don't know about others opinions.
211 2016-08-04 19:09:10	0|sdaftuar|i think we can just tell users who want to downgrade to 0.13.0 after segwit activates to do a reindex?
212 2016-08-04 19:09:27	0|sdaftuar|they'll end up redownloading blocks, without witnesses, but that seems fine
213 2016-08-04 19:09:30	0|luke-jr|sipa: I care only to the extent that it shouldn't make a "working" node that doesn't match consensus
214 2016-08-04 19:09:32	0|sdaftuar|kind of a weird case to try to optimize
215 2016-08-04 19:09:45	0|wumpus|agree with luke-jr - it should give a clear error
216 2016-08-04 19:09:49	0|luke-jr|ie, it should give a hard error or reindex or something
217 2016-08-04 19:09:55	0|wumpus|it should not seem to work, but if it requires a reindex that's fine
218 2016-08-04 19:10:04	0|jonasschnelli|sdaftuar: Yes. I think this would be good. I don't think we need to cover the downgrade case.
219 2016-08-04 19:10:05	0|sipa|luke-jr: i think the only risk (but i haven't tried) is that it either works fine or crashes
220 2016-08-04 19:10:11	0|gmaxwell|Loss of #8438 is unfortunate but if including it would mean delaying the release I don't think it would be good to do so.
221 2016-08-04 19:10:15	0|sipa|luke-jr: the UTXO set is identical across the two
222 2016-08-04 19:10:42	0|luke-jr|I suggest someone should try it before final 0.13.0
223 2016-08-04 19:10:53	0|luke-jr|probably not difficult using testnet?
224 2016-08-04 19:10:56	0|sdaftuar|sipa: i guess its somewhat unfortunate that it will seem to work fine.
225 2016-08-04 19:10:57	0|sipa|luke-jr: ack
226 2016-08-04 19:11:00	0|gmaxwell|it can be tested by changing the testnet parameters.
227 2016-08-04 19:11:11	0|sipa|detecting this situation is not hard, btw
228 2016-08-04 19:11:24	0|sdaftuar|oh because 0.13.1 will use OPT_WITNESS?
229 2016-08-04 19:11:32	0|sipa|sdaftuar: yes
230 2016-08-04 19:12:03	0|sdaftuar|hm so maybe we should add code then to look for that, hmm
231 2016-08-04 19:12:17	0|sipa|it's very last minute though...
232 2016-08-04 19:12:24	0|sdaftuar|i agree. and seemingly not that important
233 2016-08-04 19:12:26	0|sipa|i wish we thought of this before
234 2016-08-04 19:12:35	0|sipa|but i don't think it's worth holding up the release for
235 2016-08-04 19:12:56	0|luke-jr|it's trivial to test I think; if everyone else is too busy, I will try to do it today
236 2016-08-04 19:13:02	0|wumpus|gmaxwell: it would mean doing another rc, we could do final next week
237 2016-08-04 19:13:58	0|gmaxwell|that woudl also allow adding segwit downgrade protection sipa is currently discussing. (should be 3loc or so)
238 2016-08-04 19:14:11	0|wumpus|I don't know if it is that critical though, at some point we should just make the cut and stop slipping
239 2016-08-04 19:14:32	0|wumpus|yes that is true
240 2016-08-04 19:15:36	0|sipa|ideally deadlines don't slip, but iirc we're much more on track than for earlier major releases
241 2016-08-04 19:15:37	0|jonasschnelli|Yes. Maybe https://github.com/bitcoin/bitcoin/pull/8438  + SW0.13 downgrade compatibility is worth a week delay
242 2016-08-04 19:15:40	0|wumpus|yes, we've caught up a bit with the rc1 slip by doing a fast rc phase
243 2016-08-04 19:15:48	0|GitHub107|[13bitcoin] 15luke-jr opened pull request #8458: [0.13] release notes: remove bad advice to switch to blockmaxweight prematurely (06master...060.13_relnotes_remove_bad_advice) 02https://github.com/bitcoin/bitcoin/pull/8458
244 2016-08-04 19:15:51	0|btcdrak|0.13.1 shouldnt be too far behind at least, but my preference is to include it and do another rc
245 2016-08-04 19:15:54	0|wumpus|note that I planned amonth for the rc1 phase
246 2016-08-04 19:15:59	0|wumpus|eh for the rc phase
247 2016-08-04 19:16:31	0|wumpus|well the downgrade protection cna't be postponed to 0.13.1
248 2016-08-04 19:16:34	0|wumpus|that'd defeat the point
249 2016-08-04 19:16:38	0|wumpus|otherwise that'd be my preference
250 2016-08-04 19:16:46	0|luke-jr|8458 also mentions an idea I had for a 3-liner to make blockmaxsize perform identical to blockmaxweight, if we do another rc.. if that's desired, someone please say so and I'll do it now
251 2016-08-04 19:17:01	0|sipa|luke-jr: i think you included a few commits too many
252 2016-08-04 19:17:01	0|wumpus|yes now everyone starts with last minute ideas
253 2016-08-04 19:17:02	0|wumpus|great
254 2016-08-04 19:17:11	0|luke-jr|oh crap, I made it against master
255 2016-08-04 19:17:34	0|GitHub168|[13bitcoin] 15luke-jr closed pull request #8458: [0.13] release notes: remove bad advice to switch to blockmaxweight prematurely (06master...060.13_relnotes_remove_bad_advice) 02https://github.com/bitcoin/bitcoin/pull/8458
256 2016-08-04 19:17:39	0|gmaxwell|luke-jr: nah, something changing transaction selection.. not a good thing now.
257 2016-08-04 19:17:41	0|luke-jr|wumpus: well, it's trivial and probably unnecessary; fine if we don't do it
258 2016-08-04 19:18:28	0|wumpus|#action check if downgrade protection is really needed, or whether it already fails
259 2016-08-04 19:18:53	0|GitHub10|[13bitcoin] 15luke-jr opened pull request #8459: [0.13] release-notes: Do not encourage changing blockmaxsize to blockmaxweight (060.13...060.13_relnotes_remove_bad_advice) 02https://github.com/bitcoin/bitcoin/pull/8459
260 2016-08-04 19:19:40	0|wumpus|other topics?
261 2016-08-04 19:19:59	0|cfields|oh
262 2016-08-04 19:20:29	0|sipa|segwit mempool malleability dos (#8279)
263 2016-08-04 19:20:33	0|cfields|(sec, someone else feel free to go ahead)
264 2016-08-04 19:20:52	0|wumpus|#topic segwit mempool malleability dos
265 2016-08-04 19:21:51	0|wumpus|wasn't that supposed to be solved in #8312?
266 2016-08-04 19:21:57	0|sdaftuar|no, it was only solved for 0.13.0
267 2016-08-04 19:22:05	0|sdaftuar|we need to decide what to do in our first segwit release
268 2016-08-04 19:22:08	0|sipa|so i suggested removing the "transaction failure purely due to witness does not cause rejection cache entry"
269 2016-08-04 19:22:43	0|sipa|with the rationale that all it does it prevent an attacker from hiding a valid transaction from you in some cases
270 2016-08-04 19:23:04	0|sipa|but it doesn't prevent it entirely (they can announce and just never send the transaction)
271 2016-08-04 19:23:35	0|sipa|sdaftuar notes that the mempool malleability attack only needs a short lived connection, while the never send tx data attack needs a persistent one
272 2016-08-04 19:23:53	0|sdaftuar|and it needs more than one persistent one -- you need several, as we retry tx download from other peers
273 2016-08-04 19:24:19	0|sipa|sdaftuar: rejection cache is also temporary, but a node won't just re-request...
274 2016-08-04 19:24:32	0|morcos|i think i'm coming around to the idea of full verification of all txs
275 2016-08-04 19:24:40	0|gmaxwell|morcos: hah
276 2016-08-04 19:24:51	0|BlueMatt|was there rationale to inv'ing with txid instead of wtxid for segwit nodes?
277 2016-08-04 19:25:31	0|BlueMatt|(just noting that wtxid would be Keep The Same Behavior As Pre SegWit (tm))
278 2016-08-04 19:26:38	0|sipa|BlueMatt: duplicating a lot of logic (mempool, orphan, caches, ...), and causes at least a potential doubling anyway (you could be inved the same tx from a pre-segwit and post-segwit node once with txid and once with wtxid, without being able to tell they're the same)
279 2016-08-04 19:27:24	0|gmaxwell|also in the long term, the high malleability of witnesses would let an attacker waste lots of bandwidth. with nodes relaying different witness versions of the same txn among each other.
280 2016-08-04 19:27:43	0|BlueMatt|sipa: if you're inved a tx with a witness from a pre-segwit node we get a double anyway since its garbage to us
281 2016-08-04 19:28:19	0|BlueMatt|gmaxwell: same is true for scriptSigs...up to IsStandard limits (which could, in fact, be more easily enforced on witnesses)
282 2016-08-04 19:28:22	0|sdaftuar|BlueMatt: that's true but shouldn't happen, as pre-segwit nodes shouldn't be accepting segwit transactions unless they're running with an unusual policy
283 2016-08-04 19:28:30	0|sipa|also, if you have a tx with malleable witness (say, op_drop argument), nodes on the network could modify the witness without invalidating, and you wouldn't even be able to tell you already had the transaction before download
284 2016-08-04 19:28:59	0|sipa|and you couldn't ban them for it, because they're all valid transactions
285 2016-08-04 19:30:01	0|gmaxwell|BlueMatt: the kind of isstandard restriction against malleability only works great for limited classes of transactions, at least in the long term that isn't great.
286 2016-08-04 19:30:10	0|BlueMatt|I'm aware
287 2016-08-04 19:30:18	0|sipa|a solution is inving with both txid and wtxid... but if we go that way, we should also adds resource information to all invs (fees, size, sigops, ...)
288 2016-08-04 19:30:35	0|BlueMatt|sipa: I was about to say that...seems like a real solution is inving both...
289 2016-08-04 19:30:42	0|morcos|aren't we approachign the size of the tx at that point
290 2016-08-04 19:31:04	0|sipa|morcos: certainly within an order of magnitude
291 2016-08-04 19:31:08	0|BlueMatt|gmaxwell: but IsStandard is allowed to serve the same purpose as always - enforce reasonable limits to ensure code sanity until we've explored edge cases thouroughly
292 2016-08-04 19:31:20	0|gmaxwell|the txid is within an order of magnitude. :P
293 2016-08-04 19:31:31	0|sdaftuar|sipa: i think we might we just end up overfitting current policy rules by adding resource information
294 2016-08-04 19:31:32	0|gmaxwell|(for the median size)
295 2016-08-04 19:31:51	0|gmaxwell|but future mempool sync these sizes will matter less.
296 2016-08-04 19:31:51	0|sipa|sdaftuar: 'overfitting' ?
297 2016-08-04 19:32:03	0|gmaxwell|sdaftuar: feerate is pretty fundimental, however.
298 2016-08-04 19:32:05	0|sdaftuar|ie policy will change, and the information will be insufficient
299 2016-08-04 19:32:08	0|gmaxwell|I think including sigops would be dumb.
300 2016-08-04 19:32:22	0|sdaftuar|gmaxwell: yes, but already handled with less bandwidth by feefilter
301 2016-08-04 19:32:29	0|gmaxwell|if sigops really matter, something is wrong.
302 2016-08-04 19:32:49	0|gmaxwell|sdaftuar: indeed. feefilter makes it implicit.
303 2016-08-04 19:32:54	0|sipa|well, ok... we could send size and fee
304 2016-08-04 19:33:05	0|sipa|but that's not much better than making feefilter mandatory
305 2016-08-04 19:33:13	0|sipa|except slightly more flexible
306 2016-08-04 19:33:20	0|sipa|and sending two hashes is annoying
307 2016-08-04 19:33:24	0|gmaxwell|we're in the weeds for this right now.
308 2016-08-04 19:33:32	0|sipa|agree
309 2016-08-04 19:33:45	0|sipa|i think there is no simple clear cut solution for this
310 2016-08-04 19:34:08	0|gmaxwell|if we're doing set recon it doesn't really matter that much how long the identifiers are.
311 2016-08-04 19:34:16	0|sipa|but that's much further out
312 2016-08-04 19:34:20	0|BlueMatt|if inv'ing both hashes werent stupid for bandwidth, Id say that was a pretty great solution
313 2016-08-04 19:34:30	0|BlueMatt|true
314 2016-08-04 19:34:58	0|gmaxwell|sipa: depends on how long it takes me to convince you to implement the relevant number theory code. :P
315 2016-08-04 19:35:52	0|gmaxwell|In any case, the witnessID really doesn't matter for much of anything except rejection here.
316 2016-08-04 19:36:00	0|sipa|alternative idea: make feefilter mandatory for segwit, and just disable rejectioncache...
317 2016-08-04 19:36:16	0|sipa|rejectioncache only helps in practice against repeated announcements of transactions below your threshold
318 2016-08-04 19:36:29	0|sipa|it's not a protection against attacks
319 2016-08-04 19:38:01	0|sdaftuar|hm, not a bad idea
320 2016-08-04 19:38:03	0|luke-jr|feefilter isn't even used by default last I knew? (because there is no min fee until the mempool fills)
321 2016-08-04 19:38:16	0|sipa|luke-jr: good point
322 2016-08-04 19:38:40	0|BlueMatt|damn, mem^H^H^Hdiskpool was too late :(
323 2016-08-04 19:38:47	0|morcos|i'm all for making fee filter mandatory, if i'd known people would have liked it this much, we should have designed it that way from teh beginning
324 2016-08-04 19:39:07	0|morcos|i'm a bit worried about removing the rejection cache entirely
325 2016-08-04 19:39:16	0|gmaxwell|luke-jr: there is a minrelayfee.
326 2016-08-04 19:39:27	0|gmaxwell|Do we not feefilter on that?
327 2016-08-04 19:39:29	0|sdaftuar|gmaxwell: but we let in free transactions still
328 2016-08-04 19:39:35	0|gmaxwell|oh right.
329 2016-08-04 19:39:58	0|morcos|anytime there is any policy difference between nodes (such as a change in minrelay fee change isDust) then you get txs bouncing aroudn the network between the sets of nodes with different policies
330 2016-08-04 19:40:06	0|gmaxwell|full validation and distinguishing why rejection happened would help.
331 2016-08-04 19:40:11	0|morcos|i think its nice to have a sort of protection against that
332 2016-08-04 19:42:13	0|gmaxwell|we could just have one rejection filter per peer type.
333 2016-08-04 19:42:25	0|gmaxwell|e.g. rejection filter for non-sw peers and a rejection filter for sw peers.
334 2016-08-04 19:43:04	0|sdaftuar|with sw peers using wtxid invs?
335 2016-08-04 19:43:30	0|sipa|sdaftuar: still not enough... you need to be able to tell whether you already have another version of the same tx
336 2016-08-04 19:43:36	0|sipa|even with only sw peers
337 2016-08-04 19:43:59	0|gmaxwell|+full validation.
338 2016-08-04 19:44:45	0|sdaftuar|sipa: i don't see why that's needed, any more than we have today with not knowing a tx is a double-spend before processing
339 2016-08-04 19:45:51	0|sipa|sdaftuar: someone connects to a 1000 nodes on the network, and relays a different version of the same valid transaction to each
340 2016-08-04 19:45:59	0|sipa|sdaftuar: now you potentially need to download 1000 transactions
341 2016-08-04 19:46:14	0|sipa|only one of which pays fees
342 2016-08-04 19:46:21	0|morcos|proposal: make feefilter mandatory. fully validate txs so we can punish peers who send us invalid stuff. don't put any witness tx in the rejection cache.  then evaluate how useful rejection cache continues to be or whether we have policy violating segwit txs bouncing around
343 2016-08-04 19:46:21	0|sdaftuar|so you mean a 3rd party uses signature malleability to do this?
344 2016-08-04 19:46:33	0|sdaftuar|because the tx originator can already do that
345 2016-08-04 19:47:00	0|sipa|sdaftuar: right, the distinction isn't all that big
346 2016-08-04 19:47:34	0|sipa|morcos: i like that... but i think it's a big change for 0.13.1
347 2016-08-04 19:47:35	0|gmaxwell|the distinction though is that an attacker doing it pays fees, eventually runs out of funds, vs an attacker doing it to other peoples' txn does not ever run out of funds.
348 2016-08-04 19:48:11	0|morcos|sipa: i think if we start with " don't put any witness tx in the rejection cache" then we'll be ok
349 2016-08-04 19:48:28	0|sipa|morcos: ack
350 2016-08-04 19:48:32	0|morcos|we can see how easy and short he other 2 are
351 2016-08-04 19:50:19	0|wumpus|that concludes the topic for this meeting?
352 2016-08-04 19:50:27	0|wumpus|any other topic proposals? cfields?
353 2016-08-04 19:50:30	0|NicolasDorier|for information, I created ##libconsensus bikeshedding room about how to handle libconsensus for anybody interested.
354 2016-08-04 19:50:49	0|cfields|NicolasDorier: haha, i was just about to paste a blurb for that
355 2016-08-04 19:51:06	0|NicolasDorier|aha I just woke up for that, will go back bed :D
356 2016-08-04 19:51:22	0|instagibbs|I have to leave now, but just letting you know salvage is finding lots of "extra" keys vs keys actually in wallet. *shrug*
357 2016-08-04 19:52:02	0|cfields|NicolasDorier and I discussed libconsensus a bit this weekend. We're hoping to discuss amongst ourselves, come up with some goals, and present them clearly for easier review
358 2016-08-04 19:52:12	0|morcos|+100
359 2016-08-04 19:52:16	0|cfields|jtimon: ^^
360 2016-08-04 19:52:25	0|wumpus|#topic libconsensus
361 2016-08-04 19:52:37	0|cfields|that was it :)
362 2016-08-04 19:52:54	0|NicolasDorier|wumpus: we have not started the bikeshedding yet, expect longer fight about it soon :p
363 2016-08-04 19:53:08	0|luke-jr|NicolasDorier: IMO just discuss it here
364 2016-08-04 19:53:14	0|luke-jr|way too many channels already
365 2016-08-04 19:53:32	0|NicolasDorier|the reason I prefer a separate channel is so we can keep history and review it more easily
366 2016-08-04 19:53:46	0|wumpus|it's not like this channel is very busy\
367 2016-08-04 19:53:47	0|NicolasDorier|things said here get lost quickly enough in the sea of discussion
368 2016-08-04 19:54:26	0|wumpus|that always happens on IRC, if you want discussions with good history/memory then it may be better to use a different discussion mechanism, or keep summaries or such
369 2016-08-04 19:54:36	0|sipa|i think having a separate place to 'form' a proposal makes sense
370 2016-08-04 19:55:01	0|luke-jr|#bitcoin-dev is also fairly quiet
371 2016-08-04 19:55:08	0|sipa|involving the whole world in a design rarely leads to something useful
372 2016-08-04 19:55:27	0|jtimon|from what I talked with NicolasDorier in previous times, the current goal is to complete a verifyBlock without necessarily exposing it in libconsensus at first
373 2016-08-04 19:55:29	0|wumpus|that's true, as long as you can get the people you want to pay attention to join it's fine
374 2016-08-04 19:55:42	0|wumpus|sometimes it's good to not have too many people there
375 2016-08-04 19:55:52	0|morcos|to be clear, the design will be presented to the bigger group for feedback and not set in stone
376 2016-08-04 19:55:52	0|wumpus|but speaking for myself, I'm already in so many channels, I have a hard time following more
377 2016-08-04 19:55:58	0|btcdrak|yeah ack on limiting channel proliferation, also it is more open in a logged channel
378 2016-08-04 19:56:35	0|morcos|i don't plan on joining the libconsensus subgroup, but i think having a focused small group will make progress actually happen
379 2016-08-04 19:56:37	0|BlueMatt|I'm with wumpus...too many channels these days (not that I've been paying enough attention to have much of a voice, but still)
380 2016-08-04 19:56:43	0|wumpus|then again that's up to you, doesn't need to be a discussion topic here to decide onthat.
381 2016-08-04 19:56:49	0|jtimon|I don't think creating a new channel or not is going to be important either way...
382 2016-08-04 19:56:59	0|cfields|morcos: sure. doesn't matter to me where the discussion takes place, just hoping to organize the discussion/planning a bit.
383 2016-08-04 19:57:03	0|wumpus|any other topics? 4 minutes to go
384 2016-08-04 19:57:14	0|sipa|i'm in favor of having it in a separate channel because i prefer not to see all discussions around it before there is clean design :)
385 2016-08-04 19:57:34	0|NicolasDorier|bikeshedding about the creation of the channel libconsensus announce the color of the future bikeshedding on the actual code ;)
386 2016-08-04 19:57:41	0|wumpus|NicolasDorier: exactly
387 2016-08-04 19:57:48	0|wumpus|let's stop that
388 2016-08-04 19:57:50	0|cfields|heh
389 2016-08-04 19:57:50	0|sipa|haha
390 2016-08-04 19:58:01	0|sipa|3 minutes
391 2016-08-04 19:58:17	0|jtimon|cfields: NicolasDorier if you can share the goals
392 2016-08-04 19:58:23	0|wumpus|looks like we're done
393 2016-08-04 19:58:25	0|lightningbot|Log:            http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-08-04-19.00.log.html
394 2016-08-04 19:58:25	0|lightningbot|Meeting ended Thu Aug  4 19:58:24 2016 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
395 2016-08-04 19:58:25	0|lightningbot|Minutes:        http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-08-04-19.00.html
396 2016-08-04 19:58:25	0|lightningbot|Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2016/bitcoin-core-dev.2016-08-04-19.00.txt
397 2016-08-04 19:58:25	0|wumpus|#endmeeting
398 2016-08-04 19:59:48	0|NicolasDorier|I have a code question: I'm working on a commit similar to one from jtimon, he is removing one check here https://github.com/jtimon/bitcoin/compare/remove-ism...consensus-post-remove-ism#diff-7ec3c68a81efff79b6ca22ac1f1eabbaL2353 about BIP30, does somebody know if it is actually safe to do so ?
399 2016-08-04 20:00:23	0|cfields|jtimon: i think everyone has different goals in mind. the point of organizing is to settle on something and work towards it. i can catch you up on our discussions this weekend, though ofc you've spent more time on it than anyone
400 2016-08-04 20:01:10	0|jtimon|cfields: well, a small summary of the "goals in common" or "easy to achieve" goals or something would be something
401 2016-08-04 20:02:49	0|NicolasDorier|jtimon: I think we still have also to discover what the goals in common are. Which is why I think we can discuss it in the channel. I guess if the three of us agree (plus anybody in the channel), there is good chance the rest agree too
402 2016-08-04 20:02:59	0|morcos|NicolasDorier: See this comment from sdaftuar: https://github.com/bitcoin/bitcoin/pull/8391#issuecomment-237584871
403 2016-08-04 20:03:31	0|NicolasDorier|correct, I'll write a bip
404 2016-08-04 20:04:34	0|jtimon|NicolasDorier: I may not be so optimist, but fair enough, I mean, great. do you guys have time now to give me a little summary of your discussions so far now (in the other channel)?
405 2016-08-04 20:04:44	0|morcos|It's related.  The point is once you are enforcing BIP34 then BIP30 is unnecessary except for historical violations.  We're changing the rule to have BIP34 be enforced as of a certain height, however, it kind of requires that the chain be the same otherwise we dont' know what the historical violations are or might be
406 2016-08-04 20:04:59	0|NicolasDorier|yes, I will jtimon, but later I'll go back bed soon (5am here)
407 2016-08-04 20:05:12	0|jtimon|yeah, I say so in my commit, it is unnecessary because we're not using ISM anymore
408 2016-08-04 20:06:19	0|jtimon|oh, no worries, another time will be fine
409 2016-08-04 20:07:00	0|cfields|jtimon: same, in the middle of several things atm. maybe some time tomorrow?
410 2016-08-04 20:07:15	0|jtimon|sure, no problem
411 2016-08-04 20:07:56	0|NicolasDorier|morcos: so basically we could remove BIP30 altogether as this is unlikely another chain manage to catch up the amount of work of the current one
412 2016-08-04 20:08:36	0|sipa|isn't there some interaction between BIP30 and the no-overwrite optimization in coinscache?
413 2016-08-04 20:08:49	0|sipa|which needs a BIP30 check during IBD
414 2016-08-04 20:09:24	0|NicolasDorier|mmh I think I won't make any change about it after all... I'm not confident of this thing
415 2016-08-04 20:09:26	0|morcos|NicolasDorier: yeah we need it on IBD
416 2016-08-04 20:09:32	0|morcos|hmm this brings up a kind of good point
417 2016-08-04 20:09:48	0|morcos|you can't remove the BIP34Hash unless you are going to checkpoint the chain at that point
418 2016-08-04 20:10:42	0|NicolasDorier|ok, so I will not touch it
419 2016-08-04 20:11:09	0|jtimon|you cannot remove it completely, but when you use bip34 you don't need to do it (this is current behaviour)
420 2016-08-04 20:11:28	0|morcos|jtimon: no, when you use BIP34 on the existing BIP34 hash then you dont' need to do it
421 2016-08-04 20:11:46	0|morcos|if you activated BIP34 at the same height on a different chain, it would be unclear whether yous till needed to do BIP30
422 2016-08-04 20:11:51	0|jtimon|morcos: only one block?
423 2016-08-04 20:11:57	0|morcos|depends on the status of historical violations on that chain
424 2016-08-04 20:12:01	0|morcos|no on all future blocks
425 2016-08-04 20:13:29	0|jtimon|https://github.com/jtimon/bitcoin/commit/273e27bd0c086f5ba20cebc2f5ec9e24f9d79015
426 2016-08-04 20:13:31	0|morcos|i can't remember off the top of my head, but if in an alternate chain you spent coinbase A before you generate coinbase A' then you activated BIP34, then you still need BIP30 to make sure the spend of A' doesn't conflict with the spend of A
427 2016-08-04 20:13:49	0|morcos|but in the BIP34hash chain we know that didn't happen
428 2016-08-04 20:13:50	0|jtimon|do you think that commit is incorrect after removing ISM?
429 2016-08-04 20:14:19	0|morcos|jtimon: its incorrect in that it'll be broken code on a mainnet chain that doesn't contain BIP34hash
430 2016-08-04 20:14:33	0|morcos|so who knows what happens if someone feeds you such a chain on startup
431 2016-08-04 20:15:07	0|jtimon|I think it will only fail if we reorg below BIP34Height (227931)
432 2016-08-04 20:15:13	0|morcos|oh wait
433 2016-08-04 20:15:47	0|morcos|maybe because BIP30 is enforced for everythign except those two exceptions you're ok
434 2016-08-04 20:15:49	0|jtimon|and what we have now, if it reorgs that long, without code for ISM...what? bip34 is going to be activated at that height no matter what
435 2016-08-04 20:16:17	0|jtimon|well, I wouldn't call them exceptions
436 2016-08-04 20:17:02	0|jtimon|pindexBIP34height will only be NULL if pindex->pprev->GetAncestor(227931) returns null
437 2016-08-04 20:18:01	0|jtimon|what is doing now is that if you are above that height and the block you activated bip34 in is 227931, then don't enforce bip30 since bip34 is enforced
438 2016-08-04 20:19:11	0|jtimon|in a reorg that deep, if it was activated with another block, then bip30 will be checked always regardless of bip34 activation (no optimization, never)
439 2016-08-04 20:19:12	0|morcos|jtimon: i honestly don't think its worth doing.  but if you really want to, i will take the time to think about it.  there are a lot of issues, down to when you flush your cache.  but please make an actual PR for me to review that other people also want to merge
440 2016-08-04 20:19:42	0|jtimon|with my suggested change, it would always do the optimization after bip34 activation height, even after such a long reorg
441 2016-08-04 20:20:22	0|jtimon|morcos: absolutely, no problem
442 2016-08-04 20:21:22	0|morcos|jtimon: ok, i think i got the problem
443 2016-08-04 20:21:43	0|jtimon|well, if you tell me now I won't open the PR :)
444 2016-08-04 20:21:47	0|morcos|imagine that you reorg to height 90000 (or are just fed an alternate chain on startup)
445 2016-08-04 20:22:09	0|morcos|ah shoot....
446 2016-08-04 20:22:11	0|morcos|nm
447 2016-08-04 20:22:23	0|morcos|wait
448 2016-08-04 20:22:24	0|morcos|yes
449 2016-08-04 20:22:25	0|morcos|thats it
450 2016-08-04 20:22:32	0|morcos|so confusing
451 2016-08-04 20:22:44	0|jtimon|let's do one thing at a time, yep, you reorg to 9000 or you fed an alternate chain?
452 2016-08-04 20:23:04	0|morcos|either, same affect you start processing different blocks starting at 90000
453 2016-08-04 20:23:05	0|jtimon|think the worse case: you reorg to block 1
454 2016-08-04 20:23:28	0|morcos|thats after coinbase A was created but before its duplicate A' was created
455 2016-08-04 20:23:32	0|morcos|now you spend coinbase A
456 2016-08-04 20:24:13	0|jtimon|bip 30 is checked before bip34 always except the exceptions
457 2016-08-04 20:24:29	0|jtimon|after bip34 activation, it is no longer necessary
458 2016-08-04 20:24:35	0|morcos|then you create coinbase A'  (allowed b/c it doesn't violate BIP30 , A is spent, and BIP34 is not active yet)
459 2016-08-04 20:24:49	0|morcos|now you activate BIP34 at height 227931
460 2016-08-04 20:25:10	0|morcos|then you can spend A' and create the same txid as your spend of A b/c you aren't enforcing BIP30 any more
461 2016-08-04 20:25:18	0|jtimon|yes, from now on you can stop checking bip30 in new blocks
462 2016-08-04 20:25:25	0|morcos|no you can't
463 2016-08-04 20:25:43	0|morcos|spending A' would be allowed and would create a duplicate txid of the spend of A
464 2016-08-04 20:27:07	0|jtimon|mhmm, whay they cannot today?
465 2016-08-04 20:27:10	0|jtimon|why
466 2016-08-04 20:27:41	0|morcos|because A' the coinbase was created before A was spent
467 2016-08-04 20:27:50	0|morcos|so A' overwrote A already
468 2016-08-04 20:28:15	0|morcos|but if in an alternate history you spent A to B first
469 2016-08-04 20:28:22	0|jtimon|doesn't include the height make it very difficult to collide with coinbases before 227931 ?
470 2016-08-04 20:28:23	0|morcos|then you have to worry about B' and B
471 2016-08-04 20:28:46	0|jtimon|instead of A'
472 2016-08-04 20:28:59	0|jtimon|think of all the coinbases before 227931
473 2016-08-04 20:29:29	0|jtimon|no? you don't want the same as neither of those
474 2016-08-04 20:29:39	0|morcos|you can try this on regtest..   enforce BIP30 only up to height 100 and BIP34 only starting at height 100.   create some coinbases, spend them and then create duplicates before 100.
475 2016-08-04 20:29:43	0|jtimon|that's with or without a reorg
476 2016-08-04 20:29:44	0|morcos|then after 100 make them colide
477 2016-08-04 20:29:56	0|Chris_Stewart_5|Is there a simple way to serialize a CKey? Doesn't seem to have the SERIALIZE_METHODS macro
478 2016-08-04 20:30:44	0|jtimon|morcos: yeah, you could "attack regtest" that way...
479 2016-08-04 20:31:07	0|morcos|well if you change the code, someone could feed you a messed up mainnet chain that way
480 2016-08-04 20:31:09	0|sipa|Chris_Stewart_5: historically, it's converted from/to a CPrivKey for serialization
481 2016-08-04 20:31:14	0|jtimon|I thought we were talking about mainnet
482 2016-08-04 20:32:24	0|morcos|jtimon: yes so on mainnet, you'd have to have a REALLY deep reorg, below 227931
483 2016-08-04 20:32:49	0|Chris_Stewart_5|sipa: Is there a simple utility function serialize CPrivKey? Can it just be passed to HexStr?
484 2016-08-04 20:33:05	0|morcos|or if somoene just fed you an alternate low work chain then i'm not really sure what would happen, its possible that your utxo could get into a messed up state than you wouldn't be able to fix when you found out about the real chain
485 2016-08-04 20:33:17	0|jtimon|and then a node without my change would always check bip30, even after bip34 activation, is that the desired behaviour?
486 2016-08-04 20:33:30	0|sipa|Chris_Stewart_5: CKey also has begin() and end(), to it can be passed to HexStr directly
487 2016-08-04 20:33:39	0|sipa|Chris_Stewart_5: and has a constructor that takes a byte array
488 2016-08-04 20:33:50	0|jtimon|my node would not check bip30 after bip34, even after the reorg
489 2016-08-04 20:34:48	0|morcos|jtimon: yes you can either always check BIP30 even after BIP34, or you can examine the chain you're on pre BIP34 and decide whether you can now skip BIP30 checks (which is what we decided to do with mainnet, but requires manual intervention)
490 2016-08-04 20:34:56	0|morcos|are you opposed to leaving in the BIP34 hash
491 2016-08-04 20:35:14	0|morcos|checking BIP30 is exteremely slow
492 2016-08-04 20:35:39	0|morcos|that's why we put in the ability to skip it
493 2016-08-04 20:36:58	0|jtimon|so, yes, theoretically always checking bip30 after bip34 has certain risk if there's that reorg...but aren't we screwed if that happens anyway?
494 2016-08-04 20:37:16	0|jtimon|like, really screwed
495 2016-08-04 20:37:51	0|jtimon|I don't know, maybe it's not worth it
496 2016-08-04 20:38:04	0|Chris_Stewart_5|sipa: Thanks
497 2016-08-04 20:39:09	0|jtimon|the advantages are code simplification and a slight optimization (really no need to call GetAncestor if you don't care about the hash), and keep the optimization in case of pre-bip34 reorg, which is arguably a disadvantage
498 2016-08-04 20:39:35	0|jtimon|no, not opposed, removing it is just a suggestion
499 2016-08-04 20:40:46	0|jtimon|if we knew we weren't doing it, we could have made https://github.com/jtimon/bitcoin/commit/6f98197634026e740a9172405386b15c3a79b5a5 without bip34 directly in #8391...
500 2016-08-04 20:41:02	0|jtimon|now I'm afraid the right time to do that will be...never
501 2016-08-04 20:41:07	0|morcos|jtimon: i think the issue is less a reorg than that someone might feed you an alternate chain on startup and get your utxo into a permanently messed up state b/c you don't know how to reorg back
502 2016-08-04 20:41:38	0|jtimon|oh, I see
503 2016-08-04 20:41:55	0|jtimon|yeah, makes sense, thanks
504 2016-08-04 20:41:55	0|morcos|i'm not positive that would happen, but seems more likely than not, and also seems like it could be affected by future changes to the way coins work that we might not anticipate
505 2016-08-04 20:44:14	0|jtimon|yeah, I wasn't properly thinking about an isolated node doing initial synchornization, that's what you meant by fed up with the wrong chain from the beginning, sorry
506 2016-08-04 20:44:45	0|jtimon|nah, better to avoid that risk, definitely not worth it
507 2016-08-04 20:44:52	0|jtimon|thansk
508 2016-08-04 20:44:52	0|morcos|jtimon: yeah
509 2016-08-04 20:44:54	0|morcos|sure
510 2016-08-04 20:47:34	0|jtimon|well, maybe the time for https://github.com/jtimon/bitcoin/commit/6f98197634026e740a9172405386b15c3a79b5a5 is whenever bip113 activation is simplified from bip9 in the same way
511 2016-08-04 20:48:59	0|jtimon|or when/if we expose consensus::Params in libconsensus
512 2016-08-04 20:49:28	0|jtimon|anyway, that's another topic
513 2016-08-04 20:49:39	0|Chris_Stewart_5|sipa: When I serialize CPriv/CPub I get something that is larger than 32 bytes?
514 2016-08-04 20:50:36	0|sipa|Chris_Stewart_5: CPrivKey uses OpenSSL DER serialization... which is 300 bytes or so for a private key
515 2016-08-04 20:50:41	0|sipa|CPubKeys are 33 or 65 bytes
516 2016-08-04 20:50:49	0|sipa|CKey is internally 32 bytes + compression flag
517 2016-08-04 20:52:19	0|Chris_Stewart_5|ahh ok, that makes more sense. So basically CKey is the interface exposed inside of core... except for serialization. Something that could be implemented in a pull?
518 2016-08-04 20:52:55	0|sipa|well there isn't one single obvious serialization for CKey
519 2016-08-04 20:55:53	0|Chris_Stewart_5|Seems like the 32 byte serialization would be the intuitive one.. at least to me. Am I missing something here?
520 2016-08-04 20:56:53	0|Chris_Stewart_5|oh nvm i see what you are talking about now, since CKeys can be both priv and pubs right?
521 2016-08-04 20:57:30	0|sipa|no
522 2016-08-04 20:57:42	0|sipa|CKey is a private key
523 2016-08-04 20:57:49	0|sipa|but you need the compression flag too
524 2016-08-04 20:58:31	0|sipa|and there is no deserialize because we use secure memory for private keys where possible
525 2016-08-04 21:08:55	0|NicolasDorier|jtimon, I will soon make a PR similar to https://github.com/jtimon/bitcoin/commit/6f98197634026e740a9172405386b15c3a79b5a5  along with another change to parametize buriedsf height for regtest
526 2016-08-04 21:10:20	0|jtimon|NicolasDorier: cool, but I believe the right time was in your PR, at least with the new ones if you didn't wanted to touch bip34
527 2016-08-04 21:11:03	0|NicolasDorier|jtimon: I disagree, my PR was making a "big" consensus change so I wanted to keep it as simple as possible to review
528 2016-08-04 21:11:29	0|NicolasDorier|I wanted to be it merged the quickier I could so I can continue my work on libconsensus
529 2016-08-04 21:11:39	0|jtimon|although I won't oppose the refactor, of course, just worry that it will be less acceptable
530 2016-08-04 21:12:16	0|jtimon|how would using an array instead of two variables have been more complicated?
531 2016-08-04 21:12:27	0|jtimon|anyway, as said I won't oppose
532 2016-08-04 21:13:16	0|NicolasDorier|jtimon: I don't think it wil lbe less acceptable, this buried deployment stuff will be a BIP. In the future other SF will end up using the same structure
533 2016-08-04 21:13:24	0|NicolasDorier|anyway, I made two differents commit
534 2016-08-04 21:13:40	0|NicolasDorier|we can take one and not the other
535 2016-08-04 21:13:51	0|jtimon|NicolasDorier: yeah I guess the BIP is an opportunity to reorganize the struct
536 2016-08-04 21:15:16	0|jtimon|anyway, I'm suffering from premature worry about controversy, let's not discuss about whether something will be acceptable or not when we both want it
537 2016-08-04 21:27:08	0|GitHub193|[13bitcoin] 15NicolasDorier opened pull request #8460: parametrize buried soft fork in regtest and refactor (06master...06buriedsf) 02https://github.com/bitcoin/bitcoin/pull/8460
538 2016-08-04 21:27:47	0|NicolasDorier|jtimon: worrying about controversy create a meta controversy :D
539 2016-08-04 21:28:21	0|NicolasDorier|just created the PR
540 2016-08-04 21:33:48	0|jtimon|yeah, I'm revieweing it
541 2016-08-04 21:35:16	0|jtimon|and looking at 8418 too
542 2016-08-04 21:36:23	0|jtimon|https://github.com/bitcoin/bitcoin/pull/8418/files#diff-c865a8939105e6350a50af02766291b7R981
543 2016-08-04 21:36:40	0|jtimon|.MineBlocksOnDemand() is starting too look like IsRegtest()
544 2016-08-04 21:37:12	0|GitHub92|[13bitcoin] 15jlopp opened pull request #8461: document return value of networkhashps for getmininginfo RPC endpoint (06master...06rpcMiningHelp) 02https://github.com/bitcoin/bitcoin/pull/8461
545 2016-08-04 21:44:54	0|jtimon|oh, it was RegTest() or Params().NetworkID() == CChainParams::REGTEST directly, never mind, it's been a while since 3824, thinking out loud
546 2016-08-04 21:59:12	0|jtimon|NicolasDorier: in #8460, how do you modify a couple of them? -buriedsfparams=bip65:1351,bip66:1251 ?
547 2016-08-04 21:59:29	0|NicolasDorier|you repeat
548 2016-08-04 21:59:35	0|NicolasDorier|-buriedsfparams=bip65:1351  -buriedsfparams=bip66:1251
549 2016-08-04 21:59:47	0|jtimon|ok, thanks
550 2016-08-04 22:02:05	0|jtimon|https://github.com/bitcoin/bitcoin/pull/8418/files#r73610819
551 2016-08-04 22:25:29	0|NicolasDorier|jtimon: I think one bool is enough. fAllowOverwriteSoftforks which both BIP9 and buried SF use
552 2016-08-04 22:27:42	0|jtimon|NicolasDorier: fine with me, sounds reasonable
553 2016-08-04 22:27:56	0|NicolasDorier|will add commit to my PR later
554 2016-08-04 22:28:03	0|jtimon|great
555 2016-08-04 22:39:48	0|jtimon|NicolasDorier: you are not removing BIP34Height BIP65Height and BIP66Height in Consensus::Params
556 2016-08-04 22:41:17	0|jtimon|/** Block hash at which BIP34 becomes active (for BIP30 optimization)*/
557 2016-08-04 22:41:17	0|jtimon|uint256 BIP34Hash;
558 2016-08-04 22:41:17	0|jtimon|you can leave:
559 2016-08-04 22:44:41	0|NicolasDorier|oh
560 2016-08-04 23:04:27	0|sipa|does anyone know offhand where the last segwit transaction in the chain is?
561 2016-08-04 23:04:31	0|sipa|*testnet chain
562 2016-08-04 23:05:28	0|sipa|i've reset the testnet chainparams for segwit to 0, and reconsiderblock/invalidateblock 100000 blocks, and it works fine
563 2016-08-04 23:45:23	0|NicolasDorier|jtimon: I fixed the nit as well as added the commit with the allowSfOverride
564 2016-08-04 23:50:56	0|jtimon|NicolasDorier: tiny last nit https://github.com/bitcoin/bitcoin/pull/8460/files#r73622457
565 2016-08-04 23:52:17	0|NicolasDorier|jtimon: this is removed in later commits
566 2016-08-04 23:52:51	0|NicolasDorier|oups
567 2016-08-04 23:52:53	0|NicolasDorier|nop
568 2016-08-04 23:54:00	0|NicolasDorier|removed :)
569 2016-08-04 23:54:30	0|NicolasDorier|I did quite a lot of rebase to clean up the history. But now it should be stable
570 2016-08-04 23:55:09	0|jtimon|good work
571 2016-08-04 23:56:52	0|NicolasDorier|thanks, going to bed for real now (9am) :D
572 2016-08-04 23:58:52	0|jtimon|oh, sorry about that