diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 1e901987a..57c685325 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -73,6 +73,7 @@ use fork_choice::{ use futures::channel::mpsc::Sender; use itertools::process_results; use itertools::Itertools; +use kzg::Kzg; use operation_pool::{AttestationRef, OperationPool, PersistedOperationPool, ReceivedPreCapella}; use parking_lot::{Mutex, RwLock}; use proto_array::{CountUnrealizedFull, DoNotReOrg, ProposerHeadError}; @@ -107,6 +108,7 @@ use store::{ use task_executor::{ShutdownReason, TaskExecutor}; use tree_hash::TreeHash; use types::beacon_state::CloneConfig; +use types::blobs_sidecar::KzgCommitments; use types::consts::eip4844::MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS; use types::consts::merge::INTERVALS_PER_SLOT; use types::*; @@ -4759,30 +4761,62 @@ impl BeaconChain { .kzg .as_ref() .ok_or(BlockProductionError::TrustedSetupNotInitialized)?; - let kzg_aggregated_proof = - kzg_utils::compute_aggregate_kzg_proof::(kzg, &blobs) - .map_err(BlockProductionError::KzgError)?; let beacon_block_root = block.canonical_root(); - let expected_kzg_commitments = block.body().blob_kzg_commitments().map_err(|_| { - BlockProductionError::InvalidBlockVariant( - "EIP4844 block does not contain kzg commitments".to_string(), - ) - })?; - let blobs_sidecar = BlobsSidecar { - beacon_block_slot: slot, - beacon_block_root, - blobs, - kzg_aggregated_proof, - }; - kzg_utils::validate_blobs_sidecar( + let expected_kzg_commitments: &KzgCommitments = + block.body().blob_kzg_commitments().map_err(|_| { + BlockProductionError::InvalidBlockVariant( + "EIP4844 block does not contain kzg commitments".to_string(), + ) + })?; + + if expected_kzg_commitments.len() != blobs.len() { + return Err(BlockProductionError::MissingKzgCommitment(format!( + "Missing KZG commitment for slot {}. Expected {}, got: {}", + slot, + blobs.len(), + expected_kzg_commitments.len() + ))); + } + + let kzg_proofs = + Self::compute_blob_kzg_proofs(kzg, &blobs, expected_kzg_commitments, slot)?; + + kzg_utils::validate_blobs::( kzg, - slot, - beacon_block_root, expected_kzg_commitments, - &blobs_sidecar, + &blobs, + &kzg_proofs, ) .map_err(BlockProductionError::KzgError)?; - self.blob_cache.put(beacon_block_root, blobs_sidecar); + + let blob_sidecars = VariableList::from( + blobs + .into_iter() + .enumerate() + .map(|(blob_index, blob)| { + let kzg_commitment = expected_kzg_commitments + .get(blob_index) + .expect("KZG commitment should exist for blob"); + + let kzg_proof = kzg_proofs + .get(blob_index) + .expect("KZG proof should exist for blob"); + + Ok(BlobSidecar { + block_root: beacon_block_root, + index: blob_index as u64, + slot, + block_parent_root: block.parent_root(), + proposer_index, + blob, + kzg_commitment: *kzg_commitment, + kzg_proof: *kzg_proof, + }) + }) + .collect::>, BlockProductionError>>()?, + ); + + self.blob_cache.put(beacon_block_root, blob_sidecars); } metrics::inc_counter(&metrics::BLOCK_PRODUCTION_SUCCESSES); @@ -4798,6 +4832,29 @@ impl BeaconChain { Ok((block, state)) } + fn compute_blob_kzg_proofs( + kzg: &Arc, + blobs: &Blobs<::EthSpec>, + expected_kzg_commitments: &KzgCommitments<::EthSpec>, + slot: Slot, + ) -> Result, BlockProductionError> { + blobs + .iter() + .enumerate() + .map(|(blob_index, blob)| { + let kzg_commitment = expected_kzg_commitments.get(blob_index).ok_or( + BlockProductionError::MissingKzgCommitment(format!( + "Missing KZG commitment for slot {} blob index {}", + slot, blob_index + )), + )?; + + kzg_utils::compute_blob_kzg_proof::(kzg, blob, kzg_commitment.clone()) + .map_err(BlockProductionError::KzgError) + }) + .collect::, BlockProductionError>>() + } + /// This method must be called whenever an execution engine indicates that a payload is /// invalid. /// diff --git a/beacon_node/beacon_chain/src/blob_cache.rs b/beacon_node/beacon_chain/src/blob_cache.rs index d03e62ab6..64f113c28 100644 --- a/beacon_node/beacon_chain/src/blob_cache.rs +++ b/beacon_node/beacon_chain/src/blob_cache.rs @@ -1,12 +1,12 @@ use lru::LruCache; use parking_lot::Mutex; -use types::{BlobsSidecar, EthSpec, Hash256}; +use types::{BlobSidecarList, EthSpec, Hash256}; pub const DEFAULT_BLOB_CACHE_SIZE: usize = 10; /// A cache blobs by beacon block root. pub struct BlobCache { - blobs: Mutex>>, + blobs: Mutex>>, } #[derive(Hash, PartialEq, Eq)] @@ -21,11 +21,15 @@ impl Default for BlobCache { } impl BlobCache { - pub fn put(&self, beacon_block: Hash256, blobs: BlobsSidecar) -> Option> { + pub fn put( + &self, + beacon_block: Hash256, + blobs: BlobSidecarList, + ) -> Option> { self.blobs.lock().put(BlobCacheId(beacon_block), blobs) } - pub fn pop(&self, root: &Hash256) -> Option> { + pub fn pop(&self, root: &Hash256) -> Option> { self.blobs.lock().pop(&BlobCacheId(*root)) } } diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 1d7f9cc3c..7a4493c97 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -135,18 +135,19 @@ fn verify_data_availability( .as_ref() .ok_or(BlobError::TrustedSetupNotInitialized)?; - if !kzg_utils::validate_blobs_sidecar( - kzg, - block_slot, - block_root, - kzg_commitments, - blob_sidecar, - ) - .map_err(BlobError::KzgError)? - { - return Err(BlobError::InvalidKzgProof); - } - Ok(()) + todo!("use `kzg_utils::validate_blobs` once the function is updated") + // if !kzg_utils::validate_blobs_sidecar( + // kzg, + // block_slot, + // block_root, + // kzg_commitments, + // blob_sidecar, + // ) + // .map_err(BlobError::KzgError)? + // { + // return Err(BlobError::InvalidKzgProof); + // } + // Ok(()) } /// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobsSidecar`]. This makes no diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 3b4c46249..911fe1d5b 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -266,12 +266,14 @@ pub enum BlockProductionError { blob_block_hash: ExecutionBlockHash, payload_block_hash: ExecutionBlockHash, }, + NoBlobsCached, FailedToReadFinalizedBlock(store::Error), MissingFinalizedBlock(Hash256), BlockTooLarge(usize), ShuttingDown, MissingSyncAggregate, MissingExecutionPayload, + MissingKzgCommitment(String), TokioJoin(tokio::task::JoinError), BeaconChain(BeaconChainError), InvalidPayloadFork, diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index 3536420a1..19788a1b4 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -42,10 +42,11 @@ pub fn validate_blobs( /// Compute the kzg proof given an ssz blob and its kzg commitment. pub fn compute_blob_kzg_proof( kzg: &Kzg, - blob: Blob, + blob: &Blob, kzg_commitment: KzgCommitment, ) -> Result { - kzg.compute_blob_kzg_proof(ssz_blob_to_crypto_blob::(blob), kzg_commitment) + // Avoid this blob clone + kzg.compute_blob_kzg_proof(ssz_blob_to_crypto_blob::(blob.clone()), kzg_commitment) } /// Compute the kzg commitment for a given blob. diff --git a/beacon_node/http_api/src/build_block_contents.rs b/beacon_node/http_api/src/build_block_contents.rs new file mode 100644 index 000000000..1908c03ea --- /dev/null +++ b/beacon_node/http_api/src/build_block_contents.rs @@ -0,0 +1,33 @@ +use beacon_chain::{BeaconChain, BeaconChainTypes, BlockProductionError}; +use eth2::types::BlockContents; +use std::sync::Arc; +use types::{AbstractExecPayload, BeaconBlock, BeaconBlockAndBlobSidecars, ForkName}; + +type Error = warp::reject::Rejection; + +pub fn build_block_contents>( + fork_name: ForkName, + chain: Arc>, + block: BeaconBlock, +) -> Result, Error> { + match fork_name { + ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { + Ok(BlockContents::Block(block)) + } + ForkName::Eip4844 => { + let block_root = &block.canonical_root(); + if let Some(blob_sidecars) = chain.blob_cache.pop(block_root) { + let block_and_blobs = BeaconBlockAndBlobSidecars { + block, + blob_sidecars, + }; + + Ok(BlockContents::BlockAndBlobSidecars(block_and_blobs)) + } else { + return Err(warp_utils::reject::block_production_error( + BlockProductionError::NoBlobsCached, + )); + } + } + } +} diff --git a/beacon_node/http_api/src/lib.rs b/beacon_node/http_api/src/lib.rs index a089b6e97..ea3985906 100644 --- a/beacon_node/http_api/src/lib.rs +++ b/beacon_node/http_api/src/lib.rs @@ -11,6 +11,7 @@ mod attester_duties; mod block_id; mod block_packing_efficiency; mod block_rewards; +mod build_block_contents; mod database; mod metrics; mod proposer_duties; @@ -2421,7 +2422,10 @@ pub fn serve( .fork_name(&chain.spec) .map_err(inconsistent_fork_rejection)?; - fork_versioned_response(endpoint_version, fork_name, block) + let block_contents = + build_block_contents::build_block_contents(fork_name, chain, block); + + fork_versioned_response(endpoint_version, fork_name, block_contents?) .map(|response| warp::reply::json(&response)) }, ); diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 324943673..1e785cd1e 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -12,7 +12,7 @@ use tokio::sync::mpsc::UnboundedSender; use tree_hash::TreeHash; use types::{ AbstractExecPayload, BlindedPayload, EthSpec, ExecPayload, ExecutionBlockHash, FullPayload, - Hash256, SignedBeaconBlock, SignedBeaconBlockAndBlobsSidecar, + Hash256, SignedBeaconBlock, }; use warp::Rejection; @@ -40,10 +40,11 @@ pub async fn publish_block( let wrapped_block: BlockWrapper = if matches!(block.as_ref(), &SignedBeaconBlock::Eip4844(_)) { if let Some(sidecar) = chain.blob_cache.pop(&block_root) { - let block_and_blobs = SignedBeaconBlockAndBlobsSidecar { - beacon_block: block, - blobs_sidecar: Arc::new(sidecar), - }; + // TODO: Needs to be adjusted + // let block_and_blobs = SignedBeaconBlockAndBlobsSidecar { + // beacon_block: block, + // blobs_sidecar: Arc::new(sidecar), + // }; unimplemented!("Needs to be adjusted") } else { //FIXME(sean): This should probably return a specific no-blob-cached error code, beacon API coordination required diff --git a/beacon_node/lighthouse_network/src/types/topics.rs b/beacon_node/lighthouse_network/src/types/topics.rs index 88764fb95..d9deaaf05 100644 --- a/beacon_node/lighthouse_network/src/types/topics.rs +++ b/beacon_node/lighthouse_network/src/types/topics.rs @@ -314,7 +314,6 @@ mod tests { VoluntaryExit, ProposerSlashing, AttesterSlashing, - BeaconBlocksAndBlobsSidecar, ] .iter() { diff --git a/beacon_node/store/src/hot_cold_store.rs b/beacon_node/store/src/hot_cold_store.rs index 9762a0f59..60e2f7759 100644 --- a/beacon_node/store/src/hot_cold_store.rs +++ b/beacon_node/store/src/hot_cold_store.rs @@ -259,7 +259,7 @@ impl HotColdDB, LevelDB> { db.blobs_db = Some(LevelDB::open(path.as_path())?); } } - let blob_info = blob_info.unwrap_or(db.get_blob_info()); + let blob_info = blob_info.unwrap_or_else(|| db.get_blob_info()); db.compare_and_set_blob_info_with_write(blob_info, new_blob_info)?; info!( db.log, @@ -1899,7 +1899,7 @@ impl, Cold: ItemStore> HotColdDB let blob_info = self.get_blob_info(); let oldest_blob_slot = blob_info .oldest_blob_slot - .unwrap_or(eip4844_fork.start_slot(E::slots_per_epoch())); + .unwrap_or_else(|| eip4844_fork.start_slot(E::slots_per_epoch())); // The last entirely pruned epoch, blobs sidecar pruning may have stopped early in the // middle of an epoch otherwise the oldest blob slot is a start slot. diff --git a/common/eth2/src/lib.rs b/common/eth2/src/lib.rs index 4535546a9..5a55f21e3 100644 --- a/common/eth2/src/lib.rs +++ b/common/eth2/src/lib.rs @@ -1388,7 +1388,7 @@ impl BeaconNodeHttpClient { slot: Slot, randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, - ) -> Result>, Error> { + ) -> Result>, Error> { self.get_validator_blocks_modular(slot, randao_reveal, graffiti, SkipRandaoVerification::No) .await } @@ -1400,7 +1400,7 @@ impl BeaconNodeHttpClient { randao_reveal: &SignatureBytes, graffiti: Option<&Graffiti>, skip_randao_verification: SkipRandaoVerification, - ) -> Result>, Error> { + ) -> Result>, Error> { let mut path = self.eth_path(V2)?; path.path_segments_mut() diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index d6db3e15f..10c5b86f6 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1259,3 +1259,63 @@ mod tests { ) } } + +/// A wrapper over a [`BeaconBlock`] or a [`BeaconBlockAndBlobSidecars`]. +#[derive(Clone, Debug, Serialize, Deserialize)] +#[serde(untagged)] +#[serde(bound = "T: EthSpec")] +pub enum BlockContents> { + BlockAndBlobSidecars(BeaconBlockAndBlobSidecars), + Block(BeaconBlock), +} + +impl> BlockContents { + pub fn block(&self) -> &BeaconBlock { + match self { + BlockContents::BlockAndBlobSidecars(block_and_sidecars) => &block_and_sidecars.block, + BlockContents::Block(block) => block, + } + } + + pub fn deconstruct(self) -> (BeaconBlock, Option>) { + match self { + BlockContents::BlockAndBlobSidecars(block_and_sidecars) => ( + block_and_sidecars.block, + Some(block_and_sidecars.blob_sidecars), + ), + BlockContents::Block(block) => (block, None), + } + } +} + +impl> ForkVersionDeserialize + for BlockContents +{ + fn deserialize_by_fork<'de, D: serde::Deserializer<'de>>( + value: serde_json::value::Value, + fork_name: ForkName, + ) -> Result { + match fork_name { + ForkName::Base | ForkName::Altair | ForkName::Merge | ForkName::Capella => { + Ok(BlockContents::Block(BeaconBlock::deserialize_by_fork::< + 'de, + D, + >(value, fork_name)?)) + } + ForkName::Eip4844 => Ok(BlockContents::BlockAndBlobSidecars( + BeaconBlockAndBlobSidecars::deserialize_by_fork::<'de, D>(value, fork_name)?, + )), + } + } +} + +impl> Into> + for BlockContents +{ + fn into(self) -> BeaconBlock { + match self { + Self::BlockAndBlobSidecars(block_and_sidecars) => block_and_sidecars.block, + Self::Block(block) => block, + } + } +} diff --git a/consensus/types/src/beacon_block_and_blob_sidecars.rs b/consensus/types/src/beacon_block_and_blob_sidecars.rs new file mode 100644 index 000000000..78e704196 --- /dev/null +++ b/consensus/types/src/beacon_block_and_blob_sidecars.rs @@ -0,0 +1,37 @@ +use crate::{ + AbstractExecPayload, BeaconBlock, BlobSidecarList, EthSpec, ForkName, ForkVersionDeserialize, +}; +use derivative::Derivative; +use serde_derive::{Deserialize, Serialize}; +use ssz_derive::Encode; +use tree_hash_derive::TreeHash; + +#[derive(Debug, Clone, Serialize, Deserialize, Encode, TreeHash, Derivative)] +#[derivative(PartialEq, Hash(bound = "T: EthSpec"))] +#[serde(bound = "T: EthSpec, Payload: AbstractExecPayload")] +pub struct BeaconBlockAndBlobSidecars> { + pub block: BeaconBlock, + pub blob_sidecars: BlobSidecarList, +} + +impl> ForkVersionDeserialize + for BeaconBlockAndBlobSidecars +{ + fn deserialize_by_fork<'de, D: serde::Deserializer<'de>>( + value: serde_json::value::Value, + fork_name: ForkName, + ) -> Result { + #[derive(Deserialize)] + #[serde(bound = "T: EthSpec")] + struct Helper { + block: serde_json::Value, + blob_sidecars: BlobSidecarList, + } + let helper: Helper = serde_json::from_value(value).map_err(serde::de::Error::custom)?; + + Ok(Self { + block: BeaconBlock::deserialize_by_fork::<'de, D>(helper.block, fork_name)?, + blob_sidecars: helper.blob_sidecars, + }) + } +} diff --git a/consensus/types/src/blob_sidecar.rs b/consensus/types/src/blob_sidecar.rs index 12a6633de..169c570d2 100644 --- a/consensus/types/src/blob_sidecar.rs +++ b/consensus/types/src/blob_sidecar.rs @@ -5,6 +5,7 @@ use kzg::{KzgCommitment, KzgProof}; use serde_derive::{Deserialize, Serialize}; use ssz::Encode; use ssz_derive::{Decode, Encode}; +use ssz_types::VariableList; use test_random_derive::TestRandom; use tree_hash_derive::TreeHash; @@ -34,15 +35,20 @@ pub struct BlobIdentifier { pub struct BlobSidecar { pub block_root: Hash256, // TODO: fix the type, should fit in u8 as well + #[serde(with = "eth2_serde_utils::quoted_u64")] pub index: u64, pub slot: Slot, pub block_parent_root: Hash256, + #[serde(with = "eth2_serde_utils::quoted_u64")] pub proposer_index: u64, + #[serde(with = "ssz_types::serde_utils::hex_fixed_vec")] pub blob: Blob, pub kzg_commitment: KzgCommitment, pub kzg_proof: KzgProof, } +pub type BlobSidecarList = VariableList, ::MaxBlobsPerBlock>; + impl SignedRoot for BlobSidecar {} impl BlobSidecar { diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index ce4837853..fe47454aa 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -99,6 +99,7 @@ pub mod slot_data; #[cfg(feature = "sqlite")] pub mod sqlite; +pub mod beacon_block_and_blob_sidecars; pub mod blob_sidecar; pub mod blobs_sidecar; pub mod signed_blob; @@ -116,6 +117,7 @@ pub use crate::beacon_block::{ BeaconBlock, BeaconBlockAltair, BeaconBlockBase, BeaconBlockCapella, BeaconBlockEip4844, BeaconBlockMerge, BeaconBlockRef, BeaconBlockRefMut, BlindedBeaconBlock, EmptyBlock, }; +pub use crate::beacon_block_and_blob_sidecars::BeaconBlockAndBlobSidecars; pub use crate::beacon_block_body::{ BeaconBlockBody, BeaconBlockBodyAltair, BeaconBlockBodyBase, BeaconBlockBodyCapella, BeaconBlockBodyEip4844, BeaconBlockBodyMerge, BeaconBlockBodyRef, BeaconBlockBodyRefMut, @@ -123,7 +125,7 @@ pub use crate::beacon_block_body::{ pub use crate::beacon_block_header::BeaconBlockHeader; pub use crate::beacon_committee::{BeaconCommittee, OwnedBeaconCommittee}; pub use crate::beacon_state::{BeaconTreeHashCache, Error as BeaconStateError, *}; -pub use crate::blob_sidecar::BlobSidecar; +pub use crate::blob_sidecar::{BlobSidecar, BlobSidecarList}; pub use crate::blobs_sidecar::{Blobs, BlobsSidecar}; pub use crate::bls_to_execution_change::BlsToExecutionChange; pub use crate::chain_spec::{ChainSpec, Config, Domain}; diff --git a/crypto/kzg/src/kzg_commitment.rs b/crypto/kzg/src/kzg_commitment.rs index 3a18fe8b5..0f2725b75 100644 --- a/crypto/kzg/src/kzg_commitment.rs +++ b/crypto/kzg/src/kzg_commitment.rs @@ -8,7 +8,7 @@ use std::fmt::{Debug, Display, Formatter}; use std::str::FromStr; use tree_hash::{PackedEncoding, TreeHash}; -#[derive(Derivative, Clone, Encode, Decode)] +#[derive(Derivative, Clone, Copy, Encode, Decode)] #[derivative(PartialEq, Eq, Hash)] #[ssz(struct_behaviour = "transparent")] pub struct KzgCommitment(pub [u8; BYTES_PER_COMMITMENT]); diff --git a/validator_client/src/block_service.rs b/validator_client/src/block_service.rs index 3b3749237..22064cb10 100644 --- a/validator_client/src/block_service.rs +++ b/validator_client/src/block_service.rs @@ -15,8 +15,8 @@ use std::time::Duration; use tokio::sync::mpsc; use tokio::time::sleep; use types::{ - AbstractExecPayload, BlindedPayload, BlockType, EthSpec, FullPayload, Graffiti, PublicKeyBytes, - Slot, + AbstractExecPayload, BeaconBlock, BlindedPayload, BlockType, EthSpec, FullPayload, Graffiti, + PublicKeyBytes, Slot, }; #[derive(Debug)] @@ -347,7 +347,7 @@ impl BlockService { RequireSynced::No, OfflineOnFailure::Yes, |beacon_node| async move { - let block = match Payload::block_type() { + let block: BeaconBlock = match Payload::block_type() { BlockType::Full => { let _get_timer = metrics::start_timer_vec( &metrics::BLOCK_SERVICE_TIMES, @@ -367,6 +367,7 @@ impl BlockService { )) })? .data + .into() } BlockType::Blinded => { let _get_timer = metrics::start_timer_vec( @@ -387,6 +388,7 @@ impl BlockService { )) })? .data + .into() } };