19:00:18 <achow101> #startmeeting 
19:00:18 <core-meetingbot> Meeting started Fri Dec 17 19:00:18 2021 UTC.  The chair is achow101. Information about MeetBot at https://bitcoin.jonasschnelli.ch/ircmeetings.
19:00:18 <core-meetingbot> Available commands: action commands idea info link nick
19:00:28 <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 nehan NicolasDorier paveljanik petertodd
19:00:29 <achow101> phantomcircuit promag provoostenator ryanofsky sdaftuar sipa vasild S3RK
19:00:38 <S3RK> hi
19:00:40 <sipa> hi
19:00:46 <achow101> any topics?
19:01:00 <sipa> none from me
19:01:02 <S3RK> bnb upper limit?
19:01:19 <achow101> #topic bnb upper limit (S3RK)
19:01:20 <core-meetingbot> topic: bnb upper limit (S3RK)
19:01:25 <sipa> i invoke murch
19:01:27 <Murch[m]> Hi
19:01:31 <S3RK> oh nice
19:01:55 <S3RK> so the current upper limit for bnb is not optimal
19:02:27 <S3RK> it's economically better to drop more than the current upper limit for fees rather than add an extra input and create change output
19:02:37 <S3RK> we can either 1) increase the limit or 2) drop the limit
19:02:38 <Murch[m]> Yeah, we ran some experiments removing the upper bound for the exact match window and that resulted in an overall lower cost
19:02:45 <michaelfolkson> hi
19:03:17 <Murch[m]> S3RK: My current position would be that I'd be apprehensive about dropping the limit altogether.
19:03:26 <achow101> if our calculation for the upper bound is incorrect, I would prefer to fix it rather than drop the limit entirely
19:03:36 <Murch[m]> If both Knapsack and SRD find very bad solutions, we may drop a ton of money to fees.
19:03:56 <S3RK> it's a bit tricky to calculate exact upper limit
19:03:59 <achow101> but IIRC part of the issue is that our drop to fees handling is independent of the selection algorithm
19:04:46 <Murch[m]> Right, but without an upper bound, BnB may end up returning a result that has a larger remainder than what's dropped and our transaction building ends up building a tx with a minuscule change
19:04:49 <S3RK> hm... I'd say that's a related but separate issue. Would love to discuss it as well, but let's talk about the bnb limit first
19:05:02 <Murch[m]> So, we could just post-filter BnB solutions to not use such a solution
19:05:32 <S3RK> Murch, why do you think knapsack wouldn't find the same solution but with change?
19:05:47 <achow101> the min change target
19:05:58 <Murch[m]> Because Knapsack minimizes the overshoot past minChange
19:05:59 <S3RK> hm...
19:06:08 <Murch[m]> It doesn't prefer input sets with lower waste
19:06:49 <S3RK> what about SRD?
19:06:54 <S3RK> ah.. also min change
19:07:05 <Murch[m]> SRD only runs once and also aims to create at least minChange
19:07:16 <S3RK> yes, that's a bit of problem
19:07:16 <Murch[m]> So you could have solutions on Knapsack and SRD that each have 10+ inputs
19:07:29 <Murch[m]> and BnB finds a solution with a single input that ends up dropping 20k sats
19:07:49 <Murch[m]> Or let's say 1k sats
19:08:05 <Murch[m]> and we end up creating a transaction with a 750 sat output
19:09:06 <S3RK> doesn't mean that right now we would say "insufficient funds" for such a tx?
19:09:12 <S3RK> sorry
19:09:15 <S3RK> does it mean taht ...
19:09:41 <Murch[m]> So, right now, BnB would not find that solution, and it wuold use one of the Knapsack or SRD
19:09:54 <achow101> actually knapsack should find the solution but with change
19:10:00 <achow101> even if min change is not met
19:10:18 <achow101> srd wouldn't
19:10:23 <S3RK> I have to confess I don't know the details how knapsack works right now
19:10:39 <Murch[m]> Right, but the scenario wasn't that we didn't have enough funds, but just that the solutions that Knapsack and SRD propose have huge input sets
19:10:58 <Murch[m]> achow101: Do I misremember that Knapsack prefers the solution that overshoots the minchange the least?
19:11:44 <achow101> that sounds right
19:12:15 <achow101> it also prefers the solution that meets min change, but will produce a solution that does not if min change cannot be met
19:12:41 <Murch[m]> Right
19:13:10 <S3RK> It seems we should codify this scenarious in tests
19:13:11 <Murch[m]> I guess we could amend Knapsack to compare the 1,000 trials via the waste metric instead of minimizing the excess, and then I'd agree.
19:13:33 <achow101> so if bnb without limit found a solution that throws away lots to fees, knapsack should find the same thing but with change
19:13:49 <achow101> the excess should dominate the waste calculation
19:13:49 <Murch[m]> Yes
19:13:54 <S3RK> achow101 +1 that was my understanding as well
19:13:55 <Murch[m]> Correct
19:14:19 <Murch[m]> That's why I painted a scenario in which they find different solutions because they optimize for different metrics in their selection ;)
19:14:53 <Murch[m]> But yeah, if we use the waste metric internally to compare input sets on Knapsack as well, it would be pretty safe to drop the limit on BnB
19:15:13 <achow101> however knapsack may also find a different solution that spends more inputs as it tries to meet min change. this would still be less waste on the knapsack side since excess dominates the bnb solution's waste
19:15:23 <Murch[m]> But it would also kinda be pointless, because then Knapsack is very similar to BnB just for a solution that targets a +minchange larger target
19:15:33 <Murch[m]> Yes
19:15:34 <achow101> even if the knapsack fees are higher
19:16:15 <Murch[m]> Knapsack and BnB are very similar in approach, except that Knapsack quasirandomly explores the binary UTXO tree, while BnB does it deterministically.
19:16:36 <achow101> I think we need to change how we drop things to fees, and then run simulations
19:17:03 <achow101> and see if bnb without limit causes some scenarios where we drop ridiculous amounts to fees
19:17:20 <S3RK> you can test on #23367 i guess
19:17:21 <gribble> https://github.com/bitcoin/bitcoin/issues/23367 | Optimize coin selection by dropping BnB upper limit by S3RK · Pull Request #23367 · bitcoin/bitcoin · GitHub
19:17:40 <Murch[m]> Well, in the experiment you ran for us, the numbers showed that we had more BnB solutions used than changeless transactions.
19:17:47 <Murch[m]> So, essentially, you've shown that already.
19:18:19 <S3RK> it doesn't mean we drop ridiculous amounts to fees
19:18:27 <S3RK> just more than we currently drop
19:18:38 <S3RK> which I argue could still be more economical
19:18:44 <Murch[m]> Yes, agreed
19:19:13 <S3RK> what if we introduce a safeguard?
19:19:25 <S3RK> put a limit on what is "ridiculous" to drop to fees?
19:19:26 <Murch[m]> But it also breaks a lot of assumptions about BnB, because previously we assumed that BnB never creates change, and when we find a really large excess, the transaction we build actually does result in a change output
19:19:50 <achow101> time for "absurdly-high-fee" errors again :p
19:19:52 <Murch[m]> Isn't that equivalent to just updating our upper bound for the exact match window? ;)
19:19:52 <S3RK> my PR does exactly this. BnB never creates change
19:20:18 <achow101> yes, we do need to look at 23367 and also run simulations on it
19:20:50 <S3RK> hm... yes we can also extend upper bound by the "absurdly-high-fee"
19:21:33 <achow101> (there used to be a mempool policy reject error named absurdly-high-fee)
19:21:52 <achow101> I think the fee had to be 0.1 or thereabouts
19:22:04 <S3RK> there is still some limit. 0.1 sounds about right
19:23:21 <S3RK> if you can write in plain english scenarios that concerns you, I can codify them as tests
19:23:38 <prayank> AI
19:23:57 <S3RK> I take it as a compliment :)
19:24:26 <achow101> one thing that concerns me is that it's probably going to hit the iteration limit very quickly
19:24:28 <S3RK> constexpr CAmount DEFAULT_TRANSACTION_MAXFEE{COIN / 10};
19:24:45 <S3RK> BnB iter limit?
19:24:53 <achow101> since we are removing one of the bounds that allow BnB to be called Branch and _Bound_
19:25:04 <S3RK> it shouldn't because there will be no branching
19:25:22 <S3RK> so it's linear to the amount of coins higher than the target
19:25:52 <Murch[m]> achow101: No, finding a valid solution backtracks just like overshooting the window does
19:26:58 <Murch[m]> Once you find a solution, it next explores the omission branch of the last included UTXO, the same as when it overshoots and stops searching the subtree because it already has too much
19:27:09 <achow101> oh right
19:27:50 <achow101> overshoot == finding a solution without limit, and both backtrack
19:30:08 <achow101> anything else to discuss?
19:30:12 <Murch[m]> It would be great if we had a test that does produce selections for which BnB without an upper bound on the exact match window breaks down
19:30:40 <S3RK> what do you mean by breaks down
19:32:19 <S3RK> I have a crazy idea. What if we create a test that iterates sending value from 1sat to 10^8sat and verifies fees
19:32:20 <Murch[m]> Well, something were SRD and BnB would be highly likely to produce an input set with a large count of inputs, while BnB finds one that overshoots the exact match window severely and the BnB input set scores best according to the waste metric
19:33:21 <Murch[m]> S3RK: I think that would be interesting, but probably the source of the issue would probably be found in the UTXO pool rather than the target amount
19:33:58 <Murch[m]> E.g. a wallet that has 1,000 UTXOs that have the same size, and one that is larger
19:34:08 <S3RK> true
19:34:53 <Murch[m]> And 100 UTXOs match the target plus minChange plus fees by the satoshi, but the one large input is just enough for the target without change but has a large excess
19:34:54 <S3RK> if we agreed that the waste metric is a good metric, than there is no problem with dropping more to fees no?
19:35:33 <S3RK> I'll try to create some interesting test scenarios. Including one where current coin selectio fails
19:35:47 <Murch[m]> Mh, I guess the fees would still be lower overall on the BnB solution in what I describe, but you'd not get the benefit of consolidating 100 UTXOs, so it feels like you paid more for less
19:36:27 <S3RK> the benefit of consolidating only works if the fees are lower than "long term fee"
19:36:33 <S3RK> if that's the case that would lower the waste
19:36:48 <S3RK> i.e. consolidating value is counted in waste
19:37:39 <S3RK> achow101 Murch what would be a good place for such tests?
19:37:40 <achow101> if fees are high, the 100 utxos might have a larger fee than the bnb excess
19:38:04 <S3RK> coinselection_test or some new/existing functional test?
19:38:07 <Murch[m]> yes, to both
19:38:17 <Murch[m]> but it still feels a bit icky, if you know what I mean
19:38:29 <S3RK> yes :)
19:38:34 <achow101> S3RK: usually rpc_fundrawtransaction
19:38:46 <achow101> and coinselection_tests
19:40:01 <S3RK> so in conclusion 1) I work on the tests
19:40:14 <S3RK> achow101 could run simulations
19:40:38 <S3RK> and you can also take a look at the PR
19:40:58 <achow101> yes
19:41:04 <Murch[m]> Yes
19:41:14 <S3RK> thanks
19:42:16 <achow101> any other topics?
19:42:40 <S3RK> congrats, it looks like you'll be wallet maintainer soon :)
19:42:53 <prayank> achow101: yes
19:44:49 <achow101> prayank: you have a topic?
19:45:10 <prayank> Not sure if this can be discussed. Wanted to understand why isnt this better approach? https://github.com/bitcoin/bitcoin/pull/22776
19:45:46 <prayank> RPC would respond with balance and getbalances already does that but we add a condition so parameter
19:45:53 <achow101> #topic add optional transaction(s) to getbalances (prayank)
19:45:53 <core-meetingbot> topic: add optional transaction(s) to getbalances (prayank)
19:46:07 <achow101> prayank: because such an RPC can be expanded to be much more than just balance fetching
19:46:17 <prayank> like?
19:47:01 <achow101> the reason I asked for simulaterawtransaction is because we can do things like seeing if a transaction has conflicts with the wallet, and if it does, what gets replaced
19:47:51 <prayank> hmm
19:48:25 <prayank> I liked the idea of having such params in few other RPC to simulate things
19:48:33 <prayank> You don't like the idea to add similar parameter in getmempool and see how mempool looks if some txs get added?
19:48:43 <achow101> IMO it's better to have simulating things in one spot, rather than spread in many different places
19:48:57 <prayank> yes its one param
19:49:08 <achow101> prayank: no, I think that should be its own RPC
19:49:42 <achow101> I don't think it makes sense to have a fetching function do simulation things
19:49:47 <prayank> ah okay then I guess we have different opinion. No problem. But the wallet conflicts thing makes sense.
19:51:17 <achow101> anything else?
19:51:40 <prayank> No. Thanks for sharing your opinion.
19:52:02 <achow101> #endmeeting