Merge branch 'master' into zpedro/devcontainer_docs

.github/workflows/test-js-packages.yml

@@ -319,7 +319,7 @@ jobs:
         run: yarn workspace @noir-lang/noir_wasm test:browser
 
   test-noir-codegen:
-    needs: [build-acvm-js, build-noirc-abi]
+    needs: [build-acvm-js, build-noirc-abi, build-nargo]
     name: noir_codegen
     runs-on: ubuntu-latest
     timeout-minutes: 30
@@ -328,6 +328,12 @@ jobs:
       - name: Checkout
         uses: actions/checkout@v4
 
+      - name: Download nargo binary
+        uses: actions/download-artifact@v3
+        with:
+          name: nargo
+          path: ./nargo
+
       - name: Download acvm_js package artifact
         uses: actions/download-artifact@v3
         with:
@@ -339,6 +345,14 @@ jobs:
         with:
           name: noirc_abi_wasm
           path: ./tooling/noirc_abi_wasm
+
+      - name: Set nargo on PATH
+        run: |
+          nargo_binary="${{ github.workspace }}/nargo/nargo"
+          chmod +x $nargo_binary
+          echo "$(dirname $nargo_binary)" >> $GITHUB_PATH
+          export PATH="$PATH:$(dirname $nargo_binary)"
+          nargo -V
 
       - name: Install Yarn dependencies
         uses: ./.github/actions/setup

acvm-repo/acir/src/circuit/mod.rs

@@ -99,7 +99,7 @@ impl FromStr for OpcodeLocation {
                     let brillig_index = parts[1].parse()?;
                     Ok(OpcodeLocation::Brillig { acir_index, brillig_index })
                 }
-                _ => unreachable!(),
+                _ => unreachable!("`OpcodeLocation` has too many components"),
             }
         }
 

acvm-repo/acvm/src/compiler/optimizers/redundant_range.rs

@@ -48,25 +48,51 @@ impl RangeOptimizer {
     /// only store the fact that we have constrained it to
     /// be 16 bits.
     fn collect_ranges(circuit: &Circuit) -> BTreeMap<Witness, u32> {
-        let mut witness_to_bit_sizes = BTreeMap::new();
+        let mut witness_to_bit_sizes: BTreeMap<Witness, u32> = BTreeMap::new();
 
         for opcode in &circuit.opcodes {
-            // Extract the witness index and number of bits,
-            // if it is a range constraint
-            let (witness, num_bits) = match extract_range_opcode(opcode) {
-                Some(func_inputs) => func_inputs,
-                None => continue,
+            let Some((witness, num_bits)) = (match opcode {
+                Opcode::AssertZero(expr) => {
+                    // If the opcode is constraining a witness to be equal to a value then it can be considered
+                    // as a range opcode for the number of bits required to hold that value.
+                    if expr.is_degree_one_univariate() {
+                        let (k, witness) = expr.linear_combinations[0];
+                        let constant = expr.q_c;
+                        let witness_value = -constant / k;
+
+                        if witness_value.is_zero() {
+                            Some((witness, 0))
+                        } else {
+                            // We subtract off 1 bit from the implied witness value to give the weakest range constraint
+                            // which would be stricter than the constraint imposed by this opcode.
+                            let implied_range_constraint_bits = witness_value.num_bits() - 1;
+                            Some((witness, implied_range_constraint_bits))
+                        }
+                    } else {
+                        None
+                    }
+                }
+
+
+                Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE {
+                    input: FunctionInput { witness, num_bits },
+                }) => {
+                    Some((*witness, *num_bits))
+                }
+
+                _ => None,
+            }) else {
+                continue;
             };
 
             // Check if the witness has already been recorded and if the witness
             // size is more than the current one, we replace it
-            let should_replace = match witness_to_bit_sizes.get(&witness).copied() {
-                Some(old_range_bits) => old_range_bits > num_bits,
-                None => true,
-            };
-            if should_replace {
-                witness_to_bit_sizes.insert(witness, num_bits);
-            }
+            witness_to_bit_sizes
+                .entry(witness)
+                .and_modify(|old_range_bits| {
+                    *old_range_bits = std::cmp::min(*old_range_bits, num_bits);
+                })
+                .or_insert(num_bits);
         }
         witness_to_bit_sizes
     }
@@ -116,16 +142,10 @@ impl RangeOptimizer {
 /// Extract the range opcode from the `Opcode` enum
 /// Returns None, if `Opcode` is not the range opcode.
 fn extract_range_opcode(opcode: &Opcode) -> Option<(Witness, u32)> {
-    // Range constraints are blackbox function calls
-    // so we first extract the function call
-    let func_call = match opcode {
-        acir::circuit::Opcode::BlackBoxFuncCall(func_call) => func_call,
-        _ => return None,
-    };
-
-    // Skip if it is not a range constraint
-    match func_call {
-        BlackBoxFuncCall::RANGE { input } => Some((input.witness, input.num_bits)),
+    match opcode {
+        Opcode::BlackBoxFuncCall(BlackBoxFuncCall::RANGE { input }) => {
+            Some((input.witness, input.num_bits))
+        }
         _ => None,
     }
 }
@@ -246,4 +266,17 @@ mod tests {
         let (optimized_circuit, _) = optimizer.replace_redundant_ranges(acir_opcode_positions);
         assert_eq!(optimized_circuit.opcodes.len(), 5);
     }
+
+    #[test]
+    fn constant_implied_ranges() {
+        // The optimizer should use knowledge about constant witness assignments to remove range opcodes.
+        let mut circuit = test_circuit(vec![(Witness(1), 16)]);
+
+        circuit.opcodes.push(Opcode::AssertZero(Witness(1).into()));
+        let acir_opcode_positions = circuit.opcodes.iter().enumerate().map(|(i, _)| i).collect();
+        let optimizer = RangeOptimizer::new(circuit);
+        let (optimized_circuit, _) = optimizer.replace_redundant_ranges(acir_opcode_positions);
+        assert_eq!(optimized_circuit.opcodes.len(), 1);
+        assert_eq!(optimized_circuit.opcodes[0], Opcode::AssertZero(Witness(1).into()));
+    }
 }

acvm-repo/acvm_js/src/execute.rs

@@ -61,8 +61,8 @@ pub async fn execute_circuit_with_black_box_solver(
     foreign_call_handler: ForeignCallHandler,
 ) -> Result<JsWitnessMap, Error> {
     console_error_panic_hook::set_once();
-    let circuit: Circuit =
-        Circuit::deserialize_circuit(&circuit).expect("Failed to deserialize circuit");
+    let circuit: Circuit = Circuit::deserialize_circuit(&circuit)
+        .map_err(|_| JsExecutionError::new("Failed to deserialize circuit. This is likely due to differing serialization formats between ACVM_JS and your compiler".to_string(), None))?;
 
     let mut acvm = ACVM::new(&solver.0, &circuit.opcodes, initial_witness.into());
 

acvm-repo/brillig_vm/src/black_box.rs

@@ -85,11 +85,7 @@ pub(crate) fn evaluate_black_box<Solver: BlackBoxFunctionSolver>(
             signature,
             result: result_register,
         } => {
-            let bb_func = match op {
-                BlackBoxOp::EcdsaSecp256k1 { .. } => BlackBoxFunc::EcdsaSecp256k1,
-                BlackBoxOp::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1,
-                _ => unreachable!(),
-            };
+            let bb_func = black_box_function_from_op(op);
 
             let public_key_x: [u8; 32] = to_u8_vec(read_heap_array(
                 memory,
@@ -124,7 +120,7 @@ pub(crate) fn evaluate_black_box<Solver: BlackBoxFunctionSolver>(
                 BlackBoxOp::EcdsaSecp256r1 { .. } => {
                     ecdsa_secp256r1_verify(&hashed_msg, &public_key_x, &public_key_y, &signature)?
                 }
-                _ => unreachable!(),
+                _ => unreachable!("`BlackBoxOp` is guarded against being a non-ecdsa operation"),
             };
 
             registers.set(*result_register, result.into());
@@ -178,6 +174,21 @@ pub(crate) fn evaluate_black_box<Solver: BlackBoxFunctionSolver>(
     }
 }
 
+fn black_box_function_from_op(op: &BlackBoxOp) -> BlackBoxFunc {
+    match op {
+        BlackBoxOp::Sha256 { .. } => BlackBoxFunc::SHA256,
+        BlackBoxOp::Blake2s { .. } => BlackBoxFunc::Blake2s,
+        BlackBoxOp::Keccak256 { .. } => BlackBoxFunc::Keccak256,
+        BlackBoxOp::HashToField128Security { .. } => BlackBoxFunc::HashToField128Security,
+        BlackBoxOp::EcdsaSecp256k1 { .. } => BlackBoxFunc::EcdsaSecp256k1,
+        BlackBoxOp::EcdsaSecp256r1 { .. } => BlackBoxFunc::EcdsaSecp256r1,
+        BlackBoxOp::SchnorrVerify { .. } => BlackBoxFunc::SchnorrVerify,
+        BlackBoxOp::PedersenCommitment { .. } => BlackBoxFunc::PedersenCommitment,
+        BlackBoxOp::PedersenHash { .. } => BlackBoxFunc::PedersenHash,
+        BlackBoxOp::FixedBaseScalarMul { .. } => BlackBoxFunc::FixedBaseScalarMul,
+    }
+}
+
 #[cfg(test)]
 mod test {
     use acir::brillig::BlackBoxOp;

compiler/noirc_driver/src/abi_gen.rs

@@ -33,7 +33,7 @@ pub(super) fn compute_function_abi(
 ) -> (Vec<AbiParameter>, Option<AbiType>) {
     let func_meta = context.def_interner.function_meta(func_id);
 
-    let (parameters, return_type) = func_meta.into_function_signature();
+    let (parameters, return_type) = func_meta.function_signature();
     let parameters = into_abi_params(context, parameters);
     let return_type = return_type.map(|typ| AbiType::from_type(context, &typ));
     (parameters, return_type)

compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs

@@ -92,6 +92,28 @@ pub(crate) fn convert_black_box_call(
                 )
             }
         }
+        BlackBoxFunc::EcdsaSecp256r1 => {
+            if let (
+                [BrilligVariable::BrilligArray(public_key_x), BrilligVariable::BrilligArray(public_key_y), BrilligVariable::BrilligArray(signature), message],
+                [BrilligVariable::Simple(result_register)],
+            ) = (function_arguments, function_results)
+            {
+                let message_hash_vector =
+                    convert_array_or_vector(brillig_context, message, bb_func);
+                brillig_context.black_box_op_instruction(BlackBoxOp::EcdsaSecp256r1 {
+                    hashed_msg: message_hash_vector.to_heap_vector(),
+                    public_key_x: public_key_x.to_heap_array(),
+                    public_key_y: public_key_y.to_heap_array(),
+                    signature: signature.to_heap_array(),
+                    result: *result_register,
+                });
+            } else {
+                unreachable!(
+                    "ICE: EcdsaSecp256r1 expects four array arguments and one register result"
+                )
+            }
+        }
+
         BlackBoxFunc::PedersenCommitment => {
             if let (
                 [message, BrilligVariable::Simple(domain_separator)],
@@ -160,7 +182,18 @@ pub(crate) fn convert_black_box_call(
                 )
             }
         }
-        _ => unimplemented!("ICE: Black box function {:?} is not implemented", bb_func),
+        BlackBoxFunc::AND => {
+            unreachable!("ICE: `BlackBoxFunc::AND` calls should be transformed into a `BinaryOp`")
+        }
+        BlackBoxFunc::XOR => {
+            unreachable!("ICE: `BlackBoxFunc::XOR` calls should be transformed into a `BinaryOp`")
+        }
+        BlackBoxFunc::RANGE => unreachable!(
+            "ICE: `BlackBoxFunc::RANGE` calls should be transformed into a `Instruction::Cast`"
+        ),
+        BlackBoxFunc::RecursiveAggregation => unimplemented!(
+            "ICE: `BlackBoxFunc::RecursiveAggregation` is not implemented by the Brillig VM"
+        ),
     }
 }
 

compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs

@@ -307,6 +307,7 @@ impl AcirContext {
             inverse_code,
             vec![AcirValue::Var(var, AcirType::field())],
             vec![AcirType::field()],
+            true,
         )?;
         let inverted_var = Self::expect_one_var(results);
 
@@ -708,6 +709,7 @@ impl AcirContext {
                     AcirValue::Var(rhs, AcirType::unsigned(bit_size)),
                 ],
                 vec![AcirType::unsigned(max_q_bits), AcirType::unsigned(max_rhs_bits)],
+                true,
             )?
             .try_into()
             .expect("quotient only returns two values");
@@ -1310,6 +1312,7 @@ impl AcirContext {
         generated_brillig: GeneratedBrillig,
         inputs: Vec<AcirValue>,
         outputs: Vec<AcirType>,
+        attempt_execution: bool,
     ) -> Result<Vec<AcirValue>, InternalError> {
         let b_inputs = try_vecmap(inputs, |i| match i {
             AcirValue::Var(var, _) => Ok(BrilligInputs::Single(self.var_to_expression(var)?)),
@@ -1329,10 +1332,15 @@ impl AcirContext {
 
         // Optimistically try executing the brillig now, if we can complete execution they just return the results.
         // This is a temporary measure pending SSA optimizations being applied to Brillig which would remove constant-input opcodes (See #2066)
-        if let Some(brillig_outputs) =
-            self.execute_brillig(&generated_brillig.byte_code, &b_inputs, &outputs)
-        {
-            return Ok(brillig_outputs);
+        //
+        // We do _not_ want to do this in the situation where the `main` function is unconstrained, as if execution succeeds
+        // the entire program will be replaced with witness constraints to its outputs.
+        if attempt_execution {
+            if let Some(brillig_outputs) =
+                self.execute_brillig(&generated_brillig.byte_code, &b_inputs, &outputs)
+            {
+                return Ok(brillig_outputs);
+            }
         }
 
         // Otherwise we must generate ACIR for it and execute at runtime.