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
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).
|
🔔 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.
4 participants
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.