From 7a6cb6b0b3eb72011d14f20fad5d2bf901ef4b1d Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Fri, 5 Jan 2024 14:36:44 +0200 Subject: [PATCH] set deprecated flags to no op, revert should_override_builder --- beacon_node/beacon_chain/src/test_utils.rs | 7 ++--- beacon_node/execution_layer/src/engine_api.rs | 2 ++ .../src/engine_api/json_structures.rs | 3 ++ beacon_node/execution_layer/src/lib.rs | 12 ++++++++ .../src/test_utils/handle_rpc.rs | 1 + .../src/test_utils/mock_execution_layer.rs | 5 +--- .../execution_layer/src/test_utils/mod.rs | 1 - beacon_node/http_api/tests/tests.rs | 22 +++++--------- beacon_node/src/cli.rs | 29 +++++++++++++++++++ 9 files changed, 57 insertions(+), 25 deletions(-) diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 2c28fc5edd2..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), diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index b45beb7daea..19b9a58eb66 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -408,6 +408,8 @@ pub struct GetPayloadResponse { pub block_value: Uint256, #[superstruct(only(Deneb))] pub blobs_bundle: BlobsBundle, + #[superstruct(only(Deneb), partial_getter(copy))] + pub should_override_builder: bool, } impl GetPayloadResponse { diff --git a/beacon_node/execution_layer/src/engine_api/json_structures.rs b/beacon_node/execution_layer/src/engine_api/json_structures.rs index 8752904c070..e8641be7953 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -298,6 +298,8 @@ pub struct JsonGetPayloadResponse { pub block_value: Uint256, #[superstruct(only(V3))] pub blobs_bundle: JsonBlobsBundleV1, + #[superstruct(only(V3))] + pub should_override_builder: bool, } impl From> for GetPayloadResponse { @@ -320,6 +322,7 @@ impl From> for GetPayloadResponse { execution_payload: response.execution_payload.into(), block_value: response.block_value, blobs_bundle: response.blobs_bundle.into(), + should_override_builder: response.should_override_builder, }) } } diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 07bc4f9119f..f0b23e8d7f6 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -1130,6 +1130,18 @@ impl ExecutionLayer { ))); } + if local.should_override_builder().unwrap_or(false) { + 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"; diff --git a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs index 02e9d063673..9dff1ac0089 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -292,6 +292,7 @@ pub async fn handle_rpc( GENERIC_ERROR_CODE, ))? .into(), + should_override_builder: false, }) .unwrap() } 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 64f9f332e3c..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, 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/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index 5ef545d2b85..483f3b733a5 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -13,7 +13,7 @@ use eth2::{ BeaconNodeHttpClient, Error, StatusCode, Timeouts, }; use execution_layer::test_utils::{ - MockBuilder, Operation, DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_BUILDER_THRESHOLD_WEI, + MockBuilder, Operation, DEFAULT_BUILDER_PAYLOAD_VALUE_WEI, DEFAULT_MOCK_EL_PAYLOAD_VALUE_WEI, }; use futures::stream::{Stream, StreamExt}; @@ -79,8 +79,7 @@ struct ApiTester { struct ApiTesterConfig { spec: ChainSpec, - retain_historic_states: bool, - builder_threshold: Option, + retain_historic_states: bool } impl Default for ApiTesterConfig { @@ -90,7 +89,6 @@ impl Default for ApiTesterConfig { Self { spec, retain_historic_states: false, - builder_threshold: None, } } } @@ -132,7 +130,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 @@ -394,16 +392,12 @@ impl ApiTester { tester .mock_builder .as_ref() - .unwrap() - .add_operation(Operation::Value(Uint256::from( - DEFAULT_BUILDER_THRESHOLD_WEI, - ))); + .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(), }; @@ -6238,7 +6232,7 @@ async fn builder_chain_health_optimistic_head_v3() { #[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 @@ -6250,7 +6244,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 @@ -6263,7 +6257,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(), }; @@ -6284,7 +6277,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 acec921c1de..c3be855c917 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -1127,6 +1127,24 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { conditions.") .takes_value(false) ) + .arg( + Arg::with_name("builder-profit-threshold") + .long("builder-profit-threshold") + .value_name("WEI_VALUE") + .help("This flag is deprecated and has no effect.") + .hidden(true) + .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("This flag is deprecated and has no effect.") + .hidden(true) + .default_value("10.0") + .takes_value(true) + ) .arg( Arg::with_name("builder-user-agent") .long("builder-user-agent") @@ -1173,6 +1191,17 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { This enables --http and --validator-monitor-auto and enables SSE logging.") .takes_value(false) ) + .arg( + Arg::with_name("always-prefer-builder-payload") + .long("always-prefer-builder-payload") + .hidden(true) + .help("This flag is deprecated and has no effect.") + // 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") + ) .arg( Arg::with_name("invalid-gossip-verified-blocks-path") .long("invalid-gossip-verified-blocks-path")