Skip to content

feat(cli): support per-keystore passwords for validator import#9236

Open
JackCC703 wants to merge 4 commits intoChainSafe:unstablefrom
JackCC703:JackCC703/import-keystore-passwords
Open

feat(cli): support per-keystore passwords for validator import#9236
JackCC703 wants to merge 4 commits intoChainSafe:unstablefrom
JackCC703:JackCC703/import-keystore-passwords

Conversation

@JackCC703
Copy link
Copy Markdown

Motivation

The current validator import flow supports either interactive password entry or a single shared password file for all keystores. That makes non-interactive imports awkward for users who have multiple keystores encrypted with different passwords.

This PR adds a per-keystore password file workflow so validator keystores can be imported in batch without requiring all of them to share the same password.

Description

  • add a new --importKeystoresPasswords CLI option for per-keystore password files
  • keep existing --importKeystoresPassword behavior unchanged for backward compatibility
  • make --importKeystoresPassword and --importKeystoresPasswords mutually exclusive
  • expect per-keystore password files to be named 0x<validator-public-key-hex>.txt
  • fail fast when --importKeystoresPasswords does not point to a valid directory
  • improve error messages when a keystore cannot be parsed while resolving its password file
  • update validator import docs
  • add unit and e2e coverage for the new import flow and related validation paths

Closes #5249

Testing

  • pnpm lint
  • pnpm check-types
  • pnpm test:unit
  • pnpm docs:lint

AI Assistance Disclosure

This PR was written primarily with Codex assistance. I used Codex for code generation, test additions and debugging. I reviewed the final changes and validated them locally before submission.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces the --importKeystoresPasswords CLI flag, enabling the use of per-keystore password files during validator import. The changes include updates to the documentation, CLI option definitions, and the core import logic to support mapping keystores to specific password files based on their public keys. Additionally, comprehensive unit and E2E tests were added. Feedback was provided to lowercase the public key hex when constructing password filenames to ensure compatibility across case-sensitive filesystems.

throw new YargsError(`Failed to read keystore ${keystorePath}: ${e instanceof Error ? e.message : String(e)}`);
}

const passwordFilepath = path.join(passwordsDir, `${pubkeyHex}.txt`);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

medium

To ensure compatibility with case-sensitive filesystems (like Linux), it is recommended to lowercase the public key hex when constructing the password filename. While EIP-2335 keystores usually contain lowercase hex, some tools might produce mixed-case strings, which would lead to file-not-found errors if the user created the password file with a different case.

Suggested change
const passwordFilepath = path.join(passwordsDir, `${pubkeyHex}.txt`);
const passwordFilepath = path.join(passwordsDir, `${pubkeyHex.toLowerCase()}.txt`);

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I changed this to always look up the password file using the lowercased pubkey. That keeps the implementation simple and avoids the extra directory scan. I also reduced the test coverage to the mixed-case keystore pubkey case.

@JackCC703 JackCC703 marked this pull request as ready for review April 19, 2026 12:52
@JackCC703 JackCC703 requested a review from a team as a code owner April 19, 2026 12:52
…t-keystore-passwords

# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@JackCC703
Copy link
Copy Markdown
Author

This PR is ready for review.
If needed, could someone please approve the workflows for this PR? Thanks.

@JackCC703
Copy link
Copy Markdown
Author

Hi maintainers, CI is green now and the bot comment has been addressed.

Could someone take a look when available? This implements #5249 for per-keystore password files during validator import. Thanks!

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.

Support different passwords for each keystore

1 participant