Add more kzg validations

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -3967,7 +3967,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 slot,
                 beacon_block_root,
                 expected_kzg_commitments,
-                blobs_sidecar.clone(),
+                &blobs_sidecar,
             )
             .map_err(BlockProductionError::KzgError)?;
             self.blob_cache.put(beacon_block_root, blobs_sidecar);

beacon_node/beacon_chain/src/blob_verification.rs

@@ -1,11 +1,9 @@
-use derivative::Derivative;
 use slot_clock::SlotClock;
-use std::sync::Arc;
 
 use crate::beacon_chain::{BeaconChain, BeaconChainTypes, MAXIMUM_GOSSIP_CLOCK_DISPARITY};
-use crate::BeaconChainError;
-use bls::PublicKey;
-use types::{consts::eip4844::BLS_MODULUS, BeaconStateError, BlobsSidecar, Hash256, Slot};
+use crate::{kzg_utils, BeaconChainError};
+use state_processing::per_block_processing::eip4844::eip4844::verify_kzg_commitments_against_transactions;
+use types::{BeaconStateError, BlobsSidecar, Hash256, KzgCommitment, Slot, Transactions};
 
 #[derive(Debug)]
 pub enum BlobError {
@@ -36,7 +34,9 @@ pub enum BlobError {
     /// ## Peer scoring
     ///
     /// The peer has sent an invalid message.
-    BlobOutOfRange { blob_index: usize },
+    BlobOutOfRange {
+        blob_index: usize,
+    },
 
     /// The blob sidecar contains a KZGCommitment that is not a valid G1 point on
     /// the bls curve.
@@ -52,13 +52,31 @@ pub enum BlobError {
     /// The signature on the blob sidecar invalid and the peer is faulty.
     ProposalSignatureInvalid,
 
+    /// No kzg ccommitment associated with blob sidecar.
+    KzgCommitmentMissing,
+
+    /// No transactions in block
+    TransactionsMissing,
+
+    /// Blob transactions in the block do not correspond to the kzg commitments.
+    TransactionCommitmentMismatch,
+
+    TrustedSetupNotInitialized,
+
+    InvalidKzgProof,
+
+    KzgError(String),
+
     /// A blob sidecar for this proposer and slot has already been observed.
     ///
     /// ## Peer scoring
     ///
     /// The `proposer` has already proposed a sidecar at this slot. The existing sidecar may or may not
     /// be equal to the given sidecar.
-    RepeatSidecar { proposer: u64, slot: Slot },
+    RepeatSidecar {
+        proposer: u64,
+        slot: Slot,
+    },
 
     /// There was an error whilst processing the sync contribution. It is not known if it is valid or invalid.
     ///
@@ -83,6 +101,10 @@ impl From<BeaconStateError> for BlobError {
 
 pub fn validate_blob_for_gossip<T: BeaconChainTypes>(
     blob_sidecar: &BlobsSidecar<T::EthSpec>,
+    kzg_commitments: &[KzgCommitment],
+    transactions: &Transactions<T::EthSpec>,
+    block_slot: Slot,
+    block_root: Hash256,
     chain: &BeaconChain<T>,
 ) -> Result<(), BlobError> {
     let blob_slot = blob_sidecar.beacon_block_slot;
@@ -109,19 +131,46 @@ pub fn validate_blob_for_gossip<T: BeaconChainTypes>(
         });
     }
 
-    // Verify that blobs are properly formatted
-    //TODO: add the check while constructing a Blob type from bytes instead of after
-    // for (i, blob) in blob_sidecar.blobs.iter().enumerate() {
-    //     if blob.iter().any(|b| *b >= *BLS_MODULUS) {
-    //         return Err(BlobError::BlobOutOfRange { blob_index: i });
-    //     }
-    // }
-
-    // Verify that the KZG proof is a valid G1 point
-    if PublicKey::deserialize(&blob_sidecar.kzg_aggregated_proof.0).is_err() {
-        return Err(BlobError::InvalidKZGCommitment);
+    // Verify that kzg commitments in the block are valid BLS g1 points
+    for commitment in kzg_commitments {
+        if kzg::bytes_to_g1(&commitment.0).is_err() {
+            return Err(BlobError::InvalidKZGCommitment);
+        }
     }
 
-    // TODO: `validate_blobs_sidecar`
+    // Validate commitments agains transactions in the block.
+    if verify_kzg_commitments_against_transactions::<T::EthSpec>(transactions, kzg_commitments)
+        .is_err()
+    {
+        return Err(BlobError::TransactionCommitmentMismatch);
+    }
+
+    // Check that blobs are < BLS_MODULUS
+    // TODO(pawan): Add this check after there's some resolution of this
+    // issue https://github.com/ethereum/c-kzg-4844/issues/11
+    // As of now, `bytes_to_bls_field` does not fail in the c-kzg library if blob >= BLS_MODULUS
+
+    // Validate that kzg proof is a valid g1 point
+    if kzg::bytes_to_g1(&blob_sidecar.kzg_aggregated_proof.0).is_err() {
+        return Err(BlobError::InvalidKzgProof);
+    }
+
+    // Validatate that the kzg proof is valid against the commitments and blobs
+    let kzg = chain
+        .kzg
+        .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(())
 }

beacon_node/beacon_chain/src/block_verification.rs

@@ -90,7 +90,7 @@ use types::{
     EthSpec, ExecutionBlockHash, Hash256, InconsistentFork, PublicKey, PublicKeyBytes,
     RelativeEpoch, SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
 };
-use types::{BlobsSidecar, ExecPayload, SignedBeaconBlockAndBlobsSidecar};
+use types::{BlobsSidecar, ExecPayload};
 
 pub const POS_PANDA_BANNER: &str = r#"
     ,,,         ,,,                                               ,,,         ,,,
@@ -905,7 +905,27 @@ impl<T: BeaconChainTypes> GossipVerifiedBlock<T> {
         validate_execution_payload_for_gossip(&parent_block, block.message(), chain)?;
 
         if let Some(blobs_sidecar) = blobs.as_ref() {
-            validate_blob_for_gossip(blobs_sidecar, chain).map_err(BlobValidation)?;
+            let kzg_commitments = block
+                .message()
+                .body()
+                .blob_kzg_commitments()
+                .map_err(|_| BlockError::BlobValidation(BlobError::KzgCommitmentMissing))?;
+            let transactions = block
+                .message()
+                .body()
+                .execution_payload_eip4844()
+                .map(|payload| payload.transactions())
+                .map_err(|_| BlockError::BlobValidation(BlobError::TransactionsMissing))?
+                .ok_or(BlockError::BlobValidation(BlobError::TransactionsMissing))?;
+            validate_blob_for_gossip(
+                blobs_sidecar,
+                kzg_commitments,
+                transactions,
+                block.slot(),
+                block_root,
+                chain,
+            )
+            .map_err(BlobValidation)?;
             //FIXME(sean) validate blobs sidecar
         }
 

beacon_node/beacon_chain/src/kzg_utils.rs

@@ -17,7 +17,7 @@ pub fn validate_blobs_sidecar<T: EthSpec>(
     slot: Slot,
     beacon_block_root: Hash256,
     expected_kzg_commitments: &[KzgCommitment],
-    blobs_sidecar: BlobsSidecar<T>,
+    blobs_sidecar: &BlobsSidecar<T>,
 ) -> Result<bool, String> {
     if slot != blobs_sidecar.beacon_block_slot
         || beacon_block_root != blobs_sidecar.beacon_block_root

consensus/state_processing/src/per_block_processing/eip4844/eip4844.rs

@@ -3,7 +3,6 @@ use eth2_hashing::hash_fixed;
 use itertools::{EitherOrBoth, Itertools};
 use safe_arith::SafeArith;
 use ssz::Decode;
-use ssz_types::VariableList;
 use types::consts::eip4844::{BLOB_TX_TYPE, VERSIONED_HASH_VERSION_KZG};
 use types::{
     AbstractExecPayload, BeaconBlockBodyRef, EthSpec, ExecPayload, KzgCommitment, Transaction,
@@ -30,7 +29,7 @@ pub fn process_blob_kzg_commitments<T: EthSpec, Payload: AbstractExecPayload<T>>
 
 pub fn verify_kzg_commitments_against_transactions<T: EthSpec>(
     transactions: &Transactions<T>,
-    kzg_commitments: &VariableList<KzgCommitment, T::MaxBlobsPerBlock>,
+    kzg_commitments: &[KzgCommitment],
 ) -> Result<bool, BlockProcessingError> {
     let nested_iter = transactions
         .into_iter()

crypto/kzg/src/lib.rs

@@ -2,6 +2,7 @@ mod kzg_commitment;
 mod kzg_proof;
 
 pub use crate::{kzg_commitment::KzgCommitment, kzg_proof::KzgProof};
+pub use c_kzg::bytes_to_g1;
 use c_kzg::{Error as CKzgError, KZGSettings, BYTES_PER_FIELD_ELEMENT, FIELD_ELEMENTS_PER_BLOB};
 use std::path::PathBuf;