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

[DO NOT REVIEW] fundingManager: Set up channel barrier on startup if fundingLocked is not yet received #309

Closed
wants to merge 1 commit into from

Conversation

halseth
Copy link
Contributor

@halseth halseth commented Sep 1, 2017

DO NOT REVIEW

Changes

This commit adds a channel barrier on fundingManager startup for channels that we still haven't
received fundingLocked for. This fixes a bug where we after restarting the fundingManager would receive the fundingLocked message, and crash when trying to close the non-existing barrier.

Tests

The fundingmanager tests are also updated to check that the fundingLocked messages are sent and handled correctly, and also exercise the scenario described above.

Follow-up

There are still cases not handled by the fundingManager, that will be tested and (hopefully) fixed in follow up PRs.

  1. If we receive the fundingLocked message, but fail to announce the channel and send our own fundingLocked, we'll never process the received fundingLocked. In case of a restart, we'll retry our own announcement, but we cannot know for sure that our peer will resend the fundingLocked, causing the channel to never become fully operational. In this case we should keep the state given to us by the received fundingLocked.
  2. If we receive two fundingLocked messages from the peer, we will crash. This happens because we don't have a channel barrier set up (we don't expect to receive a second), and try to close the non-existing barrier.
  3. If we fail finishing the opening process (say because the peer goes offline at some point), then we try to continue where we left off, on restart of the fundingManager. However, we must also continue the process when a peer reestablish the connection: Re-send FundingLocked in ChannelLink upon re-connect if at state zero #303

Common for these scenarios is that they arise from the interaction between the received fundingLocked messages and the state of our sent messages. Decoupling the state machines of these two will probably simplify this logic.

@mention-bot
Copy link

@halseth, thanks for your PR! By analyzing the history of the files in this pull request, we identified @bryanvu, @Roasbeef and @AndrewSamokhvalov to be potential reviewers.

@halseth halseth changed the title fundingManager: Set up channel barrier on startup if fundingLocked is not yet received [DO NOT REVIEW] fundingManager: Set up channel barrier on startup if fundingLocked is not yet received Sep 6, 2017
This commit adds a channel barrier on fundingManager startup for
channels that we still haven't received fundingLocked for. This
fixes a bug where we after restarting the fundingManager would
receive the fundingLocked message, and crash when trying to close
the non-existing barrier.

The fundingmanager tests are also updated to check that the
fundingLocked messages are sent and handled correcly, and also
exercise the scanario described above.
@halseth
Copy link
Contributor Author

halseth commented Sep 19, 2017

This change has been included in #328

@halseth halseth closed this Sep 19, 2017
@halseth halseth deleted the funding-locked-test branch July 12, 2018 13:39
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

2 participants