Update state before producing attestation (#1596)

## Issue Addressed

Partly addresses #1547 

## Proposed Changes

This fix addresses the missing attestations at slot 0 of an epoch (also sometimes slot 1 when slot 0 was skipped).
There are 2 cases:
1. BN receives the block for the attestation slot after 4 seconds (1/3rd of the slot).
2. No block is proposed for this slot.

In both cases, when we produce the attestation, we pass the head state to the 
`produce_unaggregated_attestation_for_block` function here
9833eca024/beacon_node/beacon_chain/src/beacon_chain.rs (L845-L850)

Since we don't advance the state in this function, we set `attestation.data.source = state.current_justified_checkpoint` which is atleast 2 epochs lower than current_epoch(wall clock epoch). 
This attestation is invalid and cannot be included in a block because of this assert from the spec:
```python
if data.target.epoch == get_current_epoch(state):
        assert data.source == state.current_justified_checkpoint
        state.current_epoch_attestations.append(pending_attestation)
```
https://github.com/ethereum/eth2.0-specs/blob/dev/specs/phase0/beacon-chain.md#attestations

This PR changes the `produce_unaggregated_attestation_for_block` function to ensure that it advances the state before producing the attestation at the new epoch.

Running this on my node, have missed 0 attestations across all 8 of my validators in a 100 epoch period 🎉 
To compare, I was missing ~14 attestations across all 8 validators in the same 100 epoch period before the fix. 

Will report missed attestations if any after running for another 100 epochs tomorrow.
This commit is contained in:
Pawan Dhananjay 2020-09-08 11:25:43 +00:00
parent 19be7abfd2
commit 00cdc4bb35

View File

@ -853,16 +853,16 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
if state.slot > slot {
return Err(Error::CannotAttestToFutureState);
} else if state.current_epoch() + 1 < epoch {
} else if state.current_epoch() < epoch {
let mut_state = state.to_mut();
while mut_state.current_epoch() + 1 < epoch {
while mut_state.current_epoch() < epoch {
// Note: here we provide `Hash256::zero()` as the root of the current state. This
// has the effect of setting the values of all historic state roots to the zero
// hash. This is an optimization, we don't need the state roots so why calculate
// them?
per_slot_processing(mut_state, Some(Hash256::zero()), &self.spec)?;
}
mut_state.build_committee_cache(RelativeEpoch::Next, &self.spec)?;
mut_state.build_committee_cache(RelativeEpoch::Current, &self.spec)?;
}
let committee_len = state.get_beacon_committee(slot, index)?.committee.len();