chore: avoid bn254_blackbox_solver polluting feature flags (noir-la…

…ng/noir#5141) chore!: remove `distinct` keyword (noir-lang/noir#5219) feat: Sync from aztec-packages (noir-lang/noir#5222) chore: add some property tests to ACVM crates (noir-lang/noir#5215) chore: inline `FieldElement.is_negative` and document (noir-lang/noir#5214) chore!: remove `param_witnesses` and `return_witnesses` from ABI (noir-lang/noir#5154) chore: move implementation of bitwise operations into `blackbox_solver` (noir-lang/noir#5209) chore: remove stale comment (noir-lang/noir#5179) chore: make `nargo` crate and debug info generic (noir-lang/noir#5184) feat!: add session id to foreign call RPC requests (noir-lang/noir#5205) chore(docs): fix incorrect docs github link in footer (noir-lang/noir#5206) fix: error for allocate instructions in acir-gen (noir-lang/noir#5200) feat: Implement println in the comptime interpreter (noir-lang/noir#5197) chore(docs): Supplement Noir Debugger's dependency versions (noir-lang/noir#5199) chore(docs): Update docs homepage (noir-lang/noir#5198) chore: add more lints related to oracle calls (noir-lang/noir#5193) feat: standardize pedersen functions to return `EmbeddedCurvePoint` (noir-lang/noir#5190)
AztecProtocol · Jun 11, 2024 · e1dc886 · e1dc886
2 parents ff8b27e + b456ac1
commit e1dc886
Show file tree

Hide file tree

Showing 63 changed files with 433 additions and 812 deletions.
diff --git a/.noir-sync-commit b/.noir-sync-commit
@@ -1 +1 @@
-14adafc965fa9c833e096ec037e086aae67703ad
+2e543b40eb83ef2080e4d8f870f525fadd631099
diff --git a/noir/noir-repo/.github/workflows/test-js-packages.yml b/noir/noir-repo/.github/workflows/test-js-packages.yml
@@ -221,32 +221,6 @@ jobs:
       - name: Run browser tests
         run: yarn workspace @noir-lang/noirc_abi test:browser
 
-  test-noir-js-backend-barretenberg:
-    needs: [build-noirc-abi]
-    name: noir-js-backend-barretenberg
-    runs-on: ubuntu-latest
-    timeout-minutes: 30
-
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v4
-
-      - name: Download wasm package artifact
-        uses: actions/download-artifact@v4
-        with:
-          name: noirc_abi_wasm
-          path: ./tooling/noirc_abi_wasm
-
-      - name: Install Yarn dependencies
-        uses: ./.github/actions/setup
-
-      - name: Build noir_js_types
-        run: yarn workspace @noir-lang/types build
-
-      - name: Run barretenberg wrapper tests
-        run: |
-          yarn workspace @noir-lang/backend_barretenberg test
-
   test-noir-js:
     needs: [build-nargo, build-acvm-js, build-noirc-abi]
     name: Noir JS
@@ -546,7 +520,6 @@ jobs:
       - test-acvm_js-node
       - test-acvm_js-browser
       - test-noirc-abi
-      - test-noir-js-backend-barretenberg
       - test-noir-js
       - test-noir-wasm
       - test-noir-codegen

diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock
diff --git a/noir/noir-repo/Cargo.toml b/noir/noir-repo/Cargo.toml
@@ -77,6 +77,14 @@ noirc_abi = { path = "tooling/noirc_abi" }
 bb_abstraction_leaks = { path = "tooling/bb_abstraction_leaks" }
 acvm_cli = { path = "tooling/acvm_cli" }
 
+# Arkworks
+ark-bn254 = { version = "^0.4.0", default-features = false, features = ["curve"] }
+ark-bls12-381 = { version = "^0.4.0", default-features = false, features = ["curve"] }
+grumpkin = { version = "0.1.0", package = "noir_grumpkin", features = ["std"] } 
+ark-ec = { version = "^0.4.0", default-features = false }
+ark-ff = { version = "^0.4.0", default-features = false }
+ark-std = { version = "^0.4.0", default-features = false }
+
 # Misc utils crates
 iter-extended = { path = "utils/iter-extended" }
 
@@ -132,6 +140,7 @@ tempfile = "3.6.0"
 jsonrpc = { version = "0.16.0", features = ["minreq_http"] }
 flate2 = "1.0.24"
 rand = "0.8.5"
+proptest = "1.2.0"
 
 im = { version = "15.1", features = ["serde"] }
 tracing = "0.1.40"

diff --git a/noir/noir-repo/acvm-repo/acir_field/Cargo.toml b/noir/noir-repo/acvm-repo/acir_field/Cargo.toml
@@ -17,12 +17,15 @@ hex.workspace = true
 num-bigint.workspace = true
 serde.workspace = true
 
-ark-bn254 = { version = "^0.4.0", default-features = false, features = ["curve"] }
-ark-bls12-381 = { version = "^0.4.0", optional = true, default-features = false, features = ["curve"] }
-ark-ff = { version = "^0.4.0", default-features = false }
+ark-bn254.workspace = true
+ark-bls12-381 = { workspace = true, optional = true }
+ark-ff.workspace = true
 
 cfg-if = "1.0.0"
 
+[dev-dependencies]
+proptest.workspace = true
+
 [features]
 bn254 = []
 bls12_381 = ["dep:ark-bls12-381"]
diff --git a/noir/noir-repo/acvm-repo/acir_field/proptest-regressions/field_element.txt b/noir/noir-repo/acvm-repo/acir_field/proptest-regressions/field_element.txt
@@ -0,0 +1,7 @@
+# Seeds for failure cases proptest has generated in the past. It is
+# automatically read and these particular cases re-run before any
+# novel cases are generated.
+#
+# It is recommended to check this file in to source control so that
+# everyone who runs the test benefits from these saved cases.
+cc c1b2755b427c874de3ee55c2969e8adc1b4b0331a28173e9e1e492b3fee6b7b7 # shrinks to hex = "3a0aaaa0aa0aaa00000aaaaaaaa0000a000aaa0a00a0000aa00a00aa0a000a0a"
diff --git a/noir/noir-repo/acvm-repo/acir_field/src/field_element.rs b/noir/noir-repo/acvm-repo/acir_field/src/field_element.rs
@@ -174,10 +174,6 @@ impl<F: PrimeField> FieldElement<F> {
         self.0
     }
 
-    fn is_negative(&self) -> bool {
-        self.neg().num_bits() < self.num_bits()
-    }
-
     fn fits_in_u128(&self) -> bool {
         self.num_bits() <= 128
     }
@@ -195,15 +191,6 @@ impl<F: PrimeField> FieldElement<F> {
         Some(FieldElement(fr))
     }
 
-    // mask_to methods will not remove any bytes from the field
-    // they are simply zeroed out
-    // Whereas truncate_to will remove those bits and make the byte array smaller
-    fn mask_to_be_bytes(&self, num_bits: u32) -> Vec<u8> {
-        let mut bytes = self.to_be_bytes();
-        mask_vector_le(&mut bytes, num_bits as usize);
-        bytes
-    }
-
     fn bits(&self) -> Vec<bool> {
         fn byte_to_bit(byte: u8) -> Vec<bool> {
             let mut bits = Vec::with_capacity(8);
@@ -220,29 +207,6 @@ impl<F: PrimeField> FieldElement<F> {
         }
         bits
     }
-
-    fn and_xor(&self, rhs: &FieldElement<F>, num_bits: u32, is_xor: bool) -> FieldElement<F> {
-        // XXX: Gadgets like SHA256 need to have their input be a multiple of 8
-        // This is not a restriction caused by SHA256, as it works on bits
-        // but most backends assume bytes.
-        // We could implicitly pad, however this may not be intuitive for users.
-        // assert!(
-        //     num_bits % 8 == 0,
-        //     "num_bits is not a multiple of 8, it is {}",
-        //     num_bits
-        // );
-
-        let lhs_bytes = self.mask_to_be_bytes(num_bits);
-        let rhs_bytes = rhs.mask_to_be_bytes(num_bits);
-
-        let and_byte_arr: Vec<_> = lhs_bytes
-            .into_iter()
-            .zip(rhs_bytes)
-            .map(|(lhs, rhs)| if is_xor { lhs ^ rhs } else { lhs & rhs })
-            .collect();
-
-        FieldElement::from_be_bytes_reduce(&and_byte_arr)
-    }
 }
 
 impl<F: PrimeField> AcirField for FieldElement<F> {
@@ -310,7 +274,10 @@ impl<F: PrimeField> AcirField for FieldElement<F> {
     }
 
     fn to_i128(self) -> i128 {
-        let is_negative = self.is_negative();
+        // Negative integers are represented by the range [p + i128::MIN, p) whilst
+        // positive integers are represented by the range [0, i128::MAX).
+        // We can then differentiate positive from negative values by their MSB.
+        let is_negative = self.neg().num_bits() < self.num_bits();
         let bytes = if is_negative { self.neg() } else { self }.to_be_bytes();
         i128::from_be_bytes(bytes[16..32].try_into().unwrap()) * if is_negative { -1 } else { 1 }
     }
@@ -376,13 +343,6 @@ impl<F: PrimeField> AcirField for FieldElement<F> {
 
         bytes[0..num_elements].to_vec()
     }
-
-    fn and(&self, rhs: &FieldElement<F>, num_bits: u32) -> FieldElement<F> {
-        self.and_xor(rhs, num_bits, false)
-    }
-    fn xor(&self, rhs: &FieldElement<F>, num_bits: u32) -> FieldElement<F> {
-        self.and_xor(rhs, num_bits, true)
-    }
 }
 
 impl<F: PrimeField> Neg for FieldElement<F> {
@@ -433,35 +393,6 @@ impl<F: PrimeField> SubAssign for FieldElement<F> {
     }
 }
 
-fn mask_vector_le(bytes: &mut [u8], num_bits: usize) {
-    // reverse to big endian format
-    bytes.reverse();
-
-    let mask_power = num_bits % 8;
-    let array_mask_index = num_bits / 8;
-
-    for (index, byte) in bytes.iter_mut().enumerate() {
-        match index.cmp(&array_mask_index) {
-            std::cmp::Ordering::Less => {
-                // do nothing if the current index is less than
-                // the array index.
-            }
-            std::cmp::Ordering::Equal => {
-                let mask = 2u8.pow(mask_power as u32) - 1;
-                // mask the byte
-                *byte &= mask;
-            }
-            std::cmp::Ordering::Greater => {
-                // Anything greater than the array index
-                // will be set to zero
-                *byte = 0;
-            }
-        }
-    }
-    // reverse back to little endian
-    bytes.reverse();
-}
-
 // For pretty printing powers
 fn superscript(n: u64) -> String {
     if n == 0 {
@@ -494,19 +425,7 @@ fn superscript(n: u64) -> String {
 #[cfg(test)]
 mod tests {
     use super::{AcirField, FieldElement};
-
-    #[test]
-    fn and() {
-        let max = 10_000u32;
-
-        let num_bits = (std::mem::size_of::<u32>() * 8) as u32 - max.leading_zeros();
-
-        for x in 0..max {
-            let x = FieldElement::<ark_bn254::Fr>::from(x as i128);
-            let res = x.and(&x, num_bits);
-            assert_eq!(res.to_be_bytes(), x.to_be_bytes());
-        }
-    }
+    use proptest::prelude::*;
 
     #[test]
     fn serialize_fixed_test_vectors() {
@@ -524,24 +443,36 @@ mod tests {
         }
     }
 
-    #[test]
-    fn deserialize_even_and_odd_length_hex() {
-        // Test cases of (odd, even) length hex strings
-        let hex_strings =
-            vec![("0x0", "0x00"), ("0x1", "0x01"), ("0x002", "0x0002"), ("0x00003", "0x000003")];
-        for (i, case) in hex_strings.into_iter().enumerate() {
-            let i_field_element = FieldElement::<ark_bn254::Fr>::from(i as i128);
-            let odd_field_element = FieldElement::<ark_bn254::Fr>::from_hex(case.0).unwrap();
-            let even_field_element = FieldElement::<ark_bn254::Fr>::from_hex(case.1).unwrap();
-
-            assert_eq!(i_field_element, odd_field_element);
-            assert_eq!(odd_field_element, even_field_element);
-        }
-    }
-
     #[test]
     fn max_num_bits_smoke() {
         let max_num_bits_bn254 = FieldElement::<ark_bn254::Fr>::max_num_bits();
         assert_eq!(max_num_bits_bn254, 254);
     }
+
+    proptest! {
+        // This currently panics due to the fact that we allow inputs which are greater than the field modulus,
+        // automatically reducing them to fit within the canonical range.
+        #[test]
+        #[should_panic(expected = "serialized field element is not equal to input")]
+        fn recovers_original_hex_string(hex in "[0-9a-f]{64}") {
+            let fe: FieldElement::<ark_bn254::Fr> = FieldElement::from_hex(&hex).expect("should accept any 32 byte hex string");
+            let output_hex = fe.to_hex();
+
+            prop_assert_eq!(hex, output_hex, "serialized field element is not equal to input");
+        }
+
+        #[test]
+        fn accepts_odd_length_hex_strings(hex in "(?:0x)[0-9a-fA-F]+") {
+            // Here we inject a "0" immediately after the "0x" (if it exists) to construct an equivalent
+            // hex string with the opposite parity length.
+            let insert_index = if hex.starts_with("0x") { 2 } else { 0 };
+            let mut opposite_parity_string = hex.to_string();
+            opposite_parity_string.insert(insert_index, '0');
+
+            let fe_1: FieldElement::<ark_bn254::Fr> = FieldElement::from_hex(&hex).unwrap();
+            let fe_2: FieldElement::<ark_bn254::Fr> = FieldElement::from_hex(&opposite_parity_string).unwrap();
+
+            prop_assert_eq!(fe_1, fe_2, "equivalent hex strings with opposite parity deserialized to different values");
+        }
+    }
 }
diff --git a/noir/noir-repo/acvm-repo/acir_field/src/generic_ark.rs b/noir/noir-repo/acvm-repo/acir_field/src/generic_ark.rs
@@ -78,7 +78,4 @@ pub trait AcirField:
     /// Returns the closest number of bytes to the bits specified
     /// This method truncates
     fn fetch_nearest_bytes(&self, num_bits: usize) -> Vec<u8>;
-
-    fn and(&self, rhs: &Self, num_bits: u32) -> Self;
-    fn xor(&self, rhs: &Self, num_bits: u32) -> Self;
 }
diff --git a/noir/noir-repo/acvm-repo/acvm/Cargo.toml b/noir/noir-repo/acvm-repo/acvm/Cargo.toml
@@ -37,7 +37,4 @@ bls12_381 = [
 ]
 
 [dev-dependencies]
-rand.workspace = true
-proptest = "1.2.0"
-paste = "1.0.14"
-ark-bls12-381 = { version = "^0.4.0", default-features = false, features = ["curve"] }
+ark-bls12-381 = { version = "^0.4.0", default-features = false, features = ["curve"] }
diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/logic.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/logic.rs
@@ -5,6 +5,7 @@ use acir::{
     native_types::{Witness, WitnessMap},
     AcirField,
 };
+use acvm_blackbox_solver::{bit_and, bit_xor};
 
 /// Solves a [`BlackBoxFunc::And`][acir::circuit::black_box_functions::BlackBoxFunc::AND] opcode and inserts
 /// the result into the supplied witness map
@@ -19,7 +20,7 @@ pub(super) fn and<F: AcirField>(
         "number of bits specified for each input must be the same"
     );
     solve_logic_opcode(initial_witness, &lhs.witness, &rhs.witness, *output, |left, right| {
-        left.and(right, lhs.num_bits)
+        bit_and(left, right, lhs.num_bits)
     })
 }
 
@@ -36,7 +37,7 @@ pub(super) fn xor<F: AcirField>(
         "number of bits specified for each input must be the same"
     );
     solve_logic_opcode(initial_witness, &lhs.witness, &rhs.witness, *output, |left, right| {
-        left.xor(right, lhs.num_bits)
+        bit_xor(left, right, lhs.num_bits)
     })
 }
 
@@ -46,11 +47,11 @@ fn solve_logic_opcode<F: AcirField>(
     a: &Witness,
     b: &Witness,
     result: Witness,
-    logic_op: impl Fn(&F, &F) -> F,
+    logic_op: impl Fn(F, F) -> F,
 ) -> Result<(), OpcodeResolutionError<F>> {
     let w_l_value = witness_to_value(initial_witness, *a)?;
     let w_r_value = witness_to_value(initial_witness, *b)?;
-    let assignment = logic_op(w_l_value, w_r_value);
+    let assignment = logic_op(*w_l_value, *w_r_value);
 
     insert_value(&result, assignment, initial_witness)
 }
diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/black_box_solvers.rs b/noir/noir-repo/acvm-repo/acvm_js/src/black_box_solvers.rs
@@ -9,7 +9,7 @@ use acvm::{acir::AcirField, FieldElement};
 pub fn and(lhs: JsString, rhs: JsString) -> JsString {
     let lhs = js_value_to_field_element(lhs.into()).unwrap();
     let rhs = js_value_to_field_element(rhs.into()).unwrap();
-    let result = lhs.and(&rhs, FieldElement::max_num_bits());
+    let result = acvm::blackbox_solver::bit_and(lhs, rhs, FieldElement::max_num_bits());
     field_element_to_js_string(&result)
 }
 
@@ -18,7 +18,7 @@ pub fn and(lhs: JsString, rhs: JsString) -> JsString {
 pub fn xor(lhs: JsString, rhs: JsString) -> JsString {
     let lhs = js_value_to_field_element(lhs.into()).unwrap();
     let rhs = js_value_to_field_element(rhs.into()).unwrap();
-    let result = lhs.xor(&rhs, FieldElement::max_num_bits());
+    let result = acvm::blackbox_solver::bit_xor(lhs, rhs, FieldElement::max_num_bits());
     field_element_to_js_string(&result)
 }