From 177aef8f1e3c316b962f8b1998d4950b760f7a29 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Mon, 5 Sep 2022 04:50:49 +0000 Subject: [PATCH] Builder profit threshold flag (#3534) ## Issue Addressed Resolves https://github.com/sigp/lighthouse/issues/3517 ## Proposed Changes Adds a `--builder-profit-threshold ` flag to the BN. If an external payload's value field is less than this value, the local payload will be used. The value of the local payload will not be checked (it can't really be checked until the engine API is updated to support this). Co-authored-by: realbigsean --- beacon_node/execution_layer/src/lib.rs | 26 +++++++- .../src/test_utils/mock_builder.rs | 8 ++- .../src/test_utils/mock_execution_layer.rs | 4 +- .../execution_layer/src/test_utils/mod.rs | 1 + beacon_node/http_api/tests/tests.rs | 60 ++++++++++++++++++- beacon_node/src/cli.rs | 15 +++++ beacon_node/src/config.rs | 2 + book/src/builders.md | 18 +++++- lighthouse/tests/beacon_node.rs | 32 ++++++++++ 9 files changed, 156 insertions(+), 10 deletions(-) diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs index 3bdca82ad..89dc3f68e 100644 --- a/beacon_node/execution_layer/src/lib.rs +++ b/beacon_node/execution_layer/src/lib.rs @@ -136,6 +136,7 @@ struct Inner { proposers: RwLock>, executor: TaskExecutor, payload_cache: PayloadCache, + builder_profit_threshold: Uint256, log: Logger, } @@ -156,6 +157,8 @@ 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, } /// Provides access to one execution engine and provides a neat interface for consumption by the @@ -176,6 +179,7 @@ impl ExecutionLayer { jwt_id, jwt_version, default_datadir, + builder_profit_threshold, } = config; if urls.len() > 1 { @@ -225,7 +229,14 @@ impl ExecutionLayer { }; let builder = builder_url - .map(|url| BuilderHttpClient::new(url).map_err(Error::Builder)) + .map(|url| { + let builder_client = BuilderHttpClient::new(url.clone()).map_err(Error::Builder); + info!(log, + "Connected to external block builder"; + "builder_url" => ?url, + "builder_profit_threshold" => builder_profit_threshold); + builder_client + }) .transpose()?; let inner = Inner { @@ -238,6 +249,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, }; @@ -631,7 +643,17 @@ impl ExecutionLayer { "block_hash" => ?header.block_hash(), ); - if header.parent_hash() != parent_hash { + let relay_value = relay.data.message.value; + let configured_value = self.inner.builder_profit_threshold; + if relay_value < configured_value { + info!( + self.log(), + "The value offered by the connected builder does not meet \ + the configured profit threshold. Using local payload."; + "configured_value" => ?configured_value, "relay_value" => ?relay_value + ); + Ok(local) + } else if header.parent_hash() != parent_hash { warn!( self.log(), "Invalid parent hash from connected builder, \ 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 6b565cb3d..b8f74c1c9 100644 --- a/beacon_node/execution_layer/src/test_utils/mock_builder.rs +++ b/beacon_node/execution_layer/src/test_utils/mock_builder.rs @@ -33,7 +33,7 @@ use types::{ pub enum Operation { FeeRecipient(Address), GasLimit(usize), - Value(usize), + Value(Uint256), ParentHash(Hash256), PrevRandao(Hash256), BlockNumber(usize), @@ -47,7 +47,7 @@ impl Operation { bid.header.fee_recipient = to_ssz_rs(&fee_recipient)? } Operation::GasLimit(gas_limit) => bid.header.gas_limit = gas_limit as u64, - Operation::Value(value) => bid.value = to_ssz_rs(&Uint256::from(value))?, + Operation::Value(value) => bid.value = to_ssz_rs(&value)?, Operation::ParentHash(parent_hash) => bid.header.parent_hash = to_ssz_rs(&parent_hash)?, Operation::PrevRandao(prev_randao) => bid.header.prev_randao = to_ssz_rs(&prev_randao)?, Operation::BlockNumber(block_number) => bid.header.block_number = block_number as u64, @@ -149,7 +149,9 @@ impl MockBuilder { } pub fn add_operation(&self, op: Operation) { - self.operations.write().push(op); + // Insert operations at the front of the vec to make sure `apply_operations` applies them + // in the order they are added. + self.operations.write().insert(0, op); } pub fn invalid_signatures(&self) { 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 cab2367cd..065abc936 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,6 +1,7 @@ use crate::{ test_utils::{ - MockServer, DEFAULT_JWT_SECRET, DEFAULT_TERMINAL_BLOCK, DEFAULT_TERMINAL_DIFFICULTY, + MockServer, DEFAULT_BUILDER_THRESHOLD_WEI, DEFAULT_JWT_SECRET, DEFAULT_TERMINAL_BLOCK, + DEFAULT_TERMINAL_DIFFICULTY, }, Config, *, }; @@ -66,6 +67,7 @@ impl MockExecutionLayer { builder_url, secret_files: vec![path], suggested_fee_recipient: Some(Address::repeat_byte(42)), + builder_profit_threshold: DEFAULT_BUILDER_THRESHOLD_WEI, ..Default::default() }; let el = diff --git a/beacon_node/execution_layer/src/test_utils/mod.rs b/beacon_node/execution_layer/src/test_utils/mod.rs index 18612bf30..aaeea8aa5 100644 --- a/beacon_node/execution_layer/src/test_utils/mod.rs +++ b/beacon_node/execution_layer/src/test_utils/mod.rs @@ -28,6 +28,7 @@ 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; mod execution_block_generator; mod handle_rpc; diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs index c8e647be8..ca240e64d 100644 --- a/beacon_node/http_api/tests/tests.rs +++ b/beacon_node/http_api/tests/tests.rs @@ -13,6 +13,7 @@ use eth2::{ }; use execution_layer::test_utils::Operation; use execution_layer::test_utils::TestingBuilder; +use execution_layer::test_utils::DEFAULT_BUILDER_THRESHOLD_WEI; use futures::stream::{Stream, StreamExt}; use futures::FutureExt; use http_api::{BlockId, StateId}; @@ -341,10 +342,20 @@ impl ApiTester { } pub async fn new_mev_tester() -> Self { - Self::new_with_hard_forks(true, true) + let tester = Self::new_with_hard_forks(true, true) .await .test_post_validator_register_validator() - .await + .await; + // Make sure bids always meet the minimum threshold. + tester + .mock_builder + .as_ref() + .unwrap() + .builder + .add_operation(Operation::Value(Uint256::from( + DEFAULT_BUILDER_THRESHOLD_WEI, + ))); + tester } fn skip_slots(self, count: u64) -> Self { @@ -3187,6 +3198,43 @@ impl ApiTester { self } + pub async fn test_payload_rejects_inadequate_builder_threshold(self) -> Self { + // Mutate value. + self.mock_builder + .as_ref() + .unwrap() + .builder + .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 = self + .client + .get_validator_blinded_blocks::>(slot, &randao_reveal, None) + .await + .unwrap() + .data + .body() + .execution_payload() + .unwrap() + .clone(); + + // 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 + } + #[cfg(target_os = "linux")] pub async fn test_get_lighthouse_health(self) -> Self { self.client.get_lighthouse_health().await.unwrap(); @@ -4159,6 +4207,14 @@ async fn builder_chain_health_optimistic_head() { .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 lighthouse_endpoints() { ApiTester::new() diff --git a/beacon_node/src/cli.rs b/beacon_node/src/cli.rs index 0f4d8a151..6473f3907 100644 --- a/beacon_node/src/cli.rs +++ b/beacon_node/src/cli.rs @@ -777,6 +777,21 @@ 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("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("count-unrealized") .long("count-unrealized") diff --git a/beacon_node/src/config.rs b/beacon_node/src/config.rs index 190dbf721..54b81fb62 100644 --- a/beacon_node/src/config.rs +++ b/beacon_node/src/config.rs @@ -309,6 +309,8 @@ 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")?; // If `--execution-endpoint` is provided, we should ignore any `--eth1-endpoints` values and // use `--execution-endpoint` instead. Also, log a deprecation warning. diff --git a/book/src/builders.md b/book/src/builders.md index 110f2450b..2c24d3100 100644 --- a/book/src/builders.md +++ b/book/src/builders.md @@ -10,8 +10,8 @@ before the validator has committed to (i.e. signed) the block. A primer on MEV c Using the builder API is not known to introduce additional slashing risks, however a live-ness risk (i.e. the ability for the chain to produce valid blocks) is introduced because your node will be -signing blocks without executing the transactions within the block. Therefore it won't know whether -the transactions are valid and it may sign a block that the network will reject. This would lead to +signing blocks without executing the transactions within the block. Therefore, it won't know whether +the transactions are valid, and it may sign a block that the network will reject. This would lead to a missed proposal and the opportunity cost of lost block rewards. ## How to connect to a builder @@ -151,6 +151,20 @@ 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. Currently, +this threshold just looks at the value of the external payload. No comparison to the local payload is made, although +this feature will likely be added in the future. + [mev-rs]: https://github.com/ralexstokes/mev-rs [mev-boost]: https://github.com/flashbots/mev-boost [gas-limit-api]: https://ethereum.github.io/keymanager-APIs/#/Gas%20Limit diff --git a/lighthouse/tests/beacon_node.rs b/lighthouse/tests/beacon_node.rs index 0988e9e2f..ab7978ca0 100644 --- a/lighthouse/tests/beacon_node.rs +++ b/lighthouse/tests/beacon_node.rs @@ -571,6 +571,38 @@ 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 + ); + }, + ); } fn run_jwt_optional_flags_test(jwt_flag: &str, jwt_id_flag: &str, jwt_version_flag: &str) {