`LightClientUpdate` structures currently use different merkle proof root
depending on the presence of `finalized_header`. By always rooting it in
the same state (the `attested_header.state_root`), logic gets simpler.
Caveats:
- In periods of extended non-finality, `update.finalized_header` may now
be outdated by several sync committee periods. The old implementation
rejected such updates as the `next_sync_committee` in them was stale,
but the new implementation can properly handle this case.
- The `next_sync_committee` can no longer be considered finalized based
on `is_finality_update`. Instead, waiting until `finalized_header` is
in the `attested_header`'s sync committee period is now necessary.
- Because `update.finalized_header > store.finalized_header` no longer
holds (for updates with finality), an `is_better_update` helper is
added to improve `best_valid_update` tracking (in the past, finalized
updates with supermajority participation would always directly apply)
This PR builds on prior work from:
- @hwwhww at https://github.com/ethereum/consensus-specs/pull/2829
The producer of `LightClientUpdate` structures usually does not know how
far the `LightClientStore` on the client side has advanced. Updates are
currently rejected when including a redundant `next_sync_committee` not
advancing the `LightClientStore`. Behaviour is changed to allow this.
- committee_index is used as an input to compute_subnet_for_attestation but it's not previously defined
- attestation.data.committee_index is incorrect, the field is "index"
When `state.finalized_checkpoint` references the genesis slot, it points
to an empty `root`, instead of the actual genesis block hash. This patch
updates the `LightClientUpdate` logic to allow including finality proofs
for genesis `finalized_checkpoint.root`, better supporting non-mainnet.
When including such a finality proof, the proof is for the empty `root`,
but `finalized_header` is kept zeroed out to signify this edge case.
The `fork_version` field in `LightClientUpdate` can be derived from the
`update.signature_slot` value by consulting the locally configured fork
schedule. The light client already needs access to the fork schedule to
determine the `GeneralizedIndex` values used for merkle proofs, and the
memory layouts of the structures (including `LightClientUpdate`). The
`fork_version` itself is network dependent and doesn't reveal that info.
* t push base design for partial withdrawals
* moor tests
* clean up withdrawals naming
* make partial withdrawal randomized tests better
* Apply suggestions from code review
Co-authored-by: Alex Stokes <r.alex.stokes@gmail.com>
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
* fix mainnet brokn test
* name swap
* lint
Co-authored-by: Alex Stokes <r.alex.stokes@gmail.com>
Co-authored-by: Hsiao-Wei Wang <hsiaowei.eth@gmail.com>
* deprecate `BeaconBlocksByRange.step`
The `step` parameter has not seen much implementation in real life
clients which instead opt to request variations on a few epochs at a
time (instead of interleaving single blocks, entire epochs are
interleaved).
At the same time, supporting `step` on the server side brings several
complications: more complex bounds checking logic, more complex loading
of blocks from linear storage (which presumably stores all blocks and
not just certain increments).
This PR suggests that we deprecate the whole idea. Backwards
compatibility is kept by simply responding with a single block when
`step > 0` - this is allowed by the spec and should thus be handled
gracefully by requesting clients already, should there exist any that
use larger step counts.
Removing `step` now allows simplifying the EL-CL protocol for serving
execution data from the EL to avoid double storage.
* Update specs/phase0/p2p-interface.md
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
Co-authored-by: Danny Ryan <dannyjryan@gmail.com>
* Ignore subset aggregates
When aggregates are propagated through the network, it is often the case
that a better aggregate has already been seen - in particular, this
happens when an aggregator has not been able to include itself in the
mesh and therefore publishes an aggregate with only its own
attestations.
This new ignore rule allows dropping all aggregates that are
(non-strict) subsets of aggregates that have already been seen on the
network. In particular, it does not mandate dropping aggregates where a
union of previous aggregates would cause it to become a subset).
The logic for allowing this is based on the premise that any aggregate
that has already been seen by a peer will also have been seen by its
neighbours - a subset aggregate (strict or not) brings no new value to
the aggregation algorithm, except in the extreme edge case where you
could combine several such sparse aggregates into a single, more dense
"combined" aggregate and thus use less block space.
Further, as a small benefit, computing the `hash_tree_root` of the full
aggregate is generally not done -however, `hash_tree_root(data)` is
already done for other purposes as this is used as index in the beacon
API.
* add subset ignore rule to sync contributions as well
* typo
As the sync committee signs the previous block, the situation arises at
every sync committee period boundary, that the new sync committee signs
a block in the previous sync committee period. The light client cannot
reliably detect this condition (e.g., assume that this is the case when
it is currently on the last slot of a sync committee period), because
the last couple slots of a sync committee period may not have a block.
For example, when receiving a `LightClientUpdate` that is constructed
as in the following illustration, it is unknown whether `sync_aggregate`
was signed by the current or next sync committee at `attested_header`.
```
slot N N + 1 | N + 2 (slot not sent!)
|
+-----------------+ \ / | +----------------+
| attested_header | <--- X ----|---- | sync_aggregate |
+-----------------+ / \ | +----------------+
missed |
|
sync committee
period boundary
```
This patch addresses this edge case by including the slot at which the
`sync_aggregate` was created into the `LightClientUpdate` object.
Note that the `signature_slot` cannot be trusted beyond the purpose of
signature verification, as it could be manipulated to any other slot
within the same sync committee period and fork version, without making
the `sync_aggregate` invalid.
Documentation on how to call `prepare_execution_payload` had the params
for `safe_block_hash` and `finalized_block` hash flipped compared to the
function definition. Also updated tests for consistency.
When nodes are syncing but have not yet reached the canonical `head`,
they cannot determine whether nodes they are connected to serve a valid
history or are making bogus claims in their `Status` advertisement.
Thus, the best course of action that a client can take is to vote for
its "current" best synced head, regardless of whether it's connected to
peers that claim to have other heads or not.
However, in the p2p spec, we penalize such peers with a `REJECT` - this
should be an `IGNORE` instead because this vote is correct per the spec,
albeit "late" according to the validating clients' view of the chain.
The spec reserves the libp2p error code range `[3, 127]` for future use
but actually defines error code `3` as `ResourceUnavailable`. This patch
updates the reserved range to `[4, 127]`.
When a light client updates its `finalized_header` using a forced update
because of the timeout, and the new header was not signed by enough sync
committee participants to pass `get_safety_threshold(store)`, it may
occur that `store.finalized_header.slot > store.optimistic_header.slot`.
This patch ensures that the `optimistic_header` is updated to the latest
`finalized_header` if that happens, so that it always indicates the
latest known and accepted head.
There were a couple instances where a division was used on an epoch
to derive the corresponding sync committee period instead of calling the
`compute_sync_committee_period` function.
These instances were changed to also use the function.
In the light client docs a mentioning of a function trigger is lacking
the `genesis_validators_root` argument. This patch adds that argument
to the documentation to match the real function signature. It also
slightly improves the grammar.
This renames the `sync_committee_aggregate` field of `LightClientUpdate`
to `sync_aggregate` for consistency with the terminology in the rest of
the spec.