New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wallet: Target a pre-defined utxo set composition by adjusting change outputs #29442
Conversation
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. Code CoverageFor detailed information about the code coverage, see the test coverage report. ReviewsSee the guideline for information on the review process. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
2de6a1b
to
a214548
Compare
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
b246995
to
85a049d
Compare
@murchandamus we'd love to get your feedback on this! I'll try to summarize at a higher level what we'd like to achieve. The We can think of this type of wallet as a slowly draining wallet: its total amount is linearly decreasing over time, and is occasionally topped-up from cold storage. We're not sure how to best achieve that. Our goal is to do it inside
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I really like the fact that it consists mostly of a pre-processing step before coin selection runs, followed by a post-processing step on the output of coin selection.
Should we also tweak which coin selection algorithms are run depending on whether we're trying to refill buckets (and how aggressive we'd like to be) or not? Some of them may not be well suited for that?
src/wallet/spend.cpp
Outdated
@@ -1128,6 +1225,36 @@ static util::Result<CreatedTransactionResult> CreateTransactionInternal( | |||
available_coins = AvailableCoins(wallet, &coin_control, coin_selection_params.m_effective_feerate); | |||
} | |||
|
|||
// Load a json file that describes a target utxo set |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Loading this file every time we fund a transaction doesn't seem reasonable. I think we should start with a static file that needs a restart whenever the node operator wants to change values, and we can later decide how to make this more dynamic (if it's even necessary).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense. I also think a static file could describe the buckets in a way that better adapts to a changing fee environment.
For example, rather than start_satoshis
and end_satoshi
we could define a bucket as:
"confirmation_target": 6,
"fee_rate_std": 10000,
"buckets": [
{
"target_satoshis": 10000,
"target_utxo_count": 150
},
The range of values you want to spend should be "target_satoshis" + the fee to spend that input at a fee rate that will confirm in "confirmation_target" blocks. Because free rates vary, we randomize the current fee rate within a range +/- "fee_rate_std".
To refill a bucket, compute the target_output
as:
target_feerate = current_feerate("confirmation_target") + random(-1 * "fee_rate_std", "fee_rate_std")
target_output = "target_satoshis" + size_of_input * target_feerate
Ideally we would only need to restart when we add/remove buckets, change their counts or when fee variance changes dramatically.
src/wallet/spend.cpp
Outdated
/** Returns a random change amount in the range of the most depleted Utxo bucket and sets `capacity` | ||
* to the capacity of that change target, if any. | ||
*/ | ||
std::optional<CAmount> GenerateChangeTargetFromUtxoTargets(const std::vector<UtxoTarget>& utxo_targets, const CAmount change_fee, double& capacity, FastRandomContext& rng) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this is really what we want. I think this function should try to refill multiple buckets at once (when amounts allow it), not just the most depleted one:
- Check which utxo buckets should be refilled:
- Initialize a
to_refill
list of(bucket, target_quantity)
- For each utxo bucket:
- If the feerate is low (below a to-be-defined threshold) and the bucket is less than 70% full (or a to-be-defined threshold that could be configurable):
- Add this bucket to
to_refill
with atarget_quantity
oftarget_utxo_count - current_utxo_count
(when feerate is low, we'd like to refill as much as possible)
- Add this bucket to
- If the feerate is higher than our threshold and the bucket is less than 30% full (or a to-be-defined threshold that could be configurable), we refill less aggressively:
- Add this bucket to
to_refill
with atarget_quantity
that we scale (somehow) based on feerate and how many utxos we're missing
- Add this bucket to
- If the feerate is low (below a to-be-defined threshold) and the bucket is less than 70% full (or a to-be-defined threshold that could be configurable):
- Order
to_refill
bytarget_quantity
, descending (we'll want to refill the buckets that are missing the most utxos first)
- Initialize a
- Check the utxos that we have outside of our buckets: we ideally want to spend those utxos to refill our buckets. We have no guarantee that those are the ones that will be used by the coin selection, but we can use their total amount as the maximum amount of funds we allocate to refill our buckets. This part is a bit fuzzy for me right now, I'm not sure this is the best approach, it definitely deserves more thoughts.
- Iterate over those utxos and add their amount to obtain
utxos_outside_buckets_total_amount
- Iterate over those utxos and add their amount to obtain
- Compute the change target based on
to_refill
andutxos_outside_buckets_total_amount
:- Initialize
change_target
to0
- Iterate over
to_refill
, and for each bucket that needs to be refilled:- Decide how much of the
target_quantity
we want to refill based onutxos_outside_buckets_total_amount
and the currentchange_target
- Add the corresponding amount to
change_target
- That step probably has a lot of issues as well and deserves more thought. If we have a lot of utxos outside of our buckets, we may end up targeting a very large change amount and thus create a huge transaction, which is generally undesirable, but maybe desirable when our buckets are close to being empty. Maybe it makes sense to bound the
change_target
to a (small) multiplier of the funding amount? Or have a very different behavior when the feerate is very low, because when that happens we may want to fully refill our buckets?
- Decide how much of the
- Initialize
As you can see, this is still very early discussion on what the algorithm should look like. It feels like we're still trying to understand the pitfalls we want to avoid we should try to write the algorithm in pseudo-code first in order to converge on an initial version (maybe to be detailed and discussed in a delving bitcoin post).
Also, when we don't want to actively refill buckets, we should target a changeless transaction, I'm not sure that is done right now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it is unavoidable that refilling buckets cannibalizes utxos from other buckets unless there is a pool of utxos larger than the largest target bucket to pick from. The goal though is that in the long run the system wastes as little value from fees and over payment as possible in aggregate.
This is how I think the process as implemented currently (85a049d) should work:
Because we always set our minimum change value to be at least as large as one of our target buckets that is depleted, all of the wallet utxos are either in the range of one of our target buckets or larger than one of our target buckets.
A changeless BnB solution is found when:
- there is a single utxo in the corresponding target bucket that does not overpay by too much in fees
- a combination of multiple utxos smaller than the target bucket exist that do not overpay by too much in fees
A CoinGrinder solution is preferred when the cost of fees and over payment of the best BnB solution is more than the fees from using:
- a single larger utxo with change outputs
- a combination of multiple smaller utxos and change outputs
The largest wallet utxo will hold the residual value after initially refilling the buckets from cold storage. This residual funding utxo will be used as a single input when:
- this utxo is available and CoinGrinder selects it to refill buckets opportunistically
- fees are low, or buckets are severely depleted, we force this input to be selected to refill buckets.
When a large residual value utxo is used to refill all buckets via change outputs, another large residual utxo may also be created. That residual value utxo can not be used again until the tx that uses it confirms. This seems like an area that can be optimized. Perhaps a separate automatic refill transaction that confirms quickly makes more sense than using it for opportunistic bucket refills.
Eventually the largest utxo in the wallet will be in the range of the target buckets. The utxo set will then no longer be able to refill buckets without also depleting other buckets. This should be a signal to refill the wallet from cold storage.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also, when we don't want to actively refill buckets, we should target a changeless transaction, I'm not sure that is done right now?
The current system should always prefer changeless transactions unless, for example, an exact match with more inputs and no change is more wasteful than selecting fewer inputs that generate a change output.
src/wallet/spend.cpp
Outdated
return change_target; | ||
} | ||
|
||
std::list<CTxOut> SplitChangeFromUtxoTargets(CAmount change_amount, std::vector<UtxoTarget> utxo_targets, CAmount change_target, const CAmount change_fee, FastRandomContext& rng, CScript script_change) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm wondering whether we should use a fully deterministic algorithm to split the change into buckets, I'm afraid this is exactly the kind of algorithm that may end up in a loop where one funding attempt does something, and the next one undoes it. It feels like adding randomization may be the simple way to avoid this? But maybe it's too early in the algorithm design phase to decide.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree that there's a risk that one funding operation adds an extra input from an existing bucket and then creates a change output to refill that same bucket, for example. My intuition though is that we can't optimize too far into the future.
Even if an input is cannibalized from a bucket, it will be "reforged" into a new utxo that hopefully has a value that is more likely to be selected by BnB as a single input to a future changeless funding transaction. The current scheme randomizes new change outputs and skews them based on the current fee rate to try to optimize for future changeless single input transactions.
Can we do better with some ideas from memory caching? Maybe use the frequency that a particular amount is requested for funding to influence whether it is used as an extra input or as the size of a new change output?
Yes, I think we should only run BnB and Coingrinder. Currently SRD and Knapsack are fallbacks that could win when BnB and CoinGrinder have higher waste metrics. When waste = selectionTotal - target + inputs × (currentFeeRate - longTermFeeRate) If one of the algorithms can find a solution with less overshoot than the cost of adding more (small value) inputs, then it seems like they would win. Coingrinder should always find a solution with the least input weight. When fees are high, I think we should not care if we overshoot our target. We should always prefer a changeless BnB solution over one with change. When we have to produce change, we would prefer the Coingrinder solution with minimal input weight. We will allocate change to outputs sized to favor BnB in the future. When fees are low, I think we will want to create more change to refill our target buckets, but still not favor more exact matches that use more inputs. |
I think we should consider a pluggable architecture for coin selection algorithms. It's clear that there are multiple personas using the bitcoin core wallet, e.g. merchant, customer, lightning wallets. They have different and conflicting needs. So we either will have to have a complicated code and configuration surface to reconcile them or make our code extendable. The interface of coin selection is pretty well defined, so it seems like it is a good fit for extensibility. The coin selection plug-in could receive utxo_pool and target amount as inputs (plus maybe some additional info), and return selected utxo's as result. @remyers @t-bast would something like this work in your case? |
That's an interesting suggestion. So far in our case I don't see a need to use a different coin selection algorithm, though I do think there might be value in having the ability to selectively disable SRD and knapsack. The current scheme proposed in this PR is more of a wrapper around the existing coin selection system. I can not rule out the possibility that there might exist a superior custom coin selection algorithm for this use case. If someone were to propose a different algorithm then that would certainly help motivate a pluggable coin selection architecture. |
Is the bitcoin core reference implementation the right place to try these customization/personas? |
That's a fair question - I think it depends on how widely useful this kind of customization/persona is for different wallet users. Even if it's not deemed general enough for more than a draft PR, comments will help improve the implementation in a fork and could feed back less invasive ideas to core. For example, just having a way to specify the |
🚧 At least one of the CI tasks failed. Make sure to run all tests locally, according to the Possibly this is due to a silent merge conflict (the changes in this pull request being Leave a comment here, if you need help tracking down a confusing failure. |
Add a coin control parameter to optionally force a particular change target for coin selection algorithms that result in a change output.
Add a config option to load a json file that defines a set of utxo target buckets with a value range and count. When the current utxo set for the wallet does not match or exceed the counts for the value ranges of the buckets, opportunistically split the change output produced by coin selection (if any) into additional outputs. Target buckets will be refilled from most to least depleted. The value of a new output to refill a target bucket will be randomly selected from its value range. Once all buckets are full the change output will not be further split.
When utxo targets are specified and a utxo target bucket is depleted, or fees are low, produce excess change that can be split to replenish the utxo target set. This is accomplished by including a large valued utxo as an input to coin selection.
When considering a transaction without change, the max_excess amount is how much extra value can be added to the target value and not be counted as waste.
d4aa61a
to
0d752b5
Compare
- When this is added to a funding RPC call then the waste for changeless BnB txs will be added to the selected recipient output position instead of added to fees.
🐙 This pull request conflicts with the target branch and needs rebase. |
remyers:2024-05-bnb-excess |
Replaced by simpler #30080 |
This PR is designed for the use case of a Lightning node that provides liquidity of predefined amounts via liquidity ads.
Coin selection is currently optimized to reduce the size of the utxo set and create change optimized for privacy. A liquidity provider instead needs to service multiple liquidity requests by spending confirmed utxos of known sizes.
Ideally most liquidity transactions would be funded by a single input or a small set of inputs optimized to reduce fees. To minimize the number of unconfirmed transactions, inputs should also be sized in a range where most transactions do not produce change. When change is created, it should be divided into outputs of the sizes needed so that the wallet's utxo set converges towards an ideal utxo set specified by the user.
I am opening this PR as a draft to get feedback and suggestions on the concept and my implementation to address this use case.
The algorithm described below can be implemented externally via RPC calls or directly in the wallet.
utxo targets file example:
target_utxo_count
for a bucket should be larger than the anticipated number of liquidity requests ofbucket_start_satoshis
within the expected confirmation time of a liquidity transaction.bucket_start_satoshis
tobucket_end_satoshis
should encompass expected fee variance.bucket_refill_feerate
should be set to the expected median fee rate (?).Algorithm steps
For each payment do the following:
bucket_refill_feerate
.m_min_change_target
to a value from the target bucket with the lowest current capacity.change_fee
(the fee for creating an output) + a random value in the range:bucket_start_satoshis
tobucket_end_satoshi
-change_fee
.GenerateChangeTarget()
in a hard coded range.preset_inputs
parameter and with the minimum change target from step 3.consolidatefeerate=0
configuration option should always be set so that utxos are not preemptively cosolidated. Coin selection sets the parameterm_long_term_feerate
to the walletsconsolidatefeerate
.