Add fork choice EF tests (#2737)

## Issue Addressed

Resolves #2545

## Proposed Changes

Adds the long-overdue EF tests for fork choice. Although we had pretty good coverage via other implementations that closely followed our approach, it is nonetheless important for us to implement these tests too.

During testing I found that we were using a hard-coded `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` value rather than one from the `ChainSpec`. This caused a failure during a minimal preset test. This doesn't represent a risk to mainnet or testnets, since the hard-coded value matched the mainnet preset.

## Failing Cases

There is one failing case which is presently marked as `SkippedKnownFailure`:

```
case 4 ("new_finalized_slot_is_justified_checkpoint_ancestor") from /home/paul/development/lighthouse/testing/ef_tests/consensus-spec-tests/tests/minimal/phase0/fork_choice/on_block/pyspec_tests/new_finalized_slot_is_justified_checkpoint_ancestor failed with NotEqual:
head check failed: Got Head { slot: Slot(40), root: 0x9183dbaed4191a862bd307d476e687277fc08469fc38618699863333487703e7 } | Expected Head { slot: Slot(24), root: 0x105b49b51bf7103c182aa58860b039550a89c05a4675992e2af703bd02c84570 }
```

This failure is due to #2741. It's not a particularly high priority issue at the moment, so we fix it after merging this PR.
This commit is contained in:
Paul Hauner
2021-11-08 07:29:04 +00:00
parent d01fe02824
commit 931daa40d7
17 changed files with 648 additions and 36 deletions

View File

@@ -10,14 +10,12 @@ use beacon_chain::{
BeaconChain, BeaconChainError, BeaconForkChoiceStore, ChainConfig, ForkChoiceError,
StateSkipConfig, WhenSlotSkipped,
};
use fork_choice::{
ForkChoiceStore, InvalidAttestation, InvalidBlock, QueuedAttestation,
SAFE_SLOTS_TO_UPDATE_JUSTIFIED,
};
use fork_choice::{ForkChoiceStore, InvalidAttestation, InvalidBlock, QueuedAttestation};
use store::MemoryStore;
use types::{
test_utils::generate_deterministic_keypair, BeaconBlock, BeaconBlockRef, BeaconState,
Checkpoint, Epoch, EthSpec, Hash256, IndexedAttestation, MainnetEthSpec, Slot, SubnetId,
ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, IndexedAttestation, MainnetEthSpec, Slot,
SubnetId,
};
pub type E = MainnetEthSpec;
@@ -232,7 +230,7 @@ impl ForkChoiceTest {
/// Moves to the next slot that is *outside* the `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` range.
pub fn move_outside_safe_to_update(self) -> Self {
while is_safe_to_update(self.harness.chain.slot().unwrap()) {
while is_safe_to_update(self.harness.chain.slot().unwrap(), &self.harness.chain.spec) {
self.harness.advance_slot()
}
self
@@ -240,7 +238,7 @@ impl ForkChoiceTest {
/// Moves to the next slot that is *inside* the `SAFE_SLOTS_TO_UPDATE_JUSTIFIED` range.
pub fn move_inside_safe_to_update(self) -> Self {
while !is_safe_to_update(self.harness.chain.slot().unwrap()) {
while !is_safe_to_update(self.harness.chain.slot().unwrap(), &self.harness.chain.spec) {
self.harness.advance_slot()
}
self
@@ -270,7 +268,13 @@ impl ForkChoiceTest {
.chain
.fork_choice
.write()
.on_block(current_slot, &block, block.canonical_root(), &state)
.on_block(
current_slot,
&block,
block.canonical_root(),
&state,
&self.harness.chain.spec,
)
.unwrap();
self
}
@@ -305,7 +309,13 @@ impl ForkChoiceTest {
.chain
.fork_choice
.write()
.on_block(current_slot, &block, block.canonical_root(), &state)
.on_block(
current_slot,
&block,
block.canonical_root(),
&state,
&self.harness.chain.spec,
)
.err()
.expect("on_block did not return an error");
comparison_func(err);
@@ -458,8 +468,8 @@ impl ForkChoiceTest {
}
}
fn is_safe_to_update(slot: Slot) -> bool {
slot % E::slots_per_epoch() < SAFE_SLOTS_TO_UPDATE_JUSTIFIED
fn is_safe_to_update(slot: Slot, spec: &ChainSpec) -> bool {
slot % E::slots_per_epoch() < spec.safe_slots_to_update_justified
}
/// - The new justified checkpoint descends from the current.