Skip to content

Conversation

@wpaulino
Copy link
Contributor

Depends on #4412.

According to the spec, handling a splice_init and splice_ack may return tx_abort to reject the splice, but we always send a warning and disconnect instead, so those are not included here.

@wpaulino wpaulino added this to the 0.3 milestone Feb 12, 2026
@wpaulino wpaulino requested a review from TheBlueMatt February 12, 2026 19:26
@wpaulino wpaulino self-assigned this Feb 12, 2026
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Feb 12, 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 12, 2026

Codecov Report

❌ Patch coverage is 57.71812% with 126 lines in your changes missing coverage. Please review.
✅ Project coverage is 86.07%. Comparing base (0b45bfd) to head (826113b).

Files with missing lines Patch % Lines
lightning/src/ln/channelmanager.rs 60.15% 80 Missing and 20 partials ⚠️
lightning/src/ln/channel.rs 44.68% 26 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main    #4415   +/-   ##
=======================================
  Coverage   86.07%   86.07%           
=======================================
  Files         156      156           
  Lines      103442   103466   +24     
  Branches   103442   103466   +24     
=======================================
+ Hits        89033    89063   +30     
+ Misses      11894    11887    -7     
- Partials     2515     2516    +1     
Flag Coverage Δ
tests 86.07% <57.71%> (+<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.

TheBlueMatt
TheBlueMatt previously approved these changes Feb 12, 2026
Copy link
Collaborator

@TheBlueMatt TheBlueMatt left a comment

Choose a reason for hiding this comment

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

lgtm

@wpaulino wpaulino dismissed TheBlueMatt’s stale review February 12, 2026 22:53

The merge-base changed after approval.

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

@TheBlueMatt
Copy link
Collaborator

note that github wont let this be merged until its rebased. Also not sure if @jkczyz wants to take a look or not.

@TheBlueMatt TheBlueMatt requested a review from jkczyz February 12, 2026 22:54
@wpaulino wpaulino force-pushed the free-holding-cells-quiescence-exit branch from d1ea893 to 5a610c7 Compare February 12, 2026 23:27
@wpaulino
Copy link
Contributor Author

Rebased and pushed fixups for #4412

Copy link
Contributor

@jkczyz jkczyz left a comment

Choose a reason for hiding this comment

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

note that github wont let this be merged until its rebased. Also not sure if @jkczyz wants to take a look or not.

Skimmed though it. Just one comment on the approach.

Comment on lines 15987 to 15989
// Note that we're not able to do this inline in `internal_tx_add_input` as we
// usually do because we first need to send the `tx_abort` in `handle_error` above
// before sending any commitment updates.
Copy link
Contributor

Choose a reason for hiding this comment

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

Any reason why we can't include exited_quiescence in MsgHandleErrInternal and do this in handle_error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea, this cleaned up nicely

After cad88af, a few code paths that also lead to a quiescence exit were
not accounted for. This commit addresses the path where we exit
quiescence due to a processing error on a counterparty's
`tx_add_input/output`, `tx_remove_input/output`, or `tx_complete`
message.
After cad88af, a few code paths that also lead to a quiescence exit were
not accounted for. This commit addresses the path where we exit
quiescence due to processing a counterparty's `tx_abort` message.
After cad88af, a few code paths that also lead to a quiescence exit were
not accounted for. This commit addresses the last remaining path where
we exit quiescence when we exchange `tx_signatures` with the
counterparty.
These errors will only ever affect our in-memory state, so there's no
need to persist the ChannelManager when we come across one. Note that
`tx_abort` is not included here because there is a possibility we force
close the channel, which we should persist.
@wpaulino wpaulino force-pushed the free-holding-cells-quiescence-exit branch from 5a610c7 to 826113b Compare February 13, 2026 00:57
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.

4 participants