19:00:37 <achow101> #startmeeting 
19:00:38 <core-meetingbot> Meeting started Fri Jun  4 19:00:37 2021 UTC.  The chair is achow101. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:00:38 <core-meetingbot> Available commands: action commands idea info link nick
19:00:49 <sipa> hi
19:00:54 <jonatack> hi
19:01:08 <achow101> #bitcoin -core-dev Wallet Meeting: achow101 _aj_ amiti ariard BlueMatt cfields Chris_Stewart_5 darosior digi_james dongcarl elichai2 emilengler fanquake fjahr gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jarolrod jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack jtimon kallewoof kanzure kvaciral laanwj lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball morcos nehan NicolasDorier paveljanik petertodd
19:01:09 <achow101> phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild
19:01:34 <achow101> I have a topic, but are there any other topics for today?
19:01:51 <sipa> topic suggestion: what to prioritize for taproot wallet support? there are probably half a dozen things that need to be done
19:01:58 <michaelfolkson> hi
19:02:06 <jarolrod> hi
19:02:17 <fjahr> hi
19:02:40 <achow101> #topic subtract fee from recipients intended behavior and edge cases (achow101)
19:02:40 <core-meetingbot> topic: subtract fee from recipients intended behavior and edge cases (achow101)
19:03:05 <achow101> What is the exact behavior and use case we expect for the subtract fee from recipients feature?
19:03:35 <sipa> can you give an example of where the expected behavior isn't clear?
19:03:45 <ryanofsky> (context is https://github.com/bitcoin/bitcoin/pull/22008#pullrequestreview-676528963)
19:03:55 <achow101> ryanofsky noticed that the current implementation (after 17331 removed the loop) will increase the recipient amounts if a dropped change output was more than the required fee
19:04:22 <achow101> I looked at the previous looping implementation, and it looks like that would actually just overpay the fees in that case. it would reduce the fee from the recipients, then drop the change output to fee
19:04:35 <achow101> I think neither of these things are what we want to do
19:05:42 <michaelfolkson> Your view is... achow101? :)
19:05:46 <sipa> what does dropped mean here? don't you "start" from a transaction without change, and add it if necessary?
19:06:04 <achow101> michaelfolkson: https://github.com/bitcoin/bitcoin/pull/22008#issuecomment-854880614
19:06:18 <jonatack> sipa: was wondering this too
19:06:36 <achow101> sipa: we start from a transaction with change, then subtract the fees from it. then if it is below dust or the exact match window, we remove the change output
19:06:52 <achow101> with subtract fee from output, we skip the "subtract fees from it" step
19:07:06 <sipa> achow101: only in the current knapsack?
19:07:14 <achow101> sipa: always
19:07:22 <achow101> we do this regardless of the algorithm
19:07:52 <achow101> with BnB, the change always ends up in the exact match window since we do the same math, so the change is always dropped
19:08:10 <ryanofsky> IMO the choice is just between slightly overpaying fees and being pedantic about the word "subtract". Or just keeping current behavior and mostly subtracting sometimes adding to recipient when it's most efficient
19:09:01 <ryanofsky> This is good for the use-case of transferring funds between wallets, which is at least is where I've used subtract from recipient before
19:09:04 <michaelfolkson> I do think paying a little more than the recipient is expecting is strange behavior for both the sender and the receiver
19:09:38 <ryanofsky> It deserves to be documented, sure
19:09:39 <achow101> what we've always done is overpay fees, probably too much
19:09:40 <sipa> ryanofsky: i think that's the most important use case
19:09:51 <sipa> moving all funds from one wallet to another
19:09:56 <ryanofsky> But I think you choose behavior based on use cases, not on being pedantic about terminology
19:10:12 <achow101> ryanofsky: the use case I've seen the most is moving all funds from one wallet to another, in which case this discussion doesn't matter
19:10:33 <achow101> but we don't really know what people actually use this for
19:10:46 <ryanofsky> achow101, it matters because in one case you overpay the miner, in the other case you have a little more funds in your new wallet
19:11:53 <sipa> is this a long term problem? i'd imagine that in a BnB + SRD world, you have two algorithms one that aims for no-change solutions, and one that aims for with-change solutions, and you pick the best one
19:11:59 <ryanofsky> but it doesn't matter that much, it is always a case where amount is less than the estimated cost of creating & spending the utxo
19:12:09 <Guest37> Im looking for a developer, anyone freelancing?
19:12:11 <sipa> and if the with-change solution results in too low change, you just discard that solution
19:12:24 <michaelfolkson> Guest37: Sorry, in the middle of a meeting
19:12:28 <sipa> or you convert it to a no-change one, and consider it in that form
19:13:02 <jonatack> use case: i subtractfeefromamount in the case of say, selling btc to a party who chooses the fee rate they want to use for the txn
19:13:11 <Guest37> its okay, after call me.paying big bucks for a bitcoin competitor. "Bitcoin Green" - Runs 100% sustainable. No electricity used ever.
19:13:16 <sipa> Guest37: go away
19:13:17 <Guest37> PM me before u miss the launch
19:13:17 <ryanofsky> sipa, this is all after coin selection, when the algorithm has already run and then we discover the change output is uneconomical
19:13:41 <Guest37> *scoff* nerds
19:13:45 <achow101> I think this case is an extreme edge case
19:13:50 <ryanofsky> so we discard the change output, and pay slightly more fees in the normal case
19:13:57 <achow101> especially after effective value
19:14:19 <ryanofsky> and in the subtract from recipients case we can do a little better and send the extra amount to recipients, which is what the code is doing now
19:14:34 <ryanofsky> but agree it is not very important
19:14:56 <sipa> ryanofsky: which means it increases the waste metric (once that's in use), and will hopefully not be favored if an actual no-change solutions exists too
19:15:17 <sipa> ?
19:15:37 <ryanofsky> Not sure, I'm not familiar with the waste metric yet
19:15:52 <achow101> sipa: I don't think the waste metric accounts for change being dust, yet
19:16:03 <sipa> it should, i think
19:16:13 <sipa> overpaying fee is waste
19:16:36 <achow101> right, we do consider that when we know there is no change
19:17:38 <achow101> actually, thinking on it, I don't think knapsack can even find a solution where change would be dust.
19:17:47 <achow101> since it targets a minimum change value
19:18:41 <ryanofsky> Then maybe it is even more rare and only happens with manual coin selection
19:18:52 <sipa> i'm thinking something similar
19:19:21 <achow101> indeed
19:19:23 <jonatack> test coverage on this may be nice, if missing
19:19:29 <sipa> in whatever situation where you'd end up with change that's uneconomical (which dust definitely is), there *should* exist a better no-change solution that BnB would find
19:20:09 <sipa> at the very least, the one that's exactly the same but with the change converged to fee
19:20:16 <sipa> (but possibly, a better one too)
19:20:24 <ryanofsky> Either way, I'm just defending current behavior, and don't see a benefit in changing besides being more strict about "subtract". But I don't think it would be a huge deal to change
19:21:11 <achow101> ryanofsky: the current behavior is accidental though, and if you look at before 17331, we would always overpay
19:21:48 <ryanofsky> Ok, I did write the code intentionally that way when I suggested in in the other PR, but maybe I misread previous behavior
19:22:41 <achow101> I didn't realize that at that time. if I did, we would have had this discussion several weeks ago
19:22:48 <ryanofsky> I was trying not to change behavior, but I think I was basing it on your existing changes in that PR not the previous code.
19:23:45 <ryanofsky> Anyway, that would be another reason to change, which is fine
19:24:40 <achow101> I'll do some further analysis on the actual conditions this could occur and we can revisit this later
19:24:50 <achow101> as usual, coin selection remains inscrutable
19:25:11 <achow101> #topic what to prioritize for taproot wallet support? (sipa)
19:25:11 <core-meetingbot> topic: what to prioritize for taproot wallet support? (sipa)
19:25:20 <ryanofsky> Sure, and you should really do what you want there ultimately
19:26:27 <michaelfolkson> Can you give an update on where we are with Taproot wallet support to start with sipa? I saw this PR though haven't looked through it yet https://github.com/bitcoin/bitcoin/pull/21365
19:26:35 <sipa> so a rough list of missing things in master: signing support, descriptor inference support, multisig support (multi(k,...) doesn't exist in taproot), actual PSBT support with extensions so spending paths can be conveyed, support for easily constructing no-keypath descriptors, ...
19:27:05 <sipa> do we want to try to get signing support in 22.0?
19:27:17 <achow101> I think we should try to get signing support for 22.0
19:27:24 <achow101> and basic descriptor inference
19:27:47 <fjahr> hell yeah :)
19:28:17 <sipa> signing for basic stuff is done (which is really only useful for keypath signing, but in theory does script paths too)
19:28:17 <achow101> michaelfolkson: we have tr() descriptor construction with keys only now. you can import them into a watch only descriptor wallet
19:28:59 <sipa> so we punt multisig support and other descriptor extensions
19:29:11 <sipa> and just try to get basic key path signing to work
19:29:16 <achow101> I think that's reasonable
19:29:54 <achow101> also 22.0 should be released before taproot activates, so I don't think we need to have support for everything at the very beginning
19:29:56 <sipa> we'll want PSBT extensions for taproot (and musig, once that's a bit further along), but that's a larger discussion than just the bitcoin core wallet
19:30:44 <michaelfolkson> So that would just be #21365 that needs review and merging for 22.0?
19:30:46 <gribble> https://github.com/bitcoin/bitcoin/issues/21365 | Basic Taproot signing support for descriptor wallets by sipa · Pull Request #21365 · bitcoin/bitcoin · GitHub
19:30:58 <sipa> plus inference support
19:31:20 <achow101> basic inference support shouldn't be particularly hard
19:31:52 <achow101> do we need to add OutputType::BECH32M?
19:32:08 <sipa> that's a good question
19:32:13 <sipa> i'd say yes
19:32:28 <sipa> because it's something receivers care about
19:32:35 <achow101> agreed
19:32:55 <achow101> that also shouldn't be hard
19:33:04 <sipa> indeed
19:33:18 <sipa> i don't know much about the interaction with descriptor wallets, but i can try
19:33:40 <achow101> I can look at it
19:33:50 <achow101> I don't think it requires much in the wallet itself
19:34:06 <michaelfolkson> Inference support is just docs, error messages, things like that?
19:34:27 <sipa> lots of RPCs report inferred descriptors
19:34:59 <achow101> michaelfolkson: inference support is getting the descriptor for a given scriptPubKey
19:35:08 <michaelfolkson> Oh ok, thanks
19:35:46 <achow101> reminder that feature freeze is in ~10 days
19:36:40 <achow101> anything else to discuss?
19:37:05 <sipa> GetDestinationForKey... doesn't need BECH32M support, right?
19:37:45 <sipa> that's only for legacy wallets i assume (and hope)
19:38:14 <achow101> sipa: should be for legacy wallets only
19:38:16 <achow101> so no
19:38:42 <sipa> dumpwallet is disabled for descriptor wallets?
19:38:46 <achow101> yes
19:39:35 <achow101> #endmeeting