From aea43b626b3384aa24f11e48bdc7c1b23c145cee Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Thu, 3 Mar 2022 02:10:57 +0000 Subject: [PATCH] Rename random to prev_randao (#3040) ## Issue Addressed As discussed on last-night's consensus call, the testnets next week will target the [Kiln Spec v2](https://hackmd.io/@n0ble/kiln-spec). Presently, we support Kiln V1. V2 is backwards compatible, except for renaming `random` to `prev_randao` in: - https://github.com/ethereum/execution-apis/pull/180 - https://github.com/ethereum/consensus-specs/pull/2835 With this PR we'll no longer be compatible with the existing Kintsugi and Kiln testnets, however we'll be ready for the testnets next week. I raised this breaking change in the call last night, we are all keen to move forward and break things. We now target the [`merge-kiln-v2`](https://github.com/MariusVanDerWijden/go-ethereum/tree/merge-kiln-v2) branch for interop with Geth. This required adding the `--http.aauthport` to the tester to avoid a port conflict at startup. ### Changes to exec integration tests There's some change in the `merge-kiln-v2` version of Geth that means it can't compile on a vanilla Github runner. Bumping the `go` version on the runner solved this issue. Whilst addressing this, I refactored the `testing/execution_integration` crate to be a *binary* rather than a *library* with tests. This means that we don't need to run the `build.rs` and build Geth whenever someone runs `make lint` or `make test-release`. This is nice for everyday users, but it's also nice for CI so that we can have a specific runner for these tests and we don't need to ensure *all* runners support everything required to build all execution clients. ## More Info - [x] ~~EF tests are failing since the rename has broken some tests that reference the old field name. I have been told there will be new tests released in the coming days (25/02/22 or 26/02/22).~~ --- .github/workflows/test-suite.yml | 3 + beacon_node/beacon_chain/src/beacon_chain.rs | 53 +++--- .../beacon_chain/src/execution_payload.rs | 28 +++- .../tests/payload_invalidation.rs | 37 ++++- beacon_node/execution_layer/src/engine_api.rs | 2 +- .../execution_layer/src/engine_api/http.rs | 24 +-- .../src/engine_api/json_structures.rs | 20 +-- beacon_node/execution_layer/src/engines.rs | 8 +- beacon_node/execution_layer/src/lib.rs | 8 +- .../test_utils/execution_block_generator.rs | 2 +- .../src/test_utils/mock_execution_layer.rs | 8 +- bors.toml | 7 +- consensus/fork_choice/src/fork_choice.rs | 23 ++- consensus/fork_choice/src/lib.rs | 2 +- .../src/fork_choice_test_definition.rs | 23 ++- consensus/proto_array/src/lib.rs | 1 + consensus/proto_array/src/proto_array.rs | 151 +++++++++++------- .../src/proto_array_fork_choice.rs | 7 +- .../src/per_block_processing.rs | 6 +- consensus/types/src/execution_payload.rs | 2 +- .../types/src/execution_payload_header.rs | 2 +- lcli/src/create_payload_header.rs | 2 +- testing/ef_tests/Makefile | 2 +- .../execution_engine_integration/Cargo.toml | 2 - testing/execution_engine_integration/Makefile | 2 +- .../{build.rs => src/build_geth.rs} | 16 +- .../src/execution_engine.rs | 16 +- .../execution_engine_integration/src/lib.rs | 12 -- .../execution_engine_integration/src/main.rs | 28 ++++ .../src/test_rig.rs | 12 +- .../tests/tests.rs | 16 -- 31 files changed, 304 insertions(+), 221 deletions(-) rename testing/execution_engine_integration/{build.rs => src/build_geth.rs} (81%) delete mode 100644 testing/execution_engine_integration/src/lib.rs create mode 100644 testing/execution_engine_integration/src/main.rs delete mode 100644 testing/execution_engine_integration/tests/tests.rs diff --git a/.github/workflows/test-suite.yml b/.github/workflows/test-suite.yml index 919bdbac2aa..0b29751106b 100644 --- a/.github/workflows/test-suite.yml +++ b/.github/workflows/test-suite.yml @@ -190,6 +190,9 @@ jobs: needs: cargo-fmt steps: - uses: actions/checkout@v1 + - uses: actions/setup-go@v2 + with: + go-version: '1.17' - name: Get latest version of stable Rust run: rustup update stable - name: Run exec engine integration tests in release diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 4cd5cfe07be..7655faa0f40 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -53,7 +53,7 @@ use eth2::types::{ EventKind, SseBlock, SseChainReorg, SseFinalizedCheckpoint, SseHead, SseLateHead, SyncDuty, }; use execution_layer::{ExecutionLayer, PayloadStatus}; -use fork_choice::{AttestationFromBlock, ForkChoice}; +use fork_choice::{AttestationFromBlock, ForkChoice, InvalidationOperation}; use futures::channel::mpsc::Sender; use itertools::process_results; use itertools::Itertools; @@ -3180,49 +3180,29 @@ impl BeaconChain { /// This method must be called whenever an execution engine indicates that a payload is /// invalid. /// - /// If the `latest_root` is known to fork-choice it will be invalidated. If it is not known, an - /// error will be returned. + /// Fork choice will be run after the invalidation. The client may be shut down if the `op` + /// results in the justified checkpoint being invalidated. /// - /// If `latest_valid_hash` is `None` or references a block unknown to fork choice, no other - /// blocks will be invalidated. If `latest_valid_hash` is a block known to fork choice, all - /// blocks between the `latest_root` and the `latest_valid_hash` will be invalidated (which may - /// cause further, second-order invalidations). - /// - /// ## Notes - /// - /// Use these rules to set `latest_root`: - /// - /// - When `forkchoiceUpdated` indicates an invalid block, set `latest_root` to be the - /// block root that was the head of the chain when `forkchoiceUpdated` was called. - /// - When `executePayload` returns an invalid block *during* block import, set - /// `latest_root` to be the parent of the beacon block containing the invalid - /// payload (because the block containing the payload is not present in fork choice). - /// - When `executePayload` returns an invalid block *after* block import, set - /// `latest_root` to be root of the beacon block containing the invalid payload. + /// See the documentation of `InvalidationOperation` for information about defining `op`. pub fn process_invalid_execution_payload( &self, - latest_root: Hash256, - latest_valid_hash: Option, + op: &InvalidationOperation, ) -> Result<(), Error> { debug!( self.log, "Invalid execution payload in block"; - "latest_valid_hash" => ?latest_valid_hash, - "latest_root" => ?latest_root, + "latest_valid_ancestor" => ?op.latest_valid_ancestor(), + "block_root" => ?op.block_root(), ); // Update fork choice. - if let Err(e) = self - .fork_choice - .write() - .on_invalid_execution_payload(latest_root, latest_valid_hash) - { + if let Err(e) = self.fork_choice.write().on_invalid_execution_payload(op) { crit!( self.log, "Failed to process invalid payload"; "error" => ?e, - "latest_valid_hash" => ?latest_valid_hash, - "latest_root" => ?latest_root, + "latest_valid_ancestor" => ?op.latest_valid_ancestor(), + "block_root" => ?op.block_root(), ); } @@ -3763,8 +3743,11 @@ impl BeaconChain { // The execution engine has stated that all blocks between the // `head_execution_block_hash` and `latest_valid_hash` are invalid. self.process_invalid_execution_payload( - head_block_root, - Some(*latest_valid_hash), + &InvalidationOperation::InvalidateMany { + head_block_root, + always_invalidate_head: true, + latest_valid_ancestor: *latest_valid_hash, + }, )?; Err(BeaconChainError::ExecutionForkChoiceUpdateInvalid { status }) @@ -3781,7 +3764,11 @@ impl BeaconChain { // // Using a `None` latest valid ancestor will result in only the head block // being invalidated (no ancestors). - self.process_invalid_execution_payload(head_block_root, None)?; + self.process_invalid_execution_payload( + &InvalidationOperation::InvalidateOne { + block_root: head_block_root, + }, + )?; Err(BeaconChainError::ExecutionForkChoiceUpdateInvalid { status }) } diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index a36154619f7..0ee9e4b8760 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -12,7 +12,7 @@ use crate::{ ExecutionPayloadError, }; use execution_layer::PayloadStatus; -use fork_choice::PayloadVerificationStatus; +use fork_choice::{InvalidationOperation, PayloadVerificationStatus}; use proto_array::{Block as ProtoBlock, ExecutionStatus}; use slog::debug; use slot_clock::SlotClock; @@ -68,7 +68,13 @@ pub fn notify_new_payload( // This block has not yet been applied to fork choice, so the latest block that was // imported to fork choice was the parent. let latest_root = block.parent_root(); - chain.process_invalid_execution_payload(latest_root, Some(latest_valid_hash))?; + chain.process_invalid_execution_payload( + &InvalidationOperation::InvalidateMany { + head_block_root: latest_root, + always_invalidate_head: false, + latest_valid_ancestor: latest_valid_hash, + }, + )?; Err(ExecutionPayloadError::RejectedByExecutionEngine { status }.into()) } @@ -145,11 +151,19 @@ pub fn validate_merge_block( .slot_clock .now() .ok_or(BeaconChainError::UnableToReadSlot)?; - // Check the optimistic sync conditions. Note that because this is the merge block, - // the justified checkpoint can't have execution enabled so we only need to check the - // current slot is at least SAFE_SLOTS_TO_IMPORT_OPTIMISTICALLY ahead of the block - // https://github.com/ethereum/consensus-specs/blob/v1.1.9/sync/optimistic.md#when-to-optimistically-import-blocks - if block.slot() + chain.spec.safe_slots_to_import_optimistically <= current_slot { + + // Ensure the block is a candidate for optimistic import. + if chain + .fork_choice + .read() + .is_optimistic_candidate_block( + current_slot, + block.slot(), + &block.parent_root(), + &chain.spec, + ) + .map_err(BeaconChainError::from)? + { debug!( chain.log, "Optimistically accepting terminal block"; diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 198f6741570..6d3ffff1944 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -231,8 +231,10 @@ fn valid_invalid_syncing() { /// `latest_valid_hash`. #[test] fn invalid_payload_invalidates_parent() { - let mut rig = InvalidPayloadRig::new(); + let mut rig = InvalidPayloadRig::new().enable_attestations(); rig.move_to_terminal_block(); + rig.import_block(Payload::Valid); // Import a valid transition block. + rig.move_to_first_justification(Payload::Syncing); let roots = vec![ rig.import_block(Payload::Syncing), @@ -258,6 +260,7 @@ fn invalid_payload_invalidates_parent() { fn justified_checkpoint_becomes_invalid() { let mut rig = InvalidPayloadRig::new().enable_attestations(); rig.move_to_terminal_block(); + rig.import_block(Payload::Valid); // Import a valid transition block. rig.move_to_first_justification(Payload::Syncing); let justified_checkpoint = rig.head_info().current_justified_checkpoint; @@ -305,7 +308,9 @@ fn pre_finalized_latest_valid_hash() { let mut rig = InvalidPayloadRig::new().enable_attestations(); rig.move_to_terminal_block(); - let blocks = rig.build_blocks(num_blocks, Payload::Syncing); + let mut blocks = vec![]; + blocks.push(rig.import_block(Payload::Valid)); // Import a valid transition block. + blocks.extend(rig.build_blocks(num_blocks - 1, Payload::Syncing)); assert_eq!(rig.head_info().finalized_checkpoint.epoch, finalized_epoch); @@ -330,7 +335,11 @@ fn pre_finalized_latest_valid_hash() { for i in E::slots_per_epoch() * finalized_epoch..num_blocks { let slot = Slot::new(i); let root = rig.block_root_at_slot(slot).unwrap(); - assert!(rig.execution_status(root).is_not_verified()); + if slot == 1 { + assert!(rig.execution_status(root).is_valid()); + } else { + assert!(rig.execution_status(root).is_not_verified()); + } } } @@ -344,7 +353,10 @@ fn latest_valid_hash_will_validate() { let mut rig = InvalidPayloadRig::new().enable_attestations(); rig.move_to_terminal_block(); - let blocks = rig.build_blocks(4, Payload::Syncing); + + let mut blocks = vec![]; + blocks.push(rig.import_block(Payload::Valid)); // Import a valid transition block. + blocks.extend(rig.build_blocks(4, Payload::Syncing)); let latest_valid_root = rig .block_root_at_slot(Slot::new(LATEST_VALID_SLOT)) @@ -357,7 +369,7 @@ fn latest_valid_hash_will_validate() { assert_eq!(rig.head_info().slot, LATEST_VALID_SLOT); - for slot in 0..=4 { + for slot in 0..=5 { let slot = Slot::new(slot); let root = if slot > 0 { // If not the genesis slot, check the blocks we just produced. @@ -386,7 +398,9 @@ fn latest_valid_hash_is_junk() { let mut rig = InvalidPayloadRig::new().enable_attestations(); rig.move_to_terminal_block(); - let blocks = rig.build_blocks(num_blocks, Payload::Syncing); + let mut blocks = vec![]; + blocks.push(rig.import_block(Payload::Valid)); // Import a valid transition block. + blocks.extend(rig.build_blocks(num_blocks, Payload::Syncing)); assert_eq!(rig.head_info().finalized_checkpoint.epoch, finalized_epoch); @@ -408,7 +422,11 @@ fn latest_valid_hash_is_junk() { for i in E::slots_per_epoch() * finalized_epoch..num_blocks { let slot = Slot::new(i); let root = rig.block_root_at_slot(slot).unwrap(); - assert!(rig.execution_status(root).is_not_verified()); + if slot == 1 { + assert!(rig.execution_status(root).is_valid()); + } else { + assert!(rig.execution_status(root).is_not_verified()); + } } } @@ -421,6 +439,7 @@ fn invalidates_all_descendants() { let mut rig = InvalidPayloadRig::new().enable_attestations(); rig.move_to_terminal_block(); + rig.import_block(Payload::Valid); // Import a valid transition block. let blocks = rig.build_blocks(num_blocks, Payload::Syncing); assert_eq!(rig.head_info().finalized_checkpoint.epoch, finalized_epoch); @@ -493,6 +512,7 @@ fn switches_heads() { let mut rig = InvalidPayloadRig::new().enable_attestations(); rig.move_to_terminal_block(); + rig.import_block(Payload::Valid); // Import a valid transition block. let blocks = rig.build_blocks(num_blocks, Payload::Syncing); assert_eq!(rig.head_info().finalized_checkpoint.epoch, finalized_epoch); @@ -571,8 +591,9 @@ fn invalid_during_processing() { #[test] fn invalid_after_optimistic_sync() { - let mut rig = InvalidPayloadRig::new(); + let mut rig = InvalidPayloadRig::new().enable_attestations(); rig.move_to_terminal_block(); + rig.import_block(Payload::Valid); // Import a valid transition block. let mut roots = vec![ rig.import_block(Payload::Syncing), diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 605679dd7e7..c178c0d5c62 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -110,7 +110,7 @@ pub struct ExecutionBlock { #[derive(Clone, Copy, Debug)] pub struct PayloadAttributes { pub timestamp: u64, - pub random: Hash256, + pub prev_randao: Hash256, pub suggested_fee_recipient: Address, } diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index 8d82b8d311b..4fa5e80a78b 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -288,7 +288,7 @@ mod test { "stateRoot": HASH_01, "receiptsRoot": HASH_00, "logsBloom": LOGS_BLOOM_01, - "random": HASH_01, + "prevRandao": HASH_01, "blockNumber": "0x0", "gasLimit": "0x1", "gasUsed": "0x2", @@ -441,7 +441,7 @@ mod test { }, Some(PayloadAttributes { timestamp: 5, - random: Hash256::zero(), + prev_randao: Hash256::zero(), suggested_fee_recipient: Address::repeat_byte(0), }), ) @@ -458,7 +458,7 @@ mod test { }, { "timestamp":"0x5", - "random": HASH_00, + "prevRandao": HASH_00, "suggestedFeeRecipient": ADDRESS_00 }] }), @@ -495,7 +495,7 @@ mod test { state_root: Hash256::repeat_byte(1), receipts_root: Hash256::repeat_byte(0), logs_bloom: vec![1; 256].into(), - random: Hash256::repeat_byte(1), + prev_randao: Hash256::repeat_byte(1), block_number: 0, gas_limit: 1, gas_used: 2, @@ -517,7 +517,7 @@ mod test { "stateRoot": HASH_01, "receiptsRoot": HASH_00, "logsBloom": LOGS_BLOOM_01, - "random": HASH_01, + "prevRandao": HASH_01, "blockNumber": "0x0", "gasLimit": "0x1", "gasUsed": "0x2", @@ -596,7 +596,7 @@ mod test { }, Some(PayloadAttributes { timestamp: 5, - random: Hash256::zero(), + prev_randao: Hash256::zero(), suggested_fee_recipient: Address::from_str("0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b").unwrap(), }) ) @@ -613,7 +613,7 @@ mod test { }, { "timestamp":"0x5", - "random": HASH_00, + "prevRandao": HASH_00, "suggestedFeeRecipient":"0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b" }] }) @@ -643,7 +643,7 @@ mod test { }, Some(PayloadAttributes { timestamp: 5, - random: Hash256::zero(), + prev_randao: Hash256::zero(), suggested_fee_recipient: Address::from_str("0xa94f5374fce5edbc8e2a8697c15331677e6ebf0b").unwrap(), }) ) @@ -687,7 +687,7 @@ mod test { "stateRoot":"0xca3149fa9e37db08d1cd49c9061db1002ef1cd58db2210f2115c8c989b2bdf45", "receiptsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", "logsBloom": LOGS_BLOOM_00, - "random": HASH_00, + "prevRandao": HASH_00, "blockNumber":"0x1", "gasLimit":"0x1c95111", "gasUsed":"0x0", @@ -710,7 +710,7 @@ mod test { state_root: Hash256::from_str("0xca3149fa9e37db08d1cd49c9061db1002ef1cd58db2210f2115c8c989b2bdf45").unwrap(), receipts_root: Hash256::from_str("0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421").unwrap(), logs_bloom: vec![0; 256].into(), - random: Hash256::zero(), + prev_randao: Hash256::zero(), block_number: 1, gas_limit: u64::from_str_radix("1c95111",16).unwrap(), gas_used: 0, @@ -735,7 +735,7 @@ mod test { state_root: Hash256::from_str("0xca3149fa9e37db08d1cd49c9061db1002ef1cd58db2210f2115c8c989b2bdf45").unwrap(), receipts_root: Hash256::from_str("0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421").unwrap(), logs_bloom: vec![0; 256].into(), - random: Hash256::zero(), + prev_randao: Hash256::zero(), block_number: 1, gas_limit: u64::from_str_radix("1c9c380",16).unwrap(), gas_used: 0, @@ -757,7 +757,7 @@ mod test { "stateRoot":"0xca3149fa9e37db08d1cd49c9061db1002ef1cd58db2210f2115c8c989b2bdf45", "receiptsRoot":"0x56e81f171bcc55a6ff8345e692c0f86e5b48e01b996cadc001622fb5e363b421", "logsBloom": LOGS_BLOOM_00, - "random": HASH_00, + "prevRandao": HASH_00, "blockNumber":"0x1", "gasLimit":"0x1c9c380", "gasUsed":"0x0", diff --git a/beacon_node/execution_layer/src/engine_api/json_structures.rs b/beacon_node/execution_layer/src/engine_api/json_structures.rs index e9559e894cc..8febe451d33 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -64,7 +64,7 @@ pub struct JsonExecutionPayloadV1 { pub receipts_root: Hash256, #[serde(with = "serde_logs_bloom")] pub logs_bloom: FixedVector, - pub random: Hash256, + pub prev_randao: Hash256, #[serde(with = "eth2_serde_utils::u64_hex_be")] pub block_number: u64, #[serde(with = "eth2_serde_utils::u64_hex_be")] @@ -91,7 +91,7 @@ impl From> for JsonExecutionPayloadV1 { state_root, receipts_root, logs_bloom, - random, + prev_randao, block_number, gas_limit, gas_used, @@ -108,7 +108,7 @@ impl From> for JsonExecutionPayloadV1 { state_root, receipts_root, logs_bloom, - random, + prev_randao, block_number, gas_limit, gas_used, @@ -130,7 +130,7 @@ impl From> for ExecutionPayload { state_root, receipts_root, logs_bloom, - random, + prev_randao, block_number, gas_limit, gas_used, @@ -147,7 +147,7 @@ impl From> for ExecutionPayload { state_root, receipts_root, logs_bloom, - random, + prev_randao, block_number, gas_limit, gas_used, @@ -165,7 +165,7 @@ impl From> for ExecutionPayload { pub struct JsonPayloadAttributesV1 { #[serde(with = "eth2_serde_utils::u64_hex_be")] pub timestamp: u64, - pub random: Hash256, + pub prev_randao: Hash256, pub suggested_fee_recipient: Address, } @@ -174,13 +174,13 @@ impl From for JsonPayloadAttributesV1 { // Use this verbose deconstruction pattern to ensure no field is left unused. let PayloadAttributes { timestamp, - random, + prev_randao, suggested_fee_recipient, } = p; Self { timestamp, - random, + prev_randao, suggested_fee_recipient, } } @@ -191,13 +191,13 @@ impl From for PayloadAttributes { // Use this verbose deconstruction pattern to ensure no field is left unused. let JsonPayloadAttributesV1 { timestamp, - random, + prev_randao, suggested_fee_recipient, } = j; Self { timestamp, - random, + prev_randao, suggested_fee_recipient, } } diff --git a/beacon_node/execution_layer/src/engines.rs b/beacon_node/execution_layer/src/engines.rs index d8e19baae13..2773ac79f8c 100644 --- a/beacon_node/execution_layer/src/engines.rs +++ b/beacon_node/execution_layer/src/engines.rs @@ -50,7 +50,7 @@ impl Logging { struct PayloadIdCacheKey { pub head_block_hash: ExecutionBlockHash, pub timestamp: u64, - pub random: Hash256, + pub prev_randao: Hash256, pub suggested_fee_recipient: Address, } @@ -77,7 +77,7 @@ impl Engine { &self, head_block_hash: ExecutionBlockHash, timestamp: u64, - random: Hash256, + prev_randao: Hash256, suggested_fee_recipient: Address, ) -> Option { self.payload_id_cache @@ -86,7 +86,7 @@ impl Engine { .get(&PayloadIdCacheKey { head_block_hash, timestamp, - random, + prev_randao, suggested_fee_recipient, }) .cloned() @@ -393,7 +393,7 @@ impl PayloadIdCacheKey { Self { head_block_hash: state.head_block_hash, timestamp: attributes.timestamp, - random: attributes.random, + prev_randao: attributes.prev_randao, suggested_fee_recipient: attributes.suggested_fee_recipient, } } diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 326c46f8700..faa67888725 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -392,7 +392,7 @@ impl ExecutionLayer { &self, parent_hash: ExecutionBlockHash, timestamp: u64, - random: Hash256, + prev_randao: Hash256, finalized_block_hash: ExecutionBlockHash, proposer_index: u64, ) -> Result, Error> { @@ -402,14 +402,14 @@ impl ExecutionLayer { self.log(), "Issuing engine_getPayload"; "suggested_fee_recipient" => ?suggested_fee_recipient, - "random" => ?random, + "prev_randao" => ?prev_randao, "timestamp" => timestamp, "parent_hash" => ?parent_hash, ); self.engines() .first_success(|engine| async move { let payload_id = if let Some(id) = engine - .get_payload_id(parent_hash, timestamp, random, suggested_fee_recipient) + .get_payload_id(parent_hash, timestamp, prev_randao, suggested_fee_recipient) .await { // The payload id has been cached for this engine. @@ -428,7 +428,7 @@ impl ExecutionLayer { }; let payload_attributes = PayloadAttributes { timestamp, - random, + prev_randao, suggested_fee_recipient, }; 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 52accad3a1f..b61092cf0e4 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 @@ -326,7 +326,7 @@ impl ExecutionBlockGenerator { receipts_root: Hash256::repeat_byte(42), state_root: Hash256::repeat_byte(43), logs_bloom: vec![0; 256].into(), - random: attributes.random, + prev_randao: attributes.prev_randao, block_number: parent.block_number() + 1, gas_limit: GAS_LIMIT, gas_used: GAS_USED, 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 index a15ab25254b..db024ba8b5d 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs @@ -111,7 +111,7 @@ impl MockExecutionLayer { 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 prev_randao = Hash256::from_low_u64_be(block_number); let finalized_block_hash = parent_hash; self.el @@ -120,7 +120,7 @@ impl MockExecutionLayer { ExecutionBlockHash::zero(), Some(PayloadAttributes { timestamp, - random, + prev_randao, suggested_fee_recipient: Address::repeat_byte(42), }), ) @@ -133,7 +133,7 @@ impl MockExecutionLayer { .get_payload::( parent_hash, timestamp, - random, + prev_randao, finalized_block_hash, validator_index, ) @@ -143,7 +143,7 @@ impl MockExecutionLayer { assert_eq!(payload.parent_hash, parent_hash); assert_eq!(payload.block_number, block_number); assert_eq!(payload.timestamp, timestamp); - assert_eq!(payload.random, random); + assert_eq!(payload.prev_randao, prev_randao); let status = self.el.notify_new_payload(&payload).await.unwrap(); assert_eq!(status, PayloadStatus::Valid); diff --git a/bors.toml b/bors.toml index 3c6826a9e88..f63901ef4ba 100644 --- a/bors.toml +++ b/bors.toml @@ -13,7 +13,12 @@ status = [ "clippy", "arbitrary-check", "cargo-audit", - "cargo-udeps" + "cargo-udeps", + "beacon-chain-tests", + "op-pool-tests", + "doppelganger-protection-test", + "execution-engine-integration-ubuntu", + "cargo-vendor" ] use_squash_merge = true timeout_sec = 10800 diff --git a/consensus/fork_choice/src/fork_choice.rs b/consensus/fork_choice/src/fork_choice.rs index cf3e806034e..b7fd67ecab0 100644 --- a/consensus/fork_choice/src/fork_choice.rs +++ b/consensus/fork_choice/src/fork_choice.rs @@ -1,4 +1,4 @@ -use crate::ForkChoiceStore; +use crate::{ForkChoiceStore, InvalidationOperation}; use proto_array::{Block as ProtoBlock, ExecutionStatus, ProtoArrayForkChoice}; use ssz_derive::{Decode, Encode}; use std::cmp::Ordering; @@ -480,11 +480,10 @@ where /// See `ProtoArrayForkChoice::process_execution_payload_invalidation` for documentation. pub fn on_invalid_execution_payload( &mut self, - head_block_root: Hash256, - latest_valid_ancestor_root: Option, + op: &InvalidationOperation, ) -> Result<(), Error> { self.proto_array - .process_execution_payload_invalidation(head_block_root, latest_valid_ancestor_root) + .process_execution_payload_invalidation(op) .map_err(Error::FailedToProcessInvalidExecutionPayload) } @@ -956,19 +955,19 @@ where return Ok(true); } - // If the block has an ancestor with a verified parent, import this block. + // If the parent block has execution enabled, always import the block. // - // TODO: This condition is not yet merged into the spec. See: + // TODO(bellatrix): this condition has not yet been merged into the spec. // - // https://github.com/ethereum/consensus-specs/pull/2841 + // See: // - // ## Note - // - // If `block_parent_root` is unknown this iter will always return `None`. + // https://github.com/ethereum/consensus-specs/pull/2844 if self .proto_array - .iter_nodes(block_parent_root) - .any(|node| node.execution_status.is_valid()) + .get_block(block_parent_root) + .map_or(false, |parent| { + parent.execution_status.is_execution_enabled() + }) { return Ok(true); } diff --git a/consensus/fork_choice/src/lib.rs b/consensus/fork_choice/src/lib.rs index ba031cdf7fe..d4a95994e0d 100644 --- a/consensus/fork_choice/src/lib.rs +++ b/consensus/fork_choice/src/lib.rs @@ -6,4 +6,4 @@ pub use crate::fork_choice::{ PayloadVerificationStatus, PersistedForkChoice, QueuedAttestation, }; pub use fork_choice_store::ForkChoiceStore; -pub use proto_array::Block as ProtoBlock; +pub use proto_array::{Block as ProtoBlock, InvalidationOperation}; diff --git a/consensus/proto_array/src/fork_choice_test_definition.rs b/consensus/proto_array/src/fork_choice_test_definition.rs index fd90d539037..f2b51c1fd40 100644 --- a/consensus/proto_array/src/fork_choice_test_definition.rs +++ b/consensus/proto_array/src/fork_choice_test_definition.rs @@ -4,6 +4,7 @@ mod no_votes; mod votes; use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkChoice}; +use crate::InvalidationOperation; use serde_derive::{Deserialize, Serialize}; use types::{ AttestationShufflingId, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256, @@ -238,12 +239,22 @@ impl ForkChoiceTestDefinition { Operation::InvalidatePayload { head_block_root, latest_valid_ancestor_root, - } => fork_choice - .process_execution_payload_invalidation( - head_block_root, - latest_valid_ancestor_root, - ) - .unwrap(), + } => { + let op = if let Some(latest_valid_ancestor) = latest_valid_ancestor_root { + InvalidationOperation::InvalidateMany { + head_block_root, + always_invalidate_head: true, + latest_valid_ancestor, + } + } else { + InvalidationOperation::InvalidateOne { + block_root: head_block_root, + } + }; + fork_choice + .process_execution_payload_invalidation(&op) + .unwrap() + } Operation::AssertWeight { block_root, weight } => assert_eq!( fork_choice.get_weight(&block_root).unwrap(), weight, diff --git a/consensus/proto_array/src/lib.rs b/consensus/proto_array/src/lib.rs index 216d189fb2a..d6f614b7c39 100644 --- a/consensus/proto_array/src/lib.rs +++ b/consensus/proto_array/src/lib.rs @@ -4,6 +4,7 @@ mod proto_array; mod proto_array_fork_choice; mod ssz_container; +pub use crate::proto_array::InvalidationOperation; pub use crate::proto_array_fork_choice::{Block, ExecutionStatus, ProtoArrayForkChoice}; pub use error::Error; diff --git a/consensus/proto_array/src/proto_array.rs b/consensus/proto_array/src/proto_array.rs index b0e8991a785..fb086a96e9c 100644 --- a/consensus/proto_array/src/proto_array.rs +++ b/consensus/proto_array/src/proto_array.rs @@ -15,6 +15,56 @@ use types::{ four_byte_option_impl!(four_byte_option_usize, usize); four_byte_option_impl!(four_byte_option_checkpoint, Checkpoint); +/// Defines an operation which may invalidate the `execution_status` of some nodes. +pub enum InvalidationOperation { + /// Invalidate only `block_root` and it's descendants. Don't invalidate any ancestors. + InvalidateOne { block_root: Hash256 }, + /// Invalidate blocks between `head_block_root` and `latest_valid_ancestor`. + /// + /// If the `latest_valid_ancestor` is known to fork choice, invalidate all blocks between + /// `head_block_root` and `latest_valid_ancestor`. The `head_block_root` will be invalidated, + /// whilst the `latest_valid_ancestor` will not. + /// + /// If `latest_valid_ancestor` is *not* known to fork choice, only invalidate the + /// `head_block_root` if `always_invalidate_head == true`. + InvalidateMany { + head_block_root: Hash256, + always_invalidate_head: bool, + latest_valid_ancestor: ExecutionBlockHash, + }, +} + +impl InvalidationOperation { + pub fn block_root(&self) -> Hash256 { + match self { + InvalidationOperation::InvalidateOne { block_root } => *block_root, + InvalidationOperation::InvalidateMany { + head_block_root, .. + } => *head_block_root, + } + } + + pub fn latest_valid_ancestor(&self) -> Option { + match self { + InvalidationOperation::InvalidateOne { .. } => None, + InvalidationOperation::InvalidateMany { + latest_valid_ancestor, + .. + } => Some(*latest_valid_ancestor), + } + } + + pub fn invalidate_block_root(&self) -> bool { + match self { + InvalidationOperation::InvalidateOne { .. } => true, + InvalidationOperation::InvalidateMany { + always_invalidate_head, + .. + } => *always_invalidate_head, + } + } +} + #[derive(Clone, PartialEq, Debug, Encode, Decode, Serialize, Deserialize)] pub struct ProtoNode { /// The `slot` is not necessary for `ProtoArray`, it just exists so external components can @@ -328,43 +378,15 @@ impl ProtoArray { } } - /// Invalidate the relevant ancestors and descendants of a block with an invalid execution - /// payload. - /// - /// The `head_block_root` should be the beacon block root of the block with the invalid - /// execution payload, _or_ its parent where the block with the invalid payload has not yet - /// been applied to `self`. - /// - /// The `latest_valid_hash` should be the hash of most recent *valid* execution payload - /// contained in an ancestor block of `head_block_root`. + /// Invalidate zero or more blocks, as specified by the `InvalidationOperation`. /// - /// This function will invalidate: - /// - /// * The block matching `head_block_root` _unless_ that block has a payload matching `latest_valid_hash`. - /// * All ancestors of `head_block_root` back to the block with payload matching - /// `latest_valid_hash` (endpoint > exclusive). In the case where the `head_block_root` is the parent - /// of the invalid block and itself matches `latest_valid_hash`, no ancestors will be invalidated. - /// * All descendants of `latest_valid_hash` if supplied and consistent with `head_block_root`, - /// or else all descendants of `head_block_root`. - /// - /// ## Details - /// - /// If `head_block_root` is not known to fork choice, an error is returned. - /// - /// If `latest_valid_hash` is `Some(hash)` where `hash` is either not known to fork choice - /// (perhaps it's junk or pre-finalization), then only the `head_block_root` block will be - /// invalidated (no ancestors). No error will be returned in this case. - /// - /// If `latest_valid_hash` is `Some(hash)` where `hash` is a known ancestor of - /// `head_block_root`, then all blocks between `head_block_root` and `latest_valid_hash` will - /// be invalidated. Additionally, all blocks that descend from a newly-invalidated block will - /// also be invalidated. + /// See the documentation of `InvalidationOperation` for usage. pub fn propagate_execution_payload_invalidation( &mut self, - head_block_root: Hash256, - latest_valid_ancestor_hash: Option, + op: &InvalidationOperation, ) -> Result<(), Error> { let mut invalidated_indices: HashSet = <_>::default(); + let head_block_root = op.block_root(); /* * Step 1: @@ -379,7 +401,8 @@ impl ProtoArray { .ok_or(Error::NodeUnknown(head_block_root))?; // Try to map the ancestor payload *hash* to an ancestor beacon block *root*. - let latest_valid_ancestor_root = latest_valid_ancestor_hash + let latest_valid_ancestor_root = op + .latest_valid_ancestor() .and_then(|hash| self.execution_block_hash_to_beacon_block_root(&hash)); // Set to `true` if both conditions are satisfied: @@ -414,7 +437,7 @@ impl ProtoArray { // an invalid justified checkpoint. if !latest_valid_ancestor_is_descendant && node.root != head_block_root { break; - } else if Some(hash) == latest_valid_ancestor_hash { + } else if op.latest_valid_ancestor() == Some(hash) { // If the `best_child` or `best_descendant` of the latest valid hash was // invalidated, set those fields to `None`. // @@ -444,36 +467,44 @@ impl ProtoArray { ExecutionStatus::Irrelevant(_) => break, } - match &node.execution_status { - // It's illegal for an execution client to declare that some previously-valid block - // is now invalid. This is a consensus failure on their behalf. - ExecutionStatus::Valid(hash) => { - return Err(Error::ValidExecutionStatusBecameInvalid { - block_root: node.root, - payload_block_hash: *hash, - }) - } - ExecutionStatus::Unknown(hash) => { - node.execution_status = ExecutionStatus::Invalid(*hash); + // Only invalidate the head block if either: + // + // - The head block was specifically indicated to be invalidated. + // - The latest valid hash is a known ancestor. + if node.root != head_block_root + || op.invalidate_block_root() + || latest_valid_ancestor_is_descendant + { + match &node.execution_status { + // It's illegal for an execution client to declare that some previously-valid block + // is now invalid. This is a consensus failure on their behalf. + ExecutionStatus::Valid(hash) => { + return Err(Error::ValidExecutionStatusBecameInvalid { + block_root: node.root, + payload_block_hash: *hash, + }) + } + ExecutionStatus::Unknown(hash) => { + invalidated_indices.insert(index); + node.execution_status = ExecutionStatus::Invalid(*hash); - // It's impossible for an invalid block to lead to a "best" block, so set these - // fields to `None`. - // - // Failing to set these values will result in `Self::node_leads_to_viable_head` - // returning `false` for *valid* ancestors of invalid blocks. - node.best_child = None; - node.best_descendant = None; + // It's impossible for an invalid block to lead to a "best" block, so set these + // fields to `None`. + // + // Failing to set these values will result in `Self::node_leads_to_viable_head` + // returning `false` for *valid* ancestors of invalid blocks. + node.best_child = None; + node.best_descendant = None; + } + // The block is already invalid, but keep going backwards to ensure all ancestors + // are updated. + ExecutionStatus::Invalid(_) => (), + // This block is pre-merge, therefore it has no execution status. Nor do its + // ancestors. + ExecutionStatus::Irrelevant(_) => break, } - // The block is already invalid, but keep going backwards to ensure all ancestors - // are updated. - ExecutionStatus::Invalid(_) => (), - // This block is pre-merge, therefore it has no execution status. Nor do its - // ancestors. - ExecutionStatus::Irrelevant(_) => break, } - invalidated_indices.insert(index); - if let Some(parent_index) = node.parent { index = parent_index } else { diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs index 2c1341be9e5..007f262fddd 100644 --- a/consensus/proto_array/src/proto_array_fork_choice.rs +++ b/consensus/proto_array/src/proto_array_fork_choice.rs @@ -1,5 +1,5 @@ use crate::error::Error; -use crate::proto_array::{Iter, ProposerBoost, ProtoArray}; +use crate::proto_array::{InvalidationOperation, Iter, ProposerBoost, ProtoArray}; use crate::ssz_container::SszContainer; use serde_derive::{Deserialize, Serialize}; use ssz::{Decode, Encode}; @@ -191,11 +191,10 @@ impl ProtoArrayForkChoice { /// See `ProtoArray::propagate_execution_payload_invalidation` for documentation. pub fn process_execution_payload_invalidation( &mut self, - head_block_root: Hash256, - latest_valid_ancestor_root: Option, + op: &InvalidationOperation, ) -> Result<(), String> { self.proto_array - .propagate_execution_payload_invalidation(head_block_root, latest_valid_ancestor_root) + .propagate_execution_payload_invalidation(op) .map_err(|e| format!("Failed to process invalid payload: {:?}", e)) } diff --git a/consensus/state_processing/src/per_block_processing.rs b/consensus/state_processing/src/per_block_processing.rs index a874ce64284..f87756c122e 100644 --- a/consensus/state_processing/src/per_block_processing.rs +++ b/consensus/state_processing/src/per_block_processing.rs @@ -329,10 +329,10 @@ pub fn partially_verify_execution_payload( ); } block_verify!( - payload.random == *state.get_randao_mix(state.current_epoch())?, + payload.prev_randao == *state.get_randao_mix(state.current_epoch())?, BlockProcessingError::ExecutionRandaoMismatch { expected: *state.get_randao_mix(state.current_epoch())?, - found: payload.random, + found: payload.prev_randao, } ); @@ -368,7 +368,7 @@ pub fn process_execution_payload( state_root: payload.state_root, receipts_root: payload.receipts_root, logs_bloom: payload.logs_bloom.clone(), - random: payload.random, + prev_randao: payload.prev_randao, block_number: payload.block_number, gas_limit: payload.gas_limit, gas_used: payload.gas_used, diff --git a/consensus/types/src/execution_payload.rs b/consensus/types/src/execution_payload.rs index fc37c1193bf..ab5e6b5aed3 100644 --- a/consensus/types/src/execution_payload.rs +++ b/consensus/types/src/execution_payload.rs @@ -21,7 +21,7 @@ pub struct ExecutionPayload { pub receipts_root: Hash256, #[serde(with = "ssz_types::serde_utils::hex_fixed_vec")] pub logs_bloom: FixedVector, - pub random: Hash256, + pub prev_randao: Hash256, #[serde(with = "eth2_serde_utils::quoted_u64")] pub block_number: u64, #[serde(with = "eth2_serde_utils::quoted_u64")] diff --git a/consensus/types/src/execution_payload_header.rs b/consensus/types/src/execution_payload_header.rs index 1c173093a42..24390bcf4cb 100644 --- a/consensus/types/src/execution_payload_header.rs +++ b/consensus/types/src/execution_payload_header.rs @@ -15,7 +15,7 @@ pub struct ExecutionPayloadHeader { pub receipts_root: Hash256, #[serde(with = "ssz_types::serde_utils::hex_fixed_vec")] pub logs_bloom: FixedVector, - pub random: Hash256, + pub prev_randao: Hash256, #[serde(with = "eth2_serde_utils::quoted_u64")] pub block_number: u64, #[serde(with = "eth2_serde_utils::quoted_u64")] diff --git a/lcli/src/create_payload_header.rs b/lcli/src/create_payload_header.rs index 04122d0e6b1..9e91f425a73 100644 --- a/lcli/src/create_payload_header.rs +++ b/lcli/src/create_payload_header.rs @@ -23,7 +23,7 @@ pub fn run(matches: &ArgMatches) -> Result<(), String> { base_fee_per_gas, timestamp: genesis_time, block_hash: eth1_block_hash, - random: eth1_block_hash.into_root(), + prev_randao: eth1_block_hash.into_root(), ..ExecutionPayloadHeader::default() }; let mut file = File::create(file_name).map_err(|_| "Unable to create file".to_string())?; diff --git a/testing/ef_tests/Makefile b/testing/ef_tests/Makefile index 816651bb45b..13d8f631cce 100644 --- a/testing/ef_tests/Makefile +++ b/testing/ef_tests/Makefile @@ -1,4 +1,4 @@ -TESTS_TAG := v1.1.9 +TESTS_TAG := v1.1.10 TESTS = general minimal mainnet TARBALLS = $(patsubst %,%-$(TESTS_TAG).tar.gz,$(TESTS)) diff --git a/testing/execution_engine_integration/Cargo.toml b/testing/execution_engine_integration/Cargo.toml index cd9836dd6cd..fc8230c7a25 100644 --- a/testing/execution_engine_integration/Cargo.toml +++ b/testing/execution_engine_integration/Cargo.toml @@ -3,8 +3,6 @@ name = "execution_engine_integration" version = "0.1.0" edition = "2021" -build = "build.rs" - [dependencies] tempfile = "3.1.0" serde_json = "1.0.58" diff --git a/testing/execution_engine_integration/Makefile b/testing/execution_engine_integration/Makefile index 8bb2b592332..70620650666 100644 --- a/testing/execution_engine_integration/Makefile +++ b/testing/execution_engine_integration/Makefile @@ -1,5 +1,5 @@ test: - cargo test --release --locked + cargo run --release --locked clean: rm -rf execution_clients diff --git a/testing/execution_engine_integration/build.rs b/testing/execution_engine_integration/src/build_geth.rs similarity index 81% rename from testing/execution_engine_integration/build.rs rename to testing/execution_engine_integration/src/build_geth.rs index bedf74fbd15..772d3e3d85f 100644 --- a/testing/execution_engine_integration/build.rs +++ b/testing/execution_engine_integration/src/build_geth.rs @@ -3,10 +3,10 @@ use std::fs; use std::path::{Path, PathBuf}; use std::process::Command; -const GETH_BRANCH: &str = "merge-kiln"; +const GETH_BRANCH: &str = "merge-kiln-v2"; const GETH_REPO_URL: &str = "https://github.com/MariusVanDerWijden/go-ethereum"; -fn main() { +pub fn build() { let manifest_dir: PathBuf = env::var("CARGO_MANIFEST_DIR").unwrap().into(); let execution_clients_dir = manifest_dir.join("execution_clients"); @@ -52,11 +52,15 @@ fn build_geth(execution_clients_dir: &Path) { .success()); // Build geth - assert!(Command::new("make") + let make_result = Command::new("make") .arg("geth") .current_dir(&repo_dir) .output() - .expect("failed to make geth") - .status - .success()); + .expect("failed to make geth"); + + if !make_result.status.success() { + dbg!(String::from_utf8_lossy(&make_result.stdout)); + dbg!(String::from_utf8_lossy(&make_result.stderr)); + panic!("make failed"); + } } diff --git a/testing/execution_engine_integration/src/execution_engine.rs b/testing/execution_engine_integration/src/execution_engine.rs index cff36a025bd..84d72100848 100644 --- a/testing/execution_engine_integration/src/execution_engine.rs +++ b/testing/execution_engine_integration/src/execution_engine.rs @@ -9,7 +9,7 @@ use unused_port::unused_tcp_port; /// Defined for each EE type (e.g., Geth, Nethermind, etc). pub trait GenericExecutionEngine: Clone { fn init_datadir() -> TempDir; - fn start_client(datadir: &TempDir, http_port: u16) -> Child; + fn start_client(datadir: &TempDir, http_port: u16, http_auth_port: u16) -> Child; } /// Holds handle to a running EE process, plus some other metadata. @@ -19,6 +19,7 @@ pub struct ExecutionEngine { #[allow(dead_code)] datadir: TempDir, http_port: u16, + http_auth_port: u16, child: Child, } @@ -35,11 +36,13 @@ impl ExecutionEngine { pub fn new(engine: E) -> Self { let datadir = E::init_datadir(); let http_port = unused_tcp_port().unwrap(); - let child = E::start_client(&datadir, http_port); + let http_auth_port = unused_tcp_port().unwrap(); + let child = E::start_client(&datadir, http_port, http_auth_port); Self { engine, datadir, http_port, + http_auth_port, child, } } @@ -47,6 +50,11 @@ impl ExecutionEngine { pub fn http_url(&self) -> SensitiveUrl { SensitiveUrl::parse(&format!("http://127.0.0.1:{}", self.http_port)).unwrap() } + + #[allow(dead_code)] // Future use. + pub fn http_ath_url(&self) -> SensitiveUrl { + SensitiveUrl::parse(&format!("http://127.0.0.1:{}", self.http_auth_port)).unwrap() + } } /* @@ -90,7 +98,7 @@ impl GenericExecutionEngine for Geth { datadir } - fn start_client(datadir: &TempDir, http_port: u16) -> Child { + fn start_client(datadir: &TempDir, http_port: u16, http_auth_port: u16) -> Child { let network_port = unused_tcp_port().unwrap(); Command::new(Self::binary_path()) @@ -101,6 +109,8 @@ impl GenericExecutionEngine for Geth { .arg("engine,eth") .arg("--http.port") .arg(http_port.to_string()) + .arg("--http.authport") + .arg(http_auth_port.to_string()) .arg("--port") .arg(network_port.to_string()) .stdout(build_stdio()) diff --git a/testing/execution_engine_integration/src/lib.rs b/testing/execution_engine_integration/src/lib.rs deleted file mode 100644 index 19a73e6bf29..00000000000 --- a/testing/execution_engine_integration/src/lib.rs +++ /dev/null @@ -1,12 +0,0 @@ -/// This library provides integration testing between Lighthouse and other execution engines. -/// -/// See the `tests/tests.rs` file to run tests. -mod execution_engine; -mod genesis_json; -mod test_rig; - -pub use execution_engine::Geth; -pub use test_rig::TestRig; - -/// Set to `false` to send logs to the console during tests. Logs are useful when debugging. -const SUPPRESS_LOGS: bool = true; diff --git a/testing/execution_engine_integration/src/main.rs b/testing/execution_engine_integration/src/main.rs new file mode 100644 index 00000000000..ef9cbd79483 --- /dev/null +++ b/testing/execution_engine_integration/src/main.rs @@ -0,0 +1,28 @@ +/// 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. +/// +/// A return code of `0` indicates the tests succeeded. +mod build_geth; +mod execution_engine; +mod genesis_json; +mod test_rig; + +use execution_engine::Geth; +use test_rig::TestRig; + +/// Set to `false` to send logs to the console during tests. Logs are useful when debugging. +const SUPPRESS_LOGS: bool = false; + +fn main() { + if cfg!(windows) { + panic!("windows is not supported, only linux"); + } + + test_geth() +} + +fn test_geth() { + build_geth::build(); + TestRig::new(Geth).perform_tests_blocking(); +} diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index 26dbc1bfdd1..3f0e9544245 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -138,7 +138,7 @@ impl TestRig { let parent_hash = terminal_pow_block_hash; let timestamp = timestamp_now(); - let random = Hash256::zero(); + let prev_randao = Hash256::zero(); let finalized_block_hash = ExecutionBlockHash::zero(); let proposer_index = 0; let valid_payload = self @@ -147,7 +147,7 @@ impl TestRig { .get_payload::( parent_hash, timestamp, - random, + prev_randao, finalized_block_hash, proposer_index, ) @@ -210,7 +210,7 @@ impl TestRig { */ let mut invalid_payload = valid_payload.clone(); - invalid_payload.random = Hash256::from_low_u64_be(42); + invalid_payload.prev_randao = Hash256::from_low_u64_be(42); let status = self .ee_a .execution_layer @@ -227,7 +227,7 @@ impl TestRig { let parent_hash = valid_payload.block_hash; let timestamp = valid_payload.timestamp + 1; - let random = Hash256::zero(); + let prev_randao = Hash256::zero(); let finalized_block_hash = ExecutionBlockHash::zero(); let proposer_index = 0; let second_payload = self @@ -236,7 +236,7 @@ impl TestRig { .get_payload::( parent_hash, timestamp, - random, + prev_randao, finalized_block_hash, proposer_index, ) @@ -266,7 +266,7 @@ impl TestRig { let finalized_block_hash = ExecutionBlockHash::zero(); let payload_attributes = Some(PayloadAttributes { timestamp: second_payload.timestamp + 1, - random: Hash256::zero(), + prev_randao: Hash256::zero(), suggested_fee_recipient: Address::zero(), }); let status = self diff --git a/testing/execution_engine_integration/tests/tests.rs b/testing/execution_engine_integration/tests/tests.rs deleted file mode 100644 index d4fcb29dca8..00000000000 --- a/testing/execution_engine_integration/tests/tests.rs +++ /dev/null @@ -1,16 +0,0 @@ -#[cfg(not(target_family = "windows"))] -mod not_windows { - use execution_engine_integration::{Geth, TestRig}; - #[test] - fn geth() { - TestRig::new(Geth).perform_tests_blocking() - } -} - -#[cfg(target_family = "windows")] -mod windows { - #[test] - fn all_tests_skipped_on_windows() { - // - } -}