Revise secrets-dir flag in the VC

[chong-he]

Issue Addressed

• Clean up VC secrets-dir flag and docs #5331

Proposed Changes

The secrets-dir flag in the validator client is not meant to be used together when starting the VC, i.e., lighthouse vc --secrets-dir ./path will not direct the VC to read the keystore password from the ./path. Instead, the secrets-dir will be created automatically when using the lighthouse account validator create command (reference), or when using the --http-store-passwords-in-secrets-dir and import the keystore via HTTP API. Therefore, I think it is better to hide the flag, and I also change the description to avoid confusion, and to deprecate the flag in the future.

When the secrets-dir is created using the above methods, it will always follow the --datadir of the VC. For example if VC is started with lighthouse vc --datadir ./folder, then a folder named secrets will be created in the directory ./folder/secrets alongside with a separate folder for the validator client ./folder/validators. Hence, this line is remained:

lighthouse/validator_client/src/cli.rs

Line 76 in 5ce1619

.conflicts_with("datadir")

Users who do not wish to store the keystore password in the validator_definitions.yml file can use the field voting_keystore_password_path and point to a desired directory. An amendment was made in the Lighthouse book to highlight this.

Edit: See the comments here: #5480 (comment)

[dapplion]

LGTM, definitely not the most UX friendly flag

[michaelsproul]

Oh, this isn't what I intended when I opened #5331. I think we should document the behaviour of the secrets-dir flag more accurately, and remove the conflicts_with so it can be used more easily

[chong-he]

Oh, this isn't what I intended when I opened #5331. I think we should document the behaviour of the secrets-dir flag more accurately, and remove the conflicts_with so it can be used more easily

I have revised the PR by removing the .conflicts_with("datadir") on the lighthouse vc and lighthouse account validator create. I have tested and here are the applications of the flag --secrets-dir:

1. With lighthouse vc --http --http-store-passwords-in-secrets-dir --datadir new --secrets-dir secrets
   We can have the datadir in one directory. During importing a new keystore via the HTTP API, the password file will be created automatically and stored under another directory set with --secrets-dir.

2. With lighthouse vc –datadir new –secrets-dir secrets where the folder secrets contain validator passwords.
   During key discovery (i.e., empty validator_definitions.yml file), the VC is able to import validator keystores and loaded the validators, pointing the voting_keystore_password_path to --secrets-dir. This is provided that the keystores were created using lighthouse account validator create.

3. With lighthouse account validator create :
   lighthouse --network mainnet account validator create --wallet-name wally --wallet-password wally.pass --count 6 --secrets-dir secrets --datadir new

   This however does not create the secrets folder under --secrets-dir. I have removed the .conflicts_with("datadir"), but the secrets folder is created in the same directory as datadir (i.e., in new/secrets). Although this is not a widely used function, but for transparency I will mention it here. I don’t think this is the expected outcome, as from the help text:

     --secrets-dir <SECRETS_DIR>
           The path where the validator keystore passwords will be stored. Defaults to ~/.lighthouse/{network}/secrets
   

   I will need some help here to fix this.

   It should be noted that if the validator_definitions.yml file already contains the keystore information, the --secrets-dir will not be effective. i.e., one cannot start the VC with lighthouse vc --secrets-dir and expects the VC to read the password from the --secrets-dir. This is another issue that will be opened afterwards. This usage is still desirable as one will not need to provide the voting_keystore_password_path in the validator_definitions.yml. This is helpful as entering voting_keystore_password_path could be manual, and inconvenient for a large number of validator keys. Having a flag --secrets-dir to direct the VC to read the password from the path solves this inconvenience. To achieve this, we would need the VC to allow not having both voting_keystore_password_pat and voting_keystore_password in the validator_definitions.yml, where currently either of these two fields is required: https://lighthouse-book.sigmaprime.io/validator-management.html#fields

[michaelsproul]

I think we should delete this line

[michaelsproul]

Thanks CK!

Lets open an issue for the lighthouse account create bug. I think we probably don't need to solve it if we deprecate the lighthouse account create command, but it's good to have a record of it until that deprecation is complete.

[michaelsproul]

@Mergifyio queue

[mergify]

queue

✅ The pull request has been merged automatically

The pull request has been merged automatically at 5c30afb