Skip to content

Avoid splice checks when responding to stfu#4630

Open
wpaulino wants to merge 1 commit into
lightningdevkit:mainfrom
wpaulino:avoid-splice-check-non-initiator-stfu
Open

Avoid splice checks when responding to stfu#4630
wpaulino wants to merge 1 commit into
lightningdevkit:mainfrom
wpaulino:avoid-splice-check-non-initiator-stfu

Conversation

@wpaulino
Copy link
Copy Markdown
Contributor

Only gate local quiescence initiation on splice RBF eligibility. If the counterparty initiated quiescence first, respond with non-initiator stfu once pending channel updates are clear.

Only gate local quiescence initiation on splice RBF eligibility. If the
counterparty initiated quiescence first, respond with non-initiator stfu
once pending channel updates are clear.
@wpaulino wpaulino added this to the 0.3 milestone May 20, 2026
@wpaulino wpaulino requested a review from jkczyz May 20, 2026 23:15
@wpaulino wpaulino self-assigned this May 20, 2026
@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented May 20, 2026

👋 Thanks for assigning @jkczyz 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.

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

I've now thoroughly reviewed every hunk in this diff. Let me trace through the key scenario to verify correctness:

Scenario: Node 0 has a pending quiescent_action (Splice at floor feerate) but can't initiate because RBF feerate is too low. Node 1 sends stfu.

Old code path (buggy):

  1. stfu() handler sets REMOTE_STFU_SENT, calls try_send_stfu()
  2. try_send_stfu() checks quiescent_action → finds Splice → RBF feerate check fails → returns None
  3. Node 0 never responds to Node 1's stfu — stuck

New code path (fixed):

  1. stfu() handler sets REMOTE_STFU_SENT, calls try_send_stfu()
  2. try_send_stfu() checks pending updates (passes), then checks is_remote_stfu_sent() → true → enters non-initiator path
  3. Clears REMOTE_STFU_SENT, sets QUIESCENT, returns Stfu { initiator: false }
  4. quiescent_action stays pending for retry after quiescence ends

The fix is correct. The guard at line 14624 guarantees the else branch is unreachable (if is_remote_stfu_sent() is false, then quiescent_action.is_some() must be true). The test exercises exactly this scenario.

No issues found.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 21, 2026

Codecov Report

❌ Patch coverage is 80.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.63%. Comparing base (ee456a8) to head (dbb1c96).

Files with missing lines Patch % Lines
lightning/src/ln/channel.rs 80.00% 3 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4630       +/-   ##
===========================================
+ Coverage   28.02%   86.63%   +58.61%     
===========================================
  Files         126      159       +33     
  Lines       69960   110500    +40540     
  Branches    69960   110500    +40540     
===========================================
+ Hits        19606    95735    +76129     
+ Misses      49020    12230    -36790     
- Partials     1334     2535     +1201     
Flag Coverage Δ
fuzzing-fake-hashes 6.62% <0.00%> (-0.01%) ⬇️
fuzzing-real-hashes 23.25% <0.00%> (-0.01%) ⬇️
tests 86.24% <80.00%> (?)

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.

Comment on lines +14651 to +14652
// Since we may have also attempted to initiate quiescence but the counterparty
// initiated first, we'll retry after we're no longer quiescent.
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

IIUC, we would attempt to contribute as an acceptor, assuming we are both initiating for a splice.

@ldk-reviews-bot
Copy link
Copy Markdown

👋 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.

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