19:00:21 <achow101> #startmeeting 
19:00:21 <core-meetingbot> Meeting started Fri Jan 13 19:00:21 2023 UTC.  The chair is achow101. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:00:21 <core-meetingbot> Available commands: action commands idea info link nick
19:00:25 <provoostenator> hi
19:00:34 <achow101> #bitcoin -core-dev Wallet Meeting: achow101 _aj_ amiti ariard BlueMatt cfields Chris_Stewart_5 darosior digi_james dongcarl elichai2 emilengler fanquake fjahr furszy gleb glozow gmaxwell gwillen hebasto instagibbs jamesob jarolrod jb55 jeremyrubin jl2012 jnewbery jonasschnelli jonatack josibake jtimon kallewoof kanzure kvaciral laanwj larryruane lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball morcos Murch nehan NicolasDorier
19:00:34 <achow101> paveljanik petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar S3RK sipa vasild
19:00:44 <achow101> Welcome to the bi-weekly wallet meeting
19:00:46 <sipa> hi
19:01:01 <achow101> There are no pre-proposed wallet meeting topics
19:01:07 <achow101> does anyone have anything to discuss?
19:01:24 <furszy> hi
19:01:34 <Murch1> furszy is doing great work, nothing else :p
19:01:57 <furszy> <3
19:02:39 <Murch1> Kinda curious how we are getting non-zero waste scores for coin selection algorithms when the feerate, LTFRE, and cost-of-change are 0
19:02:40 <Murch1> May be a bug
19:02:48 <Murch1> but I guess furszy and I will be looking a bit more into that
19:02:57 <achow101> Murch1: in a test?
19:03:03 <Murch1> Aye
19:03:07 <achow101> excess
19:03:16 <furszy> yeah
19:03:23 <Murch1> #26720 
19:03:25 <gribble> https://github.com/bitcoin/bitcoin/issues/26720 | wallet: coin selection, dont return results that exceed the max allowed weight by furszy · Pull Request #26720 · bitcoin/bitcoin · GitHub
19:03:26 <achow101> that's the only other term in that equation
19:03:32 <Murch1> Excess only appears in transactions without change
19:04:00 <achow101> I'll put that on my todo list
19:04:08 <furszy> as there is no change, the waste is merely calculated by waste = selected_effective_value - target;
19:04:11 <Murch1> it's 0 for transactions with change
19:04:14 <Murch1> See GetSelectionWaste(…)
19:04:44 <Murch1> Clarification, there is a change output, but the cost_of_change is set to 0 for the test.
19:05:24 <Murch1> furszy: If that were so, that would be a bug
19:05:45 <furszy> the test is only running the selection process, it's not creating a tx
19:05:55 <furszy> not sure what you think that there is a change output
19:05:58 <achow101> Murch1: it's probably excess. IIRC we assume cost_of_change == 0 means changeless solution which means include excess
19:06:15 <Murch1> Nuh uh
19:06:17 <furszy> *why you think
19:06:26 <furszy> achow101: yep
19:07:09 <Murch1> It's running Knapsack and SRD
19:07:09 <Murch1> They produce change
19:07:12 <Murch1> waste = input_weight × (feerate - LTFRE) + cost_of_change + excess
19:07:13 <Murch1> feerate = LTFRE = 0
19:07:13 <Murch1> cost_of_change = 0
19:07:21 <Murch1> excess is zero for solutions with change
19:07:36 <Murch1> so both Knapsack and SRD should have a waste score of 0 throughout the test
19:07:41 <achow101> Murch1: ignoring the fact of whether or not change is produced, we determine whether to use excess depending on the value of cost_of_change. If it happens to be 0, then we include excess
19:07:41 <Murch1> but furszy reports they do not
19:07:44 <Murch1> ergo Bug
19:07:56 <Murch1> achow101: ooooooooh.
19:07:59 <Murch1> Gah.
19:08:13 <Murch1> That is TERRIBLE
19:08:25 <Murch1> but okay, I see why I'm wrong
19:08:26 <Murch1> thanks
19:08:33 <achow101> well it works in normal usage :)
19:08:54 <Murch1> IMO, the issue here is that we are setting a bunch of values that are never 0 to 0 in a test that is supposed to test how the function works when everything is not 0
19:08:57 <achow101> but yes, it might make sense to change that
19:09:39 <Murch1> Yes, let's please not set everything to zero when that is completely unnatural and never occurs in operation
19:10:04 <Murch1> Sure, we might have tests to see that it would continue working, but it just baffles me that we are testing coin selection but make it behave completely different
19:10:06 <achow101> right, a lot of the tests pre-date the more recent coin selection things and so in that conversion, it was convenient to use a default value of 0 because then they wouldn't implode
19:10:14 <Murch1> I guess I had a topic after all :p
19:10:35 <Murch1> Okay, how about we just don't do that
19:10:42 <achow101> however I think our coin selection decisions are stable enough at this point we should go through the tests and actually update them and evaluate whether they still make sense
19:11:08 <Murch1> Also, tbh, the trigger of adding excess when cost_of_change is 0, is kinda bad
19:11:14 <Murch1> it should be whether or not there is change
19:11:16 <achow101> note: I am not volunteering to do this but will gladly review
19:11:32 <Murch1> Yeah, that's alright
19:12:19 <Murch1> Also, furszy: my apologies, I was wrong and you were right all along!
19:12:19 <Murch1> haha
19:12:45 <achow101> glad I could resolve that without having to actually read any code :)
19:13:13 <Murch1> I bet
19:13:27 <furszy> hehe, i tried to explain what achow101 said in a line in 4 long comments in the PR :p
19:13:57 <furszy> should have used the "excess" wording
19:13:59 <Murch1> Yeah, in theory it should work, but in practice…
19:14:55 <Murch1> Nah, sorry, that was my fault, I did not realize that setting cost_of_change = 0 actually makes everything evaluate as if it were changeless solutions 🤦‍♂️
19:15:22 <achow101> anything else to discuss?
19:15:39 <Murch1> BIP47 is terrible
19:15:49 <Murch1> No discussion necessary, though
19:15:53 <Murch1> :p
19:16:10 <Murch1> err, no
19:16:45 <Murch1> Also, are we defaulting to P2TR change yet? ^^
19:17:17 <Murch1> At least when sending to P2PKH for wallets that don't have a legacy descriptor?
19:17:18 <achow101> I think we might want to avoid changing that since it ends up being a dead giveaway of what our change is
19:17:44 <Murch1> I mean, we already match the recipient output when we have a descriptor for that type
19:18:21 <achow101> if you do not have a descriptor for legacy, and have a bech32m descriptor, we will use a bech32m as change
19:18:43 <Murch1> noice
19:18:49 <achow101> so I think the answer is that we already do what you suggest
19:18:52 <Murch1> uploaded an image: (179KiB) < https://libera.ems.host/_matrix/media/v3/download/matrix.org/euMDnthvCgXYBsOVqnUMibAL/image.png >
19:19:06 <Murch1> P2TR outputs have been going up quite a bit
19:19:44 <achow101> A more relevant question might be when should we change the default address type to p2tr
19:19:57 <Murch1> v25
19:20:39 <achow101> I think we would need to do some research on what wallets will accept and correctly interpret bech32m
19:21:22 <Murch1> I gotchu: https://whentaproot.org/#support
19:21:57 <Murch1> It's just a few stragglers of minor relevance such as Binance, Coinbase, Bitfinex, Bitso, Crypto.com, Fireblocks, Gemini, Luno, and Paxful that have not implemented support yet
19:22:13 <achow101> it's only the biggest exchange, nbd
19:22:50 <Murch1> 😄
19:22:53 <achow101> I guess choosing the address type to use is really easy to do now so maybe it doesn't matter as much?
19:23:26 <Murch1> If we announce that we are switching the default address type early and loudly enough, they might also just finally do the ~2 line code change
19:23:57 <Murch1> But yeah, for real, maybe v26 is more realistic
19:25:17 <achow101> any other topics?
19:25:36 <Murch1> We should do a wallet offsite
19:25:44 <Murch1> just wallet devs hanging out for a few days
19:26:05 <Murch1> preferably in the winter on an island where it's warm
19:26:57 <Murch1> I hear the Caribbean is nice this time of the year
19:27:04 <achow101> perhaps
19:27:32 <achow101> I don't know if there's a whole lot to talk about that we can't do during a coredev event?
19:27:47 <Murch1> 🤷
19:27:56 <Murch1> I think you are only catching part of the point :p
19:30:03 <achow101> #endmeeting