diff --git a/Makefile b/Makefile index c1190ac98cf..fc381da1d84 100644 --- a/Makefile +++ b/Makefile @@ -205,7 +205,7 @@ cli: docker run --rm --user=root \ -v ${PWD}:/home/runner/actions-runner/lighthouse sigmaprime/github-runner \ bash -c 'cd lighthouse && make && ./scripts/cli.sh' - + # Runs the entire test suite, downloading test vectors if required. test-full: cargo-fmt test-release test-debug test-ef test-exec-engine diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 7d1e058a1b2..887a2411a87 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -4021,6 +4021,7 @@ impl BeaconChain { slot: Slot, validator_graffiti: Option, verification: ProduceBlockVerification, + builder_boost_factor: Option, block_production_version: BlockProductionVersion, ) -> Result, BlockProductionError> { metrics::inc_counter(&metrics::BLOCK_PRODUCTION_REQUESTS); @@ -4049,6 +4050,7 @@ impl BeaconChain { randao_reveal, validator_graffiti, verification, + builder_boost_factor, block_production_version, ) .await @@ -4652,6 +4654,7 @@ impl BeaconChain { randao_reveal: Signature, validator_graffiti: Option, verification: ProduceBlockVerification, + builder_boost_factor: Option, block_production_version: BlockProductionVersion, ) -> Result, BlockProductionError> { // Part 1/3 (blocking) @@ -4668,6 +4671,7 @@ impl BeaconChain { produce_at_slot, randao_reveal, validator_graffiti, + builder_boost_factor, block_production_version, ) }, @@ -4757,6 +4761,7 @@ impl BeaconChain { } } + #[allow(clippy::too_many_arguments)] fn produce_partial_beacon_block( self: &Arc, mut state: BeaconState, @@ -4764,6 +4769,7 @@ impl BeaconChain { produce_at_slot: Slot, randao_reveal: Signature, validator_graffiti: Option, + builder_boost_factor: Option, block_production_version: BlockProductionVersion, ) -> Result, BlockProductionError> { let eth1_chain = self @@ -4825,6 +4831,7 @@ impl BeaconChain { parent_root, proposer_index, builder_params, + builder_boost_factor, block_production_version, )?; Some(prepare_payload_handle) diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 093255b201e..e25976c2a57 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -405,6 +405,7 @@ pub fn get_execution_payload( parent_block_root: Hash256, proposer_index: u64, builder_params: BuilderParams, + builder_boost_factor: Option, block_production_version: BlockProductionVersion, ) -> Result, BlockProductionError> { // Compute all required values from the `state` now to avoid needing to pass it into a spawned @@ -449,6 +450,7 @@ pub fn get_execution_payload( builder_params, withdrawals, parent_beacon_block_root, + builder_boost_factor, block_production_version, ) .await @@ -485,6 +487,7 @@ pub async fn prepare_execution_payload( builder_params: BuilderParams, withdrawals: Option>, parent_beacon_block_root: Option, + builder_boost_factor: Option, block_production_version: BlockProductionVersion, ) -> Result, BlockProductionError> where @@ -575,6 +578,7 @@ where builder_params, fork, &chain.spec, + builder_boost_factor, block_production_version, ) .await diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index eb73478dee9..639ff595afd 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -464,14 +464,13 @@ where } pub fn mock_execution_layer(self) -> Self { - self.mock_execution_layer_with_config(None) + self.mock_execution_layer_with_config() } - pub fn mock_execution_layer_with_config(mut self, builder_threshold: Option) -> Self { + pub fn mock_execution_layer_with_config(mut self) -> Self { let mock = mock_execution_layer_from_parts::( self.spec.as_ref().expect("cannot build without spec"), self.runtime.task_executor.clone(), - builder_threshold, ); self.execution_layer = Some(mock.el.clone()); self.mock_execution_layer = Some(mock); @@ -574,7 +573,6 @@ where pub fn mock_execution_layer_from_parts( spec: &ChainSpec, task_executor: TaskExecutor, - builder_threshold: Option, ) -> MockExecutionLayer { let shanghai_time = spec.capella_fork_epoch.map(|epoch| { HARNESS_GENESIS_TIME + spec.seconds_per_slot * T::slots_per_epoch() * epoch.as_u64() @@ -593,7 +591,6 @@ pub fn mock_execution_layer_from_parts( DEFAULT_TERMINAL_BLOCK, shanghai_time, cancun_time, - builder_threshold, Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()), spec.clone(), Some(kzg), @@ -860,6 +857,7 @@ where randao_reveal, Some(graffiti), ProduceBlockVerification::VerifyRandao, + None, BlockProductionVersion::FullV2, ) .await @@ -921,6 +919,7 @@ where randao_reveal, Some(graffiti), ProduceBlockVerification::VerifyRandao, + None, BlockProductionVersion::FullV2, ) .await diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 8ba099ec73c..28770b93e24 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -2421,7 +2421,7 @@ async fn weak_subjectivity_sync_test(slots: Vec, checkpoint_slot: Slot) { .unwrap(); let mock = - mock_execution_layer_from_parts(&harness.spec, harness.runtime.task_executor.clone(), None); + mock_execution_layer_from_parts(&harness.spec, harness.runtime.task_executor.clone()); // Initialise a new beacon chain from the finalized checkpoint. // The slot clock must be set to a time ahead of the checkpoint state. diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 6841f4d50ef..868d8194466 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -335,10 +335,7 @@ struct Inner { proposers: RwLock>, executor: TaskExecutor, payload_cache: PayloadCache, - builder_profit_threshold: Uint256, log: Logger, - always_prefer_builder_payload: bool, - ignore_builder_override_suggestion_threshold: f32, /// Track whether the last `newPayload` call errored. /// /// This is used *only* in the informational sync status endpoint, so that a VC using this @@ -365,11 +362,7 @@ pub struct Config { pub jwt_version: Option, /// Default directory for the jwt secret if not provided through cli. pub default_datadir: PathBuf, - /// The minimum value of an external payload for it to be considered in a proposal. - pub builder_profit_threshold: u128, pub execution_timeout_multiplier: Option, - pub always_prefer_builder_payload: bool, - pub ignore_builder_override_suggestion_threshold: f32, } /// Provides access to one execution engine and provides a neat interface for consumption by the @@ -379,40 +372,6 @@ pub struct ExecutionLayer { inner: Arc>, } -/// This function will return the percentage difference between 2 U256 values, using `base_value` -/// as the denominator. It is accurate to 7 decimal places which is about the precision of -/// an f32. -/// -/// If some error is encountered in the calculation, None will be returned. -fn percentage_difference_u256(base_value: Uint256, comparison_value: Uint256) -> Option { - if base_value == Uint256::zero() { - return None; - } - // this is the total supply of ETH in WEI - let max_value = Uint256::from(12u8) * Uint256::exp10(25); - if base_value > max_value || comparison_value > max_value { - return None; - } - - // Now we should be able to calculate the difference without division by zero or overflow - const PRECISION: usize = 7; - let precision_factor = Uint256::exp10(PRECISION); - let scaled_difference = if base_value <= comparison_value { - (comparison_value - base_value) * precision_factor - } else { - (base_value - comparison_value) * precision_factor - }; - let scaled_proportion = scaled_difference / base_value; - // max value of scaled difference is 1.2 * 10^33, well below the max value of a u128 / f64 / f32 - let percentage = - 100.0f64 * scaled_proportion.low_u128() as f64 / precision_factor.low_u128() as f64; - if base_value <= comparison_value { - Some(percentage as f32) - } else { - Some(-percentage as f32) - } -} - impl ExecutionLayer { /// Instantiate `Self` with an Execution engine specified in `Config`, using JSON-RPC via HTTP. pub fn from_config(config: Config, executor: TaskExecutor, log: Logger) -> Result { @@ -425,10 +384,7 @@ impl ExecutionLayer { jwt_id, jwt_version, default_datadir, - builder_profit_threshold, execution_timeout_multiplier, - always_prefer_builder_payload, - ignore_builder_override_suggestion_threshold, } = config; if urls.len() > 1 { @@ -489,10 +445,7 @@ impl ExecutionLayer { execution_blocks: Mutex::new(LruCache::new(EXECUTION_BLOCKS_LRU_CACHE_SIZE)), executor, payload_cache: PayloadCache::default(), - builder_profit_threshold: Uint256::from(builder_profit_threshold), log, - always_prefer_builder_payload, - ignore_builder_override_suggestion_threshold, last_new_payload_errored: RwLock::new(false), }; @@ -530,7 +483,6 @@ impl ExecutionLayer { self.log(), "Using external block builder"; "builder_url" => ?builder_url, - "builder_profit_threshold" => self.inner.builder_profit_threshold.as_u128(), "local_user_agent" => builder_client.get_user_agent(), ); self.inner.builder.swap(Some(Arc::new(builder_client))); @@ -836,6 +788,7 @@ impl ExecutionLayer { builder_params: BuilderParams, current_fork: ForkName, spec: &ChainSpec, + builder_boost_factor: Option, block_production_version: BlockProductionVersion, ) -> Result, Error> { let payload_result_type = match block_production_version { @@ -846,6 +799,7 @@ impl ExecutionLayer { forkchoice_update_params, builder_params, current_fork, + builder_boost_factor, spec, ) .await @@ -870,6 +824,7 @@ impl ExecutionLayer { forkchoice_update_params, builder_params, current_fork, + None, spec, ) .await? @@ -990,6 +945,7 @@ impl ExecutionLayer { (relay_result, local_result) } + #[allow(clippy::too_many_arguments)] async fn determine_and_fetch_payload( &self, parent_hash: ExecutionBlockHash, @@ -997,6 +953,7 @@ impl ExecutionLayer { forkchoice_update_params: ForkchoiceUpdateParameters, builder_params: BuilderParams, current_fork: ForkName, + builder_boost_factor: Option, spec: &ChainSpec, ) -> Result>, Error> { let Some(builder) = self.builder() else { @@ -1148,62 +1105,50 @@ impl ExecutionLayer { ))); } - if self.inner.always_prefer_builder_payload { - return ProvenancedPayload::try_from(relay.data.message); - } - let relay_value = *relay.data.message.value(); + + let boosted_relay_value = match builder_boost_factor { + Some(builder_boost_factor) => { + (relay_value / 100).saturating_mul(builder_boost_factor.into()) + } + None => relay_value, + }; + let local_value = *local.block_value(); - if local_value >= relay_value { + if local_value >= boosted_relay_value { info!( self.log(), "Local block is more profitable than relay block"; "local_block_value" => %local_value, - "relay_value" => %relay_value + "relay_value" => %relay_value, + "boosted_relay_value" => %boosted_relay_value, + "builder_boost_factor" => ?builder_boost_factor, ); return Ok(ProvenancedPayload::Local(BlockProposalContentsType::Full( local.try_into()?, ))); } - if relay_value < self.inner.builder_profit_threshold { + if local.should_override_builder().unwrap_or(false) { info!( self.log(), - "Builder payload ignored"; - "info" => "using local payload", - "reason" => format!("payload value of {} does not meet user-configured profit-threshold of {}", relay_value, self.inner.builder_profit_threshold), - "relay_block_hash" => ?header.block_hash(), - "parent_hash" => ?parent_hash, + "Using local payload because execution engine suggested we ignore builder payload"; + "local_block_value" => %local_value, + "relay_value" => %relay_value ); return Ok(ProvenancedPayload::Local(BlockProposalContentsType::Full( local.try_into()?, ))); } - if local.should_override_builder().unwrap_or(false) { - let percentage_difference = - percentage_difference_u256(local_value, relay_value); - if percentage_difference.map_or(false, |percentage| { - percentage < self.inner.ignore_builder_override_suggestion_threshold - }) { - info!( - self.log(), - "Using local payload because execution engine suggested we ignore builder payload"; - "local_block_value" => %local_value, - "relay_value" => %relay_value - ); - return Ok(ProvenancedPayload::Local(BlockProposalContentsType::Full( - local.try_into()?, - ))); - } - } - info!( self.log(), "Relay block is more profitable than local block"; "local_block_value" => %local_value, - "relay_value" => %relay_value + "relay_value" => %relay_value, + "boosted_relay_value" => %boosted_relay_value, + "builder_boost_factor" => ?builder_boost_factor ); Ok(ProvenancedPayload::try_from(relay.data.message)?) @@ -2374,42 +2319,4 @@ mod test { }) .await; } - - #[tokio::test] - async fn percentage_difference_u256_tests() { - // ensure function returns `None` when base value is zero - assert_eq!(percentage_difference_u256(0.into(), 1.into()), None); - // ensure function returns `None` when either value is greater than 120 Million ETH - let max_value = Uint256::from(12u8) * Uint256::exp10(25); - assert_eq!( - percentage_difference_u256(1u8.into(), max_value + Uint256::from(1u8)), - None - ); - assert_eq!( - percentage_difference_u256(max_value + Uint256::from(1u8), 1u8.into()), - None - ); - // it should work up to max value - assert_eq!( - percentage_difference_u256(max_value, max_value / Uint256::from(2u8)), - Some(-50f32) - ); - // should work when base value is greater than comparison value - assert_eq!( - percentage_difference_u256(4u8.into(), 3u8.into()), - Some(-25f32) - ); - // should work when comparison value is greater than base value - assert_eq!( - percentage_difference_u256(4u8.into(), 5u8.into()), - Some(25f32) - ); - // should be accurate to 7 decimal places - let result = - percentage_difference_u256(Uint256::from(31415926u64), Uint256::from(13371337u64)) - .expect("should get percentage"); - // result = -57.4377116 - assert!(result > -57.43772); - assert!(result <= -57.43771); - } } 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 72f0388e24d..7afeafc321e 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 @@ -1,7 +1,6 @@ use crate::{ test_utils::{ - MockServer, DEFAULT_BUILDER_THRESHOLD_WEI, DEFAULT_JWT_SECRET, DEFAULT_TERMINAL_BLOCK, - DEFAULT_TERMINAL_DIFFICULTY, + MockServer, DEFAULT_JWT_SECRET, DEFAULT_TERMINAL_BLOCK, DEFAULT_TERMINAL_DIFFICULTY, }, Config, *, }; @@ -30,7 +29,6 @@ impl MockExecutionLayer { DEFAULT_TERMINAL_BLOCK, None, None, - None, Some(JwtKey::from_slice(&DEFAULT_JWT_SECRET).unwrap()), spec, None, @@ -43,7 +41,6 @@ impl MockExecutionLayer { terminal_block: u64, shanghai_time: Option, cancun_time: Option, - builder_threshold: Option, jwt_key: Option, spec: ChainSpec, kzg: Option, @@ -72,7 +69,6 @@ impl MockExecutionLayer { execution_endpoints: vec![url], secret_files: vec![path], suggested_fee_recipient: Some(Address::repeat_byte(42)), - builder_profit_threshold: builder_threshold.unwrap_or(DEFAULT_BUILDER_THRESHOLD_WEI), ..Default::default() }; let el = @@ -143,6 +139,7 @@ impl MockExecutionLayer { builder_params, ForkName::Merge, &self.spec, + None, BlockProductionVersion::FullV2, ) .await @@ -182,6 +179,7 @@ impl MockExecutionLayer { builder_params, ForkName::Merge, &self.spec, + None, BlockProductionVersion::BlindedV2, ) .await diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index 92be94e6031..f0be5111472 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -35,7 +35,6 @@ pub use mock_execution_layer::MockExecutionLayer; 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 { diff --git a/beacon_node/http_api/src/produce_block.rs b/beacon_node/http_api/src/produce_block.rs index ff1c7d345c2..3ab66614e4e 100644 --- a/beacon_node/http_api/src/produce_block.rs +++ b/beacon_node/http_api/src/produce_block.rs @@ -60,6 +60,7 @@ pub async fn produce_block_v3( slot, query.graffiti.map(Into::into), randao_verification, + query.builder_boost_factor, BlockProductionVersion::V3, ) .await @@ -140,6 +141,7 @@ pub async fn produce_blinded_block_v2( slot, query.graffiti.map(Into::into), randao_verification, + None, BlockProductionVersion::BlindedV2, ) .await @@ -170,6 +172,7 @@ pub async fn produce_block_v2( slot, query.graffiti.map(Into::into), randao_verification, + None, BlockProductionVersion::FullV2, ) .await diff --git a/beacon_node/http_api/tests/interactive_tests.rs b/beacon_node/http_api/tests/interactive_tests.rs index 945f538a3a7..210c4d2550c 100644 --- a/beacon_node/http_api/tests/interactive_tests.rs +++ b/beacon_node/http_api/tests/interactive_tests.rs @@ -620,7 +620,7 @@ pub async fn proposer_boost_re_org_test( .into(); let (unsigned_block_type, _) = tester .client - .get_validator_blocks_v3::(slot_c, &randao_reveal, None) + .get_validator_blocks_v3::(slot_c, &randao_reveal, None, None) .await .unwrap(); diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 1b694ecf9d1..300c5a10600 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -13,8 +13,7 @@ use eth2::{ BeaconNodeHttpClient, Error, StatusCode, Timeouts, }; use execution_layer::test_utils::{ - MockBuilder, Operation, DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_BUILDER_THRESHOLD_WEI, - DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI, + MockBuilder, Operation, DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI, }; use futures::stream::{Stream, StreamExt}; use futures::FutureExt; @@ -80,7 +79,6 @@ struct ApiTester { struct ApiTesterConfig { spec: ChainSpec, retain_historic_states: bool, - builder_threshold: Option, } impl Default for ApiTesterConfig { @@ -90,7 +88,6 @@ impl Default for ApiTesterConfig { Self { spec, retain_historic_states: false, - builder_threshold: None, } } } @@ -132,7 +129,7 @@ impl ApiTester { .logger(logging::test_logger()) .deterministic_keypairs(VALIDATOR_COUNT) .fresh_ephemeral_store() - .mock_execution_layer_with_config(config.builder_threshold) + .mock_execution_layer_with_config() .build(); harness @@ -391,19 +388,12 @@ impl ApiTester { .test_post_validator_register_validator() .await; // Make sure bids always meet the minimum threshold. - tester - .mock_builder - .as_ref() - .unwrap() - .add_operation(Operation::Value(Uint256::from( - DEFAULT_BUILDER_THRESHOLD_WEI, - ))); + tester.mock_builder.as_ref().unwrap(); tester } - pub async fn new_mev_tester_no_builder_threshold() -> Self { + pub async fn new_mev_tester_default_payload_value() -> Self { let mut config = ApiTesterConfig { - builder_threshold: Some(0), retain_historic_states: false, spec: E::default_spec(), }; @@ -2730,7 +2720,7 @@ impl ApiTester { let (response, metadata) = self .client - .get_validator_blocks_v3_ssz::(slot, &randao_reveal, None) + .get_validator_blocks_v3_ssz::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -3553,7 +3543,59 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) + .await + .unwrap(); + + let payload: BlindedPayload = match payload_type.data { + ProduceBlockV3Response::Blinded(payload) => { + payload.body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Full(_) => panic!("Expecting a blinded payload"), + }; + + 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); + + self + } + + pub async fn test_payload_v3_zero_builder_boost_factor(self) -> Self { + let slot = self.chain.slot().unwrap(); + let epoch = self.chain.epoch().unwrap(); + + let (proposer_index, randao_reveal) = self.get_test_randao(slot, epoch).await; + + let (payload_type, _) = self + .client + .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(0)) + .await + .unwrap(); + + let payload: FullPayload = match payload_type.data { + ProduceBlockV3Response::Full(payload) => { + payload.block().body().execution_payload().unwrap().into() + } + ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), + }; + + 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); + + self + } + + pub async fn test_payload_v3_max_builder_boost_factor(self) -> Self { + let slot = self.chain.slot().unwrap(); + let epoch = self.chain.epoch().unwrap(); + + let (proposer_index, randao_reveal) = self.get_test_randao(slot, epoch).await; + + let (payload_type, _) = self + .client + .get_validator_blocks_v3::(slot, &randao_reveal, None, Some(u64::MAX)) .await .unwrap(); @@ -3657,7 +3699,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -3733,7 +3775,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -3823,7 +3865,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -3909,7 +3951,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -3995,7 +4037,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -4079,7 +4121,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -4135,7 +4177,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -4201,7 +4243,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -4309,7 +4351,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) .await .unwrap(); @@ -4329,7 +4371,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) .await .unwrap(); @@ -4457,7 +4499,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) .await .unwrap(); @@ -4487,7 +4529,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(next_slot, &randao_reveal, None) + .get_validator_blocks_v3::(next_slot, &randao_reveal, None, None) .await .unwrap(); @@ -4567,7 +4609,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -4584,70 +4626,6 @@ impl ApiTester { self } - pub async fn test_payload_rejects_inadequate_builder_threshold(self) -> Self { - // Mutate value. - self.mock_builder - .as_ref() - .unwrap() - .add_operation(Operation::Value(Uint256::from( - DEFAULT_BUILDER_THRESHOLD_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(); - - // 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()); - self - } - - pub async fn test_payload_v3_rejects_inadequate_builder_threshold(self) -> Self { - // Mutate value. - self.mock_builder - .as_ref() - .unwrap() - .add_operation(Operation::Value(Uint256::from( - DEFAULT_BUILDER_THRESHOLD_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_type, _) = self - .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) - .await - .unwrap(); - - match payload_type.data { - ProduceBlockV3Response::Full(_) => (), - ProduceBlockV3Response::Blinded(_) => panic!("Expecting a full payload"), - }; - - self - } - pub async fn test_builder_payload_chosen_when_more_profitable(self) -> Self { // Mutate value. self.mock_builder @@ -4700,7 +4678,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -4764,7 +4742,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -4828,7 +4806,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -4890,7 +4868,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -4962,7 +4940,7 @@ impl ApiTester { let (payload_type, _) = self .client - .get_validator_blocks_v3::(slot, &randao_reveal, None) + .get_validator_blocks_v3::(slot, &randao_reveal, None, None) .await .unwrap(); @@ -6048,6 +6026,22 @@ async fn post_validator_register_valid() { .await; } +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn post_validator_zero_builder_boost_factor() { + ApiTester::new_mev_tester() + .await + .test_payload_v3_zero_builder_boost_factor() + .await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn post_validator_max_builder_boost_factor() { + ApiTester::new_mev_tester() + .await + .test_payload_v3_max_builder_boost_factor() + .await; +} + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn post_validator_register_valid_v3() { ApiTester::new_mev_tester() @@ -6232,25 +6226,9 @@ async fn builder_chain_health_optimistic_head_v3() { .await; } -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn builder_inadequate_builder_threshold() { - ApiTester::new_mev_tester() - .await - .test_payload_rejects_inadequate_builder_threshold() - .await; -} - -#[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn builder_inadequate_builder_threshold_v3() { - ApiTester::new_mev_tester() - .await - .test_payload_v3_rejects_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() + ApiTester::new_mev_tester_default_payload_value() .await .test_builder_payload_chosen_when_more_profitable() .await @@ -6262,7 +6240,7 @@ async fn builder_payload_chosen_by_profit() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn builder_payload_chosen_by_profit_v3() { - ApiTester::new_mev_tester_no_builder_threshold() + ApiTester::new_mev_tester_default_payload_value() .await .test_builder_payload_v3_chosen_when_more_profitable() .await @@ -6275,7 +6253,6 @@ async fn builder_payload_chosen_by_profit_v3() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn builder_works_post_capella() { let mut config = ApiTesterConfig { - builder_threshold: Some(0), retain_historic_states: false, spec: E::default_spec(), }; @@ -6296,7 +6273,6 @@ async fn builder_works_post_capella() { #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn builder_works_post_deneb() { let mut config = ApiTesterConfig { - builder_threshold: Some(0), retain_historic_states: false, spec: E::default_spec(), }; diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 0fc485b1526..6dba679aadc 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -1131,32 +1131,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { Arg::with_name("builder-profit-threshold") .long("builder-profit-threshold") .value_name("WEI_VALUE") - .help("The minimum reward in wei provided to the proposer by a block builder for \ - an external payload to be considered for inclusion in a proposal. If this \ - threshold is not met, the local EE's payload will be used. This is currently \ - *NOT* in comparison to the value of the local EE's payload. It simply checks \ - whether the total proposer reward from an external payload is equal to or \ - greater than this value. In the future, a comparison to a local payload is \ - likely to be added. Example: Use 250000000000000000 to set the threshold to \ - 0.25 ETH.") - .default_value("0") - .takes_value(true) - ) - .arg( - Arg::with_name("ignore-builder-override-suggestion-threshold") - .long("ignore-builder-override-suggestion-threshold") - .value_name("PERCENTAGE") - .help("When the EE advises Lighthouse to ignore the builder payload, this flag \ - specifies a percentage threshold for the difference between the reward from \ - the builder payload and the local EE's payload. This threshold must be met \ - for Lighthouse to consider ignoring the EE's suggestion. If the reward from \ - the builder's payload doesn't exceed the local payload by at least this \ - percentage, the local payload will be used. The conditions under which the \ - EE may make this suggestion depend on the EE's implementation, with the \ - primary intent being to safeguard against potential censorship attacks \ - from builders. Setting this flag to 0 will cause Lighthouse to always \ - ignore the EE's suggestion. Default: 10.0 (equivalent to 10%).") - .default_value("10.0") + .help("This flag is deprecated and has no effect.") .takes_value(true) ) .arg( @@ -1208,12 +1183,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .arg( Arg::with_name("always-prefer-builder-payload") .long("always-prefer-builder-payload") - .help("If set, the beacon node always uses the payload from the builder instead of the local payload.") - // The builder profit threshold flag is used to provide preference - // to local payloads, therefore it fundamentally conflicts with - // always using the builder. - .conflicts_with("builder-profit-threshold") - .conflicts_with("ignore-builder-override-suggestion-threshold") + .help("This flag is deprecated and has no effect.") ) .arg( Arg::with_name("invalid-gossip-verified-blocks-path") diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 7c48ae76092..6b58f13b8ac 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -309,6 +309,21 @@ pub fn get_config( clap_utils::parse_optional(cli_args, "builder-user-agent")?; } + if cli_args.is_present("builder-profit-threshold") { + warn!( + log, + "Ignoring --builder-profit-threshold"; + "info" => "this flag is deprecated and will be removed" + ); + } + if cli_args.is_present("always-prefer-builder-payload") { + warn!( + log, + "Ignoring --always-prefer-builder-payload"; + "info" => "this flag is deprecated and will be removed" + ); + } + // Set config values from parse values. el_config.secret_files = vec![secret_file.clone()]; el_config.execution_endpoints = vec![execution_endpoint.clone()]; @@ -317,12 +332,6 @@ pub fn get_config( el_config.jwt_id = clap_utils::parse_optional(cli_args, "execution-jwt-id")?; el_config.jwt_version = clap_utils::parse_optional(cli_args, "execution-jwt-version")?; el_config.default_datadir = client_config.data_dir().clone(); - el_config.builder_profit_threshold = - clap_utils::parse_required(cli_args, "builder-profit-threshold")?; - el_config.always_prefer_builder_payload = - cli_args.is_present("always-prefer-builder-payload"); - el_config.ignore_builder_override_suggestion_threshold = - clap_utils::parse_required(cli_args, "ignore-builder-override-suggestion-threshold")?; let execution_timeout_multiplier = clap_utils::parse_required(cli_args, "execution-timeout-multiplier")?; el_config.execution_timeout_multiplier = Some(execution_timeout_multiplier); diff --git a/book/src/builders.md b/book/src/builders.md index d5cb4c61b2e..e48cc0a884c 100644 --- a/book/src/builders.md +++ b/book/src/builders.md @@ -176,31 +176,6 @@ By default, Lighthouse is strict with these conditions, but we encourage users t - `--builder-fallback-disable-checks` - This flag disables all checks related to chain health. This means the builder API will always be used for payload construction, regardless of recent chain conditions. -## Builder Profit Threshold - -If you are generally uneasy with the risks associated with outsourced payload production (liveness/censorship) but would -consider using it for the chance of out-sized rewards, this flag may be useful: - -`--builder-profit-threshold ` - -The number provided indicates the minimum reward that an external payload must provide the proposer for it to be considered -for inclusion in a proposal. For example, if you'd only like to use an external payload for a reward of >= 0.25 ETH, you -would provide your beacon node with `--builder-profit-threshold 250000000000000000`. If it's your turn to propose and the -most valuable payload offered by builders is only 0.1 ETH, the local execution engine's payload will be used. - -Since the [Capella](https://ethereum.org/en/history/#capella) upgrade, a comparison of the external payload and local payload will be made according to the [engine_getPayloadV2](https://github.com/ethereum/execution-apis/blob/main/src/engine/shanghai.md#engine_getpayloadv2) API. The logic is as follows: - -``` -if local payload value >= builder payload value: - use local payload -else if builder payload value >= builder_profit_threshold or builder_profit_threshold == 0: - use builder payload -else: - use local payload -``` - -If you would like to always use the builder payload, you can add the flag `--always-prefer-builder-payload` to the beacon node. - ## Checking your builder config You can check that your builder is configured correctly by looking for these log messages. diff --git a/book/src/help_bn.md b/book/src/help_bn.md index 5ab7c3849bd..06cef220083 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -9,8 +9,7 @@ USAGE: lighthouse beacon_node [FLAGS] [OPTIONS] FLAGS: - --always-prefer-builder-payload If set, the beacon node always uses the payload from the builder instead - of the local payload. + --always-prefer-builder-payload This flag is deprecated and has no effect. --always-prepare-payload Send payload attributes with every fork choice update. This is intended for use by block builders, relays and developers. You should set a fee recipient on this BN and also consider adjusting the --prepare-payload- @@ -175,12 +174,8 @@ OPTIONS: `SLOTS_PER_EPOCH`, it will NOT query any connected builders, and will use the local execution engine for payload construction. [default: 8] --builder-profit-threshold - The minimum reward in wei provided to the proposer by a block builder for an external payload to be - considered for inclusion in a proposal. If this threshold is not met, the local EE's payload will be used. - This is currently *NOT* in comparison to the value of the local EE's payload. It simply checks whether the - total proposer reward from an external payload is equal to or greater than this value. In the future, a - comparison to a local payload is likely to be added. Example: Use 250000000000000000 to set the threshold to - 0.25 ETH. [default: 0] + This flag is deprecated and has no effect. + --builder-user-agent The HTTP user agent to send alongside requests to the builder URL. The default is Lighthouse's version string. @@ -313,14 +308,6 @@ OPTIONS: --http-tls-key The path of the private key to be used when serving the HTTP API server over TLS. Must not be password- protected. - --ignore-builder-override-suggestion-threshold - When the EE advises Lighthouse to ignore the builder payload, this flag specifies a percentage threshold for - the difference between the reward from the builder payload and the local EE's payload. This threshold must - be met for Lighthouse to consider ignoring the EE's suggestion. If the reward from the builder's payload - doesn't exceed the local payload by at least this percentage, the local payload will be used. The conditions - under which the EE may make this suggestion depend on the EE's implementation, with the primary intent being - to safeguard against potential censorship attacks from builders. Setting this flag to 0 will cause - Lighthouse to always ignore the EE's suggestion. Default: 10.0 (equivalent to 10%). [default: 10.0] --invalid-gossip-verified-blocks-path If a block succeeds gossip validation whilst failing full validation, store the block SSZ as a file at this path. This feature is only recommended for developers. This directory is not pruned, users should be careful diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 5327e155cbd..c633a6c6c6d 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1827,6 +1827,7 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, + builder_booster_factor: Option, ) -> Result { let mut path = self.eth_path(V3)?; @@ -1849,6 +1850,11 @@ impl BeaconNodeHttpClient { .append_pair("skip_randao_verification", ""); } + if let Some(builder_booster_factor) = builder_booster_factor { + path.query_pairs_mut() + .append_pair("builder_boost_factor", &builder_booster_factor.to_string()); + } + Ok(path) } @@ -1858,12 +1864,14 @@ impl BeaconNodeHttpClient { slot: Slot, randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, + builder_booster_factor: Option, ) -> Result<(JsonProduceBlockV3Response, ProduceBlockV3Metadata), Error> { self.get_validator_blocks_v3_modular( slot, randao_reveal, graffiti, SkipRandaoVerification::No, + builder_booster_factor, ) .await } @@ -1875,9 +1883,16 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, + builder_booster_factor: Option, ) -> Result<(JsonProduceBlockV3Response, ProduceBlockV3Metadata), Error> { let path = self - .get_validator_blocks_v3_path(slot, randao_reveal, graffiti, skip_randao_verification) + .get_validator_blocks_v3_path( + slot, + randao_reveal, + graffiti, + skip_randao_verification, + builder_booster_factor, + ) .await?; let opt_result = self @@ -1918,12 +1933,14 @@ impl BeaconNodeHttpClient { slot: Slot, randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, + builder_booster_factor: Option, ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { self.get_validator_blocks_v3_modular_ssz::( slot, randao_reveal, graffiti, SkipRandaoVerification::No, + builder_booster_factor, ) .await } @@ -1935,9 +1952,16 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, + builder_booster_factor: Option, ) -> Result<(ProduceBlockV3Response, ProduceBlockV3Metadata), Error> { let path = self - .get_validator_blocks_v3_path(slot, randao_reveal, graffiti, skip_randao_verification) + .get_validator_blocks_v3_path( + slot, + randao_reveal, + graffiti, + skip_randao_verification, + builder_booster_factor, + ) .await?; let opt_response = self diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index 259aa2cfba9..46c0dff8b40 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -729,6 +729,7 @@ pub struct ValidatorBlocksQuery { pub randao_reveal: SignatureBytes, pub graffiti: Option, pub skip_randao_verification: SkipRandaoVerification, + pub builder_boost_factor: Option, } #[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Deserialize)] diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 621424c305a..b100457c7e6 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -581,102 +581,6 @@ fn builder_fallback_flags() { assert_eq!(config.chain.builder_fallback_disable_checks, true); }, ); - run_payload_builder_flag_test_with_config( - "builder", - "http://meow.cats", - Some("builder-profit-threshold"), - Some("1000000000000000000000000"), - |config| { - assert_eq!( - config - .execution_layer - .as_ref() - .unwrap() - .builder_profit_threshold, - 1000000000000000000000000 - ); - }, - ); - run_payload_builder_flag_test_with_config( - "builder", - "http://meow.cats", - None, - None, - |config| { - assert_eq!( - config - .execution_layer - .as_ref() - .unwrap() - .builder_profit_threshold, - 0 - ); - }, - ); - run_payload_builder_flag_test_with_config( - "builder", - "http://meow.cats", - Some("always-prefer-builder-payload"), - None, - |config| { - assert_eq!( - config - .execution_layer - .as_ref() - .unwrap() - .always_prefer_builder_payload, - true - ); - }, - ); - run_payload_builder_flag_test_with_config( - "builder", - "http://meow.cats", - None, - None, - |config| { - assert_eq!( - config - .execution_layer - .as_ref() - .unwrap() - .always_prefer_builder_payload, - false - ); - }, - ); - run_payload_builder_flag_test_with_config( - "builder", - "http://meow.cats", - Some("ignore-builder-override-suggestion-threshold"), - Some("53.4"), - |config| { - assert_eq!( - config - .execution_layer - .as_ref() - .unwrap() - .ignore_builder_override_suggestion_threshold, - 53.4f32 - ); - }, - ); - run_payload_builder_flag_test_with_config( - "builder", - "http://meow.cats", - None, - None, - |config| { - assert_eq!( - config - .execution_layer - .as_ref() - .unwrap() - .ignore_builder_override_suggestion_threshold, - 10.0f32 - ); - }, - ); } #[test] diff --git a/scripts/local_testnet/README.md b/scripts/local_testnet/README.md index 87565b0cae6..2862fde0759 100644 --- a/scripts/local_testnet/README.md +++ b/scripts/local_testnet/README.md @@ -187,10 +187,9 @@ Update the genesis time to now using: 1. Add builder URL to `BN_ARGS` in `./vars.env`, e.g. `--builder http://localhost:8650`. Some mock builder server options: - [`mock-relay`](https://github.com/realbigsean/mock-relay) - [`dummy-builder`](https://github.com/michaelsproul/dummy_builder) -2. (Optional) Add `--always-prefer-builder-payload` to `BN_ARGS`. -3. The above mock builders do not support non-mainnet presets as of now, and will require setting `SECONDS_PER_SLOT` and `SECONDS_PER_ETH1_BLOCK` to `12` in `./vars.env`. -4. Start the testnet with the following command (the `-p` flag enables the validator client `--builder-proposals` flag): +2. The above mock builders do not support non-mainnet presets as of now, and will require setting `SECONDS_PER_SLOT` and `SECONDS_PER_ETH1_BLOCK` to `12` in `./vars.env`. +3. Start the testnet with the following command (the `-p` flag enables the validator client `--builder-proposals` flag): ```bash ./start_local_testnet.sh -p genesis.json ``` -5. Block production using builder flow will start at epoch 4. +4. Block production using builder flow will start at epoch 4. diff --git a/testing/execution_engine_integration/src/test_rig.rs b/testing/execution_engine_integration/src/test_rig.rs index 42667f27c6b..b0701e80a1b 100644 --- a/testing/execution_engine_integration/src/test_rig.rs +++ b/testing/execution_engine_integration/src/test_rig.rs @@ -334,6 +334,7 @@ impl TestRig { builder_params, TEST_FORK, &self.spec, + None, BlockProductionVersion::FullV2, ) .await @@ -485,6 +486,7 @@ impl TestRig { builder_params, TEST_FORK, &self.spec, + None, BlockProductionVersion::FullV2, ) .await