From 7091adf58c0537ab2d46e2fba495abaa3f248ae3 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 27 Sep 2021 19:28:26 +1000 Subject: [PATCH] Integrate execute_payload --- .../beacon_chain/src/block_verification.rs | 75 +++++++++++++------ beacon_node/client/Cargo.toml | 1 - .../beacon_processor/worker/gossip_methods.rs | 4 +- 3 files changed, 53 insertions(+), 27 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 91ba04d8e1..58877ed20c 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -48,8 +48,9 @@ use crate::{ BLOCK_PROCESSING_CACHE_LOCK_TIMEOUT, MAXIMUM_GOSSIP_CLOCK_DISPARITY, VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT, }, - eth1_chain, metrics, BeaconChain, BeaconChainError, BeaconChainTypes, + metrics, BeaconChain, BeaconChainError, BeaconChainTypes, }; +use execution_layer::ExecutePayloadResponse; use fork_choice::{ForkChoice, ForkChoiceStore}; use parking_lot::RwLockReadGuard; use proto_array::Block as ProtoBlock; @@ -242,19 +243,25 @@ pub enum ExecutionPayloadError { /// ## Peer scoring /// /// As this is our fault, do not penalize the peer - NoEth1Connection, + NoExecutionConnection, /// Error occurred during engine_executePayload /// /// ## Peer scoring /// /// Some issue with our configuration, do not penalize peer - Eth1VerificationError(eth1_chain::Error), + RequestFailed(execution_layer::Error), /// The execution engine returned INVALID for the payload /// /// ## Peer scoring /// /// The block is invalid and the peer is faulty RejectedByExecutionEngine, + /// The execution engine returned SYNCING for the payload + /// + /// ## Peer scoring + /// + /// It is not known if the block is valid or invalid. + ExecutionEngineIsSyncing, /// The execution payload timestamp does not match the slot /// /// ## Peer scoring @@ -281,6 +288,24 @@ pub enum ExecutionPayloadError { TransactionDataExceedsSizeLimit, } +impl From for ExecutionPayloadError { + fn from(e: execution_layer::Error) -> Self { + ExecutionPayloadError::RequestFailed(e) + } +} + +impl From for BlockError { + fn from(e: ExecutionPayloadError) -> Self { + BlockError::ExecutionPayloadError(e) + } +} + +impl From for BlockError { + fn from(e: InconsistentFork) -> Self { + BlockError::InconsistentFork(e) + } +} + impl std::fmt::Display for BlockError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { match self { @@ -1056,31 +1081,33 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> { // This is the soonest we can run these checks as they must be called AFTER per_slot_processing if is_execution_enabled(&state, block.message().body()) { - let eth1_chain = chain - .eth1_chain + let execution_layer = chain + .execution_layer .as_ref() - .ok_or(BlockError::ExecutionPayloadError( - ExecutionPayloadError::NoEth1Connection, - ))?; - - let payload_valid = eth1_chain - .on_payload(block.message().body().execution_payload().ok_or_else(|| { - BlockError::InconsistentFork(InconsistentFork { + .ok_or(ExecutionPayloadError::NoExecutionConnection)?; + let execution_payload = + block + .message() + .body() + .execution_payload() + .ok_or_else(|| InconsistentFork { fork_at_slot: eth2::types::ForkName::Merge, object_fork: block.message().body().fork_name(), - }) - })?) - .map_err(|e| { - BlockError::ExecutionPayloadError(ExecutionPayloadError::Eth1VerificationError( - e, - )) - })?; + })?; - if !payload_valid { - return Err(BlockError::ExecutionPayloadError( - ExecutionPayloadError::RejectedByExecutionEngine, - )); - } + let payload_status = execution_layer + .block_on(|execution_layer| execution_layer.execute_payload(execution_payload)) + .map_err(ExecutionPayloadError::from)?; + + match payload_status { + ExecutePayloadResponse::Valid => Ok(()), + ExecutePayloadResponse::Invalid => { + Err(ExecutionPayloadError::RejectedByExecutionEngine) + } + ExecutePayloadResponse::Syncing => { + Err(ExecutionPayloadError::ExecutionEngineIsSyncing) + } + }?; } // If the block is sufficiently recent, notify the validator monitor. diff --git a/beacon_node/client/Cargo.toml b/beacon_node/client/Cargo.toml index 9cdd47e670..1441216cfb 100644 --- a/beacon_node/client/Cargo.toml +++ b/beacon_node/client/Cargo.toml @@ -47,5 +47,4 @@ http_metrics = { path = "../http_metrics" } slasher = { path = "../../slasher" } slasher_service = { path = "../../slasher/service" } monitoring_api = {path = "../../common/monitoring_api"} -sensitive_url = { path = "../../common/sensitive_url" } execution_layer = { path = "../execution_layer" } diff --git a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs index fa7b88e33e..33fdacdbaf 100644 --- a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs @@ -696,8 +696,8 @@ impl Worker { // TODO: check that this is what we're supposed to do when we don't want to // penalize a peer for our configuration issue // in the verification process BUT is this the proper way to handle it? - Err(e @BlockError::ExecutionPayloadError(ExecutionPayloadError::Eth1VerificationError(_))) - | Err(e @BlockError::ExecutionPayloadError(ExecutionPayloadError::NoEth1Connection)) => { + Err(e @BlockError::ExecutionPayloadError(ExecutionPayloadError::RequestFailed(_))) + | Err(e @BlockError::ExecutionPayloadError(ExecutionPayloadError::NoExecutionConnection)) => { debug!(self.log, "Could not verify block for gossip, ignoring the block"; "error" => %e); self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);