-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
multi: cap anchors feerate at configurable maximum #4855
multi: cap anchors feerate at configurable maximum #4855
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As this is scheduled to land last, we can also use this PR to flip to zero-htlc fee anchors by default, and add the logic to reject the legacy anchors type (assuming we still need to advertise it), and also set up the proper feature dep.
fundingmanager.go
Outdated
|
||
// For anchor channels cap the initial commit fee rate at our defined | ||
// maximum. | ||
if commitType == lnwallet.CommitmentTypeAnchors && |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this'll need to check a new bit once the zero fee changes are in.
8f706f1
to
e918469
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good! only one small nit
math.Max(maxFeeRate, float64(chainfee.FeePerKwFloor)), | ||
) | ||
|
||
// Cap anchor fee rates. | ||
if lc.channelState.ChanType.HasAnchors() && feeRate > maxAnchorFeeRate { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
above we use commitType == lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx
, should that match here? otherwise the maximum will also apply to anchor_outputs
channels?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, see this commit in the zero htlc fee PR: 7b0212f#diff-e308f199858d836b06a0c920d2dd019fbb3acb7ef0d9e3d8a360601623128d78R322
So we'll always set both the anchors bit and the zero fee htlc bit if ommitType == lnwallet.CommitmentTypeAnchorsZeroFeeHtlcTx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Correct that this'll apply to "regular" anchors as well, though with the dependent PR we essentially don't advertise it at all from teh perspective of the public network.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it will also apply the the existing few legacy anchor channels, but I think that's okay.
// DefaultAnchorsCommitMaxFeeRateSatPerVByte is the default max fee rate in | ||
// sat/vbyte the initiator will use for anchor channels. This should be enough | ||
// to ensure propagation before anchoring down the commitment transaction. | ||
const DefaultAnchorsCommitMaxFeeRateSatPerVByte = 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very char. much wow
e918469
to
7aac569
Compare
This commit caps the update fee the initiator will send when the anchors channel type is used. We do not limit anything on the receiver side. 10 sat/vbyte is the current default max fee rate we use. This should be enough to ensure propagation before anchoring down the commitment transaction.
60434d3
to
413ece2
Compare
Needs a rebase! |
Rebased |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 🐋
This commit caps the update fee the initiator will send when the anchors
channel type is used. We do not limit anything on the receiver side.
10 sat/vbyte is the current default max fee rate we use. This should be
enough to ensure propagation before anchoring down the commitment
transaction.
Replaces #4795