diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 9fb895f78..aa719b1a6 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3908,14 +3908,13 @@ impl BeaconChain { ForkName::Base | ForkName::Altair => return Ok(()), _ => { // We are post-bellatrix - if execution_layer + if let Some(payload_attributes) = execution_layer .payload_attributes(next_slot, params.head_root) .await - .is_some() { // We are a proposer, check for terminal_pow_block_hash if let Some(terminal_pow_block_hash) = execution_layer - .get_terminal_pow_block_hash(&self.spec) + .get_terminal_pow_block_hash(&self.spec, payload_attributes.timestamp) .await .map_err(Error::ForkchoiceUpdate)? { diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 747b8a468..5c7c3c05d 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -393,7 +393,7 @@ where } let terminal_pow_block_hash = execution_layer - .get_terminal_pow_block_hash(spec) + .get_terminal_pow_block_hash(spec, timestamp) .await .map_err(BlockProductionError::TerminalPoWBlockLookupFailed)?; diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 1dc6f4b83..2adae6c16 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -30,6 +30,7 @@ use rayon::prelude::*; use sensitive_url::SensitiveUrl; use slog::Logger; use slot_clock::TestingSlotClock; +use state_processing::per_block_processing::compute_timestamp_at_slot; use state_processing::{ state_advance::{complete_state_advance, partial_state_advance}, StateRootStrategy, @@ -521,6 +522,11 @@ where self.chain.head_beacon_state_cloned() } + pub fn get_timestamp_at_slot(&self) -> u64 { + let state = self.get_current_state(); + compute_timestamp_at_slot(&state, &self.spec).unwrap() + } + pub fn get_current_state_and_root(&self) -> (BeaconState, Hash256) { let head = self.chain.head_snapshot(); let state_root = head.beacon_state_root(); diff --git a/beacon_node/beacon_chain/tests/merge.rs b/beacon_node/beacon_chain/tests/merge.rs index 91d5eb21c..19e8902a3 100644 --- a/beacon_node/beacon_chain/tests/merge.rs +++ b/beacon_node/beacon_chain/tests/merge.rs @@ -1,7 +1,7 @@ #![cfg(not(debug_assertions))] // Tests run too slow in debug. use beacon_chain::test_utils::BeaconChainHarness; -use execution_layer::test_utils::{generate_pow_block, DEFAULT_TERMINAL_BLOCK}; +use execution_layer::test_utils::{generate_pow_block, Block, DEFAULT_TERMINAL_BLOCK}; use types::*; const VALIDATOR_COUNT: usize = 32; @@ -22,6 +22,7 @@ fn verify_execution_payload_chain(chain: &[FullPayload]) { prev_ep.execution_payload.block_number + 1, ep.execution_payload.block_number ); + assert!(ep.execution_payload.timestamp > prev_ep.execution_payload.timestamp); } prev_ep = Some(ep.clone()); } @@ -169,6 +170,30 @@ async fn base_altair_merge_with_terminal_block_after_fork() { .move_to_terminal_block() .unwrap(); + // Add a slot duration to get to the next slot + let timestamp = harness.get_timestamp_at_slot() + harness.spec.seconds_per_slot; + + harness + .execution_block_generator() + .modify_last_block(|block| { + if let Block::PoW(terminal_block) = block { + terminal_block.timestamp = timestamp; + } + }); + + harness.extend_slots(1).await; + + let one_after_merge_head = &harness.chain.head_snapshot().beacon_block; + assert_eq!( + *one_after_merge_head + .message() + .body() + .execution_payload() + .unwrap(), + FullPayload::default() + ); + assert_eq!(one_after_merge_head.slot(), merge_fork_slot + 2); + /* * Next merge block should include an exec payload. */ diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 7e04a3fac..4f957d638 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -106,6 +106,8 @@ pub struct ExecutionBlock { pub block_number: u64, pub parent_hash: ExecutionBlockHash, pub total_difficulty: Uint256, + #[serde(with = "eth2_serde_utils::u64_hex_be")] + pub timestamp: u64, } /// Representation of an exection block with enough detail to reconstruct a payload. diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 9bb4ead35..d85f9eb81 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -902,6 +902,7 @@ impl ExecutionLayer { pub async fn get_terminal_pow_block_hash( &self, spec: &ChainSpec, + timestamp: u64, ) -> Result, Error> { let _timer = metrics::start_timer_vec( &metrics::EXECUTION_LAYER_REQUEST_TIMES, @@ -924,8 +925,19 @@ impl ExecutionLayer { } } - self.get_pow_block_hash_at_total_difficulty(engine, spec) - .await + let block = self.get_pow_block_at_total_difficulty(engine, spec).await?; + if let Some(pow_block) = block { + // If `terminal_block.timestamp == transition_block.timestamp`, + // we violate the invariant that a block's timestamp must be + // strictly greater than its parent's timestamp. + // The execution layer will reject a fcu call with such payload + // attributes leading to a missed block. + // Hence, we return `None` in such a case. + if pow_block.timestamp >= timestamp { + return Ok(None); + } + } + Ok(block.map(|b| b.block_hash)) }) .await .map_err(Box::new) @@ -953,11 +965,11 @@ impl ExecutionLayer { /// `get_pow_block_at_terminal_total_difficulty` /// /// https://github.com/ethereum/consensus-specs/blob/v1.1.5/specs/merge/validator.md - async fn get_pow_block_hash_at_total_difficulty( + async fn get_pow_block_at_total_difficulty( &self, engine: &Engine, spec: &ChainSpec, - ) -> Result, ApiError> { + ) -> Result, ApiError> { let mut block = engine .api .get_block_by_number(BlockByNumberQuery::Tag(LATEST_TAG)) @@ -970,7 +982,7 @@ impl ExecutionLayer { let block_reached_ttd = block.total_difficulty >= spec.terminal_total_difficulty; if block_reached_ttd { if block.parent_hash == ExecutionBlockHash::zero() { - return Ok(Some(block.block_hash)); + return Ok(Some(block)); } let parent = self .get_pow_block(engine, block.parent_hash) @@ -979,7 +991,7 @@ impl ExecutionLayer { let parent_reached_ttd = parent.total_difficulty >= spec.terminal_total_difficulty; if block_reached_ttd && !parent_reached_ttd { - return Ok(Some(block.block_hash)); + return Ok(Some(block)); } else { block = parent; } @@ -1197,19 +1209,54 @@ mod test { .move_to_block_prior_to_terminal_block() .with_terminal_block(|spec, el, _| async move { el.engine().upcheck().await; - assert_eq!(el.get_terminal_pow_block_hash(&spec).await.unwrap(), None) + assert_eq!( + el.get_terminal_pow_block_hash(&spec, timestamp_now()) + .await + .unwrap(), + None + ) }) .await .move_to_terminal_block() .with_terminal_block(|spec, el, terminal_block| async move { assert_eq!( - el.get_terminal_pow_block_hash(&spec).await.unwrap(), + el.get_terminal_pow_block_hash(&spec, timestamp_now()) + .await + .unwrap(), Some(terminal_block.unwrap().block_hash) ) }) .await; } + #[tokio::test] + async fn rejects_terminal_block_with_equal_timestamp() { + let runtime = TestRuntime::default(); + MockExecutionLayer::default_params(runtime.task_executor.clone()) + .move_to_block_prior_to_terminal_block() + .with_terminal_block(|spec, el, _| async move { + el.engine().upcheck().await; + assert_eq!( + el.get_terminal_pow_block_hash(&spec, timestamp_now()) + .await + .unwrap(), + None + ) + }) + .await + .move_to_terminal_block() + .with_terminal_block(|spec, el, terminal_block| async move { + let timestamp = terminal_block.as_ref().map(|b| b.timestamp).unwrap(); + assert_eq!( + el.get_terminal_pow_block_hash(&spec, timestamp) + .await + .unwrap(), + None + ) + }) + .await; + } + #[tokio::test] async fn verifies_valid_terminal_block_hash() { let runtime = TestRuntime::default(); @@ -1269,3 +1316,12 @@ mod test { fn noop(_: &ExecutionLayer, _: &ExecutionPayload) -> Option> { None } + +#[cfg(test)] +/// Returns the duration since the unix epoch. +fn timestamp_now() -> u64 { + SystemTime::now() + .duration_since(UNIX_EPOCH) + .unwrap_or_else(|_| Duration::from_secs(0)) + .as_secs() +} diff --git a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs index 7d8cdb299..bf8ed4947 100644 --- a/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs +++ b/beacon_node/execution_layer/src/test_utils/execution_block_generator.rs @@ -60,12 +60,14 @@ impl Block { block_number: block.block_number, parent_hash: block.parent_hash, total_difficulty: block.total_difficulty, + timestamp: block.timestamp, }, Block::PoS(payload) => ExecutionBlock { block_hash: payload.block_hash, block_number: payload.block_number, parent_hash: payload.parent_hash, total_difficulty, + timestamp: payload.timestamp, }, } } @@ -100,6 +102,7 @@ pub struct PoWBlock { pub block_hash: ExecutionBlockHash, pub parent_hash: ExecutionBlockHash, pub total_difficulty: Uint256, + pub timestamp: u64, } pub struct ExecutionBlockGenerator { @@ -266,6 +269,26 @@ impl ExecutionBlockGenerator { Ok(()) } + pub fn modify_last_block(&mut self, block_modifier: impl FnOnce(&mut Block)) { + if let Some((last_block_hash, block_number)) = + self.block_hashes.keys().max().and_then(|block_number| { + self.block_hashes + .get(block_number) + .map(|block| (block, *block_number)) + }) + { + let mut block = self.blocks.remove(last_block_hash).unwrap(); + block_modifier(&mut block); + // Update the block hash after modifying the block + match &mut block { + Block::PoW(b) => b.block_hash = ExecutionBlockHash::from_root(b.tree_hash_root()), + Block::PoS(b) => b.block_hash = ExecutionBlockHash::from_root(b.tree_hash_root()), + } + self.block_hashes.insert(block_number, block.block_hash()); + self.blocks.insert(block.block_hash(), block); + } + } + pub fn get_payload(&mut self, id: &PayloadId) -> Option> { self.payload_ids.get(id).cloned() } @@ -423,6 +446,7 @@ pub fn generate_pow_block( block_hash: ExecutionBlockHash::zero(), parent_hash, total_difficulty, + timestamp: block_number, }; block.block_hash = ExecutionBlockHash::from_root(block.tree_hash_root()); diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index 723da25ff..970c619a5 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -6,7 +6,7 @@ use crate::engine_api::{ }; use bytes::Bytes; use environment::null_logger; -use execution_block_generator::{Block, PoWBlock}; +use execution_block_generator::PoWBlock; use handle_rpc::handle_rpc; use parking_lot::{Mutex, RwLock, RwLockWriteGuard}; use serde::{Deserialize, Serialize}; @@ -21,7 +21,7 @@ use tokio::{runtime, sync::oneshot}; use types::{EthSpec, ExecutionBlockHash, Uint256}; use warp::{http::StatusCode, Filter, Rejection}; -pub use execution_block_generator::{generate_pow_block, ExecutionBlockGenerator}; +pub use execution_block_generator::{generate_pow_block, Block, ExecutionBlockGenerator}; pub use mock_execution_layer::MockExecutionLayer; pub const DEFAULT_TERMINAL_DIFFICULTY: u64 = 6400; @@ -334,6 +334,7 @@ impl MockServer { block_hash, parent_hash, total_difficulty, + timestamp: block_number, }); self.ctx diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index ee5e9cf2c..9c09ec8d9 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -219,7 +219,7 @@ impl TestRig { let terminal_pow_block_hash = self .ee_a .execution_layer - .get_terminal_pow_block_hash(&self.spec) + .get_terminal_pow_block_hash(&self.spec, timestamp_now()) .await .unwrap() .unwrap(); @@ -228,7 +228,7 @@ impl TestRig { terminal_pow_block_hash, self.ee_b .execution_layer - .get_terminal_pow_block_hash(&self.spec) + .get_terminal_pow_block_hash(&self.spec, timestamp_now()) .await .unwrap() .unwrap()