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

lightningd/opening_control.c: Remove 'Try fundchannel_cancel again' error. #3778

Merged
merged 1 commit into from
Jun 22, 2020

Conversation

ZmnSCPxj
Copy link
Collaborator

Changelog-Changed: fundchannel_cancel will now succeed even when executed while a fundchannel_complete is ongoing; in that case, it will be considered as cancelling the funding after the fundchannel_complete succeeds.

Let me introduce the concept of "Sequential Consistency":
All operations on parallel processes form a single total order agreed upon by all processes.

So for example, suppose we have parallel invocations of fundchannel_complete and fundchannel_cancel:

                      +--[fundchannel_complete]-->
                      |
--[fundchannel_start]-+
                      |
                      +--[fundchannel_cancel]---->

What "Sequential Consistency" means is that the above parallel operations can be serialized as a single total order as:

--[fundchannel_start]--[fundchannel_complete]--[fundchannel_cancel]-->

Or:

--[fundchannel_start]--[fundchannel_cancel]--[fundchannel_complete]-->

In the first case, fundchannel_complete succeeds, and the fundchannel_cancel invocation also succeeds, sending an error to the peer to make them forget the chanel.

In the second case, fundchannel_cancel succeeds, and the succeeding fundchannel_complete invocation fails, since the funding is already cancelled and there is nothing to complete.

Note that in both cases, fundchannel_cancel always succeeds.

Unfortunately, prior to this commit, fundchannel_cancel could fail with a Try fundchannel_cancel again error if the fundchannel_complete is ongoing when the fundchannel_cancel is initiated.
This violates Sequential Consistency, as there is no single total order that would have caused fundchannel_cancel to fail.

This commit is a minimal patch which just reschedules fundchannel_cancel to occur after any fundchannel_complete that is ongoing.

…rror.

Changelog-Changed: `fundchannel_cancel` will now succeed even when executed while a `fundchannel_complete` is ongoing; in that case, it will be considered as cancelling the funding *after* the `fundchannel_complete` succeeds.

Let me introduce the concept of "Sequential Consistency":
All operations on parallel processes form a single total order agreed upon by all processes.

So for example, suppose we have parallel invocations of `fundchannel_complete` and `fundchannel_cancel`:

                          +--[fundchannel_complete]-->
                          |
    --[fundchannel_start]-+
                          |
                          +--[fundchannel_cancel]---->

What "Sequential Consistency" means is that the above parallel operations can be serialized as a single total order as:

    --[fundchannel_start]--[fundchannel_complete]--[fundchannel_cancel]-->

Or:

    --[fundchannel_start]--[fundchannel_cancel]--[fundchannel_complete]-->

In the first case, `fundchannel_complete` succeeds, and the `fundchannel_cancel` invocation also succeeds, sending an `error` to the peer to make them forget the chanel.

In the second case, `fundchannel_cancel` succeeds, and the succeeding `fundchannel_complete` invocation fails, since the funding is already cancelled and there is nothing to complete.

Note that in both cases, `fundchannel_cancel` **always** succeeds.

Unfortunately, prior to this commit, `fundchannel_cancel` could fail with a `Try fundchannel_cancel again` error if the `fundchannel_complete` is ongoing when the `fundchannel_cancel` is initiated.
This violates Sequential Consistency, as there is no single total order that would have caused `fundchannel_cancel` to fail.

This commit is a minimal patch which just reschedules `fundchannel_cancel` to occur after any `fundchannel_complete` that is ongoing.
@ZmnSCPxj ZmnSCPxj requested a review from cdecker as a code owner June 18, 2020 06:51
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 6f35452

Thanks! This is consistent with the modern cancel semantics. Any API which says "try again later" is always suspect!

@ZmnSCPxj ZmnSCPxj merged commit 5db69f1 into ElementsProject:master Jun 22, 2020
@ZmnSCPxj ZmnSCPxj deleted the stronger-cancel branch June 22, 2020 03:19
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