1 2017-05-04 01:06:25	0|cfields|sipa: you're going to hate me for this...
  2 2017-05-04 01:07:31	0|bitcoin-git|[13bitcoin] 15theuni opened pull request #10335: back-compat: add fallback getentropy implementation (06master...06getentropy-back-compat) 02https://github.com/bitcoin/bitcoin/pull/10335
  3 2017-05-04 01:12:01	0|sipa|cfields: we don't use getentropy on linux
  4 2017-05-04 01:12:22	0|warren|cfields: btw, Ubuntu 12.04 Precise is hitting EOL, will gitian switch to a newer base soon?
  5 2017-05-04 01:12:46	0|sipa|cfields: or we shouldn't, i guess
  6 2017-05-04 01:13:26	0|sipa|if getrandom isn't available on linux, i think we should fallback to using /dev/urandom directly rather than through a fallback for a wrapper for a BSD function :)
  7 2017-05-04 01:16:00	0|cfields|sipa: atm it's being used in linux, best i can tell
  8 2017-05-04 01:16:20	0|cfields|(i get a runtime link error, so it must be)
  9 2017-05-04 01:17:21	0|cfields|warren: gitian uses Trusty currently
 10 2017-05-04 01:17:36	0|warren|oops
 11 2017-05-04 01:19:06	0|sipa|cfields: yes, we compile in code to use getrandom whenever configure detects it
 12 2017-05-04 01:19:57	0|cfields|sipa: sorry, i was referring to getentropy
 13 2017-05-04 01:20:06	0|sipa|eh, i mean getentropy
 14 2017-05-04 01:20:31	0|sipa|cfields: the getentropy implementation in glibc on Linux just uses SYS_getrandom, just like we already do directly
 15 2017-05-04 01:21:19	0|sipa|so a fallback that just returns 0 should be fine
 16 2017-05-04 01:21:24	0|sipa|we check the return status
 17 2017-05-04 01:22:21	0|sipa|ah, no
 18 2017-05-04 01:22:22	0|sipa|hmm
 19 2017-05-04 01:27:22	0|cfields|sipa: looks like getentropy needs to check for ENOSYS, and do GetDevURandom as a fallback. Then the weak getentropy just returns ENOSYS.
 20 2017-05-04 01:27:27	0|cfields|I think that works?
 21 2017-05-04 01:28:06	0|sipa|cfields: yup
 22 2017-05-04 01:28:25	0|cfields|ok, will do.
 23 2017-05-04 01:28:26	0|sipa|cfields: i think our GetOSEntropy call should always compile in the GetDevURandom fallback
 24 2017-05-04 01:28:41	0|sipa|(so move it out of the #ifdef ... #elif .. #endif switch)
 25 2017-05-04 01:28:54	0|cfields|yep, agreed
 26 2017-05-04 01:29:01	0|sipa|then the getrandom/getentropy calls can fail without problems
 27 2017-05-04 01:29:27	0|sipa|and agreed, a simple fallback returning ENOSYS is perfect then
 28 2017-05-04 03:32:36	0|cfields|<sipa> cfields: i think our GetOSEntropy call should always compile in the GetDevURandom fallback
 29 2017-05-04 03:32:43	0|cfields|sipa: looking again, I'm not sure what you meant by that
 30 2017-05-04 03:33:03	0|cfields|we always compile GetDevURandom, except for win32
 31 2017-05-04 03:34:39	0|sipa|cfields: i mean the GetDevURandom call at the bottom of GetOSEntropy
 32 2017-05-04 03:35:01	0|sipa|which is only compiled if getrandom and getentropy are not availablr
 33 2017-05-04 03:35:05	0|sipa|that should change
 34 2017-05-04 03:35:51	0|cfields|sipa: so you want early returns for any better function, and fallback if one isn't hit by the end?
 35 2017-05-04 03:36:08	0|sipa|right
 36 2017-05-04 03:36:28	0|cfields|makes sense, will do
 37 2017-05-04 03:37:18	0|sipa|getrandom currently has a fallback already
 38 2017-05-04 03:37:21	0|sipa|but not the rest
 39 2017-05-04 03:43:45	0|Lightsword|so I should remove the fallback here as well? https://github.com/bitcoin/bitcoin/pull/10301/files#diff-35f8a407f8c21cda300a45f50b6e9c74R172
 40 2017-05-04 06:35:28	0|jonasschnelli|Grml... if we enforce a min HD recover gap limit (currently 20 keypool keys), all tests are broken (mosts tests run with a keypool of size 1). I guess I have to add a -hdignoregaplimit and set it to true by default for the tests.
 41 2017-05-04 07:28:52	0|jonasschnelli|wumpus: Re multiple tx-row-entries after a Qt bump: https://github.com/bitcoin/bitcoin/pull/9697#issuecomment-298339022
 42 2017-05-04 07:29:12	0|jonasschnelli|At the moment, list transactions has the same behavior, though you can see conflicts there.
 43 2017-05-04 07:29:18	0|jonasschnelli|Not sure how we should "solve" that in Qt...
 44 2017-05-04 07:29:38	0|jonasschnelli|I think I once had a PR that marked conflicted tx with a special color (maybe orange or something)
 45 2017-05-04 07:30:00	0|jonasschnelli|Or should we hide them completely (only the own bump conflicts)
 46 2017-05-04 07:30:02	0|jonasschnelli|?
 47 2017-05-04 07:52:41	0|wumpus|jonasschnelli: the problem is that the transactions are only marked after a restart IMO
 48 2017-05-04 07:53:28	0|wumpus|not so much the marking itself
 49 2017-05-04 07:54:08	0|jonasschnelli|wumpus: Hmm.. you mean the opacity change, right?
 50 2017-05-04 07:55:06	0|jonasschnelli|Yes. The opacity is only changes after restart,... though, that's solvable.
 51 2017-05-04 07:55:37	0|wumpus|jonasschnelli: or how the amounts are shown (e.g. [] around it)
 52 2017-05-04 07:55:39	0|jonasschnelli|In a follow up PR we could introduce a new icon like this: https://github.com/bitcoin/bitcoin/pull/7826#issuecomment-220644102
 53 2017-05-04 07:55:53	0|jonasschnelli|Ah.. yes. the [] is also part of it. Will fix that now
 54 2017-05-04 07:58:33	0|wumpus|yeah :)
 55 2017-05-04 08:18:16	0|wumpus|I don't understand why we'd need *yet another* workaround for getentropy on linux
 56 2017-05-04 08:18:45	0|wumpus|man if it's really that difficult let's just use /dev/urandom and give this up
 57 2017-05-04 08:22:09	0|jonasschnelli|wumpus: Agree. If /dev/urandom is not reliable, your OS is not and thus, getentropy doesn't give much more value... or do I misinterpret this?
 58 2017-05-04 08:22:43	0|wumpus|well the idea is that it can be run in a jail/container without /dev
 59 2017-05-04 08:23:25	0|jonasschnelli|I see...
 60 2017-05-04 08:23:36	0|wumpus|it's not so much that the syscall is more reliable (though that may be the case) but that it's always available (if the OS happens to have it avaiable...)
 61 2017-05-04 08:24:14	0|wumpus|I just don't see the point in a getentropy() implementation that reads /dev/iurandom anyway
 62 2017-05-04 08:24:26	0|wumpus|we already have a function for that
 63 2017-05-04 08:24:33	0|wumpus|this is getting way more complicated than I imaginedc
 64 2017-05-04 11:18:25	0|bitcoin-git|[13bitcoin] 15snvakula opened pull request #10336: Get actual path for EUID instead of HOME dir (06master...06contrib) 02https://github.com/bitcoin/bitcoin/pull/10336
 65 2017-05-04 13:21:01	0|jonasschnelli|morcos: ping re mempool stats: https://github.com/bitcoin/bitcoin/pull/8501#issuecomment-270714233
 66 2017-05-04 13:25:22	0|jonasschnelli|morcos: Would you say it makes sense to keep a singe samples vector and add fixed time-limits for sample frequency and purge out in-between values? Example: we add a sample whenever the mempool has changed while respecting a min delta of 2s, == no extra thread required.
 67 2017-05-04 13:26:07	0|jonasschnelli|Then, during the cleanup (every 100 samples or so), we purge out in-between samples to ensure a min-frequency of, say, delta 30s for samples older then 2h (TBD)
 68 2017-05-04 13:26:29	0|jonasschnelli|same for older then 24h (maybe ensure a min-delta of 2min), etc.
 69 2017-05-04 13:27:13	0|jonasschnelli|The question then is, if we should interpolate fix time steps during cleanup/purge. Because the samples may be distributed "randomly" over time.
 70 2017-05-04 13:39:10	0|morcos|jonasschnelli: that seems considerably messier to me than keeping several samples vectors (one for each time scale we might care about) and only recording a data point in the appropriate vector if its time to. i don't think that requires any extra thread either (althgough its not clear that it wouldn't be better in a separate thread)
 71 2017-05-04 13:39:21	0|morcos|periodic cleanup just seems weird
 72 2017-05-04 13:40:31	0|jonasschnelli|morcos: Separate vectors would require move samples from one to another vector... though not sure if this bad or good. :)
 73 2017-05-04 13:40:32	0|morcos|i think the lack of precision in recording the data point at exactly the right time is just something i would ignore witht he exception of if we miss a sample period entirely we should do some sort of filling in
 74 2017-05-04 13:40:42	0|morcos|i don't understand that
 75 2017-05-04 13:41:28	0|morcos|every 2 seconds record in the short vector..  on the 60th second add the same sample point to the short vector and the medium vector at the same time.. on the hour add to all 3
 76 2017-05-04 13:41:42	0|morcos|there is some very small data duplication, but no moving, cleaning or reorganizing
 77 2017-05-04 13:41:48	0|jonasschnelli|Oh. Yes. I overcomplicated.
 78 2017-05-04 13:42:02	0|jonasschnelli|Yes. Some duplication.. but I guess that okay.
 79 2017-05-04 13:42:11	0|morcos|very little
 80 2017-05-04 13:42:38	0|jonasschnelli|Yeah. Use shared pointers *duck*
 81 2017-05-04 13:43:04	0|jonasschnelli|About the periodic cleanup:
 82 2017-05-04 13:43:34	0|jonasschnelli|I though calculating the dynamic allocation on each added sample seems a bit to expansive,.. that's why I added the periodic check
 83 2017-05-04 13:43:57	0|jonasschnelli|But if we have fixed timespans with a thread, then this can be solved simpler.
 84 2017-05-04 13:44:31	0|jonasschnelli|Though I wanted to avoid grabbing the cs_mempool to calculate the dynamic mempool size, etc.
 85 2017-05-04 13:45:00	0|jonasschnelli|Stats could then have an more significan impact on the mempool performance then with "passive collecting"
 86 2017-05-04 13:50:52	0|sipa|wumpus: presumably his configure did not detect getrandom (old kernel headers?), but did detect getentropy?
 87 2017-05-04 13:52:32	0|morcos|jonasschnelli: you could cache the latest value for things that were being calculated anyway, and then only record it on the periodic sample
 88 2017-05-04 13:53:56	0|morcos|but i'm not sure what you mean about the dynamic allocation for the added sample..  i'm assuming that at least for the smaller time scales you're going to have a limited time span for which you keep samples..  like just a recent buffer of the last couple hours at 2 sec sample periods or something..  so it seems like it should not really require much allocation
 89 2017-05-04 13:54:40	0|morcos|anyway.. i'm happy to revisit it in a week or so..  but i'm mostly out of the office this week.. so just trying to keep head above water
 90 2017-05-04 13:56:40	0|jonasschnelli|What I meant is the check against the set max memory size for the stats.... say someone has set 10MB as max memory for stats then you have to know when you overshoot it. Not sure if vector-size * sizeof(sample) and cutoff the unnecessary samples is very expansive... I though doing it every 100-added sample makes sense.
 91 2017-05-04 13:57:13	0|jonasschnelli|morcos: Yes. No hurry... I'll ping you once I have more...
 92 2017-05-04 13:58:56	0|morcos|jonasschnelli: yeah i was imagining you initially just calculate how many samples you're going to get to keep given their memory max  and then have a ring buffer, but you coudl do it many ways.. personally i think its kind of an over configuration option.  i'd have a -stats=0 option to save the memory, but then otherwise everyone gets whatever the default memory usage we think makes sense.  not everything has to be con
 93 2017-05-04 13:59:37	0|jonasschnelli|Yes. fair enough... thanks for your inputs
 94 2017-05-04 14:06:16	0|sipa|wumpus: we don't need another wrapper around urandom though; only a weak alias to a function that always returns ENOSYS, and a change in GetOSEntropy that falls back to GetDevURandom in case getentropy fails
 95 2017-05-04 14:06:44	0|sipa|wumpus: or, alternatively, just never use getentropy on Linux
 96 2017-05-04 14:59:22	0|wumpus|sipa: not using getentropy on linux would make sense, after all there is a specific syscall handling for thatt
 97 2017-05-04 15:09:20	0|BlueMatt|jonasschnelli: how is #10238 intended to interact with #10235? It looks like it was designed to be complementary, but which performance improvements are you envisioning using 10238 for?
 98 2017-05-04 15:09:22	0|gribble|https://github.com/bitcoin/bitcoin/issues/10238 | Change setKeyPool to hold flexible entries by jonasschnelli · Pull Request #10238 · bitcoin/bitcoin · GitHub
 99 2017-05-04 15:09:23	0|gribble|https://github.com/bitcoin/bitcoin/issues/10235 | Track keypool entries as internal vs external in memory by TheBlueMatt · Pull Request #10235 · bitcoin/bitcoin · GitHub
100 2017-05-04 15:18:11	0|sipa|wumpus: will do
101 2017-05-04 15:49:55	0|jonasschnelli|BlueMatt: I wrote 10238 to ensure we have a fast way to lookup CKeyIds... without this, we have to read each keypool key from the database on each SyncTransaction
102 2017-05-04 15:50:06	0|jonasschnelli|(with HD restore)
103 2017-05-04 15:50:31	0|jonasschnelli|I shouldn't conceptually conflict with your 10235
104 2017-05-04 15:50:50	0|BlueMatt|ahh, ok, i was missing the motivation part, thanks
105 2017-05-04 15:51:28	0|jonasschnelli|I though the description was good... need to check...
106 2017-05-04 15:51:35	0|BlueMatt|oh, maybe i missed that
107 2017-05-04 15:51:46	0|jonasschnelli|Currently we only keep the keypool index in memory.. the rest is database AFAIK
108 2017-05-04 15:53:25	0|morcos|wumpus: jonasschnelli: does anyone care about the QT option in Custom fee to pay "total at least"?
109 2017-05-04 15:54:26	0|morcos|I've been looking at some of these recently reported issues with transations paying too high a fee, and they seem likely to be a result of coin selection logic
110 2017-05-04 15:54:53	0|morcos|I think it might make for a cleaner redesign to make all fee calcluations in terms of fee rate and then take that into account when selecting inputs the first time around
111 2017-05-04 15:55:17	0|jonasschnelli|morcos: not sure... I'm not sure if we want this feature...
112 2017-05-04 15:55:21	0|morcos|The "total at least" option only applies to pre-selected coins anyway, but it seems like it would make for cleaner code to just dump that
113 2017-05-04 15:55:49	0|morcos|I mean why does anyone care about a minimum amount of fee, only the rate matters, its not like there is a dust issue with fees
114 2017-05-04 15:56:53	0|morcos|Another thing that might be nice to clean up, is the fact that setting Custom Fee in the GUI changes the paytxfee used by the command line, that just seems like a mistake to me, but i suppose changing that now might be a surprise to some poeple
115 2017-05-04 16:24:01	0|gmaxwell|The total at least stuff makes no sense, it should go. Feerate at least, sure.  Total no more than, sure.
116 2017-05-04 16:35:48	0|morcos|gmaxwell:hmm... total no more than... you mean just the existing maxtxfee?  shoot..  i had forgotten about that
117 2017-05-04 16:36:48	0|morcos|i wanted to reduce each coin by the fee needed to spend it before doing coin selection, and then we could be a bit smarter about which ones we added..
118 2017-05-04 16:37:36	0|morcos|i suppose it would make some sense to have the -maxtxfee failsafe, but maybe that is at a different place in the code
119 2017-05-04 16:37:58	0|gmaxwell|Yes, I think thats at a different place in the code.
120 2017-05-04 16:38:50	0|morcos|no its not now, but i could move it to a different place.. like repeat coin selection if your coin selection violated maxtxfee
121 2017-05-04 16:38:59	0|morcos|i want to also get rid of the knapsack solver
122 2017-05-04 16:39:53	0|morcos|if you have enough inputs that are lower than the target to reach target + CENT, just start randomly adding them until you are > target+CENT, and then you're done
123 2017-05-04 16:40:44	0|morcos|the idea that you are trying to get exactly target + CENT is stupid.   target + 3.5 CENTS is just as good or whatever
124 2017-05-04 16:41:07	0|gmaxwell|getting a result with no change is also attractive, but I think that should be a seperate algorithim run first.
125 2017-05-04 16:41:45	0|morcos|gmaxwell: hmm.. i think thats rare enough that i'm not sure its worth trying to hard for
126 2017-05-04 16:42:24	0|gmaxwell|I don't think it should be, at least if we're reasonable about how much fee we'll throw away.
127 2017-05-04 16:42:48	0|morcos|its the way we do that now that is kind of causing the super high fees i think
128 2017-05-04 16:42:49	0|BlueMatt|so apparently the coin selection dialog's "received by" address is, essentially, complete gibberish
129 2017-05-04 16:43:03	0|morcos|but that is fixable while still trying to do that, for sure
130 2017-05-04 16:43:20	0|BlueMatt|and its "tree" (which I thought was supposed to group things ala listaddressgroupings) does not at all do that, and is equally useless
131 2017-05-04 16:48:10	0|gmaxwell|morcos: I've tried a bit and have not managed to make it cause high fees since the changes we made to fix the fees in later passes. How confident are we that there is still an issue?
132 2017-05-04 16:49:14	0|morcos|i think its pretty plausible
133 2017-05-04 16:49:50	0|morcos|if nTotalLower is < target + CENT, then you have a decent chance of the problem happening
134 2017-05-04 16:50:14	0|morcos|once nTotalLower < target + CENT, the knapsack algorithm tries to find you exactly target
135 2017-05-04 16:50:23	0|morcos|which leads to decent chance that your change is dust
136 2017-05-04 16:50:28	0|morcos|so your change gets eliminated
137 2017-05-04 16:51:04	0|morcos|then when you callculate the fee and you need to go re-find inputs which meet target_2  (= target + fee)
138 2017-05-04 16:51:31	0|morcos|you end up choosing fewer different inputs, have a lower fee, and now no change to dump the excess fee on
139 2017-05-04 16:52:50	0|sipa|why does CENT even still occur in the algorithm?
140 2017-05-04 16:53:04	0|morcos|actually, i used to hate that, now i think its the best part of the algorithm!
141 2017-05-04 16:53:26	0|morcos|its the goal size for change, it shouldn't be an exact target, but a minimum as i mentioned above
142 2017-05-04 16:53:54	0|morcos|but it's better to have a target like that, orders of magnitude higher than dust, rather than just randomly creating change ouputs that might barely pass dust
143 2017-05-04 16:54:48	0|morcos|what's silly is that if you can't get CENT, then you aim for 0.
144 2017-05-04 16:55:29	0|morcos|but i think the key improvement is to look at the net value of inputs in coin selection given the desired fee rate
145 2017-05-04 16:56:37	0|morcos|i suppose i could have written this all up first (i still will) but my idea was you'd have some sort of economic threshold such as only use inputs whose net_input_value > total_input_value/2 unless your tx confirm target > 48, then you use all inputs whos net_input_value > 0
146 2017-05-04 16:56:53	0|morcos|the current algorithm throws in stupid inputs whose net_input_value is 0
147 2017-05-04 16:56:58	0|morcos|i mean negative
148 2017-05-04 16:59:10	0|morcos|btw.. i'm probably out for the meeting today.. so if anyone has any fee estimation questions, feel free to ask before..   i'm still working through comments on PR
149 2017-05-04 17:14:22	0|gmaxwell|morcos: I'd rather two two total solutions, e.g. a change and no-change solution, and then use post selection.
150 2017-05-04 17:15:53	0|morcos|What information would you use from the change solution to make that decision?  It seems to me either you find a good no-change solution or you don't?
151 2017-05-04 17:19:05	0|bitcoin-git|[13bitcoin] 15sipa opened pull request #10338: Maintain state across GetStrongRandBytes calls (06master...06stateful_rng) 02https://github.com/bitcoin/bitcoin/pull/10338
152 2017-05-04 17:21:57	0|gmaxwell|both may be 'less than perfect', e.g.  no-change might give away more than we'd like, but change may involve higher fees, so it's no better.
153 2017-05-04 17:22:24	0|gmaxwell|e.g. because the extra output increases the size, and then an extra input is needed... which increases the size further.
154 2017-05-04 17:22:56	0|gmaxwell|and so you'd prefer the no change even though it gave away more than you might normally like, because the other option was worse.
155 2017-05-04 17:23:02	0|morcos|hmm.. so my way of thinking about it, which is i thought your logic, that a solution that is 10x as big and costs 10x as much is not worse
156 2017-05-04 17:23:11	0|morcos|granted, the change output vs no change is strictly worse
157 2017-05-04 17:23:29	0|morcos|but that in my mind is a bit how you would go about figuring out whether you have an acceptable no change solution
158 2017-05-04 17:23:38	0|gmaxwell|it's true that you'll pay the fees eventually.
159 2017-05-04 17:24:35	0|gmaxwell|but then there is another angle is that our strategy should be changing if we think the feerate is 'high' for the moment, vs 'low'... in the latter we should optimize for larger transactions.
160 2017-05-04 17:24:50	0|morcos|yeah, that last part is important, but i think a complicated optimization
161 2017-05-04 17:24:57	0|morcos|maybe not ready for that yet
162 2017-05-04 17:25:21	0|morcos|i'm including a tiny bit of that in not being willing to spend most of inputs unless its a low confirm target
163 2017-05-04 17:25:51	0|morcos|but then there get to be all kinds of complicated questiosn, what kinds of fee rates does this user usually use and compare to current market conditions and size of current transaction
164 2017-05-04 17:26:20	0|gmaxwell|it's a fair point that you don't want to tie up all your inputs on a rare 25-confirm-target payment.
165 2017-05-04 17:26:22	0|morcos|that was all kind of a next level in my mind...  first level was meant to try to eliminate some stupid edge cases but not deviate too much from existing logic
166 2017-05-04 17:26:40	0|morcos|yeah..  so many ways to so grade tx selection
167 2017-05-04 17:26:41	0|gmaxwell|Fair enough.
168 2017-05-04 17:28:26	0|instagibbs|morcos, I think earlier you basically described murch's algorithm, first it does some quick search for "exact match", meaning sum of effective amounts of the inputs that don't create a change output, and then if that fails backs off to random selection
169 2017-05-04 17:29:35	0|morcos|instagibbs: ah yes, ok i'll go reread what he said...   i wanted to keep random selection from the lower than target value inputs if possible..  and do them on net value... that seems to me the important improvements
170 2017-05-04 17:30:17	0|instagibbs|sure, that seems to mesh. He said he got 50x more exact matches than Core. Which means it might be worth it.
171 2017-05-04 17:30:33	0|instagibbs|(on a real world scrubbed dataset)
172 2017-05-04 17:31:40	0|instagibbs|I still think 10333 is complementary. Anytime we screw up somehow hitting the feerate we want, we can try and salvage. Hopefully will be much less important at least.
173 2017-05-04 17:32:08	0|gmaxwell|and exploiting no change is important for reducing the utxo set size; I think if payment amounts come from a unimodal distribution the utxo count will grow expoentially under any algorithim that minimizes input counts and always creates change.
174 2017-05-04 17:33:12	0|morcos|instagibbs: yes, i haven't read the code yet, but i'm not opposed to the idea...   but i think if we consider net values for inputs, then there is no such thing as screwing up the fee.
175 2017-05-04 17:33:54	0|sipa|gmaxwell: but aiming for no change probably requires that every wallet permanently has a set of utxos of various sizes
176 2017-05-04 17:34:02	0|sipa|gmaxwell: which on itself may not be ideal
177 2017-05-04 17:34:40	0|gmaxwell|well then there is the extra change output, which I'd like to work out how to do, at least when the change would be large we should split the outputs.
178 2017-05-04 17:36:10	0|instagibbs|morcos, yeah I can't convince myself otherwise, but having a safety net feels better. It's not a lot of logic. Treating the cause is clearly fixing coin selection.
179 2017-05-04 17:43:07	0|gmaxwell|as far as the safty net, I think it's fine if it's just a dumb check at the end that aborts the transfer if it fails.
180 2017-05-04 17:43:32	0|gmaxwell|people get spazzy about automatic fees because they don't know what it could pay.
181 2017-05-04 17:43:39	0|gmaxwell|omg what if it pays all my monies!
182 2017-05-04 18:27:31	0|sipa|cfields: a bit ironic... if you build with new glibc on an old kernel, but later run on a new kernel... using getentropy actually improves things
183 2017-05-04 18:33:26	0|cfields|sipa: ironic how?
184 2017-05-04 18:34:59	0|sipa|cfields: in that it would seem that getentropy is never useful on linux
185 2017-05-04 18:35:14	0|sipa|s/ironic/surprising/
186 2017-05-04 18:35:19	0|cfields|ah
187 2017-05-04 18:38:18	0|cfields|well, it basically just forces the syscall to be tried on all systems, when built on something newish. but yea, the path isn't exactly obvious in all cases :)
188 2017-05-04 18:39:01	0|sipa|we could of course just hardcode the SYS_getrandom constant :p
189 2017-05-04 18:39:12	0|cfields|I suppose we could add a weak getrandom() that does the syscall and avoid the loops
190 2017-05-04 18:39:15	0|cfields|haha
191 2017-05-04 18:45:27	0|cfields|sipa: i find the logic harder to follow in your version :\
192 2017-05-04 18:46:58	0|cfields|i'm not sure if we want to cascade like that?
193 2017-05-04 18:53:17	0|sipa|ok, just a suggestion
194 2017-05-04 19:00:49	0|sipa|yow
195 2017-05-04 19:00:56	0|sipa|meeting?
196 2017-05-04 19:01:07	0|sdaftuar|did anyone volunteer to chair?
197 2017-05-04 19:01:13	0|sipa|#startmeeting
198 2017-05-04 19:01:14	0|lightningbot|Meeting started Thu May  4 19:01:13 2017 UTC.  The chair is sipa. Information about MeetBot at http://wiki.debian.org/MeetBot.
199 2017-05-04 19:01:14	0|lightningbot|Useful Commands: #action #agreed #help #info #idea #link #topic.
200 2017-05-04 19:01:24	0|sipa|topics?
201 2017-05-04 19:01:36	0|BlueMatt|oh, wumpus is out?
202 2017-05-04 19:01:41	0|sipa|he is
203 2017-05-04 19:01:45	0|instagibbs|hi
204 2017-05-04 19:01:51	0|BlueMatt|#10337
205 2017-05-04 19:01:52	0|gribble|https://github.com/bitcoin/bitcoin/issues/10337 | Coin Control Dialog is not (very) useful for manual privacy protection · Issue #10337 · bitcoin/bitcoin · GitHub
206 2017-05-04 19:01:56	0|BlueMatt|:(
207 2017-05-04 19:02:06	0|jonasschnelli|Proposed low prio topic: HD restore update / questions
208 2017-05-04 19:02:11	0|sipa|#topic #10337
209 2017-05-04 19:02:12	0|gribble|https://github.com/bitcoin/bitcoin/issues/10337 | Coin Control Dialog is not (very) useful for manual privacy protection · Issue #10337 · bitcoin/bitcoin · GitHub
210 2017-05-04 19:02:22	0|sipa|gmaxwell: ping people?
211 2017-05-04 19:02:30	0|luke-jr|BlueMatt: I agree change shouldn't be grouped as it is, but I don't understand how "received by address" is wrong
212 2017-05-04 19:02:38	0|BlueMatt|turns out the coin control dialog is almost entirely useless
213 2017-05-04 19:03:01	0|jonasschnelli|BlueMatt: entierly useless? I disagree
214 2017-05-04 19:03:09	0|BlueMatt|the "received by address" thing works fine for coins you just received, but if it is a change output it walks back the first input until it finds a non-change output
215 2017-05-04 19:03:11	0|luke-jr|BlueMatt: seems plenty useful to me
216 2017-05-04 19:03:21	0|BlueMatt|so it, essentially, picks an address at random if the output is change
217 2017-05-04 19:03:24	0|luke-jr|BlueMatt: I see different "received by address" for change too
218 2017-05-04 19:03:34	0|BlueMatt|and groups by it, which obviously isnt what you want for privacy
219 2017-05-04 19:03:46	0|BlueMatt|luke-jr: for addresses marked as change in the wallet? no
220 2017-05-04 19:04:05	0|luke-jr|BlueMatt: yes..
221 2017-05-04 19:04:09	0|BlueMatt|for random addresses of yours it should work, but not for addresses via getrawchangeaddress
222 2017-05-04 19:04:23	0|luke-jr|I don't use that RPC, but it works for normal change
223 2017-05-04 19:04:27	0|jtimon|hi
224 2017-05-04 19:04:28	0|BlueMatt|(or, ofc, normal change)
225 2017-05-04 19:05:04	0|BlueMatt|https://github.com/bitcoin/bitcoin/blob/master/src/qt/walletmodel.cpp#L620
226 2017-05-04 19:05:35	0|sipa|it sounds to me like it is doing a mix of "receive by address" and "linked grouping"
227 2017-05-04 19:05:44	0|sipa|both are perhaps useful
228 2017-05-04 19:05:56	0|BlueMatt|more importantly, really, is that I've repeatedly seen the tree mode of the coin picker dialog as the same as listaddressgroupings, which it is clearly not
229 2017-05-04 19:06:06	0|BlueMatt|sipa: well the change-output-results-in-random-grouping thing is kinda strange
230 2017-05-04 19:06:21	0|sipa|right, it shouldn't walk for receive address
231 2017-05-04 19:06:35	0|luke-jr|BlueMatt: I have nfc what that code does, but it *looks* right in the end window :/
232 2017-05-04 19:06:39	0|sipa|and it should alwaya walk (to some deterministic representative) for grouping
233 2017-05-04 19:06:45	0|sipa|*alwaya
234 2017-05-04 19:06:48	0|sipa|*alwayS
235 2017-05-04 19:06:53	0|BlueMatt|*always
236 2017-05-04 19:07:11	0|BlueMatt|but, yea, anyway, I think this should really emulate the grouping rpc
237 2017-05-04 19:07:38	0|BlueMatt|the "where did these coins come from" question is not really useful for anything but coins you just got, in which case they will already be ungrouped
238 2017-05-04 19:07:39	0|sipa|a "received by address" is still useful i think, but it's not the same as grouping
239 2017-05-04 19:07:50	0|kanzure|hi.
240 2017-05-04 19:07:52	0|BlueMatt|yes, but if it is not received directly, it should be "Change"
241 2017-05-04 19:08:03	0|sipa|BlueMatt: seems reasonable to me
242 2017-05-04 19:08:28	0|BlueMatt|anyway, other topics?
243 2017-05-04 19:08:39	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
244 2017-05-04 19:08:47	0|sipa|#topic HD restore update
245 2017-05-04 19:08:57	0|jonasschnelli|#10240
246 2017-05-04 19:08:59	0|gribble|https://github.com/bitcoin/bitcoin/issues/10240 | Add HD wallet auto-restore functionality by jonasschnelli · Pull Request #10240 · bitcoin/bitcoin · GitHub
247 2017-05-04 19:09:14	0|jonasschnelli|I have worked out a solution that seems to work for encrypted and encrypted&pruned wallets.
248 2017-05-04 19:09:24	0|jonasschnelli|It can halt the sync / validation progress.
249 2017-05-04 19:09:34	0|jonasschnelli|But,.. I'm not sure what gap limit and default keypool size we should use
250 2017-05-04 19:09:35	0|BlueMatt|(to generate new keys)
251 2017-05-04 19:09:39	0|jonasschnelli|100 and 20 seems very little
252 2017-05-04 19:10:11	0|sipa|BlueMatt: that requires non-hardened derivation
253 2017-05-04 19:10:19	0|jonasschnelli|BlueMatt: IMO encryption (private key wallets) with public key derivation should be avoided
254 2017-05-04 19:10:32	0|BlueMatt|sipa: yes, is that a concern?
255 2017-05-04 19:10:41	0|sipa|BlueMatt: yes, we have a dumpprivkey command
256 2017-05-04 19:10:55	0|jonasschnelli|We should aim – longterm – for watchonly-hd (see NicolasDorier workd) and add a signing-agent ala gpg / ssh
257 2017-05-04 19:10:56	0|sipa|leaking one private key means you leak your whole walley
258 2017-05-04 19:11:10	0|BlueMatt|jonasschnelli: why? your list of funds is already public to the encrypted wallet holders? that wouldnt change?
259 2017-05-04 19:11:13	0|jonasschnelli|But that is alredy the next topic. :)
260 2017-05-04 19:11:25	0|luke-jr|sipa: dumpprivkey isn't supposed to be safe
261 2017-05-04 19:11:32	0|sipa|luke-jr: i know
262 2017-05-04 19:11:36	0|luke-jr|but we could make it fail for non-hardened keys?
263 2017-05-04 19:11:43	0|sipa|luke-jr: but it breaks expectations
264 2017-05-04 19:11:52	0|sipa|people have a mental model about how it works
265 2017-05-04 19:12:15	0|BlueMatt|sipa: maybe I'm confused on the format of HD...seems you can build a list of derivation secrets which is based on a non-encrypted private key which is unexportable?
266 2017-05-04 19:12:19	0|BlueMatt|sipa: and then that would not be the case
267 2017-05-04 19:12:21	0|jonasschnelli|Yes. But I vaguely remember that we once said we don't want to mix private-key wallets with public key derivation... and this makes very much sense to me
268 2017-05-04 19:12:32	0|BlueMatt|(dont know the format of HD, but we could do something else if its way better)
269 2017-05-04 19:12:48	0|sipa|BlueMatt: if you have the parent public key + private child key, you can compute all private child keys
270 2017-05-04 19:12:57	0|jonasschnelli|If we would do child pubkey derivation, keypools could be removed (at least for HD)
271 2017-05-04 19:13:05	0|sipa|BlueMatt: that is an inevitable weakness of EC based derivation
272 2017-05-04 19:13:19	0|jonasschnelli|What sipa said
273 2017-05-04 19:13:21	0|sipa|BlueMatt: and it is reason why bip32 has hardened keys
274 2017-05-04 19:13:30	0|sipa|which core uses
275 2017-05-04 19:13:31	0|BlueMatt|sipa: even if your list of privkeys is based on adding a new random value to the previous privkey where the new random value is just a hashchain of a private secret?
276 2017-05-04 19:13:53	0|sipa|BlueMatt: then you lose public derivation
277 2017-05-04 19:14:13	0|sipa|(the ability to compute child pubkeys without knowing the parent privkey)
278 2017-05-04 19:14:25	0|BlueMatt|lets take this offline
279 2017-05-04 19:14:29	0|sipa|sounds good
280 2017-05-04 19:14:36	0|jonasschnelli|back to the topic: what GAP limit should we enforce by default?
281 2017-05-04 19:14:45	0|BlueMatt|1000
282 2017-05-04 19:14:52	0|BlueMatt|default keypool 10k
283 2017-05-04 19:14:55	0|jonasschnelli|Yeah.. I like this
284 2017-05-04 19:15:01	0|jonasschnelli|But only for encrypted wallets?
285 2017-05-04 19:15:09	0|jonasschnelli|IMO we should (only encrypted)
286 2017-05-04 19:15:10	0|sipa|but in general i believe that most cases where the public derivation is wanted, just use huge precomputed key lists
287 2017-05-04 19:15:14	0|jonasschnelli|non encrypted can say with 100
288 2017-05-04 19:15:18	0|sipa|jonasschnelli: meh
289 2017-05-04 19:15:26	0|sipa|why bother differentiating?
290 2017-05-04 19:15:28	0|gmaxwell|why only encryption? I don't think that makes sense.
291 2017-05-04 19:15:36	0|jonasschnelli|True, the gap limit must be the same.. right
292 2017-05-04 19:15:42	0|gmaxwell|less complexity please, and keys are cheap.
293 2017-05-04 19:15:42	0|jonasschnelli|sry
294 2017-05-04 19:15:59	0|jonasschnelli|Okay, ... I'll bump it to 1000
295 2017-05-04 19:16:10	0|bitcoin-git|[13bitcoin] 15jtimon opened pull request #10339: Optimization: Calculate block hash less times (06master...06b15-optimization-blockhash) 02https://github.com/bitcoin/bitcoin/pull/10339
296 2017-05-04 19:16:29	0|jonasschnelli|Next question: the tests are mostly running with a keypool size of 1... so the gap limit stuff is only enforced for the new test in #10240
297 2017-05-04 19:16:30	0|gribble|https://github.com/bitcoin/bitcoin/issues/10240 | Add HD wallet auto-restore functionality by jonasschnelli · Pull Request #10240 · bitcoin/bitcoin · GitHub
298 2017-05-04 19:16:35	0|jonasschnelli|Is that a problem?
299 2017-05-04 19:16:36	0|gmaxwell|jtimon: thanks for working on that.
300 2017-05-04 19:17:22	0|jonasschnelli|(but maybe take the test question offline).
301 2017-05-04 19:17:37	0|jonasschnelli|Well,.. keypool size is answered... all good. Thanks for reviews. :p
302 2017-05-04 19:17:51	0|jtimon|gmaxwell: no problem, thank you for pointing it out the other day
303 2017-05-04 19:19:09	0|jonasschnelli|other topics?
304 2017-05-04 19:19:43	0|BlueMatt|review, review, review, review, review :)
305 2017-05-04 19:19:54	0|sipa|yes, let's go over priority reviews
306 2017-05-04 19:20:05	0|jonasschnelli|https://github.com/bitcoin/bitcoin/projects/8
307 2017-05-04 19:20:05	0|sipa|#topic review requests
308 2017-05-04 19:20:17	0|Chris_Stewart_5|Can #9980 be merged? Might be some what controversial
309 2017-05-04 19:20:24	0|gribble|https://github.com/bitcoin/bitcoin/issues/9980 | Fix mem access violation merkleblock by Christewart · Pull Request #9980 · bitcoin/bitcoin · GitHub
310 2017-05-04 19:20:35	0|BlueMatt|Chris_Stewart_5: we're super backlogged on review right now :(
311 2017-05-04 19:20:41	0|Chris_Stewart_5|I thought jnewberry did a good job with the comments
312 2017-05-04 19:20:56	0|jonasschnelli|Chris_Stewart_5: I can't see an tested or untested ACK there
313 2017-05-04 19:21:10	0|BlueMatt|like, super backlogged-not-gonna-get-everything-for-0.15 backlogged :(
314 2017-05-04 19:21:31	0|Chris_Stewart_5|That's fine :-). Thought I would bring it up since asking for topics
315 2017-05-04 19:21:39	0|BlueMatt|we can add it to the review heap
316 2017-05-04 19:22:10	0|BlueMatt|can we remove #8694 until it gets fixed+rebased?
317 2017-05-04 19:22:13	0|gribble|https://github.com/bitcoin/bitcoin/issues/8694 | Basic multiwallet support by luke-jr · Pull Request #8694 · bitcoin/bitcoin · GitHub
318 2017-05-04 19:22:25	0|BlueMatt|it seems to have been in a constant state of not-reviewable since it was added to the "high priority for review"
319 2017-05-04 19:22:26	0|instagibbs|sorry, when it 0.15 feature freeze
320 2017-05-04 19:22:32	0|jonasschnelli|luke-jr: plans to rebase it?
321 2017-05-04 19:22:50	0|BlueMatt|instagibbs: 2017-07-16
322 2017-05-04 19:22:58	0|instagibbs|eep, ok
323 2017-05-04 19:23:25	0|jonasschnelli|I though MW was one of the high profiled features targets for 0.15..
324 2017-05-04 19:23:27	0|jtimon|is there anything else that needs to be done for #9494 ?
325 2017-05-04 19:23:28	0|gribble|https://github.com/bitcoin/bitcoin/issues/9494 | Introduce an ArgsManager class encapsulating cs_args, mapArgs and mapMultiArgs by jtimon · Pull Request #9494 · bitcoin/bitcoin · GitHub
326 2017-05-04 19:23:28	0|luke-jr|I just did? :/
327 2017-05-04 19:23:29	0|BlueMatt|sdaftuar: asks for #9208
328 2017-05-04 19:23:31	0|gribble|https://github.com/bitcoin/bitcoin/issues/9208 | Improve DisconnectTip performance by sdaftuar · Pull Request #9208 · bitcoin/bitcoin · GitHub
329 2017-05-04 19:23:56	0|luke-jr|not really sure how to address the mapMultiArgs thing
330 2017-05-04 19:24:04	0|luke-jr|besides refactoring everything using it
331 2017-05-04 19:24:06	0|jonasschnelli|I add 9208 to the review-prio-list
332 2017-05-04 19:24:09	0|jtimon|on the list we have #8855 from me, which remains being simple to review
333 2017-05-04 19:24:11	0|gribble|https://github.com/bitcoin/bitcoin/issues/8855 | Use a proper factory for creating chainparams by jtimon · Pull Request #8855 · bitcoin/bitcoin · GitHub
334 2017-05-04 19:24:27	0|BlueMatt|jonasschnelli: you already have one
335 2017-05-04 19:24:39	0|gmaxwell|I really would like to see us get per-txout + atomic merged sooner rather than later, so we can get more testing time on the code.
336 2017-05-04 19:24:39	0|jonasschnelli|BlueMatt: It's sdaftuar. :P
337 2017-05-04 19:24:52	0|BlueMatt|ohoh
338 2017-05-04 19:24:55	0|BlueMatt|yes, sorry
339 2017-05-04 19:24:55	0|jtimon|luke-jr: refactoring everything that uses mapMultiArgs is what #9494 does
340 2017-05-04 19:24:57	0|gribble|https://github.com/bitcoin/bitcoin/issues/9494 | Introduce an ArgsManager class encapsulating cs_args, mapArgs and mapMultiArgs by jtimon · Pull Request #9494 · bitcoin/bitcoin · GitHub
341 2017-05-04 19:24:57	0|jonasschnelli|No worries... I protect the ratio. :)
342 2017-05-04 19:25:05	0|BlueMatt|jonasschnelli: I read that as "add for me", not "added"
343 2017-05-04 19:25:06	0|luke-jr|jtimon: k, I'll take a look
344 2017-05-04 19:25:09	0|instagibbs|jtimon, will review
345 2017-05-04 19:25:18	0|cfields|gmaxwell: agreed
346 2017-05-04 19:25:20	0|jtimon|awesome, thanks
347 2017-05-04 19:25:58	0|sipa|let's put 9494 on the list this week
348 2017-05-04 19:26:04	0|BlueMatt|either way, #8694 probably needs deleted
349 2017-05-04 19:26:06	0|gribble|https://github.com/bitcoin/bitcoin/issues/8694 | Basic multiwallet support by luke-jr · Pull Request #8694 · bitcoin/bitcoin · GitHub
350 2017-05-04 19:26:11	0|jonasschnelli|I guess soon we have to introduce a review/open-new-PR ratio (only allowed to open a PR is you have carefully reviewed other PRs)
351 2017-05-04 19:26:11	0|luke-jr|BlueMatt: why?
352 2017-05-04 19:26:20	0|BlueMatt|luke-jr: because its not reviewable?
353 2017-05-04 19:26:20	0|luke-jr|oh, from the list only, ok
354 2017-05-04 19:26:25	0|BlueMatt|yeayea
355 2017-05-04 19:26:34	0|sipa|i want to keep 8694 as a priority for 0.15
356 2017-05-04 19:26:51	0|jonasschnelli|#8855 is already in the list by jtimon
357 2017-05-04 19:26:52	0|gribble|https://github.com/bitcoin/bitcoin/issues/8855 | Use a proper factory for creating chainparams by jtimon · Pull Request #8855 · bitcoin/bitcoin · GitHub
358 2017-05-04 19:26:52	0|jtimon|sipa: there's already one from me on the list
359 2017-05-04 19:27:08	0|BlueMatt|sipa: I'm just saying gotta remove it from the list because its not reviewable atm, even if we want it for 0.15
360 2017-05-04 19:27:23	0|sipa|BlueMatt: agree
361 2017-05-04 19:27:44	0|sipa|jtimon: let's focus on the args refactoring first... it seems that that will more easily go stale
362 2017-05-04 19:27:50	0|luke-jr|0.15 and priority-review are two diff lists for a reason; let's do jtimon's PR first
363 2017-05-04 19:28:09	0|sipa|luke-jr: agree
364 2017-05-04 19:28:59	0|sipa|any further topics?
365 2017-05-04 19:29:14	0|gmaxwell|sipa: where are things with per-txo?
366 2017-05-04 19:29:31	0|jonasschnelli|#10195
367 2017-05-04 19:29:34	0|gribble|https://github.com/bitcoin/bitcoin/issues/10195 | Switch chainstate db and cache to per-txout model by sipa · Pull Request #10195 · bitcoin/bitcoin · GitHub
368 2017-05-04 19:29:38	0|BlueMatt|gmaxwell: needs more review, could use side-by-side benchmarks incl: memory usage, disk usage, performance numbers
369 2017-05-04 19:29:50	0|sipa|yes, i'm planning to do benchmarks
370 2017-05-04 19:30:19	0|gmaxwell|BlueMatt: "much faster"
371 2017-05-04 19:30:29	0|sipa|other todos are better upgrade code (with a fancy progress bar...), that doesn't leave gigabytes of uncompacted data in the chainstate
372 2017-05-04 19:30:41	0|sipa|but i believe it is functionally complete and tested
373 2017-05-04 19:31:45	0|BlueMatt|alright, if there are no more topics I'd emplore people to keep reviewing the big 0.15 things, since it looks like we're gonna slip a few, which is sad
374 2017-05-04 19:31:46	0|sipa|it seems to make the chainstate some 20% larger
375 2017-05-04 19:32:18	0|sipa|i'll report numbers on the PR, no need to discuss here
376 2017-05-04 19:32:25	0|sipa|BlueMatt: ack
377 2017-05-04 19:32:32	0|lightningbot|Log:            http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-05-04-19.01.log.html
378 2017-05-04 19:32:32	0|lightningbot|Meeting ended Thu May  4 19:32:31 2017 UTC.  Information about MeetBot at http://wiki.debian.org/MeetBot . (v 0.1.4)
379 2017-05-04 19:32:32	0|lightningbot|Minutes:        http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-05-04-19.01.html
380 2017-05-04 19:32:32	0|lightningbot|Minutes (text): http://www.erisian.com.au/meetbot/bitcoin-core-dev/2017/bitcoin-core-dev.2017-05-04-19.01.txt
381 2017-05-04 19:32:32	0|sipa|#endmeeting
382 2017-05-04 19:32:39	0|sipa|#topic lunch
383 2017-05-04 19:32:54	0|luke-jr|._.
384 2017-05-04 19:33:09	0|jonasschnelli|sipa: those 120% happens also for the in-memory dbcache?
385 2017-05-04 19:33:16	0|jonasschnelli|*those +20%
386 2017-05-04 19:33:18	0|sipa|jonasschnelli: in memory it's much more
387 2017-05-04 19:33:31	0|sipa|but that's not a fair comparison
388 2017-05-04 19:33:46	0|jtimon|of "preparation PRs" for 10195: 10249 10250 have been merged, 10248 has not, still 24 commits...
389 2017-05-04 19:33:58	0|sipa|as it's much more granular, fewer cached outputs may have better performamce
390 2017-05-04 19:34:12	0|sipa|#10248
391 2017-05-04 19:34:13	0|gribble|https://github.com/bitcoin/bitcoin/issues/10248 | Introduce CHashVerifier to hash while deserializing by sipa · Pull Request #10248 · bitcoin/bitcoin · GitHub
392 2017-05-04 19:34:31	0|sipa|jtimon: most are small, but yes, it's a big change
393 2017-05-04 19:35:08	0|BlueMatt|also #10199 is pretty critical for 0.15
394 2017-05-04 19:35:10	0|gribble|https://github.com/bitcoin/bitcoin/issues/10199 | Better fee estimates by morcos · Pull Request #10199 · bitcoin/bitcoin · GitHub
395 2017-05-04 19:35:16	0|jtimon|yeah, I was just wondering if further preparations could be separated, to make review less scary, but in the end they will be the same commits
396 2017-05-04 19:35:24	0|cfields|I think #10189 is ready to go, and it's blocking a few people. jtimon at least.
397 2017-05-04 19:35:26	0|gribble|https://github.com/bitcoin/bitcoin/issues/10189 | devtools/net: add a verifier for scriptable changes. Use it to make CNode::id private. by theuni · Pull Request #10189 · bitcoin/bitcoin · GitHub
398 2017-05-04 19:35:55	0|sipa|i could use 10189 for some of the changes in 10195 as well
399 2017-05-04 19:38:07	0|jtimon|cfields: not really blocking (scripted-diff PRs can be done without the script for travis being merged and review should be similar), but yeah, I think it's ready to go too. I drafted some documentation, maybe that helps: https://0bin.net/paste/mTtIoaSoedZ3al-y#dWgD7aBKElhKofVS5nHJBvuvlDLRFmgRy8a03KroOFM
400 2017-05-04 19:41:39	0|sipa|jtimon: looks like something for developer-notes.md
401 2017-05-04 19:41:40	0|cfields|jtimon: thanks!
402 2017-05-04 19:42:00	0|cfields|jtimon: not sure what 3) is saying through
403 2017-05-04 19:42:22	0|jtimon|sipa: right, well, cfields feel free to use whatever parts of it make sense to you
404 2017-05-04 19:42:57	0|cfields|jtimon: lgtm, I'm just not following that one part
405 2017-05-04 19:43:54	0|jtimon|well, I needed point 3 because I was some times editing the script trying to do more specific things, so I didn't want to keep changes done by an earlier version of the script or by me manually because I stupid and I forget the commit should only do what the script does and nothing else or something
406 2017-05-04 19:44:14	0|jtimon|maybe we can keep it out, I don't know
407 2017-05-04 19:45:22	0|cfields|jtimon: oh, i see.
408 2017-05-04 19:47:04	0|cfields|jtimon: maybe just say "1) Writer: apply your script to a clean tree and commit with an appropriate message:"
409 2017-05-04 19:47:31	0|cfields|then list the other steps. I was confused because you mention committing before running the script :)
410 2017-05-04 19:50:27	0|praxeology|While upgrading the chainstate to keying on id+o.ix, does it pretty much freeze the init process of the program?
411 2017-05-04 19:50:37	0|sipa|praxeology: yeah...
412 2017-05-04 19:50:46	0|sipa|praxeology: and it takes a while
413 2017-05-04 19:51:03	0|praxeology|Sounds like you are using the same db, not trying to make a new one?
414 2017-05-04 19:51:09	0|sipa|indeed
415 2017-05-04 19:51:11	0|jtimon|cfields: yeah that may do it without explicitly saying the "git checkout ." trick
416 2017-05-04 19:51:32	0|sipa|praxeology: it's an in-place upgrade, and it is interruotible
417 2017-05-04 19:53:09	0|praxeology|interruptible?  so you make the new entries, then delete the old ones?  and if interrupt, you delete the new ones?
418 2017-05-04 19:56:11	0|sipa|it adds and deletes simultaneously, atomically
419 2017-05-04 19:56:20	0|sipa|you can just kill it at any point
420 2017-05-04 19:57:25	0|praxeology|so the new code can handle lookup and  editing of both kinds of entries?
421 2017-05-04 20:00:05	0|praxeology|I guess it doesn't matter that much if  I understand how it works if I'm not going to help code review/test/code the progress UI
422 2017-05-04 20:00:48	0|sipa|praxeology: no, at startup it just iterates through all old entries and converts them
423 2017-05-04 20:01:03	0|sipa|if you interrupt during that conversion, you need to continue later
424 2017-05-04 20:01:18	0|sipa|but by interruptible i mean you don't lose progress from doing that
425 2017-05-04 20:01:56	0|praxeology|"need to continue later" or else your client doesn't process new blocks/transactions?
426 2017-05-04 20:02:22	0|gmaxwell|when the software restarts it will continue where it left off.
427 2017-05-04 20:02:34	0|gmaxwell|this is all in the init process before it even makes any connections to anything.
428 2017-05-04 20:02:49	0|sipa|praxeology: by interrupt i mean you quit during the conversion
429 2017-05-04 20:02:59	0|sipa|praxeology: the node can't do anything until the conversion is complete
430 2017-05-04 20:03:09	0|praxeology|ok I see
431 2017-05-04 20:04:14	0|sipa|it would be possible to do the conversion on the fly, but it would result in a long term slowdown
432 2017-05-04 20:04:38	0|praxeology|also more complicated code that only need be used once
433 2017-05-04 20:05:27	0|praxeology|Are you making the upgrade optional?
434 2017-05-04 20:05:30	0|sipa|no
435 2017-05-04 20:06:07	0|sipa|the new database code cannot read or write the old format anymore, and supporting that would be complicated and slow
436 2017-05-04 20:06:11	0|gmaxwell|It's optional: don't use newer software if you don't want it upgrading.
437 2017-05-04 20:06:14	0|gmaxwell|:)
438 2017-05-04 20:06:24	0|praxeology|Alrighty... I can see why this would be a delayed commit to the main branch.
439 2017-05-04 20:08:04	0|praxeology|I wish I could clone myself
440 2017-05-04 20:08:16	0|gmaxwell|praxeology: we do not do development in master. (few large OSS projects that use Git do)-- things that go into master are at least believed to be done and more or less releasable.
441 2017-05-04 20:09:32	0|praxeology|Yea I know.  I am saying that I see why this feature is/may be pushed back to later and later release versions due to the requirement of locking up the node for... how long does it take to process that 2GB of data?
442 2017-05-04 20:10:19	0|sipa|maybe an hour on slow hardware
443 2017-05-04 20:10:30	0|sipa|slow disk, mostly
444 2017-05-04 20:10:48	0|gmaxwell|15 minutes minutes on fast hardware, I'd guess?
445 2017-05-04 20:10:53	0|sipa|yeah
446 2017-05-04 20:10:54	0|praxeology|and then is leveldb optimized to handle such a db transformation?  And does it compress well after deleting all the old entries?
447 2017-05-04 20:11:24	0|sipa|nope, it seems
448 2017-05-04 20:11:31	0|sipa|but there is an explicit call to ask it to compact
449 2017-05-04 20:11:36	0|sipa|which i'll experiment with
450 2017-05-04 20:11:48	0|praxeology|Why not create a new db?
451 2017-05-04 20:12:05	0|sipa|Why create a new db?
452 2017-05-04 20:12:23	0|praxeology|if leveldb has bad performance w/ this sort of transformation
453 2017-05-04 20:12:59	0|sipa|calling db.CompactRange(); is much simpler than dealing with multiple database at once during the conversion
454 2017-05-04 20:13:28	0|praxeology|because you want atomic?
455 2017-05-04 20:13:32	0|gmaxwell|praxeology: it doesn't have bad performance.
456 2017-05-04 20:13:40	0|sipa|praxeology: no...
457 2017-05-04 20:14:00	0|sipa|praxeology: the argument you're bringing up in favor of multiple databases is because leveldb doesn't deal well with the conversion
458 2017-05-04 20:14:15	0|sipa|i offer an alternative to having multiple databases at once which also deals well with the conversion
459 2017-05-04 20:14:27	0|sipa|which is simpler
460 2017-05-04 20:14:31	0|praxeology|ok
461 2017-05-04 20:14:43	0|gmaxwell|You need to ask it to free the space or its very lazy about it, thats all.
462 2017-05-04 20:15:45	0|gmaxwell|Unrelated, while watching a debug=1 logging rescan.. why is it battering out leveldb activity during rescan?
463 2017-05-04 20:16:01	0|sipa|gmaxwell: huh
464 2017-05-04 20:16:56	0|gmaxwell|https://0bin.net/paste/VZ72NOA3G8n-lgpF#FXx9b3te-4BbUbBH7BfGjjpXOOWUiJmL1YToCU6HRHG
465 2017-05-04 20:17:17	0|praxeology|ah, so CompactRange() will just clean up a sorted key range of the db?  Which could be done incrementally along with a process that alters the format with a sorted key iterator
466 2017-05-04 20:17:23	0|gmaxwell|the repeatedbash is the some instrumentation I added to count repeated hashings of the block header.. every 6 or so of those is processing a new block.
467 2017-05-04 20:17:40	0|gmaxwell|praxeology: ya, thats what he's going to have it do now.
468 2017-05-04 20:17:59	0|sipa|gmaxwell: that looks like just background compactions
469 2017-05-04 20:18:05	0|gmaxwell|e.g. every time the leading byte of the key changes, compact.
470 2017-05-04 20:21:10	0|praxeology|I've heard rumors that leveldb is unreliable.  Is that still true?
471 2017-05-04 20:21:50	0|sipa|praxeology: see my answer here https://bitcoin.stackexchange.com/a/51446/208
472 2017-05-04 20:25:07	0|gmaxwell|praxeology: a lot of those rumors are a result of several things (1) (primary) leveldb has agressive checking and will actually catch lower level corruption that other things miss,  (2) there there is no windows filesystem interface for leveldb and the one we previously used was incorrect, leading to unnecessary corruption with unclean shutdowns, (3) there is a usenix paper that panned leveldb f
473 2017-05-04 20:25:13	0|gmaxwell|or not handling crashes given worst case behavior of the the VFS contract that they inferred.
474 2017-05-04 20:25:45	0|gmaxwell|(3) appears to be completely inconsequential in practice, e.g. I left systems running for months in constant high load plus crash loops and it always recovered.
475 2017-05-04 20:26:19	0|gmaxwell|When we do get reliablity reports from users we sometimes investigate them in detail, and frequently find actual disk corruption.
476 2017-05-04 20:26:37	0|gmaxwell|Unfortunately, there is a lot of windows-grade hardware out there.
477 2017-05-04 20:26:38	0|sipa|or overheating CPUs
478 2017-05-04 20:27:03	0|praxeology|I would guess memory failure would be more common that cpu failure in my experience
479 2017-05-04 20:27:32	0|gmaxwell|yes, esp laptops, many brands of laptop cannot be run at full speed for an hour at a time without actually ending up with apparent 'memory' corruption. (presumably cpu caches).
480 2017-05-04 20:28:01	0|sipa|praxeology: i would think so too, but that's not very consistent with what we're seeing (for example, for some people, running with -par=1 actually improves things)
481 2017-05-04 20:29:09	0|gmaxwell|well it could be ram which behaves more poorly with the cpu taxing the power rails and making things hot. who knows.
482 2017-05-04 20:30:23	0|praxeology|I've had terrible experience with cheap consumer memory from lower cost computer brands, when operating under load
483 2017-05-04 20:30:54	0|sipa|i think we're seeing fewer db corruptions than a few years ago
484 2017-05-04 20:31:07	0|praxeology|Are you saying there are still issues with crash recovery in windows?
485 2017-05-04 20:31:08	0|sipa|that may be due to better windows env support in leveldb
486 2017-05-04 20:31:20	0|sipa|or due to people running on better hardware
487 2017-05-04 20:31:49	0|sipa|or just fewer people running bitcoin core on windows
488 2017-05-04 20:32:09	0|sipa|not all corruption reports are on windows, to be clear
489 2017-05-04 20:32:17	0|sipa|but historically those were a majority
490 2017-05-04 20:32:53	0|praxeology|I have like 3 windows machines running core on windows, and they haven't corrupted.  But I have all very reliable machines that don't crash or shutdown unexpectedly
491 2017-05-04 20:34:47	0|sipa|praxeology: ha, that's good to know
492 2017-05-04 20:35:01	0|praxeology|Alright well best of luck w/ the utxo index change.  I'd consider help testing or maybe doing some UI work on it.  But I am very busy
493 2017-05-04 20:35:13	0|sipa|thanks regardless!
494 2017-05-04 20:36:42	0|praxeology|Maybe if you point me to the code and ask me for help directly I'll add it to my todo list and maybe I will do it if my priorities make it there.
495 2017-05-04 20:47:52	0|BlueMatt|jonasschnelli: you might kill me for #10340, sorry :/
496 2017-05-04 20:47:53	0|gribble|https://github.com/bitcoin/bitcoin/issues/10340 | Add harmless missing cs_wallet lock in qt CoinControlDialog by TheBlueMatt · Pull Request #10340 · bitcoin/bitcoin · GitHub
497 2017-05-04 20:47:54	0|bitcoin-git|[13bitcoin] 15TheBlueMatt opened pull request #10340: Add harmless missing cs_wallet lock in qt CoinControlDialog (06master...062017-05-fix-mapwallet-zap-runtime) 02https://github.com/bitcoin/bitcoin/pull/10340
498 2017-05-04 21:57:30	0|luke-jr|hmm, master doesn't seem to build for me
499 2017-05-04 21:58:27	0|sipa|what error?
500 2017-05-04 22:00:26	0|luke-jr|wallet/rpcwallet.cpp:732:98: error: taking address of temporary [-fpermissive]
501 2017-05-04 22:02:14	0|luke-jr|I'm not entirely clear why this shouldn't be okay
502 2017-05-04 22:02:23	0|luke-jr|it's returning a reference
503 2017-05-04 22:03:24	0|gmaxwell|you can't return a reference to something on the stack.. which is what that sounds like?
504 2017-05-04 22:04:00	0|luke-jr|no, the univalue method is returning a reference to a member
505 2017-05-04 22:04:01	0|luke-jr|const std::string* account = request.params[0].get_str() != "*" ? &request.params[0].get_str() : nullptr;
506 2017-05-04 22:04:11	0|luke-jr|GCC doesn't like us taking the pointer of it for some reason
507 2017-05-04 22:04:19	0|sipa|that's invalid
508 2017-05-04 22:04:28	0|sipa|it's taking a pointer to a temporary
509 2017-05-04 22:04:42	0|sipa|ah, wait
510 2017-05-04 22:05:36	0|luke-jr|FWIW, this didn't fail until I disabled ccache, so I suspect it's a new error
511 2017-05-04 22:05:54	0|luke-jr|GCC 5.4
512 2017-05-04 22:37:35	0|ryanofsky|i think i added the line causing that error, but i also don't see why it's an error