Enforce minimum RBF feerate on counterparty tx_init_rbf - #4569

Merged
wpaulino merged 2 commits into
lightningdevkit: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 into
lightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate

Conversation

@jkczyz

Copy link
Copy Markdown
Contributor

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

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 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 wpaulinoApril 14, 2026 23:05
Comment threadlightning/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

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

Copy link
Copy Markdown
Collaborator

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

codecovBot commented Apr 15, 2026

Copy link
Copy Markdown

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 
FlagCoverage Δ
fuzzing38.18% <0.00%> (-2.02%)⬇️
tests86.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.

jkczyzand 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
jkczyzforce-pushed the 2026-04-enforce-bip125-feerate branch from 9ce9405 to 154f06bCompareApril 15, 2026 01:58
@wpaulino
wpaulino merged commit 6573d42 into lightningdevkit:mainApr 15, 2026
24 of 25 checks passed
@jkczyzjkczyz mentioned this pull request Jun 15, 2026
50 tasks
Sign up for freeto 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

@jkczyz@ldk-reviews-bot@ldk-claude-review-bot@wpaulino
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Enforce minimum RBF feerate on counterparty tx_init_rbf - #4569

Merged
wpaulino merged 2 commits into
lightningdevkit: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 into
lightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate

Conversation

@jkczyz

Copy link
Copy Markdown
Contributor

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

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 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 wpaulinoApril 14, 2026 23:05
Comment threadlightning/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

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

Copy link
Copy Markdown
Collaborator

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

codecovBot commented Apr 15, 2026

Copy link
Copy Markdown

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 
FlagCoverage Δ
fuzzing38.18% <0.00%> (-2.02%)⬇️
tests86.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.

jkczyzand 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
jkczyzforce-pushed the 2026-04-enforce-bip125-feerate branch from 9ce9405 to 154f06bCompareApril 15, 2026 01:58
@wpaulino
wpaulino merged commit 6573d42 into lightningdevkit:mainApr 15, 2026
24 of 25 checks passed
@jkczyzjkczyz mentioned this pull request Jun 15, 2026
50 tasks
Sign up for freeto 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

@jkczyz@ldk-reviews-bot@ldk-claude-review-bot@wpaulino
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enforce minimum RBF feerate on counterparty tx_init_rbf - #4569

Merged
wpaulino merged 2 commits into
lightningdevkit: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 into
lightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate

Conversation

@jkczyz

Copy link
Copy Markdown
Contributor

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

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 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 wpaulinoApril 14, 2026 23:05
Comment threadlightning/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

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

Copy link
Copy Markdown
Collaborator

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

codecovBot commented Apr 15, 2026

Copy link
Copy Markdown

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 
FlagCoverage Δ
fuzzing38.18% <0.00%> (-2.02%)⬇️
tests86.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.

jkczyzand 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
jkczyzforce-pushed the 2026-04-enforce-bip125-feerate branch from 9ce9405 to 154f06bCompareApril 15, 2026 01:58
@wpaulino
wpaulino merged commit 6573d42 into lightningdevkit:mainApr 15, 2026
24 of 25 checks passed
@jkczyzjkczyz mentioned this pull request Jun 15, 2026
50 tasks
Sign up for freeto 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

@jkczyz@ldk-reviews-bot@ldk-claude-review-bot@wpaulino
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enforce minimum RBF feerate on counterparty tx_init_rbf - #4569

Merged
wpaulino merged 2 commits into
lightningdevkit: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 into
lightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate

Conversation

@jkczyz

Copy link
Copy Markdown
Contributor

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

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 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 wpaulinoApril 14, 2026 23:05
Comment threadlightning/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

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

Copy link
Copy Markdown
Collaborator

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

codecovBot commented Apr 15, 2026

Copy link
Copy Markdown

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 
FlagCoverage Δ
fuzzing38.18% <0.00%> (-2.02%)⬇️
tests86.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.

jkczyzand 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
jkczyzforce-pushed the 2026-04-enforce-bip125-feerate branch from 9ce9405 to 154f06bCompareApril 15, 2026 01:58
@wpaulino
wpaulino merged commit 6573d42 into lightningdevkit:mainApr 15, 2026
24 of 25 checks passed
@jkczyzjkczyz mentioned this pull request Jun 15, 2026
50 tasks
Sign up for freeto 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

@jkczyz@ldk-reviews-bot@ldk-claude-review-bot@wpaulino
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Enforce minimum RBF feerate on counterparty tx_init_rbf - #4569

Merged
wpaulino merged 2 commits into
lightningdevkit: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 into
lightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate

Conversation

@jkczyz

Copy link
Copy Markdown
Contributor

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

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 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 wpaulinoApril 14, 2026 23:05
Comment threadlightning/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

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

Copy link
Copy Markdown
Collaborator

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

codecovBot commented Apr 15, 2026

Copy link
Copy Markdown

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 
FlagCoverage Δ
fuzzing38.18% <0.00%> (-2.02%)⬇️
tests86.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.

jkczyzand 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
jkczyzforce-pushed the 2026-04-enforce-bip125-feerate branch from 9ce9405 to 154f06bCompareApril 15, 2026 01:58
@wpaulino
wpaulino merged commit 6573d42 into lightningdevkit:mainApr 15, 2026
24 of 25 checks passed
@jkczyzjkczyz mentioned this pull request Jun 15, 2026
50 tasks
Sign up for freeto 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

@jkczyz@ldk-reviews-bot@ldk-claude-review-bot@wpaulino
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enforce minimum RBF feerate on counterparty tx_init_rbf - #4569

Merged
wpaulino merged 2 commits into
lightningdevkit: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 into
lightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate

Conversation

@jkczyz

Copy link
Copy Markdown
Contributor

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

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 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 wpaulinoApril 14, 2026 23:05
Comment threadlightning/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

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

Copy link
Copy Markdown
Collaborator

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

codecovBot commented Apr 15, 2026

Copy link
Copy Markdown

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 
FlagCoverage Δ
fuzzing38.18% <0.00%> (-2.02%)⬇️
tests86.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.

jkczyzand 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
jkczyzforce-pushed the 2026-04-enforce-bip125-feerate branch from 9ce9405 to 154f06bCompareApril 15, 2026 01:58
@wpaulino
wpaulino merged commit 6573d42 into lightningdevkit:mainApr 15, 2026
24 of 25 checks passed
@jkczyzjkczyz mentioned this pull request Jun 15, 2026
50 tasks
Sign up for freeto 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

@jkczyz@ldk-reviews-bot@ldk-claude-review-bot@wpaulino
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Enforce minimum RBF feerate on counterparty tx_init_rbf - #4569

Merged
wpaulino merged 2 commits into
lightningdevkit: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 into
lightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate

Conversation

@jkczyz

Copy link
Copy Markdown
Contributor

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

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 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 wpaulinoApril 14, 2026 23:05
Comment threadlightning/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

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

Copy link
Copy Markdown
Collaborator

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

codecovBot commented Apr 15, 2026

Copy link
Copy Markdown

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 
FlagCoverage Δ
fuzzing38.18% <0.00%> (-2.02%)⬇️
tests86.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.

jkczyzand 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
jkczyzforce-pushed the 2026-04-enforce-bip125-feerate branch from 9ce9405 to 154f06bCompareApril 15, 2026 01:58
@wpaulino
wpaulino merged commit 6573d42 into lightningdevkit:mainApr 15, 2026
24 of 25 checks passed
@jkczyzjkczyz mentioned this pull request Jun 15, 2026
50 tasks
Sign up for freeto 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

@jkczyz@ldk-reviews-bot@ldk-claude-review-bot@wpaulino
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Enforce minimum RBF feerate on counterparty tx_init_rbf - #4569

Merged
wpaulino merged 2 commits into
lightningdevkit: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 into
lightningdevkit:mainfrom
jkczyz:2026-04-enforce-bip125-feerate

Conversation

@jkczyz

Copy link
Copy Markdown
Contributor

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

ldk-reviews-bot commented Apr 14, 2026

Copy link
Copy Markdown

👋 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 wpaulinoApril 14, 2026 23:05
Comment threadlightning/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

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

Copy link
Copy Markdown
Collaborator

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

codecovBot commented Apr 15, 2026

Copy link
Copy Markdown

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 
FlagCoverage Δ
fuzzing38.18% <0.00%> (-2.02%)⬇️
tests86.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.

jkczyzand 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
jkczyzforce-pushed the 2026-04-enforce-bip125-feerate branch from 9ce9405 to 154f06bCompareApril 15, 2026 01:58
@wpaulino
wpaulino merged commit 6573d42 into lightningdevkit:mainApr 15, 2026
24 of 25 checks passed
@jkczyzjkczyz mentioned this pull request Jun 15, 2026
50 tasks
Sign up for freeto 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

@jkczyz@ldk-reviews-bot@ldk-claude-review-bot@wpaulino