From 7818100777fe01ca5b7ca869145e20eeef0fcaa8 Mon Sep 17 00:00:00 2001 From: ethDreamer <37123614+ethDreamer@users.noreply.github.com> Date: Wed, 8 Nov 2023 22:05:44 -0600 Subject: [PATCH] Verify KZG in Bulk During Block Sync (#4903) --- .../beacon_chain/src/block_verification.rs | 53 +++++++------- .../src/block_verification_types.rs | 7 ++ .../src/data_availability_checker.rs | 71 ++++++++++++++++++- .../tests/attestation_production.rs | 4 +- beacon_node/beacon_chain/tests/store_tests.rs | 4 +- .../network_beacon_processor/sync_methods.rs | 12 ++-- 6 files changed, 110 insertions(+), 41 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 7f1a596ec3..65cf7a728b 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -583,29 +583,33 @@ pub fn signature_verify_chain_segment( &chain.spec, )?; + // unzip chain segment and verify kzg in bulk + let (roots, blocks): (Vec<_>, Vec<_>) = chain_segment.into_iter().unzip(); + let maybe_available_blocks = chain + .data_availability_checker + .verify_kzg_for_rpc_blocks(blocks)?; + // zip it back up + let mut signature_verified_blocks = roots + .into_iter() + .zip(maybe_available_blocks) + .map(|(block_root, maybe_available_block)| { + let consensus_context = ConsensusContext::new(maybe_available_block.slot()) + .set_current_block_root(block_root); + SignatureVerifiedBlock { + block: maybe_available_block, + block_root, + parent: None, + consensus_context, + } + }) + .collect::>(); + + // verify signatures let pubkey_cache = get_validator_pubkey_cache(chain)?; let mut signature_verifier = get_signature_verifier(&state, &pubkey_cache, &chain.spec); - - let mut signature_verified_blocks = Vec::with_capacity(chain_segment.len()); - - for (block_root, block) in &chain_segment { - let mut consensus_context = - ConsensusContext::new(block.slot()).set_current_block_root(*block_root); - - signature_verifier.include_all_signatures(block.as_block(), &mut consensus_context)?; - - let maybe_available_block = chain - .data_availability_checker - .check_rpc_block_availability(block.clone())?; - - // Save the block and its consensus context. The context will have had its proposer index - // and attesting indices filled in, which can be used to accelerate later block processing. - signature_verified_blocks.push(SignatureVerifiedBlock { - block: maybe_available_block, - block_root: *block_root, - parent: None, - consensus_context, - }); + for svb in &mut signature_verified_blocks { + signature_verifier + .include_all_signatures(svb.block.as_block(), &mut svb.consensus_context)?; } if signature_verifier.verify().is_err() { @@ -1159,10 +1163,7 @@ impl IntoExecutionPendingBlock for Arc IntoExecutionPendingBlock for RpcBlock .map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?; let maybe_available = chain .data_availability_checker - .check_rpc_block_availability(self.clone()) + .verify_kzg_for_rpc_block(self.clone()) .map_err(|e| { BlockSlashInfo::SignatureNotChecked( self.signed_block_header(), diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index d236e94f93..9cd853ba8c 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -45,6 +45,13 @@ impl RpcBlock { RpcBlockInner::BlockAndBlobs(block, _) => block, } } + + pub fn blobs(&self) -> Option<&BlobSidecarList> { + match &self.block { + RpcBlockInner::Block(_) => None, + RpcBlockInner::BlockAndBlobs(_, blobs) => Some(blobs), + } + } } /// Note: This variant is intentionally private because we want to safely construct the diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index a8d077a6d0..ad328077d0 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -240,9 +240,12 @@ impl DataAvailabilityChecker { .put_pending_executed_block(executed_block) } - /// Checks if a block is available, returns a `MaybeAvailableBlock` that may include the fully - /// available block. - pub fn check_rpc_block_availability( + /// Verifies kzg commitments for an RpcBlock, returns a `MaybeAvailableBlock` that may + /// include the fully available block. + /// + /// WARNING: This function assumes all required blobs are already present, it does NOT + /// check if there are any missing blobs. + pub fn verify_kzg_for_rpc_block( &self, block: RpcBlock, ) -> Result, AvailabilityCheckError> { @@ -279,6 +282,68 @@ impl DataAvailabilityChecker { } } + /// Checks if a vector of blocks are available. Returns a vector of `MaybeAvailableBlock` + /// This is more efficient than calling `verify_kzg_for_rpc_block` in a loop as it does + /// all kzg verification at once + /// + /// WARNING: This function assumes all required blobs are already present, it does NOT + /// check if there are any missing blobs. + pub fn verify_kzg_for_rpc_blocks( + &self, + blocks: Vec>, + ) -> Result>, AvailabilityCheckError> { + let mut results = Vec::with_capacity(blocks.len()); + let all_blobs: BlobSidecarList = blocks + .iter() + .filter(|block| self.blobs_required_for_block(block.as_block())) + // this clone is cheap as it's cloning an Arc + .filter_map(|block| block.blobs().cloned()) + .flatten() + .collect::>() + .into(); + + // verify kzg for all blobs at once + if !all_blobs.is_empty() { + let kzg = self + .kzg + .as_ref() + .ok_or(AvailabilityCheckError::KzgNotInitialized)?; + verify_kzg_for_blob_list(&all_blobs, kzg)?; + } + + for block in blocks { + let (block_root, block, blobs) = block.deconstruct(); + match blobs { + None => { + if self.blobs_required_for_block(&block) { + results.push(MaybeAvailableBlock::AvailabilityPending { block_root, block }) + } else { + results.push(MaybeAvailableBlock::Available(AvailableBlock { + block_root, + block, + blobs: None, + })) + } + } + Some(blob_list) => { + let verified_blobs = if self.blobs_required_for_block(&block) { + Some(blob_list) + } else { + None + }; + // already verified kzg for all blobs + results.push(MaybeAvailableBlock::Available(AvailableBlock { + block_root, + block, + blobs: verified_blobs, + })) + } + } + } + + Ok(results) + } + /// Determines the blob requirements for a block. If the block is pre-deneb, no blobs are required. /// If the block's epoch is from prior to the data availability boundary, no blobs are required. fn blobs_required_for_block(&self, block: &SignedBeaconBlock) -> bool { diff --git a/beacon_node/beacon_chain/tests/attestation_production.rs b/beacon_node/beacon_chain/tests/attestation_production.rs index a8ad75304b..fdc37b5529 100644 --- a/beacon_node/beacon_chain/tests/attestation_production.rs +++ b/beacon_node/beacon_chain/tests/attestation_production.rs @@ -140,7 +140,7 @@ async fn produces_attestations() { available_block, ) = chain .data_availability_checker - .check_rpc_block_availability(rpc_block) + .verify_kzg_for_rpc_block(rpc_block) .unwrap() else { panic!("block should be available") @@ -218,7 +218,7 @@ async fn early_attester_cache_old_request() { harness .chain .data_availability_checker - .check_rpc_block_availability(rpc_block) + .verify_kzg_for_rpc_block(rpc_block) .unwrap() else { panic!("block should be available") diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 5107396ac2..d3f6428851 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -2464,10 +2464,10 @@ async fn weak_subjectivity_sync_test(slots: Vec, checkpoint_slot: Slot) { if let MaybeAvailableBlock::Available(block) = harness .chain .data_availability_checker - .check_rpc_block_availability( + .verify_kzg_for_rpc_block( RpcBlock::new(Some(block_root), Arc::new(full_block), Some(blobs)).unwrap(), ) - .expect("should check availability") + .expect("should verify kzg") { available_blocks.push(block); } diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index b034d0ff31..d76ce5aadd 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -560,14 +560,10 @@ impl NetworkBeaconProcessor { downloaded_blocks: Vec>, ) -> (usize, Result<(), ChainSegmentFailed>) { let total_blocks = downloaded_blocks.len(); - let available_blocks = match downloaded_blocks - .into_iter() - .map(|block| { - self.chain - .data_availability_checker - .check_rpc_block_availability(block) - }) - .collect::, _>>() + let available_blocks = match self + .chain + .data_availability_checker + .verify_kzg_for_rpc_blocks(downloaded_blocks) { Ok(blocks) => blocks .into_iter()