Add a flag to always use payloads from builders (#4052)

## Issue Addressed

#4040 

## Proposed Changes

- Add the `always_prefer_builder_payload`  field to `Config` in `beacon_node/client/src/config.rs`.
- Add that same field to `Inner` in `beacon_node/execution_layer/src/lib.rs`
- Modify the logic for picking the payload in `beacon_node/execution_layer/src/lib.rs`
- Add the `always-prefer-builder-payload` flag to the beacon node CLI
- Test the new flags in `lighthouse/tests/beacon_node.rs`

Co-authored-by: Paul Hauner <[email protected]>

Author: danielrachi1

beacon_node/client/src/config.rs

@@ -79,6 +79,7 @@ pub struct Config {
     pub monitoring_api: Option<monitoring_api::Config>,
     pub slasher: Option<slasher::Config>,
     pub logger_config: LoggerConfig,
+    pub always_prefer_builder_payload: bool,
 }
 
 impl Default for Config {
@@ -105,6 +106,7 @@ impl Default for Config {
             validator_monitor_pubkeys: vec![],
             validator_monitor_individual_tracking_threshold: DEFAULT_INDIVIDUAL_TRACKING_THRESHOLD,
             logger_config: LoggerConfig::default(),
+            always_prefer_builder_payload: false,
         }
     }
 }

beacon_node/execution_layer/src/lib.rs

@@ -219,6 +219,7 @@ struct Inner<E: EthSpec> {
     payload_cache: PayloadCache<E>,
     builder_profit_threshold: Uint256,
     log: Logger,
+    always_prefer_builder_payload: bool,
 }
 
 #[derive(Debug, Default, Clone, Serialize, Deserialize)]
@@ -241,6 +242,7 @@ 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<u32>,
+    pub always_prefer_builder_payload: bool,
 }
 
 /// Provides access to one execution engine and provides a neat interface for consumption by the
@@ -263,6 +265,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
             default_datadir,
             builder_profit_threshold,
             execution_timeout_multiplier,
+            always_prefer_builder_payload,
         } = config;
 
         if urls.len() > 1 {
@@ -335,6 +338,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
             payload_cache: PayloadCache::default(),
             builder_profit_threshold: Uint256::from(builder_profit_threshold),
             log,
+            always_prefer_builder_payload,
         };
 
         Ok(Self {
@@ -796,7 +800,9 @@ impl<T: EthSpec> ExecutionLayer<T> {
 
                             let relay_value = relay.data.message.value;
                             let local_value = *local.block_value();
-                            if local_value >= relay_value {
+                            if !self.inner.always_prefer_builder_payload
+                                && local_value >= relay_value
+                            {
                                 info!(
                                     self.log(),
                                     "Local block is more profitable than relay block";

beacon_node/src/cli.rs

@@ -957,4 +957,13 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                       This is equivalent to --http and --validator-monitor-auto.")
                 .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")
+        )
 }

beacon_node/src/config.rs

@@ -753,6 +753,11 @@ pub fn get_config<E: EthSpec>(
     client_config.chain.optimistic_finalized_sync =
         !cli_args.is_present("disable-optimistic-finalized-sync");
 
+    // Payload selection configs
+    if cli_args.is_present("always-prefer-builder-payload") {
+        client_config.always_prefer_builder_payload = true;
+    }
+
     Ok(client_config)
 }
 

beacon_node/tests/test.rs

@@ -1,5 +1,5 @@
 #![cfg(test)]
-#![recursion_limit = "256"]
+#![recursion_limit = "512"]
 
 use beacon_chain::StateSkipConfig;
 use node_test_rig::{

lighthouse/tests/beacon_node.rs

@@ -340,6 +340,21 @@ fn trusted_peers_flag() {
         });
 }
 
+#[test]
+fn always_prefer_builder_payload_flag() {
+    CommandLineTest::new()
+        .flag("always-prefer-builder-payload", None)
+        .run_with_zero_port()
+        .with_config(|config| assert!(config.always_prefer_builder_payload));
+}
+
+#[test]
+fn no_flag_sets_always_prefer_builder_payload_to_false() {
+    CommandLineTest::new()
+        .run_with_zero_port()
+        .with_config(|config| assert!(!config.always_prefer_builder_payload));
+}
+
 // Tests for Eth1 flags.
 #[test]
 fn dummy_eth1_flag() {