Fee calculation rounding in wallet anchor #1354
Conversation
wallet_anchor.go
Outdated
| // expects. We add 999 to round up to the nearest 1000 to prevent issues | ||
| // where the fee doesn't meet the min_relay_fee because of rounding | ||
| // down. | ||
| satPerVByte := uint64((feeRate.FeePerKVByte() + 999) / 1000) |
There was a problem hiding this comment.
How about just: uint64(math.Ceil(float64(feeRate.FeePerKVByte()) / 1000))?
There was a problem hiding this comment.
You like that more? I thought this was pretty elegant. It would have prevented an extra import if it weren't for the fact that we already use math. It also doesn't need the cast to float, which is obviously a huge performance gain. 😉
There was a problem hiding this comment.
Sure, it is elegant. But I tend to always go with the more clear and easy to understand version. IMO with the "elegant" version you need the comment above (why are we adding 999?) while math.Ceil() by definition/name says: Rounding up.
But not going to block the PR on it...
There was a problem hiding this comment.
I'm with Oli on this. I need to think less with math.Ceil. But Gij's + 999 is way more elegant. But I love to think less.
Pull Request Test Coverage Report for Build 13161173239Details
💛 - Coveralls |
There was a problem hiding this comment.
By the way, FundPsbt already supports sat_per_kw in the lnd repo. This is slated for release with v0.19. So we could remove the rounding fix after we bump tapd to lnd v0.19. lightningnetwork/lnd#9013
This commit fixes the an error with fee calculation in the wallet anchor. In cases where we pass a fee as sat/kvb we divide by a thousand because that's what `FundPsbt` expects. However, with real world values like 1952 sat/kvb, this division results in a rounded down fee of 1 sat/vbyte, whihc then can be below the min_relay_fee value. The itest has been updated to include the correct fee calculation when using sat/kvb values that would previously result in rounding errors. This ensures that the fee is always above the min_relay_fee value.
gijswijs
force-pushed
the
fee-rounding
branch
from
3f44908 to
f1ec4b8
Compare
This PR fixes an error with fee calculation in the wallet anchor. In cases where we pass a fee as sat/kvb we divide by a thousand
because that's what
FundPsbtexpects. However, with real world values like 1952 sat/kvb, this division results in a rounded down fee of 1sat/vbyte, which then can be below the min_relay_fee value.
The itest has been updated to include the correct fee calculation when using sat/kvb values that would previously result in rounding errors. This ensures that the fee is always above the min_relay_fee value.
fixes: #1171