Builder profit threshold flag (#3534)

## Issue Addressed Resolves #3517 ## Proposed Changes Adds a `--builder-profit-threshold <wei value>` 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 <[email protected]>
sigp · Sep 5, 2022 · 177aef8 · 177aef8
1 parent 95c5663
commit 177aef8
Show file tree

Hide file tree

Showing 9 changed files with 156 additions and 10 deletions.
diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs
@@ -136,6 +136,7 @@ struct Inner<E: EthSpec> {
     proposers: RwLock<HashMap<ProposerKey, Proposer>>,
     executor: TaskExecutor,
     payload_cache: PayloadCache<E>,
+    builder_profit_threshold: Uint256,
     log: Logger,
 }
 
@@ -156,6 +157,8 @@ pub struct Config {
     pub jwt_version: Option<String>,
     /// 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<T: EthSpec> ExecutionLayer<T> {
             jwt_id,
             jwt_version,
             default_datadir,
+            builder_profit_threshold,
         } = config;
 
         if urls.len() > 1 {
@@ -225,7 +229,14 @@ impl<T: EthSpec> ExecutionLayer<T> {
         };
 
         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<T: EthSpec> ExecutionLayer<T> {
             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<T: EthSpec> ExecutionLayer<T> {
                                 "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
@@ -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<E: EthSpec> MockBuilder<E> {
     }
 
     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
@@ -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<T: EthSpec> MockExecutionLayer<T> {
             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
@@ -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
@@ -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::<E, BlindedPayload<E>>(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
@@ -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
@@ -309,6 +309,8 @@ pub fn get_config<E: EthSpec>(
         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
@@ -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 <WEI_VALUE>`
+
+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
@@ -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) {