Skip to content

refactor(eth2api): make ValidatorCache immutable in BeaconNodeClient - #667

Merged
varex83 merged 4 commits into
mainfrom
feat/fix-482
Sep 3, 2026
Merged

refactor(eth2api): make ValidatorCache immutable in BeaconNodeClient#667
varex83 merged 4 commits into
mainfrom
feat/fix-482

Conversation

@varex83agent

Copy link
Copy Markdown
Collaborator

Summary

BeaconNodeClient held its ValidatorCache as Arc<RwLock<Option<ValidatorCache>>>, initialized to None and populated later via set_validator_cache, so every read had to handle a "not yet set" case that leaked into callers (the scheduler's .expect("validator cache is available")).

The validator set is actually known at node construction (from the cluster's validators), so this optionality was never needed. This PR builds a single ValidatorCache once in node::run and threads it into both the scheduler and submission BeaconNodeClients at construction.

Changes

  • BeaconNodeClient::new now takes the ValidatorCache and stores it as a plain field (ValidatorCache is already Arc-backed, so clones seeded into each client share state — a single per-epoch refresh still updates every consumer).
  • Removed the Arc<RwLock<Option<..>>> wrapping, the set_validator_cache setter, and the NoActiveValidatorCache error variant.
  • validator_cache() returns &ValidatorCache; dropped the .expect/TODO at the scheduler read site.
  • Updated wiring (node::run / wire_core_workflow) to construct-and-seed the shared cache up front instead of set_validator_cache after the fact, and all tests to the new constructor.

Testing

  • cargo +nightly fmt --all --check
  • cargo clippy --workspace --all-targets --all-features -- -D warnings — clean
  • cargo test -p pluto-eth2api -p pluto-core -p pluto-app — all pass (eth2api 133, core 605, app 141)

Closes #482

🤖 Generated with Claude Code

varex83agent and others added 3 commits August 25, 2026 16:51
The validator set is known at node construction (from the cluster
validators), so the cache no longer needs to be optional, mutable, or
shared behind a lock. Build a single `ValidatorCache` in `node::run` and
thread it into both the scheduler and submission `BeaconNodeClient`s at
construction.

- `BeaconNodeClient::new` now takes the `ValidatorCache` and stores it as
  a plain field (the type is already `Arc`-backed, so clones share state).
- Remove the `Arc<RwLock<Option<..>>>` wrapping, the `set_validator_cache`
  setter, and the `NoActiveValidatorCache` error variant.
- `validator_cache()` returns `&ValidatorCache`; drops the `.expect`/TODO
  at the scheduler read site.

Closes #482

Co-Authored-By: Bohdan Ohorodnii <35969035+varex83@users.noreply.github.com>
…ore_workflow

WireInputs takes the two beacon API clients. Wiring derives the cache from the cluster validators once and constructs both BeaconNodeClients, the per-epoch refresher and the validator API from it. The wiring test asserts the scheduler's first validators request carries the cluster pubkeys.

@emlautarom1 emlautarom1 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM, injecting at construction is always better than having some setter.

I pushed one extra change that makes the validator cache shared by construction instead of by convention: run is private and has no test, and wiring_seeds_shared_validator_cache sets up the clients in its own helper, so it passes even when wire_core_workflow is never called. This is solved by making wire_core_workflow build a single cache from validators and construct the clients, the refresher and the validator API from it. I also rewrote the test as wiring_seeds_validator_cache to check this behavior.

Comment thread crates/app/src/node/mod.rs Outdated
Comment on lines +381 to +386
// One pubkey-scoped validator cache shared by the scheduler's beacon
// client, the submission client, and the validator API, so every consumer
// resolves the same cluster validator set. `ValidatorCache` is `Arc`-backed,
// so the clones seeded into each client (and the one wired into the
// per-epoch refresh subscriber in `wire_core_workflow`) share state, letting
// a single refresh update every consumer at once.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Keep it short:

Suggested change
// One pubkey-scoped validator cache shared by the scheduler's beacon
// client, the submission client, and the validator API, so every consumer
// resolves the same cluster validator set. `ValidatorCache` is `Arc`-backed,
// so the clones seeded into each client (and the one wired into the
// per-epoch refresh subscriber in `wire_core_workflow`) share state, letting
// a single refresh update every consumer at once.
// One pubkey-scoped validator cache shared by the scheduler's beacon
// client, the submission client, and the validator API, so every consumer
// resolves the same cluster validator set.

Comment thread crates/app/src/node/wire.rs Outdated
Comment on lines +261 to +264
/// Pubkey-scoped validator cache shared by the beacon/submission clients
/// and the validator API. A clone of the same `Arc`-backed cache seeded
/// into those clients, so the per-epoch trim + refresh subscriber wired
/// below refreshes every consumer at once.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove this comment:

Suggested change
/// Pubkey-scoped validator cache shared by the beacon/submission clients
/// and the validator API. A clone of the same `Arc`-backed cache seeded
/// into those clients, so the per-epoch trim + refresh subscriber wired
/// below refreshes every consumer at once.

Comment thread crates/app/src/node/wire.rs Outdated
Comment on lines +460 to +463
// The pubkey-scoped validator cache is built and seeded into the
// beacon/submission clients at construction (in `node::run`), and passed in
// here so the per-epoch trim + refresh subscriber registered below (and the
// validator API) share the same `Arc`-backed state.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Remove this comment:

Suggested change
// The pubkey-scoped validator cache is built and seeded into the
// beacon/submission clients at construction (in `node::run`), and passed in
// here so the per-epoch trim + refresh subscriber registered below (and the
// validator API) share the same `Arc`-backed state.

# Conflicts:
#	Cargo.lock
#	crates/app/src/node/wire.rs
@varex83
varex83 merged commit 0043ac2 into main Sep 3, 2026
16 checks passed
@varex83
varex83 deleted the feat/fix-482 branch September 3, 2026 13:47
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.

Make ValidatorCache in BeaconNodeClient immutable

3 participants