p2pderivatives / rust-bitcoin-coin-selection

10 stars 5 forks source link

BnB implementation does not account for fees #17

Closed murchandamus closed 7 months ago

murchandamus commented 1 year ago

The BnB algorithm in Bitcoin Core aims to create a changeless solution. It must pick an input set that can pay for the outputs and all fees for the transaction.

It seems to me that the BnB implementation in this repository uses the value of the UTXOs and not the effective value. The effective value is the remaining value of a UTXO after deducting the fee for the input at the current feerate, and is therefore always feerate specific. Using the effective values of the UTXOs allows the coin selection to automatically account for the fees of the inputs while selecting and deselecting inputs to combine input sets: whatever has been picked, the inputs have paid for themselves already, while the remaining fees of the transaction can be included in the selection target.

As “fee” or “feerate” does not appear in the implementation so far, it seems to me that the BnB implementation here would select funds to meet the target but the resulting input set can be insufficient to pay sufficient transaction fees in a resulting transaction. Since there is no change output, this would lead to a failed transaction building attempt.

I would suggest to add a test that requires building an input set at a slightly higher transaction fee, e.g. 50 ṩ/vB.

yancyribbens commented 9 months ago

closed by https://github.com/p2pderivatives/rust-bitcoin-coin-selection/pull/28

yancyribbens commented 8 months ago

@murchandamus I'm ready to cut a new release. I am wondering if you could check https://github.com/p2pderivatives/rust-bitcoin-coin-selection/pull/30 to make sure this closes the issue you opened. Thanks!

yancyribbens commented 7 months ago

closed by https://github.com/p2pderivatives/rust-bitcoin-coin-selection/pull/30