From 90b6ae62e630c1b39e7d7f0595b0ec8027c0291c Mon Sep 17 00:00:00 2001 From: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Date: Wed, 1 Feb 2023 19:37:46 -0600 Subject: [PATCH] Use Local Payload if More Profitable than Builder (#3934) * Use Local Payload if More Profitable than Builder * Rename clone -> clone_from_ref * Minimize Clones of GetPayloadResponse * Cleanup & Fix Tests * Added Tests for Payload Choice by Profit * Fix Outdated Comments --- beacon_node/beacon_chain/src/test_utils.rs | 8 +- beacon_node/execution_layer/src/engine_api.rs | 55 +++-- .../execution_layer/src/engine_api/http.rs | 23 +-- beacon_node/execution_layer/src/lib.rs | 112 ++++++++--- .../src/test_utils/handle_rpc.rs | 5 +- .../src/test_utils/mock_builder.rs | 4 +- .../src/test_utils/mock_execution_layer.rs | 4 +- .../execution_layer/src/test_utils/mod.rs | 2 + beacon_node/http_api/tests/tests.rs | 190 ++++++++++++++++-- consensus/types/src/execution_payload.rs | 10 + 10 files changed, 344 insertions(+), 69 deletions(-) diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 875ff845af..daba7115e0 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -426,6 +426,7 @@ where DEFAULT_TERMINAL_BLOCK, shanghai_time, eip4844_time, + None, Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()), spec, None, @@ -435,7 +436,11 @@ where self } - pub fn mock_execution_layer_with_builder(mut self, beacon_url: SensitiveUrl) -> Self { + pub fn mock_execution_layer_with_builder( + mut self, + beacon_url: SensitiveUrl, + builder_threshold: Option, + ) -> Self { // Get a random unused port let port = unused_port::unused_tcp_port().unwrap(); let builder_url = SensitiveUrl::parse(format!("http://127.0.0.1:{port}").as_str()).unwrap(); @@ -452,6 +457,7 @@ where DEFAULT_TERMINAL_BLOCK, shanghai_time, eip4844_time, + builder_threshold, Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()), spec.clone(), Some(builder_url.clone()), diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index da5e991b09..9918b679c3 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -14,8 +14,8 @@ use std::convert::TryFrom; use strum::IntoStaticStr; use superstruct::superstruct; pub use types::{ - Address, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadHeader, FixedVector, - ForkName, Hash256, Uint256, VariableList, Withdrawal, + Address, EthSpec, ExecutionBlockHash, ExecutionPayload, ExecutionPayloadHeader, + ExecutionPayloadRef, FixedVector, ForkName, Hash256, Uint256, VariableList, Withdrawal, }; use types::{ExecutionPayloadCapella, ExecutionPayloadEip4844, ExecutionPayloadMerge}; @@ -322,6 +322,8 @@ pub struct ProposeBlindedBlockResponse { #[superstruct( variants(Merge, Capella, Eip4844), variant_attributes(derive(Clone, Debug, PartialEq),), + map_into(ExecutionPayload), + map_ref_into(ExecutionPayloadRef), cast_error(ty = "Error", expr = "Error::IncorrectStateVariant"), partial_getter_error(ty = "Error", expr = "Error::IncorrectStateVariant") )] @@ -336,22 +338,47 @@ pub struct GetPayloadResponse { pub block_value: Uint256, } -impl GetPayloadResponse { - pub fn execution_payload(self) -> ExecutionPayload { - match self { - GetPayloadResponse::Merge(response) => { - ExecutionPayload::Merge(response.execution_payload) - } - GetPayloadResponse::Capella(response) => { - ExecutionPayload::Capella(response.execution_payload) - } - GetPayloadResponse::Eip4844(response) => { - ExecutionPayload::Eip4844(response.execution_payload) - } +impl<'a, T: EthSpec> From> for ExecutionPayloadRef<'a, T> { + fn from(response: GetPayloadResponseRef<'a, T>) -> Self { + map_get_payload_response_ref_into_execution_payload_ref!(&'a _, response, |inner, cons| { + cons(&inner.execution_payload) + }) + } +} + +impl From> for ExecutionPayload { + fn from(response: GetPayloadResponse) -> Self { + map_get_payload_response_into_execution_payload!(response, |inner, cons| { + cons(inner.execution_payload) + }) + } +} + +impl From> for (ExecutionPayload, Uint256) { + fn from(response: GetPayloadResponse) -> Self { + match response { + GetPayloadResponse::Merge(inner) => ( + ExecutionPayload::Merge(inner.execution_payload), + inner.block_value, + ), + GetPayloadResponse::Capella(inner) => ( + ExecutionPayload::Capella(inner.execution_payload), + inner.block_value, + ), + GetPayloadResponse::Eip4844(inner) => ( + ExecutionPayload::Eip4844(inner.execution_payload), + inner.block_value, + ), } } } +impl GetPayloadResponse { + pub fn execution_payload_ref(&self) -> ExecutionPayloadRef { + self.to_ref().into() + } +} + #[derive(Clone, Copy, Debug)] pub struct EngineCapabilities { pub new_payload_v1: bool, diff --git a/beacon_node/execution_layer/src/engine_api/http.rs b/beacon_node/execution_layer/src/engine_api/http.rs index d1faab42c9..3871ca27af 100644 --- a/beacon_node/execution_layer/src/engine_api/http.rs +++ b/beacon_node/execution_layer/src/engine_api/http.rs @@ -804,7 +804,7 @@ impl HttpJsonRpc { pub async fn get_payload_v1( &self, payload_id: PayloadId, - ) -> Result, Error> { + ) -> Result, Error> { let params = json!([JsonPayloadIdRequest::from(payload_id)]); let payload_v1: JsonExecutionPayloadV1 = self @@ -815,7 +815,11 @@ impl HttpJsonRpc { ) .await?; - Ok(JsonExecutionPayload::V1(payload_v1).into()) + Ok(GetPayloadResponse::Merge(GetPayloadResponseMerge { + execution_payload: payload_v1.into(), + // Have to guess zero here as we don't know the value + block_value: Uint256::zero(), + })) } pub async fn get_payload_v2( @@ -1015,16 +1019,10 @@ impl HttpJsonRpc { &self, fork_name: ForkName, payload_id: PayloadId, - ) -> Result, Error> { + ) -> Result, Error> { let engine_capabilities = self.get_engine_capabilities(None).await?; if engine_capabilities.get_payload_v2 { - // TODO: modify this method to return GetPayloadResponse instead - // of throwing away the `block_value` and returning only the - // ExecutionPayload - Ok(self - .get_payload_v2(fork_name, payload_id) - .await? - .execution_payload()) + self.get_payload_v2(fork_name, payload_id).await } else if engine_capabilities.new_payload_v1 { self.get_payload_v1(payload_id).await } else { @@ -1675,10 +1673,11 @@ mod test { } })], |client| async move { - let payload = client + let payload: ExecutionPayload<_> = client .get_payload_v1::(str_to_payload_id("0xa247243752eb10b4")) .await - .unwrap(); + .unwrap() + .into(); let expected = ExecutionPayload::Merge(ExecutionPayloadMerge { parent_hash: ExecutionBlockHash::from_str("0x3b8fb240d288781d4aac94d3fd16809ee413bc99294a085798a589dae51ddd4a").unwrap(), diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index ad72453f12..8f206886e4 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -119,9 +119,13 @@ impl From for Error { } pub enum BlockProposalContents> { - Payload(Payload), + Payload { + payload: Payload, + block_value: Uint256, + }, PayloadAndBlobs { payload: Payload, + block_value: Uint256, kzg_commitments: Vec, blobs: Vec>, }, @@ -130,9 +134,13 @@ pub enum BlockProposalContents> { impl> BlockProposalContents { pub fn payload(&self) -> &Payload { match self { - Self::Payload(payload) => payload, + Self::Payload { + payload, + block_value: _, + } => payload, Self::PayloadAndBlobs { payload, + block_value: _, kzg_commitments: _, blobs: _, } => payload, @@ -140,9 +148,13 @@ impl> BlockProposalContents Payload { match self { - Self::Payload(payload) => payload, + Self::Payload { + payload, + block_value: _, + } => payload, Self::PayloadAndBlobs { payload, + block_value: _, kzg_commitments: _, blobs: _, } => payload, @@ -150,9 +162,13 @@ impl> BlockProposalContents Option<&[KzgCommitment]> { match self { - Self::Payload(_) => None, + Self::Payload { + payload: _, + block_value: _, + } => None, Self::PayloadAndBlobs { payload: _, + block_value: _, kzg_commitments, blobs: _, } => Some(kzg_commitments), @@ -160,21 +176,43 @@ impl> BlockProposalContents Option<&[Blob]> { match self { - Self::Payload(_) => None, + Self::Payload { + payload: _, + block_value: _, + } => None, Self::PayloadAndBlobs { payload: _, + block_value: _, kzg_commitments: _, blobs, } => Some(blobs), } } + pub fn block_value(&self) -> &Uint256 { + match self { + Self::Payload { + payload: _, + block_value, + } => block_value, + Self::PayloadAndBlobs { + payload: _, + block_value, + kzg_commitments: _, + blobs: _, + } => block_value, + } + } pub fn default_at_fork(fork_name: ForkName) -> Result { Ok(match fork_name { ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { - BlockProposalContents::Payload(Payload::default_at_fork(fork_name)?) + BlockProposalContents::Payload { + payload: Payload::default_at_fork(fork_name)?, + block_value: Uint256::zero(), + } } ForkName::Eip4844 => BlockProposalContents::PayloadAndBlobs { payload: Payload::default_at_fork(fork_name)?, + block_value: Uint256::zero(), blobs: vec![], kzg_commitments: vec![], }, @@ -366,12 +404,12 @@ impl ExecutionLayer { &self.inner.builder } - /// Cache a full payload, keyed on the `tree_hash_root` of its `transactions` field. - fn cache_payload(&self, payload: &ExecutionPayload) -> Option> { - self.inner.payload_cache.put(payload.clone()) + /// Cache a full payload, keyed on the `tree_hash_root` of the payload + fn cache_payload(&self, payload: ExecutionPayloadRef) -> Option> { + self.inner.payload_cache.put(payload.clone_from_ref()) } - /// Attempt to retrieve a full payload from the payload cache by the `transactions_root`. + /// Attempt to retrieve a full payload from the payload cache by the payload root pub fn get_payload_by_root(&self, root: &Hash256) -> Option> { self.inner.payload_cache.pop(root) } @@ -808,6 +846,18 @@ impl ExecutionLayer { "parent_hash" => ?parent_hash, ); + let relay_value = relay.data.message.value; + let local_value = *local.block_value(); + if local_value >= relay_value { + info!( + self.log(), + "Local block is more profitable than relay block"; + "local_block_value" => %local_value, + "relay_value" => %relay_value + ); + return Ok(ProvenancedPayload::Local(local)); + } + match verify_builder_bid( &relay, parent_hash, @@ -818,7 +868,10 @@ impl ExecutionLayer { spec, ) { Ok(()) => Ok(ProvenancedPayload::Builder( - BlockProposalContents::Payload(relay.data.message.header), + BlockProposalContents::Payload { + payload: relay.data.message.header, + block_value: relay.data.message.value, + }, )), Err(reason) if !reason.payload_invalid() => { info!( @@ -869,12 +922,18 @@ impl ExecutionLayer { spec, ) { Ok(()) => Ok(ProvenancedPayload::Builder( - BlockProposalContents::Payload(relay.data.message.header), + BlockProposalContents::Payload { + payload: relay.data.message.header, + block_value: relay.data.message.value, + }, )), // If the payload is valid then use it. The local EE failed // to produce a payload so we have no alternative. Err(e) if !e.payload_invalid() => Ok(ProvenancedPayload::Builder( - BlockProposalContents::Payload(relay.data.message.header), + BlockProposalContents::Payload { + payload: relay.data.message.header, + block_value: relay.data.message.value, + }, )), Err(reason) => { metrics::inc_counter_vec( @@ -988,7 +1047,7 @@ impl ExecutionLayer { payload_attributes: &PayloadAttributes, forkchoice_update_params: ForkchoiceUpdateParameters, current_fork: ForkName, - f: fn(&ExecutionLayer, &ExecutionPayload) -> Option>, + f: fn(&ExecutionLayer, ExecutionPayloadRef) -> Option>, ) -> Result, Error> { self.engine() .request(move |engine| async move { @@ -1071,9 +1130,9 @@ impl ExecutionLayer { ); engine.api.get_payload::(current_fork, payload_id).await }; - let (blob, payload) = tokio::join!(blob_fut, payload_fut); - let payload = payload.map(|full_payload| { - if full_payload.fee_recipient() != payload_attributes.suggested_fee_recipient() { + let (blob, payload_response) = tokio::join!(blob_fut, payload_fut); + let (execution_payload, block_value) = payload_response.map(|payload_response| { + if payload_response.execution_payload_ref().fee_recipient() != payload_attributes.suggested_fee_recipient() { error!( self.log(), "Inconsistent fee recipient"; @@ -1082,28 +1141,32 @@ impl ExecutionLayer { indicate that fees are being diverted to another address. Please \ ensure that the value of suggested_fee_recipient is set correctly and \ that the Execution Engine is trusted.", - "fee_recipient" => ?full_payload.fee_recipient(), + "fee_recipient" => ?payload_response.execution_payload_ref().fee_recipient(), "suggested_fee_recipient" => ?payload_attributes.suggested_fee_recipient(), ); } - if f(self, &full_payload).is_some() { + if f(self, payload_response.execution_payload_ref()).is_some() { warn!( self.log(), "Duplicate payload cached, this might indicate redundant proposal \ attempts." ); } - full_payload.into() + payload_response.into() })?; if let Some(blob) = blob.transpose()? { // FIXME(sean) cache blobs Ok(BlockProposalContents::PayloadAndBlobs { - payload, + payload: execution_payload.into(), + block_value, blobs: blob.blobs, kzg_commitments: blob.kzgs, }) } else { - Ok(BlockProposalContents::Payload(payload)) + Ok(BlockProposalContents::Payload { + payload: execution_payload.into(), + block_value, + }) } }) .await @@ -2089,7 +2152,10 @@ mod test { } } -fn noop(_: &ExecutionLayer, _: &ExecutionPayload) -> Option> { +fn noop( + _: &ExecutionLayer, + _: ExecutionPayloadRef, +) -> Option> { None } 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 31a8a5da19..138c8f6bcb 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -1,6 +1,7 @@ use super::Context; use crate::engine_api::{http::*, *}; use crate::json_structures::*; +use crate::test_utils::DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI; use serde::de::DeserializeOwned; use serde_json::Value as JsonValue; use std::sync::Arc; @@ -211,14 +212,14 @@ pub async fn handle_rpc( JsonExecutionPayload::V1(execution_payload) => { serde_json::to_value(JsonGetPayloadResponseV1 { execution_payload, - block_value: 0.into(), + block_value: DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI.into(), }) .unwrap() } JsonExecutionPayload::V2(execution_payload) => { serde_json::to_value(JsonGetPayloadResponseV2 { execution_payload, - block_value: 0.into(), + block_value: DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI.into(), }) .unwrap() } diff --git a/beacon_node/execution_layer/src/test_utils/mock_builder.rs b/beacon_node/execution_layer/src/test_utils/mock_builder.rs index 06b5e81eb3..40a0c41afa 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -1,4 +1,4 @@ -use crate::test_utils::DEFAULT_JWT_SECRET; +use crate::test_utils::{DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_JWT_SECRET}; use crate::{Config, ExecutionLayer, PayloadAttributes}; use async_trait::async_trait; use eth2::types::{BlockId, StateId, ValidatorId}; @@ -328,7 +328,7 @@ impl mev_build_rs::BlindedBlockProvider for MockBuilder { let mut message = BuilderBid { header, - value: ssz_rs::U256::default(), + value: to_ssz_rs(&Uint256::from(DEFAULT_BUILDER_PAYLOAD_VALUE_WEI))?, public_key: self.builder_sk.public_key(), }; 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 ad73b2b4e7..1a5d1fd198 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 @@ -29,6 +29,7 @@ impl MockExecutionLayer { DEFAULT_TERMINAL_BLOCK, None, None, + None, Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()), spec, None, @@ -41,6 +42,7 @@ impl MockExecutionLayer { terminal_block: u64, shanghai_time: Option, eip4844_time: Option, + builder_threshold: Option, jwt_key: Option, spec: ChainSpec, builder_url: Option, @@ -69,7 +71,7 @@ impl MockExecutionLayer { builder_url, secret_files: vec![path], suggested_fee_recipient: Some(Address::repeat_byte(42)), - builder_profit_threshold: DEFAULT_BUILDER_THRESHOLD_WEI, + builder_profit_threshold: builder_threshold.unwrap_or(DEFAULT_BUILDER_THRESHOLD_WEI), ..Default::default() }; let el = diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index adf9358f06..077d29575e 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -32,6 +32,8 @@ pub const DEFAULT_TERMINAL_DIFFICULTY: u64 = 6400; pub const DEFAULT_TERMINAL_BLOCK: u64 = 64; pub const DEFAULT_JWT_SECRET: [u8; 32] = [42; 32]; pub const DEFAULT_BUILDER_THRESHOLD_WEI: u128 = 1_000_000_000_000_000_000; +pub const DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI: u128 = 10_000_000_000_000_000; +pub const DEFAULT_BUILDER_PAYLOAD_VALUE_WEI: u128 = 20_000_000_000_000_000; pub const DEFAULT_ENGINE_CAPABILITIES: EngineCapabilities = EngineCapabilities { new_payload_v1: true, new_payload_v2: true, diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 86733cf63a..43099c7a91 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -11,9 +11,11 @@ use eth2::{ types::{BlockId as CoreBlockId, StateId as CoreStateId, *}, BeaconNodeHttpClient, Error, StatusCode, Timeouts, }; -use execution_layer::test_utils::Operation; use execution_layer::test_utils::TestingBuilder; use execution_layer::test_utils::DEFAULT_BUILDER_THRESHOLD_WEI; +use execution_layer::test_utils::{ + Operation, DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI, +}; use futures::stream::{Stream, StreamExt}; use futures::FutureExt; use http_api::{BlockId, StateId}; @@ -72,38 +74,53 @@ struct ApiTester { mock_builder: Option>>, } +struct ApiTesterConfig { + spec: ChainSpec, + builder_threshold: Option, +} + +impl Default for ApiTesterConfig { + fn default() -> Self { + let mut spec = E::default_spec(); + spec.shard_committee_period = 2; + Self { + spec, + builder_threshold: None, + } + } +} + impl ApiTester { pub async fn new() -> Self { // This allows for testing voluntary exits without building out a massive chain. - let mut spec = E::default_spec(); - spec.shard_committee_period = 2; - Self::new_from_spec(spec).await + Self::new_from_config(ApiTesterConfig::default()).await } pub async fn new_with_hard_forks(altair: bool, bellatrix: bool) -> Self { - let mut spec = E::default_spec(); - spec.shard_committee_period = 2; + let mut config = ApiTesterConfig::default(); // Set whether the chain has undergone each hard fork. if altair { - spec.altair_fork_epoch = Some(Epoch::new(0)); + config.spec.altair_fork_epoch = Some(Epoch::new(0)); } if bellatrix { - spec.bellatrix_fork_epoch = Some(Epoch::new(0)); + config.spec.bellatrix_fork_epoch = Some(Epoch::new(0)); } - Self::new_from_spec(spec).await + Self::new_from_config(config).await } - pub async fn new_from_spec(spec: ChainSpec) -> Self { + pub async fn new_from_config(config: ApiTesterConfig) -> Self { // Get a random unused port + let spec = config.spec; let port = unused_port::unused_tcp_port().unwrap(); let beacon_url = SensitiveUrl::parse(format!("http://127.0.0.1:{port}").as_str()).unwrap(); let harness = Arc::new( BeaconChainHarness::builder(MainnetEthSpec) .spec(spec.clone()) + .logger(logging::test_logger()) .deterministic_keypairs(VALIDATOR_COUNT) .fresh_ephemeral_store() - .mock_execution_layer_with_builder(beacon_url.clone()) + .mock_execution_layer_with_builder(beacon_url.clone(), config.builder_threshold) .build(), ); @@ -358,6 +375,28 @@ impl ApiTester { tester } + pub async fn new_mev_tester_no_builder_threshold() -> Self { + let mut config = ApiTesterConfig { + builder_threshold: Some(0), + spec: E::default_spec(), + }; + config.spec.altair_fork_epoch = Some(Epoch::new(0)); + config.spec.bellatrix_fork_epoch = Some(Epoch::new(0)); + let tester = Self::new_from_config(config) + .await + .test_post_validator_register_validator() + .await; + tester + .mock_builder + .as_ref() + .unwrap() + .builder + .add_operation(Operation::Value(Uint256::from( + DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, + ))); + tester + } + fn skip_slots(self, count: u64) -> Self { for _ in 0..count { self.chain @@ -3278,6 +3317,117 @@ impl ApiTester { self } + pub async fn test_builder_payload_chosen_when_more_profitable(self) -> Self { + // Mutate value. + self.mock_builder + .as_ref() + .unwrap() + .builder + .add_operation(Operation::Value(Uint256::from( + DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI + 1, + ))); + + let slot = self.chain.slot().unwrap(); + let epoch = self.chain.epoch().unwrap(); + + let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; + + let payload: BlindedPayload = self + .client + .get_validator_blinded_blocks::>(slot, &randao_reveal, None) + .await + .unwrap() + .data + .body() + .execution_payload() + .unwrap() + .into(); + + // The builder's payload should've been chosen, so this cache should not be populated + assert!(self + .chain + .execution_layer + .as_ref() + .unwrap() + .get_payload_by_root(&payload.tree_hash_root()) + .is_none()); + self + } + + pub async fn test_local_payload_chosen_when_equally_profitable(self) -> Self { + // Mutate value. + self.mock_builder + .as_ref() + .unwrap() + .builder + .add_operation(Operation::Value(Uint256::from( + DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI, + ))); + + let slot = self.chain.slot().unwrap(); + let epoch = self.chain.epoch().unwrap(); + + let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; + + let payload: BlindedPayload = self + .client + .get_validator_blinded_blocks::>(slot, &randao_reveal, None) + .await + .unwrap() + .data + .body() + .execution_payload() + .unwrap() + .into(); + + // The local payload should've been chosen, so this cache should be populated + assert!(self + .chain + .execution_layer + .as_ref() + .unwrap() + .get_payload_by_root(&payload.tree_hash_root()) + .is_some()); + self + } + + pub async fn test_local_payload_chosen_when_more_profitable(self) -> Self { + // Mutate value. + self.mock_builder + .as_ref() + .unwrap() + .builder + .add_operation(Operation::Value(Uint256::from( + DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI - 1, + ))); + + let slot = self.chain.slot().unwrap(); + let epoch = self.chain.epoch().unwrap(); + + let (_, randao_reveal) = self.get_test_randao(slot, epoch).await; + + let payload: BlindedPayload = self + .client + .get_validator_blinded_blocks::>(slot, &randao_reveal, None) + .await + .unwrap() + .data + .body() + .execution_payload() + .unwrap() + .into(); + + // The local payload should've been chosen, so this cache should be populated + assert!(self + .chain + .execution_layer + .as_ref() + .unwrap() + .get_payload_by_root(&payload.tree_hash_root()) + .is_some()); + self + } + #[cfg(target_os = "linux")] pub async fn test_get_lighthouse_health(self) -> Self { self.client.get_lighthouse_health().await.unwrap(); @@ -3747,9 +3897,9 @@ async fn get_events() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn get_events_altair() { - let mut spec = E::default_spec(); - spec.altair_fork_epoch = Some(Epoch::new(0)); - ApiTester::new_from_spec(spec) + let mut config = ApiTesterConfig::default(); + config.spec.altair_fork_epoch = Some(Epoch::new(0)); + ApiTester::new_from_config(config) .await .test_get_events_altair() .await; @@ -4262,6 +4412,18 @@ async fn builder_inadequate_builder_threshold() { .await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn builder_payload_chosen_by_profit() { + ApiTester::new_mev_tester_no_builder_threshold() + .await + .test_builder_payload_chosen_when_more_profitable() + .await + .test_local_payload_chosen_when_equally_profitable() + .await + .test_local_payload_chosen_when_more_profitable() + .await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn lighthouse_endpoints() { ApiTester::new() diff --git a/consensus/types/src/execution_payload.rs b/consensus/types/src/execution_payload.rs index a57d411412..1721960f8b 100644 --- a/consensus/types/src/execution_payload.rs +++ b/consensus/types/src/execution_payload.rs @@ -87,6 +87,16 @@ pub struct ExecutionPayload { pub withdrawals: Withdrawals, } +impl<'a, T: EthSpec> ExecutionPayloadRef<'a, T> { + // this emulates clone on a normal reference type + pub fn clone_from_ref(&self) -> ExecutionPayload { + map_execution_payload_ref!(&'a _, self, move |payload, cons| { + cons(payload); + payload.clone().into() + }) + } +} + impl ExecutionPayload { pub fn from_ssz_bytes(bytes: &[u8], fork_name: ForkName) -> Result { match fork_name {