< BlueMatt> ryanofsky: yo
< BlueMatt> whats the status of listunspent in #8456
< gribble> https://github.com/bitcoin/bitcoin/issues/8456 | [RPC] Simplified bumpfee command. by mrbandrews · Pull Request #8456 · bitcoin/bitcoin · GitHub
< BlueMatt> morcos: as pointed out on github, there are some uses for getbalance "*" that I dont think we can break yet (https://github.com/bitcoin/bitcoin/issues/8183#issuecomment-272598823) so I think this needs fixing in some way for bumpfee
< morcos> BlueMatt: I still haven't investigated thoroughly myself yet, but you left a comment on 8456 about getbalance ""?
< morcos> Forget about getbalance "", that's done, its been broken since 0.11, got worse in 0.12
< morcos> Regardless of bumpfee, it uses GetAccountBalance
< morcos> and that is unsupported
< morcos> getbalance "*" is a different question, b/c i agree we need a way to return watch only balances.. so i think something should be done..
< morcos> so to me it's a question of how much bumpfee messes up getbalance "*"
< morcos> BlueMatt: Wait a second... How does bumpfee change the output of getbalance "*" anyway?
< morcos> Are you sure its not just broken for any time you have double spends in your wallet (until one of them is confirmed)
< jonasschnelli> Ping BlueMatt luke-jr: https://github.com/bitcoin/bitcoin/pull/9294
< cannon-c> Quick question, I am looking to using bitcoin core in conjunction with ssss (Shamirs secret sharing scheme)
< cannon-c> to create fragmented seed backups which can be re-created using pre-defined threshold of number of shares
< cannon-c> Does Core already have something like this?
< cannon-c> I dont want to duplicate something that already exists
< cannon-c> One use case, creating the xpriv on airgap system, with fragmented backup as contingency of access
< cannon-c> to family if something happens to me
< cannon-c> or great for distributed backups
< btcdrak> jonasschnelli: testing
< jonasschnelli> btcdrak: thanks!
< morcos> wumpus: I think #9380 is *almost* ready for merge, ideally it would be merged in time to keep the translation strings for -blockmintxfee. I think the only open question is if people prefer a different name for -dustrelayfee.
< gribble> https://github.com/bitcoin/bitcoin/issues/9380 | Separate different uses of minimum fees by morcos · Pull Request #9380 · bitcoin/bitcoin · GitHub
< instagibbs> cannon-c, try #bitcoin
< jtimon> morcos: re #9380 I would leave -dustrelayfee, or change to -dustfee maybe, no strong opinion on my part
< gribble> https://github.com/bitcoin/bitcoin/issues/9380 | Separate different uses of minimum fees by morcos · Pull Request #9380 · bitcoin/bitcoin · GitHub
< bitcoin-git> [bitcoin] laanwj pushed 4 new commits to master: https://github.com/bitcoin/bitcoin/compare/8a445c5651ed...dd98f0453824
< bitcoin-git> bitcoin/master daec955 Alex Morcos: Introduce -blockmintxfee
< bitcoin-git> bitcoin/master 7b1add3 Alex Morcos: Introduce -incrementalrelayfee
< bitcoin-git> bitcoin/master eb30d1a Alex Morcos: Introduce -dustrelayfee
< bitcoin-git> [bitcoin] laanwj closed pull request #9380: Separate different uses of minimum fees (master...minfees) https://github.com/bitcoin/bitcoin/pull/9380
< BlueMatt> morcos: the difference is that bumpfee takes us to a world where having "double spends" in your wallet is entirely supported
< morcos> It's already supported once we merged BIP 152
< morcos> uh
< morcos> 125
< morcos> whatever
< BlueMatt> instead of, previously, you'd have to do something unsupported like run your wallet on multiple machines to get it
< BlueMatt> hmm?
< morcos> actually it was supported even before that
< BlueMatt> only abandon transaction supported it?
< BlueMatt> well, you generating them, that is
< BlueMatt> you receiving them maybe
< morcos> yeah exactly
< BlueMatt> but now we are actively supporting you generating them
< BlueMatt> which is a huge difference
< BlueMatt> (is getbalance "*" broken for receiving double spends, too?)
< morcos> and previously you could generate them yourself also...
< BlueMatt> manually, sure
< BlueMatt> but "unsupported, might break getbalance" is perfectly reasonable in that case
< morcos> the only thing stopping you from doing it automatically would be if your mempool had your original spend
< morcos> so the coins didn't show up
< morcos> so if it came out of your mempool for any reason... expired after 3 days for instance
< BlueMatt> hmm? we re-add your txn to mempool?
< morcos> nothing stopping you from double spending... with 0.12, we made it so you had to abandon it first but that wasn't a requirement before that
< BlueMatt> so only abandon would stop it?
< morcos> i'm saying if you automatically created tx A, then A was evicted from your mempool, never made it in, or was expired
< BlueMatt> i mean sure, if you manually createrawtransaction/signrawtransaction you could doublespend easily
< BlueMatt> if it was expired it would be re-added
< morcos> nothing would prevent you from doing an sendtoaddress and creating A' doublespend
< morcos> no it wouldn't
< morcos> we just added that
< BlueMatt> oh?
< BlueMatt> wait, what?
< BlueMatt> we've always re-announced transactions on a regular basis?
< morcos> reannounce txs already in your mempool
< BlueMatt> I mean to be clear, I'm fine with some documentation noting that getbalance "*" is broken, and should not be used, and is made worse with bumpfee
< morcos> i'm inflating my argument slightly, since before mempool limiting, things didn't really fall out of your mempool very much
< BlueMatt> really?
< BlueMatt> yea, ok, i suppose thats why
< morcos> See #9290
< gribble> https://github.com/bitcoin/bitcoin/issues/9290 | Make RelayWalletTransaction attempt to AcceptToMemoryPool. by gmaxwell · Pull Request #9290 · bitcoin/bitcoin · GitHub
< BlueMatt> ahh
< morcos> Anyway, i guess my point is its not fair to accept a bumpfee PR to take on fixing years of broken code just b/c it might lead to the bugs being exposed more...
< morcos> the bugs are exposed more by allowing double spends
< morcos> we've already crossed that bridge
< morcos> its stupid to say we won't make it easier for you b/c our code is broken in some ways reporting on them
< BlueMatt> well i believe the bugs are only currently exposed if you're doing something entirely unsupported (or, in the case fixed by #9290 by bugs which we need to/did fix)
< gribble> https://github.com/bitcoin/bitcoin/issues/9290 | Make RelayWalletTransaction attempt to AcceptToMemoryPool. by gmaxwell · Pull Request #9290 · bitcoin/bitcoin · GitHub
< morcos> i don't really feel strongly about merging bumpfee for 0.14.. thats not really what i'm arguing about.. and i think your last minute concerns are a good reflection that it seems a bit rushed trying to get it in
< BlueMatt> considering its deprecated I dont see why we cant just put a note in the docs and say "this is broken"
< BlueMatt> (bumpfee "*", that is)
< morcos> i just don't think that when we decide it is ready to merge we should be weighing it down with the responsiblity of fixign eveyrthing else
< morcos> why don't you think bugs are exposed if you receive a double spend?
< BlueMatt> agreed, totally get that, but in this case I do think we are exposing bugs which were only previously exposed if the user was doing something insane
< morcos> a BIP 125 compatible one
< BlueMatt> though, really, if bumpfee slips 0.14, we should just remove accounts for 0.14
< morcos> thats not insance at all
< BlueMatt> then bumpfee isnt exposing shit :p
< BlueMatt> are the bugs exposed if you receive a bumpfee from someone else? I havent thought that far into it
< morcos> ah
< morcos> perhaps not...
< BlueMatt> no, asking, i have no idea
< BlueMatt> not saying i dont think they are, just havent looked
< morcos> well the issue is 0-confirm things reduce your balance, regardless of whether they are in the mempool or not
< morcos> if its a tx from someoen else, it won't add to yoru balance if its 0 confirm period...
< morcos> but i bet getbalance "*" 0 is broken if you receive a double spend
< BlueMatt> could see that, but getbalance "*" 1 can go negative with bumpfee, I believe
< morcos> anyway... i just don't think bumpfee CHANGES anything about this...
< BlueMatt> or maybe not?
< morcos> lets just agree it shouldn't be fixed on the bumpfee PR... i'll argue less (or none) about saying we shouldn't do a release with bumpfee before its fixed
< BlueMatt> if I'm right (and might not be), during normal use of the wallet, getbalance "*" 1 is (probaboy, I think?) correct, and is currently the only way to get your balance if you include watchonly
< BlueMatt> with bumpfee, I believe, this changes
< BlueMatt> so it is no longer correct
< BlueMatt> unrelatedly, did we document the listunspent changes for bumpfee?
< BlueMatt> I do think that needs documentation, if no code changes
< morcos> yeah.. i actually think we should probably just return the txs in listunspent
< BlueMatt> both of them?
< BlueMatt> that seems shit
< morcos> its trivial to do b/c listunspent already passes a fOnlyConfirmed bool set to fallse to AvailableCoins that no other uses of it do
< BlueMatt> (does it currently return abandoned tx or tx not in mempool?)
< morcos> i think so
< morcos> i don't know let me check
< BlueMatt> no, it just calls AvailableCoins
< BlueMatt> so only if its "trusted" and in mempool
< morcos> no it doesn't check IsTrusted
< morcos> but you are right it checks the mempool
< BlueMatt> wait, no, wtf is fOnlyConfirmed
< BlueMatt> oh, yes, you're right
< morcos> which maybe is exactly what we want?
< BlueMatt> I think if we add an fOnlyConfirmed to both of the replaces/replaced checks that would be least-surprising to users
< morcos> agreed
< BlueMatt> but, frankly, I'm open to anything that is documented
< BlueMatt> would you like me to go finish review assuming that gets fixed? if we feel it could still be merged for 0.14 I'm happy to
< BlueMatt> not sure how wumpus or sipa feel about it now
< morcos> ehh... if it was me, i'd put it in 0.14.. but i'm not going to advocate for it... maybe gmaxwell feels strongly?
< morcos> i think #9294 is maybe higher priority
< gribble> https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub
< BlueMatt> I mean the way i have left to review this is to literally go read all of wallet and rpcwallet
< BlueMatt> is that still waiting on fixes?
< BlueMatt> oh, no, ok, will review that one at least
< morcos> i don't know.. i just started reading BIP 32.. ha
< sdaftuar> if we're concerned about bumpfee for 0.14, i think we could merge it and mark it as experimental? i'm also ok with merging after the 0.14 split and use the 0.15 release cycle as time to get the bugs out while it simmers in master
< BlueMatt> jonasschnelli: you around?
< sdaftuar> i'd be concerned about holding it up further though for unreleated walelt bugs that it exposes
< BlueMatt> sdaftuar: well the only thing I found so far that really is a problem, I think, is listunspent
< BlueMatt> the getbalance "*" thing I really hate, but am not sure is a reasonable fix
< * BlueMatt> is still hoping #9561 and #9535 get another last-minute review or two and get merged
< gribble> https://github.com/bitcoin/bitcoin/issues/9561 | Wake message handling thread when we receive a new block by TheBlueMatt · Pull Request #9561 · bitcoin/bitcoin · GitHub
< gribble> https://github.com/bitcoin/bitcoin/issues/9535 | Split CNode::cs_vSend: message processing and message sending by TheBlueMatt · Pull Request #9535 · bitcoin/bitcoin · GitHub
< BlueMatt> but it seems like no one is around today :(
< BlueMatt> damn holidays
< BlueMatt> always getting in the way
< sdaftuar> yeah i think listunspent could be a fix-by-documentation, at least for now. makes more sense if we indicate bumpfee is an experimental feature?
< BlueMatt> im fine with that
< BlueMatt> though I think the above-discussed fix might also be sufficient
< BlueMatt> would just need a quick pass to check all the other places fOnlyConfirmed is set in callers
< morcos> BlueMatt: i already did that, only set by listunspent
< BlueMatt> cool
< morcos> what about #9499, that is a pretty big win if you ask me... especially without multiple block downloads
< gribble> https://github.com/bitcoin/bitcoin/issues/9499 | Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction by TheBlueMatt · Pull Request #9499 · bitcoin/bitcoin · GitHub
< BlueMatt> yes, and probably has enough review to get a merge, I'd say
< sipa> BlueMatt: i'll do another pass today
< sipa> now meeting ethan
< BlueMatt> thanks
< bitcoin-git> [bitcoin] laanwj pushed 1 new commit to master: https://github.com/bitcoin/bitcoin/commit/b0819c7e9b428631b806d97ff19beb2e218df31f
< bitcoin-git> bitcoin/master b0819c7 Wladimir J. van der Laan: qt: periodic translations update
< BlueMatt> cool, tell him i said hi
< sipa> he says hello
< * instagibbs> attempting 9561 review
< instagibbs> 9499 is def ready
< sipa> #9499
< gribble> https://github.com/bitcoin/bitcoin/issues/9499 | Use recent-rejects, orphans, and recently-replaced txn for compact-block-reconstruction by TheBlueMatt · Pull Request #9499 · bitcoin/bitcoin · GitHub
< sipa> #9561
< gribble> https://github.com/bitcoin/bitcoin/issues/9561 | Wake message handling thread when we receive a new block by TheBlueMatt · Pull Request #9561 · bitcoin/bitcoin · GitHub
< sipa> agree
< instagibbs> oops, I meant 9535
< instagibbs> for my attempted review...
< sipa> #9535
< gribble> https://github.com/bitcoin/bitcoin/issues/9535 | Split CNode::cs_vSend: message processing and message sending by TheBlueMatt · Pull Request #9535 · bitcoin/bitcoin · GitHub
< cfields> agree on all of those
< BlueMatt> #9484 obv
< gribble> https://github.com/bitcoin/bitcoin/issues/9484 | Introduce assumevalid setting to skip validation presumed valid scripts. by gmaxwell · Pull Request #9484 · bitcoin/bitcoin · GitHub
< cfields> ah right, i need to sync up and ack the hash
< cfields> (i'm sure it's fine :)
< sipa> no, you should not be sure it is fine
< sipa> (i, however, am)
< BlueMatt> I did validate that hash prior to ack
< BlueMatt> if we're willing to push a day, I think both #9294 and #8456 could make it, but not sure they're gonna make it today
< gribble> https://github.com/bitcoin/bitcoin/issues/9294 | Use internal HD chain for change outputs (hd split) by jonasschnelli · Pull Request #9294 · bitcoin/bitcoin · GitHub
< BlueMatt> well, maybe two days
< gribble> https://github.com/bitcoin/bitcoin/issues/8456 | [RPC] Simplified bumpfee command. by mrbandrews · Pull Request #8456 · bitcoin/bitcoin · GitHub
< BlueMatt> but i do think they're both super close
< bitcoin-git> [bitcoin] sipa pushed 3 new commits to master: https://github.com/bitcoin/bitcoin/compare/b0819c7e9b42...812714fd80e9
< bitcoin-git> bitcoin/master e440ac7 Gregory Maxwell: Introduce assumevalid setting to skip presumed valid scripts....
< bitcoin-git> bitcoin/master 7b5e3fe John Newbery: Add assumevalid testcase...
< bitcoin-git> bitcoin/master 812714f Pieter Wuille: Merge #9484: Introduce assumevalid setting to skip validation presumed valid scripts....
< bitcoin-git> [bitcoin] sipa closed pull request #9484: Introduce assumevalid setting to skip validation presumed valid scripts. (master...script_elide_verified) https://github.com/bitcoin/bitcoin/pull/9484
< bitcoin-git> [bitcoin] theuni opened pull request #9566: threading: use std::chrono for timestamps (master...nuke-boost-chrono2) https://github.com/bitcoin/bitcoin/pull/9566