From 5c8c8da8b11892a458e16ccefe349b2f3144477b Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 8 Jan 2024 23:12:39 +0200 Subject: [PATCH] Use blocks v3 endpoint in the VC (#4813) * block v3 endpoint init * block v3 flow * block v3 flow * continue refactor * the full flow... * add api logic * add api logic * add new endpoint version * added v3 endpoint * some debugging * merge v2 flow with v3 * debugging * tests passing * tests passing * revert cargo lock * initial v3 test * blinded payload test case passing * fix clippy issues * cleanup * cleanup * remove dead code * fixed logs * add block value * block value fix * linting * merge unstable * refactor * add consensus block value * lint * update header name to consensus block value * prevent setting the participation flag * clone get_epoch_participation result * fmt * clone epoch participation outside of the loop * add block v3 to vc * add v3 logic into vc * add produce-block-v3 * refactor based on feedback * update * remove comments * refactor * header bugfix * fmt * resolve merge conflicts * fix merge * fix merge * refactor * refactor * cleanup * lint * changes based on feedback * revert * remove block v3 fallback to v2 * publish_block_v3 should return irrecoveerable errors * comments * comments * fixed issues from merge * merge conflicts * Don't activate at fork; support builder_proposals * Update CLI flags & book * Remove duplicate `current_slot` parameter in `publish_block` function, and remove unnecessary clone. * Revert changes on making block errors irrecoverable. --------- Co-authored-by: Michael Sproul Co-authored-by: Jimmy Chen --- book/src/help_vc.md | 4 + lighthouse/tests/validator_client.rs | 15 + validator_client/src/block_service.rs | 403 ++++++++++++++++++------ validator_client/src/cli.rs | 9 + validator_client/src/config.rs | 7 + validator_client/src/validator_store.rs | 6 + 6 files changed, 347 insertions(+), 97 deletions(-) diff --git a/book/src/help_vc.md b/book/src/help_vc.md index 4471b0e1044..62b64efd411 100644 --- a/book/src/help_vc.md +++ b/book/src/help_vc.md @@ -56,6 +56,10 @@ FLAGS: machine. Note that logs can often contain sensitive information about your validator and so this flag should be used with caution. For Windows users, the log file permissions will be inherited from the parent folder. --metrics Enable the Prometheus metrics HTTP server. Disabled by default. + --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. --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 4234de613da..025188fed4e 100644 --- a/lighthouse/tests/validator_client.rs +++ b/lighthouse/tests/validator_client.rs @@ -421,6 +421,21 @@ 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() { CommandLineTest::new() diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs index 00d9b2e86db..b65e301de86 100644 --- a/validator_client/src/block_service.rs +++ b/validator_client/src/block_service.rs @@ -28,7 +28,10 @@ use types::{ #[derive(Debug)] pub enum BlockError { + /// A recoverable error that can be retried, as the validator has not signed anything. Recoverable(String), + /// An irrecoverable error has occurred during block proposal and should not be retried, as a + /// block may have already been signed. Irrecoverable(String), } @@ -320,89 +323,212 @@ impl BlockService { ) } - 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; + if self.validator_store.produce_block_v3() { + for validator_pubkey in proposers { + let builder_proposals = self + .validator_store + .get_builder_proposals(&validator_pubkey); + // Translate `builder_proposals` to a boost factor. Builder proposals set to `true` + // requires no boost factor, it just means "use a builder proposal if the BN returns + // one". On the contrary, `builder_proposals: false` indicates a preference for + // local payloads, so we set the builder boost factor to 0. + let builder_boost_factor = if !builder_proposals { Some(0) } else { None }; + 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 { - Err(BlockError::Recoverable(e)) => { + Ok(_) => {} + Err(BlockError::Recoverable(e)) | Err(BlockError::Irrecoverable(e)) => { error!( log, "Error whilst producing block"; "error" => ?e, "block_slot" => ?slot, - "info" => "blinded proposal failed, attempting full block" + "info" => "block v3 proposal failed, this error may or may not result in a missed 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!( + } + } + }, + "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" => "full block attempted after a blinded failure", + "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!( + 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"; - "error" => ?e, + "message" => ?e, "block_slot" => ?slot, - "info" => "this error may or may not result in a missed block", - ) + "info" => "proposal did not use a builder", + ); } - 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(()) } - /// Produce a block at the given slot for validator_pubkey - async fn publish_block( + #[allow(clippy::too_many_arguments)] + async fn sign_and_publish_block( &self, + proposer_fallback: ProposerFallback, + slot: Slot, + graffiti: Option, + validator_pubkey: &PublicKeyBytes, + unsigned_block: UnsignedBlock, + ) -> Result<(), BlockError> { + let log = self.context.log(); + let signing_timer = metrics::start_timer(&metrics::BLOCK_SIGNING_TIMES); + + let res = match unsigned_block { + UnsignedBlock::Full(block_contents) => { + let (block, maybe_blobs) = block_contents.deconstruct(); + self.validator_store + .sign_block(*validator_pubkey, block, slot) + .await + .map(|b| SignedBlock::Full(PublishBlockRequest::new(b, maybe_blobs))) + } + UnsignedBlock::Blinded(block) => self + .validator_store + .sign_block(*validator_pubkey, block, slot) + .await + .map(SignedBlock::Blinded), + }; + + let signed_block = match res { + Ok(block) => block, + 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 signing_time_ms = + Duration::from_secs_f64(signing_timer.map_or(0.0, |t| t.stop_and_record())).as_millis(); + + info!( + log, + "Publishing signed block"; + "slot" => slot.as_u64(), + "signing_time_ms" => signing_time_ms, + ); + + // Publish block with first available beacon node. + // + // Try the proposer nodes first, since we've likely gone to efforts to + // protect them from DoS attacks and they're most likely to successfully + // publish a block. + proposer_fallback + .request_proposers_first( + RequireSynced::No, + OfflineOnFailure::Yes, + |beacon_node| async { + self.publish_signed_block_contents(&signed_block, beacon_node) + .await + }, + ) + .await?; + + info!( + log, + "Successfully published block"; + "block_type" => ?signed_block.block_type(), + "deposits" => signed_block.num_deposits(), + "attestations" => signed_block.num_attestations(), + "graffiti" => ?graffiti.map(|g| g.as_utf8_lossy()), + "slot" => signed_block.slot().as_u64(), + ); + Ok(()) + } + + async fn publish_block_v3( + self, slot: Slot, validator_pubkey: PublicKeyBytes, - builder_proposal: bool, + builder_boost_factor: Option, ) -> Result<(), BlockError> { let log = self.context.log(); let _timer = metrics::start_timer_vec(&metrics::BLOCK_SERVICE_TIMES, &[metrics::BEACON_BLOCK]); - let current_slot = self.slot_clock.now().ok_or_else(|| { - BlockError::Recoverable("Unable to determine current slot from clock".to_string()) - })?; - let randao_reveal = match self .validator_store .randao_reveal(validator_pubkey, slot.epoch(E::slots_per_epoch())) @@ -440,7 +566,6 @@ impl BlockService { let randao_reveal_ref = &randao_reveal; let self_ref = &self; let proposer_index = self.validator_store.validator_index(&validator_pubkey); - let validator_pubkey_ref = &validator_pubkey; let proposer_fallback = ProposerFallback { beacon_nodes: self.beacon_nodes.clone(), proposer_nodes: self.proposer_nodes.clone(), @@ -460,40 +585,63 @@ impl BlockService { .request_proposers_last( RequireSynced::No, OfflineOnFailure::Yes, - move |beacon_node| { - Self::get_validator_block( + |beacon_node| async move { + let _get_timer = metrics::start_timer_vec( + &metrics::BLOCK_SERVICE_TIMES, + &[metrics::BEACON_BLOCK_HTTP_GET], + ); + let block_response = Self::get_validator_block_v3( beacon_node, slot, randao_reveal_ref, graffiti, proposer_index, - builder_proposal, + builder_boost_factor, log, ) + .await + .map_err(|e| { + BlockError::Recoverable(format!( + "Error from beacon node when producing block: {:?}", + e + )) + }); + + Ok::<_, BlockError>(block_response) }, ) + .await??; + + self_ref + .sign_and_publish_block( + proposer_fallback, + slot, + graffiti, + &validator_pubkey, + unsigned_block, + ) .await?; - let signing_timer = metrics::start_timer(&metrics::BLOCK_SIGNING_TIMES); + Ok(()) + } - let res = match unsigned_block { - UnsignedBlock::Full(block_contents) => { - let (block, maybe_blobs) = block_contents.deconstruct(); - self_ref - .validator_store - .sign_block(*validator_pubkey_ref, block, current_slot) - .await - .map(|b| SignedBlock::Full(PublishBlockRequest::new(b, maybe_blobs))) - } - UnsignedBlock::Blinded(block) => self_ref - .validator_store - .sign_block(*validator_pubkey_ref, block, current_slot) - .await - .map(SignedBlock::Blinded), - }; + /// 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 signed_block = match res { - Ok(block) => 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. @@ -514,41 +662,59 @@ impl BlockService { } }; - let signing_time_ms = - Duration::from_secs_f64(signing_timer.map_or(0.0, |t| t.stop_and_record())).as_millis(); + 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, - "Publishing signed block"; + "Requesting unsigned block"; "slot" => slot.as_u64(), - "signing_time_ms" => signing_time_ms, ); - // Publish block with first available beacon node. + // Request block from first responsive beacon node. // - // Try the proposer nodes first, since we've likely gone to efforts to - // protect them from DoS attacks and they're most likely to successfully - // publish a block. - proposer_fallback - .request_proposers_first( + // 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, - |beacon_node| async { - self.publish_signed_block_contents(&signed_block, beacon_node) - .await + move |beacon_node| { + Self::get_validator_block( + beacon_node, + slot, + randao_reveal_ref, + graffiti, + proposer_index, + builder_proposal, + log, + ) }, ) .await?; - info!( - log, - "Successfully published block"; - "block_type" => ?signed_block.block_type(), - "deposits" => signed_block.num_deposits(), - "attestations" => signed_block.num_attestations(), - "graffiti" => ?graffiti.map(|g| g.as_utf8_lossy()), - "slot" => signed_block.slot().as_u64(), - ); + self_ref + .sign_and_publish_block( + proposer_fallback, + slot, + graffiti, + &validator_pubkey, + unsigned_block, + ) + .await?; Ok(()) } @@ -585,6 +751,49 @@ impl BlockService { Ok::<_, BlockError>(()) } + async fn get_validator_block_v3( + beacon_node: &BeaconNodeHttpClient, + slot: Slot, + randao_reveal_ref: &SignatureBytes, + graffiti: Option, + proposer_index: Option, + builder_boost_factor: Option, + log: &Logger, + ) -> Result, BlockError> { + let (block_response, _) = beacon_node + .get_validator_blocks_v3::( + slot, + randao_reveal_ref, + graffiti.as_ref(), + builder_boost_factor, + ) + .await + .map_err(|e| { + BlockError::Recoverable(format!( + "Error from beacon node when producing block: {:?}", + e + )) + })?; + + let unsigned_block = match block_response.data { + eth2::types::ProduceBlockV3Response::Full(block) => UnsignedBlock::Full(block), + eth2::types::ProduceBlockV3Response::Blinded(block) => UnsignedBlock::Blinded(block), + }; + + 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) + } + async fn get_validator_block( beacon_node: &BeaconNodeHttpClient, slot: Slot, diff --git a/validator_client/src/cli.rs b/validator_client/src/cli.rs index 6957934fb8d..cd3ad494dad 100644 --- a/validator_client/src/cli.rs +++ b/validator_client/src/cli.rs @@ -136,6 +136,15 @@ pub fn cli_app<'a, 'b>() -> App<'a, 'b> { .value_name("FEE-RECIPIENT") .takes_value(true) ) + .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.") + .takes_value(false) + ) /* REST API related arguments */ .arg( Arg::with_name("http") diff --git a/validator_client/src/config.rs b/validator_client/src/config.rs index 95d42d6d83a..4b7da764287 100644 --- a/validator_client/src/config.rs +++ b/validator_client/src/config.rs @@ -75,6 +75,8 @@ pub struct Config { pub enable_latency_measurement_service: bool, /// Defines the number of validators per `validator/register_validator` request sent to the BN. pub validator_registration_batch_size: usize, + /// Enables block production via the block v3 endpoint. This configuration option can be removed post deneb. + pub produce_block_v3: bool, } impl Default for Config { @@ -115,6 +117,7 @@ impl Default for Config { broadcast_topics: vec![ApiTopic::Subscriptions], enable_latency_measurement_service: true, validator_registration_batch_size: 500, + produce_block_v3: false, } } } @@ -339,6 +342,10 @@ impl Config { config.builder_proposals = true; } + if cli_args.is_present("produce-block-v3") { + config.produce_block_v3 = true; + } + config.gas_limit = cli_args .value_of("gas-limit") .map(|gas_limit| { diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index 60155d8efb7..19726c2aec1 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -97,6 +97,7 @@ pub struct ValidatorStore { fee_recipient_process: Option
, gas_limit: Option, builder_proposals: bool, + produce_block_v3: bool, task_executor: TaskExecutor, _phantom: PhantomData, } @@ -128,6 +129,7 @@ impl ValidatorStore { fee_recipient_process: config.fee_recipient, gas_limit: config.gas_limit, builder_proposals: config.builder_proposals, + produce_block_v3: config.produce_block_v3, task_executor, _phantom: PhantomData, } @@ -336,6 +338,10 @@ 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(