Skip to content

Commit

Permalink
Update to consensus spec v1.4.0-beta.6 (#5094)
Browse files Browse the repository at this point in the history 
* get latest ef tests passing

* fix tests

* Fix invalid payload recovery tests

* Merge branch 'unstable' into update-to-spec-v1.4.0-beta.6

* Revert "fix tests"

This reverts commit 0c875b0.

* Fix fork choice def. tests

* Update beacon_node/beacon_chain/tests/payload_invalidation.rs
  • Loading branch information
@realbigsean
realbigsean authored Feb 8, 2024
1 parent e353358 commit 4172d9f
Show file tree
Hide file tree
Showing 7 changed files with 119 additions and 76 deletions.
107 changes: 60 additions & 47 deletions beacon_node/beacon_chain/tests/payload_invalidation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1820,81 +1820,94 @@ struct InvalidHeadSetup {
}

impl InvalidHeadSetup {
/// This function aims to produce two things:
///
/// 1. A chain where the only viable head block has an invalid execution payload.
/// 2. A block (`fork_block`) which will become the head of the chain when
/// it is imported.
async fn new() -> InvalidHeadSetup {
let slots_per_epoch = E::slots_per_epoch();
let mut rig = InvalidPayloadRig::new().enable_attestations();
rig.move_to_terminal_block();
rig.import_block(Payload::Valid).await; // Import a valid transition block.

// Import blocks until the first time the chain finalizes.
// Import blocks until the first time the chain finalizes. This avoids
// some edge-cases around genesis.
while rig.cached_head().finalized_checkpoint().epoch == 0 {
rig.import_block(Payload::Syncing).await;
}

let slots_per_epoch = E::slots_per_epoch();
let start_slot = rig.cached_head().head_slot() + 1;
let mut opt_fork_block = None;

assert_eq!(start_slot % slots_per_epoch, 1);
for i in 0..slots_per_epoch - 1 {
let slot = start_slot + i;
let slot_offset = slot.as_u64() % slots_per_epoch;

rig.harness.set_current_slot(slot);

if slot_offset == slots_per_epoch - 1 {
// Optimistic head block right before epoch boundary.
let is_valid = Payload::Syncing;
rig.import_block_parametric(is_valid, is_valid, Some(slot), |error| {
matches!(
error,
BlockError::ExecutionPayloadError(
ExecutionPayloadError::RejectedByExecutionEngine { .. }
)
)
})
.await;
} else if 3 * slot_offset < 2 * slots_per_epoch {
// Valid block in previous epoch.
rig.import_block(Payload::Valid).await;
} else if slot_offset == slots_per_epoch - 2 {
// Fork block one slot prior to invalid head, not applied immediately.
let parent_state = rig
.harness
.chain
.state_at_slot(slot - 1, StateSkipConfig::WithStateRoots)
.unwrap();
let (fork_block_tuple, _) = rig.harness.make_block(parent_state, slot).await;
opt_fork_block = Some(fork_block_tuple.0);
} else {
// Skipped slot.
};
// Define a helper function.
let chain = rig.harness.chain.clone();
let get_unrealized_justified_epoch = move || {
chain
.canonical_head
.fork_choice_read_lock()
.unrealized_justified_checkpoint()
.epoch
};

// Import more blocks until there is a new and higher unrealized
// justified checkpoint.
//
// The result will be a single chain where the head block has a higher
// unrealized justified checkpoint than all other blocks in the chain.
let initial_unrealized_justified = get_unrealized_justified_epoch();
while get_unrealized_justified_epoch() == initial_unrealized_justified {
rig.import_block(Payload::Syncing).await;
}

// Create a forked block that competes with the head block. Both the
// head block and this fork block will share the same parent.
//
// The fork block and head block will both have an unrealized justified
// checkpoint at epoch `N` whilst their parent is at `N - 1`.
let head_slot = rig.cached_head().head_slot();
let parent_slot = head_slot - 1;
let fork_block_slot = head_slot + 1;
let parent_state = rig
.harness
.chain
.state_at_slot(parent_slot, StateSkipConfig::WithStateRoots)
.unwrap();
let (fork_block_tuple, _) = rig.harness.make_block(parent_state, fork_block_slot).await;
let fork_block = fork_block_tuple.0;

let invalid_head = rig.cached_head();
assert_eq!(
invalid_head.head_slot() % slots_per_epoch,
slots_per_epoch - 1
);

// Advance clock to new epoch to realize the justification of soon-to-be-invalid head block.
rig.harness.set_current_slot(invalid_head.head_slot() + 1);
// Advance the chain forward two epochs past the current head block.
//
// This ensures that `voting_source.epoch + 2 >= current_epoch` is
// `false` in the `node_is_viable_for_head` function. In effect, this
// ensures that no other block but the current head block is viable as a
// head block.
let invalid_head_epoch = invalid_head.head_slot().epoch(slots_per_epoch);
let new_wall_clock_epoch = invalid_head_epoch + 2;
rig.harness
.set_current_slot(new_wall_clock_epoch.start_slot(slots_per_epoch));

// Invalidate the head block.
rig.invalidate_manually(invalid_head.head_block_root())
.await;

// Since our setup ensures that there is only a single, invalid block
// that's viable for head (according to FFG filtering), setting the
// head block as invalid should not result in another head being chosen.
// Rather, it should fail to run fork choice and leave the invalid block as
// the head.
assert!(rig
.canonical_head()
.head_execution_status()
.unwrap()
.is_invalid());

// Finding a new head should fail since the only possible head is not valid.
// Ensure that we're getting the correct error when trying to find a new
// head.
rig.assert_get_head_error_contains("InvalidBestNode");

Self {
rig,
fork_block: opt_fork_block.unwrap(),
fork_block,
invalid_head,
}
}
Expand Down
53 changes: 41 additions & 12 deletions consensus/proto_array/src/fork_choice_test_definition/ffg_updates.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,7 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition {
expected_head: get_root(3),
});

// Ensure that with justified epoch 1 we find 2
// Ensure that with justified epoch 1 we find 3
//
// 0
// |
Expand All @@ -68,11 +68,15 @@ pub fn get_ffg_case_01_test_definition() -> ForkChoiceTestDefinition {
// 2 <- start
// |
// 3 <- head
//
// Since https://github.com/ethereum/consensus-specs/pull/3431 it is valid
// to elect head blocks that have a higher justified checkpoint than the
// store.
ops.push(Operation::FindHead {
justified_checkpoint: get_checkpoint(1),
finalized_checkpoint: get_checkpoint(0),
justified_state_balances: balances.clone(),
expected_head: get_root(2),
expected_head: get_root(3),
});

// Ensure that with justified epoch 2 we find 3
Expand Down Expand Up @@ -247,14 +251,19 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition {
justified_state_balances: balances.clone(),
expected_head: get_root(10),
});
// Same as above, but with justified epoch 3 (should be invalid).
ops.push(Operation::InvalidFindHead {
// Same as above, but with justified epoch 3.
//
// Since https://github.com/ethereum/consensus-specs/pull/3431 it is valid
// to elect head blocks that have a higher justified checkpoint than the
// store.
ops.push(Operation::FindHead {
justified_checkpoint: Checkpoint {
epoch: Epoch::new(3),
root: get_root(6),
},
finalized_checkpoint: get_checkpoint(0),
justified_state_balances: balances.clone(),
expected_head: get_root(10),
});

// Add a vote to 1.
Expand Down Expand Up @@ -305,14 +314,19 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition {
justified_state_balances: balances.clone(),
expected_head: get_root(9),
});
// Save as above but justified epoch 3 (should fail).
ops.push(Operation::InvalidFindHead {
// Save as above but justified epoch 3.
//
// Since https://github.com/ethereum/consensus-specs/pull/3431 it is valid
// to elect head blocks that have a higher justified checkpoint than the
// store.
ops.push(Operation::FindHead {
justified_checkpoint: Checkpoint {
epoch: Epoch::new(3),
root: get_root(5),
},
finalized_checkpoint: get_checkpoint(0),
justified_state_balances: balances.clone(),
expected_head: get_root(9),
});

// Add a vote to 2.
Expand Down Expand Up @@ -363,14 +377,19 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition {
justified_state_balances: balances.clone(),
expected_head: get_root(10),
});
// Same as above but justified epoch 3 (should fail).
ops.push(Operation::InvalidFindHead {
// Same as above but justified epoch 3.
//
// Since https://github.com/ethereum/consensus-specs/pull/3431 it is valid
// to elect head blocks that have a higher justified checkpoint than the
// store.
ops.push(Operation::FindHead {
justified_checkpoint: Checkpoint {
epoch: Epoch::new(3),
root: get_root(6),
},
finalized_checkpoint: get_checkpoint(0),
justified_state_balances: balances.clone(),
expected_head: get_root(10),
});

// Ensure that if we start at 1 we find 9 (just: 0, fin: 0).
Expand Down Expand Up @@ -405,14 +424,19 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition {
justified_state_balances: balances.clone(),
expected_head: get_root(9),
});
// Same as above but justified epoch 3 (should fail).
ops.push(Operation::InvalidFindHead {
// Same as above but justified epoch 3.
//
// Since https://github.com/ethereum/consensus-specs/pull/3431 it is valid
// to elect head blocks that have a higher justified checkpoint than the
// store.
ops.push(Operation::FindHead {
justified_checkpoint: Checkpoint {
epoch: Epoch::new(3),
root: get_root(5),
},
finalized_checkpoint: get_checkpoint(0),
justified_state_balances: balances.clone(),
expected_head: get_root(9),
});

// Ensure that if we start at 2 we find 10 (just: 0, fin: 0).
Expand Down Expand Up @@ -444,14 +468,19 @@ pub fn get_ffg_case_02_test_definition() -> ForkChoiceTestDefinition {
justified_state_balances: balances.clone(),
expected_head: get_root(10),
});
// Same as above but justified epoch 3 (should fail).
ops.push(Operation::InvalidFindHead {
// Same as above but justified epoch 3.
//
// Since https://github.com/ethereum/consensus-specs/pull/3431 it is valid
// to elect head blocks that have a higher justified checkpoint than the
// store.
ops.push(Operation::FindHead {
justified_checkpoint: Checkpoint {
epoch: Epoch::new(3),
root: get_root(6),
},
finalized_checkpoint: get_checkpoint(0),
justified_state_balances: balances,
expected_head: get_root(10),
});

// END OF TESTS
Expand Down
14 changes: 10 additions & 4 deletions consensus/proto_array/src/fork_choice_test_definition/no_votes.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition {
root: Hash256::zero(),
},
},
// Ensure the head is still 4 whilst the justified epoch is 0.
// Ensure the head is now 5 whilst the justified epoch is 0.
//
// 0
// / \
Expand All @@ -203,9 +203,10 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition {
root: Hash256::zero(),
},
justified_state_balances: balances.clone(),
expected_head: get_root(4),
expected_head: get_root(5),
},
// Ensure there is an error when starting from a block that has the wrong justified epoch.
// Ensure there is no error when starting from a block that has the
// wrong justified epoch.
//
// 0
// / \
Expand All @@ -214,7 +215,11 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition {
// 4 3
// |
// 5 <- starting from 5 with justified epoch 0 should error.
Operation::InvalidFindHead {
//
// Since https://github.com/ethereum/consensus-specs/pull/3431 it is valid
// to elect head blocks that have a higher justified checkpoint than the
// store.
Operation::FindHead {
justified_checkpoint: Checkpoint {
epoch: Epoch::new(1),
root: get_root(5),
Expand All @@ -224,6 +229,7 @@ pub fn get_no_votes_test_definition() -> ForkChoiceTestDefinition {
root: Hash256::zero(),
},
justified_state_balances: balances.clone(),
expected_head: get_root(5),
},
// Set the justified epoch to 2 and the start block to 5 and ensure 5 is the head.
//
Expand Down
13 changes: 3 additions & 10 deletions consensus/proto_array/src/proto_array.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -961,16 +961,9 @@ impl ProtoArray {
node_justified_checkpoint
};

let mut correct_justified = self.justified_checkpoint.epoch == genesis_epoch
|| voting_source.epoch == self.justified_checkpoint.epoch;

if let Some(node_unrealized_justified_checkpoint) = node.unrealized_justified_checkpoint {
if !correct_justified && self.justified_checkpoint.epoch + 1 == current_epoch {
correct_justified = node_unrealized_justified_checkpoint.epoch
>= self.justified_checkpoint.epoch
&& voting_source.epoch + 2 >= current_epoch;
}
}
let correct_justified = self.justified_checkpoint.epoch == genesis_epoch
|| voting_source.epoch == self.justified_checkpoint.epoch
|| voting_source.epoch + 2 >= current_epoch;

let correct_finalized = self.finalized_checkpoint.epoch == genesis_epoch
|| self.is_finalized_checkpoint_or_descendant::<E>(node.root);
Expand Down
2 changes: 1 addition & 1 deletion testing/ef_tests/Makefile
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
TESTS_TAG := v1.4.0-beta.4
TESTS_TAG := v1.4.0-beta.6
TESTS = general minimal mainnet
TARBALLS = $(patsubst %,%-$(TESTS_TAG).tar.gz,$(TESTS))

Expand Down
3 changes: 2 additions & 1 deletion testing/ef_tests/check_all_files_accessed.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,8 @@
"bls12-381-tests/deserialization_G1",
"bls12-381-tests/deserialization_G2",
"bls12-381-tests/hash_to_G2",
"tests/.*/eip6110"
"tests/.*/eip6110",
"tests/.*/whisk"
]


Expand Down
3 changes: 2 additions & 1 deletion testing/ef_tests/tests/tests.rs
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#![cfg(feature = "ef_tests")]

use ef_tests::{KzgInclusionMerkleProofValidityHandler, *};
use ef_tests::*;
use types::{MainnetEthSpec, MinimalEthSpec, *};

// Check that the hand-computed multiplications on EthSpec are correctly computed.
Expand Down Expand Up @@ -605,6 +605,7 @@ fn merkle_proof_validity() {
}

#[test]
#[cfg(feature = "fake_crypto")]
fn kzg_inclusion_merkle_proof_validity() {
KzgInclusionMerkleProofValidityHandler::<MainnetEthSpec>::default().run();
KzgInclusionMerkleProofValidityHandler::<MinimalEthSpec>::default().run();
Expand Down

0 comments on commit 4172d9f

Please sign in to comment.