Remove strict fee recipient (#3552)

## Issue Addressed

Resolves: #3550

Remove the `--strict-fee-recipient` flag. It will cause missed proposals prior to the bellatrix transition.

Co-authored-by: realbigsean <sean@sigmaprime.io>

lighthouse/tests/validator_client.rs

@@ -430,20 +430,6 @@ fn builder_registration_timestamp_override_flag() {
             assert_eq!(config.builder_registration_timestamp_override, Some(100))
         });
 }
-#[test]
-fn strict_fee_recipient_flag() {
-    CommandLineTest::new()
-        .flag("strict-fee-recipient", None)
-        .run()
-        .with_config(|config| assert!(config.strict_fee_recipient));
-}
-#[test]
-fn no_strict_fee_recipient_flag() {
-    CommandLineTest::new()
-        .run()
-        .with_config(|config| assert!(!config.strict_fee_recipient));
-}
-
 #[test]
 fn monitoring_endpoint() {
     CommandLineTest::new()

validator_client/src/block_service.rs

@@ -43,7 +43,6 @@ pub struct BlockServiceBuilder<T, E: EthSpec> {
     context: Option<RuntimeContext<E>>,
     graffiti: Option<Graffiti>,
     graffiti_file: Option<GraffitiFile>,
-    strict_fee_recipient: bool,
 }
 
 impl<T: SlotClock + 'static, E: EthSpec> BlockServiceBuilder<T, E> {
@@ -55,7 +54,6 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockServiceBuilder<T, E> {
             context: None,
             graffiti: None,
             graffiti_file: None,
-            strict_fee_recipient: false,
         }
     }
 
@@ -89,11 +87,6 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockServiceBuilder<T, E> {
         self
     }
 
-    pub fn strict_fee_recipient(mut self, strict_fee_recipient: bool) -> Self {
-        self.strict_fee_recipient = strict_fee_recipient;
-        self
-    }
-
     pub fn build(self) -> Result<BlockService<T, E>, String> {
         Ok(BlockService {
             inner: Arc::new(Inner {
@@ -111,7 +104,6 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockServiceBuilder<T, E> {
                     .ok_or("Cannot build BlockService without runtime_context")?,
                 graffiti: self.graffiti,
                 graffiti_file: self.graffiti_file,
-                strict_fee_recipient: self.strict_fee_recipient,
             }),
         })
     }
@@ -125,7 +117,6 @@ pub struct Inner<T, E: EthSpec> {
     context: RuntimeContext<E>,
     graffiti: Option<Graffiti>,
     graffiti_file: Option<GraffitiFile>,
-    strict_fee_recipient: bool,
 }
 
 /// Attempts to produce attestations for any block producer(s) at the start of the epoch.
@@ -324,9 +315,7 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {
         let self_ref = &self;
         let proposer_index = self.validator_store.validator_index(&validator_pubkey);
         let validator_pubkey_ref = &validator_pubkey;
-        let fee_recipient = self.validator_store.get_fee_recipient(&validator_pubkey);
 
-        let strict_fee_recipient = self.strict_fee_recipient;
         // Request block from first responsive beacon node.
         let block = self
             .beacon_nodes
@@ -377,17 +366,6 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {
                         }
                     };
 
-                    // Ensure the correctness of the execution payload's fee recipient.
-                    if strict_fee_recipient {
-                        if let Ok(execution_payload) = block.body().execution_payload() {
-                            if Some(execution_payload.fee_recipient()) != fee_recipient {
-                                return Err(BlockError::Recoverable(
-                                    "Incorrect fee recipient used by builder".to_string(),
-                                ));
-                            }
-                        }
-                    }
-
                     if proposer_index != Some(block.proposer_index()) {
                         return Err(BlockError::Recoverable(
                             "Proposer index does not match block proposer. Beacon chain re-orged"

validator_client/src/cli.rs

@@ -268,18 +268,17 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> {
                     headers during proposals and will sign over headers. Useful for outsourcing \
                     execution payload construction during proposals.")
                 .takes_value(false),
-        )
+        ).arg(
-        .arg(
             Arg::with_name("strict-fee-recipient")
                 .long("strict-fee-recipient")
-                .help("If this flag is set, Lighthouse will refuse to sign any block whose \
+                .help("[DEPRECATED] If this flag is set, Lighthouse will refuse to sign any block whose \
-                    `fee_recipient` does not match the `suggested_fee_recipient` sent by this validator. \
+                        `fee_recipient` does not match the `suggested_fee_recipient` sent by this validator. \
-                     This applies to both the normal block proposal flow, as well as block proposals \
+                         This applies to both the normal block proposal flow, as well as block proposals \
-                     through the builder API. Proposals through the builder API are more likely to have a \
+                         through the builder API. Proposals through the builder API are more likely to have a \
-                     discrepancy in `fee_recipient` so you should be aware of how your connected relay \
+                         discrepancy in `fee_recipient` so you should be aware of how your connected relay \
-                     sends proposer payments before using this flag. If this flag is used, a fee recipient \
+                         sends proposer payments before using this flag. If this flag is used, a fee recipient \
-                     mismatch in the builder API flow will result in a fallback to the local execution engine \
+                         mismatch in the builder API flow will result in a fallback to the local execution engine \
-                     for payload construction, where a strict fee recipient check will still be applied.")
+                         for payload construction, where a strict fee recipient check will still be applied.")
                 .takes_value(false),
         )
         .arg(

validator_client/src/config.rs

@@ -61,9 +61,6 @@ pub struct Config {
     /// A list of custom certificates that the validator client will additionally use when
     /// connecting to a beacon node over SSL/TLS.
     pub beacon_nodes_tls_certs: Option<Vec<PathBuf>>,
-    /// Enabling this will make sure the validator client never signs a block whose `fee_recipient`
-    /// does not match the `suggested_fee_recipient`.
-    pub strict_fee_recipient: bool,
 }
 
 impl Default for Config {
@@ -99,7 +96,6 @@ impl Default for Config {
             builder_proposals: false,
             builder_registration_timestamp_override: None,
             gas_limit: None,
-            strict_fee_recipient: false,
         }
     }
 }
@@ -334,7 +330,11 @@ impl Config {
         }
 
         if cli_args.is_present("strict-fee-recipient") {
-            config.strict_fee_recipient = true;
+            warn!(
+                log,
+                "The flag `--strict-fee-recipient` has been deprecated due to a bug causing \
+                missed proposals. The flag will be ignored."
+            );
         }
 
         Ok(config)

validator_client/src/lib.rs

@@ -420,7 +420,6 @@ impl<T: EthSpec> ProductionValidatorClient<T> {
             .runtime_context(context.service_context("block".into()))
             .graffiti(config.graffiti)
             .graffiti_file(config.graffiti_file.clone())
-            .strict_fee_recipient(config.strict_fee_recipient)
             .build()?;
 
         let attestation_service = AttestationServiceBuilder::new()