-
Notifications
You must be signed in to change notification settings - Fork 46
gl-cli: add signer generate-secret command #657
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
Draft
Lagrang3
wants to merge
1
commit into
Blockstream:main
Choose a base branch
from
Lagrang3:glcli_generate_hsm_secret
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @Lagrang3! This will generate an older version of the hsm_secret and while we do support it, you won't get the benefit of recoverable addresses that you will with the modern version of the hsm secret. So for reference, we support 4 types of hsm_secrets:
Legacy 32 byte secrets - This is your usual 32 bytes, so in your case your mnemonic produces a seed and you store the first 32 bytes of it in the file. Your password case is also a variant of this, this is quite different to how it is in CLN, In CLN if the secret is 32 bytes it means that there is no passphrase. We rely on the 32 bytes == no passphrase in our recovery tools such as exposesecret. In your case, we're adding a passphrase option that leads to a completely different 32 byte secret.
Legacy 73 byte encrypted secrets - This is how passphrase support used to work, the passphrase would be used to encrypt the 32 byte secret to produce an encrypted 73 byte secret.
Modern mnemonics (no passphrase) - In this case we'll have 32 bytes of zero padding followed by the mnemonic itself. So it would look something like:
You can see how it's done in generate_hsm() and derive_seed_hash()
My recommendation would be to follow the modern standard for hsm_secrets so taproot wallet addresses are recoverable!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice explanation! Thank you.
I do prefer the modern mnemonic with passphrase.
I've implemented the legacy 32 byte secret because I had troubles with registering a node with the modern
hsm_secret. It Might have been an issue with the signer not being recently enough. Let me investigate.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it is
greenlight/libs/gl-client/src/signer/mod.rs
Line 120 in 62c8174
The current signer only reads the first 32 bytes from the
hsm_secretfile.Ok. It seems a bit more of work is needed to make the Signer work with the modern
hsm_secretformat.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Aah yep, that won't work for the modern hsms unfortunately :( Let me know if you would like to pair on this 😄