Repository navigation
feat(validator): Web3Signer remote signing and the keymanager /eth/v1/remotekeys route - #776
pablodeymo wants to merge 5 commits into
Conversation
🤖 Claude Code ReviewPR 776 review: Web3Signer remote signingThe overall design is sound. Signing behind a short read guard with an Correctness / safety1. Signatures from the remote signer are never verified ( Any 96 bytes in the response are accepted and published. Failure modes:
Each of these makes the client gossip invalid attestations or blocks. That costs peer score, and for a block it wastes a proposal. Selection proofs are also fed straight into The fix is cheap: one 2. One slow signer stalls every attestation in the slot (
Suggested fixes:
3. Race between remote import and remote delete leaves a key signing that is not on disk (
The result is a key reported as 4. Inconsistent state on the If the store-side duplicate check fires, the remote definition has already been persisted. The file then says remote, memory says local, and the response says Performance5. Selection proofs are serial round trips, computed twice per duty
The same slot selection proof is also requested once at subscription time and again at aggregation time. Two changes would help:
6. Signing metric now includes network time
Protocol / Web3Signer compatibility7. Unconfirmed request types for gloas Check that Web3Signer actually supports 8. Test uses a pre-Bellatrix header ( For 9. No TLS backend check Confirm the workspace Smaller items
Automated review by Claude · claude-opus-5-5 · custom prompt |
🤖 Codex Code Review
Tests could not run: Rustup attempted to write to its read-only toolchain directory. Automated review by OpenAI Codex · gpt-6.1-sol · custom prompt |
🤖 Gemini Code ReviewOverall, this is a well-structured and well-tested implementation of Web3Signer remote signing and the Here are a few issues and improvements to consider:
Automated review by Gemini · gemini-4-argon · custom prompt |
🤖 Grok Code ReviewI'll read the keymanager import/delete paths and signing lock usage to verify the remote-signer changes. Automated review by Grok (xAI) · grok-4.7 · custom prompt |
🤖 DeepSeek Code ReviewOverall direction is sound, but I found a few issues worth addressing before merge.
Automated review by DeepSeek · deepseek-v4-pro · custom prompt |
🤖 Kimi Code Review
Otherwise, the change looks carefully done: store read guards are narrowed correctly, attestation/proposal guards still run before signing, attestation fork information uses the target epoch, and the block-header root construction preserves the signed block root. Automated review by Kimi (Moonshot AI) · kimi-k3.1-pro · custom prompt |
MegaRedHand
left a comment
MegaRedHand
left a comment
There was a problem hiding this comment.
Review of the Web3Signer and /eth/v1/remotekeys changes, 15 inline comments.
The main concern is liveness: a slow or unreachable signer can now stall duties for every validator, local keys included (subscriptions.rs, attestation.rs, aggregation.rs). The rest are correctness issues in the keymanager routes, two places where secrets can reach the logs, a docs claim about TLS trust roots that doesn't match the reqwest features in use, and some smaller performance items.
None of this was tested against a real Web3Signer. The BLOCK_V2 comment on signing.rs is from memory of its schema, not checked against its source.
| .collect() | ||
| }; | ||
| let mut subscriptions = Vec::with_capacity(duties.len()); | ||
| for duty in duties { |
There was a problem hiding this comment.
subscribe now makes one remote selection-proof call per duty, one after another, inside refresh_epoch on the main duty loop, with no overall deadline. A slow or unreachable signer therefore stalls duties for every validator, local keys included.
Example: 100 validators, two epochs of duties held, signer host blackholed so each call waits the full 8s REQUEST_TIMEOUT. That's 200 × 8s ≈ 27 minutes stuck in refresh_epoch, about 133 slots of missed proposals and attestations for every key. Even a healthy signer at ~10ms per call adds ~2s before the boundary slot's proposal.
The proposal and attestation paths have slot-budget timeouts; this one has none. Running these calls concurrently with a deadline, or moving them off the duty loop, would fix it.
There was a problem hiding this comment.
Fixed in a13eda9. subscribe asks for every selection proof at once through signing::sign_all, each bounded by SIGNING_BUDGET (2s), so an unreachable signer costs the epoch boundary 2s at most instead of one timeout per duty. The proofs that arrive are remembered, so the aggregation tick does not ask again.
| // behind it for as long as the guard was held. | ||
| let signatures = { | ||
| let _timing = crate::metrics::time_signing(); | ||
| futures::future::join_all( |
There was a problem hiding this comment.
join_all waits for the slowest signature before anything is published, so one unresponsive signer delays or loses the whole slot's batch, including local keys and keys on other, healthy signers.
Example: 50 local keys plus 1 key on a hung Web3Signer. The hung request takes the full 8s timeout, which is about all of remaining_in(slot) after attestation data is fetched at 1/3 slot. The attest timeout then drops the future, and the 50 local signatures, already computed and already recorded by the guard, are never submitted and can't be retried. Before this change a remote failure couldn't affect local keys.
A per-request deadline shorter than the slot budget, publishing whatever finished, would keep one signer from taking down the rest.
There was a problem hiding this comment.
Fixed in a13eda9. Attestations go through sign_all: each signature has 2s, one still outstanding then is a signing failure for that key alone, and the batch is published with whatever arrived, local keys and other signers' keys included.
| .collect() | ||
| }; | ||
| let mut selected: Vec<(AttesterDutyDto, _)> = Vec::new(); | ||
| for duty in duties { |
There was a problem hiding this comment.
Only attestations were made concurrent. Here, selection proofs and then the aggregate-and-proof signatures are still awaited one round trip at a time, with only the last third of the slot left. With many remote keys the timeout abandons the batch and every aggregate is lost. sync_committee::publish_messages, contributions and payload_attestation have the same shape.
Each selection proof is also requested again per duty: once by subscribe at the epoch boundary and again here, so 2–3 remote calls per duty per epoch where one cached result would do.
There was a problem hiding this comment.
Fixed in a13eda9. Aggregation (selection proofs, then the aggregate-and-proofs), sync committee messages, sync selection proofs, contributions and payload attestations all sign their batch through sign_all. SigningContext::sign_selection_proof remembers an attestation selection proof once produced, so the one subscribe asked for is reused here.
| // handler and the keystores import both hold `definitions_lock`. | ||
| let mut prepared = Vec::with_capacity(request.remote_keys.len()); | ||
| { | ||
| let store = context.store.read().await; |
There was a problem hiding this comment.
This read guard is held across the whole prepare_import loop, and each key in that loop does a full definitions.save() (whole YAML file rewritten, then renamed) and builds a throwaway reqwest Client. There's no batch cap; the keystores route caps at 100.
Example: importing 5,000 keys a signer already holds, which is a normal migration, rewrites a growing file 5,000 times (O(N²) bytes written), synchronously in an async handler. Meanwhile a concurrent keystores import queued on store.write() behind this guard blocks new readers (tokio's RwLock lets a queued writer do that), so every duty's store.read() in SigningContext::sign waits for the whole batch and attestations and proposals stall.
Capping the batch, saving the definitions once per request, and not holding the store guard across file I/O would avoid this.
There was a problem hiding this comment.
Fixed in a13eda9. The import is capped at 1000 keys (MAX_REMOTE_KEYS_PER_IMPORT), reads the store once under a guard released at once, saves the definitions once per request, and builds the signer clients before taking the store's write lock, which now covers map inserts only. Since the merge with main (9787aab) the same request also registers its keys in the slashing-protection record, in one write, before any is loaded.
Not changed: both delete routes still hold the store's write lock across a file read and save per key, as /keystores did before this PR. I left that for a follow-up.
| /// key material on an HTTP call. A definitions file with no entry for them is | ||
| /// enough to keep them from being loaded again. | ||
| fn persist_delete(context: &KeymanagerContext, pubkey: &BlsPubkey) -> crate::Result<()> { | ||
| pub(super) fn persist_delete(context: &KeymanagerContext, pubkey: &BlsPubkey) -> crate::Result<()> { |
There was a problem hiding this comment.
persist_delete removes any definitions entry with this pubkey, whatever its type, and both delete_ones call it whenever the in-memory key belongs to their route. So when memory and the file disagree on local vs. remote, one route deletes the other route's file entry, despite the description saying the routes partition the store.
Example: memory holds a local key and the file holds a remote entry for the same pubkey. One way to get there: remotekeys::prepare_import persists the remote entry, then finish_import finds the key already in the store and answers duplicate. Now DELETE /eth/v1/keystores computes on_disk = false (the entry is remote) and in_memory = true, and persist_delete erases the remote definition while reporting deleted. remotekeys::delete_one has the mirror case.
Having persist_delete remove only an entry of the expected SigningDefinition variant would fix it.
There was a problem hiding this comment.
Fixed in a13eda9. persist_delete takes an Origin and removes only an entry of that route's kind (test: a_delete_never_erases_the_other_routes_file_entry). The state in the example can no longer be reached through the API either: duplicates are now judged before anything is written.
| request: &SigningRequest<'_>, | ||
| ) -> Result<BlsSignature> { | ||
| let pubkey_hex = encode_hex(&pubkey.0); | ||
| let url = format!("{}/api/v1/eth2/sign/{pubkey_hex}", self.base_url); |
There was a problem hiding this comment.
A URL with a query string or fragment passes validation in new, but the signing path is appended by string concatenation, so it ends up inside the query: https://signer:9000?token=abc becomes https://signer:9000?token=abc/api/v1/eth2/sign/0x.., which requests /. The likely 404 is then reported as "remote signer ... does not hold this key", which is misleading. And since only userinfo is refused, token=abc is printed in every signing error.
Rejecting a query or fragment in Web3Signer::new, or building the path with Url::join / path_segments_mut, fixes both.
There was a problem hiding this comment.
Fixed in a13eda9. Web3Signer::new refuses a URL with a query string or a fragment (a_url_with_a_query_or_fragment_is_refused).
| let signature = match self | ||
| .context | ||
| .sign_aggregate_and_proof(&store, &pubkey, root, slot) | ||
| .sign_aggregate_and_proof(store, &pubkey, message) |
There was a problem hiding this comment.
Remote signing failures here ("Failed to sign aggregate" just below), for the selection proof at line 99, and in subscribe are only logged with warn!, never counted with inc_signing_failures(). The PR description and docs/cli.md say a signer refusal or an unreachable signer is "logged, counted as a signing failure".
With Web3Signer down or refusing during aggregation, every aggregate is skipped while signing_failures stays at 0, so alerts on that metric don't fire for lost aggregation duties. Local signing couldn't fail past the key lookup before this PR, so these paths never mattered until now.
There was a problem hiding this comment.
Fixed in a13eda9. Selection-proof, aggregate and subscription signing failures all call inc_signing_failures() now, and so do the sync committee and payload attestation paths.
| /// Nothing is asked of the signer here; see [`Self::load`] for why. The | ||
| /// URL is checked for shape, which is the one failure worth refusing up | ||
| /// front: an entry nobody could ever reach. | ||
| pub fn insert_remote(&mut self, pubkey: BlsPubkey, url: &str) -> Result<()> { |
There was a problem hiding this comment.
insert_remote builds a reqwest Client (rustls config plus root store) while the store write lock is held, which goes against the stated rule that the write lock only covers a cheap insert. Importing keys on several new signer URLs builds one TLS client per URL under store.write(), blocking every duty's per-signature read() meanwhile.
Meanwhile remotekeys::prepare_import builds a Web3Signer only to validate the URL and then drops it, so each key pays for a client twice. Building the Arc<Web3Signer> once in prepare (or adding a validate-only function) and passing it to a write-locked insert that only touches the map would fix both.
There was a problem hiding this comment.
Fixed in a13eda9. The keymanager import builds or reuses the Arc<Web3Signer> once in prepare_import, outside the store's lock, and insert_remote_signer is a map insert. insert_remote, which builds a client, is left for startup only.
| // The header is taken from the decoded container, never from anything | ||
| // reassembled here, and it is taken once: its root is what the | ||
| // guard's slot is recorded against and what the signature covers. | ||
| let header = produced.header(); |
There was a problem hiding this comment.
produced.header() and produced.block_root() each compute the body's hash tree root, so the body, execution payload transactions included, is hashed twice per proposal (three times in debug builds) inside the proposal's time budget.
let header = produced.header(); let block_root = header.hash_tree_root(); only rehashes the 5-field header, and the debug_assert comparing the two then checks nothing new and can go.
There was a problem hiding this comment.
Fixed in a13eda9. The block root is header.hash_tree_root(), and the debug_assert is gone.
| /// every mutation, since each can orphan one: an insert that replaces a | ||
| /// remote entry with a local one, a remove, a remote insert on a new URL | ||
| /// for a key that was the last on its old one. | ||
| fn drop_unused_signers(&mut self) { |
There was a problem hiding this comment.
drop_unused_signers treats a client as unused by Arc::strong_count, but the clone signer() hands to an in-flight signature also counts. If a URL's last key is deleted through the keymanager while one of its requests is in flight, the count is 2, the client is kept, and nothing prunes it after the request finishes until some later mutation. So the guarantee release_it_together tests doesn't hold under concurrency.
Keeping a Weak<Web3Signer> in the URL map, with the strong references held by the keys that use it, would remove the need for manual pruning.
There was a problem hiding this comment.
Fixed in a13eda9. The URL map holds Weak<Web3Signer>; the keys and any signature in flight hold the strong references, so nothing is pruned by count (a_client_held_by_an_in_flight_signature_is_released_when_it_ends).
|
Thanks for the review. All 15 threads are answered inline; the fixes are in a13eda9. Three things changed since then that the review has not seen:
Still open: nothing has been run against a real Web3Signer, and the |
… route to
the validator client. A validator's key can now stay in a Web3Signer: each
signature is a POST /api/v1/eth2/sign/{pubkey} carrying the message in full,
the fork it is signed under and the signing root, so the signer's own
slashing protection applies to it. Every signature the client produces is
covered, the gloas envelope and payload attestation included.
Every SigningContext::sign_* is async over the shared store lock: it resolves
the signer under a read guard scoped to the lookup and signs after releasing
it, because a remote signature is an HTTP round trip and the lock is
write-preferring. A slot's attestations are signed concurrently so many keys
behind one signer cost one round trip. Block signing takes the header and
aggregate signing the AggregateAndProof, since the signer reads them.
validator_definitions.yml stays backward compatible: a remote entry carries
`url` where a local one has its keystore paths. /eth/v1/keystores and
/eth/v1/remotekeys partition the store, each listing and deleting its own
kind. --web3signer-url is the default signer for a remote key imported
without a url of its own.
A remote key is covered by this client's EIP-3076 record like a local one,
beside the signer's own: the record is opened over every loaded key, a block
or attestation is checked and recorded before the signer is asked, and POST
/eth/v1/remotekeys registers its keys in one durable write before any is
loaded, taking its entries off the file again when that write fails. DELETE
/eth/v1/keystores answers `not_found` for a key the remote route holds and
exports nothing for it.
…entries, and keep signer URLs and response bodies out of the logs. A keystore import for a key a remote signer holds, and a remote import for a key the definitions file already lists, now answer `duplicate` instead of overwriting the other route's entry; both are judged on the file under the definitions lock and again on the store under the write lock. A remote signer URL that carries credentials is refused, since the URL is named in every signing error, and a failed response's body is cut to 200 characters before it is reported.
…per-signature budget, and make the keymanager routes judge and change the file and the store together. Each duty now asks for its signatures at once through `signing::sign_all`, each given `SIGNING_BUDGET` (2s), so a hung signer costs only its own keys and the batch goes out with whatever arrived. Selection proofs are remembered in `SigningContext` once produced, so the aggregation tick reuses the subscription's. Selection, aggregate and subscription failures are now counted as signing failures. Both imports hold `definitions_lock` until the store insert and judge duplicates before writing anything; `persist_delete` only removes its own route's kind of entry. A remote import is capped at 1000 keys, saves the definitions once, builds signer clients outside the store lock, and the store holds those clients weakly. Definitions entries carry `type: local_keystore` / `type: web3signer`; an untyped entry naming both a keystore and a url, and any unknown field, is refused. Signer URL errors no longer echo credentials, and a URL with a query or fragment is refused. Remote block signing is refused before bellatrix (BLOCK_V2 wants the full block there). A proposal's block root is taken from its header. The docs now say TLS uses the bundled webpki roots, not the system's. Both deletes share `flush_earlier_removal`, so a retry after a removal whose directory flush failed is not told the key is gone before the removal is durable.
…try of the definitions file, and read past it on a local one. Refusing every unknown field stopped the client on any file Lighthouse wrote, since Lighthouse puts its own per-validator settings on each entry. The hazard the refusal was for is a remote entry: there an unread field is most likely one of Lighthouse's signer settings (`root_certificate_path`, `request_timeout_ms`), and skipping it would sign through a connection the operator did not describe. So a web3signer entry carrying one is refused at startup, naming every such field, whether the entry states its `type` or is read as remote by its `url`. `description` is the one field a remote entry may carry unread: Lighthouse writes it on every entry, an empty one included, and it changes nothing about how the key signs. A local entry is read past any field this client does not have, as it was before entries had a `type`; those fields are not written back.
… and the signing root before it is used. Whatever 96 bytes the signer answered were returned as the signature. A signer holding another key under the pubkey, one computing a different root for a fork it does not know, or a proxy rewriting the body would have this client publish a signature no peer accepts, and decide `is_aggregator` on bytes that are not the selection proof. `SigningContext::sign` now checks the answer and reports one that does not verify as a signing failure for that key: logged, counted, never retried, and never remembered as a selection proof. The fake signer in the tests signs the request's `signingRoot` with the test key, so the signing path is exercised with real signatures, and can answer with another key's signature, another root's, or bytes that are no signature.
46f289c to
d39e249
Compare
MegaRedHand
left a comment
MegaRedHand
left a comment
There was a problem hiding this comment.
Reviewed the remote-signing path and the two keymanager routes. The main concerns are about how this behaves with many remote keys (the subscriptions.rs and signing.rs comments) and one credential leak in keys/store.rs. The two delete comments on keystores.rs and remotekeys.rs only happen when the definitions file and memory disagree about a key's kind, i.e. after a hand edit while the client runs; both imports otherwise prevent that.
Heads-up: this conflicts with #779 (six files), and #779 signs exits through the old synchronous sign_root. Whichever lands second needs a VOLUNTARY_EXIT signing body and an async sign_voluntary_exit; details on #779.
| ), | ||
| } | ||
| } | ||
| let selections = sign_all( |
There was a problem hiding this comment.
This puts a signer round trip ahead of every epoch's slot-0 block. refresh_epoch (lib.rs:768) calls subscribe before propose (lib.rs:332), so with remote keys the boundary proposal now waits on these selection proofs: up to the full SIGNING_BUDGET (2s) if the signer hangs, and that delays blocks from local keys too. On main these were local signatures.
Could the selection proofs come off the proposal's path, e.g. run after propose, or in a spawned task that sends the subscriptions once they're in?
| /// deadline is dropped and reported as an [`Error::Signing`] for its key, so | ||
| /// one unresponsive signer delays the batch by the budget at most and never | ||
| /// takes the signatures that did arrive down with it. | ||
| pub async fn sign_all<T, F>(requests: impl IntoIterator<Item = (BlsPubkey, F)>) -> Vec<Result<T>> |
There was a problem hiding this comment.
There's no cap on how many of these are in flight. reqwest is built without its http2 feature (default-features = false, features = ["rustls-tls"]), and Web3Signer is usually plain http anyway, so every request in flight is its own TCP connection. With 1000+ remote keys that's 1000+ sockets at an epoch boundary, on top of the beacon-node, metrics and keymanager ones.
ethlambda validator also never raises RLIMIT_NOFILE; only run_node calls raise_fd_limit. On the default soft limit of 1024, connect() fails with EMFILE and every remote signature fails, attestations and proposals included.
Suggest capping concurrency per signer (a semaphore, or buffer_unordered) and raising the fd limit in the validator sub-command too.
| // `is_aggregator` on bytes that are not the selection proof. | ||
| // A local key needs no such check: its signature is computed | ||
| // here, over this root. | ||
| if !verifies(pubkey, root, &signature) { |
There was a problem hiding this comment.
verifies is a full BLS verify (pairing plus subgroup check, ~1ms), run synchronously inside the future that sign_all wraps in timeout_at. Responses that arrive before the deadline are verified one after another on the duty task, and that time isn't bounded by the budget.
With ~1000 remote keys at an epoch boundary, that's roughly 1–3s of CPU ahead of the slot-0 proposal, so "one slow signer delays a duty by SIGNING_BUDGET at most" doesn't hold. Batch-verifying the batch's signatures, or verifying in spawn_blocking, would fix it.
| slot: Slot, | ||
| ) -> Result<BlsSignature> { | ||
| self.sign_root(store, pubkey, self.selection_proof_signing_root(slot)) | ||
| if let Some(proof) = self.selection_proofs().get(&(*pubkey, slot)) { |
There was a problem hiding this comment.
The cache is checked before the store, so a key deleted through the keymanager mid-epoch still gets its selection proof here while its duties stay scheduled. The aggregation tick then fetches the aggregate and fails at sign_aggregate_and_proof with UnknownValidator. The new inc_signing_failures() calls (aggregation.rs:126 and :228, subscriptions.rs:88) count that as a signing failure, which main didn't, so alerts fire on a deliberate delete.
Minor. Checking that the store still holds the key before returning a cached proof, or dropping a key's proofs when it's deleted, would fix it.
| SigningDefinition::Web3Signer { url } => { | ||
| let pubkey = parse_pubkey(&definition.voting_public_key).map_err(|err| { | ||
| Error::Keystore { | ||
| path: url.clone(), |
There was a problem hiding this comment.
This reports the raw URL from the file. With url: https://user:pass@signer:9000 and a typo in voting_public_key, startup aborts with an error that contains the credentials, before Web3Signer::new gets the chance to refuse them. That breaks the "every URL error is reported through redact" guarantee. Either redact it here, or name the entry by its position in the file instead.
| ) -> Json<StatusResponse> { | ||
| let _definitions_guard = context.definitions_lock.lock().await; | ||
|
|
||
| let mut store = context.store.write().await; |
There was a problem hiding this comment.
I know the description calls this out as matching /keystores, but it gets worse here. The write lock is held across delete_one for every pubkey, and each one parses the definitions file (definitions_entry) and then saves and fsyncs it (persist_delete). The request isn't capped, so a DELETE of a few hundred keys stalls every signature's read-lock lookup for that long, local keys included, since tokio::sync::RwLock is write-preferring.
Memory-first order can stay. Judge every key against one read of the file under definitions_lock, take the write lock only to remove the keys from memory, release it, then save the file once.
| if in_memory { | ||
| store.remove(pubkey); | ||
| } | ||
| context.settings.forget(pubkey); |
There was a problem hiding this comment.
forget runs whenever the file has a remote entry, including when the key in memory is a local one this route leaves alone (on_disk true, in_memory false, store.contains true). That local key keeps proposing but loses its fee recipient, graffiti and gas-limit overrides, so its next blocks pay the default recipient.
Only reachable when file and memory disagree about the key's kind, but forget should be skipped while the store still holds the key as local.
| // the key elsewhere, is what turns a failed delete into two hosts signing | ||
| // for one validator. | ||
| match persist_delete(context, pubkey) { | ||
| match persist_delete(context, pubkey, Origin::Local) { |
There was a problem hiding this comment.
The same disagreement, the other way round: memory holds a local key and the file holds a remote entry for it. delete_one falls through to delete_local (remote_on_disk && store.contains(pubkey)), persist_delete(.., Origin::Local) leaves the remote entry in place, and the response is deleted plus the slashing-protection export.
On restart the remote entry loads and signs through Web3Signer, whose own record knows nothing about votes the key casts after the operator moved it elsewhere. That's the "reported gone while a restart brings it back" outcome the comment above warns against. Only reachable after a hand edit, but I'd answer an error here rather than deleted while an entry of the other kind is left in the file.
| // left behind by a failed delete, is still "already known", and | ||
| // overwriting it would turn a key the operator disabled into a signing | ||
| // one. Judged before anything is built or staged. | ||
| let on_disk = definitions.0.iter().any(|definition| { |
There was a problem hiding this comment.
Nit: this decodes every definition's pubkey once per imported key. An import at the 1000-key cap against a file with ~1000 entries is ~1M parse_pubkey calls, each allocating, under definitions_lock. The same scan is copied into keystores.rs:597, definitions_entry, persist_delete and the rollback retain. A HashSet<BlsPubkey> built once per request (or a ValidatorDefinitions::find) would remove both the cost and the copies.
| // the whole batch, local keys included, while it waits. | ||
| let signatures = | ||
| { | ||
| let _timing = crate::metrics::time_signing(); |
There was a problem hiding this comment.
Nit: time_signing() now wraps sign_all, so for remote keys the signing-time histogram measures the network round trip plus up to 2s of budget waiting rather than signing. The same applies in sync_committee.rs and payload_attestation.rs. Worth either the remote-signer latency histogram the description lists as a follow-up, or a note in docs/metrics.md that the metric's meaning changed.
🗒️ Description / Motivation
SigningMethodhad one variant. Every other consensus client supports Web3Signer, and operators who keep keys in a signer cannot run this client without it.What Changed
crates/validator/src/keys/web3signer.rs(new): theWeb3Signerclient. Each signature is aPOST /api/v1/eth2/sign/{pubkey}withtype,fork_info,signingRootand the typed message, per Web3Signer's OpenAPI. All ten signature kinds the client produces map to a request type, the gloasEXECUTION_PAYLOAD_ENVELOPEandPAYLOAD_ATTESTATION_MESSAGEincluded.412(slashing protection),404(key not held) and transport failures areError::Signing, logged and counted, never retried. A URL carrying credentials, a query string or a fragment is refused, and every URL error is reported throughredact, so no credential reaches a log; a failed response's body is cut to 200 characters.signing.rs: everySigningContext::sign_*isasyncover&RwLock<ValidatorStore>. It resolves the signer under a read guard scoped to the lookup and signs after releasing it. A remote signer's answer is verified against the validator's public key and the signing root before it is used.sign_allasks for a duty's signatures at once and gives eachSIGNING_BUDGET(2s), so a hung signer costs only its own keys. Selection proofs are remembered once produced.sign_blocktakes theBeaconBlockHeader; remote block signing is refused before bellatrix, whereBLOCK_V2wants the full block.attestation,aggregation,sync_committee,payload_attestation,subscriptions): each signs its batch throughsign_alland publishes whatever arrived; every signing failure is counted.attestationandproposalkeep main's order: check and durably record in slashing protection, then sign.keys/store.rs:SigningMethod::Web3Signer { signer: Arc<Web3Signer> }; one client per signer URL, held weakly by the store and strongly by the keys and in-flight signatures that use it.insert_remote_signeris a map insert, so a keymanager import builds its clients outside the store's lock.keys/definitions.rs: entries carrytype: local_keystoreortype: web3signer, Lighthouse's names. An entry without atype(every existing file) is read by its fields; one naming both a keystore and aurlis refused. Aweb3signerentry carrying a field this client does not implement (root_certificate_path, say) is refused,descriptionexcepted; a local entry is read past such fields, so a Lighthouse-written file loads.http_api/remotekeys.rs(new):GET,POST,DELETE /eth/v1/remotekeys. An import is capped at 1000 keys, holdsdefinitions_lockuntil the keys are in the store, saves the definitions once, registers the keys in the slashing-protection record in one write before any is loaded, and takes its entries off the file again when that write fails.DELETEleaves the record in place.http_api/keystores.rs: the two routes partition the store./keystoreslists and deletes local keys only; an import of a key the other route holds, in memory or in the file, isduplicate;persist_deleteremoves only its own route's kind of entry;DELETE /keystoresanswersnot_foundfor a key a remote signer holds and exports nothing for it.bin/ethlambda/src/validator.rs:--web3signer-url, the default signer for a remote key imported without aurl. Validated at startup.docs/cli.md(flag, a "Remote signing (Web3Signer)" section, the route),docs/spec_deviations.md,CLAUDE.md.Correctness / Behavior Guarantees
SIGNING_BUDGETat most and never costs another key its signature.readonlyis alwaysfalseon/remotekeys;DELETE /remotekeyshas noslashing_protectionfield, as the spec defines none there.Tests Added / Run
keys/web3signer.rs: request wire shapes pinned against Web3Signer's schema, response parsing (JSON and bare hex), URL validation (credentials, query, fragment, redaction), body truncation.signing.rs:fork_infoon both sides of mainnet's altair boundary;sign_all's budget; the selection-proof cache; aremotemodule with a local axum fake signer that signs for real, covering the request body, the 412, 404 and unreachable paths, a pre-bellatrix block, and an answer by another key, over another root, or that is no signature.keys/definitions.rsandkeys/store.rs: legacy file parses unchanged, typed and untyped entries, an entry with both methods refused, unimplemented fields refused on a remote entry and skipped on a local one, client sharing per URL and release with a signature in flight.http_api/remotekeys.rsandkeystores.rs: import/list/delete round trip with persistence, default-URL fallback, duplicates across both routes, each route deleting only its own keys and exporting nothing for the other's, registration in slashing protection and its rollback.cargo test -p ethlambda-validator -p ethlambda -p ethlambda-rpc --profile release-fast: 421, 265 and 346 passed.make lint: clean.cargo fmt --check: clean.Related Issues / PRs
consensys/web3signerimage before relying on it.suggested_fee_recipient,graffitiandgas_limitto the definitions file; until it lands, aweb3signerentry carrying them is refused as unimplemented./keystoresalready did.GET /api/v1/eth2/publicKeys), a remote-signer request-latency histogram.✅ Verification Checklist
make fmt— cleanmake lint(clippy with-D warnings) — cleanmake test— the validator crate, the binary and the rpc crate were run atrelease-fast(1032 tests); CI runs the rest