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

dual-fund: tx-abort fixups #6461

Merged

Conversation

niftynei
Copy link
Collaborator

@niftynei niftynei commented Jul 30, 2023

Reported by @t-bast from cross-compat with the Eclair dual-funding implementation.

Now we re-init the dualopend daemon when closed from an 'abort'. Seems to work very nicely?

Also fixes a problem where the channel won't advance to "NORMAL" if aborted on a subsequent RBF attempt.

Also fix bug where the funder would increase the reservation on RBFs of already reserved utxos, which then led to us not unreserving them on open failure. It's a halfway move towards us not reserving utxos at all during the negotiation phase of a channel open.

This way unreserving the PSBT will work as intended, and we don't have
to keep track of how many times we've called reserved for any one input.

Technically we're supposed to not reserve inputs at *all* while doing
opens, this moves us slightly closer to that.
Bug Report:
- initiate a channel open eclair -> cln
- wait for the transaction to be published
- eclair initiates rbf, and cancels it by sending tx_abort before exchanging commit_sig
- at that point everything looks good, cln echoes the tx_abort and stays connected
- eclair initiates another RBF attempt and sends tx_init_rbf: for some unknown reason,
  cln answers with channel_reestablish (??) followed by an error saying
  "Bad reestablish message: WIRE_TX_INIT_RBF"

Diagnosis:
  CLN is doing a reconnect after a tx-abort is sent.

Extra Test:
  Realized that if we abort, we won't correctly advanced to NORMAL if
  blocks are mined while we're in hanging state. CLN should advance
  after block containing channel open is mined.

Reported-By: @t-bast
Clean restart of daemon after a tx-abort is a nice way to work around
the 'persistent' disconnect that we t-bast noticed.

Changelog-Fixed: `dualopend`: Fix behavior for tx-aborts. No longer hangs, appropriately continues re-init of RBF requests without reconnction msg exchange.
Copy link
Contributor

@rustyrussell rustyrussell left a comment

Choose a reason for hiding this comment

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

Ack 17e8911

@rustyrussell rustyrussell merged commit 9b8909e into ElementsProject:master Jul 30, 2023
38 checks passed
@t-bast
Copy link

t-bast commented Aug 9, 2023

I've tested this e2e against eclair, it's all good 👍

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.

None yet

3 participants