From 8b56446b647de9630d98fbc3b9e74c58e77e0630 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 29 Nov 2022 16:04:16 +0530 Subject: [PATCH] Add more kzg validations --- beacon_node/beacon_chain/src/beacon_chain.rs | 6 +- .../beacon_chain/src/blob_verification.rs | 2 +- .../beacon_chain/src/block_verification.rs | 57 ++++++++++++++++++- beacon_node/beacon_chain/src/errors.rs | 4 +- beacon_node/beacon_chain/src/kzg_utils.rs | 13 ++--- crypto/kzg/src/lib.rs | 1 + 6 files changed, 67 insertions(+), 16 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 92b1e06c5..e5d06b78c 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3943,16 +3943,14 @@ impl BeaconChain { let kzg = if let Some(kzg) = &self.kzg { kzg } else { - return Err(BlockProductionError::KzgError( - "Trusted setup not initialized".to_string(), - )); + return Err(BlockProductionError::TrustedSetupNotInitialized); }; let kzg_aggregated_proof = kzg_utils::compute_aggregate_kzg_proof::(&kzg, &blobs) .map_err(|e| BlockProductionError::KzgError(e))?; let beacon_block_root = block.canonical_root(); let expected_kzg_commitments = block.body().blob_kzg_commitments().map_err(|_| { - BlockProductionError::KzgError( + BlockProductionError::InvalidBlockVariant( "EIP4844 block does not contain kzg commitments".to_string(), ) })?; diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 8756766c4..11b8f20fd 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -65,7 +65,7 @@ pub enum BlobError { InvalidKzgProof, - KzgError(String), + KzgError(kzg::Error), /// A blob sidecar for this proposer and slot has already been observed. /// diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 71fbb4537..036102baf 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -48,6 +48,7 @@ use crate::execution_payload::{ is_optimistic_candidate_block, validate_execution_payload_for_gossip, validate_merge_block, AllowOptimisticImport, PayloadNotifier, }; +use crate::kzg_utils; use crate::snapshot_cache::PreProcessingSnapshot; use crate::validator_monitor::HISTORIC_EPOCHS as VALIDATOR_MONITOR_HISTORIC_EPOCHS; use crate::validator_pubkey_cache::ValidatorPubkeyCache; @@ -69,6 +70,7 @@ use safe_arith::ArithError; use slog::{debug, error, warn, Logger}; use slot_clock::SlotClock; use ssz::Encode; +use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions; use state_processing::per_block_processing::is_merge_transition_block; use state_processing::{ block_signature_verifier::{BlockSignatureVerifier, Error as BlockSignatureVerifierError}, @@ -584,6 +586,8 @@ pub fn signature_verify_chain_segment( drop(pubkey_cache); //FIXME(sean) batch verify kzg blobs + // TODO(pawan): do not have the sidecar here, maybe validate kzg proofs in a different function + // with relevant block params? let mut signature_verified_blocks = chain_segment .into_iter() @@ -926,14 +930,14 @@ impl GossipVerifiedBlock { chain, ) .map_err(BlobValidation)?; - //FIXME(sean) validate blobs sidecar } // Having checked the proposer index and the block root we can cache them. let consensus_context = ConsensusContext::new(block.slot()) .set_current_block_root(block_root) .set_proposer_index(block.message().proposer_index()) - //FIXME(sean) set blobs sidecar validation results + .set_blobs_sidecar_validated(true) // Validated in `validate_blob_for_gossip` + .set_blobs_verified_vs_txs(true) // Validated in `validate_blob_for_gossip` .set_blobs_sidecar(blobs); Ok(Self { @@ -1479,7 +1483,54 @@ impl ExecutionPendingBlock { }); } - //FIXME(sean) validate blobs sidecar + // TODO: are we guaranteed that consensus_context.blobs_sidecar == blobs ? + /* Verify kzg proofs and kzg commitments against transactions if required */ + if let Some(ref sidecar) = blobs { + let kzg = chain.kzg.as_ref().ok_or(BlockError::BlobValidation( + BlobError::TrustedSetupNotInitialized, + ))?; + let transactions = block + .message() + .body() + .execution_payload_eip4844() + .map(|payload| payload.transactions()) + .map_err(|_| BlockError::BlobValidation(BlobError::TransactionsMissing))? + .ok_or(BlockError::BlobValidation(BlobError::TransactionsMissing))?; + let kzg_commitments = block + .message() + .body() + .blob_kzg_commitments() + .map_err(|_| BlockError::BlobValidation(BlobError::KzgCommitmentMissing))?; + if !consensus_context.blobs_sidecar_validated() { + if !kzg_utils::validate_blobs_sidecar( + &kzg, + block.slot(), + block_root, + kzg_commitments, + sidecar, + ) + .map_err(|e| BlockError::BlobValidation(BlobError::KzgError(e)))? + { + return Err(BlockError::BlobValidation(BlobError::InvalidKzgProof)); + } + } + if !consensus_context.blobs_verified_vs_txs() + && verify_kzg_commitments_against_transactions::( + transactions, + kzg_commitments, + ) + .is_err() + { + return Err(BlockError::BlobValidation( + BlobError::TransactionCommitmentMismatch, + )); + } + // TODO(pawan): confirm with sean. are we expected to set the context here? because the ConsensusContext + // setters don't take mutable references. + // Set the consensus context after completing the required kzg valdiations + // consensus_context.set_blobs_sidecar_validated(true); + // consensus_context.set_blobs_verified_vs_txs(true); + } Ok(Self { block, diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index 71cbe0b33..4df50dfcf 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -267,7 +267,9 @@ pub enum BlockProductionError { TokioJoin(tokio::task::JoinError), BeaconChain(BeaconChainError), InvalidPayloadFork, - KzgError(String), + TrustedSetupNotInitialized, + InvalidBlockVariant(String), + KzgError(kzg::Error), } easy_from_to!(BlockProcessingError, BlockProductionError); diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index 3e181facf..7e62e0c31 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -1,4 +1,4 @@ -use kzg::Kzg; +use kzg::{Error as KzgError, Kzg}; use types::{Blob, BlobsSidecar, EthSpec, Hash256, KzgCommitment, KzgProof, Slot}; // TODO(pawan): make this generic over blob size @@ -18,7 +18,7 @@ pub fn validate_blobs_sidecar( beacon_block_root: Hash256, expected_kzg_commitments: &[KzgCommitment], blobs_sidecar: &BlobsSidecar, -) -> Result { +) -> Result { if slot != blobs_sidecar.beacon_block_slot || beacon_block_root != blobs_sidecar.beacon_block_root || blobs_sidecar.blobs.len() != expected_kzg_commitments.len() @@ -31,27 +31,26 @@ pub fn validate_blobs_sidecar( .into_iter() .map(|blob| ssz_blob_to_crypto_blob::(blob.clone())) // TODO(pawan): avoid this clone .collect::>>() - .ok_or_else(|| "Invalid blobs in sidecar".to_string())?; + .ok_or_else(|| KzgError::InvalidBlob("Invalid blobs in sidecar".to_string()))?; kzg.verify_aggregate_kzg_proof( &blobs, expected_kzg_commitments, blobs_sidecar.kzg_aggregated_proof, ) - .map_err(|e| format!("Failed to verify kzg proof: {:?}", e)) } pub fn compute_aggregate_kzg_proof( kzg: &Kzg, blobs: &[Blob], -) -> Result { +) -> Result { let blobs = blobs .into_iter() .map(|blob| ssz_blob_to_crypto_blob::(blob.clone())) // TODO(pawan): avoid this clone .collect::>>() - .ok_or_else(|| "Invalid blobs in sidecar".to_string())?; + .ok_or_else(|| KzgError::InvalidBlob("Invalid blobs".to_string()))?; + kzg.compute_aggregate_kzg_proof(&blobs) - .map_err(|e| format!("Failed to compute kzg proof: {:?}", e)) } pub fn blob_to_kzg_commitment(kzg: &Kzg, blob: Blob) -> Option { diff --git a/crypto/kzg/src/lib.rs b/crypto/kzg/src/lib.rs index ee7a4ad58..914157fec 100644 --- a/crypto/kzg/src/lib.rs +++ b/crypto/kzg/src/lib.rs @@ -21,6 +21,7 @@ pub enum Error { KzgVerificationFailed(CKzgError), InvalidLength(String), KzgProofComputationFailed(CKzgError), + InvalidBlob(String), } /// A wrapper over a kzg library that holds the trusted setup parameters.