-
Notifications
You must be signed in to change notification settings - Fork 38.7k
wallet: Revert input selection post-pruning #8298
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: Revert input selection post-pruning #8298
Conversation
This reverts PR bitcoin#4906, "Coinselection prunes extraneous inputs from ApproximateBestSubset". Apparently the previous behavior of slightly over-estimating the set of inputs was useful in cleaning up UTXOs. See also bitcoin#7664, bitcoin#7657, as well as 2016-07-01 discussion on #bitcoin-core-dev IRC.
|
I think we should do simulations to see the exact effect of this feature.
But apparently it is controversial enough that it should not have been
merged in the first place, so removal for 0.13 seems fine.
|
|
Yes, apparently simulations are not clear that this was an improvement at all (see discussion in #4906). I know it's a huge burden for anyone proposing changes to coin selection, but I think it should have been proven that this is better than the status quo first before it was merged. Or maybe even that's putting the cart before the horse: there should be a list of criteria for coin-selection algorithms first, before we can evaluate what (incremental) improvements are good or bad. |
|
utACK 20f3cd7 |
|
utACK 20f3cd7 |
|
I actually disagree with reverting this, conditional to the bug here: #7664 (comment) being fixed. If that's fixed, it'll be a good thing to have kept the "extraneous inputs" around, as they'll actually come in handy. |
|
@RHavar the challenge right now is that we're pretty close to a release to undertake a serious effort at coin selection reworking; so whatever we do has the be as minimal as possible |
|
In that case, I think (temporarily?) reverting it is the way to go. The behavior on 0.12 was definitely a big regression =) |
20f3cd7 wallet: Revert input selection post-pruning (Wladimir J. van der Laan)
20f3cd7 wallet: Revert input selection post-pruning (Wladimir J. van der Laan)
20f3cd7 wallet: Revert input selection post-pruning (Wladimir J. van der Laan)
This reverts PR #4906, "Coinselection prunes extraneous inputs from ApproximateBestSubset".
Apparently the previous behavior of slightly over-estimating the set of inputs was useful in cleaning up UTXOs.
See also #7664, #7657, as well as 2016-07-01 discussion on #bitcoin-core-dev IRC.