From 6ec0ce60702939762c5e17d67d4149fbe64f69df Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 15 Feb 2023 16:44:13 +1100 Subject: [PATCH 1/5] Implement get validator block endpoint for EIP-4844 --- beacon_node/beacon_chain/src/beacon_chain.rs | 47 ++++++++++------ beacon_node/beacon_chain/src/blob_cache.rs | 8 +-- beacon_node/beacon_chain/src/errors.rs | 1 + .../http_api/src/build_block_contents.rs | 34 +++++++++++ beacon_node/http_api/src/lib.rs | 6 +- beacon_node/http_api/src/publish_blocks.rs | 11 ++-- common/eth2/src/lib.rs | 4 +- consensus/types/src/beacon_block.rs | 11 ++++ .../src/beacon_block_and_blob_sidecars.rs | 37 ++++++++++++ consensus/types/src/blob_sidecar.rs | 6 ++ consensus/types/src/block_contents.rs | 56 +++++++++++++++++++ consensus/types/src/lib.rs | 6 +- validator_client/src/block_service.rs | 8 ++- 13 files changed, 201 insertions(+), 34 deletions(-) create mode 100644 beacon_node/http_api/src/build_block_contents.rs create mode 100644 consensus/types/src/beacon_block_and_blob_sidecars.rs create mode 100644 consensus/types/src/block_contents.rs diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 1e901987a..277194105 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -4759,30 +4759,41 @@ 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( - kzg, - slot, - beacon_block_root, - expected_kzg_commitments, - &blobs_sidecar, - ) - .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)| { + BlobSidecar { + block_root: beacon_block_root, + index: blob_index as u64, + slot, + block_parent_root: block.parent_root(), + proposer_index, + blob, + kzg_commitment: expected_kzg_commitments[blob_index].clone(), + kzg_proof: Default::default(), // TODO: compute KZG proof + } + }) + .collect::>>(), + ); + + // TODO: validate blobs + // kzg_utils::validate_blobs_sidecar( + // kzg, + // slot, + // beacon_block_root, + // expected_kzg_commitments, + // &blobs_sidecar, + // ).map_err(BlockProductionError::KzgError)?; + self.blob_cache.put(beacon_block_root, blob_sidecars); } metrics::inc_counter(&metrics::BLOCK_PRODUCTION_SUCCESSES); diff --git a/beacon_node/beacon_chain/src/blob_cache.rs b/beacon_node/beacon_chain/src/blob_cache.rs index d03e62ab6..e3b0a06b6 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::{BlobSidecars, 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,11 @@ impl Default for BlobCache { } impl BlobCache { - pub fn put(&self, beacon_block: Hash256, blobs: BlobsSidecar) -> Option> { + pub fn put(&self, beacon_block: Hash256, blobs: BlobSidecars) -> 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/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 3b4c46249..8f9aa0293 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -266,6 +266,7 @@ pub enum BlockProductionError { blob_block_hash: ExecutionBlockHash, payload_block_hash: ExecutionBlockHash, }, + NoBlobsCached, FailedToReadFinalizedBlock(store::Error), MissingFinalizedBlock(Hash256), BlockTooLarge(usize), 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..d456c320a --- /dev/null +++ b/beacon_node/http_api/src/build_block_contents.rs @@ -0,0 +1,34 @@ +use beacon_chain::{BeaconChain, BeaconChainTypes, BlockProductionError}; +use std::sync::Arc; +use types::{ + AbstractExecPayload, BeaconBlock, BeaconBlockAndBlobSidecars, BlockContents, 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/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/consensus/types/src/beacon_block.rs b/consensus/types/src/beacon_block.rs index 0f26cd0e5..a92926582 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -731,6 +731,17 @@ impl From>> } } +impl> From> + for BeaconBlock +{ + fn from(block_contents: BlockContents) -> Self { + match block_contents { + BlockContents::BlockAndBlobSidecars(block_and_sidecars) => block_and_sidecars.block, + BlockContents::Block(block) => block, + } + } +} + impl> ForkVersionDeserialize for BeaconBlock { 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..c518f765b --- /dev/null +++ b/consensus/types/src/beacon_block_and_blob_sidecars.rs @@ -0,0 +1,37 @@ +use crate::{ + AbstractExecPayload, BeaconBlock, BlobSidecars, 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: BlobSidecars, +} + +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: BlobSidecars, + } + 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..251bef034 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 BlobSidecars = VariableList, ::MaxBlobsPerBlock>; + impl SignedRoot for BlobSidecar {} impl BlobSidecar { diff --git a/consensus/types/src/block_contents.rs b/consensus/types/src/block_contents.rs new file mode 100644 index 000000000..d5a500280 --- /dev/null +++ b/consensus/types/src/block_contents.rs @@ -0,0 +1,56 @@ +use crate::{ + AbstractExecPayload, BeaconBlock, BeaconBlockAndBlobSidecars, BlobSidecars, EthSpec, ForkName, + ForkVersionDeserialize, +}; +use derivative::Derivative; +use serde_derive::{Deserialize, Serialize}; + +/// A wrapper over a [`BeaconBlock`] or a [`BeaconBlockAndBlobSidecars`]. +#[derive(Clone, Debug, Derivative, Serialize, Deserialize)] +#[derivative(PartialEq, Hash(bound = "T: EthSpec"))] +#[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)?, + )), + } + } +} diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index ce4837853..50b9547c9 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -99,8 +99,10 @@ 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 block_contents; pub mod signed_blob; pub mod signed_block_and_blobs; pub mod transaction; @@ -116,6 +118,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,8 +126,9 @@ 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, BlobSidecars}; pub use crate::blobs_sidecar::{Blobs, BlobsSidecar}; +pub use crate::block_contents::BlockContents; pub use crate::bls_to_execution_change::BlsToExecutionChange; pub use crate::chain_spec::{ChainSpec, Config, Domain}; pub use crate::checkpoint::Checkpoint; 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() } }; From 62627d984c155cd799b32f39d9d69a91b5f238e3 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 15 Mar 2023 12:30:59 +1100 Subject: [PATCH 2/5] Comment out code that fails to compile --- .../beacon_chain/src/blob_verification.rs | 25 +++++++++++-------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 1d7f9cc3c..34aaf6435 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -135,17 +135,20 @@ 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); - } + // TODO: use `kzg_utils::validate_blobs` once the function is updated + // I believe this is currently being worked on in another branch. + // + // if !kzg_utils::validate_blobs_sidecar( + // kzg, + // block_slot, + // block_root, + // kzg_commitments, + // blob_sidecar, + // ) + // .map_err(BlobError::KzgError)? + // { + // return Err(BlobError::InvalidKzgProof); + // } Ok(()) } From 02a88f07049c7f6d5de91542050f0b1d0006af69 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 15 Mar 2023 15:15:46 +1100 Subject: [PATCH 3/5] Add KZG proof and blob validation --- beacon_node/beacon_chain/src/beacon_chain.rs | 82 +++++++++++++++---- beacon_node/beacon_chain/src/errors.rs | 1 + beacon_node/beacon_chain/src/kzg_utils.rs | 5 +- .../lighthouse_network/src/types/topics.rs | 1 - beacon_node/store/src/hot_cold_store.rs | 4 +- crypto/kzg/src/kzg_commitment.rs | 2 +- 6 files changed, 71 insertions(+), 24 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 277194105..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::*; @@ -4760,39 +4762,60 @@ impl BeaconChain { .as_ref() .ok_or(BlockProductionError::TrustedSetupNotInitialized)?; 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 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, + expected_kzg_commitments, + &blobs, + &kzg_proofs, + ) + .map_err(BlockProductionError::KzgError)?; let blob_sidecars = VariableList::from( blobs .into_iter() .enumerate() .map(|(blob_index, blob)| { - BlobSidecar { + 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: expected_kzg_commitments[blob_index].clone(), - kzg_proof: Default::default(), // TODO: compute KZG proof - } + kzg_commitment: *kzg_commitment, + kzg_proof: *kzg_proof, + }) }) - .collect::>>(), + .collect::>, BlockProductionError>>()?, ); - // TODO: validate blobs - // kzg_utils::validate_blobs_sidecar( - // kzg, - // slot, - // beacon_block_root, - // expected_kzg_commitments, - // &blobs_sidecar, - // ).map_err(BlockProductionError::KzgError)?; self.blob_cache.put(beacon_block_root, blob_sidecars); } @@ -4809,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/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 8f9aa0293..911fe1d5b 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -273,6 +273,7 @@ pub enum BlockProductionError { 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/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/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]); From 5887c8fe92b40afc8f2e4b5147fef2f2cc2538e7 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 15 Mar 2023 15:29:16 +1100 Subject: [PATCH 4/5] Update commented code to use todo! --- beacon_node/beacon_chain/src/blob_verification.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 34aaf6435..7a4493c97 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -135,9 +135,7 @@ fn verify_data_availability( .as_ref() .ok_or(BlobError::TrustedSetupNotInitialized)?; - // TODO: use `kzg_utils::validate_blobs` once the function is updated - // I believe this is currently being worked on in another branch. - // + todo!("use `kzg_utils::validate_blobs` once the function is updated") // if !kzg_utils::validate_blobs_sidecar( // kzg, // block_slot, @@ -149,7 +147,7 @@ fn verify_data_availability( // { // return Err(BlobError::InvalidKzgProof); // } - Ok(()) + // Ok(()) } /// A wrapper over a [`SignedBeaconBlock`] or a [`SignedBeaconBlockAndBlobsSidecar`]. This makes no From 3d99e1f14dc1b9050d619bbcae154d96fb184f5f Mon Sep 17 00:00:00 2001 From: realbigsean Date: Wed, 15 Mar 2023 12:15:08 -0400 Subject: [PATCH 5/5] move block contents to api crate, rename blob sidecars list --- beacon_node/beacon_chain/src/blob_cache.rs | 12 ++-- .../http_api/src/build_block_contents.rs | 5 +- common/eth2/src/types.rs | 60 +++++++++++++++++++ consensus/types/src/beacon_block.rs | 11 ---- .../src/beacon_block_and_blob_sidecars.rs | 6 +- consensus/types/src/blob_sidecar.rs | 2 +- consensus/types/src/block_contents.rs | 56 ----------------- consensus/types/src/lib.rs | 4 +- 8 files changed, 75 insertions(+), 81 deletions(-) delete mode 100644 consensus/types/src/block_contents.rs diff --git a/beacon_node/beacon_chain/src/blob_cache.rs b/beacon_node/beacon_chain/src/blob_cache.rs index e3b0a06b6..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::{BlobSidecars, 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: BlobSidecars) -> 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/http_api/src/build_block_contents.rs b/beacon_node/http_api/src/build_block_contents.rs index d456c320a..1908c03ea 100644 --- a/beacon_node/http_api/src/build_block_contents.rs +++ b/beacon_node/http_api/src/build_block_contents.rs @@ -1,8 +1,7 @@ use beacon_chain::{BeaconChain, BeaconChainTypes, BlockProductionError}; +use eth2::types::BlockContents; use std::sync::Arc; -use types::{ - AbstractExecPayload, BeaconBlock, BeaconBlockAndBlobSidecars, BlockContents, ForkName, -}; +use types::{AbstractExecPayload, BeaconBlock, BeaconBlockAndBlobSidecars, ForkName}; type Error = warp::reject::Rejection; 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.rs b/consensus/types/src/beacon_block.rs index a92926582..0f26cd0e5 100644 --- a/consensus/types/src/beacon_block.rs +++ b/consensus/types/src/beacon_block.rs @@ -731,17 +731,6 @@ impl From>> } } -impl> From> - for BeaconBlock -{ - fn from(block_contents: BlockContents) -> Self { - match block_contents { - BlockContents::BlockAndBlobSidecars(block_and_sidecars) => block_and_sidecars.block, - BlockContents::Block(block) => block, - } - } -} - impl> ForkVersionDeserialize for BeaconBlock { diff --git a/consensus/types/src/beacon_block_and_blob_sidecars.rs b/consensus/types/src/beacon_block_and_blob_sidecars.rs index c518f765b..78e704196 100644 --- a/consensus/types/src/beacon_block_and_blob_sidecars.rs +++ b/consensus/types/src/beacon_block_and_blob_sidecars.rs @@ -1,5 +1,5 @@ use crate::{ - AbstractExecPayload, BeaconBlock, BlobSidecars, EthSpec, ForkName, ForkVersionDeserialize, + AbstractExecPayload, BeaconBlock, BlobSidecarList, EthSpec, ForkName, ForkVersionDeserialize, }; use derivative::Derivative; use serde_derive::{Deserialize, Serialize}; @@ -11,7 +11,7 @@ use tree_hash_derive::TreeHash; #[serde(bound = "T: EthSpec, Payload: AbstractExecPayload")] pub struct BeaconBlockAndBlobSidecars> { pub block: BeaconBlock, - pub blob_sidecars: BlobSidecars, + pub blob_sidecars: BlobSidecarList, } impl> ForkVersionDeserialize @@ -25,7 +25,7 @@ impl> ForkVersionDeserialize #[serde(bound = "T: EthSpec")] struct Helper { block: serde_json::Value, - blob_sidecars: BlobSidecars, + blob_sidecars: BlobSidecarList, } let helper: Helper = serde_json::from_value(value).map_err(serde::de::Error::custom)?; diff --git a/consensus/types/src/blob_sidecar.rs b/consensus/types/src/blob_sidecar.rs index 251bef034..169c570d2 100644 --- a/consensus/types/src/blob_sidecar.rs +++ b/consensus/types/src/blob_sidecar.rs @@ -47,7 +47,7 @@ pub struct BlobSidecar { pub kzg_proof: KzgProof, } -pub type BlobSidecars = VariableList, ::MaxBlobsPerBlock>; +pub type BlobSidecarList = VariableList, ::MaxBlobsPerBlock>; impl SignedRoot for BlobSidecar {} diff --git a/consensus/types/src/block_contents.rs b/consensus/types/src/block_contents.rs deleted file mode 100644 index d5a500280..000000000 --- a/consensus/types/src/block_contents.rs +++ /dev/null @@ -1,56 +0,0 @@ -use crate::{ - AbstractExecPayload, BeaconBlock, BeaconBlockAndBlobSidecars, BlobSidecars, EthSpec, ForkName, - ForkVersionDeserialize, -}; -use derivative::Derivative; -use serde_derive::{Deserialize, Serialize}; - -/// A wrapper over a [`BeaconBlock`] or a [`BeaconBlockAndBlobSidecars`]. -#[derive(Clone, Debug, Derivative, Serialize, Deserialize)] -#[derivative(PartialEq, Hash(bound = "T: EthSpec"))] -#[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)?, - )), - } - } -} diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs index 50b9547c9..fe47454aa 100644 --- a/consensus/types/src/lib.rs +++ b/consensus/types/src/lib.rs @@ -102,7 +102,6 @@ pub mod sqlite; pub mod beacon_block_and_blob_sidecars; pub mod blob_sidecar; pub mod blobs_sidecar; -pub mod block_contents; pub mod signed_blob; pub mod signed_block_and_blobs; pub mod transaction; @@ -126,9 +125,8 @@ 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, BlobSidecars}; +pub use crate::blob_sidecar::{BlobSidecar, BlobSidecarList}; pub use crate::blobs_sidecar::{Blobs, BlobsSidecar}; -pub use crate::block_contents::BlockContents; pub use crate::bls_to_execution_change::BlsToExecutionChange; pub use crate::chain_spec::{ChainSpec, Config, Domain}; pub use crate::checkpoint::Checkpoint;