fix(core): cache beacon config for signing-domain resolution

[iamquang95]

Fixes #563.

Signing-domain resolution hit the BN live on every verification — 4 HTTP calls per attester partial-sig check — and a BN failure mid-verification was misreported to the VC as 400 "partial signature verification failed". Charon avoids both via caching in its go-eth2-client fork.

• BeaconNodeClient (now a single Arc<Inner>) embeds a config cache for spec/genesis/fork_schedule: 5-min TTL, concurrent cold lookups coalesce into one fetch, errors and empty/malformed responses never cached.
• Both scheduling and submission clients are warmed at startup, before duty scheduling; warm failure aborts startup.
• All consumers read config through the cached client; raw fetch_domain* methods are deleted, leaving one domain derivation.
• SigningError → HTTP by provenance: BN/config failure 502, invalid submitted signature 400, invalid trusted key material (e.g. bad pubshare) 500.

[iamquang95]

@claude review this PR

[claude]

Claude finished @iamquang95's task in 5m 7s —— View job

---

Review: fix(core): cache beacon config for signing-domain resolution

• Gather context (read diff, changed files)
• Review confcache.rs (TTL, coalescing, error handling)
• Review beacon_node.rs / extensions.rs refactor
• Review validatorapi/component.rs error mapping
• Review startup warming in app/src/node
• Cross-check against Charon Go behavior
• Post review

---

Solid, well-scoped change. The direction is right and the surrounding refactor is a genuine improvement beyond the stated fix: collapsing BeaconNodeClient into a single Arc<Inner> removes the Arc<Arc<...>> double-wrap at ~15 call sites, new_slot_ticker is now synchronous because the scheduler no longer refetches genesis/slots inside it, and rejecting an empty fork schedule before it can enter the cache (extensions.rs:141-148) closes a nasty failure mode. Error-provenance mapping in signing_error_to_api_error is correct — I checked BlstImpl::verify (crates/crypto/src/blst_impl.rs:219-223), which deserializes the public key before the signature, so a VC-supplied garbage signature with a valid pubkey still lands on InvalidSignature → 400 while a bad configured pubshare lands on InvalidPublicKey → 500, as intended.

A few things below. Nothing blocking except possibly #2, which is a judgment call about how far you want to go on this PR.

1. The coalescing test's key assertion doesn't test what its comment says

crates/eth2api/src/confcache.rs:222-223

// Fork version at epoch 20 comes from the second schedule entry.
assert_eq!(first[..4], [0x01, 0x00, 0x00, 0x00]);

compute_domain writes the domain type into domain[..DOMAIN_TYPE_LEN] (extensions.rs:224, DOMAIN_TYPE_LEN == 4); the fork version only ever reaches domain[4..] via the ForkData tree hash. So first[..4] is DOMAIN_BEACON_ATTESTER, which the fixture sets to "0x01000000" — the same value as ALTAIR_FORK_VERSION, which is what makes this read as meaningful. The assertion passes for any fork version the schedule resolves to.

To actually pin epoch-20 fork selection, compare against a known-good full domain, or assert the epochs differ across the boundary:

assert_ne!(client.domain(attester, 5).await.unwrap(), first);

Fix this →

2. TTL expiry reintroduces the exact failure this PR fixes, once every 5 minutes

crates/eth2api/src/confcache.rs:21,41-68

The fetch runs under the write lock and there's no stale-on-error fallback, so:

• The first signing verification after each TTL boundary blocks on live BN I/O, and if the BN is down or slow at that instant it fails with SigningError::BeaconNode → 502 mid-verification. That's a smaller window than the status quo, but it's the same failure.
• BeaconNodeClient::domain() (beacon_node.rs:130-132) awaits spec() → genesis() → fork_schedule() serially, and all three were warmed together at startup so they expire together. Worst case is 3 sequential round-trips of stall for the whole verification pipeline every 5 minutes.

Charon's go-eth2-client fork caches spec/genesis/fork-schedule for the lifetime of the service (no TTL), which is why it doesn't have this window. The 5-min TTL is a deliberate and defensible deviation (picking up BN upgrades), but the cheap way to get both properties is serve-stale-on-refresh-failure: on a failed refresh, keep and return the last good value, log, and retry on the next call. Errors still aren't cached; you just don't throw away a value you already have.

Also worth a comment either way: because component.rs wraps these in tokio::time::timeout(UPSTREAM_REQUEST_TIMEOUT, ...) (e.g. component.rs:1727,1733), a timeout that fires while the lock-holder is fetching drops the guard, and the next waiter restarts the fetch from scratch — under a slow BN the coalescing property degrades to serialized retries rather than one shared fetch.

3. Doc comments overclaim what warming buys

crates/app/src/node/mod.rs:332-334 — "so signing-domain resolution never blocks on a live fetch" — and crates/eth2api/src/beacon_node.rs:65-66. True for the first STATIC_CONFIG_TTL, then not. Worth softening unless #2 lands.

4. SigningError::Helper(_) → 502 is a blanket mapping

crates/core/src/validatorapi/component.rs:2274-2278. HelperError also covers SlotComputation and FetchSlotsPerEpoch, which aren't BN-connectivity failures. Today the only producer on this path is epoch_from_slot, so 502 is right in practice — but matching the specific variants (or noting the assumption in the doc comment) keeps it honest if HelperError grows.

5. "All consumers read config through the cached client" isn't quite true

crates/app/src/monitoringapi/checker.rs:247-251 and crates/app/src/sse/mod.rs:394-399 still call fetch_genesis_time/fetch_slots_config on the raw client, and node/mod.rs:459 deliberately hands monitoring_beacon = eth2_cl.clone() (raw) to the readiness checker. Both are one-shot at startup so there's no hot-path cost — but either route them through BeaconNodeClient for consistency or narrow the claim in the description.

6. Nits

• verify_fork_schedule fallback is now unreachable (node/mod.rs:949-952): fork_schedule() rejects an empty schedule upstream, so .first() can never be None and unwrap_or_else(|| "unknown") is dead. Also note the empty-schedule case now surfaces as ParseError("empty fork schedule") rather than the friendlier ForkScheduleMismatch { beacon_node_network: "unknown" }.
• BeaconMock::beacon_client() returns a fresh client — and a fresh cache — on every call (testutil/src/beaconmock/mod.rs:184-187). Several tests call it more than once (e.g. component.rs builds one via beacon_client_at(&mock.uri()) and another via mock.beacon_client() to compute the expected signing root), so caches don't shard. Harmless today, but a footgun the moment a test asserts request counts. Consider storing one BeaconNodeClient in BeaconMock and returning a clone.
• Parsing still happens per call. spec() hands back an Arc<Value>, but slots_config(), fork_config(), domain_type() and domain_from_config() re-parse it each time; fork_schedule_from_spec in particular does 12 format!s plus a HashMap alloc on every voluntary-exit domain resolution. The HTTP calls are gone — which is the point — but caching the parsed values would finish the job. Fine as a follow-up.

Verification

I could not run cargo clippy / cargo test in this environment — both need approval here, so treat the CI result as the source of truth for the gates. Everything above is from reading the diff and the surrounding code.
· Branch: feat/fix-563

[emlautarom1]

Couple of notes:

• The endpoints suggested to be cached with a TTL never change in the most popular Beacon clients (Lighthouse, Prysm and Teku, representing ~90%). We might as well just cache for the lifetime of the application and avoid the TTL complexity.
• We should explore dropping the eth2api crate as is: instead of relying on code generation, we can use agents to have a tailor made crate that exposes the endpoints that we need precisely, with full control over the underlying HTTP client. This is a lot more work but simplifies the build process, total LOC, improves error reporting, etc.

With this, I'm not sure we want to go this route right now unless it's a hard requirement for the MVP (ie. testnet shows that without it Pluto misbehaves)