12016-12-02T00:00:54  *** aalex has quit IRC
  22016-12-02T00:03:00  *** cysm has joined #bitcoin-core-dev
  32016-12-02T00:03:36  *** bsm117532 has joined #bitcoin-core-dev
  42016-12-02T00:04:21  *** Evel-Knievel has quit IRC
  52016-12-02T00:07:40  <bitcoin-git> [bitcoin] sipa pushed 6 new commits to master: https://github.com/bitcoin/bitcoin/compare/ad826b3df9f7...dc6dee41f7cf
  62016-12-02T00:07:41  <bitcoin-git> bitcoin/master 4a6b1f3 Matt Corallo: Expose AcceptBlockHeader through main.h
  72016-12-02T00:07:41  <bitcoin-git> bitcoin/master 63fd101 Matt Corallo: Split ::HEADERS processing into two separate cs_main locks...
  82016-12-02T00:07:42  <bitcoin-git> bitcoin/master a8b936d Matt Corallo: Use exposed ProcessNewBlockHeaders from ProcessMessages
  92016-12-02T00:07:51  <bitcoin-git> [bitcoin] sipa closed pull request #9183: Final Preparation for main.cpp Split (master...net_processing_5) https://github.com/bitcoin/bitcoin/pull/9183
 102016-12-02T00:08:50  *** jtimon has quit IRC
 112016-12-02T00:10:04  <bitcoin-git> [bitcoin] TheBlueMatt opened pull request #9260: Mrs Peacock in The Library with The Candlestick (killed main.{h,cpp}) (master...net_processing_file) https://github.com/bitcoin/bitcoin/pull/9260
 122016-12-02T00:10:34  *** bsm117532 has quit IRC
 132016-12-02T00:10:35  *** Evel-Knievel has joined #bitcoin-core-dev
 142016-12-02T00:11:33  <sipa> BlueMatt: haha
 152016-12-02T00:11:50  <BlueMatt> did you see the pr text?
 162016-12-02T00:12:30  <sipa> yes
 172016-12-02T00:12:45  *** alpalp has joined #bitcoin-core-dev
 182016-12-02T00:16:58  <bitcoin272> hey guys I'm curious, why was 25 chosen for the ancestor count?
 192016-12-02T00:18:25  <BlueMatt> ugh, git isnt smart enough to realize when you rename a file and then create a dumb #include "newfile.h" is a move :(
 202016-12-02T00:19:14  <Eliel> BlueMatt: it should understand it if you do the rename with git mv
 212016-12-02T00:19:47  <sipa> BlueMatt: where does it not realize this?
 222016-12-02T00:20:01  <BlueMatt> main.h -> validation.h; main.h == #include "validation.h"
 232016-12-02T00:20:06  <BlueMatt> because i dont want to touch literally every file
 242016-12-02T00:20:39  <sipa> i don't understand what the problem is
 252016-12-02T00:21:08  <BlueMatt> will make rebase of ~everything impossible
 262016-12-02T00:21:21  <BlueMatt> git is smart when rebasing/merging across moves otherwise
 272016-12-02T00:22:02  <BlueMatt> Eliel: no, it doesnt cache that info
 282016-12-02T00:22:07  <Eliel> if it won't understand it in a single commit, try first renaming and then creating the new file.
 292016-12-02T00:22:12  <BlueMatt> it regenerates it itself, so there is no place for it to figure it out
 302016-12-02T00:22:14  <Eliel> in two commits
 312016-12-02T00:22:16  <BlueMatt> yea
 322016-12-02T00:26:56  *** bsm117532 has joined #bitcoin-core-dev
 332016-12-02T00:36:36  *** justanotheruser is now known as Hismione
 342016-12-02T00:40:30  *** bitcoin272 has quit IRC
 352016-12-02T00:41:57  <sipa> BlueMatt: does it still do this tracking after a merge commit is introduced around the two commits?
 362016-12-02T00:42:02  <sipa> maybe it treats it as one then
 372016-12-02T00:42:16  <BlueMatt> sipa: I have no idea...
 382016-12-02T00:42:37  <gmaxwell> bitcoin272: measurements on the actual network, combined with the compute time created for longer chains (They're more expensive to process).
 392016-12-02T00:42:57  <sipa> BlueMatt: can you check? if not, it's probably not worth deviating from the "every commit needs to compile and run tests" policy
 402016-12-02T00:43:06  <BlueMatt> willdo
 412016-12-02T00:43:12  <sipa> actually, i'll check myself - i want to know
 422016-12-02T00:43:28  <BlueMatt> heh, ok
 432016-12-02T00:43:42  * BlueMatt is currently checking which files still dont compile with 1GB of memory in kvm....
 442016-12-02T00:44:00  <sipa> good
 452016-12-02T00:45:04  <BlueMatt> lots of them, it looks like :(
 462016-12-02T00:45:45  <sipa> BlueMatt: doesn't seem to work
 472016-12-02T00:45:52  <BlueMatt> across a merge?
 482016-12-02T00:45:55  <BlueMatt> damn git
 492016-12-02T00:46:21  <BlueMatt> does rebase work at all across a rename? merge does, but rebase might now
 502016-12-02T00:46:25  <BlueMatt> not, actually
 512016-12-02T00:46:27  <Eliel> maybe you can make it a symlink instead?
 522016-12-02T00:46:39  <BlueMatt> eww
 532016-12-02T00:46:48  <BlueMatt> I think i might rather sed all the files
 542016-12-02T00:46:51  <sipa> i tried rebasing #8580 on top of a merged 9260 (with --no-ff)
 552016-12-02T00:46:53  <gribble> https://github.com/bitcoin/bitcoin/issues/8580 | Make CTransaction actually immutable by sipa · Pull Request #8580 · bitcoin/bitcoin · GitHub
 562016-12-02T00:46:57  <sipa> and it conflicts in main.h
 572016-12-02T00:47:00  <BlueMatt> damn
 582016-12-02T00:47:16  <sipa> like... the whole file is a conflict block
 592016-12-02T00:47:20  <BlueMatt> ok, well I guess its gonna be rebase-hell either way....
 602016-12-02T00:47:36  <sipa> we could choose to leave one of the two named main.h/.cpp
 612016-12-02T00:47:42  <BlueMatt> yea...
 622016-12-02T00:47:53  <sipa> which at least would make rebases that touch the not-moved-out part work
 632016-12-02T00:48:00  <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"
 642016-12-02T00:48:25  <BlueMatt> then do it like when we close merge window so that its easier
 652016-12-02T00:48:45  <Eliel> would symlink cause more trouble than it'd solve?
 662016-12-02T00:48:46  <BlueMatt> alternatively, I could actually go sed the entire codebase
 672016-12-02T00:48:59  <BlueMatt> Eliel: I dont think it'd cause trouble since we have the #define guards
 682016-12-02T00:49:02  <BlueMatt> but its super ugly
 692016-12-02T00:49:14  <BlueMatt> (and, actually, git might not track that, either?)
 702016-12-02T00:49:21  <Eliel> right, true
 712016-12-02T00:49:22  *** alpalp has quit IRC
 722016-12-02T00:49:41  <Eliel> but yes, I think sedding the whole codebase is best
 732016-12-02T00:49:43  <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
 742016-12-02T00:49:58  <BlueMatt> yea
 752016-12-02T00:52:13  <sipa> so just remove the last two commits?
 762016-12-02T00:53:27  <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
 772016-12-02T00:54:31  <sipa> ah, i'd just have left validation.cpp to be main.cpp
 782016-12-02T00:54:47  <sipa> that move can easily be do at the same time as the .h rename
 792016-12-02T00:55:39  <BlueMatt> welllll....i mean sed wont cuase (m)any rebase issues........
 802016-12-02T00:56:25  <sipa> yes, but it's a weird state to have main.h but validation.cpp
 812016-12-02T00:56:33  <sipa> and there is no need to that move early
 822016-12-02T00:56:56  <BlueMatt> there is also no need to wait on the wholesale main/validation move/sed
 832016-12-02T00:56:57  <BlueMatt> :p
 842016-12-02T00:57:16  <sipa> well, so either do the whole thing now, or not at all
 852016-12-02T00:57:28  <BlueMatt> I'm doing it now :)
 862016-12-02T01:24:51  *** randy-waterhouse has quit IRC
 872016-12-02T01:29:49  *** Chris_Stewart_5 has quit IRC
 882016-12-02T01:42:50  *** Chris_Stewart_5 has joined #bitcoin-core-dev
 892016-12-02T01:50:37  <phantomcircuit> sipa, after looking at 8831 again i can see why you wanted to not have the ReadKeyValue logic in CWallet
 902016-12-02T01:50:55  <phantomcircuit> im not sure a class with virtual functions is going to be enough either though
 912016-12-02T01:52:23  *** mrkent has quit IRC
 922016-12-02T01:54:30  *** abpa has quit IRC
 932016-12-02T02:11:07  <phantomcircuit> sipa, i could just add a bunch of methods to CWallet like LoadName LoadPurpose etc
 942016-12-02T02:17:12  *** rafalcpp has quit IRC
 952016-12-02T02:17:58  *** rafalcpp has joined #bitcoin-core-dev
 962016-12-02T02:25:41  <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?
 972016-12-02T02:27:36  <sipa> morcos: uh, right
 982016-12-02T02:28:46  <morcos> gmaxwell: in the code that doesn't store an orphan if its parent was rejected...  are we sure that's a good idea?
 992016-12-02T02:29:04  <morcos> if a parent had too low fee
1002016-12-02T02:29:21  <morcos> it is entirely possible that the parent + orphan would then stay in the mempool
1012016-12-02T02:29:50  <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..
1022016-12-02T02:30:12  <morcos> what do you mean we won't request it?
1032016-12-02T02:30:34  <gmaxwell> while it's in the reject filter we won't request it.
1042016-12-02T02:31:15  <gmaxwell> ('cause thats what the reject filter does)
1052016-12-02T02:31:47  <morcos> argh, thats just kind of an accident about the way alreadyhave works though right?
1062016-12-02T02:31:53  <gmaxwell> Am I confused? Highly likely... I have a cold.
1072016-12-02T02:32:13  <gmaxwell> Well I thought that was the intent-- we don't want to request things we 'know' we will just reject.
1082016-12-02T02:32:25  <morcos> i mean in the orphan processing code we're specifically requesting the parents, but you're right we "AlreadyHave" things that are recentrejects
1092016-12-02T02:32:59  <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.
1102016-12-02T02:33:08  <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)
1112016-12-02T02:33:21  <morcos> and requesting them as a parent, in which case, maybe you want to try again
1122016-12-02T02:34:46  <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
1132016-12-02T02:34:53  <morcos> but i guess we can't quite do that
1142016-12-02T02:35:08  <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.
1152016-12-02T02:35:40  <morcos> does that work for non-POD
1162016-12-02T02:36:10  <gmaxwell> 'works like' I don't mean the lockless aspects of it either, but just the fact that there is a generation/deleted flag.
1172016-12-02T02:37:37  <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
1182016-12-02T02:37:45  <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.
1192016-12-02T02:38:01  <gmaxwell> But I dunno if it's better to spend time working on that or mempool sync.
1202016-12-02T02:39:13  <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
1212016-12-02T02:39:55  <gmaxwell> hah
1222016-12-02T02:39:59  <BlueMatt> I'm still a fan
1232016-12-02T02:40:55  <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)
1242016-12-02T02:41:40  <gmaxwell> but it's easy to venture into overdesign.
1252016-12-02T02:41:48  <gmaxwell> OTOH, Fibre exists and is pretty awesome.
1262016-12-02T02:42:01  <gmaxwell> and I could have also said that it was a crazy excercise in chasing optimality.
1272016-12-02T02:43:09  <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
1282016-12-02T02:43:45  <BlueMatt> lol, by adding #include <boost/thread.hpp> to fix windows build error, net_processing.cpp ticked over the 1GB-memory-usage mark :(
1292016-12-02T02:43:50  <BlueMatt> fucking boost
1302016-12-02T02:44:01  <BlueMatt> gmaxwell: I'm more excited by better relay privacy
1312016-12-02T02:44:21  <gmaxwell> morcos: I've been monitoring a bit of that on and off.
1322016-12-02T02:44:42  <sipa> BlueMatt: i think more recent gccs have lower memory usage? :0
1332016-12-02T02:44:45  <gmaxwell> actually I find a lot of the blocks that are almost perfectly reconstucted miss due to replacements / doublespends.
1342016-12-02T02:45:03  <morcos> gmaxwell: how much of the gap can you close?
1352016-12-02T02:45:05  <BlueMatt> sipa: I'm sure, this is what was in default-debian on digitalocean
1362016-12-02T02:45:11  <morcos> replacements/doublespends that you heard about?
1372016-12-02T02:45:15  <gmaxwell> Yes.
1382016-12-02T02:45:27  <morcos> but weren't RBF?
1392016-12-02T02:45:33  <BlueMatt> huh, can anyone reproduce travis' hangs on #9260
1402016-12-02T02:45:33  <morcos> why didnt you have them?
1412016-12-02T02:45:35  <BlueMatt> i cant...
1422016-12-02T02:45:35  <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
1432016-12-02T02:45:41  <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)
1442016-12-02T02:45:55  <gmaxwell> morcos: some were RBF.
1452016-12-02T02:46:05  <gmaxwell> Though I did see a doublespend that wasn't at least once.
1462016-12-02T02:46:13  <BlueMatt> oh, maybe I can
1472016-12-02T02:46:14  <morcos> yeah, so thats a good use of your eager eviction
1482016-12-02T02:46:32  <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.
1492016-12-02T02:47:01  <morcos> gmaxwell: ha, even easier than i thought
1502016-12-02T02:47:02  <BlueMatt> oops lol
1512016-12-02T02:47:16  <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...
1522016-12-02T02:47:42  <gmaxwell> morcos: I think 'few' might only be <5 so you might want to turn that up.
1532016-12-02T02:48:05  <gmaxwell> The purpose of that logging was to explore the impacts of prefill policies, and we wouldn't ever prefill more than a couple.
1542016-12-02T02:48:11  <sipa> yup, 5
1552016-12-02T02:48:14  <sipa> <=5
1562016-12-02T02:48:40  <gmaxwell> (I don't think prefill is worth working on until we eliminate all the preventable misses)
1572016-12-02T02:49:08  <gmaxwell> (e.g. by using the orpan pool and by keeping at least replacements around in some kind of pool...)
1582016-12-02T02:49:28  *** abpa has joined #bitcoin-core-dev
1592016-12-02T02:49:31  *** rafalcpp has quit IRC
1602016-12-02T02:49:49  *** rafalcpp has joined #bitcoin-core-dev
1612016-12-02T02:50:11  <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
1622016-12-02T02:50:36  <morcos> for instance one kind of obvious prefill policy is if the tx is non-standard (such as >100k) prefill, but then thats big
1632016-12-02T02:50:57  <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.
1642016-12-02T02:53:22  <gmaxwell> morcos: of course, there is also the Fibre technique where you send FEC data... and then all the peers could usefully contribute. :P
1652016-12-02T02:54:02  <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.
1662016-12-02T02:54:44  <morcos> yeah we should probably right a new mining api first.  :)
1672016-12-02T02:54:50  <morcos> s/right/write/
1682016-12-02T02:54:54  <gmaxwell> Yea, no kidding.
1692016-12-02T02:56:40  <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'.
1702016-12-02T03:11:03  *** jtimon has joined #bitcoin-core-dev
1712016-12-02T03:12:18  * jtimon rebased #8855 again
1722016-12-02T03:12:20  <gribble> https://github.com/bitcoin/bitcoin/issues/8855 | Use a proper factory for creating chainparams by jtimon · Pull Request #8855 · bitcoin/bitcoin · GitHub
1732016-12-02T03:12:43  <BlueMatt> sipa: even with gcc 6.2 net_processing ticks over 1GB (incl host)
1742016-12-02T03:15:25  <gmaxwell> :(
1752016-12-02T03:15:31  <gmaxwell> BlueMatt: you have failed at main splitting! :)
1762016-12-02T03:15:48  <jtimon> intirestingly enough, with +15-22 in main.cpp, #8498 doesn't need rebase since sep1
1772016-12-02T03:15:50  <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
1782016-12-02T03:15:58  <BlueMatt> clearly
1792016-12-02T03:16:03  <BlueMatt> to be fair, so does init
1802016-12-02T03:16:24  <jtimon> what did I missed?
1812016-12-02T03:17:56  * jtimon went to the logs
1822016-12-02T03:18:34  <bitcoin-git> [bitcoin] morcos opened pull request #9261: Add unstored orphans with rejected parents to recentRejects (master...orphanRejects) https://github.com/bitcoin/bitcoin/pull/9261
1832016-12-02T03:26:45  <jtimon> BlueMatt: not renaming main to validation in the same PR would make it simpler to review
1842016-12-02T03:27:00  <BlueMatt> jtimon: that pr should be easy to review
1852016-12-02T03:27:09  <BlueMatt> the main.cpp rename is a clean rename, so that especially
1862016-12-02T03:29:22  <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
1872016-12-02T03:29:34  <jtimon> but will keep looking
1882016-12-02T03:31:01  <jtimon> but yeah, review it's not a good argument
1892016-12-02T03:34:31  <BlueMatt> jtimon: see scrollback, there are ~no functions which are not block/tx processing left in validation.cpp
1902016-12-02T03:34:38  <BlueMatt> jtimon: except, yes, globals
1912016-12-02T03:36:11  <jtimon> https://github.com/bitcoin/bitcoin/pull/9260#issuecomment-264365400
1922016-12-02T03:36:36  <BlueMatt> jtimon: what /did/ you review?
1932016-12-02T03:43:22  <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
1942016-12-02T03:43:22  <jtimon> moveonlies
1952016-12-02T03:46:59  *** Giszmo has quit IRC
1962016-12-02T03:54:08  <jtimon> re rename right, git knows the file is renamed but you eat the include changes which I agree is not hard to review
1972016-12-02T03:55:06  *** dermoth has quit IRC
1982016-12-02T03:57:16  <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
1992016-12-02T03:58:18  <wumpus> cfields: I'm not specifically aiming 8811 for any release but I'd sure hope it can be merged before 0.14
2002016-12-02T03:58:46  <wumpus> cfields: as it changes all the RPC dispatch tables it's pretty annoying to keep up to date
2012016-12-02T03:59:29  <wumpus> cfields: I just wish people would test it a bit
2022016-12-02T04:02:18  <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
2032016-12-02T04:05:13  <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)
2042016-12-02T04:05:14  *** dermoth has joined #bitcoin-core-dev
2052016-12-02T04:09:50  <bitcoin-git> [bitcoin] instagibbs opened pull request #9262: Don't consider coins available if too many ancestors in mempool (master...toolong) https://github.com/bitcoin/bitcoin/pull/9262
2062016-12-02T04:37:14  *** fanquake has joined #bitcoin-core-dev
2072016-12-02T04:47:26  *** juscamarena has quit IRC
2082016-12-02T04:48:59  *** CubicEarth has quit IRC
2092016-12-02T04:49:06  <bitcoin-git> [bitcoin] laanwj pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/dc6dee41f7cf...c4d22f6eb216
2102016-12-02T04:49:06  <bitcoin-git> bitcoin/master 10ae7a7 Matt Corallo: Revert "Use async name resolving to improve net thread responsiveness"...
2112016-12-02T04:49:07  <bitcoin-git> bitcoin/master c4d22f6 Wladimir J. van der Laan: Merge #9229: Remove calls to getaddrinfo_a...
2122016-12-02T04:49:21  <bitcoin-git> [bitcoin] laanwj closed pull request #9229: Remove calls to getaddrinfo_a (master...2016-11-gai) https://github.com/bitcoin/bitcoin/pull/9229
2132016-12-02T04:49:31  *** CubicEarth has joined #bitcoin-core-dev
2142016-12-02T04:49:57  <wumpus> btw flagging things as "needs backport" with "0.14" doesn't make much sense :-)
2152016-12-02T04:51:30  <bitcoin-git> [bitcoin] laanwj pushed 1 new commit to 0.13: https://github.com/bitcoin/bitcoin/commit/b172377857f9b5a0b2f43e0e57be9acf82a6c50a
2162016-12-02T04:51:30  <bitcoin-git> bitcoin/0.13 b172377 Matt Corallo: Revert "Use async name resolving to improve net thread responsiveness"...
2172016-12-02T04:52:51  <sipa> wumpus: i merged a few things that still need backporting
2182016-12-02T04:53:19  <wumpus> sipa: that's fine, we should probably do that in a pull grouping them together
2192016-12-02T04:53:51  <wumpus> sipa: I just have a really bad feeling about the async resolving thing so backported that immediately
2202016-12-02T04:54:08  *** CubicEarth has quit IRC
2212016-12-02T04:55:03  <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
2222016-12-02T04:55:43  <wumpus> if not they should be labeled "Needs backport" with milestone 0.13.2
2232016-12-02T04:56:13  <sipa> wumpus: yes
2242016-12-02T05:00:08  <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
2252016-12-02T05:00:32  <wumpus> mind the "This is tricky.  See getaddrinfo_a.c for the reason why this works." comment :)
2262016-12-02T05:02:23  <sipa> that does not look thread safe at all
2272016-12-02T05:02:40  <Lightsword> wumpus, that’s not going to be externally exploitable is it for things that uses getaddrinfo?
2282016-12-02T05:03:26  <wumpus> Lightsword: I don't *think* so, but it depends on what kind of bug this turns out to be
2292016-12-02T05:03:35  <Lightsword> has upstream confirmed?
2302016-12-02T05:05:29  <wumpus> it's not triggered by any specific data the remote is sending, if that's what you're worried about
2312016-12-02T05:06:14  <wumpus> also it's not possible for P2P clients to make bitcoind do DNS lookups
2322016-12-02T05:06:40  <wumpus> so no, I don't think this is a security issue for us, but you can't be too careful right...
2332016-12-02T05:08:50  <wumpus> it's a confirmed use after free, not a NULL pointer dereference
2342016-12-02T05:09:15  <fanquake> When are we planning on releasing 0.13.2 ? Looks like 5 backports left.
2352016-12-02T05:09:36  <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
2362016-12-02T05:09:39  <fanquake> *the first rc of 0.13.2
2372016-12-02T05:09:55  <wumpus> Lightsword: sort of https://sourceware.org/bugzilla/show_bug.cgi?id=20874
2382016-12-02T05:11:06  <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
2392016-12-02T05:13:24  <fanquake> Ok. I need to start attending the meetings, but difficult with time-diff though.
2402016-12-02T05:13:47  <wumpus> yes it's not a good time for australia/most of asia
2412016-12-02T05:14:43  <fanquake> Also, not sure why I said 5 PRs left, there is only 9239, 9194 and 9191 (which includes multiple, mostly test-related fixes).
2422016-12-02T05:15:13  <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.
2432016-12-02T05:21:23  <wumpus> not just to reddit :) https://bitcoincore.org/en/meetings/2016/10/20/
2442016-12-02T05:34:24  <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.
2452016-12-02T05:36:58  <sipa> the meeting is on SHA256(days_since_1970) % 24 UTC.
2462016-12-02T05:38:40  <gmaxwell> I ws thinking of suggesting alternating but then though that there probably isn't a good time for both asia and europe.
2472016-12-02T05:38:42  <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 :-)
2482016-12-02T05:39:50  <wumpus> gmaxwell: right I don't think there is
2492016-12-02T05:39:55  <sipa> gmaxwell: 8 am UTC would be relatively good for both asia and europe
2502016-12-02T05:40:27  <wumpus> 8 am UTC would be quite a good time for me
2512016-12-02T05:41:00  <sipa> (9am in western europe, 4pm in hong kong, midnight on west coast)
2522016-12-02T05:41:09  <sipa> 3am on east coast, however
2532016-12-02T05:41:15  <gmaxwell> thats too late for east coast US though I don't know if we currently have any eastcoasters.
2542016-12-02T05:41:22  <gmaxwell> oh instagibbs is duh.
2552016-12-02T05:41:25  <sipa> morcos, suhas, instagibbs
2562016-12-02T05:41:34  <gmaxwell> oh double duh.
2572016-12-02T05:41:46  <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
2582016-12-02T05:41:56  <gmaxwell> I just think of NY as existing in a parallel universe.
2592016-12-02T05:42:19  <wumpus> e.g. just rebroad asking for it is not enough :p
2602016-12-02T05:42:31  <gmaxwell> well we lose jl2012 due to this.
2612016-12-02T05:42:50  <wumpus> yes and fanquake
2622016-12-02T05:42:58  *** ThomasV has joined #bitcoin-core-dev
2632016-12-02T05:43:55  <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
2642016-12-02T05:44:08  <BlueMatt> who in their right mind would agree to that? thats like stupid high-risk for users
2652016-12-02T05:45:14  <wumpus> it could make sense for software that auto-updates quickly and automatically
2662016-12-02T05:45:18  <wumpus> but no, certainly not for bitcoin
2672016-12-02T05:46:57  <BlueMatt> I mean they're talking about it for "critical infrastructure" (ie common libraries)
2682016-12-02T05:47:16  <BlueMatt> like, sure, maybe google updates their libcs quickly, but the vast majority of folks do not at all
2692016-12-02T05:47:16  <sipa> 4pm UTC: 8am westcoast, 11am eastcoast, 5pm europe, midnight hong kong
2702016-12-02T05:48:46  <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
2712016-12-02T05:49:24  <wumpus> then again that's not google's fault - finding the vulnerabilities before the 'bad people' do is still important
2722016-12-02T05:49:47  <BlueMatt> wumpus: sure, but that doesnt mean you announce them publicly 7 days after the first release with the fix
2732016-12-02T05:50:23  <wumpus> (or alternatively, after the "bad people" have already used them for years to get access anwyay...)
2742016-12-02T05:50:37  <BlueMatt> yes, you fix quickly, and then announce it much later
2752016-12-02T05:50:38  <gmaxwell> BlueMatt: it's fine for projects that are alread well fuzzed, I suppose.
2762016-12-02T05:51:01  <gmaxwell> NSA already found those vulns last week anyways.
2772016-12-02T05:51:02  <BlueMatt> it might be better than /not/ doing the fuzzing, but its still a super shitty policy
2782016-12-02T05:51:46  <wumpus> it's the old responsible disclosure discussion
2792016-12-02T05:51:47  *** Hismione is now known as justanotheruser
2802016-12-02T05:52:23  <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
2812016-12-02T05:53:05  <wumpus> BlueMatt: sure, though part of it is how long to wait with public announcement
2822016-12-02T05:53:18  <BlueMatt> suresure, but who the fuck ever suggested 7 days?
2832016-12-02T06:01:46  <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.
2842016-12-02T06:02:31  <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.
2852016-12-02T06:05:43  <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
2862016-12-02T06:06:07  <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
2872016-12-02T06:07:46  <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.
2882016-12-02T06:07:51  <midnightmagic> *pretty short
2892016-12-02T06:10:46  * midnightmagic tries to remember that italian guy again..
2902016-12-02T06:12:40  *** Ylbam has quit IRC
2912016-12-02T06:13:19  <midnightmagic> ah, here he is. I've come to appreciate the way he does things: http://aluigi.altervista.org/
2922016-12-02T06:13:31  <midnightmagic> (not even vendor notification.)
2932016-12-02T06:23:15  *** Ylbam has joined #bitcoin-core-dev
2942016-12-02T06:44:57  *** cryptapus has joined #bitcoin-core-dev
2952016-12-02T06:45:18  *** cryptapus_afk has quit IRC
2962016-12-02T07:00:05  *** dermoth has quit IRC
2972016-12-02T07:00:51  *** dermoth has joined #bitcoin-core-dev
2982016-12-02T07:02:12  *** ThomasV has quit IRC
2992016-12-02T07:03:10  <bitcoin-git> [bitcoin] luke-jr opened pull request #9263: release notes: Only free transactions are being removed, not priority. (master...relnotes_freetxn) https://github.com/bitcoin/bitcoin/pull/9263
3002016-12-02T07:05:24  *** RoyceX has joined #bitcoin-core-dev
3012016-12-02T07:05:28  *** RoyceX has joined #bitcoin-core-dev
3022016-12-02T07:08:17  *** Cheeseo has quit IRC
3032016-12-02T07:09:08  *** paveljanik has quit IRC
3042016-12-02T07:10:55  *** aalex has joined #bitcoin-core-dev
3052016-12-02T07:12:57  <bitcoin-git> [bitcoin] laanwj opened pull request #9264: 0.13.2 backports (0.13...2016_12_backports_0_13) https://github.com/bitcoin/bitcoin/pull/9264
3062016-12-02T07:16:52  <jonasschnelli> What is preferable: two keypools one for change, one for external  OR  a keypool with keys flagged for internal or external use?
3072016-12-02T07:17:15  <bitcoin-git> [bitcoin] laanwj closed pull request #9191: [qa] 0.13.2 Backports (0.13...Mf1611-q01302) https://github.com/bitcoin/bitcoin/pull/9191
3082016-12-02T07:17:35  <wumpus> possibly the flagging approach is easier to do in a backwards compatible way - old versions can ignore the flags
3092016-12-02T07:18:39  *** aalex has quit IRC
3102016-12-02T07:19:07  <jonasschnelli> wumpus: good point..
3112016-12-02T07:21:04  <jonasschnelli> for the deserialization (SerializationOp(Stream& s, ...)), if the stream is longer then the acctual READWRITE, it will be ignored? right? (for backward comp.)
3122016-12-02T07:21:26  <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?
3132016-12-02T07:21:29  <bitcoin-git> [bitcoin] laanwj pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/c4d22f6eb216...3fbf07926240
3142016-12-02T07:21:30  <bitcoin-git> bitcoin/master d824ad0 Alex Morcos: Disable fee estimates for a confirm target of 1 block
3152016-12-02T07:21:31  <bitcoin-git> bitcoin/master e878689 Alex Morcos: Make GUI incapable of setting tx confirm target of 1
3162016-12-02T07:21:31  <bitcoin-git> bitcoin/master 3fbf079 Wladimir J. van der Laan: Merge #9239: Disable fee estimates for 1 block target...
3172016-12-02T07:21:39  <bitcoin-git> [bitcoin] laanwj closed pull request #9239: Disable fee estimates for 1 block target (master...blockstreamtil2blocks) https://github.com/bitcoin/bitcoin/pull/9239
3182016-12-02T07:25:01  <luke-jr> wumpus: doh, I was about to do that (more backports)
3192016-12-02T07:26:10  <dcousens> BlueMatt: don't miners use priority for their own transactions?
3202016-12-02T07:26:25  <fanquake> jtimon takes 51s here
3212016-12-02T07:26:26  <dcousens> (not the coinbase, just, "other" transactions)
3222016-12-02T07:29:02  <jonasschnelli> wumpus: I could do a BP of #8989
3232016-12-02T07:29:04  <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
3242016-12-02T07:29:32  <wumpus> jonasschnelli: not sure that's what we want though?
3252016-12-02T07:29:34  <luke-jr> dcousens: fee delta works fine for that use case
3262016-12-02T07:29:55  <wumpus> jonasschnelli: I mean, this is a minor release, how much do we want the GUI to change?
3272016-12-02T07:29:56  <jonasschnelli> Yes. It would be a notable change.
3282016-12-02T07:30:15  <wumpus> maybe it's ok though in this case I'm not sure
3292016-12-02T07:30:20  <jonasschnelli> Can we BP #9239 without the GUI changes?
3302016-12-02T07:30:22  <gribble> https://github.com/bitcoin/bitcoin/issues/9239 | Disable fee estimates for 1 block target by morcos · Pull Request #9239 · bitcoin/bitcoin · GitHub
3312016-12-02T07:30:24  <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
3322016-12-02T07:30:39  <wumpus> you could reply that in the #9239 topic, probably morcos has an opinoin on it too :)
3332016-12-02T07:30:40  <gribble> https://github.com/bitcoin/bitcoin/issues/9239 | Disable fee estimates for 1 block target by morcos · Pull Request #9239 · bitcoin/bitcoin · GitHub
3342016-12-02T07:31:07  <jonasschnelli> wumpus: or just disabled (shorten the slider range) for target 1 in the backport
3352016-12-02T07:31:12  * wumpus wishes gribble had rate-limiting
3362016-12-02T07:31:16  <wumpus> jonasschnelli: yep, exactly
3372016-12-02T07:31:56  <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.
3382016-12-02T07:31:59  <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
3392016-12-02T07:32:26  <jonasschnelli> Yes. IMO 8989 and 9239 is sort of one BP "package"
3402016-12-02T07:32:43  <wumpus> jonasschnelli: in that case we should just backport both I guess
3412016-12-02T07:32:48  <jonasschnelli> Agree
3422016-12-02T07:33:07  <jonasschnelli> I can do that next week...
3432016-12-02T07:33:17  <jonasschnelli> (if nobody did it in the meantime)
3442016-12-02T07:34:13  <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 :)
3452016-12-02T07:34:23  <luke-jr> wumpus: well, I had started.. but I can rebase :x
3462016-12-02T07:34:44  <luke-jr> I went through the PRs manually, not using tags, so I probably got a few you missed
3472016-12-02T07:34:53  <luke-jr> (well, *almost* manually.. XD)
3482016-12-02T07:35:08  <wumpus> well I strictly backport what is tagged, not more, if not it should have been tagged
3492016-12-02T07:37:24  <luke-jr> wumpus: erg, your backports aren't on top of Marco's? :x
3502016-12-02T07:37:41  <wumpus> luke-jr: no, I forgot about that one, will rebase
3512016-12-02T07:38:21  <wumpus> now they are (luckily no new conflicts)
3522016-12-02T07:41:26  <luke-jr> woo no conflicts here either it seems
3532016-12-02T07:42:48  *** jannes has joined #bitcoin-core-dev
3542016-12-02T07:57:10  <bitcoin-git> [bitcoin] laanwj opened pull request #9265: bitcoin-cli: Make error message less confusing (master...2016_12_rpccli_message) https://github.com/bitcoin/bitcoin/pull/9265
3552016-12-02T07:59:15  <luke-jr> wumpus: pushed backports-0.13 to my github; want to just pull it into yours?
3562016-12-02T08:00:19  *** xiangfu has quit IRC
3572016-12-02T08:00:31  <wumpus> luke-jr: will have a look in a moment
3582016-12-02T08:00:59  *** xiangfu has joined #bitcoin-core-dev
3592016-12-02T08:02:46  *** BashCo has quit IRC
3602016-12-02T08:03:22  *** BashCo has joined #bitcoin-core-dev
3612016-12-02T08:07:44  *** BashCo has quit IRC
3622016-12-02T08:13:59  <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
3632016-12-02T08:15:02  <luke-jr> the priority code will be as well.
3642016-12-02T08:15:21  <sipa> no, it won't
3652016-12-02T08:15:23  <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
3662016-12-02T08:15:28  <sipa> it serves no function anymore
3672016-12-02T08:15:40  <luke-jr> sipa: yes it does, the same function it always served: anti-spam
3682016-12-02T08:15:52  <wumpus> luke-jr: still makes sense to clean them up, but not sure whether the backport is risky
3692016-12-02T08:16:04  <luke-jr> wumpus: I don't care strongly if you want to skip those
3702016-12-02T08:16:25  <luke-jr> the backport may be risky, but if we have an rc or two, I'd put it in
3712016-12-02T08:16:44  <wumpus> luke-jr: also the menu reparenting may have subtle behavior issues I hadn't considered (though none reported yet)
3722016-12-02T08:16:53  <wumpus> yes, sure
3732016-12-02T08:16:57  <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?
3742016-12-02T08:17:22  <sipa> the only scalable way to deal with this is economic incentives
3752016-12-02T08:17:25  <luke-jr> sipa: it would be better than 1000 kb of spam :P
3762016-12-02T08:17:40  <wumpus> lukanyhow I'll pull them into #9264
3772016-12-02T08:17:41  <sipa> and face it, that is how it works in practice now
3782016-12-02T08:17:42  <gribble> https://github.com/bitcoin/bitcoin/issues/9264 | 0.13.2 backports by laanwj · Pull Request #9264 · bitcoin/bitcoin · GitHub
3792016-12-02T08:17:45  <luke-jr> if we go to exclusively economic incentives, we'd need to remove ALL spam filtering..
3802016-12-02T08:18:05  <sipa> luke-jr: there are still local node dos protections
3812016-12-02T08:18:24  <sipa> which everyone has an incentive to maintain
3822016-12-02T08:18:36  <sipa> and don't requires miners to be gatekeepers of good and bad transactions
3832016-12-02T08:19:10  <luke-jr> sipa: if we had a system that wasn't suffering from spam, removing priority might make sense. but we don't.
3842016-12-02T08:19:30  <sipa> luke-jr: removing priority will have 0 impact
3852016-12-02T08:21:14  <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
3862016-12-02T08:21:56  <luke-jr> has someone shown that to be true?
3872016-12-02T08:22:05  *** BashCo has joined #bitcoin-core-dev
3882016-12-02T08:22:09  <luke-jr> last time I looked, a large % of transactions in blocks were in the priority area
3892016-12-02T08:22:33  <luke-jr> (not majority-large, but not <5% either)
3902016-12-02T08:22:55  <sipa> fair enough, i have no actual data on his
3912016-12-02T08:22:59  <sipa> *this
3922016-12-02T08:23:06  <sipa> but how do you measure that?
3932016-12-02T08:23:56  <luke-jr> I wrote a RPC call that analyzed the sort order
3942016-12-02T08:27:41  *** paveljanik has joined #bitcoin-core-dev
3952016-12-02T08:27:57  * luke-jr tries porting it to 0.13
3962016-12-02T08:28:46  *** paveljanik has quit IRC
3972016-12-02T08:40:08  *** arowser_ has quit IRC
3982016-12-02T08:40:22  *** arowser has joined #bitcoin-core-dev
3992016-12-02T08:46:35  *** ThomasV has joined #bitcoin-core-dev
4002016-12-02T08:53:46  *** jannes has quit IRC
4012016-12-02T08:54:09  <luke-jr> ugh this thing is slooooooow
4022016-12-02T09:00:16  *** molz has joined #bitcoin-core-dev
4032016-12-02T09:00:17  *** abpa has quit IRC
4042016-12-02T09:01:17  *** moli has quit IRC
4052016-12-02T09:07:42  <jtimon> for those interested in more configurable testchains: https://github.com/bitcoin/bitcoin/pull/8994#issuecomment-264406053
4062016-12-02T09:17:24  *** ThomasV has quit IRC
4072016-12-02T09:17:55  *** laurentmt has joined #bitcoin-core-dev
4082016-12-02T09:29:34  *** juscamarena has joined #bitcoin-core-dev
4092016-12-02T09:35:47  *** e4xit_ has joined #bitcoin-core-dev
4102016-12-02T09:37:54  *** e4xit has quit IRC
4112016-12-02T09:37:54  *** e4xit_ is now known as e4xit
4122016-12-02T09:50:18  *** kadoban has quit IRC
4132016-12-02T10:02:17  <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.
4142016-12-02T10:23:05  *** cannon-c has joined #bitcoin-core-dev
4152016-12-02T10:25:21  *** laurentmt1 has joined #bitcoin-core-dev
4162016-12-02T10:26:19  *** laurentmt has quit IRC
4172016-12-02T10:26:19  *** laurentmt1 is now known as laurentmt
4182016-12-02T10:35:14  *** xiangfu has quit IRC
4192016-12-02T10:35:46  *** xiangfu has joined #bitcoin-core-dev
4202016-12-02T10:39:34  *** moli has joined #bitcoin-core-dev
4212016-12-02T10:42:21  *** molz has quit IRC
4222016-12-02T11:02:28  *** cannon-c has quit IRC
4232016-12-02T11:09:48  *** Guyver2 has joined #bitcoin-core-dev
4242016-12-02T11:14:11  *** JackH has joined #bitcoin-core-dev
4252016-12-02T11:17:25  *** laurentmt has quit IRC
4262016-12-02T11:21:52  *** ThomasV has joined #bitcoin-core-dev
4272016-12-02T12:00:57  <luke-jr> sipa: CPFP and some other weirdness I don't recognise kinda made my analyzer useless :/
4282016-12-02T12:22:18  *** BashCo_ has joined #bitcoin-core-dev
4292016-12-02T12:22:27  <luke-jr> jonasschnelli: is it just me, or is 0a261b63fd4f1b07431f8a65762ef9f1ef1c11c8 a bugfix that should be backported?
4302016-12-02T12:25:41  *** BashCo has quit IRC
4312016-12-02T12:37:03  *** ThomasV has quit IRC
4322016-12-02T12:52:46  <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.
4332016-12-02T12:56:07  <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.. :/
4342016-12-02T13:00:02  <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)
4352016-12-02T13:00:19  <gmaxwell> s/drob/drop/
4362016-12-02T13:01:31  <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.
4372016-12-02T13:01:38  <luke-jr> wouldn't the latter just be the min fee setting?
4382016-12-02T13:05:20  <gmaxwell> luke-jr: sort of. minfee setting is really a relay setting, it gets in the way of CPFP. Should be intentionally low
4392016-12-02T13:07:50  <luke-jr> perhaps the gradual min-feerate would be a good thing to reintroduce
4402016-12-02T13:07:56  <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.
4412016-12-02T13:08:22  <luke-jr> hmm, would gradual min-feerate break prediction?
4422016-12-02T13:09:37  <gmaxwell> No. At least if constructed right it should improve it (esp if the prediction is aware of it).
4432016-12-02T13:10:20  <morcos> gmaxwell: i'd be strongly in favor of a separate min mining feerate
4442016-12-02T13:10:56  <morcos> do you actually want to remove blockmaxsize (and blockmaxweight?) as options?
4452016-12-02T13:11:42  *** MykelSIlver has joined #bitcoin-core-dev
4462016-12-02T13:11:44  <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
4472016-12-02T13:11:49  <morcos> but thats counterintuitive
4482016-12-02T13:12:01  <morcos> and i'd rather not risk it by default
4492016-12-02T13:12:03  <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)
4502016-12-02T13:12:26  <morcos> right.. so default no setting for blockmaxsize in particular (to avoid size accounting unless you set it)
4512016-12-02T13:12:50  <gmaxwell> right.
4522016-12-02T13:13:53  <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
4532016-12-02T13:13:55  <gribble> https://github.com/bitcoin/bitcoin/issues/9239 | Disable fee estimates for 1 block target by morcos · Pull Request #9239 · bitcoin/bitcoin · GitHub
4542016-12-02T13:14:07  <morcos> which is exactly what happens most of the time now anyway.. so will look like no change in behavior
4552016-12-02T13:14:41  <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)
4562016-12-02T13:15:01  * luke-jr notes that >1 MB blocks are still a problem as-is; it's not like gradual mining min-fee is implemented yet
4572016-12-02T13:16:14  <gmaxwell> luke-jr: yes, but a pointless softcap that virtually everone overrides is still not helping.
4582016-12-02T13:16:27  <luke-jr> gmaxwell: nobody overrides it >1 MB yet AFAIK.
4592016-12-02T13:17:08  <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
4602016-12-02T13:17:16  <morcos> 1) min mining feerate
4612016-12-02T13:17:33  <morcos> 2) rate used to define dust (should change rarely)
4622016-12-02T13:17:57  <morcos> 3) rate used as the minimum increment to pay for relay bandwidth (closest analog to existing minrelaytxfee)
4632016-12-02T13:18:30  <luke-jr> morcos: how would 3 be different from current minrelaytxfee?
4642016-12-02T13:18:46  <morcos> I don't think 3) actually needs to a have a floor other than 0, but i don't suppose it hurts
4652016-12-02T13:18:55  <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.
4662016-12-02T13:19:58  <luke-jr> as soon as it matters, would still be better than immediately by default
4672016-12-02T13:20:27  <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. :)
4682016-12-02T13:20:33  <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
4692016-12-02T13:20:38  *** aalex has joined #bitcoin-core-dev
4702016-12-02T13:21:40  <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.
4712016-12-02T13:21:50  <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.
4722016-12-02T13:22:14  <luke-jr> but we don't have anything sensible and dynamic yet.
4732016-12-02T13:22:16  <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
4742016-12-02T13:24:02  <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.
4752016-12-02T13:24:35  <luke-jr> it doesn't impede anything..
4762016-12-02T13:25:27  <gmaxwell> I promise it does, touching any of this means arguing with you and few people have interest in that.
4772016-12-02T13:25:55  <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.
4782016-12-02T13:26:48  <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
4792016-12-02T13:27:08  <jonasschnelli> Also, CKeyPool has no version-int.
4802016-12-02T13:27:10  <luke-jr> [12:22:27] <luke-jr> jonasschnelli: is it just me, or is 0a261b63fd4f1b07431f8a65762ef9f1ef1c11c8 a bugfix that should be backported?
4812016-12-02T13:27:37  *** aalex has quit IRC
4822016-12-02T13:27:52  <gmaxwell> I said it would be incompatible a bunch of times.
4832016-12-02T13:28:38  <jonasschnelli> I though we could make it compatible.. but yes. I guess it wont.
4842016-12-02T13:29:39  <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.
4852016-12-02T13:29:48  <jonasschnelli> GetKey could derive the priv key on the fly
4862016-12-02T13:30:15  <jonasschnelli> luke-jr: we could bp... but o
4872016-12-02T13:30:33  <jonasschnelli> but i guess its not an important fix
4882016-12-02T13:31:08  *** laurentmt has joined #bitcoin-core-dev
4892016-12-02T13:32:14  <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.
4902016-12-02T13:33:07  <jonasschnelli> gmaxwell: Yes. That would be preferable. I think the splitting should be combined with the on-thy-fly derivation and the flexible keypath.
4912016-12-02T13:33:19  <jonasschnelli> Ideally +pub-CKD
4922016-12-02T13:34:59  <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?
4932016-12-02T13:35:57  <jonasschnelli>  bip44/45 maybe. But I don't think its wise to force users to do pub key derivation.
4942016-12-02T13:36:34  <jonasschnelli> a flexible would allow to use bip44 and we could still stick to m/0'/k' by default
4952016-12-02T13:36:41  <jonasschnelli> a flexible keypath
4962016-12-02T13:37:21  <jcorgan> reviewing the flexpath PR is on my list this morning
4972016-12-02T13:38:15  <jonasschnelli> thanks
4982016-12-02T13:39:41  <jcorgan> but, i'm mostly just wondering if there are any good practices from other wallets we might benefit from understanding
4992016-12-02T13:39:50  <jcorgan> just thinking out loud
5002016-12-02T13:40:53  *** laurentmt has quit IRC
5012016-12-02T13:43:35  <gmaxwell> Doing public derrivation while the software supports private key export is really extremely inadvisable.
5022016-12-02T13:43:49  *** MarcoFalke has joined #bitcoin-core-dev
5032016-12-02T13:44:02  <gmaxwell> I also don't think it makes sense to spend time on advanced features when basic functionality is still kinda broken.
5042016-12-02T13:44:12  <gmaxwell> but I'm not the one working on it. :)
5052016-12-02T13:44:46  <jcorgan> agree, but it's important to define what unbroken basic functionality should be :)
5062016-12-02T13:45:39  <jcorgan> in other words, if there is to be breaking changes, and you only want to break it once, better get it right
5072016-12-02T13:46:50  <jcorgan> i didn't mean to jump into the middle of your thread, just noticed it and taking an interest. carry on.
5082016-12-02T13:47:46  <jonasschnelli> I agree with gmaxwell. We first need to solve the key-chain split.
5092016-12-02T13:48:20  <jonasschnelli> child key derivation is an interesting feature if you want to use core together with a hardware wallet.
5102016-12-02T13:48:27  <jonasschnelli> child pub key d.
5112016-12-02T13:50:35  <bitcoin-git> [bitcoin] luke-jr opened pull request #9266: Qt/RPCConsole: Put column enum in the right places (master...bugfix_datarole) https://github.com/bitcoin/bitcoin/pull/9266
5122016-12-02T13:54:15  *** Giszmo has joined #bitcoin-core-dev
5132016-12-02T14:02:33  *** paveljanik has joined #bitcoin-core-dev
5142016-12-02T14:02:33  *** paveljanik has joined #bitcoin-core-dev
5152016-12-02T14:29:24  *** aalex has joined #bitcoin-core-dev
5162016-12-02T14:34:22  *** ThomasV has joined #bitcoin-core-dev
5172016-12-02T14:43:25  *** moli has quit IRC
5182016-12-02T14:49:52  *** fanquake has quit IRC
5192016-12-02T14:50:36  *** moli has joined #bitcoin-core-dev
5202016-12-02T14:50:52  *** wasi has quit IRC
5212016-12-02T14:51:36  *** wasi has joined #bitcoin-core-dev
5222016-12-02T14:56:17  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/3fbf07926240...31bcc667863f
5232016-12-02T14:56:17  <bitcoin-git> bitcoin/master fe37fbe Wladimir J. van der Laan: bitcoin-cli: Make error message less confusing...
5242016-12-02T14:56:18  <bitcoin-git> bitcoin/master 31bcc66 MarcoFalke: Merge #9265: bitcoin-cli: Make error message less confusing...
5252016-12-02T14:56:32  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9265: bitcoin-cli: Make error message less confusing (master...2016_12_rpccli_message) https://github.com/bitcoin/bitcoin/pull/9265
5262016-12-02T14:59:44  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/31bcc667863f...5412c08c3cf1
5272016-12-02T14:59:44  <bitcoin-git> bitcoin/master b7aa290 S. Matthew English: unification of Bloom filter representation...
5282016-12-02T14:59:45  <bitcoin-git> bitcoin/master 5412c08 MarcoFalke: Merge #9223: unification of Bloom filter representation...
5292016-12-02T14:59:55  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9223: unification of Bloom filter representation (master...patch-10) https://github.com/bitcoin/bitcoin/pull/9223
5302016-12-02T15:21:16  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/5412c08c3cf1...98514988a3d3
5312016-12-02T15:21:16  <bitcoin-git> bitcoin/master 08ed8c1 Gregory Maxwell: Developer docs about existing subtrees....
5322016-12-02T15:21:17  <bitcoin-git> bitcoin/master 9851498 MarcoFalke: Merge #9246: Developer docs about existing subtrees....
5332016-12-02T15:21:27  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9246: Developer docs about existing subtrees. (master...devdocs_for_subtrees) https://github.com/bitcoin/bitcoin/pull/9246
5342016-12-02T15:25:32  *** bsm117532 has quit IRC
5352016-12-02T15:26:37  *** bsm117532 has joined #bitcoin-core-dev
5362016-12-02T15:41:56  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/98514988a3d3...d7ba4a233bd5
5372016-12-02T15:41:56  <bitcoin-git> bitcoin/master 0828619 Suhas Daftuar: [qa] Dump debug logs on travis failures.
5382016-12-02T15:41:57  <bitcoin-git> bitcoin/master d7ba4a2 MarcoFalke: Merge #9257: [qa] Dump debug logs on travis failures....
5392016-12-02T15:42:13  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9257: [qa] Dump debug logs on travis failures. (master...travis-debug-logs) https://github.com/bitcoin/bitcoin/pull/9257
5402016-12-02T15:43:11  *** kadoban has joined #bitcoin-core-dev
5412016-12-02T15:44:22  *** fengling has quit IRC
5422016-12-02T15:45:02  <bitcoin-git> [bitcoin] morcos opened pull request #9267: Disable fee estimates for a confirm target of 1 block (0.13...backport9239) https://github.com/bitcoin/bitcoin/pull/9267
5432016-12-02T15:45:18  *** fengling has joined #bitcoin-core-dev
5442016-12-02T15:45:41  *** Magma has quit IRC
5452016-12-02T15:45:57  *** Magma has joined #bitcoin-core-dev
5462016-12-02T15:53:11  *** Magma has quit IRC
5472016-12-02T15:55:31  *** abpa has joined #bitcoin-core-dev
5482016-12-02T15:55:53  *** molz has joined #bitcoin-core-dev
5492016-12-02T15:56:46  *** abpa has quit IRC
5502016-12-02T15:58:05  *** moli has quit IRC
5512016-12-02T16:06:55  *** laurentmt has joined #bitcoin-core-dev
5522016-12-02T16:06:58  *** laurentmt has quit IRC
5532016-12-02T16:08:48  *** fengling has quit IRC
5542016-12-02T16:10:37  <MarcoFalke> sipa: I think this is needed to get our subtree clean again
5552016-12-02T16:10:41  <MarcoFalke> https://github.com/bitcoin-core/ctaes/pull/5
5562016-12-02T16:11:51  *** ThomasV has quit IRC
5572016-12-02T16:24:43  *** laurentmt has joined #bitcoin-core-dev
5582016-12-02T16:25:59  *** laurentmt has quit IRC
5592016-12-02T16:26:00  <instagibbs> can anyone explain why this isn't setting the argument correctly?:
5602016-12-02T16:26:00  <instagibbs>          self.extra_args = [['-usehd={:d}'.format(i%2==0)] for i in range(4)]
5612016-12-02T16:26:01  <instagibbs> +        self.extra_args[0].append("-limitancestorcount=10")
5622016-12-02T16:26:19  <instagibbs> I'm trying to set a custom limit in test, and it's not being enforced
5632016-12-02T16:30:17  *** 44UAAAJXO has joined #bitcoin-core-dev
5642016-12-02T16:30:17  *** fengling has joined #bitcoin-core-dev
5652016-12-02T16:31:33  <instagibbs> ahhhh nevermind, the node is being spun down and up later in the test
5662016-12-02T16:32:27  *** laurentmt has joined #bitcoin-core-dev
5672016-12-02T16:35:40  *** laurentmt has quit IRC
5682016-12-02T16:37:07  <MarcoFalke> Yeah, we should use self.extra_args as argument where possible, instead of writing out the args every time.
5692016-12-02T16:43:35  *** Magma has joined #bitcoin-core-dev
5702016-12-02T16:44:33  *** 44UAAAJXO has quit IRC
5712016-12-02T16:45:04  *** abpa has joined #bitcoin-core-dev
5722016-12-02T16:47:52  *** kadoban has quit IRC
5732016-12-02T16:49:25  *** jtimon has quit IRC
5742016-12-02T16:58:15  *** BashCo_ has quit IRC
5752016-12-02T16:59:08  *** molz has quit IRC
5762016-12-02T17:03:19  <instagibbs> sdaftuar, how does one get the cached value?
5772016-12-02T17:03:37  <sdaftuar> instagibbs: oy, i'm thinking about it now. ancestor count is easy, but the descenadnt count is not...
5782016-12-02T17:03:54  <instagibbs> how often will that one trigger in wallet context though?
5792016-12-02T17:03:55  <sdaftuar> unfortunately the test we want to do is, what is the max number of descendants that any of pcoin's ancestors have?
5802016-12-02T17:04:11  <sdaftuar> and also, i think we should compare those values to some lower thresholds (say 10 instead of 25)
5812016-12-02T17:04:37  <instagibbs> I'm finding bugs with my implementation with non-standard values
5822016-12-02T17:05:10  <sdaftuar> well i think your implementation would never skip over any pcoins?  except if the chain limits are violated in a reorg
5832016-12-02T17:05:10  <instagibbs> it always only fails at the 25 threshhold, unsure why. If we have an ancestor one already cached, we might just use that
5842016-12-02T17:05:21  <instagibbs> seems less error-prone, and should catch the common case?
5852016-12-02T17:06:16  <instagibbs> im not sure what you mean?
5862016-12-02T17:06:17  <sdaftuar> so to answer your first question, each mempool entry has a value nCountWithAncestors
5872016-12-02T17:06:36  <instagibbs> how do I access that entry
5882016-12-02T17:06:57  <instagibbs> (looking)
5892016-12-02T17:07:39  <sdaftuar> instagibbs: might need to add some kind of accessor, but mempool.mapTx.find(txid) or something should do it.
5902016-12-02T17:08:02  <sdaftuar> since we're already checking to see that it's in the mempool, this seems okay to me
5912016-12-02T17:08:58  <sdaftuar> CalculateMemPoolAncestors() can be a bit expensive, fyi
5922016-12-02T17:09:23  <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
5932016-12-02T17:09:42  <sdaftuar> (which i guess is equivalent to calling CMPA with lower thresholds)
5942016-12-02T17:09:52  *** moli has joined #bitcoin-core-dev
5952016-12-02T17:10:45  <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.
5962016-12-02T17:11:22  <instagibbs> I don't understand. I likely don't understand what CMPA is actually doing. I thought it was already checking for limit violations
5972016-12-02T17:12:16  <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.
5982016-12-02T17:12:20  <sdaftuar> but you know it's already in the mempool
5992016-12-02T17:12:38  <sdaftuar> so almost by definition, those limits won't be violated
6002016-12-02T17:12:55  <sdaftuar> (turns out there is some weird edge case behavior where it could be, but i think that's beside the point)
6012016-12-02T17:14:44  <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.
6022016-12-02T17:18:46  <instagibbs> ok now I'm confused why this works in the general case then.
6032016-12-02T17:18:51  <instagibbs> with default values
6042016-12-02T17:18:59  <sdaftuar> hmm.  do you have a test i can look at?
6052016-12-02T17:19:43  *** laurentmt has joined #bitcoin-core-dev
6062016-12-02T17:19:45  <instagibbs> yes but its very easy to test the base case
6072016-12-02T17:19:54  <instagibbs> let me push my test im working on
6082016-12-02T17:20:02  *** laurentmt has quit IRC
6092016-12-02T17:20:54  <instagibbs> just send all your coins to yourself in a loop 26 times
6102016-12-02T17:21:14  <instagibbs> last time will trigger ATMP failure without these lines, with it triggers "Insufficient funds"
6112016-12-02T17:21:30  <sdaftuar> interesting, i'll take a look
6122016-12-02T17:25:25  <instagibbs> you're right though, this makes no sense in retrospect
6132016-12-02T17:28:24  * sdaftuar just learned about set_trace, whoa!
6142016-12-02T17:28:37  <instagibbs> wait... how do you write all those tests o_0
6152016-12-02T17:28:46  <sdaftuar> slowly :)
6162016-12-02T17:29:21  <instagibbs> i think it spawns zombies if you run in batch mode, be careful :P
6172016-12-02T17:30:20  <instagibbs> that test has "10" as the limit, but it successfully sends 25 times
6182016-12-02T17:30:28  <instagibbs> no matter what it's set to
6192016-12-02T17:30:36  *** BashCo has joined #bitcoin-core-dev
6202016-12-02T17:30:47  <instagibbs> and the error message changes to mempool entry failure if you set a different value...
6212016-12-02T17:31:47  <sdaftuar> ah i think i see what's happening.
6222016-12-02T17:32:02  <sdaftuar> there might be some edge case bugs in CMPA we ought to fix
6232016-12-02T17:33:09  <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.
6242016-12-02T17:33:17  <sdaftuar> ugh, this is kind of messy
6252016-12-02T17:34:05  <gmaxwell> sdaftuar: you're clear on the goal right? don't consider any coin we couldn't actually spend.
6262016-12-02T17:34:31  <sdaftuar> gmaxwell: yes, understood.
6272016-12-02T17:35:16  <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.
6282016-12-02T17:36:10  <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. :)
6292016-12-02T17:36:39  <sdaftuar> gmaxwell: instagibbs: i have two approaches to suggest, not sure which is better:
6302016-12-02T17:37:53  <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)
6312016-12-02T17:38:23  <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).
6322016-12-02T17:38:45  <sdaftuar> and then find a later point in the wallet code to abort the send if the descendant count would be violated
6332016-12-02T17:39:34  <instagibbs> I like (2) for now, just because I can't tell how CMPA is working sometimes :)
6342016-12-02T17:39:38  <gmaxwell> for the case that people hit, really it's just the ancestor limit that actually matters: people chaining change.
6352016-12-02T17:39:43  <instagibbs> yep
6362016-12-02T17:39:53  <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
6372016-12-02T17:40:12  <sdaftuar> 2) is easy, but may result in some rare annoyances
6382016-12-02T17:40:44  <gmaxwell> sdaftuar: this is a sign we should reconsider that relay policy... the size is cornercase enough that it shouldn't really matter.
6392016-12-02T17:41:49  <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
6402016-12-02T17:42:00  <sdaftuar> descendant count/size stuff that limits DoS attacks on the mempool the most
6412016-12-02T17:42:08  <instagibbs> Ok, I'm going to learn more about CMPA, then likely do (2) instead
6422016-12-02T17:42:38  <sdaftuar> "those other ones clear" <--- should be until the parent confirms
6432016-12-02T17:44:52  <gmaxwell> sdaftuar: why do you suggest above going 'much less' than the ancestor limit?
6442016-12-02T17:45:58  <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)
6452016-12-02T17:46:02  *** bitcoin837 has joined #bitcoin-core-dev
6462016-12-02T17:46:17  <sdaftuar> but also practically, being near the ancestor limit makes it more likely some peer will think you're violating, say, the descendant limit
6472016-12-02T17:46:44  <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?
6482016-12-02T17:46:46  <gmaxwell> makes sense. well doing 5 less would probably provide plenty of safty there.
6492016-12-02T17:46:46  <sdaftuar> (breaking relay)
6502016-12-02T17:48:15  <instagibbs> bitcoin837, #bitcoin
6512016-12-02T17:48:27  <bitcoin837> sure
6522016-12-02T17:48:31  <instagibbs> np
6532016-12-02T17:53:42  *** laurentmt has joined #bitcoin-core-dev
6542016-12-02T18:07:41  <sdaftuar> instagibbs: gmaxwell: there's a problem with this approach
6552016-12-02T18:08:07  <sdaftuar> just talked to morcos, and one thing we realized is that you might be combining lots of inputs, each with many ancestors
6562016-12-02T18:08:18  <sdaftuar> so the resulting transaction would fail
6572016-12-02T18:08:18  * BlueMatt sooo doesnt want to have to rebase #9260...anyone wanna review?
6582016-12-02T18:08:20  <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
6592016-12-02T18:09:02  <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
6602016-12-02T18:09:21  *** bitcoin837 has quit IRC
6612016-12-02T18:11:34  <instagibbs> wait, why is this different than what we were going to do?
6622016-12-02T18:11:43  <instagibbs> i thought that was the definition of ancestor number
6632016-12-02T18:11:54  <instagibbs> oh i see, right
6642016-12-02T18:12:21  <instagibbs> I had this thought a couple minutes ago and somehow lost it. Yes, 2 inputs may have n-1 history, making 2n-2
6652016-12-02T18:12:39  <sdaftuar> yep.  or making n-1, if they're different outputs of the same tx
6662016-12-02T18:12:47  <sdaftuar> we have to check later on
6672016-12-02T18:12:59  <instagibbs> Yep. glad someone else thought of it and actually communicated
6682016-12-02T18:13:15  <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.
6692016-12-02T18:14:02  <instagibbs> does the wallet prefer shorter unconfirmed chains vs longer?
6702016-12-02T18:14:09  <instagibbs> I know it prefers confirmed change first, etc
6712016-12-02T18:14:14  <gmaxwell> It is completely blind to it right now.
6722016-12-02T18:14:24  <instagibbs> ok, so it might not be all that rare
6732016-12-02T18:14:42  <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.
6742016-12-02T18:15:21  <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
6752016-12-02T18:15:28  <sdaftuar> we already call SelectCoinsMinConf repeatedly
6762016-12-02T18:15:33  <gmaxwell> thats how we handled unconfirmed, yep.
6772016-12-02T18:15:40  <instagibbs> yeah its the same idea
6782016-12-02T18:16:10  <instagibbs> prefer deep confirmed coins from others, prefer confirmed coins, ok now try unconfirmed <-- I think today
6792016-12-02T18:16:50  <sdaftuar> the other approach would be if there's a way to do it directly in ApproximateBestSubset?
6802016-12-02T18:16:50  <gmaxwell> it does 6,1,0  and it could become 6,1,0+1i,0+10i,0+20i.
6812016-12-02T18:16:53  <sdaftuar> not sure if that's a good idea
6822016-12-02T18:17:26  <instagibbs> ABS assumes you have enough coins i believe
6832016-12-02T18:17:37  <instagibbs> which means if you run into too long chain, that will be different and fail
6842016-12-02T18:18:10  <gmaxwell> not a great idea to do it in ABS. Just making it a parameter to SelectCoinsMinConf would be straightforward.
6852016-12-02T18:19:03  <instagibbs> gmaxwell, why are we using complex numbers for the arg :P
6862016-12-02T18:19:48  <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?
6872016-12-02T18:19:52  <sdaftuar> does that sound right?
6882016-12-02T18:22:56  <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?
6892016-12-02T18:23:15  <instagibbs> SCMF can say "no"
6902016-12-02T18:23:30  <instagibbs> pass failure to SC, returning Insufficent Funds
6912016-12-02T18:23:30  <sdaftuar> right, but what is going to do differnetly when invoked with different limits?
6922016-12-02T18:24:37  <gmaxwell> sdaftuar: you start with the most restrictive limits first.
6932016-12-02T18:24:48  <gmaxwell> and it will only consider coins which will not violate.
6942016-12-02T18:25:29  <instagibbs> if it fails to get enough coins under each limit, raise it, if it can never get enough, fails
6952016-12-02T18:27:03  <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
6962016-12-02T18:27:09  <gmaxwell> nd.
6972016-12-02T18:29:01  <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
6982016-12-02T18:29:22  <instagibbs> ABS wouldnt be in charge of it, would have to be higher
6992016-12-02T18:29:39  <instagibbs> ABS I believe just has a set of inputs, and tries to make "good" fits based on value
7002016-12-02T18:30:04  <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
7012016-12-02T18:30:29  <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?
7022016-12-02T18:30:44  <instagibbs> hm the issue seems to continue all the way until you create the actual transaction
7032016-12-02T18:30:50  <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.
7042016-12-02T18:31:25  <sdaftuar> gmaxwell: did you swap ancestor and descendant in that last line?
7052016-12-02T18:31:49  <gmaxwell> yes.
7062016-12-02T18:32:10  <gmaxwell> Oh you're also saying for ancestors. Indeed, we can only be greedy, you're right.
7072016-12-02T18:32:32  <sdaftuar> if ABS were somehow aware of the constraint, we might get lucky and succeed
7082016-12-02T18:32:37  <sdaftuar> but if it's not...
7092016-12-02T18:33:01  <gmaxwell> it's not a framework that would likely do well with that kind of constraint in any case.
7102016-12-02T18:33:43  <gmaxwell> For some reason I thought the combination rule for ancestor limit was MAX (a depth) not sum.
7112016-12-02T18:34:02  <instagibbs> Yeah me too :/
7122016-12-02T18:34:10  <sdaftuar> sum reflects how much recordkeeping (and pointer hopping) we do
7132016-12-02T18:34:56  <instagibbs> So, we could still have the wallet prefer shorter inputs, and post-transaction finalization, have a CMPA check before returning?
7142016-12-02T18:35:10  <instagibbs> shorter-chain inputs greedily*
7152016-12-02T18:35:37  <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
7162016-12-02T18:35:52  <gmaxwell> sdaftuar: yes, while realizing that I was wrong from your comment it was clear enough to me why it's sum.
7172016-12-02T18:36:32  <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
7182016-12-02T18:37:35  <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
7192016-12-02T18:37:59  <sdaftuar> and certainly the most important thing is to not commit a transaction that then fails in ATMP
7202016-12-02T18:38:26  <sdaftuar> "This must not fail." :)
7212016-12-02T18:40:09  <instagibbs> so we'll still need a CMPA call, or something similar, to get the actual ancestor count before returning right
7222016-12-02T18:40:21  <instagibbs> otherwise "this can fail" :)
7232016-12-02T18:40:30  <sdaftuar> yes.  once we've put the transaction together, we can call CMPA and know whether it'll pass.
7242016-12-02T18:40:51  <sipa> i think we should deal correctly with a failed ATMP call, like removing the tx from the wallet in that case
7252016-12-02T18:41:17  <sipa> (in addition to doing sanity checks ahead of time)
7262016-12-02T18:41:42  <sdaftuar> sipa: agreed
7272016-12-02T18:41:57  <instagibbs> what's the barrier to that fix? I don't know the wallet well enough
7282016-12-02T18:42:07  <sdaftuar> i don't think we ever delete anything now, do we?
7292016-12-02T18:42:30  <instagibbs> removeprunedfunds :P
7302016-12-02T18:42:36  <instagibbs> but no, not normallly
7312016-12-02T18:43:02  <sdaftuar> oh, neat.  i didn't know that existed!
7322016-12-02T18:43:38  <instagibbs> it's meant to be for importing/removing proofs of payment without scanning
7332016-12-02T18:44:45  <morcos> is the reason we commit first in case the node crashes?
7342016-12-02T18:44:52  <sdaftuar> so we can just zap tx's if we create them but ATMP fails?  that seems easy
7352016-12-02T18:45:04  <morcos> we could alwasy have ATMP(fJustCheck)
7362016-12-02T18:45:32  <sdaftuar> morcos: i assumed that's why we commit first
7372016-12-02T18:45:48  *** laurentmt has quit IRC
7382016-12-02T18:46:03  <sdaftuar> morcos: ATMP(fJustCheck) doesn't work very well with RBF and mempool eviction
7392016-12-02T18:46:04  <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?
7402016-12-02T18:46:16  <instagibbs> ^ ok that
7412016-12-02T18:46:24  <morcos> ah, yes...
7422016-12-02T18:46:38  <sdaftuar> maybe RBF isn't a big deal, not sure
7432016-12-02T18:47:27  <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.
7442016-12-02T18:47:32  <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
7452016-12-02T18:47:54  <gmaxwell> right, it's important to never remove something from the wallet that could have relayed.
7462016-12-02T18:48:16  *** moli has quit IRC
7472016-12-02T18:48:19  <morcos> or remembered in the future opportunistic eviction rejection map
7482016-12-02T18:48:31  <sdaftuar> morcos: oh! now i see what you mean.
7492016-12-02T18:49:42  <gmaxwell> ::sigh::
7502016-12-02T18:50:07  <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.
7512016-12-02T18:51:29  <instagibbs> we can still do that, yes?
7522016-12-02T18:51:50  <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.
7532016-12-02T18:51:54  <gmaxwell> instagibbs: sure.
7542016-12-02T18:53:00  <morcos> gmaxwell: yeah i think we get it, we're jsut trying to fix up the edges too
7552016-12-02T18:53:47  *** MykelSIlver has quit IRC
7562016-12-02T18:53:49  <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
7572016-12-02T18:54:33  <instagibbs> is this referring to ATMP failed transactions?
7582016-12-02T18:54:36  <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
7592016-12-02T18:55:08  <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
7602016-12-02T18:55:17  <sdaftuar> instagibbs: i think it's an easy fix, just try to reaccept to the mempool first
7612016-12-02T18:56:30  <morcos> sdaftuar: i dont know if i 100% agree thats a bug, but certainly a separate issue
7622016-12-02T18:56:42  <gmaxwell> We we should fix rebroadcast transactions to try remempolling things. :)
7632016-12-02T18:57:18  <sdaftuar> morcos: right, i guess reaccepting makes it harder for you to abandon a too-low-fee tx?
7642016-12-02T18:57:29  <gmaxwell> thats a day one bug, considering reorgs/doublespends.
7652016-12-02T18:57:29  *** MykelSIlver has joined #bitcoin-core-dev
7662016-12-02T18:57:42  <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
7672016-12-02T18:58:00  <morcos> sdaftuar: no its easy enough to skip abandoned, we already skip reaccepting those on startup
7682016-12-02T18:58:19  <morcos> but yes, it makes the auto-defacto-abandoned state disappear, which is not necessarily a good thing
7692016-12-02T18:58:48  <instagibbs> ok, I need coffee bbl
7702016-12-02T18:58:54  *** moli has joined #bitcoin-core-dev
7712016-12-02T18:59:01  <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.
7722016-12-02T19:02:33  <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
7732016-12-02T19:03:45  *** ThomasV has joined #bitcoin-core-dev
7742016-12-02T19:07:15  *** MykelSIlver has quit IRC
7752016-12-02T19:07:33  *** MykelSIlver has joined #bitcoin-core-dev
7762016-12-02T19:08:08  *** Chris_Stewart_5 has quit IRC
7772016-12-02T19:08:33  *** Chris_Stewart_5 has joined #bitcoin-core-dev
7782016-12-02T19:10:06  <gmaxwell> morcos: the non-mempool part of resend wallet txn wouldn't be needed if we had mempool sync. :P
7792016-12-02T19:10:39  <morcos> if we get it right, we can just skip blocks and PoW too.  :P :P
7802016-12-02T19:11:43  *** moli has quit IRC
7812016-12-02T19:17:56  *** jtimon has joined #bitcoin-core-dev
7822016-12-02T19:17:58  *** laurentmt has joined #bitcoin-core-dev
7832016-12-02T19:18:40  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/d7ba4a233bd5...9e4bb312e695
7842016-12-02T19:18:40  <bitcoin-git> bitcoin/master facbfa5 MarcoFalke: [qa] Get rid of duplicate code
7852016-12-02T19:18:41  <bitcoin-git> bitcoin/master 9e4bb31 MarcoFalke: Merge #9221: [qa] Get rid of duplicate code...
7862016-12-02T19:18:53  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9221: [qa] Get rid of duplicate code (master...Mf1611-qaMaxUploadDuplCode) https://github.com/bitcoin/bitcoin/pull/9221
7872016-12-02T19:18:59  *** moli has joined #bitcoin-core-dev
7882016-12-02T19:19:31  *** LeMiner has quit IRC
7892016-12-02T19:19:31  *** LeMiner has joined #bitcoin-core-dev
7902016-12-02T19:22:49  *** laurentmt has quit IRC
7912016-12-02T19:34:02  *** pindarhk has quit IRC
7922016-12-02T19:34:02  *** aspect_ has quit IRC
7932016-12-02T19:34:02  *** btcdrak has quit IRC
7942016-12-02T19:34:02  *** mappum has quit IRC
7952016-12-02T19:34:02  *** eragmus has quit IRC
7962016-12-02T19:34:54  <bitcoin-git> [bitcoin] MarcoFalke pushed 2 new commits to master: https://github.com/bitcoin/bitcoin/compare/9e4bb312e695...c36229b0b2e9
7972016-12-02T19:34:54  <bitcoin-git> bitcoin/master 8a70a9d wodry: Improvement of documentation of command line parameter 'whitelist'
7982016-12-02T19:34:55  <bitcoin-git> bitcoin/master c36229b MarcoFalke: Merge #9251: Improvement of documentation of command line parameter 'whitelist'...
7992016-12-02T19:35:04  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9251: Improvement of documentation of command line parameter 'whitelist' (master...patch-3) https://github.com/bitcoin/bitcoin/pull/9251
8002016-12-02T19:54:58  *** mappum has joined #bitcoin-core-dev
8012016-12-02T19:59:36  <bitcoin-git> [bitcoin] MarcoFalke closed pull request #9064: unify capitalization of "bitcoin address" (master...changeCaps) https://github.com/bitcoin/bitcoin/pull/9064
8022016-12-02T20:08:53  *** pindarhk has joined #bitcoin-core-dev
8032016-12-02T20:10:58  *** mrkent has joined #bitcoin-core-dev
8042016-12-02T20:11:40  *** eragmus has joined #bitcoin-core-dev
8052016-12-02T20:15:09  *** aspect_ has joined #bitcoin-core-dev
8062016-12-02T20:19:18  *** btcdrak has joined #bitcoin-core-dev
8072016-12-02T20:22:17  *** moli has quit IRC
8082016-12-02T20:25:35  *** Guyver2 has quit IRC
8092016-12-02T20:28:33  <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?
8102016-12-02T20:28:35  <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
8112016-12-02T20:28:52  <sdaftuar> second -- i tried to use git blame -C on the net_processing.cpp file, to verify that you didn't introduce any changes
8122016-12-02T20:29:16  <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
8132016-12-02T20:29:52  <sdaftuar> i can't see any change in that code, but i'm puzzled as to why git thinks a change was introduced
8142016-12-02T20:30:14  <sipa> sdaftuar: git diff --patience COMMIT~:oldfile COMMIT:newfile
8152016-12-02T20:31:25  *** CubicEarth has joined #bitcoin-core-dev
8162016-12-02T20:31:51  *** moli has joined #bitcoin-core-dev
8172016-12-02T20:33:53  <sdaftuar> sipa: thanks, i'll try that
8182016-12-02T20:35:49  *** afk11 has quit IRC
8192016-12-02T20:37:39  *** moli has quit IRC
8202016-12-02T20:40:20  <morcos> sdaftuar: its not ENTIRELY move only
8212016-12-02T20:40:48  <sdaftuar> yeah i know... just trying to figure out what the real diff is
8222016-12-02T20:41:06  <sipa> the only diff i saw was some header changes, and the snippet i pasted in a comment
8232016-12-02T20:41:48  *** afk11 has joined #bitcoin-core-dev
8242016-12-02T20:41:48  *** afk11 has quit IRC
8252016-12-02T20:41:48  *** afk11 has joined #bitcoin-core-dev
8262016-12-02T20:43:24  <sdaftuar> sipa: i think there's a one-line change about feeFilterRounder too?
8272016-12-02T20:44:03  <sipa> sdaftuar: ah, he squashed in some changes
8282016-12-02T20:44:10  <sipa> i reviewed an earlier version
8292016-12-02T20:44:12  <sdaftuar> ah ok
8302016-12-02T20:44:20  <sipa>  84922e4
8312016-12-02T20:46:43  *** Atomicat has quit IRC
8322016-12-02T20:48:18  <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..
8332016-12-02T20:48:45  <morcos> investigating now, but i bet it hit that bug... whew.   dodged a bullet
8342016-12-02T20:48:48  <sdaftuar> nice.  lets not PR that one. :)
8352016-12-02T20:51:26  *** Atomicat has joined #bitcoin-core-dev
8362016-12-02T20:55:30  *** owowo has quit IRC
8372016-12-02T20:55:48  *** ThomasV has quit IRC
8382016-12-02T20:56:12  *** moli has joined #bitcoin-core-dev
8392016-12-02T21:00:10  *** aalex_ has joined #bitcoin-core-dev
8402016-12-02T21:03:14  <instagibbs> sdaftuar, ok updated my PR, thanks for the help
8412016-12-02T21:03:23  *** aalex has quit IRC
8422016-12-02T21:05:29  <jonasschnelli> Isn't this a bug: https://github.com/bitcoin/bitcoin/blob/master/qa/rpc-tests/keypool.py#L40
8432016-12-02T21:05:36  <sdaftuar> instagibbs: cool, i'll take a look
8442016-12-02T21:05:41  <jonasschnelli> We fill the keypool with a new target size of 3
8452016-12-02T21:05:52  <jonasschnelli> then we manage to reserve 4 keys: https://github.com/bitcoin/bitcoin/blob/master/qa/rpc-tests/keypool.py#L45
8462016-12-02T21:08:08  *** k0ng has quit IRC
8472016-12-02T21:11:20  *** owowo has joined #bitcoin-core-dev
8482016-12-02T21:13:09  <cfields> jonasschnelli: seems to always reserve target + 1
8492016-12-02T21:14:01  <cfields> ./bitcoin-cli -testnet keypoolrefill 104
8502016-12-02T21:14:05  <cfields> 2016-12-02 21:13:24 keypool added key 108, size=105
8512016-12-02T21:19:19  <sipa> i think that may be the vchDefaultKey...
8522016-12-02T21:25:04  *** MykelSIlver has quit IRC
8532016-12-02T21:52:55  *** paveljanik has quit IRC
8542016-12-02T21:53:24  *** paveljanik has joined #bitcoin-core-dev
8552016-12-02T21:59:17  *** moli has quit IRC
8562016-12-02T22:02:30  *** Chris_Stewart_5 has quit IRC
8572016-12-02T22:02:36  *** moli has joined #bitcoin-core-dev
8582016-12-02T22:02:58  <MarcoFalke> no, don't think this is vchDefaultKey
8592016-12-02T22:03:54  <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)
8602016-12-02T22:04:21  <MarcoFalke> Now if you call it from another place which does not grab the key, you end up with off-by-one
8612016-12-02T22:08:31  *** cheese_ has joined #bitcoin-core-dev
8622016-12-02T22:08:31  *** cheese_ has joined #bitcoin-core-dev
8632016-12-02T22:12:02  *** RoyceX has quit IRC
8642016-12-02T22:18:45  *** Chris_Stewart_5 has joined #bitcoin-core-dev
8652016-12-02T22:34:35  <instagibbs> sdaftuar: hmm the in mempool check done in AvailableCoins doesn't cover that case?
8662016-12-02T22:34:58  <instagibbs> Oh I see confirmation plus not mempool
8672016-12-02T22:38:19  *** moli has quit IRC
8682016-12-02T22:38:32  *** aalex__ has joined #bitcoin-core-dev
8692016-12-02T22:42:13  *** aalex_ has quit IRC
8702016-12-02T22:48:21  *** MarcoFalke has left #bitcoin-core-dev
8712016-12-02T22:50:41  *** moli has joined #bitcoin-core-dev
8722016-12-02T22:55:53  *** owowo has quit IRC
8732016-12-02T23:00:31  *** owowo has joined #bitcoin-core-dev
8742016-12-02T23:05:32  *** kadoban has joined #bitcoin-core-dev
8752016-12-02T23:11:29  *** JackH has quit IRC
8762016-12-02T23:20:47  <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
8772016-12-02T23:21:17  <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
8782016-12-02T23:21:32  <sipa> but validation + net_processing together now take 2.14G!
8792016-12-02T23:21:44  <sipa> bad job!
8802016-12-02T23:21:56  <BlueMatt> lol
8812016-12-02T23:22:41  <luke-jr> need to do -flto with under 1 GB RAM use ‼‼‼‼! :p
8822016-12-02T23:22:54  <BlueMatt> luke-jr: I cant get lto to build anymore
8832016-12-02T23:25:02  *** bsm117532 has quit IRC
8842016-12-02T23:26:25  <gmaxwell> BlueMatt: build time is greater for me too.
8852016-12-02T23:26:45  <BlueMatt> greater like slower or faster?
8862016-12-02T23:26:49  <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
8872016-12-02T23:27:08  <gmaxwell> BlueMatt: slower. takes more time.
8882016-12-02T23:27:30  <BlueMatt> gmaxwell: yea, not surprising given how much shit we have in headers (and how much boost has in headers!)
8892016-12-02T23:27:34  <gmaxwell> bad except when napping is required. presumably its faster on things other than my laptop.
8902016-12-02T23:27:40  <gmaxwell> yea, need to get rid of boost.
8912016-12-02T23:27:47  <sipa> gmaxwell: are you using parallel lto linking?
8922016-12-02T23:28:04  <gmaxwell> sipa: no just the seperate files require more time in total.
8932016-12-02T23:28:14  <gmaxwell> due to all the garbage in headers.
8942016-12-02T23:28:17  <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
8952016-12-02T23:28:32  <gmaxwell> I'm not complaining, its the right thing to do.
8962016-12-02T23:29:04  <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
8972016-12-02T23:29:09  <gmaxwell> :P
8982016-12-02T23:29:16  <BlueMatt> i know, but I am complaining about headers in bitcoin core (and boost)
8992016-12-02T23:29:20  <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
9002016-12-02T23:29:39  <BlueMatt> sipa: I dont think we're talking about lto
9012016-12-02T23:30:03  <sipa> ah.
9022016-12-02T23:36:23  *** bsm117532 has joined #bitcoin-core-dev
9032016-12-02T23:37:08  <BlueMatt> sdaftuar: ok with not fixing your comments in #9260?
9042016-12-02T23:37:09  <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
9052016-12-02T23:37:12  <gmaxwell> I'm not talking about LTO.
9062016-12-02T23:39:09  *** bsm117532 has quit IRC
9072016-12-02T23:41:38  <BlueMatt> paveljanik: you got a minute to ack 9260, since you already looked over it?
9082016-12-02T23:41:51  * BlueMatt is still hoping it can be merged prior to any other main.cpp changes :p
9092016-12-02T23:50:27  *** justanotheruser has quit IRC
9102016-12-02T23:50:34  *** justan0theruser has joined #bitcoin-core-dev
9112016-12-02T23:58:01  *** abpa has quit IRC