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

Existing Mnemonic #123

Merged
merged 31 commits into from
Nov 3, 2020
Merged

Existing Mnemonic #123

merged 31 commits into from
Nov 3, 2020

Conversation

CarlBeek
Copy link
Collaborator

Adds support for existing mnemonics (with automatic language detection) and deriving keys from a non-0 index to start.

#94 and #122 are others' attempts at this, but former doesn't check the mnemonic and the latter doesn't offer start-indices.

@unexpectedbit
Copy link

It generates different keys when inputting the mnemonic phrase (with no password) that was generated using the existing tool from master branch.

@CarlBeek CarlBeek changed the base branch from master to dev October 1, 2020 08:30
@torfbolt
Copy link

torfbolt commented Oct 4, 2020

@unexpectedbit did you compare to keys generated with the current master, or with a previous master? Afaik there were recent changes to the key derivation algorithm, so old keys might not be reproducible even without this PR.

@unexpectedbit
Copy link

unexpectedbit commented Oct 6, 2020

After manually modifying the master branch files with changes from this pull request, the correct (same) keys are generated!

@duelinggalois
Copy link

This may be due to the changes to the key derivation algorithm mentioned above, but I was unable to create the same keys, I was however able to do so using the fork that @torfbolt created.

@unexpectedbit
Copy link

@rev3ks Did you try generating the keys using the "existing_mnemonic" branch files or did you modify "master" branch files with code changes from this PR? I think that the rest of code in files of "existing_mnemonic" branch is obsolete and therefore generates different keys.

@duelinggalois
Copy link

duelinggalois commented Oct 10, 2020

I used this branch not master and ran ./deposit.sh existing-mnemonic --mnemonic "..." --num_validators 1 --chain medalla, the public key was the same, but the signature and withdraw_credentials did not.

* dev:
  Clean checksum output
  Special snowflake native shasum for macos
  Persist envars by making them a part of the pyinstaller process in makefile
  Add Windows build SHA1 -> SHA256
  Revert, the 2nd
  revert change on venv activation
  Adds SHA256 checksums to circleci and makes builds determanistic.
@CarlBeek CarlBeek marked this pull request as ready for review October 21, 2020 12:46
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

My first pass. 👀 Good job!

eth2deposit/cli/existing_mnemonic.py Outdated Show resolved Hide resolved
eth2deposit/key_handling/key_derivation/mnemonic.py Outdated Show resolved Hide resolved
eth2deposit/key_handling/key_derivation/mnemonic.py Outdated Show resolved Hide resolved
eth2deposit/key_handling/key_derivation/mnemonic.py Outdated Show resolved Hide resolved
eth2deposit/key_handling/key_derivation/mnemonic.py Outdated Show resolved Hide resolved
eth2deposit/key_handling/key_derivation/mnemonic.py Outdated Show resolved Hide resolved
tests/test_cli.py Outdated Show resolved Hide resolved
eth2deposit/credentials.py Outdated Show resolved Hide resolved
eth2deposit/cli/new_mnemonic.py Show resolved Hide resolved
@hwwhww hwwhww mentioned this pull request Oct 27, 2020
@fabdarice
Copy link

I was not able to re-generate the same public keys using the seed.
To be fair, I remember generating those prior to medalla launch so it's been a while ago..

@hwwhww
Copy link
Contributor

hwwhww commented Nov 1, 2020

@fabdarice

The BLS library version was IETF draft 02 when Medalla was launched, but now it's IETF BLS draft 04. Unfortunately, the KeyGen algorithm was changed when 02->04 so you're unable to use this PR to re-generate the early Medalla keys. 😢

The IETF BLS draft 04 will be the BLS version for phase 0 launch. You can also use it to re-generate the Zinken testnet keys.

Sorry for the confusion!

@fabdarice
Copy link

@fabdarice

The BLS library version was IETF draft 02 when Medalla was launched, but now it's IETF BLS draft 04. Unfortunately, the KeyGen algorithm was changed when 02->04 so you're unable to use this PR to re-generate the early Medalla keys.

The IETF BLS draft 04 will be the BLS version for phase 0 launch. You can also use it to re-generate the Zinken testnet keys.

Sorry for the confusion!

Yep, saw that. No problem. This PR will come in handy for those who lost their validator keys.

Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

README.md Show resolved Hide resolved
eth2deposit/cli/generate_keys.py Outdated Show resolved Hide resolved
eth2deposit/cli/existing_mnemonic.py Outdated Show resolved Hide resolved
eth2deposit/cli/generate_keys.py Outdated Show resolved Hide resolved
CarlBeek and others added 4 commits November 3, 2020 12:20
`type=click.IntRange(0, 2**32)` got corrected to `type=click.IntRange(0, 2**32 - 1)`

Co-authored-by: Hsiao-Wei Wang <hwwang156@gmail.com>
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
@CarlBeek CarlBeek requested a review from hwwhww November 3, 2020 16:53
Copy link
Contributor

@hwwhww hwwhww left a comment

Choose a reason for hiding this comment

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

I pushed some minor fixes her and it LGTM 👍

#143 added the cli key regeneration test case.

@CarlBeek CarlBeek merged commit 2d434a3 into dev Nov 3, 2020
@hwwhww hwwhww mentioned this pull request Nov 3, 2020
@antliv
Copy link

antliv commented Nov 4, 2021

U burnt me

@CarlBeek CarlBeek deleted the existing_mnemonic branch March 16, 2022 11:18
everhusk pushed a commit to earthwallet/earth-wallet-cli that referenced this pull request Aug 3, 2023
everhusk pushed a commit to earthwallet/earth-wallet-cli that referenced this pull request Aug 3, 2023
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

7 participants