From a1033a92471f3d2b5f289dc37b56badc0a1b8bce Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sat, 2 Oct 2021 11:39:11 +1000 Subject: [PATCH] Add `BeaconChainHarness` tests for The Merge (#2661) * Start adding merge tests * Expose MockExecutionLayer * Add mock_execution_layer to BeaconChainHarness * Progress with merge test * Return more detailed errors with gas limit issues * Use a better gas limit in block gen * Ensure TTD is met in block gen * Fix basic_merge tests * Start geth testing * Fix conflicts after rebase * Remove geth tests * Improve merge test * Address clippy lints * Make pow block gen a pure function * Add working new test, breaking existing test * Fix test names * Add should_panic * Don't run merge tests in debug * Detect a tokio runtime when starting MockServer * Fix clippy lint, include merge tests --- beacon_node/beacon_chain/Cargo.toml | 1 + beacon_node/beacon_chain/src/test_utils.rs | 117 ++++++++++- beacon_node/beacon_chain/tests/main.rs | 1 + beacon_node/beacon_chain/tests/merge.rs | 182 ++++++++++++++++++ beacon_node/execution_layer/Cargo.toml | 1 + beacon_node/execution_layer/src/engine_api.rs | 2 + .../execution_layer/src/engine_api/http.rs | 4 +- beacon_node/execution_layer/src/lib.rs | 175 +---------------- .../test_utils/execution_block_generator.rs | 118 ++++++++---- .../src/test_utils/handle_rpc.rs | 7 - .../src/test_utils/mock_execution_layer.rs | 182 ++++++++++++++++++ .../execution_layer/src/test_utils/mod.rs | 55 ++++-- .../src/per_block_processing.rs | 34 ++-- .../src/per_block_processing/errors.rs | 12 ++ 14 files changed, 642 insertions(+), 249 deletions(-) create mode 100644 beacon_node/beacon_chain/tests/merge.rs create mode 100644 beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs diff --git a/beacon_node/beacon_chain/Cargo.toml b/beacon_node/beacon_chain/Cargo.toml index c9063a3cc..2cb024f00 100644 --- a/beacon_node/beacon_chain/Cargo.toml +++ b/beacon_node/beacon_chain/Cargo.toml @@ -57,6 +57,7 @@ eth2 = { path = "../../common/eth2" } strum = { version = "0.21.0", features = ["derive"] } logging = { path = "../../common/logging" } execution_layer = { path = "../execution_layer" } +sensitive_url = { path = "../../common/sensitive_url" } [[test]] name = "beacon_chain_tests" diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index d407d8354..ac34ecf86 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -11,16 +11,24 @@ use crate::{ StateSkipConfig, }; use bls::get_withdrawal_credentials; +use execution_layer::{ + test_utils::{ + ExecutionBlockGenerator, ExecutionLayerRuntime, MockExecutionLayer, DEFAULT_TERMINAL_BLOCK, + }, + ExecutionLayer, +}; use futures::channel::mpsc::Receiver; pub use genesis::interop_genesis_state; use int_to_bytes::int_to_bytes32; use logging::test_logger; use merkle_proof::MerkleTree; use parking_lot::Mutex; +use parking_lot::RwLockWriteGuard; use rand::rngs::StdRng; use rand::Rng; use rand::SeedableRng; use rayon::prelude::*; +use sensitive_url::SensitiveUrl; use slog::Logger; use slot_clock::TestingSlotClock; use state_processing::state_advance::complete_state_advance; @@ -35,13 +43,13 @@ use tree_hash::TreeHash; use types::sync_selection_proof::SyncSelectionProof; pub use types::test_utils::generate_deterministic_keypairs; use types::{ - typenum::U4294967296, AggregateSignature, Attestation, AttestationData, AttesterSlashing, - BeaconBlock, BeaconState, BeaconStateHash, ChainSpec, Checkpoint, Deposit, DepositData, Domain, - Epoch, EthSpec, ForkName, Graffiti, Hash256, IndexedAttestation, Keypair, ProposerSlashing, - PublicKeyBytes, SelectionProof, SignatureBytes, SignedAggregateAndProof, SignedBeaconBlock, - SignedBeaconBlockHash, SignedContributionAndProof, SignedRoot, SignedVoluntaryExit, Slot, - SubnetId, SyncCommittee, SyncCommitteeContribution, SyncCommitteeMessage, VariableList, - VoluntaryExit, + typenum::U4294967296, Address, AggregateSignature, Attestation, AttestationData, + AttesterSlashing, BeaconBlock, BeaconState, BeaconStateHash, ChainSpec, Checkpoint, Deposit, + DepositData, Domain, Epoch, EthSpec, ForkName, Graffiti, Hash256, IndexedAttestation, Keypair, + ProposerSlashing, PublicKeyBytes, SelectionProof, SignatureBytes, SignedAggregateAndProof, + SignedBeaconBlock, SignedBeaconBlockHash, SignedContributionAndProof, SignedRoot, + SignedVoluntaryExit, Slot, SubnetId, SyncCommittee, SyncCommitteeContribution, + SyncCommitteeMessage, VariableList, VoluntaryExit, }; // 4th September 2019 @@ -147,6 +155,9 @@ pub struct Builder { store: Option>>, initial_mutator: Option>, store_mutator: Option>, + execution_layer: Option, + execution_layer_runtime: Option, + mock_execution_layer: Option>, log: Logger, } @@ -254,6 +265,9 @@ where store: None, initial_mutator: None, store_mutator: None, + execution_layer: None, + mock_execution_layer: None, + execution_layer_runtime: None, log: test_logger(), } } @@ -311,6 +325,47 @@ where self } + pub fn execution_layer(mut self, urls: &[&str]) -> Self { + let spec = self.spec.clone().expect("cannot build without spec"); + assert!( + self.execution_layer.is_none(), + "execution layer already defined" + ); + + let el_runtime = ExecutionLayerRuntime::default(); + + let urls = urls + .iter() + .map(|s| SensitiveUrl::parse(*s)) + .collect::>() + .unwrap(); + let execution_layer = ExecutionLayer::from_urls( + urls, + spec.terminal_total_difficulty, + spec.terminal_block_hash, + Some(Address::repeat_byte(42)), + el_runtime.task_executor.clone(), + el_runtime.log.clone(), + ) + .unwrap(); + + self.execution_layer = Some(execution_layer); + self.execution_layer_runtime = Some(el_runtime); + self + } + + pub fn mock_execution_layer(mut self) -> Self { + let spec = self.spec.clone().expect("cannot build without spec"); + let mock = MockExecutionLayer::new( + spec.terminal_total_difficulty, + DEFAULT_TERMINAL_BLOCK, + spec.terminal_block_hash, + ); + self.execution_layer = Some(mock.el.clone()); + self.mock_execution_layer = Some(mock); + self + } + pub fn build(self) -> BeaconChainHarness> { let (shutdown_tx, shutdown_receiver) = futures::channel::mpsc::channel(1); @@ -326,6 +381,7 @@ where .custom_spec(spec) .store(self.store.expect("cannot build without store")) .store_migrator_config(MigratorConfig::default().blocking()) + .execution_layer(self.execution_layer) .dummy_eth1_backend() .expect("should build dummy backend") .shutdown_sender(shutdown_tx) @@ -364,6 +420,8 @@ where chain: Arc::new(chain), validator_keypairs, shutdown_receiver, + mock_execution_layer: self.mock_execution_layer, + execution_layer_runtime: self.execution_layer_runtime, rng: make_rng(), } } @@ -380,6 +438,9 @@ pub struct BeaconChainHarness { pub spec: ChainSpec, pub shutdown_receiver: Receiver, + pub mock_execution_layer: Option>, + pub execution_layer_runtime: Option, + pub rng: Mutex, } @@ -407,6 +468,14 @@ where &self.chain.log } + pub fn execution_block_generator(&self) -> RwLockWriteGuard<'_, ExecutionBlockGenerator> { + self.mock_execution_layer + .as_ref() + .expect("harness was not built with mock execution layer") + .server + .execution_block_generator() + } + pub fn get_all_validators(&self) -> Vec { (0..self.validator_keypairs.len()).collect() } @@ -1436,6 +1505,40 @@ where self.make_block(state, slot) } + /// Uses `Self::extend_chain` to build the chain out to the `target_slot`. + pub fn extend_to_slot(&self, target_slot: Slot) -> Hash256 { + if self.chain.slot().unwrap() == self.chain.head_info().unwrap().slot { + self.advance_slot(); + } + + let num_slots = target_slot + .as_usize() + .checked_sub(self.chain.slot().unwrap().as_usize()) + .expect("target_slot must be >= current_slot") + .checked_add(1) + .unwrap(); + + self.extend_slots(num_slots) + } + + /// Uses `Self::extend_chain` to `num_slots` blocks. + /// + /// Utilizes: + /// + /// - BlockStrategy::OnCanonicalHead, + /// - AttestationStrategy::AllValidators, + pub fn extend_slots(&self, num_slots: usize) -> Hash256 { + if self.chain.slot().unwrap() == self.chain.head_info().unwrap().slot { + self.advance_slot(); + } + + self.extend_chain( + num_slots, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ) + } + /// Deprecated: Use add_attested_blocks_at_slots() instead /// /// Extend the `BeaconChain` with some blocks and attestations. Returns the root of the diff --git a/beacon_node/beacon_chain/tests/main.rs b/beacon_node/beacon_chain/tests/main.rs index fb942d24e..fa31af840 100644 --- a/beacon_node/beacon_chain/tests/main.rs +++ b/beacon_node/beacon_chain/tests/main.rs @@ -1,6 +1,7 @@ mod attestation_production; mod attestation_verification; mod block_verification; +mod merge; mod op_verification; mod store_tests; mod sync_committee_verification; diff --git a/beacon_node/beacon_chain/tests/merge.rs b/beacon_node/beacon_chain/tests/merge.rs new file mode 100644 index 000000000..35dda493e --- /dev/null +++ b/beacon_node/beacon_chain/tests/merge.rs @@ -0,0 +1,182 @@ +#![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 types::*; + +const VALIDATOR_COUNT: usize = 32; + +type E = MainnetEthSpec; + +fn verify_execution_payload_chain(chain: &[ExecutionPayload]) { + let mut prev_ep: Option> = None; + + for ep in chain { + assert!(*ep != ExecutionPayload::default()); + assert!(ep.block_hash != Hash256::zero()); + + // Check against previous `ExecutionPayload`. + if let Some(prev_ep) = prev_ep { + assert_eq!(prev_ep.block_hash, ep.parent_hash); + assert_eq!(prev_ep.block_number + 1, ep.block_number); + } + prev_ep = Some(ep.clone()); + } +} + +#[test] +// TODO(merge): This isn't working cause the non-zero values in `initialize_beacon_state_from_eth1` +// are causing failed lookups to the execution node. I need to come back to this. +#[should_panic] +fn merge_with_terminal_block_hash_override() { + let altair_fork_epoch = Epoch::new(0); + let merge_fork_epoch = Epoch::new(0); + + let mut spec = E::default_spec(); + spec.altair_fork_epoch = Some(altair_fork_epoch); + spec.merge_fork_epoch = Some(merge_fork_epoch); + + let genesis_pow_block_hash = generate_pow_block( + spec.terminal_total_difficulty, + DEFAULT_TERMINAL_BLOCK, + 0, + Hash256::zero(), + ) + .unwrap() + .block_hash; + + spec.terminal_block_hash = genesis_pow_block_hash; + + let harness = BeaconChainHarness::builder(E::default()) + .spec(spec) + .deterministic_keypairs(VALIDATOR_COUNT) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + + assert_eq!( + harness + .execution_block_generator() + .latest_block() + .unwrap() + .block_hash(), + genesis_pow_block_hash, + "pre-condition" + ); + + assert!( + harness + .chain + .head() + .unwrap() + .beacon_block + .as_merge() + .is_ok(), + "genesis block should be a merge block" + ); + + let mut execution_payloads = vec![]; + for i in 0..E::slots_per_epoch() * 3 { + harness.extend_slots(1); + + let block = harness.chain.head().unwrap().beacon_block; + + let execution_payload = block.message().body().execution_payload().unwrap().clone(); + if i == 0 { + assert_eq!(execution_payload.block_hash, genesis_pow_block_hash); + } + execution_payloads.push(execution_payload); + } + + verify_execution_payload_chain(&execution_payloads); +} + +#[test] +fn base_altair_merge_with_terminal_block_after_fork() { + let altair_fork_epoch = Epoch::new(4); + let altair_fork_slot = altair_fork_epoch.start_slot(E::slots_per_epoch()); + let merge_fork_epoch = Epoch::new(8); + let merge_fork_slot = merge_fork_epoch.start_slot(E::slots_per_epoch()); + + let mut spec = E::default_spec(); + spec.altair_fork_epoch = Some(altair_fork_epoch); + spec.merge_fork_epoch = Some(merge_fork_epoch); + + let mut execution_payloads = vec![]; + + let harness = BeaconChainHarness::builder(E::default()) + .spec(spec) + .deterministic_keypairs(VALIDATOR_COUNT) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + + /* + * Start with the base fork. + */ + + assert!(harness.chain.head().unwrap().beacon_block.as_base().is_ok()); + + /* + * Do the Altair fork. + */ + + harness.extend_to_slot(altair_fork_slot); + + let altair_head = harness.chain.head().unwrap().beacon_block; + assert!(altair_head.as_altair().is_ok()); + assert_eq!(altair_head.slot(), altair_fork_slot); + + /* + * Do the merge fork, without a terminal PoW block. + */ + + harness.extend_to_slot(merge_fork_slot); + + let merge_head = harness.chain.head().unwrap().beacon_block; + assert!(merge_head.as_merge().is_ok()); + assert_eq!(merge_head.slot(), merge_fork_slot); + assert_eq!( + *merge_head.message().body().execution_payload().unwrap(), + ExecutionPayload::default() + ); + + /* + * Next merge block shouldn't include an exec payload. + */ + + harness.extend_slots(1); + + let one_after_merge_head = harness.chain.head().unwrap().beacon_block; + assert_eq!( + *one_after_merge_head + .message() + .body() + .execution_payload() + .unwrap(), + ExecutionPayload::default() + ); + assert_eq!(one_after_merge_head.slot(), merge_fork_slot + 1); + + /* + * Trigger the terminal PoW block. + */ + + harness + .execution_block_generator() + .move_to_terminal_block() + .unwrap(); + + /* + * Next merge block should include an exec payload. + */ + + for _ in 0..4 { + harness.extend_slots(1); + + let block = harness.chain.head().unwrap().beacon_block; + execution_payloads.push(block.message().body().execution_payload().unwrap().clone()); + } + + verify_execution_payload_chain(&execution_payloads); +} diff --git a/beacon_node/execution_layer/Cargo.toml b/beacon_node/execution_layer/Cargo.toml index cf6a4c822..dbbbfe5cc 100644 --- a/beacon_node/execution_layer/Cargo.toml +++ b/beacon_node/execution_layer/Cargo.toml @@ -27,3 +27,4 @@ lru = "0.6.0" exit-future = "0.2.0" tree_hash = { path = "../../consensus/tree_hash"} tree_hash_derive = { path = "../../consensus/tree_hash_derive"} +parking_lot = "0.11.0" diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index e395cc44e..af571213b 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -107,7 +107,9 @@ pub enum BlockByNumberQuery<'a> { #[derive(Clone, Copy, Debug, PartialEq, Serialize, Deserialize)] #[serde(rename_all = "camelCase")] pub struct ExecutionBlock { + #[serde(rename = "hash")] pub block_hash: Hash256, + #[serde(rename = "number", with = "eth2_serde_utils::u64_hex_be")] pub block_number: u64, pub parent_hash: Hash256, pub total_difficulty: Uint256, diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index 65b5b102b..a4ec9232e 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -519,7 +519,7 @@ mod test { F: Future, { request_func(self.echo_client.clone()).await; - let request_bytes = self.server.last_echo_request().await; + let request_bytes = self.server.last_echo_request(); let request_json: serde_json::Value = serde_json::from_slice(&request_bytes).expect("request was not valid json"); if request_json != expected_json { @@ -542,7 +542,7 @@ mod test { F: Future, { for response in preloaded_responses { - self.server.push_preloaded_response(response).await; + self.server.push_preloaded_response(response); } request_func(self.rpc_client.clone()).await; self diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index d2f7a29d0..bba43dca5 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -552,171 +552,15 @@ impl ExecutionLayer { #[cfg(test)] mod test { use super::*; - use crate::test_utils::{MockServer, DEFAULT_TERMINAL_DIFFICULTY}; - use environment::null_logger; + use crate::test_utils::MockExecutionLayer as GenericMockExecutionLayer; use types::MainnetEthSpec; - struct SingleEngineTester { - server: MockServer, - el: ExecutionLayer, - runtime: Option>, - _runtime_shutdown: exit_future::Signal, - } - - impl SingleEngineTester { - pub fn new() -> Self { - let server = MockServer::unit_testing(); - - let url = SensitiveUrl::parse(&server.url()).unwrap(); - let log = null_logger().unwrap(); - - let runtime = Arc::new( - tokio::runtime::Builder::new_multi_thread() - .enable_all() - .build() - .unwrap(), - ); - let (runtime_shutdown, exit) = exit_future::signal(); - let (shutdown_tx, _) = futures::channel::mpsc::channel(1); - let executor = - TaskExecutor::new(Arc::downgrade(&runtime), exit, log.clone(), shutdown_tx); - - let el = ExecutionLayer::from_urls( - vec![url], - DEFAULT_TERMINAL_DIFFICULTY.into(), - Hash256::zero(), - Some(Address::repeat_byte(42)), - executor, - log, - ) - .unwrap(); - - Self { - server, - el, - runtime: Some(runtime), - _runtime_shutdown: runtime_shutdown, - } - } - - pub async fn produce_valid_execution_payload_on_head(self) -> Self { - let latest_execution_block = { - let block_gen = self.server.execution_block_generator().await; - block_gen.latest_block().unwrap() - }; - - let parent_hash = latest_execution_block.block_hash(); - let block_number = latest_execution_block.block_number() + 1; - let timestamp = block_number; - let random = Hash256::from_low_u64_be(block_number); - - let _payload_id = self - .el - .prepare_payload(parent_hash, timestamp, random) - .await - .unwrap(); - - let payload = self - .el - .get_payload::(parent_hash, timestamp, random) - .await - .unwrap(); - let block_hash = payload.block_hash; - assert_eq!(payload.parent_hash, parent_hash); - assert_eq!(payload.block_number, block_number); - assert_eq!(payload.timestamp, timestamp); - assert_eq!(payload.random, random); - - let (payload_response, mut payload_handle) = - self.el.execute_payload(&payload).await.unwrap(); - assert_eq!(payload_response, ExecutePayloadResponse::Valid); - - payload_handle.publish_async(ConsensusStatus::Valid).await; - - self.el - .forkchoice_updated(block_hash, Hash256::zero()) - .await - .unwrap(); - - let head_execution_block = { - let block_gen = self.server.execution_block_generator().await; - block_gen.latest_block().unwrap() - }; - - assert_eq!(head_execution_block.block_number(), block_number); - assert_eq!(head_execution_block.block_hash(), block_hash); - assert_eq!(head_execution_block.parent_hash(), parent_hash); - - self - } - - pub async fn move_to_block_prior_to_terminal_block(self) -> Self { - let target_block = { - let block_gen = self.server.execution_block_generator().await; - block_gen.terminal_block_number.checked_sub(1).unwrap() - }; - self.move_to_pow_block(target_block).await - } - - pub async fn move_to_terminal_block(self) -> Self { - let target_block = { - let block_gen = self.server.execution_block_generator().await; - block_gen.terminal_block_number - }; - self.move_to_pow_block(target_block).await - } - - pub async fn move_to_pow_block(self, target_block: u64) -> Self { - { - let mut block_gen = self.server.execution_block_generator().await; - let next_block = block_gen.latest_block().unwrap().block_number() + 1; - assert!(target_block >= next_block); - - block_gen - .insert_pow_blocks(next_block..=target_block) - .unwrap(); - } - self - } - - pub async fn with_terminal_block<'a, T, U>(self, func: T) -> Self - where - T: Fn(ExecutionLayer, Option) -> U, - U: Future, - { - let terminal_block_number = self - .server - .execution_block_generator() - .await - .terminal_block_number; - let terminal_block = self - .server - .execution_block_generator() - .await - .execution_block_by_number(terminal_block_number); - - func(self.el.clone(), terminal_block).await; - self - } - - pub fn shutdown(&mut self) { - if let Some(runtime) = self.runtime.take() { - Arc::try_unwrap(runtime).unwrap().shutdown_background() - } - } - } - - impl Drop for SingleEngineTester { - fn drop(&mut self) { - self.shutdown() - } - } + type MockExecutionLayer = GenericMockExecutionLayer; #[tokio::test] async fn produce_three_valid_pos_execution_blocks() { - SingleEngineTester::new() + MockExecutionLayer::default_params() .move_to_terminal_block() - .await .produce_valid_execution_payload_on_head() .await .produce_valid_execution_payload_on_head() @@ -727,15 +571,13 @@ mod test { #[tokio::test] async fn finds_valid_terminal_block_hash() { - SingleEngineTester::new() + MockExecutionLayer::default_params() .move_to_block_prior_to_terminal_block() - .await .with_terminal_block(|el, _| async move { assert_eq!(el.get_terminal_pow_block_hash().await.unwrap(), None) }) .await .move_to_terminal_block() - .await .with_terminal_block(|el, terminal_block| async move { assert_eq!( el.get_terminal_pow_block_hash().await.unwrap(), @@ -747,9 +589,8 @@ mod test { #[tokio::test] async fn verifies_valid_terminal_block_hash() { - SingleEngineTester::new() + MockExecutionLayer::default_params() .move_to_terminal_block() - .await .with_terminal_block(|el, terminal_block| async move { assert_eq!( el.is_valid_terminal_pow_block_hash(terminal_block.unwrap().block_hash) @@ -763,9 +604,8 @@ mod test { #[tokio::test] async fn rejects_invalid_terminal_block_hash() { - SingleEngineTester::new() + MockExecutionLayer::default_params() .move_to_terminal_block() - .await .with_terminal_block(|el, terminal_block| async move { let invalid_terminal_block = terminal_block.unwrap().parent_hash; @@ -781,9 +621,8 @@ mod test { #[tokio::test] async fn rejects_unknown_terminal_block_hash() { - SingleEngineTester::new() + MockExecutionLayer::default_params() .move_to_terminal_block() - .await .with_terminal_block(|el, _| async move { let missing_terminal_block = Hash256::repeat_byte(42); 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 13ed71242..ae7924e90 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 @@ -7,6 +7,9 @@ use tree_hash::TreeHash; use tree_hash_derive::TreeHash; use types::{EthSpec, ExecutionPayload, Hash256, Uint256}; +const GAS_LIMIT: u64 = 16384; +const GAS_USED: u64 = GAS_LIMIT - 1; + #[derive(Clone, Debug, PartialEq)] #[allow(clippy::large_enum_variant)] // This struct is only for testing. pub enum Block { @@ -43,7 +46,7 @@ impl Block { } } - pub fn as_execution_block(&self, total_difficulty: u64) -> ExecutionBlock { + pub fn as_execution_block(&self, total_difficulty: Uint256) -> ExecutionBlock { match self { Block::PoW(block) => ExecutionBlock { block_hash: block.block_hash, @@ -55,7 +58,7 @@ impl Block { block_hash: payload.block_hash, block_number: payload.block_number, parent_hash: payload.parent_hash, - total_difficulty: total_difficulty.into(), + total_difficulty, }, } } @@ -79,8 +82,9 @@ pub struct ExecutionBlockGenerator { /* * PoW block parameters */ - pub terminal_total_difficulty: u64, + pub terminal_total_difficulty: Uint256, pub terminal_block_number: u64, + pub terminal_block_hash: Hash256, /* * PoS block parameters */ @@ -90,12 +94,17 @@ pub struct ExecutionBlockGenerator { } impl ExecutionBlockGenerator { - pub fn new(terminal_total_difficulty: u64, terminal_block_number: u64) -> Self { + pub fn new( + terminal_total_difficulty: Uint256, + terminal_block_number: u64, + terminal_block_hash: Hash256, + ) -> Self { let mut gen = Self { blocks: <_>::default(), block_hashes: <_>::default(), terminal_total_difficulty, terminal_block_number, + terminal_block_hash, pending_payloads: <_>::default(), next_payload_id: 0, payload_ids: <_>::default(), @@ -140,6 +149,25 @@ impl ExecutionBlockGenerator { .map(|block| block.as_execution_block(self.terminal_total_difficulty)) } + pub fn move_to_block_prior_to_terminal_block(&mut self) -> Result<(), String> { + let target_block = self + .terminal_block_number + .checked_sub(1) + .ok_or("terminal pow block is 0")?; + self.move_to_pow_block(target_block) + } + + pub fn move_to_terminal_block(&mut self) -> Result<(), String> { + self.move_to_pow_block(self.terminal_block_number) + } + + pub fn move_to_pow_block(&mut self, target_block: u64) -> Result<(), String> { + let next_block = self.latest_block().unwrap().block_number() + 1; + assert!(target_block >= next_block); + + self.insert_pow_blocks(next_block..=target_block) + } + pub fn insert_pow_blocks( &mut self, block_numbers: impl Iterator, @@ -152,13 +180,6 @@ impl ExecutionBlockGenerator { } pub fn insert_pow_block(&mut self, block_number: u64) -> Result<(), String> { - if block_number > self.terminal_block_number { - return Err(format!( - "{} is beyond terminal pow block {}", - block_number, self.terminal_block_number - )); - } - let parent_hash = if block_number == 0 { Hash256::zero() } else if let Some(hash) = self.block_hashes.get(&(block_number - 1)) { @@ -170,23 +191,12 @@ impl ExecutionBlockGenerator { )); }; - let increment = self - .terminal_total_difficulty - .checked_div(self.terminal_block_number) - .expect("terminal block number must be non-zero"); - let total_difficulty = increment - .checked_mul(block_number) - .expect("overflow computing total difficulty") - .into(); - - let mut block = PoWBlock { + let block = generate_pow_block( + self.terminal_total_difficulty, + self.terminal_block_number, block_number, - block_hash: Hash256::zero(), parent_hash, - total_difficulty, - }; - - block.block_hash = block.tree_hash_root(); + )?; self.insert_block(Block::PoW(block)) } @@ -213,11 +223,10 @@ impl ExecutionBlockGenerator { } pub fn prepare_payload(&mut self, payload: JsonPreparePayloadRequest) -> Result { - if !self - .blocks - .iter() - .any(|(_, block)| block.block_number() == self.terminal_block_number) - { + if !self.blocks.iter().any(|(_, block)| { + block.block_hash() == self.terminal_block_hash + || block.block_number() == self.terminal_block_number + }) { return Err("refusing to create payload id before terminal block".to_string()); } @@ -237,8 +246,8 @@ impl ExecutionBlockGenerator { logs_bloom: vec![0; 256].into(), random: payload.random, block_number: parent.block_number() + 1, - gas_limit: 10, - gas_used: 9, + gas_limit: GAS_LIMIT, + gas_used: GAS_USED, timestamp: payload.timestamp, extra_data: "block gen was here".as_bytes().to_vec().into(), base_fee_per_gas: Hash256::from_low_u64_le(1), @@ -311,6 +320,42 @@ impl ExecutionBlockGenerator { } } +pub fn generate_pow_block( + terminal_total_difficulty: Uint256, + terminal_block_number: u64, + block_number: u64, + parent_hash: Hash256, +) -> Result { + if block_number > terminal_block_number { + return Err(format!( + "{} is beyond terminal pow block {}", + block_number, terminal_block_number + )); + } + + let total_difficulty = if block_number == terminal_block_number { + terminal_total_difficulty + } else { + let increment = terminal_total_difficulty + .checked_div(Uint256::from(terminal_block_number)) + .expect("terminal block number must be non-zero"); + increment + .checked_mul(Uint256::from(block_number)) + .expect("overflow computing total difficulty") + }; + + let mut block = PoWBlock { + block_number, + block_hash: Hash256::zero(), + parent_hash, + total_difficulty, + }; + + block.block_hash = block.tree_hash_root(); + + Ok(block) +} + #[cfg(test)] mod test { use super::*; @@ -322,8 +367,11 @@ mod test { const TERMINAL_BLOCK: u64 = 10; const DIFFICULTY_INCREMENT: u64 = 1; - let mut generator: ExecutionBlockGenerator = - ExecutionBlockGenerator::new(TERMINAL_DIFFICULTY, TERMINAL_BLOCK); + let mut generator: ExecutionBlockGenerator = ExecutionBlockGenerator::new( + TERMINAL_DIFFICULTY.into(), + TERMINAL_BLOCK, + Hash256::zero(), + ); for i in 0..=TERMINAL_BLOCK { if i > 0 { diff --git a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs index 38a0f211b..0501263e7 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -30,7 +30,6 @@ pub async fn handle_rpc( "latest" => Ok(serde_json::to_value( ctx.execution_block_generator .read() - .await .latest_execution_block(), ) .unwrap()), @@ -50,7 +49,6 @@ pub async fn handle_rpc( Ok(serde_json::to_value( ctx.execution_block_generator .read() - .await .execution_block_by_hash(hash), ) .unwrap()) @@ -60,7 +58,6 @@ pub async fn handle_rpc( let payload_id = ctx .execution_block_generator .write() - .await .prepare_payload(request)?; Ok(serde_json::to_value(JsonPayloadIdResponse { payload_id }).unwrap()) @@ -70,7 +67,6 @@ pub async fn handle_rpc( let status = ctx .execution_block_generator .write() - .await .execute_payload(request.into()); Ok(serde_json::to_value(ExecutePayloadResponseWrapper { status }).unwrap()) @@ -82,7 +78,6 @@ pub async fn handle_rpc( let response = ctx .execution_block_generator .write() - .await .get_payload(id) .ok_or_else(|| format!("no payload for id {}", id))?; @@ -93,7 +88,6 @@ pub async fn handle_rpc( let request: JsonConsensusValidatedRequest = get_param_0(params)?; ctx.execution_block_generator .write() - .await .consensus_validated(request.block_hash, request.status)?; Ok(JsonValue::Null) @@ -102,7 +96,6 @@ pub async fn handle_rpc( let request: JsonForkChoiceUpdatedRequest = get_param_0(params)?; ctx.execution_block_generator .write() - .await .forkchoice_updated(request.head_block_hash, request.finalized_block_hash)?; Ok(JsonValue::Null) diff --git a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs new file mode 100644 index 000000000..782e86df0 --- /dev/null +++ b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs @@ -0,0 +1,182 @@ +use crate::{ + test_utils::{MockServer, DEFAULT_TERMINAL_BLOCK, DEFAULT_TERMINAL_DIFFICULTY}, + *, +}; +use environment::null_logger; +use sensitive_url::SensitiveUrl; +use std::sync::Arc; +use task_executor::TaskExecutor; +use types::{Address, EthSpec, Hash256, Uint256}; + +pub struct ExecutionLayerRuntime { + pub runtime: Option>, + pub _runtime_shutdown: exit_future::Signal, + pub task_executor: TaskExecutor, + pub log: Logger, +} + +impl Default for ExecutionLayerRuntime { + fn default() -> Self { + let runtime = Arc::new( + tokio::runtime::Builder::new_multi_thread() + .enable_all() + .build() + .unwrap(), + ); + let (runtime_shutdown, exit) = exit_future::signal(); + let (shutdown_tx, _) = futures::channel::mpsc::channel(1); + let log = null_logger().unwrap(); + let task_executor = + TaskExecutor::new(Arc::downgrade(&runtime), exit, log.clone(), shutdown_tx); + + Self { + runtime: Some(runtime), + _runtime_shutdown: runtime_shutdown, + task_executor, + log, + } + } +} + +impl Drop for ExecutionLayerRuntime { + fn drop(&mut self) { + if let Some(runtime) = self.runtime.take() { + Arc::try_unwrap(runtime).unwrap().shutdown_background() + } + } +} + +pub struct MockExecutionLayer { + pub server: MockServer, + pub el: ExecutionLayer, + pub el_runtime: ExecutionLayerRuntime, +} + +impl MockExecutionLayer { + pub fn default_params() -> Self { + Self::new( + DEFAULT_TERMINAL_DIFFICULTY.into(), + DEFAULT_TERMINAL_BLOCK, + Hash256::zero(), + ) + } + + pub fn new( + terminal_total_difficulty: Uint256, + terminal_block: u64, + terminal_block_hash: Hash256, + ) -> Self { + let el_runtime = ExecutionLayerRuntime::default(); + let handle = el_runtime.runtime.as_ref().unwrap().handle(); + + let server = MockServer::new( + handle, + terminal_total_difficulty, + terminal_block, + terminal_block_hash, + ); + + let url = SensitiveUrl::parse(&server.url()).unwrap(); + + let el = ExecutionLayer::from_urls( + vec![url], + terminal_total_difficulty, + Hash256::zero(), + Some(Address::repeat_byte(42)), + el_runtime.task_executor.clone(), + el_runtime.log.clone(), + ) + .unwrap(); + + Self { + server, + el, + el_runtime, + } + } + + pub async fn produce_valid_execution_payload_on_head(self) -> Self { + let latest_execution_block = { + let block_gen = self.server.execution_block_generator(); + block_gen.latest_block().unwrap() + }; + + let parent_hash = latest_execution_block.block_hash(); + let block_number = latest_execution_block.block_number() + 1; + let timestamp = block_number; + let random = Hash256::from_low_u64_be(block_number); + + let _payload_id = self + .el + .prepare_payload(parent_hash, timestamp, random) + .await + .unwrap(); + + let payload = self + .el + .get_payload::(parent_hash, timestamp, random) + .await + .unwrap(); + let block_hash = payload.block_hash; + assert_eq!(payload.parent_hash, parent_hash); + assert_eq!(payload.block_number, block_number); + assert_eq!(payload.timestamp, timestamp); + assert_eq!(payload.random, random); + + let (payload_response, mut payload_handle) = + self.el.execute_payload(&payload).await.unwrap(); + assert_eq!(payload_response, ExecutePayloadResponse::Valid); + + payload_handle.publish_async(ConsensusStatus::Valid).await; + + self.el + .forkchoice_updated(block_hash, Hash256::zero()) + .await + .unwrap(); + + let head_execution_block = { + let block_gen = self.server.execution_block_generator(); + block_gen.latest_block().unwrap() + }; + + assert_eq!(head_execution_block.block_number(), block_number); + assert_eq!(head_execution_block.block_hash(), block_hash); + assert_eq!(head_execution_block.parent_hash(), parent_hash); + + self + } + + pub fn move_to_block_prior_to_terminal_block(self) -> Self { + self.server + .execution_block_generator() + .move_to_block_prior_to_terminal_block() + .unwrap(); + self + } + + pub fn move_to_terminal_block(self) -> Self { + self.server + .execution_block_generator() + .move_to_terminal_block() + .unwrap(); + self + } + + pub async fn with_terminal_block<'a, U, V>(self, func: U) -> Self + where + U: Fn(ExecutionLayer, Option) -> V, + V: Future, + { + let terminal_block_number = self + .server + .execution_block_generator() + .terminal_block_number; + let terminal_block = self + .server + .execution_block_generator() + .execution_block_by_number(terminal_block_number); + + func(self.el.clone(), terminal_block).await; + self + } +} diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index b7969d0c3..87490042b 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -3,8 +3,8 @@ use crate::engine_api::http::JSONRPC_VERSION; use bytes::Bytes; use environment::null_logger; -use execution_block_generator::ExecutionBlockGenerator; use handle_rpc::handle_rpc; +use parking_lot::{Mutex, RwLock, RwLockWriteGuard}; use serde::{Deserialize, Serialize}; use serde_json::json; use slog::{info, Logger}; @@ -12,15 +12,19 @@ use std::future::Future; use std::marker::PhantomData; use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4}; use std::sync::Arc; -use tokio::sync::{oneshot, Mutex, RwLock, RwLockWriteGuard}; -use types::EthSpec; +use tokio::{runtime, sync::oneshot}; +use types::{EthSpec, Hash256, Uint256}; use warp::Filter; +pub use execution_block_generator::{generate_pow_block, ExecutionBlockGenerator}; +pub use mock_execution_layer::{ExecutionLayerRuntime, MockExecutionLayer}; + pub const DEFAULT_TERMINAL_DIFFICULTY: u64 = 6400; pub const DEFAULT_TERMINAL_BLOCK: u64 = 64; mod execution_block_generator; mod handle_rpc; +mod mock_execution_layer; pub struct MockServer { _shutdown_tx: oneshot::Sender<()>, @@ -31,10 +35,24 @@ pub struct MockServer { impl MockServer { pub fn unit_testing() -> Self { + Self::new( + &runtime::Handle::current(), + DEFAULT_TERMINAL_DIFFICULTY.into(), + DEFAULT_TERMINAL_BLOCK, + Hash256::zero(), + ) + } + + pub fn new( + handle: &runtime::Handle, + terminal_difficulty: Uint256, + terminal_block: u64, + terminal_block_hash: Hash256, + ) -> Self { let last_echo_request = Arc::new(RwLock::new(None)); let preloaded_responses = Arc::new(Mutex::new(vec![])); let execution_block_generator = - ExecutionBlockGenerator::new(DEFAULT_TERMINAL_DIFFICULTY, DEFAULT_TERMINAL_BLOCK); + ExecutionBlockGenerator::new(terminal_difficulty, terminal_block, terminal_block_hash); let ctx: Arc> = Arc::new(Context { config: <_>::default(), @@ -52,9 +70,17 @@ impl MockServer { let _ = shutdown_rx.await; }; - let (listen_socket_addr, server_future) = serve(ctx.clone(), shutdown_future).unwrap(); + // The `serve` function will panic unless it's run inside a tokio runtime, so use `block_on` + // if we're not in a runtime. However, we can't *always* use `block_on` since tokio will + // panic if we try to block inside an async context. + let serve = || serve(ctx.clone(), shutdown_future).unwrap(); + let (listen_socket_addr, server_future) = if runtime::Handle::try_current().is_err() { + handle.block_on(async { serve() }) + } else { + serve() + }; - tokio::spawn(server_future); + handle.spawn(server_future); Self { _shutdown_tx: shutdown_tx, @@ -64,10 +90,8 @@ impl MockServer { } } - pub async fn execution_block_generator( - &self, - ) -> RwLockWriteGuard<'_, ExecutionBlockGenerator> { - self.ctx.execution_block_generator.write().await + pub fn execution_block_generator(&self) -> RwLockWriteGuard<'_, ExecutionBlockGenerator> { + self.ctx.execution_block_generator.write() } pub fn url(&self) -> String { @@ -78,16 +102,15 @@ impl MockServer { ) } - pub async fn last_echo_request(&self) -> Bytes { + pub fn last_echo_request(&self) -> Bytes { self.last_echo_request .write() - .await .take() .expect("last echo request is none") } - pub async fn push_preloaded_response(&self, response: serde_json::Value) { - self.ctx.preloaded_responses.lock().await.push(response) + pub fn push_preloaded_response(&self, response: serde_json::Value) { + self.ctx.preloaded_responses.lock().push(response) } } @@ -180,7 +203,7 @@ pub fn serve( .ok_or_else(|| warp::reject::custom(MissingIdField))?; let preloaded_response = { - let mut preloaded_responses = ctx.preloaded_responses.lock().await; + let mut preloaded_responses = ctx.preloaded_responses.lock(); if !preloaded_responses.is_empty() { Some(preloaded_responses.remove(0)) } else { @@ -222,7 +245,7 @@ pub fn serve( .and(warp::body::bytes()) .and(ctx_filter) .and_then(|bytes: Bytes, ctx: Arc>| async move { - *ctx.last_echo_request.write().await = Some(bytes.clone()); + *ctx.last_echo_request.write() = Some(bytes.clone()); Ok::<_, warp::reject::Rejection>( warp::http::Response::builder().status(200).body(bytes), ) diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index 1a1f8c58a..b2c489c28 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -296,13 +296,16 @@ pub fn get_new_eth1_data( } /// https://github.com/ethereum/consensus-specs/blob/dev/specs/merge/beacon-chain.md#is_valid_gas_limit -pub fn is_valid_gas_limit( +pub fn verify_is_valid_gas_limit( payload: &ExecutionPayload, parent: &ExecutionPayloadHeader, -) -> Result { +) -> Result<(), BlockProcessingError> { // check if payload used too much gas if payload.gas_used > payload.gas_limit { - return Ok(false); + return Err(BlockProcessingError::ExecutionInvalidGasLimit { + used: payload.gas_used, + limit: payload.gas_limit, + }); } // check if payload changed the gas limit too much if payload.gas_limit @@ -310,21 +313,30 @@ pub fn is_valid_gas_limit( .gas_limit .safe_add(parent.gas_limit.safe_div(T::gas_limit_denominator())?)? { - return Ok(false); + return Err(BlockProcessingError::ExecutionInvalidGasLimitIncrease { + limit: payload.gas_limit, + parent_limit: parent.gas_limit, + }); } if payload.gas_limit <= parent .gas_limit .safe_sub(parent.gas_limit.safe_div(T::gas_limit_denominator())?)? { - return Ok(false); + return Err(BlockProcessingError::ExecutionInvalidGasLimitDecrease { + limit: payload.gas_limit, + parent_limit: parent.gas_limit, + }); } // check if the gas limit is at least the minimum gas limit if payload.gas_limit < T::min_gas_limit() { - return Ok(false); + return Err(BlockProcessingError::ExecutionInvalidGasLimitTooSmall { + limit: payload.gas_limit, + min: T::min_gas_limit(), + }); } - Ok(true) + Ok(()) } /// https://github.com/ethereum/consensus-specs/blob/dev/specs/merge/beacon-chain.md#process_execution_payload @@ -355,13 +367,7 @@ pub fn process_execution_payload( found: payload.block_number, } ); - block_verify!( - is_valid_gas_limit(payload, state.latest_execution_payload_header()?)?, - BlockProcessingError::ExecutionInvalidGasLimit { - used: payload.gas_used, - limit: payload.gas_limit, - } - ); + verify_is_valid_gas_limit(payload, state.latest_execution_payload_header()?)?; } block_verify!( payload.random == *state.get_randao_mix(state.current_epoch())?, diff --git a/consensus/state_processing/src/per_block_processing/errors.rs b/consensus/state_processing/src/per_block_processing/errors.rs index 825b965dc..c06f3d20e 100644 --- a/consensus/state_processing/src/per_block_processing/errors.rs +++ b/consensus/state_processing/src/per_block_processing/errors.rs @@ -73,6 +73,18 @@ pub enum BlockProcessingError { used: u64, limit: u64, }, + ExecutionInvalidGasLimitIncrease { + limit: u64, + parent_limit: u64, + }, + ExecutionInvalidGasLimitDecrease { + limit: u64, + parent_limit: u64, + }, + ExecutionInvalidGasLimitTooSmall { + limit: u64, + min: u64, + }, ExecutionInvalidTimestamp { expected: u64, found: u64,