lighthouse/beacon_node/beacon_chain/tests
Michael Sproul edf23bb40e Fix attestation shuffling filter (#3629)
## Issue Addressed

Fix a bug in block production that results in blocks with 0 attestations during the first slot of an epoch.

The bug is marked by debug logs of the form:

> DEBG Discarding attestation because of missing ancestor, block_root: 0x3cc00d9c9e0883b2d0db8606278f2b8423d4902f9a1ee619258b5b60590e64f8, pivot_slot: 4042591

It occurs when trying to look up the shuffling decision root for an attestation from a slot which is prior to fork choice's finalized block. This happens frequently when proposing in the first slot of the epoch where we have:

- `current_epoch == n`
- `attestation.data.target.epoch == n - 1`
- attestation shuffling epoch `== n - 3` (decision block being the last block of `n - 3`)
- `state.finalized_checkpoint.epoch == n - 2` (first block of `n - 2` is finalized)

Hence the shuffling decision slot is out of range of the fork choice backwards iterator _by a single slot_.

Unfortunately this bug was hidden when we weren't pruning fork choice, and then reintroduced in v2.5.1 when we fixed the pruning (https://github.com/sigp/lighthouse/releases/tag/v2.5.1). There's no way to turn that off or disable the filtering in our current release, so we need a new release to fix this issue.

Fortunately, it also does not occur on every epoch boundary because of the gradual pruning of fork choice every 256 blocks (~8 epochs):

01e84b71f5/consensus/proto_array/src/proto_array_fork_choice.rs (L16)

01e84b71f5/consensus/proto_array/src/proto_array.rs (L713-L716)

So the probability of proposing a 0-attestation block given a proposal assignment is approximately `1/32 * 1/8 = 0.39%`.

## Proposed Changes

- Load the block's shuffling ID from fork choice and verify it against the expected shuffling ID of the head state. This code was initially written before we had settled on a representation of shuffling IDs, so I think it's a nice simplification to make use of them here rather than more ad-hoc logic that fundamentally does the same thing.

## Additional Info

Thanks to @moshe-blox for noticing this issue and bringing it to our attention.
2022-10-18 04:02:06 +00:00
..
attestation_production.rs Use async code when interacting with EL (#3244) 2022-07-03 05:36:50 +00:00
attestation_verification.rs Use async code when interacting with EL (#3244) 2022-07-03 05:36:50 +00:00
block_verification.rs Consensus context with proposer index caching (#3604) 2022-10-15 22:25:54 +00:00
main.rs Retrospective invalidation of exec. payloads for opt. sync (#2837) 2022-02-28 22:07:48 +00:00
merge.rs Don't create a execution payload with same timestamp as terminal block (#3331) 2022-07-18 23:15:41 +00:00
op_verification.rs Use async code when interacting with EL (#3244) 2022-07-03 05:36:50 +00:00
payload_invalidation.rs Deduplicate block root computation (#3590) 2022-09-23 03:52:42 +00:00
store_tests.rs Fix attestation shuffling filter (#3629) 2022-10-18 04:02:06 +00:00
sync_committee_verification.rs Use async code when interacting with EL (#3244) 2022-07-03 05:36:50 +00:00
tests.rs Deduplicate block root computation (#3590) 2022-09-23 03:52:42 +00:00