Skip to content

Ensure minimum RBF feerate satisfies BIP125#4494

Open
jkczyz wants to merge 2 commits intolightningdevkit:mainfrom
jkczyz:2026-03-rbf-feerate
Open

Ensure minimum RBF feerate satisfies BIP125#4494
jkczyz wants to merge 2 commits intolightningdevkit:mainfrom
jkczyz:2026-03-rbf-feerate

Conversation

@jkczyz
Copy link
Copy Markdown
Contributor

@jkczyz jkczyz commented Mar 18, 2026

The spec's 25/24 multiplier doesn't always satisfy BIP125's relay requirement of an absolute fee increase at low feerates, while a flat +25 sat/kwu increment falls below the spec's 25/24 rule above 600 sat/kwu. Use max(prev + 25, ceil(prev * 25/24)) for our own RBFs to satisfy both constraints, while still accepting the bare 25/24 rule from counterparties.

Potential spec change in lightning/bolts#1327.

@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Mar 18, 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.

The spec's 25/24 multiplier doesn't always satisfy BIP125's relay
requirement of an absolute fee increase. Use a flat +25 sat/kwu
(0.1 sat/vB) increment for our own RBFs instead, while still
accepting the 25/24 rule from counterparties.

Extract a `min_rbf_feerate` helper to consolidate the two call sites
and add a test that a counterparty feerate satisfying 25/24 (but not
+25) is accepted.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jkczyz jkczyz force-pushed the 2026-03-rbf-feerate branch from 1098cc3 to 9b9c114 Compare March 30, 2026 22:08
@jkczyz jkczyz marked this pull request as ready for review March 30, 2026 22:15
@jkczyz
Copy link
Copy Markdown
Contributor Author

jkczyz commented Mar 30, 2026

Will need for proper integration testing of LDK Node's rbf_channel API.

@jkczyz jkczyz requested review from TheBlueMatt and wpaulino and removed request for valentinewallace March 30, 2026 22:19
Comment on lines +6487 to +6495
/// Returns the minimum feerate for our own RBF attempts given a previous feerate.
///
/// The spec (tx_init_rbf) requires the new feerate to be >= 25/24 of the previous feerate.
/// However, that multiplier doesn't always satisfy BIP125's relay requirement of an absolute fee
/// increase, so for our own RBFs we use a flat +25 sat/kwu (0.1 sat/vB) increment instead. We
/// still accept the 25/24 rule from counterparties in [`FundedChannel::validate_tx_init_rbf`].
fn min_rbf_feerate(prev_feerate: u32) -> FeeRate {
FeeRate::from_sat_per_kwu((prev_feerate as u64).saturating_add(25))
}
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.

Bug: For prev_feerate > 600, this returns a value below the spec's 25/24 rule that counterparties enforce in validate_tx_init_rbf (line 12719). This means our own RBF attempts will be rejected by any spec-compliant counterparty at higher feerates.

Example at prev_feerate = 1000:

  • min_rbf_feerate returns 1025
  • Counterparty checks 1025 * 24 = 24600 < 1000 * 25 = 25000rejected

The crossover is at exactly 600 sat/kwu (25 * 24 = 600). Above that, +25 is insufficient to satisfy >= ceil(prev * 25/24). Real-world feerates are often well above 600 sat/kwu.

Additionally, maybe_adjust_for_rbf (line 12172) adjusts contributions to this floor, so automated feerate adjustment will also produce rejected RBFs.

Consider using max(prev + 25, ceil(prev * 25/24)) to satisfy both the BIP125-motivated flat increment and the spec's multiplicative rule:

Suggested change
/// Returns the minimum feerate for our own RBF attempts given a previous feerate.
///
/// The spec (tx_init_rbf) requires the new feerate to be >= 25/24 of the previous feerate.
/// However, that multiplier doesn't always satisfy BIP125's relay requirement of an absolute fee
/// increase, so for our own RBFs we use a flat +25 sat/kwu (0.1 sat/vB) increment instead. We
/// still accept the 25/24 rule from counterparties in [`FundedChannel::validate_tx_init_rbf`].
fn min_rbf_feerate(prev_feerate: u32) -> FeeRate {
FeeRate::from_sat_per_kwu((prev_feerate as u64).saturating_add(25))
}
fn min_rbf_feerate(prev_feerate: u32) -> FeeRate {
let flat_increment = (prev_feerate as u64).saturating_add(25);
let spec_increment = ((prev_feerate as u64) * 25).div_ceil(24);
FeeRate::from_sat_per_kwu(cmp::max(flat_increment, spec_increment))
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Using max now which also matches the alternative spec proposal.

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

ldk-claude-review-bot commented Mar 30, 2026

No issues found.

All three issues from my prior review have been addressed in this revision:

  • min_rbf_feerate now uses max(prev + 25, ceil(prev * 25/24)), correctly satisfying both BIP125 relay and the spec's 25/24 rule at all feerates.
  • Doc comments on FundingTemplate accurately describe the dual-rule formula.
  • Test comment at splicing_tests.rs:6050-6051 correctly shows the max() computation at 10,000 sat/kwu.

The new test test_splice_rbf_at_high_feerate properly exercises the crossover region (1000 sat/kwu, where 25/24 dominates). The new acceptor-side test block in test_splice_rbf_insufficient_feerate correctly validates that a counterparty feerate satisfying only the bare 25/24 rule (264) is accepted even though our own floor is higher (278).

@jkczyz jkczyz changed the title Use +25 sat/kwu increment for our minimum RBF feerate Ensure minimum RBF feerate satisfies BIP125 Mar 30, 2026
@jkczyz jkczyz force-pushed the 2026-03-rbf-feerate branch from a18dac2 to a8d3bda Compare March 30, 2026 22:52
Comment on lines +6050 to +6051
// Node 1 initiates a splice at a HIGH feerate (10,000 sat/kwu). The minimum RBF feerate will be
// 25/24 of 10,000 = 10,417 sat/kwu — far above what node 0's tight budget can handle.
// 10,000 + 25 = 10,025 sat/kwu — far above what node 0's tight budget can handle.
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: This comment is incorrect. At 10,000 sat/kwu, the 25/24 rule dominates over +25:

  • min_rbf_feerate(10000) = max(10025, ceil(10000 * 25/24)) = max(10025, 10417) = 10417

The comment was mechanically updated from "25/24 of 10,000 = 10,417" to "+25 = 10,025" but the crossover point is 600 sat/kwu, and 10,000 is well above it.

Suggested change
// Node 1 initiates a splice at a HIGH feerate (10,000 sat/kwu). The minimum RBF feerate will be
// 25/24 of 10,000 = 10,417 sat/kwu — far above what node 0's tight budget can handle.
// 10,000 + 25 = 10,025 sat/kwu — far above what node 0's tight budget can handle.
// Node 1 initiates a splice at a HIGH feerate (10,000 sat/kwu). The minimum RBF feerate will be
// max(10,000 + 25, ceil(10,000 * 25/24)) = 10,417 sat/kwu — far above what node 0's tight budget can handle.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 30, 2026

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 86.20%. Comparing base (8d00139) to head (a8d3bda).
⚠️ Report is 108 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/channel.rs 90.90% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4494      +/-   ##
==========================================
- Coverage   86.24%   86.20%   -0.04%     
==========================================
  Files         160      161       +1     
  Lines      107909   108611     +702     
  Branches   107909   108611     +702     
==========================================
+ Hits        93061    93625     +564     
- Misses      12212    12351     +139     
+ Partials     2636     2635       -1     
Flag Coverage Δ
tests 86.20% <94.44%> (-0.04%) ⬇️

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.

The flat +25 sat/kwu increment falls below the spec's 25/24
multiplicative rule for feerates above 600 sat/kwu, which would cause
our RBF attempts to be rejected by spec-compliant counterparties.
Take the max of both to satisfy BIP125 relay requirements at low
feerates and the spec rule at higher feerates.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@jkczyz jkczyz force-pushed the 2026-03-rbf-feerate branch from a8d3bda to f56d3c8 Compare March 31, 2026 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: No status

Development

Successfully merging this pull request may close these issues.

3 participants