19:01:43 <achow101> #startmeeting 
19:01:44 <core-meetingbot> Meeting started Fri May 20 19:01:43 2022 UTC.  The chair is achow101. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:01:44 <core-meetingbot> Available commands: action commands idea info link nick
19:01:46 <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 larryruane lightlike luke-jr maaku marcofalke meshcollider michagogo moneyball morcos Murch nehan NicolasDorier paveljanik
19:01:46 <achow101> petertodd phantomcircuit promag provoostenator ryanofsky sdaftuar S3RK sipa vasild
19:02:06 <achow101> we haven't had a wallet meeting in a while
19:02:27 <sipa> hi
19:02:32 <achow101> there aren't any pre-proposed wallet meeting topics. Does anyone have anything to discuss?
19:03:15 <Murch> hi
19:04:35 <kanzure> hi
19:04:37 <achow101> or any PRs to shill?
19:04:43 <Murch> I don't have anything from the top of my head
19:04:58 <Murch> https://github.com/bitcoin/bitcoin/pull/25083
19:05:15 <Murch> I think it's almost ready to go, could use another review or two
19:06:13 <achow101> I think #24649 and  #25122 are close as well
19:06:14 <gribble> https://github.com/bitcoin/bitcoin/issues/24649 | wallet: do not count wallet utxos as external by S3RK · Pull Request #24649 · bitcoin/bitcoin · GitHub
19:06:15 <gribble> https://github.com/bitcoin/bitcoin/issues/25122 | rpc: getreceivedbylabel, return early if no addresses were found in the address book by furszy · Pull Request #25122 · bitcoin/bitcoin · GitHub
19:06:57 <Murch> Oh, I should look at #25122
19:06:58 <gribble> https://github.com/bitcoin/bitcoin/issues/25122 | rpc: getreceivedbylabel, return early if no addresses were found in the address book by furszy · Pull Request #25122 · bitcoin/bitcoin · GitHub
19:07:02 <Murch> I have reviewed the other already
19:08:13 <achow101> It looks like there have been a lot of proposed changes to AvailableCoins that conflict with each other, but all look nice to get in
19:08:29 <Murch> Yeah, that's right
19:09:00 <achow101> e.g. #25005 #25118 #25083 #24699 #24584
19:09:01 <gribble> https://github.com/bitcoin/bitcoin/issues/25005 | wallet: remove extra wtx lookup in AvailableCoins + several code cleanups. by furszy · Pull Request #25005 · bitcoin/bitcoin · GitHub
19:09:02 <gribble> https://github.com/bitcoin/bitcoin/issues/25118 | wallet: unify “allow/block other inputs“ concept by furszy · Pull Request #25118 · bitcoin/bitcoin · GitHub
19:09:03 <gribble> https://github.com/bitcoin/bitcoin/issues/25083 | Set effective_value when initializing a COutput by ishaanam · Pull Request #25083 · bitcoin/bitcoin · GitHub
19:09:04 <gribble> https://github.com/bitcoin/bitcoin/issues/24699 | wallet: Improve AvailableCoins performance by reducing duplicated operations by achow101 · Pull Request #24699 · bitcoin/bitcoin · GitHub
19:09:06 <achow101> any thoughts on what to prioritize?
19:09:07 <gribble> https://github.com/bitcoin/bitcoin/issues/24584 | wallet: avoid mixing different `OutputTypes` during coin selection by josibake · Pull Request #24584 · bitcoin/bitcoin · GitHub
19:09:42 <Murch> I don't think Josi's is close to getting merged.
19:09:54 <Murch> I haven't looked at #25005, so I can't comment on that
19:09:55 <gribble> https://github.com/bitcoin/bitcoin/issues/25005 | wallet: remove extra wtx lookup in AvailableCoins + several code cleanups. by furszy · Pull Request #25005 · bitcoin/bitcoin · GitHub
19:10:20 <achow101> 25083 and 25118 both are pretty simple
19:10:20 <Murch> How useful is 24699 now that we use different keys?
19:10:24 <achow101> so I think they can go in soon
19:11:15 <Murch> I'm biased, one of my PRs is building on #25083 ^^
19:11:16 <gribble> https://github.com/bitcoin/bitcoin/issues/25083 | Set effective_value when initializing a COutput by ishaanam · Pull Request #25083 · bitcoin/bitcoin · GitHub
19:11:21 <achow101> I think 24699 is still a worthwhile promovement. it improves more than just the reused addresses case
19:12:02 <achow101> *improvement
19:12:24 <Murch> okay
19:12:33 <Murch> I'll put all of the above on my review list
19:13:27 <Murch> I think just in the order that they're ready is fine
19:13:44 <Murch> I don't think rebasing would get too bad for any
19:14:23 <Murch> You should add josibake to your ping list ^^
19:14:47 <achow101> I think 24584 might have a larger conflict with the others
19:14:53 <achow101> but it's also not ready imo
19:15:27 <Murch> Yeah, and it is also the biggest one
19:15:49 <achow101> anything else look ready or nearly ready?
19:15:51 <jonatack> hi  (...reading up)
19:17:25 <achow101> anything else to discuss?
19:17:43 <Murch> Not from the top of my head. Really curious to see some simulation results for #24584
19:17:47 <gribble> https://github.com/bitcoin/bitcoin/issues/24584 | wallet: avoid mixing different `OutputTypes` during coin selection by josibake · Pull Request #24584 · bitcoin/bitcoin · GitHub
19:18:17 <Murch> I'm wondering how splitting the UTXO pool in multiple ways will affect the tx costs
19:18:34 <Murch> Oh one more
19:18:55 <Murch> There is this very tight testing corset that S3RK proposed
19:19:12 <Murch> So far you and I reviewed it, achow101
19:19:41 <Murch> Yours sounded like a soft concept nack or approach nack
19:20:09 <achow101> #24580 ?
19:20:10 <gribble> https://github.com/bitcoin/bitcoin/issues/24580 | test: coinselection edge cases by S3RK · Pull Request #24580 · bitcoin/bitcoin · GitHub
19:20:19 <Murch> I think maintaining the test when we start changing the coin selection algos will be annoying, but it's also kinda nice to see the exact boundaries
19:20:26 <Murch> yeah thanks!
19:20:59 <Murch> I think that S3RK might be a bit stuck there, no way forward not really shot down either
19:21:40 <achow101> perhaps more people should look at it
19:21:59 <Murch> Could we perhaps give more input how it should be developed to address maintainability concerns?
19:23:36 <achow101> maybe more programmatic math rather than hardcoded math
19:23:59 <Murch> Yeah
19:24:28 <Murch> Do you know what the status with Bruno's "Prefer changeless" PR is?
19:24:29 <Murch> That also seemed a bit stuck
19:24:29 <instagibbs> woof, lots of magic numbers
19:24:41 <instagibbs> re: #24580
19:24:42 <gribble> https://github.com/bitcoin/bitcoin/issues/24580 | test: coinselection edge cases by S3RK · Pull Request #24580 · bitcoin/bitcoin · GitHub
19:26:19 <Murch> https://github.com/bitcoin/bitcoin/pull/23475
19:26:23 <achow101> Murch: it seems like it needs conceptual review as to whether that's a good idea?
19:27:03 <Murch> achow101: I think I'd be "concept ack, approach nack", wasn't sure whether that came out clearly
19:27:33 <Murch> Is brunoerg here?
19:27:48 <jonatack> having good boundary testing of expected behavior in place before making big changes can be good if the testing is of the right kind... maybe hoist the magic numbers up to constants or derive them programmatically as mentioned
19:28:16 <Murch> jonatack: that sounds like a good idea
19:28:39 <jonatack> might be good for people to weigh in on https://github.com/bitcoin/bitcoin/issues/25130
19:28:39 <instagibbs> derive please, don't make follow-on contributors hate life
19:29:23 <instagibbs> can even be done and merged piece by piece
19:29:24 <achow101> #23475 would interact pretty poorly with #24752
19:29:26 <gribble> https://github.com/bitcoin/bitcoin/issues/23475 | wallet: add config to prioritize a solution that doesnt create change in coin selection by brunoerg · Pull Request #23475 · bitcoin/bitcoin · GitHub
19:29:27 <gribble> https://github.com/bitcoin/bitcoin/issues/24752 | wallet: increase BnB upper limit by S3RK · Pull Request #24752 · bitcoin/bitcoin · GitHub
19:29:45 <Murch> instagibbs, jonatack: I think one issue is also that it sorta depends on the current selection of algorithms we use, and sort of tests whether the outcome was Knapsack, or BnB at times
19:30:09 <instagibbs> Murch, yeah I'm not wading in on the overall thrust, deferring on that
19:30:34 <Murch> Yeah, #23475 needs some sanity limit, similar to the avoid_partial_spend approach
19:30:35 <gribble> https://github.com/bitcoin/bitcoin/issues/23475 | wallet: add config to prioritize a solution that doesnt create change in coin selection by brunoerg · Pull Request #23475 · bitcoin/bitcoin · GitHub
19:31:27 <Murch> Okay, I guess the overall result is, we could all take another glance at these and perhaps be a bit more direct with feedback? ;)
19:31:40 <achow101> yes
19:31:45 <jonatack> instagibbs: Murch: agree, may need more abstraction
19:32:53 <achow101> anything else to discuss?
19:34:11 <Murch> No, just … by the way, I'm of the opinion that Knapsack needs to die
19:34:12 <Murch> :D
19:34:16 <achow101> #endmeeting