Skip to content

feat: implement state transition function - #16

Merged
MegaRedHand merged 5 commits into
mainfrom
state-transition-function
Jan 12, 2026
Merged

MegaRedHand merged 5 commits into
mainfrom
state-transition-function

Conversation

@MegaRedHand

Copy link
Copy Markdown
Collaborator

Also changes blocks to include aggregated attestations, which are introduced for devnet 2

@MegaRedHand
MegaRedHand merged commit 6d771dd into main Jan 12, 2026
@MegaRedHand
MegaRedHand deleted the state-transition-function branch January 12, 2026 18:40
tcoratger pushed a commit to tcoratger/ethlambda that referenced this pull request Oct 30, 2026
* refactor(types): move the pure signing helpers off the state transition

They depend on nothing but their arguments and on containers this crate
already owns, so a consumer that only signs a message had to take on
blst, c-kzg and RocksDB to reach them. Re-exported at the old path.

* test(types): restore the domain coverage lost in the move

The relocated tests dropped two assertions: that the domain's tail is
the truncated fork data root, and that it commits to the genesis
validators root. The second is what stops a signature replaying onto
another chain.

* refactor(types): reach the signing helpers by their own path

Importing from `ethlambda_types` directly matches the re-export just
below it and keeps `signing` out of the module list that exists to give
each transitioned type one name. Adds the `compute_signing_root` test
the move never had.

* docs(beacon): name both helpers that stayed behind

The module doc said only merkle branch verification was still implemented
here, which the next function in the file contradicts.

* feat(validator): add the crate skeleton and its error type

* refactor(validator): carry what the callers will need to act on

The duty loop decides retry-or-skip every slot, failover distinguishes a
node that is down from one that is slow, and a signing failure has to name
its validator. Each was a bare string, so each call site would have
re-derived it. Adds the inconsistent-response case a conformant-looking
node can still produce.

* feat(validator): decrypt EIP-2335 keystores

Verified against both specification test vectors, scrypt and pbkdf2,
including the NFKD password normalisation they exercise. The control-code
strip has to run on Unicode scalar values before UTF-8 encoding, not on
the encoded bytes afterwards: byte-level filtering corrupts any multi-byte
character whose continuation byte falls in the same numeric range as a C1
control code, which is exactly what the key-emoji in the vectors exercises.

* fix(validator): reject malformed keystore parameters and scrub key material

A scrypt cost that is not a power of two silently derived the wrong key
and surfaced as a wrong password, blaming the operator for a corrupt
file. The decrypted secret, the derived key and the password buffer now
zeroize on drop rather than living as long as the process.

* feat(validator): load validator definitions and derive signing keys

* fix(validator): make the definitions file survive a crash mid-write

The keymanager API rewrites this file while validators are signing, and an
in-place truncating write would lose every validator rather than the one
being changed. Writes to a sibling file and renames over the target.

* feat(validator): sign attestations under the target epoch's domain

The fork version is resolved per signature rather than cached, so a
signature produced either side of a fork boundary is valid there.

* feat(validator): add the Beacon API wire types

They live here rather than as serde derives on the consensus containers:
quoted integers and hex bytes are a property of this transport, and a
Deserialize on a container is a footgun in the crate that defines it.

* test(validator): catch a signing domain taken from the wrong epoch

Every fixture had the slot's epoch equal to the target's, so swapping one
for the other passed the whole suite. The network would have dropped the
attestations silently.

* test(validator): cover the wire types nothing was parsing

Four DTOs had no test, so a dropped quoted-integer annotation on any of
them would have surfaced first as a beacon node rejecting the request.

* feat(validator): add the Beacon API client behind one trait

The trait is what makes the duty services testable: a reorg invalidating
duties or a node failing mid-slot cannot be exercised against reqwest.

* feat(validator): fail over across beacon nodes in list order

* feat(validator): add the slot clock

Seeded from the beacon node's genesis and then independent of it: the
node's event stream is best-effort, and a dropped event must not mean a
missed duty.

* fix(validator): read the spec response the way nodes actually send it

The fork-version loop asked for PHASE0_FORK_VERSION, which no node sends,
so the genesis fork version silently kept its mainnet value on every
network. A malformed value was indistinguishable from an absent one, both
quietly keeping a default the network would reject every signature under.
Moves the parsing to the crate that owns wire formats, which keeps
serde_json out of ethlambda-types' production dependencies.

* test(validator): let the mock express a node that fails one call

A single blanket failure flag could not describe a node that answers at
startup and then fails every duties poll, which is the shape the duties and
attestation services most need to test against.

* fix(validator): refuse a zero slot duration instead of dividing by it

A beacon node reporting SECONDS_PER_SLOT of 0 reached the clock unchecked
and panicked later inside slot_at, far from the cause. Rejected at the
response boundary, with a constructor precondition behind it. Adds the
tests at the exact attestation instant that a comparison slip would
otherwise have slipped past.

* feat(validator): track attester duties and invalidate them on a reorg

A dependent_root change means the committee shuffling moved, so acting on
the held schedule would attest from the wrong committee.

* feat(validator): subscribe to the attestation subnets its duties need

* feat(validator): sign and publish this slot's attestations

One data fetch is shared across every validator attesting at the slot, and
one validator failing to sign never costs the others their attestation.

* fix(validator): keep a reorg signal a failed lookahead would have eaten

A lookahead fetch is speculative, but its failure was discarding the
current epoch's committee change along with it, so subscriptions were
never re-sent. Adds the tests for the lookahead and for epoch-scoped
lookup, neither of which any mutation would previously have broken.

* feat(validator): add the metrics and keymanager modules

Not yet declared in lib.rs, so nothing compiles them until the next
commit wires them into the duty loop. Split out so that commit stays
readable rather than carrying three subsystems at once.

The keymanager persists an import before inserting it into the live
store: a key that signs before it is durable would vanish on a crash,
and this client has no slashing protection to notice the gap.

* feat(validator): assemble the client and run the duty loop

Wires the slot clock, duties service, subscriptions and attestation
service into one loop, and serves metrics and the keymanager alongside
it. A retryable error keeps the schedule already held and tries again;
anything else stops, rather than hiding its cause behind a warning every
slot.

Also records that attest() must not be retried within a slot: the
signatures already exist, and re-fetching would sign a different message
for the same slot with nothing here to catch it.

Declares tokio's time feature rather than inheriting it through
workspace feature unification.

* feat(cli): run the validator client as ethlambda validator

Added to EXPLICIT so the sub-command is not given the default `node`
token, which would have parsed it as `ethlambda node validator` and
failed with a message about node flags. The historical flat invocation
the Dockerfile, lean-quickstart and the hive shim rely on is unchanged,
and still covered by its own test.

* fix(validator): reject attestation data for a mismatched slot or target epoch

HttpBeaconNode::attestation_data took the response's slot verbatim, so a
stale or buggy beacon node could hand back AttestationData for a slot
other than the one requested and this client would sign it without
complaint. With no slashing-protection database, two such answers
landing under one target epoch is an unnoticed double vote.

Validate in AttestationService::attest, not in the HTTP client, so the
check also covers the mock and any future BeaconNodeApi implementation.
Also compare the target checkpoint's epoch against the epoch the
requested slot belongs to: the signing domain is chosen from that field,
so an internally inconsistent response is a second way a broken node
could get a signature out of this client that it should not.

* fix(validator): stop holding store lock guards across beacon-node awaits

Both call sites into the shared validator store passed
`&*store.read().await` straight into a match/if-let scrutinee. Rust
extends a scrutinee temporary's lifetime across the whole construct, so
the read lock ended up held for the entire refresh_epoch or attest call:
every network round trip they make, not just the store access.

Meanwhile the keymanager's key import takes the write lock across a
batch of deliberately slow EIP-2335 key derivations and disk writes.
tokio::sync::RwLock is write-preferring, so a queued import would jump
ahead of the duty loop's next read and stall attestation signing for as
long as the batch ran.

Fix both sides: the duty loop now takes only the pubkeys it needs as
owned data before calling refresh_epoch, letting the guard drop
immediately; attest takes the RwLock itself and scopes its own read
guard to the synchronous signing loop, dropped before the submission
call; and the keymanager decrypts and persists every key to disk before
ever taking the write lock, which is now held only across the cheap,
synchronous insert_secret calls.

* fix(validator): count partial attestation-pool rejections instead of failing the whole batch

The Beacon API answers POST /eth/v2/beacon/pool/attestations with 400
and per-index detail (IndexedErrorMessage) when part of a submitted
batch is rejected, and still stores and gossips the rest. Collapsing
any non-2xx into one opaque BeaconNodeStatus error, as post_no_content
does for every other endpoint, meant inc_attestations_published never
fired even when most of the batch succeeded, and one bad duty destroyed
the observability of every other validator that slot.

submit_attestations now returns how many of the batch were accepted.
HttpBeaconNode parses the IndexedErrorMessage body on a 400 from this
endpoint specifically, logs which submitted index failed and why, and
reports the true accepted count as a success rather than an error. A
total rejection or an unparsable body still surfaces as Err, unchanged
from before.

* fix(validator): bind metrics before beacon-node startup and round out observability

Three smaller review findings, bundled since they all touch the same
observability surface:

- run() called genesis()/spec() before binding the metrics and health
  listener, so a validator that could not reach its beacon node exited
  with no HTTP surface at all. Bind metrics first, so /health and
  /metrics are up while an operator waits to see whether startup will
  succeed.
- ethlambda_validator_beacon_node_available only updated inside
  refresh_epoch, once an epoch, so an outage starting right after a
  successful refresh kept reporting "available" for up to an epoch
  while every attest call failed underneath it. It now also updates
  from attest's own failures.
- Added the signing-duration and publication-delay histograms the
  design doc promised but never implemented, plus a validators_resolved
  gauge so "no validator indices resolved" is visible to monitoring
  continuously rather than only in one log line the first time it
  happens. All three are registered in init() alongside the existing
  series.

Also comments two things a reader could otherwise "clean up" by
mistake: the stopping branch in run()'s refresh_epoch match is
unreachable today (every error that function can produce is
retryable), and Error::Signing is unconstructed today but reserved for
a future remote-signer SigningMethod variant.

* fix(validator): write key material at 0600 and refuse to follow symlinks

Every secret this crate writes (the keymanager API token, imported
keystores, and the plaintext passwords that unlock them) went through
std::fs::write with no explicit mode, landing at 0644 under a standard
umask and readable by any other local account. The password file is
the sharper half of that: it sits right next to a world-readable copy
of the keystore it decrypts, so any other account could recover a
validator's BLS secret.

Route every such write through a new secure_fs helper that opens with
mode 0600 via OpenOptionsExt, matching the 0600 convention this crate
already claims (Lighthouse's) but did not enforce. The keystore and
password paths are additionally derived from the public key and are
therefore predictable, so their writes use O_NOFOLLOW rather than a
plain open: an attacker able to plant a symlink at one of those paths
ahead of a legitimate import must not be able to redirect the write to
a target the operator can write to. O_NOFOLLOW still opens and
truncates an existing regular file, so re-importing an already-known
key continues to overwrite its files exactly as before.

* fix(validator): serialize the definitions file's read-modify-write cycle

import and delete each took their own independent snapshot of
ValidatorDefinitions (open, mutate, save) with nothing serializing that
cycle across concurrent requests. The in-memory store's RwLock did not
help, since the snapshot is taken outside of it. A delete that removed
a key and saved could be undone by a concurrent import whose snapshot
was taken beforehand: it would write the file back with the deleted
key still enabled, silently resurrecting a validator the operator had
just deactivated. With no slashing protection in this crate, that is
exactly the failure this API exists to prevent.

Add a definitions_lock to KeymanagerContext, held by import and delete
across their entire open-mutate-save cycle. It guards only the
definitions file, not the validator store, so holding it across an
import's slow EIP-2335 key derivation cannot stall the duty loop, which
only ever waits on the store lock. Both handlers take definitions_lock
before the store lock, consistently, so the two never deadlock.

Along the way: cap a single import request at 100 keystores
(MAX_KEYSTORES_PER_IMPORT), since each one runs its KDF synchronously
while holding definitions_lock and an unbounded batch could tie it up
indefinitely; and de-duplicate persist_import's definitions entry by
voting_public_key, so re-importing an already-known key updates it in
place instead of appending a second, conflicting entry.

A true concurrent race is not reliably deterministic to assert on in a
test. import_waits_for_an_in_flight_holder_of_the_definitions_lock
proves the lock is actually acquired and enforced (a second request
blocks while it is held); a_delete_survives_a_subsequent_import_of_a_different_key
is the weaker, deterministic fallback: two sequential requests, which
mainly exercises the dedup fix rather than the race itself, since a
genuine interleaving is no longer reachable through the public API
once the lock is real.

* fix(validator): compare the bearer token in constant time and finish hardening

require_bearer compared the presented token with plain String equality,
which short-circuits at the first mismatched byte. This service is
localhost-bound by default, but --http-address is operator-configurable
with nothing enforcing that, so a deployment that moves it off loopback
would leak timing information about how much of the token a caller
guessed right. Compare the bytes with subtle::ConstantTimeEq instead.

Also: make the empty DELETE interchange's fake-empty history deliberate
rather than an accident of KeymanagerContext carrying no real
genesis_validators_root. It stays all-zero (documented in the module,
and pinned by a test) so a conformant importer rejects the file instead
of trusting a validator that "has never signed", and every DELETE now
warns that the interchange it returns cannot be used to migrate the
key elsewhere. Wrap ImportRequest.passwords in Zeroizing<Vec<String>>
so it gets the same scrub-on-drop guarantee Keystore::decrypt's output
already has. And in Keystore::decrypt, construct the Zeroizing<[u8; 32]>
before copying the secret into it, rather than after, so the secret is
never briefly held in a bare, unprotected array on the stack.

* fix(validator): derive the consensus-version header from the target epoch

attest took an opaque fork_name from the caller, computed from the
current epoch and disconnected from the attestation it was actually
describing. The signing domain already keys off data.target.epoch;
now that attest validates that field against the requested slot, the
header can and should be derived from the same, already-consistent
source instead of trusted blindly from outside.

* Refuse to sign an attestation this process already signed for that validator

Two double-vote shapes were reachable through ordinary operation, not through
operator error, and with no slashing-protection record nothing caught either.

A backward wall-clock step: the duty loop derives its slot from
SystemTime::now(), which is not monotonic, so an NTP correction of a few
seconds re-enters a slot already attested. The re-fetched attestation data
differs if a block arrived in between, and two different attestations under one
target epoch is a double vote.

A schedule replaced mid-epoch: a duty refresh that fails at an epoch boundary
and succeeds a few slots later can move a validator to a different slot in the
same epoch, and the loop attests a second time under that target.

AttestationGuard records, per validator, the highest source and target epoch
signed in this run, and refuses anything that does not advance the target or
that reaches back below the source. That is EIP-3076's minimal variant, chosen
because one record per validator cannot evaluate the full surround condition
and requiring monotonic progress makes the question unnecessary. The check runs
before the signature is produced rather than after: a signature that exists is
one that can escape.

This is not slashing protection and does not reverse the scope decision
recorded in lib.rs. It holds nothing on disk, so it knows nothing about a
previous run or about another process holding the same keys, and a restart
empties it. The module documentation opens by saying so.

Keyed by validator, not by slot: validators in one epoch attest at different
slots, so a per-slot or per-epoch record would wrongly refuse the second
validator. A test pins that.

Two existing tests gave two duties the same pubkey under different validator
indices, which the guard now correctly refuses. That shape cannot occur: a
pubkey resolves to exactly one validator index, and ValidatorStore is keyed by
pubkey so a duplicate collapses. Both tests now use two keys, which is what
they were always testing for and leaves their assertions unchanged.

Adds ethlambda_validator_attestations_refused_total, separate from
signing_failures_total: that counts signatures attempted and failed, this
counts signatures deliberately not attempted. It should normally read zero.

* Refuse an empty keymanager API token instead of authenticating everyone with it

An empty api-token.txt authenticated every request. Two facts combined:
load_or_create_token returned Ok("") for any existing file, including a
zero-byte one, and the bearer comparison uses subtle's ct_eq, which reports two
empty slices as equal because its accumulator starts at 1 and the fold body
never runs. A request carrying `Authorization: Bearer ` with a trailing space
strips to Some(""), so empty matched empty and the request was served.

It needed no unusual operator action to reach. secure_fs::write_private opens
with truncate(true) and then writes, so a first boot that died or failed
between those two steps left a zero-byte file that every later boot accepted.

Fixed at both layers. load_or_create_token now refuses a file holding fewer
than MIN_TOKEN_LEN characters, and the middleware independently refuses an
empty configured token: router() takes its token from its caller, so the
comparison cannot rely on the loader being the only source. The length check
sits before the constant-time comparison and leaks nothing, since whether the
configured token is empty is not a secret and no comparison against the
presented value has happened at that point.

Refusing rather than regenerating is deliberate. A token file that exists but
cannot be a token means something went wrong that an operator should see, and
silently minting a new one would change the credential their tooling holds.

The middleware regression test was checked against the unfixed code: it fails
without the guard and passes with it. One test pins subtle's empty-slice
behaviour directly, since the guard's stated reason depends on it.

* Stop a deleted validator key from coming back after a restart

Two paths let an operator believe a key was removed when the definitions file
still held it, so the next restart loaded it and the validator signed again.
With no slashing-protection record, a key returning after the operator moved it
to another host is a double vote.

ValidatorStore::load never checked the definitions entry's voting_public_key
against the key its keystore actually holds. Every signing path uses the
derived key, so a mismatch could not produce a wrong signature, but the
keymanager's delete removes from the store by the derived key and from the
definitions file by the declared one, so a mismatch made retain match nothing
while the response reported "deleted". load now refuses such an entry, which
makes persist_delete's assumption true by construction instead of by hope. It
is fatal rather than skipped, consistent with every other failure in that loop:
a definitions file that does not describe its own keystores is a configuration
error to fix, not one to run half of.

delete_one decided "not_found" from the in-memory store alone. After a failed
delete the in-memory removal has already happened and the entry survives on
disk, so a retry answered not_found, which reads as "already gone". Acting on
that by importing the key elsewhere is what turns a failed delete into two
hosts signing for one validator. It now reads the definitions file first,
through definitions_contain, before the removal makes the store an unreliable
witness. A disabled entry counts as present, and an unparseable declared key
counts as absent, matching exactly what persist_delete will and will not
remove.

Four tests: three on the load-time cross-check including the positive case, and
one driving a real HTTP delete against the state a failed delete leaves. The
first uses `let else` rather than expect_err because ValidatorStore has no
Debug by design, and a test is not a reason to weaken that.

* Roll back a failed import so the next key in the batch does not commit it

One ValidatorDefinitions vector is threaded through every key in an import
request, and each key mutates it and re-saves it. persist_import pushed or
replaced its entry before calling save, and a save failure left that mutation
in place, so the next key's successful save wrote it to disk. A key this
request reported as `error` was therefore enabled on the next restart and
signing. With no slashing-protection record, a validator the operator believes
was never imported is exactly the kind that ends up running in two places.

persist_import now records whether it appended or replaced, and restores the
vector before returning the error. The append case pops rather than removing by
index, which is correct because this function pushes and saves with nothing in
between.

Two tests, one per path. Both make the save fail while the writes preceding it
still succeed, by putting a directory where validator_definitions.yml goes: the
keystore and password writes land, so the mutation is reached, and only the
final rename fails. The replace-path test gives the previous entry
enabled: false so the restored value is distinguishable from what the import
would have written, rather than passing by coincidence.

* Make failover work for a node that answers wrongly rather than not at all

Two bugs, both from an Ok that failover could not see past.

A beacon node stuck on a stale head answered attestation_data successfully with
data for the wrong slot. The slot and target-epoch checks lived in
AttestationService, above FallbackBeaconNode, so try_each had already accepted
that answer and returned by the time they ran. The node was therefore seen as
a success every slot, the next node was never consulted, and the validator
missed every attestation while both nodes looked up. A comment in
attestation.rs claimed rejecting there "lets failover try the next node",
which was the opposite of what happened.

The checks now live in the implementations, behind a contract stated on
BeaconNodeApi::attestation_data, so a wrong answer is an Err that failover
treats like any other. One shared validate_attestation_data rather than two
copies: the implementation calls it so failover can act, and the service calls
it again before signing, because the trait is public and nothing forces an
implementation to honour its contract. MockBeaconNode honours it too, or the
test double would be more permissive than any real node and tests built on it
would stop reflecting production.

is_syncing returning Ok(true) is a node reporting itself unusable, but
try_each treats any Ok as the answer, so a syncing node at the head of the list
returned true and refresh_epoch declined to refresh duties for as long as it
was syncing, with a healthy node behind it unused. It no longer uses try_each:
it looks for a node that is not syncing, reports false as soon as it finds one,
and reports true only when every reachable node said so. A node that is down is
skipped, and only if none answers at all is this an error.

Six tests covering both, including a stale node failed over from, every node
stale, a syncing node masking a healthy one, and one node down with one syncing.

Also corrects two comments that asserted safety properties the code did not
have: try_each's claim to skip syncing nodes, and http.rs's claim that
inconsistent responses are raised by the caller.

* Bound a slot's attestation work by what is left of that slot

Nothing bounded it. HttpBeaconNode's per-request timeout is 8 seconds, failover
tries each node in turn, and attest makes two calls, so one hung node could
carry a 12-second slot's duty past the slot itself and into the next one, whose
duty was then late in turn. An attestation that misses its slot is worth
little; one that also delays the next slot's is worth less than nothing.

SlotClock gains end_of and remaining_in, and the duty loop wraps attest in a
timeout of whatever is left. remaining_in returns zero for a slot already gone
rather than erroring, so a hopeless duty can be passed straight to the timeout
and fail at once instead of needing a branch.

Aborting the future can drop it after signing and before submission. That is
safe only because AttestationGuard records at signing time, so the abandoned
attestation cannot be re-signed next slot under the same target; it is simply
lost, which is what attest's own "do not retry within a slot" rule already
required. Without that guard this timeout would have opened a double-vote path
rather than closed a latency one, so the call site says so.

Adds ethlambda_validator_attestation_deadline_missed_total, distinct from
attestation_failures_total: that counts a duty that failed and returned, this
counts one that never returned in time, which points at a slow beacon node
rather than a rejected attestation.

Also registers attestations_refused_total in metrics::init(), which the earlier
commit adding it missed. That file documents why every series must be forced
there: an alert cannot fire on a series that does not exist, so absent and zero
are not the same thing.

* Document the missing slashing protection where an operator will actually see it

This client's most consequential property was recorded in exactly one place: a
rustdoc comment on the validator crate, pointing at
docs/superpowers/specs/2026-09-11-ethlambda-validator-design.md, which does not
exist in the repository. Nobody running a binary reads either. An operator
could build this branch, point it at mainnet keys and never meet the warning.

Now said in five places.

A startup warning, unconditional and at warn level, before any key is loaded or
any beacon node contacted. Not behind a flag: a warning an operator can silence
is one they will, and the consequence here is losing stake.

A spec_deviations.md entry. That page was scoped to leanSpec, so its
introduction now states that this repository builds for two chains against two
references, and that the validator client is measured against consensus-specs
and the keymanager API. The entry covers what EIP-3076 requires, what exists
instead, what AttestationGuard does and does not cover, what it means for a key
migrated through the keymanager API, and what to do operationally. The
introduction flags it as the only deviation on the page that costs money rather
than performance.

A cli.md section, which previously did not mention the subcommand at all: every
flag, what the client does today, and an explicit list of what is not
implemented, so nobody infers from "validator client" that it proposes blocks.

The ethlambda_validator_* series in metrics.md, none of which were documented,
including why three similar-looking failure counters are separate series.

The crate doc now points at the deviations entry rather than a file that is not
there.

* Refuse to sign a second block for a slot this process already proposed

The proposal-shaped counterpart to AttestationGuard, and the same disclaimer
applies: this is not slashing protection, keeps nothing on disk, and a restart
empties it. It closes the double-proposal shapes reachable inside one run.

Two of them, both reachable without a hostile beacon node or a second instance.
The duty loop takes its slot from SystemTime::now(), which is not monotonic, so
an NTP correction can re-enter a slot already proposed; the block produced the
second time differs from the first, because the node has packed whatever
attestations arrived in between. And proposer duties are only final once the
epoch's randao is fixed, so a refresh landing late can move a validator between
slots inside one epoch.

A proposer slashing is worse than a double vote in one specific way: it needs no
second validator to be caught. The two signed headers are the whole evidence.

The rule is EIP-3076's minimal variant for blocks, per validator: remember the
highest slot proposed and refuse anything that does not strictly advance it.
Equality is refused along with regression, because the same slot proposed twice
is slashable unless the two blocks are byte-identical, and this guard does not
keep enough to tell those apart.

A separate guard rather than a field on AttestationGuard. The two judge
different things, an attestation on its source and target epochs and a block on
its slot, and a validator can legitimately propose in an epoch it also attests
in. One merged record would either refuse a legal proposal or admit an illegal
one depending on which field won.

Tested including the case a HashMap makes easy to get wrong: slot 0 is a real
slot, so a validator that proposed it must not be allowed to propose it again.

* Sign the RANDAO reveal and the block, not just the attestation

A proposer owes two signatures beyond the attestation this crate could already
produce, and neither shares the attestation's domain or its message shape.

The RANDAO reveal signs the epoch itself, not anything about the block. That is
what makes it both unforgeable and unchooseable: a proposer has exactly one
valid signature to offer for its slot's epoch, so it cannot grind the
randomness by trying alternatives. The message is the epoch's merkle root,
which for a uint64 is its eight little-endian bytes zero-padded to thirty-two,
and a test pins that rather than trusting it, since signing the big-endian
bytes or the raw eight would produce a reveal the network rejects with no way
to tell why.

The block signature takes an already-computed root rather than a block, so it
stays independent of which fork's block shape a beacon node produced. Its
domain comes from the epoch containing the block's own slot, which is the one
place a reader might reach for the attestation rule by habit: an attestation's
domain comes from its target epoch, a block's does not.

All three now funnel through one private sign_root, so there is a single place
a signature is produced and a single place the remote-signer method will be
added. It stays private deliberately: a caller that can hand in an arbitrary
root can make this client sign anything, and the domain separation that keeps
an attestation from being read as a block lives in the methods above it.

That separation is stated as a test rather than as three different constants.
One object root signed under the three domains must give three distinct signing
roots. It is not hypothetical for the reveal: its message is a bare uint64
merkle root, which is also a well-formed block root, so without the domain a
reveal for epoch N would be a valid proposer signature for a block whose root
happened to be N's merkle root.

The fork-boundary test crosses a real one. domain() only varies with the epoch
insofar as the epoch selects a different fork version, so two large epochs
inside one fork would assert nothing.

* Wake the duty loop at the slot boundary so a proposal has somewhere to happen

The loop woke once per slot, one third of the way in, because an attestation was
the only thing it had to do. A proposer publishes at the boundary, so a loop
that only ever runs a third of the way into a slot cannot propose for it: by the
time it wakes, the slot it would be proposing for is already a third gone.

So it now wakes at the boundary and sleeps the rest of the way in. Everything a
slot needs is reachable from there, and nothing about the attestation changes:
it still runs at the same instant, still gets the rest of the slot as its
budget.

One thing does improve. The epoch refresh used to run at the attester offset
itself, so every second it spent came straight out of the attestation's
lateness. It now runs at the boundary with a third of a slot of headroom ahead
of it, and if it overruns anyway the remaining sleep is zero and the attestation
is late rather than skipped. It is still unbounded; bounding it to the headroom
would be worse, since the per-request timeout alone is twice that.

next_slot_start replaces next_attestation. Two differences worth naming. It
returns the slot to act on rather than an Option, because before genesis the
answer is slot 0 and the wait until it, so a client started early now sleeps
once, exactly until genesis, rather than waking every slot-length to ask again.
And standing exactly on a boundary returns the following slot, which the loop
depends on: it calls this immediately after finishing a slot's work, and
returning the slot just handled would spin it at zero delay forever.

until_attestation is both the sleep between a slot's proposal and attestation
work and, in the next commit, the proposal's budget. A proposer still waiting on
its beacon node when the attestation is due has already lost the block and must
not also cost this client's attesters their votes.

slot_duration() goes with next_attestation: the before-genesis fallback was its
only caller.

* Decode and republish a produced block as SSZ rather than JSON

To sign a block this client has to compute its hash_tree_root, and a root can
only come from the typed container. The JSON route would mean hand-writing a
field-for-field mapping of a whole BeaconBlockBody, its ExecutionPayload and
every operation list inside them, then trusting that mapping to be exact. One
wrong field order, one missing list, one integer read as decimal that was meant
as hex, and the client signs a root that is not the block's. That failure is
silent where it happens and surfaces as a block the network rejects.

Decoding the specification's own serialization removes the class of bug
outright. The bytes the node sent are the block; the root falls out of them.
Both endpoints support it: Accept on produceBlockV3, Content-Type on
publishBlockV2.

BlockContents lives here rather than in ethlambda-types because it is not a
consensus container. It is defined in beacon-APIs and nowhere else;
consensus-specs never mentions the name and so never states its SSZ encoding
either. The three-field container is what every implementation encodes, but it
is convention, not specification, and ethlambda-types is the authority on what
the chain itself agrees about.

Fulu needs its own pair, which is the part worth reading twice. PeerDAS did not
turn the response back into a bare block, which is the natural guess given that
fulu moves blob distribution to column sampling. The shape is unchanged. What
changed is kzg_proofs: one proof per cell rather than per blob, with a list
limit of FIELD_ELEMENTS_PER_EXT_BLOB * MAX_BLOB_COMMITMENTS_PER_BLOCK.

A draft of this claimed a six-blob fulu block would not fit electra's container
and a guard assertion in the test caught it: six blobs is 768 cell proofs, well
under 4096. The limits agree until a block holds more than thirty-two blobs.
Both facts are now tests, because the harmless one is what makes the other
dangerous: a single shared type appears to work today and would fail silently
later, since fulu made the per-block blob limit depend on the epoch precisely so
later forks can raise it.

Pre-deneb forks are refused rather than decoded. They are reachable only on a
chain that has not reached deneb, and admitting them means four more block
shapes to serve nobody.

Result is deliberately not imported in this module; libssz_derive's generated
code names Result unqualified and means std's, so this crate's one-argument
alias in scope makes every derive in the file fail to compile.

* Ask a beacon node for a block, and publish the signed one back

The two calls block proposal is made of, with the same contract the attestation
path already has and for the same reason.

A produced block is checked against the request inside the implementation, not
above it. FallbackBeaconNode wraps the call, so a check performed by the caller
runs only after one node's answer has been accepted and returned, and a node
stuck on a stale head is never failed over from. Two fields are checked, both
because both end up signed. The slot selects the signing domain and is the whole
of what the proposal guard keys on. The proposer index is not something this
client picks either: a node on a different fork computes a different proposer,
and a block naming someone else can only be signed uselessly while still burning
the guard's record for that slot, which then refuses the real duty.

builder_boost_factor=0 is how this client says it does not do the builder flow.
The parameter is a bid comparison, so zero makes the local execution payload win
unconditionally, and that is the specification's own way to demand an unblinded
block rather than a separate flag. A blinded answer is still rejected rather
than assumed away, because the parameter is a request and signing a block this
client cannot unblind would burn the slot for something that can never be sent.

The fork comes from Eth-Consensus-Version and a missing or unknown value is a
hard error, not a default. SSZ carries no type tag, so guessing the fork means
decoding into the wrong shape and signing whatever root falls out. The match is
case-insensitive: the schema's enum is lowercase, but nothing in the
specification says a client must compare it that way. A node that ignores the
Accept header and answers JSON is caught by content type, so it is reported as
the mismatch it is rather than as a malformed block.

publish_block returns Imported or BroadcastNotImported rather than unit, because
publishBlockV2 answers 202 for something that is neither success nor failure:
broadcast, but not in the node's own database. Flattening that into Ok would
report a proposal as clean when the node that made it cannot follow its own
block, which usually means an unsynced execution layer or a parent that is not
what this client thought.

The body is borrowed rather than owned so failover can offer it to the next node
without copying. A block with blobs runs to megabytes and would otherwise be
cloned once per configured node on every proposal, succeed or fail.

Publishing stays first-wins rather than broadcasting to every node. A node that
accepts a block gossips it, so the others receive it over the network anyway,
and a broadcast makes the outcome ambiguous: three nodes answering 200, 202 and
a timeout have no single answer to report.

The mock answers produce_block with the same fixture block_contents' own tests
decode, rather than a second one that could drift from it, and honours the
contract so a test double behaves like a stale node rather than a more
permissive one than any real implementation.

* Produce, sign and publish a block for a proposer duty

The attestation path's counterpart, shaped like it. What differs is the cost of
each step, and that changes where the checks sit.

The guard is consulted twice. The early check is not about safety, it is about
cost: producing a block makes the beacon node drive an execution-layer payload
build, the most expensive thing this client can ask of it, and finding out
afterwards that the slot was already proposed wastes all of it. The check before
signing is the one that matters, and it is the one that records. Recording there
rather than after publication is what makes the duty loop's deadline safe, since
a proposal abandoned between signing and publishing cannot be re-signed.

A test pins that the early check does not record, using one service and one node
throughout. Two services would carry two empty guards and the test would pass
whether or not the first one recorded anything.

Two facts about forks turned out to be separate, which a test caught by
asserting they were the same. The block is decoded, and must be published, under
the fork the node named in its response header; a re-encoding of what the node
produced has to be announced as what it is. The signing domain comes from this
client's own schedule, fetched from /config/spec at startup. They normally agree
because they came from the same beacon node. When they do not, one of them is
wrong about where a fork boundary sits, and signing anyway yields a block
rejected for a reason nothing in the logs would explain, so they are now
compared and a mismatch refuses before signing.

ProducedBlock therefore carries its fork rather than having it recomputed from
the slot. That is also finer-grained than the payload shape: deneb and electra
share a container but are different forks, and the wire has to be told which.

The store's read lock is taken twice, never across an await, for the reason the
attestation path documents: the keymanager's import holds the write lock across
a batch of slow key derivations, and a write-preferring RwLock would let it
queue ahead of every later duty.

The block's root is taken once from the decoded container, and the same value is
what the guard records against and what the signature covers.

A 202 is logged at warn and counted separately rather than folded into the
proposal counter. The block reached the network, so the proposal may well have
worked; what failed is the node's own import, which points at its execution
layer rather than at this client.

* Propose this slot's block from the duty loop

The last piece: the loop now looks for a proposer duty at the boundary it wakes
on, and runs it before sleeping the rest of the way to the attester offset.

The budget is that offset, not the end of the slot, and it is deliberately
tighter than the attestation's own. The specification defines no
block-production deadline; the nearest thing it does define is the point
attesters stop waiting for a block and vote for the previous head, which is the
same third of a slot. A block published after it has already lost most of its
value, while every second spent there comes straight out of this client's own
attestations, which are due for every validator it holds rather than for the one
proposing. So an overrunning proposal is abandoned and the slot's attesters
still vote.

Dropping that future mid-flight is safe for the same reason it is on the
attestation path, and only for that reason: ProposalService records the slot in
its guard at signing time, so a block abandoned between signing and publication
cannot be signed a second time.

The duty is cloned out of the schedule before the await. It is three small
fields, and holding a borrow across the proposal would stop the next epoch's
refresh from replacing the schedule it came from.

--graffiti is refused rather than truncated when it is too long. The operator
asked for a string, and a silently clipped one would appear in every block they
propose, where they are least likely to look for it. The limit is stated and
checked in bytes because that is what the field holds: seventeen accented
characters are thirty-four bytes, and an operator told "32 characters" would
have no way to work out why theirs was rejected.

It defaults to empty rather than to this client's name and version, which is
what most clients do. Putting it there tells anyone reading the chain which
software built a block, and an operator who wants that can ask for it.

* Tell the beacon node where to pay block rewards, and check that it did

Two halves of one problem, and the second exists because the first is only a
request.

prepare_beacon_proposer is re-sent every epoch rather than once at startup,
because the node forgets. It keeps a preparation for the epoch it arrived in and
two more, and loses all of them when it restarts, so a client that registered
once would silently stop being registered a few minutes later and find out by
proposing a block that paid someone else. A failed registration is logged rather
than propagated: its consequence is a payload built for the wrong address, which
the check below catches, while propagating it would cost this epoch's attester
duties, which is worse and unrelated.

The specification is explicit that a node need not honour the preparation, and
says a client should confirm the fee recipient in a produced block before
signing it. It does not say what to do when they differ, and both answers cost
money. Refusing forfeits the consensus-layer reward as well as the
execution-layer one and costs the network a slot; signing forfeits only the
execution reward, which was already going elsewhere the moment the node built
the payload. So this signs and complains at error level with a counter beside
it, because the thing that actually fixes a misconfigured beacon node is the
operator noticing, not a dropped block.

Failover has a gap here worth naming rather than hiding: a preparation reaches
only the node that answered, so failing over to a second node later in the epoch
finds it unprepared. The check before signing is what catches that.

--suggested-fee-recipient is optional but startup warns loudly when it is
absent, because without it every proposed block pays its execution rewards to an
address the beacon node chose. It stays optional because a client running only
attester duties has no use for one. A malformed address is refused at startup
rather than at first use: a proposal duty can be days away, which is the worst
moment to discover a typo in a flag.

* Document what the validator client does now that it proposes blocks

The three pages that describe this client all said it attests and does nothing
else, which stopped being true.

cli.md gains the two new flags, a walk through what a slot now looks like from
the boundary inward, and the reasoning behind the two choices an operator would
otherwise have to read the source to understand: why a proposal is abandoned at
the attester offset rather than at the end of the slot, and why blocks go over
SSZ when everything else on this path is JSON. Its "not implemented" list is
shorter by one entry and now names the builder flow explicitly, since asking for
an unblinded block and refusing a blinded one is a deliberate limit rather than
a gap.

It also gains a short section on proposer duties being the more fragile
schedule. An attester schedule depends on the block two epochs back and survives
anything shallower; a proposer schedule depends on the block one epoch back, so
a reorg that leaves attester duties untouched can still move a proposer. This
client refetches once per epoch, so that window is real and an operator who
loses a proposal after a reorg deserves to find the explanation written down.

metrics.md gains the six proposal series, and a note on which two should read
zero for the life of a healthy deployment and are therefore worth alerting on at
any value rather than on a rate: the two refusal counters mean this client's own
guards caught something, and the fee-recipient counter means blocks are paying
someone else.

spec_deviations.md now covers proposals in the slashing-protection entry rather
than attestations alone, including the rule the proposal guard enforces and why
a proposer slashing is the worse of the two: a double vote needs a second
validator's attestation to be caught, while two signed headers for one slot are
the whole of the evidence by themselves.

* Say "or a double block proposal" in the startup warning

The client proposes blocks now, so the one message an operator is guaranteed to
read was naming half of what it can lose stake for. A double vote needs a second
validator's attestation to be caught; two signed headers for one slot are the
whole of the evidence by themselves, so if either belongs in a warning it is
that one.

* Work out whether a validator is an aggregator, and sign what that needs

Aggregation's two pure pieces, ahead of the endpoints that use them.

The selection rule is not a choice and the code says why. A validator signs the
slot under a dedicated domain, hashes that signature, and aggregates when the
first eight bytes read as a little-endian integer divide evenly by a modulus
derived from the committee's size. BLS signatures are deterministic, so a
validator gets exactly one answer per slot and cannot search for a better one,
and cannot decline either, since the same computation is what a beacon node
checks the resulting aggregate against.

The modulus floors at one, which is not a guard against division by zero so much
as the intended answer for a small committee: every member of a committee that
cannot supply the target number of aggregators should supply one.

Two tests exist because the obvious one cannot fail. Reading the digest
big-endian would still select about one validator in `modulo`, so a statistical
test looks correct either way; only comparing the two orderings on a signature
where they disagree catches it. The statistical test is kept anyway, with loose
bounds, since the count is binomial and pinning it exactly would be testing
SHA-256 rather than this function.

The two new signatures round out the five this client produces, and the
domain-separation test now asserts the property across all of them rather than
three. One case is sharp enough to state separately: a RANDAO reveal signs an
epoch's merkle root and a selection proof signs a slot's, both bare uint64
roots, so for epoch N and slot N the signed object is byte for byte identical
and only the domain tells them apart. A test asserts the collision first, so it
cannot quietly stop proving anything.

Neither new signature is slashable. That is worth stating for the aggregate one,
which is the only signature here covering an attestation and not being guarded:
the slashing conditions are about a validator's own vote, and an aggregator is
republishing other validators' votes with a wrapper saying who collected them.

TARGET_AGGREGATORS_PER_COMMITTEE stays in this crate rather than moving to
ethlambda-types. It governs how this client behaves, not what the chain agrees
about, and no container's shape depends on it.

* Give the slot clock a second offset, two thirds in, for aggregation

A slot now has three points a duty falls on rather than two: the boundary for a
proposal, a third in for an attestation, two thirds in for an aggregation.

Aggregation has to come after attestation, not merely elsewhere. An aggregator
folds together votes its beacon node has collected, and before the attesters
have voted there is nothing to fold. A test asserts the ordering across several
slots rather than trusting the two constants to stay in the right order.

The offsets now multiply before dividing. Dividing first would put a 10-second
slot's duties at 3 and 6 seconds instead of 3.33 and 6.66, which is a third of a
second of drift on something that is already a deadline. The named constants
went from one divisor to a part-of-three pair, so the arithmetic reads as what
it is rather than as two unrelated fractions.

Duration division keeps nanosecond precision, so the test that pins the uneven
case asserts exact thirds rather than rounded milliseconds, and checks the two
gaps are equal rather than just checking each endpoint.

* Claim the aggregator role in the subscriptions that already go out

The flag was hardcoded false with a comment saying aggregation was out of
scope. It is in scope now, and the flag is not a preference: it is the answer
the selection rule computes from a signature over the slot, and the beacon node
checks the same thing when the aggregate arrives.

What the flag buys is why it has to be sent an epoch ahead rather than
discovered when the aggregation duty runs. It tells the node to hold the subnet
subscribed and collect the votes this client will later ask it to fold. A client
that decided at aggregation time would be asking a node that had not been
listening.

selection_for is one function for both callers because they have to agree.
Computing the two differently would either claim a role this client never
performs, leaving a committee's aggregate to nobody, or perform one it never
claimed, against a node with nothing collected. It returns the proof rather than
a bool, since the caller that acts on it needs the signature itself: it goes
into the published AggregateAndProof, which is what makes the selection
verifiable rather than self-declared.

A duty whose selection cannot be computed is subscribed with the flag clear
rather than dropped. The subscription is what puts the beacon node on the subnet
at all, so losing it would cost that validator its plain attestation as well as
its aggregate.

Two tests, because one is not enough. The first checks the flag against the
selection rule over a range of slots, which a bug that always answered the same
way would also satisfy. The second checks that across 64 slots the role is
claimed for some and not others.

refresh_epoch now takes the store and signing context, since the subscription it
sends needs to sign.

* Fetch and publish the aggregate this client was selected to produce

The half that was missing. Subscriptions already claimed the role; now the
client performs it, so the claim is no longer a promise it breaks.

This client does not build the aggregate. The beacon node does, out of votes it
collected on the subnet the subscription put it on, and hands back the best one
it has. What this client contributes is the wrapper: which of its validators is
publishing, the proof that validator was selected, and a signature over both.
That division is what keeps electra's trap out of reach. A gossiped aggregate
must cover exactly one committee even though the container widened to allow
more, and the multi-committee form exists only on chain, assembled by a
proposer; a client that never constructs an aggregate cannot get that wrong.

The attestation duty now hands its data forward rather than the aggregation
duty re-fetching it. The aggregate to ask for is the one covering the votes
this client's validators just cast, and a head that moved in between would give
different data whose aggregate contains none of them. It is an Option, not a
zeroed default, because a caller handed a default would ask for the aggregate
of an attestation nobody made. It is Some whenever the fetch succeeded even if
nothing was published, since an aggregator collects the whole committee's
votes and still owes the duty when its own signatures were refused.

Only the v2 endpoints exist. The v1 pair was removed, not deprecated, because
electra forces AttestationData.index to zero: the data root no longer
distinguishes one committee's votes from another's in the same slot, so
committee_index had to become a separate required parameter and the old shape
had no way to ask the question. There is no v1 fallback and there should not be.

One fetch per committee rather than per validator. Two of this client's
validators can be selected for the same committee and publish one wrapper each
around the identical aggregate; asking twice would be the same answer at twice
the cost.

A 404 is an ordinary outcome, not a failure. It means the node had nothing to
fold, and one committee's missing aggregate must not stop another's from going
out. It is still worth failing over, since a second node may have been on the
subnet when the votes arrived.

The batch goes out under a single consensus-version header, so all its
aggregates must share a fork. With failover in play they can come from different
nodes, which is the only way they would not.

The aggregation deadline is the end of the slot rather than the attester offset
the proposal uses, because what it competes with differs: a proposal that
overruns eats this client's own attestations, while an aggregation is already
the last duty in its slot. Abandoning it is cheap and safe, since nothing here
is slashable and other aggregators were selected for the same committee.

The aggregate query string is built by a pure function with a test, because it
uses a string continuation: Rust strips the newline and the indentation after
it, and a misplaced one would put a space inside a query parameter, making the
node answer about a different committee with nothing naming the cause.

* Document aggregation, and say where the duty offsets come from

The three pages now cover all three duties. cli.md gains what a slot looks like
with aggregation in it, and why the role is not a choice: a validator signs the
slot, the hash of that signature decides, signatures are deterministic, and the
beacon node checks the same thing when the aggregate arrives.

Both cli.md and the slot clock now say where the offsets come from, because the
specification moved and this code did not. The honest-validator guide used to
express them as fractions of SECONDS_PER_SLOT and now states them as basis
points of a slot duration the beacon node reports: 3333 and 6667. On mainnet's
12-second slot those are 3999 ms and 8000 ms, so the aggregation offset here is
exact and the attestation offset is a millisecond late, well inside the network
latency it competes with. Thirds are kept rather than reading two config values
for a millisecond, but that stops being a rounding question at gloas, which
moves the offsets to 2500 and 5000 basis points. Written down so whoever meets
that fork knows it is a decision and not an oversight.

Also drops a test that asserted Root::ZERO equals Root::ZERO. Its real content
was that both duty aliases name one generic type, which the compiler already
enforces, so it looked like it checked something and did not.

* Stop an overrunning slot from costing the next slot its duties as well

The duty loop asked the clock for "the next slot after now", which is the wrong
question once the previous slot's work has run long.

A slot's duties are bounded by that slot's end, so a hung beacon node returns
the loop to this call at, or just after, the following slot's boundary. The
clock then answered with the slot *after* the one the loop was standing in, and
the loop slept straight past a slot whose attestation was still a third of a
slot away. One hung request cost two slots of duties for every validator held,
which is the cascade the per-slot budget was added to prevent.

Concretely on mainnet: the attestation timeout fires at exactly end_of(S), which
is start_of(S+1). slot_at(now) is then S+1, the old code returned S+2, and slot
S+1 got neither a proposal nor an attestation despite four seconds of its
attestation window remaining.

So the clock now takes the slot just served. If the wall clock has already
entered a later slot, that slot is returned with no delay and served at once;
its own offsets degrade to zero on their own, so its attestation goes out late
rather than not at all. Otherwise the answer is the slot after the one served,
which is also what keeps the loop from spinning on a slot it just finished.

A backwards clock step is handled by the same rule and idles until the stepped
clock catches up, which is right: the earlier slots have been served, and the
duties for a slot that has not happened yet cannot be fetched.

The aggregation work landing alongside this makes it more likely, not less: the
loop body now sleeps to the two-thirds offset before its last duty, so it ends
within a few seconds of the boundary as a matter of course and any hiccup
crosses it.

* Send subscriptions and fee recipients to every node, not the first that answers

try_each is the right rule for a query: there is one right answer and any node
can give it. It is the wrong rule for these two calls, which install state on a
node rather than ask it something. A node that was never told is a node that
cannot serve the duty later.

With two nodes configured, try_each told node 1 which committees this client
attests in and which it will aggregate for, and node 2 heard nothing. When node
1 went down mid-epoch, every later call failed over to the node least prepared
to answer it: not holding those attestation subnets open, and holding none of
the votes an aggregate is folded from, so 404 for every committee this client
asked about. The failover node exists for exactly that moment and was the one
guaranteed to be useless in it.

The fee recipient had the same shape. That one was already documented as a known
limitation with the pre-signing check named as the mitigation, which was true but
is no longer the best available answer.

try_all succeeds if any node accepted rather than requiring all, so one
unreachable node does not stop the reachable ones being told, and logs per node
so a partially registered client is visible rather than silent.

A test pins the opposite rule too, so the two do not drift: a block request
still stops at the first node that answers, because asking every node for a
block would make every node build one.

* Refuse a deneb block by name instead of failing to decode it as electra

Both forks were routed through the same container, which cannot work: a deneb
BeaconBlockBody has twelve fields where electra's has thirteen, so the two have
different fixed-size prefixes and a deneb body fails electra's decoder on its
first offset.

The failure was at least the safe one. It is a hard decode error, not a silent
mis-decode producing a signature over the wrong block, and a post-electra chain
never reaches it because the proposal path already refuses to sign when the
node's fork and this client's schedule disagree. What it cost was honesty: on a
deneb chain the client claimed support and then proposed nothing, reporting a
malformed block each time.

Deneb now joins the refused arm rather than gaining a container pair, because
the pair would serve nobody. The attestations this client submits are electra's
SingleAttestation, which has no pre-electra form, so a chain it cannot propose
on is one it could not attest on either. Refusing by name says that; a decode
error does not.

* Say why the store's read guard is narrow, now that the old reason is stale

Four comments justified scoping the read guard away from every await by saying
the keymanager's import holds the write lock across a batch of slow EIP-2335
derivations. That was true once. The import now does the derivation in
prepare_import, outside the lock, and takes the write lock only for the
synchronous inserts, which its own comment explains.

The scoping is still right; the reason given for it was no longer the reason.
What actually makes a wide read guard costly is that tokio's RwLock is
write-preferring, so a single keymanager writer queuing behind a long-held
reader blocks every reader after it. Keeping these guards narrow bounds that
wait, and is the duty path keeping its half of a bargain the import already
keeps.

* Refuse to sign against a beacon node that is optimistic, not just one syncing

The client asked its node whether it was syncing and signed whenever the answer
was no. A node can answer no and still be tracking a head its execution client
has not validated, and the specification is explicit about that case: an
optimistic validator MUST NOT produce a block and MUST NOT participate in
attestation, naming the proposer, attester, selection and aggregate domains.

The field wa…
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.

1 participant