[builder API] Make gas limit configurable, and add per-validator buil…

…derAPI usage (#3342)

* make gas limit configurable

* per-validator builder API enabling

* relax fee recipient checks

* update fee recipient test

account_manager/src/validator/import.rs

@@ -280,6 +280,8 @@ pub fn cli_run(matches: &ArgMatches, validator_dir: PathBuf) -> Result<(), Strin
             password_opt,
             graffiti,
             suggested_fee_recipient,
+            None,
+            None,
         )
         .map_err(|e| format!("Unable to create new validator definition: {:?}", e))?;
 

beacon_node/execution_layer/src/lib.rs

@@ -594,7 +594,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
 
                             let header = relay.data.message.header;
                             if header.fee_recipient() != suggested_fee_recipient {
-                                warn!(self.log(), "Incorrect fee recipient from connected builder, falling back to local execution engine.");
+                                info!(self.log(), "Fee recipient from connected builder does not match, using it anyways.");
                                 Ok(local)
                             } else if header.parent_hash() != parent_hash {
                                 warn!(self.log(), "Invalid parent hash from connected builder, falling back to local execution engine.");

beacon_node/http_api/tests/tests.rs

@@ -2538,22 +2538,22 @@ impl ApiTester {
         self
     }
 
-    pub async fn test_payload_rejects_changed_fee_recipient(self) -> Self {
+    pub async fn test_payload_accepts_changed_fee_recipient(self) -> Self {
+        let test_fee_recipient = "0x4242424242424242424242424242424242424242"
+            .parse::<Address>()
+            .unwrap();
+
         // Mutate fee recipient.
         self.mock_builder
             .as_ref()
             .unwrap()
             .builder
-            .add_operation(Operation::FeeRecipient(
-                "0x4242424242424242424242424242424242424242"
-                    .parse::<Address>()
-                    .unwrap(),
-            ));
+            .add_operation(Operation::FeeRecipient(test_fee_recipient));
 
         let slot = self.chain.slot().unwrap();
         let epoch = self.chain.epoch().unwrap();
 
-        let (proposer_index, randao_reveal) = self.get_test_randao(slot, epoch).await;
+        let (_, randao_reveal) = self.get_test_randao(slot, epoch).await;
 
         let payload = self
             .client
@@ -2566,20 +2566,19 @@ impl ApiTester {
             .unwrap()
             .clone();
 
-        let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64);
         assert_eq!(
             payload.execution_payload_header.fee_recipient,
-            expected_fee_recipient
+            test_fee_recipient
         );
 
-        // If this cache is populated, it indicates fallback to the local EE was correctly used.
+        // This cache should not be populated because fallback should not have been used.
         assert!(self
             .chain
             .execution_layer
             .as_ref()
             .unwrap()
             .get_payload_by_root(&payload.tree_hash_root())
-            .is_some());
+            .is_none());
         self
     }
 
@@ -3666,7 +3665,7 @@ async fn post_validator_register_gas_limit_mutation() {
 async fn post_validator_register_fee_recipient_mutation() {
     ApiTester::new_mev_tester()
         .await
-        .test_payload_rejects_changed_fee_recipient()
+        .test_payload_accepts_changed_fee_recipient()
         .await;
 }
 

common/account_utils/src/validator_definitions.rs

@@ -106,6 +106,12 @@ pub struct ValidatorDefinition {
     #[serde(skip_serializing_if = "Option::is_none")]
     pub suggested_fee_recipient: Option<Address>,
     #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub gas_limit: Option<u64>,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub builder_proposals: Option<bool>,
+    #[serde(default)]
     pub description: String,
     #[serde(flatten)]
     pub signing_definition: SigningDefinition,
@@ -123,6 +129,8 @@ impl ValidatorDefinition {
         voting_keystore_password: Option<ZeroizeString>,
         graffiti: Option<GraffitiString>,
         suggested_fee_recipient: Option<Address>,
+        gas_limit: Option<u64>,
+        builder_proposals: Option<bool>,
     ) -> Result<Self, Error> {
         let voting_keystore_path = voting_keystore_path.as_ref().into();
         let keystore =
@@ -135,6 +143,8 @@ impl ValidatorDefinition {
             description: keystore.description().unwrap_or("").to_string(),
             graffiti,
             suggested_fee_recipient,
+            gas_limit,
+            builder_proposals,
             signing_definition: SigningDefinition::LocalKeystore {
                 voting_keystore_path,
                 voting_keystore_password_path: None,
@@ -281,6 +291,8 @@ impl ValidatorDefinitions {
                     description: keystore.description().unwrap_or("").to_string(),
                     graffiti: None,
                     suggested_fee_recipient: None,
+                    gas_limit: None,
+                    builder_proposals: None,
                     signing_definition: SigningDefinition::LocalKeystore {
                         voting_keystore_path,
                         voting_keystore_password_path,
@@ -523,4 +535,84 @@ mod tests {
             Some(Address::from_str("0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d").unwrap())
         );
     }
+
+    #[test]
+    fn gas_limit_checks() {
+        let no_gas_limit = r#"---
+        description: ""
+        enabled: true
+        type: local_keystore
+        suggested_fee_recipient: "0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d"
+        voting_keystore_path: ""
+        voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7"
+        "#;
+        let def: ValidatorDefinition = serde_yaml::from_str(no_gas_limit).unwrap();
+        assert!(def.gas_limit.is_none());
+
+        let invalid_gas_limit = r#"---
+        description: ""
+        enabled: true
+        type: local_keystore
+        suggested_fee_recipient: "0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d"
+        gas_limit: "banana"
+        voting_keystore_path: ""
+        voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7"
+        "#;
+
+        let def: Result<ValidatorDefinition, _> = serde_yaml::from_str(invalid_gas_limit);
+        assert!(def.is_err());
+
+        let valid_gas_limit = r#"---
+        description: ""
+        enabled: true
+        type: local_keystore
+        suggested_fee_recipient: "0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d"
+        gas_limit: 35000000
+        voting_keystore_path: ""
+        voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7"
+        "#;
+
+        let def: ValidatorDefinition = serde_yaml::from_str(valid_gas_limit).unwrap();
+        assert_eq!(def.gas_limit, Some(35000000));
+    }
+
+    #[test]
+    fn builder_proposals_checks() {
+        let no_builder_proposals = r#"---
+        description: ""
+        enabled: true
+        type: local_keystore
+        suggested_fee_recipient: "0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d"
+        voting_keystore_path: ""
+        voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7"
+        "#;
+        let def: ValidatorDefinition = serde_yaml::from_str(no_builder_proposals).unwrap();
+        assert!(def.builder_proposals.is_none());
+
+        let invalid_builder_proposals = r#"---
+        description: ""
+        enabled: true
+        type: local_keystore
+        suggested_fee_recipient: "0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d"
+        builder_proposals: "banana"
+        voting_keystore_path: ""
+        voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7"
+        "#;
+
+        let def: Result<ValidatorDefinition, _> = serde_yaml::from_str(invalid_builder_proposals);
+        assert!(def.is_err());
+
+        let valid_builder_proposals = r#"---
+        description: ""
+        enabled: true
+        type: local_keystore
+        suggested_fee_recipient: "0xa2e334e71511686bcfe38bb3ee1ad8f6babcc03d"
+        builder_proposals: true
+        voting_keystore_path: ""
+        voting_public_key: "0xaf3c7ddab7e293834710fca2d39d068f884455ede270e0d0293dc818e4f2f0f975355067e8437955cb29aec674e5c9e7"
+        "#;
+
+        let def: ValidatorDefinition = serde_yaml::from_str(valid_builder_proposals).unwrap();
+        assert_eq!(def.builder_proposals, Some(true));
+    }
 }

common/eth2/src/lighthouse_vc/http_client.rs

@@ -462,7 +462,9 @@ impl ValidatorClientHttpClient {
     pub async fn patch_lighthouse_validators(
         &self,
         voting_pubkey: &PublicKeyBytes,
-        enabled: bool,
+        enabled: Option<bool>,
+        gas_limit: Option<u64>,
+        builder_proposals: Option<bool>,
     ) -> Result<(), Error> {
         let mut path = self.server.full.clone();
 
@@ -472,7 +474,15 @@ impl ValidatorClientHttpClient {
             .push("validators")
             .push(&voting_pubkey.to_string());
 
-        self.patch(path, &ValidatorPatchRequest { enabled }).await
+        self.patch(
+            path,
+            &ValidatorPatchRequest {
+                enabled,
+                gas_limit,
+                builder_proposals,
+            },
+        )
+        .await
     }
 
     fn make_keystores_url(&self) -> Result<Url, Error> {

common/eth2/src/lighthouse_vc/types.rs

@@ -26,6 +26,12 @@ pub struct ValidatorRequest {
     #[serde(default)]
     #[serde(skip_serializing_if = "Option::is_none")]
     pub suggested_fee_recipient: Option<Address>,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub gas_limit: Option<u64>,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub builder_proposals: Option<bool>,
     #[serde(with = "eth2_serde_utils::quoted_u64")]
     pub deposit_gwei: u64,
 }
@@ -49,6 +55,12 @@ pub struct CreatedValidator {
     #[serde(default)]
     #[serde(skip_serializing_if = "Option::is_none")]
     pub suggested_fee_recipient: Option<Address>,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub gas_limit: Option<u64>,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub builder_proposals: Option<bool>,
     pub eth1_deposit_tx_data: String,
     #[serde(with = "eth2_serde_utils::quoted_u64")]
     pub deposit_gwei: u64,
@@ -62,7 +74,15 @@ pub struct PostValidatorsResponseData {
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
 pub struct ValidatorPatchRequest {
-    pub enabled: bool,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub enabled: Option<bool>,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub gas_limit: Option<u64>,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub builder_proposals: Option<bool>,
 }
 
 #[derive(Clone, PartialEq, Serialize, Deserialize)]
@@ -72,6 +92,8 @@ pub struct KeystoreValidatorsPostRequest {
     pub keystore: Keystore,
     pub graffiti: Option<GraffitiString>,
     pub suggested_fee_recipient: Option<Address>,
+    pub gas_limit: Option<u64>,
+    pub builder_proposals: Option<bool>,
 }
 
 #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)]
@@ -84,6 +106,12 @@ pub struct Web3SignerValidatorRequest {
     #[serde(default)]
     #[serde(skip_serializing_if = "Option::is_none")]
     pub suggested_fee_recipient: Option<Address>,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub gas_limit: Option<u64>,
+    #[serde(default)]
+    #[serde(skip_serializing_if = "Option::is_none")]
+    pub builder_proposals: Option<bool>,
     pub voting_public_key: PublicKey,
     pub url: String,
     #[serde(default)]

lighthouse/tests/account_manager.rs

@@ -494,6 +494,7 @@ fn validator_import_launchpad() {
         description: "".into(),
         graffiti: None,
         suggested_fee_recipient: None,
+        gas_limit: None,
         voting_public_key: keystore.public_key().unwrap(),
         signing_definition: SigningDefinition::LocalKeystore {
             voting_keystore_path,
@@ -614,6 +615,7 @@ fn validator_import_launchpad_no_password_then_add_password() {
         description: "".into(),
         graffiti: None,
         suggested_fee_recipient: None,
+        gas_limit: None,
         voting_public_key: keystore.public_key().unwrap(),
         signing_definition: SigningDefinition::LocalKeystore {
             voting_keystore_path,
@@ -638,6 +640,7 @@ fn validator_import_launchpad_no_password_then_add_password() {
         description: "".into(),
         graffiti: None,
         suggested_fee_recipient: None,
+        gas_limit: None,
         voting_public_key: keystore.public_key().unwrap(),
         signing_definition: SigningDefinition::LocalKeystore {
             voting_keystore_path: dst_keystore_dir.join(KEYSTORE_NAME),
@@ -738,6 +741,7 @@ fn validator_import_launchpad_password_file() {
         voting_public_key: keystore.public_key().unwrap(),
         graffiti: None,
         suggested_fee_recipient: None,
+        gas_limit: None,
         signing_definition: SigningDefinition::LocalKeystore {
             voting_keystore_path,
             voting_keystore_password_path: None,

lighthouse/tests/validator_client.rs

@@ -389,6 +389,33 @@ fn no_doppelganger_protection_flag() {
         .with_config(|config| assert!(!config.enable_doppelganger_protection));
 }
 #[test]
+fn no_gas_limit_flag() {
+    CommandLineTest::new()
+        .run()
+        .with_config(|config| assert!(config.gas_limit.is_none()));
+}
+#[test]
+fn gas_limit_flag() {
+    CommandLineTest::new()
+        .flag("gas-limit", Some("600"))
+        .flag("builder-proposals", None)
+        .run()
+        .with_config(|config| assert_eq!(config.gas_limit, Some(600)));
+}
+#[test]
+fn no_builder_proposals_flag() {
+    CommandLineTest::new()
+        .run()
+        .with_config(|config| assert!(!config.builder_proposals));
+}
+#[test]
+fn builder_proposals_flag() {
+    CommandLineTest::new()
+        .flag("builder-proposals", None)
+        .run()
+        .with_config(|config| assert!(config.builder_proposals));
+}
+#[test]
 fn no_builder_registration_timestamp_override_flag() {
     CommandLineTest::new()
         .run()

scripts/local_testnet/start_local_testnet.sh

@@ -10,14 +10,14 @@ ulimit -n 65536
 
 # VC_COUNT is defaulted in vars.env
 DEBUG_LEVEL=${DEBUG_LEVEL:-info}
-PRIVATE_TX_PROPOSALS=
+BUILDER_PROPOSALS=
 
 # Get options
 while getopts "v:d:ph" flag; do
   case "${flag}" in
     v) VC_COUNT=${OPTARG};;
     d) DEBUG_LEVEL=${OPTARG};;
-    p) PRIVATE_TX_PROPOSALS="-p";;
+    p) BUILDER_PROPOSALS="-p";;
     h)
         validators=$(( $VALIDATOR_COUNT / $BN_COUNT ))
         echo "Start local testnet, defaults: 1 eth1 node, $BN_COUNT beacon nodes,"
@@ -119,7 +119,7 @@ done
 
 # Start requested number of validator clients
 for (( vc=1; vc<=$VC_COUNT; vc++ )); do
-    execute_command_add_PID validator_node_$vc.log ./validator_client.sh $PRIVATE_TX_PROPOSALS -d $DEBUG_LEVEL $DATADIR/node_$vc http://localhost:$((BN_http_port_base + $vc))
+    execute_command_add_PID validator_node_$vc.log ./validator_client.sh $BUILDER_PROPOSALS -d $DEBUG_LEVEL $DATADIR/node_$vc http://localhost:$((BN_http_port_base + $vc))
 done
 
 echo "Started!"