diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 1b05c7d39f..3e5f5e740f 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -5,6 +5,7 @@ use crate::{kzg_utils, BeaconChainError}; use bls::PublicKey; use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; use types::consts::eip4844::BLS_MODULUS; +use types::signed_beacon_block::BlobReconstructionError; use types::{BeaconStateError, BlobsSidecar, Hash256, KzgCommitment, Slot, Transactions}; #[derive(Debug)] @@ -91,6 +92,12 @@ pub enum BlobError { MissingBlobs, } +impl From for BlobError { + fn from(_: BlobReconstructionError) -> Self { + BlobError::MissingBlobs + } +} + impl From for BlobError { fn from(e: BeaconChainError) -> Self { BlobError::BeaconChainError(e) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index b9e65bc0a2..6f330b8eaf 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -87,6 +87,7 @@ use std::time::Duration; use store::{Error as DBError, HotStateSummary, KeyValueStore, StoreOp}; use task_executor::JoinHandle; use tree_hash::TreeHash; +use types::signed_beacon_block::BlobReconstructionError; use types::signed_block_and_blobs::BlockWrapper; use types::{ BeaconBlockRef, BeaconState, BeaconStateError, BlindedPayload, ChainSpec, CloneConfig, Epoch, @@ -479,6 +480,12 @@ impl From for BlockError { } } +impl From for BlockError { + fn from(e: BlobReconstructionError) -> Self { + BlockError::BlobValidation(BlobError::from(e)) + } +} + /// Stores information about verifying a payload against an execution engine. pub struct PayloadVerificationOutcome { pub payload_verification_status: PayloadVerificationStatus, @@ -905,7 +912,7 @@ impl GossipVerifiedBlock { // Validate the block's execution_payload (if any). validate_execution_payload_for_gossip(&parent_block, block.message(), chain)?; - if let Some(blobs_sidecar) = block.blobs() { + if let Some(blobs_sidecar) = block.blobs(Some(block_root))? { let kzg_commitments = block .message() .body() @@ -919,7 +926,7 @@ impl GossipVerifiedBlock { .map_err(|_| BlockError::BlobValidation(BlobError::TransactionsMissing))? .ok_or(BlockError::BlobValidation(BlobError::TransactionsMissing))?; validate_blob_for_gossip( - blobs_sidecar, + &blobs_sidecar, kzg_commitments, transactions, block.slot(), @@ -1134,12 +1141,8 @@ impl IntoExecutionPendingBlock for Arc &SignedBeaconBlock { @@ -1563,7 +1566,7 @@ impl ExecutionPendingBlock { if let Some(data_availability_boundary) = chain.data_availability_boundary() { if block_slot.epoch(T::EthSpec::slots_per_epoch()) >= data_availability_boundary { let sidecar = block - .blobs() + .blobs(Some(block_root))? .ok_or(BlockError::BlobValidation(BlobError::MissingBlobs))?; let kzg = chain.kzg.as_ref().ok_or(BlockError::BlobValidation( BlobError::TrustedSetupNotInitialized, @@ -1586,7 +1589,7 @@ impl ExecutionPendingBlock { block.slot(), block_root, kzg_commitments, - sidecar, + &sidecar, ) .map_err(|e| BlockError::BlobValidation(BlobError::KzgError(e)))? { diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 085f5036f4..f024e49e2a 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -43,9 +43,7 @@ pub async fn publish_block( network_tx, PubsubMessage::BeaconBlockAndBlobsSidecars(block_and_blobs.clone()), )?; - BlockWrapper::BlockAndBlob { - block_sidecar_pair: block_and_blobs, - } + BlockWrapper::BlockAndBlob(block_and_blobs) } else { //FIXME(sean): This should probably return a specific no-blob-cached error code, beacon API coordination required return Err(warp_utils::reject::broadcast_without_import(format!( @@ -54,7 +52,7 @@ pub async fn publish_block( } } else { crate::publish_pubsub_message(network_tx, PubsubMessage::BeaconBlock(block.clone()))?; - BlockWrapper::Block { block } + BlockWrapper::Block(block) }; // Determine the delay after the start of the slot, register it with metrics. diff --git a/beacon_node/network/src/beacon_processor/mod.rs b/beacon_node/network/src/beacon_processor/mod.rs index 887ecc4977..9de006c84f 100644 --- a/beacon_node/network/src/beacon_processor/mod.rs +++ b/beacon_node/network/src/beacon_processor/mod.rs @@ -1699,7 +1699,7 @@ impl BeaconProcessor { message_id, peer_id, peer_client, - BlockWrapper::Block { block }, + BlockWrapper::Block(block), work_reprocessing_tx, duplicate_cache, seen_timestamp, @@ -1721,7 +1721,7 @@ impl BeaconProcessor { message_id, peer_id, peer_client, - BlockWrapper::BlockAndBlob { block_sidecar_pair }, + BlockWrapper::BlockAndBlob(block_sidecar_pair), work_reprocessing_tx, duplicate_cache, seen_timestamp, diff --git a/beacon_node/network/src/beacon_processor/worker/sync_methods.rs b/beacon_node/network/src/beacon_processor/worker/sync_methods.rs index dafc00bddb..b9ad82cf8b 100644 --- a/beacon_node/network/src/beacon_processor/worker/sync_methods.rs +++ b/beacon_node/network/src/beacon_processor/worker/sync_methods.rs @@ -194,11 +194,9 @@ impl Worker { let unwrapped = downloaded_blocks .into_iter() .map(|block| match block { - BlockWrapper::Block { block } => block, + BlockWrapper::Block(block) => block, //FIXME(sean) handle blobs in backfill - BlockWrapper::BlockAndBlob { - block_sidecar_pair: _, - } => todo!(), + BlockWrapper::BlockAndBlob(_) => todo!(), }) .collect(); diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index 95acadffb6..f82417db3a 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/beacon_node/network/src/sync/block_sidecar_coupling.rs @@ -51,16 +51,14 @@ impl BlockBlobRequestInfo { { let blobs_sidecar = accumulated_sidecars.pop_front().ok_or("missing sidecar")?; - Ok(BlockWrapper::BlockAndBlob { - block_sidecar_pair: SignedBeaconBlockAndBlobsSidecar { + Ok(BlockWrapper::BlockAndBlob( + SignedBeaconBlockAndBlobsSidecar { beacon_block, blobs_sidecar, }, - }) + )) } else { - Ok(BlockWrapper::Block { - block: beacon_block, - }) + Ok(BlockWrapper::Block(beacon_block)) } }) .collect::, _>>(); diff --git a/beacon_node/network/src/sync/manager.rs b/beacon_node/network/src/sync/manager.rs index 305f9f7069..109ed81941 100644 --- a/beacon_node/network/src/sync/manager.rs +++ b/beacon_node/network/src/sync/manager.rs @@ -734,14 +734,14 @@ impl SyncManager { RequestId::SingleBlock { id } => self.block_lookups.single_block_lookup_response( id, peer_id, - beacon_block.map(|block| BlockWrapper::Block { block }), + beacon_block.map(|block| BlockWrapper::Block(block)), seen_timestamp, &mut self.network, ), RequestId::ParentLookup { id } => self.block_lookups.parent_lookup_response( id, peer_id, - beacon_block.map(|block| BlockWrapper::Block { block }), + beacon_block.map(|block| BlockWrapper::Block(block)), seen_timestamp, &mut self.network, ), @@ -756,7 +756,7 @@ impl SyncManager { batch_id, &peer_id, id, - beacon_block.map(|block| BlockWrapper::Block { block }), + beacon_block.map(|block| BlockWrapper::Block(block)), ) { Ok(ProcessResult::SyncCompleted) => self.update_sync_state(), Ok(ProcessResult::Successful) => {} @@ -780,7 +780,7 @@ impl SyncManager { chain_id, batch_id, id, - beacon_block.map(|block| BlockWrapper::Block { block }), + beacon_block.map(|block| BlockWrapper::Block(block)), ); self.update_sync_state(); } @@ -930,9 +930,11 @@ impl SyncManager { RequestId::SingleBlock { id } => self.block_lookups.single_block_lookup_response( id, peer_id, - block_sidecar_pair.map(|block_sidecar_pair| BlockWrapper::BlockAndBlob { - // TODO: why is this in an arc - block_sidecar_pair: (*block_sidecar_pair).clone(), + block_sidecar_pair.map(|block_sidecar_pair| { + BlockWrapper::BlockAndBlob( + // TODO: why is this in an arc + (*block_sidecar_pair).clone(), + ) }), seen_timestamp, &mut self.network, @@ -940,9 +942,11 @@ impl SyncManager { RequestId::ParentLookup { id } => self.block_lookups.parent_lookup_response( id, peer_id, - block_sidecar_pair.map(|block_sidecar_pair| BlockWrapper::BlockAndBlob { - // TODO: why is this in an arc - block_sidecar_pair: (*block_sidecar_pair).clone(), + block_sidecar_pair.map(|block_sidecar_pair| { + BlockWrapper::BlockAndBlob( + // TODO: why is this in an arc + (*block_sidecar_pair).clone(), + ) }), seen_timestamp, &mut self.network, diff --git a/beacon_node/network/src/sync/range_sync/batch.rs b/beacon_node/network/src/sync/range_sync/batch.rs index b0d266e074..f7e5945988 100644 --- a/beacon_node/network/src/sync/range_sync/batch.rs +++ b/beacon_node/network/src/sync/range_sync/batch.rs @@ -25,11 +25,11 @@ impl BatchTy { match self { BatchTy::Blocks(blocks) => blocks .into_iter() - .map(|block| BlockWrapper::Block { block }) + .map(|block| BlockWrapper::Block(block)) .collect(), BatchTy::BlocksAndBlobs(block_sidecar_pair) => block_sidecar_pair .into_iter() - .map(|block_sidecar_pair| BlockWrapper::BlockAndBlob { block_sidecar_pair }) + .map(|block_sidecar_pair| BlockWrapper::BlockAndBlob(block_sidecar_pair)) .collect(), } } @@ -413,7 +413,7 @@ impl BatchInfo { match self.batch_type { ExpectedBatchTy::OnlyBlockBlobs => { let blocks = blocks.into_iter().map(|block| { - let BlockWrapper::BlockAndBlob { block_sidecar_pair: block_and_blob } = block else { + let BlockWrapper::BlockAndBlob(block_and_blob) = block else { panic!("Batches should never have a mixed type. This is a bug. Contact D") }; block_and_blob @@ -422,7 +422,7 @@ impl BatchInfo { } ExpectedBatchTy::OnlyBlock => { let blocks = blocks.into_iter().map(|block| { - let BlockWrapper::Block { block } = block else { + let BlockWrapper::Block(block) = block else { panic!("Batches should never have a mixed type. This is a bug. Contact D") }; block diff --git a/consensus/state_processing/src/consensus_context.rs b/consensus/state_processing/src/consensus_context.rs index f5585426ce..3a5aebc474 100644 --- a/consensus/state_processing/src/consensus_context.rs +++ b/consensus/state_processing/src/consensus_context.rs @@ -21,8 +21,6 @@ pub struct ConsensusContext { /// Cache of indexed attestations constructed during block processing. indexed_attestations: HashMap<(AttestationData, BitList), IndexedAttestation>, - /// Should only be populated if the sidecar has not been validated. - blobs_sidecar: Option>>, /// Whether `validate_blobs_sidecar` has successfully passed. blobs_sidecar_validated: bool, /// Whether `verify_kzg_commitments_against_transactions` has successfully passed. @@ -49,7 +47,6 @@ impl ConsensusContext { proposer_index: None, current_block_root: None, indexed_attestations: HashMap::new(), - blobs_sidecar: None, blobs_sidecar_validated: false, blobs_verified_vs_txs: false, } diff --git a/consensus/types/src/blobs_sidecar.rs b/consensus/types/src/blobs_sidecar.rs index f1e2a4bb16..d522227a6f 100644 --- a/consensus/types/src/blobs_sidecar.rs +++ b/consensus/types/src/blobs_sidecar.rs @@ -28,6 +28,15 @@ impl BlobsSidecar { Self::default() } + pub fn empty_from_parts(beacon_block_root: Hash256, beacon_block_slot: Slot) -> Self { + Self { + beacon_block_root, + beacon_block_slot, + blobs: VariableList::empty(), + kzg_aggregated_proof: KzgProof::empty(), + } + } + #[allow(clippy::integer_arithmetic)] pub fn max_size() -> usize { // Fixed part diff --git a/consensus/types/src/signed_beacon_block.rs b/consensus/types/src/signed_beacon_block.rs index 14f9358f61..89ccb95a16 100644 --- a/consensus/types/src/signed_beacon_block.rs +++ b/consensus/types/src/signed_beacon_block.rs @@ -36,6 +36,10 @@ impl From for Hash256 { } } +pub enum BlobReconstructionError { + BlobsMissing, +} + /// A `BeaconBlock` and a signature from its proposer. #[superstruct( variants(Base, Altair, Merge, Capella, Eip4844), @@ -235,6 +239,29 @@ impl> SignedBeaconBlock pub fn canonical_root(&self) -> Hash256 { self.message().tree_hash_root() } + + /// Reconstructs an empty `BlobsSidecar`, using the given block root if provided, else calculates it. + /// If this block has kzg commitments, an error will be returned. If this block is from prior to the + /// Eip4844 fork, `None` will be returned. + pub fn reconstruct_empty_blobs( + &self, + block_root_opt: Option, + ) -> Result>, BlobReconstructionError> { + self.message() + .body() + .blob_kzg_commitments() + .map(|kzg_commitments| { + if kzg_commitments.len() > 0 { + Err(BlobReconstructionError::BlobsMissing) + } else { + Ok(Some(BlobsSidecar::empty_from_parts( + block_root_opt.unwrap_or(self.canonical_root()), + self.slot(), + ))) + } + }) + .unwrap_or(Ok(None)) + } } // We can convert pre-Bellatrix blocks without payloads into blocks with payloads. diff --git a/consensus/types/src/signed_block_and_blobs.rs b/consensus/types/src/signed_block_and_blobs.rs index 09ff89e7b3..4fcd09de4d 100644 --- a/consensus/types/src/signed_block_and_blobs.rs +++ b/consensus/types/src/signed_block_and_blobs.rs @@ -1,3 +1,4 @@ +use crate::signed_beacon_block::BlobReconstructionError; use crate::{BlobsSidecar, EthSpec, Hash256, SignedBeaconBlock, SignedBeaconBlockEip4844, Slot}; use serde_derive::{Deserialize, Serialize}; use ssz::{Decode, DecodeError}; @@ -35,60 +36,52 @@ impl SignedBeaconBlockAndBlobsSidecar { /// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobsSidecar`]. #[derive(Clone, Debug)] pub enum BlockWrapper { - Block { - block: Arc>, - }, - BlockAndBlob { - block_sidecar_pair: SignedBeaconBlockAndBlobsSidecar, - }, + Block(Arc>), + BlockAndBlob(SignedBeaconBlockAndBlobsSidecar), } impl BlockWrapper { pub fn slot(&self) -> Slot { match self { - BlockWrapper::Block { block } => block.slot(), - BlockWrapper::BlockAndBlob { block_sidecar_pair } => { + BlockWrapper::Block(block) => block.slot(), + BlockWrapper::BlockAndBlob(block_sidecar_pair) => { block_sidecar_pair.beacon_block.slot() } } } pub fn block(&self) -> &SignedBeaconBlock { match self { - BlockWrapper::Block { block } => &block, - BlockWrapper::BlockAndBlob { block_sidecar_pair } => &block_sidecar_pair.beacon_block, + BlockWrapper::Block(block) => &block, + BlockWrapper::BlockAndBlob(block_sidecar_pair) => &block_sidecar_pair.beacon_block, } } pub fn block_cloned(&self) -> Arc> { match self { - BlockWrapper::Block { block } => block.clone(), - BlockWrapper::BlockAndBlob { block_sidecar_pair } => { + BlockWrapper::Block(block) => block.clone(), + BlockWrapper::BlockAndBlob(block_sidecar_pair) => { block_sidecar_pair.beacon_block.clone() } } } - pub fn blobs(&self) -> Option<&BlobsSidecar> { + pub fn blobs( + &self, + block_root: Option, + ) -> Result>>, BlobReconstructionError> { match self { - BlockWrapper::Block { .. } => None, - BlockWrapper::BlockAndBlob { block_sidecar_pair } => { - Some(&block_sidecar_pair.blobs_sidecar) - } - } - } - - pub fn blobs_cloned(&self) -> Option>> { - match self { - BlockWrapper::Block { block: _ } => None, - BlockWrapper::BlockAndBlob { block_sidecar_pair } => { - Some(block_sidecar_pair.blobs_sidecar.clone()) + BlockWrapper::Block(block) => block + .reconstruct_empty_blobs(block_root) + .map(|blob_opt| blob_opt.map(Arc::new)), + BlockWrapper::BlockAndBlob(block_sidecar_pair) => { + Ok(Some(block_sidecar_pair.blobs_sidecar.clone())) } } } pub fn message(&self) -> crate::BeaconBlockRef { match self { - BlockWrapper::Block { block } => block.message(), - BlockWrapper::BlockAndBlob { block_sidecar_pair } => { + BlockWrapper::Block(block) => block.message(), + BlockWrapper::BlockAndBlob(block_sidecar_pair) => { block_sidecar_pair.beacon_block.message() } } @@ -100,8 +93,8 @@ impl BlockWrapper { pub fn deconstruct(self) -> (Arc>, Option>>) { match self { - BlockWrapper::Block { block } => (block, None), - BlockWrapper::BlockAndBlob { block_sidecar_pair } => { + BlockWrapper::Block(block) => (block, None), + BlockWrapper::BlockAndBlob(block_sidecar_pair) => { let SignedBeaconBlockAndBlobsSidecar { beacon_block, blobs_sidecar, @@ -112,29 +105,25 @@ impl BlockWrapper { } } -// TODO: probably needes to be changed. This is needed because SignedBeaconBlockAndBlobsSidecar +// FIXME(sean): probably needs to be changed. This is needed because SignedBeaconBlockAndBlobsSidecar // does not implement Hash impl std::hash::Hash for BlockWrapper { fn hash(&self, state: &mut H) { match self { - BlockWrapper::Block { block } => block.hash(state), - BlockWrapper::BlockAndBlob { - block_sidecar_pair: block_and_blob, - } => block_and_blob.beacon_block.hash(state), + BlockWrapper::Block(block) => block.hash(state), + BlockWrapper::BlockAndBlob(block_and_blob) => block_and_blob.beacon_block.hash(state), } } } impl From> for BlockWrapper { fn from(block: SignedBeaconBlock) -> Self { - BlockWrapper::Block { - block: Arc::new(block), - } + BlockWrapper::Block(Arc::new(block)) } } impl From>> for BlockWrapper { fn from(block: Arc>) -> Self { - BlockWrapper::Block { block } + BlockWrapper::Block(block) } } diff --git a/crypto/kzg/src/kzg_proof.rs b/crypto/kzg/src/kzg_proof.rs index cb6e14df4a..be85088f76 100644 --- a/crypto/kzg/src/kzg_proof.rs +++ b/crypto/kzg/src/kzg_proof.rs @@ -12,6 +12,14 @@ const KZG_PROOF_BYTES_LEN: usize = 48; #[ssz(struct_behaviour = "transparent")] pub struct KzgProof(pub [u8; KZG_PROOF_BYTES_LEN]); +impl KzgProof { + pub fn empty() -> Self { + let mut bytes = [0; KZG_PROOF_BYTES_LEN]; + bytes[0] = 192; + Self(bytes) + } +} + impl fmt::Display for KzgProof { fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!(f, "{}", eth2_serde_utils::hex::encode(self.0))