Uh oh!
There was an error while loading. Please reload this page.
Allow counterparty tx_abort before handling initial commitment signed - #4204
Conversation
Upon processing the counterparty's initial `commitment_signed` for a splice, we queue a monitor update with the new commitment transactions spending the new funding transaction. Once handled, funding transaction signatures can be exchanged and we must start monitoring the chain for a possible commitment transaction broadcast spending the new funding transaction. Aborting the splice negotiation then would therefore be unsafe, hence why we currently force close in such cases. However, there is no reason to force close prior to receiving their initial `commitment_signed`, as this would imply the funding transaction signatures have yet to be exchanged, thus making a commitment broadcast spending said transaction impossible allowing us to abort the splice negotiation safely.
👋 Thanks for assigning @jkczyz as a reviewer! |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@## main #4204 +/- ##
==========================================
+ Coverage 89.28% 89.30% +0.02%
==========================================
Files 180 180 Lines 137913 138055 +142 Branches 137913 138055 +142 ==========================================
+ Hits 123134 123293 +159 + Misses 12163 12150 -13 + Partials 2616 2612 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
TheBlueMatt
left a comment
There was a problem hiding this comment.
Only kinda skimmed the test changes, but code diff lgtm. One thing we should probably make sure to do is add splicing logic to chanmon_consistency and have a hard-coded seed for doing a splice in full_stack. That should give us somewhat-okay coverage of the debug assertions and test accidental force-closes (not that either would have caught this, just a general note).
ldk-reviews-bot
commented
Nov 5, 2025
🔔 1st Reminder Hey @jkczyz! This PR has been waiting for your review. |
| funding_negotiation, | ||
| FundingNegotiation::AwaitingSignatures { .. } | ||
| ); | ||
| if counterparty_aborted { |
There was a problem hiding this comment.
Why do we condition on counterparty_aborted? Would it be a problem if always do the new check?
There was a problem hiding this comment.
If we don't, then we would always reset the splice state when we haven't received their initial commitment_signed.
| .expect("We have a pending splice awaiting signatures") | ||
| .has_received_commitment_signed() | ||
| { | ||
| // We only force close once the counterparty tries to abort after committing to |
There was a problem hiding this comment.
This implies that this is somehow a limitation in LDK, but it isn't, its a rather fundamental limitation of the protocol - if we have received the CS, the peer cannot know whether we have provided them with funding signatures, at which point cancelling the splice would be unsafe no matter the implementation in LDK.
TheBlueMatt
commented
Nov 12, 2025
Backported in #4221 |
Upon processing the counterparty's initial
commitment_signedfor a splice, we queue a monitor update with the new commitment transactions spending the new funding transaction. Once handled, funding transaction signatures can be exchanged and we must start monitoring the chain for a possible commitment transaction broadcast spending the new funding transaction. Aborting the splice negotiation then would therefore be unsafe, hence why we currently force close in such cases. However, there is no reason to force close prior to receiving their initialcommitment_signed, as this would imply the funding transaction signatures have yet to be exchanged, thus making a commitment broadcast spending said transaction impossible allowing us to abort the splice negotiation safely.