diff --git a/beacon_node/execution_layer/src/engine_api.rs b/beacon_node/execution_layer/src/engine_api.rs index 19b9a58eb66..b45beb7daea 100644 --- a/beacon_node/execution_layer/src/engine_api.rs +++ b/beacon_node/execution_layer/src/engine_api.rs @@ -408,8 +408,6 @@ 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 e8641be7953..8752904c070 100644 --- a/beacon_node/execution_layer/src/engine_api/json_structures.rs +++ b/beacon_node/execution_layer/src/engine_api/json_structures.rs @@ -298,8 +298,6 @@ 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 { @@ -322,7 +320,6 @@ 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 40fbb6390b0..50c2386cda0 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -340,8 +340,6 @@ struct Inner { 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 @@ -371,8 +369,6 @@ pub struct Config { /// 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 @@ -382,40 +378,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 { @@ -430,8 +392,6 @@ impl ExecutionLayer { default_datadir, builder_profit_threshold, execution_timeout_multiplier, - always_prefer_builder_payload, - ignore_builder_override_suggestion_threshold, } = config; if urls.len() > 1 { @@ -494,8 +454,6 @@ impl ExecutionLayer { 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), }; @@ -1156,10 +1114,6 @@ impl ExecutionLayer { ))); } - if self.inner.always_prefer_builder_payload { - return ProvenancedPayload::try_from(relay.data.message); - } - let builder_boost_factor = builder_boost_factor.unwrap_or(DEFAULT_BUILDER_BOOST_FACTOR); @@ -1182,42 +1136,6 @@ impl ExecutionLayer { ))); } - if boosted_relay_value < self.inner.builder_profit_threshold { - 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, - "boosted_relay_value" => %boosted_relay_value, - "builder_boost_factor" => %builder_boost_factor, - ); - 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, boosted_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, - "boosted_relay_value" => %boosted_relay_value, - "builder_boost_factor" => %builder_boost_factor - ); - return Ok(ProvenancedPayload::Local(BlockProposalContentsType::Full( - local.try_into()?, - ))); - } - } - info!( self.log(), "Relay block is more profitable than local block"; @@ -2395,42 +2313,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/handle_rpc.rs b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs index 9dff1ac0089..02e9d063673 100644 --- a/beacon_node/execution_layer/src/test_utils/handle_rpc.rs +++ b/beacon_node/execution_layer/src/test_utils/handle_rpc.rs @@ -292,7 +292,6 @@ pub async fn handle_rpc( GENERIC_ERROR_CODE, ))? .into(), - should_override_builder: false, }) .unwrap() } diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index d6adffd1af6..5ef545d2b85 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -4636,70 +4636,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, 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 @@ -6300,22 +6236,6 @@ 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() diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 0fc485b1526..a7a7f1780dc 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -1142,23 +1142,6 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .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") - .takes_value(true) - ) .arg( Arg::with_name("builder-user-agent") .long("builder-user-agent") @@ -1205,16 +1188,6 @@ 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") - .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") - ) .arg( Arg::with_name("invalid-gossip-verified-blocks-path") .long("invalid-gossip-verified-blocks-path") diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 7c48ae76092..dbfd0e45181 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -319,10 +319,6 @@ pub fn get_config( 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..6be125d855f 100644 --- a/book/src/help_bn.md +++ b/book/src/help_bn.md @@ -174,14 +174,7 @@ OPTIONS: If this node is proposing a block and has seen this number of skip slots on the canonical chain in the past `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] - --builder-user-agent + --builder-user-agent The HTTP user agent to send alongside requests to the builder URL. The default is Lighthouse's version string. --checkpoint-block @@ -313,14 +306,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/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 621424c305a..7d914f23f53 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -613,70 +613,6 @@ fn builder_fallback_flags() { ); }, ); - 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]