Add operator_key crate to bundle key serialization functionality#390
Merged
mergify[bot] merged 6 commits intosigp:unstablefrom Jul 1, 2025
Merged
Add operator_key crate to bundle key serialization functionality#390mergify[bot] merged 6 commits intosigp:unstablefrom
operator_key crate to bundle key serialization functionality#390mergify[bot] merged 6 commits intosigp:unstablefrom
Conversation
Member
Author
|
will add some tests |
|
This pull request has merge conflicts. Could you please resolve them @dknopik? 🙏 |
mergify
bot
added
waiting-on-author
and removed
ready-for-review
mergify
bot
added
ready-for-review
Zacholme7
reviewed
Jun 27, 2025
| // Encode them to onchain format | ||
| let private_pem_encoded = Zeroizing::new(BASE64_STANDARD.encode(&private_pem)); | ||
| let public_pem_encoded = BASE64_STANDARD.encode(&public_pem); | ||
| let public_pem_encoded = operator_key::public::to_base64(&private_key)?; |
Member
There was a problem hiding this comment.
is there a mixup here? Setting public_pem_encoded to the encoding of private_key
Member
Author
There was a problem hiding this comment.
operator_key::public::to_base64 takes a public or private key, as both have the needed information to encode the public key. This is why I wrote the full path here to make clear what kind of key we are encoding here.
I'll add some doc comments to the new functions to explain the exact behaviour, have been meaning to do that anyway but forgot.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new operator_key crate for unified handling of operator key formats and updates existing tools to use it.
- Added
operator_keycrate supporting encrypted, legacy, public, and unencrypted key formats. - Updated
keygen,keysplit, andclientto replace ad-hoc parsing/serialization withoperator_keyAPIs. - Removed deprecated
parse_rsautility and refactoredssv_typesto depend onoperator_key.
Reviewed Changes
Copilot reviewed 20 out of 21 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| anchor/common/operator_key/Cargo.toml | Adds new crate; declares dependencies (incl. eth2_keystore) |
| anchor/common/operator_key/src/lib.rs | Defines ConversionError and modules |
| anchor/keygen/src/lib.rs | Switch to operator_key for public key encoding; expose SecurePassword |
| anchor/keysplit/src/cli.rs | Replace parse_rsa with operator_key::public::from_base64 |
| anchor/common/ssv_types/src/operator.rs | Refactor to use operator_key::public::from_base64 |
| anchor/client/src/lib.rs | Use operator_key::legacy for on-disk key parsing |
| anchor/common/ssv_types/Cargo.toml | Add operator_key dependency |
|
|
||
| #[derive(Zeroize, ZeroizeOnDrop, PartialEq, Debug)] | ||
| pub struct SecurePassword(String); | ||
| pub struct SecurePassword(pub String); |
There was a problem hiding this comment.
[nitpick] Exposing the inner String publicly bypasses the zeroizing safeguards and breaks encapsulation for a sensitive type. Consider keeping the field private and providing controlled accessors if needed.
Member
Author
There was a problem hiding this comment.
Reasonable point, but I'll revamp the password mechanism anyway to incorporate PW files with the next pr
Zacholme7
reviewed
Jul 1, 2025
| /// | ||
| /// [`Cipher::Aes128Ctr`] is used as cipher, and `scrypt` as constructed by [`default_kdf`] is | ||
| /// used as key derivation function. | ||
| pub fn encrypt(key: &Rsa<Private>, password: &str) -> Result<EncryptedKey, EncryptionError> { |
Member
There was a problem hiding this comment.
This isnt hooked up yet, right? Hooking this up was the follow up you mentioned?
Member
Author
There was a problem hiding this comment.
Correct, I split it for an easier review.
diegomrsantos
pushed a commit
to diegomrsantos/anchor
that referenced
this pull request
Sep 17, 2025
…igp#390) We have decided to switch to the same key format as used by SSV. The previous key format used by us (herein referred to as `legacy`) is to be supported in the next release version of Anchor, where we automatically convert to the new format on startup. Add a new crate called `operator_key`. There, we support the following: | Module Name| Format | Reading | Writing | Notes | |-|-|:-:|:-:|-| | `encrypted` | Encrypted private key | :white_check_mark: | :white_check_mark: | This is basically the `Crypto` object of an Ethereum keystore with a `pubKey` tacked on to it optionally | | `legacy` | Legacy Anchor key | :white_check_mark: | :x: | Writing not supported as Anchor `v0.2.0` will not write this kind of key anymore, just read it for conversion. | | `public` | Base64 public key | :white_check_mark: | :white_check_mark: | A base64 encoded PKCS8 PEM key with PKCS1 header :upside_down_face:. This is the format found onchain. | | `unencrypted` | Unencrypted private key | :white_check_mark: | :white_check_mark: | As can be specified in go-ssv's config. PKCS1 private key, base64 encoded. Correct header. | Existing functionality is adapted to use the new functions.
jking-aus
pushed a commit
to jking-aus/anchor
that referenced
this pull request
Oct 8, 2025
…igp#390) We have decided to switch to the same key format as used by SSV. The previous key format used by us (herein referred to as `legacy`) is to be supported in the next release version of Anchor, where we automatically convert to the new format on startup. Add a new crate called `operator_key`. There, we support the following: | Module Name| Format | Reading | Writing | Notes | |-|-|:-:|:-:|-| | `encrypted` | Encrypted private key | :white_check_mark: | :white_check_mark: | This is basically the `Crypto` object of an Ethereum keystore with a `pubKey` tacked on to it optionally | | `legacy` | Legacy Anchor key | :white_check_mark: | :x: | Writing not supported as Anchor `v0.2.0` will not write this kind of key anymore, just read it for conversion. | | `public` | Base64 public key | :white_check_mark: | :white_check_mark: | A base64 encoded PKCS8 PEM key with PKCS1 header :upside_down_face:. This is the format found onchain. | | `unencrypted` | Unencrypted private key | :white_check_mark: | :white_check_mark: | As can be specified in go-ssv's config. PKCS1 private key, base64 encoded. Correct header. | Existing functionality is adapted to use the new functions.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Issue Addressed
We have decided to switch to the same key format as used by SSV. The previous key format used by us (herein referred to as
legacy) is to be supported in the next release version of Anchor, where we automatically convert to the new format on startup.Proposed Changes
Add a new crate called
operator_key. There, we support the following:encryptedCryptoobject of an Ethereum keystore with apubKeytacked on to it optionallylegacyv0.2.0will not write this kind of key anymore, just read it for conversion.publicunencryptedExisting functionality is adapted to use the new functions.
Additional Info
Not in the scope of this PR is actually using the new functionality to use new keys. A second PR will adjust the key split tool and client start up logic to convert to and use the new key format.