Merge pull request #1745 from o1-labs/audit/bound-check-pad-length

Fix: Prevent underconstraint in xor(), and(), and not() functions

CHANGELOG.md

@@ -21,6 +21,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 - `SmartContract.emitEventIf()` to conditionally emit an event https://github.com/o1-labs/o1js/pull/1746
 
+### Changed
+
+- Reduced maximum bit length for `xor`, `not`, and `and`, operations from 254 to 240 bits to improve performance and simplify implementation. https://github.com/o1-labs/o1js/pull/1745
+
 ## [1.5.0](https://github.com/o1-labs/o1js/compare/ed198f305...1c736add) - 2024-07-09
 
 ### Breaking changes

src/lib/provable/gadgets/bitwise.ts

@@ -19,14 +19,9 @@ export {
 };
 
 function not(a: Field, length: number, checked: boolean = false) {
-  // check that input length is positive
-  assert(length > 0, `Input length needs to be positive values.`);
-
-  // Check that length does not exceed maximum field size in bits
-  assert(
-    length < Field.sizeInBits,
-    `Length ${length} exceeds maximum of ${Field.sizeInBits} bits.`
-  );
+  // Validate at 240 bits to ensure padLength (next multiple of 16) doesn't exceed 254 bits,
+  // preventing potential underconstraint issues in the circuit
+  validateBitLength(length, 240, 'not');
 
   // obtain pad length until the length is a multiple of 16 for n-bit length lookup table
   let padLength = Math.ceil(length / 16) * 16;
@@ -52,11 +47,9 @@ function not(a: Field, length: number, checked: boolean = false) {
 }
 
 function xor(a: Field, b: Field, length: number) {
-  // check that both input lengths are positive
-  assert(length > 0, `Input lengths need to be positive values.`);
-
-  // check that length does not exceed maximum 254 size in bits
-  assert(length <= 254, `Length ${length} exceeds maximum of 254 bits.`);
+  // Validate at 240 bits to ensure padLength (next multiple of 16) doesn't exceed 254 bits,
+  // preventing potential underconstraint issues in the circuit
+  validateBitLength(length, 240, 'xor');
 
   // obtain pad length until the length is a multiple of 16 for n-bit length lookup table
   let padLength = Math.ceil(length / 16) * 16;
@@ -150,14 +143,9 @@ function buildXor(a: Field, b: Field, out: Field, padLength: number) {
 }
 
 function and(a: Field, b: Field, length: number) {
-  // check that both input lengths are positive
-  assert(length > 0, `Input lengths need to be positive values.`);
-
-  // check that length does not exceed maximum field size in bits
-  assert(
-    length <= Field.sizeInBits,
-    `Length ${length} exceeds maximum of ${Field.sizeInBits} bits.`
-  );
+  // Validate at 240 bits to ensure padLength (next multiple of 16) doesn't exceed 254 bits,
+  // preventing potential underconstraint issues in the circuit
+  validateBitLength(length, 240, 'and');
 
   // obtain pad length until the length is a multiple of 16 for n-bit length lookup table
   let padLength = Math.ceil(length / 16) * 16;
@@ -343,3 +331,26 @@ function leftShift32(field: Field, bits: number) {
   let { remainder: shifted } = divMod32(field.mul(1n << BigInt(bits)));
   return shifted;
 }
+
+/**
+ * Validates the bit length for bitwise operations.
+ *
+ * @param length - The input length to validate.
+ * @param maxLength - The maximum allowed length.
+ * @param functionName - The name of the calling function for error messages.
+ *
+ * @throws {Error} If the input length is not positive or exceeds the maximum length.
+ */
+function validateBitLength(
+  length: number,
+  maxLength: number,
+  functionName: string
+) {
+  // check that both input lengths are positive
+  assert(length > 0, `${functionName}: Input length must be a positive value.`);
+  // check that length does not exceed maximum `maxLength` size in bits
+  assert(
+    length <= maxLength,
+    `${functionName}: Length ${length} exceeds maximum of ${maxLength} bits.`
+  );
+}

src/lib/provable/test/bitwise.unit-test.ts

@@ -36,19 +36,19 @@ let Bitwise = ZkProgram({
     xor: {
       privateInputs: [Field, Field],
       async method(a: Field, b: Field) {
-        return Gadgets.xor(a, b, 254);
+        return Gadgets.xor(a, b, 240);
       },
     },
     notUnchecked: {
       privateInputs: [Field],
       async method(a: Field) {
-        return Gadgets.not(a, 254, false);
+        return Gadgets.not(a, 240, false);
       },
     },
     notChecked: {
       privateInputs: [Field],
       async method(a: Field) {
-        return Gadgets.not(a, 254, true);
+        return Gadgets.not(a, 240, true);
       },
     },
     and: {
@@ -153,7 +153,7 @@ await equivalentAsync({ from: [uint(64), uint(64)], to: field }, { runs })(
 
 await equivalentAsync({ from: [maybeField], to: field }, { runs })(
   (x) => {
-    return Fp.not(x, 254);
+    return Fp.not(x, 240);
   },
   async (x) => {
     let proof = await Bitwise.notUnchecked(x);
@@ -162,8 +162,8 @@ await equivalentAsync({ from: [maybeField], to: field }, { runs })(
 );
 await equivalentAsync({ from: [maybeField], to: field }, { runs })(
   (x) => {
-    if (x > 2n ** 254n) throw Error('Does not fit into 254 bit');
-    return Fp.not(x, 254);
+    if (x > 2n ** 240n) throw Error('Does not fit into 240 bit');
+    return Fp.not(x, 240);
   },
   async (x) => {
     let proof = await Bitwise.notChecked(x);
@@ -246,13 +246,13 @@ function xorChain(bits: number) {
 constraintSystem.fromZkProgram(
   Bitwise,
   'xor',
-  ifNotAllConstant(contains(xorChain(254)))
+  ifNotAllConstant(contains(xorChain(240)))
 );
 
 constraintSystem.fromZkProgram(
   Bitwise,
   'notChecked',
-  ifNotAllConstant(contains(xorChain(254)))
+  ifNotAllConstant(contains(xorChain(240)))
 );
 
 constraintSystem.fromZkProgram(