Skip to content

Conversation

wpaulino
Copy link
Contributor

@wpaulino wpaulino commented Sep 4, 2025

No description provided.

@wpaulino wpaulino added this to the 0.2 milestone Sep 4, 2025
@wpaulino wpaulino self-assigned this Sep 4, 2025
@ldk-reviews-bot
Copy link

ldk-reviews-bot commented Sep 4, 2025

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

Copy link

codecov bot commented Sep 4, 2025

Codecov Report

❌ Patch coverage is 92.92929% with 21 lines in your changes missing coverage. Please review.
✅ Project coverage is 88.67%. Comparing base (f3c22a7) to head (307fa56).
⚠️ Report is 9 commits behind head on main.

Files with missing lines Patch % Lines
lightning/src/ln/splicing_tests.rs 92.67% 14 Missing and 6 partials ⚠️
lightning/src/util/test_utils.rs 95.65% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #4054      +/-   ##
==========================================
+ Coverage   87.84%   88.67%   +0.82%     
==========================================
  Files         176      176              
  Lines      131728   131839     +111     
  Branches   131728   131839     +111     
==========================================
+ Hits       115712   116903    +1191     
+ Misses      13384    12279    -1105     
- Partials     2632     2657      +25     
Flag Coverage Δ
fuzzing 21.61% <0.00%> (-0.01%) ⬇️
tests 88.50% <92.92%> (+0.82%) ⬆️

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.

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.

Some commits, eg Don't process alternative funding confirmations duplicatively could use some motivation in the commit message.

Also, yea, probably worth splitting this up somehow, its kinda a large PR, maybe move the tests and channel-state changes to another one?

@ldk-reviews-bot
Copy link

🔔 1st Reminder

Hey @jkczyz! 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.

@wpaulino wpaulino changed the title Add basic end-to-end splice tests fixing miscellaneous issues along the way Add basic end-to-end splice tests Sep 8, 2025
@ldk-reviews-bot
Copy link

🔔 2nd Reminder

Hey @jkczyz! 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.

@ldk-reviews-bot
Copy link

🔔 1st Reminder

Hey @TheBlueMatt! 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.

We plan to reuse it for dual-funding/splicing, and those require
standard SegWit inputs only.
This guarantees we get a unique txid when calling
`provide_anchor_reserves` successively as we're immediately mining (and
therefore advancing the chain) the transaction after creating it.
This adds a new test for both splice-in and splice-out in favor of
maintaining the existing test. Helpers have been added to DRY up a lot
of the logic necessary for driving the splice state machine forward.
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.

I mean, basically LGTM, but I don't really think this needs a second reviewer now. I'll just defer to @jkczyz who can merge when he's happy.

@wpaulino wpaulino requested a review from jkczyz September 16, 2025 20:58
@jkczyz jkczyz merged commit 9d6d08b into lightningdevkit:main Sep 17, 2025
24 of 25 checks passed
@wpaulino wpaulino deleted the test-basic-splice branch September 17, 2025 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants