From a9f075c3c0fb25213609e2fc243fbb3e1e6c1772 Mon Sep 17 00:00:00 2001 From: realbigsean Date: Thu, 8 Sep 2022 23:46:02 +0000 Subject: [PATCH] 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 --- lighthouse/tests/validator_client.rs | 14 -------------- validator_client/src/block_service.rs | 22 ---------------------- validator_client/src/cli.rs | 19 +++++++++---------- validator_client/src/config.rs | 10 +++++----- validator_client/src/lib.rs | 1 - 5 files changed, 14 insertions(+), 52 deletions(-) diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 39ea2bfaa..a9b76c275 100644 --- a/lighthouse/tests/validator_client.rs +++ b/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() diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs index 2a8d16422..ac1ba1167 100644 --- a/validator_client/src/block_service.rs +++ b/validator_client/src/block_service.rs @@ -43,7 +43,6 @@ pub struct BlockServiceBuilder { context: Option>, graffiti: Option, graffiti_file: Option, - strict_fee_recipient: bool, } impl BlockServiceBuilder { @@ -55,7 +54,6 @@ impl BlockServiceBuilder { context: None, graffiti: None, graffiti_file: None, - strict_fee_recipient: false, } } @@ -89,11 +87,6 @@ impl BlockServiceBuilder { 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, String> { Ok(BlockService { inner: Arc::new(Inner { @@ -111,7 +104,6 @@ impl BlockServiceBuilder { .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 { context: RuntimeContext, graffiti: Option, graffiti_file: Option, - strict_fee_recipient: bool, } /// Attempts to produce attestations for any block producer(s) at the start of the epoch. @@ -324,9 +315,7 @@ impl BlockService { 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 BlockService { } }; - // 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" diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index c59e1cf5d..5c7205a4a 100644 --- a/validator_client/src/cli.rs +++ b/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 \ - `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 \ - 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 \ - 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 \ - for payload construction, where a strict fee recipient check will still be applied.") + .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. \ + 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 \ + 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 \ + 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.") .takes_value(false), ) .arg( diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index fe5f7fc00..22472f751 100644 --- a/validator_client/src/config.rs +++ b/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>, - /// 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) diff --git a/validator_client/src/lib.rs b/validator_client/src/lib.rs index c05507576..9db4cc031 100644 --- a/validator_client/src/lib.rs +++ b/validator_client/src/lib.rs @@ -420,7 +420,6 @@ impl ProductionValidatorClient { .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()