Skip to content

Conversation

@laanwj
Copy link
Member

@laanwj laanwj commented Jul 1, 2016

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.

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.
@laanwj laanwj added the Wallet label Jul 1, 2016
@maflcko
Copy link
Member

maflcko commented Jul 1, 2016 via email

@laanwj
Copy link
Member Author

laanwj commented Jul 1, 2016

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.

@maflcko maflcko added this to the 0.13.0 milestone Jul 1, 2016
@maflcko
Copy link
Member

maflcko commented Jul 1, 2016

utACK 20f3cd7

@petertodd
Copy link
Contributor

utACK 20f3cd7

@RHavar
Copy link
Contributor

RHavar commented Jul 6, 2016

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.

@gmaxwell
Copy link
Contributor

gmaxwell commented Jul 6, 2016

@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

@RHavar
Copy link
Contributor

RHavar commented Jul 6, 2016

In that case, I think (temporarily?) reverting it is the way to go. The behavior on 0.12 was definitely a big regression =)

@laanwj laanwj merged commit 20f3cd7 into bitcoin:master Jul 6, 2016
laanwj added a commit that referenced this pull request Jul 6, 2016
20f3cd7 wallet: Revert input selection post-pruning (Wladimir J. van der Laan)
codablock pushed a commit to codablock/dash that referenced this pull request Dec 28, 2017
20f3cd7 wallet: Revert input selection post-pruning (Wladimir J. van der Laan)
andvgal pushed a commit to energicryptocurrency/gen2-energi that referenced this pull request Jan 6, 2019
20f3cd7 wallet: Revert input selection post-pruning (Wladimir J. van der Laan)
@bitcoin bitcoin locked as resolved and limited conversation to collaborators Sep 8, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants