Skip to content

Support splicing in ChannelContext::funding_tx_constructed #3982

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged

Conversation

jkczyz
Copy link
Contributor

@jkczyz jkczyz commented Aug 1, 2025

The ChannelState::NegotiatingFunding assertion check in ChannelContext::get_initial_commitment_signed will fail when implementing splicing's channel_reestablish logic. In order to support it and channel establishment, enter ChannelState::FundingNegotiated prior to calling the method and update the assertion accordingly.

Additionally, update relevant signing methods to use an Option instead of a Result with a ChannelError since failures result in a TxAbort anyhow.

Commits from this PR were extracted from #3886 to facilitate splicing testing.

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Aug 1, 2025

👋 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 wpaulino August 1, 2025 23:30
Copy link

codecov bot commented Aug 1, 2025

Codecov Report

❌ Patch coverage is 74.57627% with 15 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.98%. Comparing base (61e5819) to head (63604cb).
⚠️ Report is 12 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/channel.rs 74.57% 15 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3982      +/-   ##
==========================================
+ Coverage   88.94%   88.98%   +0.04%     
==========================================
  Files         174      174              
  Lines      124201   124223      +22     
  Branches   124201   124223      +22     
==========================================
+ Hits       110472   110544      +72     
+ Misses      11251    11204      -47     
+ Partials     2478     2475       -3     
Flag Coverage Δ
fuzzing 22.23% <10.34%> (-0.41%) ⬇️
tests 88.81% <74.57%> (+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.

@ldk-reviews-bot
Copy link

🔔 1st Reminder

Hey @wpaulino! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Copy link
Contributor

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

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

Feel free to squash

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

@jkczyz jkczyz force-pushed the 2025-08-get-initial-commitment-signed branch from e309a6b to f3eb10b Compare August 4, 2025 23:59
@wpaulino
Copy link
Contributor

wpaulino commented Aug 5, 2025

CI is failing

jkczyz added 4 commits August 5, 2025 13:38
The ChannelState::NegotiatingFunding assertion check in
ChannelContext::get_initial_commitment_signed will fail when
implementing splicing's channel_reestablish logic. In order to support
it and channel establishment, enter ChannelState::FundingNegotiated
prior to calling the method and update the assertion accordingly.

Also allows writing a channel in the FundedNegotiated state when an
interactive signing session is active. This is necessary as it indicates
a previously funded channel being spliced.
When ChannelContext::get_initial_commitment_signed is called for V2
channel establishment, any errors should result in closing the channel.
However, in the future, when this is used for splicing it should abort
instead of closing the channel. Move the error construction to the call
sites in anticipation of this.
@jkczyz jkczyz force-pushed the 2025-08-get-initial-commitment-signed branch from f3eb10b to 63604cb Compare August 5, 2025 18:43
Copy link
Contributor

@wpaulino wpaulino left a comment

Choose a reason for hiding this comment

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

Will merge with just my approval after CI, the changes were fairly trivial.

@wpaulino wpaulino merged commit 9350c57 into lightningdevkit:main Aug 5, 2025
24 of 25 checks passed
@jkczyz jkczyz self-assigned this Aug 7, 2025
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