`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
Various cleanups and minor fixes:
- Consistent terminology:
- `signed_block` -> `attested_block`
- `finalized_block_header` -> `finalized_header`
- `snapshot_period` -> `store_period`
- Use correct block in finality test (`blocks[-1]` instead of new one)
- Add `signed_block_header` func to get header from `SignedBeaconBlock`
- Remove `block_header` from `get_sync_aggregate` helper arguments
- Use `state_transition_with_full_block` as shortcut for multiple calls
- Have `finalized_header` actually be header instead of full block body
- Consistent ordering of `assert` to match structure definition
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.
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.
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.
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.
This renames the `sync_committee_aggregate` field of `LightClientUpdate`
to `sync_aggregate` for consistency with the terminology in the rest of
the spec.
Some tests are currently restricted to a single phase using @with_phases
even though they could likely run unchanged in later phases. This patch
changes the default for such tests to also run in later phases. If the
beacon chain changes enough in later phases to break these tests, this
highlights that the tests need to be adjusted or extended accordingly.
Building merkle proofs is required functionality for implementing light
client sync. Although the spec currently only defines a function to
verify merkle proofs (`is_valid_merkle_branch`) there are still a few
PySpec unit tests that produce merkle proofs. This patch adds a new
generator to extract test vectors from those static unit tests, so that
light client implementations can validate their merkle proof logic.
The `test_next_sync_committee_tree` currently only supports the minimal
preset, as it incorrectly initializes the `next_sync_committee`. On the
mainnet preset, `SYNC_COMMITTEE_SIZE` is 512, but the default states use
only 256 validators, leading to an IndexError during the test execution.
`next_sync_committee` is already initialized correctly prior to the test
run using the spec's `get_next_sync_committee` function, which fills up
extra committee slots with duplicate validators in this scenario. This
makes it unnecessary to manually initialize the `next_sync_committee`.
Removed the incorrect initialization to allow testing on mainnet preset.
There are three defined unit tests for the light client sync protocol.
They all follow a similar structure. However, there is an inconcistency
how they pass the slot to compute_aggregate_sync_committee_signature.
In one instance it is passed as `block.slot`. In the other two cases
it is passed as `block_header.slot`. As the `block_header` is created
from the `block`, they share the same value. This patch makes the way
how the slot is passed consistent across all of the test cases.
Etan Kissling
Hsiao-Wei Wang
Justin Traglia
protolambda
Danny Ryan
Alex Stokes
Antonio Sanso