diff --git a/.noir-sync-commit b/.noir-sync-commit index ae21821926e..9eafde097d5 100644 --- a/.noir-sync-commit +++ b/.noir-sync-commit @@ -1 +1 @@ -7cc8dbfe670aeaf9ec84f30265417658f5465d11 +8bb3908281d531160db7d7898c67fb2647792e6e diff --git a/noir/noir-repo/.github/workflows/reports.yml b/noir/noir-repo/.github/workflows/reports.yml index 6c93156eddd..811f4d6f37b 100644 --- a/noir/noir-repo/.github/workflows/reports.yml +++ b/noir/noir-repo/.github/workflows/reports.yml @@ -299,7 +299,7 @@ jobs: fail-fast: false matrix: include: - - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts, is_library: true } + # - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-contracts, is_library: true } - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-inner, take_average: true } - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-tail, take_average: true } - project: { repo: AztecProtocol/aztec-packages, path: noir-projects/noir-protocol-circuits/crates/private-kernel-reset, take_average: true } diff --git a/noir/noir-repo/.github/workflows/test-js-packages.yml b/noir/noir-repo/.github/workflows/test-js-packages.yml index 5222b667c2a..2782b1aa44a 100644 --- a/noir/noir-repo/.github/workflows/test-js-packages.yml +++ b/noir/noir-repo/.github/workflows/test-js-packages.yml @@ -597,7 +597,7 @@ jobs: - name: Run nargo test working-directory: ./test-repo/${{ matrix.project.path }} run: | - nargo test --silence-warnings --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }} | tee ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl + nargo test --silence-warnings --pedantic-solving --skip-brillig-constraints-check --format json ${{ matrix.project.nargo_args }} | tee ${{ github.workspace }}/noir-repo/.github/critical_libraries_status/${{ matrix.project.repo }}/${{ matrix.project.path }}.actual.jsonl env: NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index 889c5c1ec6f..4d3e2cbd86d 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -72,6 +72,7 @@ dependencies = [ "keccak", "libaes", "num-bigint", + "num-prime", "p256", "proptest", "sha2", @@ -1761,6 +1762,12 @@ version = "1.0.7" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3f9eec918d3f24069decb9af1554cad7c880e2da24a9afd88aca000531ab82c1" +[[package]] +name = "foldhash" +version = "0.1.3" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f81ec6369c545a7d40e4589b5597581fa1c441fe1cce96dd1de43159910a36a2" + [[package]] name = "form_urlencoded" version = "1.2.1" @@ -2056,6 +2063,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a9bfc1af68b1726ea47d3d5109de126281def866b33970e10fbab11b5dafab3" dependencies = [ "allocator-api2", + "equivalent", + "foldhash", ] [[package]] @@ -2846,6 +2855,15 @@ dependencies = [ "fid-rs", ] +[[package]] +name = "lru" +version = "0.12.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "234cf4f4a04dc1f57e24b96cc0cd600cf2af460d4161ac5ecdd0af8e1f3b2a38" +dependencies = [ + "hashbrown 0.15.1", +] + [[package]] name = "lsp-types" version = "0.88.0" @@ -3447,6 +3465,7 @@ checksum = "a5e44f723f1133c9deac646763579fdb3ac745e418f2a7af9cd0c431da1f20b9" dependencies = [ "num-integer", "num-traits", + "rand", ] [[package]] @@ -3474,6 +3493,33 @@ dependencies = [ "num-traits", ] +[[package]] +name = "num-modular" +version = "0.5.1" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "64a5fe11d4135c3bcdf3a95b18b194afa9608a5f6ff034f5d857bc9a27fb0119" +dependencies = [ + "num-bigint", + "num-integer", + "num-traits", +] + +[[package]] +name = "num-prime" +version = "0.4.4" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "e238432a7881ec7164503ccc516c014bf009be7984cde1ba56837862543bdec3" +dependencies = [ + "bitvec", + "either", + "lru", + "num-bigint", + "num-integer", + "num-modular", + "num-traits", + "rand", +] + [[package]] name = "num-traits" version = "0.2.19" diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/bigint.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/bigint.rs index 7ce34dbc6fe..eb5a6f81e7d 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/bigint.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/bigint.rs @@ -12,12 +12,16 @@ use crate::pwg::OpcodeResolutionError; /// - When it encounters a bigint operation opcode, it performs the operation on the stored values /// and store the result using the provided ID. /// - When it gets a to_bytes opcode, it simply looks up the value and resolves the output witness accordingly. -#[derive(Default)] pub(crate) struct AcvmBigIntSolver { bigint_solver: BigIntSolver, } impl AcvmBigIntSolver { + pub(crate) fn with_pedantic_solving(pedantic_solving: bool) -> AcvmBigIntSolver { + let bigint_solver = BigIntSolver::with_pedantic_solving(pedantic_solving); + AcvmBigIntSolver { bigint_solver } + } + pub(crate) fn bigint_from_bytes( &mut self, inputs: &[FunctionInput], @@ -39,6 +43,9 @@ impl AcvmBigIntSolver { outputs: &[Witness], initial_witness: &mut WitnessMap, ) -> Result<(), OpcodeResolutionError> { + if self.bigint_solver.pedantic_solving() && outputs.len() != 32 { + panic!("--pedantic-solving: bigint_to_bytes: outputs.len() != 32: {}", outputs.len()); + } let mut bytes = self.bigint_solver.bigint_to_bytes(input)?; while bytes.len() < outputs.len() { bytes.push(0); 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 index 8468b0ca27a..3fa72d9b215 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/logic.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/logic.rs @@ -14,13 +14,14 @@ pub(super) fn and( lhs: &FunctionInput, rhs: &FunctionInput, output: &Witness, + pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { assert_eq!( lhs.num_bits(), rhs.num_bits(), "number of bits specified for each input must be the same" ); - solve_logic_opcode(initial_witness, lhs, rhs, *output, |left, right| { + solve_logic_opcode(initial_witness, lhs, rhs, *output, pedantic_solving, |left, right| { bit_and(left, right, lhs.num_bits()) }) } @@ -32,13 +33,14 @@ pub(super) fn xor( lhs: &FunctionInput, rhs: &FunctionInput, output: &Witness, + pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { assert_eq!( lhs.num_bits(), rhs.num_bits(), "number of bits specified for each input must be the same" ); - solve_logic_opcode(initial_witness, lhs, rhs, *output, |left, right| { + solve_logic_opcode(initial_witness, lhs, rhs, *output, pedantic_solving, |left, right| { bit_xor(left, right, lhs.num_bits()) }) } @@ -49,11 +51,13 @@ fn solve_logic_opcode( a: &FunctionInput, b: &FunctionInput, result: Witness, + pedantic_solving: bool, logic_op: impl Fn(F, F) -> F, ) -> Result<(), OpcodeResolutionError> { - // TODO(https://github.com/noir-lang/noir/issues/5985): re-enable these once we figure out how to combine these with existing + // TODO(https://github.com/noir-lang/noir/issues/5985): re-enable these by + // default once we figure out how to combine these with existing // noirc_frontend/noirc_evaluator overflow error messages - let skip_bitsize_checks = true; + let skip_bitsize_checks = !pedantic_solving; let w_l_value = input_to_value(initial_witness, *a, skip_bitsize_checks)?; let w_r_value = input_to_value(initial_witness, *b, skip_bitsize_checks)?; let assignment = logic_op(w_l_value, w_r_value); diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs index 5137b18179b..2f10e333c24 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -76,9 +76,15 @@ pub(crate) fn solve( BlackBoxFuncCall::AES128Encrypt { inputs, iv, key, outputs } => { solve_aes128_encryption_opcode(initial_witness, inputs, iv, key, outputs) } - BlackBoxFuncCall::AND { lhs, rhs, output } => and(initial_witness, lhs, rhs, output), - BlackBoxFuncCall::XOR { lhs, rhs, output } => xor(initial_witness, lhs, rhs, output), - BlackBoxFuncCall::RANGE { input } => solve_range_opcode(initial_witness, input), + BlackBoxFuncCall::AND { lhs, rhs, output } => { + and(initial_witness, lhs, rhs, output, backend.pedantic_solving()) + } + BlackBoxFuncCall::XOR { lhs, rhs, output } => { + xor(initial_witness, lhs, rhs, output, backend.pedantic_solving()) + } + BlackBoxFuncCall::RANGE { input } => { + solve_range_opcode(initial_witness, input, backend.pedantic_solving()) + } BlackBoxFuncCall::Blake2s { inputs, outputs } => { solve_generic_256_hash_opcode(initial_witness, inputs, None, outputs, blake2s) } diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/range.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/range.rs index 4f9be14360e..d7083e4b9c0 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/range.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/range.rs @@ -7,10 +7,11 @@ use acir::{circuit::opcodes::FunctionInput, native_types::WitnessMap, AcirField} pub(crate) fn solve_range_opcode( initial_witness: &WitnessMap, input: &FunctionInput, + pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { // TODO(https://github.com/noir-lang/noir/issues/5985): - // re-enable bitsize checks - let skip_bitsize_checks = true; + // re-enable bitsize checks by default + let skip_bitsize_checks = !pedantic_solving; let w_value = input_to_value(initial_witness, *input, skip_bitsize_checks)?; if w_value.num_bits() > input.num_bits() { return Err(OpcodeResolutionError::UnsatisfiedConstrain { diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/memory_op.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/memory_op.rs index a9ed7f5d15b..1a6519d19c5 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/memory_op.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/memory_op.rs @@ -66,6 +66,7 @@ impl MemoryOpSolver { op: &MemOp, initial_witness: &mut WitnessMap, predicate: &Option>, + pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { let operation = get_value(&op.operation, initial_witness)?; @@ -83,7 +84,9 @@ impl MemoryOpSolver { let is_read_operation = operation.is_zero(); // Fetch whether or not the predicate is false (e.g. equal to zero) - let skip_operation = is_predicate_false(initial_witness, predicate)?; + let opcode_location = ErrorLocation::Unresolved; + let skip_operation = + is_predicate_false(initial_witness, predicate, pedantic_solving, &opcode_location)?; if is_read_operation { // `value_read = arr[memory_index]` @@ -131,6 +134,9 @@ mod tests { use super::MemoryOpSolver; + // use pedantic_solving for tests + const PEDANTIC_SOLVING: bool = true; + #[test] fn test_solver() { let mut initial_witness = WitnessMap::from(BTreeMap::from_iter([ @@ -150,7 +156,9 @@ mod tests { block_solver.init(&init, &initial_witness).unwrap(); for op in trace { - block_solver.solve_memory_op(&op, &mut initial_witness, &None).unwrap(); + block_solver + .solve_memory_op(&op, &mut initial_witness, &None, PEDANTIC_SOLVING) + .unwrap(); } assert_eq!(initial_witness[&Witness(4)], FieldElement::from(2u128)); @@ -175,7 +183,9 @@ mod tests { let mut err = None; for op in invalid_trace { if err.is_none() { - err = block_solver.solve_memory_op(&op, &mut initial_witness, &None).err(); + err = block_solver + .solve_memory_op(&op, &mut initial_witness, &None, PEDANTIC_SOLVING) + .err(); } } @@ -209,7 +219,12 @@ mod tests { for op in invalid_trace { if err.is_none() { err = block_solver - .solve_memory_op(&op, &mut initial_witness, &Some(Expression::zero())) + .solve_memory_op( + &op, + &mut initial_witness, + &Some(Expression::zero()), + PEDANTIC_SOLVING, + ) .err(); } } @@ -241,7 +256,12 @@ mod tests { for op in invalid_trace { if err.is_none() { err = block_solver - .solve_memory_op(&op, &mut initial_witness, &Some(Expression::zero())) + .solve_memory_op( + &op, + &mut initial_witness, + &Some(Expression::zero()), + PEDANTIC_SOLVING, + ) .err(); } } diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs index f9188cca700..104a15c17cc 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs @@ -144,6 +144,8 @@ pub enum OpcodeResolutionError { AcirMainCallAttempted { opcode_location: ErrorLocation }, #[error("{results_size:?} result values were provided for {outputs_size:?} call output witnesses, most likely due to bad ACIR codegen")] AcirCallOutputsMismatch { opcode_location: ErrorLocation, results_size: u32, outputs_size: u32 }, + #[error("(--pedantic): Predicates are expected to be 0 or 1, but found: {pred_value}")] + PredicateLargerThanOne { opcode_location: ErrorLocation, pred_value: F }, } impl From for OpcodeResolutionError { @@ -218,11 +220,12 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { assertion_payloads: &'a [(OpcodeLocation, AssertionPayload)], ) -> Self { let status = if opcodes.is_empty() { ACVMStatus::Solved } else { ACVMStatus::InProgress }; + let bigint_solver = AcvmBigIntSolver::with_pedantic_solving(backend.pedantic_solving()); ACVM { status, backend, block_solvers: HashMap::default(), - bigint_solver: AcvmBigIntSolver::default(), + bigint_solver, opcodes, instruction_pointer: 0, witness_map: initial_witness, @@ -373,7 +376,12 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { } Opcode::MemoryOp { block_id, op, predicate } => { let solver = self.block_solvers.entry(*block_id).or_default(); - solver.solve_memory_op(op, &mut self.witness_map, predicate) + solver.solve_memory_op( + op, + &mut self.witness_map, + predicate, + self.backend.pedantic_solving(), + ) } Opcode::BrilligCall { .. } => match self.solve_brillig_call_opcode() { Ok(Some(foreign_call)) => return self.wait_for_foreign_call(foreign_call), @@ -477,7 +485,14 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { unreachable!("Not executing a BrilligCall opcode"); }; - if is_predicate_false(&self.witness_map, predicate)? { + let opcode_location = + ErrorLocation::Resolved(OpcodeLocation::Acir(self.instruction_pointer())); + if is_predicate_false( + &self.witness_map, + predicate, + self.backend.pedantic_solving(), + &opcode_location, + )? { return BrilligSolver::::zero_out_brillig_outputs(&mut self.witness_map, outputs) .map(|_| None); } @@ -545,8 +560,15 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { return StepResult::Status(self.solve_opcode()); }; + let opcode_location = + ErrorLocation::Resolved(OpcodeLocation::Acir(self.instruction_pointer())); let witness = &mut self.witness_map; - let should_skip = match is_predicate_false(witness, predicate) { + let should_skip = match is_predicate_false( + witness, + predicate, + self.backend.pedantic_solving(), + &opcode_location, + ) { Ok(result) => result, Err(err) => return StepResult::Status(self.handle_opcode_resolution(Err(err))), }; @@ -587,15 +609,19 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { else { unreachable!("Not executing a Call opcode"); }; + + let opcode_location = + ErrorLocation::Resolved(OpcodeLocation::Acir(self.instruction_pointer())); if *id == AcirFunctionId(0) { - return Err(OpcodeResolutionError::AcirMainCallAttempted { - opcode_location: ErrorLocation::Resolved(OpcodeLocation::Acir( - self.instruction_pointer(), - )), - }); + return Err(OpcodeResolutionError::AcirMainCallAttempted { opcode_location }); } - if is_predicate_false(&self.witness_map, predicate)? { + if is_predicate_false( + &self.witness_map, + predicate, + self.backend.pedantic_solving(), + &opcode_location, + )? { // Zero out the outputs if we have a false predicate for output in outputs { insert_value(output, F::zero(), &mut self.witness_map)?; @@ -615,9 +641,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { let result_values = &self.acir_call_results[self.acir_call_counter]; if outputs.len() != result_values.len() { return Err(OpcodeResolutionError::AcirCallOutputsMismatch { - opcode_location: ErrorLocation::Resolved(OpcodeLocation::Acir( - self.instruction_pointer(), - )), + opcode_location, results_size: result_values.len() as u32, outputs_size: outputs.len() as u32, }); @@ -736,9 +760,25 @@ fn any_witness_from_expression(expr: &Expression) -> Option { pub(crate) fn is_predicate_false( witness: &WitnessMap, predicate: &Option>, + pedantic_solving: bool, + opcode_location: &ErrorLocation, ) -> Result> { match predicate { - Some(pred) => get_value(pred, witness).map(|pred_value| pred_value.is_zero()), + Some(pred) => { + let pred_value = get_value(pred, witness)?; + let predicate_is_false = pred_value.is_zero(); + if pedantic_solving { + // We expect that the predicate should resolve to either 0 or 1. + if !predicate_is_false && !pred_value.is_one() { + let opcode_location = *opcode_location; + return Err(OpcodeResolutionError::PredicateLargerThanOne { + opcode_location, + pred_value, + }); + } + } + Ok(predicate_is_false) + } // If the predicate is `None`, then we treat it as an unconditional `true` None => Ok(false), } diff --git a/noir/noir-repo/acvm-repo/acvm/tests/solver.rs b/noir/noir-repo/acvm-repo/acvm/tests/solver.rs index 5998b5e6d05..2fd61050377 100644 --- a/noir/noir-repo/acvm-repo/acvm/tests/solver.rs +++ b/noir/noir-repo/acvm-repo/acvm/tests/solver.rs @@ -15,7 +15,7 @@ use acir::{ }; use acvm::pwg::{ACVMStatus, ErrorLocation, ForeignCallWaitInfo, OpcodeResolutionError, ACVM}; -use acvm_blackbox_solver::StubbedBlackBoxSolver; +use acvm_blackbox_solver::{BigIntSolver, StubbedBlackBoxSolver}; use bn254_blackbox_solver::{field_from_hex, Bn254BlackBoxSolver, POSEIDON2_CONFIG}; use brillig_vm::brillig::HeapValueType; @@ -27,6 +27,7 @@ use proptest::sample::select; #[test] fn bls12_381_circuit() { + let solver = StubbedBlackBoxSolver::default(); type Bls12FieldElement = GenericFieldElement; let addition = Opcode::AssertZero(Expression { @@ -46,7 +47,7 @@ fn bls12_381_circuit() { ]) .into(); - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &[], &[]); + let mut acvm = ACVM::new(&solver, &opcodes, witness_assignments, &[], &[]); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved, "should be fully solved"); @@ -59,6 +60,7 @@ fn bls12_381_circuit() { #[test] fn inversion_brillig_oracle_equivalence() { + let solver = StubbedBlackBoxSolver::default(); // Opcodes below describe the following: // fn main(x : Field, y : pub Field) { // let z = x + y; @@ -168,13 +170,7 @@ fn inversion_brillig_oracle_equivalence() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, - &opcodes, - witness_assignments, - &unconstrained_functions, - &[], - ); + let mut acvm = ACVM::new(&solver, &opcodes, witness_assignments, &unconstrained_functions, &[]); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -203,6 +199,7 @@ fn inversion_brillig_oracle_equivalence() { #[test] fn double_inversion_brillig_oracle() { + let solver = StubbedBlackBoxSolver::default(); // Opcodes below describe the following: // fn main(x : Field, y : pub Field) { // let z = x + y; @@ -334,13 +331,7 @@ fn double_inversion_brillig_oracle() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, - &opcodes, - witness_assignments, - &unconstrained_functions, - &[], - ); + let mut acvm = ACVM::new(&solver, &opcodes, witness_assignments, &unconstrained_functions, &[]); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -387,6 +378,7 @@ fn double_inversion_brillig_oracle() { #[test] fn oracle_dependent_execution() { + let solver = StubbedBlackBoxSolver::default(); // This test ensures that we properly track the list of opcodes which still need to be resolved // across any brillig foreign calls we may have to perform. // @@ -491,13 +483,7 @@ fn oracle_dependent_execution() { let witness_assignments = BTreeMap::from([(w_x, FieldElement::from(2u128)), (w_y, FieldElement::from(2u128))]).into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, - &opcodes, - witness_assignments, - &unconstrained_functions, - &[], - ); + let mut acvm = ACVM::new(&solver, &opcodes, witness_assignments, &unconstrained_functions, &[]); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -543,6 +529,7 @@ fn oracle_dependent_execution() { #[test] fn brillig_oracle_predicate() { + let solver = StubbedBlackBoxSolver::default(); let fe_0 = FieldElement::zero(); let fe_1 = FieldElement::one(); let w_x = Witness(1); @@ -613,13 +600,7 @@ fn brillig_oracle_predicate() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, - &opcodes, - witness_assignments, - &unconstrained_functions, - &[], - ); + let mut acvm = ACVM::new(&solver, &opcodes, witness_assignments, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved, "should be fully solved"); @@ -629,6 +610,7 @@ fn brillig_oracle_predicate() { #[test] fn unsatisfied_opcode_resolved() { + let solver = StubbedBlackBoxSolver::default(); let a = Witness(0); let b = Witness(1); let c = Witness(2); @@ -654,8 +636,7 @@ fn unsatisfied_opcode_resolved() { let opcodes = vec![Opcode::AssertZero(opcode_a)]; let unconstrained_functions = vec![]; - let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, values, &unconstrained_functions, &[]); + let mut acvm = ACVM::new(&solver, &opcodes, values, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!( solver_status, @@ -669,6 +650,7 @@ fn unsatisfied_opcode_resolved() { #[test] fn unsatisfied_opcode_resolved_brillig() { + let solver = StubbedBlackBoxSolver::default(); let a = Witness(0); let b = Witness(1); let c = Witness(2); @@ -778,8 +760,7 @@ fn unsatisfied_opcode_resolved_brillig() { Opcode::AssertZero(opcode_a), ]; let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, values, &unconstrained_functions, &[]); + let mut acvm = ACVM::new(&solver, &opcodes, values, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!( solver_status, @@ -794,6 +775,8 @@ fn unsatisfied_opcode_resolved_brillig() { #[test] fn memory_operations() { + let solver = StubbedBlackBoxSolver::default(); + let initial_witness = WitnessMap::from(BTreeMap::from_iter([ (Witness(1), FieldElement::from(1u128)), (Witness(2), FieldElement::from(2u128)), @@ -828,8 +811,7 @@ fn memory_operations() { let opcodes = vec![init, read_op, expression]; let unconstrained_functions = vec![]; - let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, initial_witness, &unconstrained_functions, &[]); + let mut acvm = ACVM::new(&solver, &opcodes, initial_witness, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); @@ -837,39 +819,6 @@ fn memory_operations() { assert_eq!(witness_map[&Witness(8)], FieldElement::from(6u128)); } -fn allowed_bigint_moduli() -> Vec> { - let bn254_fq: Vec = vec![ - 0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, 0x81, - 0x97, 0x5d, 0x58, 0x81, 0x81, 0xb6, 0x45, 0x50, 0xb8, 0x29, 0xa0, 0x31, 0xe1, 0x72, 0x4e, - 0x64, 0x30, - ]; - let bn254_fr: Vec = vec![ - 1, 0, 0, 240, 147, 245, 225, 67, 145, 112, 185, 121, 72, 232, 51, 40, 93, 88, 129, 129, - 182, 69, 80, 184, 41, 160, 49, 225, 114, 78, 100, 48, - ]; - let secpk1_fr: Vec = vec![ - 0x41, 0x41, 0x36, 0xD0, 0x8C, 0x5E, 0xD2, 0xBF, 0x3B, 0xA0, 0x48, 0xAF, 0xE6, 0xDC, 0xAE, - 0xBA, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, - 0xFF, 0xFF, - ]; - let secpk1_fq: Vec = vec![ - 0x2F, 0xFC, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, - 0xFF, 0xFF, - ]; - let secpr1_fq: Vec = vec![ - 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, 0x00, - 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0xFF, 0xFF, - 0xFF, 0xFF, - ]; - let secpr1_fr: Vec = vec![ - 81, 37, 99, 252, 194, 202, 185, 243, 132, 158, 23, 167, 173, 250, 230, 188, 255, 255, 255, - 255, 255, 255, 255, 255, 0, 0, 0, 0, 255, 255, 255, 255, - ]; - - vec![bn254_fq, bn254_fr, secpk1_fr, secpk1_fq, secpr1_fq, secpr1_fr] -} - /// Whether to use a FunctionInput::constant or FunctionInput::witness: /// /// (value, use_constant) @@ -916,6 +865,7 @@ fn solve_array_input_blackbox_call( inputs: Vec, num_outputs: usize, num_bits: Option, + pedantic_solving: bool, f: F, ) -> Result, OpcodeResolutionError> where @@ -923,6 +873,7 @@ where (Vec>, Vec), ) -> Result, OpcodeResolutionError>, { + let solver = Bn254BlackBoxSolver(pedantic_solving); let initial_witness_vec: Vec<_> = inputs.iter().enumerate().map(|(i, (x, _))| (Witness(i as u32), *x)).collect(); let outputs: Vec<_> = (0..num_outputs) @@ -934,8 +885,7 @@ where let op = Opcode::BlackBoxFuncCall(f((inputs.clone(), outputs.clone()))?); let opcodes = vec![op]; let unconstrained_functions = vec![]; - let mut acvm = - ACVM::new(&Bn254BlackBoxSolver, &opcodes, initial_witness, &unconstrained_functions, &[]); + let mut acvm = ACVM::new(&solver, &opcodes, initial_witness, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); @@ -947,7 +897,7 @@ where } prop_compose! { - fn bigint_with_modulus()(modulus in select(allowed_bigint_moduli())) + fn bigint_with_modulus()(modulus in select(BigIntSolver::allowed_bigint_moduli())) (inputs in proptest::collection::vec(any::<(u8, bool)>(), modulus.len()), modulus in Just(modulus)) -> (Vec, Vec) { let inputs = inputs.into_iter().zip(modulus.iter()).map(|((input, use_constant), modulus_byte)| { @@ -1028,7 +978,9 @@ fn bigint_solve_binary_op_opt( modulus: Vec, lhs: Vec, rhs: Vec, + pedantic_solving: bool, ) -> Result, OpcodeResolutionError> { + let solver = StubbedBlackBoxSolver(pedantic_solving); let initial_witness_vec: Vec<_> = lhs .iter() .chain(rhs.iter()) @@ -1070,8 +1022,7 @@ fn bigint_solve_binary_op_opt( opcodes.push(bigint_to_op); let unconstrained_functions = vec![]; - let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, initial_witness, &unconstrained_functions, &[]); + let mut acvm = ACVM::new(&solver, &opcodes, initial_witness, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); @@ -1094,6 +1045,7 @@ fn solve_blackbox_func_call( lhs: (FieldElement, bool), // if false, use a Witness rhs: (FieldElement, bool), // if false, use a Witness ) -> Result> { + let solver = StubbedBlackBoxSolver::default(); let (lhs, lhs_constant) = lhs; let (rhs, rhs_constant) = rhs; @@ -1113,8 +1065,7 @@ fn solve_blackbox_func_call( let op = Opcode::BlackBoxFuncCall(blackbox_func_call(lhs_opt, rhs_opt)?); let opcodes = vec![op]; let unconstrained_functions = vec![]; - let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, initial_witness, &unconstrained_functions, &[]); + let mut acvm = ACVM::new(&solver, &opcodes, initial_witness, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); @@ -1230,10 +1181,12 @@ where fn run_both_poseidon2_permutations( inputs: Vec, ) -> Result<(Vec, Vec), OpcodeResolutionError> { + let pedantic_solving = true; let result = solve_array_input_blackbox_call( inputs.clone(), inputs.len(), None, + pedantic_solving, poseidon2_permutation_op, )?; @@ -1283,8 +1236,9 @@ fn bigint_solve_binary_op( modulus: Vec, lhs: Vec, rhs: Vec, + pedantic_solving: bool, ) -> Vec { - bigint_solve_binary_op_opt(Some(middle_op), modulus, lhs, rhs).unwrap() + bigint_solve_binary_op_opt(Some(middle_op), modulus, lhs, rhs, pedantic_solving).unwrap() } // Using the given BigInt modulus, solve the following circuit: @@ -1294,8 +1248,38 @@ fn bigint_solve_binary_op( fn bigint_solve_from_to_le_bytes( modulus: Vec, inputs: Vec, + pedantic_solving: bool, ) -> Vec { - bigint_solve_binary_op_opt(None, modulus, inputs, vec![]).unwrap() + bigint_solve_binary_op_opt(None, modulus, inputs, vec![], pedantic_solving).unwrap() +} + +// Test bigint_solve_from_to_le_bytes with a guaranteed-invalid modulus +// and optional pedantic_solving +fn bigint_from_to_le_bytes_disallowed_modulus_helper( + modulus: &mut Vec, + patch_location: usize, + patch_amount: u8, + zero_or_ones_constant: bool, + use_constant: bool, + pedantic_solving: bool, +) -> (Vec, Vec) { + let allowed_moduli: HashSet> = + BigIntSolver::allowed_bigint_moduli().into_iter().collect(); + let mut patch_location = patch_location % modulus.len(); + let patch_amount = patch_amount.clamp(1, u8::MAX); + while allowed_moduli.contains(modulus) { + modulus[patch_location] = patch_amount.wrapping_add(modulus[patch_location]); + patch_location += 1; + patch_location %= modulus.len(); + } + + let zero_function_input = + if zero_or_ones_constant { FieldElement::zero() } else { FieldElement::one() }; + let zero: Vec<_> = modulus.iter().map(|_| (zero_function_input, use_constant)).collect(); + let expected_results: Vec<_> = drop_use_constant(&zero); + let results = bigint_solve_from_to_le_bytes(modulus.clone(), zero, pedantic_solving); + + (results, expected_results) } fn function_input_from_option( @@ -1388,6 +1372,7 @@ fn prop_assert_injective( distinct_inputs: Vec, num_outputs: usize, num_bits: Option, + pedantic_solving: bool, op: F, ) -> (bool, String) where @@ -1399,11 +1384,22 @@ where { let equal_inputs = drop_use_constant_eq(&inputs, &distinct_inputs); let message = format!("not injective:\n{:?}\n{:?}", &inputs, &distinct_inputs); - let outputs_not_equal = - solve_array_input_blackbox_call(inputs, num_outputs, num_bits, op.clone()) - .expect("injectivity test operations to have valid input") - != solve_array_input_blackbox_call(distinct_inputs, num_outputs, num_bits, op) - .expect("injectivity test operations to have valid input"); + let outputs_not_equal = solve_array_input_blackbox_call( + inputs, + num_outputs, + num_bits, + pedantic_solving, + op.clone(), + ) + .expect("injectivity test operations to have valid input") + != solve_array_input_blackbox_call( + distinct_inputs, + num_outputs, + num_bits, + pedantic_solving, + op, + ) + .expect("injectivity test operations to have valid input"); (equal_inputs || outputs_not_equal, message) } @@ -1483,10 +1479,12 @@ fn poseidon2_permutation_zeroes() { #[test] fn sha256_compression_zeros() { + let pedantic_solving = true; let results = solve_array_input_blackbox_call( [(FieldElement::zero(), false); 24].into(), 8, None, + pedantic_solving, sha256_compression_op, ); let expected_results: Vec<_> = vec![ @@ -1501,7 +1499,8 @@ fn sha256_compression_zeros() { #[test] fn blake2s_zeros() { - let results = solve_array_input_blackbox_call(vec![], 32, None, blake2s_op); + let pedantic_solving = true; + let results = solve_array_input_blackbox_call(vec![], 32, None, pedantic_solving, blake2s_op); let expected_results: Vec<_> = vec![ 105, 33, 122, 48, 121, 144, 128, 148, 225, 17, 33, 208, 66, 53, 74, 124, 31, 85, 182, 72, 44, 161, 165, 30, 27, 37, 13, 253, 30, 208, 238, 249, @@ -1514,7 +1513,8 @@ fn blake2s_zeros() { #[test] fn blake3_zeros() { - let results = solve_array_input_blackbox_call(vec![], 32, None, blake3_op); + let pedantic_solving = true; + let results = solve_array_input_blackbox_call(vec![], 32, None, pedantic_solving, blake3_op); let expected_results: Vec<_> = vec![ 175, 19, 73, 185, 245, 249, 161, 166, 160, 64, 77, 234, 54, 220, 201, 73, 155, 203, 37, 201, 173, 193, 18, 183, 204, 154, 147, 202, 228, 31, 50, 98, @@ -1527,10 +1527,12 @@ fn blake3_zeros() { #[test] fn keccakf1600_zeros() { + let pedantic_solving = true; let results = solve_array_input_blackbox_call( [(FieldElement::zero(), false); 25].into(), 25, Some(64), + pedantic_solving, keccakf1600_op, ); let expected_results: Vec<_> = vec![ @@ -1646,7 +1648,8 @@ proptest! { fn sha256_compression_injective(inputs_distinct_inputs in any_distinct_inputs(None, 24, 24)) { let (inputs, distinct_inputs) = inputs_distinct_inputs; if inputs.len() == 24 && distinct_inputs.len() == 24 { - let (result, message) = prop_assert_injective(inputs, distinct_inputs, 8, None, sha256_compression_op); + let pedantic_solving = true; + let (result, message) = prop_assert_injective(inputs, distinct_inputs, 8, None, pedantic_solving, sha256_compression_op); prop_assert!(result, "{}", message); } } @@ -1654,14 +1657,16 @@ proptest! { #[test] fn blake2s_injective(inputs_distinct_inputs in any_distinct_inputs(None, 0, 32)) { let (inputs, distinct_inputs) = inputs_distinct_inputs; - let (result, message) = prop_assert_injective(inputs, distinct_inputs, 32, None, blake2s_op); + let pedantic_solving = true; + let (result, message) = prop_assert_injective(inputs, distinct_inputs, 32, None, pedantic_solving, blake2s_op); prop_assert!(result, "{}", message); } #[test] fn blake3_injective(inputs_distinct_inputs in any_distinct_inputs(None, 0, 32)) { let (inputs, distinct_inputs) = inputs_distinct_inputs; - let (result, message) = prop_assert_injective(inputs, distinct_inputs, 32, None, blake3_op); + let pedantic_solving = true; + let (result, message) = prop_assert_injective(inputs, distinct_inputs, 32, None, pedantic_solving, blake3_op); prop_assert!(result, "{}", message); } @@ -1670,7 +1675,8 @@ proptest! { let (inputs, distinct_inputs) = inputs_distinct_inputs; assert_eq!(inputs.len(), 25); assert_eq!(distinct_inputs.len(), 25); - let (result, message) = prop_assert_injective(inputs, distinct_inputs, 25, Some(64), keccakf1600_op); + let pedantic_solving = true; + let (result, message) = prop_assert_injective(inputs, distinct_inputs, 25, Some(64), pedantic_solving, keccakf1600_op); prop_assert!(result, "{}", message); } @@ -1679,12 +1685,13 @@ proptest! { #[should_panic(expected = "Failure(BlackBoxFunctionFailed(Poseidon2Permutation, \"the number of inputs does not match specified length. 6 != 7\"))")] fn poseidon2_permutation_invalid_size_fails(inputs_distinct_inputs in any_distinct_inputs(None, 6, 6)) { let (inputs, distinct_inputs) = inputs_distinct_inputs; - let (result, message) = prop_assert_injective(inputs, distinct_inputs, 1, None, poseidon2_permutation_invalid_len_op); + let pedantic_solving = true; + let (result, message) = prop_assert_injective(inputs, distinct_inputs, 1, None, pedantic_solving, poseidon2_permutation_invalid_len_op); prop_assert!(result, "{}", message); } #[test] - fn bigint_from_to_le_bytes_zero_one(modulus in select(allowed_bigint_moduli()), zero_or_ones_constant: bool, use_constant: bool) { + fn bigint_from_to_le_bytes_zero_one(modulus in select(BigIntSolver::allowed_bigint_moduli()), zero_or_ones_constant: bool, use_constant: bool) { let zero_function_input = if zero_or_ones_constant { FieldElement::one() } else { @@ -1692,25 +1699,46 @@ proptest! { }; let zero_or_ones: Vec<_> = modulus.iter().map(|_| (zero_function_input, use_constant)).collect(); let expected_results = drop_use_constant(&zero_or_ones); - let results = bigint_solve_from_to_le_bytes(modulus.clone(), zero_or_ones); + let pedantic_solving = true; + let results = bigint_solve_from_to_le_bytes(modulus.clone(), zero_or_ones, pedantic_solving); prop_assert_eq!(results, expected_results) } #[test] fn bigint_from_to_le_bytes((input, modulus) in bigint_with_modulus()) { let expected_results: Vec<_> = drop_use_constant(&input); - let results = bigint_solve_from_to_le_bytes(modulus.clone(), input); + let pedantic_solving = true; + let results = bigint_solve_from_to_le_bytes(modulus.clone(), input, pedantic_solving); prop_assert_eq!(results, expected_results) } #[test] // TODO(https://github.com/noir-lang/noir/issues/5580): desired behavior? - fn bigint_from_to_le_bytes_extra_input_bytes((input, modulus) in bigint_with_modulus(), extra_bytes_len: u8, extra_bytes in proptest::collection::vec(any::<(u8, bool)>(), u8::MAX as usize)) { - let mut input = input; - let mut extra_bytes: Vec<_> = extra_bytes.into_iter().take(extra_bytes_len as usize).map(|(x, use_constant)| (FieldElement::from(x as u128), use_constant)).collect(); + fn bigint_from_to_le_bytes_extra_input_bytes((mut input, modulus) in bigint_with_modulus(), extra_bytes_len: u8, extra_bytes in proptest::collection::vec(any::<(u8, bool)>(), u8::MAX as usize)) { + let mut extra_bytes: Vec<_> = extra_bytes + .into_iter() + .take(extra_bytes_len as usize) + .map(|(x, use_constant)| (FieldElement::from(x as u128), use_constant)) + .collect(); + input.append(&mut extra_bytes); + let expected_results: Vec<_> = drop_use_constant(&input); + let pedantic_solving = false; + let results = bigint_solve_from_to_le_bytes(modulus.clone(), input, pedantic_solving); + prop_assert_eq!(results, expected_results) + } + + #[test] + #[should_panic(expected = "--pedantic-solving: bigint_from_bytes: inputs.len() > modulus.len()")] + fn bigint_from_to_le_bytes_extra_input_bytes_with_pedantic((mut input, modulus) in bigint_with_modulus(), extra_bytes_len: u8, extra_bytes in proptest::collection::vec(any::<(u8, bool)>(), u8::MAX as usize)) { + let mut extra_bytes: Vec<_> = extra_bytes + .into_iter() + .take(extra_bytes_len as usize) + .map(|(x, use_constant)| (FieldElement::from(x as u128), use_constant)) + .collect(); input.append(&mut extra_bytes); let expected_results: Vec<_> = drop_use_constant(&input); - let results = bigint_solve_from_to_le_bytes(modulus.clone(), input); + let pedantic_solving = true; + let results = bigint_solve_from_to_le_bytes(modulus.clone(), input, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1723,94 +1751,109 @@ proptest! { let larger_value = FieldElement::from(std::cmp::max((u8::MAX as u16) + 1, larger_value) as u128); input[patch_location] = (larger_value, use_constant); let expected_results: Vec<_> = drop_use_constant(&input); - let results = bigint_solve_from_to_le_bytes(modulus.clone(), input); + let pedantic_solving = true; + let results = bigint_solve_from_to_le_bytes(modulus.clone(), input, pedantic_solving); prop_assert_eq!(results, expected_results) } #[test] // TODO(https://github.com/noir-lang/noir/issues/5578): this test attempts to use a guaranteed-invalid BigInt modulus // #[should_panic(expected = "attempt to add with overflow")] - fn bigint_from_to_le_bytes_disallowed_modulus(mut modulus in select(allowed_bigint_moduli()), patch_location: usize, patch_amount: u8, zero_or_ones_constant: bool, use_constant: bool) { - let allowed_moduli: HashSet> = allowed_bigint_moduli().into_iter().collect(); - let mut patch_location = patch_location % modulus.len(); - let patch_amount = patch_amount.clamp(1, u8::MAX); - while allowed_moduli.contains(&modulus) { - modulus[patch_location] = patch_amount.wrapping_add(modulus[patch_location]); - patch_location += 1; - patch_location %= modulus.len(); - } - - let zero_function_input = if zero_or_ones_constant { - FieldElement::zero() - } else { - FieldElement::one() - }; - let zero: Vec<_> = modulus.iter().map(|_| (zero_function_input, use_constant)).collect(); - let expected_results: Vec<_> = drop_use_constant(&zero); - let results = bigint_solve_from_to_le_bytes(modulus.clone(), zero); + fn bigint_from_to_le_bytes_disallowed_modulus(mut modulus in select(BigIntSolver::allowed_bigint_moduli()), patch_location: usize, patch_amount: u8, zero_or_ones_constant: bool, use_constant: bool) { + let pedantic_solving = false; + let (results, expected_results) = bigint_from_to_le_bytes_disallowed_modulus_helper( + &mut modulus, + patch_location, + patch_amount, + zero_or_ones_constant, + use_constant, + pedantic_solving, + ); + prop_assert_eq!(results, expected_results) + } + #[test] + #[should_panic(expected = "--pedantic-solving: bigint_from_bytes: disallowed modulus [")] + fn bigint_from_to_le_bytes_disallowed_modulus_panics_with_pedantic(mut modulus in select(BigIntSolver::allowed_bigint_moduli()), patch_location: usize, patch_amount: u8, zero_or_ones_constant: bool, use_constant: bool) { + let pedantic_solving = true; + let (results, expected_results) = bigint_from_to_le_bytes_disallowed_modulus_helper( + &mut modulus, + patch_location, + patch_amount, + zero_or_ones_constant, + use_constant, + pedantic_solving, + ); prop_assert_eq!(results, expected_results) } #[test] fn bigint_add_commutative((xs, ys, modulus) in bigint_pair_with_modulus()) { - let lhs_results = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), xs.clone(), ys.clone()); - let rhs_results = bigint_solve_binary_op(bigint_add_op(), modulus, ys, xs); + let pedantic_solving = true; + let lhs_results = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), xs.clone(), ys.clone(), pedantic_solving); + let rhs_results = bigint_solve_binary_op(bigint_add_op(), modulus, ys, xs, pedantic_solving); prop_assert_eq!(lhs_results, rhs_results) } #[test] fn bigint_mul_commutative((xs, ys, modulus) in bigint_pair_with_modulus()) { - let lhs_results = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs.clone(), ys.clone()); - let rhs_results = bigint_solve_binary_op(bigint_mul_op(), modulus, ys, xs); + let pedantic_solving = true; + let lhs_results = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs.clone(), ys.clone(), pedantic_solving); + let rhs_results = bigint_solve_binary_op(bigint_mul_op(), modulus, ys, xs, pedantic_solving); prop_assert_eq!(lhs_results, rhs_results) } #[test] fn bigint_add_associative((xs, ys, zs, modulus) in bigint_triple_with_modulus()) { + let pedantic_solving = true; + // f(f(xs, ys), zs) == - let op_xs_ys = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), xs.clone(), ys.clone()); + let op_xs_ys = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), xs.clone(), ys.clone(), pedantic_solving); let xs_ys = use_witnesses(op_xs_ys); - let op_xs_ys_op_zs = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), xs_ys, zs.clone()); + let op_xs_ys_op_zs = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), xs_ys, zs.clone(), pedantic_solving); // f(xs, f(ys, zs)) - let op_ys_zs = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), ys.clone(), zs.clone()); + let op_ys_zs = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), ys.clone(), zs.clone(), pedantic_solving); let ys_zs = use_witnesses(op_ys_zs); - let op_xs_op_ys_zs = bigint_solve_binary_op(bigint_add_op(), modulus, xs, ys_zs); + let op_xs_op_ys_zs = bigint_solve_binary_op(bigint_add_op(), modulus, xs, ys_zs, pedantic_solving); prop_assert_eq!(op_xs_ys_op_zs, op_xs_op_ys_zs) } #[test] fn bigint_mul_associative((xs, ys, zs, modulus) in bigint_triple_with_modulus()) { + let pedantic_solving = true; + // f(f(xs, ys), zs) == - let op_xs_ys = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs.clone(), ys.clone()); + let op_xs_ys = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs.clone(), ys.clone(), pedantic_solving); let xs_ys = use_witnesses(op_xs_ys); - let op_xs_ys_op_zs = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs_ys, zs.clone()); + let op_xs_ys_op_zs = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs_ys, zs.clone(), pedantic_solving); // f(xs, f(ys, zs)) - let op_ys_zs = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), ys.clone(), zs.clone()); + let op_ys_zs = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), ys.clone(), zs.clone(), pedantic_solving); let ys_zs = use_witnesses(op_ys_zs); - let op_xs_op_ys_zs = bigint_solve_binary_op(bigint_mul_op(), modulus, xs, ys_zs); + let op_xs_op_ys_zs = bigint_solve_binary_op(bigint_mul_op(), modulus, xs, ys_zs, pedantic_solving); prop_assert_eq!(op_xs_ys_op_zs, op_xs_op_ys_zs) } #[test] fn bigint_mul_add_distributive((xs, ys, zs, modulus) in bigint_triple_with_modulus()) { + let pedantic_solving = true; + // xs * (ys + zs) == - let add_ys_zs = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), ys.clone(), zs.clone()); + let add_ys_zs = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), ys.clone(), zs.clone(), pedantic_solving); let add_ys_zs = use_witnesses(add_ys_zs); - let mul_xs_add_ys_zs = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs.clone(), add_ys_zs); + let mul_xs_add_ys_zs = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs.clone(), add_ys_zs, pedantic_solving); // xs * ys + xs * zs - let mul_xs_ys = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs.clone(), ys); + let mul_xs_ys = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs.clone(), ys, pedantic_solving); let mul_xs_ys = use_witnesses(mul_xs_ys); - let mul_xs_zs = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs, zs); + let mul_xs_zs = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs, zs, pedantic_solving); let mul_xs_zs = use_witnesses(mul_xs_zs); - let add_mul_xs_ys_mul_xs_zs = bigint_solve_binary_op(bigint_add_op(), modulus, mul_xs_ys, mul_xs_zs); + let add_mul_xs_ys_mul_xs_zs = bigint_solve_binary_op(bigint_add_op(), modulus, mul_xs_ys, mul_xs_zs, pedantic_solving); prop_assert_eq!(mul_xs_add_ys_zs, add_mul_xs_ys_mul_xs_zs) } @@ -1820,7 +1863,8 @@ proptest! { fn bigint_add_zero_l((xs, modulus) in bigint_with_modulus()) { let zero = bigint_zeroed(&xs); let expected_results = drop_use_constant(&xs); - let results = bigint_solve_binary_op(bigint_add_op(), modulus, zero, xs); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_add_op(), modulus, zero, xs, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1829,7 +1873,8 @@ proptest! { fn bigint_mul_zero_l((xs, modulus) in bigint_with_modulus()) { let zero = bigint_zeroed(&xs); let expected_results = drop_use_constant(&zero); - let results = bigint_solve_binary_op(bigint_mul_op(), modulus, zero, xs); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_mul_op(), modulus, zero, xs, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1837,14 +1882,16 @@ proptest! { fn bigint_mul_one_l((xs, modulus) in bigint_with_modulus()) { let one = bigint_to_one(&xs); let expected_results: Vec<_> = drop_use_constant(&xs); - let results = bigint_solve_binary_op(bigint_mul_op(), modulus, one, xs); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_mul_op(), modulus, one, xs, pedantic_solving); prop_assert_eq!(results, expected_results) } #[test] fn bigint_sub_self((xs, modulus) in bigint_with_modulus()) { let expected_results = drop_use_constant(&bigint_zeroed(&xs)); - let results = bigint_solve_binary_op(bigint_sub_op(), modulus, xs.clone(), xs); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_sub_op(), modulus, xs.clone(), xs, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1852,7 +1899,8 @@ proptest! { fn bigint_sub_zero((xs, modulus) in bigint_with_modulus()) { let zero = bigint_zeroed(&xs); let expected_results: Vec<_> = drop_use_constant(&xs); - let results = bigint_solve_binary_op(bigint_sub_op(), modulus, xs, zero); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_sub_op(), modulus, xs, zero, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1860,14 +1908,16 @@ proptest! { fn bigint_sub_one((xs, modulus) in bigint_with_modulus()) { let one = bigint_to_one(&xs); let expected_results: Vec<_> = drop_use_constant(&xs); - let results = bigint_solve_binary_op(bigint_sub_op(), modulus, xs, one); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_sub_op(), modulus, xs, one, pedantic_solving); prop_assert!(results != expected_results, "{:?} == {:?}", results, expected_results) } #[test] fn bigint_div_self((xs, modulus) in bigint_with_modulus()) { let one = drop_use_constant(&bigint_to_one(&xs)); - let results = bigint_solve_binary_op(bigint_div_op(), modulus, xs.clone(), xs); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_div_op(), modulus, xs.clone(), xs, pedantic_solving); prop_assert_eq!(results, one) } @@ -1876,7 +1926,18 @@ proptest! { fn bigint_div_by_zero((xs, modulus) in bigint_with_modulus()) { let zero = bigint_zeroed(&xs); let expected_results = drop_use_constant(&zero); - let results = bigint_solve_binary_op(bigint_div_op(), modulus, xs, zero); + let pedantic_solving = false; + let results = bigint_solve_binary_op(bigint_div_op(), modulus, xs, zero, pedantic_solving); + prop_assert_eq!(results, expected_results) + } + + #[test] + #[should_panic(expected = "Attempted to divide BigInt by zero")] + fn bigint_div_by_zero_with_pedantic((xs, modulus) in bigint_with_modulus()) { + let zero = bigint_zeroed(&xs); + let expected_results = drop_use_constant(&zero); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_div_op(), modulus, xs, zero, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1884,7 +1945,8 @@ proptest! { fn bigint_div_one((xs, modulus) in bigint_with_modulus()) { let one = bigint_to_one(&xs); let expected_results = drop_use_constant(&xs); - let results = bigint_solve_binary_op(bigint_div_op(), modulus, xs, one); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_div_op(), modulus, xs, one, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1892,16 +1954,18 @@ proptest! { fn bigint_div_zero((xs, modulus) in bigint_with_modulus()) { let zero = bigint_zeroed(&xs); let expected_results = drop_use_constant(&zero); - let results = bigint_solve_binary_op(bigint_div_op(), modulus, zero, xs); + let pedantic_solving = true; + let results = bigint_solve_binary_op(bigint_div_op(), modulus, zero, xs, pedantic_solving); prop_assert_eq!(results, expected_results) } #[test] fn bigint_add_sub((xs, ys, modulus) in bigint_pair_with_modulus()) { let expected_results = drop_use_constant(&xs); - let add_results = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), xs, ys.clone()); + let pedantic_solving = true; + let add_results = bigint_solve_binary_op(bigint_add_op(), modulus.clone(), xs, ys.clone(), pedantic_solving); let add_bigint = use_witnesses(add_results); - let results = bigint_solve_binary_op(bigint_sub_op(), modulus, add_bigint, ys); + let results = bigint_solve_binary_op(bigint_sub_op(), modulus, add_bigint, ys, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1909,9 +1973,10 @@ proptest! { #[test] fn bigint_sub_add((xs, ys, modulus) in bigint_pair_with_modulus()) { let expected_results = drop_use_constant(&xs); - let sub_results = bigint_solve_binary_op(bigint_sub_op(), modulus.clone(), xs, ys.clone()); + let pedantic_solving = true; + let sub_results = bigint_solve_binary_op(bigint_sub_op(), modulus.clone(), xs, ys.clone(), pedantic_solving); let add_bigint = use_witnesses(sub_results); - let results = bigint_solve_binary_op(bigint_add_op(), modulus, add_bigint, ys); + let results = bigint_solve_binary_op(bigint_add_op(), modulus, add_bigint, ys, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1919,9 +1984,10 @@ proptest! { #[test] fn bigint_div_mul((xs, ys, modulus) in bigint_pair_with_modulus()) { let expected_results = drop_use_constant(&xs); - let div_results = bigint_solve_binary_op(bigint_div_op(), modulus.clone(), xs, ys.clone()); + let pedantic_solving = true; + let div_results = bigint_solve_binary_op(bigint_div_op(), modulus.clone(), xs, ys.clone(), pedantic_solving); let div_bigint = use_witnesses(div_results); - let results = bigint_solve_binary_op(bigint_mul_op(), modulus, div_bigint, ys); + let results = bigint_solve_binary_op(bigint_mul_op(), modulus, div_bigint, ys, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1929,9 +1995,10 @@ proptest! { #[test] fn bigint_mul_div((xs, ys, modulus) in bigint_pair_with_modulus()) { let expected_results = drop_use_constant(&xs); - let mul_results = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs, ys.clone()); + let pedantic_solving = true; + let mul_results = bigint_solve_binary_op(bigint_mul_op(), modulus.clone(), xs, ys.clone(), pedantic_solving); let mul_bigint = use_witnesses(mul_results); - let results = bigint_solve_binary_op(bigint_div_op(), modulus, mul_bigint, ys); + let results = bigint_solve_binary_op(bigint_div_op(), modulus, mul_bigint, ys, pedantic_solving); prop_assert_eq!(results, expected_results) } diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs index c3627d0eff5..e4d52063977 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs +++ b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs @@ -30,12 +30,27 @@ pub async fn execute_circuit( program: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, +) -> Result { + let pedantic_solving = false; + execute_circuit_pedantic(program, initial_witness, foreign_call_handler, pedantic_solving).await +} + +/// `execute_circuit` with pedantic ACVM solving +async fn execute_circuit_pedantic( + program: Vec, + initial_witness: JsWitnessMap, + foreign_call_handler: ForeignCallHandler, + pedantic_solving: bool, ) -> Result { console_error_panic_hook::set_once(); - let mut witness_stack = - execute_program_with_native_type_return(program, initial_witness, &foreign_call_handler) - .await?; + let mut witness_stack = execute_program_with_native_type_return( + program, + initial_witness, + &foreign_call_handler, + pedantic_solving, + ) + .await?; let witness_map = witness_stack.pop().expect("Should have at least one witness on the stack").witness; Ok(witness_map.into()) @@ -53,6 +68,23 @@ pub async fn execute_circuit_with_return_witness( program: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, +) -> Result { + let pedantic_solving = false; + execute_circuit_with_return_witness_pedantic( + program, + initial_witness, + foreign_call_handler, + pedantic_solving, + ) + .await +} + +/// `executeCircuitWithReturnWitness` with pedantic ACVM execution +async fn execute_circuit_with_return_witness_pedantic( + program: Vec, + initial_witness: JsWitnessMap, + foreign_call_handler: ForeignCallHandler, + pedantic_solving: bool, ) -> Result { console_error_panic_hook::set_once(); @@ -63,6 +95,7 @@ pub async fn execute_circuit_with_return_witness( &program, initial_witness, &foreign_call_handler, + pedantic_solving, ) .await?; let solved_witness = @@ -87,12 +120,27 @@ pub async fn execute_program( program: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, +) -> Result { + let pedantic_solving = false; + execute_program_pedantic(program, initial_witness, foreign_call_handler, pedantic_solving).await +} + +/// `execute_program` with pedantic ACVM solving +async fn execute_program_pedantic( + program: Vec, + initial_witness: JsWitnessMap, + foreign_call_handler: ForeignCallHandler, + pedantic_solving: bool, ) -> Result { console_error_panic_hook::set_once(); - let witness_stack = - execute_program_with_native_type_return(program, initial_witness, &foreign_call_handler) - .await?; + let witness_stack = execute_program_with_native_type_return( + program, + initial_witness, + &foreign_call_handler, + pedantic_solving, + ) + .await?; Ok(witness_stack.into()) } @@ -101,6 +149,7 @@ async fn execute_program_with_native_type_return( program: Vec, initial_witness: JsWitnessMap, foreign_call_executor: &ForeignCallHandler, + pedantic_solving: bool, ) -> Result, Error> { let program: Program = Program::deserialize_program(&program) .map_err(|_| JsExecutionError::new( @@ -109,16 +158,22 @@ async fn execute_program_with_native_type_return( None, None))?; - execute_program_with_native_program_and_return(&program, initial_witness, foreign_call_executor) - .await + execute_program_with_native_program_and_return( + &program, + initial_witness, + foreign_call_executor, + pedantic_solving, + ) + .await } async fn execute_program_with_native_program_and_return( program: &Program, initial_witness: JsWitnessMap, foreign_call_executor: &ForeignCallHandler, + pedantic_solving: bool, ) -> Result, Error> { - let blackbox_solver = Bn254BlackBoxSolver; + let blackbox_solver = Bn254BlackBoxSolver(pedantic_solving); let executor = ProgramExecutor::new( &program.functions, &program.unconstrained_functions, diff --git a/noir/noir-repo/acvm-repo/blackbox_solver/Cargo.toml b/noir/noir-repo/acvm-repo/blackbox_solver/Cargo.toml index 532f6f2c54f..2c2e0d7675e 100644 --- a/noir/noir-repo/acvm-repo/blackbox_solver/Cargo.toml +++ b/noir/noir-repo/acvm-repo/blackbox_solver/Cargo.toml @@ -43,6 +43,7 @@ libaes = "0.7.0" [dev-dependencies] proptest.workspace = true +num-prime = { version = "0.4.4", default-features = false, features = ["big-int"] } [features] bn254 = ["acir/bn254"] diff --git a/noir/noir-repo/acvm-repo/blackbox_solver/src/bigint.rs b/noir/noir-repo/acvm-repo/blackbox_solver/src/bigint.rs index dab611a81e8..f7be1e80a55 100644 --- a/noir/noir-repo/acvm-repo/blackbox_solver/src/bigint.rs +++ b/noir/noir-repo/acvm-repo/blackbox_solver/src/bigint.rs @@ -10,14 +10,28 @@ use crate::BlackBoxResolutionError; /// - When it encounters a bigint operation opcode, it performs the operation on the stored values /// and store the result using the provided ID. /// - When it gets a to_bytes opcode, it simply looks up the value and resolves the output witness accordingly. -#[derive(Default, Debug, Clone, PartialEq, Eq)] - +#[derive(Debug, Clone, PartialEq, Eq)] pub struct BigIntSolver { bigint_id_to_value: HashMap, bigint_id_to_modulus: HashMap, + + // Use pedantic ACVM solving + pedantic_solving: bool, } impl BigIntSolver { + pub fn with_pedantic_solving(pedantic_solving: bool) -> BigIntSolver { + BigIntSolver { + bigint_id_to_value: Default::default(), + bigint_id_to_modulus: Default::default(), + pedantic_solving, + } + } + + pub fn pedantic_solving(&self) -> bool { + self.pedantic_solving + } + pub fn get_bigint( &self, id: u32, @@ -51,6 +65,17 @@ impl BigIntSolver { modulus: &[u8], output: u32, ) -> Result<(), BlackBoxResolutionError> { + if self.pedantic_solving { + if !self.is_valid_modulus(modulus) { + panic!("--pedantic-solving: bigint_from_bytes: disallowed modulus {:?}", modulus); + } + if inputs.len() > modulus.len() { + panic!( + "--pedantic-solving: bigint_from_bytes: inputs.len() > modulus.len() {:?}", + (inputs.len(), modulus.len()) + ); + } + } let bigint = BigUint::from_bytes_le(inputs); self.bigint_id_to_value.insert(output, bigint); let modulus = BigUint::from_bytes_le(modulus); @@ -84,8 +109,14 @@ impl BigIntSolver { } BlackBoxFunc::BigIntMul => lhs * rhs, BlackBoxFunc::BigIntDiv => { + if self.pedantic_solving && rhs == BigUint::ZERO { + return Err(BlackBoxResolutionError::Failed( + func, + "Attempted to divide BigInt by zero".to_string(), + )); + } lhs * rhs.modpow(&(&modulus - BigUint::from(2_u32)), &modulus) - } //TODO ensure that modulus is prime + } _ => unreachable!("ICE - bigint_op must be called for an operation"), }; if result > modulus { @@ -96,16 +127,57 @@ impl BigIntSolver { self.bigint_id_to_modulus.insert(output, modulus); Ok(()) } + + pub fn allowed_bigint_moduli() -> Vec> { + let bn254_fq: Vec = vec![ + 0x47, 0xFD, 0x7C, 0xD8, 0x16, 0x8C, 0x20, 0x3C, 0x8d, 0xca, 0x71, 0x68, 0x91, 0x6a, + 0x81, 0x97, 0x5d, 0x58, 0x81, 0x81, 0xb6, 0x45, 0x50, 0xb8, 0x29, 0xa0, 0x31, 0xe1, + 0x72, 0x4e, 0x64, 0x30, + ]; + let bn254_fr: Vec = vec![ + 1, 0, 0, 240, 147, 245, 225, 67, 145, 112, 185, 121, 72, 232, 51, 40, 93, 88, 129, 129, + 182, 69, 80, 184, 41, 160, 49, 225, 114, 78, 100, 48, + ]; + let secpk1_fr: Vec = vec![ + 0x41, 0x41, 0x36, 0xD0, 0x8C, 0x5E, 0xD2, 0xBF, 0x3B, 0xA0, 0x48, 0xAF, 0xE6, 0xDC, + 0xAE, 0xBA, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, + ]; + let secpk1_fq: Vec = vec![ + 0x2F, 0xFC, 0xFF, 0xFF, 0xFE, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, + 0xFF, 0xFF, 0xFF, 0xFF, + ]; + let secpr1_fq: Vec = vec![ + 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0x00, 0x00, + 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, + 0xFF, 0xFF, 0xFF, 0xFF, + ]; + let secpr1_fr: Vec = vec![ + 81, 37, 99, 252, 194, 202, 185, 243, 132, 158, 23, 167, 173, 250, 230, 188, 255, 255, + 255, 255, 255, 255, 255, 255, 0, 0, 0, 0, 255, 255, 255, 255, + ]; + vec![bn254_fq, bn254_fr, secpk1_fr, secpk1_fq, secpr1_fq, secpr1_fr] + } + + pub fn is_valid_modulus(&self, modulus: &[u8]) -> bool { + Self::allowed_bigint_moduli().into_iter().any(|allowed_modulus| allowed_modulus == modulus) + } } /// Wrapper over the generic bigint solver to automatically assign bigint IDs. -#[derive(Default, Debug, Clone, PartialEq, Eq)] +#[derive(Debug, Clone, PartialEq, Eq)] pub struct BigIntSolverWithId { solver: BigIntSolver, last_id: u32, } impl BigIntSolverWithId { + pub fn with_pedantic_solving(pedantic_solving: bool) -> BigIntSolverWithId { + let solver = BigIntSolver::with_pedantic_solving(pedantic_solving); + BigIntSolverWithId { solver, last_id: Default::default() } + } + pub fn create_bigint_id(&mut self) -> u32 { let output = self.last_id; self.last_id += 1; @@ -145,3 +217,23 @@ impl BigIntSolverWithId { Ok(id) } } + +#[test] +fn all_allowed_bigint_moduli_are_prime() { + use num_prime::nt_funcs::is_prime; + use num_prime::Primality; + + for modulus in BigIntSolver::allowed_bigint_moduli() { + let modulus = BigUint::from_bytes_le(&modulus); + let prime_test_config = None; + match is_prime(&modulus, prime_test_config) { + Primality::Yes => (), + Primality::No => panic!("not all allowed_bigint_moduli are prime: {modulus}"), + Primality::Probable(probability) => { + if probability < 0.90 { + panic!("not all allowed_bigint_moduli are prime within the allowed probability: {} < 0.90", probability); + } + } + } + } +} diff --git a/noir/noir-repo/acvm-repo/blackbox_solver/src/curve_specific_solver.rs b/noir/noir-repo/acvm-repo/blackbox_solver/src/curve_specific_solver.rs index b8fc3f47033..37fe5d05363 100644 --- a/noir/noir-repo/acvm-repo/blackbox_solver/src/curve_specific_solver.rs +++ b/noir/noir-repo/acvm-repo/blackbox_solver/src/curve_specific_solver.rs @@ -7,6 +7,7 @@ use crate::BlackBoxResolutionError; /// /// Returns an [`BlackBoxResolutionError`] if the backend does not support the given [`acir::BlackBoxFunc`]. pub trait BlackBoxFunctionSolver { + fn pedantic_solving(&self) -> bool; fn multi_scalar_mul( &self, points: &[F], @@ -29,7 +30,16 @@ pub trait BlackBoxFunctionSolver { ) -> Result, BlackBoxResolutionError>; } -pub struct StubbedBlackBoxSolver; +// pedantic_solving: bool +pub struct StubbedBlackBoxSolver(pub bool); + +// pedantic_solving enabled by default +impl Default for StubbedBlackBoxSolver { + fn default() -> StubbedBlackBoxSolver { + let pedantic_solving = true; + StubbedBlackBoxSolver(pedantic_solving) + } +} impl StubbedBlackBoxSolver { fn fail(black_box_function: BlackBoxFunc) -> BlackBoxResolutionError { @@ -41,6 +51,9 @@ impl StubbedBlackBoxSolver { } impl BlackBoxFunctionSolver for StubbedBlackBoxSolver { + fn pedantic_solving(&self) -> bool { + self.0 + } fn multi_scalar_mul( &self, _points: &[F], diff --git a/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index 6118233b0b7..ffe7fa50089 100644 --- a/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs @@ -1,5 +1,6 @@ // TODO(https://github.com/noir-lang/noir/issues/4932): rename this file to something more generic use ark_ec::AffineRepr; +use ark_ff::MontConfig; use num_bigint::BigUint; use crate::FieldElement; @@ -13,6 +14,7 @@ pub fn multi_scalar_mul( points: &[FieldElement], scalars_lo: &[FieldElement], scalars_hi: &[FieldElement], + pedantic_solving: bool, ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { if points.len() != 3 * scalars_lo.len() || scalars_lo.len() != scalars_hi.len() { return Err(BlackBoxResolutionError::Failed( @@ -24,6 +26,9 @@ pub fn multi_scalar_mul( let mut output_point = ark_grumpkin::Affine::zero(); for i in (0..points.len()).step_by(3) { + if pedantic_solving && points[i + 2] > FieldElement::one() { + panic!("--pedantic-solving: is_infinity expected to be a bool, but found to be > 1") + } let point = create_point(points[i], points[i + 1], points[i + 2] == FieldElement::from(1_u128)) .map_err(|e| BlackBoxResolutionError::Failed(BlackBoxFunc::MultiScalarMul, e))?; @@ -48,12 +53,12 @@ pub fn multi_scalar_mul( let grumpkin_integer = BigUint::from_bytes_be(&bytes); // Check if this is smaller than the grumpkin modulus - // if grumpkin_integer >= ark_grumpkin::FrConfig::MODULUS.into() { - // return Err(BlackBoxResolutionError::Failed( - // BlackBoxFunc::MultiScalarMul, - // format!("{} is not a valid grumpkin scalar", grumpkin_integer.to_str_radix(16)), - // )); - // } + if pedantic_solving && grumpkin_integer >= ark_grumpkin::FrConfig::MODULUS.into() { + return Err(BlackBoxResolutionError::Failed( + BlackBoxFunc::MultiScalarMul, + format!("{} is not a valid grumpkin scalar", grumpkin_integer.to_str_radix(16)), + )); + } let iteration_output_point = ark_grumpkin::Affine::from(point.mul_bigint(grumpkin_integer.to_u64_digits())); @@ -75,11 +80,28 @@ pub fn multi_scalar_mul( pub fn embedded_curve_add( input1: [FieldElement; 3], input2: [FieldElement; 3], + pedantic_solving: bool, ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { + if pedantic_solving && (input1[2] > FieldElement::one() || input2[2] > FieldElement::one()) { + panic!("--pedantic-solving: is_infinity expected to be a bool, but found to be > 1") + } + let point1 = create_point(input1[0], input1[1], input1[2] == FieldElement::one()) .map_err(|e| BlackBoxResolutionError::Failed(BlackBoxFunc::EmbeddedCurveAdd, e))?; let point2 = create_point(input2[0], input2[1], input2[2] == FieldElement::one()) .map_err(|e| BlackBoxResolutionError::Failed(BlackBoxFunc::EmbeddedCurveAdd, e))?; + + if pedantic_solving { + for point in [point1, point2] { + if point == ark_grumpkin::Affine::zero() { + return Err(BlackBoxResolutionError::Failed( + BlackBoxFunc::EmbeddedCurveAdd, + format!("Infinite input: embedded_curve_add({}, {})", point1, point2), + )); + } + } + } + let res = ark_grumpkin::Affine::from(point1 + point2); if let Some((res_x, res_y)) = res.xy() { Ok(( @@ -118,6 +140,7 @@ fn create_point( #[cfg(test)] mod tests { use super::*; + use ark_ff::BigInteger; fn get_generator() -> [FieldElement; 3] { let generator = ark_grumpkin::Affine::generator(); @@ -131,7 +154,13 @@ mod tests { // We check that multiplying 1 by generator results in the generator let generator = get_generator(); - let res = multi_scalar_mul(&generator, &[FieldElement::one()], &[FieldElement::zero()])?; + let pedantic_solving = true; + let res = multi_scalar_mul( + &generator, + &[FieldElement::one()], + &[FieldElement::zero()], + pedantic_solving, + )?; assert_eq!(generator[0], res.0); assert_eq!(generator[1], res.1); @@ -144,7 +173,8 @@ mod tests { let scalars_lo = [FieldElement::one()]; let scalars_hi = [FieldElement::from(2u128)]; - let res = multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; + let pedantic_solving = true; + let res = multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving)?; let x = "0702ab9c7038eeecc179b4f209991bcb68c7cb05bf4c532d804ccac36199c9a9"; let y = "23f10e9e43a3ae8d75d24154e796aae12ae7af546716e8f81a2564f1b5814130"; @@ -165,30 +195,33 @@ mod tests { "Limb 0000000000000000000000000000000100000000000000000000000000000000 is not less than 2^128".into(), )); - let res = multi_scalar_mul(&points, &[FieldElement::one()], &[invalid_limb]); + let pedantic_solving = true; + let res = + multi_scalar_mul(&points, &[FieldElement::one()], &[invalid_limb], pedantic_solving); assert_eq!(res, expected_error); - let res = multi_scalar_mul(&points, &[invalid_limb], &[FieldElement::one()]); + let res = + multi_scalar_mul(&points, &[invalid_limb], &[FieldElement::one()], pedantic_solving); assert_eq!(res, expected_error); } - // #[test] - // fn rejects_grumpkin_modulus() { - // let x = ark_grumpkin::FrConfig::MODULUS.to_bytes_be(); - - // let low = FieldElement::from_be_bytes_reduce(&x[16..32]); - // let high = FieldElement::from_be_bytes_reduce(&x[0..16]); + #[test] + fn rejects_grumpkin_modulus_when_pedantic() { + let x = ark_grumpkin::FrConfig::MODULUS.to_bytes_be(); + let low = FieldElement::from_be_bytes_reduce(&x[16..32]); + let high = FieldElement::from_be_bytes_reduce(&x[0..16]); - // let res = multi_scalar_mul(&get_generator(), &[low], &[high]); + let pedantic_solving = true; + let res = multi_scalar_mul(&get_generator(), &[low], &[high], pedantic_solving); - // assert_eq!( - // res, - // Err(BlackBoxResolutionError::Failed( - // BlackBoxFunc::MultiScalarMul, - // "30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47 is not a valid grumpkin scalar".into(), - // )) - // ); - // } + assert_eq!( + res, + Err(BlackBoxResolutionError::Failed( + BlackBoxFunc::MultiScalarMul, + "30644e72e131a029b85045b68181585d97816a916871ca8d3c208c16d87cfd47 is not a valid grumpkin scalar".into(), + )) + ); + } #[test] fn rejects_invalid_point() { @@ -197,10 +230,12 @@ mod tests { let valid_scalar_low = FieldElement::zero(); let valid_scalar_high = FieldElement::zero(); + let pedantic_solving = true; let res = multi_scalar_mul( &[invalid_point_x, invalid_point_y, FieldElement::zero()], &[valid_scalar_low], &[valid_scalar_high], + pedantic_solving, ); assert_eq!( @@ -218,7 +253,8 @@ mod tests { let scalars_lo = [FieldElement::from(2u128)]; let scalars_hi = []; - let res = multi_scalar_mul(&points, &scalars_lo, &scalars_hi); + let pedantic_solving = true; + let res = multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving); assert_eq!( res, @@ -234,9 +270,11 @@ mod tests { let x = FieldElement::from(1u128); let y = FieldElement::from(2u128); + let pedantic_solving = true; let res = embedded_curve_add( [x, y, FieldElement::from(0u128)], [x, y, FieldElement::from(0u128)], + pedantic_solving, ); assert_eq!( @@ -248,16 +286,39 @@ mod tests { ); } + #[test] + fn rejects_addition_of_infinite_points_when_pedantic() { + let x = FieldElement::from(1u128); + let y = FieldElement::from(1u128); + + let pedantic_solving = true; + let res = embedded_curve_add( + [x, y, FieldElement::from(1u128)], + [x, y, FieldElement::from(1u128)], + pedantic_solving, + ); + + assert_eq!( + res, + Err(BlackBoxResolutionError::Failed( + BlackBoxFunc::EmbeddedCurveAdd, + "Infinite input: embedded_curve_add(infinity, infinity)".into(), + )) + ); + } + #[test] fn output_of_msm_matches_add() -> Result<(), BlackBoxResolutionError> { let points = get_generator(); let scalars_lo = [FieldElement::from(2u128)]; let scalars_hi = [FieldElement::zero()]; - let msm_res = multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; + let pedantic_solving = true; + let msm_res = multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving)?; let add_res = embedded_curve_add( [points[0], points[1], FieldElement::from(0u128)], [points[0], points[1], FieldElement::from(0u128)], + pedantic_solving, )?; assert_eq!(msm_res.0, add_res.0); diff --git a/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/lib.rs b/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/lib.rs index f738a375ab1..fc031faefa2 100644 --- a/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/noir/noir-repo/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -20,16 +20,21 @@ pub use poseidon2::{ type FieldElement = acir::acir_field::GenericFieldElement; #[derive(Default)] -pub struct Bn254BlackBoxSolver; +// pedantic_solving: bool +pub struct Bn254BlackBoxSolver(pub bool); impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { + fn pedantic_solving(&self) -> bool { + self.0 + } + fn multi_scalar_mul( &self, points: &[FieldElement], scalars_lo: &[FieldElement], scalars_hi: &[FieldElement], ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { - multi_scalar_mul(points, scalars_lo, scalars_hi) + multi_scalar_mul(points, scalars_lo, scalars_hi, self.pedantic_solving()) } fn ec_add( @@ -44,6 +49,7 @@ impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { embedded_curve_add( [*input1_x, *input1_y, *input1_infinite], [*input2_x, *input2_y, *input2_infinite], + self.pedantic_solving(), ) } diff --git a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs index 5b3688339b5..8602f74e0d6 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs @@ -111,6 +111,8 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { black_box_solver: &'a B, profiling_active: bool, ) -> Self { + let bigint_solver = + BrilligBigIntSolver::with_pedantic_solving(black_box_solver.pedantic_solving()); Self { calldata, program_counter: 0, @@ -121,7 +123,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { memory: Memory::default(), call_stack: Vec::new(), black_box_solver, - bigint_solver: Default::default(), + bigint_solver, profiling_active, profiling_samples: Vec::with_capacity(bytecode.len()), } @@ -854,7 +856,8 @@ mod tests { }]; // Start VM - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(calldata, &opcodes, vec![], &solver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::Finished { return_data_offset: 0, return_data_size: 0 }); @@ -904,7 +907,8 @@ mod tests { Opcode::JumpIf { condition: destination, location: 6 }, ]; - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(calldata, &opcodes, vec![], &solver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -972,7 +976,8 @@ mod tests { }, ]; - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(calldata, &opcodes, vec![], &solver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1044,7 +1049,8 @@ mod tests { }, }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(calldata, opcodes, vec![], &solver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1104,7 +1110,8 @@ mod tests { }, }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(calldata, opcodes, vec![], &solver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1150,7 +1157,8 @@ mod tests { }, Opcode::Mov { destination: MemoryAddress::direct(2), source: MemoryAddress::direct(0) }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(calldata, opcodes, vec![], &solver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1215,7 +1223,8 @@ mod tests { condition: MemoryAddress::direct(1), }, ]; - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(calldata, opcodes, vec![], &solver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1311,7 +1320,8 @@ mod tests { .chain(cast_opcodes) .chain([equal_opcode, not_equal_opcode, less_than_opcode, less_than_equal_opcode]) .collect(); - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(calldata, &opcodes, vec![], &solver, false); // Calldata copy let status = vm.process_opcode(); @@ -1411,7 +1421,8 @@ mod tests { ]; let opcodes = [&start[..], &loop_body[..]].concat(); - let vm = brillig_execute_and_get_vm(vec![], &opcodes); + let solver = StubbedBlackBoxSolver::default(); + let vm = brillig_execute_and_get_vm(vec![], &opcodes, &solver); vm.get_memory()[4..].to_vec() } @@ -1439,7 +1450,8 @@ mod tests { value: FieldElement::from(27_usize), }, ]; - let mut vm = VM::new(vec![], opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(vec![], opcodes, vec![], &solver, false); let status = vm.process_opcode(); assert_eq!(status, VMStatus::InProgress); @@ -1553,7 +1565,8 @@ mod tests { ]; let opcodes = [&start[..], &loop_body[..]].concat(); - let vm = brillig_execute_and_get_vm(memory, &opcodes); + let solver = StubbedBlackBoxSolver::default(); + let vm = brillig_execute_and_get_vm(memory, &opcodes, &solver); vm.memory.read(r_sum).to_field() } @@ -1644,7 +1657,8 @@ mod tests { ]; let opcodes = [&start[..], &recursive_fn[..]].concat(); - let vm = brillig_execute_and_get_vm(vec![], &opcodes); + let solver = StubbedBlackBoxSolver::default(); + let vm = brillig_execute_and_get_vm(vec![], &opcodes, &solver); vm.get_memory()[4..].to_vec() } @@ -1659,11 +1673,12 @@ mod tests { } /// Helper to execute brillig code - fn brillig_execute_and_get_vm( + fn brillig_execute_and_get_vm<'a, F: AcirField>( calldata: Vec, - opcodes: &[Opcode], - ) -> VM<'_, F, StubbedBlackBoxSolver> { - let mut vm = VM::new(calldata, opcodes, vec![], &StubbedBlackBoxSolver, false); + opcodes: &'a [Opcode], + solver: &'a StubbedBlackBoxSolver, + ) -> VM<'a, F, StubbedBlackBoxSolver> { + let mut vm = VM::new(calldata, opcodes, vec![], solver, false); brillig_execute(&mut vm); assert_eq!(vm.call_stack, vec![]); vm @@ -1705,7 +1720,8 @@ mod tests { }, ]; - let mut vm = brillig_execute_and_get_vm(vec![], &double_program); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = brillig_execute_and_get_vm(vec![], &double_program, &solver); // Check that VM is waiting assert_eq!( @@ -1798,7 +1814,8 @@ mod tests { }, ]; - let mut vm = brillig_execute_and_get_vm(initial_matrix.clone(), &invert_program); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = brillig_execute_and_get_vm(initial_matrix.clone(), &invert_program, &solver); // Check that VM is waiting assert_eq!( @@ -1908,7 +1925,9 @@ mod tests { }, ]; - let mut vm = brillig_execute_and_get_vm(input_string.clone(), &string_double_program); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = + brillig_execute_and_get_vm(input_string.clone(), &string_double_program, &solver); // Check that VM is waiting assert_eq!( @@ -2006,7 +2025,8 @@ mod tests { }, ]; - let mut vm = brillig_execute_and_get_vm(initial_matrix.clone(), &invert_program); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = brillig_execute_and_get_vm(initial_matrix.clone(), &invert_program, &solver); // Check that VM is waiting assert_eq!( @@ -2128,7 +2148,8 @@ mod tests { ]; let mut initial_memory = matrix_a.clone(); initial_memory.extend(matrix_b.clone()); - let mut vm = brillig_execute_and_get_vm(initial_memory, &matrix_mul_program); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = brillig_execute_and_get_vm(initial_memory, &matrix_mul_program, &solver); // Check that VM is waiting assert_eq!( @@ -2268,9 +2289,11 @@ mod tests { ]) .collect(); + let solver = StubbedBlackBoxSolver::default(); let mut vm = brillig_execute_and_get_vm( memory.into_iter().map(|mem_value| mem_value.to_field()).collect(), &program, + &solver, ); // Check that VM is waiting @@ -2342,7 +2365,8 @@ mod tests { }, ]; - let mut vm = VM::new(calldata, &opcodes, vec![], &StubbedBlackBoxSolver, false); + let solver = StubbedBlackBoxSolver::default(); + let mut vm = VM::new(calldata, &opcodes, vec![], &solver, false); vm.process_opcode(); vm.process_opcode(); diff --git a/noir/noir-repo/compiler/noirc_driver/src/lib.rs b/noir/noir-repo/compiler/noirc_driver/src/lib.rs index 87fbd3d1d3d..87da9cd658e 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/lib.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/lib.rs @@ -151,6 +151,12 @@ pub struct CompileOptions { #[arg(long, hide = true, allow_hyphen_values = true)] pub max_bytecode_increase_percent: Option, + /// Use pedantic ACVM solving, i.e. double-check some black-box function + /// assumptions when solving. + /// This is disabled by default. + #[arg(long, default_value = "false")] + pub pedantic_solving: bool, + /// Used internally to test for non-determinism in the compiler. #[clap(long, hide = true)] pub check_non_determinism: bool, @@ -311,8 +317,12 @@ pub fn check_crate( crate_id: CrateId, options: &CompileOptions, ) -> CompilationResult<()> { - let diagnostics = - CrateDefMap::collect_defs(crate_id, context, options.debug_comptime_in_file.as_deref()); + let diagnostics = CrateDefMap::collect_defs( + crate_id, + context, + options.debug_comptime_in_file.as_deref(), + options.pedantic_solving, + ); let crate_files = context.crate_files(&crate_id); let warnings_and_errors: Vec = diagnostics .into_iter() diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/acir/acir_variable.rs b/noir/noir-repo/compiler/noirc_evaluator/src/acir/acir_variable.rs index a253999bfd0..cf6b1fcc7f7 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/acir/acir_variable.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/acir/acir_variable.rs @@ -1421,7 +1421,7 @@ impl> AcirContext { } }?; output_count = input_size + (16 - input_size % 16); - (vec![], vec![F::from(output_count as u128)]) + (vec![], vec![]) } BlackBoxFunc::RecursiveAggregation => { let proof_type_var = match inputs.pop() { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs index 2ddcea26570..1fc39b58223 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_black_box.rs @@ -355,15 +355,14 @@ pub(crate) fn convert_black_box_call { if let ( [inputs, BrilligVariable::BrilligArray(iv), BrilligVariable::BrilligArray(key)], - [BrilligVariable::SingleAddr(out_len), BrilligVariable::BrilligVector(outputs)], + [outputs], ) = (function_arguments, function_results) { let inputs = convert_array_or_vector(brillig_context, *inputs, bb_func); let iv = brillig_context.codegen_brillig_array_to_heap_array(*iv); let key = brillig_context.codegen_brillig_array_to_heap_array(*key); - let outputs_vector = - brillig_context.codegen_brillig_vector_to_heap_vector(*outputs); + let outputs_vector = convert_array_or_vector(brillig_context, *outputs, bb_func); brillig_context.black_box_op_instruction(BlackBoxOp::AES128Encrypt { inputs, @@ -372,11 +371,6 @@ pub(crate) fn convert_black_box_call for DummyBlackBoxSolver { + fn pedantic_solving(&self) -> bool { + true + } + fn multi_scalar_mul( &self, _points: &[FieldElement], diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index b0e5506e8dd..951761e041e 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -493,11 +493,12 @@ fn simplify_black_box_func( block: BasicBlockId, call_stack: CallStackId, ) -> SimplifyResult { + let pedantic_solving = true; cfg_if::cfg_if! { if #[cfg(feature = "bn254")] { - let solver = bn254_blackbox_solver::Bn254BlackBoxSolver; + let solver = bn254_blackbox_solver::Bn254BlackBoxSolver(pedantic_solving); } else { - let solver = acvm::blackbox_solver::StubbedBlackBoxSolver; + let solver = acvm::blackbox_solver::StubbedBlackBoxSolver(pedantic_solving); } }; match bb_func { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 17aefdf21b8..8dca867fc6d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -632,7 +632,8 @@ impl<'brillig> Context<'brillig> { let bytecode = &generated_brillig.byte_code; let foreign_call_results = Vec::new(); - let black_box_solver = Bn254BlackBoxSolver; + let pedantic_solving = true; + let black_box_solver = Bn254BlackBoxSolver(pedantic_solving); let profiling_active = false; let mut vm = VM::new(calldata, bytecode, foreign_call_results, &black_box_solver, profiling_active); diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index b3a8ecf1314..56a2723a038 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -703,9 +703,7 @@ impl<'a> FunctionContext<'a> { // Don't mutate the reference count if we're assigning an array literal to a Let: // `let mut foo = [1, 2, 3];` // we consider the array to be moved, so we should have an initial rc of just 1. - // - // TODO: this exception breaks #6763 - let should_inc_rc = true; // !let_expr.expression.is_array_or_slice_literal(); + let should_inc_rc = !let_expr.expression.is_array_or_slice_literal(); values = values.map(|value| { let value = value.eval(self); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs index ae622f46686..9d521545e7a 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/expression.rs @@ -821,8 +821,8 @@ impl FunctionDefinition { is_unconstrained: bool, generics: &UnresolvedGenerics, parameters: &[(Ident, UnresolvedType)], - body: &BlockExpression, - where_clause: &[UnresolvedTraitConstraint], + body: BlockExpression, + where_clause: Vec, return_type: &FunctionReturnType, ) -> FunctionDefinition { let p = parameters @@ -843,9 +843,9 @@ impl FunctionDefinition { visibility: ItemVisibility::Private, generics: generics.clone(), parameters: p, - body: body.clone(), + body, span: name.span(), - where_clause: where_clause.to_vec(), + where_clause, return_type: return_type.clone(), return_visibility: Visibility::Private, } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/ast/function.rs b/noir/noir-repo/compiler/noirc_frontend/src/ast/function.rs index 99ae78c93ea..8957564e0d6 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/ast/function.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/ast/function.rs @@ -19,22 +19,27 @@ pub struct NoirFunction { pub def: FunctionDefinition, } -/// Currently, we support three types of functions: +/// Currently, we support four types of functions: /// - Normal functions /// - LowLevel/Foreign which link to an OPCODE in ACIR /// - BuiltIn which are provided by the runtime +/// - TraitFunctionWithoutBody for which we don't type-check their body #[derive(Copy, Clone, Debug, PartialEq, Eq)] pub enum FunctionKind { LowLevel, Builtin, Normal, Oracle, + TraitFunctionWithoutBody, } impl FunctionKind { pub fn can_ignore_return_type(self) -> bool { match self { - FunctionKind::LowLevel | FunctionKind::Builtin | FunctionKind::Oracle => true, + FunctionKind::LowLevel + | FunctionKind::Builtin + | FunctionKind::Oracle + | FunctionKind::TraitFunctionWithoutBody => true, FunctionKind::Normal => false, } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs index 24c1e7d929e..d88bb62e871 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -97,6 +97,7 @@ impl<'context> Elaborator<'context> { self.crate_id, self.debug_comptime_in_file, self.interpreter_call_stack.clone(), + self.pedantic_solving, ); elaborator.function_context.push(FunctionContext::default()); @@ -247,7 +248,7 @@ impl<'context> Elaborator<'context> { if value != Value::Unit { let items = value - .into_top_level_items(location, self.interner) + .into_top_level_items(location, self) .map_err(|error| error.into_compilation_error_pair())?; self.add_items(items, generated_items, location); @@ -474,7 +475,7 @@ impl<'context> Elaborator<'context> { Some(DependencyId::Function(function)) => Some(function), _ => None, }; - Interpreter::new(self, self.crate_id, current_function) + Interpreter::new(self, self.crate_id, current_function, self.pedantic_solving) } pub(super) fn debug_comptime T>( diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs index c24cc1a9c86..73b8ef6a6bd 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/expressions.rs @@ -927,7 +927,7 @@ impl<'context> Elaborator<'context> { }; let location = Location::new(span, self.file); - match value.into_expression(self.interner, location) { + match value.into_expression(self, location) { Ok(new_expr) => { // At this point the Expression was already elaborated and we got a Value. // We'll elaborate this value turned into Expression to inline it and get diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs index c54f9355250..d664f856262 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/mod.rs @@ -181,6 +181,9 @@ pub struct Elaborator<'context> { /// like `Foo { inner: 5 }`: in that case we already elaborated the code that led to /// that comptime value and any visibility errors were already reported. silence_field_visibility_errors: usize, + + /// Use pedantic ACVM solving + pedantic_solving: bool, } #[derive(Default)] @@ -198,6 +201,7 @@ struct FunctionContext { } impl<'context> Elaborator<'context> { + #[allow(clippy::too_many_arguments)] pub fn new( interner: &'context mut NodeInterner, def_maps: &'context mut DefMaps, @@ -206,6 +210,7 @@ impl<'context> Elaborator<'context> { crate_id: CrateId, debug_comptime_in_file: Option, interpreter_call_stack: im::Vector, + pedantic_solving: bool, ) -> Self { Self { scopes: ScopeForest::default(), @@ -233,6 +238,7 @@ impl<'context> Elaborator<'context> { interpreter_call_stack, in_comptime_context: false, silence_field_visibility_errors: 0, + pedantic_solving, } } @@ -240,6 +246,7 @@ impl<'context> Elaborator<'context> { context: &'context mut Context, crate_id: CrateId, debug_comptime_in_file: Option, + pedantic_solving: bool, ) -> Self { Self::new( &mut context.def_interner, @@ -249,6 +256,7 @@ impl<'context> Elaborator<'context> { crate_id, debug_comptime_in_file, im::Vector::new(), + pedantic_solving, ) } @@ -257,8 +265,16 @@ impl<'context> Elaborator<'context> { crate_id: CrateId, items: CollectedItems, debug_comptime_in_file: Option, + pedantic_solving: bool, ) -> Vec<(CompilationError, FileId)> { - Self::elaborate_and_return_self(context, crate_id, items, debug_comptime_in_file).errors + Self::elaborate_and_return_self( + context, + crate_id, + items, + debug_comptime_in_file, + pedantic_solving, + ) + .errors } pub fn elaborate_and_return_self( @@ -266,8 +282,10 @@ impl<'context> Elaborator<'context> { crate_id: CrateId, items: CollectedItems, debug_comptime_in_file: Option, + pedantic_solving: bool, ) -> Self { - let mut this = Self::from_context(context, crate_id, debug_comptime_in_file); + let mut this = + Self::from_context(context, crate_id, debug_comptime_in_file, pedantic_solving); this.elaborate_items(items); this.check_and_pop_function_context(); this @@ -332,6 +350,12 @@ impl<'context> Elaborator<'context> { self.elaborate_functions(functions); } + for (trait_id, unresolved_trait) in items.traits { + self.current_trait = Some(trait_id); + self.elaborate_functions(unresolved_trait.fns_with_default_impl); + } + self.current_trait = None; + for impls in items.impls.into_values() { self.elaborate_impls(impls); } @@ -454,9 +478,10 @@ impl<'context> Elaborator<'context> { self.add_trait_constraints_to_scope(&func_meta); let (hir_func, body_type) = match kind { - FunctionKind::Builtin | FunctionKind::LowLevel | FunctionKind::Oracle => { - (HirFunction::empty(), Type::Error) - } + FunctionKind::Builtin + | FunctionKind::LowLevel + | FunctionKind::Oracle + | FunctionKind::TraitFunctionWithoutBody => (HirFunction::empty(), Type::Error), FunctionKind::Normal => { let (block, body_type) = self.elaborate_block(body); let expr_id = self.intern_expr(block, body_span); @@ -476,11 +501,7 @@ impl<'context> Elaborator<'context> { // when multiple impls are available. Instead we default first to choose the Field or u64 impl. self.check_and_pop_function_context(); - // Now remove all the `where` clause constraints we added - for constraint in &func_meta.trait_constraints { - self.interner - .remove_assumed_trait_implementations_for_trait(constraint.trait_bound.trait_id); - } + self.remove_trait_constraints_from_scope(&func_meta); let func_scope_tree = self.scopes.end_function(); @@ -1001,6 +1022,32 @@ impl<'context> Elaborator<'context> { constraint.trait_bound.trait_id, ); } + + // Also assume `self` implements the current trait if we are inside a trait definition + if let Some(trait_id) = self.current_trait { + let the_trait = self.interner.get_trait(trait_id); + let constraint = the_trait.as_constraint(the_trait.name.span()); + let self_type = + self.self_type.clone().expect("Expected a self type if there's a current trait"); + self.add_trait_bound_to_scope( + func_meta, + &self_type, + &constraint.trait_bound, + constraint.trait_bound.trait_id, + ); + } + } + + fn remove_trait_constraints_from_scope(&mut self, func_meta: &FuncMeta) { + for constraint in &func_meta.trait_constraints { + self.interner + .remove_assumed_trait_implementations_for_trait(constraint.trait_bound.trait_id); + } + + // Also remove the assumed trait implementation for `self` if this is a trait definition + if let Some(trait_id) = self.current_trait { + self.interner.remove_assumed_trait_implementations_for_trait(trait_id); + } } fn add_trait_bound_to_scope( diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs index 0cf53fad0e4..f9f6fff56e0 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/path_resolution.rs @@ -286,7 +286,8 @@ impl<'context> Elaborator<'context> { // Check if namespace let found_ns = if current_module_id_is_struct { - match self.resolve_struct_function(starting_module, current_module, current_ident) { + match self.resolve_struct_function(importing_module, current_module, current_ident) + { StructMethodLookupResult::NotFound(vec) => { if vec.is_empty() { return Err(PathResolutionError::Unresolved(current_ident.clone())); @@ -306,7 +307,7 @@ impl<'context> Elaborator<'context> { StructMethodLookupResult::FoundStructMethod(per_ns) => per_ns, StructMethodLookupResult::FoundTraitMethod(per_ns, trait_id) => { let trait_ = self.interner.get_trait(trait_id); - self.usage_tracker.mark_as_used(starting_module, &trait_.name); + self.usage_tracker.mark_as_used(importing_module, &trait_.name); per_ns } StructMethodLookupResult::FoundOneTraitMethodButNotInScope( @@ -324,7 +325,7 @@ impl<'context> Elaborator<'context> { StructMethodLookupResult::FoundMultipleTraitMethods(vec) => { let traits = vecmap(vec, |trait_id| { let trait_ = self.interner.get_trait(trait_id); - self.usage_tracker.mark_as_used(starting_module, &trait_.name); + self.usage_tracker.mark_as_used(importing_module, &trait_.name); self.fully_qualified_trait_path(trait_) }); return Err(PathResolutionError::MultipleTraitsInScope { @@ -382,7 +383,7 @@ impl<'context> Elaborator<'context> { fn resolve_struct_function( &self, - starting_module_id: ModuleId, + importing_module_id: ModuleId, current_module: &ModuleData, ident: &Ident, ) -> StructMethodLookupResult { @@ -402,7 +403,7 @@ impl<'context> Elaborator<'context> { } // Otherwise, the function could be defined in zero, one or more traits. - let starting_module = self.get_module(starting_module_id); + let starting_module = self.get_module(importing_module_id); // Gather a list of items for which their trait is in scope. let mut results = Vec::new(); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs index 133473219f6..242f5f0b496 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/patterns.rs @@ -856,7 +856,7 @@ impl<'context> Elaborator<'context> { let impl_kind = match method { HirMethodReference::FuncId(_) => ImplKind::NotATraitMethod, - HirMethodReference::TraitMethodId(method_id, generics) => { + HirMethodReference::TraitMethodId(method_id, generics, _) => { let mut constraint = self.interner.get_trait(method_id.trait_id).as_constraint(span); constraint.trait_bound.trait_generics = generics; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs index e1be45927ca..a10939dde16 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/traits.rs @@ -28,6 +28,11 @@ impl<'context> Elaborator<'context> { self.recover_generics(|this| { this.current_trait = Some(*trait_id); + let the_trait = this.interner.get_trait(*trait_id); + let self_typevar = the_trait.self_type_typevar.clone(); + let self_type = Type::TypeVariable(self_typevar.clone()); + this.self_type = Some(self_type.clone()); + let resolved_generics = this.interner.get_trait(*trait_id).generics.clone(); this.add_existing_generics( &unresolved_trait.trait_def.generics, @@ -48,12 +53,15 @@ impl<'context> Elaborator<'context> { .add_trait_dependency(DependencyId::Trait(bound.trait_id), *trait_id); } + this.interner.update_trait(*trait_id, |trait_def| { + trait_def.set_trait_bounds(resolved_trait_bounds); + trait_def.set_where_clause(where_clause); + }); + let methods = this.resolve_trait_methods(*trait_id, unresolved_trait); this.interner.update_trait(*trait_id, |trait_def| { trait_def.set_methods(methods); - trait_def.set_trait_bounds(resolved_trait_bounds); - trait_def.set_where_clause(where_clause); }); }); @@ -94,7 +102,7 @@ impl<'context> Elaborator<'context> { parameters, return_type, where_clause, - body: _, + body, is_unconstrained, visibility: _, is_comptime: _, @@ -103,7 +111,6 @@ impl<'context> Elaborator<'context> { self.recover_generics(|this| { let the_trait = this.interner.get_trait(trait_id); let self_typevar = the_trait.self_type_typevar.clone(); - let self_type = Type::TypeVariable(self_typevar.clone()); let name_span = the_trait.name.span(); this.add_existing_generic( @@ -115,9 +122,12 @@ impl<'context> Elaborator<'context> { span: name_span, }, ); - this.self_type = Some(self_type.clone()); let func_id = unresolved_trait.method_ids[&name.0.contents]; + let mut where_clause = where_clause.to_vec(); + + // Attach any trait constraints on the trait to the function + where_clause.extend(unresolved_trait.trait_def.where_clause.clone()); this.resolve_trait_function( trait_id, @@ -127,6 +137,8 @@ impl<'context> Elaborator<'context> { parameters, return_type, where_clause, + body, + unresolved_trait.trait_def.visibility, func_id, ); @@ -188,29 +200,45 @@ impl<'context> Elaborator<'context> { generics: &UnresolvedGenerics, parameters: &[(Ident, UnresolvedType)], return_type: &FunctionReturnType, - where_clause: &[UnresolvedTraitConstraint], + where_clause: Vec, + body: &Option, + trait_visibility: ItemVisibility, func_id: FuncId, ) { let old_generic_count = self.generics.len(); self.scopes.start_function(); - let kind = FunctionKind::Normal; + let has_body = body.is_some(); + + let body = match body { + Some(body) => body.clone(), + None => BlockExpression { statements: Vec::new() }, + }; + let kind = + if has_body { FunctionKind::Normal } else { FunctionKind::TraitFunctionWithoutBody }; let mut def = FunctionDefinition::normal( name, is_unconstrained, generics, parameters, - &BlockExpression { statements: Vec::new() }, + body, where_clause, return_type, ); - // Trait functions are always public - def.visibility = ItemVisibility::Public; + + // Trait functions always have the same visibility as the trait they are in + def.visibility = trait_visibility; let mut function = NoirFunction { kind, def }; self.define_function_meta(&mut function, func_id, Some(trait_id)); - self.elaborate_function(func_id); + + // Here we elaborate functions without a body, mainly to check the arguments and return types. + // Later on we'll elaborate functions with a body by fully type-checking them. + if !has_body { + self.elaborate_function(func_id); + } + let _ = self.scopes.end_function(); // Don't check the scope tree for unused variables, they can't be used in a declaration anyway. self.generics.truncate(old_generic_count); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs index 550ee41fbd4..c3375fc35ad 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/elaborator/types.rs @@ -566,12 +566,17 @@ impl<'context> Elaborator<'context> { } // this resolves Self::some_static_method, inside an impl block (where we don't have a concrete self_type) + // or inside a trait default method. // // Returns the trait method, trait constraint, and whether the impl is assumed to exist by a where clause or not // E.g. `t.method()` with `where T: Foo` in scope will return `(Foo::method, T, vec![Bar])` fn resolve_trait_static_method_by_self(&mut self, path: &Path) -> Option { - let trait_impl = self.current_trait_impl?; - let trait_id = self.interner.try_get_trait_implementation(trait_impl)?.borrow().trait_id; + let trait_id = if let Some(current_trait) = self.current_trait { + current_trait + } else { + let trait_impl = self.current_trait_impl?; + self.interner.try_get_trait_implementation(trait_impl)?.borrow().trait_id + }; if path.kind == PathKind::Plain && path.segments.len() == 2 { let name = &path.segments[0].ident.0.contents; @@ -1395,6 +1400,25 @@ impl<'context> Elaborator<'context> { }; let func_meta = self.interner.function_meta(&func_id); + // If inside a trait method, check if it's a method on `self` + if let Some(trait_id) = func_meta.trait_id { + if Some(object_type) == self.self_type.as_ref() { + let the_trait = self.interner.get_trait(trait_id); + let constraint = the_trait.as_constraint(the_trait.name.span()); + if let Some(HirMethodReference::TraitMethodId(method_id, generics, _)) = self + .lookup_method_in_trait( + the_trait, + method_name, + &constraint.trait_bound, + the_trait.id, + ) + { + // If it is, it's an assumed trait + return Some(HirMethodReference::TraitMethodId(method_id, generics, true)); + } + } + } + for constraint in &func_meta.trait_constraints { if *object_type == constraint.typ { if let Some(the_trait) = @@ -1432,6 +1456,7 @@ impl<'context> Elaborator<'context> { return Some(HirMethodReference::TraitMethodId( trait_method, trait_bound.trait_generics.clone(), + false, )); } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index e9e37243e07..12b99818c46 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -67,6 +67,9 @@ pub struct Interpreter<'local, 'interner> { /// Stateful bigint calculator. bigint_solver: BigIntSolverWithId, + + /// Use pedantic ACVM solving + pedantic_solving: bool, } #[allow(unused)] @@ -75,14 +78,17 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { elaborator: &'local mut Elaborator<'interner>, crate_id: CrateId, current_function: Option, + pedantic_solving: bool, ) -> Self { + let bigint_solver = BigIntSolverWithId::with_pedantic_solving(pedantic_solving); Self { elaborator, crate_id, current_function, bound_generics: Vec::new(), in_loop: false, - bigint_solver: BigIntSolverWithId::default(), + bigint_solver, + pedantic_solving, } } @@ -1322,7 +1328,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { let bindings = unwrap_rc(bindings); let mut result = self.call_function(function_id, arguments, bindings, location)?; if call.is_macro_call { - let expr = result.into_expression(self.elaborator.interner, location)?; + let expr = result.into_expression(self.elaborator, location)?; let expr = self.elaborate_in_function(self.current_function, |elaborator| { elaborator.elaborate_expression(expr).0 }); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs index 20cddcf43b4..8b2132e8df0 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin.rs @@ -23,6 +23,7 @@ use crate::{ Pattern, Signedness, Statement, StatementKind, UnaryOp, UnresolvedType, UnresolvedTypeData, Visibility, }, + elaborator::Elaborator, hir::{ comptime::{ errors::IResult, @@ -32,9 +33,11 @@ use crate::{ def_collector::dc_crate::CollectedItems, def_map::ModuleDefId, }, - hir_def::expr::{HirExpression, HirLiteral}, - hir_def::function::FunctionBody, - hir_def::{self}, + hir_def::{ + self, + expr::{HirExpression, HirLiteral}, + function::FunctionBody, + }, node_interner::{DefinitionKind, NodeInterner, TraitImplKind}, parser::{Parser, StatementOrExpressionOrLValue}, token::{Attribute, Token}, @@ -160,7 +163,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { "modulus_le_bits" => modulus_le_bits(arguments, location), "modulus_le_bytes" => modulus_le_bytes(arguments, location), "modulus_num_bits" => modulus_num_bits(arguments, location), - "quoted_as_expr" => quoted_as_expr(interner, arguments, return_type, location), + "quoted_as_expr" => quoted_as_expr(self.elaborator, arguments, return_type, location), "quoted_as_module" => quoted_as_module(self, arguments, return_type, location), "quoted_as_trait_constraint" => quoted_as_trait_constraint(self, arguments, location), "quoted_as_type" => quoted_as_type(self, arguments, location), @@ -688,15 +691,19 @@ fn slice_insert( // fn as_expr(quoted: Quoted) -> Option fn quoted_as_expr( - interner: &NodeInterner, + elaborator: &mut Elaborator, arguments: Vec<(Value, Location)>, return_type: Type, location: Location, ) -> IResult { let argument = check_one_argument(arguments, location)?; - let result = - parse(interner, argument, Parser::parse_statement_or_expression_or_lvalue, "an expression"); + let result = parse( + elaborator, + argument, + Parser::parse_statement_or_expression_or_lvalue, + "an expression", + ); let value = result.ok().map( @@ -721,13 +728,9 @@ fn quoted_as_module( ) -> IResult { let argument = check_one_argument(arguments, location)?; - let path = parse( - interpreter.elaborator.interner, - argument, - Parser::parse_path_no_turbofish_or_error, - "a path", - ) - .ok(); + let path = + parse(interpreter.elaborator, argument, Parser::parse_path_no_turbofish_or_error, "a path") + .ok(); let option_value = path.and_then(|path| { let module = interpreter .elaborate_in_function(interpreter.current_function, |elaborator| { @@ -747,7 +750,7 @@ fn quoted_as_trait_constraint( ) -> IResult { let argument = check_one_argument(arguments, location)?; let trait_bound = parse( - interpreter.elaborator.interner, + interpreter.elaborator, argument, Parser::parse_trait_bound_or_error, "a trait constraint", @@ -768,8 +771,7 @@ fn quoted_as_type( location: Location, ) -> IResult { let argument = check_one_argument(arguments, location)?; - let typ = - parse(interpreter.elaborator.interner, argument, Parser::parse_type_or_error, "a type")?; + let typ = parse(interpreter.elaborator, argument, Parser::parse_type_or_error, "a type")?; let typ = interpreter .elaborate_in_function(interpreter.current_function, |elab| elab.resolve_type(typ)); Ok(Value::Type(typ)) @@ -2476,7 +2478,7 @@ fn function_def_set_parameters( )?; let parameter_type = get_type((tuple.pop().unwrap(), parameters_argument_location))?; let parameter_pattern = parse( - interpreter.elaborator.interner, + interpreter.elaborator, (tuple.pop().unwrap(), parameters_argument_location), Parser::parse_pattern_or_error, "a pattern", @@ -2593,12 +2595,11 @@ fn module_add_item( ) -> IResult { let (self_argument, item) = check_two_arguments(arguments, location)?; let module_id = get_module(self_argument)?; - let module_data = interpreter.elaborator.get_module(module_id); let parser = Parser::parse_top_level_items; - let top_level_statements = - parse(interpreter.elaborator.interner, item, parser, "a top-level item")?; + let top_level_statements = parse(interpreter.elaborator, item, parser, "a top-level item")?; + let module_data = interpreter.elaborator.get_module(module_id); interpreter.elaborate_in_module(module_id, module_data.location.file, |elaborator| { let mut generated_items = CollectedItems::default(); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs index cf90aab32e0..a3f84a00bfb 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/builtin/builtin_helpers.rs @@ -5,10 +5,11 @@ use acvm::FieldElement; use iter_extended::try_vecmap; use noirc_errors::Location; +use crate::elaborator::Elaborator; use crate::hir::comptime::display::tokens_to_string; use crate::hir::comptime::value::add_token_spans; use crate::lexer::Lexer; -use crate::parser::Parser; +use crate::parser::{Parser, ParserError}; use crate::{ ast::{ BlockExpression, ExpressionKind, Ident, IntegerBitSize, LValue, Pattern, Signedness, @@ -493,7 +494,7 @@ pub(super) fn lex(input: &str) -> Vec { } pub(super) fn parse<'a, T, F>( - interner: &NodeInterner, + elaborator: &mut Elaborator, (value, location): (Value, Location), parser: F, rule: &'static str, @@ -503,7 +504,12 @@ where { let tokens = get_quoted((value, location))?; let quoted = add_token_spans(tokens.clone(), location.span); - parse_tokens(tokens, quoted, interner, location, parser, rule) + let (result, warnings) = + parse_tokens(tokens, quoted, elaborator.interner, location, parser, rule)?; + for warning in warnings { + elaborator.errors.push((warning.into(), location.file)); + } + Ok(result) } pub(super) fn parse_tokens<'a, T, F>( @@ -513,7 +519,7 @@ pub(super) fn parse_tokens<'a, T, F>( location: Location, parsing_function: F, rule: &'static str, -) -> IResult +) -> IResult<(T, Vec)> where F: FnOnce(&mut Parser<'a>) -> T, { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs index 99cc11ecd2a..0221280ae1b 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs @@ -40,6 +40,7 @@ impl<'local, 'context> Interpreter<'local, 'context> { arguments, return_type, location, + self.pedantic_solving, ) } } @@ -52,6 +53,7 @@ fn call_foreign( args: Vec<(Value, Location)>, return_type: Type, location: Location, + pedantic_solving: bool, ) -> IResult { use BlackBoxFunc::*; @@ -79,9 +81,11 @@ fn call_foreign( location, acvm::blackbox_solver::ecdsa_secp256r1_verify, ), - "embedded_curve_add" => embedded_curve_add(args, location), - "multi_scalar_mul" => multi_scalar_mul(interner, args, location), - "poseidon2_permutation" => poseidon2_permutation(interner, args, location), + "embedded_curve_add" => embedded_curve_add(args, location, pedantic_solving), + "multi_scalar_mul" => multi_scalar_mul(interner, args, location, pedantic_solving), + "poseidon2_permutation" => { + poseidon2_permutation(interner, args, location, pedantic_solving) + } "keccakf1600" => keccakf1600(interner, args, location), "range" => apply_range_constraint(args, location), "sha256_compression" => sha256_compression(interner, args, location), @@ -269,13 +273,17 @@ fn ecdsa_secp256_verify( /// point2: EmbeddedCurvePoint, /// ) -> [Field; 3] /// ``` -fn embedded_curve_add(arguments: Vec<(Value, Location)>, location: Location) -> IResult { +fn embedded_curve_add( + arguments: Vec<(Value, Location)>, + location: Location, + pedantic_solving: bool, +) -> IResult { let (point1, point2) = check_two_arguments(arguments, location)?; let (p1x, p1y, p1inf) = get_embedded_curve_point(point1)?; let (p2x, p2y, p2inf) = get_embedded_curve_point(point2)?; - let (x, y, inf) = Bn254BlackBoxSolver + let (x, y, inf) = Bn254BlackBoxSolver(pedantic_solving) .ec_add(&p1x, &p1y, &p1inf.into(), &p2x, &p2y, &p2inf.into()) .map_err(|e| InterpreterError::BlackBoxError(e, location))?; @@ -292,6 +300,7 @@ fn multi_scalar_mul( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, + pedantic_solving: bool, ) -> IResult { let (points, scalars) = check_two_arguments(arguments, location)?; @@ -306,7 +315,7 @@ fn multi_scalar_mul( scalars_hi.push(hi); } - let (x, y, inf) = Bn254BlackBoxSolver + let (x, y, inf) = Bn254BlackBoxSolver(pedantic_solving) .multi_scalar_mul(&points, &scalars_lo, &scalars_hi) .map_err(|e| InterpreterError::BlackBoxError(e, location))?; @@ -318,13 +327,14 @@ fn poseidon2_permutation( interner: &mut NodeInterner, arguments: Vec<(Value, Location)>, location: Location, + pedantic_solving: bool, ) -> IResult { let (input, state_length) = check_two_arguments(arguments, location)?; let (input, typ) = get_array_map(interner, input, get_field)?; let state_length = get_u32(state_length)?; - let fields = Bn254BlackBoxSolver + let fields = Bn254BlackBoxSolver(pedantic_solving) .poseidon2_permutation(&input, state_length) .map_err(|error| InterpreterError::BlackBoxError(error, location))?; @@ -435,6 +445,7 @@ mod tests { for blackbox in BlackBoxFunc::iter() { let name = blackbox.name(); + let pedantic_solving = true; match call_foreign( interpreter.elaborator.interner, &mut interpreter.bigint_solver, @@ -442,6 +453,7 @@ mod tests { Vec::new(), Type::Unit, no_location, + pedantic_solving, ) { Ok(_) => { // Exists and works with no args (unlikely) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/tests.rs index 2d3bf928917..ce8d43e56d5 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/tests.rs @@ -58,8 +58,14 @@ pub(crate) fn with_interpreter( let main = context.get_main_function(&krate).expect("Expected 'main' function"); - let mut elaborator = - Elaborator::elaborate_and_return_self(&mut context, krate, collector.items, None); + let pedantic_solving = true; + let mut elaborator = Elaborator::elaborate_and_return_self( + &mut context, + krate, + collector.items, + None, + pedantic_solving, + ); let errors = elaborator.errors.clone(); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs index e5c55e2b0be..b5496507e80 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/comptime/value.rs @@ -12,6 +12,7 @@ use crate::{ IntegerBitSize, LValue, Literal, Path, Pattern, Signedness, Statement, StatementKind, UnresolvedType, UnresolvedTypeData, }, + elaborator::Elaborator, hir::{def_map::ModuleId, type_check::generics::TraitGenerics}, hir_def::expr::{ HirArrayLiteral, HirConstructorExpression, HirExpression, HirIdent, HirLambda, HirLiteral, @@ -158,7 +159,7 @@ impl Value { pub(crate) fn into_expression( self, - interner: &mut NodeInterner, + elaborator: &mut Elaborator, location: Location, ) -> IResult { let kind = match self { @@ -212,22 +213,23 @@ impl Value { ExpressionKind::Literal(Literal::Str(unwrap_rc(value))) } Value::Function(id, typ, bindings) => { - let id = interner.function_definition_id(id); + let id = elaborator.interner.function_definition_id(id); let impl_kind = ImplKind::NotATraitMethod; let ident = HirIdent { location, id, impl_kind }; - let expr_id = interner.push_expr(HirExpression::Ident(ident, None)); - interner.push_expr_location(expr_id, location.span, location.file); - interner.push_expr_type(expr_id, typ); - interner.store_instantiation_bindings(expr_id, unwrap_rc(bindings)); + let expr_id = elaborator.interner.push_expr(HirExpression::Ident(ident, None)); + elaborator.interner.push_expr_location(expr_id, location.span, location.file); + elaborator.interner.push_expr_type(expr_id, typ); + elaborator.interner.store_instantiation_bindings(expr_id, unwrap_rc(bindings)); ExpressionKind::Resolved(expr_id) } Value::Tuple(fields) => { - let fields = try_vecmap(fields, |field| field.into_expression(interner, location))?; + let fields = + try_vecmap(fields, |field| field.into_expression(elaborator, location))?; ExpressionKind::Tuple(fields) } Value::Struct(fields, typ) => { let fields = try_vecmap(fields, |(name, field)| { - let field = field.into_expression(interner, location)?; + let field = field.into_expression(elaborator, location)?; Ok((Ident::new(unwrap_rc(name), location.span), field)) })?; @@ -246,12 +248,12 @@ impl Value { } Value::Array(elements, _) => { let elements = - try_vecmap(elements, |element| element.into_expression(interner, location))?; + try_vecmap(elements, |element| element.into_expression(elaborator, location))?; ExpressionKind::Literal(Literal::Array(ArrayLiteral::Standard(elements))) } Value::Slice(elements, _) => { let elements = - try_vecmap(elements, |element| element.into_expression(interner, location))?; + try_vecmap(elements, |element| element.into_expression(elaborator, location))?; ExpressionKind::Literal(Literal::Slice(ArrayLiteral::Standard(elements))) } Value::Quoted(tokens) => { @@ -262,12 +264,18 @@ impl Value { let parser = Parser::for_tokens(tokens_to_parse); return match parser.parse_result(Parser::parse_expression_or_error) { - Ok(expr) => Ok(expr), + Ok((expr, warnings)) => { + for warning in warnings { + elaborator.errors.push((warning.into(), location.file)); + } + + Ok(expr) + } Err(mut errors) => { let error = errors.swap_remove(0); let file = location.file; let rule = "an expression"; - let tokens = tokens_to_string(tokens, interner); + let tokens = tokens_to_string(tokens, elaborator.interner); Err(InterpreterError::FailedToParseMacro { error, file, tokens, rule }) } }; @@ -293,7 +301,7 @@ impl Value { | Value::Closure(..) | Value::ModuleDefinition(_) => { let typ = self.get_type().into_owned(); - let value = self.display(interner).to_string(); + let value = self.display(elaborator.interner).to_string(); return Err(InterpreterError::CannotInlineMacro { typ, value, location }); } }; @@ -533,16 +541,16 @@ impl Value { pub(crate) fn into_top_level_items( self, location: Location, - interner: &NodeInterner, + elaborator: &mut Elaborator, ) -> IResult> { let parser = Parser::parse_top_level_items; match self { Value::Quoted(tokens) => { - parse_tokens(tokens, interner, parser, location, "top-level item") + parse_tokens(tokens, elaborator, parser, location, "top-level item") } _ => { let typ = self.get_type().into_owned(); - let value = self.display(interner).to_string(); + let value = self.display(elaborator.interner).to_string(); Err(InterpreterError::CannotInlineMacro { value, typ, location }) } } @@ -556,7 +564,7 @@ pub(crate) fn unwrap_rc(rc: Rc) -> T { fn parse_tokens<'a, T, F>( tokens: Rc>, - interner: &NodeInterner, + elaborator: &mut Elaborator, parsing_function: F, location: Location, rule: &'static str, @@ -566,11 +574,16 @@ where { let parser = Parser::for_tokens(add_token_spans(tokens.clone(), location.span)); match parser.parse_result(parsing_function) { - Ok(expr) => Ok(expr), + Ok((expr, warnings)) => { + for warning in warnings { + elaborator.errors.push((warning.into(), location.file)); + } + Ok(expr) + } Err(mut errors) => { let error = errors.swap_remove(0); let file = location.file; - let tokens = tokens_to_string(tokens, interner); + let tokens = tokens_to_string(tokens, elaborator.interner); Err(InterpreterError::FailedToParseMacro { error, file, tokens, rule }) } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index b59c09eccaa..9081cb1cccd 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -275,6 +275,7 @@ impl DefCollector { ast: SortedModule, root_file_id: FileId, debug_comptime_in_file: Option<&str>, + pedantic_solving: bool, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; let crate_id = def_map.krate; @@ -287,7 +288,12 @@ impl DefCollector { let crate_graph = &context.crate_graph[crate_id]; for dep in crate_graph.dependencies.clone() { - errors.extend(CrateDefMap::collect_defs(dep.crate_id, context, debug_comptime_in_file)); + errors.extend(CrateDefMap::collect_defs( + dep.crate_id, + context, + debug_comptime_in_file, + pedantic_solving, + )); let dep_def_map = context.def_map(&dep.crate_id).expect("ice: def map was just created"); @@ -452,8 +458,13 @@ impl DefCollector { }) }); - let mut more_errors = - Elaborator::elaborate(context, crate_id, def_collector.items, debug_comptime_in_file); + let mut more_errors = Elaborator::elaborate( + context, + crate_id, + def_collector.items, + debug_comptime_in_file, + pedantic_solving, + ); errors.append(&mut more_errors); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs index eff48ce22a6..ec13cb2db15 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_collector/dc_mod.rs @@ -482,12 +482,14 @@ impl<'a> ModCollector<'a> { is_comptime, } => { let func_id = context.def_interner.push_empty_fn(); - method_ids.insert(name.to_string(), func_id); + if !method_ids.contains_key(&name.0.contents) { + method_ids.insert(name.to_string(), func_id); + } let location = Location::new(name.span(), self.file_id); let modifiers = FunctionModifiers { name: name.to_string(), - visibility: ItemVisibility::Public, + visibility: trait_definition.visibility, // TODO(Maddiaa): Investigate trait implementations with attributes see: https://github.com/noir-lang/noir/issues/2629 attributes: crate::token::Attributes::empty(), is_unconstrained: *is_unconstrained, @@ -521,8 +523,8 @@ impl<'a> ModCollector<'a> { *is_unconstrained, generics, parameters, - body, - where_clause, + body.clone(), + where_clause.clone(), return_type, )); unresolved_functions.push_fn( diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs index 123bf3a910c..f7fc6ca08ea 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/def_map/mod.rs @@ -78,6 +78,7 @@ impl CrateDefMap { crate_id: CrateId, context: &mut Context, debug_comptime_in_file: Option<&str>, + pedantic_solving: bool, ) -> Vec<(CompilationError, FileId)> { // Check if this Crate has already been compiled // XXX: There is probably a better alternative for this. @@ -120,6 +121,7 @@ impl CrateDefMap { ast, root_file_id, debug_comptime_in_file, + pedantic_solving, )); errors.extend( diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs index 8817904c6f5..a4b3c1b9c07 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/errors.rs @@ -607,7 +607,7 @@ impl<'a> From<&'a ResolverError> for Diagnostic { }, ResolverError::UnsupportedNumericGenericType(err) => err.into(), ResolverError::TypeIsMorePrivateThenItem { typ, item, span } => { - Diagnostic::simple_warning( + Diagnostic::simple_error( format!("Type `{typ}` is more private than item `{item}`"), String::new(), *span, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/visibility.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/visibility.rs index c2fe887fe62..5e6dffa56e4 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/visibility.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/visibility.rs @@ -1,5 +1,5 @@ use crate::graph::CrateId; -use crate::node_interner::{FuncId, NodeInterner, StructId}; +use crate::node_interner::{FuncId, NodeInterner, StructId, TraitId}; use crate::Type; use std::collections::BTreeMap; @@ -79,26 +79,47 @@ pub fn struct_member_is_visible( visibility: ItemVisibility, current_module_id: ModuleId, def_maps: &BTreeMap, +) -> bool { + type_member_is_visible(struct_id.module_id(), visibility, current_module_id, def_maps) +} + +pub fn trait_member_is_visible( + trait_id: TraitId, + visibility: ItemVisibility, + current_module_id: ModuleId, + def_maps: &BTreeMap, +) -> bool { + type_member_is_visible(trait_id.0, visibility, current_module_id, def_maps) +} + +fn type_member_is_visible( + type_module_id: ModuleId, + visibility: ItemVisibility, + current_module_id: ModuleId, + def_maps: &BTreeMap, ) -> bool { match visibility { ItemVisibility::Public => true, ItemVisibility::PublicCrate => { - struct_id.parent_module_id(def_maps).krate == current_module_id.krate + let type_parent_module_id = + type_module_id.parent(def_maps).expect("Expected parent module to exist"); + type_parent_module_id.krate == current_module_id.krate } ItemVisibility::Private => { - let struct_parent_module_id = struct_id.parent_module_id(def_maps); - if struct_parent_module_id.krate != current_module_id.krate { + let type_parent_module_id = + type_module_id.parent(def_maps).expect("Expected parent module to exist"); + if type_parent_module_id.krate != current_module_id.krate { return false; } - if struct_parent_module_id.local_id == current_module_id.local_id { + if type_parent_module_id.local_id == current_module_id.local_id { return true; } let def_map = &def_maps[¤t_module_id.krate]; module_descendent_of_target( def_map, - struct_parent_module_id.local_id, + type_parent_module_id.local_id, current_module_id.local_id, ) } @@ -115,35 +136,37 @@ pub fn method_call_is_visible( let modifiers = interner.function_modifiers(&func_id); match modifiers.visibility { ItemVisibility::Public => true, - ItemVisibility::PublicCrate => { - if object_type.is_primitive() { - current_module.krate.is_stdlib() - } else { - interner.function_module(func_id).krate == current_module.krate + ItemVisibility::PublicCrate | ItemVisibility::Private => { + let func_meta = interner.function_meta(&func_id); + if let Some(struct_id) = func_meta.struct_id { + return struct_member_is_visible( + struct_id, + modifiers.visibility, + current_module, + def_maps, + ); } - } - ItemVisibility::Private => { + + if let Some(trait_id) = func_meta.trait_id { + return trait_member_is_visible( + trait_id, + modifiers.visibility, + current_module, + def_maps, + ); + } + if object_type.is_primitive() { let func_module = interner.function_module(func_id); - item_in_module_is_visible( + return item_in_module_is_visible( def_maps, current_module, func_module, modifiers.visibility, - ) - } else { - let func_meta = interner.function_meta(&func_id); - if let Some(struct_id) = func_meta.struct_id { - struct_member_is_visible( - struct_id, - modifiers.visibility, - current_module, - def_maps, - ) - } else { - true - } + ); } + + true } } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/expr.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/expr.rs index e243fc88cff..9b3bf4962bb 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/expr.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/expr.rs @@ -209,14 +209,14 @@ pub enum HirMethodReference { /// Or a method can come from a Trait impl block, in which case /// the actual function called will depend on the instantiated type, /// which can be only known during monomorphization. - TraitMethodId(TraitMethodId, TraitGenerics), + TraitMethodId(TraitMethodId, TraitGenerics, bool /* assumed */), } impl HirMethodReference { pub fn func_id(&self, interner: &NodeInterner) -> Option { match self { HirMethodReference::FuncId(func_id) => Some(*func_id), - HirMethodReference::TraitMethodId(method_id, _) => { + HirMethodReference::TraitMethodId(method_id, _, _) => { let id = interner.trait_method_id(*method_id); match &interner.try_definition(id)?.kind { DefinitionKind::Function(func_id) => Some(*func_id), @@ -246,7 +246,7 @@ impl HirMethodCallExpression { HirMethodReference::FuncId(func_id) => { (interner.function_definition_id(func_id), ImplKind::NotATraitMethod) } - HirMethodReference::TraitMethodId(method_id, trait_generics) => { + HirMethodReference::TraitMethodId(method_id, trait_generics, assumed) => { let id = interner.trait_method_id(method_id); let constraint = TraitConstraint { typ: object_type, @@ -256,7 +256,8 @@ impl HirMethodCallExpression { span: location.span, }, }; - (id, ImplKind::TraitMethod(TraitMethod { method_id, constraint, assumed: false })) + + (id, ImplKind::TraitMethod(TraitMethod { method_id, constraint, assumed })) } }; let func_var = HirIdent { location, id, impl_kind }; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs index db6c3507b15..aa04738733f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir_def/function.rs @@ -175,12 +175,13 @@ pub enum FunctionBody { impl FuncMeta { /// A stub function does not have a body. This includes Builtin, LowLevel, - /// and Oracle functions in addition to method declarations within a trait. + /// and Oracle functions in addition to method declarations within a trait + /// without a body. /// /// We don't check the return type of these functions since it will always have /// an empty body, and we don't check for unused parameters. pub fn is_stub(&self) -> bool { - self.kind.can_ignore_return_type() || self.trait_id.is_some() + self.kind.can_ignore_return_type() } pub fn function_signature(&self) -> FunctionSignature { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs index fbc80cc0ac9..ce855f62526 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -261,7 +261,7 @@ impl<'interner> Monomorphizer<'interner> { ); Definition::Builtin(opcode.to_string()) } - FunctionKind::Normal => { + FunctionKind::Normal | FunctionKind::TraitFunctionWithoutBody => { let id = self.queue_function(id, expr_id, typ, turbofish_generics, trait_method); Definition::Function(id) diff --git a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs index 1df2cd76721..3ed183df49c 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/node_interner.rs @@ -2,7 +2,6 @@ use std::borrow::Cow; use std::fmt; use std::hash::Hash; use std::marker::Copy; -use std::ops::Deref; use fm::FileId; use iter_extended::vecmap; @@ -1478,25 +1477,6 @@ impl NodeInterner { Ok(impl_kind) } - /// Given a `ObjectType: TraitId` pair, find all implementations without taking constraints into account or - /// applying any type bindings. Useful to look for a specific trait in a type that is used in a macro. - pub fn lookup_all_trait_implementations( - &self, - object_type: &Type, - trait_id: TraitId, - ) -> Vec<&TraitImplKind> { - let trait_impl = self.trait_implementation_map.get(&trait_id); - - let trait_impl = trait_impl.map(|trait_impl| { - let impls = trait_impl.iter().filter_map(|(typ, impl_kind)| match &typ { - Type::Forall(_, typ) => (typ.deref() == object_type).then_some(impl_kind), - _ => None, - }); - impls.collect() - }); - trait_impl.unwrap_or_default() - } - /// Similar to `lookup_trait_implementation` but does not apply any type bindings on success. /// On error returns either: /// - 1+ failing trait constraints, including the original. diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs index 32cacf5e4ed..1b16b911b18 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/errors.rs @@ -112,6 +112,8 @@ pub enum ParserErrorReason { DeprecatedAttributeExpectsAStringArgument, #[error("Unsafe block must start with a safety comment")] MissingSafetyComment, + #[error("Missing parameters for function definition")] + MissingParametersForFunctionDefinition, } /// Represents a parsing error, or a parsing error in the making. @@ -285,6 +287,13 @@ impl<'a> From<&'a ParserError> for Diagnostic { "Unsafe block must start with a safety comment: //@safety".into(), error.span, ), + ParserErrorReason::MissingParametersForFunctionDefinition => { + Diagnostic::simple_error( + "Missing parameters for function definition".into(), + "Add a parameter list: `()`".into(), + error.span, + ) + } other => Diagnostic::simple_error(format!("{other}"), String::new(), error.span), }, None => { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs index 9ce288c97fa..33c7a8aad51 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser.rs @@ -128,9 +128,12 @@ impl<'a> Parser<'a> { } /// Invokes `parsing_function` (`parsing_function` must be some `parse_*` method of the parser) - /// and returns the result if the parser has no errors, and if the parser consumed all tokens. + /// and returns the result (and any warnings) if the parser has no errors, and if the parser consumed all tokens. /// Otherwise returns the list of errors. - pub fn parse_result(mut self, parsing_function: F) -> Result> + pub fn parse_result( + mut self, + parsing_function: F, + ) -> Result<(T, Vec), Vec> where F: FnOnce(&mut Parser<'a>) -> T, { @@ -140,23 +143,14 @@ impl<'a> Parser<'a> { return Err(self.errors); } - if self.errors.is_empty() { - Ok(item) + let all_warnings = self.errors.iter().all(|error| error.is_warning()); + if all_warnings { + Ok((item, self.errors)) } else { Err(self.errors) } } - /// Invokes `parsing_function` (`parsing_function` must be some `parse_*` method of the parser) - /// and returns the result if the parser has no errors, and if the parser consumed all tokens. - /// Otherwise returns None. - pub fn parse_option(self, parsing_function: F) -> Option - where - F: FnOnce(&mut Parser<'a>) -> Option, - { - self.parse_result(parsing_function).unwrap_or_default() - } - /// Bumps this parser by one token. Returns the token that was previously the "current" token. fn bump(&mut self) -> SpannedToken { self.previous_token_span = self.current_token_span; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs index 438757b5d79..29e864200f3 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/function.rs @@ -94,6 +94,17 @@ impl<'a> Parser<'a> { let generics = self.parse_generics(); let parameters = self.parse_function_parameters(allow_self); + let parameters = match parameters { + Some(parameters) => parameters, + None => { + self.push_error( + ParserErrorReason::MissingParametersForFunctionDefinition, + name.span(), + ); + Vec::new() + } + }; + let (return_type, return_visibility) = if self.eat(Token::Arrow) { let visibility = self.parse_visibility(); (FunctionReturnType::Ty(self.parse_type_or_error()), visibility) @@ -131,14 +142,14 @@ impl<'a> Parser<'a> { /// FunctionParametersList = FunctionParameter ( ',' FunctionParameter )* ','? /// /// FunctionParameter = Visibility PatternOrSelf ':' Type - fn parse_function_parameters(&mut self, allow_self: bool) -> Vec { + fn parse_function_parameters(&mut self, allow_self: bool) -> Option> { if !self.eat_left_paren() { - return Vec::new(); + return None; } - self.parse_many("parameters", separated_by_comma_until_right_paren(), |parser| { + Some(self.parse_many("parameters", separated_by_comma_until_right_paren(), |parser| { parser.parse_function_parameter(allow_self) - }) + })) } fn parse_function_parameter(&mut self, allow_self: bool) -> Option { @@ -490,4 +501,16 @@ mod tests { assert!(noir_function.def.is_unconstrained); assert_eq!(noir_function.def.visibility, ItemVisibility::Public); } + + #[test] + fn parse_function_without_parentheses() { + let src = " + fn foo {} + ^^^ + "; + let (src, span) = get_source_with_error_span(src); + let (_, errors) = parse_program(&src); + let reason = get_single_error_reason(&errors, span); + assert!(matches!(reason, ParserErrorReason::MissingParametersForFunctionDefinition)); + } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/module.rs b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/module.rs index 263338863c0..da733168099 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/module.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/parser/parser/module.rs @@ -74,7 +74,6 @@ mod tests { fn parse_submodule() { let src = "mod foo { mod bar; }"; let (module, errors) = parse_program(src); - dbg!(&errors); expect_no_errors(&errors); assert_eq!(module.items.len(), 1); let item = &module.items[0]; @@ -90,7 +89,6 @@ mod tests { fn parse_contract() { let src = "contract foo {}"; let (module, errors) = parse_program(src); - dbg!(&errors); expect_no_errors(&errors); assert_eq!(module.items.len(), 1); let item = &module.items[0]; diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs index f29747431ea..dabfcfe9191 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests.rs @@ -114,6 +114,7 @@ pub(crate) fn get_program_with_maybe_parser_errors( }; let debug_comptime_in_file = None; + let pedantic_solving = true; // Now we want to populate the CrateDefMap using the DefCollector errors.extend(DefCollector::collect_crate_and_dependencies( @@ -122,6 +123,7 @@ pub(crate) fn get_program_with_maybe_parser_errors( program.clone().into_sorted(), root_file_id, debug_comptime_in_file, + pedantic_solving, )); } (program, context, errors) @@ -2842,6 +2844,21 @@ fn trait_constraint_on_tuple_type() { assert_no_errors(src); } +#[test] +fn trait_constraint_on_tuple_type_pub_crate() { + let src = r#" + pub(crate) trait Foo { + fn foo(self, x: A) -> bool; + } + + pub fn bar(x: (T, U), y: V) -> bool where (T, U): Foo { + x.foo(y) + } + + fn main() {}"#; + assert_no_errors(src); +} + #[test] fn incorrect_generic_count_on_struct_impl() { let src = r#" @@ -2940,7 +2957,7 @@ fn uses_self_type_inside_trait() { fn uses_self_type_in_trait_where_clause() { let src = r#" pub trait Trait { - fn trait_func() -> bool; + fn trait_func(self) -> bool; } pub trait Foo where Self: Trait { diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs index 89a049ebc9d..8256744e18f 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/metaprogramming.rs @@ -10,6 +10,7 @@ use crate::{ resolution::errors::ResolverError, type_check::TypeCheckError, }, + parser::ParserErrorReason, }; use super::{assert_no_errors, get_program_errors}; @@ -161,3 +162,33 @@ fn uses_correct_type_for_attribute_arguments() { "#; assert_no_errors(src); } + +#[test] +fn does_not_fail_to_parse_macro_on_parser_warning() { + let src = r#" + #[make_bar] + pub unconstrained fn foo() {} + + comptime fn make_bar(_: FunctionDefinition) -> Quoted { + quote { + pub fn bar() { + unsafe { + foo(); + } + } + } + } + + fn main() { + bar() + } + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::ParseError(parser_error) = &errors[0].0 else { + panic!("Expected a ParseError, got {:?}", errors[0].0); + }; + + assert!(matches!(parser_error.reason(), Some(ParserErrorReason::MissingSafetyComment))); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs index a5d1591e012..ba70daa297d 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/traits.rs @@ -593,7 +593,7 @@ fn trait_bounds_which_are_dependent_on_generic_types_are_resolved_correctly() { // Regression test for https://github.com/noir-lang/noir/issues/6420 let src = r#" trait Foo { - fn foo() -> Field; + fn foo(self) -> Field; } trait Bar: Foo { @@ -614,7 +614,8 @@ fn trait_bounds_which_are_dependent_on_generic_types_are_resolved_correctly() { where T: MarkerTrait, { - fn foo() -> Field { + fn foo(self) -> Field { + let _ = self; 42 } } @@ -717,7 +718,7 @@ fn calls_trait_function_if_it_is_in_scope() { } #[test] -fn calls_trait_function_it_is_only_candidate_in_scope() { +fn calls_trait_function_if_it_is_only_candidate_in_scope() { let src = r#" use private_mod::Foo; @@ -753,6 +754,36 @@ fn calls_trait_function_it_is_only_candidate_in_scope() { assert_no_errors(src); } +#[test] +fn calls_trait_function_if_it_is_only_candidate_in_scope_in_nested_module_using_super() { + let src = r#" + mod moo { + use super::public_mod::Foo; + + pub fn method() { + let _ = super::Bar::foo(); + } + } + + fn main() {} + + pub struct Bar {} + + pub mod public_mod { + pub trait Foo { + fn foo() -> i32; + } + + impl Foo for super::Bar { + fn foo() -> i32 { + 42 + } + } + } + "#; + assert_no_errors(src); +} + #[test] fn errors_if_trait_is_not_in_scope_for_function_call_and_there_are_multiple_candidates() { let src = r#" @@ -847,3 +878,65 @@ fn errors_if_multiple_trait_methods_are_in_scope() { traits.sort(); assert_eq!(traits, vec!["private_mod::Foo", "private_mod::Foo2"]); } + +#[test] +fn type_checks_trait_default_method_and_errors() { + let src = r#" + pub trait Foo { + fn foo(self) -> i32 { + let _ = self; + true + } + } + + fn main() {} + "#; + let errors = get_program_errors(src); + assert_eq!(errors.len(), 1); + + let CompilationError::TypeError(TypeCheckError::TypeMismatchWithSource { + expected, + actual, + .. + }) = &errors[0].0 + else { + panic!("Expected a type mismatch error, got {:?}", errors[0].0); + }; + + assert_eq!(expected.to_string(), "i32"); + assert_eq!(actual.to_string(), "bool"); +} + +#[test] +fn type_checks_trait_default_method_and_does_not_error() { + let src = r#" + pub trait Foo { + fn foo(self) -> i32 { + let _ = self; + 1 + } + } + + fn main() {} + "#; + assert_no_errors(src); +} + +#[test] +fn type_checks_trait_default_method_and_does_not_error_using_self() { + let src = r#" + pub trait Foo { + fn foo(self) -> i32 { + self.bar() + } + + fn bar(self) -> i32 { + let _ = self; + 1 + } + } + + fn main() {} + "#; + assert_no_errors(src); +} diff --git a/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs b/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs index 824a1de4c37..917394316cf 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/tests/visibility.rs @@ -229,6 +229,29 @@ fn errors_if_pub_trait_returns_private_struct() { assert_type_is_more_private_than_item_error(src, "Bar", "foo"); } +#[test] +fn does_not_error_if_trait_with_default_visibility_returns_struct_with_default_visibility() { + let src = r#" + struct Foo {} + + trait Bar { + fn bar(self) -> Foo; + } + + impl Bar for Foo { + fn bar(self) -> Foo { + self + } + } + + fn main() { + let foo = Foo {}; + let _ = foo.bar(); + } + "#; + assert_no_errors(src); +} + #[test] fn errors_if_trying_to_access_public_function_inside_private_module() { let src = r#" diff --git a/noir/noir-repo/docs/docs/noir/concepts/traits.md b/noir/noir-repo/docs/docs/noir/concepts/traits.md index b6c0a886eb0..1b232dcf8ab 100644 --- a/noir/noir-repo/docs/docs/noir/concepts/traits.md +++ b/noir/noir-repo/docs/docs/noir/concepts/traits.md @@ -582,3 +582,5 @@ pub trait Trait {} // This trait alias is now public pub trait Baz = Foo + Bar; ``` + +Trait methods have the same visibility as the trait they are in. diff --git a/noir/noir-repo/examples/codegen_verifier/codegen_verifier.sh b/noir/noir-repo/examples/codegen_verifier/codegen_verifier.sh index f224f74168f..9d1a6251dde 100755 --- a/noir/noir-repo/examples/codegen_verifier/codegen_verifier.sh +++ b/noir/noir-repo/examples/codegen_verifier/codegen_verifier.sh @@ -11,7 +11,7 @@ $BACKEND contract -o ./src/contract.sol # We now generate a proof and check whether the verifier contract will verify it. -nargo execute witness +nargo execute --pedantic-solving witness PROOF_PATH=./target/proof $BACKEND prove -b ./target/hello_world.json -w ./target/witness.gz -o $PROOF_PATH diff --git a/noir/noir-repo/examples/prove_and_verify/prove_and_verify.sh b/noir/noir-repo/examples/prove_and_verify/prove_and_verify.sh index df3ec3ff97a..411f5258caf 100755 --- a/noir/noir-repo/examples/prove_and_verify/prove_and_verify.sh +++ b/noir/noir-repo/examples/prove_and_verify/prove_and_verify.sh @@ -3,7 +3,7 @@ set -eu BACKEND=${BACKEND:-bb} -nargo execute witness +nargo execute --pedantic-solving witness # TODO: `bb` should create `proofs` directory if it doesn't exist. mkdir -p proofs @@ -11,4 +11,4 @@ $BACKEND prove -b ./target/hello_world.json -w ./target/witness.gz # TODO: backend should automatically generate vk if necessary. $BACKEND write_vk -b ./target/hello_world.json -$BACKEND verify -k ./target/vk -p ./proofs/proof \ No newline at end of file +$BACKEND verify -k ./target/vk -p ./proofs/proof diff --git a/noir/noir-repo/noir_stdlib/src/aes128.nr b/noir/noir-repo/noir_stdlib/src/aes128.nr index 7b0876b86f3..d781812eee9 100644 --- a/noir/noir-repo/noir_stdlib/src/aes128.nr +++ b/noir/noir-repo/noir_stdlib/src/aes128.nr @@ -1,4 +1,8 @@ #[foreign(aes128_encrypt)] // docs:start:aes128 -pub fn aes128_encrypt(input: [u8; N], iv: [u8; 16], key: [u8; 16]) -> [u8] {} +pub fn aes128_encrypt( + input: [u8; N], + iv: [u8; 16], + key: [u8; 16], +) -> [u8; N + 16 - N % 16] {} // docs:end:aes128 diff --git a/noir/noir-repo/test_programs/compile_success_empty/trait_default_method_uses_op/Nargo.toml b/noir/noir-repo/test_programs/compile_success_empty/trait_default_method_uses_op/Nargo.toml new file mode 100644 index 00000000000..d8b882c1696 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/trait_default_method_uses_op/Nargo.toml @@ -0,0 +1,7 @@ +[package] +name = "trait_default_method_uses_op" +type = "bin" +authors = [""] +compiler_version = ">=0.32.0" + +[dependencies] diff --git a/noir/noir-repo/test_programs/compile_success_empty/trait_default_method_uses_op/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/trait_default_method_uses_op/src/main.nr new file mode 100644 index 00000000000..415f36fe207 --- /dev/null +++ b/noir/noir-repo/test_programs/compile_success_empty/trait_default_method_uses_op/src/main.nr @@ -0,0 +1,7 @@ +pub trait Foo: Eq { + fn foo(self) -> bool { + self == self + } +} + +fn main() {} diff --git a/noir/noir-repo/test_programs/compile_success_empty/trait_function_calls/src/main.nr b/noir/noir-repo/test_programs/compile_success_empty/trait_function_calls/src/main.nr index f9a338bfa47..bdc4a3d8c90 100644 --- a/noir/noir-repo/test_programs/compile_success_empty/trait_function_calls/src/main.nr +++ b/noir/noir-repo/test_programs/compile_success_empty/trait_function_calls/src/main.nr @@ -23,25 +23,32 @@ // 1a) trait default method -> trait default method trait Trait1a { fn trait_method1(self) -> Field { - self.trait_method2() * 7892 - self.vl + self.trait_method2() * 7892 - self.vl() } fn trait_method2(self) -> Field { let _ = self; 43278 } + fn vl(self) -> Field; } struct Struct1a { vl: Field, } -impl Trait1a for Struct1a {} +impl Trait1a for Struct1a { + fn vl(self) -> Field { + self.vl + } +} // 1b) trait default method -> trait overriden method trait Trait1b { fn trait_method1(self) -> Field { - self.trait_method2() * 2832 - self.vl + self.trait_method2() * 2832 - self.vl() } fn trait_method2(self) -> Field { + let _ = self; 9323 } + fn vl(self) -> Field; } struct Struct1b { vl: Field, @@ -51,13 +58,17 @@ impl Trait1b for Struct1b { let _ = self; 2394 } + fn vl(self) -> Field { + self.vl + } } // 1c) trait default method -> trait overriden (no default) method trait Trait1c { fn trait_method1(self) -> Field { - self.trait_method2() * 7635 - self.vl + self.trait_method2() * 7635 - self.vl() } fn trait_method2(self) -> Field; + fn vl(self) -> Field; } struct Struct1c { vl: Field, @@ -67,16 +78,20 @@ impl Trait1c for Struct1c { let _ = self; 5485 } + fn vl(self) -> Field { + self.vl + } } // 1d) trait overriden method -> trait default method trait Trait1d { fn trait_method1(self) -> Field { - self.trait_method2() * 2825 - self.vl + self.trait_method2() * 2825 - self.vl() } fn trait_method2(self) -> Field { let _ = self; 29341 } + fn vl(self) -> Field; } struct Struct1d { vl: Field, @@ -85,15 +100,20 @@ impl Trait1d for Struct1d { fn trait_method1(self) -> Field { self.trait_method2() * 9342 - self.vl } + fn vl(self) -> Field { + self.vl + } } // 1e) trait overriden method -> trait overriden method trait Trait1e { fn trait_method1(self) -> Field { - self.trait_method2() * 85465 - self.vl + self.trait_method2() * 85465 - self.vl() } fn trait_method2(self) -> Field { + let _ = self; 2381 } + fn vl(self) -> Field; } struct Struct1e { vl: Field, @@ -106,13 +126,17 @@ impl Trait1e for Struct1e { let _ = self; 58945 } + fn vl(self) -> Field { + self.vl + } } // 1f) trait overriden method -> trait overriden (no default) method trait Trait1f { fn trait_method1(self) -> Field { - self.trait_method2() * 43257 - self.vl + self.trait_method2() * 43257 - self.vl() } fn trait_method2(self) -> Field; + fn vl(self) -> Field; } struct Struct1f { vl: Field, @@ -125,6 +149,9 @@ impl Trait1f for Struct1f { let _ = self; 5748 } + fn vl(self) -> Field { + self.vl + } } // 1g) trait overriden (no default) method -> trait default method trait Trait1g { @@ -146,6 +173,7 @@ impl Trait1g for Struct1g { trait Trait1h { fn trait_method1(self) -> Field; fn trait_method2(self) -> Field { + let _ = self; 7823 } } @@ -182,24 +210,30 @@ impl Trait1i for Struct1i { // 2a) trait default method -> trait default function trait Trait2a { fn trait_method1(self) -> Field { - Self::trait_function2() * 2385 - self.vl + Self::trait_function2() * 2385 - self.vl() } fn trait_function2() -> Field { 7843 } + fn vl(self) -> Field; } struct Struct2a { vl: Field, } -impl Trait2a for Struct2a {} +impl Trait2a for Struct2a { + fn vl(self) -> Field { + self.vl + } +} // 2b) trait default method -> trait overriden function trait Trait2b { fn trait_method1(self) -> Field { - Self::trait_function2() * 6583 - self.vl + Self::trait_function2() * 6583 - self.vl() } fn trait_function2() -> Field { 3752 } + fn vl(self) -> Field; } struct Struct2b { vl: Field, @@ -208,13 +242,17 @@ impl Trait2b for Struct2b { fn trait_function2() -> Field { 8477 } + fn vl(self) -> Field { + self.vl + } } // 2c) trait default method -> trait overriden (no default) function trait Trait2c { fn trait_method1(self) -> Field { - Self::trait_function2() * 2831 - self.vl + Self::trait_function2() * 2831 - self.vl() } fn trait_function2() -> Field; + fn vl(self) -> Field; } struct Struct2c { vl: Field, @@ -223,15 +261,19 @@ impl Trait2c for Struct2c { fn trait_function2() -> Field { 8342 } + fn vl(self) -> Field { + self.vl + } } // 2d) trait overriden method -> trait default function trait Trait2d { fn trait_method1(self) -> Field { - Self::trait_function2() * 924 - self.vl + Self::trait_function2() * 924 - self.vl() } fn trait_function2() -> Field { 384 } + fn vl(self) -> Field; } struct Struct2d { vl: Field, @@ -240,15 +282,19 @@ impl Trait2d for Struct2d { fn trait_method1(self) -> Field { Self::trait_function2() * 3984 - self.vl } + fn vl(self) -> Field { + self.vl + } } // 2e) trait overriden method -> trait overriden function trait Trait2e { fn trait_method1(self) -> Field { - Self::trait_function2() * 3642 - self.vl + Self::trait_function2() * 3642 - self.vl() } fn trait_function2() -> Field { 97342 } + fn vl(self) -> Field; } struct Struct2e { vl: Field, @@ -260,13 +306,17 @@ impl Trait2e for Struct2e { fn trait_function2() -> Field { 39400 } + fn vl(self) -> Field { + self.vl + } } // 2f) trait overriden method -> trait overriden (no default) function trait Trait2f { fn trait_method1(self) -> Field { - Self::trait_function2() * 2783 - self.vl + Self::trait_function2() * 2783 - self.vl() } fn trait_function2() -> Field; + fn vl(self) -> Field; } struct Struct2f { vl: Field, @@ -278,6 +328,9 @@ impl Trait2f for Struct2f { fn trait_function2() -> Field { 72311 } + fn vl(self) -> Field { + self.vl + } } // 2g) trait overriden (no default) method -> trait default function trait Trait2g { @@ -332,25 +385,32 @@ impl Trait2i for Struct2i { // 3a) trait default function -> trait default method trait Trait3a { fn trait_function1(a: Field, b: Self) -> Field { - b.trait_method2() * 8344 - b.vl + a + b.trait_method2() * 8344 - b.vl() + a } fn trait_method2(self) -> Field { let _ = self; 19212 } + fn vl(self) -> Field; } struct Struct3a { vl: Field, } -impl Trait3a for Struct3a {} +impl Trait3a for Struct3a { + fn vl(self) -> Field { + self.vl + } +} // 3b) trait default function -> trait overriden method trait Trait3b { fn trait_function1(a: Field, b: Self) -> Field { - b.trait_method2() * 9233 - b.vl + a + b.trait_method2() * 9233 - b.vl() + a } fn trait_method2(self) -> Field { + let _ = self; 9111 } + fn vl(self) -> Field; } struct Struct3b { vl: Field, @@ -360,13 +420,17 @@ impl Trait3b for Struct3b { let _ = self; 2392 } + fn vl(self) -> Field { + self.vl + } } // 3c) trait default function -> trait overriden (no default) method trait Trait3c { fn trait_function1(a: Field, b: Self) -> Field { - b.trait_method2() * 2822 - b.vl + a + b.trait_method2() * 2822 - b.vl() + a } fn trait_method2(self) -> Field; + fn vl(self) -> Field; } struct Struct3c { vl: Field, @@ -376,16 +440,20 @@ impl Trait3c for Struct3c { let _ = self; 7743 } + fn vl(self) -> Field { + self.vl + } } // 3d) trait overriden function -> trait default method trait Trait3d { fn trait_function1(a: Field, b: Self) -> Field { - b.trait_method2() * 291 - b.vl + a + b.trait_method2() * 291 - b.vl() + a } fn trait_method2(self) -> Field { let _ = self; 3328 } + fn vl(self) -> Field; } struct Struct3d { vl: Field, @@ -394,15 +462,20 @@ impl Trait3d for Struct3d { fn trait_function1(a: Field, b: Self) -> Field { b.trait_method2() * 4933 - b.vl + a } + fn vl(self) -> Field { + self.vl + } } // 3e) trait overriden function -> trait overriden method trait Trait3e { fn trait_function1(a: Field, b: Self) -> Field { - b.trait_method2() * 71231 - b.vl + a + b.trait_method2() * 71231 - b.vl() + a } fn trait_method2(self) -> Field { + let _ = self; 373 } + fn vl(self) -> Field; } struct Struct3e { vl: Field, @@ -415,13 +488,17 @@ impl Trait3e for Struct3e { let _ = self; 80002 } + fn vl(self) -> Field { + self.vl + } } // 3f) trait overriden function -> trait overriden (no default) method trait Trait3f { fn trait_function1(a: Field, b: Self) -> Field { - b.trait_method2() * 28223 - b.vl + a + b.trait_method2() * 28223 - b.vl() + a } fn trait_method2(self) -> Field; + fn vl(self) -> Field; } struct Struct3f { vl: Field, @@ -434,6 +511,9 @@ impl Trait3f for Struct3f { let _ = self; 63532 } + fn vl(self) -> Field { + self.vl + } } // 3g) trait overriden (no default) function -> trait default method trait Trait3g { @@ -455,6 +535,7 @@ impl Trait3g for Struct3g { trait Trait3h { fn trait_function1(a: Field, b: Self) -> Field; fn trait_method2(self) -> Field { + let _ = self; 293 } } diff --git a/noir/noir-repo/test_programs/execution_success/aes128_encrypt/src/main.nr b/noir/noir-repo/test_programs/execution_success/aes128_encrypt/src/main.nr index 7738bed5255..243984a8300 100644 --- a/noir/noir-repo/test_programs/execution_success/aes128_encrypt/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/aes128_encrypt/src/main.nr @@ -22,12 +22,12 @@ unconstrained fn decode_hex(s: str) -> [u8; M] { unconstrained fn cipher(plaintext: [u8; 12], iv: [u8; 16], key: [u8; 16]) -> [u8; 16] { let result = std::aes128::aes128_encrypt(plaintext, iv, key); - result.as_array() + result } fn main(inputs: str<12>, iv: str<16>, key: str<16>, output: str<32>) { let result: [u8; 16] = - std::aes128::aes128_encrypt(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()).as_array(); + std::aes128::aes128_encrypt(inputs.as_bytes(), iv.as_bytes(), key.as_bytes()); let output_bytes: [u8; 16] = unsafe { //@safety: testing context diff --git a/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr b/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr index 68b9f2407b9..8de4d0f2508 100644 --- a/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr +++ b/noir/noir-repo/test_programs/execution_success/reference_counts/src/main.nr @@ -2,7 +2,7 @@ use std::mem::array_refcount; fn main() { let mut array = [0, 1, 2]; - assert_refcount(array, 2); + assert_refcount(array, 1); borrow(array, array_refcount(array)); borrow_mut(&mut array, array_refcount(array)); diff --git a/noir/noir-repo/test_programs/noir_test_success/comptime_blackbox/src/main.nr b/noir/noir-repo/test_programs/noir_test_success/comptime_blackbox/src/main.nr index c3784e73b09..859790f7d2d 100644 --- a/noir/noir-repo/test_programs/noir_test_success/comptime_blackbox/src/main.nr +++ b/noir/noir-repo/test_programs/noir_test_success/comptime_blackbox/src/main.nr @@ -121,10 +121,12 @@ fn test_sha256() { #[test] fn test_embedded_curve_ops() { let (sum, mul) = comptime { - let g1 = EmbeddedCurvePoint { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false }; let s1 = EmbeddedCurveScalar { lo: 1, hi: 0 }; - let sum = g1 + g1; - let mul = multi_scalar_mul([g1, g1], [s1, s1]); + let s2 = EmbeddedCurveScalar { lo: 2, hi: 0 }; + let g1 = EmbeddedCurvePoint { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false }; + let g2 = multi_scalar_mul([g1], [s2]); + let sum = g1 + g2; + let mul = multi_scalar_mul([g1, g2], [s1, s1]); (sum, mul) }; assert_eq(sum, mul); diff --git a/noir/noir-repo/tooling/acvm_cli/src/cli/execute_cmd.rs b/noir/noir-repo/tooling/acvm_cli/src/cli/execute_cmd.rs index c3c83a8f000..9fed8cbd497 100644 --- a/noir/noir-repo/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/noir/noir-repo/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -35,12 +35,19 @@ pub(crate) struct ExecuteCommand { /// Set to print output witness to stdout #[clap(long, short, action)] print: bool, + + /// Use pedantic ACVM solving, i.e. double-check some black-box function + /// assumptions when solving. + /// This is disabled by default. + #[clap(long, default_value = "false")] + pedantic_solving: bool, } fn run_command(args: ExecuteCommand) -> Result { let bytecode = read_bytecode_from_file(&args.working_directory, &args.bytecode)?; let circuit_inputs = read_inputs_from_file(&args.working_directory, &args.input_witness)?; - let output_witness = execute_program_from_witness(circuit_inputs, &bytecode)?; + let output_witness = + execute_program_from_witness(circuit_inputs, &bytecode, args.pedantic_solving)?; assert_eq!(output_witness.length(), 1, "ACVM CLI only supports a witness stack of size 1"); let output_witness_string = create_output_witness_string( &output_witness.peek().expect("Should have a witness stack item").witness, @@ -67,13 +74,14 @@ pub(crate) fn run(args: ExecuteCommand) -> Result { pub(crate) fn execute_program_from_witness( inputs_map: WitnessMap, bytecode: &[u8], + pedantic_solving: bool, ) -> Result, CliError> { let program: Program = Program::deserialize_program(bytecode) .map_err(|_| CliError::CircuitDeserializationError())?; execute_program( &program, inputs_map, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallBuilder { output: PrintOutput::Stdout, ..Default::default() } .build(), ) diff --git a/noir/noir-repo/tooling/debugger/src/context.rs b/noir/noir-repo/tooling/debugger/src/context.rs index 256bb148ae9..9741749df64 100644 --- a/noir/noir-repo/tooling/debugger/src/context.rs +++ b/noir/noir-repo/tooling/debugger/src/context.rs @@ -233,7 +233,7 @@ pub struct ExecutionFrame<'a, B: BlackBoxFunctionSolver> { } pub(super) struct DebugContext<'a, B: BlackBoxFunctionSolver> { - acvm: ACVM<'a, FieldElement, B>, + pub(crate) acvm: ACVM<'a, FieldElement, B>, current_circuit_id: u32, brillig_solver: Option>, @@ -971,6 +971,7 @@ mod tests { #[test] fn test_resolve_foreign_calls_stepping_into_brillig() { + let solver = StubbedBlackBoxSolver::default(); let fe_1 = FieldElement::one(); let w_x = Witness(1); @@ -1031,7 +1032,7 @@ mod tests { debug_artifact, )); let mut context = DebugContext::new( - &StubbedBlackBoxSolver, + &solver, circuits, debug_artifact, initial_witness, @@ -1116,6 +1117,7 @@ mod tests { #[test] fn test_break_brillig_block_while_stepping_acir_opcodes() { + let solver = StubbedBlackBoxSolver::default(); let fe_0 = FieldElement::zero(); let fe_1 = FieldElement::one(); let w_x = Witness(1); @@ -1199,7 +1201,7 @@ mod tests { )); let brillig_funcs = &[brillig_bytecode]; let mut context = DebugContext::new( - &StubbedBlackBoxSolver, + &solver, circuits, debug_artifact, initial_witness, @@ -1240,6 +1242,7 @@ mod tests { #[test] fn test_address_debug_location_mapping() { + let solver = StubbedBlackBoxSolver::default(); let brillig_one = BrilligBytecode { bytecode: vec![BrilligOpcode::Return, BrilligOpcode::Return] }; let brillig_two = BrilligBytecode { @@ -1286,7 +1289,7 @@ mod tests { let brillig_funcs = &[brillig_one, brillig_two]; let context = DebugContext::new( - &StubbedBlackBoxSolver, + &solver, &circuits, &debug_artifact, WitnessMap::new(), diff --git a/noir/noir-repo/tooling/lsp/src/lib.rs b/noir/noir-repo/tooling/lsp/src/lib.rs index a152cc17c7b..97a1c7ad27b 100644 --- a/noir/noir-repo/tooling/lsp/src/lib.rs +++ b/noir/noir-repo/tooling/lsp/src/lib.rs @@ -452,7 +452,7 @@ fn prepare_package_from_source_string() { "#; let client = ClientSocket::new_closed(); - let mut state = LspState::new(&client, acvm::blackbox_solver::StubbedBlackBoxSolver); + let mut state = LspState::new(&client, acvm::blackbox_solver::StubbedBlackBoxSolver::default()); let (mut context, crate_id) = prepare_source(source.to_string(), &mut state); let _check_result = noirc_driver::check_crate(&mut context, crate_id, &Default::default()); diff --git a/noir/noir-repo/tooling/lsp/src/requests/mod.rs b/noir/noir-repo/tooling/lsp/src/requests/mod.rs index f7b58d7d42f..d81108c2ec5 100644 --- a/noir/noir-repo/tooling/lsp/src/requests/mod.rs +++ b/noir/noir-repo/tooling/lsp/src/requests/mod.rs @@ -633,7 +633,7 @@ mod initialization { #[test] async fn test_on_initialize() { let client = ClientSocket::new_closed(); - let mut state = LspState::new(&client, StubbedBlackBoxSolver); + let mut state = LspState::new(&client, StubbedBlackBoxSolver::default()); let params = InitializeParams::default(); let response = on_initialize(&mut state, params).await.unwrap(); assert!(matches!( diff --git a/noir/noir-repo/tooling/lsp/src/solver.rs b/noir/noir-repo/tooling/lsp/src/solver.rs index a36e30a944e..df5c8eeb44f 100644 --- a/noir/noir-repo/tooling/lsp/src/solver.rs +++ b/noir/noir-repo/tooling/lsp/src/solver.rs @@ -6,6 +6,10 @@ use acvm::BlackBoxFunctionSolver; pub(super) struct WrapperSolver(pub(super) Box>); impl BlackBoxFunctionSolver for WrapperSolver { + fn pedantic_solving(&self) -> bool { + self.0.pedantic_solving() + } + fn multi_scalar_mul( &self, points: &[acvm::FieldElement], diff --git a/noir/noir-repo/tooling/lsp/src/test_utils.rs b/noir/noir-repo/tooling/lsp/src/test_utils.rs index c0505107842..c2c19b0efc7 100644 --- a/noir/noir-repo/tooling/lsp/src/test_utils.rs +++ b/noir/noir-repo/tooling/lsp/src/test_utils.rs @@ -5,7 +5,7 @@ use lsp_types::{Position, Range, Url}; pub(crate) async fn init_lsp_server(directory: &str) -> (LspState, Url) { let client = ClientSocket::new_closed(); - let mut state = LspState::new(&client, StubbedBlackBoxSolver); + let mut state = LspState::new(&client, StubbedBlackBoxSolver::default()); let root_path = std::env::current_dir() .unwrap() diff --git a/noir/noir-repo/tooling/nargo_cli/benches/criterion.rs b/noir/noir-repo/tooling/nargo_cli/benches/criterion.rs index 3d81ade65bc..e43e498ea06 100644 --- a/noir/noir-repo/tooling/nargo_cli/benches/criterion.rs +++ b/noir/noir-repo/tooling/nargo_cli/benches/criterion.rs @@ -141,10 +141,11 @@ fn criterion_test_execution(c: &mut Criterion, test_program_dir: &Path, force_br let artifacts = artifacts.as_ref().expect("setup compiled them"); for (program, initial_witness) in artifacts { + let solver = bn254_blackbox_solver::Bn254BlackBoxSolver::default(); let _witness_stack = black_box(nargo::ops::execute_program( black_box(&program.program), black_box(initial_witness.clone()), - &bn254_blackbox_solver::Bn254BlackBoxSolver, + &solver, &mut foreign_call_executor, )) .expect("failed to execute program"); diff --git a/noir/noir-repo/tooling/nargo_cli/build.rs b/noir/noir-repo/tooling/nargo_cli/build.rs index 0817045c138..3394d00b3a2 100644 --- a/noir/noir-repo/tooling/nargo_cli/build.rs +++ b/noir/noir-repo/tooling/nargo_cli/build.rs @@ -361,7 +361,7 @@ fn generate_compile_success_empty_tests(test_file: &mut File, test_data_dir: &Pa panic!("JSON was not well-formatted {:?}\n\n{:?}", e, std::str::from_utf8(&output.stdout)) }}); let num_opcodes = &json["programs"][0]["functions"][0]["opcodes"]; - assert_eq!(num_opcodes.as_u64().expect("number of opcodes should fit in a u64"), 0); + assert_eq!(num_opcodes.as_u64().expect("number of opcodes should fit in a u64"), 0, "expected the number of opcodes to be 0"); "#; generate_test_cases( diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/dap_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/dap_cmd.rs index a84e961cfe7..7fbf685688a 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -50,6 +50,12 @@ pub(crate) struct DapCommand { #[clap(long)] preflight_skip_instrumentation: bool, + + /// Use pedantic ACVM solving, i.e. double-check some black-box function + /// assumptions when solving. + /// This is disabled by default. + #[arg(long, default_value = "false")] + pedantic_solving: bool, } fn parse_expression_width(input: &str) -> Result { @@ -137,6 +143,7 @@ fn load_and_compile_project( fn loop_uninitialized_dap( mut server: Server, expression_width: ExpressionWidth, + pedantic_solving: bool, ) -> Result<(), DapError> { loop { let req = match server.poll_request()? { @@ -197,7 +204,7 @@ fn loop_uninitialized_dap( noir_debugger::run_dap_loop( server, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), compiled_program, initial_witness, )?; @@ -269,5 +276,6 @@ pub(crate) fn run(args: DapCommand, _config: NargoConfig) -> Result<(), CliError let input = BufReader::new(std::io::stdin()); let server = Server::new(input, output); - loop_uninitialized_dap(server, args.expression_width).map_err(CliError::DapError) + loop_uninitialized_dap(server, args.expression_width, args.pedantic_solving) + .map_err(CliError::DapError) } diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs index f4dd607a53e..2ed30639d32 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -85,7 +85,14 @@ pub(crate) fn run(args: DebugCommand, config: NargoConfig) -> Result<(), CliErro let compiled_program = nargo::ops::transform_program(compiled_program, target_width); - run_async(package, compiled_program, &args.prover_name, &args.witness_name, target_dir) + run_async( + package, + compiled_program, + &args.prover_name, + &args.witness_name, + target_dir, + args.compile_options.pedantic_solving, + ) } pub(crate) fn compile_bin_package_for_debugging( @@ -172,6 +179,7 @@ fn run_async( prover_name: &str, witness_name: &Option, target_dir: &PathBuf, + pedantic_solving: bool, ) -> Result<(), CliError> { use tokio::runtime::Builder; let runtime = Builder::new_current_thread().enable_all().build().unwrap(); @@ -179,7 +187,7 @@ fn run_async( runtime.block_on(async { println!("[{}] Starting debugger", package.name); let (return_value, witness_stack) = - debug_program_and_decode(program, package, prover_name)?; + debug_program_and_decode(program, package, prover_name, pedantic_solving)?; if let Some(solved_witness_stack) = witness_stack { println!("[{}] Circuit witness successfully solved", package.name); @@ -206,12 +214,13 @@ fn debug_program_and_decode( program: CompiledProgram, package: &Package, prover_name: &str, + pedantic_solving: bool, ) -> Result<(Option, Option>), CliError> { // Parse the initial witness values from Prover.toml let (inputs_map, _) = read_inputs_from_file(&package.root_dir, prover_name, Format::Toml, &program.abi)?; let program_abi = program.abi.clone(); - let witness_stack = debug_program(program, &inputs_map)?; + let witness_stack = debug_program(program, &inputs_map, pedantic_solving)?; match witness_stack { Some(witness_stack) => { @@ -229,9 +238,14 @@ fn debug_program_and_decode( pub(crate) fn debug_program( compiled_program: CompiledProgram, inputs_map: &InputMap, + pedantic_solving: bool, ) -> Result>, CliError> { let initial_witness = compiled_program.abi.encode(inputs_map, None)?; - noir_debugger::run_repl_session(&Bn254BlackBoxSolver, compiled_program, initial_witness) - .map_err(CliError::from) + noir_debugger::run_repl_session( + &Bn254BlackBoxSolver(pedantic_solving), + compiled_program, + initial_witness, + ) + .map_err(CliError::from) } diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs index 709caf71185..88e74340c71 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -73,6 +73,7 @@ pub(crate) fn run(args: ExecuteCommand, config: NargoConfig) -> Result<(), CliEr args.oracle_resolver.as_deref(), Some(workspace.root_dir.clone()), Some(package.name.to_string()), + args.compile_options.pedantic_solving, )?; println!("[{}] Circuit witness successfully solved", package.name); @@ -108,12 +109,19 @@ fn execute_program_and_decode( foreign_call_resolver_url: Option<&str>, root_path: Option, package_name: Option, + pedantic_solving: bool, ) -> Result { // Parse the initial witness values from Prover.toml let (inputs_map, expected_return) = read_inputs_from_file(&package.root_dir, prover_name, Format::Toml, &program.abi)?; - let witness_stack = - execute_program(&program, &inputs_map, foreign_call_resolver_url, root_path, package_name)?; + let witness_stack = execute_program( + &program, + &inputs_map, + foreign_call_resolver_url, + root_path, + package_name, + pedantic_solving, + )?; // Get the entry point witness for the ABI let main_witness = &witness_stack.peek().expect("Should have at least one witness on the stack").witness; @@ -134,13 +142,14 @@ pub(crate) fn execute_program( foreign_call_resolver_url: Option<&str>, root_path: Option, package_name: Option, + pedantic_solving: bool, ) -> Result, CliError> { let initial_witness = compiled_program.abi.encode(inputs_map, None)?; let solved_witness_stack_err = nargo::ops::execute_program( &compiled_program.program, initial_witness, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallExecutor::new( PrintOutput::Stdout, foreign_call_resolver_url, diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/info_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/info_cmd.rs index 6320cbbf350..651ddbc0b56 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/info_cmd.rs @@ -83,6 +83,7 @@ pub(crate) fn run(mut args: InfoCommand, config: NargoConfig) -> Result<(), CliE binary_packages, &args.prover_name, args.compile_options.expression_width, + args.compile_options.pedantic_solving, )? } else { binary_packages @@ -238,6 +239,7 @@ fn profile_brillig_execution( binary_packages: Vec<(Package, ProgramArtifact)>, prover_name: &str, expression_width: Option, + pedantic_solving: bool, ) -> Result, CliError> { let mut program_info = Vec::new(); for (package, program_artifact) in binary_packages.iter() { @@ -253,7 +255,7 @@ fn profile_brillig_execution( let (_, profiling_samples) = nargo::ops::execute_program_with_profiling( &program_artifact.bytecode, initial_witness, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallBuilder::default().build(), )?; diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/lsp_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/lsp_cmd.rs index bfaa913b33a..dc5d0995c06 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/lsp_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/lsp_cmd.rs @@ -25,7 +25,8 @@ pub(crate) fn run(_args: LspCommand, _config: NargoConfig) -> Result<(), CliErro runtime.block_on(async { let (server, _) = async_lsp::MainLoop::new_server(|client| { - let router = NargoLspService::new(&client, Bn254BlackBoxSolver); + let pedantic_solving = true; + let router = NargoLspService::new(&client, Bn254BlackBoxSolver(pedantic_solving)); ServiceBuilder::new() .layer(TracingLayer::default()) diff --git a/noir/noir-repo/tooling/nargo_cli/tests/stdlib-props.rs b/noir/noir-repo/tooling/nargo_cli/tests/stdlib-props.rs index 24d53b48a6c..780b34bf0c3 100644 --- a/noir/noir-repo/tooling/nargo_cli/tests/stdlib-props.rs +++ b/noir/noir-repo/tooling/nargo_cli/tests/stdlib-props.rs @@ -78,7 +78,8 @@ fn run_snippet_proptest( Err(e) => panic!("failed to compile program; brillig = {force_brillig}:\n{source}\n{e:?}"), }; - let blackbox_solver = bn254_blackbox_solver::Bn254BlackBoxSolver; + let pedandic_solving = true; + let blackbox_solver = bn254_blackbox_solver::Bn254BlackBoxSolver(pedandic_solving); let foreign_call_executor = RefCell::new(DefaultForeignCallBuilder::default().build()); // Generate multiple input/output diff --git a/noir/noir-repo/tooling/nargo_cli/tests/stdlib-tests.rs b/noir/noir-repo/tooling/nargo_cli/tests/stdlib-tests.rs index 6aae94f6645..b162b282781 100644 --- a/noir/noir-repo/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/noir/noir-repo/tooling/nargo_cli/tests/stdlib-tests.rs @@ -84,8 +84,9 @@ fn run_stdlib_tests(force_brillig: bool, inliner_aggressiveness: i64) { let test_report: Vec<(String, TestStatus)> = test_functions .into_iter() .map(|(test_name, test_function)| { + let pedantic_solving = true; let status = run_test( - &bn254_blackbox_solver::Bn254BlackBoxSolver, + &bn254_blackbox_solver::Bn254BlackBoxSolver(pedantic_solving), &mut context, &test_function, PrintOutput::Stdout, diff --git a/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs b/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs index bab15529744..dbb2a2c38bc 100644 --- a/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs +++ b/noir/noir-repo/tooling/profiler/src/cli/execution_flamegraph_cmd.rs @@ -26,6 +26,12 @@ pub(crate) struct ExecutionFlamegraphCommand { /// The output folder for the flamegraph svg files #[clap(long, short)] output: PathBuf, + + /// Use pedantic ACVM solving, i.e. double-check some black-box function + /// assumptions when solving. + /// This is disabled by default. + #[clap(long, default_value = "false")] + pedantic_solving: bool, } pub(crate) fn run(args: ExecutionFlamegraphCommand) -> eyre::Result<()> { @@ -34,6 +40,7 @@ pub(crate) fn run(args: ExecutionFlamegraphCommand) -> eyre::Result<()> { &args.prover_toml_path, &InfernoFlamegraphGenerator { count_name: "samples".to_string() }, &args.output, + args.pedantic_solving, ) } @@ -42,6 +49,7 @@ fn run_with_generator( prover_toml_path: &Path, flamegraph_generator: &impl FlamegraphGenerator, output_path: &Path, + pedantic_solving: bool, ) -> eyre::Result<()> { let program = read_program_from_file(artifact_path).context("Error reading program from file")?; @@ -54,7 +62,7 @@ fn run_with_generator( let (_, mut profiling_samples) = nargo::ops::execute_program_with_profiling( &program.bytecode, initial_witness, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallBuilder::default().with_output(PrintOutput::Stdout).build(), )?; println!("Executed");