lncli: Added support for backup/verify/restore of single chanpoint to/from file #7437

ardevd · 2023-02-21T12:54:52Z

Added support for backup/verify/restore of a single chanpoint to file.

Change Description

Previously, specifying both a chan_point and a output file to write the backup to resulted in the output file argument being ignored and json output to be written to the console. Effectively discarding the desire to write to file.

Also, there was no way to restore a single channel from a file nor was there a way to verify such backups.

Steps to Test

lncli exportchanbackup <chan_point> --output_file <filename>
lncli verifychanbackup --single_file <filename> lncli restorechanbackup --single_file `

Pull Request Checklist

Testing

Your PR passes all CI checks.
Tests covering the positive and negative (error paths) are included.
Bug fixes contain tests triggering the bug to prevent regressions.

Code Style and Documentation

The change obeys the Code Documentation and Commenting guidelines, and lines wrap at 80.
Commits follow the Ideal Git Commit Structure.
Any new logging statements use an appropriate subsystem and logging level.
There is a change description in the release notes, or [skip ci] in the commit message for small changes.

ardevd · 2023-02-21T14:16:00Z

Someone should figure out whether this change makes sense. It seems to me that backup restoration currently does not support restoring a single chan_point from file. Whether this design is intentional is unknown to me.

guggero

Thanks for the fix. Looks good, just one nit. And you're right, it doesn't look like you can specify a single file when restoring just yet. Maybe it would make sense to add a --single_file flag to the restorechanbackup command as well in this PR?

cmd/lncli/commands.go

ardevd · 2023-02-21T21:17:54Z

Agreed! I'll look at implementing the restore part as well. Thanks.

guggero

Code looks good, thanks. Please take a look at the pull request check list and contribution guideline about commit structure and naming as well as other requirements (like running the linter).

cmd/lncli/commands.go

ardevd · 2023-02-25T19:14:17Z

@guggero Should I add an entry in release notes myself in this merge request?

guggero · 2023-02-27T08:58:04Z

@guggero Should I add an entry in release notes myself in this merge request?

Yes. You can add a new file docs/release-notes/release-notes-0.16.1.md since this PR won't make it into 0.16.0 as the RC just dropped last week.

ardevd · 2023-03-01T08:36:51Z

Not entirely sure why the CI is failing. Anything I can do to fix it?

yyforyongyu

Not entirely sure why the CI is failing. Anything I can do to fix it?

There are known flakes that are under investigation so we'll see what's failed exactly. Meanwhile the linter should not fail tho, plus you may want to squash and clean your commit messages a bit.

guggero

Can you rebase and squash the commits into a single one for the changes in cmd/lncli and one for the release notes please?

guggero · 2023-03-01T10:26:27Z

cmd/lncli/commands.go


 	   * A single channel packed SCB, which can be obtained from
 	     exportchanbackup. This should be passed in hex encoded format.

 	   * A packed multi-channel SCB, which couples several individual
 	     static channel backups in single blob.

+	   * A file path which points to a packed single-channel backup within a
+	     file, using the same format that lnd does in its channel.backup file.


This line is longer than 80 characters (when using 8 spaces for the tab character in your editor), that's why the linter is complaining.

ardevd · 2023-03-01T21:18:31Z

Rebased and squashed!

guggero

tACK, LGTM 🎉

This will need to wait with being merged until after 0.16.0 final is out.

guggero · 2023-03-17T07:49:47Z

@ardevd can you rebase this on top of the 0-16-1-staging branch please? Then we can get it merged.

ardevd · 2023-03-17T19:26:48Z

Rebased now, but conflicts with release notes.

guggero · 2023-03-18T12:26:22Z

Yes, can you resolve the conflict? When doing git rebase you should be given a file that contains both versions and then you can edit the way it should be resolved. Or your IDE should present you some kind of merge conflict resolution window.

ardevd · 2023-03-20T18:44:11Z

Should be good to go.

guggero · 2023-03-20T18:45:14Z

No, it looks like you merged the target branch into yours. Instead you should rebase your branch on top of 0-16-1-staging.

author ardevd <edvard.holst@gmail.com> 1676983861 +0100 committer ardevd <ardevd@users.noreply.github.com> 1677705118 +0100 Write to the specified file if the user specifies a chan_point for backup and also specifies an output file to write the backup to. Fixes lightningnetwork#7436

ardevd · 2023-03-25T14:32:59Z

@guggero should be good to go now

guggero · 2023-03-27T08:55:35Z

Looks good, thanks a lot, @ardevd. Integration test failures are flakes, as this is a CLI change only.

guggero reviewed Feb 21, 2023

View reviewed changes

cmd/lncli/commands.go Outdated Show resolved Hide resolved

ardevd changed the title ~~Draft: fix(lncli): Write to file for chan_point backups.~~ Draft: fix(lncli): Added support for backup/verify/restore of single chanpoints to/from file Feb 22, 2023

ardevd changed the title ~~Draft: fix(lncli): Added support for backup/verify/restore of single chanpoints to/from file~~ Draft: fix(lncli): Added support for backup/verify/restore of single chanpoint to/from file Feb 22, 2023

ardevd changed the title ~~Draft: fix(lncli): Added support for backup/verify/restore of single chanpoint to/from file~~ fix(lncli): Added support for backup/verify/restore of single chanpoint to/from file Feb 22, 2023

guggero reviewed Feb 23, 2023

View reviewed changes

cmd/lncli/commands.go Outdated Show resolved Hide resolved

cmd/lncli/commands.go Outdated Show resolved Hide resolved

ardevd changed the title ~~fix(lncli): Added support for backup/verify/restore of single chanpoint to/from file~~ lncli: Added support for backup/verify/restore of single chanpoint to/from file Feb 25, 2023

yyforyongyu reviewed Mar 1, 2023

View reviewed changes

guggero reviewed Mar 1, 2023

View reviewed changes

ardevd force-pushed the 7436-chanbackup-writer-fix branch 3 times, most recently from ee28e33 to e118289 Compare March 1, 2023 21:17

guggero self-requested a review March 1, 2023 22:20

guggero approved these changes Mar 2, 2023

View reviewed changes

yyforyongyu approved these changes Mar 6, 2023

View reviewed changes

guggero added this to the v0.16.1 milestone Mar 6, 2023

saubyk assigned ardevd Mar 9, 2023

saubyk changed the base branch from master to 0-16-1-staging March 16, 2023 15:25

ardevd force-pushed the 7436-chanbackup-writer-fix branch from 318b6f2 to 0756093 Compare March 17, 2023 19:26

docs: added release notes

d1f961c

ardevd force-pushed the 7436-chanbackup-writer-fix branch from c5db55d to d1f961c Compare March 25, 2023 14:32

guggero merged commit 5272e91 into lightningnetwork:0-16-1-staging Mar 27, 2023

ardevd deleted the 7436-chanbackup-writer-fix branch March 27, 2023 10:20

guggero mentioned this pull request Mar 29, 2023

[bug]: exportchanbackup --output_file argument does nothing when chanpoint is provided #7436

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

lncli: Added support for backup/verify/restore of single chanpoint to/from file #7437

lncli: Added support for backup/verify/restore of single chanpoint to/from file #7437

ardevd commented Feb 21, 2023 •

edited

ardevd commented Feb 21, 2023

guggero left a comment

ardevd commented Feb 21, 2023

guggero left a comment

ardevd commented Feb 25, 2023

guggero commented Feb 27, 2023

ardevd commented Mar 1, 2023

yyforyongyu left a comment

guggero left a comment

guggero Mar 1, 2023

ardevd commented Mar 1, 2023

guggero left a comment

guggero commented Mar 17, 2023

ardevd commented Mar 17, 2023 •

edited

guggero commented Mar 18, 2023

ardevd commented Mar 20, 2023

guggero commented Mar 20, 2023

ardevd commented Mar 25, 2023

guggero commented Mar 27, 2023

lncli: Added support for backup/verify/restore of single chanpoint to/from file #7437

lncli: Added support for backup/verify/restore of single chanpoint to/from file #7437

Conversation

ardevd commented Feb 21, 2023 • edited

Change Description

Steps to Test

Pull Request Checklist

Testing

Code Style and Documentation

ardevd commented Feb 21, 2023

guggero left a comment

Choose a reason for hiding this comment

ardevd commented Feb 21, 2023

guggero left a comment

Choose a reason for hiding this comment

ardevd commented Feb 25, 2023

guggero commented Feb 27, 2023

ardevd commented Mar 1, 2023

yyforyongyu left a comment

Choose a reason for hiding this comment

guggero left a comment

Choose a reason for hiding this comment

guggero Mar 1, 2023

Choose a reason for hiding this comment

ardevd commented Mar 1, 2023

guggero left a comment

Choose a reason for hiding this comment

guggero commented Mar 17, 2023

ardevd commented Mar 17, 2023 • edited

guggero commented Mar 18, 2023

ardevd commented Mar 20, 2023

guggero commented Mar 20, 2023

ardevd commented Mar 25, 2023

guggero commented Mar 27, 2023

ardevd commented Feb 21, 2023 •

edited

ardevd commented Mar 17, 2023 •

edited