From 86891e6d0f111c318660aaea63ed39c58dd716a5 Mon Sep 17 00:00:00 2001 From: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Date: Sun, 15 Dec 2024 21:43:58 -0800 Subject: [PATCH] builder gas limit & some refactoring (#6583) * Cache gas_limit * Payload Parameters Refactor * Enforce Proposer Gas Limit * Fixed and Added New Tests * Fix Beacon Chain Tests --- .../beacon_chain/src/execution_payload.rs | 24 +- .../tests/payload_invalidation.rs | 22 +- beacon_node/execution_layer/src/lib.rs | 284 +++++++++++------- .../test_utils/execution_block_generator.rs | 35 ++- .../src/test_utils/mock_builder.rs | 63 +++- .../src/test_utils/mock_execution_layer.rs | 29 +- .../execution_layer/src/test_utils/mod.rs | 5 +- beacon_node/http_api/src/lib.rs | 27 +- .../http_api/tests/interactive_tests.rs | 13 +- beacon_node/http_api/tests/tests.rs | 234 +++++++++++---- consensus/types/src/chain_spec.rs | 27 ++ consensus/types/src/payload.rs | 33 ++ testing/ef_tests/src/cases/fork_choice.rs | 11 +- .../src/test_rig.rs | 34 ++- 14 files changed, 598 insertions(+), 243 deletions(-) diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index f2420eea0d2..5e13f0624da 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -14,7 +14,7 @@ use crate::{ }; use execution_layer::{ BlockProposalContents, BlockProposalContentsType, BuilderParams, NewPayloadRequest, - PayloadAttributes, PayloadStatus, + PayloadAttributes, PayloadParameters, PayloadStatus, }; use fork_choice::{InvalidationOperation, PayloadVerificationStatus}; use proto_array::{Block as ProtoBlock, ExecutionStatus}; @@ -375,8 +375,9 @@ pub fn get_execution_payload( let timestamp = compute_timestamp_at_slot(state, state.slot(), spec).map_err(BeaconStateError::from)?; let random = *state.get_randao_mix(current_epoch)?; - let latest_execution_payload_header_block_hash = - state.latest_execution_payload_header()?.block_hash(); + let latest_execution_payload_header = state.latest_execution_payload_header()?; + let latest_execution_payload_header_block_hash = latest_execution_payload_header.block_hash(); + let latest_execution_payload_header_gas_limit = latest_execution_payload_header.gas_limit(); let withdrawals = match state { &BeaconState::Capella(_) | &BeaconState::Deneb(_) | &BeaconState::Electra(_) => { Some(get_expected_withdrawals(state, spec)?.0.into()) @@ -406,6 +407,7 @@ pub fn get_execution_payload( random, proposer_index, latest_execution_payload_header_block_hash, + latest_execution_payload_header_gas_limit, builder_params, withdrawals, parent_beacon_block_root, @@ -443,6 +445,7 @@ pub async fn prepare_execution_payload( random: Hash256, proposer_index: u64, latest_execution_payload_header_block_hash: ExecutionBlockHash, + latest_execution_payload_header_gas_limit: u64, builder_params: BuilderParams, withdrawals: Option>, parent_beacon_block_root: Option, @@ -526,13 +529,20 @@ where parent_beacon_block_root, ); + let target_gas_limit = execution_layer.get_proposer_gas_limit(proposer_index).await; + let payload_parameters = PayloadParameters { + parent_hash, + parent_gas_limit: latest_execution_payload_header_gas_limit, + proposer_gas_limit: target_gas_limit, + payload_attributes: &payload_attributes, + forkchoice_update_params: &forkchoice_update_params, + current_fork: fork, + }; + let block_contents = execution_layer .get_payload( - parent_hash, - &payload_attributes, - forkchoice_update_params, + payload_parameters, builder_params, - fork, &chain.spec, builder_boost_factor, block_production_version, diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 1325875a275..729d88450f4 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -986,10 +986,13 @@ async fn payload_preparation() { // Provide preparation data to the EL for `proposer`. el.update_proposer_preparation( Epoch::new(1), - &[ProposerPreparationData { - validator_index: proposer as u64, - fee_recipient, - }], + [( + &ProposerPreparationData { + validator_index: proposer as u64, + fee_recipient, + }, + &None, + )], ) .await; @@ -1119,10 +1122,13 @@ async fn payload_preparation_before_transition_block() { // Provide preparation data to the EL for `proposer`. el.update_proposer_preparation( Epoch::new(0), - &[ProposerPreparationData { - validator_index: proposer as u64, - fee_recipient, - }], + [( + &ProposerPreparationData { + validator_index: proposer as u64, + fee_recipient, + }, + &None, + )], ) .await; diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 08a00d7bf8d..ae0dca9833f 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -28,7 +28,7 @@ use sensitive_url::SensitiveUrl; use serde::{Deserialize, Serialize}; use slog::{crit, debug, error, info, warn, Logger}; use slot_clock::SlotClock; -use std::collections::HashMap; +use std::collections::{hash_map::Entry, HashMap}; use std::fmt; use std::future::Future; use std::io::Write; @@ -319,10 +319,52 @@ impl> BlockProposalContents { + pub parent_hash: ExecutionBlockHash, + pub parent_gas_limit: u64, + pub proposer_gas_limit: Option, + pub payload_attributes: &'a PayloadAttributes, + pub forkchoice_update_params: &'a ForkchoiceUpdateParameters, + pub current_fork: ForkName, +} + #[derive(Clone, PartialEq)] pub struct ProposerPreparationDataEntry { update_epoch: Epoch, preparation_data: ProposerPreparationData, + gas_limit: Option, +} + +impl ProposerPreparationDataEntry { + pub fn update(&mut self, updated: Self) -> bool { + let mut changed = false; + // Update `gas_limit` if `updated.gas_limit` is `Some` and: + // - `self.gas_limit` is `None`, or + // - both are `Some` but the values differ. + if let Some(updated_gas_limit) = updated.gas_limit { + if self.gas_limit != Some(updated_gas_limit) { + self.gas_limit = Some(updated_gas_limit); + changed = true; + } + } + + // Update `update_epoch` if it differs + if self.update_epoch != updated.update_epoch { + self.update_epoch = updated.update_epoch; + changed = true; + } + + // Update `preparation_data` if it differs + if self.preparation_data != updated.preparation_data { + self.preparation_data = updated.preparation_data; + changed = true; + } + + changed + } } #[derive(Hash, PartialEq, Eq)] @@ -711,23 +753,29 @@ impl ExecutionLayer { } /// Updates the proposer preparation data provided by validators - pub async fn update_proposer_preparation( - &self, - update_epoch: Epoch, - preparation_data: &[ProposerPreparationData], - ) { + pub async fn update_proposer_preparation<'a, I>(&self, update_epoch: Epoch, proposer_data: I) + where + I: IntoIterator)>, + { let mut proposer_preparation_data = self.proposer_preparation_data().await; - for preparation_entry in preparation_data { + + for (preparation_entry, gas_limit) in proposer_data { let new = ProposerPreparationDataEntry { update_epoch, preparation_data: preparation_entry.clone(), + gas_limit: *gas_limit, }; - let existing = - proposer_preparation_data.insert(preparation_entry.validator_index, new.clone()); - - if existing != Some(new) { - metrics::inc_counter(&metrics::EXECUTION_LAYER_PROPOSER_DATA_UPDATED); + match proposer_preparation_data.entry(preparation_entry.validator_index) { + Entry::Occupied(mut entry) => { + if entry.get_mut().update(new) { + metrics::inc_counter(&metrics::EXECUTION_LAYER_PROPOSER_DATA_UPDATED); + } + } + Entry::Vacant(entry) => { + entry.insert(new); + metrics::inc_counter(&metrics::EXECUTION_LAYER_PROPOSER_DATA_UPDATED); + } } } } @@ -809,6 +857,13 @@ impl ExecutionLayer { } } + pub async fn get_proposer_gas_limit(&self, proposer_index: u64) -> Option { + self.proposer_preparation_data() + .await + .get(&proposer_index) + .and_then(|entry| entry.gas_limit) + } + /// Maps to the `engine_getPayload` JSON-RPC call. /// /// However, it will attempt to call `self.prepare_payload` if it cannot find an existing @@ -818,14 +873,10 @@ impl ExecutionLayer { /// /// The result will be returned from the first node that returns successfully. No more nodes /// will be contacted. - #[allow(clippy::too_many_arguments)] pub async fn get_payload( &self, - parent_hash: ExecutionBlockHash, - payload_attributes: &PayloadAttributes, - forkchoice_update_params: ForkchoiceUpdateParameters, + payload_parameters: PayloadParameters<'_>, builder_params: BuilderParams, - current_fork: ForkName, spec: &ChainSpec, builder_boost_factor: Option, block_production_version: BlockProductionVersion, @@ -833,11 +884,8 @@ impl ExecutionLayer { let payload_result_type = match block_production_version { BlockProductionVersion::V3 => match self .determine_and_fetch_payload( - parent_hash, - payload_attributes, - forkchoice_update_params, + payload_parameters, builder_params, - current_fork, builder_boost_factor, spec, ) @@ -857,25 +905,11 @@ impl ExecutionLayer { &metrics::EXECUTION_LAYER_REQUEST_TIMES, &[metrics::GET_BLINDED_PAYLOAD], ); - self.determine_and_fetch_payload( - parent_hash, - payload_attributes, - forkchoice_update_params, - builder_params, - current_fork, - None, - spec, - ) - .await? + self.determine_and_fetch_payload(payload_parameters, builder_params, None, spec) + .await? } BlockProductionVersion::FullV2 => self - .get_full_payload_with( - parent_hash, - payload_attributes, - forkchoice_update_params, - current_fork, - noop, - ) + .get_full_payload_with(payload_parameters, noop) .await .and_then(GetPayloadResponseType::try_into) .map(ProvenancedPayload::Local)?, @@ -922,17 +956,15 @@ impl ExecutionLayer { async fn fetch_builder_and_local_payloads( &self, builder: &BuilderHttpClient, - parent_hash: ExecutionBlockHash, builder_params: &BuilderParams, - payload_attributes: &PayloadAttributes, - forkchoice_update_params: ForkchoiceUpdateParameters, - current_fork: ForkName, + payload_parameters: PayloadParameters<'_>, ) -> ( Result>>, builder_client::Error>, Result, Error>, ) { let slot = builder_params.slot; let pubkey = &builder_params.pubkey; + let parent_hash = payload_parameters.parent_hash; info!( self.log(), @@ -950,17 +982,12 @@ impl ExecutionLayer { .await }), timed_future(metrics::GET_BLINDED_PAYLOAD_LOCAL, async { - self.get_full_payload_caching( - parent_hash, - payload_attributes, - forkchoice_update_params, - current_fork, - ) - .await - .and_then(|local_result_type| match local_result_type { - GetPayloadResponseType::Full(payload) => Ok(payload), - GetPayloadResponseType::Blinded(_) => Err(Error::PayloadTypeMismatch), - }) + self.get_full_payload_caching(payload_parameters) + .await + .and_then(|local_result_type| match local_result_type { + GetPayloadResponseType::Full(payload) => Ok(payload), + GetPayloadResponseType::Blinded(_) => Err(Error::PayloadTypeMismatch), + }) }) ); @@ -984,26 +1011,17 @@ impl ExecutionLayer { (relay_result, local_result) } - #[allow(clippy::too_many_arguments)] async fn determine_and_fetch_payload( &self, - parent_hash: ExecutionBlockHash, - payload_attributes: &PayloadAttributes, - forkchoice_update_params: ForkchoiceUpdateParameters, + payload_parameters: PayloadParameters<'_>, builder_params: BuilderParams, - current_fork: ForkName, builder_boost_factor: Option, spec: &ChainSpec, ) -> Result>, Error> { let Some(builder) = self.builder() else { // no builder.. return local payload return self - .get_full_payload_caching( - parent_hash, - payload_attributes, - forkchoice_update_params, - current_fork, - ) + .get_full_payload_caching(payload_parameters) .await .and_then(GetPayloadResponseType::try_into) .map(ProvenancedPayload::Local); @@ -1034,26 +1052,15 @@ impl ExecutionLayer { ), } return self - .get_full_payload_caching( - parent_hash, - payload_attributes, - forkchoice_update_params, - current_fork, - ) + .get_full_payload_caching(payload_parameters) .await .and_then(GetPayloadResponseType::try_into) .map(ProvenancedPayload::Local); } + let parent_hash = payload_parameters.parent_hash; let (relay_result, local_result) = self - .fetch_builder_and_local_payloads( - builder.as_ref(), - parent_hash, - &builder_params, - payload_attributes, - forkchoice_update_params, - current_fork, - ) + .fetch_builder_and_local_payloads(builder.as_ref(), &builder_params, payload_parameters) .await; match (relay_result, local_result) { @@ -1118,14 +1125,9 @@ impl ExecutionLayer { ); // check relay payload validity - if let Err(reason) = verify_builder_bid( - &relay, - parent_hash, - payload_attributes, - Some(local.block_number()), - current_fork, - spec, - ) { + if let Err(reason) = + verify_builder_bid(&relay, payload_parameters, Some(local.block_number()), spec) + { // relay payload invalid -> return local metrics::inc_counter_vec( &metrics::EXECUTION_LAYER_GET_PAYLOAD_BUILDER_REJECTIONS, @@ -1202,14 +1204,7 @@ impl ExecutionLayer { "parent_hash" => ?parent_hash, ); - match verify_builder_bid( - &relay, - parent_hash, - payload_attributes, - None, - current_fork, - spec, - ) { + match verify_builder_bid(&relay, payload_parameters, None, spec) { Ok(()) => Ok(ProvenancedPayload::try_from(relay.data.message)?), Err(reason) => { metrics::inc_counter_vec( @@ -1234,32 +1229,28 @@ impl ExecutionLayer { /// Get a full payload and cache its result in the execution layer's payload cache. async fn get_full_payload_caching( &self, - parent_hash: ExecutionBlockHash, - payload_attributes: &PayloadAttributes, - forkchoice_update_params: ForkchoiceUpdateParameters, - current_fork: ForkName, + payload_parameters: PayloadParameters<'_>, ) -> Result, Error> { - self.get_full_payload_with( - parent_hash, - payload_attributes, - forkchoice_update_params, - current_fork, - Self::cache_payload, - ) - .await + self.get_full_payload_with(payload_parameters, Self::cache_payload) + .await } async fn get_full_payload_with( &self, - parent_hash: ExecutionBlockHash, - payload_attributes: &PayloadAttributes, - forkchoice_update_params: ForkchoiceUpdateParameters, - current_fork: ForkName, + payload_parameters: PayloadParameters<'_>, cache_fn: fn( &ExecutionLayer, PayloadContentsRefTuple, ) -> Option>, ) -> Result, Error> { + let PayloadParameters { + parent_hash, + payload_attributes, + forkchoice_update_params, + current_fork, + .. + } = payload_parameters; + self.engine() .request(move |engine| async move { let payload_id = if let Some(id) = engine @@ -1984,6 +1975,10 @@ enum InvalidBuilderPayload { payload: Option, expected: Option, }, + GasLimitMismatch { + payload: u64, + expected: u64, + }, } impl fmt::Display for InvalidBuilderPayload { @@ -2022,19 +2017,51 @@ impl fmt::Display for InvalidBuilderPayload { opt_string(expected) ) } + InvalidBuilderPayload::GasLimitMismatch { payload, expected } => { + write!(f, "payload gas limit was {} not {}", payload, expected) + } } } } +/// Calculate the expected gas limit for a block. +pub fn expected_gas_limit( + parent_gas_limit: u64, + target_gas_limit: u64, + spec: &ChainSpec, +) -> Option { + // Calculate the maximum gas limit difference allowed safely + let max_gas_limit_difference = parent_gas_limit + .checked_div(spec.gas_limit_adjustment_factor) + .and_then(|result| result.checked_sub(1)) + .unwrap_or(0); + + // Adjust the gas limit safely + if target_gas_limit > parent_gas_limit { + let gas_diff = target_gas_limit.saturating_sub(parent_gas_limit); + parent_gas_limit.checked_add(std::cmp::min(gas_diff, max_gas_limit_difference)) + } else { + let gas_diff = parent_gas_limit.saturating_sub(target_gas_limit); + parent_gas_limit.checked_sub(std::cmp::min(gas_diff, max_gas_limit_difference)) + } +} + /// Perform some cursory, non-exhaustive validation of the bid returned from the builder. fn verify_builder_bid( bid: &ForkVersionedResponse>, - parent_hash: ExecutionBlockHash, - payload_attributes: &PayloadAttributes, + payload_parameters: PayloadParameters<'_>, block_number: Option, - current_fork: ForkName, spec: &ChainSpec, ) -> Result<(), Box> { + let PayloadParameters { + parent_hash, + payload_attributes, + current_fork, + parent_gas_limit, + proposer_gas_limit, + .. + } = payload_parameters; + let is_signature_valid = bid.data.verify_signature(spec); let header = &bid.data.message.header(); @@ -2050,6 +2077,8 @@ fn verify_builder_bid( .cloned() .map(|withdrawals| Withdrawals::::from(withdrawals).tree_hash_root()); let payload_withdrawals_root = header.withdrawals_root().ok(); + let expected_gas_limit = proposer_gas_limit + .and_then(|target_gas_limit| expected_gas_limit(parent_gas_limit, target_gas_limit, spec)); if header.parent_hash() != parent_hash { Err(Box::new(InvalidBuilderPayload::ParentHash { @@ -2086,6 +2115,14 @@ fn verify_builder_bid( payload: payload_withdrawals_root, expected: expected_withdrawals_root, })) + } else if expected_gas_limit + .map(|gas_limit| header.gas_limit() != gas_limit) + .unwrap_or(false) + { + Err(Box::new(InvalidBuilderPayload::GasLimitMismatch { + payload: header.gas_limit(), + expected: expected_gas_limit.unwrap_or(0), + })) } else { Ok(()) } @@ -2138,6 +2175,27 @@ mod test { .await; } + #[tokio::test] + async fn test_expected_gas_limit() { + let spec = ChainSpec::mainnet(); + assert_eq!( + expected_gas_limit(30_000_000, 30_000_000, &spec), + Some(30_000_000) + ); + assert_eq!( + expected_gas_limit(30_000_000, 40_000_000, &spec), + Some(30_029_295) + ); + assert_eq!( + expected_gas_limit(30_029_295, 40_000_000, &spec), + Some(30_058_619) + ); + assert_eq!( + expected_gas_limit(30_058_619, 30_000_000, &spec), + Some(30_029_266) + ); + } + #[tokio::test] async fn test_forked_terminal_block() { let runtime = TestRuntime::default(); 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 4deb91e0567..4fab7150ce3 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 @@ -28,8 +28,8 @@ use super::DEFAULT_TERMINAL_BLOCK; const TEST_BLOB_BUNDLE: &[u8] = include_bytes!("fixtures/mainnet/test_blobs_bundle.ssz"); -const GAS_LIMIT: u64 = 16384; -const GAS_USED: u64 = GAS_LIMIT - 1; +pub const DEFAULT_GAS_LIMIT: u64 = 30_000_000; +const GAS_USED: u64 = DEFAULT_GAS_LIMIT - 1; #[derive(Clone, Debug, PartialEq)] #[allow(clippy::large_enum_variant)] // This struct is only for testing. @@ -38,6 +38,10 @@ pub enum Block { PoS(ExecutionPayload), } +pub fn mock_el_extra_data() -> types::VariableList { + "block gen was here".as_bytes().to_vec().into() +} + impl Block { pub fn block_number(&self) -> u64 { match self { @@ -67,6 +71,13 @@ impl Block { } } + pub fn gas_limit(&self) -> u64 { + match self { + Block::PoW(_) => DEFAULT_GAS_LIMIT, + Block::PoS(payload) => payload.gas_limit(), + } + } + pub fn as_execution_block(&self, total_difficulty: Uint256) -> ExecutionBlock { match self { Block::PoW(block) => ExecutionBlock { @@ -570,10 +581,10 @@ impl ExecutionBlockGenerator { logs_bloom: vec![0; 256].into(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, - gas_limit: GAS_LIMIT, + gas_limit: DEFAULT_GAS_LIMIT, gas_used: GAS_USED, timestamp: pa.timestamp, - extra_data: "block gen was here".as_bytes().to_vec().into(), + extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), transactions: vec![].into(), @@ -587,10 +598,10 @@ impl ExecutionBlockGenerator { logs_bloom: vec![0; 256].into(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, - gas_limit: GAS_LIMIT, + gas_limit: DEFAULT_GAS_LIMIT, gas_used: GAS_USED, timestamp: pa.timestamp, - extra_data: "block gen was here".as_bytes().to_vec().into(), + extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), transactions: vec![].into(), @@ -603,10 +614,10 @@ impl ExecutionBlockGenerator { logs_bloom: vec![0; 256].into(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, - gas_limit: GAS_LIMIT, + gas_limit: DEFAULT_GAS_LIMIT, gas_used: GAS_USED, timestamp: pa.timestamp, - extra_data: "block gen was here".as_bytes().to_vec().into(), + extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), transactions: vec![].into(), @@ -623,10 +634,10 @@ impl ExecutionBlockGenerator { logs_bloom: vec![0; 256].into(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, - gas_limit: GAS_LIMIT, + gas_limit: DEFAULT_GAS_LIMIT, gas_used: GAS_USED, timestamp: pa.timestamp, - extra_data: "block gen was here".as_bytes().to_vec().into(), + extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), transactions: vec![].into(), @@ -642,10 +653,10 @@ impl ExecutionBlockGenerator { logs_bloom: vec![0; 256].into(), prev_randao: pa.prev_randao, block_number: parent.block_number() + 1, - gas_limit: GAS_LIMIT, + gas_limit: DEFAULT_GAS_LIMIT, gas_used: GAS_USED, timestamp: pa.timestamp, - extra_data: "block gen was here".as_bytes().to_vec().into(), + extra_data: mock_el_extra_data::(), base_fee_per_gas: Uint256::from(1u64), block_hash: ExecutionBlockHash::zero(), transactions: vec![].into(), 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 341daedbc8d..879b54eb075 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -1,5 +1,5 @@ use crate::test_utils::{DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_JWT_SECRET}; -use crate::{Config, ExecutionLayer, PayloadAttributes}; +use crate::{Config, ExecutionLayer, PayloadAttributes, PayloadParameters}; use eth2::types::{BlobsBundle, BlockId, StateId, ValidatorId}; use eth2::{BeaconNodeHttpClient, Timeouts, CONSENSUS_VERSION_HEADER}; use fork_choice::ForkchoiceUpdateParameters; @@ -54,6 +54,10 @@ impl Operation { } } +pub fn mock_builder_extra_data() -> types::VariableList { + "mock_builder".as_bytes().to_vec().into() +} + #[derive(Debug)] // We don't use the string value directly, but it's used in the Debug impl which is required by `warp::reject::Reject`. struct Custom(#[allow(dead_code)] String); @@ -72,6 +76,8 @@ pub trait BidStuff { fn set_withdrawals_root(&mut self, withdrawals_root: Hash256); fn sign_builder_message(&mut self, sk: &SecretKey, spec: &ChainSpec) -> Signature; + + fn stamp_payload(&mut self); } impl BidStuff for BuilderBid { @@ -203,6 +209,29 @@ impl BidStuff for BuilderBid { let message = self.signing_root(domain); sk.sign(message) } + + // this helps differentiate a builder block from a regular block + fn stamp_payload(&mut self) { + let extra_data = mock_builder_extra_data::(); + match self.to_mut().header_mut() { + ExecutionPayloadHeaderRefMut::Bellatrix(header) => { + header.extra_data = extra_data; + header.block_hash = ExecutionBlockHash::from_root(header.tree_hash_root()); + } + ExecutionPayloadHeaderRefMut::Capella(header) => { + header.extra_data = extra_data; + header.block_hash = ExecutionBlockHash::from_root(header.tree_hash_root()); + } + ExecutionPayloadHeaderRefMut::Deneb(header) => { + header.extra_data = extra_data; + header.block_hash = ExecutionBlockHash::from_root(header.tree_hash_root()); + } + ExecutionPayloadHeaderRefMut::Electra(header) => { + header.extra_data = extra_data; + header.block_hash = ExecutionBlockHash::from_root(header.tree_hash_root()); + } + } + } } #[derive(Clone)] @@ -286,6 +315,7 @@ impl MockBuilder { while let Some(op) = guard.pop() { op.apply(bid); } + bid.stamp_payload(); } } @@ -413,11 +443,12 @@ pub fn serve( let block = head.data.message(); let head_block_root = block.tree_hash_root(); - let head_execution_hash = block + let head_execution_payload = block .body() .execution_payload() - .map_err(|_| reject("pre-merge block"))? - .block_hash(); + .map_err(|_| reject("pre-merge block"))?; + let head_execution_hash = head_execution_payload.block_hash(); + let head_gas_limit = head_execution_payload.gas_limit(); if head_execution_hash != parent_hash { return Err(reject("head mismatch")); } @@ -529,14 +560,24 @@ pub fn serve( finalized_hash: Some(finalized_execution_hash), }; + let proposer_gas_limit = builder + .val_registration_cache + .read() + .get(&pubkey) + .map(|v| v.message.gas_limit); + + let payload_parameters = PayloadParameters { + parent_hash: head_execution_hash, + parent_gas_limit: head_gas_limit, + proposer_gas_limit, + payload_attributes: &payload_attributes, + forkchoice_update_params: &forkchoice_update_params, + current_fork: fork, + }; + let payload_response_type = builder .el - .get_full_payload_caching( - head_execution_hash, - &payload_attributes, - forkchoice_update_params, - fork, - ) + .get_full_payload_caching(payload_parameters) .await .map_err(|_| reject("couldn't get payload"))?; @@ -648,8 +689,6 @@ pub fn serve( } }; - message.set_gas_limit(cached_data.gas_limit); - builder.apply_operations(&mut message); let mut signature = 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 a9f1313e462..48372a39be1 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 @@ -90,6 +90,7 @@ impl MockExecutionLayer { }; let parent_hash = latest_execution_block.block_hash(); + let parent_gas_limit = latest_execution_block.gas_limit(); let block_number = latest_execution_block.block_number() + 1; let timestamp = block_number; let prev_randao = Hash256::from_low_u64_be(block_number); @@ -131,14 +132,20 @@ impl MockExecutionLayer { let payload_attributes = PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None, None); + let payload_parameters = PayloadParameters { + parent_hash, + parent_gas_limit, + proposer_gas_limit: None, + payload_attributes: &payload_attributes, + forkchoice_update_params: &forkchoice_update_params, + current_fork: ForkName::Bellatrix, + }; + let block_proposal_content_type = self .el .get_payload( - parent_hash, - &payload_attributes, - forkchoice_update_params, + payload_parameters, builder_params, - ForkName::Bellatrix, &self.spec, None, BlockProductionVersion::FullV2, @@ -171,14 +178,20 @@ impl MockExecutionLayer { let payload_attributes = PayloadAttributes::new(timestamp, prev_randao, suggested_fee_recipient, None, None); + let payload_parameters = PayloadParameters { + parent_hash, + parent_gas_limit, + proposer_gas_limit: None, + payload_attributes: &payload_attributes, + forkchoice_update_params: &forkchoice_update_params, + current_fork: ForkName::Bellatrix, + }; + let block_proposal_content_type = self .el .get_payload( - parent_hash, - &payload_attributes, - forkchoice_update_params, + payload_parameters, builder_params, - ForkName::Bellatrix, &self.spec, None, BlockProductionVersion::BlindedV2, diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index 1e71fde2551..faf6d4ef0b6 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -25,12 +25,13 @@ use types::{EthSpec, ExecutionBlockHash, Uint256}; use warp::{http::StatusCode, Filter, Rejection}; use crate::EngineCapabilities; +pub use execution_block_generator::DEFAULT_GAS_LIMIT; pub use execution_block_generator::{ generate_blobs, generate_genesis_block, generate_genesis_header, generate_pow_block, - static_valid_tx, Block, ExecutionBlockGenerator, + mock_el_extra_data, static_valid_tx, Block, ExecutionBlockGenerator, }; pub use hook::Hook; -pub use mock_builder::{MockBuilder, Operation}; +pub use mock_builder::{mock_builder_extra_data, MockBuilder, Operation}; pub use mock_execution_layer::MockExecutionLayer; pub const DEFAULT_TERMINAL_DIFFICULTY: u64 = 6400; diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index fe05f55a01a..23d177da785 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -3704,7 +3704,10 @@ pub fn serve( ); execution_layer - .update_proposer_preparation(current_epoch, &preparation_data) + .update_proposer_preparation( + current_epoch, + preparation_data.iter().map(|data| (data, &None)), + ) .await; chain @@ -3762,7 +3765,7 @@ pub fn serve( let spec = &chain.spec; let (preparation_data, filtered_registration_data): ( - Vec, + Vec<(ProposerPreparationData, Option)>, Vec, ) = register_val_data .into_iter() @@ -3792,12 +3795,15 @@ pub fn serve( // Filter out validators who are not 'active' or 'pending'. is_active_or_pending.then_some({ ( - ProposerPreparationData { - validator_index: validator_index as u64, - fee_recipient: register_data - .message - .fee_recipient, - }, + ( + ProposerPreparationData { + validator_index: validator_index as u64, + fee_recipient: register_data + .message + .fee_recipient, + }, + Some(register_data.message.gas_limit), + ), register_data, ) }) @@ -3807,7 +3813,10 @@ pub fn serve( // Update the prepare beacon proposer cache based on this request. execution_layer - .update_proposer_preparation(current_epoch, &preparation_data) + .update_proposer_preparation( + current_epoch, + preparation_data.iter().map(|(data, limit)| (data, limit)), + ) .await; // Call prepare beacon proposer blocking with the latest update in order to make diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index c3ed3347821..627b0d0b179 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/beacon_node/http_api/tests/interactive_tests.rs @@ -447,9 +447,14 @@ pub async fn proposer_boost_re_org_test( // Send proposer preparation data for all validators. let proposer_preparation_data = all_validators .iter() - .map(|i| ProposerPreparationData { - validator_index: *i as u64, - fee_recipient: Address::from_low_u64_be(*i as u64), + .map(|i| { + ( + ProposerPreparationData { + validator_index: *i as u64, + fee_recipient: Address::from_low_u64_be(*i as u64), + }, + None, + ) }) .collect::>(); harness @@ -459,7 +464,7 @@ pub async fn proposer_boost_re_org_test( .unwrap() .update_proposer_preparation( head_slot.epoch(E::slots_per_epoch()) + 1, - &proposer_preparation_data, + proposer_preparation_data.iter().map(|(a, b)| (a, b)), ) .await; diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 940f3ae9c0c..080a393b4d0 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -13,8 +13,10 @@ use eth2::{ Error::ServerMessage, StatusCode, Timeouts, }; +use execution_layer::expected_gas_limit; use execution_layer::test_utils::{ - MockBuilder, Operation, DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI, + mock_builder_extra_data, mock_el_extra_data, MockBuilder, Operation, + DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_GAS_LIMIT, DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI, }; use futures::stream::{Stream, StreamExt}; use futures::FutureExt; @@ -348,7 +350,6 @@ impl ApiTester { let bls_to_execution_change = harness.make_bls_to_execution_change(4, Address::zero()); let chain = harness.chain.clone(); - let log = test_logger(); let ApiServer { @@ -3755,7 +3756,11 @@ impl ApiTester { self } - pub async fn test_post_validator_register_validator(self) -> Self { + async fn generate_validator_registration_data( + &self, + fee_recipient_generator: impl Fn(usize) -> Address, + gas_limit: u64, + ) -> (Vec, Vec
) { let mut registrations = vec![]; let mut fee_recipients = vec![]; @@ -3766,15 +3771,13 @@ impl ApiTester { epoch: genesis_epoch, }; - let expected_gas_limit = 11_111_111; - for (val_index, keypair) in self.validator_keypairs().iter().enumerate() { let pubkey = keypair.pk.compress(); - let fee_recipient = Address::from_low_u64_be(val_index as u64); + let fee_recipient = fee_recipient_generator(val_index); let data = ValidatorRegistrationData { fee_recipient, - gas_limit: expected_gas_limit, + gas_limit, timestamp: 0, pubkey, }; @@ -3797,6 +3800,17 @@ impl ApiTester { registrations.push(signed); } + (registrations, fee_recipients) + } + + pub async fn test_post_validator_register_validator(self) -> Self { + let (registrations, fee_recipients) = self + .generate_validator_registration_data( + |val_index| Address::from_low_u64_be(val_index as u64), + DEFAULT_GAS_LIMIT, + ) + .await; + self.client .post_validator_register_validator(®istrations) .await @@ -3811,14 +3825,22 @@ impl ApiTester { .zip(fee_recipients.into_iter()) .enumerate() { - let actual = self + let actual_fee_recipient = self .chain .execution_layer .as_ref() .unwrap() .get_suggested_fee_recipient(val_index as u64) .await; - assert_eq!(actual, fee_recipient); + let actual_gas_limit = self + .chain + .execution_layer + .as_ref() + .unwrap() + .get_proposer_gas_limit(val_index as u64) + .await; + assert_eq!(actual_fee_recipient, fee_recipient); + assert_eq!(actual_gas_limit, Some(DEFAULT_GAS_LIMIT)); } self @@ -3839,46 +3861,12 @@ impl ApiTester { ) .await; - let mut registrations = vec![]; - let mut fee_recipients = vec![]; - - let genesis_epoch = self.chain.spec.genesis_slot.epoch(E::slots_per_epoch()); - let fork = Fork { - current_version: self.chain.spec.genesis_fork_version, - previous_version: self.chain.spec.genesis_fork_version, - epoch: genesis_epoch, - }; - - let expected_gas_limit = 11_111_111; - - for (val_index, keypair) in self.validator_keypairs().iter().enumerate() { - let pubkey = keypair.pk.compress(); - let fee_recipient = Address::from_low_u64_be(val_index as u64); - - let data = ValidatorRegistrationData { - fee_recipient, - gas_limit: expected_gas_limit, - timestamp: 0, - pubkey, - }; - - let domain = self.chain.spec.get_domain( - genesis_epoch, - Domain::ApplicationMask(ApplicationDomain::Builder), - &fork, - Hash256::zero(), - ); - let message = data.signing_root(domain); - let signature = keypair.sk.sign(message); - - let signed = SignedValidatorRegistrationData { - message: data, - signature, - }; - - fee_recipients.push(fee_recipient); - registrations.push(signed); - } + let (registrations, fee_recipients) = self + .generate_validator_registration_data( + |val_index| Address::from_low_u64_be(val_index as u64), + DEFAULT_GAS_LIMIT, + ) + .await; self.client .post_validator_register_validator(®istrations) @@ -3911,6 +3899,47 @@ impl ApiTester { self } + pub async fn test_post_validator_register_validator_higher_gas_limit(&self) { + let (registrations, fee_recipients) = self + .generate_validator_registration_data( + |val_index| Address::from_low_u64_be(val_index as u64), + DEFAULT_GAS_LIMIT + 10_000_000, + ) + .await; + + self.client + .post_validator_register_validator(®istrations) + .await + .unwrap(); + + for (val_index, (_, fee_recipient)) in self + .chain + .head_snapshot() + .beacon_state + .validators() + .into_iter() + .zip(fee_recipients.into_iter()) + .enumerate() + { + let actual_fee_recipient = self + .chain + .execution_layer + .as_ref() + .unwrap() + .get_suggested_fee_recipient(val_index as u64) + .await; + let actual_gas_limit = self + .chain + .execution_layer + .as_ref() + .unwrap() + .get_proposer_gas_limit(val_index as u64) + .await; + assert_eq!(actual_fee_recipient, fee_recipient); + assert_eq!(actual_gas_limit, Some(DEFAULT_GAS_LIMIT + 10_000_000)); + } + } + pub async fn test_post_validator_liveness_epoch(self) -> Self { let epoch = self.chain.epoch().unwrap(); let head_state = self.chain.head_beacon_state_cloned(); @@ -4031,7 +4060,7 @@ impl ApiTester { let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64); assert_eq!(payload.fee_recipient(), expected_fee_recipient); - assert_eq!(payload.gas_limit(), 11_111_111); + assert_eq!(payload.gas_limit(), DEFAULT_GAS_LIMIT); self } @@ -4058,7 +4087,8 @@ impl ApiTester { let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64); assert_eq!(payload.fee_recipient(), expected_fee_recipient); - assert_eq!(payload.gas_limit(), 16_384); + // This is the graffiti of the mock execution layer, not the builder. + assert_eq!(payload.extra_data(), mock_el_extra_data::()); self } @@ -4085,7 +4115,7 @@ impl ApiTester { let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64); assert_eq!(payload.fee_recipient(), expected_fee_recipient); - assert_eq!(payload.gas_limit(), 11_111_111); + assert_eq!(payload.gas_limit(), DEFAULT_GAS_LIMIT); self } @@ -4109,7 +4139,7 @@ impl ApiTester { let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64); assert_eq!(payload.fee_recipient(), expected_fee_recipient); - assert_eq!(payload.gas_limit(), 11_111_111); + assert_eq!(payload.gas_limit(), DEFAULT_GAS_LIMIT); // If this cache is empty, it indicates fallback was not used, so the payload came from the // mock builder. @@ -4126,10 +4156,16 @@ impl ApiTester { pub async fn test_payload_accepts_mutated_gas_limit(self) -> Self { // Mutate gas limit. + let builder_limit = expected_gas_limit( + DEFAULT_GAS_LIMIT, + DEFAULT_GAS_LIMIT + 10_000_000, + self.chain.spec.as_ref(), + ) + .expect("calculate expected gas limit"); self.mock_builder .as_ref() .unwrap() - .add_operation(Operation::GasLimit(30_000_000)); + .add_operation(Operation::GasLimit(builder_limit as usize)); let slot = self.chain.slot().unwrap(); let epoch = self.chain.epoch().unwrap(); @@ -4149,7 +4185,7 @@ impl ApiTester { let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64); assert_eq!(payload.fee_recipient(), expected_fee_recipient); - assert_eq!(payload.gas_limit(), 30_000_000); + assert_eq!(payload.gas_limit(), builder_limit); // This cache should not be populated because fallback should not have been used. assert!(self @@ -4159,6 +4195,49 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_none()); + // Another way is to check for the extra data of the mock builder + assert_eq!(payload.extra_data(), mock_builder_extra_data::()); + + self + } + + pub async fn test_builder_payload_rejected_when_gas_limit_incorrect(self) -> Self { + self.test_post_validator_register_validator_higher_gas_limit() + .await; + + // Mutate gas limit. + self.mock_builder + .as_ref() + .unwrap() + .add_operation(Operation::GasLimit(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(); + + // If this cache is populated, it indicates fallback to the local EE was correctly used. + assert!(self + .chain + .execution_layer + .as_ref() + .unwrap() + .get_payload_by_root(&payload.tree_hash_root()) + .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); + self } @@ -4232,6 +4311,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_none()); + // Another way is to check for the extra data of the mock builder + assert_eq!(payload.extra_data(), mock_builder_extra_data::()); + self } @@ -4315,6 +4397,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); + self } @@ -4404,6 +4489,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); + self } @@ -4491,6 +4579,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); + self } @@ -4577,6 +4668,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); + self } @@ -4647,6 +4741,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); + self } @@ -4707,6 +4804,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); + self } @@ -4780,6 +4880,8 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_none()); + // Another way is to check for the extra data of the mock builder + assert_eq!(payload.extra_data(), mock_builder_extra_data::()); // Without proposing, advance into the next slot, this should make us cross the threshold // number of skips, causing us to use the fallback. @@ -4809,6 +4911,8 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); self } @@ -4915,6 +5019,8 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); // Fill another epoch with blocks, should be enough to finalize. (Sneaky plus 1 because this // scenario starts at an epoch boundary). @@ -4954,6 +5060,8 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_none()); + // Another way is to check for the extra data of the mock builder + assert_eq!(payload.extra_data(), mock_builder_extra_data::()); self } @@ -5072,6 +5180,8 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); self } @@ -5149,6 +5259,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_none()); + // Another way is to check for the extra data of the mock builder + assert_eq!(payload.extra_data(), mock_builder_extra_data::()); + self } @@ -5214,6 +5327,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); + self } @@ -5279,6 +5395,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_some()); + // another way is to check for the extra data of the local EE + assert_eq!(payload.extra_data(), mock_el_extra_data::()); + self } @@ -5343,6 +5462,9 @@ impl ApiTester { .unwrap() .get_payload_by_root(&payload.tree_hash_root()) .is_none()); + // Another way is to check for the extra data of the mock builder + assert_eq!(payload.extra_data(), mock_builder_extra_data::()); + self } @@ -6682,6 +6804,8 @@ async fn post_validator_register_valid_v3() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn post_validator_register_gas_limit_mutation() { ApiTester::new_mev_tester() + .await + .test_builder_payload_rejected_when_gas_limit_incorrect() .await .test_payload_accepts_mutated_gas_limit() .await; diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs index 79dcc65ea3c..0b33a76ff19 100644 --- a/consensus/types/src/chain_spec.rs +++ b/consensus/types/src/chain_spec.rs @@ -127,6 +127,11 @@ pub struct ChainSpec { pub deposit_network_id: u64, pub deposit_contract_address: Address, + /* + * Execution Specs + */ + pub gas_limit_adjustment_factor: u64, + /* * Altair hard fork params */ @@ -715,6 +720,11 @@ impl ChainSpec { .parse() .expect("chain spec deposit contract address"), + /* + * Execution Specs + */ + gas_limit_adjustment_factor: 1024, + /* * Altair hard fork params */ @@ -1029,6 +1039,11 @@ impl ChainSpec { .parse() .expect("chain spec deposit contract address"), + /* + * Execution Specs + */ + gas_limit_adjustment_factor: 1024, + /* * Altair hard fork params */ @@ -1285,6 +1300,10 @@ pub struct Config { #[serde(with = "serde_utils::address_hex")] deposit_contract_address: Address, + #[serde(default = "default_gas_limit_adjustment_factor")] + #[serde(with = "serde_utils::quoted_u64")] + gas_limit_adjustment_factor: u64, + #[serde(default = "default_gossip_max_size")] #[serde(with = "serde_utils::quoted_u64")] gossip_max_size: u64, @@ -1407,6 +1426,10 @@ const fn default_max_per_epoch_activation_churn_limit() -> u64 { 8 } +const fn default_gas_limit_adjustment_factor() -> u64 { + 1024 +} + const fn default_gossip_max_size() -> u64 { 10485760 } @@ -1659,6 +1682,8 @@ impl Config { deposit_network_id: spec.deposit_network_id, deposit_contract_address: spec.deposit_contract_address, + gas_limit_adjustment_factor: spec.gas_limit_adjustment_factor, + gossip_max_size: spec.gossip_max_size, max_request_blocks: spec.max_request_blocks, min_epochs_for_block_requests: spec.min_epochs_for_block_requests, @@ -1733,6 +1758,7 @@ impl Config { deposit_chain_id, deposit_network_id, deposit_contract_address, + gas_limit_adjustment_factor, gossip_max_size, min_epochs_for_block_requests, max_chunk_size, @@ -1794,6 +1820,7 @@ impl Config { deposit_chain_id, deposit_network_id, deposit_contract_address, + gas_limit_adjustment_factor, terminal_total_difficulty, terminal_block_hash, terminal_block_hash_activation_epoch, diff --git a/consensus/types/src/payload.rs b/consensus/types/src/payload.rs index b82a897da5d..e68801840af 100644 --- a/consensus/types/src/payload.rs +++ b/consensus/types/src/payload.rs @@ -32,6 +32,7 @@ pub trait ExecPayload: Debug + Clone + PartialEq + Hash + TreeHash + fn prev_randao(&self) -> Hash256; fn block_number(&self) -> u64; fn timestamp(&self) -> u64; + fn extra_data(&self) -> VariableList; fn block_hash(&self) -> ExecutionBlockHash; fn fee_recipient(&self) -> Address; fn gas_limit(&self) -> u64; @@ -225,6 +226,13 @@ impl ExecPayload for FullPayload { }) } + fn extra_data<'a>(&'a self) -> VariableList { + map_full_payload_ref!(&'a _, self.to_ref(), move |payload, cons| { + cons(payload); + payload.execution_payload.extra_data.clone() + }) + } + fn block_hash<'a>(&'a self) -> ExecutionBlockHash { map_full_payload_ref!(&'a _, self.to_ref(), move |payload, cons| { cons(payload); @@ -357,6 +365,13 @@ impl ExecPayload for FullPayloadRef<'_, E> { }) } + fn extra_data<'a>(&'a self) -> VariableList { + map_full_payload_ref!(&'a _, self, move |payload, cons| { + cons(payload); + payload.execution_payload.extra_data.clone() + }) + } + fn block_hash<'a>(&'a self) -> ExecutionBlockHash { map_full_payload_ref!(&'a _, self, move |payload, cons| { cons(payload); @@ -542,6 +557,13 @@ impl ExecPayload for BlindedPayload { }) } + fn extra_data<'a>(&'a self) -> VariableList::MaxExtraDataBytes> { + map_blinded_payload_ref!(&'a _, self.to_ref(), move |payload, cons| { + cons(payload); + payload.execution_payload_header.extra_data.clone() + }) + } + fn block_hash<'a>(&'a self) -> ExecutionBlockHash { map_blinded_payload_ref!(&'a _, self.to_ref(), move |payload, cons| { cons(payload); @@ -643,6 +665,13 @@ impl<'b, E: EthSpec> ExecPayload for BlindedPayloadRef<'b, E> { }) } + fn extra_data<'a>(&'a self) -> VariableList::MaxExtraDataBytes> { + map_blinded_payload_ref!(&'a _, self, move |payload, cons| { + cons(payload); + payload.execution_payload_header.extra_data.clone() + }) + } + fn block_hash<'a>(&'a self) -> ExecutionBlockHash { map_blinded_payload_ref!(&'a _, self, move |payload, cons| { cons(payload); @@ -745,6 +774,10 @@ macro_rules! impl_exec_payload_common { self.$wrapped_field.timestamp } + fn extra_data(&self) -> VariableList { + self.$wrapped_field.extra_data.clone() + } + fn block_hash(&self) -> ExecutionBlockHash { self.$wrapped_field.block_hash } diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 7d4d229fef7..427bcf5e9c5 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -809,10 +809,13 @@ impl Tester { if expected_should_override_fcu.validator_is_connected { el.update_proposer_preparation( next_slot_epoch, - &[ProposerPreparationData { - validator_index: dbg!(proposer_index) as u64, - fee_recipient: Default::default(), - }], + [( + &ProposerPreparationData { + validator_index: dbg!(proposer_index) as u64, + fee_recipient: Default::default(), + }, + &None, + )], ) .await; } else { diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index 0289fd4206b..f6645093049 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -3,9 +3,10 @@ use crate::execution_engine::{ }; use crate::transactions::transactions; use ethers_providers::Middleware; +use execution_layer::test_utils::DEFAULT_GAS_LIMIT; use execution_layer::{ BlockProposalContentsType, BuilderParams, ChainHealth, ExecutionLayer, PayloadAttributes, - PayloadStatus, + PayloadParameters, PayloadStatus, }; use fork_choice::ForkchoiceUpdateParameters; use reqwest::{header::CONTENT_TYPE, Client}; @@ -251,6 +252,7 @@ impl TestRig { */ let parent_hash = terminal_pow_block_hash; + let parent_gas_limit = DEFAULT_GAS_LIMIT; let timestamp = timestamp_now(); let prev_randao = Hash256::zero(); let head_root = Hash256::zero(); @@ -324,15 +326,22 @@ impl TestRig { Some(vec![]), None, ); + + let payload_parameters = PayloadParameters { + parent_hash, + parent_gas_limit, + proposer_gas_limit: None, + payload_attributes: &payload_attributes, + forkchoice_update_params: &forkchoice_update_params, + current_fork: TEST_FORK, + }; + let block_proposal_content_type = self .ee_a .execution_layer .get_payload( - parent_hash, - &payload_attributes, - forkchoice_update_params, + payload_parameters, builder_params, - TEST_FORK, &self.spec, None, BlockProductionVersion::FullV2, @@ -476,15 +485,22 @@ impl TestRig { Some(vec![]), None, ); + + let payload_parameters = PayloadParameters { + parent_hash, + parent_gas_limit, + proposer_gas_limit: None, + payload_attributes: &payload_attributes, + forkchoice_update_params: &forkchoice_update_params, + current_fork: TEST_FORK, + }; + let block_proposal_content_type = self .ee_a .execution_layer .get_payload( - parent_hash, - &payload_attributes, - forkchoice_update_params, + payload_parameters, builder_params, - TEST_FORK, &self.spec, None, BlockProductionVersion::FullV2,