19:08:58 <sipa> #startmeeting
19:08:58 <lightningbot> Meeting started Fri Jan 25 19:08:58 2019 UTC.  The chair is sipa. Information about MeetBot at http://wiki.debian.org/MeetBot.
19:08:58 <lightningbot> Useful Commands: #action #agreed #help #info #idea #link #topic.
19:09:03 <meshcollider> Hi yeah my internet is very spotty so I probably won't be here for most of this sorry :)
19:09:20 <sipa> okay
19:09:53 <provoostenator> https://github.com/bitcoin/bitcoin/milestone/35
19:09:58 <sipa> #topic wallet goals for 0.18
19:10:26 <gmaxwell> This is relevant
19:10:27 <gmaxwell> 00:45:35 < kallewoof> I can't make the meetings as always, but would like to add #13756 to priority for review list, unless people object. (ping
19:10:27 <gmaxwell> wumpus fanquake ...)
19:10:27 <gmaxwell> 00:45:38 < gribble> https://github.com/bitcoin/bitcoin/issues/13756 | wallet: "avoid_reuse" wallet flag for improved privacy by kallewoof · Pull
19:10:27 <gmaxwell> Request #13756 · bitcoin/bitcoin · GitHub
19:10:29 <gribble> https://github.com/bitcoin/bitcoin/issues/13756 | wallet: "avoid_reuse" wallet flag for improved privacy by kallewoof · Pull Request #13756 · bitcoin/bitcoin · GitHub
19:10:32 <gribble> https://github.com/bitcoin/bitcoin/issues/13756 | wallet: "avoid_reuse" wallet flag for improved privacy by kallewoof · Pull Request #13756 · bitcoin/bitcoin · GitHub
19:10:35 <provoostenator> +1
19:10:40 <sipa> gmaxwell: thanks
19:11:38 <sipa> gwillen: will you have time in the near future to rebase/address comments in #14978 ?
19:11:38 <provoostenator> I'd also love to get a number of pull requests into this release that would allow using achow101's HWI library:
19:11:42 <gribble> https://github.com/bitcoin/bitcoin/issues/14978 | Factor out PSBT utilities from RPCs for use in GUI code; related refactoring. by gwillen · Pull Request #14978 · bitcoin/bitcoin · GitHub
19:12:12 <provoostenator> Though that may be a bit ambitious.
19:12:18 <achow101> how likely is it for #14491, #14075, and #14021 to get into 0.18?
19:12:22 <gribble> https://github.com/bitcoin/bitcoin/issues/14491 | Allow descriptor imports with importmulti by MeshCollider · Pull Request #14491 · bitcoin/bitcoin · GitHub
19:12:25 <gribble> https://github.com/bitcoin/bitcoin/issues/14075 | Import watch only pubkeys to the keypool if private keys are disabled by achow101 · Pull Request #14075 · bitcoin/bitcoin · GitHub
19:12:27 <gribble> https://github.com/bitcoin/bitcoin/issues/14021 | Import key origin data through descriptors in importmulti by achow101 · Pull Request #14021 · bitcoin/bitcoin · GitHub
19:12:38 <achow101> (those are required for HWI)
19:13:46 <provoostenator> I think descriptor imports is pretty close
19:14:07 <gwillen> sipa: yes, I will definitely try to go another round on 14978 ASAP
19:14:37 <sipa> i'm hoping to open a PR for signing with a descriptor soon, but ideally on top of your PR
19:14:44 <provoostenator> The key origin PR is tiny (on top of desciptor), with lots of tests
19:15:02 <sipa> provoostenator: cool, so that should be easy once descriptor import is in
19:15:15 <provoostenator> The keypool import stuff still has me a bit in doubt what the right approach is.
19:15:21 <achow101> gwillen: I rebased and updated #13932 last night
19:15:24 <gribble> https://github.com/bitcoin/bitcoin/issues/13932 | Additional utility RPCs for PSBT by achow101 · Pull Request #13932 · bitcoin/bitcoin · GitHub
19:16:01 <provoostenator> I commented "I would prefer if TopUpKeyPool can deal with imported keys.", though I'm not sure how realistic that is without a complete wallet descriptor support redo.
19:16:03 <gmaxwell> Is there anything we can do to head off funds loss from typos/copypaste errors with decriptor importing?  The use of descriptors as user/api handled key material seems like a step backward for Bitcoin,  as we're now introducing ways for simple typos (or clbuttic errors) to cause funds to go off to space.
19:16:45 <provoostenator> gmaxwell: we could make importing descriptors without xpub fail, unless opted in?
19:16:53 <gwillen> achow101: sweet, thanks! I will go look.
19:16:59 <provoostenator> (xpub or other checksummed approach)
19:17:07 <gwillen> sipa: ok, I will prioritize, thanks
19:17:08 <sipa> gmaxwell: the xpubs inside descriptors have a checksum, though the paths/functions don't
19:17:21 <gmaxwell> provoostenator: if the part of the descriptor outside of the xpub is corrupted funds are still lost.
19:17:45 <gmaxwell> That may be a little less likely, but its still exposed.
19:17:52 <provoostenator> That seems somewhat unlikely though for the most basic descriptors.
19:18:19 <provoostenator> Because the parser will fail, unless the error happens to produce another valid descriptor.
19:18:55 <gmaxwell> What does wsh(multi(3,xpub661MyMwAqRbcFW31YEwpkMuc5THy2PSt5bDMsktWQcFF8syAmRUapSCGu8ED9W6oDMSgv6Zz8idoc4a6mr8BDzTJY47LJhkJ8UB7WEGuduB/1/0/*,xpub69H7F5d8KSRgmmdJg2KhpAK8SR3DjMwAdkxj3ZuxV27CprR9LgpeyGmXUbC6wb7ERfvrnKZjXoUmmDznezpbZb7ap6r1D3tgFxHmwMkQTPH/0/0/*)) do?
19:19:28 <gmaxwell> (3 of 2 multisig)
19:19:32 <provoostenator> We could use the Electrum style xpub, ypub, zpub, etc-pub to indicate what type we expect the descriptor to be. But there's no software that could produce the data.
19:20:08 <provoostenator> (a generic problem if we had any kind of checksum mechanism to the descriptor language)
19:20:21 <gmaxwell> in any case, filtering to xpubs unless overridden seems like a good idea, at least.
19:20:44 <gwillen> given that the xpubs themselves already have a checksum, you could add a checksum of _just_ everything else to the end, it could be very short and still be sufficient
19:20:46 <sipa> also, anything with a private key has a checksum
19:20:50 <gmaxwell> but it still seems far too dangerous to ever have users using directly to import keys.
19:20:59 <provoostenator> 3 of 2 multisig should hopefully fail as an invalid descriptor?  Could be worth adding a test for that...
19:21:52 <provoostenator> gwillen: the problem is, what software will make the checksum? We'd have to define a standard for that too. Maybe that's a good thing.
19:22:11 <gmaxwell> one could adjust the descriptor language to support a ",checksum" at the end. Even if it just blindly supported them with the checkvalue truncated to facilitate editing, it would provide some protection against other corruption. I dunno.
19:22:28 <gwillen> provoostenator: yeah, we would have to go "oops, lack of checksum was an omission in the spec, new spec"
19:22:43 <gmaxwell> I don't really understand how it got to this point. When descriptors were first defined, I didn't expect them to be used for key management in liue of importing addresses.
19:22:44 <gwillen> which makes sense if they are going to be an interchange format, which it seems they are otherwise well-suited for, except this issue
19:22:44 <sipa> gwillen: that's the great thing about not having a spec
19:22:53 <achow101> provoostenator: it's not like descriptors are really standardized yet though...
19:22:54 <gwillen> sipa: :D
19:23:22 <gmaxwell> I do worry about breaking editability. Though there could just be a simple utility(function) to recompute the checksum on any descriptor.
19:23:32 <gmaxwell> That you'd just use after editing, if you intended to edit.
19:23:38 <provoostenator> gmaxwell: the optional ,checksum makes sense. And then importmulti could refuse descriptors without such checksum, unless "I-know-what-I'm-doing" is added to the command.
19:23:49 <sipa> provoostenator, gmaxwell: that seems reasonable
19:24:13 <sipa> i don't mind breaking compatibility at this point
19:24:19 <provoostenator> Yeah, we would need some utility to calculate the checksum for those making descriptors manually.
19:24:34 <gmaxwell> Seems straightforward.
19:25:15 <sipa> gmaxwell: i think i imagined initially that indeed there would be syntactic sugar encodings for common subsets of descriptors (like zpub etc) with checksums etc
19:25:59 <sipa> but it seems people (myself included) are pretty excited about having the descriptor itself be a visible thing... so adding a checksum to the descriptor itself makes sense
19:26:18 <gmaxwell> Better to fix it now, if not later.   I wonder if we need the 'accept import override' if there is a utility function that just adds the checksum?
19:26:45 <gmaxwell> Yea, I think they're cool, I'm excited about having them available too.  Just dreading the footgun. But it seems there is a solution.
19:26:47 <gwillen> the voice in the back of my head worries about the checksum being an optional thing stuck on the end, that people will strip it or ignore it
19:26:53 <gwillen> but I don't have a better idea in hand
19:27:15 * sipa fires up BCH code search?
19:27:19 <gwillen> there's no obvious better place to put it
19:27:53 <gmaxwell> gwillen: well we could mandate it, and not have anything that ignores it, but have a utility function that regenerates.
19:28:05 <gmaxwell> If someone wants to do extra work to screw themselves, oh well.
19:28:08 <instagibbs> Put bytes in pseudorandom places, annoying to extract and by then, might as well validate it
19:28:15 <instagibbs> (sarcasm font)
19:28:27 <gmaxwell> But the default should be as safe as we can make it without undue tradeoffs.
19:28:46 <gmaxwell> sipa: whats the character set of descriptors?
19:28:53 <provoostenator> Or you could design the descriptor language such that there's no way to accidentally break it with a N character mistake.
19:29:16 <gmaxwell> sipa: (does it form a field...)
19:29:43 <gmaxwell> provoostenator: it's easy to do that if the number of candidate characters is a prime power. Otherwise we only know how to make it immune to a 1 character mistake.
19:30:03 <provoostenator> I'm in favor of prime power.
19:30:33 <gmaxwell> I don't think right now descriptors have a defined charset?
19:30:38 <sipa> gmaxwell: indeed
19:30:49 <sipa> but at least 0-9 a-z A-Z / * [ ]
19:30:54 <gmaxwell> alphanum + some extras like /,()[]* ?
19:31:02 <gmaxwell> oh right mixed case.
19:31:26 <provoostenator> Pubkeys and privkeys have defined charset, so you can include the checksum part of those in the total checksum? (or just checksum the whole thing)
19:31:28 <gwillen> can't you always just add "virtual" characters to the charset until you reach a prime power
19:31:36 <gwillen> at the expense of increasing checksum size a bit
19:31:56 <gmaxwell> Checksum can be defined over a smaller charset, with a folding routine so that outside of character set characters get treated as some inside set character for checksum purposes.
19:32:11 <gmaxwell> gwillen: no, because the check digit could be one of those characters.
19:33:19 <gmaxwell> You could however make the checksum base 25, encoded to alpha, and then fold all other characters to it... in any case, probably not something to hash out in the meeting.
19:33:38 <gmaxwell> and doing something dumb (sha256, ugh) would be still better than nothing.
19:33:57 <sipa> we can reuse bech32 character set easily
19:34:11 <sipa> and use something like what bech32 does for the prefix
19:34:22 <gmaxwell> sipa: with the hrp han... right...
19:34:25 <sipa> (which works for all of ascii)
19:34:34 <gmaxwell> doesn't have the great distance properties, but probably good enough.
19:34:57 <sipa> before we move to a different topic... what general length of checksum are people thinking is acceptable?
19:35:07 <gwillen> how would people feel about the syntax being something like "(descriptor,checksum)" instead of "descriptor,checksum"
19:35:26 <gwillen> adding two extra characters is kind of silly but it feels to me like it would make the checksum less likely to get lost in copy-paste confusion
19:35:58 <gmaxwell> sipa: descriptors are already long, I don't see a problem with adding 5-8 extra characters.
19:36:18 <gwillen> (also, the checksum charset should exclude "()[]*/.", it should be alphanum, for the same reason)
19:36:22 <provoostenator> Sticking to bech32 characters would be nice.
19:36:42 <provoostenator> Because longer term it's probably nice if desciptors can be printed as QR codes.
19:36:50 <provoostenator> I'm thinking paper backups.
19:36:53 <provoostenator> (of pub keys)
19:37:05 <gmaxwell> I guess spaces are syntatically meaningless in descriptors? if so checksum should be computed after stripping them.
19:37:22 <instagibbs> spaces are currently rejected anywhere
19:37:26 <instagibbs> in Core
19:37:27 <gwillen> oof, it would be good if descriptor format were canonical, no spaces
19:37:29 <gmaxwell> provoostenator: well unfortunately the other characters in descriptors make QR codes inefficient.
19:37:30 <gwillen> :+1:
19:37:34 <gmaxwell> oh okay, cool.
19:37:44 <sipa> gmaxwell: i think the parser will reject anything with spaces
19:37:52 <instagibbs> sipa, it will(I've lost time figuring this out)
19:38:01 <instagibbs> the error message is quite vague
19:38:03 <gmaxwell> instagibbs: improve error messages?
19:38:07 <instagibbs> gmaxwell, yeah
19:38:33 <gmaxwell> sorry for the derail, but I'm glad there seems to be support for fixing this.
19:39:01 <sipa> gmaxwell: fwiw, your 3-of-2 multisig descriptor is rejected
19:39:37 <instagibbs> I mean you might accidentally do a wrong number or something, 1-of-2 instead of 2-of-2, similar failure
19:39:38 <gmaxwell> sipa: thats great.
19:39:50 <gmaxwell> or 2 of 2 instead of 1 of 2.
19:39:57 <gmaxwell> sipa: is 0 of 2 rejected? :P
19:39:59 <sipa> and i think p2sh descriptors with multisig redeemscripts over 510 bytes are also rejected
19:40:12 <sipa> gmaxwell: yes, 0 of 2 is also rejected
19:40:20 <gmaxwell> The fact that a lot of mistakes will fail the parser also favors a weaker check.
19:40:32 <sipa> ok, let's move to a different topic
19:40:36 <sipa> if there are others?
19:43:01 <gmaxwell> Is anyone working on improving avoidpartialspends?
19:43:02 <jnewbery> for wallet goals for v0.18, I'm hopeful we can finish multiwallet in the GUI
19:43:12 <jnewbery> promag's been doing great work
19:43:17 <gmaxwell> It's off by default, which makes it kinda useless in terms of overall privacy effects.
19:43:24 <sipa> gmaxwell: what needs to be done?
19:43:57 <gmaxwell> sipa: It needs to grow a value threshold. "Avoidpartialspends unless the fee would increase by more than X"  which we could ship enabled.
19:44:28 <gmaxwell> And privacy maniacs could turn it on unconditionally, but everyone would at least be happy with a threshold of 0. :P
19:44:59 <gmaxwell> (I think we should set the threshold by default to the dust limit or something similar, the exact value doesn't matter too much)
19:45:22 <gmaxwell> Reminder: this is the wallet feature that causes it to try to spend all payments to a reused address at once.
19:45:50 <gmaxwell> Doing so can cause higher fees, so its off by default, and as a reasult I doubt anyone uses it.
19:45:55 <gmaxwell> result*
19:45:58 <sipa> right
19:46:10 <sipa> i agree that makes it pretty pointless
19:46:59 <gmaxwell> sometimes it doesn't even increase fees at all, I know of now reason why people wouldn't prefer it in at least that case.  But even when it causes higher fees, usually it's just pulling in fees from the future, not really paying more.
19:47:14 <gmaxwell> This returns to a larger open question about fees now vs fees later.
19:47:45 <sdaftuar> gmaxwell: one concern i had with unconditional use of avoidpartialspends was edge cases with small value inputs.  eg i dust your wallet with junk, and cause you trouble
19:47:59 <sdaftuar> i think we mitigated that mostly, maybe entirely, with a cap on the number of inputs we could pull in?
19:48:23 <gmaxwell> and/or with a threshold you'll pay in extra fees?
19:48:32 <sdaftuar> yeah, we should just do that.
19:49:06 <gmaxwell> but I agree there should be a cap on the group size, but I'm not sure how best to do that.
19:50:11 <gmaxwell> For the longer term question, I think eventually we want the wallet fee estimation to have an idea of "fees expected to be higher/lower in the future", and in response we should favor or disfavor including more inputs.
19:50:24 <sipa> gmaxwell: don'
19:50:28 <sipa> gmaxwell: don't we already have that?
19:50:40 <sipa> (using long term fee estimation)
19:51:27 <gmaxwell> I think we do in one really narrow case (we use the long term fee for estimating the cost of spending the change output in BNB)? I don't think we have it more generally.
19:52:25 <gmaxwell> I think because fees have been low most of the time in the last 6 months not a lot of attention is going to them, ... which is unfortunate at least in terms of right now would be a great time for wallets to be agressively aggregating.
19:53:08 <gmaxwell> (well not litterally right now, there are fees at the moment.. :P)
19:53:35 <sipa> right
19:54:21 <gmaxwell> so okay well that would be a bonus feature for avoidpartial: use long term vs current to switch between threshold vs always on.
19:54:55 <gmaxwell> If fees are high only be willing to pay slightly more, if fees are low use it.
19:55:50 <jnewbery> BitGo claim to already do 'predictive UTXO management' (ie use more inputs when fees are expected to be higher in future, and fewer inputs when fees are expected to be lower in future)
19:56:26 <gmaxwell> it's been discussed a lot in the past, it's just not clear how well it can work without a human hand available to rescue it if it becomes stupid.
19:57:17 <gmaxwell> It's a lot easier to do smart stuff if you can count on an expert non-artifical-intelligence to override if it goes dumb, you don't have to solve ever corner case or potential avenue for abuse.
19:58:15 <bitcoin-git> [13bitcoin] 15jamesob opened pull request #15264: validation: remove useless uncache accounting in ATMPW (06master...062019-01-remove-useless-uncaching-atmpw) 02https://github.com/bitcoin/bitcoin/pull/15264
19:59:48 <sipa> last half minute topic?
20:00:25 <sipa> #endmeeting