Remove TTD flags and safe-slots-to-import-* (#6489)

* Delete SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY

* Update fork choice tests

* Remove TTD related flags

* Add deprecation warning

* Remove more dead code

* Delete EF on_merge_block tests

* Remove even more dead code

* Address Mac's review comments
This commit is contained in:
Michael Sproul
2024-10-21 12:28:55 +11:00
committed by GitHub
parent 6eaa370188
commit a732a87846
25 changed files with 44 additions and 1116 deletions

View File

@@ -55,8 +55,8 @@ use crate::data_availability_checker::{AvailabilityCheckError, MaybeAvailableBlo
use crate::data_column_verification::GossipDataColumnError;
use crate::eth1_finalization_cache::Eth1FinalizationData;
use crate::execution_payload::{
is_optimistic_candidate_block, validate_execution_payload_for_gossip, validate_merge_block,
AllowOptimisticImport, NotifyExecutionLayer, PayloadNotifier,
validate_execution_payload_for_gossip, validate_merge_block, AllowOptimisticImport,
NotifyExecutionLayer, PayloadNotifier,
};
use crate::kzg_utils::blobs_to_data_column_sidecars;
use crate::observed_block_producers::SeenBlock;
@@ -74,7 +74,7 @@ use lighthouse_metrics::TryExt;
use parking_lot::RwLockReadGuard;
use proto_array::Block as ProtoBlock;
use safe_arith::ArithError;
use slog::{debug, error, warn, Logger};
use slog::{debug, error, Logger};
use slot_clock::SlotClock;
use ssz::Encode;
use ssz_derive::{Decode, Encode};
@@ -95,9 +95,9 @@ use store::{Error as DBError, HotStateSummary, KeyValueStore, StoreOp};
use task_executor::JoinHandle;
use types::{
data_column_sidecar::DataColumnSidecarError, BeaconBlockRef, BeaconState, BeaconStateError,
BlobsList, ChainSpec, DataColumnSidecarList, Epoch, EthSpec, ExecPayload, ExecutionBlockHash,
FullPayload, Hash256, InconsistentFork, PublicKey, PublicKeyBytes, RelativeEpoch,
SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
BlobsList, ChainSpec, DataColumnSidecarList, Epoch, EthSpec, ExecutionBlockHash, FullPayload,
Hash256, InconsistentFork, PublicKey, PublicKeyBytes, RelativeEpoch, SignedBeaconBlock,
SignedBeaconBlockHeader, Slot,
};
pub const POS_PANDA_BANNER: &str = r#"
@@ -1388,28 +1388,6 @@ impl<T: BeaconChainTypes> ExecutionPendingBlock<T> {
}
let payload_verification_status = payload_notifier.notify_new_payload().await?;
// If the payload did not validate or invalidate the block, check to see if this block is
// valid for optimistic import.
if payload_verification_status.is_optimistic() {
let block_hash_opt = block
.message()
.body()
.execution_payload()
.map(|full_payload| full_payload.block_hash());
// Ensure the block is a candidate for optimistic import.
if !is_optimistic_candidate_block(&chain, block.slot(), block.parent_root()).await?
{
warn!(
chain.log,
"Rejecting optimistic block";
"block_hash" => ?block_hash_opt,
"msg" => "the execution engine is not synced"
);
return Err(ExecutionPayloadError::UnverifiedNonOptimisticCandidate.into());
}
}
Ok(PayloadVerificationOutcome {
payload_verification_status,
is_valid_merge_transition_block,

View File

@@ -277,9 +277,7 @@ pub async fn validate_merge_block<'a, T: BeaconChainTypes>(
}
.into()),
None => {
if allow_optimistic_import == AllowOptimisticImport::Yes
&& is_optimistic_candidate_block(chain, block.slot(), block.parent_root()).await?
{
if allow_optimistic_import == AllowOptimisticImport::Yes {
debug!(
chain.log,
"Optimistically importing merge transition block";
@@ -297,36 +295,6 @@ pub async fn validate_merge_block<'a, T: BeaconChainTypes>(
}
}
/// Check to see if a block with the given parameters is valid to be imported optimistically.
pub async fn is_optimistic_candidate_block<T: BeaconChainTypes>(
chain: &Arc<BeaconChain<T>>,
block_slot: Slot,
block_parent_root: Hash256,
) -> Result<bool, BeaconChainError> {
let current_slot = chain.slot()?;
let inner_chain = chain.clone();
// Use a blocking task to check if the block is an optimistic candidate. Interacting
// with the `fork_choice` lock in an async task can block the core executor.
chain
.spawn_blocking_handle(
move || {
inner_chain
.canonical_head
.fork_choice_read_lock()
.is_optimistic_candidate_block(
current_slot,
block_slot,
&block_parent_root,
&inner_chain.spec,
)
},
"validate_merge_block_optimistic_candidate",
)
.await?
.map_err(BeaconChainError::from)
}
/// Validate the gossip block's execution_payload according to the checks described here:
/// https://github.com/ethereum/consensus-specs/blob/dev/specs/merge/p2p-interface.md#beacon_block
pub fn validate_execution_payload_for_gossip<T: BeaconChainTypes>(

View File

@@ -1,20 +1,14 @@
#![cfg(not(debug_assertions))]
use beacon_chain::otb_verification_service::{
load_optimistic_transition_blocks, validate_optimistic_transition_blocks,
OptimisticTransitionBlock,
};
use beacon_chain::{
canonical_head::{CachedHead, CanonicalHead},
test_utils::{BeaconChainHarness, EphemeralHarnessType},
BeaconChainError, BlockError, ChainConfig, ExecutionPayloadError, NotifyExecutionLayer,
OverrideForkchoiceUpdate, StateSkipConfig, WhenSlotSkipped,
INVALID_FINALIZED_MERGE_TRANSITION_BLOCK_SHUTDOWN_REASON,
INVALID_JUSTIFIED_PAYLOAD_SHUTDOWN_REASON,
};
use execution_layer::{
json_structures::{JsonForkchoiceStateV1, JsonPayloadAttributes, JsonPayloadAttributesV1},
test_utils::ExecutionBlockGenerator,
ExecutionLayer, ForkchoiceState, PayloadAttributes,
};
use fork_choice::{Error as ForkChoiceError, InvalidationOperation, PayloadVerificationStatus};
@@ -1270,552 +1264,6 @@ async fn attesting_to_optimistic_head() {
get_aggregated_by_slot_and_root().unwrap();
}
/// A helper struct to build out a chain of some configurable length which undergoes the merge
/// transition.
struct OptimisticTransitionSetup {
blocks: Vec<Arc<SignedBeaconBlock<E>>>,
execution_block_generator: ExecutionBlockGenerator<E>,
}
impl OptimisticTransitionSetup {
async fn new(num_blocks: usize, ttd: u64) -> Self {
let mut spec = E::default_spec();
spec.terminal_total_difficulty = Uint256::from(ttd);
let mut rig = InvalidPayloadRig::new_with_spec(spec).enable_attestations();
rig.move_to_terminal_block();
let mut blocks = Vec::with_capacity(num_blocks);
for _ in 0..num_blocks {
let root = rig.import_block(Payload::Valid).await;
let block = rig.harness.chain.get_block(&root).await.unwrap().unwrap();
blocks.push(Arc::new(block));
}
let execution_block_generator = rig
.harness
.mock_execution_layer
.as_ref()
.unwrap()
.server
.execution_block_generator()
.clone();
Self {
blocks,
execution_block_generator,
}
}
}
/// Build a chain which has optimistically imported a transition block.
///
/// The initial chain will be built with respect to `block_ttd`, whilst the `rig` which imports the
/// chain will operate with respect to `rig_ttd`. This allows for testing mismatched TTDs.
async fn build_optimistic_chain(
block_ttd: u64,
rig_ttd: u64,
num_blocks: usize,
) -> InvalidPayloadRig {
let OptimisticTransitionSetup {
blocks,
execution_block_generator,
} = OptimisticTransitionSetup::new(num_blocks, block_ttd).await;
// Build a brand-new testing harness. We will apply the blocks from the previous harness to
// this one.
let mut spec = E::default_spec();
spec.terminal_total_difficulty = Uint256::from(rig_ttd);
let rig = InvalidPayloadRig::new_with_spec(spec);
let spec = &rig.harness.chain.spec;
let mock_execution_layer = rig.harness.mock_execution_layer.as_ref().unwrap();
// Ensure all the execution blocks from the first rig are available in the second rig.
*mock_execution_layer.server.execution_block_generator() = execution_block_generator;
// Make the execution layer respond `SYNCING` to all `newPayload` requests.
mock_execution_layer
.server
.all_payloads_syncing_on_new_payload(true);
// Make the execution layer respond `SYNCING` to all `forkchoiceUpdated` requests.
mock_execution_layer
.server
.all_payloads_syncing_on_forkchoice_updated();
// Make the execution layer respond `None` to all `getBlockByHash` requests.
mock_execution_layer
.server
.all_get_block_by_hash_requests_return_none();
let current_slot = std::cmp::max(
blocks[0].slot() + spec.safe_slots_to_import_optimistically,
num_blocks.into(),
);
rig.harness.set_current_slot(current_slot);
for block in blocks {
rig.harness
.chain
.process_block(
block.canonical_root(),
block,
NotifyExecutionLayer::Yes,
BlockImportSource::Lookup,
|| Ok(()),
)
.await
.unwrap();
}
rig.harness.chain.recompute_head_at_current_slot().await;
// Make the execution layer respond normally to `getBlockByHash` requests.
mock_execution_layer
.server
.all_get_block_by_hash_requests_return_natural_value();
// Perform some sanity checks to ensure that the transition happened exactly where we expected.
let pre_transition_block_root = rig
.harness
.chain
.block_root_at_slot(Slot::new(0), WhenSlotSkipped::None)
.unwrap()
.unwrap();
let pre_transition_block = rig
.harness
.chain
.get_block(&pre_transition_block_root)
.await
.unwrap()
.unwrap();
let post_transition_block_root = rig
.harness
.chain
.block_root_at_slot(Slot::new(1), WhenSlotSkipped::None)
.unwrap()
.unwrap();
let post_transition_block = rig
.harness
.chain
.get_block(&post_transition_block_root)
.await
.unwrap()
.unwrap();
assert_eq!(
pre_transition_block_root,
post_transition_block.parent_root(),
"the blocks form a single chain"
);
assert!(
pre_transition_block
.message()
.body()
.execution_payload()
.unwrap()
.is_default_with_empty_roots(),
"the block *has not* undergone the merge transition"
);
assert!(
!post_transition_block
.message()
.body()
.execution_payload()
.unwrap()
.is_default_with_empty_roots(),
"the block *has* undergone the merge transition"
);
// Assert that the transition block was optimistically imported.
//
// Note: we're using the "fallback" check for optimistic status, so if the block was
// pre-finality then we'll just use the optimistic status of the finalized block.
assert!(
rig.harness
.chain
.canonical_head
.fork_choice_read_lock()
.is_optimistic_or_invalid_block(&post_transition_block_root)
.unwrap(),
"the transition block should be imported optimistically"
);
// Get the mock execution layer to respond to `getBlockByHash` requests normally again.
mock_execution_layer
.server
.all_get_block_by_hash_requests_return_natural_value();
rig
}
#[tokio::test]
async fn optimistic_transition_block_valid_unfinalized() {
let ttd = 42;
let num_blocks = 16_usize;
let rig = build_optimistic_chain(ttd, ttd, num_blocks).await;
let post_transition_block_root = rig
.harness
.chain
.block_root_at_slot(Slot::new(1), WhenSlotSkipped::None)
.unwrap()
.unwrap();
let post_transition_block = rig
.harness
.chain
.get_block(&post_transition_block_root)
.await
.unwrap()
.unwrap();
assert!(
rig.cached_head()
.finalized_checkpoint()
.epoch
.start_slot(E::slots_per_epoch())
< post_transition_block.slot(),
"the transition block should not be finalized"
);
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert_eq!(
otbs.len(),
1,
"There should be one optimistic transition block"
);
let valid_otb = OptimisticTransitionBlock::from_block(post_transition_block.message());
assert_eq!(
valid_otb, otbs[0],
"The optimistic transition block stored in the database should be what we expect",
);
validate_optimistic_transition_blocks(&rig.harness.chain, otbs)
.await
.expect("should validate fine");
// now that the transition block has been validated, it should have been removed from the database
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert!(
otbs.is_empty(),
"The valid optimistic transition block should have been removed from the database",
);
}
#[tokio::test]
async fn optimistic_transition_block_valid_finalized() {
let ttd = 42;
let num_blocks = 130_usize;
let rig = build_optimistic_chain(ttd, ttd, num_blocks).await;
let post_transition_block_root = rig
.harness
.chain
.block_root_at_slot(Slot::new(1), WhenSlotSkipped::None)
.unwrap()
.unwrap();
let post_transition_block = rig
.harness
.chain
.get_block(&post_transition_block_root)
.await
.unwrap()
.unwrap();
assert!(
rig.cached_head()
.finalized_checkpoint()
.epoch
.start_slot(E::slots_per_epoch())
> post_transition_block.slot(),
"the transition block should be finalized"
);
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert_eq!(
otbs.len(),
1,
"There should be one optimistic transition block"
);
let valid_otb = OptimisticTransitionBlock::from_block(post_transition_block.message());
assert_eq!(
valid_otb, otbs[0],
"The optimistic transition block stored in the database should be what we expect",
);
validate_optimistic_transition_blocks(&rig.harness.chain, otbs)
.await
.expect("should validate fine");
// now that the transition block has been validated, it should have been removed from the database
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert!(
otbs.is_empty(),
"The valid optimistic transition block should have been removed from the database",
);
}
#[tokio::test]
async fn optimistic_transition_block_invalid_unfinalized() {
let block_ttd = 42;
let rig_ttd = 1337;
let num_blocks = 22_usize;
let rig = build_optimistic_chain(block_ttd, rig_ttd, num_blocks).await;
let post_transition_block_root = rig
.harness
.chain
.block_root_at_slot(Slot::new(1), WhenSlotSkipped::None)
.unwrap()
.unwrap();
let post_transition_block = rig
.harness
.chain
.get_block(&post_transition_block_root)
.await
.unwrap()
.unwrap();
assert!(
rig.cached_head()
.finalized_checkpoint()
.epoch
.start_slot(E::slots_per_epoch())
< post_transition_block.slot(),
"the transition block should not be finalized"
);
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert_eq!(
otbs.len(),
1,
"There should be one optimistic transition block"
);
let invalid_otb = OptimisticTransitionBlock::from_block(post_transition_block.message());
assert_eq!(
invalid_otb, otbs[0],
"The optimistic transition block stored in the database should be what we expect",
);
// No shutdown should've been triggered.
assert_eq!(rig.harness.shutdown_reasons(), vec![]);
// It shouldn't be known as invalid yet
assert!(!rig
.execution_status(post_transition_block_root)
.is_invalid());
validate_optimistic_transition_blocks(&rig.harness.chain, otbs)
.await
.unwrap();
// Still no shutdown should've been triggered.
assert_eq!(rig.harness.shutdown_reasons(), vec![]);
// It should be marked invalid now
assert!(rig
.execution_status(post_transition_block_root)
.is_invalid());
// the invalid merge transition block should NOT have been removed from the database
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert_eq!(
otbs.len(),
1,
"The invalid merge transition block should still be in the database",
);
assert_eq!(
invalid_otb, otbs[0],
"The optimistic transition block stored in the database should be what we expect",
);
}
#[tokio::test]
async fn optimistic_transition_block_invalid_unfinalized_syncing_ee() {
let block_ttd = 42;
let rig_ttd = 1337;
let num_blocks = 22_usize;
let rig = build_optimistic_chain(block_ttd, rig_ttd, num_blocks).await;
let post_transition_block_root = rig
.harness
.chain
.block_root_at_slot(Slot::new(1), WhenSlotSkipped::None)
.unwrap()
.unwrap();
let post_transition_block = rig
.harness
.chain
.get_block(&post_transition_block_root)
.await
.unwrap()
.unwrap();
assert!(
rig.cached_head()
.finalized_checkpoint()
.epoch
.start_slot(E::slots_per_epoch())
< post_transition_block.slot(),
"the transition block should not be finalized"
);
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert_eq!(
otbs.len(),
1,
"There should be one optimistic transition block"
);
let invalid_otb = OptimisticTransitionBlock::from_block(post_transition_block.message());
assert_eq!(
invalid_otb, otbs[0],
"The optimistic transition block stored in the database should be what we expect",
);
// No shutdown should've been triggered.
assert_eq!(rig.harness.shutdown_reasons(), vec![]);
// It shouldn't be known as invalid yet
assert!(!rig
.execution_status(post_transition_block_root)
.is_invalid());
// Make the execution layer respond `None` to all `getBlockByHash` requests to simulate a
// syncing EE.
let mock_execution_layer = rig.harness.mock_execution_layer.as_ref().unwrap();
mock_execution_layer
.server
.all_get_block_by_hash_requests_return_none();
validate_optimistic_transition_blocks(&rig.harness.chain, otbs)
.await
.unwrap();
// Still no shutdown should've been triggered.
assert_eq!(rig.harness.shutdown_reasons(), vec![]);
// It should still be marked as optimistic.
assert!(rig
.execution_status(post_transition_block_root)
.is_strictly_optimistic());
// the optimistic merge transition block should NOT have been removed from the database
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert_eq!(
otbs.len(),
1,
"The optimistic merge transition block should still be in the database",
);
assert_eq!(
invalid_otb, otbs[0],
"The optimistic transition block stored in the database should be what we expect",
);
// Allow the EL to respond to `getBlockByHash`, as if it has finished syncing.
mock_execution_layer
.server
.all_get_block_by_hash_requests_return_natural_value();
validate_optimistic_transition_blocks(&rig.harness.chain, otbs)
.await
.unwrap();
// Still no shutdown should've been triggered.
assert_eq!(rig.harness.shutdown_reasons(), vec![]);
// It should be marked invalid now
assert!(rig
.execution_status(post_transition_block_root)
.is_invalid());
// the invalid merge transition block should NOT have been removed from the database
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert_eq!(
otbs.len(),
1,
"The invalid merge transition block should still be in the database",
);
assert_eq!(
invalid_otb, otbs[0],
"The optimistic transition block stored in the database should be what we expect",
);
}
#[tokio::test]
async fn optimistic_transition_block_invalid_finalized() {
let block_ttd = 42;
let rig_ttd = 1337;
let num_blocks = 130_usize;
let rig = build_optimistic_chain(block_ttd, rig_ttd, num_blocks).await;
let post_transition_block_root = rig
.harness
.chain
.block_root_at_slot(Slot::new(1), WhenSlotSkipped::None)
.unwrap()
.unwrap();
let post_transition_block = rig
.harness
.chain
.get_block(&post_transition_block_root)
.await
.unwrap()
.unwrap();
assert!(
rig.cached_head()
.finalized_checkpoint()
.epoch
.start_slot(E::slots_per_epoch())
> post_transition_block.slot(),
"the transition block should be finalized"
);
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert_eq!(
otbs.len(),
1,
"There should be one optimistic transition block"
);
let invalid_otb = OptimisticTransitionBlock::from_block(post_transition_block.message());
assert_eq!(
invalid_otb, otbs[0],
"The optimistic transition block stored in the database should be what we expect",
);
// No shutdown should've been triggered yet.
assert_eq!(rig.harness.shutdown_reasons(), vec![]);
validate_optimistic_transition_blocks(&rig.harness.chain, otbs)
.await
.expect("should invalidate merge transition block and shutdown the client");
// The beacon chain should have triggered a shutdown.
assert_eq!(
rig.harness.shutdown_reasons(),
vec![ShutdownReason::Failure(
INVALID_FINALIZED_MERGE_TRANSITION_BLOCK_SHUTDOWN_REASON
)]
);
// the invalid merge transition block should NOT have been removed from the database
let otbs = load_optimistic_transition_blocks(&rig.harness.chain)
.expect("should load optimistic transition block from db");
assert_eq!(
otbs.len(),
1,
"The invalid merge transition block should still be in the database",
);
assert_eq!(
invalid_otb, otbs[0],
"The optimistic transition block stored in the database should be what we expect",
);
}
/// Helper for running tests where we generate a chain with an invalid head and then a
/// `fork_block` to recover it.
struct InvalidHeadSetup {