From 369b624b19da6617c133966a42d96b5abbac1569 Mon Sep 17 00:00:00 2001 From: Mac L Date: Wed, 18 Oct 2023 04:08:55 +0000 Subject: [PATCH] Fix broken Nethermind integration tests (#4836) ## Issue Addressed CI is currently blocked by persistently failing integration tests. ## Proposed Changes Use latest Nethermind release and apply the appropriate fixes as there have been breaking changes. Also increase the timeout since I had some local timeouts. Co-authored-by: Michael Sproul Co-authored-by: antondlr Co-authored-by: Jimmy Chen --- .github/workflows/test-suite.yml | 6 ++- lighthouse/tests/beacon_node.rs | 2 +- .../src/build_utils.rs | 1 + .../src/genesis_json.rs | 33 +++++++----- .../execution_engine_integration/src/geth.rs | 11 +++- .../execution_engine_integration/src/main.rs | 3 ++ .../src/nethermind.rs | 15 ++++-- .../src/test_rig.rs | 51 +++++++++++-------- 8 files changed, 82 insertions(+), 40 deletions(-) diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index 1d80feadd..f0811257a 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -317,10 +317,11 @@ jobs: ./doppelganger_protection.sh success genesis.json execution-engine-integration-ubuntu: name: execution-engine-integration-ubuntu - runs-on: ubuntu-latest + runs-on: ${{ github.repository == 'sigp/lighthouse' && fromJson('["self-hosted", "linux", "CI", "small"]') || 'ubuntu-latest' }} steps: - uses: actions/checkout@v3 - name: Get latest version of stable Rust + if: env.SELF_HOSTED_RUNNERS == 'false' uses: moonrepo/setup-rust@v1 with: channel: stable @@ -328,6 +329,9 @@ jobs: cache: false env: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} + - name: Add go compiler to $PATH + if: env.SELF_HOSTED_RUNNERS == 'true' + run: echo "/usr/local/go/bin" >> $GITHUB_PATH - name: Run exec engine integration tests in release run: make test-exec-engine check-code: diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 95172980f..7937b1d49 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -835,7 +835,7 @@ fn run_jwt_optional_flags_test(jwt_flag: &str, jwt_id_flag: &str, jwt_version_fl let id = "bn-1"; let version = "Lighthouse-v2.1.3"; CommandLineTest::new() - .flag("execution-endpoint", Some(execution_endpoint.clone())) + .flag("execution-endpoint", Some(execution_endpoint)) .flag(jwt_flag, dir.path().join(jwt_file).as_os_str().to_str()) .flag(jwt_id_flag, Some(id)) .flag(jwt_version_flag, Some(version)) diff --git a/testing/execution_engine_integration/src/build_utils.rs b/testing/execution_engine_integration/src/build_utils.rs index 15e7fdc0f..5d9652066 100644 --- a/testing/execution_engine_integration/src/build_utils.rs +++ b/testing/execution_engine_integration/src/build_utils.rs @@ -66,6 +66,7 @@ pub fn get_latest_release(repo_dir: &Path, branch_name: &str) -> Result Value { "muirGlacierBlock":0, "berlinBlock":0, "londonBlock":0, - "clique": { - "period": 5, - "epoch": 30000 - }, - "terminalTotalDifficulty":0 + "mergeNetsplitBlock": 0, + "shanghaiTime": 0, + "terminalTotalDifficulty": 0, + "terminalTotalDifficultyPassed": true }, "nonce":"0x42", "timestamp":"0x0", @@ -72,8 +71,10 @@ pub fn nethermind_genesis_json() -> Value { "accountStartNonce": "0x0", "maximumExtraDataSize": "0x20", "minGasLimit": "0x1388", - "networkID": "0x1469ca", - "MergeForkIdTransition": "0x3e8", + "networkID": "0x00146A2E", + "MergeForkIdTransition": "0x0", + "maxCodeSize": "0x6000", + "maxCodeSizeTransition": "0x0", "eip150Transition": "0x0", "eip158Transition": "0x0", "eip160Transition": "0x0", @@ -101,7 +102,15 @@ pub fn nethermind_genesis_json() -> Value { "eip1559Transition": "0x0", "eip3198Transition": "0x0", "eip3529Transition": "0x0", - "eip3541Transition": "0x0" + "eip3541Transition": "0x0", + "eip3540TransitionTimestamp": "0x0", + "eip3651TransitionTimestamp": "0x0", + "eip3670TransitionTimestamp": "0x0", + "eip3675TransitionTimestamp": "0x0", + "eip3855TransitionTimestamp": "0x0", + "eip3860TransitionTimestamp": "0x0", + "eip4895TransitionTimestamp": "0x0", + "terminalTotalDifficulty": "0x0" }, "genesis": { "seal": { @@ -112,10 +121,10 @@ pub fn nethermind_genesis_json() -> Value { }, "difficulty": "0x01", "author": "0x0000000000000000000000000000000000000000", - "timestamp": "0x0", + "timestamp": "0x63585F88", "parentHash": "0x0000000000000000000000000000000000000000000000000000000000000000", "extraData": "", - "gasLimit": "0x1C9C380" + "gasLimit": "0x400000" }, "accounts": { "0x7b8C3a386C0eea54693fFB0DA17373ffC9228139": { @@ -123,9 +132,9 @@ pub fn nethermind_genesis_json() -> Value { }, "0xdA2DD7560DB7e212B945fC72cEB54B7D8C886D77": { "balance": "10000000000000000000000000" - }, + } }, "nodes": [] - } + } ) } diff --git a/testing/execution_engine_integration/src/geth.rs b/testing/execution_engine_integration/src/geth.rs index 5c83a97e2..0bd96a5c9 100644 --- a/testing/execution_engine_integration/src/geth.rs +++ b/testing/execution_engine_integration/src/geth.rs @@ -3,7 +3,7 @@ use crate::execution_engine::GenericExecutionEngine; use crate::genesis_json::geth_genesis_json; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Output}; -use std::{env, fs::File}; +use std::{env, fs}; use tempfile::TempDir; use unused_port::unused_tcp4_port; @@ -36,6 +36,13 @@ pub fn build(execution_clients_dir: &Path) { }); } +pub fn clean(execution_clients_dir: &Path) { + let repo_dir = execution_clients_dir.join("go-ethereum"); + if let Err(e) = fs::remove_dir_all(repo_dir) { + eprintln!("Error while deleting folder: {}", e); + } +} + /* * Geth-specific Implementation for GenericExecutionEngine */ @@ -60,7 +67,7 @@ impl GenericExecutionEngine for GethEngine { let datadir = TempDir::new().unwrap(); let genesis_json_path = datadir.path().join("genesis.json"); - let mut file = File::create(&genesis_json_path).unwrap(); + let mut file = fs::File::create(&genesis_json_path).unwrap(); let json = geth_genesis_json(); serde_json::to_writer(&mut file, &json).unwrap(); diff --git a/testing/execution_engine_integration/src/main.rs b/testing/execution_engine_integration/src/main.rs index e46bc13c8..efb06833f 100644 --- a/testing/execution_engine_integration/src/main.rs +++ b/testing/execution_engine_integration/src/main.rs @@ -1,3 +1,5 @@ +#![recursion_limit = "256"] // for inline json + /// This binary runs integration tests between Lighthouse and execution engines. /// /// It will first attempt to build any supported integration clients, then it will run tests. @@ -31,6 +33,7 @@ fn test_geth() { let test_dir = build_utils::prepare_dir(); geth::build(&test_dir); TestRig::new(GethEngine).perform_tests_blocking(); + geth::clean(&test_dir); } fn test_nethermind() { diff --git a/testing/execution_engine_integration/src/nethermind.rs b/testing/execution_engine_integration/src/nethermind.rs index 8925f1cc8..aad37c32b 100644 --- a/testing/execution_engine_integration/src/nethermind.rs +++ b/testing/execution_engine_integration/src/nethermind.rs @@ -2,7 +2,7 @@ use crate::build_utils; use crate::execution_engine::GenericExecutionEngine; use crate::genesis_json::nethermind_genesis_json; use std::env; -use std::fs::File; +use std::fs; use std::path::{Path, PathBuf}; use std::process::{Child, Command, Output}; use tempfile::TempDir; @@ -11,7 +11,7 @@ use unused_port::unused_tcp4_port; /// We've pinned the Nethermind version since our method of using the `master` branch to /// find the latest tag isn't working. It appears Nethermind don't always tag on `master`. /// We should fix this so we always pull the latest version of Nethermind. -const NETHERMIND_BRANCH: &str = "release/1.18.2"; +const NETHERMIND_BRANCH: &str = "release/1.21.0"; const NETHERMIND_REPO_URL: &str = "https://github.com/NethermindEth/nethermind"; fn build_result(repo_dir: &Path) -> Output { @@ -47,6 +47,12 @@ pub fn build(execution_clients_dir: &Path) { build_utils::check_command_output(build_result(&repo_dir), || { format!("nethermind build failed using release {last_release}") }); + + // Cleanup some disk space by removing nethermind's tests + let tests_dir = execution_clients_dir.join("nethermind/src/tests"); + if let Err(e) = fs::remove_dir_all(tests_dir) { + eprintln!("Error while deleting folder: {}", e); + } } /* @@ -68,7 +74,8 @@ impl NethermindEngine { .join("bin") .join("Release") .join("net7.0") - .join("Nethermind.Runner") + .join("linux-x64") + .join("nethermind") } } @@ -76,7 +83,7 @@ impl GenericExecutionEngine for NethermindEngine { fn init_datadir() -> TempDir { let datadir = TempDir::new().unwrap(); let genesis_json_path = datadir.path().join("genesis.json"); - let mut file = File::create(genesis_json_path).unwrap(); + let mut file = fs::File::create(genesis_json_path).unwrap(); let json = nethermind_genesis_json(); serde_json::to_writer(&mut file, &json).unwrap(); datadir diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index 2aaff30f5..48195f871 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -18,7 +18,9 @@ use types::{ Address, ChainSpec, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadHeader, ForkName, FullPayload, Hash256, MainnetEthSpec, PublicKeyBytes, Slot, Uint256, }; -const EXECUTION_ENGINE_START_TIMEOUT: Duration = Duration::from_secs(30); +const EXECUTION_ENGINE_START_TIMEOUT: Duration = Duration::from_secs(60); + +const TEST_FORK: ForkName = ForkName::Capella; struct ExecutionPair { /// The Lighthouse `ExecutionLayer` struct, connected to the `execution_engine` via HTTP. @@ -110,7 +112,7 @@ impl TestRig { 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 mut spec = MainnetEthSpec::default_spec(); + let mut spec = TEST_FORK.make_genesis_spec(MainnetEthSpec::default_spec()); spec.terminal_total_difficulty = Uint256::zero(); let fee_recipient = None; @@ -269,12 +271,11 @@ impl TestRig { Slot::new(1), // Insert proposer for the next slot head_root, proposer_index, - // TODO: think about how to test different forks PayloadAttributes::new( timestamp, prev_randao, Address::repeat_byte(42), - None, + Some(vec![]), None, ), ) @@ -314,8 +315,13 @@ impl TestRig { .execution_layer .get_suggested_fee_recipient(proposer_index) .await; - let payload_attributes = - PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None, None); + let payload_attributes = PayloadAttributes::new( + timestamp, + prev_randao, + suggested_fee_recipient, + Some(vec![]), + None, + ); let valid_payload = self .ee_a .execution_layer @@ -324,8 +330,7 @@ impl TestRig { &payload_attributes, forkchoice_update_params, builder_params, - // FIXME: think about how to test other forks - ForkName::Merge, + TEST_FORK, &self.spec, ) .await @@ -456,8 +461,13 @@ impl TestRig { .execution_layer .get_suggested_fee_recipient(proposer_index) .await; - let payload_attributes = - PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None, None); + let payload_attributes = PayloadAttributes::new( + timestamp, + prev_randao, + suggested_fee_recipient, + Some(vec![]), + None, + ); let second_payload = self .ee_a .execution_layer @@ -466,8 +476,7 @@ impl TestRig { &payload_attributes, forkchoice_update_params, builder_params, - // FIXME: think about how to test other forks - ForkName::Merge, + TEST_FORK, &self.spec, ) .await @@ -498,11 +507,15 @@ impl TestRig { */ let head_block_hash = valid_payload.block_hash(); let finalized_block_hash = ExecutionBlockHash::zero(); - // TODO: think about how to handle different forks // To save sending proposer preparation data, just set the fee recipient // to the fee recipient configured for EE A. - let payload_attributes = - PayloadAttributes::new(timestamp, prev_randao, Address::repeat_byte(42), None, None); + let payload_attributes = PayloadAttributes::new( + timestamp, + prev_randao, + Address::repeat_byte(42), + Some(vec![]), + None, + ); let slot = Slot::new(42); let head_block_root = Hash256::repeat_byte(100); let validator_index = 0; @@ -536,11 +549,7 @@ impl TestRig { .notify_new_payload(second_payload.clone().try_into().unwrap()) .await .unwrap(); - // TODO: we should remove the `Accepted` status here once Geth fixes it - assert!(matches!( - status, - PayloadStatus::Syncing | PayloadStatus::Accepted - )); + assert!(matches!(status, PayloadStatus::Syncing)); /* * Execution Engine B: @@ -641,11 +650,13 @@ async fn check_payload_reconstruction( .get_engine_capabilities(None) .await .unwrap(); + assert!( // if the engine doesn't have these capabilities, we need to update the client in our tests capabilities.get_payload_bodies_by_hash_v1 && capabilities.get_payload_bodies_by_range_v1, "Testing engine does not support payload bodies methods" ); + let mut bodies = ee .execution_layer .get_payload_bodies_by_hash(vec![payload.block_hash()])