Don't trim HTLCs when calculating the commit tx fee including the fee spike multiple#4574
Draft
tankyleo wants to merge 11 commits intolightningdevkit:mainfrom
Draft
Don't trim HTLCs when calculating the commit tx fee including the fee spike multiple#4574tankyleo wants to merge 11 commits intolightningdevkit:mainfrom
tankyleo wants to merge 11 commits intolightningdevkit:mainfrom
Conversation
As a result, we now validate that both commitments retain at least one output under the new funding scope, which is crucial for zero-reserve channels.
We previously determined this value by subtracting the htlcs, the anchors, and the commitment transaction fee. This ignored the reserve, as well as the at-least-one-output requirement in zero-reserve channels. This new field now accounts for both of these constraints. It can be seen as the total spliceable balance from the channel.
This is equivalent to the previous commit, see the debug assertions added in the previous commit. We now also get to communicate the exact maximum back to the user, instead of some "balance is lower than our reserve" message, which is hard to react to.
As a channel fundee, we previously could send HTLCs such that a fee spike in a legacy channel triggered the no-outputs case in zero-reserve channels. We now also maintain a buffer from the no-outputs case when we are the channel fundee.
We previously accounted for HTLC trims at the spiked feerate when calculating the reserved commitment transaction fees. This could cause an underestimate of the real current commitment fee at the current channel feerate. This is because a 2x increase in the feerate could trim enough HTLCs to result in a smaller commitment transaction fee. Also, the previous code only reserved the fee for an exact 2x increase in the feerate, instead of reserving the fee for any increase in the feerate between 1x to 2x. Fixes lightningdevkit#4563.
Focus on reserved commitment transaction fees, and available capacity in the 0-reserve, fundee case.
In the next commit, we will make changes to how the fee spike buffer is calculated which require the real feerate to always be passed to `tx_builder::get_next_commitment_stats`, even in the case where we include a fee spike multiple.
We previously accounted for HTLC trims at the spiked feerate when calculating the commitment transaction fee including the fee spike multiple. This only ensured that the funder of the channel could afford the commitment transaction fee for an exact 2x increase in the feerate. Now, we check that the funder can cover any increase in the feerate between 1x to 2x.
Make sure it correctly does not trim HTLCs at the spiked feerate, see the previous commit for further details.
|
👋 Hi! I see this is a draft PR. |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #4574 +/- ##
==========================================
+ Coverage 87.07% 87.09% +0.02%
==========================================
Files 161 161
Lines 109248 109332 +84
Branches 109248 109332 +84
==========================================
+ Hits 95127 95228 +101
+ Misses 11640 11621 -19
- Partials 2481 2483 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Based on #4550, draft until that PR goes in, otherwise good for review