Skip to content

Enforce minimum RBF feerate on counterparty tx_init_rbf#4569

Merged
wpaulino merged 2 commits intolightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate
Apr 15, 2026
Merged

Enforce minimum RBF feerate on counterparty tx_init_rbf#4569
wpaulino merged 2 commits intolightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate

Conversation

@jkczyz
Copy link
Copy Markdown
Contributor

@jkczyz jkczyz commented Apr 14, 2026

When RBF-ing a splice transaction, the spec requires each tx_init_rbf to bump the feerate by at least 25/24 of the previous feerate. At low feerates, this multiplicative rule alone can produce absolute fee increases too small for Bitcoin Core to relay the replacement under BIP125's minimum relay fee policy. lightning/bolts#1327 addresses this by adding an additive floor of +25 sat/kwu alongside the multiplicative rule, using whichever produces a higher feerate.

We already enforce this combined rule on our own RBF attempts. This enforces it on counterparty tx_init_rbf messages too, by reusing the existing min_rbf_feerate function in validate_tx_init_rbf.

Also fixes the 25/24 computation to use floor division per the spec's "rounded down" wording. The prior div_ceil could reject valid counterparty feerates by 1 sat/kwu when prev * 25 is not evenly divisible by 24.

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Apr 14, 2026

👋 Thanks for assigning @wpaulino as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@jkczyz jkczyz requested a review from wpaulino April 14, 2026 23:05
Comment thread lightning/src/ln/splicing_tests.rs Outdated
// Acceptor-side: a counterparty feerate that satisfies the spec's 25/24 rule (264) is
// accepted, even though our own RBF floor (+25 sat/kwu = 278) is higher.
// Acceptor-side: a counterparty feerate that only satisfies the bare 25/24 rule (264) is
// rejected — the spec requires max(prev + 25, ceil(prev * 25/24)) = 278 at low feerates.
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Nit: the comment says ceil(prev * 25/24) but the production min_rbf_feerate now uses floor division (* 25 / 24). Should read floor(prev * 25/24).

(The numeric result is the same here since the flat increment dominates at 253 sat/kwu, but the comment is misleading about the rounding direction.)

Suggested change
// rejected — the spec requires max(prev + 25, ceil(prev * 25/24)) = 278 at low feerates.
// rejected — the spec requires max(prev + 25, floor(prev * 25/24)) = 278 at low feerates.

@ldk-claude-review-bot
Copy link
Copy Markdown
Collaborator

ldk-claude-review-bot commented Apr 14, 2026

I've thoroughly reviewed the entire diff. The changes are correct:

  1. min_rbf_feerate function (channel.rs:6780): The switch from div_ceil to floor division (* 25 / 24) correctly matches the spec's "rounded down" wording. No overflow risk — u32::MAX * 25 fits in u64.

  2. validate_tx_init_rbf (channel.rs:13010-13017): Correctly replaces the inline new * 24 < prev * 25 check (which was equivalent to ceil(prev*25/24)) with min_rbf_feerate() which enforces both the floor-division 25/24 rule and the flat +25 BIP125 floor. The FeeRate comparison semantics are correct.

  3. Tests: Both the low-feerate (prev=253) and high-feerate (prev=1000) test cases exercise the right boundary values and assert the correct accept/reject behavior.

  4. Doc/comment updates: All accurate, including the 10,417 → 10,416 fix at line 6256.

No new issues beyond my prior review comment (line 4604 nit about floor(...) notation).

No new issues found. The only issue from this review was already posted in a prior pass (lightning/src/ln/splicing_tests.rs:4604 — comment should say floor(prev * 25/24) instead of prev * 25/24 for clarity on rounding direction).

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 87.01%. Comparing base (2adb690) to head (154f06b).
⚠️ Report is 59 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4569      +/-   ##
==========================================
- Coverage   87.09%   87.01%   -0.09%     
==========================================
  Files         163      163              
  Lines      108856   109002     +146     
  Branches   108856   109002     +146     
==========================================
+ Hits        94808    94844      +36     
- Misses      11563    11674     +111     
+ Partials     2485     2484       -1     
Flag Coverage Δ
fuzzing 38.18% <0.00%> (-2.02%) ⬇️
tests 86.11% <100.00%> (-0.10%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

jkczyz and others added 2 commits April 14, 2026 20:46
The spec's tx_init_rbf recipient requirements now mandate rejecting a
feerate below max(prev + 25 sat/kwu, ceil(prev * 25/24)), matching the
sender requirement. Previously we only enforced the 25/24 rule on
counterparties. Reuse the existing min_rbf_feerate function for both
our own and counterparty validation.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The spec says the 25/24 multiplicative feerate is "rounded down", but
min_rbf_feerate used ceiling division. This made the computed minimum 1
sat/kwu too high when prev * 25 is not evenly divisible by 24, which
could reject valid counterparty feerates.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jkczyz jkczyz force-pushed the 2026-04-enforce-bip125-feerate branch from 9ce9405 to 154f06b Compare April 15, 2026 01:58
@wpaulino wpaulino merged commit 6573d42 into lightningdevkit:main Apr 15, 2026
24 of 25 checks passed
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