From f04486dc7148ef6b26765c567dae289faa97cfe8 Mon Sep 17 00:00:00 2001 From: Pawan Dhananjay Date: Tue, 17 Jan 2023 12:12:15 +0530 Subject: [PATCH] Update kzg library to use bytes only interface --- Cargo.lock | 2 +- .../beacon_chain/src/blob_verification.rs | 17 ------------- beacon_node/beacon_chain/src/kzg_utils.rs | 19 ++++++-------- beacon_node/http_api/src/block_id.rs | 4 +-- crypto/kzg/Cargo.toml | 2 +- crypto/kzg/src/lib.rs | 25 +++++++------------ 6 files changed, 20 insertions(+), 49 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 923d013ab..a4a991f23 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -788,7 +788,7 @@ dependencies = [ [[package]] name = "c-kzg" version = "0.1.0" -source = "git+https://github.com/pawanjay176/c-kzg-4844?rev=c9e4fa0dabdd000738b7fcdf85a72880a5da8748#c9e4fa0dabdd000738b7fcdf85a72880a5da8748" +source = "git+https://github.com/ethereum/c-kzg-4844?rev=69f6155d7524247be9d3f54ab3bfbe33a0345622#69f6155d7524247be9d3f54ab3bfbe33a0345622" dependencies = [ "hex", "libc", diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 06cebf9ea..0725bd865 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -136,13 +136,6 @@ pub fn validate_blob_for_gossip( }); } - // 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); - } - } - // Validate commitments agains transactions in the block. if verify_kzg_commitments_against_transactions::(transactions, kzg_commitments) .is_err() @@ -150,16 +143,6 @@ pub fn validate_blob_for_gossip( 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 diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index 48a3f6e8a..8589a6fe4 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -1,14 +1,11 @@ use kzg::{Error as KzgError, Kzg, BYTES_PER_BLOB}; use types::{Blob, BlobsSidecar, EthSpec, Hash256, KzgCommitment, KzgProof, Slot}; -fn ssz_blob_to_crypto_blob(blob: Blob) -> Option<[u8; BYTES_PER_BLOB]> { - if blob.len() != BYTES_PER_BLOB { - return None; - } +fn ssz_blob_to_crypto_blob(blob: Blob) -> kzg::Blob { let blob_vec: Vec = blob.into(); let mut arr = [0; BYTES_PER_BLOB]; arr.copy_from_slice(&blob_vec); - Some(arr) + arr.into() } pub fn validate_blobs_sidecar( @@ -29,8 +26,7 @@ pub fn validate_blobs_sidecar( .blobs .into_iter() .map(|blob| ssz_blob_to_crypto_blob::(blob.clone())) // TODO(pawan): avoid this clone - .collect::>>() - .ok_or_else(|| KzgError::InvalidBlob("Invalid blobs in sidecar".to_string()))?; + .collect::>(); kzg.verify_aggregate_kzg_proof( &blobs, @@ -46,13 +42,12 @@ pub fn compute_aggregate_kzg_proof( let blobs = blobs .into_iter() .map(|blob| ssz_blob_to_crypto_blob::(blob.clone())) // TODO(pawan): avoid this clone - .collect::>>() - .ok_or_else(|| KzgError::InvalidBlob("Invalid blobs".to_string()))?; + .collect::>(); kzg.compute_aggregate_kzg_proof(&blobs) } -pub fn blob_to_kzg_commitment(kzg: &Kzg, blob: Blob) -> Option { - let blob = ssz_blob_to_crypto_blob::(blob)?; - Some(kzg.blob_to_kzg_commitment(blob)) +pub fn blob_to_kzg_commitment(kzg: &Kzg, blob: Blob) -> KzgCommitment { + let blob = ssz_blob_to_crypto_blob::(blob); + kzg.blob_to_kzg_commitment(blob) } diff --git a/beacon_node/http_api/src/block_id.rs b/beacon_node/http_api/src/block_id.rs index fd45e14fa..45c7bed1f 100644 --- a/beacon_node/http_api/src/block_id.rs +++ b/beacon_node/http_api/src/block_id.rs @@ -216,10 +216,10 @@ impl BlockId { pub async fn blobs_sidecar( &self, chain: &BeaconChain, - ) -> Result<(Arc>), warp::Rejection> { + ) -> Result>, warp::Rejection> { let root = self.root(chain)?.0; match chain.store.get_blobs(&root) { - Ok(Some(blob)) => Ok((Arc::new(blob))), + Ok(Some(blob)) => Ok(Arc::new(blob)), Ok(None) => Err(warp_utils::reject::custom_not_found(format!( "Blob with block root {} is not in the store", root diff --git a/crypto/kzg/Cargo.toml b/crypto/kzg/Cargo.toml index 53c64a4b3..052e9a8c9 100644 --- a/crypto/kzg/Cargo.toml +++ b/crypto/kzg/Cargo.toml @@ -18,7 +18,7 @@ eth2_serde_utils = "0.1.1" hex = "0.4.2" eth2_hashing = "0.3.0" ethereum-types = "0.12.1" -c-kzg = {git = "https://github.com/pawanjay176/c-kzg-4844", rev = "c9e4fa0dabdd000738b7fcdf85a72880a5da8748" } +c-kzg = {git = "https://github.com/ethereum/c-kzg-4844", rev = "69f6155d7524247be9d3f54ab3bfbe33a0345622" } [features] default = ["mainnet-spec"] diff --git a/crypto/kzg/src/lib.rs b/crypto/kzg/src/lib.rs index 5ac93d82a..c179be06f 100644 --- a/crypto/kzg/src/lib.rs +++ b/crypto/kzg/src/lib.rs @@ -3,20 +3,16 @@ mod kzg_proof; mod trusted_setup; pub use crate::{kzg_commitment::KzgCommitment, kzg_proof::KzgProof, trusted_setup::TrustedSetup}; -pub use c_kzg::bytes_to_g1; pub use c_kzg::{ - Blob, Error as CKzgError, KzgSettings, BYTES_PER_BLOB, BYTES_PER_FIELD_ELEMENT, + Blob, Error as CKzgError, KZGSettings, BYTES_PER_BLOB, BYTES_PER_FIELD_ELEMENT, FIELD_ELEMENTS_PER_BLOB, }; use std::path::PathBuf; #[derive(Debug)] -/// TODO(pawan): add docs after the c_kzg interface changes to bytes only. pub enum Error { InvalidTrustedSetup(CKzgError), - InvalidKzgCommitment(CKzgError), InvalidKzgProof(CKzgError), - KzgVerificationFailed(CKzgError), InvalidLength(String), KzgProofComputationFailed(CKzgError), InvalidBlob(String), @@ -24,7 +20,7 @@ pub enum Error { /// A wrapper over a kzg library that holds the trusted setup parameters. pub struct Kzg { - trusted_setup: KzgSettings, + trusted_setup: KZGSettings, } impl Kzg { @@ -35,7 +31,7 @@ impl Kzg { /// The number of G2 points should be equal to 65. pub fn new_from_trusted_setup(trusted_setup: TrustedSetup) -> Result { Ok(Self { - trusted_setup: KzgSettings::load_trusted_setup( + trusted_setup: KZGSettings::load_trusted_setup( trusted_setup.g1_points(), trusted_setup.g2_points(), ) @@ -50,14 +46,14 @@ impl Kzg { #[deprecated] pub fn new_from_file(file_path: PathBuf) -> Result { Ok(Self { - trusted_setup: KzgSettings::load_trusted_setup_file(file_path) + trusted_setup: KZGSettings::load_trusted_setup_file(file_path) .map_err(Error::InvalidTrustedSetup)?, }) } /// Compute the aggregated kzg proof given an array of blobs. pub fn compute_aggregate_kzg_proof(&self, blobs: &[Blob]) -> Result { - c_kzg::KzgProof::compute_aggregate_kzg_proof(blobs, &self.trusted_setup) + c_kzg::KZGProof::compute_aggregate_kzg_proof(blobs, &self.trusted_setup) .map_err(Error::KzgProofComputationFailed) .map(|proof| KzgProof(proof.to_bytes())) } @@ -77,12 +73,9 @@ impl Kzg { } let commitments = expected_kzg_commitments .into_iter() - .map(|comm| { - c_kzg::KzgCommitment::from_bytes(&comm.0).map_err(Error::InvalidKzgCommitment) - }) - .collect::, Error>>()?; - let proof = - c_kzg::KzgProof::from_bytes(&kzg_aggregated_proof.0).map_err(Error::InvalidKzgProof)?; + .map(|comm| comm.0.into()) + .collect::>(); + let proof: c_kzg::KZGProof = kzg_aggregated_proof.0.into(); proof .verify_aggregate_kzg_proof(blobs, &commitments, &self.trusted_setup) .map_err(Error::InvalidKzgProof) @@ -91,7 +84,7 @@ impl Kzg { /// Converts a blob to a kzg commitment. pub fn blob_to_kzg_commitment(&self, blob: Blob) -> KzgCommitment { KzgCommitment( - c_kzg::KzgCommitment::blob_to_kzg_commitment(blob, &self.trusted_setup).to_bytes(), + c_kzg::KZGCommitment::blob_to_kzg_commitment(blob, &self.trusted_setup).to_bytes(), ) } }