Skip to content

Comments

Merge initial and retry stfu send paths#4432

Merged
jkczyz merged 3 commits intolightningdevkit:mainfrom
wpaulino:stfu-send-cleanups
Feb 24, 2026
Merged

Merge initial and retry stfu send paths#4432
jkczyz merged 3 commits intolightningdevkit:mainfrom
wpaulino:stfu-send-cleanups

Conversation

@wpaulino
Copy link
Contributor

In 15b04b5, we fixed a case in `FundedChannel::try_send_stfu` where we'd
send `stfu` unnecessarily for a new splice while one is already pending.
The same case also existed in `FundedChannel::send_stfu`, but was not
fixed. There's no good reason for both of these methods to exist, so we
merge them into one as `FundedChannel::try_send_stfu`. We also add a
test that reproduces the `FundedChannel::send_stfu` issue to ensure it's
fixed and does not regress.

Fixes #4429.

@wpaulino wpaulino added this to the 0.3 milestone Feb 20, 2026
@wpaulino wpaulino requested a review from jkczyz February 20, 2026 23:43
@wpaulino wpaulino self-assigned this Feb 20, 2026
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Feb 20, 2026

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

@codecov
Copy link

codecov bot commented Feb 21, 2026

Codecov Report

❌ Patch coverage is 92.62295% with 9 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (62c7575) to head (1e57132).
⚠️ Report is 21 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/channel.rs 89.18% 8 Missing ⚠️
lightning/src/ln/channelmanager.rs 97.91% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4432      +/-   ##
==========================================
- Coverage   85.87%   85.86%   -0.01%     
==========================================
  Files         157      159       +2     
  Lines      103769   104295     +526     
  Branches   103769   104295     +526     
==========================================
+ Hits        89115    89558     +443     
- Misses      12158    12234      +76     
- Partials     2496     2503       +7     
Flag Coverage Δ
tests 85.86% <92.62%> (-0.01%) ⬇️

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 -11530 to -11531
// TODO(splicing): If we didn't win quiescence, then we can contribute as an acceptor
// instead of waiting for the splice to lock.
Copy link
Contributor

Choose a reason for hiding this comment

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

Probably no need to worry since this is handled in #4416, but looks like this TODO wasn't moved.

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

In 15b04b5, we fixed a case in `FundedChannel::try_send_stfu` where we'd
send `stfu` unnecessarily for a new splice while one is already pending.
The same case also existed in `FundedChannel::send_stfu`, but was not
fixed. There's no good reason for both of these methods to exist, so we
merge them into one as `FundedChannel::try_send_stfu`. We also add a
test that reproduces the `FundedChannel::send_stfu` issue to ensure it's
fixed and does not regress.
With the introduction of `QuiescentAction`, the flag has essentially
become duplicate state, so we opt to remove it in favor of just checking
whether we have a pending `FundedChannel::quiescent_action`. Since the
quiescent flags are never persisted, we can simply remove it and update
the other flags, freeing up a bit for future use.
Otherwise, now that we no longer have the awaiting quiescence state
flag, we may end up sending `stfu` for a channel we intend to close.
@jkczyz jkczyz merged commit 144a24b into lightningdevkit:main Feb 24, 2026
21 of 22 checks passed
@wpaulino wpaulino deleted the stfu-send-cleanups branch February 24, 2026 17:58
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.

Fuzzer hit splice-while-splice-pending debug_assert! in FundedChannel::stfu()

4 participants