Skip to content
This repository has been archived by the owner on Apr 15, 2024. It is now read-only.

feat: adds support for BIP39 mnemonics #577

Merged
merged 12 commits into from
Nov 7, 2023

Conversation

rach-id
Copy link
Member

@rach-id rach-id commented Nov 5, 2023

Overview

Closes #495

Checklist

  • New and updated code has appropriate documentation
  • New and updated code has new and/or updated testing
  • Required CI checks are passing
  • Visual proof for any user facing features like CLI or documentation updates
  • Linked issues closed with keywords

Summary by CodeRabbit

  • New Features
    • Added the ability to derive a private key from a mnemonic phrase.
    • Introduced the feature to import an EVM address from a 24-word BIP39 mnemonic phrase.
  • Tests
    • Added tests to validate the functionality of deriving a private key from a mnemonic phrase.

@rach-id rach-id self-assigned this Nov 5, 2023
@rach-id rach-id requested a review from evan-forbes as a code owner November 5, 2023 21:20
Copy link
Contributor

coderabbitai bot commented Nov 5, 2023

Walkthrough

The changes introduced in the evm.go and evm_test.go files primarily revolve around the addition of mnemonic phrase functionality for EVM addresses. This includes the ability to generate a private key from a mnemonic phrase, import an EVM address from a mnemonic phrase, and the corresponding tests to ensure the functionality works as expected.

Changes

File Changes
cmd/blobstream/keys/evm/evm.go
  • Added imports for bufio, crypto/ecdsa, errors, github.com/cosmos/cosmos-sdk/client/input, github.com/cosmos/cosmos-sdk/crypto/hd, and github.com/cosmos/go-bip39.
  • Added constant mnemonicEntropySize.
  • Added function GetBIP39Passphrase() to get the BIP39 passphrase from the user.
  • Added function MnemonicToPrivateKey() to derive a private key from a mnemonic.
  • Modified the Add command to generate a mnemonic and import the account using the mnemonic and passphrase.
  • Added the ImportMnemonic command to import an EVM address from a 24-word BIP39 mnemonic phrase.
  • Added the GetNewPassphrase() function to get a new passphrase from the user.
  • Modified the GetBIP39Passphrase() function to limit the passphrase length to 100 characters.
  • Updated the GetNewPassphrase() function to print a note about the store encryption passphrase.
  • Updated the GetBIP39Passphrase() function to print a note about the BIP39 passphrase. |
    | cmd/blobstream/keys/evm/evm_test.go |
  • Added a test function TestMnemonicToPrivateKey to the evm_test package. This function tests the generation of private keys using mnemonics. It includes test cases for valid mnemonics with and without a passphrase, as well as an invalid mnemonic case. The test compares the generated private key and address with the expected values. The evm.MnemonicToPrivateKey function is called within the test to generate the private key. The diff does not involve any modifications to the logic or functionality. |

Poem

🍂 As the leaves fall, so does the old code,
Making way for the new, down the mnemonic road.
EVM addresses dance, in the crisp November air,
With BIP39 phrases, they've got a new flair.

🐇 Hop, hop, hop, goes the CodeRabbit,
Celebrating the changes, in its usual habit.
On this day in history, a change was made,
In the world of code, a new foundation laid.

🌟 So here's to the devs, working night and day,
Your efforts and passion, truly light the way.
As the moon rises, over the autumn sky,
We celebrate your work, with a contented sigh. 🌙


Tips

Chat with CodeRabbit Bot (@coderabbitai)

  • If you reply to a review comment from CodeRabbit, the bot will automatically respond.
  • To engage with CodeRabbit bot directly around the specific lines of code in the PR, mention @coderabbitai in your review comment
  • Note: Review comments are made on code diffs or files, not on the PR overview.
  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger a review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai help to get help.
  • @coderabbitai resolve to resolve all the CodeRabbit review comments.

Note: For conversation with the bot, please use the review comments on code diffs or files.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • The JSON schema for the configuration file is available here.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/coderabbit-overrides.v2.json

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between cae6432 and 93263a0.
Files ignored due to filter (1)
  • go.mod
Files selected for processing (2)
  • cmd/blobstream/keys/evm/evm.go (6 hunks)
  • cmd/blobstream/keys/evm/evm_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
  • cmd/blobstream/keys/evm/evm_test.go
Additional comments: 4
cmd/blobstream/keys/evm/evm.go (4)
  • 1-18: The new imports look fine. Ensure that all the imported packages are being used in the code.

  • 28-28: The constant mnemonicEntropySize is set to 256, which is the standard entropy size for a 24-word BIP39 mnemonic. This is correct.

  • 414-486: The ImportMnemonic command is correctly implemented. It prompts the user for a mnemonic, validates it, derives a private key from it, and imports the corresponding EVM address into the keystore. The command correctly handles errors and edge cases.

  • 639-662: The MnemonicToPrivateKey function correctly derives a private key from a BIP39 mnemonic. It uses the correct derivation path for Ethereum (m/44'/60'/0'/0). The function correctly handles errors and edge cases.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 1

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 93263a0 and cef0b12.
Files selected for processing (1)
  • cmd/blobstream/keys/evm/evm.go (6 hunks)
Additional comments: 2
cmd/blobstream/keys/evm/evm.go (2)
  • 639-662: The MnemonicToPrivateKey function has been added to derive a private key from a mnemonic phrase. The function uses the Ledger derivation path to generate the first private key. This function looks good and follows the BIP39 standard for generating a private key from a mnemonic.

  • 414-488: The ImportMnemonic command has been added to import an EVM address from a 24-word BIP39 mnemonic phrase. The function asks for the mnemonic and passphrase from the user, derives the private key from the mnemonic, and imports the corresponding EVM address into the keystore. This function looks good and follows the BIP39 standard for importing a mnemonic.

Comment on lines 92 to 124
}
}

account, err := s.EVMKeyStore.NewAccount(passphrase)
// read entropy seed straight from tmcrypto.Rand and convert to mnemonic
entropySeed, err := bip39.NewEntropy(mnemonicEntropySize)
if err != nil {
return err
}

mnemonic, err := bip39.NewMnemonic(entropySeed)
if err != nil {
return err
}

// get the private key
ethPrivKey, err := MnemonicToPrivateKey(mnemonic, passphrase)
if err != nil {
return err
}

account, err := s.EVMKeyStore.ImportECDSA(ethPrivKey, passphrase)
if err != nil {
return err
}

logger.Info("account created successfully", "address", account.Address.String())

fmt.Println("\n\n**Important** write this mnemonic phrase in a safe place." +
"\nIt is the only way to recover your account if you ever forget your password.")
fmt.Printf("\n%s\n\n", mnemonic)
return nil
},
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The Add function has been updated to generate a BIP39 mnemonic and derive a private key from it. The mnemonic is then printed to the console. This is a security risk as the mnemonic can be seen by anyone with access to the console logs. Consider removing the print statement or replacing it with a more secure method of delivering the mnemonic to the user.

- fmt.Printf("\n%s\n\n", mnemonic)

Commitable suggestion

[!IMPORTANT]
Carefully review the code before committing. Make sure it correctly replaces the highlighted code, has no missing lines and indentaion issues.

Suggested change
}
}
account, err := s.EVMKeyStore.NewAccount(passphrase)
// read entropy seed straight from tmcrypto.Rand and convert to mnemonic
entropySeed, err := bip39.NewEntropy(mnemonicEntropySize)
if err != nil {
return err
}
mnemonic, err := bip39.NewMnemonic(entropySeed)
if err != nil {
return err
}
// get the private key
ethPrivKey, err := MnemonicToPrivateKey(mnemonic, passphrase)
if err != nil {
return err
}
account, err := s.EVMKeyStore.ImportECDSA(ethPrivKey, passphrase)
if err != nil {
return err
}
logger.Info("account created successfully", "address", account.Address.String())
fmt.Println("\n\n**Important** write this mnemonic phrase in a safe place." +
"\nIt is the only way to recover your account if you ever forget your password.")
fmt.Printf("\n%s\n\n", mnemonic)
return nil
},
}
}
}
// read entropy seed straight from tmcrypto.Rand and convert to mnemonic
entropySeed, err := bip39.NewEntropy(mnemonicEntropySize)
if err != nil {
return err
}
mnemonic, err := bip39.NewMnemonic(entropySeed)
if err != nil {
return err
}
// get the private key
ethPrivKey, err := MnemonicToPrivateKey(mnemonic, passphrase)
if err != nil {
return err
}
account, err := s.EVMKeyStore.ImportECDSA(ethPrivKey, passphrase)
if err != nil {
return err
}
logger.Info("account created successfully", "address", account.Address.String())
fmt.Println("\n\n**Important** write this mnemonic phrase in a safe place." +
"\nIt is the only way to recover your account if you ever forget your password.")
// fmt.Printf("\n%s\n\n", mnemonic)
return nil
},
}

@codecov-commenter
Copy link

codecov-commenter commented Nov 5, 2023

Codecov Report

Merging #577 (c1e3c9d) into main (616068c) will decrease coverage by 6.88%.
Report is 1 commits behind head on main.
The diff coverage is 8.88%.

❗ Current head c1e3c9d differs from pull request most recent head 8b68381. Consider uploading reports for the commit 8b68381 to get more accurate results

@@            Coverage Diff             @@
##             main     #577      +/-   ##
==========================================
- Coverage   34.84%   27.96%   -6.88%     
==========================================
  Files          27       29       +2     
  Lines        2230     2821     +591     
==========================================
+ Hits          777      789      +12     
- Misses       1360     1937     +577     
- Partials       93       95       +2     
Files Coverage Δ
cmd/blobstream/keys/evm/evm.go 2.25% <8.88%> (ø)

... and 1 file with indirect coverage changes

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between cef0b12 and dc24dd4.
Files selected for processing (1)
  • cmd/blobstream/keys/evm/evm.go (6 hunks)
Additional comments: 5
cmd/blobstream/keys/evm/evm.go (5)
  • 1-18: The added imports seem to be necessary for the new functionality. Ensure that all the imported packages are being used in the code.

  • 28-28: The constant mnemonicEntropySize is set to 256, which is the correct size for a 24-word BIP39 mnemonic.

  • 92-126: The Add function has been modified to generate a mnemonic phrase and import an EVM account using the mnemonic. The function now generates a mnemonic, derives a private key from it, and imports the account into the keystore. The mnemonic is then printed to the console. This is a significant change and should be tested thoroughly.

  • 415-495: The ImportMnemonic function has been added. This function allows the user to import an EVM address from a 24-word BIP39 mnemonic phrase. The function reads the mnemonic from user input, validates it, and then derives a private key from it. The account is then imported into the keystore. This is a significant change and should be tested thoroughly.

  • 646-669: The MnemonicToPrivateKey function has been added. This function derives a private key from a given mnemonic and passphrase. It uses the Ledger derivation path to generate the first private key. This function is used in both the Add and ImportMnemonic functions.

@rach-id rach-id marked this pull request as draft November 6, 2023 09:59
@rach-id rach-id marked this pull request as ready for review November 6, 2023 10:48
Copy link
Member Author

@rach-id rach-id left a comment

Choose a reason for hiding this comment

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

[note for reviewers]
It would be helpful to test the command while reviewing to see if the prompt messages are correct and intuitive.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between dc24dd4 and 30c2e86.
Files ignored due to filter (1)
  • go.mod
Files selected for processing (2)
  • cmd/blobstream/keys/evm/evm.go (7 hunks)
  • cmd/blobstream/keys/evm/evm_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
  • cmd/blobstream/keys/evm/evm_test.go
Additional comments: 11
cmd/blobstream/keys/evm/evm.go (11)
  • 1-18: The new imports are correctly added and are being used in the code. Ensure that these packages are added to the project dependencies.

  • 25-30: The new constant mnemonicEntropySize is correctly defined and used in the code.

  • 86-93: The function GetBIP39Passphrase() is correctly called and its return value is checked for errors. The user is correctly informed about the purpose of the passphrase.

  • 101-120: The generation of the mnemonic and the derivation of the private key from it are correctly implemented. The error handling is also correctly done.

  • 122-125: The import of the account is correctly done and the error is correctly handled.

  • 129-136: The mnemonic is correctly printed to the console. The condition to check whether to print the passphrase is correctly implemented.

  • 267-273: The ImportMnemonic command is correctly added to the importCmd command.

  • 435-504: The ImportMnemonic command is correctly implemented. The mnemonic is correctly read from the user input and its validity is checked. The passphrase is correctly read from the user input or from the command flags. The private key is correctly derived from the mnemonic and the account is correctly imported.

  • 637-656: The function GetNewPassphrase() is correctly implemented. The passphrase is correctly read from the user input and its confirmation is correctly checked. The loop correctly continues until the passphrase and its confirmation match.

  • 659-683: The function GetBIP39Passphrase() is correctly implemented. The passphrase is correctly read from the user input and its length is correctly checked. The passphrase confirmation is correctly checked. The loop correctly continues until the passphrase and its confirmation match or the passphrase length is correct.

  • 686-709: The function MnemonicToPrivateKey() is correctly implemented. The seed is correctly derived from the mnemonic and the passphrase. The master key is correctly computed from the seed. The private key is correctly derived from the master key. The Ethereum private key is correctly derived from the private key.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between dc24dd4 and 30c2e86.
Files ignored due to filter (1)
  • go.mod
Files selected for processing (2)
  • cmd/blobstream/keys/evm/evm.go (7 hunks)
  • cmd/blobstream/keys/evm/evm_test.go (1 hunks)
Files skipped from review due to trivial changes (1)
  • cmd/blobstream/keys/evm/evm_test.go
Additional comments: 11
cmd/blobstream/keys/evm/evm.go (11)
  • 1-18: The new imports seem to be used appropriately in the code. Ensure that these packages are added to the project dependencies.

  • 28-28: The constant mnemonicEntropySize is set to 256, which is the standard entropy size for a 24-word BIP39 mnemonic. This is correct.

  • 86-93: The function GetBIP39Passphrase() is used to get the BIP39 passphrase from the user. The user is informed about the importance of the passphrase. This is a good practice for user interaction.

  • 104-120: The mnemonic is generated using the bip39.NewEntropy and bip39.NewMnemonic functions. The private key is derived from the mnemonic using the MnemonicToPrivateKey function. This is the correct way to generate a mnemonic and derive a private key from it.

  • 122-125: The private key is imported into the keystore using the s.EVMKeyStore.ImportECDSA function. This is the correct way to import a private key into a keystore.

  • 129-136: The mnemonic is displayed to the user with a warning about its importance. This is a good practice for user interaction.

  • 267-271: The ImportMnemonic command is added to the importCmd command. This is the correct way to add a subcommand in Cobra.

  • 466-504: The ImportMnemonic command is implemented correctly. It gets the mnemonic from the user, validates it, gets the BIP39 passphrase, gets the store encryption passphrase, derives the private key from the mnemonic, and imports the private key into the keystore.

  • 637-655: The GetNewPassphrase function is updated to print a note about the store encryption passphrase. This is a good practice for user interaction.

  • 659-683: The GetBIP39Passphrase function is updated to limit the passphrase length to 100 characters and print a note about the BIP39 passphrase. This is a good practice for user interaction.

  • 686-709: The MnemonicToPrivateKey function is implemented correctly. It derives the master key from the mnemonic and passphrase using the bip39.NewSeedWithErrorChecking function, and then derives the private key from the master key using the hd.DerivePrivateKeyForPath function. This is the correct way to derive a private key from a mnemonic.

evan-forbes
evan-forbes previously approved these changes Nov 6, 2023
cmd/blobstream/keys/evm/evm.go Outdated Show resolved Hide resolved
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Review Status

Actionable comments generated: 0

Configuration used: CodeRabbit UI

Commits Files that changed from the base of the PR and between 30c2e86 and 8d6eda7.
Files selected for processing (1)
  • cmd/blobstream/keys/evm/evm.go (7 hunks)
Additional comments: 9
cmd/blobstream/keys/evm/evm.go (9)
  • 1-18: The new imports seem to be used appropriately in the code. Ensure that these packages are added to the project dependencies.

  • 28-28: The constant mnemonicEntropySize is set to 256, which is the standard entropy size for a 24-word BIP39 mnemonic. This is correct.

  • 86-93: The function GetBIP39Passphrase() is used to get the BIP39 passphrase from the user. Ensure that the passphrase is stored securely and not logged or exposed in any way.

  • 104-120: The Add command is modified to generate a mnemonic phrase and import the corresponding private key. This is a significant change and should be tested thoroughly.

  • 267-271: The ImportMnemonic command is added to import an EVM private key from a 24-word BIP39 mnemonic phrase. This is a significant change and should be tested thoroughly.

  • 429-506: The ImportMnemonic command is implemented correctly. It gets the mnemonic from the user, validates it, gets the BIP39 passphrase, and then uses the MnemonicToPrivateKey function to derive the private key from the mnemonic. The private key is then imported into the keystore. Ensure that the mnemonic and passphrase are stored securely and not logged or exposed in any way.

  • 637-656: The GetNewPassphrase function is updated to improve user interaction. It now asks the user to enter the passphrase twice and checks if they match. This is a good practice to prevent typos.

  • 659-683: The GetBIP39Passphrase function is implemented correctly. It gets the BIP39 passphrase from the user, validates its length, and asks the user to enter it twice to check if they match. This is a good practice to prevent typos.

  • 686-709: The MnemonicToPrivateKey function is implemented correctly. It derives a private key from a BIP39 mnemonic using the default derivation path. This function should be tested thoroughly.

Copy link
Member

@evan-forbes evan-forbes left a comment

Choose a reason for hiding this comment

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

👍

@rach-id rach-id merged commit 4f66954 into celestiaorg:main Nov 7, 2023
18 checks passed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support BIP39 mnemonics for keys
3 participants