1 2016-12-02 00:07:41	0|bitcoin-git|[13bitcoin] 15sipa pushed 6 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/ad826b3df9f7...dc6dee41f7cf
  2 2016-12-02 00:07:42	0|bitcoin-git|13bitcoin/06master 144a6b1f3 15Matt Corallo: Expose AcceptBlockHeader through main.h
  3 2016-12-02 00:07:42	0|bitcoin-git|13bitcoin/06master 1463fd101 15Matt Corallo: Split ::HEADERS processing into two separate cs_main locks...
  4 2016-12-02 00:07:43	0|bitcoin-git|13bitcoin/06master 14a8b936d 15Matt Corallo: Use exposed ProcessNewBlockHeaders from ProcessMessages
  5 2016-12-02 00:07:52	0|bitcoin-git|[13bitcoin] 15sipa closed pull request #9183: Final Preparation for main.cpp Split (06master...06net_processing_5) 02https://github.com/bitcoin/bitcoin/pull/9183
  6 2016-12-02 00:10:05	0|bitcoin-git|[13bitcoin] 15TheBlueMatt opened pull request #9260: Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) (06master...06net_processing_file) 02https://github.com/bitcoin/bitcoin/pull/9260
  7 2016-12-02 00:11:34	0|sipa|BlueMatt: haha
  8 2016-12-02 00:11:50	0|BlueMatt|did you see the pr text?
  9 2016-12-02 00:12:30	0|sipa|yes
 10 2016-12-02 00:16:59	0|bitcoin272|hey guys I'm curious, why was 25 chosen for the ancestor count?
 11 2016-12-02 00:18:26	0|BlueMatt|ugh, git isnt smart enough to realize when you rename a file and then create a dumb #include "newfile.h" is a move :(
 12 2016-12-02 00:19:15	0|Eliel|BlueMatt: it should understand it if you do the rename with git mv
 13 2016-12-02 00:19:47	0|sipa|BlueMatt: where does it not realize this?
 14 2016-12-02 00:20:02	0|BlueMatt|main.h -> validation.h; main.h == #include "validation.h"
 15 2016-12-02 00:20:07	0|BlueMatt|because i dont want to touch literally every file
 16 2016-12-02 00:20:39	0|sipa|i don't understand what the problem is
 17 2016-12-02 00:21:09	0|BlueMatt|will make rebase of ~everything impossible
 18 2016-12-02 00:21:21	0|BlueMatt|git is smart when rebasing/merging across moves otherwise
 19 2016-12-02 00:22:02	0|BlueMatt|Eliel: no, it doesnt cache that info
 20 2016-12-02 00:22:07	0|Eliel|if it won't understand it in a single commit, try first renaming and then creating the new file.
 21 2016-12-02 00:22:12	0|BlueMatt|it regenerates it itself, so there is no place for it to figure it out
 22 2016-12-02 00:22:15	0|Eliel|in two commits
 23 2016-12-02 00:22:16	0|BlueMatt|yea
 24 2016-12-02 00:41:57	0|sipa|BlueMatt: does it still do this tracking after a merge commit is introduced around the two commits?
 25 2016-12-02 00:42:03	0|sipa|maybe it treats it as one then
 26 2016-12-02 00:42:17	0|BlueMatt|sipa: I have no idea...
 27 2016-12-02 00:42:38	0|gmaxwell|bitcoin272: measurements on the actual network, combined with the compute time created for longer chains (They're more expensive to process).
 28 2016-12-02 00:42:58	0|sipa|BlueMatt: can you check? if not, it's probably not worth deviating from the "every commit needs to compile and run tests" policy
 29 2016-12-02 00:43:06	0|BlueMatt|willdo
 30 2016-12-02 00:43:13	0|sipa|actually, i'll check myself - i want to know
 31 2016-12-02 00:43:29	0|BlueMatt|heh, ok
 32 2016-12-02 00:44:01	0|sipa|good
 33 2016-12-02 00:45:04	0|BlueMatt|lots of them, it looks like :(
 34 2016-12-02 00:45:45	0|sipa|BlueMatt: doesn't seem to work
 35 2016-12-02 00:45:52	0|BlueMatt|across a merge?
 36 2016-12-02 00:45:56	0|BlueMatt|damn git
 37 2016-12-02 00:46:22	0|BlueMatt|does rebase work at all across a rename? merge does, but rebase might now
 38 2016-12-02 00:46:25	0|BlueMatt|not, actually
 39 2016-12-02 00:46:28	0|Eliel|maybe you can make it a symlink instead?
 40 2016-12-02 00:46:39	0|BlueMatt|eww
 41 2016-12-02 00:46:49	0|BlueMatt|I think i might rather sed all the files
 42 2016-12-02 00:46:52	0|sipa|i tried rebasing #8580 on top of a merged 9260 (with --no-ff)
 43 2016-12-02 00:46:54	0|gribble|https://github.com/bitcoin/bitcoin/issues/8580 | Make CTransaction actually immutable by sipa · Pull Request #8580 · bitcoin/bitcoin · GitHub
 44 2016-12-02 00:46:58	0|sipa|and it conflicts in main.h
 45 2016-12-02 00:47:01	0|BlueMatt|damn
 46 2016-12-02 00:47:17	0|sipa|like... the whole file is a conflict block
 47 2016-12-02 00:47:20	0|BlueMatt|ok, well I guess its gonna be rebase-hell either way....
 48 2016-12-02 00:47:36	0|sipa|we could choose to leave one of the two named main.h/.cpp
 49 2016-12-02 00:47:43	0|BlueMatt|yea...
 50 2016-12-02 00:47:54	0|sipa|which at least would make rebases that touch the not-moved-out part work
 51 2016-12-02 00:48:00	0|BlueMatt|i mean could leave it as main.h and change the #define in main.h to VALIDATION and add a TODO thats like "move this shits"
 52 2016-12-02 00:48:26	0|BlueMatt|then do it like when we close merge window so that its easier
 53 2016-12-02 00:48:45	0|Eliel|would symlink cause more trouble than it'd solve?
 54 2016-12-02 00:48:47	0|BlueMatt|alternatively, I could actually go sed the entire codebase
 55 2016-12-02 00:49:00	0|BlueMatt|Eliel: I dont think it'd cause trouble since we have the #define guards
 56 2016-12-02 00:49:03	0|BlueMatt|but its super ugly
 57 2016-12-02 00:49:14	0|BlueMatt|(and, actually, git might not track that, either?)
 58 2016-12-02 00:49:21	0|Eliel|right, true
 59 2016-12-02 00:49:41	0|Eliel|but yes, I think sedding the whole codebase is best
 60 2016-12-02 00:49:44	0|sipa|validation.h is much larger than net_processing.h, so i'd suggest having main.h and net_processing.h, rather than validation.h and main.h
 61 2016-12-02 00:49:58	0|BlueMatt|yea
 62 2016-12-02 00:52:14	0|sipa|so just remove the last two commits?
 63 2016-12-02 00:53:27	0|BlueMatt|sipa: I went ahead and did as you suggested and left main.cpp moved to validation.cpp, and just added a TODO to main.h to move it
 64 2016-12-02 00:54:32	0|sipa|ah, i'd just have left validation.cpp to be main.cpp
 65 2016-12-02 00:54:47	0|sipa|that move can easily be do at the same time as the .h rename
 66 2016-12-02 00:55:39	0|BlueMatt|welllll....i mean sed wont cuase (m)any rebase issues........
 67 2016-12-02 00:56:25	0|sipa|yes, but it's a weird state to have main.h but validation.cpp
 68 2016-12-02 00:56:33	0|sipa|and there is no need to that move early
 69 2016-12-02 00:56:57	0|BlueMatt|:p
 70 2016-12-02 00:56:57	0|BlueMatt|there is also no need to wait on the wholesale main/validation move/sed
 71 2016-12-02 00:57:17	0|sipa|well, so either do the whole thing now, or not at all
 72 2016-12-02 00:57:29	0|BlueMatt|I'm doing it now :)
 73 2016-12-02 01:50:38	0|phantomcircuit|sipa, after looking at 8831 again i can see why you wanted to not have the ReadKeyValue logic in CWallet
 74 2016-12-02 01:50:56	0|phantomcircuit|im not sure a class with virtual functions is going to be enough either though
 75 2016-12-02 02:11:08	0|phantomcircuit|sipa, i could just add a bunch of methods to CWallet like LoadName LoadPurpose etc
 76 2016-12-02 02:25:41	0|morcos|sipa: for bitcoin272's problem, the wallet doesn't rebroadcast things that aren't in its own mempool correct? so is his node restarting (which would then maybe accept some of the later chain txs) or something else?
 77 2016-12-02 02:27:37	0|sipa|morcos: uh, right
 78 2016-12-02 02:28:46	0|morcos|gmaxwell: in the code that doesn't store an orphan if its parent was rejected...  are we sure that's a good idea?
 79 2016-12-02 02:29:05	0|morcos|if a parent had too low fee
 80 2016-12-02 02:29:21	0|morcos|it is entirely possible that the parent + orphan would then stay in the mempool
 81 2016-12-02 02:29:51	0|gmaxwell|morcos: well we don't have the parent now and won't request it so the orphan will not get resolved. We might want to keep it around for BIP152 but right now BIP152 makes NO use of the orphan pool..
 82 2016-12-02 02:30:12	0|morcos|what do you mean we won't request it?
 83 2016-12-02 02:30:34	0|gmaxwell|while it's in the reject filter we won't request it.
 84 2016-12-02 02:31:15	0|gmaxwell|('cause thats what the reject filter does)
 85 2016-12-02 02:31:47	0|morcos|argh, thats just kind of an accident about the way alreadyhave works though right?
 86 2016-12-02 02:31:54	0|gmaxwell|Am I confused? Highly likely... I have a cold.
 87 2016-12-02 02:32:14	0|gmaxwell|Well I thought that was the intent-- we don't want to request things we 'know' we will just reject.
 88 2016-12-02 02:32:26	0|morcos|i mean in the orphan processing code we're specifically requesting the parents, but you're right we "AlreadyHave" things that are recentrejects
 89 2016-12-02 02:32:59	0|gmaxwell|Yea, we could request it again if the orphan's fee was high for example and maybe then they both could be accepted.. would make ancestor feerate mining work better.
 90 2016-12-02 02:33:09	0|morcos|yeah, but i guess i'm drawing a distinction between requesting txs in response to inv's (in which case you dont' want them if you recentlyrejected)
 91 2016-12-02 02:33:21	0|morcos|and requesting them as a parent, in which case, maybe you want to try again
 92 2016-12-02 02:34:47	0|morcos|in fact, we could almost bypass the mempool minfee check altogether since most of our recent peers won't send us stuff that violates it anyway and then it would basically just work
 93 2016-12-02 02:34:54	0|morcos|but i guess we can't quite do that
 94 2016-12-02 02:35:09	0|gmaxwell|I'd like rejection to work differently, putting rejected things in a datastructure that works like the new sigcache open-hashtable where they're taged for eager eviction but kept around until actually evicted... and then BIP152 can use that pool, and orphan resolution can use it and so on.
 95 2016-12-02 02:35:40	0|morcos|does that work for non-POD
 96 2016-12-02 02:36:11	0|gmaxwell|'works like' I don't mean the lockless aspects of it either, but just the fact that there is a generation/deleted flag.
 97 2016-12-02 02:37:37	0|morcos|ok.. i think i'll still PR that takes non-stored orphans with rejected parents and adds them to rejects, but just comment that we might want to remove that if we fix up the rest.  i think its strictly better than existing
 98 2016-12-02 02:37:46	0|gmaxwell|the principle is that we already took the cost of transfering the data, we should keep as of the most useful 'useless' stuff as we can afford... in case it turns up in a block or as a parent.
 99 2016-12-02 02:38:01	0|gmaxwell|But I dunno if it's better to spend time working on that or mempool sync.
100 2016-12-02 02:39:14	0|morcos|gmaxwell: arghh.. you guys and your mempool sync..   i tend to like the other methods better.. but BlueMatt was trying to convince me nothing can match the privacy of mempool sync
101 2016-12-02 02:39:56	0|gmaxwell|hah
102 2016-12-02 02:40:00	0|BlueMatt|I'm still a fan
103 2016-12-02 02:40:56	0|gmaxwell|well perhaps I'm also chasing it because in theory it's possible to get pretty close to optimal bandwidth efficiency and today we waste a lot of bandwidth on relay. (though it's gotten incrementally better in the last several releases)
104 2016-12-02 02:41:41	0|gmaxwell|but it's easy to venture into overdesign.
105 2016-12-02 02:41:48	0|gmaxwell|OTOH, Fibre exists and is pretty awesome.
106 2016-12-02 02:42:02	0|gmaxwell|and I could have also said that it was a crazy excercise in chasing optimality.
107 2016-12-02 02:43:10	0|morcos|we should set up some data-collecting nodes that see how much better our block reconstruction would be if we'd kept everything we were told about
108 2016-12-02 02:43:45	0|BlueMatt|lol, by adding #include <boost/thread.hpp> to fix windows build error, net_processing.cpp ticked over the 1GB-memory-usage mark :(
109 2016-12-02 02:43:51	0|BlueMatt|fucking boost
110 2016-12-02 02:44:01	0|BlueMatt|gmaxwell: I'm more excited by better relay privacy
111 2016-12-02 02:44:21	0|gmaxwell|morcos: I've been monitoring a bit of that on and off.
112 2016-12-02 02:44:42	0|sipa|BlueMatt: i think more recent gccs have lower memory usage? :0
113 2016-12-02 02:44:46	0|gmaxwell|actually I find a lot of the blocks that are almost perfectly reconstucted miss due to replacements / doublespends.
114 2016-12-02 02:45:04	0|morcos|gmaxwell: how much of the gap can you close?
115 2016-12-02 02:45:06	0|BlueMatt|sipa: I'm sure, this is what was in default-debian on digitalocean
116 2016-12-02 02:45:12	0|morcos|replacements/doublespends that you heard about?
117 2016-12-02 02:45:15	0|gmaxwell|Yes.
118 2016-12-02 02:45:27	0|morcos|but weren't RBF?
119 2016-12-02 02:45:34	0|BlueMatt|huh, can anyone reproduce travis' hangs on #9260
120 2016-12-02 02:45:34	0|morcos|why didnt you have them?
121 2016-12-02 02:45:35	0|BlueMatt|i cant...
122 2016-12-02 02:45:35	0|gribble|https://github.com/bitcoin/bitcoin/issues/9260 | Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) by TheBlueMatt · Pull Request #9260 · bitcoin/bitcoin · GitHub
123 2016-12-02 02:45:41	0|gmaxwell|As in I replaced the transaction, and the block confirmed the other (presumably the miner didn't support replacement or it didn't make itt othem yet)
124 2016-12-02 02:45:56	0|gmaxwell|morcos: some were RBF.
125 2016-12-02 02:46:05	0|gmaxwell|Though I did see a doublespend that wasn't at least once.
126 2016-12-02 02:46:14	0|BlueMatt|oh, maybe I can
127 2016-12-02 02:46:15	0|morcos|yeah, so thats a good use of your eager eviction
128 2016-12-02 02:46:33	0|gmaxwell|morcos: if you have debug=1 on, and the BIP152 missed by only a few txids it will log the missing txids and you can check your logs to see if you'd previously heard them.
129 2016-12-02 02:47:02	0|BlueMatt|oops lol
130 2016-12-02 02:47:02	0|morcos|gmaxwell: ha, even easier than i thought
131 2016-12-02 02:47:16	0|gmaxwell|during the period I last looked this was the overwhelming majority of blocks that missed only a couple.  But there is a lot of variance since it depends on miner policy...
132 2016-12-02 02:47:42	0|gmaxwell|morcos: I think 'few' might only be <5 so you might want to turn that up.
133 2016-12-02 02:48:06	0|gmaxwell|The purpose of that logging was to explore the impacts of prefill policies, and we wouldn't ever prefill more than a couple.
134 2016-12-02 02:48:11	0|sipa|yup, 5
135 2016-12-02 02:48:14	0|sipa|<=5
136 2016-12-02 02:48:41	0|gmaxwell|(I don't think prefill is worth working on until we eliminate all the preventable misses)
137 2016-12-02 02:49:09	0|gmaxwell|(e.g. by using the orpan pool and by keeping at least replacements around in some kind of pool...)
138 2016-12-02 02:50:12	0|morcos|sdaftuar and i were saying that we might also want to have a super-HB peer among our 3 HB peers, as you maybe don't want 3 peers prefilling for you
139 2016-12-02 02:50:37	0|morcos|for instance one kind of obvious prefill policy is if the tx is non-standard (such as >100k) prefill, but then thats big
140 2016-12-02 02:50:58	0|gmaxwell|yes, though it's little enough data that it probably doesn't matter.   The spec recommends you don't prefill more than 10kb in total.
141 2016-12-02 02:53:23	0|gmaxwell|morcos: of course, there is also the Fibre technique where you send FEC data... and then all the peers could usefully contribute. :P
142 2016-12-02 02:54:03	0|gmaxwell|We also don't have a way to NAK a HB request-- I reasoned that this was okay because even if every peer requests HB from you... you still end up not really any worse than relaying a full block to a single peer.  Same kind of thinking applies for excessive prefill.
143 2016-12-02 02:54:44	0|morcos|yeah we should probably right a new mining api first.  :)
144 2016-12-02 02:54:51	0|morcos|s/right/write/
145 2016-12-02 02:54:55	0|gmaxwell|Yea, no kidding.
146 2016-12-02 02:56:40	0|gmaxwell|I've also thought that we should probably not be using HB mode at all unless we have inbound connections or we're mining (or we've been asked by the user).  but that kind of complexity also got answered with 'the overhead is irrelevant'.
147 2016-12-02 03:12:21	0|gribble|https://github.com/bitcoin/bitcoin/issues/8855 | Use a proper factory for creating chainparams by jtimon · Pull Request #8855 · bitcoin/bitcoin · GitHub
148 2016-12-02 03:12:43	0|BlueMatt|sipa: even with gcc 6.2 net_processing ticks over 1GB (incl host)
149 2016-12-02 03:15:25	0|gmaxwell|:(
150 2016-12-02 03:15:32	0|gmaxwell|BlueMatt: you have failed at main splitting! :)
151 2016-12-02 03:15:49	0|jtimon|intirestingly enough, with +15-22 in main.cpp, #8498 doesn't need rebase since sep1
152 2016-12-02 03:15:50	0|gribble|https://github.com/bitcoin/bitcoin/issues/8498 | Optimization: Minimize the number of times it is checked that no money... by jtimon · Pull Request #8498 · bitcoin/bitcoin · GitHub
153 2016-12-02 03:15:59	0|BlueMatt|clearly
154 2016-12-02 03:16:04	0|BlueMatt|to be fair, so does init
155 2016-12-02 03:16:24	0|jtimon|what did I missed?
156 2016-12-02 03:18:34	0|bitcoin-git|[13bitcoin] 15morcos opened pull request #9261: Add unstored orphans with rejected parents to recentRejects (06master...06orphanRejects) 02https://github.com/bitcoin/bitcoin/pull/9261
157 2016-12-02 03:26:45	0|jtimon|BlueMatt: not renaming main to validation in the same PR would make it simpler to review
158 2016-12-02 03:27:01	0|BlueMatt|jtimon: that pr should be easy to review
159 2016-12-02 03:27:09	0|BlueMatt|the main.cpp rename is a clean rename, so that especially
160 2016-12-02 03:29:23	0|jtimon|I guess I'm not enthusiastic about renaming main in general, I believe it still does a lot of things beyond consensus validation, plus it still includes most globals
161 2016-12-02 03:29:34	0|jtimon|but will keep looking
162 2016-12-02 03:31:01	0|jtimon|but yeah, review it's not a good argument
163 2016-12-02 03:34:32	0|BlueMatt|jtimon: see scrollback, there are ~no functions which are not block/tx processing left in validation.cpp
164 2016-12-02 03:34:39	0|BlueMatt|jtimon: except, yes, globals
165 2016-12-02 03:36:11	0|jtimon|https://github.com/bitcoin/bitcoin/pull/9260#issuecomment-264365400
166 2016-12-02 03:36:37	0|BlueMatt|jtimon: what /did/ you review?
167 2016-12-02 03:43:23	0|jtimon|commit by commit, if the moveonlys are moveonlys (ie https://github.com/bitcoin/bitcoin/pull/9260/commits/84922e4bf4c38227fbbbede50e09c87fe2a5c7f0 ) and what you say in https://github.com/bitcoin/bitcoin/pull/9260/commits/87c35f584397e2309970afdcca8e03731a86639e is true, everything seems fine or it shouldn't compile, to give an utACK I would need to grep mapOrphanTransactions and mapOrphanTransactionsByPrev and verify the
168 2016-12-02 03:43:23	0|jtimon|moveonlies
169 2016-12-02 03:54:09	0|jtimon|re rename right, git knows the file is renamed but you eat the include changes which I agree is not hard to review
170 2016-12-02 03:57:17	0|wumpus|cfields: I thought about this global version/context parameters thing in RPC a bit, and there's several other potentially controversial solutions that are used for JSON API versioning we could consider as well: an alternative (say "v2") entry point, or using a HTTP header. An advantage would be that these are conceptually separate from normal arguments
171 2016-12-02 03:58:18	0|wumpus|cfields: I'm not specifically aiming 8811 for any release but I'd sure hope it can be merged before 0.14
172 2016-12-02 03:58:46	0|wumpus|cfields: as it changes all the RPC dispatch tables it's pretty annoying to keep up to date
173 2016-12-02 03:59:29	0|wumpus|cfields: I just wish people would test it a bit
174 2016-12-02 04:02:19	0|wumpus|cfields: regarding testing it: maybe it would make sense to port at least one of the RPC tests to fully using named arguments? currently it adds a test but that one is specifically for the named-arguments-parsing functionality
175 2016-12-02 04:05:14	0|jtimon|sipa: BlueMatt: if we're doing file renames, maybe we can move ahead with some in https://github.com/bitcoin/bitcoin/pull/8328 (those people can agree on)
176 2016-12-02 04:09:50	0|bitcoin-git|[13bitcoin] 15instagibbs opened pull request #9262: Don't consider coins available if too many ancestors in mempool (06master...06toolong) 02https://github.com/bitcoin/bitcoin/pull/9262
177 2016-12-02 04:49:06	0|bitcoin-git|[13bitcoin] 15laanwj pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/dc6dee41f7cf...c4d22f6eb216
178 2016-12-02 04:49:07	0|bitcoin-git|13bitcoin/06master 1410ae7a7 15Matt Corallo: Revert "Use async name resolving to improve net thread responsiveness"...
179 2016-12-02 04:49:07	0|bitcoin-git|13bitcoin/06master 14c4d22f6 15Wladimir J. van der Laan: Merge #9229: Remove calls to getaddrinfo_a...
180 2016-12-02 04:49:22	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9229: Remove calls to getaddrinfo_a (06master...062016-11-gai) 02https://github.com/bitcoin/bitcoin/pull/9229
181 2016-12-02 04:49:58	0|wumpus|btw flagging things as "needs backport" with "0.14" doesn't make much sense :-)
182 2016-12-02 04:51:31	0|bitcoin-git|13bitcoin/060.13 14b172377 15Matt Corallo: Revert "Use async name resolving to improve net thread responsiveness"...
183 2016-12-02 04:51:31	0|bitcoin-git|[13bitcoin] 15laanwj pushed 1 new commit to 060.13: 02https://github.com/bitcoin/bitcoin/commit/b172377857f9b5a0b2f43e0e57be9acf82a6c50a
184 2016-12-02 04:52:51	0|sipa|wumpus: i merged a few things that still need backporting
185 2016-12-02 04:53:19	0|wumpus|sipa: that's fine, we should probably do that in a pull grouping them together
186 2016-12-02 04:53:51	0|wumpus|sipa: I just have a really bad feeling about the async resolving thing so backported that immediately
187 2016-12-02 04:55:04	0|wumpus|sipa: I suppose they're all in this list? https://github.com/bitcoin/bitcoin/pulls?q=is%3Apr+label%3A%22Needs+backport%22+is%3Aclosed+milestone%3A0.13.2
188 2016-12-02 04:55:43	0|wumpus|if not they should be labeled "Needs backport" with milestone 0.13.2
189 2016-12-02 04:56:13	0|sipa|wumpus: yes
190 2016-12-02 05:00:08	0|wumpus|heh this is the code in libc where it crashes: https://0bin.net/paste/V1n0GkHdlDatZrnO#N5htO2+DbXw1EtNNKz4oB-3ykuixE4KGJTLiZ56/V9L  to be specific: the if (--waitlist) line
191 2016-12-02 05:00:33	0|wumpus|mind the "This is tricky.  See getaddrinfo_a.c for the reason why this works." comment :)
192 2016-12-02 05:02:24	0|sipa|that does not look thread safe at all
193 2016-12-02 05:02:41	0|Lightsword|wumpus, that’s not going to be externally exploitable is it for things that uses getaddrinfo?
194 2016-12-02 05:03:27	0|wumpus|Lightsword: I don't *think* so, but it depends on what kind of bug this turns out to be
195 2016-12-02 05:03:36	0|Lightsword|has upstream confirmed?
196 2016-12-02 05:05:30	0|wumpus|it's not triggered by any specific data the remote is sending, if that's what you're worried about
197 2016-12-02 05:06:15	0|wumpus|also it's not possible for P2P clients to make bitcoind do DNS lookups
198 2016-12-02 05:06:40	0|wumpus|so no, I don't think this is a security issue for us, but you can't be too careful right...
199 2016-12-02 05:08:50	0|wumpus|it's a confirmed use after free, not a NULL pointer dereference
200 2016-12-02 05:09:15	0|fanquake|When are we planning on releasing 0.13.2 ? Looks like 5 backports left.
201 2016-12-02 05:09:37	0|wumpus|it's doing "0x7ffff79b77f0 <__gai_notify+48>        subl   $0x1,(%rdx)" where rdx is, say, 0x441f0fc3c08944, then if you try to inspect that "0x441f0fc3c08944:       Cannot access memory at address 0x441f0fc3c08944" -- too bad, already unmapped
202 2016-12-02 05:09:40	0|fanquake|*the first rc of 0.13.2
203 2016-12-02 05:09:56	0|wumpus|Lightsword: sort of https://sourceware.org/bugzilla/show_bug.cgi?id=20874
204 2016-12-02 05:11:07	0|wumpus|fanquake: well yesterday in the meeting there was agreement that all the 0.13.2 backports are labeled - so after these are backported rc1 can be released any time
205 2016-12-02 05:13:25	0|fanquake|Ok. I need to start attending the meetings, but difficult with time-diff though.
206 2016-12-02 05:13:47	0|wumpus|yes it's not a good time for australia/most of asia
207 2016-12-02 05:14:44	0|fanquake|Also, not sure why I said 5 PRs left, there is only 9239, 9194 and 9191 (which includes multiple, mostly test-related fixes).
208 2016-12-02 05:15:14	0|fanquake|It'll be right. Always have the logs, just need to make time for reading them. I think someone also posts a meeting summary to reddit or something.
209 2016-12-02 05:21:23	0|wumpus|not just to reddit :) https://bitcoincore.org/en/meetings/2016/10/20/
210 2016-12-02 05:34:25	0|wumpus|anyhow we could, say, alternate between two meeting times if there is a lot of demand for people from asia/australia to attend the meetings. But I've never really got that impression.
211 2016-12-02 05:36:59	0|sipa|the meeting is on SHA256(days_since_1970) % 24 UTC.
212 2016-12-02 05:38:40	0|gmaxwell|I ws thinking of suggesting alternating but then though that there probably isn't a good time for both asia and europe.
213 2016-12-02 05:38:43	0|wumpus|sipa: theoretically that would be fair, in practice if you don't take the distribution of people over timezones into account, you may end up with a time that's only good for people who live in the middle of the atlantic ocean :-)
214 2016-12-02 05:39:51	0|wumpus|gmaxwell: right I don't think there is
215 2016-12-02 05:39:56	0|sipa|gmaxwell: 8 am UTC would be relatively good for both asia and europe
216 2016-12-02 05:40:27	0|wumpus|8 am UTC would be quite a good time for me
217 2016-12-02 05:41:01	0|sipa|(9am in western europe, 4pm in hong kong, midnight on west coast)
218 2016-12-02 05:41:10	0|sipa|3am on east coast, however
219 2016-12-02 05:41:16	0|gmaxwell|thats too late for east coast US though I don't know if we currently have any eastcoasters.
220 2016-12-02 05:41:23	0|gmaxwell|oh instagibbs is duh.
221 2016-12-02 05:41:26	0|sipa|morcos, suhas, instagibbs
222 2016-12-02 05:41:34	0|gmaxwell|oh double duh.
223 2016-12-02 05:41:47	0|wumpus|in any case every time would be bad for someone - so to motivate a (even bi-weekly) time change there has to be enough demand
224 2016-12-02 05:41:57	0|gmaxwell|I just think of NY as existing in a parallel universe.
225 2016-12-02 05:42:19	0|wumpus|e.g. just rebroad asking for it is not enough :p
226 2016-12-02 05:42:31	0|gmaxwell|well we lose jl2012 due to this.
227 2016-12-02 05:42:51	0|wumpus|yes and fanquake
228 2016-12-02 05:43:56	0|BlueMatt|lol, google has a thing where they will hook up your project to run in fuzzing on some machiens 24/7, except to do it you have to agree that they will announce the bug to the world 7 days after you release a fix
229 2016-12-02 05:44:09	0|BlueMatt|who in their right mind would agree to that? thats like stupid high-risk for users
230 2016-12-02 05:45:14	0|wumpus|it could make sense for software that auto-updates quickly and automatically
231 2016-12-02 05:45:18	0|wumpus|but no, certainly not for bitcoin
232 2016-12-02 05:46:58	0|BlueMatt|I mean they're talking about it for "critical infrastructure" (ie common libraries)
233 2016-12-02 05:47:16	0|BlueMatt|like, sure, maybe google updates their libcs quickly, but the vast majority of folks do not at all
234 2016-12-02 05:47:17	0|sipa|4pm UTC: 8am westcoast, 11am eastcoast, 5pm europe, midnight hong kong
235 2016-12-02 05:48:47	0|wumpus|BlueMatt: for libraries it's much harder to say, agreed, there will be tons of especially embedded platforms that never update them at all
236 2016-12-02 05:49:25	0|wumpus|then again that's not google's fault - finding the vulnerabilities before the 'bad people' do is still important
237 2016-12-02 05:49:48	0|BlueMatt|wumpus: sure, but that doesnt mean you announce them publicly 7 days after the first release with the fix
238 2016-12-02 05:50:23	0|wumpus|(or alternatively, after the "bad people" have already used them for years to get access anwyay...)
239 2016-12-02 05:50:38	0|BlueMatt|yes, you fix quickly, and then announce it much later
240 2016-12-02 05:50:39	0|gmaxwell|BlueMatt: it's fine for projects that are alread well fuzzed, I suppose.
241 2016-12-02 05:51:02	0|gmaxwell|NSA already found those vulns last week anyways.
242 2016-12-02 05:51:03	0|BlueMatt|it might be better than /not/ doing the fuzzing, but its still a super shitty policy
243 2016-12-02 05:51:46	0|wumpus|it's the old responsible disclosure discussion
244 2016-12-02 05:52:23	0|BlueMatt|wumpus: responsible disclosure (usually) is a discussion about what to do when upstream tells you to fuck off, not what to do when upstream is responsive
245 2016-12-02 05:53:05	0|wumpus|BlueMatt: sure, though part of it is how long to wait with public announcement
246 2016-12-02 05:53:19	0|BlueMatt|suresure, but who the fuck ever suggested 7 days?
247 2016-12-02 06:01:46	0|midnightmagic|responsible disclosure is using a timeline which is fair both to the public who is affected by the bug and the vendor, without unfairly prioritizing one over the other.
248 2016-12-02 06:02:31	0|midnightmagic|The timelines *were once* measured in weeks, since the public good of disclosure was more important than protecting the financial interests of a corporation who wasn't remunerating the researcher.
249 2016-12-02 06:05:43	0|BlueMatt|midnightmagic: yes, but one week? fuck people who just blindly update and would be protected wouldnt even get protected if you only wait 7 days
250 2016-12-02 06:06:08	0|BlueMatt|midnightmagic: I mean I'm with you....fuck the vendor and their financial interests, users must be protected, but 7 days is short enough that you're harming users, too
251 2016-12-02 06:07:46	0|midnightmagic|7 days is pretty sure. At least Gobbles and/or that italian ultra-prolific guy isn't just dropping 0-days and letting the vendors scramble.
252 2016-12-02 06:07:51	0|midnightmagic|*pretty short
253 2016-12-02 06:13:20	0|midnightmagic|ah, here he is. I've come to appreciate the way he does things: http://aluigi.altervista.org/
254 2016-12-02 06:13:32	0|midnightmagic|(not even vendor notification.)
255 2016-12-02 07:03:11	0|bitcoin-git|[13bitcoin] 15luke-jr opened pull request #9263: release notes: Only free transactions are being removed, not priority. (06master...06relnotes_freetxn) 02https://github.com/bitcoin/bitcoin/pull/9263
256 2016-12-02 07:12:57	0|bitcoin-git|[13bitcoin] 15laanwj opened pull request #9264: 0.13.2 backports (060.13...062016_12_backports_0_13) 02https://github.com/bitcoin/bitcoin/pull/9264
257 2016-12-02 07:16:53	0|jonasschnelli|What is preferable: two keypools one for change, one for external  OR  a keypool with keys flagged for internal or external use?
258 2016-12-02 07:17:16	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9191: [qa] 0.13.2 Backports (060.13...06Mf1611-q01302) 02https://github.com/bitcoin/bitcoin/pull/9191
259 2016-12-02 07:17:36	0|wumpus|possibly the flagging approach is easier to do in a backwards compatible way - old versions can ignore the flags
260 2016-12-02 07:19:07	0|jonasschnelli|wumpus: good point..
261 2016-12-02 07:21:05	0|jonasschnelli|for the deserialization (SerializationOp(Stream& s, ...)), if the stream is longer then the acctual READWRITE, it will be ignored? right? (for backward comp.)
262 2016-12-02 07:21:26	0|jtimon|python3 ./qa/pull-tester/rpc-tests.py mempool_packages takes 31s in my computer, would it be crazy to move it from the extended test to the regular ones?
263 2016-12-02 07:21:30	0|bitcoin-git|[13bitcoin] 15laanwj pushed 3 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/c4d22f6eb216...3fbf07926240
264 2016-12-02 07:21:31	0|bitcoin-git|13bitcoin/06master 14d824ad0 15Alex Morcos: Disable fee estimates for a confirm target of 1 block
265 2016-12-02 07:21:31	0|bitcoin-git|13bitcoin/06master 14e878689 15Alex Morcos: Make GUI incapable of setting tx confirm target of 1
266 2016-12-02 07:21:32	0|bitcoin-git|13bitcoin/06master 143fbf079 15Wladimir J. van der Laan: Merge #9239: Disable fee estimates for 1 block target...
267 2016-12-02 07:21:40	0|bitcoin-git|[13bitcoin] 15laanwj closed pull request #9239: Disable fee estimates for 1 block target (06master...06blockstreamtil2blocks) 02https://github.com/bitcoin/bitcoin/pull/9239
268 2016-12-02 07:25:02	0|luke-jr|wumpus: doh, I was about to do that (more backports)
269 2016-12-02 07:26:10	0|dcousens|BlueMatt: don't miners use priority for their own transactions?
270 2016-12-02 07:26:25	0|fanquake|jtimon takes 51s here
271 2016-12-02 07:26:27	0|dcousens|(not the coinbase, just, "other" transactions)
272 2016-12-02 07:29:03	0|jonasschnelli|wumpus: I could do a BP of #8989
273 2016-12-02 07:29:04	0|gribble|https://github.com/bitcoin/bitcoin/issues/8989 | [Qt] overhaul smart-fee slider, adjust default confirmation target by jonasschnelli · Pull Request #8989 · bitcoin/bitcoin · GitHub
274 2016-12-02 07:29:33	0|wumpus|jonasschnelli: not sure that's what we want though?
275 2016-12-02 07:29:35	0|luke-jr|dcousens: fee delta works fine for that use case
276 2016-12-02 07:29:56	0|jonasschnelli|Yes. It would be a notable change.
277 2016-12-02 07:29:56	0|wumpus|jonasschnelli: I mean, this is a minor release, how much do we want the GUI to change?
278 2016-12-02 07:30:15	0|wumpus|maybe it's ok though in this case I'm not sure
279 2016-12-02 07:30:21	0|jonasschnelli|Can we BP #9239 without the GUI changes?
280 2016-12-02 07:30:23	0|gribble|https://github.com/bitcoin/bitcoin/issues/9239 | Disable fee estimates for 1 block target by morcos · Pull Request #9239 · bitcoin/bitcoin · GitHub
281 2016-12-02 07:30:25	0|jtimon|fanquake: thanks, still, I don't see a consistency in times between extended and non-extended tests, I have a little commit in a long branch that I can cherry pick based only on what makes sense for my computer based only on time, introducing -skipslow that works with or without extended, but it looks a little bit too complicated
282 2016-12-02 07:30:39	0|wumpus|you could reply that in the #9239 topic, probably morcos has an opinoin on it too :)
283 2016-12-02 07:30:41	0|gribble|https://github.com/bitcoin/bitcoin/issues/9239 | Disable fee estimates for 1 block target by morcos · Pull Request #9239 · bitcoin/bitcoin · GitHub
284 2016-12-02 07:31:07	0|jonasschnelli|wumpus: or just disabled (shorten the slider range) for target 1 in the backport
285 2016-12-02 07:31:16	0|wumpus|jonasschnelli: yep, exactly
286 2016-12-02 07:31:57	0|wumpus|jonasschnelli: although that may turn out to be a riskier/less-tested solution than #8989 which at least has been tested in master. DIfficult.
287 2016-12-02 07:31:59	0|gribble|https://github.com/bitcoin/bitcoin/issues/8989 | [Qt] overhaul smart-fee slider, adjust default confirmation target by jonasschnelli · Pull Request #8989 · bitcoin/bitcoin · GitHub
288 2016-12-02 07:32:27	0|jonasschnelli|Yes. IMO 8989 and 9239 is sort of one BP "package"
289 2016-12-02 07:32:43	0|wumpus|jonasschnelli: in that case we should just backport both I guess
290 2016-12-02 07:32:49	0|jonasschnelli|Agree
291 2016-12-02 07:33:07	0|jonasschnelli|I can do that next week...
292 2016-12-02 07:33:18	0|jonasschnelli|(if nobody did it in the meantime)
293 2016-12-02 07:34:14	0|wumpus|luke-jr: good that you hadn't started yet, then, would have been a waste of work as some had manual conflicts to resolve (though you could check if you resolved them in the same way :)
294 2016-12-02 07:34:23	0|luke-jr|wumpus: well, I had started.. but I can rebase :x
295 2016-12-02 07:34:45	0|luke-jr|I went through the PRs manually, not using tags, so I probably got a few you missed
296 2016-12-02 07:34:54	0|luke-jr|(well, *almost* manually.. XD)
297 2016-12-02 07:35:09	0|wumpus|well I strictly backport what is tagged, not more, if not it should have been tagged
298 2016-12-02 07:37:24	0|luke-jr|wumpus: erg, your backports aren't on top of Marco's? :x
299 2016-12-02 07:37:42	0|wumpus|luke-jr: no, I forgot about that one, will rebase
300 2016-12-02 07:38:21	0|wumpus|now they are (luckily no new conflicts)
301 2016-12-02 07:41:27	0|luke-jr|woo no conflicts here either it seems
302 2016-12-02 07:57:11	0|bitcoin-git|[13bitcoin] 15laanwj opened pull request #9265: bitcoin-cli: Make error message less confusing (06master...062016_12_rpccli_message) 02https://github.com/bitcoin/bitcoin/pull/9265
303 2016-12-02 07:59:15	0|luke-jr|wumpus: pushed backports-0.13 to my github; want to just pull it into yours?
304 2016-12-02 08:00:31	0|wumpus|luke-jr: will have a look in a moment
305 2016-12-02 08:14:00	0|BlueMatt|dcousens: and the "add fee" logic will continue to be maintained....but that isnt the "priority" code - this refers specifically to coin days-destroyed logic
306 2016-12-02 08:15:03	0|luke-jr|the priority code will be as well.
307 2016-12-02 08:15:22	0|sipa|no, it won't
308 2016-12-02 08:15:24	0|wumpus|luke-jr: looks ok to me - though I'm not entirely sure about the qt memory leak commits, they are all pretty minor one-time leaks, so users shouldn't notice it
309 2016-12-02 08:15:28	0|sipa|it serves no function anymore
310 2016-12-02 08:15:41	0|luke-jr|sipa: yes it does, the same function it always served: anti-spam
311 2016-12-02 08:15:52	0|wumpus|luke-jr: still makes sense to clean them up, but not sure whether the backport is risky
312 2016-12-02 08:16:05	0|luke-jr|wumpus: I don't care strongly if you want to skip those
313 2016-12-02 08:16:26	0|luke-jr|the backport may be risky, but if we have an rc or two, I'd put it in
314 2016-12-02 08:16:45	0|wumpus|luke-jr: also the menu reparenting may have subtle behavior issues I hadn't considered (though none reported yet)
315 2016-12-02 08:16:53	0|wumpus|yes, sure
316 2016-12-02 08:16:57	0|sipa|luke-jr: so blocks with 950 kb of spam is fine, and 50kb of transactions from bitcoin old timers that doesn't really pays miners a competitive price will save anything?
317 2016-12-02 08:17:23	0|sipa|the only scalable way to deal with this is economic incentives
318 2016-12-02 08:17:25	0|luke-jr|sipa: it would be better than 1000 kb of spam :P
319 2016-12-02 08:17:41	0|wumpus|lukanyhow I'll pull them into #9264
320 2016-12-02 08:17:42	0|sipa|and face it, that is how it works in practice now
321 2016-12-02 08:17:43	0|gribble|https://github.com/bitcoin/bitcoin/issues/9264 | 0.13.2 backports by laanwj · Pull Request #9264 · bitcoin/bitcoin · GitHub
322 2016-12-02 08:17:45	0|luke-jr|if we go to exclusively economic incentives, we'd need to remove ALL spam filtering..
323 2016-12-02 08:18:05	0|sipa|luke-jr: there are still local node dos protections
324 2016-12-02 08:18:24	0|sipa|which everyone has an incentive to maintain
325 2016-12-02 08:18:37	0|sipa|and don't requires miners to be gatekeepers of good and bad transactions
326 2016-12-02 08:19:11	0|luke-jr|sipa: if we had a system that wasn't suffering from spam, removing priority might make sense. but we don't.
327 2016-12-02 08:19:30	0|sipa|luke-jr: removing priority will have 0 impact
328 2016-12-02 08:21:15	0|sipa|wallets don't use it anymore, (almost all) miners don't use it anymore - even if it was usable as a means to distinguish better from worse usage of block space, it isn't anymore
329 2016-12-02 08:21:57	0|luke-jr|has someone shown that to be true?
330 2016-12-02 08:22:10	0|luke-jr|last time I looked, a large % of transactions in blocks were in the priority area
331 2016-12-02 08:22:33	0|luke-jr|(not majority-large, but not <5% either)
332 2016-12-02 08:22:56	0|sipa|fair enough, i have no actual data on his
333 2016-12-02 08:22:59	0|sipa|*this
334 2016-12-02 08:23:06	0|sipa|but how do you measure that?
335 2016-12-02 08:23:57	0|luke-jr|I wrote a RPC call that analyzed the sort order
336 2016-12-02 08:54:10	0|luke-jr|ugh this thing is slooooooow
337 2016-12-02 09:07:42	0|jtimon|for those interested in more configurable testchains: https://github.com/bitcoin/bitcoin/pull/8994#issuecomment-264406053
338 2016-12-02 10:02:18	0|luke-jr|oh, it'd also imply removing the soft blockmaxsize, which is essential since >1 MB blocks are not safe right now. which would therefore imply segwit should be rejected. not a good hole to go down IMO.
339 2016-12-02 12:00:57	0|luke-jr|sipa: CPFP and some other weirdness I don't recognise kinda made my analyzer useless :/
340 2016-12-02 12:22:28	0|luke-jr|jonasschnelli: is it just me, or is 0a261b63fd4f1b07431f8a65762ef9f1ef1c11c8 a bugfix that should be backported?
341 2016-12-02 12:52:46	0|gmaxwell|luke-jr: I think we should remove the softmax blocksize. I think it's harmful to the network to have some miners producing smaller blocks when there is a high fee backlog.
342 2016-12-02 12:56:08	0|luke-jr|so just ignore all the much bigger problems large blocks are causing? why is fee so important? it doesn't seem likely larger blocks are going to significantly change the fee rate either.. :/
343 2016-12-02 13:00:02	0|gmaxwell|luke-jr: having it set lower makes the larger block problem _worse_: vitually all miners just set it to the maximum, and then we pretend like we've done something useful there instead of actually doing something useful. (like implementing a dynamic soft cap so that when demand goes down feerates don't drob absurdly low)
344 2016-12-02 13:00:20	0|gmaxwell|s/drob/drop/
345 2016-12-02 13:01:32	0|luke-jr|not sure I follow; that sounds like the topic has changed to the *default* maxblocksize, but I'm talking about the option itself.
346 2016-12-02 13:01:38	0|luke-jr|wouldn't the latter just be the min fee setting?
347 2016-12-02 13:05:20	0|gmaxwell|luke-jr: sort of. minfee setting is really a relay setting, it gets in the way of CPFP. Should be intentionally low
348 2016-12-02 13:07:51	0|luke-jr|perhaps the gradual min-feerate would be a good thing to reintroduce
349 2016-12-02 13:07:56	0|gmaxwell|I think the problems of propagation are more or less solved for the moment, so the concerns are bulk blockchain growth and fee behavior predictablity.  Having a couple miners going around confirming stuff with absurdly low fees or failing to confirm stuff with decent fees doesn't help.
350 2016-12-02 13:08:22	0|luke-jr|hmm, would gradual min-feerate break prediction?
351 2016-12-02 13:09:38	0|gmaxwell|No. At least if constructed right it should improve it (esp if the prediction is aware of it).
352 2016-12-02 13:10:21	0|morcos|gmaxwell: i'd be strongly in favor of a separate min mining feerate
353 2016-12-02 13:10:57	0|morcos|do you actually want to remove blockmaxsize (and blockmaxweight?) as options?
354 2016-12-02 13:11:44	0|morcos|i don't feel strongly about that, but i do think we should avoid setting blockmaxsize as default. i tried to benchmark the behavior and didn't show it being a big performance hit
355 2016-12-02 13:11:49	0|morcos|but thats counterintuitive
356 2016-12-02 13:12:01	0|morcos|and i'd rather not risk it by default
357 2016-12-02 13:12:04	0|gmaxwell|I think we should default them to maximum at a minimum. Removing them-- I don't really care probably removing them would irritate someone, so not worth doing. I don't think they're useful controls (beyond overriding our dumb maximum)
358 2016-12-02 13:12:27	0|morcos|right.. so default no setting for blockmaxsize in particular (to avoid size accounting unless you set it)
359 2016-12-02 13:12:51	0|gmaxwell|right.
360 2016-12-02 13:13:54	0|morcos|wumpus: you can backport just the first commit of #9239. i will separately test, but that was the intent.  the only difference will be you will slide the slider to 1 but it will give you an answer for 2
361 2016-12-02 13:13:56	0|gribble|https://github.com/bitcoin/bitcoin/issues/9239 | Disable fee estimates for 1 block target by morcos · Pull Request #9239 · bitcoin/bitcoin · GitHub
362 2016-12-02 13:14:07	0|morcos|which is exactly what happens most of the time now anyway.. so will look like no change in behavior
363 2016-12-02 13:14:41	0|gmaxwell|Today, with BIP152 and fibre propagation has very little to do with blockmaxsize. And blockmaxweight is the wrong dimension for applying backpressure on fees: it's blind to demand, you'll still dumbly produce smaller blocks when there is a nice backlog and dumly produce blocks that are too big when there is none (at least the minfee stops the bleeding there)
364 2016-12-02 13:16:15	0|gmaxwell|luke-jr: yes, but a pointless softcap that virtually everone overrides is still not helping.
365 2016-12-02 13:16:28	0|luke-jr|gmaxwell: nobody overrides it >1 MB yet AFAIK.
366 2016-12-02 13:17:09	0|morcos|I think I went through this before, but can't see where I wrote it up.  I think we actually need 3 minimum rates and minrelaytxfee (a default minimum for the mempool) is not one of them
367 2016-12-02 13:17:17	0|morcos|1) min mining feerate
368 2016-12-02 13:17:33	0|morcos|2) rate used to define dust (should change rarely)
369 2016-12-02 13:17:58	0|morcos|3) rate used as the minimum increment to pay for relay bandwidth (closest analog to existing minrelaytxfee)
370 2016-12-02 13:18:31	0|luke-jr|morcos: how would 3 be different from current minrelaytxfee?
371 2016-12-02 13:18:46	0|morcos|I don't think 3) actually needs to a have a floor other than 0, but i don't suppose it hurts
372 2016-12-02 13:18:56	0|gmaxwell|luke-jr: they all will as soon as it matters, we trained them to with an unreasonable default.  Last non-empty block that was under 990k was 217 blocks ago.
373 2016-12-02 13:19:58	0|luke-jr|as soon as it matters, would still be better than immediately by default
374 2016-12-02 13:20:27	0|gmaxwell|morcos: having a floor makes the system behavior more stable in any case, no real reason to go forwarding around low fee stuff that won't get mined ever just because we have nothing better to relay. :)
375 2016-12-02 13:20:34	0|morcos|although i guess, having a user set minimum for your mempool might be something people want (and a good fail safe if we get something wrong) so maybe we do want all 4
376 2016-12-02 13:21:40	0|morcos|gmaxwell: sure, but its inherently rate limited by the mempool min fee..  but i agree. i don't know what i'm arguing that point... and separating 3 and 4 is the least important concern.
377 2016-12-02 13:21:51	0|gmaxwell|luke-jr: yes it is worse because it puts everyone in the mode of just nailing it to the maximum, which would undermine doing something sensible and dynamic.
378 2016-12-02 13:22:14	0|luke-jr|but we don't have anything sensible and dynamic yet.
379 2016-12-02 13:22:16	0|morcos|i guess the only reason it matters is if someone wants to set their minimum higher to say 10 sat/byte, then thats not clear that they really want their increment requirement to be 10 sat/byte
380 2016-12-02 13:24:02	0|gmaxwell|luke-jr: of course not, we have a nearly useless setting instead which you spend a lot of effort defending. This impedes doing something smarter.
381 2016-12-02 13:24:35	0|luke-jr|it doesn't impede anything..
382 2016-12-02 13:25:28	0|gmaxwell|I promise it does, touching any of this means arguing with you and few people have interest in that.
383 2016-12-02 13:25:55	0|gmaxwell|and there is the polite lie that something already is there, there is a blocksize setting, you can make it larger or smaller, problem solved.
384 2016-12-02 13:26:48	0|jonasschnelli|I think the HD chain splitting will not be backward compatible. Adding a flag to CKeyPool would result in possible external keys from the internal chain on older versions of core
385 2016-12-02 13:27:09	0|jonasschnelli|Also, CKeyPool has no version-int.
386 2016-12-02 13:27:11	0|luke-jr|[12:22:27] <luke-jr> jonasschnelli: is it just me, or is 0a261b63fd4f1b07431f8a65762ef9f1ef1c11c8 a bugfix that should be backported?
387 2016-12-02 13:27:53	0|gmaxwell|I said it would be incompatible a bunch of times.
388 2016-12-02 13:28:39	0|jonasschnelli|I though we could make it compatible.. but yes. I guess it wont.
389 2016-12-02 13:29:40	0|jonasschnelli|Then I try to add a new record type, maybe "hdkey" that stores the keypath (int / ext chain) as well as the masterkey and the according pubkey.
390 2016-12-02 13:29:49	0|jonasschnelli|GetKey could derive the priv key on the fly
391 2016-12-02 13:30:16	0|jonasschnelli|luke-jr: we could bp... but o
392 2016-12-02 13:30:34	0|jonasschnelli|but i guess its not an important fix
393 2016-12-02 13:32:15	0|gmaxwell|what I was commenting on before was that we probably want to do several incompatible changes at once, so we don't end up having to support dozens of old versions.
394 2016-12-02 13:33:08	0|jonasschnelli|gmaxwell: Yes. That would be preferable. I think the splitting should be combined with the on-thy-fly derivation and the flexible keypath.
395 2016-12-02 13:33:19	0|jonasschnelli|Ideally +pub-CKD
396 2016-12-02 13:35:00	0|jcorgan|i haven't kept up recently, but are there any relevant BIPS or defacto practices from other wallets we should be paying attention to in this area?
397 2016-12-02 13:35:58	0|jonasschnelli|bip44/45 maybe. But I don't think its wise to force users to do pub key derivation.
398 2016-12-02 13:36:35	0|jonasschnelli|a flexible would allow to use bip44 and we could still stick to m/0'/k' by default
399 2016-12-02 13:36:41	0|jonasschnelli|a flexible keypath
400 2016-12-02 13:37:21	0|jcorgan|reviewing the flexpath PR is on my list this morning
401 2016-12-02 13:38:15	0|jonasschnelli|thanks
402 2016-12-02 13:39:42	0|jcorgan|but, i'm mostly just wondering if there are any good practices from other wallets we might benefit from understanding
403 2016-12-02 13:39:51	0|jcorgan|just thinking out loud
404 2016-12-02 13:43:36	0|gmaxwell|Doing public derrivation while the software supports private key export is really extremely inadvisable.
405 2016-12-02 13:44:03	0|gmaxwell|I also don't think it makes sense to spend time on advanced features when basic functionality is still kinda broken.
406 2016-12-02 13:44:12	0|gmaxwell|but I'm not the one working on it. :)
407 2016-12-02 13:44:46	0|jcorgan|agree, but it's important to define what unbroken basic functionality should be :)
408 2016-12-02 13:45:40	0|jcorgan|in other words, if there is to be breaking changes, and you only want to break it once, better get it right
409 2016-12-02 13:46:51	0|jcorgan|i didn't mean to jump into the middle of your thread, just noticed it and taking an interest. carry on.
410 2016-12-02 13:47:47	0|jonasschnelli|I agree with gmaxwell. We first need to solve the key-chain split.
411 2016-12-02 13:48:20	0|jonasschnelli|child key derivation is an interesting feature if you want to use core together with a hardware wallet.
412 2016-12-02 13:48:28	0|jonasschnelli|child pub key d.
413 2016-12-02 13:50:35	0|bitcoin-git|[13bitcoin] 15luke-jr opened pull request #9266: Qt/RPCConsole: Put column enum in the right places (06master...06bugfix_datarole) 02https://github.com/bitcoin/bitcoin/pull/9266
414 2016-12-02 14:56:17	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/3fbf07926240...31bcc667863f
415 2016-12-02 14:56:18	0|bitcoin-git|13bitcoin/06master 1431bcc66 15MarcoFalke: Merge #9265: bitcoin-cli: Make error message less confusing...
416 2016-12-02 14:56:18	0|bitcoin-git|13bitcoin/06master 14fe37fbe 15Wladimir J. van der Laan: bitcoin-cli: Make error message less confusing...
417 2016-12-02 14:56:32	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #9265: bitcoin-cli: Make error message less confusing (06master...062016_12_rpccli_message) 02https://github.com/bitcoin/bitcoin/pull/9265
418 2016-12-02 14:59:45	0|bitcoin-git|13bitcoin/06master 14b7aa290 15S. Matthew English: unification of Bloom filter representation...
419 2016-12-02 14:59:45	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/31bcc667863f...5412c08c3cf1
420 2016-12-02 14:59:46	0|bitcoin-git|13bitcoin/06master 145412c08 15MarcoFalke: Merge #9223: unification of Bloom filter representation...
421 2016-12-02 14:59:56	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #9223: unification of Bloom filter representation (06master...06patch-10) 02https://github.com/bitcoin/bitcoin/pull/9223
422 2016-12-02 15:21:16	0|bitcoin-git|13bitcoin/06master 1408ed8c1 15Gregory Maxwell: Developer docs about existing subtrees....
423 2016-12-02 15:21:16	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/5412c08c3cf1...98514988a3d3
424 2016-12-02 15:21:17	0|bitcoin-git|13bitcoin/06master 149851498 15MarcoFalke: Merge #9246: Developer docs about existing subtrees....
425 2016-12-02 15:21:28	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #9246: Developer docs about existing subtrees. (06master...06devdocs_for_subtrees) 02https://github.com/bitcoin/bitcoin/pull/9246
426 2016-12-02 15:41:57	0|bitcoin-git|13bitcoin/06master 140828619 15Suhas Daftuar: [qa] Dump debug logs on travis failures.
427 2016-12-02 15:41:57	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/98514988a3d3...d7ba4a233bd5
428 2016-12-02 15:41:58	0|bitcoin-git|13bitcoin/06master 14d7ba4a2 15MarcoFalke: Merge #9257: [qa] Dump debug logs on travis failures....
429 2016-12-02 15:42:14	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #9257: [qa] Dump debug logs on travis failures. (06master...06travis-debug-logs) 02https://github.com/bitcoin/bitcoin/pull/9257
430 2016-12-02 15:45:02	0|bitcoin-git|[13bitcoin] 15morcos opened pull request #9267: Disable fee estimates for a confirm target of 1 block (060.13...06backport9239) 02https://github.com/bitcoin/bitcoin/pull/9267
431 2016-12-02 16:10:37	0|MarcoFalke|sipa: I think this is needed to get our subtree clean again
432 2016-12-02 16:10:41	0|MarcoFalke|https://github.com/bitcoin-core/ctaes/pull/5
433 2016-12-02 16:26:01	0|instagibbs|can anyone explain why this isn't setting the argument correctly?:
434 2016-12-02 16:26:01	0|instagibbs|+        self.extra_args[0].append("-limitancestorcount=10")
435 2016-12-02 16:26:01	0|instagibbs|self.extra_args = [['-usehd={:d}'.format(i%2==0)] for i in range(4)]
436 2016-12-02 16:26:20	0|instagibbs|I'm trying to set a custom limit in test, and it's not being enforced
437 2016-12-02 16:31:33	0|instagibbs|ahhhh nevermind, the node is being spun down and up later in the test
438 2016-12-02 16:37:07	0|MarcoFalke|Yeah, we should use self.extra_args as argument where possible, instead of writing out the args every time.
439 2016-12-02 17:03:20	0|instagibbs|sdaftuar, how does one get the cached value?
440 2016-12-02 17:03:38	0|sdaftuar|instagibbs: oy, i'm thinking about it now. ancestor count is easy, but the descenadnt count is not...
441 2016-12-02 17:03:55	0|instagibbs|how often will that one trigger in wallet context though?
442 2016-12-02 17:03:56	0|sdaftuar|unfortunately the test we want to do is, what is the max number of descendants that any of pcoin's ancestors have?
443 2016-12-02 17:04:11	0|sdaftuar|and also, i think we should compare those values to some lower thresholds (say 10 instead of 25)
444 2016-12-02 17:04:37	0|instagibbs|I'm finding bugs with my implementation with non-standard values
445 2016-12-02 17:05:10	0|instagibbs|it always only fails at the 25 threshhold, unsure why. If we have an ancestor one already cached, we might just use that
446 2016-12-02 17:05:10	0|sdaftuar|well i think your implementation would never skip over any pcoins?  except if the chain limits are violated in a reorg
447 2016-12-02 17:05:21	0|instagibbs|seems less error-prone, and should catch the common case?
448 2016-12-02 17:06:17	0|instagibbs|im not sure what you mean?
449 2016-12-02 17:06:18	0|sdaftuar|so to answer your first question, each mempool entry has a value nCountWithAncestors
450 2016-12-02 17:06:36	0|instagibbs|how do I access that entry
451 2016-12-02 17:06:58	0|instagibbs|(looking)
452 2016-12-02 17:07:39	0|sdaftuar|instagibbs: might need to add some kind of accessor, but mempool.mapTx.find(txid) or something should do it.
453 2016-12-02 17:08:02	0|sdaftuar|since we're already checking to see that it's in the mempool, this seems okay to me
454 2016-12-02 17:08:58	0|sdaftuar|CalculateMemPoolAncestors() can be a bit expensive, fyi
455 2016-12-02 17:09:24	0|sdaftuar|but in order to do the descendant calculation properly, we'd need to call CMPA on the pcoins in question, and then check the descendant count of each ancestor returned
456 2016-12-02 17:09:43	0|sdaftuar|(which i guess is equivalent to calling CMPA with lower thresholds)
457 2016-12-02 17:10:46	0|sdaftuar|i'm not sure though how reasonable it is to call CMPA inside AvailableCoins?  if somehow your wallet has a lot of in-mempool stuff, this could be slow.  not sure how to think about it.
458 2016-12-02 17:11:23	0|instagibbs|I don't understand. I likely don't understand what CMPA is actually doing. I thought it was already checking for limit violations
459 2016-12-02 17:12:16	0|sdaftuar|you're trying to see if a transaction that spends pcoins would be a limit violation.  you're checking to see if pcoins itself would fail to get into the mempool based on the configured limits.
460 2016-12-02 17:12:20	0|sdaftuar|but you know it's already in the mempool
461 2016-12-02 17:12:39	0|sdaftuar|so almost by definition, those limits won't be violated
462 2016-12-02 17:12:56	0|sdaftuar|(turns out there is some weird edge case behavior where it could be, but i think that's beside the point)
463 2016-12-02 17:14:44	0|sdaftuar|oh, maybe i didn't make this clear: we call CMPA when we try to accept a new tx to the mempool.  so if it's already gotten in -- a precondition of your code -- then calling it again on that tx, with the same limits, really shouldn't fail.
464 2016-12-02 17:18:47	0|instagibbs|ok now I'm confused why this works in the general case then.
465 2016-12-02 17:18:51	0|instagibbs|with default values
466 2016-12-02 17:19:00	0|sdaftuar|hmm.  do you have a test i can look at?
467 2016-12-02 17:19:45	0|instagibbs|yes but its very easy to test the base case
468 2016-12-02 17:19:54	0|instagibbs|let me push my test im working on
469 2016-12-02 17:20:55	0|instagibbs|just send all your coins to yourself in a loop 26 times
470 2016-12-02 17:21:14	0|instagibbs|last time will trigger ATMP failure without these lines, with it triggers "Insufficient funds"
471 2016-12-02 17:21:30	0|sdaftuar|interesting, i'll take a look
472 2016-12-02 17:25:25	0|instagibbs|you're right though, this makes no sense in retrospect
473 2016-12-02 17:28:38	0|instagibbs|wait... how do you write all those tests o_0
474 2016-12-02 17:28:47	0|sdaftuar|slowly :)
475 2016-12-02 17:29:22	0|instagibbs|i think it spawns zombies if you run in batch mode, be careful :P
476 2016-12-02 17:30:20	0|instagibbs|that test has "10" as the limit, but it successfully sends 25 times
477 2016-12-02 17:30:28	0|instagibbs|no matter what it's set to
478 2016-12-02 17:30:48	0|instagibbs|and the error message changes to mempool entry failure if you set a different value...
479 2016-12-02 17:31:48	0|sdaftuar|ah i think i see what's happening.
480 2016-12-02 17:32:03	0|sdaftuar|there might be some edge case bugs in CMPA we ought to fix
481 2016-12-02 17:33:10	0|sdaftuar|oh but it looks like for limit purposes, CMPA is trying to calculate whether adding the tx given would cause the limits to be exceeded.
482 2016-12-02 17:33:18	0|sdaftuar|ugh, this is kind of messy
483 2016-12-02 17:34:05	0|gmaxwell|sdaftuar: you're clear on the goal right? don't consider any coin we couldn't actually spend.
484 2016-12-02 17:34:32	0|sdaftuar|gmaxwell: yes, understood.
485 2016-12-02 17:35:17	0|gmaxwell|I wouldn't worry much about the coinselection being slow if you have many unspent inputs-- so long as slow isn't so slow as to cause rpc timeouts.
486 2016-12-02 17:36:11	0|gmaxwell|The code that figured out if all the parents were confirmed or ismine used to have factorial complexity... nice natural rate limitor on building large unconfirmed chains. :)
487 2016-12-02 17:36:39	0|sdaftuar|gmaxwell: instagibbs: i have two approaches to suggest, not sure which is better:
488 2016-12-02 17:37:54	0|sdaftuar|1) do basically what you do here, except using CMPA correctly (i'd need to figure out exactly how to do that, certainly you could pass in a tx that spends pcoins, or maybe there's a way to call CMPA with adjusted limit values that works, not sure)
489 2016-12-02 17:38:23	0|sdaftuar|2) don't bother calling CMPA in AvailableCoins, and instead just check to see if pcoins has more than N ancestors (a cached value) for some N much less than the default ancestor limit (maybe 5 or 10).
490 2016-12-02 17:38:45	0|sdaftuar|and then find a later point in the wallet code to abort the send if the descendant count would be violated
491 2016-12-02 17:39:35	0|instagibbs|I like (2) for now, just because I can't tell how CMPA is working sometimes :)
492 2016-12-02 17:39:39	0|gmaxwell|for the case that people hit, really it's just the ancestor limit that actually matters: people chaining change.
493 2016-12-02 17:39:43	0|instagibbs|yep
494 2016-12-02 17:39:54	0|sdaftuar|yeah the problem with 1) is that you don't know the size of the tx you're generating, so it's always going to be possible for the tx to ultimately fail
495 2016-12-02 17:40:12	0|sdaftuar|2) is easy, but may result in some rare annoyances
496 2016-12-02 17:40:45	0|gmaxwell|sdaftuar: this is a sign we should reconsider that relay policy... the size is cornercase enough that it shouldn't really matter.
497 2016-12-02 17:41:50	0|sdaftuar|gmaxwell: we knew the descendant count/size was a theoretical problem from the start (someone else spending outputs of a tx that pays to you can prevent you from relaying new transactions until those other ones clear), but i believe it's the
498 2016-12-02 17:42:00	0|sdaftuar|descendant count/size stuff that limits DoS attacks on the mempool the most
499 2016-12-02 17:42:08	0|instagibbs|Ok, I'm going to learn more about CMPA, then likely do (2) instead
500 2016-12-02 17:42:38	0|sdaftuar|"those other ones clear" <--- should be until the parent confirms
501 2016-12-02 17:44:53	0|gmaxwell|sdaftuar: why do you suggest above going 'much less' than the ancestor limit?
502 2016-12-02 17:45:58	0|sdaftuar|gmaxwell: i just think that we should steer well clear of the relay policy limits.  partly just philosophy (we didn't think there were any common use cases close the values we chose)
503 2016-12-02 17:46:17	0|sdaftuar|but also practically, being near the ancestor limit makes it more likely some peer will think you're violating, say, the descendant limit
504 2016-12-02 17:46:44	0|bitcoin837|hey guys, before I write my own, does anyone already have a bash script that interacts with bitcoin-cli to do a sendmany split of a large chunk to your own deposit addresses?
505 2016-12-02 17:46:46	0|gmaxwell|makes sense. well doing 5 less would probably provide plenty of safty there.
506 2016-12-02 17:46:47	0|sdaftuar|(breaking relay)
507 2016-12-02 17:48:15	0|instagibbs|bitcoin837, #bitcoin
508 2016-12-02 17:48:28	0|bitcoin837|sure
509 2016-12-02 17:48:31	0|instagibbs|np
510 2016-12-02 18:07:42	0|sdaftuar|instagibbs: gmaxwell: there's a problem with this approach
511 2016-12-02 18:08:07	0|sdaftuar|just talked to morcos, and one thing we realized is that you might be combining lots of inputs, each with many ancestors
512 2016-12-02 18:08:19	0|sdaftuar|so the resulting transaction would fail
513 2016-12-02 18:08:21	0|gribble|https://github.com/bitcoin/bitcoin/issues/9260 | Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) by TheBlueMatt · Pull Request #9260 · bitcoin/bitcoin · GitHub
514 2016-12-02 18:09:02	0|sdaftuar|so the best thing we could do in AvailableCoins is eliminate any coins that are already at the ancestor limit, i guess.  but we need additional logic in SelectCoins..() i think
515 2016-12-02 18:11:34	0|instagibbs|wait, why is this different than what we were going to do?
516 2016-12-02 18:11:43	0|instagibbs|i thought that was the definition of ancestor number
517 2016-12-02 18:11:55	0|instagibbs|oh i see, right
518 2016-12-02 18:12:22	0|instagibbs|I had this thought a couple minutes ago and somehow lost it. Yes, 2 inputs may have n-1 history, making 2n-2
519 2016-12-02 18:12:40	0|sdaftuar|yep.  or making n-1, if they're different outputs of the same tx
520 2016-12-02 18:12:48	0|sdaftuar|we have to check later on
521 2016-12-02 18:13:00	0|instagibbs|Yep. glad someone else thought of it and actually communicated
522 2016-12-02 18:13:16	0|gmaxwell|In the case people have actually encountered, it's just a decendant chain limit AFAIK.   Many of the other cases would be pretty hard to hit with the wallet behavior.
523 2016-12-02 18:14:02	0|instagibbs|does the wallet prefer shorter unconfirmed chains vs longer?
524 2016-12-02 18:14:09	0|instagibbs|I know it prefers confirmed change first, etc
525 2016-12-02 18:14:15	0|gmaxwell|It is completely blind to it right now.
526 2016-12-02 18:14:25	0|instagibbs|ok, so it might not be all that rare
527 2016-12-02 18:14:43	0|gmaxwell|To make it prefer shorter is easy: first make it so it can reject if it's too long, and try again with a higher limit if that fails.
528 2016-12-02 18:15:22	0|sdaftuar|i think one way we could do that is to make SelectCoinsMinConf do the checking against some passed in limit, and then call it twice or something
529 2016-12-02 18:15:29	0|sdaftuar|we already call SelectCoinsMinConf repeatedly
530 2016-12-02 18:15:34	0|gmaxwell|thats how we handled unconfirmed, yep.
531 2016-12-02 18:15:41	0|instagibbs|yeah its the same idea
532 2016-12-02 18:16:10	0|instagibbs|prefer deep confirmed coins from others, prefer confirmed coins, ok now try unconfirmed <-- I think today
533 2016-12-02 18:16:51	0|gmaxwell|it does 6,1,0  and it could become 6,1,0+1i,0+10i,0+20i.
534 2016-12-02 18:16:51	0|sdaftuar|the other approach would be if there's a way to do it directly in ApproximateBestSubset?
535 2016-12-02 18:16:54	0|sdaftuar|not sure if that's a good idea
536 2016-12-02 18:17:26	0|instagibbs|ABS assumes you have enough coins i believe
537 2016-12-02 18:17:37	0|instagibbs|which means if you run into too long chain, that will be different and fail
538 2016-12-02 18:18:11	0|gmaxwell|not a great idea to do it in ABS. Just making it a parameter to SelectCoinsMinConf would be straightforward.
539 2016-12-02 18:19:04	0|instagibbs|gmaxwell, why are we using complex numbers for the arg :P
540 2016-12-02 18:19:48	0|sdaftuar|ok, so if we do it in SCMC, then i think the approach would be to filter out of availableCoins the ones with >N ancestors, and testing whether the resulting set of inputs would pass CMPA before returning?
541 2016-12-02 18:19:52	0|sdaftuar|does that sound right?
542 2016-12-02 18:22:56	0|sdaftuar|hmm.  this doens't quite make sense -- once ABS returns coins with enough value, but that violate the chain limits, trying again isn't likely to help, is it?
543 2016-12-02 18:23:16	0|instagibbs|SCMF can say "no"
544 2016-12-02 18:23:30	0|instagibbs|pass failure to SC, returning Insufficent Funds
545 2016-12-02 18:23:31	0|sdaftuar|right, but what is going to do differnetly when invoked with different limits?
546 2016-12-02 18:24:38	0|gmaxwell|sdaftuar: you start with the most restrictive limits first.
547 2016-12-02 18:24:49	0|gmaxwell|and it will only consider coins which will not violate.
548 2016-12-02 18:25:30	0|instagibbs|if it fails to get enough coins under each limit, raise it, if it can never get enough, fails
549 2016-12-02 18:27:04	0|gmaxwell|and if that can't find a solution, you relax the limits and try again.  The first limit is very confirmed, then confirmed, then unconfirmed but 1 deep, then unconfirmed and deeper...  The reason to try with multiple limits is so that it doesn't do something dumb like build a chain of 19 deep, then at the 20's also spend all your other unconfirmed coins... so that the next call has nothing to spe
550 2016-12-02 18:27:10	0|gmaxwell|nd.
551 2016-12-02 18:29:02	0|sdaftuar|gmaxwell: i'm trying to understand how ABS works now, but it's not obvious to me how effective it would be at randomly finding a set of inputs that don't violate the chain limits as the set of available coins it's given increases
552 2016-12-02 18:29:23	0|instagibbs|ABS wouldnt be in charge of it, would have to be higher
553 2016-12-02 18:29:40	0|instagibbs|ABS I believe just has a set of inputs, and tries to make "good" fits based on value
554 2016-12-02 18:30:05	0|sdaftuar|instagibbs: right.  so let's say you have a set of inputs with at most 1 ancestors that has enough value to createa a tx, but the resulting tx has too many ancestors
555 2016-12-02 18:30:29	0|sdaftuar|what are the chances that when you add some more inputs (where it is possible to find a suitable input set that passes the chain limits), that ABS will return it to you?
556 2016-12-02 18:30:44	0|instagibbs|hm the issue seems to continue all the way until you create the actual transaction
557 2016-12-02 18:30:51	0|gmaxwell|yes, there is nothing really we can do about the ancestor limit right now-- it even depends on future and non-local information, we can deal with the decendant limit now.
558 2016-12-02 18:31:25	0|sdaftuar|gmaxwell: did you swap ancestor and descendant in that last line?
559 2016-12-02 18:31:49	0|gmaxwell|yes.
560 2016-12-02 18:32:11	0|gmaxwell|Oh you're also saying for ancestors. Indeed, we can only be greedy, you're right.
561 2016-12-02 18:32:33	0|sdaftuar|if ABS were somehow aware of the constraint, we might get lucky and succeed
562 2016-12-02 18:32:37	0|sdaftuar|but if it's not...
563 2016-12-02 18:33:02	0|gmaxwell|it's not a framework that would likely do well with that kind of constraint in any case.
564 2016-12-02 18:33:44	0|gmaxwell|For some reason I thought the combination rule for ancestor limit was MAX (a depth) not sum.
565 2016-12-02 18:34:03	0|instagibbs|Yeah me too :/
566 2016-12-02 18:34:11	0|sdaftuar|sum reflects how much recordkeeping (and pointer hopping) we do
567 2016-12-02 18:34:56	0|instagibbs|So, we could still have the wallet prefer shorter inputs, and post-transaction finalization, have a CMPA check before returning?
568 2016-12-02 18:35:11	0|instagibbs|shorter-chain inputs greedily*
569 2016-12-02 18:35:37	0|sdaftuar|yes i still think it'd be useful to do what i wrote above: in SCMC, filter out from AvailableCoins the ones that have more than N ancestors, for different values of N
570 2016-12-02 18:35:52	0|gmaxwell|sdaftuar: yes, while realizing that I was wrong from your comment it was clear enough to me why it's sum.
571 2016-12-02 18:36:33	0|sdaftuar|but there are then two failures from SCMC: not enough value, in which case it's worth tryign again with a higher limit; or chain limit exceeded, in which case I'm not sure it's worth calling again with a higher limit
572 2016-12-02 18:37:36	0|sdaftuar|we might fail sometimes to generate a transaction when there are coins available that would work, but this seems like the best we can do right now
573 2016-12-02 18:38:00	0|sdaftuar|and certainly the most important thing is to not commit a transaction that then fails in ATMP
574 2016-12-02 18:38:26	0|sdaftuar|"This must not fail." :)
575 2016-12-02 18:40:10	0|instagibbs|so we'll still need a CMPA call, or something similar, to get the actual ancestor count before returning right
576 2016-12-02 18:40:22	0|instagibbs|otherwise "this can fail" :)
577 2016-12-02 18:40:30	0|sdaftuar|yes.  once we've put the transaction together, we can call CMPA and know whether it'll pass.
578 2016-12-02 18:40:51	0|sipa|i think we should deal correctly with a failed ATMP call, like removing the tx from the wallet in that case
579 2016-12-02 18:41:17	0|sipa|(in addition to doing sanity checks ahead of time)
580 2016-12-02 18:41:42	0|sdaftuar|sipa: agreed
581 2016-12-02 18:41:58	0|instagibbs|what's the barrier to that fix? I don't know the wallet well enough
582 2016-12-02 18:42:07	0|sdaftuar|i don't think we ever delete anything now, do we?
583 2016-12-02 18:42:30	0|instagibbs|removeprunedfunds :P
584 2016-12-02 18:42:36	0|instagibbs|but no, not normallly
585 2016-12-02 18:43:02	0|sdaftuar|oh, neat.  i didn't know that existed!
586 2016-12-02 18:43:38	0|instagibbs|it's meant to be for importing/removing proofs of payment without scanning
587 2016-12-02 18:44:45	0|morcos|is the reason we commit first in case the node crashes?
588 2016-12-02 18:44:53	0|sdaftuar|so we can just zap tx's if we create them but ATMP fails?  that seems easy
589 2016-12-02 18:45:05	0|morcos|we could alwasy have ATMP(fJustCheck)
590 2016-12-02 18:45:33	0|sdaftuar|morcos: i assumed that's why we commit first
591 2016-12-02 18:46:03	0|sdaftuar|morcos: ATMP(fJustCheck) doesn't work very well with RBF and mempool eviction
592 2016-12-02 18:46:05	0|instagibbs|morcos, hmm I thought there was issues with that, otherwise we'd have an easy way to check in rpc if policy is ok with it?
593 2016-12-02 18:46:17	0|instagibbs|^ ok that
594 2016-12-02 18:46:24	0|morcos|ah, yes...
595 2016-12-02 18:46:38	0|sdaftuar|maybe RBF isn't a big deal, not sure
596 2016-12-02 18:47:27	0|gmaxwell|someone should just remove that comment, it's not true. "Must not fail perminantly" it's fine if it fails until some transactions confirm.
597 2016-12-02 18:47:33	0|morcos|ok.. well we just want to be a bit careful.. b/c even things that do get rejected from ATMP, first make it into the memory pool.   so we'll want to make sure no one ever makes it so those coudl get relayed or remembered in some other way, if we're about nuke them from teh wallet
598 2016-12-02 18:47:55	0|gmaxwell|right, it's important to never remove something from the wallet that could have relayed.
599 2016-12-02 18:48:19	0|morcos|or remembered in the future opportunistic eviction rejection map
600 2016-12-02 18:48:32	0|sdaftuar|morcos: oh! now i see what you mean.
601 2016-12-02 18:49:43	0|gmaxwell|::sigh::
602 2016-12-02 18:50:08	0|gmaxwell|The thing I was hoping this issue would fix is the wallet needlessly maining excessively deep transactions when it could choose inputs and avoid it.
603 2016-12-02 18:51:30	0|instagibbs|we can still do that, yes?
604 2016-12-02 18:51:50	0|gmaxwell|Removing things from the wallet on ATMP failure might be fine (though if it'll relay after the next block ... it might have been better to just leave it), but it won't avoid needlessly creating unattractive transactions.
605 2016-12-02 18:51:54	0|gmaxwell|instagibbs: sure.
606 2016-12-02 18:53:01	0|morcos|gmaxwell: yeah i think we get it, we're jsut trying to fix up the edges too
607 2016-12-02 18:53:50	0|morcos|but as i think you mentioned the other day, as it is now it won't get relayed after the next block, b/c you don't rebroadcast whats not in your mempool and you don't try to reaccept whats in your wallet
608 2016-12-02 18:54:34	0|instagibbs|is this referring to ATMP failed transactions?
609 2016-12-02 18:54:36	0|morcos|but we all agree that first you try with things that in the simple case (single stranded chains) will stay well clear of the limits
610 2016-12-02 18:55:09	0|sdaftuar|instagibbs: yeah there's a bug now, where the code that tries to periodically relay unconfirmed wallet transactions skips over things that aren't in the mempool
611 2016-12-02 18:55:17	0|sdaftuar|instagibbs: i think it's an easy fix, just try to reaccept to the mempool first
612 2016-12-02 18:56:31	0|morcos|sdaftuar: i dont know if i 100% agree thats a bug, but certainly a separate issue
613 2016-12-02 18:56:43	0|gmaxwell|We we should fix rebroadcast transactions to try remempolling things. :)
614 2016-12-02 18:57:19	0|sdaftuar|morcos: right, i guess reaccepting makes it harder for you to abandon a too-low-fee tx?
615 2016-12-02 18:57:29	0|gmaxwell|thats a day one bug, considering reorgs/doublespends.
616 2016-12-02 18:57:42	0|morcos|gmaxwell: at the risk of tangents.. the danger with that is that i think we'll go into these endless work loops of trying things that have long since been double spent or whatever
617 2016-12-02 18:58:01	0|morcos|sdaftuar: no its easy enough to skip abandoned, we already skip reaccepting those on startup
618 2016-12-02 18:58:20	0|morcos|but yes, it makes the auto-defacto-abandoned state disappear, which is not necessarily a good thing
619 2016-12-02 18:58:48	0|instagibbs|ok, I need coffee bbl
620 2016-12-02 18:59:02	0|gmaxwell|morcos: well it's once per half an hour or whatever... and work per transaction. We also can tell when transactions are conflicted and could skip those.
621 2016-12-02 19:02:33	0|morcos|gmaxwell: i guess we could just ask some users with big wallets to see how long that takes them on startup (maybe we need to put in benching for it), since we already do it then
622 2016-12-02 19:10:07	0|gmaxwell|morcos: the non-mempool part of resend wallet txn wouldn't be needed if we had mempool sync. :P
623 2016-12-02 19:10:40	0|morcos|if we get it right, we can just skip blocks and PoW too.  :P :P
624 2016-12-02 19:18:40	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/d7ba4a233bd5...9e4bb312e695
625 2016-12-02 19:18:41	0|bitcoin-git|13bitcoin/06master 149e4bb31 15MarcoFalke: Merge #9221: [qa] Get rid of duplicate code...
626 2016-12-02 19:18:41	0|bitcoin-git|13bitcoin/06master 14facbfa5 15MarcoFalke: [qa] Get rid of duplicate code
627 2016-12-02 19:18:54	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #9221: [qa] Get rid of duplicate code (06master...06Mf1611-qaMaxUploadDuplCode) 02https://github.com/bitcoin/bitcoin/pull/9221
628 2016-12-02 19:34:54	0|bitcoin-git|13bitcoin/06master 148a70a9d 15wodry: Improvement of documentation of command line parameter 'whitelist'
629 2016-12-02 19:34:54	0|bitcoin-git|[13bitcoin] 15MarcoFalke pushed 2 new commits to 06master: 02https://github.com/bitcoin/bitcoin/compare/9e4bb312e695...c36229b0b2e9
630 2016-12-02 19:34:55	0|bitcoin-git|13bitcoin/06master 14c36229b 15MarcoFalke: Merge #9251: Improvement of documentation of command line parameter 'whitelist'...
631 2016-12-02 19:35:05	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #9251: Improvement of documentation of command line parameter 'whitelist' (06master...06patch-3) 02https://github.com/bitcoin/bitcoin/pull/9251
632 2016-12-02 19:59:37	0|bitcoin-git|[13bitcoin] 15MarcoFalke closed pull request #9064: unify capitalization of "bitcoin address" (06master...06changeCaps) 02https://github.com/bitcoin/bitcoin/pull/9064
633 2016-12-02 20:28:34	0|sdaftuar|BlueMatt: i was trying to verify that the second commit in #9260 is move only.  first, do you have any clever ways to suggest that i could use to do that?
634 2016-12-02 20:28:35	0|gribble|https://github.com/bitcoin/bitcoin/issues/9260 | Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) by TheBlueMatt · Pull Request #9260 · bitcoin/bitcoin · GitHub
635 2016-12-02 20:28:53	0|sdaftuar|second -- i tried to use git blame -C on the net_processing.cpp file, to verify that you didn't introduce any changes
636 2016-12-02 20:29:16	0|sdaftuar|that almost works, though for some reason i can't figure out, it assigns blame to you for a handful of lines around the Misbehaving() function
637 2016-12-02 20:29:53	0|sdaftuar|i can't see any change in that code, but i'm puzzled as to why git thinks a change was introduced
638 2016-12-02 20:30:15	0|sipa|sdaftuar: git diff --patience COMMIT~:oldfile COMMIT:newfile
639 2016-12-02 20:33:54	0|sdaftuar|sipa: thanks, i'll try that
640 2016-12-02 20:40:20	0|morcos|sdaftuar: its not ENTIRELY move only
641 2016-12-02 20:40:49	0|sdaftuar|yeah i know... just trying to figure out what the real diff is
642 2016-12-02 20:41:07	0|sipa|the only diff i saw was some header changes, and the snippet i pasted in a comment
643 2016-12-02 20:43:25	0|sdaftuar|sipa: i think there's a one-line change about feeFilterRounder too?
644 2016-12-02 20:44:04	0|sipa|sdaftuar: ah, he squashed in some changes
645 2016-12-02 20:44:11	0|sipa|i reviewed an earlier version
646 2016-12-02 20:44:13	0|sdaftuar|ah ok
647 2016-12-02 20:44:21	0|sipa|84922e4
648 2016-12-02 20:48:19	0|morcos|btw..  that "almost bug" with saferModifyNewCoins, i forgot i had a node running with my new coinsviewcache patches.  it lost consensus a couple days ago..
649 2016-12-02 20:48:46	0|morcos|investigating now, but i bet it hit that bug... whew.   dodged a bullet
650 2016-12-02 20:48:49	0|sdaftuar|nice.  lets not PR that one. :)
651 2016-12-02 21:03:15	0|instagibbs|sdaftuar, ok updated my PR, thanks for the help
652 2016-12-02 21:05:30	0|jonasschnelli|Isn't this a bug: https://github.com/bitcoin/bitcoin/blob/master/qa/rpc-tests/keypool.py#L40
653 2016-12-02 21:05:36	0|sdaftuar|instagibbs: cool, i'll take a look
654 2016-12-02 21:05:41	0|jonasschnelli|We fill the keypool with a new target size of 3
655 2016-12-02 21:05:52	0|jonasschnelli|then we manage to reserve 4 keys: https://github.com/bitcoin/bitcoin/blob/master/qa/rpc-tests/keypool.py#L45
656 2016-12-02 21:13:09	0|cfields|jonasschnelli: seems to always reserve target + 1
657 2016-12-02 21:14:02	0|cfields|./bitcoin-cli -testnet keypoolrefill 104
658 2016-12-02 21:14:05	0|cfields|2016-12-02 21:13:24 keypool added key 108, size=105
659 2016-12-02 21:19:19	0|sipa|i think that may be the vchDefaultKey...
660 2016-12-02 22:02:58	0|MarcoFalke|no, don't think this is vchDefaultKey
661 2016-12-02 22:03:55	0|MarcoFalke|the target+1 comes from a time when the function was only used by one caller (Maybe getnewaddress or something, where you first call keypoolrefill and then grab one key)
662 2016-12-02 22:04:21	0|MarcoFalke|Now if you call it from another place which does not grab the key, you end up with off-by-one
663 2016-12-02 22:34:36	0|instagibbs|sdaftuar: hmm the in mempool check done in AvailableCoins doesn't cover that case?
664 2016-12-02 22:34:59	0|instagibbs|Oh I see confirmation plus not mempool
665 2016-12-02 23:20:47	0|BlueMatt|wumpus: re: compilation memusage, validation.cpp takes ~1.19G, init ~1.1G, net_processing ~0.95G, rpcrawtx ~0.95G, rpcdump ~0.99G, rpcwallet ~1.0G, wallet ~1.2G, walletdb ~0.82G...main took ~1.46G
666 2016-12-02 23:21:17	0|BlueMatt|so while splitting main didnt cut out worst-case memusage by a /ton/, it did potentially make it possible to compile with 1.5G again, whereas it previously wasnt once you include the host
667 2016-12-02 23:21:32	0|sipa|but validation + net_processing together now take 2.14G!
668 2016-12-02 23:21:45	0|sipa|bad job!
669 2016-12-02 23:21:57	0|BlueMatt|lol
670 2016-12-02 23:22:42	0|luke-jr|need to do -flto with under 1 GB RAM use ‼‼‼‼! :p
671 2016-12-02 23:22:55	0|BlueMatt|luke-jr: I cant get lto to build anymore
672 2016-12-02 23:26:25	0|gmaxwell|BlueMatt: build time is greater for me too.
673 2016-12-02 23:26:46	0|BlueMatt|greater like slower or faster?
674 2016-12-02 23:26:49	0|luke-jr|it would be nice if you could compile "split LTO data" for libraries, so one can just use non-LTO for their system but have the ability to do LTO on demand
675 2016-12-02 23:27:08	0|gmaxwell|BlueMatt: slower. takes more time.
676 2016-12-02 23:27:31	0|BlueMatt|gmaxwell: yea, not surprising given how much shit we have in headers (and how much boost has in headers!)
677 2016-12-02 23:27:34	0|gmaxwell|bad except when napping is required. presumably its faster on things other than my laptop.
678 2016-12-02 23:27:40	0|gmaxwell|yea, need to get rid of boost.
679 2016-12-02 23:27:47	0|sipa|gmaxwell: are you using parallel lto linking?
680 2016-12-02 23:28:05	0|gmaxwell|sipa: no just the seperate files require more time in total.
681 2016-12-02 23:28:14	0|gmaxwell|due to all the garbage in headers.
682 2016-12-02 23:28:18	0|BlueMatt|gmaxwell: well even if it is slower, at least you dont have to wait for net_processing if you only want to touch validation or vica versa :p
683 2016-12-02 23:28:32	0|gmaxwell|I'm not complaining, its the right thing to do.
684 2016-12-02 23:29:05	0|gmaxwell|though the recompile benefit is mixed, it isn't all that often that I don't end up editing a header that causes everything to get recompiled. P
685 2016-12-02 23:29:10	0|gmaxwell|:P
686 2016-12-02 23:29:16	0|BlueMatt|i know, but I am complaining about headers in bitcoin core (and boost)
687 2016-12-02 23:29:20	0|sipa|gmaxwell: which gcc version? earlier gccs compiled both to object code and gimple in lto mode; in later ones the default is generating only gimple, which is faster
688 2016-12-02 23:29:40	0|BlueMatt|sipa: I dont think we're talking about lto
689 2016-12-02 23:30:04	0|sipa|ah.
690 2016-12-02 23:37:09	0|BlueMatt|sdaftuar: ok with not fixing your comments in #9260?
691 2016-12-02 23:37:10	0|gribble|https://github.com/bitcoin/bitcoin/issues/9260 | Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) by TheBlueMatt · Pull Request #9260 · bitcoin/bitcoin · GitHub
692 2016-12-02 23:37:14	0|gmaxwell|I'm not talking about LTO.
693 2016-12-02 23:41:38	0|BlueMatt|paveljanik: you got a minute to ack 9260, since you already looked over it?