wallet: reduce coin selection iterations #24712

Remove coin selection iteration requiring 6 confs

It's not clear what purpose we serve with this additional iteration of coin selection that restricts foreign UTXOs to at least six confirmations.

If the wallet hasn't been used in a long time, requiring six confirmations or one is equivalent. If the wallet has just received funds, the important distinction is still between confirmed and unconfirmed UTXOs: we haven't had any reorgs of more than a single block in multiple years, and the ones we had have been benign—the differences of included transactions between blocks at the same height is generally minuscule and can be explained by transaction relay latency or the rate of block template updates. The risk of a UTXO getting reorged out due to requiring a single confirmation instead of six can be discounted and doesn't suffice to instigate a full additional coin selection iteration.

If the wallet has just received funds, the important distinction is still between confirmed and unconfirmed UTXOs: we haven't had any reorgs of more than a single block in multiple years, and the ones we had have been benign—the differences of included transactions between blocks at the same height is generally minuscule and can be explained by transaction relay latency or the rate of block template updates.

How will this work in an attack shared in this tweet? https://nitter.net/AchimWar/status/1505980778458484736

The risk of a UTXO getting reorged out due to requiring a single confirmation instead of six can be discounted and doesn't suffice to instigate a full additional coin selection iteration.

I am not sure if all users will be okay with this discount. Disagree and if it ain't broke, don't fix it.

@1440000bytes: The likelihood of a situation arising in which this is relevant is so minuscule that performing the additional computation step is not justified, especially since it would only cause the transaction to revert to unconfirmed unless we are the target of a Finney attack. If we are the target of a Finney attack, the result is that we didn't get paid in the first place and we would have to redo the subsequent payment once the dust settles.

Concept ACK

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #23829 (refactor: use braced init for integer literals instead of c style casts by PastaPastaPasta)

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.

This can also happen in normal operation, absent any attack, if fee bumping becomes more common. Two competing blocks often contain the same tx with different fee rates (thus different txid). Even if it happens only once a month, I think it can be inconvenient for users to having to re-send a payment, risking double-pay if the new payment gets confirmed in a chain with the (presumed) stale block.

Concept NACK

The likelihood of a situation arising in which this is relevant is so minuscule that performing the additional computation step is not justified

This situation looks possible with governments involved in bitcoin, mining, councils etc. We have not disabled firewall in our office because there were no attacks in last few months.

We are going through lot of things that I don't want to share here as it will be offtopic but still affects bitcoin and I expect bitcoin developers to respect it.

This can also happen in normal operation, absent any attack, if fee bumping becomes more common. Two competing blocks often contain the same tx with different fee rates (thus different txid). Even if it happens only once a month, I think it can be inconvenient for users to having to re-send a payment, risking double-pay if the new payment gets confirmed in a chain with the (presumed) stale block.

This is another point and I appreciate adversarial thinking by maintainers.

tl;dr: I respect PR author's contribution in coin selection and bitcoin however I am not okay with this change

we haven't had any reorgs of more than a single block in multiple years, and the ones we had have been benign

But a change like this creates an incentive to do so.

Agree with @1440000bytes - this probably isn't a good idea.

(Maybe if we could automatically detect and handle problems?)

Agree with @1440000bytes and @luke-jr here, NACK as it is. Probably 6 could be configurable number and then user could set it to 1 if he wants to?

Okay, thanks for the feedback.