fix: Move BigInt modulus checks to runtime in brillig (noir-lang#5374)

# Description ## Problem\* Resolves noir-lang#5373 ## Summary\* Bigint modulus was checked but never written. Since changing bigint::to_le_bytes to write a modulus ID would require a breaking change, this fixes the modulus issue without changing the opcode spec, by completely offloading modulus checks to the runtime itself. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[For Experimental Features]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings.
blocksense-network · Jul 11, 2024 · 741d339 · 741d339
1 parent f7c4bdb
commit 741d339
Show file tree

Hide file tree

Showing 4 changed files with 20 additions and 44 deletions.
diff --git a/acvm-repo/blackbox_solver/src/bigint.rs b/acvm-repo/blackbox_solver/src/bigint.rs
@@ -18,7 +18,7 @@ pub struct BigIntSolver {
 }
 
 impl BigIntSolver {
-    pub(crate) fn get_bigint(
+    pub fn get_bigint(
         &self,
         id: u32,
         func: BlackBoxFunc,
@@ -32,7 +32,7 @@ impl BigIntSolver {
             .cloned()
     }
 
-    pub(crate) fn get_modulus(
+    pub fn get_modulus(
         &self,
         id: u32,
         func: BlackBoxFunc,

diff --git a/acvm-repo/brillig_vm/src/black_box.rs b/acvm-repo/brillig_vm/src/black_box.rs
@@ -421,6 +421,14 @@ impl BrilligBigintSolver {
         rhs: u32,
         func: BlackBoxFunc,
     ) -> Result<u32, BlackBoxResolutionError> {
+        let modulus_lhs = self.bigint_solver.get_modulus(lhs, func)?;
+        let modulus_rhs = self.bigint_solver.get_modulus(rhs, func)?;
+        if modulus_lhs != modulus_rhs {
+            return Err(BlackBoxResolutionError::Failed(
+                func,
+                "moduli should be identical in BigInt operation".to_string(),
+            ));
+        }
         let id = self.create_bigint_id();
         self.bigint_solver.bigint_op(lhs, rhs, id, func)?;
         Ok(id)

diff --git a/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs b/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs
@@ -6,7 +6,7 @@ use acvm::{
 use crate::brillig::brillig_ir::{
     brillig_variable::{BrilligVariable, BrilligVector, SingleAddrVariable},
     debug_show::DebugToString,
-    BrilligBinaryOp, BrilligContext,
+    BrilligContext,
 };
 
 /// Transforms SSA's black box function calls into the corresponding brillig instructions
@@ -239,11 +239,10 @@ pub(crate) fn convert_black_box_call<F: AcirField + DebugToString>(
         BlackBoxFunc::RecursiveAggregation => {}
         BlackBoxFunc::BigIntAdd => {
             if let (
-                [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(rhs_modulus)],
-                [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(modulus_id)],
+                [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(_lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(_rhs_modulus)],
+                [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(_modulus_id)],
             ) = (function_arguments, function_results)
             {
-                prepare_bigint_output(brillig_context, lhs_modulus, rhs_modulus, modulus_id);
                 brillig_context.black_box_op_instruction(BlackBoxOp::BigIntAdd {
                     lhs: lhs.address,
                     rhs: rhs.address,
@@ -257,11 +256,10 @@ pub(crate) fn convert_black_box_call<F: AcirField + DebugToString>(
         }
         BlackBoxFunc::BigIntSub => {
             if let (
-                [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(rhs_modulus)],
-                [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(modulus_id)],
+                [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(_lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(_rhs_modulus)],
+                [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(_modulus_id)],
             ) = (function_arguments, function_results)
             {
-                prepare_bigint_output(brillig_context, lhs_modulus, rhs_modulus, modulus_id);
                 brillig_context.black_box_op_instruction(BlackBoxOp::BigIntSub {
                     lhs: lhs.address,
                     rhs: rhs.address,
@@ -275,11 +273,10 @@ pub(crate) fn convert_black_box_call<F: AcirField + DebugToString>(
         }
         BlackBoxFunc::BigIntMul => {
             if let (
-                [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(rhs_modulus)],
-                [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(modulus_id)],
+                [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(_lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(_rhs_modulus)],
+                [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(_modulus_id)],
             ) = (function_arguments, function_results)
             {
-                prepare_bigint_output(brillig_context, lhs_modulus, rhs_modulus, modulus_id);
                 brillig_context.black_box_op_instruction(BlackBoxOp::BigIntMul {
                     lhs: lhs.address,
                     rhs: rhs.address,
@@ -293,11 +290,10 @@ pub(crate) fn convert_black_box_call<F: AcirField + DebugToString>(
         }
         BlackBoxFunc::BigIntDiv => {
             if let (
-                [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(rhs_modulus)],
-                [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(modulus_id)],
+                [BrilligVariable::SingleAddr(lhs), BrilligVariable::SingleAddr(_lhs_modulus), BrilligVariable::SingleAddr(rhs), BrilligVariable::SingleAddr(_rhs_modulus)],
+                [BrilligVariable::SingleAddr(output), BrilligVariable::SingleAddr(_modulus_id)],
             ) = (function_arguments, function_results)
             {
-                prepare_bigint_output(brillig_context, lhs_modulus, rhs_modulus, modulus_id);
                 brillig_context.black_box_op_instruction(BlackBoxOp::BigIntDiv {
                     lhs: lhs.address,
                     rhs: rhs.address,
@@ -416,27 +412,3 @@ fn convert_array_or_vector<F: AcirField + DebugToString>(
         ),
     }
 }
-
-fn prepare_bigint_output<F: AcirField + DebugToString>(
-    brillig_context: &mut BrilligContext<F>,
-    lhs_modulus: &SingleAddrVariable,
-    rhs_modulus: &SingleAddrVariable,
-    modulus_id: &SingleAddrVariable,
-) {
-    // Check moduli
-    let condition = brillig_context.allocate_register();
-    let condition_adr = SingleAddrVariable { address: condition, bit_size: 1 };
-    brillig_context.binary_instruction(
-        *lhs_modulus,
-        *rhs_modulus,
-        condition_adr,
-        BrilligBinaryOp::Equals,
-    );
-    brillig_context.codegen_constrain(
-        condition_adr,
-        Some("moduli should be identical in BigInt operation".to_string()),
-    );
-    brillig_context.deallocate_register(condition);
-
-    brillig_context.mov_instruction(modulus_id.address, lhs_modulus.address);
-}
diff --git a/tooling/debugger/ignored-tests.txt b/tooling/debugger/ignored-tests.txt
@@ -1,6 +1,4 @@
-bigint
 brillig_references
-brillig_to_bytes_integration
 debug_logs
 fold_after_inlined_calls
 fold_basic
@@ -12,7 +10,5 @@ fold_fibonacci
 fold_numeric_generic_poseidon
 is_unconstrained
 macros
-modulus
 references
-regression_4709
-to_bytes_integration
+regression_4709