Pass blobs into ValidatorStore::sign_block (#7497)

dknopik · web-flow · commit 0688932de28d · 2025-05-21T00:50:16.000Z
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.
diff --git a/Cargo.lock b/Cargo.lock
diff --git 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
@@ -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::<E>::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::<E>::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::<E>::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
@@ -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<T: SlotClock + 'static, E: EthSpec> ValidatorStore for LighthouseValidatorS
         current_slot: Slot,
     ) -> Result<SignedBlock<E>, 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
@@ -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<S: ValidatorStore + 'static, T: SlotClock + 'static> BlockService<S, T> {
     ) -> 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<S: ValidatorStore + 'static, T: SlotClock + 'static> BlockService<S, T> {
             })
             .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<S: ValidatorStore + 'static, T: SlotClock + 'static> BlockService<S, T> {
         signed_block: &SignedBlock<S::E>,
         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<S: ValidatorStore + 'static, T: SlotClock + 'static> BlockService<S, T> {
                 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<S: ValidatorStore + 'static, T: SlotClock + 'static> BlockService<S, T> {
                 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<S: ValidatorStore + 'static, T: SlotClock + 'static> BlockService<S, T> {
                 ))
             })?;
 
-        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<S: ValidatorStore + 'static, T: SlotClock + 'static> BlockService<S, T> {
     }
 }
 
-pub enum UnsignedBlock<E: EthSpec> {
-    Full(FullBlockContents<E>),
-    Blinded(BlindedBeaconBlock<E>),
-}
-
-impl<E: EthSpec> UnsignedBlock<E> {
-    pub fn proposer_index(&self) -> u64 {
-        match self {
-            UnsignedBlock::Full(block) => block.block().proposer_index(),
-            UnsignedBlock::Blinded(block) => block.proposer_index(),
-        }
-    }
+/// 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,
 }
 
-#[derive(Debug)]
-pub enum SignedBlock<E: EthSpec> {
-    Full(PublishBlockRequest<E>),
-    Blinded(Arc<SignedBlindedBeaconBlock<E>>),
-}
-
-impl<E: EthSpec> SignedBlock<E> {
-    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<E: EthSpec> From<&SignedBlock<E>> for BlockMetadata {
+    fn from(value: &SignedBlock<E>) -> 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
@@ -5,5 +5,6 @@ edition = { workspace = true }
 authors = ["Sigma Prime <contact@sigmaprime.io>"]
 
 [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
@@ -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<ProposalData>;
 }
 
-#[derive(Clone, Debug, PartialEq)]
+#[derive(Debug)]
 pub enum UnsignedBlock<E: EthSpec> {
-    Full(BeaconBlock<E>),
+    Full(FullBlockContents<E>),
     Blinded(BlindedBeaconBlock<E>),
 }
 
-impl<E: EthSpec> From<BeaconBlock<E>> for UnsignedBlock<E> {
-    fn from(block: BeaconBlock<E>) -> Self {
-        UnsignedBlock::Full(block)
-    }
-}
-
-impl<E: EthSpec> From<BlindedBeaconBlock<E>> for UnsignedBlock<E> {
-    fn from(block: BlindedBeaconBlock<E>) -> Self {
-        UnsignedBlock::Blinded(block)
-    }
-}
-
 #[derive(Clone, Debug, PartialEq)]
 pub enum SignedBlock<E: EthSpec> {
-    Full(SignedBeaconBlock<E>),
-    Blinded(SignedBlindedBeaconBlock<E>),
-}
-
-impl<E: EthSpec> From<SignedBeaconBlock<E>> for SignedBlock<E> {
-    fn from(block: SignedBeaconBlock<E>) -> Self {
-        SignedBlock::Full(block)
-    }
-}
-
-impl<E: EthSpec> From<SignedBlindedBeaconBlock<E>> for SignedBlock<E> {
-    fn from(block: SignedBlindedBeaconBlock<E>) -> Self {
-        SignedBlock::Blinded(block)
-    }
+    Full(PublishBlockRequest<E>),
+    Blinded(Arc<SignedBlindedBeaconBlock<E>>),
 }
 
 /// A wrapper around `PublicKeyBytes` which encodes information about the status of a validator
