From 0688932de28de0bf18b00ed7a2e0f9b5f6cc09fd Mon Sep 17 00:00:00 2001 From: Daniel Knopik <107140945+dknopik@users.noreply.github.com> Date: Wed, 21 May 2025 02:50:16 +0200 Subject: [PATCH] Pass blobs into `ValidatorStore::sign_block` (#7497) While the Lighthouse implementation of the `ValidatorStore` does not really care about blobs, Anchor needs to be able to return different blobs from `sign_blocks` than what was passed into it, in case it decides to sign another Anchor node's block. Only passing the unsigned block into `sign_block` and only returning a signed block from it (without any blobs and proofs) was an oversight in #6705. - Replace `validator_store::{Uns,S}ignedBlock` with `validator_store::block_service::{Uns,S}ignedBlock`, as we need all data in there. - In `lighthouse_validator_store`, just add the received blobs back to the signed block after signing it. --- Cargo.lock | 2 + testing/web3signer_tests/Cargo.toml | 1 + testing/web3signer_tests/src/lib.rs | 30 ++--- .../lighthouse_validator_store/src/lib.rs | 15 ++- .../validator_services/src/block_service.rs | 119 +++++++----------- validator_client/validator_store/Cargo.toml | 1 + validator_client/validator_store/src/lib.rs | 43 ++----- 7 files changed, 83 insertions(+), 128 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 204bc39082..30be5fa233 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -9804,6 +9804,7 @@ dependencies = [ name = "validator_store" version = "0.1.0" dependencies = [ + "eth2", "slashing_protection", "types", ] @@ -10059,6 +10060,7 @@ dependencies = [ "account_utils", "async-channel 1.9.0", "environment", + "eth2", "eth2_keystore", "eth2_network_config", "futures", diff --git a/testing/web3signer_tests/Cargo.toml b/testing/web3signer_tests/Cargo.toml index f68fa56e16..b4637b4030 100644 --- a/testing/web3signer_tests/Cargo.toml +++ b/testing/web3signer_tests/Cargo.toml @@ -10,6 +10,7 @@ edition = { workspace = true } account_utils = { workspace = true } async-channel = { workspace = true } environment = { workspace = true } +eth2 = { workspace = true } eth2_keystore = { workspace = true } eth2_network_config = { workspace = true } futures = { workspace = true } diff --git a/testing/web3signer_tests/src/lib.rs b/testing/web3signer_tests/src/lib.rs index 8678eff0ee..4bc0f62346 100644 --- a/testing/web3signer_tests/src/lib.rs +++ b/testing/web3signer_tests/src/lib.rs @@ -20,6 +20,7 @@ mod tests { use account_utils::validator_definitions::{ SigningDefinition, ValidatorDefinition, ValidatorDefinitions, Web3SignerDefinition, }; + use eth2::types::FullBlockContents; use eth2_keystore::KeystoreBuilder; use eth2_network_config::Eth2NetworkConfig; use initialized_validators::{ @@ -45,7 +46,9 @@ mod tests { use tokio::time::sleep; use types::{attestation::AttestationBase, *}; use url::Url; - use validator_store::{Error as ValidatorStoreError, SignedBlock, ValidatorStore}; + use validator_store::{ + Error as ValidatorStoreError, SignedBlock, UnsignedBlock, ValidatorStore, + }; /// If the we are unable to reach the Web3Signer HTTP API within this time out then we will /// assume it failed to start. @@ -595,8 +598,9 @@ mod tests { async move { let block = BeaconBlock::::Base(BeaconBlockBase::empty(&spec)); let block_slot = block.slot(); + let unsigned_block = UnsignedBlock::Full(FullBlockContents::Block(block)); validator_store - .sign_block(pubkey, block.into(), block_slot) + .sign_block(pubkey, unsigned_block, block_slot) .await .unwrap() } @@ -665,12 +669,10 @@ mod tests { async move { let mut altair_block = BeaconBlockAltair::empty(&spec); altair_block.slot = altair_fork_slot; + let unsigned_block = + UnsignedBlock::Full(FullBlockContents::Block(altair_block.into())); validator_store - .sign_block( - pubkey, - BeaconBlock::::Altair(altair_block).into(), - altair_fork_slot, - ) + .sign_block(pubkey, unsigned_block, altair_fork_slot) .await .unwrap() } @@ -752,12 +754,10 @@ mod tests { async move { let mut bellatrix_block = BeaconBlockBellatrix::empty(&spec); bellatrix_block.slot = bellatrix_fork_slot; + let unsigned_block = + UnsignedBlock::Full(FullBlockContents::Block(bellatrix_block.into())); validator_store - .sign_block( - pubkey, - BeaconBlock::::Bellatrix(bellatrix_block).into(), - bellatrix_fork_slot, - ) + .sign_block(pubkey, unsigned_block, bellatrix_fork_slot) .await .unwrap() } @@ -876,8 +876,9 @@ mod tests { .assert_signatures_match("first_block", |pubkey, validator_store| async move { let block = first_block(); let slot = block.slot(); + let unsigned_block = UnsignedBlock::Full(FullBlockContents::Block(block)); validator_store - .sign_block(pubkey, block.into(), slot) + .sign_block(pubkey, unsigned_block, slot) .await .unwrap() }) @@ -887,8 +888,9 @@ mod tests { move |pubkey, validator_store| async move { let block = double_vote_block(); let slot = block.slot(); + let unsigned_block = UnsignedBlock::Full(FullBlockContents::Block(block)); validator_store - .sign_block(pubkey, block.into(), slot) + .sign_block(pubkey, unsigned_block, slot) .await .map(|_| ()) }, diff --git a/validator_client/lighthouse_validator_store/src/lib.rs b/validator_client/lighthouse_validator_store/src/lib.rs index d07f95f11c..2cb6ba435e 100644 --- a/validator_client/lighthouse_validator_store/src/lib.rs +++ b/validator_client/lighthouse_validator_store/src/lib.rs @@ -1,5 +1,6 @@ use account_utils::validator_definitions::{PasswordStorage, ValidatorDefinition}; use doppelganger_service::DoppelgangerService; +use eth2::types::PublishBlockRequest; use initialized_validators::InitializedValidators; use logging::crit; use parking_lot::{Mutex, RwLock}; @@ -733,14 +734,18 @@ impl ValidatorStore for LighthouseValidatorS current_slot: Slot, ) -> Result, Error> { match block { - UnsignedBlock::Full(block) => self - .sign_abstract_block(validator_pubkey, block, current_slot) - .await - .map(SignedBlock::Full), + UnsignedBlock::Full(block) => { + let (block, blobs) = block.deconstruct(); + self.sign_abstract_block(validator_pubkey, block, current_slot) + .await + .map(|block| { + SignedBlock::Full(PublishBlockRequest::new(Arc::new(block), blobs)) + }) + } UnsignedBlock::Blinded(block) => self .sign_abstract_block(validator_pubkey, block, current_slot) .await - .map(SignedBlock::Blinded), + .map(|block| SignedBlock::Blinded(Arc::new(block))), } } diff --git a/validator_client/validator_services/src/block_service.rs b/validator_client/validator_services/src/block_service.rs index 2f29c1feb7..01f786e160 100644 --- a/validator_client/validator_services/src/block_service.rs +++ b/validator_client/validator_services/src/block_service.rs @@ -1,6 +1,5 @@ use beacon_node_fallback::{ApiTopic, BeaconNodeFallback, Error as FallbackError, Errors}; use bls::SignatureBytes; -use eth2::types::{FullBlockContents, PublishBlockRequest}; use eth2::{BeaconNodeHttpClient, StatusCode}; use graffiti_file::{determine_graffiti, GraffitiFile}; use logging::crit; @@ -13,11 +12,8 @@ use std::time::Duration; use task_executor::TaskExecutor; use tokio::sync::mpsc; use tracing::{debug, error, info, trace, warn}; -use types::{ - BlindedBeaconBlock, BlockType, ChainSpec, EthSpec, Graffiti, PublicKeyBytes, - SignedBlindedBeaconBlock, Slot, -}; -use validator_store::{Error as ValidatorStoreError, ValidatorStore}; +use types::{BlockType, ChainSpec, EthSpec, Graffiti, PublicKeyBytes, Slot}; +use validator_store::{Error as ValidatorStoreError, SignedBlock, UnsignedBlock, ValidatorStore}; #[derive(Debug)] pub enum BlockError { @@ -335,26 +331,10 @@ impl BlockService { ) -> Result<(), BlockError> { let signing_timer = validator_metrics::start_timer(&validator_metrics::BLOCK_SIGNING_TIMES); - let (block, maybe_blobs) = match unsigned_block { - UnsignedBlock::Full(block_contents) => { - let (block, maybe_blobs) = block_contents.deconstruct(); - (block.into(), maybe_blobs) - } - UnsignedBlock::Blinded(block) => (block.into(), None), - }; - let res = self .validator_store - .sign_block(*validator_pubkey, block, slot) - .await - .map(|block| match block { - validator_store::SignedBlock::Full(block) => { - SignedBlock::Full(PublishBlockRequest::new(Arc::new(block), maybe_blobs)) - } - validator_store::SignedBlock::Blinded(block) => { - SignedBlock::Blinded(Arc::new(block)) - } - }); + .sign_block(*validator_pubkey, unsigned_block, slot) + .await; let signed_block = match res { Ok(block) => block, @@ -398,12 +378,13 @@ impl BlockService { }) .await?; + let metadata = BlockMetadata::from(&signed_block); info!( - block_type = ?signed_block.block_type(), - deposits = signed_block.num_deposits(), - attestations = signed_block.num_attestations(), + block_type = ?metadata.block_type, + deposits = metadata.num_deposits, + attestations = metadata.num_attestations, graffiti = ?graffiti.map(|g| g.as_utf8_lossy()), - slot = signed_block.slot().as_u64(), + slot = metadata.slot.as_u64(), "Successfully published block" ); Ok(()) @@ -508,7 +489,6 @@ impl BlockService { signed_block: &SignedBlock, beacon_node: BeaconNodeHttpClient, ) -> Result<(), BlockError> { - let slot = signed_block.slot(); match signed_block { SignedBlock::Full(signed_block) => { let _post_timer = validator_metrics::start_timer_vec( @@ -518,7 +498,9 @@ impl BlockService { beacon_node .post_beacon_blocks_v2_ssz(signed_block, None) .await - .or_else(|e| handle_block_post_error(e, slot))? + .or_else(|e| { + handle_block_post_error(e, signed_block.signed_block().message().slot()) + })? } SignedBlock::Blinded(signed_block) => { let _post_timer = validator_metrics::start_timer_vec( @@ -528,7 +510,7 @@ impl BlockService { beacon_node .post_beacon_blinded_blocks_v2_ssz(signed_block, None) .await - .or_else(|e| handle_block_post_error(e, slot))? + .or_else(|e| handle_block_post_error(e, signed_block.message().slot()))? } } Ok::<_, BlockError>(()) @@ -557,13 +539,17 @@ impl BlockService { )) })?; - let unsigned_block = match block_response.data { - eth2::types::ProduceBlockV3Response::Full(block) => UnsignedBlock::Full(block), - eth2::types::ProduceBlockV3Response::Blinded(block) => UnsignedBlock::Blinded(block), + let (block_proposer, unsigned_block) = match block_response.data { + eth2::types::ProduceBlockV3Response::Full(block) => { + (block.block().proposer_index(), UnsignedBlock::Full(block)) + } + eth2::types::ProduceBlockV3Response::Blinded(block) => { + (block.proposer_index(), UnsignedBlock::Blinded(block)) + } }; info!(slot = slot.as_u64(), "Received unsigned block"); - if proposer_index != Some(unsigned_block.proposer_index()) { + if proposer_index != Some(block_proposer) { return Err(BlockError::Recoverable( "Proposer index does not match block proposer. Beacon chain re-orged".to_string(), )); @@ -573,49 +559,30 @@ impl BlockService { } } -pub enum UnsignedBlock { - Full(FullBlockContents), - Blinded(BlindedBeaconBlock), +/// Wrapper for values we want to log about a block we signed, for easy extraction from the possible +/// variants. +struct BlockMetadata { + block_type: BlockType, + slot: Slot, + num_deposits: usize, + num_attestations: usize, } -impl UnsignedBlock { - pub fn proposer_index(&self) -> u64 { - match self { - UnsignedBlock::Full(block) => block.block().proposer_index(), - UnsignedBlock::Blinded(block) => block.proposer_index(), - } - } -} - -#[derive(Debug)] -pub enum SignedBlock { - Full(PublishBlockRequest), - Blinded(Arc>), -} - -impl SignedBlock { - pub fn block_type(&self) -> BlockType { - match self { - SignedBlock::Full(_) => BlockType::Full, - SignedBlock::Blinded(_) => BlockType::Blinded, - } - } - pub fn slot(&self) -> Slot { - match self { - SignedBlock::Full(block) => block.signed_block().message().slot(), - SignedBlock::Blinded(block) => block.message().slot(), - } - } - pub fn num_deposits(&self) -> usize { - match self { - SignedBlock::Full(block) => block.signed_block().message().body().deposits().len(), - SignedBlock::Blinded(block) => block.message().body().deposits().len(), - } - } - pub fn num_attestations(&self) -> usize { - match self { - SignedBlock::Full(block) => block.signed_block().message().body().attestations_len(), - SignedBlock::Blinded(block) => block.message().body().attestations_len(), +impl From<&SignedBlock> for BlockMetadata { + fn from(value: &SignedBlock) -> Self { + match value { + SignedBlock::Full(block) => BlockMetadata { + block_type: BlockType::Full, + slot: block.signed_block().message().slot(), + num_deposits: block.signed_block().message().body().deposits().len(), + num_attestations: block.signed_block().message().body().attestations_len(), + }, + SignedBlock::Blinded(block) => BlockMetadata { + block_type: BlockType::Blinded, + slot: block.message().slot(), + num_deposits: block.message().body().deposits().len(), + num_attestations: block.message().body().attestations_len(), + }, } } } diff --git a/validator_client/validator_store/Cargo.toml b/validator_client/validator_store/Cargo.toml index 91df9dc3ab..8c5451b2d0 100644 --- a/validator_client/validator_store/Cargo.toml +++ b/validator_client/validator_store/Cargo.toml @@ -5,5 +5,6 @@ edition = { workspace = true } authors = ["Sigma Prime "] [dependencies] +eth2 = { workspace = true } slashing_protection = { workspace = true } types = { workspace = true } diff --git a/validator_client/validator_store/src/lib.rs b/validator_client/validator_store/src/lib.rs index 9de3a6d66a..c3b551c249 100644 --- a/validator_client/validator_store/src/lib.rs +++ b/validator_client/validator_store/src/lib.rs @@ -1,12 +1,13 @@ +use eth2::types::{FullBlockContents, PublishBlockRequest}; use slashing_protection::NotSafe; use std::fmt::Debug; use std::future::Future; +use std::sync::Arc; use types::{ - Address, Attestation, AttestationError, BeaconBlock, BlindedBeaconBlock, Epoch, EthSpec, - Graffiti, Hash256, PublicKeyBytes, SelectionProof, Signature, SignedAggregateAndProof, - SignedBeaconBlock, SignedBlindedBeaconBlock, SignedContributionAndProof, - SignedValidatorRegistrationData, Slot, SyncCommitteeContribution, SyncCommitteeMessage, - SyncSelectionProof, SyncSubnetId, ValidatorRegistrationData, + Address, Attestation, AttestationError, BlindedBeaconBlock, Epoch, EthSpec, Graffiti, Hash256, + PublicKeyBytes, SelectionProof, Signature, SignedAggregateAndProof, SignedBlindedBeaconBlock, + SignedContributionAndProof, SignedValidatorRegistrationData, Slot, SyncCommitteeContribution, + SyncCommitteeMessage, SyncSelectionProof, SyncSubnetId, ValidatorRegistrationData, }; #[derive(Debug, PartialEq, Clone)] @@ -170,40 +171,16 @@ pub trait ValidatorStore: Send + Sync { fn proposal_data(&self, pubkey: &PublicKeyBytes) -> Option; } -#[derive(Clone, Debug, PartialEq)] +#[derive(Debug)] pub enum UnsignedBlock { - Full(BeaconBlock), + Full(FullBlockContents), Blinded(BlindedBeaconBlock), } -impl From> for UnsignedBlock { - fn from(block: BeaconBlock) -> Self { - UnsignedBlock::Full(block) - } -} - -impl From> for UnsignedBlock { - fn from(block: BlindedBeaconBlock) -> Self { - UnsignedBlock::Blinded(block) - } -} - #[derive(Clone, Debug, PartialEq)] pub enum SignedBlock { - Full(SignedBeaconBlock), - Blinded(SignedBlindedBeaconBlock), -} - -impl From> for SignedBlock { - fn from(block: SignedBeaconBlock) -> Self { - SignedBlock::Full(block) - } -} - -impl From> for SignedBlock { - fn from(block: SignedBlindedBeaconBlock) -> Self { - SignedBlock::Blinded(block) - } + Full(PublishBlockRequest), + Blinded(Arc>), } /// A wrapper around `PublicKeyBytes` which encodes information about the status of a validator