Skip to content
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

Add closing test reconnect with 3rd-stage txs #2820

Merged
merged 1 commit into from
Feb 13, 2024

Conversation

t-bast
Copy link
Member

@t-bast t-bast commented Feb 12, 2024

We had some cases where channels were staying stuck in the CLOSING state without going to the CLOSED state in previous versions of eclair where we published different versions of our 3rd-stage transactions after a restart and a feerate increase.

This happened in two cases:

  • when we published a different version of our claim-main, since we only store the latest (it's an Option, not a Seq) and put a watch on this specific txid instead of watching the corresponding output
  • when we re-published 3rd-stage txs after receiving CMD_FULFILL_HTLC because we replaced the previous claim-htlc-delayed transactions

This was on my backlog for a while, and I wanted to take another look at it after the splicing implementation. Fortunately, this was fixed as part of the splicing changes, but it's worth having a test to ensure that there's no regression from future changes.

We had some cases where channels were staying stuck in the CLOSING state
without going to the CLOSED state in previous versions of eclair where
we published different versions of our 3rd-stage transactions after a
restart and a feerate increase.

This happened in two cases:

- when we published a different version of our claim-main, since we only
  store the latest (it's an `Option`, not a `Seq`) and put a watch on
  this specific `txid` instead of watching the corresponding output
- when we re-published 3rd-stage txs after receiving `CMD_FULFILL_HTLC`
  because we replaced the previous `claim-htlc-delayed` transactions

This was on my backlog for a while, and I wanted to take another look at
it after the splicing implementation. Fortunately, this was fixed as
part of the splicing changes, but it's worth having a test to ensure
that there's no regression from future changes.
@t-bast t-bast requested review from sstone and pm47 February 12, 2024 16:28
@t-bast t-bast merged commit b5e83a6 into master Feb 13, 2024
1 check passed
@t-bast t-bast deleted the third-stage-txs-closing-test branch February 13, 2024 16:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants