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

chanbackup: continue recovery if channel already exists #3737

Merged
merged 2 commits into from Nov 23, 2019

Conversation

guggero
Copy link
Collaborator

@guggero guggero commented Nov 18, 2019

While recovering a multi backup file, if for some reason one of the channels in that backup already exists, the whole recovery is aborted.
This leaves the restore in a half-finished state and a re-try or resume is not possible.
With this small fix, the same multi-file can be added again and all channels that were not imported before can now be added.

@guggero
Copy link
Collaborator Author

guggero commented Nov 19, 2019

@Roasbeef bump as potential candidate for 0.8.2.

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

Sound change. I've also seen instances in the past where a user hits CRTL-C early because things are taking too long, but then can't restart the process easily.

Small diff, so no major comments, but it would be nice to have a test for this either at the unit test, or integration test level, to ensure we're checking the proper error (though at a cursory scan, it looks correct to me).

chanbackup/recover.go Show resolved Hide resolved
@Roasbeef Roasbeef added this to the 0.8.2 milestone Nov 20, 2019
@guggero
Copy link
Collaborator Author

guggero commented Nov 20, 2019

Added a test on the itest level to make sure an SCB file can be imported twice without running into an error.
Unit test would be quite a bigger diff since the *channeldb.DB and *contractcourt.ChainArbitrator of chanDBRestorer would need to be abstracted behind an interface to be able to mock them.

Copy link
Contributor

@cfromknecht cfromknecht left a comment

Choose a reason for hiding this comment

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

LGTM 🔥

Copy link
Member

@Roasbeef Roasbeef left a comment

Choose a reason for hiding this comment

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

LGTM 🍁

@Roasbeef Roasbeef merged commit 74849e7 into lightningnetwork:master Nov 23, 2019
@guggero guggero deleted the resume-scb branch November 23, 2019 12:48
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

4 participants