Avoid selecting redundant UTXOs during coin selection#2094
Open
oliv3rdrt wants to merge 1 commit into
Open
Conversation
Two things caused the coin selectors to include UTXOs that were not needed: - In select_coins_srd, once the drawn effective value reached the target the whole heap was added to the result, including smaller UTXOs that happened to be drawn before a larger one that already covered the target. Only the most valuable groups needed to reach the target are added now. - In select_random_coins the results of the different algorithms were compared by waste only, so on a tie the first one (srd) always won even when another result selected fewer inputs. Ties are now broken by the number of selected inputs. Add a test that checks srd no longer keeps redundant UTXOs.
oliv3rdrt
requested review from
ImplOfAnImpl,
anyxem and
erubboli
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
The wallet's coin selection could pick UTXOs that were not needed. It is easy to see with
config-broadcast no, where a transaction ends up spending the largest UTXO together with several smaller ones that are not required.There were two causes:
In
select_coins_srd, once the randomly drawn effective value reached the target, the whole heap was added to the result. If a small UTXO was drawn before a larger one that already covered the target, the small one was included even though it was redundant.In
select_random_coins, results from the different algorithms were compared bywasteonly. When the waste was equal (which is common), the first result (srd) always won, even if another algorithm selected fewer inputs.Fix
select_coins_srdnow adds only the most valuable groups needed to reach the target (the heap is already ordered by effective value), leaving out the redundant smaller ones.select_random_coinsnow breaks waste ties by the number of selected inputs, preferring the result with fewer inputs.Tests
Added
test_srd_solver_omits_redundant_utxos: it gives SRD one large UTXO that already covers the target plus several small ones, and asserts only the large UTXO is selected. The existing selection tests still pass.Closes #1923