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

Clean up VC secrets-dir flag and docs #5331

Closed
michaelsproul opened this issue Feb 29, 2024 · 3 comments
Closed

Clean up VC secrets-dir flag and docs #5331

michaelsproul opened this issue Feb 29, 2024 · 3 comments
Assignees
Labels
docs Documentation UX-and-logs val-client Relates to the validator client binary

Comments

@michaelsproul
Copy link
Member

Description

@chong-he noticed that the VC flag --secrets-dir conflicts with datadir:

.arg(
Arg::with_name("secrets-dir")
.long("secrets-dir")
.value_name("SECRETS_DIRECTORY")
.help(
"The directory which contains the password to unlock the validator \
voting keypairs. Each password should be contained in a file where the \
name is the 0x-prefixed hex representation of the validators voting public \
key. Defaults to ~/.lighthouse/{network}/secrets.",
)
.takes_value(true)
.conflicts_with("datadir")
)

This is despite SigP using them together in our infra!

The trick is to put the datadir flag first:

# This works
lighthouse --datadir $PATH vc --secrets-dir $SECRETS

# This doesn't
lighthouse vc --datadir $PATH --secrets-dir $SECRETS

It seems we should remove the conflicts_with clause.

Additionally, secrets-dir doesn't seem to function as its docs describe:

The directory which contains the password to unlock the validator
voting keypairs. Each password should be contained in a file where the
name is the 0x-prefixed hex representation of the validators voting public
key. Defaults to ~/.lighthouse/{network}/secrets.

The VC uses the flag for two things:

  1. During validator "discovery" of keys that are not already present in the validator definitions YAML, here:
    let voting_keystore_password_path = Some(default_keystore_password_path(
    &keystore,
    secrets_dir.as_ref(),
    ))
    .filter(|path| path.exists());

    If a key already has a validator definition, then the VC will not attempt to read the password from the secrets-dir, it will expect voting_keystore_password_path to have been set accordingly.
  2. When creating validators via the HTTP API and the --http-store-passwords-in-secrets-dir is used.

The docs make it seem like Lighthouse might try to read the password from the secrets-dir if no password or path is present in the definitions, but this is not the case.

Version

v5.0.0

Steps to resolve

  • Remove conflicts_with.
  • Update the docs to clarify the behaviour.
@michaelsproul michaelsproul added val-client Relates to the validator client binary docs Documentation UX-and-logs labels Feb 29, 2024
@chong-he
Copy link
Member

I would like to work on this

@michaelsproul
Copy link
Member Author

Another option:

  • Use the secrets-dir even when the validator definitions contains an entry for a key

@chong-he
Copy link
Member

Closed by #5480

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation UX-and-logs val-client Relates to the validator client binary
Projects
None yet
Development

No branches or pull requests

2 participants