Skip to content

Correct non-dust HTLC accounting in next_remote_commit_tx_fee_msat #3933

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

Merged

Conversation

tankyleo
Copy link
Contributor

next_remote_commit_tx_fee_msat previously mistakenly classified HTLCs with values equal to the dust limit as dust.

This did not cause any force closes because the code that builds commitment transactions for signing correctly trims dust HTLCs.

Nonetheless, this can cause next_remote_commit_tx_fee_msat to predict a weight for the next remote commitment transaction that is significantly lower than the eventual weight. This allows a malicious channel funder to create an unbroadcastable commitment for the channel fundee by adding HTLCs with values equal to the dust limit to the commitment transaction; according to the fundee, the funder has not exhausted their reserve because all the added HTLCs are dust, while in reality all the HTLCs are non-dust, and the funder does not have the funds to pay the minimum feerate to enter the mempool.

`next_remote_commit_tx_fee_msat` previously mistakenly classified HTLCs
with values equal to the dust limit as dust.

This did not cause any force closes because the code that builds
commitment transactions for signing correctly trims dust HTLCs.

Nonetheless, this can cause `next_remote_commit_tx_fee_msat` to predict
a weight for the next remote commitment transaction that is
significantly lower than the eventual weight. This allows a malicious
channel funder to create an unbroadcastable commitment for the channel
fundee by adding HTLCs with values equal to the dust limit to the
commitment transaction; according to the fundee, the funder has not
exhausted their reserve because all the added HTLCs are dust, while in
reality all the HTLCs are non-dust, and the funder does not have the
funds to pay the minimum feerate to enter the mempool.
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Jul 15, 2025

👋 I see @jkczyz was un-assigned.
If you'd like another reviewer assignment, please click here.

@tankyleo tankyleo requested a review from TheBlueMatt July 15, 2025 19:47
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@ldk-reviews-bot
Copy link

👋 The first review has been submitted!

Do you think this PR is ready for a second reviewer? If so, click here to assign a second reviewer.

@ldk-reviews-bot ldk-reviews-bot requested a review from jkczyz July 15, 2025 20:12
@ldk-reviews-bot
Copy link

✅ Added second reviewer: @jkczyz

@wpaulino wpaulino removed the request for review from jkczyz July 15, 2025 20:15
@TheBlueMatt TheBlueMatt mentioned this pull request Jul 15, 2025
@TheBlueMatt
Copy link
Collaborator

Backported in #3933

@tankyleo
Copy link
Contributor Author

tankyleo commented Jul 15, 2025

Backported in #3933

#3932 rather :)

@TheBlueMatt TheBlueMatt merged commit 212b848 into lightningdevkit:main Jul 16, 2025
26 of 28 checks passed
TheBlueMatt added a commit to TheBlueMatt/rust-lightning that referenced this pull request Jul 24, 2025
v0.1.5 - Jul 16, 2025 - "Async Path Reduction"

Performance Improvements
========================

 * `NetworkGraph`'s expensive internal consistency checks have now been
   disabled in debug builds in addition to release builds (lightningdevkit#3687).

Bug Fixes
=========

 * Pathfinding which results in a multi-path payment is now substantially
   smarter, using fewer paths and better optimizing fees and successes (lightningdevkit#3890).
 * A counterparty delaying claiming multiple HTLCs with different expiries can
   no longer cause our `ChannelMonitor` to continuously rebroadcast invalid
   transactions or RBF bump attempts (lightningdevkit#3923).
 * Reorgs can no longer cause us to fail to claim HTLCs after a counterparty
   delayed claiming multiple HTLCs with different expiries (lightningdevkit#3923).
 * Force-closing a channel while it is blocked on another channel's async
   `ChannelMonitorUpdate` can no longer lead to a panic (lightningdevkit#3858).
 * `ChannelMonitorUpdate`s can no longer be released to storage too early when
   doing async updates or on restart. This only impacts async
   `ChannelMonitorUpdate` persistence and can lead to loss of funds only in rare
   cases with `ChannelMonitorUpdate` persistence order inversions (lightningdevkit#3907).

Security
========

0.1.5 fixes a vulnerability which could allow a peer to overdraw their reserve
value, potentially cutting into commitment transaction fees on channels with a
low reserve.
 * Due to a bug in checking whether an HTLC is dust during acceptance, near-dust
   HTLCs were not counted towards the commitment transaction fee, but did
   eventually contribute to it when we built a commitment transaction. This can
   be used by a counterparty to overdraw their reserve value, or, for channels
   with a low reserve value, cut into the commitment transaction fee (lightningdevkit#3933).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants