lnwallet: enforce fee floor and dust-reserve adherence #1215
Conversation
Roasbeef
left a comment
Roasbeef
left a comment
There was a problem hiding this comment.
Nice! Only major comment is that we also need to bump up the reserve (before we send it to the remote party), if we detect that the value is below the dust limit.
lnwallet/fee_estimator.go
Outdated
There was a problem hiding this comment.
It shouldn't, but it doesn't hurt to check the error.
lnwallet/fee_estimator.go
Outdated
There was a problem hiding this comment.
May be worth adding a debug logging statement here for future diagnostic purposes.
lnwallet/reservation.go
Outdated
There was a problem hiding this comment.
We also need to ensure that the reserve we propose is above the dust limit. If it is, then we should increase the reserve until it's safely above the dust limit.
fundingmanager.go
Outdated
There was a problem hiding this comment.
What's the rational for this change?
lnwallet/fee_estimator.go
Outdated
There was a problem hiding this comment.
Could you fetch the RelayFee in Start() instead?
lnwallet/fee_estimator.go
Outdated
lnwallet/reservation.go
Outdated
There was a problem hiding this comment.
Re-reading BOLT#2 it looks like the reserve must be >= then both sides's dust limits.
There was a problem hiding this comment.
- So when receiving an
open_channel: make sureopen_channel.reserve >= open_channel.dustlimit && open_channel.reserve >= ourDustlimit - When sending
accept_channel: make sureaccept_channel.reserve >= accept_channel.dustlimit && accept_channel.reserve >= open_channel.dustLimit
===============================
- When sending an
openChannel: make sureopenChannel.reserve >= openChannel.dustLimit && openChannel.reserve >= ourDustlimit, if they reject because of low reserve, we must resend with higher reserve - When receiving
accept_channel: make sureaccept_channel.reserve >= accept_channel.dustlimit && accept_channel.reserve >= openChannel.dustLimit
There was a problem hiding this comment.
I believe this should be good to go now.
In this commit, we fix an issue where sometimes transactions wouldn't provide enough of a fee to be relayed by the backend node. This would especially cause issues when sweeping outputs after a contract breach, etc. Now, we'll fetch the minimum relay fee from the backend node and ensure it is set as a lower bound if the estimated fee ends up dipping below it.
3 participants
In this PR, we fix an issue where sometimes transactions wouldn't provide enough of a fee to be relayed by the backend node. This was caused due to
bitcoindno longer using the minimum relay fee when estimating fees (see bitcoin/bitcoin@fd29d3d). This would especially cause issues when sweeping outputs after a contract breach, etc. Now, we'll fetch the minimum relay fee from the backend node and ensure it is set as a lower bound if the estimated fee ends up dipping below it.We also enforce that the channel reserve is above the dust limit. The minimum channel size has also been modified to reflect this change. This allows us to avoid dust outputs from being created in the commitment transactions. This fixes some incompatibility issues between
c-lightningandlnd, due tolndnot fully being BOLT-2 compliant.Fixes #1030.