Include excess counterparty commitment transaction fees in dust exposure #3045
Conversation
This file contains 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
In most cases we already call both in a pair, and in fact always consolidate some of the returned values across both accessors, so there's not much reason to have them be separate methods. Here we merge them.
The previous commit left indentation in `get_pending_htlc_stats` deliberately wrong, to ease reviewability. This commit fixes the indentation.
As LDK changes, `UserConfig::default()` may imply marginally different behavior, whereas `test_default_channel_config` is intended to tweak defaults to provide a stable behavior for test contexts. This commit changes a few uses of `UserConfig::default()` to `test_default_channel_config` in cases that will fail over the coming commits due to dust changes.
Transaction fees on counterparty commitment transactions are ultimately not our money and thus are really "dust" from our PoV - they're funds that may be ours during off-chain updates but are not ours once we go on-chain. Thus, here, we count any such fees in excess of our own fee estimates towards dust exposure. We don't bother to make an inbound/outbound channel distinction here as in most cases users will use `MaxDustExposure::FeeRateMultiplier` which will scale with the fee we set on outbound channels anyway. Note that this also enables the dust exposure checks on anchor channels during feerate updates. We'd previously elided these as increases in the channel feerates do not change the HTLC dust exposure, but now do for the fee dust exposure.
Now that we're including excess counterparty commitment transaction fees in our dust calculation, we need to update the docs accordingly. We do so here, describing some of the considerations and risks that come with the new changes. We also take this opportunity to double the default value, as users have regularly complained that non-anchor channels fail to send HTLCs with the default settings with some feerates. Fixes lightningdevkit#2922
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #3045 +/- ##
==========================================
+ Coverage 89.16% 90.65% +1.48%
==========================================
Files 118 118
Lines 97726 102485 +4759
Branches 97726 102485 +4759
==========================================
+ Hits 87142 92904 +5762
+ Misses 8345 7135 -1210
- Partials 2239 2446 +207 ☔ View full report in Codecov by Sentry. |
|
Fuzz tests failing 😢 |
|
Grr, its just the stupid no_existing_breakage test. I'll work on it but IMO we can merge it with that broken, cut an RC, and then fix for release. |
jkczyz
approved these changes
May 7, 2024
dunxen
approved these changes
May 7, 2024
TheBlueMatt
added a commit
to TheBlueMatt/rust-lightning
that referenced
this pull request
May 7, 2024
…-are-dust Include excess counterparty commitment transaction fees in dust exposure
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.
Transaction fees on counterparty commitment transactions are
ultimately not our money and thus are really "dust" from our PoV -
they're funds that may be ours during off-chain updates but are not
ours once we go on-chain.
Thus, here, we count any such fees in excess of our own fee
estimates towards dust exposure. We don't bother to make an
inbound/outbound channel distinction here as in most cases users
will use
MaxDustExposure::FeeRateMultiplierwhich will scalewith the fee we set on outbound channels anyway.
Note that this also enables the dust exposure checks on anchor
channels during feerate updates. We'd previously elided these as
increases in the channel feerates do not change the HTLC dust
exposure, but now do for the fee dust exposure.
Also updates our default limit, fixing #2922