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.

Sign up for GitHub

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

Jump to bottom

allow export of static backup #639

Merged
merged 3 commits into from
Oct 13, 2023
Merged

allow export of static backup #639

merged 3 commits into from
Oct 13, 2023

Conversation

ubbabeck
Copy link
Contributor

@ubbabeck ubbabeck commented Sep 27, 2023 •
edited
Loading

implements #626

Q:

Should the file be written as bytes or string?

test

I am currently in the process of setting up clighting so i have not had the change to test whenever the file works or not.

@ubbabeck ubbabeck self-assigned this Sep 27, 2023
@ubbabeck ubbabeck force-pushed the static-backup branch 4 times, most recently from 3ca6280 to 0b3b02e Compare September 29, 2023 14:31
@ubbabeck ubbabeck added QA-testing QA testing needed feature labels Oct 1, 2023
@ubbabeck ubbabeck requested review from JssDWt and roeierez October 2, 2023 08:40
@ubbabeck ubbabeck marked this pull request as ready for review October 2, 2023 08:40
@ubbabeck ubbabeck requested a review from ademar111190 October 2, 2023 08:42
@ubbabeck ubbabeck changed the title [WIP] allow export of static backup allow export of static backup Oct 2, 2023
@ubbabeck ubbabeck linked an issue Oct 2, 2023 that may be closed by this pull request
Integrate static backup #626
Closed
roeierez
roeierez requested changes Oct 2, 2023
View reviewed changes
lib/bloc/account/credentials_manager.dart Outdated Show resolved Hide resolved
lib/routes/dev/developers_view.dart Outdated

void _exportStaticBackup(BuildContext context) async {
final accBloc = context.read<AccountBloc>();
const name = "emergency.recover";
Copy link
Member

Choose a reason for hiding this comment

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

Is this the name cln use?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes
https://github.com/ElementsProject/lightning/blob/df10c62508aecb7aaee0fdc4166face87663f8b9/plugins/chanbackup.c#L26

Copy link
Member

@roeierez roeierez Oct 5, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I see but this, as implemented right now, is confusing because recovering from emergency file is different than recovering from the gPRc interface.
Here is how we should recover from the static backup hex data: https://docs.corelightning.org/reference/lightning-recoverchannel

If we call it emergency.recover the user would think the right way to recover is this: https://docs.corelightning.org/reference/lightning-emergencyrecover

So maybe we should call it just scb.recover (as the parameter name for the recoverchannel command) ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I didn't find that recoverchannel function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed the file to scb.recover and also return the value as is.

lib/routes/dev/developers_view.dart Outdated Show resolved Hide resolved
@ubbabeck ubbabeck requested a review from roeierez October 4, 2023 16:27
ademar111190
ademar111190 approved these changes Oct 10, 2023
View reviewed changes
Copy link
Collaborator

@ademar111190 ademar111190 left a comment

Choose a reason for hiding this comment

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

LGTM just one request:
Can you update the breez translations to this revision 782e52c6f272a4901ff85ecb76981085b355acd1 and change the two "in code" texts to texts references? (I'm adding the reference id)

lib/routes/dev/developers_view.dart Show resolved Hide resolved
lib/routes/dev/developers_view.dart Outdated Show resolved Hide resolved
JssDWt
JssDWt approved these changes Oct 11, 2023
View reviewed changes
Copy link
Contributor

@JssDWt JssDWt left a comment

Choose a reason for hiding this comment

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

utACK

@ubbabeck
Copy link
Contributor Author

ubbabeck commented Oct 13, 2023 •
edited
Loading

Test

@ubbabeck ubbabeck merged commit bd019d2 into main Oct 13, 2023
@Nackoo2000
Copy link
Collaborator

@ubbabeck I tested this from the developer's screen and the menu there displays this option.
if there is a successful download it means it works right?

@ubbabeck
Copy link
Contributor Author

if there is a successful download it means it works right?

yes, and if the node does not have an active an error will show.

@erdemyerebasmaz erdemyerebasmaz deleted the static-backup branch November 16, 2023 20:12
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ademar111190 ademar111190 ademar111190 approved these changes

@JssDWt JssDWt JssDWt approved these changes

@erdemyerebasmaz erdemyerebasmaz Awaiting requested review from erdemyerebasmaz

@roeierez roeierez Awaiting requested review from roeierez

Assignees

@ubbabeck ubbabeck

Labels
feature QA-testing QA testing needed
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Integrate static backup
5 participants
@ubbabeck @Nackoo2000 @ademar111190 @roeierez @JssDWt