From 3ea8476315bf8745dacabe59dbca1f7179328490 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sun, 25 Feb 2024 14:53:04 +0200 Subject: [PATCH] default vc to block v3 endpoint and deprecate block-v3 flag --- book/src/help_vc.md | 4 +- lighthouse/tests/validator_client.rs | 14 - validator_client/src/block_service.rs | 280 ++----------------- validator_client/src/cli.rs | 5 +- validator_client/src/config.rs | 7 - validator_client/src/http_metrics/metrics.rs | 1 - validator_client/src/validator_store.rs | 6 - 7 files changed, 29 insertions(+), 288 deletions(-) diff --git a/book/src/help_vc.md b/book/src/help_vc.md index 3d2519aac57..45a8f008335 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -68,9 +68,7 @@ FLAGS: If this flag is set, Lighthouse will always prefer blocks constructed by builders, regardless of payload value. --produce-block-v3 - Enable block production via the block v3 endpoint for this validator client. This should only be enabled - when paired with a beacon node that has this endpoint implemented. This flag will be enabled by default in - future. + This flag is deprecated and no longer in use. --unencrypted-http-transport This is a safety flag to ensure that the user is aware that the http transport is unencrypted and using a custom HTTP address is unsafe. diff --git a/lighthouse/tests/validator_client.rs b/lighthouse/tests/validator_client.rs index 764fd87ccdf..65ffc4f005f 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -421,20 +421,6 @@ fn no_doppelganger_protection_flag() { .run() .with_config(|config| assert!(!config.enable_doppelganger_protection)); } -#[test] -fn produce_block_v3_flag() { - CommandLineTest::new() - .flag("produce-block-v3", None) - .run() - .with_config(|config| assert!(config.produce_block_v3)); -} - -#[test] -fn no_produce_block_v3_flag() { - CommandLineTest::new() - .run() - .with_config(|config| assert!(!config.produce_block_v3)); -} #[test] fn no_gas_limit_flag() { diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs index 445d4f1a5d9..0d49f966ae0 100644 --- a/validator_client/src/block_service.rs +++ b/validator_client/src/block_service.rs @@ -323,105 +323,32 @@ impl BlockService { ) } - if self.validator_store.produce_block_v3() { - for validator_pubkey in proposers { - let builder_boost_factor = self.get_builder_boost_factor(&validator_pubkey); - let service = self.clone(); - let log = log.clone(); - self.inner.context.executor.spawn( - async move { - let result = service - .publish_block_v3(slot, validator_pubkey, builder_boost_factor) - .await; - - match result { - Ok(_) => {} - Err(BlockError::Recoverable(e)) | Err(BlockError::Irrecoverable(e)) => { - error!( - log, - "Error whilst producing block"; - "error" => ?e, - "block_slot" => ?slot, - "info" => "block v3 proposal failed, this error may or may not result in a missed block" - ); - } + for validator_pubkey in proposers { + let builder_boost_factor = self.get_builder_boost_factor(&validator_pubkey); + let service = self.clone(); + let log = log.clone(); + self.inner.context.executor.spawn( + async move { + let result = service + .publish_block(slot, validator_pubkey, builder_boost_factor) + .await; + + match result { + Ok(_) => {} + Err(BlockError::Recoverable(e)) | Err(BlockError::Irrecoverable(e)) => { + error!( + log, + "Error whilst producing block"; + "error" => ?e, + "block_slot" => ?slot, + "info" => "block v3 proposal failed, this error may or may not result in a missed block" + ); } - }, - "block service", - ) - } - } else { - for validator_pubkey in proposers { - let builder_proposals = self - .validator_store - .get_builder_proposals(&validator_pubkey); - let service = self.clone(); - let log = log.clone(); - self.inner.context.executor.spawn( - async move { - if builder_proposals { - let result = service - .publish_block(slot, validator_pubkey, true) - .await; - - match result { - Err(BlockError::Recoverable(e)) => { - error!( - log, - "Error whilst producing block"; - "error" => ?e, - "block_slot" => ?slot, - "info" => "blinded proposal failed, attempting full block" - ); - if let Err(e) = service - .publish_block(slot, validator_pubkey, false) - .await - { - // Log a `crit` since a full block - // (non-builder) proposal failed. - crit!( - log, - "Error whilst producing block"; - "error" => ?e, - "block_slot" => ?slot, - "info" => "full block attempted after a blinded failure", - ); - } - } - Err(BlockError::Irrecoverable(e)) => { - // Only log an `error` since it's common for - // builders to timeout on their response, only - // to publish the block successfully themselves. - error!( - log, - "Error whilst producing block"; - "error" => ?e, - "block_slot" => ?slot, - "info" => "this error may or may not result in a missed block", - ) - } - Ok(_) => {} - }; - } else if let Err(e) = service - .publish_block(slot, validator_pubkey, false) - .await - { - // Log a `crit` since a full block (non-builder) - // proposal failed. - crit!( - log, - "Error whilst producing block"; - "message" => ?e, - "block_slot" => ?slot, - "info" => "proposal did not use a builder", - ); - } - }, - "block service", - ) - } + } + }, + "block service", + ) } - Ok(()) } @@ -513,7 +440,7 @@ impl BlockService { Ok(()) } - async fn publish_block_v3( + async fn publish_block( self, slot: Slot, validator_pubkey: PublicKeyBytes, @@ -584,7 +511,7 @@ impl BlockService { &metrics::BLOCK_SERVICE_TIMES, &[metrics::BEACON_BLOCK_HTTP_GET], ); - let block_response = Self::get_validator_block_v3( + let block_response = Self::get_validator_block( beacon_node, slot, randao_reveal_ref, @@ -619,100 +546,6 @@ impl BlockService { Ok(()) } - /// Produce a block at the given slot for validator_pubkey - async fn publish_block( - &self, - slot: Slot, - validator_pubkey: PublicKeyBytes, - builder_proposal: bool, - ) -> Result<(), BlockError> { - let log = self.context.log(); - let _timer = - metrics::start_timer_vec(&metrics::BLOCK_SERVICE_TIMES, &[metrics::BEACON_BLOCK]); - - let randao_reveal = match self - .validator_store - .randao_reveal(validator_pubkey, slot.epoch(E::slots_per_epoch())) - .await - { - Ok(signature) => signature.into(), - Err(ValidatorStoreError::UnknownPubkey(pubkey)) => { - // A pubkey can be missing when a validator was recently removed - // via the API. - warn!( - log, - "Missing pubkey for block"; - "info" => "a validator may have recently been removed from this VC", - "pubkey" => ?pubkey, - "slot" => ?slot - ); - return Ok(()); - } - Err(e) => { - return Err(BlockError::Recoverable(format!( - "Unable to sign block: {:?}", - e - ))) - } - }; - - let graffiti = determine_graffiti( - &validator_pubkey, - log, - self.graffiti_file.clone(), - self.validator_store.graffiti(&validator_pubkey), - self.graffiti, - ); - - let randao_reveal_ref = &randao_reveal; - let self_ref = &self; - let proposer_index = self.validator_store.validator_index(&validator_pubkey); - let proposer_fallback = ProposerFallback { - beacon_nodes: self.beacon_nodes.clone(), - proposer_nodes: self.proposer_nodes.clone(), - }; - - info!( - log, - "Requesting unsigned block"; - "slot" => slot.as_u64(), - ); - - // Request block from first responsive beacon node. - // - // Try the proposer nodes last, since it's likely that they don't have a - // great view of attestations on the network. - let unsigned_block = proposer_fallback - .request_proposers_last( - RequireSynced::No, - OfflineOnFailure::Yes, - move |beacon_node| { - Self::get_validator_block( - beacon_node, - slot, - randao_reveal_ref, - graffiti, - proposer_index, - builder_proposal, - log, - ) - }, - ) - .await?; - - self_ref - .sign_and_publish_block( - proposer_fallback, - slot, - graffiti, - &validator_pubkey, - unsigned_block, - ) - .await?; - - Ok(()) - } - async fn publish_signed_block_contents( &self, signed_block: &SignedBlock, @@ -745,7 +578,7 @@ impl BlockService { Ok::<_, BlockError>(()) } - async fn get_validator_block_v3( + async fn get_validator_block( beacon_node: &BeaconNodeHttpClient, slot: Slot, randao_reveal_ref: &SignatureBytes, @@ -788,65 +621,6 @@ impl BlockService { Ok::<_, BlockError>(unsigned_block) } - async fn get_validator_block( - beacon_node: &BeaconNodeHttpClient, - slot: Slot, - randao_reveal_ref: &SignatureBytes, - graffiti: Option, - proposer_index: Option, - builder_proposal: bool, - log: &Logger, - ) -> Result, BlockError> { - let unsigned_block = if !builder_proposal { - let _get_timer = metrics::start_timer_vec( - &metrics::BLOCK_SERVICE_TIMES, - &[metrics::BEACON_BLOCK_HTTP_GET], - ); - UnsignedBlock::Full( - beacon_node - .get_validator_blocks::(slot, randao_reveal_ref, graffiti.as_ref()) - .await - .map_err(|e| { - BlockError::Recoverable(format!( - "Error from beacon node when producing block: {:?}", - e - )) - })? - .data, - ) - } else { - let _get_timer = metrics::start_timer_vec( - &metrics::BLOCK_SERVICE_TIMES, - &[metrics::BLINDED_BEACON_BLOCK_HTTP_GET], - ); - UnsignedBlock::Blinded( - beacon_node - .get_validator_blinded_blocks::(slot, randao_reveal_ref, graffiti.as_ref()) - .await - .map_err(|e| { - BlockError::Recoverable(format!( - "Error from beacon node when producing block: {:?}", - e - )) - })? - .data, - ) - }; - - info!( - log, - "Received unsigned block"; - "slot" => slot.as_u64(), - ); - if proposer_index != Some(unsigned_block.proposer_index()) { - return Err(BlockError::Recoverable( - "Proposer index does not match block proposer. Beacon chain re-orged".to_string(), - )); - } - - Ok::<_, BlockError>(unsigned_block) - } - /// Returns the builder boost factor of the given public key. /// The priority order for fetching this value is: /// diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 16a265212e5..982db381db0 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -139,10 +139,7 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .arg( Arg::with_name("produce-block-v3") .long("produce-block-v3") - .help("Enable block production via the block v3 endpoint for this validator client. \ - This should only be enabled when paired with a beacon node \ - that has this endpoint implemented. This flag will be enabled by default in \ - future.") + .help("This flag is deprecated and is no longer in use.") .takes_value(false) ) .arg( diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index ae59829a3e6..68244513d04 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -78,8 +78,6 @@ pub struct Config { pub validator_registration_batch_size: usize, /// Enable slashing protection even while using web3signer keys. pub enable_web3signer_slashing_protection: bool, - /// Enables block production via the block v3 endpoint. This configuration option can be removed post deneb. - pub produce_block_v3: bool, /// Specifies the boost factor, a percentage multiplier to apply to the builder's payload value. pub builder_boost_factor: Option, /// If true, Lighthouse will prefer builder proposals, if available. @@ -129,7 +127,6 @@ impl Default for Config { enable_latency_measurement_service: true, validator_registration_batch_size: 500, enable_web3signer_slashing_protection: true, - produce_block_v3: false, builder_boost_factor: None, prefer_builder_proposals: false, distributed: false, @@ -379,10 +376,6 @@ impl Config { config.builder_proposals = true; } - if cli_args.is_present("produce-block-v3") { - config.produce_block_v3 = true; - } - if cli_args.is_present("prefer-builder-proposals") { config.prefer_builder_proposals = true; } diff --git a/validator_client/src/http_metrics/metrics.rs b/validator_client/src/http_metrics/metrics.rs index 52b52126bd6..234c242fdf0 100644 --- a/validator_client/src/http_metrics/metrics.rs +++ b/validator_client/src/http_metrics/metrics.rs @@ -11,7 +11,6 @@ pub const UNREGISTERED: &str = "unregistered"; pub const FULL_UPDATE: &str = "full_update"; pub const BEACON_BLOCK: &str = "beacon_block"; pub const BEACON_BLOCK_HTTP_GET: &str = "beacon_block_http_get"; -pub const BLINDED_BEACON_BLOCK_HTTP_GET: &str = "blinded_beacon_block_http_get"; pub const BEACON_BLOCK_HTTP_POST: &str = "beacon_block_http_post"; pub const BLINDED_BEACON_BLOCK_HTTP_POST: &str = "blinded_beacon_block_http_post"; pub const ATTESTATIONS: &str = "attestations"; diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index b8c11a79bc0..006633e9d94 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -98,7 +98,6 @@ pub struct ValidatorStore { gas_limit: Option, builder_proposals: bool, enable_web3signer_slashing_protection: bool, - produce_block_v3: bool, prefer_builder_proposals: bool, builder_boost_factor: Option, task_executor: TaskExecutor, @@ -133,7 +132,6 @@ impl ValidatorStore { gas_limit: config.gas_limit, builder_proposals: config.builder_proposals, enable_web3signer_slashing_protection: config.enable_web3signer_slashing_protection, - produce_block_v3: config.produce_block_v3, prefer_builder_proposals: config.prefer_builder_proposals, builder_boost_factor: config.builder_boost_factor, task_executor, @@ -348,10 +346,6 @@ impl ValidatorStore { self.spec.fork_at_epoch(epoch) } - pub fn produce_block_v3(&self) -> bool { - self.produce_block_v3 - } - /// Returns a `SigningMethod` for `validator_pubkey` *only if* that validator is considered safe /// by doppelganger protection. fn doppelganger_checked_signing_method(