Merge pull request #19 from Layr-Labs/fixing-proof-calculation

Fix kzg verification bug
Layr-Labs · Aug 14, 2024 · fe3b113 · fe3b113
2 parents 30f3a14 + 7236bfe
commit fe3b113
Show file tree

Hide file tree

Showing 9 changed files with 60 additions and 69 deletions.
diff --git a/.gitmodules b/.gitmodules
@@ -16,7 +16,7 @@
 [submodule "contracts"]
 	path = contracts
 	url = https://github.com/Layr-Labs/nitro-contracts.git
-	branch = eigenda--v3.0.3
+	branch = new-osp-fixes-v3.0.3
 [submodule "nitro-testnode"]
 	path = nitro-testnode
 	url = https://github.com/Layr-Labs/nitro-testnode.git

diff --git a/arbitrator/prover/src/kzgbn254.rs b/arbitrator/prover/src/kzgbn254.rs
@@ -1,14 +1,13 @@
-use crate::Bytes32;
+use crate::{utils::append_left_padded_biguint_be, Bytes32};
 use ark_bn254::G2Affine;
 use ark_ec::{AffineRepr, CurveGroup};
 use ark_ff::{BigInteger, PrimeField};
 use ark_serialize::CanonicalSerialize;
 use eyre::{ensure, Result};
-use hex::encode;
 use kzgbn254::{blob::Blob, kzg::Kzg, polynomial::PolynomialFormat};
 use num::BigUint;
-use num_traits::ToBytes;
 use sha2::{Digest, Sha256};
+use sha3::Keccak256;
 use std::io::Write;
 
 lazy_static::lazy_static! {
@@ -36,31 +35,31 @@ pub fn prove_kzg_preimage_bn254(
 ) -> Result<()> {
     let mut kzg = KZG.clone();
 
-    println!("preimage: {} {}", preimage.len(), encode(&preimage));
-    println!("offset: {}", offset);
-
     // expand roots of unity
     kzg.calculate_roots_of_unity(preimage.len() as u64)?;
 
-    // preimage is already padded, unpadding and repadding already padded data can destroy context post IFFT
-    // as some elements in the bn254 field are represented by 32 bytes, we know that the preimage is padded
-    // to 32 bytes per DA spec as the preimage is retrieved from DA, so we can use this unchecked function
-    let blob = Blob::from_padded_bytes_unchecked(preimage);
+    // preimage is already padded and is the actual blob data, NOT the IFFT'd form.
+    let blob = Blob::from_padded_bytes_unchecked(&preimage);
 
-    let blob_polynomial_evaluation_form = blob.to_polynomial(PolynomialFormat::InEvaluationForm)?;
+    let blob_polynomial_evaluation_form =
+        blob.to_polynomial(PolynomialFormat::InCoefficientForm)?;
     let blob_commitment = kzg.commit(&blob_polynomial_evaluation_form)?;
 
-    let mut commitment_bytes = Vec::new();
-    blob_commitment.serialize_uncompressed(&mut commitment_bytes)?;
+    let commitment_x_bigint: BigUint = blob_commitment.x.into();
+    let commitment_y_bigint: BigUint = blob_commitment.y.into();
+    let mut commitment_encoded_bytes = Vec::with_capacity(32);
+    append_left_padded_biguint_be(&mut commitment_encoded_bytes, &commitment_x_bigint);
+    append_left_padded_biguint_be(&mut commitment_encoded_bytes, &commitment_y_bigint);
 
-    let mut expected_hash: Bytes32 = Sha256::digest(&*commitment_bytes).into();
-    expected_hash[0] = 1;
+    let mut keccak256_hasher = Keccak256::new();
+    keccak256_hasher.update(&commitment_encoded_bytes);
+    let commitment_hash: Bytes32 = keccak256_hasher.finalize().into();
 
     ensure!(
-        hash == expected_hash,
+        hash == commitment_hash,
         "Trying to prove versioned hash {} preimage but recomputed hash {}",
         hash,
-        expected_hash,
+        commitment_hash,
     );
 
     ensure!(
@@ -69,21 +68,10 @@ pub fn prove_kzg_preimage_bn254(
         offset,
     );
 
-    // retrieve commitment to preimage
-    let preimage_polynomial = blob.to_polynomial(PolynomialFormat::InCoefficientForm)?;
-    let preimage_commitment = kzg.commit(&preimage_polynomial)?;
-    let mut preimage_commitment_bytes = Vec::new();
-    preimage_commitment.serialize_uncompressed(&mut preimage_commitment_bytes)?;
-    println!(
-        "preimage commitment: {}",
-        encode(&preimage_commitment_bytes)
-    );
-
     let mut proving_offset = offset;
-
     let length_usize = preimage.len() as u64;
 
-    assert!(length_usize / 32 == preimage_polynomial.len() as u64);
+    assert!(length_usize / 32 == blob_polynomial_evaluation_form.len() as u64);
 
     // address proving past end edge case later
     let proving_past_end = offset as u64 >= length_usize;
@@ -94,14 +82,15 @@ pub fn prove_kzg_preimage_bn254(
     }
 
     // Y = ϕ(offset) --> evaluation point for computing quotient proof
-    let proven_y_fr = preimage_polynomial
+    // confirming if this is actually ok ?
+    let proven_y_fr = blob_polynomial_evaluation_form
         .get_at_index(proving_offset as usize / 32)
         .ok_or_else(|| {
             eyre::eyre!(
                 "Index ({}) out of bounds for preimage of length {} with data of ({} field elements x 32 bytes)",
                 proving_offset,
                 length_usize,
-                preimage_polynomial.len()
+                blob_polynomial_evaluation_form.len()
             )
         })?;
 
@@ -112,6 +101,7 @@ pub fn prove_kzg_preimage_bn254(
     let proven_y = proven_y_fr.into_bigint().to_bytes_be();
     let z = z_fr.into_bigint().to_bytes_be();
 
+    // probably should be a constant on the contract.
     let g2_generator = G2Affine::generator();
     let z_g2 = (g2_generator * z_fr).into_affine();
 
@@ -123,8 +113,20 @@ pub fn prove_kzg_preimage_bn254(
         .clone();
     let g2_tau_minus_g2_z = (g2_tau - z_g2).into_affine();
 
-    let kzg_proof = kzg
-        .compute_kzg_proof_with_roots_of_unity(&preimage_polynomial, proving_offset as u64 / 32)?;
+    let kzg_proof = kzg.compute_kzg_proof_with_roots_of_unity(
+        &blob_polynomial_evaluation_form,
+        proving_offset as u64 / 32,
+    )?;
+
+    let offset_usize = proving_offset as usize;
+    // This should cause failure when proving past offset.
+    if !proving_past_end {
+        ensure!(
+            *proven_y == preimage[offset_usize..offset_usize + 32],
+            "KZG proof produced wrong preimage for offset {}",
+            offset,
+        );
+    }
 
     let xminusz_x0: BigUint = g2_tau_minus_g2_z.x.c0.into();
     let xminusz_x1: BigUint = g2_tau_minus_g2_z.x.c1.into();
@@ -138,13 +140,6 @@ pub fn prove_kzg_preimage_bn254(
     append_left_padded_biguint_be(&mut xminusz_encoded_bytes, &xminusz_y1);
     append_left_padded_biguint_be(&mut xminusz_encoded_bytes, &xminusz_y0);
 
-    // encode the commitment
-    let commitment_x_bigint: BigUint = preimage_commitment.x.into();
-    let commitment_y_bigint: BigUint = preimage_commitment.y.into();
-    let mut commitment_encoded_bytes = Vec::with_capacity(32);
-    append_left_padded_biguint_be(&mut commitment_encoded_bytes, &commitment_x_bigint);
-    append_left_padded_biguint_be(&mut commitment_encoded_bytes, &commitment_y_bigint);
-
     // encode the proof
     let proof_x_bigint: BigUint = kzg_proof.x.into();
     let proof_y_bigint: BigUint = kzg_proof.y.into();
@@ -155,9 +150,7 @@ pub fn prove_kzg_preimage_bn254(
     let mut length_bytes = Vec::with_capacity(32);
     append_left_padded_biguint_be(&mut length_bytes, &BigUint::from(length_usize));
 
-    println!("length usize: {}", length_usize);
-    println!("length bytes: {}", encode(&length_bytes));
-    out.write_all(&*hash)?; // hash [:32]
+    out.write_all(&commitment_hash.to_vec())?; // hash [:32]
     out.write_all(&*z)?; // evaluation point [32:64]
     out.write_all(&*proven_y)?; // expected output [64:96]
     out.write_all(&xminusz_encoded_bytes)?; // g2TauMinusG2z [96:224]
@@ -167,11 +160,3 @@ pub fn prove_kzg_preimage_bn254(
 
     Ok(())
 }
-
-// Helper function to append BigUint bytes into the vector with padding; left padded big endian bytes to 32
-fn append_left_padded_biguint_be(vec: &mut Vec<u8>, biguint: &BigUint) {
-    let bytes = biguint.to_bytes_be();
-    let padding = 32 - bytes.len();
-    vec.extend_from_slice(&vec![0; padding]);
-    vec.extend_from_slice(&bytes);
-}
diff --git a/arbitrator/prover/src/machine.rs b/arbitrator/prover/src/machine.rs
@@ -2466,8 +2466,6 @@ impl Machine {
                         if !preimage.len().is_power_of_two() {
                             bail!("EigenDA hash preimage length should be a power of two but is instead {}", preimage.len());
                         }
-
-                        println!("EIGENDA HASH PREIMAGE: {:?}", preimage);
                     }
 
                     let offset = usize::try_from(offset).unwrap();

diff --git a/arbitrator/prover/src/utils.rs b/arbitrator/prover/src/utils.rs
@@ -10,6 +10,7 @@ use c_kzg::{Blob, KzgCommitment};
 use digest::Digest;
 use eyre::{eyre, Result};
 use kzgbn254::{blob::Blob as EigenDABlob, kzg::Kzg as KzgBN254, polynomial::PolynomialFormat};
+use num::BigUint;
 use serde::{Deserialize, Serialize};
 use sha2::Sha256;
 use sha3::Keccak256;
@@ -192,6 +193,14 @@ pub fn split_import(qualified: &str) -> Result<(&str, &str)> {
     Ok((module, name))
 }
 
+// Helper function to append BigUint bytes into the vector with padding; left padded big endian bytes to 32
+pub fn append_left_padded_biguint_be(vec: &mut Vec<u8>, biguint: &BigUint) {
+    let bytes = biguint.to_bytes_be();
+    let padding = 32 - bytes.len();
+    vec.extend(std::iter::repeat(0).take(padding));
+    vec.extend_from_slice(&bytes);
+}
+
 #[cfg(feature = "native")]
 pub fn hash_preimage(preimage: &[u8], ty: PreimageType) -> Result<[u8; 32]> {
     match ty {
@@ -218,14 +227,18 @@ pub fn hash_preimage(preimage: &[u8], ty: PreimageType) -> Result<[u8; 32]> {
 
             let blob = EigenDABlob::from_padded_bytes_unchecked(preimage);
 
-            let blob_polynomial = blob.to_polynomial(PolynomialFormat::InEvaluationForm)?;
+            let blob_polynomial = blob.to_polynomial(PolynomialFormat::InCoefficientForm)?;
             let blob_commitment = kzg_bn254.commit(&blob_polynomial)?;
 
-            let mut commitment_bytes = Vec::new();
-            blob_commitment.serialize_uncompressed(&mut commitment_bytes)?;
+            let commitment_x_bigint: BigUint = blob_commitment.x.into();
+            let commitment_y_bigint: BigUint = blob_commitment.y.into();
+            let mut commitment_encoded_bytes = Vec::with_capacity(32);
+            append_left_padded_biguint_be(&mut commitment_encoded_bytes, &commitment_x_bigint);
+            append_left_padded_biguint_be(&mut commitment_encoded_bytes, &commitment_y_bigint);
 
-            let mut commitment_hash: [u8; 32] = Sha256::digest(&commitment_bytes).into();
-            commitment_hash[0] = 1;
+            let mut keccak256_hasher = Keccak256::new();
+            keccak256_hasher.update(&commitment_encoded_bytes);
+            let commitment_hash: [u8; 32] = keccak256_hasher.finalize().into();
 
             Ok(commitment_hash)
         }

diff --git a/arbitrator/prover/test-cases/go/main.go b/arbitrator/prover/test-cases/go/main.go
@@ -211,7 +211,7 @@ func main() {
 		}
 	}
 	// EIGENDA COMMIT HASH
-	_, err = wavmio.ResolveTypedPreimage(arbutil.EigenDaPreimageType, common.HexToHash("01605220b6928163676612ca50bbe5e0c595052876796dbedeae8ef597c9fdcf"))
+	_, err = wavmio.ResolveTypedPreimage(arbutil.EigenDaPreimageType, common.HexToHash("13bbacb54f9aa9896af97156ca4dfc626e94031c5ed78fea68659e4ec9c9c55a"))
 	if err != nil {
 		panic(fmt.Sprintf("failed to resolve eigenda preimage: %v", err))
 	}

diff --git a/arbitrator/prover/test-cases/rust/src/bin/host-io.rs b/arbitrator/prover/test-cases/rust/src/bin/host-io.rs
@@ -111,7 +111,7 @@ fn main() {
         for i in 0..5{
             // test-files srs 011e229d75b13559dcb2d757ecae9b66fa579268e28e196789503322115c06e1
             // mainnet srs 01605220b6928163676612ca50bbe5e0c595052876796dbedeae8ef597c9fdcf
-            let eigen_hash = hex!("01605220b6928163676612ca50bbe5e0c595052876796dbedeae8ef597c9fdcf");
+            let eigen_hash = hex!("13bbacb54f9aa9896af97156ca4dfc626e94031c5ed78fea68659e4ec9c9c55a");
             bytebuffer = Bytes32(eigen_hash);
 
             let actual_len = wavm_read_eigen_da_hash_preimage(bytebuffer.0.as_mut_ptr(), i * 32);

diff --git a/arbos/programs/api.go b/arbos/programs/api.go
@@ -220,7 +220,7 @@ func newApiClosures(
 		if suberr != nil {
 			addr = zeroAddr
 		}
-		if !errors.Is(vm.ErrExecutionReverted, suberr) {
+		if !errors.Is(vm.ErrExecutionReverted, suberr) { //nolint:all
 			res = nil // returnData is only provided in the revert case (opCreate)
 		}
 		interpreter.SetReturnData(res)

diff --git a/cmd/nitro/config_test.go b/cmd/nitro/config_test.go
@@ -73,7 +73,7 @@ func TestReloads(t *testing.T) {
 			hot := node.Type().Field(i).Tag.Get("reload") == "hot"
 			dot := path + "." + node.Type().Field(i).Name
 			if hot && cold {
-				t.Fatalf(fmt.Sprintf(
+				t.Fatalf(fmt.Sprintf( //nolint:all
 					"Option %v%v%v is reloadable but %v%v%v is not",
 					colors.Red, dot, colors.Clear,
 					colors.Red, path, colors.Clear,

diff --git a/scripts/create-test-preimages.py b/scripts/create-test-preimages.py
@@ -28,12 +28,7 @@ def kzg_test_data():
 def eigen_test_data():    
     # the value we are returning is the same string that is returned by the old eigen_test_data but encoded in the style the high level eigenDA client would
     # 00bca02094eb78126a517b206a88c73cfa9ec6f704c7030d18212cace820f025
-    data = bytes([
-    12, 74, 134, 141, 159, 142, 12, 228, 147, 176, 42, 148, 17, 187, 240, 48, 98, 179, 158, 173, 119, 72, 129, 73, 181, 94, 239, 1, 22, 164, 231, 89, 
-    45, 148, 221, 13, 66, 188, 31, 31, 18, 90, 120, 195, 53, 74, 121, 91, 29, 163, 78, 174, 81, 239, 152, 253, 188, 242, 52, 132, 164, 53, 20, 26, 
-    36, 75, 123, 21, 222, 118, 68, 224, 87, 187, 179, 60, 161, 97, 0, 70, 93, 178, 98, 55, 27, 137, 136, 121, 63, 52, 185, 46, 242, 115, 75, 192, 
-    2, 157, 190, 53, 1, 226, 207, 111, 114, 218, 52, 217, 26, 155, 70, 232, 114, 94, 128, 254, 14, 177, 62, 97, 214, 62, 14, 115, 50, 178, 184, 207
-    ])
+    data = bytes([0 ,0 ,0 ,0 ,0 ,64 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,48 ,48 ,98 ,99 ,97 ,48 ,50 ,48 ,57 ,52 ,101 ,98 ,55 ,56 ,49 ,50 ,54 ,97 ,53 ,49 ,55 ,98 ,50 ,48 ,54 ,97 ,56 ,56 ,99 ,55 ,51 ,0 ,99 ,102 ,97 ,57 ,101 ,99 ,54 ,102 ,55 ,48 ,52 ,99 ,55 ,48 ,51 ,48 ,100 ,49 ,56 ,50 ,49 ,50 ,99 ,97 ,99 ,101 ,56 ,50 ,48 ,102 ,48 ,0 ,50 ,53 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0 ,0])
 
     return data