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

Unable to import keys exported from Prysm #3941

Closed
yorickdowne opened this issue Apr 20, 2022 · 9 comments · Fixed by #3970
Closed

Unable to import keys exported from Prysm #3941

yorickdowne opened this issue Apr 20, 2022 · 9 comments · Fixed by #3970

Comments

@yorickdowne
Copy link

Describe the bug

  1. Prysm saves the exported keys as keystore-N.json, with N an integer starting at 0. Lodestar does not recognize this filename format and will display "no keystores found".

  2. When renaming the file to keystore-m_12381_3600_0_0_0-1650442778.json, Lodestar finds it but fails to import it:

 ✖ Error: : should have required property 'path'
    at Object.validateKeystore (/usr/app/node_modules/@chainsafe/bls-keystore/lib/functional.js:111:15)
    at Function.fromObject (/usr/app/node_modules/@chainsafe/bls-keystore/lib/class.js:46:22)
    at Function.parse (/usr/app/node_modules/@chainsafe/bls-keystore/lib/class.js:53:25)
    at Object.handler (/usr/app/node_modules/@chainsafe/lodestar-cli/src/cmds/account/cmds/validator/import.ts:104:33)
    at Object.runCommand (/usr/app/node_modules/yargs/build/index.cjs:446:48)
    at Object.parseArgs [as _parseArgs] (/usr/app/node_modules/yargs/build/index.cjs:2697:57)
    at Object.runCommand (/usr/app/node_modules/yargs/build/index.cjs:410:36)
    at Object.parseArgs [as _parseArgs] (/usr/app/node_modules/yargs/build/index.cjs:2697:57)
    at Object.runCommand (/usr/app/node_modules/yargs/build/index.cjs:410:36)
    at Object.parseArgs [as _parseArgs] (/usr/app/node_modules/yargs/build/index.cjs:2697:57)

The contents of keystore-0.json as created by Prysm for a Prater validator are:

{
        "crypto": {
                "checksum": {
                        "function": "sha256",
                        "message": "2f19b909311ea48051bcbdcf3d5dbe333b9cc1b3d1111daf9b0debdb87fece75",
                        "params": {}
                },
                "cipher": {
                        "function": "aes-128-ctr",
                        "message": "12f902e22ec3f6b681682b98df514d78f8d0dc90eadd24b87ad16d55e29b8073",
                        "params": {
                                "iv": "8ca56a4c09d0eb534854d27c76433304"
                        }
                },
                "kdf": {
                        "function": "pbkdf2",
                        "message": "",
                        "params": {
                                "c": 262144,
                                "dklen": 32,
                                "prf": "hmac-sha256",
                                "salt": "d1b006046f2f40d2665b1c3a26ad0b6731d504f54deb36105125b22290b5cfca"
                        }
                }
        },
        "uuid": "66169f02-9160-42b5-883a-7efa3fb46ee5",
        "pubkey": "8ed78a5495b54d5b6cc8bf170534ecb633b9694fba121ca680744fa9633f1b67cc77c045f88a6f97be781fe6c2867646",
        "version": 4,
        "name": "keystore"
}
@dadepo
Copy link
Contributor

dadepo commented Apr 20, 2022

This looks like the keystore generated by Prysm does not follow the schema as defined in the specification here

In the JSON schema, path is specified as required, and we validate this in Lodestar.

@dadepo
Copy link
Contributor

dadepo commented Apr 20, 2022

Looks like Prysm added path since February as can be seen from this PR here. @yorickdowne You may want to check the version you are running?

@yorickdowne
Copy link
Author

Yeah Prysm needs a new release. That PR isn’t in GA yet. Thanks!!

@dapplion
Copy link
Contributor

Prysm saves the exported keys as keystore-N.json, with N an integer starting at 0. Lodestar does not recognize this filename format and will display "no keystores found".

Hey @yorickdowne ! Thank you for reporting, could you detail the command and directory structure that caused "no keystores found"? Lodestar should import any .json regardless of name.

@yorickdowne yorickdowne reopened this May 2, 2022
@yorickdowne
Copy link
Author

      - node
      - --max-old-space-size=8192
      - /usr/app/node_modules/.bin/lodestar
      - account
      - validator
      - import
      - --rootDir
      - /var/lib/lodestar/validators
      - --directory
      - /val_keys
      - --network
      - ${NETWORK}

There's an entry script that does this:

  mkdir /val_keys
  cp /validator_keys/* /val_keys/
  chown user:user /val_keys/*

And validator_keys looks like this:

total 8.0K
-r--r----- 1 ubuntu ubuntu 1.4K May  2 04:54 deposit_data-1651467260.json
-r--r----- 1 ubuntu ubuntu  710 May  2 05:25 keystore-0.json

If I change the filename like this:

total 8.0K
-r--r----- 1 ubuntu ubuntu 1.4K May  2 04:54 deposit_data-1651467260.json
-r--r----- 1 ubuntu ubuntu  710 May  2 05:25 keystore-m_12381_3600_1_0_0-1651467259.json

then import works.

@philknows
Copy link
Member

@dadepo @dapplion Is there some requirement for filenames to be in a certain format for import? If so, what is the reasoning for it? Isn't there another way to detect that the file is proper without having to have the keys in a certain filename?

@dadepo
Copy link
Contributor

dadepo commented May 2, 2022

@dapplion @philknows

I took a look into this and this is what I found out:

We are explicitly matching for the format exported by the eth2.0-deposit-cli library, as can be seen here

But in the cli documentation we state that file name which contains 'keystore' and has the '.json' extension will be attempted to be imported which is not what the code is doing.

From the comment on the code, it looks like sticking with this format is a preventive measure. So the question now is: do we stick with this format and update the documentation appropriately? or do we update the code to be more lenient (making it inline with what the documentation says)

@philknows
Copy link
Member

IMO, it's a pretty restrictive requirement to have it follow the exact format exported by the eth2.0-deposit-cli especially because the filename is pretty garbled with numbers and is poor UX as other implementations won't export keys in the same filename format.

My opinion would be to update the code to be more lenient to just keystore and .json as the only requirements. I would also test to see what happens if you import a keystore.json that is not properly formatted just to ensure it gets rejected.

Would those requirements affect your implementation in any negative way @yorickdowne ?

@yorickdowne
Copy link
Author

That’d be perfect. Keystores are likely to come from staking-deposit-cli or an export from another client

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 a pull request may close this issue.

4 participants