search

bitcoindevkit / coin-select

Tool to help you select inputs for making bitcoin transactions.
Other
13 stars 7 forks source link

Option to calculate fee based on rounded-up vbytes #26

Open jp1ac4 opened 4 months ago

jp1ac4 commented 4 months ago

When targeting a fee rate in sat/vb, could there be the option to calculate the final fee based on the rounded-up vbytes, i.e. vb = (wu / 4).ceil().

Currently, when using the rounded-up vbytes to calculate the fee rate after using coin selection, we can end up with a transaction paying a slightly lower value than our target.

I think it might be enough to modify the method CoinSelector::implied_fee_from_feerate so that it can optionally round up the vbytes. It should be sufficient to do the rounding up at the end once the selection is complete, and not for intermediate calculations, e.g. when determining the cost of adding a single input, as we don't want to round up once for each input.

Perhaps this option could be set as part of TargetFee.

If you think this would be fine to add, I'd be happy to work on it. Thanks.

jp1ac4 commented 1 week ago

I'll close this for now as we'll probably follow the same approach of not rounding up vbytes when calculating the fee rate.

murchandamus commented 1 week ago

I’m not entirely sure how this issue came to be in my notifications, but wanted to point out that Bitcoin Core always uses vsize in mempool admittance.

So, if you were to calculate the fee for a transaction with 497 weight units at 1.3 sat/vB, you might calculate that you need to pay an absolute fee of 161.525 sats and round that up to 162 sats. However, Bitcoin Core would evaluate the transaction’s feerate as being 162 sats/125 vB = 1.296 sat/vB.

jp1ac4 commented 1 week ago

Thanks for pointing that out. I'll reopen this issue then as it seems further consideration is required.

I think it would make sense to have the option to follow the same feerate logic as used in Bitcoin Core.

LLFourn commented 17 hours ago

Thanks @jp1ac4 we should definitely fix this. It should just do what core does. I agree that the main fix is going to be in implied_fee.

PS @murchandamus maybe you are subscribed to the repo. If not please do and drop in and correct us in the future.