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-funding: handle failures #4424

Merged
merged 6 commits into from
Mar 15, 2021

Conversation

niftynei
Copy link
Collaborator

Prior to this, multifundchannels that involved v2 opens were left in a bit of a lurch if any of the v2 peers had issues while opening.

In theory, we should be able to 'live-action' update the PSBT for the remaining peers, if any has an issue, however this would require quite a bit of re-writing of the redo code in multifundchannel.

Plus, it's only really an option is something happens while we're still in the update mode.

More easily, we can merely signal to the peer that we're going to restart the negotiation, by sending an error which fails the in-progress negotiation, and then cleanly restart.

For peers that have reached the 'commitment exchange' already, there's no saving these without spending an input so we just ignore them/mark as failed and retry.

@niftynei niftynei requested a review from cdecker as a code owner March 11, 2021 04:52
@niftynei niftynei added this to the v0.9.4 milestone Mar 11, 2021
@niftynei niftynei mentioned this pull request Mar 12, 2021
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 1becab4

Allows us to clean up an in-progress open that we won't be completing

Changelog-Added: EXPERIMENTAL JSON-RPC: Permit user-initiated aborting of in-progress opens. Only valid for not-yet-committed opens and RBF-attempts
is_v2 instead of protocol == OPEN_CHANNEL
Makes it easiser to figure out what's going on.

Also, make sure that every 'fail_destination' error description includes
quote escapes.
There's a version of this that keeps the PSBT in memory and does some
fancy addition/subtraction of unuseable parts for the v2's, however
it's much easier and simpler to simply error on the peer and re-start
from the very beginning.

This only works if we haven't gotten commitments from the peer yet (in
fact either method would only work if we haven't got commitments from
the peer yet), so if we've got commitments from them we simply mark them
as failed an go again.

In a perfect world, we'd remember what inputs we used last time, and
reuse those again on the re-attempt, which would pefectly guarantee both
that the failed opens (ones w/ commitments exchanged) would be canceled
after this completes (and we could re-try the failed again).

As it is, this is not perfect. It is, however, servicable.
Previously this ported errors around as JSON. A nicer thing to do is to
deconstruct/reconstruct it; this also allows us to create our own errors
from within the multifundchannel family.
@rustyrussell
Copy link
Contributor

rustyrussell commented Mar 15, 2021

Trivial conflict resolve rebase.

Ack 7b0be0c

@rustyrussell rustyrussell merged commit 2586641 into ElementsProject:master Mar 15, 2021
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