Merge pull request #20 from ethereum/check_encodings

Add checks that field elements are canonical

bindings/csharp/Ckzg.Test/E2eTests.cs

@@ -5,30 +5,41 @@ namespace Ckzg.Test;
 [TestFixture]
 public class BasicKzgTests
 {
-    [TestCase]
+    private IntPtr _ts;
-    public unsafe void Test_Computes_And_Verifies()
+
+    [SetUp]
+    public void Setup()
     {
-        IntPtr ts = Ckzg.LoadTrustedSetup("trusted_setup.txt");
+        _ts = Ckzg.LoadTrustedSetup("trusted_setup.txt");
-        Assert.That((ulong)ts, Is.Not.EqualTo((ulong)0));
+        Assert.That(_ts, Is.Not.EqualTo(IntPtr.Zero));
-        byte[] blob = Enumerable.Range(0, 4096 * 32).Select(x => (byte)(x % 256)).ToArray();
+    }
+
+    [TestCase(0xff, -1, -1)]
+    [TestCase(0x73, -1, -1)]
+    [TestCase(0x72, 0, 0)]
+    [TestCase(0x00, 0, 0)]
+    public unsafe void Test_Computes_And_Verifies(byte highByteValue, int expectedProofComputed, int expectedProofVerified)
+    {
+        byte[] blob = Enumerable.Range(0, 4096 * 32).Select(x => x % 32 == 31 ? highByteValue : (byte)(x % 256)).ToArray();
 
         byte[] proof = new byte[48];
         byte[] commitment = new byte[48];
         fixed (byte* commitmentPtr = commitment, blobPtr = blob, proofPtr = proof)
         {
-            Ckzg.ComputeAggregatedKzgProof(proofPtr, blobPtr, 1, ts);
+            int proofComputed = Ckzg.ComputeAggregatedKzgProof(proofPtr, blobPtr, 1, _ts);
-            Ckzg.BlobToKzgCommitment(commitmentPtr, blobPtr, ts);
+            Assert.That(proofComputed, Is.EqualTo(expectedProofComputed));
-            int result = Ckzg.VerifyAggregatedKzgProof(blobPtr, commitmentPtr, 1, proofPtr, ts);
+
-            Assert.That(result, Is.EqualTo(0));
+            Ckzg.BlobToKzgCommitment(commitmentPtr, blobPtr, _ts);
-            Ckzg.FreeTrustedSetup(ts);
+            int proofVerified = Ckzg.VerifyAggregatedKzgProof(blobPtr, commitmentPtr, 1, proofPtr, _ts);
+            Assert.That(proofVerified, Is.EqualTo(expectedProofVerified));
+
+            Ckzg.FreeTrustedSetup(_ts);
         }
     }
 
     [TestCase]
     public unsafe void Test_PointEvaluationPrecompile_Verifies()
     {
-        IntPtr ts = Ckzg.LoadTrustedSetup("trusted_setup.txt");
-        Assert.That((ulong)ts, Is.Not.EqualTo((ulong)0));
         byte[] commitment = new byte[48];
         commitment[0] = 0xc0;
         byte[] x = new byte[32];
@@ -38,8 +49,8 @@ public class BasicKzgTests
 
         fixed (byte* commitmentPtr = commitment, xPtr = x, yPtr = y, proofPtr = proof)
         {
-            int result = Ckzg.VerifyKzgProof(commitmentPtr, xPtr, yPtr, proofPtr, ts);
+            int result = Ckzg.VerifyKzgProof(commitmentPtr, xPtr, yPtr, proofPtr, _ts);
-            Ckzg.FreeTrustedSetup(ts);
+            Ckzg.FreeTrustedSetup(_ts);
             Assert.That(result, Is.EqualTo(0));
         }
     }

bindings/node.js/test.ts

@@ -20,7 +20,19 @@ const SETUP_FILE_PATH = existsSync(setupFileName)
 
 const BLOB_BYTE_COUNT = FIELD_ELEMENTS_PER_BLOB * BYTES_PER_FIELD_ELEMENT;
 
-const generateRandomBlob = () => new Uint8Array(randomBytes(BLOB_BYTE_COUNT));
+const MAX_TOP_BYTE = 114;
+
+const generateRandomBlob = () => {
+  return new Uint8Array(
+    randomBytes(BLOB_BYTE_COUNT).map((x, i) => {
+      // Set the top byte to be low enough that the field element doesn't overflow the BLS modulus
+      if (x > MAX_TOP_BYTE && i % BYTES_PER_FIELD_ELEMENT == 31) {
+        return Math.floor(Math.random() * MAX_TOP_BYTE);
+      }
+      return x;
+    }),
+  );
+};
 
 describe("C-KZG", () => {
   beforeAll(async () => {
@@ -50,8 +62,7 @@ describe("C-KZG", () => {
     );
   });
 
-  // Just don't call verifyAggregateKzgProof when there are no blobs or commitments
+  it("verifies the aggregate proof of empty blobs and commitments", () => {
-  it.skip("verifies the aggregate proof of empty blobs and commitments", () => {
     expect(verifyAggregateKzgProof([], [], computeAggregateKzgProof([]))).toBe(
       true,
     );

bindings/python/tests.py

@@ -7,7 +7,9 @@ BLOB_SIZE = 4096
 MAX_BLOBS_PER_BLOCK = 16
 
 blobs = [
-  random.randbytes(32 * BLOB_SIZE) for _ in range(3)
+  # use zero final bytes to easily ensure the encodings are valid
+  b''.join([b''.join([random.randbytes(31), bytes(1)]) for _ in range(BLOB_SIZE)])
+  for _ in range(3)
 ]
 
 ts = ckzg.load_trusted_setup("../../src/trusted_setup.txt")
@@ -26,8 +28,8 @@ assert ckzg.verify_aggregate_kzg_proof(blobs_bytes, kzg_commitments, proof, ts),
 
 # Verification fails at wrong value
 
-other = b'x' if not blobs_bytes.endswith(b'x') else b'y'
+other = b'x' if not blobs_bytes.startswith(b'x') else b'y'
-other_bytes = blobs_bytes[:-1] + other
+other_bytes = other + blobs_bytes[1:]
 
 assert not ckzg.verify_aggregate_kzg_proof(other_bytes, kzg_commitments, proof, ts), 'verify succeeded incorrectly'
 

src/c_kzg_4844.c

@@ -840,12 +840,20 @@ static void compute_powers(BLSFieldElement out[], BLSFieldElement *x, uint64_t n
   }
 }
 
-void bytes_to_bls_field(BLSFieldElement *out, const uint8_t bytes[32]) {
+static void hash_to_bls_field(BLSFieldElement *out, const uint8_t bytes[32]) {
   blst_scalar tmp;
   blst_scalar_from_lendian(&tmp, bytes);
   blst_fr_from_scalar(out, &tmp);
 }
 
+C_KZG_RET bytes_to_bls_field(BLSFieldElement *out, const uint8_t bytes[32]) {
+  blst_scalar tmp;
+  blst_scalar_from_lendian(&tmp, bytes);
+  if (!blst_scalar_fr_check(&tmp)) return C_KZG_BADARGS;
+  blst_fr_from_scalar(out, &tmp);
+  return C_KZG_OK;
+}
+
 static void poly_lincomb(Polynomial out, const Polynomial vectors[], const fr_t scalars[], uint64_t n) {
   fr_t tmp;
   uint64_t i, j;
@@ -923,14 +931,19 @@ static C_KZG_RET poly_to_kzg_commitment(KZGCommitment *out, const Polynomial p,
   return g1_lincomb(out, s->g1_values, p, FIELD_ELEMENTS_PER_BLOB);
 }
 
-static void poly_from_blob(Polynomial p, const Blob blob) {
+static C_KZG_RET poly_from_blob(Polynomial p, const Blob blob) {
-  for (size_t i = 0; i < FIELD_ELEMENTS_PER_BLOB; i++)
+  C_KZG_RET ret;
-    bytes_to_bls_field(&p[i], &blob[i * BYTES_PER_FIELD_ELEMENT]);
+  for (size_t i = 0; i < FIELD_ELEMENTS_PER_BLOB; i++) {
+    ret = bytes_to_bls_field(&p[i], &blob[i * BYTES_PER_FIELD_ELEMENT]);
+    if (ret != C_KZG_OK) return ret;
+  }
+  return C_KZG_OK;
 }
 
 C_KZG_RET blob_to_kzg_commitment(KZGCommitment *out, const Blob blob, const KZGSettings *s) {
   Polynomial p;
-  poly_from_blob(p, blob);
+  C_KZG_RET ret = poly_from_blob(p, blob);
+  if (ret != C_KZG_OK) return ret;
   return poly_to_kzg_commitment(out, p, s);
 }
 
@@ -968,8 +981,9 @@ C_KZG_RET verify_kzg_proof(bool *out,
     const KZGProof *kzg_proof,
     const KZGSettings *s) {
   BLSFieldElement frz, fry;
-  bytes_to_bls_field(&frz, z);
+  C_KZG_RET ret;
-  bytes_to_bls_field(&fry, y);
+  ret = bytes_to_bls_field(&frz, z); if (ret != C_KZG_OK) return ret;
+  ret = bytes_to_bls_field(&fry, y); if (ret != C_KZG_OK) return ret;
   return verify_kzg_proof_impl(out, commitment, &frz, &fry, kzg_proof, s);
 }
 
@@ -1143,14 +1157,14 @@ static C_KZG_RET compute_challenges(BLSFieldElement *out, BLSFieldElement r_powe
 
   /* Compute r_powers */
   BLSFieldElement r;
-  bytes_to_bls_field(&r, r_bytes);
+  hash_to_bls_field(&r, r_bytes);
   compute_powers(r_powers, &r, n);
 
   /* Compute eval_challenge */
   uint8_t eval_challenge[32] = {0};
   hash_input[32] = 0x1;
   hash(eval_challenge, hash_input, 33);
-  bytes_to_bls_field(out, eval_challenge);
+  hash_to_bls_field(out, eval_challenge);
 
   free(bytes);
   return C_KZG_OK;
@@ -1185,15 +1199,20 @@ C_KZG_RET compute_aggregate_kzg_proof(KZGProof *out,
   Polynomial* polys = calloc(n, sizeof(Polynomial));
   if (0 < n && polys == NULL) { free(commitments); return C_KZG_MALLOC; }
 
+  C_KZG_RET ret;
   for (size_t i = 0; i < n; i++) {
-    poly_from_blob(polys[i], blobs[i]);
+    ret = poly_from_blob(polys[i], blobs[i]);
+    if (ret != C_KZG_OK) {
+      if (commitments != NULL) free(commitments);
+      if (polys != NULL) free(polys);
+      return ret;
+    }
     poly_to_kzg_commitment(&commitments[i], polys[i], s);
   }
 
   Polynomial aggregated_poly;
   KZGCommitment aggregated_poly_commitment;
   BLSFieldElement evaluation_challenge;
-  C_KZG_RET ret;
   ret = compute_aggregated_poly_and_commitment(aggregated_poly, &aggregated_poly_commitment, &evaluation_challenge, polys, commitments, n);
   if (commitments != NULL) free(commitments);
   if (polys != NULL) free(polys);
@@ -1210,13 +1229,15 @@ C_KZG_RET verify_aggregate_kzg_proof(bool *out,
     const KZGSettings *s) {
   Polynomial* polys = calloc(n, sizeof(Polynomial));
   if (polys == NULL) return C_KZG_MALLOC;
-  for (size_t i = 0; i < n; i++)
+  C_KZG_RET ret;
-    poly_from_blob(polys[i], blobs[i]);
+  for (size_t i = 0; i < n; i++) {
+    ret = poly_from_blob(polys[i], blobs[i]);
+    if (ret != C_KZG_OK) { free(polys); return ret; }
+  }
 
   Polynomial aggregated_poly;
   KZGCommitment aggregated_poly_commitment;
   BLSFieldElement evaluation_challenge;
-  C_KZG_RET ret;
   ret = compute_aggregated_poly_and_commitment(aggregated_poly, &aggregated_poly_commitment, &evaluation_challenge, polys, expected_kzg_commitments, n);
   free(polys);
   if (ret != C_KZG_OK) return ret;

src/c_kzg_4844.h

@@ -89,7 +89,7 @@ typedef struct {
 C_KZG_RET bytes_to_g1(g1_t* out, const uint8_t in[48]);
 void bytes_from_g1(uint8_t out[48], const g1_t *in);
 
-void bytes_to_bls_field(BLSFieldElement *out, const uint8_t in[BYTES_PER_FIELD_ELEMENT]);
+C_KZG_RET bytes_to_bls_field(BLSFieldElement *out, const uint8_t in[BYTES_PER_FIELD_ELEMENT]);
 
 C_KZG_RET load_trusted_setup(KZGSettings *out,
     const uint8_t g1_bytes[], /* n1 * 48 bytes */