From b5ee668727bb68b3083e7bda51bc1ce90cf6ee16 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Wed, 4 Dec 2024 15:17:08 -0500 Subject: [PATCH 01/22] test input sizes and predicate values with --pedantic_solving flag, add method to check for valid bigint modulus, add error for predicate > 1, add cli option for pedantic solving --- acvm-repo/acvm/src/pwg/blackbox/bigint.rs | 4 ++ acvm-repo/acvm/src/pwg/blackbox/mod.rs | 20 ++++++- acvm-repo/acvm/src/pwg/blackbox/range.rs | 5 +- acvm-repo/acvm/src/pwg/memory_op.rs | 4 +- acvm-repo/acvm/src/pwg/mod.rs | 60 +++++++++++++++---- acvm-repo/blackbox_solver/src/bigint.rs | 5 ++ compiler/noirc_driver/src/lib.rs | 6 ++ tooling/acvm_cli/src/cli/execute_cmd.rs | 10 +++- tooling/debugger/src/context.rs | 5 +- tooling/debugger/src/dap.rs | 4 ++ tooling/debugger/src/lib.rs | 6 +- tooling/debugger/src/repl.rs | 5 ++ tooling/nargo/src/ops/execute.rs | 13 ++++ tooling/nargo/src/ops/test.rs | 2 + tooling/nargo_cli/src/cli/dap_cmd.rs | 10 +++- tooling/nargo_cli/src/cli/debug_cmd.rs | 11 ++-- tooling/nargo_cli/src/cli/execute_cmd.rs | 6 +- tooling/nargo_cli/src/cli/info_cmd.rs | 3 + tooling/nargo_cli/tests/stdlib-props.rs | 2 + .../src/cli/execution_flamegraph_cmd.rs | 9 +++ 20 files changed, 165 insertions(+), 25 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs index ccad3510682..edd2bfcac41 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs @@ -59,4 +59,8 @@ impl AcvmBigIntSolver { self.bigint_solver.bigint_op(lhs, rhs, output, func)?; Ok(()) } + + pub(crate) fn is_valid_modulus(&self, modulus: &[u8]) -> bool { + self.bigint_solver.is_valid_modulus(modulus) + } } diff --git a/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/acvm-repo/acvm/src/pwg/blackbox/mod.rs index c3b1627ba65..6ca6e6beeb9 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -65,6 +65,7 @@ pub(crate) fn solve( initial_witness: &mut WitnessMap, bb_func: &BlackBoxFuncCall, bigint_solver: &mut AcvmBigIntSolver, + pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { let inputs = bb_func.get_inputs_vec(); if !contains_all_inputs(initial_witness, &inputs) { @@ -75,13 +76,14 @@ pub(crate) fn solve( )); } + // TODO: check input value sizes when pedantic_solving match bb_func { 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::RANGE { input } => solve_range_opcode(initial_witness, input, pedantic_solving), BlackBoxFuncCall::Blake2s { inputs, outputs } => { solve_generic_256_hash_opcode(initial_witness, inputs, None, outputs, blake2s) } @@ -147,6 +149,10 @@ pub(crate) fn solve( *output, ), BlackBoxFuncCall::MultiScalarMul { points, scalars, outputs } => { + if pedantic_solving && points.len() != scalars.len() { + // TODO: better error or ICE + panic!("MultiScalarMul") + } multi_scalar_mul(backend, initial_witness, points, scalars, *outputs) } BlackBoxFuncCall::EmbeddedCurveAdd { input1, input2, outputs } => { @@ -161,15 +167,27 @@ pub(crate) fn solve( bigint_solver.bigint_op(*lhs, *rhs, *output, bb_func.get_black_box_func()) } BlackBoxFuncCall::BigIntFromLeBytes { inputs, modulus, output } => { + if pedantic_solving && (!bigint_solver.is_valid_modulus(modulus) || inputs.len() > 32) { + // TODO: better error or ICE + panic!("BigIntFromLeBytes") + } bigint_solver.bigint_from_bytes(inputs, modulus, *output, initial_witness) } BlackBoxFuncCall::BigIntToLeBytes { input, outputs } => { + if pedantic_solving && outputs.len() != 32 { + // TODO: better error or ICE + panic!("BigIntToLeBytes") + } bigint_solver.bigint_to_bytes(*input, outputs, initial_witness) } BlackBoxFuncCall::Sha256Compression { inputs, hash_values, outputs } => { solve_sha_256_permutation_opcode(initial_witness, inputs, hash_values, outputs) } BlackBoxFuncCall::Poseidon2Permutation { inputs, outputs, len } => { + if pedantic_solving && inputs.len() != outputs.len() { + // TODO: better error or ICE + panic!("Poseidon2Permutation") + } solve_poseidon2_permutation_opcode(backend, initial_witness, inputs, outputs, *len) } } diff --git a/acvm-repo/acvm/src/pwg/blackbox/range.rs b/acvm-repo/acvm/src/pwg/blackbox/range.rs index 4f9be14360e..d7083e4b9c0 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/range.rs +++ b/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/acvm-repo/acvm/src/pwg/memory_op.rs b/acvm-repo/acvm/src/pwg/memory_op.rs index a9ed7f5d15b..fcf94360d99 100644 --- a/acvm-repo/acvm/src/pwg/memory_op.rs +++ b/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,8 @@ 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]` diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 20c12a72fc0..c5ad72302b4 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -144,6 +144,11 @@ 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: {predicate_value}")] + PredicateLargerThanOne { + opcode_location: ErrorLocation, + predicate_value: F, + }, } impl From for OpcodeResolutionError { @@ -207,6 +212,9 @@ pub struct ACVM<'a, F, B: BlackBoxFunctionSolver> { profiling_active: bool, profiling_samples: ProfilingSamples, + + // when set, check BlackBoxFuncCall and AcvmBigIntSolver assumptions during solving + pub pedantic_solving: bool, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { @@ -216,6 +224,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { initial_witness: WitnessMap, unconstrained_functions: &'a [BrilligBytecode], assertion_payloads: &'a [(OpcodeLocation, AssertionPayload)], + pedantic_solving: bool, ) -> Self { let status = if opcodes.is_empty() { ACVMStatus::Solved } else { ACVMStatus::InProgress }; ACVM { @@ -233,6 +242,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { assertion_payloads, profiling_active: false, profiling_samples: Vec::new(), + pedantic_solving: pedantic_solving, } } @@ -367,6 +377,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { &mut self.witness_map, bb_func, &mut self.bigint_solver, + self.pedantic_solving, ), Opcode::MemoryInit { block_id, init, .. } => { let solver = self.block_solvers.entry(*block_id).or_default(); @@ -374,7 +385,7 @@ 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.pedantic_solving) } Opcode::BrilligCall { .. } => match self.solve_brillig_call_opcode() { Ok(Some(foreign_call)) => return self.wait_for_foreign_call(foreign_call), @@ -478,7 +489,10 @@ 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.pedantic_solving, &opcode_location)? { return BrilligSolver::::zero_out_brillig_outputs(&mut self.witness_map, outputs) .map(|_| None); } @@ -546,8 +560,11 @@ 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.pedantic_solving, &opcode_location) { Ok(result) => result, Err(err) => return StepResult::Status(self.handle_opcode_resolution(Err(err))), }; @@ -588,15 +605,17 @@ 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(), - )), + opcode_location, }); } - if is_predicate_false(&self.witness_map, predicate)? { + if is_predicate_false(&self.witness_map, predicate, self.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)?; @@ -616,9 +635,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, }); @@ -737,9 +754,30 @@ 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); + if pedantic_solving { + pred_value.and_then(|predicate_value| { + if predicate_value.is_zero() { + Ok(true) + } else if predicate_value.is_one() { + Ok(false) + } else { + let opcode_location = opcode_location.clone(); + Err(OpcodeResolutionError::PredicateLargerThanOne { + opcode_location, + predicate_value, + }) + } + }) + } else { + pred_value.map(|pred_value| pred_value.is_zero()) + } + }, // If the predicate is `None`, then we treat it as an unconditional `true` None => Ok(false), } diff --git a/acvm-repo/blackbox_solver/src/bigint.rs b/acvm-repo/blackbox_solver/src/bigint.rs index 540862843ab..30c92af15f5 100644 --- a/acvm-repo/blackbox_solver/src/bigint.rs +++ b/acvm-repo/blackbox_solver/src/bigint.rs @@ -96,6 +96,11 @@ impl BigIntSolver { self.bigint_id_to_modulus.insert(output, modulus); Ok(()) } + + pub fn is_valid_modulus(&self, modulus: &[u8]) -> bool { + let modulus = BigUint::from_bytes_le(modulus); + self.bigint_id_to_modulus.values().any(|value| modulus == *value) + } } /// Wrapper over the generic bigint solver to automatically assign bigint IDs. diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 5bedefaf563..29a46fdca2d 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -144,6 +144,12 @@ pub struct CompileOptions { /// A lower value keeps the original program if it was smaller, even if it has more jumps. #[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, } pub fn parse_expression_width(input: &str) -> Result { diff --git a/tooling/acvm_cli/src/cli/execute_cmd.rs b/tooling/acvm_cli/src/cli/execute_cmd.rs index bf5969718e5..c5795714545 100644 --- a/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -34,12 +34,18 @@ 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, @@ -66,6 +72,7 @@ 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())?; @@ -74,6 +81,7 @@ pub(crate) fn execute_program_from_witness( inputs_map, &Bn254BlackBoxSolver, &mut DefaultForeignCallExecutor::new(true, None, None, None), + pedantic_solving, ) .map_err(CliError::CircuitExecutionError) } diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index bec30976552..997b1698155 100644 --- a/tooling/debugger/src/context.rs +++ b/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>, @@ -261,6 +261,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { initial_witness: WitnessMap, foreign_call_executor: Box, unconstrained_functions: &'a [BrilligBytecode], + pedantic_solving: bool, ) -> Self { let source_to_opcodes = build_source_to_opcode_debug_mappings(debug_artifact); let current_circuit_id: u32 = 0; @@ -273,6 +274,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { initial_witness, unconstrained_functions, &initial_circuit.assert_messages, + pedantic_solving, ), current_circuit_id, brillig_solver: None, @@ -573,6 +575,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { callee_witness_map, self.unconstrained_functions, &callee_circuit.assert_messages, + self.acvm.pedantic_solving, ); let caller_acvm = std::mem::replace(&mut self.acvm, callee_acvm); self.acvm_stack diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index cfe33a61cb5..b071e5d802f 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -65,6 +65,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession< debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, unconstrained_functions: &'a [BrilligBytecode], + pedantic_solving: bool, ) -> Self { let context = DebugContext::new( solver, @@ -73,6 +74,7 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession< initial_witness, Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)), unconstrained_functions, + pedantic_solving, ); Self { server, @@ -607,6 +609,7 @@ pub fn run_session>( solver: &B, program: CompiledProgram, initial_witness: WitnessMap, + pedantic_solving: bool, ) -> Result<(), ServerError> { let debug_artifact = DebugArtifact { debug_symbols: program.debug, file_map: program.file_map }; let mut session = DapSession::new( @@ -616,6 +619,7 @@ pub fn run_session>( &debug_artifact, initial_witness, &program.program.unconstrained_functions, + pedantic_solving, ); session.run_loop() diff --git a/tooling/debugger/src/lib.rs b/tooling/debugger/src/lib.rs index 37ac088ca35..a2ec382a775 100644 --- a/tooling/debugger/src/lib.rs +++ b/tooling/debugger/src/lib.rs @@ -19,8 +19,9 @@ pub fn run_repl_session>( solver: &B, program: CompiledProgram, initial_witness: WitnessMap, + pedantic_solving: bool, ) -> Result>, NargoError> { - repl::run(solver, program, initial_witness) + repl::run(solver, program, initial_witness, pedantic_solving) } pub fn run_dap_loop>( @@ -28,6 +29,7 @@ pub fn run_dap_loop>( solver: &B, program: CompiledProgram, initial_witness: WitnessMap, + pedantic_solving: bool, ) -> Result<(), ServerError> { - dap::run_session(server, solver, program, initial_witness) + dap::run_session(server, solver, program, initial_witness, pedantic_solving) } diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index 486e84060f0..f2a691bca68 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -41,6 +41,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, unconstrained_functions: &'a [BrilligBytecode], + pedantic_solving: bool, ) -> Self { let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)); @@ -51,6 +52,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { initial_witness.clone(), foreign_call_executor, unconstrained_functions, + pedantic_solving, ); let last_result = if context.get_current_debug_location().is_none() { // handle circuit with no opcodes @@ -322,6 +324,7 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { self.initial_witness.clone(), foreign_call_executor, self.unconstrained_functions, + self.context.acvm.pedantic_solving, ); for debug_location in breakpoints { self.context.add_breakpoint(debug_location); @@ -423,6 +426,7 @@ pub fn run>( blackbox_solver: &B, program: CompiledProgram, initial_witness: WitnessMap, + pedantic_solving: bool, ) -> Result>, NargoError> { let circuits = &program.program.functions; let debug_artifact = @@ -434,6 +438,7 @@ pub fn run>( debug_artifact, initial_witness, unconstrained_functions, + pedantic_solving, )); let ref_context = &context; diff --git a/tooling/nargo/src/ops/execute.rs b/tooling/nargo/src/ops/execute.rs index 57116ec2efd..c688cbd87c7 100644 --- a/tooling/nargo/src/ops/execute.rs +++ b/tooling/nargo/src/ops/execute.rs @@ -37,6 +37,10 @@ struct ProgramExecutor<'a, F, B: BlackBoxFunctionSolver, E: ForeignCallExecut // Flag that states whether we want to profile the VM. Profiling can add extra // execution costs so we want to make sure we only trigger it explicitly. profiling_active: bool, + + // Flag to use pedantic ACVM solving, i.e. double-checking some black box + // function assumptions during solving. + pedantic_solving: bool, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> @@ -48,6 +52,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> blackbox_solver: &'a B, foreign_call_executor: &'a mut E, profiling_active: bool, + pedantic_solving: bool, ) -> Self { ProgramExecutor { functions, @@ -58,6 +63,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> call_stack: Vec::default(), current_function_index: 0, profiling_active, + pedantic_solving, } } @@ -77,6 +83,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> initial_witness, self.unconstrained_functions, &circuit.assert_messages, + self.pedantic_solving, ); acvm.with_profiler(self.profiling_active); @@ -203,6 +210,7 @@ pub fn execute_program, E: ForeignCal initial_witness: WitnessMap, blackbox_solver: &B, foreign_call_executor: &mut E, + pedantic_solving: bool, ) -> Result, NargoError> { let profiling_active = false; let (witness_stack, profiling_samples) = execute_program_inner( @@ -211,6 +219,7 @@ pub fn execute_program, E: ForeignCal blackbox_solver, foreign_call_executor, profiling_active, + pedantic_solving, )?; assert!(profiling_samples.is_empty(), "Expected no profiling samples"); @@ -226,6 +235,7 @@ pub fn execute_program_with_profiling< initial_witness: WitnessMap, blackbox_solver: &B, foreign_call_executor: &mut E, + pedantic_solving: bool, ) -> Result<(WitnessStack, ProfilingSamples), NargoError> { let profiling_active = true; execute_program_inner( @@ -234,6 +244,7 @@ pub fn execute_program_with_profiling< blackbox_solver, foreign_call_executor, profiling_active, + pedantic_solving, ) } @@ -244,6 +255,7 @@ fn execute_program_inner, E: ForeignC blackbox_solver: &B, foreign_call_executor: &mut E, profiling_active: bool, + pedantic_solving: bool, ) -> Result<(WitnessStack, ProfilingSamples), NargoError> { let mut executor = ProgramExecutor::new( &program.functions, @@ -251,6 +263,7 @@ fn execute_program_inner, E: ForeignC blackbox_solver, foreign_call_executor, profiling_active, + pedantic_solving, ); let (main_witness, profiling_samples) = executor.execute_circuit(initial_witness)?; executor.witness_stack.push(0, main_witness); diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index e258627b522..b0ad10e7385 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -75,6 +75,7 @@ pub fn run_test>( WitnessMap::new(), blackbox_solver, &mut foreign_call_executor, + config.pedantic_solving, ); let status = test_status_program_compile_pass( @@ -130,6 +131,7 @@ pub fn run_test>( root_path.clone(), package_name.clone(), ), + config.pedantic_solving, ) .map_err(|err| err.to_string()) }; diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index a84e961cfe7..7c2977fd50d 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/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()? { @@ -200,6 +207,7 @@ fn loop_uninitialized_dap( &Bn254BlackBoxSolver, compiled_program, initial_witness, + pedantic_solving, )?; break; } @@ -269,5 +277,5 @@ 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/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index e837f297475..00fa6739c19 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -85,7 +85,7 @@ 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 +172,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 +180,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 +207,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 +231,10 @@ 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) + noir_debugger::run_repl_session(&Bn254BlackBoxSolver, compiled_program, initial_witness, pedantic_solving) .map_err(CliError::from) } diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index fa95d3123c6..556392e9b4d 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -78,6 +78,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); @@ -100,12 +101,13 @@ fn execute_program_and_decode( foreign_call_resolver_url: Option<&str>, root_path: Option, package_name: Option, + pedantic_solving: bool, ) -> Result<(Option, WitnessStack), 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 witness_stack = - execute_program(&program, &inputs_map, foreign_call_resolver_url, root_path, package_name)?; + 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; @@ -120,6 +122,7 @@ 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)?; @@ -133,6 +136,7 @@ pub(crate) fn execute_program( root_path, package_name, ), + pedantic_solving, ); match solved_witness_stack_err { Ok(solved_witness_stack) => Ok(solved_witness_stack), diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index 769a1f79d81..9c6ebdc3956 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -92,6 +92,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 @@ -247,6 +248,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() { @@ -264,6 +266,7 @@ fn profile_brillig_execution( initial_witness, &Bn254BlackBoxSolver, &mut DefaultForeignCallExecutor::new(false, None, None, None), + pedantic_solving, )?; let expression_width = get_target_width(package.expression_width, expression_width); diff --git a/tooling/nargo_cli/tests/stdlib-props.rs b/tooling/nargo_cli/tests/stdlib-props.rs index 86c225831b9..b9cdc52f9f8 100644 --- a/tooling/nargo_cli/tests/stdlib-props.rs +++ b/tooling/nargo_cli/tests/stdlib-props.rs @@ -87,11 +87,13 @@ fn run_snippet_proptest( let initial_witness = program.abi.encode(&io.inputs, None).expect("failed to encode"); let mut foreign_call_executor = foreign_call_executor.borrow_mut(); + let pedandic_solving = true; let witness_stack: WitnessStack = execute_program( &program.program, initial_witness, &blackbox_solver, &mut *foreign_call_executor, + pedandic_solving, ) .expect("failed to execute"); diff --git a/tooling/profiler/src/cli/execution_flamegraph_cmd.rs b/tooling/profiler/src/cli/execution_flamegraph_cmd.rs index 6d6da89f660..f48c00278bc 100644 --- a/tooling/profiler/src/cli/execution_flamegraph_cmd.rs +++ b/tooling/profiler/src/cli/execution_flamegraph_cmd.rs @@ -25,6 +25,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<()> { @@ -33,6 +39,7 @@ pub(crate) fn run(args: ExecutionFlamegraphCommand) -> eyre::Result<()> { &args.prover_toml_path, &InfernoFlamegraphGenerator { count_name: "samples".to_string() }, &args.output, + args.pedantic_solving, ) } @@ -41,6 +48,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")?; @@ -55,6 +63,7 @@ fn run_with_generator( initial_witness, &Bn254BlackBoxSolver, &mut DefaultForeignCallExecutor::new(true, None, None, None), + pedantic_solving, )?; println!("Executed"); From f65a0b0d0656d3f34f896f226192760f60c7e99f Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Thu, 5 Dec 2024 16:52:23 -0500 Subject: [PATCH 02/22] add pedantic_solving parameter wherever needed, default to true when testing or constant-folding, add tests for pedantic_solving, test to ensure allowed bingint moduli are prime --- Cargo.lock | 55 +++ acvm-repo/acvm/src/pwg/blackbox/bigint.rs | 14 +- .../src/pwg/blackbox/embedded_curve_ops.rs | 3 +- acvm-repo/acvm/src/pwg/blackbox/logic.rs | 12 +- acvm-repo/acvm/src/pwg/blackbox/mod.rs | 48 +-- acvm-repo/acvm/src/pwg/brillig.rs | 12 +- acvm-repo/acvm/src/pwg/memory_op.rs | 29 +- acvm-repo/acvm/src/pwg/mod.rs | 55 +-- acvm-repo/acvm/tests/solver.rs | 387 ++++++++++++------ acvm-repo/blackbox_solver/Cargo.toml | 1 + acvm-repo/blackbox_solver/src/bigint.rs | 78 +++- .../src/curve_specific_solver.rs | 2 + acvm-repo/bn254_blackbox_solver/src/lib.rs | 2 + acvm-repo/brillig_vm/src/black_box.rs | 18 +- acvm-repo/brillig_vm/src/lib.rs | 5 + compiler/noirc_driver/src/lib.rs | 1 + .../noirc_evaluator/src/acir/acir_variable.rs | 11 +- .../src/ssa/ir/instruction/call/blackbox.rs | 3 +- .../src/ssa/opt/constant_folding.rs | 11 +- .../noirc_frontend/src/elaborator/comptime.rs | 3 +- compiler/noirc_frontend/src/elaborator/mod.rs | 21 +- .../src/hir/comptime/interpreter.rs | 5 + .../src/hir/comptime/interpreter/foreign.rs | 42 +- .../noirc_frontend/src/hir/comptime/tests.rs | 10 +- .../src/hir/def_collector/dc_crate.rs | 11 +- .../noirc_frontend/src/hir/def_map/mod.rs | 2 + compiler/noirc_frontend/src/tests.rs | 2 + tooling/acvm_cli/src/cli/execute_cmd.rs | 3 +- tooling/lsp/src/solver.rs | 3 +- tooling/nargo_cli/src/cli/dap_cmd.rs | 3 +- tooling/nargo_cli/src/cli/debug_cmd.rs | 18 +- tooling/nargo_cli/src/cli/execute_cmd.rs | 10 +- 32 files changed, 645 insertions(+), 235 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 2f19ed704b2..7980a9ea4a3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -68,6 +68,7 @@ dependencies = [ "keccak", "libaes", "num-bigint", + "num-prime", "p256", "proptest", "sha2", @@ -158,6 +159,12 @@ dependencies = [ "memchr", ] +[[package]] +name = "allocator-api2" +version = "0.2.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" + [[package]] name = "android-tzdata" version = "0.1.1" @@ -1584,6 +1591,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" @@ -1879,6 +1892,11 @@ name = "hashbrown" version = "0.15.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a9bfc1af68b1726ea47d3d5109de126281def866b33970e10fbab11b5dafab3" +dependencies = [ + "allocator-api2", + "equivalent", + "foldhash", +] [[package]] name = "heck" @@ -2617,6 +2635,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" @@ -3259,6 +3286,7 @@ checksum = "a5e44f723f1133c9deac646763579fdb3ac745e418f2a7af9cd0c431da1f20b9" dependencies = [ "num-integer", "num-traits", + "rand 0.8.5", ] [[package]] @@ -3286,6 +3314,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 0.8.5", +] + [[package]] name = "num-traits" version = "0.2.19" diff --git a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs index edd2bfcac41..64356d9e5a0 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs @@ -24,12 +24,13 @@ impl AcvmBigIntSolver { modulus: &[u8], output: u32, initial_witness: &mut WitnessMap, + pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { let bytes = inputs .iter() .map(|input| input_to_value(initial_witness, *input, false).unwrap().to_u128() as u8) .collect::>(); - self.bigint_solver.bigint_from_bytes(&bytes, modulus, output)?; + self.bigint_solver.bigint_from_bytes(&bytes, modulus, output, pedantic_solving)?; Ok(()) } @@ -38,7 +39,11 @@ impl AcvmBigIntSolver { input: u32, outputs: &[Witness], initial_witness: &mut WitnessMap, + pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { + if 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); @@ -55,12 +60,9 @@ impl AcvmBigIntSolver { rhs: u32, output: u32, func: BlackBoxFunc, + pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { - self.bigint_solver.bigint_op(lhs, rhs, output, func)?; + self.bigint_solver.bigint_op(lhs, rhs, output, func, pedantic_solving)?; Ok(()) } - - pub(crate) fn is_valid_modulus(&self, modulus: &[u8]) -> bool { - self.bigint_solver.is_valid_modulus(modulus) - } } diff --git a/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs b/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs index 9e511571275..cb195d32acc 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs @@ -13,6 +13,7 @@ pub(super) fn multi_scalar_mul( points: &[FunctionInput], scalars: &[FunctionInput], outputs: (Witness, Witness, Witness), + pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { let points: Result, _> = points.iter().map(|input| input_to_value(initial_witness, *input, false)).collect(); @@ -31,7 +32,7 @@ pub(super) fn multi_scalar_mul( } // Call the backend's multi-scalar multiplication function let (res_x, res_y, is_infinite) = - backend.multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; + backend.multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving)?; // Insert the resulting point into the witness map insert_value(&outputs.0, res_x, initial_witness)?; diff --git a/acvm-repo/acvm/src/pwg/blackbox/logic.rs b/acvm-repo/acvm/src/pwg/blackbox/logic.rs index 8468b0ca27a..3fa72d9b215 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/logic.rs +++ b/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/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/acvm-repo/acvm/src/pwg/blackbox/mod.rs index 6ca6e6beeb9..2b91a75e627 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -76,14 +76,19 @@ pub(crate) fn solve( )); } - // TODO: check input value sizes when pedantic_solving match bb_func { 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, pedantic_solving), + BlackBoxFuncCall::AND { lhs, rhs, output } => { + and(initial_witness, lhs, rhs, output, pedantic_solving) + } + BlackBoxFuncCall::XOR { lhs, rhs, output } => { + xor(initial_witness, lhs, rhs, output, pedantic_solving) + } + BlackBoxFuncCall::RANGE { input } => { + solve_range_opcode(initial_witness, input, pedantic_solving) + } BlackBoxFuncCall::Blake2s { inputs, outputs } => { solve_generic_256_hash_opcode(initial_witness, inputs, None, outputs, blake2s) } @@ -149,11 +154,7 @@ pub(crate) fn solve( *output, ), BlackBoxFuncCall::MultiScalarMul { points, scalars, outputs } => { - if pedantic_solving && points.len() != scalars.len() { - // TODO: better error or ICE - panic!("MultiScalarMul") - } - multi_scalar_mul(backend, initial_witness, points, scalars, *outputs) + multi_scalar_mul(backend, initial_witness, points, scalars, *outputs, pedantic_solving) } BlackBoxFuncCall::EmbeddedCurveAdd { input1, input2, outputs } => { embedded_curve_add(backend, initial_witness, **input1, **input2, *outputs) @@ -163,31 +164,22 @@ pub(crate) fn solve( BlackBoxFuncCall::BigIntAdd { lhs, rhs, output } | BlackBoxFuncCall::BigIntSub { lhs, rhs, output } | BlackBoxFuncCall::BigIntMul { lhs, rhs, output } - | BlackBoxFuncCall::BigIntDiv { lhs, rhs, output } => { - bigint_solver.bigint_op(*lhs, *rhs, *output, bb_func.get_black_box_func()) - } - BlackBoxFuncCall::BigIntFromLeBytes { inputs, modulus, output } => { - if pedantic_solving && (!bigint_solver.is_valid_modulus(modulus) || inputs.len() > 32) { - // TODO: better error or ICE - panic!("BigIntFromLeBytes") - } - bigint_solver.bigint_from_bytes(inputs, modulus, *output, initial_witness) - } + | BlackBoxFuncCall::BigIntDiv { lhs, rhs, output } => bigint_solver.bigint_op( + *lhs, + *rhs, + *output, + bb_func.get_black_box_func(), + pedantic_solving, + ), + BlackBoxFuncCall::BigIntFromLeBytes { inputs, modulus, output } => bigint_solver + .bigint_from_bytes(inputs, modulus, *output, initial_witness, pedantic_solving), BlackBoxFuncCall::BigIntToLeBytes { input, outputs } => { - if pedantic_solving && outputs.len() != 32 { - // TODO: better error or ICE - panic!("BigIntToLeBytes") - } - bigint_solver.bigint_to_bytes(*input, outputs, initial_witness) + bigint_solver.bigint_to_bytes(*input, outputs, initial_witness, pedantic_solving) } BlackBoxFuncCall::Sha256Compression { inputs, hash_values, outputs } => { solve_sha_256_permutation_opcode(initial_witness, inputs, hash_values, outputs) } BlackBoxFuncCall::Poseidon2Permutation { inputs, outputs, len } => { - if pedantic_solving && inputs.len() != outputs.len() { - // TODO: better error or ICE - panic!("Poseidon2Permutation") - } solve_poseidon2_permutation_opcode(backend, initial_witness, inputs, outputs, *len) } } diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index a5f5783478e..e3502cfc04f 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -67,6 +67,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { acir_index: usize, brillig_function_id: BrilligFunctionId, profiling_active: bool, + pedantic_solving: bool, ) -> Result> { let vm = Self::setup_brillig_vm( initial_witness, @@ -75,6 +76,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { brillig_bytecode, bb_solver, profiling_active, + pedantic_solving, )?; Ok(Self { vm, acir_index, function_id: brillig_function_id }) } @@ -86,6 +88,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { brillig_bytecode: &'b [BrilligOpcode], bb_solver: &'b B, profiling_active: bool, + pedantic_solving: bool, ) -> Result, OpcodeResolutionError> { // Set input values let mut calldata: Vec = Vec::new(); @@ -133,7 +136,14 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { // Instantiate a Brillig VM given the solved calldata // along with the Brillig bytecode. - let vm = VM::new(calldata, brillig_bytecode, vec![], bb_solver, profiling_active); + let vm = VM::new( + calldata, + brillig_bytecode, + vec![], + bb_solver, + profiling_active, + pedantic_solving, + ); Ok(vm) } diff --git a/acvm-repo/acvm/src/pwg/memory_op.rs b/acvm-repo/acvm/src/pwg/memory_op.rs index fcf94360d99..9409ee36249 100644 --- a/acvm-repo/acvm/src/pwg/memory_op.rs +++ b/acvm-repo/acvm/src/pwg/memory_op.rs @@ -85,7 +85,8 @@ impl MemoryOpSolver { // Fetch whether or not the predicate is false (e.g. equal to zero) let opcode_location = ErrorLocation::Unresolved; - let skip_operation = is_predicate_false(initial_witness, predicate, pedantic_solving, &opcode_location)?; + let skip_operation = + is_predicate_false(initial_witness, predicate, pedantic_solving, &opcode_location)?; if is_read_operation { // `value_read = arr[memory_index]` @@ -152,7 +153,10 @@ mod tests { block_solver.init(&init, &initial_witness).unwrap(); for op in trace { - block_solver.solve_memory_op(&op, &mut initial_witness, &None).unwrap(); + let pedantic_solving = true; + block_solver + .solve_memory_op(&op, &mut initial_witness, &None, pedantic_solving) + .unwrap(); } assert_eq!(initial_witness[&Witness(4)], FieldElement::from(2u128)); @@ -177,7 +181,10 @@ 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(); + let pedantic_solving = true; + err = block_solver + .solve_memory_op(&op, &mut initial_witness, &None, pedantic_solving) + .err(); } } @@ -210,8 +217,14 @@ mod tests { let mut err = None; for op in invalid_trace { if err.is_none() { + let pedantic_solving = true; 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(); } } @@ -242,8 +255,14 @@ mod tests { let mut err = None; for op in invalid_trace { if err.is_none() { + let pedantic_solving = true; 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/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index c5ad72302b4..0237afd7e5e 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -145,10 +145,7 @@ pub enum OpcodeResolutionError { #[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: {predicate_value}")] - PredicateLargerThanOne { - opcode_location: ErrorLocation, - predicate_value: F, - }, + PredicateLargerThanOne { opcode_location: ErrorLocation, predicate_value: F }, } impl From for OpcodeResolutionError { @@ -242,7 +239,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { assertion_payloads, profiling_active: false, profiling_samples: Vec::new(), - pedantic_solving: pedantic_solving, + pedantic_solving, } } @@ -489,10 +486,14 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { unreachable!("Not executing a BrilligCall opcode"); }; - let opcode_location = ErrorLocation::Resolved(OpcodeLocation::Acir( - self.instruction_pointer(), - )); - if is_predicate_false(&self.witness_map, predicate, self.pedantic_solving, &opcode_location)? { + let opcode_location = + ErrorLocation::Resolved(OpcodeLocation::Acir(self.instruction_pointer())); + if is_predicate_false( + &self.witness_map, + predicate, + self.pedantic_solving, + &opcode_location, + )? { return BrilligSolver::::zero_out_brillig_outputs(&mut self.witness_map, outputs) .map(|_| None); } @@ -510,6 +511,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.instruction_pointer, *id, self.profiling_active, + self.pedantic_solving, )?, }; @@ -560,14 +562,14 @@ 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 opcode_location = + ErrorLocation::Resolved(OpcodeLocation::Acir(self.instruction_pointer())); let witness = &mut self.witness_map; - let should_skip = match is_predicate_false(witness, predicate, self.pedantic_solving, &opcode_location) { - Ok(result) => result, - Err(err) => return StepResult::Status(self.handle_opcode_resolution(Err(err))), - }; + let should_skip = + match is_predicate_false(witness, predicate, self.pedantic_solving, &opcode_location) { + Ok(result) => result, + Err(err) => return StepResult::Status(self.handle_opcode_resolution(Err(err))), + }; if should_skip { let resolution = BrilligSolver::::zero_out_brillig_outputs(witness, outputs); return StepResult::Status(self.handle_opcode_resolution(resolution)); @@ -582,6 +584,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.instruction_pointer, *id, self.profiling_active, + self.pedantic_solving, ); match solver { Ok(solver) => StepResult::IntoBrillig(solver), @@ -606,16 +609,18 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { unreachable!("Not executing a Call opcode"); }; - let opcode_location = ErrorLocation::Resolved(OpcodeLocation::Acir( - self.instruction_pointer(), - )); + let opcode_location = + ErrorLocation::Resolved(OpcodeLocation::Acir(self.instruction_pointer())); if *id == AcirFunctionId(0) { - return Err(OpcodeResolutionError::AcirMainCallAttempted { - opcode_location, - }); + return Err(OpcodeResolutionError::AcirMainCallAttempted { opcode_location }); } - if is_predicate_false(&self.witness_map, predicate, self.pedantic_solving, &opcode_location)? { + if is_predicate_false( + &self.witness_map, + predicate, + self.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)?; @@ -767,7 +772,7 @@ pub(crate) fn is_predicate_false( } else if predicate_value.is_one() { Ok(false) } else { - let opcode_location = opcode_location.clone(); + let opcode_location = *opcode_location; Err(OpcodeResolutionError::PredicateLargerThanOne { opcode_location, predicate_value, @@ -777,7 +782,7 @@ pub(crate) fn is_predicate_false( } else { pred_value.map(|pred_value| pred_value.is_zero()) } - }, + } // If the predicate is `None`, then we treat it as an unconditional `true` None => Ok(false), } diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index 8b164b7c0f2..b6e9c151be1 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/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; @@ -47,7 +47,15 @@ fn bls12_381_circuit() { ]) .into(); - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &[], &[]); + let pedantic_solving = true; + let mut acvm = ACVM::new( + &StubbedBlackBoxSolver, + &opcodes, + witness_assignments, + &[], + &[], + pedantic_solving, + ); // 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"); @@ -169,12 +177,14 @@ fn inversion_brillig_oracle_equivalence() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; + let pedantic_solving = true; let mut acvm = ACVM::new( &StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions, &[], + pedantic_solving, ); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -335,12 +345,14 @@ fn double_inversion_brillig_oracle() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; + let pedantic_solving = true; let mut acvm = ACVM::new( &StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions, &[], + pedantic_solving, ); // use the partial witness generation solver with our acir program @@ -492,12 +504,14 @@ 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 pedantic_solving = true; let mut acvm = ACVM::new( &StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions, &[], + pedantic_solving, ); // use the partial witness generation solver with our acir program @@ -614,12 +628,14 @@ fn brillig_oracle_predicate() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; + let pedantic_solving = true; let mut acvm = ACVM::new( &StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions, &[], + pedantic_solving, ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved, "should be fully solved"); @@ -655,8 +671,15 @@ 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 pedantic_solving = true; + let mut acvm = ACVM::new( + &StubbedBlackBoxSolver, + &opcodes, + values, + &unconstrained_functions, + &[], + pedantic_solving, + ); let solver_status = acvm.solve(); assert_eq!( solver_status, @@ -779,8 +802,15 @@ 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 pedantic_solving = true; + let mut acvm = ACVM::new( + &StubbedBlackBoxSolver, + &opcodes, + values, + &unconstrained_functions, + &[], + pedantic_solving, + ); let solver_status = acvm.solve(); assert_eq!( solver_status, @@ -829,8 +859,15 @@ 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 pedantic_solving = true; + let mut acvm = ACVM::new( + &StubbedBlackBoxSolver, + &opcodes, + initial_witness, + &unconstrained_functions, + &[], + pedantic_solving, + ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); @@ -838,39 +875,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) @@ -917,6 +921,7 @@ fn solve_array_input_blackbox_call( inputs: Vec, num_outputs: usize, num_bits: Option, + pedantic_solving: bool, f: F, ) -> Result, OpcodeResolutionError> where @@ -935,8 +940,14 @@ 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( + &Bn254BlackBoxSolver, + &opcodes, + initial_witness, + &unconstrained_functions, + &[], + pedantic_solving, + ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); @@ -948,7 +959,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)| { @@ -1029,6 +1040,7 @@ fn bigint_solve_binary_op_opt( modulus: Vec, lhs: Vec, rhs: Vec, + pedantic_solving: bool, ) -> Result, OpcodeResolutionError> { let initial_witness_vec: Vec<_> = lhs .iter() @@ -1071,8 +1083,14 @@ 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( + &StubbedBlackBoxSolver, + &opcodes, + initial_witness, + &unconstrained_functions, + &[], + pedantic_solving, + ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); @@ -1114,8 +1132,15 @@ 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 pedantic_solving = true; + let mut acvm = ACVM::new( + &StubbedBlackBoxSolver, + &opcodes, + initial_witness, + &unconstrained_functions, + &[], + pedantic_solving, + ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); @@ -1213,10 +1238,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, )?; @@ -1256,8 +1283,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: @@ -1267,8 +1295,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( @@ -1361,6 +1419,7 @@ fn prop_assert_injective( distinct_inputs: Vec, num_outputs: usize, num_bits: Option, + pedantic_solving: bool, op: F, ) -> (bool, String) where @@ -1372,11 +1431,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) } @@ -1456,10 +1526,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].try_into().unwrap(), 8, None, + pedantic_solving, sha256_compression_op, ); let expected_results: Vec<_> = vec![ @@ -1474,7 +1546,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, @@ -1487,7 +1560,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, @@ -1500,10 +1574,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![ @@ -1619,7 +1695,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); } } @@ -1627,14 +1704,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); } @@ -1643,7 +1722,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); } @@ -1652,12 +1732,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 { @@ -1665,25 +1746,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 results = bigint_solve_from_to_le_bytes(modulus.clone(), 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 pedantic_solving = true; + let results = bigint_solve_from_to_le_bytes(modulus.clone(), input, pedantic_solving); prop_assert_eq!(results, expected_results) } @@ -1696,94 +1798,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) } @@ -1793,7 +1910,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) } @@ -1802,7 +1920,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) } @@ -1810,14 +1929,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) } @@ -1825,7 +1946,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) } @@ -1833,14 +1955,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) } @@ -1849,7 +1973,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 = "--pedantic-solving: BigIntDiv 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) } @@ -1857,7 +1992,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) } @@ -1865,16 +2001,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) } @@ -1882,9 +2020,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) } @@ -1892,9 +2031,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) } @@ -1902,9 +2042,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/acvm-repo/blackbox_solver/Cargo.toml b/acvm-repo/blackbox_solver/Cargo.toml index fe3a938c503..0ec7267506a 100644 --- a/acvm-repo/blackbox_solver/Cargo.toml +++ b/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/acvm-repo/blackbox_solver/src/bigint.rs b/acvm-repo/blackbox_solver/src/bigint.rs index 30c92af15f5..c50b88e4043 100644 --- a/acvm-repo/blackbox_solver/src/bigint.rs +++ b/acvm-repo/blackbox_solver/src/bigint.rs @@ -50,7 +50,18 @@ impl BigIntSolver { inputs: &[u8], modulus: &[u8], output: u32, + pedantic_solving: bool, ) -> Result<(), BlackBoxResolutionError> { + if pedantic_solving && (!self.is_valid_modulus(modulus) || inputs.len() > modulus.len()) { + if !self.is_valid_modulus(modulus) { + panic!("--pedantic-solving: bigint_from_bytes: disallowed modulus {:?}", modulus); + } else { + 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); @@ -69,6 +80,7 @@ impl BigIntSolver { rhs: u32, output: u32, func: BlackBoxFunc, + pedantic_solving: bool, ) -> Result<(), BlackBoxResolutionError> { let modulus = self.get_modulus(lhs, func)?; let lhs = self.get_bigint(lhs, func)?; @@ -84,8 +96,11 @@ impl BigIntSolver { } BlackBoxFunc::BigIntMul => lhs * rhs, BlackBoxFunc::BigIntDiv => { + if pedantic_solving && rhs == BigUint::ZERO { + panic!("--pedantic-solving: BigIntDiv by zero"); + } 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 { @@ -97,9 +112,40 @@ impl BigIntSolver { 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 { - let modulus = BigUint::from_bytes_le(modulus); - self.bigint_id_to_modulus.values().any(|value| modulus == *value) + Self::allowed_bigint_moduli().into_iter().any(|allowed_modulus| allowed_modulus == modulus) } } @@ -121,9 +167,10 @@ impl BigIntSolverWithId { &mut self, inputs: &[u8], modulus: &[u8], + pedantic_solving: bool, ) -> Result { let id = self.create_bigint_id(); - self.solver.bigint_from_bytes(inputs, modulus, id)?; + self.solver.bigint_from_bytes(inputs, modulus, id, pedantic_solving)?; Ok(id) } @@ -136,6 +183,7 @@ impl BigIntSolverWithId { lhs: u32, rhs: u32, func: BlackBoxFunc, + pedantic_solving: bool, ) -> Result { let modulus_lhs = self.solver.get_modulus(lhs, func)?; let modulus_rhs = self.solver.get_modulus(rhs, func)?; @@ -146,7 +194,27 @@ impl BigIntSolverWithId { )); } let id = self.create_bigint_id(); - self.solver.bigint_op(lhs, rhs, id, func)?; + self.solver.bigint_op(lhs, rhs, id, func, pedantic_solving)?; 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"), + 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/acvm-repo/blackbox_solver/src/curve_specific_solver.rs b/acvm-repo/blackbox_solver/src/curve_specific_solver.rs index 869017f52ee..77fc54ac133 100644 --- a/acvm-repo/blackbox_solver/src/curve_specific_solver.rs +++ b/acvm-repo/blackbox_solver/src/curve_specific_solver.rs @@ -19,6 +19,7 @@ pub trait BlackBoxFunctionSolver { points: &[F], scalars_lo: &[F], scalars_hi: &[F], + pedantic_solving: bool, ) -> Result<(F, F, F), BlackBoxResolutionError>; fn ec_add( &self, @@ -62,6 +63,7 @@ impl BlackBoxFunctionSolver for StubbedBlackBoxSolver { _points: &[F], _scalars_lo: &[F], _scalars_hi: &[F], + _pedantic_solving: bool, ) -> Result<(F, F, F), BlackBoxResolutionError> { Err(Self::fail(BlackBoxFunc::MultiScalarMul)) } diff --git a/acvm-repo/bn254_blackbox_solver/src/lib.rs b/acvm-repo/bn254_blackbox_solver/src/lib.rs index d74c17a52b5..d76ac85c12b 100644 --- a/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -48,6 +48,8 @@ impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { points: &[FieldElement], scalars_lo: &[FieldElement], scalars_hi: &[FieldElement], + // check in local multi_scalar_mul + _pedantic_solving: bool, ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { multi_scalar_mul(points, scalars_lo, scalars_hi) } diff --git a/acvm-repo/brillig_vm/src/black_box.rs b/acvm-repo/brillig_vm/src/black_box.rs index 19e2dd7553d..8107cc00ad2 100644 --- a/acvm-repo/brillig_vm/src/black_box.rs +++ b/acvm-repo/brillig_vm/src/black_box.rs @@ -45,6 +45,7 @@ pub(crate) fn evaluate_black_box solver: &Solver, memory: &mut Memory, bigint_solver: &mut BrilligBigIntSolver, + pedantic_solving: bool, ) -> Result<(), BlackBoxResolutionError> { match op { BlackBoxOp::AES128Encrypt { inputs, iv, key, outputs } => { @@ -178,7 +179,8 @@ pub(crate) fn evaluate_black_box scalars_hi.push(*scalar); } } - let (x, y, is_infinite) = solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; + let (x, y, is_infinite) = + solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving)?; memory.write_slice( memory.read_ref(result.pointer), &[ @@ -226,7 +228,8 @@ pub(crate) fn evaluate_black_box let lhs = memory.read(*lhs).try_into().unwrap(); let rhs = memory.read(*rhs).try_into().unwrap(); - let new_id = bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntAdd)?; + let new_id = + bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntAdd, pedantic_solving)?; memory.write(*output, new_id.into()); Ok(()) } @@ -234,7 +237,8 @@ pub(crate) fn evaluate_black_box let lhs = memory.read(*lhs).try_into().unwrap(); let rhs = memory.read(*rhs).try_into().unwrap(); - let new_id = bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntSub)?; + let new_id = + bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntSub, pedantic_solving)?; memory.write(*output, new_id.into()); Ok(()) } @@ -242,7 +246,8 @@ pub(crate) fn evaluate_black_box let lhs = memory.read(*lhs).try_into().unwrap(); let rhs = memory.read(*rhs).try_into().unwrap(); - let new_id = bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntMul)?; + let new_id = + bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntMul, pedantic_solving)?; memory.write(*output, new_id.into()); Ok(()) } @@ -250,7 +255,8 @@ pub(crate) fn evaluate_black_box let lhs = memory.read(*lhs).try_into().unwrap(); let rhs = memory.read(*rhs).try_into().unwrap(); - let new_id = bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntDiv)?; + let new_id = + bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntDiv, pedantic_solving)?; memory.write(*output, new_id.into()); Ok(()) } @@ -260,7 +266,7 @@ pub(crate) fn evaluate_black_box let modulus = read_heap_vector(memory, modulus); let modulus: Vec = modulus.iter().map(|&x| x.try_into().unwrap()).collect(); - let new_id = bigint_solver.bigint_from_bytes(&input, &modulus)?; + let new_id = bigint_solver.bigint_from_bytes(&input, &modulus, pedantic_solving)?; memory.write(*output, new_id.into()); Ok(()) diff --git a/acvm-repo/brillig_vm/src/lib.rs b/acvm-repo/brillig_vm/src/lib.rs index 5b3688339b5..dda381e6226 100644 --- a/acvm-repo/brillig_vm/src/lib.rs +++ b/acvm-repo/brillig_vm/src/lib.rs @@ -100,6 +100,8 @@ pub struct VM<'a, F, B: BlackBoxFunctionSolver> { profiling_active: bool, // Samples for profiling the VM execution. profiling_samples: BrilligProfilingSamples, + // Use pedantic ACVM solving + pedantic_solving: bool, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { @@ -110,6 +112,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { foreign_call_results: Vec>, black_box_solver: &'a B, profiling_active: bool, + pedantic_solving: bool, ) -> Self { Self { calldata, @@ -124,6 +127,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { bigint_solver: Default::default(), profiling_active, profiling_samples: Vec::with_capacity(bytecode.len()), + pedantic_solving, } } @@ -405,6 +409,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { self.black_box_solver, &mut self.memory, &mut self.bigint_solver, + self.pedantic_solving, ) { Ok(()) => self.increment_program_counter(), Err(e) => self.fail(e.to_string()), diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index 29a46fdca2d..4cf97fccf08 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -314,6 +314,7 @@ pub fn check_crate( context, options.debug_comptime_in_file.as_deref(), error_on_unused_imports, + options.pedantic_solving, ); errors.extend(diagnostics.into_iter().map(|(error, file_id)| { let diagnostic = CustomDiagnostic::from(&error); diff --git a/compiler/noirc_evaluator/src/acir/acir_variable.rs b/compiler/noirc_evaluator/src/acir/acir_variable.rs index 9f2c649ee3e..089e5a7a81f 100644 --- a/compiler/noirc_evaluator/src/acir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/acir/acir_variable.rs @@ -134,6 +134,9 @@ pub(crate) struct AcirContext> { expression_width: ExpressionWidth, pub(crate) warnings: Vec, + + // Use pedantic ACVM solving + pedantic_solving: bool, } impl> AcirContext { @@ -1893,7 +1896,9 @@ impl> AcirContext { inputs: &[BrilligInputs], outputs_types: &[AcirType], ) -> Option> { - let mut memory = (execute_brillig(code, &self.blackbox_solver, inputs)?).into_iter(); + let mut memory = + (execute_brillig(code, &self.blackbox_solver, inputs, self.pedantic_solving)?) + .into_iter(); let outputs_var = vecmap(outputs_types.iter(), |output| match output { AcirType::NumericType(_) => { @@ -2190,6 +2195,7 @@ fn execute_brillig>( code: &[BrilligOpcode], blackbox_solver: &B, inputs: &[BrilligInputs], + pedantic_solving: bool, ) -> Option>> { // Set input values let mut calldata: Vec = Vec::new(); @@ -2215,7 +2221,8 @@ fn execute_brillig>( // Instantiate a Brillig VM given the solved input registers and memory, along with the Brillig bytecode. let profiling_active = false; - let mut vm = VM::new(calldata, code, Vec::new(), blackbox_solver, profiling_active); + let mut vm = + VM::new(calldata, code, Vec::new(), blackbox_solver, profiling_active, pedantic_solving); // Run the Brillig VM on these inputs, bytecode, etc! let vm_status = vm.process_opcodes(); diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs index 301b75e0bd4..60444b91cce 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs @@ -99,8 +99,9 @@ pub(super) fn simplify_msm( } } + let pedantic_solving = true; let Ok((result_x, result_y, result_is_infinity)) = - solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi) + solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving) else { return SimplifyResult::None; }; diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index d1d84c305d2..3e216893e6b 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -620,8 +620,15 @@ impl<'brillig> Context<'brillig> { let foreign_call_results = Vec::new(); let black_box_solver = Bn254BlackBoxSolver; let profiling_active = false; - let mut vm = - VM::new(calldata, bytecode, foreign_call_results, &black_box_solver, profiling_active); + let pedantic_solving = true; + let mut vm = VM::new( + calldata, + bytecode, + foreign_call_results, + &black_box_solver, + profiling_active, + pedantic_solving, + ); let vm_status: VMStatus<_> = vm.process_opcodes(); let VMStatus::Finished { return_data_offset, return_data_size } = vm_status else { return EvaluationResult::CannotEvaluate(*func_id); diff --git a/compiler/noirc_frontend/src/elaborator/comptime.rs b/compiler/noirc_frontend/src/elaborator/comptime.rs index 962356d6dd9..bbbb1063529 100644 --- a/compiler/noirc_frontend/src/elaborator/comptime.rs +++ b/compiler/noirc_frontend/src/elaborator/comptime.rs @@ -94,6 +94,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()); @@ -479,7 +480,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/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index 478504a79be..fa5903e4718 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -170,6 +170,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)] @@ -194,6 +197,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(), @@ -220,6 +224,7 @@ impl<'context> Elaborator<'context> { interpreter_call_stack, in_comptime_context: false, silence_field_visibility_errors: 0, + pedantic_solving, } } @@ -227,6 +232,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, @@ -235,6 +241,7 @@ impl<'context> Elaborator<'context> { crate_id, debug_comptime_in_file, im::Vector::new(), + pedantic_solving, ) } @@ -243,8 +250,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( @@ -252,8 +267,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 diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 49fd86b73bb..e7b40528137 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -66,6 +66,9 @@ pub struct Interpreter<'local, 'interner> { /// Stateful bigint calculator. bigint_solver: BigIntSolverWithId, + + /// Use pedantic ACVM solving + pedantic_solving: bool, } #[allow(unused)] @@ -74,6 +77,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { elaborator: &'local mut Elaborator<'interner>, crate_id: CrateId, current_function: Option, + pedantic_solving: bool, ) -> Self { Self { elaborator, @@ -82,6 +86,7 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { bound_generics: Vec::new(), in_loop: false, bigint_solver: BigIntSolverWithId::default(), + pedantic_solving, } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs index d2611f72535..5c9520981af 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs +++ b/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,19 +53,33 @@ fn call_foreign( args: Vec<(Value, Location)>, return_type: Type, location: Location, + pedantic_solving: bool, ) -> IResult { use BlackBoxFunc::*; match name { "aes128_encrypt" => aes128_encrypt(interner, args, location), - "bigint_from_le_bytes" => { - bigint_from_le_bytes(interner, bigint_solver, args, return_type, location) - } + "bigint_from_le_bytes" => bigint_from_le_bytes( + interner, + bigint_solver, + args, + return_type, + location, + pedantic_solving, + ), "bigint_to_le_bytes" => bigint_to_le_bytes(bigint_solver, args, location), - "bigint_add" => bigint_op(bigint_solver, BigIntAdd, args, return_type, location), - "bigint_sub" => bigint_op(bigint_solver, BigIntSub, args, return_type, location), - "bigint_mul" => bigint_op(bigint_solver, BigIntMul, args, return_type, location), - "bigint_div" => bigint_op(bigint_solver, BigIntDiv, args, return_type, location), + "bigint_add" => { + bigint_op(bigint_solver, BigIntAdd, args, return_type, location, pedantic_solving) + } + "bigint_sub" => { + bigint_op(bigint_solver, BigIntSub, args, return_type, location, pedantic_solving) + } + "bigint_mul" => { + bigint_op(bigint_solver, BigIntMul, args, return_type, location, pedantic_solving) + } + "bigint_div" => { + bigint_op(bigint_solver, BigIntDiv, args, return_type, location, pedantic_solving) + } "blake2s" => blake_hash(interner, args, location, acvm::blackbox_solver::blake2s), "blake3" => blake_hash(interner, args, location, acvm::blackbox_solver::blake3), "ecdsa_secp256k1" => ecdsa_secp256_verify( @@ -80,7 +95,7 @@ fn call_foreign( acvm::blackbox_solver::ecdsa_secp256r1_verify, ), "embedded_curve_add" => embedded_curve_add(args, location), - "multi_scalar_mul" => multi_scalar_mul(interner, args, location), + "multi_scalar_mul" => multi_scalar_mul(interner, args, location, pedantic_solving), "poseidon2_permutation" => poseidon2_permutation(interner, args, location), "keccakf1600" => keccakf1600(interner, args, location), "range" => apply_range_constraint(args, location), @@ -148,6 +163,7 @@ fn bigint_from_le_bytes( arguments: Vec<(Value, Location)>, return_type: Type, location: Location, + pedantic_solving: bool, ) -> IResult { let (bytes, modulus) = check_two_arguments(arguments, location)?; @@ -155,7 +171,7 @@ fn bigint_from_le_bytes( let (modulus, _) = get_slice_map(interner, modulus, get_u8)?; let id = solver - .bigint_from_bytes(&bytes, &modulus) + .bigint_from_bytes(&bytes, &modulus, pedantic_solving) .map_err(|e| InterpreterError::BlackBoxError(e, location))?; Ok(to_bigint(id, return_type)) @@ -191,6 +207,7 @@ fn bigint_op( arguments: Vec<(Value, Location)>, return_type: Type, location: Location, + pedantic_solving: bool, ) -> IResult { let (lhs, rhs) = check_two_arguments(arguments, location)?; @@ -198,7 +215,7 @@ fn bigint_op( let rhs = get_bigint_id(rhs)?; let id = solver - .bigint_op(lhs, rhs, func) + .bigint_op(lhs, rhs, func, pedantic_solving) .map_err(|e| InterpreterError::BlackBoxError(e, location))?; Ok(to_bigint(id, return_type)) @@ -293,6 +310,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)?; @@ -308,7 +326,7 @@ fn multi_scalar_mul( } let (x, y, inf) = Bn254BlackBoxSolver - .multi_scalar_mul(&points, &scalars_lo, &scalars_hi) + .multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving) .map_err(|e| InterpreterError::BlackBoxError(e, location))?; Ok(to_field_array(&[x, y, inf])) @@ -436,6 +454,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, @@ -443,6 +462,7 @@ mod tests { Vec::new(), Type::Unit, no_location, + pedantic_solving, ) { Ok(_) => { // Exists and works with no args (unlikely) diff --git a/compiler/noirc_frontend/src/hir/comptime/tests.rs b/compiler/noirc_frontend/src/hir/comptime/tests.rs index 2d3bf928917..ce8d43e56d5 100644 --- a/compiler/noirc_frontend/src/hir/comptime/tests.rs +++ b/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/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 51e62599b05..8d53af3b7f1 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -273,6 +273,7 @@ impl DefCollector { ast: SortedModule, root_file_id: FileId, debug_comptime_in_file: Option<&str>, + pedantic_solving: bool, error_on_unused_items: bool, ) -> Vec<(CompilationError, FileId)> { let mut errors: Vec<(CompilationError, FileId)> = vec![]; @@ -291,6 +292,7 @@ impl DefCollector { dep.crate_id, context, debug_comptime_in_file, + pedantic_solving, error_on_usage_tracker, )); @@ -457,8 +459,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/compiler/noirc_frontend/src/hir/def_map/mod.rs b/compiler/noirc_frontend/src/hir/def_map/mod.rs index 3bb16a92fdb..e87779a0efa 100644 --- a/compiler/noirc_frontend/src/hir/def_map/mod.rs +++ b/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, error_on_unused_imports: bool, ) -> Vec<(CompilationError, FileId)> { // Check if this Crate has already been compiled @@ -121,6 +122,7 @@ impl CrateDefMap { ast, root_file_id, debug_comptime_in_file, + pedantic_solving, error_on_unused_imports, )); diff --git a/compiler/noirc_frontend/src/tests.rs b/compiler/noirc_frontend/src/tests.rs index cba29d58ea3..abbfcdff260 100644 --- a/compiler/noirc_frontend/src/tests.rs +++ b/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; let error_on_unused_imports = true; // Now we want to populate the CrateDefMap using the DefCollector @@ -123,6 +124,7 @@ pub(crate) fn get_program_with_maybe_parser_errors( program.clone().into_sorted(), root_file_id, debug_comptime_in_file, + pedantic_solving, error_on_unused_imports, )); } diff --git a/tooling/acvm_cli/src/cli/execute_cmd.rs b/tooling/acvm_cli/src/cli/execute_cmd.rs index c5795714545..7d2abd9fc37 100644 --- a/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -45,7 +45,8 @@ pub(crate) struct ExecuteCommand { 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, args.pedantic_solving)?; + 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, diff --git a/tooling/lsp/src/solver.rs b/tooling/lsp/src/solver.rs index 3c2d7499880..8cdcb6cb658 100644 --- a/tooling/lsp/src/solver.rs +++ b/tooling/lsp/src/solver.rs @@ -21,11 +21,12 @@ impl BlackBoxFunctionSolver for WrapperSolver { points: &[acvm::FieldElement], scalars_lo: &[acvm::FieldElement], scalars_hi: &[acvm::FieldElement], + pedantic_solving: bool, ) -> Result< (acvm::FieldElement, acvm::FieldElement, acvm::FieldElement), acvm::BlackBoxResolutionError, > { - self.0.multi_scalar_mul(points, scalars_lo, scalars_hi) + self.0.multi_scalar_mul(points, scalars_lo, scalars_hi, pedantic_solving) } fn ec_add( diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index 7c2977fd50d..2f20625da25 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -277,5 +277,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, args.pedantic_solving).map_err(CliError::DapError) + loop_uninitialized_dap(server, args.expression_width, args.pedantic_solving) + .map_err(CliError::DapError) } diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 00fa6739c19..0ec689b591a 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/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, args.compile_options.pedantic_solving) + 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( @@ -235,6 +242,11 @@ pub(crate) fn debug_program( ) -> Result>, CliError> { let initial_witness = compiled_program.abi.encode(inputs_map, None)?; - noir_debugger::run_repl_session(&Bn254BlackBoxSolver, compiled_program, initial_witness, pedantic_solving) - .map_err(CliError::from) + noir_debugger::run_repl_session( + &Bn254BlackBoxSolver, + compiled_program, + initial_witness, + pedantic_solving, + ) + .map_err(CliError::from) } diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index 556392e9b4d..c4971f6a97b 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -106,8 +106,14 @@ fn execute_program_and_decode( // 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 witness_stack = - execute_program(&program, &inputs_map, foreign_call_resolver_url, root_path, package_name, pedantic_solving)?; + 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; From d3e7bc52f7684e2358a0898949ebfea1e4614a02 Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Fri, 6 Dec 2024 10:40:49 -0500 Subject: [PATCH 03/22] Update acvm-repo/blackbox_solver/src/bigint.rs Co-authored-by: Akosh Farkash --- acvm-repo/blackbox_solver/src/bigint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/blackbox_solver/src/bigint.rs b/acvm-repo/blackbox_solver/src/bigint.rs index c50b88e4043..558137130a2 100644 --- a/acvm-repo/blackbox_solver/src/bigint.rs +++ b/acvm-repo/blackbox_solver/src/bigint.rs @@ -209,7 +209,7 @@ fn all_allowed_bigint_moduli_are_prime() { let prime_test_config = None; match is_prime(&modulus, prime_test_config) { Primality::Yes => (), - Primality::No => panic!("not all allowed_bigint_moduli are prime"), + 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); From b83d6b59c4a565c115f51e138e6662871518ea48 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Fri, 6 Dec 2024 11:50:35 -0500 Subject: [PATCH 04/22] remove default from structs that require the pedantic_solving parameter, store pedantic_solving in the bigint and blackbox solver structs, use pedantic_solving for tests --- Cargo.lock | 12 ++--- acvm-repo/acvm/src/pwg/blackbox/bigint.rs | 17 ++++--- .../src/pwg/blackbox/embedded_curve_ops.rs | 3 +- acvm-repo/acvm/src/pwg/blackbox/mod.rs | 14 +++--- acvm-repo/acvm/src/pwg/brillig.rs | 4 -- acvm-repo/acvm/src/pwg/mod.rs | 19 +++----- acvm-repo/blackbox_solver/src/bigint.rs | 45 ++++++++++++++----- .../src/curve_specific_solver.rs | 9 ++-- acvm-repo/bn254_blackbox_solver/src/lib.rs | 8 ++-- acvm-repo/brillig_vm/src/black_box.rs | 13 +++--- acvm-repo/brillig_vm/src/lib.rs | 8 +--- .../noirc_evaluator/src/acir/acir_variable.rs | 8 +--- .../src/ssa/ir/instruction/call.rs | 5 ++- .../src/ssa/ir/instruction/call/blackbox.rs | 3 +- .../src/ssa/opt/constant_folding.rs | 5 +-- .../src/hir/comptime/interpreter.rs | 3 +- .../src/hir/comptime/interpreter/foreign.rs | 30 ++++++------- tooling/acvm_cli/src/cli/execute_cmd.rs | 3 +- tooling/debugger/src/context.rs | 3 -- tooling/debugger/src/dap.rs | 4 -- tooling/debugger/src/lib.rs | 6 +-- tooling/debugger/src/repl.rs | 5 --- tooling/lsp/src/solver.rs | 7 ++- tooling/nargo/src/ops/execute.rs | 13 ------ tooling/nargo/src/ops/test.rs | 2 - tooling/nargo_cli/src/cli/dap_cmd.rs | 3 +- tooling/nargo_cli/src/cli/debug_cmd.rs | 3 +- tooling/nargo_cli/src/cli/execute_cmd.rs | 3 +- tooling/nargo_cli/src/cli/info_cmd.rs | 3 +- tooling/nargo_cli/src/cli/lsp_cmd.rs | 3 +- tooling/nargo_cli/tests/stdlib-props.rs | 5 +-- tooling/nargo_cli/tests/stdlib-tests.rs | 3 +- .../src/cli/execution_flamegraph_cmd.rs | 3 +- 33 files changed, 124 insertions(+), 151 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index c1376995a73..d9f888f1374 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -159,12 +159,6 @@ dependencies = [ "memchr", ] -[[package]] -name = "allocator-api2" -version = "0.2.21" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" - [[package]] name = "aligned-vec" version = "0.6.1" @@ -174,6 +168,12 @@ dependencies = [ "equator", ] +[[package]] +name = "allocator-api2" +version = "0.2.21" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "683d7910e743518b0e34f1186f92494becacb047c7b6bf616c96772180fef923" + [[package]] name = "android-tzdata" version = "0.1.1" diff --git a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs index 64356d9e5a0..c6088dd312b 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs @@ -12,25 +12,30 @@ 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], modulus: &[u8], output: u32, initial_witness: &mut WitnessMap, - pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { let bytes = inputs .iter() .map(|input| input_to_value(initial_witness, *input, false).unwrap().to_u128() as u8) .collect::>(); - self.bigint_solver.bigint_from_bytes(&bytes, modulus, output, pedantic_solving)?; + self.bigint_solver.bigint_from_bytes(&bytes, modulus, output)?; Ok(()) } @@ -39,9 +44,8 @@ impl AcvmBigIntSolver { input: u32, outputs: &[Witness], initial_witness: &mut WitnessMap, - pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { - if pedantic_solving && outputs.len() != 32 { + 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)?; @@ -60,9 +64,8 @@ impl AcvmBigIntSolver { rhs: u32, output: u32, func: BlackBoxFunc, - pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { - self.bigint_solver.bigint_op(lhs, rhs, output, func, pedantic_solving)?; + self.bigint_solver.bigint_op(lhs, rhs, output, func)?; Ok(()) } } diff --git a/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs b/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs index cb195d32acc..9e511571275 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/embedded_curve_ops.rs @@ -13,7 +13,6 @@ pub(super) fn multi_scalar_mul( points: &[FunctionInput], scalars: &[FunctionInput], outputs: (Witness, Witness, Witness), - pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { let points: Result, _> = points.iter().map(|input| input_to_value(initial_witness, *input, false)).collect(); @@ -32,7 +31,7 @@ pub(super) fn multi_scalar_mul( } // Call the backend's multi-scalar multiplication function let (res_x, res_y, is_infinite) = - backend.multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving)?; + backend.multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; // Insert the resulting point into the witness map insert_value(&outputs.0, res_x, initial_witness)?; diff --git a/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/acvm-repo/acvm/src/pwg/blackbox/mod.rs index 2b91a75e627..fa2b2f39818 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -65,7 +65,6 @@ pub(crate) fn solve( initial_witness: &mut WitnessMap, bb_func: &BlackBoxFuncCall, bigint_solver: &mut AcvmBigIntSolver, - pedantic_solving: bool, ) -> Result<(), OpcodeResolutionError> { let inputs = bb_func.get_inputs_vec(); if !contains_all_inputs(initial_witness, &inputs) { @@ -81,13 +80,13 @@ pub(crate) fn solve( solve_aes128_encryption_opcode(initial_witness, inputs, iv, key, outputs) } BlackBoxFuncCall::AND { lhs, rhs, output } => { - and(initial_witness, lhs, rhs, output, pedantic_solving) + and(initial_witness, lhs, rhs, output, backend.pedantic_solving()) } BlackBoxFuncCall::XOR { lhs, rhs, output } => { - xor(initial_witness, lhs, rhs, output, pedantic_solving) + xor(initial_witness, lhs, rhs, output, backend.pedantic_solving()) } BlackBoxFuncCall::RANGE { input } => { - solve_range_opcode(initial_witness, input, pedantic_solving) + solve_range_opcode(initial_witness, input, backend.pedantic_solving()) } BlackBoxFuncCall::Blake2s { inputs, outputs } => { solve_generic_256_hash_opcode(initial_witness, inputs, None, outputs, blake2s) @@ -154,7 +153,7 @@ pub(crate) fn solve( *output, ), BlackBoxFuncCall::MultiScalarMul { points, scalars, outputs } => { - multi_scalar_mul(backend, initial_witness, points, scalars, *outputs, pedantic_solving) + multi_scalar_mul(backend, initial_witness, points, scalars, *outputs) } BlackBoxFuncCall::EmbeddedCurveAdd { input1, input2, outputs } => { embedded_curve_add(backend, initial_witness, **input1, **input2, *outputs) @@ -169,12 +168,11 @@ pub(crate) fn solve( *rhs, *output, bb_func.get_black_box_func(), - pedantic_solving, ), BlackBoxFuncCall::BigIntFromLeBytes { inputs, modulus, output } => bigint_solver - .bigint_from_bytes(inputs, modulus, *output, initial_witness, pedantic_solving), + .bigint_from_bytes(inputs, modulus, *output, initial_witness), BlackBoxFuncCall::BigIntToLeBytes { input, outputs } => { - bigint_solver.bigint_to_bytes(*input, outputs, initial_witness, pedantic_solving) + bigint_solver.bigint_to_bytes(*input, outputs, initial_witness) } BlackBoxFuncCall::Sha256Compression { inputs, hash_values, outputs } => { solve_sha_256_permutation_opcode(initial_witness, inputs, hash_values, outputs) diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index e3502cfc04f..8f9733ac200 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -67,7 +67,6 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { acir_index: usize, brillig_function_id: BrilligFunctionId, profiling_active: bool, - pedantic_solving: bool, ) -> Result> { let vm = Self::setup_brillig_vm( initial_witness, @@ -76,7 +75,6 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { brillig_bytecode, bb_solver, profiling_active, - pedantic_solving, )?; Ok(Self { vm, acir_index, function_id: brillig_function_id }) } @@ -88,7 +86,6 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { brillig_bytecode: &'b [BrilligOpcode], bb_solver: &'b B, profiling_active: bool, - pedantic_solving: bool, ) -> Result, OpcodeResolutionError> { // Set input values let mut calldata: Vec = Vec::new(); @@ -142,7 +139,6 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { vec![], bb_solver, profiling_active, - pedantic_solving, ); Ok(vm) } diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 0237afd7e5e..7c62982f2b3 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -209,9 +209,6 @@ pub struct ACVM<'a, F, B: BlackBoxFunctionSolver> { profiling_active: bool, profiling_samples: ProfilingSamples, - - // when set, check BlackBoxFuncCall and AcvmBigIntSolver assumptions during solving - pub pedantic_solving: bool, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { @@ -221,14 +218,14 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { initial_witness: WitnessMap, unconstrained_functions: &'a [BrilligBytecode], assertion_payloads: &'a [(OpcodeLocation, AssertionPayload)], - pedantic_solving: bool, ) -> 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, @@ -239,7 +236,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { assertion_payloads, profiling_active: false, profiling_samples: Vec::new(), - pedantic_solving, } } @@ -374,7 +370,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { &mut self.witness_map, bb_func, &mut self.bigint_solver, - self.pedantic_solving, ), Opcode::MemoryInit { block_id, init, .. } => { let solver = self.block_solvers.entry(*block_id).or_default(); @@ -382,7 +377,7 @@ 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, self.pedantic_solving) + 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), @@ -491,7 +486,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { if is_predicate_false( &self.witness_map, predicate, - self.pedantic_solving, + self.backend.pedantic_solving(), &opcode_location, )? { return BrilligSolver::::zero_out_brillig_outputs(&mut self.witness_map, outputs) @@ -511,7 +506,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.instruction_pointer, *id, self.profiling_active, - self.pedantic_solving, )?, }; @@ -566,7 +560,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { ErrorLocation::Resolved(OpcodeLocation::Acir(self.instruction_pointer())); let witness = &mut self.witness_map; let should_skip = - match is_predicate_false(witness, predicate, self.pedantic_solving, &opcode_location) { + 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))), }; @@ -584,7 +578,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { self.instruction_pointer, *id, self.profiling_active, - self.pedantic_solving, ); match solver { Ok(solver) => StepResult::IntoBrillig(solver), @@ -618,7 +611,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { if is_predicate_false( &self.witness_map, predicate, - self.pedantic_solving, + self.backend.pedantic_solving(), &opcode_location, )? { // Zero out the outputs if we have a false predicate diff --git a/acvm-repo/blackbox_solver/src/bigint.rs b/acvm-repo/blackbox_solver/src/bigint.rs index 558137130a2..97cad74fc13 100644 --- a/acvm-repo/blackbox_solver/src/bigint.rs +++ b/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, @@ -50,9 +64,8 @@ impl BigIntSolver { inputs: &[u8], modulus: &[u8], output: u32, - pedantic_solving: bool, ) -> Result<(), BlackBoxResolutionError> { - if pedantic_solving && (!self.is_valid_modulus(modulus) || inputs.len() > modulus.len()) { + if self.pedantic_solving && (!self.is_valid_modulus(modulus) || inputs.len() > modulus.len()) { if !self.is_valid_modulus(modulus) { panic!("--pedantic-solving: bigint_from_bytes: disallowed modulus {:?}", modulus); } else { @@ -80,7 +93,6 @@ impl BigIntSolver { rhs: u32, output: u32, func: BlackBoxFunc, - pedantic_solving: bool, ) -> Result<(), BlackBoxResolutionError> { let modulus = self.get_modulus(lhs, func)?; let lhs = self.get_bigint(lhs, func)?; @@ -96,8 +108,11 @@ impl BigIntSolver { } BlackBoxFunc::BigIntMul => lhs * rhs, BlackBoxFunc::BigIntDiv => { - if pedantic_solving && rhs == BigUint::ZERO { - panic!("--pedantic-solving: BigIntDiv by zero"); + 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) } @@ -150,13 +165,21 @@ impl BigIntSolver { } /// 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; @@ -167,10 +190,9 @@ impl BigIntSolverWithId { &mut self, inputs: &[u8], modulus: &[u8], - pedantic_solving: bool, ) -> Result { let id = self.create_bigint_id(); - self.solver.bigint_from_bytes(inputs, modulus, id, pedantic_solving)?; + self.solver.bigint_from_bytes(inputs, modulus, id)?; Ok(id) } @@ -183,7 +205,6 @@ impl BigIntSolverWithId { lhs: u32, rhs: u32, func: BlackBoxFunc, - pedantic_solving: bool, ) -> Result { let modulus_lhs = self.solver.get_modulus(lhs, func)?; let modulus_rhs = self.solver.get_modulus(rhs, func)?; @@ -194,7 +215,7 @@ impl BigIntSolverWithId { )); } let id = self.create_bigint_id(); - self.solver.bigint_op(lhs, rhs, id, func, pedantic_solving)?; + self.solver.bigint_op(lhs, rhs, id, func)?; Ok(id) } } diff --git a/acvm-repo/blackbox_solver/src/curve_specific_solver.rs b/acvm-repo/blackbox_solver/src/curve_specific_solver.rs index 77fc54ac133..82b74518286 100644 --- a/acvm-repo/blackbox_solver/src/curve_specific_solver.rs +++ b/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 schnorr_verify( &self, public_key_x: &F, @@ -19,7 +20,6 @@ pub trait BlackBoxFunctionSolver { points: &[F], scalars_lo: &[F], scalars_hi: &[F], - pedantic_solving: bool, ) -> Result<(F, F, F), BlackBoxResolutionError>; fn ec_add( &self, @@ -37,7 +37,7 @@ pub trait BlackBoxFunctionSolver { ) -> Result, BlackBoxResolutionError>; } -pub struct StubbedBlackBoxSolver; +pub struct StubbedBlackBoxSolver(pub /* pedantic_solving: */ bool); impl StubbedBlackBoxSolver { fn fail(black_box_function: BlackBoxFunc) -> BlackBoxResolutionError { @@ -49,6 +49,10 @@ impl StubbedBlackBoxSolver { } impl BlackBoxFunctionSolver for StubbedBlackBoxSolver { + fn pedantic_solving(&self) -> bool { + self.0 + } + fn schnorr_verify( &self, _public_key_x: &F, @@ -63,7 +67,6 @@ impl BlackBoxFunctionSolver for StubbedBlackBoxSolver { _points: &[F], _scalars_lo: &[F], _scalars_hi: &[F], - _pedantic_solving: bool, ) -> Result<(F, F, F), BlackBoxResolutionError> { Err(Self::fail(BlackBoxFunc::MultiScalarMul)) } diff --git a/acvm-repo/bn254_blackbox_solver/src/lib.rs b/acvm-repo/bn254_blackbox_solver/src/lib.rs index d76ac85c12b..23a44ad8db4 100644 --- a/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -22,9 +22,13 @@ pub use poseidon2::{ type FieldElement = acir::acir_field::GenericFieldElement; #[derive(Default)] -pub struct Bn254BlackBoxSolver; +pub struct Bn254BlackBoxSolver(pub /* pedantic_solving: */ bool); impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { + fn pedantic_solving(&self) -> bool { + self.0 + } + fn schnorr_verify( &self, public_key_x: &FieldElement, @@ -48,8 +52,6 @@ impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { points: &[FieldElement], scalars_lo: &[FieldElement], scalars_hi: &[FieldElement], - // check in local multi_scalar_mul - _pedantic_solving: bool, ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { multi_scalar_mul(points, scalars_lo, scalars_hi) } diff --git a/acvm-repo/brillig_vm/src/black_box.rs b/acvm-repo/brillig_vm/src/black_box.rs index 8107cc00ad2..0d97d28b9ed 100644 --- a/acvm-repo/brillig_vm/src/black_box.rs +++ b/acvm-repo/brillig_vm/src/black_box.rs @@ -45,7 +45,6 @@ pub(crate) fn evaluate_black_box solver: &Solver, memory: &mut Memory, bigint_solver: &mut BrilligBigIntSolver, - pedantic_solving: bool, ) -> Result<(), BlackBoxResolutionError> { match op { BlackBoxOp::AES128Encrypt { inputs, iv, key, outputs } => { @@ -180,7 +179,7 @@ pub(crate) fn evaluate_black_box } } let (x, y, is_infinite) = - solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving)?; + solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; memory.write_slice( memory.read_ref(result.pointer), &[ @@ -229,7 +228,7 @@ pub(crate) fn evaluate_black_box let rhs = memory.read(*rhs).try_into().unwrap(); let new_id = - bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntAdd, pedantic_solving)?; + bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntAdd)?; memory.write(*output, new_id.into()); Ok(()) } @@ -238,7 +237,7 @@ pub(crate) fn evaluate_black_box let rhs = memory.read(*rhs).try_into().unwrap(); let new_id = - bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntSub, pedantic_solving)?; + bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntSub)?; memory.write(*output, new_id.into()); Ok(()) } @@ -247,7 +246,7 @@ pub(crate) fn evaluate_black_box let rhs = memory.read(*rhs).try_into().unwrap(); let new_id = - bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntMul, pedantic_solving)?; + bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntMul)?; memory.write(*output, new_id.into()); Ok(()) } @@ -256,7 +255,7 @@ pub(crate) fn evaluate_black_box let rhs = memory.read(*rhs).try_into().unwrap(); let new_id = - bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntDiv, pedantic_solving)?; + bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntDiv)?; memory.write(*output, new_id.into()); Ok(()) } @@ -266,7 +265,7 @@ pub(crate) fn evaluate_black_box let modulus = read_heap_vector(memory, modulus); let modulus: Vec = modulus.iter().map(|&x| x.try_into().unwrap()).collect(); - let new_id = bigint_solver.bigint_from_bytes(&input, &modulus, pedantic_solving)?; + let new_id = bigint_solver.bigint_from_bytes(&input, &modulus)?; memory.write(*output, new_id.into()); Ok(()) diff --git a/acvm-repo/brillig_vm/src/lib.rs b/acvm-repo/brillig_vm/src/lib.rs index dda381e6226..8e5d425e91c 100644 --- a/acvm-repo/brillig_vm/src/lib.rs +++ b/acvm-repo/brillig_vm/src/lib.rs @@ -100,8 +100,6 @@ pub struct VM<'a, F, B: BlackBoxFunctionSolver> { profiling_active: bool, // Samples for profiling the VM execution. profiling_samples: BrilligProfilingSamples, - // Use pedantic ACVM solving - pedantic_solving: bool, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { @@ -112,8 +110,8 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { foreign_call_results: Vec>, black_box_solver: &'a B, profiling_active: bool, - pedantic_solving: bool, ) -> Self { + let bigint_solver = BrilligBigIntSolver::with_pedantic_solving(black_box_solver.pedantic_solving()); Self { calldata, program_counter: 0, @@ -124,10 +122,9 @@ 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()), - pedantic_solving, } } @@ -409,7 +406,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> VM<'a, F, B> { self.black_box_solver, &mut self.memory, &mut self.bigint_solver, - self.pedantic_solving, ) { Ok(()) => self.increment_program_counter(), Err(e) => self.fail(e.to_string()), diff --git a/compiler/noirc_evaluator/src/acir/acir_variable.rs b/compiler/noirc_evaluator/src/acir/acir_variable.rs index 089e5a7a81f..20070a9fa5a 100644 --- a/compiler/noirc_evaluator/src/acir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/acir/acir_variable.rs @@ -134,9 +134,6 @@ pub(crate) struct AcirContext> { expression_width: ExpressionWidth, pub(crate) warnings: Vec, - - // Use pedantic ACVM solving - pedantic_solving: bool, } impl> AcirContext { @@ -1897,7 +1894,7 @@ impl> AcirContext { outputs_types: &[AcirType], ) -> Option> { let mut memory = - (execute_brillig(code, &self.blackbox_solver, inputs, self.pedantic_solving)?) + (execute_brillig(code, &self.blackbox_solver, inputs)?) .into_iter(); let outputs_var = vecmap(outputs_types.iter(), |output| match output { @@ -2195,7 +2192,6 @@ fn execute_brillig>( code: &[BrilligOpcode], blackbox_solver: &B, inputs: &[BrilligInputs], - pedantic_solving: bool, ) -> Option>> { // Set input values let mut calldata: Vec = Vec::new(); @@ -2222,7 +2218,7 @@ fn execute_brillig>( // Instantiate a Brillig VM given the solved input registers and memory, along with the Brillig bytecode. let profiling_active = false; let mut vm = - VM::new(calldata, code, Vec::new(), blackbox_solver, profiling_active, pedantic_solving); + VM::new(calldata, code, Vec::new(), blackbox_solver, profiling_active); // Run the Brillig VM on these inputs, bytecode, etc! let vm_status = vm.process_opcodes(); diff --git a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs index 7709e5bc0e1..34c25e8761d 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call.rs @@ -533,11 +533,12 @@ fn simplify_black_box_func( block: BasicBlockId, call_stack: &CallStack, ) -> 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/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs index 60444b91cce..301b75e0bd4 100644 --- a/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs +++ b/compiler/noirc_evaluator/src/ssa/ir/instruction/call/blackbox.rs @@ -99,9 +99,8 @@ pub(super) fn simplify_msm( } } - let pedantic_solving = true; let Ok((result_x, result_y, result_is_infinity)) = - solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving) + solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi) else { return SimplifyResult::None; }; diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index 0b75a0aef74..988e6f93bc2 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -629,16 +629,15 @@ impl<'brillig> Context<'brillig> { let bytecode = &generated_brillig.byte_code; let foreign_call_results = Vec::new(); - let black_box_solver = Bn254BlackBoxSolver; - let profiling_active = false; 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, - pedantic_solving, ); let vm_status: VMStatus<_> = vm.process_opcodes(); let VMStatus::Finished { return_data_offset, return_data_size } = vm_status else { diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs index 8a33e69caa7..c93fa4266a2 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter.rs @@ -79,13 +79,14 @@ impl<'local, 'interner> Interpreter<'local, 'interner> { 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, } } diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs index 5c9520981af..81f930a5081 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs @@ -65,20 +65,19 @@ fn call_foreign( args, return_type, location, - pedantic_solving, ), "bigint_to_le_bytes" => bigint_to_le_bytes(bigint_solver, args, location), "bigint_add" => { - bigint_op(bigint_solver, BigIntAdd, args, return_type, location, pedantic_solving) + bigint_op(bigint_solver, BigIntAdd, args, return_type, location) } "bigint_sub" => { - bigint_op(bigint_solver, BigIntSub, args, return_type, location, pedantic_solving) + bigint_op(bigint_solver, BigIntSub, args, return_type, location) } "bigint_mul" => { - bigint_op(bigint_solver, BigIntMul, args, return_type, location, pedantic_solving) + bigint_op(bigint_solver, BigIntMul, args, return_type, location) } "bigint_div" => { - bigint_op(bigint_solver, BigIntDiv, args, return_type, location, pedantic_solving) + bigint_op(bigint_solver, BigIntDiv, args, return_type, location) } "blake2s" => blake_hash(interner, args, location, acvm::blackbox_solver::blake2s), "blake3" => blake_hash(interner, args, location, acvm::blackbox_solver::blake3), @@ -94,9 +93,9 @@ fn call_foreign( location, acvm::blackbox_solver::ecdsa_secp256r1_verify, ), - "embedded_curve_add" => embedded_curve_add(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), + "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), @@ -163,7 +162,6 @@ fn bigint_from_le_bytes( arguments: Vec<(Value, Location)>, return_type: Type, location: Location, - pedantic_solving: bool, ) -> IResult { let (bytes, modulus) = check_two_arguments(arguments, location)?; @@ -171,7 +169,7 @@ fn bigint_from_le_bytes( let (modulus, _) = get_slice_map(interner, modulus, get_u8)?; let id = solver - .bigint_from_bytes(&bytes, &modulus, pedantic_solving) + .bigint_from_bytes(&bytes, &modulus) .map_err(|e| InterpreterError::BlackBoxError(e, location))?; Ok(to_bigint(id, return_type)) @@ -207,7 +205,6 @@ fn bigint_op( arguments: Vec<(Value, Location)>, return_type: Type, location: Location, - pedantic_solving: bool, ) -> IResult { let (lhs, rhs) = check_two_arguments(arguments, location)?; @@ -215,7 +212,7 @@ fn bigint_op( let rhs = get_bigint_id(rhs)?; let id = solver - .bigint_op(lhs, rhs, func, pedantic_solving) + .bigint_op(lhs, rhs, func) .map_err(|e| InterpreterError::BlackBoxError(e, location))?; Ok(to_bigint(id, return_type)) @@ -287,13 +284,13 @@ 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))?; @@ -325,8 +322,8 @@ fn multi_scalar_mul( scalars_hi.push(hi); } - let (x, y, inf) = Bn254BlackBoxSolver - .multi_scalar_mul(&points, &scalars_lo, &scalars_hi, pedantic_solving) + let (x, y, inf) = Bn254BlackBoxSolver(pedantic_solving) + .multi_scalar_mul(&points, &scalars_lo, &scalars_hi) .map_err(|e| InterpreterError::BlackBoxError(e, location))?; Ok(to_field_array(&[x, y, inf])) @@ -337,13 +334,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))?; diff --git a/tooling/acvm_cli/src/cli/execute_cmd.rs b/tooling/acvm_cli/src/cli/execute_cmd.rs index 7d2abd9fc37..c76e9ae0504 100644 --- a/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -80,9 +80,8 @@ pub(crate) fn execute_program_from_witness( execute_program( &program, inputs_map, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallExecutor::new(true, None, None, None), - pedantic_solving, ) .map_err(CliError::CircuitExecutionError) } diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 997b1698155..0d1164cf9bd 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -261,7 +261,6 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { initial_witness: WitnessMap, foreign_call_executor: Box, unconstrained_functions: &'a [BrilligBytecode], - pedantic_solving: bool, ) -> Self { let source_to_opcodes = build_source_to_opcode_debug_mappings(debug_artifact); let current_circuit_id: u32 = 0; @@ -274,7 +273,6 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { initial_witness, unconstrained_functions, &initial_circuit.assert_messages, - pedantic_solving, ), current_circuit_id, brillig_solver: None, @@ -575,7 +573,6 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { callee_witness_map, self.unconstrained_functions, &callee_circuit.assert_messages, - self.acvm.pedantic_solving, ); let caller_acvm = std::mem::replace(&mut self.acvm, callee_acvm); self.acvm_stack diff --git a/tooling/debugger/src/dap.rs b/tooling/debugger/src/dap.rs index b071e5d802f..cfe33a61cb5 100644 --- a/tooling/debugger/src/dap.rs +++ b/tooling/debugger/src/dap.rs @@ -65,7 +65,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession< debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, unconstrained_functions: &'a [BrilligBytecode], - pedantic_solving: bool, ) -> Self { let context = DebugContext::new( solver, @@ -74,7 +73,6 @@ impl<'a, R: Read, W: Write, B: BlackBoxFunctionSolver> DapSession< initial_witness, Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)), unconstrained_functions, - pedantic_solving, ); Self { server, @@ -609,7 +607,6 @@ pub fn run_session>( solver: &B, program: CompiledProgram, initial_witness: WitnessMap, - pedantic_solving: bool, ) -> Result<(), ServerError> { let debug_artifact = DebugArtifact { debug_symbols: program.debug, file_map: program.file_map }; let mut session = DapSession::new( @@ -619,7 +616,6 @@ pub fn run_session>( &debug_artifact, initial_witness, &program.program.unconstrained_functions, - pedantic_solving, ); session.run_loop() diff --git a/tooling/debugger/src/lib.rs b/tooling/debugger/src/lib.rs index a2ec382a775..37ac088ca35 100644 --- a/tooling/debugger/src/lib.rs +++ b/tooling/debugger/src/lib.rs @@ -19,9 +19,8 @@ pub fn run_repl_session>( solver: &B, program: CompiledProgram, initial_witness: WitnessMap, - pedantic_solving: bool, ) -> Result>, NargoError> { - repl::run(solver, program, initial_witness, pedantic_solving) + repl::run(solver, program, initial_witness) } pub fn run_dap_loop>( @@ -29,7 +28,6 @@ pub fn run_dap_loop>( solver: &B, program: CompiledProgram, initial_witness: WitnessMap, - pedantic_solving: bool, ) -> Result<(), ServerError> { - dap::run_session(server, solver, program, initial_witness, pedantic_solving) + dap::run_session(server, solver, program, initial_witness) } diff --git a/tooling/debugger/src/repl.rs b/tooling/debugger/src/repl.rs index f2a691bca68..486e84060f0 100644 --- a/tooling/debugger/src/repl.rs +++ b/tooling/debugger/src/repl.rs @@ -41,7 +41,6 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { debug_artifact: &'a DebugArtifact, initial_witness: WitnessMap, unconstrained_functions: &'a [BrilligBytecode], - pedantic_solving: bool, ) -> Self { let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)); @@ -52,7 +51,6 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { initial_witness.clone(), foreign_call_executor, unconstrained_functions, - pedantic_solving, ); let last_result = if context.get_current_debug_location().is_none() { // handle circuit with no opcodes @@ -324,7 +322,6 @@ impl<'a, B: BlackBoxFunctionSolver> ReplDebugger<'a, B> { self.initial_witness.clone(), foreign_call_executor, self.unconstrained_functions, - self.context.acvm.pedantic_solving, ); for debug_location in breakpoints { self.context.add_breakpoint(debug_location); @@ -426,7 +423,6 @@ pub fn run>( blackbox_solver: &B, program: CompiledProgram, initial_witness: WitnessMap, - pedantic_solving: bool, ) -> Result>, NargoError> { let circuits = &program.program.functions; let debug_artifact = @@ -438,7 +434,6 @@ pub fn run>( debug_artifact, initial_witness, unconstrained_functions, - pedantic_solving, )); let ref_context = &context; diff --git a/tooling/lsp/src/solver.rs b/tooling/lsp/src/solver.rs index 8cdcb6cb658..ba184abffe1 100644 --- a/tooling/lsp/src/solver.rs +++ b/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 schnorr_verify( &self, public_key_x: &acvm::FieldElement, @@ -21,12 +25,11 @@ impl BlackBoxFunctionSolver for WrapperSolver { points: &[acvm::FieldElement], scalars_lo: &[acvm::FieldElement], scalars_hi: &[acvm::FieldElement], - pedantic_solving: bool, ) -> Result< (acvm::FieldElement, acvm::FieldElement, acvm::FieldElement), acvm::BlackBoxResolutionError, > { - self.0.multi_scalar_mul(points, scalars_lo, scalars_hi, pedantic_solving) + self.0.multi_scalar_mul(points, scalars_lo, scalars_hi) } fn ec_add( diff --git a/tooling/nargo/src/ops/execute.rs b/tooling/nargo/src/ops/execute.rs index c688cbd87c7..57116ec2efd 100644 --- a/tooling/nargo/src/ops/execute.rs +++ b/tooling/nargo/src/ops/execute.rs @@ -37,10 +37,6 @@ struct ProgramExecutor<'a, F, B: BlackBoxFunctionSolver, E: ForeignCallExecut // Flag that states whether we want to profile the VM. Profiling can add extra // execution costs so we want to make sure we only trigger it explicitly. profiling_active: bool, - - // Flag to use pedantic ACVM solving, i.e. double-checking some black box - // function assumptions during solving. - pedantic_solving: bool, } impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> @@ -52,7 +48,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> blackbox_solver: &'a B, foreign_call_executor: &'a mut E, profiling_active: bool, - pedantic_solving: bool, ) -> Self { ProgramExecutor { functions, @@ -63,7 +58,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> call_stack: Vec::default(), current_function_index: 0, profiling_active, - pedantic_solving, } } @@ -83,7 +77,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver, E: ForeignCallExecutor> initial_witness, self.unconstrained_functions, &circuit.assert_messages, - self.pedantic_solving, ); acvm.with_profiler(self.profiling_active); @@ -210,7 +203,6 @@ pub fn execute_program, E: ForeignCal initial_witness: WitnessMap, blackbox_solver: &B, foreign_call_executor: &mut E, - pedantic_solving: bool, ) -> Result, NargoError> { let profiling_active = false; let (witness_stack, profiling_samples) = execute_program_inner( @@ -219,7 +211,6 @@ pub fn execute_program, E: ForeignCal blackbox_solver, foreign_call_executor, profiling_active, - pedantic_solving, )?; assert!(profiling_samples.is_empty(), "Expected no profiling samples"); @@ -235,7 +226,6 @@ pub fn execute_program_with_profiling< initial_witness: WitnessMap, blackbox_solver: &B, foreign_call_executor: &mut E, - pedantic_solving: bool, ) -> Result<(WitnessStack, ProfilingSamples), NargoError> { let profiling_active = true; execute_program_inner( @@ -244,7 +234,6 @@ pub fn execute_program_with_profiling< blackbox_solver, foreign_call_executor, profiling_active, - pedantic_solving, ) } @@ -255,7 +244,6 @@ fn execute_program_inner, E: ForeignC blackbox_solver: &B, foreign_call_executor: &mut E, profiling_active: bool, - pedantic_solving: bool, ) -> Result<(WitnessStack, ProfilingSamples), NargoError> { let mut executor = ProgramExecutor::new( &program.functions, @@ -263,7 +251,6 @@ fn execute_program_inner, E: ForeignC blackbox_solver, foreign_call_executor, profiling_active, - pedantic_solving, ); let (main_witness, profiling_samples) = executor.execute_circuit(initial_witness)?; executor.witness_stack.push(0, main_witness); diff --git a/tooling/nargo/src/ops/test.rs b/tooling/nargo/src/ops/test.rs index b0ad10e7385..e258627b522 100644 --- a/tooling/nargo/src/ops/test.rs +++ b/tooling/nargo/src/ops/test.rs @@ -75,7 +75,6 @@ pub fn run_test>( WitnessMap::new(), blackbox_solver, &mut foreign_call_executor, - config.pedantic_solving, ); let status = test_status_program_compile_pass( @@ -131,7 +130,6 @@ pub fn run_test>( root_path.clone(), package_name.clone(), ), - config.pedantic_solving, ) .map_err(|err| err.to_string()) }; diff --git a/tooling/nargo_cli/src/cli/dap_cmd.rs b/tooling/nargo_cli/src/cli/dap_cmd.rs index 2f20625da25..7fbf685688a 100644 --- a/tooling/nargo_cli/src/cli/dap_cmd.rs +++ b/tooling/nargo_cli/src/cli/dap_cmd.rs @@ -204,10 +204,9 @@ fn loop_uninitialized_dap( noir_debugger::run_dap_loop( server, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), compiled_program, initial_witness, - pedantic_solving, )?; break; } diff --git a/tooling/nargo_cli/src/cli/debug_cmd.rs b/tooling/nargo_cli/src/cli/debug_cmd.rs index 0ec689b591a..a9ebfae0c7d 100644 --- a/tooling/nargo_cli/src/cli/debug_cmd.rs +++ b/tooling/nargo_cli/src/cli/debug_cmd.rs @@ -243,10 +243,9 @@ pub(crate) fn debug_program( let initial_witness = compiled_program.abi.encode(inputs_map, None)?; noir_debugger::run_repl_session( - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), compiled_program, initial_witness, - pedantic_solving, ) .map_err(CliError::from) } diff --git a/tooling/nargo_cli/src/cli/execute_cmd.rs b/tooling/nargo_cli/src/cli/execute_cmd.rs index c4971f6a97b..0c67a22fc46 100644 --- a/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -135,14 +135,13 @@ pub(crate) fn execute_program( let solved_witness_stack_err = nargo::ops::execute_program( &compiled_program.program, initial_witness, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallExecutor::new( true, foreign_call_resolver_url, root_path, package_name, ), - pedantic_solving, ); match solved_witness_stack_err { Ok(solved_witness_stack) => Ok(solved_witness_stack), diff --git a/tooling/nargo_cli/src/cli/info_cmd.rs b/tooling/nargo_cli/src/cli/info_cmd.rs index 9c6ebdc3956..4f843e882b1 100644 --- a/tooling/nargo_cli/src/cli/info_cmd.rs +++ b/tooling/nargo_cli/src/cli/info_cmd.rs @@ -264,9 +264,8 @@ fn profile_brillig_execution( let (_, profiling_samples) = nargo::ops::execute_program_with_profiling( &program_artifact.bytecode, initial_witness, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallExecutor::new(false, None, None, None), - pedantic_solving, )?; let expression_width = get_target_width(package.expression_width, expression_width); diff --git a/tooling/nargo_cli/src/cli/lsp_cmd.rs b/tooling/nargo_cli/src/cli/lsp_cmd.rs index bfaa913b33a..dc5d0995c06 100644 --- a/tooling/nargo_cli/src/cli/lsp_cmd.rs +++ b/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/tooling/nargo_cli/tests/stdlib-props.rs b/tooling/nargo_cli/tests/stdlib-props.rs index b9cdc52f9f8..20c8d3565e4 100644 --- a/tooling/nargo_cli/tests/stdlib-props.rs +++ b/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(DefaultForeignCallExecutor::new(false, None, None, None)); @@ -87,13 +88,11 @@ fn run_snippet_proptest( let initial_witness = program.abi.encode(&io.inputs, None).expect("failed to encode"); let mut foreign_call_executor = foreign_call_executor.borrow_mut(); - let pedandic_solving = true; let witness_stack: WitnessStack = execute_program( &program.program, initial_witness, &blackbox_solver, &mut *foreign_call_executor, - pedandic_solving, ) .expect("failed to execute"); diff --git a/tooling/nargo_cli/tests/stdlib-tests.rs b/tooling/nargo_cli/tests/stdlib-tests.rs index 99f0c9a2e7f..96074f8ca2c 100644 --- a/tooling/nargo_cli/tests/stdlib-tests.rs +++ b/tooling/nargo_cli/tests/stdlib-tests.rs @@ -82,8 +82,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, true, diff --git a/tooling/profiler/src/cli/execution_flamegraph_cmd.rs b/tooling/profiler/src/cli/execution_flamegraph_cmd.rs index f48c00278bc..196e0dad21e 100644 --- a/tooling/profiler/src/cli/execution_flamegraph_cmd.rs +++ b/tooling/profiler/src/cli/execution_flamegraph_cmd.rs @@ -61,9 +61,8 @@ fn run_with_generator( let (_, mut profiling_samples) = nargo::ops::execute_program_with_profiling( &program.bytecode, initial_witness, - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallExecutor::new(true, None, None, None), - pedantic_solving, )?; println!("Executed"); From b314b2189db378e05dd5291d1a377bacafe74894 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Mon, 9 Dec 2024 13:20:07 -0500 Subject: [PATCH 05/22] wip moving 'pedantic_solving' --- acvm-repo/acvm/src/pwg/blackbox/bigint.rs | 4 +- acvm-repo/acvm/src/pwg/blackbox/mod.rs | 14 ++-- acvm-repo/acvm/src/pwg/brillig.rs | 8 +-- acvm-repo/acvm/src/pwg/mod.rs | 21 ++++-- acvm-repo/acvm/tests/solver.rs | 53 ++++++--------- acvm-repo/acvm_js/src/execute.rs | 47 +++++++++++-- acvm-repo/blackbox_solver/src/bigint.rs | 11 ++- .../src/curve_specific_solver.rs | 8 +++ acvm-repo/bn254_blackbox_solver/src/lib.rs | 2 +- acvm-repo/brillig_vm/src/black_box.rs | 15 ++-- acvm-repo/brillig_vm/src/lib.rs | 68 ++++++++++++------- .../noirc_evaluator/src/acir/acir_variable.rs | 7 +- .../noirc_evaluator/src/brillig/brillig_ir.rs | 4 ++ .../src/ssa/opt/constant_folding.rs | 9 +-- .../src/hir/comptime/interpreter/foreign.rs | 36 ++++------ tooling/debugger/src/context.rs | 9 ++- tooling/nargo_cli/tests/stdlib-props.rs | 2 +- 17 files changed, 182 insertions(+), 136 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs index c6088dd312b..9c8b117a096 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/bigint.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/bigint.rs @@ -19,9 +19,7 @@ pub(crate) struct AcvmBigIntSolver { impl AcvmBigIntSolver { pub(crate) fn with_pedantic_solving(pedantic_solving: bool) -> AcvmBigIntSolver { let bigint_solver = BigIntSolver::with_pedantic_solving(pedantic_solving); - AcvmBigIntSolver { - bigint_solver, - } + AcvmBigIntSolver { bigint_solver } } pub(crate) fn bigint_from_bytes( diff --git a/acvm-repo/acvm/src/pwg/blackbox/mod.rs b/acvm-repo/acvm/src/pwg/blackbox/mod.rs index d9f7694042d..2f10e333c24 100644 --- a/acvm-repo/acvm/src/pwg/blackbox/mod.rs +++ b/acvm-repo/acvm/src/pwg/blackbox/mod.rs @@ -145,14 +145,12 @@ pub(crate) fn solve( BlackBoxFuncCall::BigIntAdd { lhs, rhs, output } | BlackBoxFuncCall::BigIntSub { lhs, rhs, output } | BlackBoxFuncCall::BigIntMul { lhs, rhs, output } - | BlackBoxFuncCall::BigIntDiv { lhs, rhs, output } => bigint_solver.bigint_op( - *lhs, - *rhs, - *output, - bb_func.get_black_box_func(), - ), - BlackBoxFuncCall::BigIntFromLeBytes { inputs, modulus, output } => bigint_solver - .bigint_from_bytes(inputs, modulus, *output, initial_witness), + | BlackBoxFuncCall::BigIntDiv { lhs, rhs, output } => { + bigint_solver.bigint_op(*lhs, *rhs, *output, bb_func.get_black_box_func()) + } + BlackBoxFuncCall::BigIntFromLeBytes { inputs, modulus, output } => { + bigint_solver.bigint_from_bytes(inputs, modulus, *output, initial_witness) + } BlackBoxFuncCall::BigIntToLeBytes { input, outputs } => { bigint_solver.bigint_to_bytes(*input, outputs, initial_witness) } diff --git a/acvm-repo/acvm/src/pwg/brillig.rs b/acvm-repo/acvm/src/pwg/brillig.rs index 8f9733ac200..a5f5783478e 100644 --- a/acvm-repo/acvm/src/pwg/brillig.rs +++ b/acvm-repo/acvm/src/pwg/brillig.rs @@ -133,13 +133,7 @@ impl<'b, B: BlackBoxFunctionSolver, F: AcirField> BrilligSolver<'b, F, B> { // Instantiate a Brillig VM given the solved calldata // along with the Brillig bytecode. - let vm = VM::new( - calldata, - brillig_bytecode, - vec![], - bb_solver, - profiling_active, - ); + let vm = VM::new(calldata, brillig_bytecode, vec![], bb_solver, profiling_active); Ok(vm) } diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 7c62982f2b3..673b2219fb6 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -377,7 +377,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, self.backend.pedantic_solving()) + 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), @@ -559,11 +564,15 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { 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, self.backend.pedantic_solving(), &opcode_location) { - Ok(result) => result, - Err(err) => return StepResult::Status(self.handle_opcode_resolution(Err(err))), - }; + 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))), + }; if should_skip { let resolution = BrilligSolver::::zero_out_brillig_outputs(witness, outputs); return StepResult::Status(self.handle_opcode_resolution(resolution)); diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index b6e9c151be1..c700654c1f7 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -28,6 +28,7 @@ use zkhash::poseidon2::poseidon2_params::Poseidon2Params; #[test] fn bls12_381_circuit() { + let solver = StubbedBlackBoxSolver::default(); type Bls12FieldElement = GenericFieldElement; let addition = Opcode::AssertZero(Expression { @@ -47,14 +48,12 @@ fn bls12_381_circuit() { ]) .into(); - let pedantic_solving = true; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &solver, &opcodes, witness_assignments, &[], &[], - pedantic_solving, ); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -68,6 +67,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; @@ -177,14 +177,12 @@ fn inversion_brillig_oracle_equivalence() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let pedantic_solving = true; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &solver, &opcodes, witness_assignments, &unconstrained_functions, &[], - pedantic_solving, ); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -214,6 +212,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; @@ -345,14 +344,12 @@ fn double_inversion_brillig_oracle() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let pedantic_solving = true; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &solver, &opcodes, witness_assignments, &unconstrained_functions, &[], - pedantic_solving, ); // use the partial witness generation solver with our acir program @@ -400,6 +397,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. // @@ -504,14 +502,12 @@ 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 pedantic_solving = true; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &solver, &opcodes, witness_assignments, &unconstrained_functions, &[], - pedantic_solving, ); // use the partial witness generation solver with our acir program @@ -558,6 +554,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); @@ -628,14 +625,12 @@ fn brillig_oracle_predicate() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let pedantic_solving = true; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &solver, &opcodes, witness_assignments, &unconstrained_functions, &[], - pedantic_solving, ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved, "should be fully solved"); @@ -646,6 +641,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); @@ -671,14 +667,12 @@ fn unsatisfied_opcode_resolved() { let opcodes = vec![Opcode::AssertZero(opcode_a)]; let unconstrained_functions = vec![]; - let pedantic_solving = true; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &solver, &opcodes, values, &unconstrained_functions, &[], - pedantic_solving, ); let solver_status = acvm.solve(); assert_eq!( @@ -781,6 +775,7 @@ fn unsatisfied_opcode_resolved_brillig() { values.insert(w_y, FieldElement::from(1_i128)); values.insert(w_result, FieldElement::from(0_i128)); + let pedantic_solving = true; let opcodes = vec![ Opcode::BrilligCall { id: BrilligFunctionId(0), @@ -802,14 +797,12 @@ fn unsatisfied_opcode_resolved_brillig() { Opcode::AssertZero(opcode_a), ]; let unconstrained_functions = vec![brillig_bytecode]; - let pedantic_solving = true; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &solver, &opcodes, values, &unconstrained_functions, &[], - pedantic_solving, ); let solver_status = acvm.solve(); assert_eq!( @@ -857,16 +850,15 @@ fn memory_operations() { q_c: FieldElement::one(), }); + let pedantic_solving = true; let opcodes = vec![init, read_op, expression]; let unconstrained_functions = vec![]; - let pedantic_solving = true; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &StubbedBlackBoxSolver(pedantic_solving), &opcodes, initial_witness, &unconstrained_functions, &[], - pedantic_solving, ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); @@ -941,12 +933,11 @@ where let opcodes = vec![op]; let unconstrained_functions = vec![]; let mut acvm = ACVM::new( - &Bn254BlackBoxSolver, + &Bn254BlackBoxSolver(pedantic_solving), &opcodes, initial_witness, &unconstrained_functions, &[], - pedantic_solving, ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); @@ -1076,6 +1067,7 @@ fn bigint_solve_binary_op_opt( outputs: output_witnesses.clone(), }); + let pedantic_solving = true; let mut opcodes = vec![bigint_from_lhs_op, bigint_from_rhs_op]; if let Some(middle_op) = middle_op { opcodes.push(Opcode::BlackBoxFuncCall(middle_op)); @@ -1084,12 +1076,11 @@ fn bigint_solve_binary_op_opt( let unconstrained_functions = vec![]; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &StubbedBlackBoxSolver(pedantic_solving), &opcodes, initial_witness, &unconstrained_functions, &[], - pedantic_solving, ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); @@ -1113,6 +1104,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; @@ -1129,17 +1121,16 @@ fn solve_blackbox_func_call( rhs_opt = Some(rhs); } + let pedantic_solving = true; let op = Opcode::BlackBoxFuncCall(blackbox_func_call(lhs_opt, rhs_opt)?); let opcodes = vec![op]; let unconstrained_functions = vec![]; - let pedantic_solving = true; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver, + &solver, &opcodes, initial_witness, &unconstrained_functions, &[], - pedantic_solving, ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); diff --git a/acvm-repo/acvm_js/src/execute.rs b/acvm-repo/acvm_js/src/execute.rs index c3627d0eff5..5bcf5a500a3 100644 --- a/acvm-repo/acvm_js/src/execute.rs +++ b/acvm-repo/acvm_js/src/execute.rs @@ -30,11 +30,23 @@ pub async fn execute_circuit( 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_circuit` with pedantic ACVM solving +#[wasm_bindgen(js_name = executeCircuitPedantic, skip_jsdoc)] +pub 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) + 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; @@ -53,6 +65,18 @@ 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 +#[wasm_bindgen(js_name = executeCircuitWithReturnWitnessPedantic, skip_jsdoc)] +pub 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 +87,7 @@ pub async fn execute_circuit_with_return_witness( &program, initial_witness, &foreign_call_handler, + pedantic_solving, ) .await?; let solved_witness = @@ -87,11 +112,23 @@ 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 +#[wasm_bindgen(js_name = executeProgramPedantic, skip_jsdoc)] +pub 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) + execute_program_with_native_type_return(program, initial_witness, &foreign_call_handler, pedantic_solving) .await?; Ok(witness_stack.into()) @@ -101,6 +138,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,7 +147,7 @@ async fn execute_program_with_native_type_return( None, None))?; - execute_program_with_native_program_and_return(&program, initial_witness, foreign_call_executor) + execute_program_with_native_program_and_return(&program, initial_witness, foreign_call_executor, pedantic_solving) .await } @@ -117,8 +155,9 @@ 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/acvm-repo/blackbox_solver/src/bigint.rs b/acvm-repo/blackbox_solver/src/bigint.rs index 97cad74fc13..7686b7d257f 100644 --- a/acvm-repo/blackbox_solver/src/bigint.rs +++ b/acvm-repo/blackbox_solver/src/bigint.rs @@ -15,7 +15,7 @@ pub struct BigIntSolver { bigint_id_to_value: HashMap, bigint_id_to_modulus: HashMap, - // Use pedantic ACVM solving + // Use pedantic ACVM solving pedantic_solving: bool, } @@ -65,7 +65,9 @@ impl BigIntSolver { modulus: &[u8], output: u32, ) -> Result<(), BlackBoxResolutionError> { - if self.pedantic_solving && (!self.is_valid_modulus(modulus) || inputs.len() > modulus.len()) { + if self.pedantic_solving + && (!self.is_valid_modulus(modulus) || inputs.len() > modulus.len()) + { if !self.is_valid_modulus(modulus) { panic!("--pedantic-solving: bigint_from_bytes: disallowed modulus {:?}", modulus); } else { @@ -174,10 +176,7 @@ pub struct BigIntSolverWithId { 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(), - } + BigIntSolverWithId { solver, last_id: Default::default() } } pub fn create_bigint_id(&mut self) -> u32 { diff --git a/acvm-repo/blackbox_solver/src/curve_specific_solver.rs b/acvm-repo/blackbox_solver/src/curve_specific_solver.rs index 91c530327dc..d98ca628686 100644 --- a/acvm-repo/blackbox_solver/src/curve_specific_solver.rs +++ b/acvm-repo/blackbox_solver/src/curve_specific_solver.rs @@ -32,6 +32,14 @@ pub trait BlackBoxFunctionSolver { pub struct StubbedBlackBoxSolver(pub /* pedantic_solving: */ 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 { BlackBoxResolutionError::Failed( diff --git a/acvm-repo/bn254_blackbox_solver/src/lib.rs b/acvm-repo/bn254_blackbox_solver/src/lib.rs index c05eba5bea6..5e5b4e9842e 100644 --- a/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -20,7 +20,7 @@ pub use poseidon2::{ type FieldElement = acir::acir_field::GenericFieldElement; #[derive(Default)] -pub struct Bn254BlackBoxSolver(pub /* pedantic_solving: */ bool); +pub struct Bn254BlackBoxSolver(pub bool); impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { fn pedantic_solving(&self) -> bool { diff --git a/acvm-repo/brillig_vm/src/black_box.rs b/acvm-repo/brillig_vm/src/black_box.rs index 461c5bf8742..79aea2adf76 100644 --- a/acvm-repo/brillig_vm/src/black_box.rs +++ b/acvm-repo/brillig_vm/src/black_box.rs @@ -167,8 +167,7 @@ pub(crate) fn evaluate_black_box scalars_hi.push(*scalar); } } - let (x, y, is_infinite) = - solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; + let (x, y, is_infinite) = solver.multi_scalar_mul(&points, &scalars_lo, &scalars_hi)?; memory.write_slice( memory.read_ref(result.pointer), &[ @@ -216,8 +215,7 @@ pub(crate) fn evaluate_black_box let lhs = memory.read(*lhs).try_into().unwrap(); let rhs = memory.read(*rhs).try_into().unwrap(); - let new_id = - bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntAdd)?; + let new_id = bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntAdd)?; memory.write(*output, new_id.into()); Ok(()) } @@ -225,8 +223,7 @@ pub(crate) fn evaluate_black_box let lhs = memory.read(*lhs).try_into().unwrap(); let rhs = memory.read(*rhs).try_into().unwrap(); - let new_id = - bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntSub)?; + let new_id = bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntSub)?; memory.write(*output, new_id.into()); Ok(()) } @@ -234,8 +231,7 @@ pub(crate) fn evaluate_black_box let lhs = memory.read(*lhs).try_into().unwrap(); let rhs = memory.read(*rhs).try_into().unwrap(); - let new_id = - bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntMul)?; + let new_id = bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntMul)?; memory.write(*output, new_id.into()); Ok(()) } @@ -243,8 +239,7 @@ pub(crate) fn evaluate_black_box let lhs = memory.read(*lhs).try_into().unwrap(); let rhs = memory.read(*rhs).try_into().unwrap(); - let new_id = - bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntDiv)?; + let new_id = bigint_solver.bigint_op(lhs, rhs, BlackBoxFunc::BigIntDiv)?; memory.write(*output, new_id.into()); Ok(()) } diff --git a/acvm-repo/brillig_vm/src/lib.rs b/acvm-repo/brillig_vm/src/lib.rs index 8e5d425e91c..d7fa921b5fc 100644 --- a/acvm-repo/brillig_vm/src/lib.rs +++ b/acvm-repo/brillig_vm/src/lib.rs @@ -111,7 +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()); + let bigint_solver = + BrilligBigIntSolver::with_pedantic_solving(black_box_solver.pedantic_solving()); Self { calldata, program_counter: 0, @@ -855,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 }); @@ -905,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); @@ -973,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); @@ -1045,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); @@ -1105,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); @@ -1151,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); @@ -1216,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); @@ -1312,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(); @@ -1412,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() } @@ -1440,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); @@ -1554,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() } @@ -1645,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() } @@ -1660,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 @@ -1706,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!( @@ -1799,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!( @@ -1909,7 +1925,8 @@ 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!( @@ -2007,7 +2024,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!( @@ -2129,7 +2147,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!( @@ -2269,9 +2288,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 @@ -2343,7 +2364,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/compiler/noirc_evaluator/src/acir/acir_variable.rs b/compiler/noirc_evaluator/src/acir/acir_variable.rs index 20070a9fa5a..9f2c649ee3e 100644 --- a/compiler/noirc_evaluator/src/acir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/acir/acir_variable.rs @@ -1893,9 +1893,7 @@ impl> AcirContext { inputs: &[BrilligInputs], outputs_types: &[AcirType], ) -> Option> { - let mut memory = - (execute_brillig(code, &self.blackbox_solver, inputs)?) - .into_iter(); + let mut memory = (execute_brillig(code, &self.blackbox_solver, inputs)?).into_iter(); let outputs_var = vecmap(outputs_types.iter(), |output| match output { AcirType::NumericType(_) => { @@ -2217,8 +2215,7 @@ fn execute_brillig>( // Instantiate a Brillig VM given the solved input registers and memory, along with the Brillig bytecode. let profiling_active = false; - let mut vm = - VM::new(calldata, code, Vec::new(), blackbox_solver, profiling_active); + let mut vm = VM::new(calldata, code, Vec::new(), blackbox_solver, profiling_active); // Run the Brillig VM on these inputs, bytecode, etc! let vm_status = vm.process_opcodes(); diff --git a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 8d5f14cee94..28acdde366e 100644 --- a/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -253,6 +253,10 @@ pub(crate) mod tests { pub(crate) struct DummyBlackBoxSolver; impl BlackBoxFunctionSolver for DummyBlackBoxSolver { + fn pedantic_solving(&self) -> bool { + true + } + fn multi_scalar_mul( &self, _points: &[FieldElement], diff --git a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs index ff686a094f5..825a3b7295d 100644 --- a/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs +++ b/compiler/noirc_evaluator/src/ssa/opt/constant_folding.rs @@ -634,13 +634,8 @@ impl<'brillig> Context<'brillig> { 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, - ); + let mut vm = + VM::new(calldata, bytecode, foreign_call_results, &black_box_solver, profiling_active); let vm_status: VMStatus<_> = vm.process_opcodes(); let VMStatus::Finished { return_data_offset, return_data_size } = vm_status else { return EvaluationResult::CannotEvaluate(*func_id); diff --git a/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs b/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs index 81f930a5081..61ec3e7d6df 100644 --- a/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs +++ b/compiler/noirc_frontend/src/hir/comptime/interpreter/foreign.rs @@ -59,26 +59,14 @@ fn call_foreign( match name { "aes128_encrypt" => aes128_encrypt(interner, args, location), - "bigint_from_le_bytes" => bigint_from_le_bytes( - interner, - bigint_solver, - args, - return_type, - location, - ), - "bigint_to_le_bytes" => bigint_to_le_bytes(bigint_solver, args, location), - "bigint_add" => { - bigint_op(bigint_solver, BigIntAdd, args, return_type, location) - } - "bigint_sub" => { - bigint_op(bigint_solver, BigIntSub, args, return_type, location) - } - "bigint_mul" => { - bigint_op(bigint_solver, BigIntMul, args, return_type, location) - } - "bigint_div" => { - bigint_op(bigint_solver, BigIntDiv, args, return_type, location) + "bigint_from_le_bytes" => { + bigint_from_le_bytes(interner, bigint_solver, args, return_type, location) } + "bigint_to_le_bytes" => bigint_to_le_bytes(bigint_solver, args, location), + "bigint_add" => bigint_op(bigint_solver, BigIntAdd, args, return_type, location), + "bigint_sub" => bigint_op(bigint_solver, BigIntSub, args, return_type, location), + "bigint_mul" => bigint_op(bigint_solver, BigIntMul, args, return_type, location), + "bigint_div" => bigint_op(bigint_solver, BigIntDiv, args, return_type, location), "blake2s" => blake_hash(interner, args, location, acvm::blackbox_solver::blake2s), "blake3" => blake_hash(interner, args, location, acvm::blackbox_solver::blake3), "ecdsa_secp256k1" => ecdsa_secp256_verify( @@ -95,7 +83,9 @@ fn call_foreign( ), "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), + "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), @@ -284,7 +274,11 @@ fn ecdsa_secp256_verify( /// point2: EmbeddedCurvePoint, /// ) -> [Field; 3] /// ``` -fn embedded_curve_add(arguments: Vec<(Value, Location)>, location: Location, pedantic_solving: bool) -> 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)?; diff --git a/tooling/debugger/src/context.rs b/tooling/debugger/src/context.rs index 0d1164cf9bd..326e76b74d3 100644 --- a/tooling/debugger/src/context.rs +++ b/tooling/debugger/src/context.rs @@ -970,6 +970,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); @@ -1028,7 +1029,7 @@ mod tests { let foreign_call_executor = Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)); let mut context = DebugContext::new( - &StubbedBlackBoxSolver, + &solver, circuits, debug_artifact, initial_witness, @@ -1113,6 +1114,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); @@ -1194,7 +1196,7 @@ mod tests { Box::new(DefaultDebugForeignCallExecutor::from_artifact(true, debug_artifact)); let brillig_funcs = &vec![brillig_bytecode]; let mut context = DebugContext::new( - &StubbedBlackBoxSolver, + &solver, circuits, debug_artifact, initial_witness, @@ -1235,6 +1237,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 { @@ -1281,7 +1284,7 @@ mod tests { let brillig_funcs = &vec![brillig_one, brillig_two]; let context = DebugContext::new( - &StubbedBlackBoxSolver, + &solver, &circuits, &debug_artifact, WitnessMap::new(), diff --git a/tooling/nargo_cli/tests/stdlib-props.rs b/tooling/nargo_cli/tests/stdlib-props.rs index 20c8d3565e4..8e104abbdd2 100644 --- a/tooling/nargo_cli/tests/stdlib-props.rs +++ b/tooling/nargo_cli/tests/stdlib-props.rs @@ -78,7 +78,7 @@ fn run_snippet_proptest( Err(e) => panic!("failed to compile program; brillig = {force_brillig}:\n{source}\n{e:?}"), }; - let pedandic_solving = true; + let pedandic_solving = true; let blackbox_solver = bn254_blackbox_solver::Bn254BlackBoxSolver(pedandic_solving); let foreign_call_executor = RefCell::new(DefaultForeignCallExecutor::new(false, None, None, None)); From 4b0df947a3e74b633412743ff286954305f39938 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Mon, 9 Dec 2024 21:54:49 -0500 Subject: [PATCH 06/22] wip debugging: use StubbedBlackBoxSolver::default to enable pedantic_solving, check grumpkin_integer and whether points are infinite for ec add and multi_scalar_mul --- acvm-repo/acvm/tests/solver.rs | 17 +-- .../src/embedded_curve_ops.rs | 104 +++++++++++++----- acvm-repo/bn254_blackbox_solver/src/lib.rs | 3 +- .../comptime_blackbox/src/main.nr | 5 +- 4 files changed, 93 insertions(+), 36 deletions(-) diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index c700654c1f7..6e980641648 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -687,6 +687,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); @@ -775,7 +776,6 @@ fn unsatisfied_opcode_resolved_brillig() { values.insert(w_y, FieldElement::from(1_i128)); values.insert(w_result, FieldElement::from(0_i128)); - let pedantic_solving = true; let opcodes = vec![ Opcode::BrilligCall { id: BrilligFunctionId(0), @@ -818,6 +818,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)), @@ -850,11 +852,10 @@ fn memory_operations() { q_c: FieldElement::one(), }); - let pedantic_solving = true; let opcodes = vec![init, read_op, expression]; let unconstrained_functions = vec![]; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver(pedantic_solving), + &solver, &opcodes, initial_witness, &unconstrained_functions, @@ -921,6 +922,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) @@ -933,7 +935,7 @@ where let opcodes = vec![op]; let unconstrained_functions = vec![]; let mut acvm = ACVM::new( - &Bn254BlackBoxSolver(pedantic_solving), + &solver, &opcodes, initial_witness, &unconstrained_functions, @@ -1033,6 +1035,7 @@ fn bigint_solve_binary_op_opt( rhs: Vec, pedantic_solving: bool, ) -> Result, OpcodeResolutionError> { + let solver = StubbedBlackBoxSolver(pedantic_solving); let initial_witness_vec: Vec<_> = lhs .iter() .chain(rhs.iter()) @@ -1067,7 +1070,6 @@ fn bigint_solve_binary_op_opt( outputs: output_witnesses.clone(), }); - let pedantic_solving = true; let mut opcodes = vec![bigint_from_lhs_op, bigint_from_rhs_op]; if let Some(middle_op) = middle_op { opcodes.push(Opcode::BlackBoxFuncCall(middle_op)); @@ -1076,7 +1078,7 @@ fn bigint_solve_binary_op_opt( let unconstrained_functions = vec![]; let mut acvm = ACVM::new( - &StubbedBlackBoxSolver(pedantic_solving), + &solver, &opcodes, initial_witness, &unconstrained_functions, @@ -1121,7 +1123,6 @@ fn solve_blackbox_func_call( rhs_opt = Some(rhs); } - let pedantic_solving = true; let op = Opcode::BlackBoxFuncCall(blackbox_func_call(lhs_opt, rhs_opt)?); let opcodes = vec![op]; let unconstrained_functions = vec![]; @@ -1970,7 +1971,7 @@ proptest! { } #[test] - #[should_panic(expected = "--pedantic-solving: BigIntDiv by zero")] + #[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); diff --git a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index e599fd25593..db1fbe22029 100644 --- a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/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( @@ -48,12 +50,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 >= 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 >= 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 = grumpkin::SWAffine::from(point.mul_bigint(grumpkin_integer.to_u64_digits())); @@ -75,11 +77,24 @@ pub fn multi_scalar_mul( pub fn embedded_curve_add( input1: [FieldElement; 3], input2: [FieldElement; 3], + pedantic_solving: bool, ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { 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.to_flags().is_infinity() { + return Err(BlackBoxResolutionError::Failed( + BlackBoxFunc::EmbeddedCurveAdd, + format!("Infinite point input: {}", point), + )); + } + } + } + let res = grumpkin::SWAffine::from(point1 + point2); if let Some((res_x, res_y)) = res.xy() { Ok(( @@ -103,7 +118,11 @@ fn create_point( is_infinite: bool, ) -> Result { if is_infinite { + // TODO: only error when pedantic? + return Ok(grumpkin::SWAffine::zero()); + // panic!("infinite input to EC operation") + // return Err(format!("Attempt to use infinite point in EC operation ({}, {})", x.to_hex(), y.to_hex())); } let point = grumpkin::SWAffine::new_unchecked(x.into_repr(), y.into_repr()); if !point.is_on_curve() { @@ -117,6 +136,7 @@ fn create_point( #[cfg(test)] mod tests { + use ark_ff::BigInteger; use super::*; fn get_generator() -> [FieldElement; 3] { @@ -131,7 +151,8 @@ 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 +165,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 +187,32 @@ 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 = grumpkin::FrConfig::MODULUS.to_bytes_be(); + #[test] + fn rejects_grumpkin_modulus_when_pedantic() { + let x = 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 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 +221,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 +244,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 +261,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 +277,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 point input: 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/acvm-repo/bn254_blackbox_solver/src/lib.rs b/acvm-repo/bn254_blackbox_solver/src/lib.rs index 5e5b4e9842e..a93e7028569 100644 --- a/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -33,7 +33,7 @@ impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { 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( @@ -48,6 +48,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/test_programs/noir_test_success/comptime_blackbox/src/main.nr b/test_programs/noir_test_success/comptime_blackbox/src/main.nr index c3784e73b09..bc5b31842c4 100644 --- a/test_programs/noir_test_success/comptime_blackbox/src/main.nr +++ b/test_programs/noir_test_success/comptime_blackbox/src/main.nr @@ -124,7 +124,10 @@ fn test_embedded_curve_ops() { 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]); + + // TODO + // let mul = multi_scalar_mul([g1, g1], [s1, s1]); + let mul = sum; (sum, mul) }; assert_eq(sum, mul); From 5fe6eaada9c7980d99b62f4d0acc1187ff96b1a5 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Wed, 11 Dec 2024 11:41:51 -0500 Subject: [PATCH 07/22] wip debugging, use 'point == ..Affine::zero()' to check for infinity, check that bool's are valid --- .../src/embedded_curve_ops.rs | 15 +++++++++------ .../comptime_blackbox/src/main.nr | 5 +---- tooling/nargo_cli/build.rs | 2 +- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index db1fbe22029..dfa08f27b4c 100644 --- a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs @@ -26,6 +26,9 @@ pub fn multi_scalar_mul( let mut output_point = grumpkin::SWAffine::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))?; @@ -79,6 +82,10 @@ pub fn embedded_curve_add( 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()) @@ -86,10 +93,10 @@ pub fn embedded_curve_add( if pedantic_solving { for point in [point1, point2] { - if point.to_flags().is_infinity() { + if point == grumpkin::SWAffine::zero() { return Err(BlackBoxResolutionError::Failed( BlackBoxFunc::EmbeddedCurveAdd, - format!("Infinite point input: {}", point), + format!("Infinite input: embedded_curve_add({}, {})", point1, point2), )); } } @@ -118,11 +125,7 @@ fn create_point( is_infinite: bool, ) -> Result { if is_infinite { - // TODO: only error when pedantic? - return Ok(grumpkin::SWAffine::zero()); - // panic!("infinite input to EC operation") - // return Err(format!("Attempt to use infinite point in EC operation ({}, {})", x.to_hex(), y.to_hex())); } let point = grumpkin::SWAffine::new_unchecked(x.into_repr(), y.into_repr()); if !point.is_on_curve() { diff --git a/test_programs/noir_test_success/comptime_blackbox/src/main.nr b/test_programs/noir_test_success/comptime_blackbox/src/main.nr index bc5b31842c4..c3784e73b09 100644 --- a/test_programs/noir_test_success/comptime_blackbox/src/main.nr +++ b/test_programs/noir_test_success/comptime_blackbox/src/main.nr @@ -124,10 +124,7 @@ fn test_embedded_curve_ops() { let g1 = EmbeddedCurvePoint { x: 1, y: 17631683881184975370165255887551781615748388533673675138860, is_infinite: false }; let s1 = EmbeddedCurveScalar { lo: 1, hi: 0 }; let sum = g1 + g1; - - // TODO - // let mul = multi_scalar_mul([g1, g1], [s1, s1]); - let mul = sum; + let mul = multi_scalar_mul([g1, g1], [s1, s1]); (sum, mul) }; assert_eq(sum, mul); diff --git a/tooling/nargo_cli/build.rs b/tooling/nargo_cli/build.rs index 41b3c0c9cf7..c3b213fa53d 100644 --- a/tooling/nargo_cli/build.rs +++ b/tooling/nargo_cli/build.rs @@ -381,7 +381,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( From a895f6429aeafa609274b3b7c8d3278a7dc2e4d0 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Wed, 11 Dec 2024 12:45:23 -0500 Subject: [PATCH 08/22] fix typo, update smoke test to add two different points, cargo clippy/fmt --- acvm-repo/acvm/tests/solver.rs | 88 +++---------------- acvm-repo/acvm_js/src/execute.rs | 39 +++++--- .../src/curve_specific_solver.rs | 3 +- .../src/embedded_curve_ops.rs | 15 +++- acvm-repo/bn254_blackbox_solver/src/lib.rs | 1 + acvm-repo/brillig_vm/src/lib.rs | 3 +- .../comptime_blackbox/src/main.nr | 8 +- 7 files changed, 61 insertions(+), 96 deletions(-) diff --git a/acvm-repo/acvm/tests/solver.rs b/acvm-repo/acvm/tests/solver.rs index 6e980641648..0edfd14bf73 100644 --- a/acvm-repo/acvm/tests/solver.rs +++ b/acvm-repo/acvm/tests/solver.rs @@ -48,13 +48,7 @@ fn bls12_381_circuit() { ]) .into(); - let mut acvm = ACVM::new( - &solver, - &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"); @@ -177,13 +171,7 @@ fn inversion_brillig_oracle_equivalence() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = ACVM::new( - &solver, - &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(); @@ -344,13 +332,7 @@ fn double_inversion_brillig_oracle() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = ACVM::new( - &solver, - &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(); @@ -502,13 +484,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( - &solver, - &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(); @@ -625,13 +601,7 @@ fn brillig_oracle_predicate() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = ACVM::new( - &solver, - &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"); @@ -667,13 +637,7 @@ fn unsatisfied_opcode_resolved() { let opcodes = vec![Opcode::AssertZero(opcode_a)]; let unconstrained_functions = vec![]; - let mut acvm = ACVM::new( - &solver, - &opcodes, - values, - &unconstrained_functions, - &[], - ); + let mut acvm = ACVM::new(&solver, &opcodes, values, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!( solver_status, @@ -797,13 +761,7 @@ fn unsatisfied_opcode_resolved_brillig() { Opcode::AssertZero(opcode_a), ]; let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = ACVM::new( - &solver, - &opcodes, - values, - &unconstrained_functions, - &[], - ); + let mut acvm = ACVM::new(&solver, &opcodes, values, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!( solver_status, @@ -854,13 +812,7 @@ fn memory_operations() { let opcodes = vec![init, read_op, expression]; let unconstrained_functions = vec![]; - let mut acvm = ACVM::new( - &solver, - &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(); @@ -934,13 +886,7 @@ where let op = Opcode::BlackBoxFuncCall(f((inputs.clone(), outputs.clone()))?); let opcodes = vec![op]; let unconstrained_functions = vec![]; - let mut acvm = ACVM::new( - &solver, - &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(); @@ -1077,13 +1023,7 @@ fn bigint_solve_binary_op_opt( opcodes.push(bigint_to_op); let unconstrained_functions = vec![]; - let mut acvm = ACVM::new( - &solver, - &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(); @@ -1126,13 +1066,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( - &solver, - &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(); diff --git a/acvm-repo/acvm_js/src/execute.rs b/acvm-repo/acvm_js/src/execute.rs index 5bcf5a500a3..643935b9a79 100644 --- a/acvm-repo/acvm_js/src/execute.rs +++ b/acvm-repo/acvm_js/src/execute.rs @@ -32,7 +32,7 @@ pub async fn execute_circuit( foreign_call_handler: ForeignCallHandler, ) -> Result { let pedantic_solving = false; - execute_program_pedantic(program, initial_witness, foreign_call_handler, pedantic_solving).await + execute_circuit_pedantic(program, initial_witness, foreign_call_handler, pedantic_solving).await } /// `execute_circuit` with pedantic ACVM solving @@ -45,9 +45,13 @@ pub async fn execute_circuit_pedantic( ) -> Result { console_error_panic_hook::set_once(); - let mut witness_stack = - execute_program_with_native_type_return(program, initial_witness, &foreign_call_handler, pedantic_solving) - .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()) @@ -67,7 +71,13 @@ pub async fn execute_circuit_with_return_witness( foreign_call_handler: ForeignCallHandler, ) -> Result { let pedantic_solving = false; - execute_circuit_with_return_witness_pedantic(program, initial_witness, foreign_call_handler, pedantic_solving).await + execute_circuit_with_return_witness_pedantic( + program, + initial_witness, + foreign_call_handler, + pedantic_solving, + ) + .await } /// `executeCircuitWithReturnWitness` with pedantic ACVM execution @@ -127,9 +137,13 @@ pub async fn execute_program_pedantic( ) -> Result { console_error_panic_hook::set_once(); - let witness_stack = - execute_program_with_native_type_return(program, initial_witness, &foreign_call_handler, pedantic_solving) - .await?; + let witness_stack = execute_program_with_native_type_return( + program, + initial_witness, + &foreign_call_handler, + pedantic_solving, + ) + .await?; Ok(witness_stack.into()) } @@ -147,8 +161,13 @@ async fn execute_program_with_native_type_return( None, None))?; - execute_program_with_native_program_and_return(&program, initial_witness, foreign_call_executor, pedantic_solving) - .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( diff --git a/acvm-repo/blackbox_solver/src/curve_specific_solver.rs b/acvm-repo/blackbox_solver/src/curve_specific_solver.rs index d98ca628686..37fe5d05363 100644 --- a/acvm-repo/blackbox_solver/src/curve_specific_solver.rs +++ b/acvm-repo/blackbox_solver/src/curve_specific_solver.rs @@ -30,7 +30,8 @@ pub trait BlackBoxFunctionSolver { ) -> Result, BlackBoxResolutionError>; } -pub struct StubbedBlackBoxSolver(pub /* pedantic_solving: */ bool); +// pedantic_solving: bool +pub struct StubbedBlackBoxSolver(pub bool); // pedantic_solving enabled by default impl Default for StubbedBlackBoxSolver { diff --git a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index dfa08f27b4c..41950027734 100644 --- a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs @@ -139,8 +139,8 @@ fn create_point( #[cfg(test)] mod tests { - use ark_ff::BigInteger; use super::*; + use ark_ff::BigInteger; fn get_generator() -> [FieldElement; 3] { let generator = grumpkin::SWAffine::generator(); @@ -155,7 +155,12 @@ mod tests { let generator = get_generator(); let pedantic_solving = true; - let res = multi_scalar_mul(&generator, &[FieldElement::one()], &[FieldElement::zero()], pedantic_solving)?; + let res = multi_scalar_mul( + &generator, + &[FieldElement::one()], + &[FieldElement::zero()], + pedantic_solving, + )?; assert_eq!(generator[0], res.0); assert_eq!(generator[1], res.1); @@ -191,10 +196,12 @@ mod tests { )); let pedantic_solving = true; - let res = multi_scalar_mul(&points, &[FieldElement::one()], &[invalid_limb], pedantic_solving); + 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()], pedantic_solving); + let res = + multi_scalar_mul(&points, &[invalid_limb], &[FieldElement::one()], pedantic_solving); assert_eq!(res, expected_error); } diff --git a/acvm-repo/bn254_blackbox_solver/src/lib.rs b/acvm-repo/bn254_blackbox_solver/src/lib.rs index a93e7028569..fc031faefa2 100644 --- a/acvm-repo/bn254_blackbox_solver/src/lib.rs +++ b/acvm-repo/bn254_blackbox_solver/src/lib.rs @@ -20,6 +20,7 @@ pub use poseidon2::{ type FieldElement = acir::acir_field::GenericFieldElement; #[derive(Default)] +// pedantic_solving: bool pub struct Bn254BlackBoxSolver(pub bool); impl BlackBoxFunctionSolver for Bn254BlackBoxSolver { diff --git a/acvm-repo/brillig_vm/src/lib.rs b/acvm-repo/brillig_vm/src/lib.rs index d7fa921b5fc..8602f74e0d6 100644 --- a/acvm-repo/brillig_vm/src/lib.rs +++ b/acvm-repo/brillig_vm/src/lib.rs @@ -1926,7 +1926,8 @@ mod tests { ]; let solver = StubbedBlackBoxSolver::default(); - let mut vm = brillig_execute_and_get_vm(input_string.clone(), &string_double_program, &solver); + let mut vm = + brillig_execute_and_get_vm(input_string.clone(), &string_double_program, &solver); // Check that VM is waiting assert_eq!( diff --git a/test_programs/noir_test_success/comptime_blackbox/src/main.nr b/test_programs/noir_test_success/comptime_blackbox/src/main.nr index c3784e73b09..859790f7d2d 100644 --- a/test_programs/noir_test_success/comptime_blackbox/src/main.nr +++ b/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); From 3b42dd9a82b8895174b744ca9009cf03447e3e8e Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Wed, 11 Dec 2024 13:42:20 -0500 Subject: [PATCH 09/22] Update acvm-repo/blackbox_solver/src/bigint.rs Co-authored-by: Akosh Farkash --- acvm-repo/blackbox_solver/src/bigint.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/acvm-repo/blackbox_solver/src/bigint.rs b/acvm-repo/blackbox_solver/src/bigint.rs index 7686b7d257f..dcf76ed32c9 100644 --- a/acvm-repo/blackbox_solver/src/bigint.rs +++ b/acvm-repo/blackbox_solver/src/bigint.rs @@ -65,12 +65,11 @@ impl BigIntSolver { modulus: &[u8], output: u32, ) -> Result<(), BlackBoxResolutionError> { - if self.pedantic_solving - && (!self.is_valid_modulus(modulus) || inputs.len() > modulus.len()) - { + if self.pedantic_solving { if !self.is_valid_modulus(modulus) { panic!("--pedantic-solving: bigint_from_bytes: disallowed modulus {:?}", modulus); - } else { + } + if inputs.len() > modulus.len() { panic!( "--pedantic-solving: bigint_from_bytes: inputs.len() > modulus.len() {:?}", (inputs.len(), modulus.len()) From f7ee3b8e6726fce94abfb0c55327bfea3f51271e Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Wed, 11 Dec 2024 13:42:46 -0500 Subject: [PATCH 10/22] Update acvm-repo/acvm/src/pwg/mod.rs Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- acvm-repo/acvm/src/pwg/mod.rs | 22 ++++++++-------------- 1 file changed, 8 insertions(+), 14 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index b3746fc1c5e..1ee8ce15068 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -765,25 +765,19 @@ pub(crate) fn is_predicate_false( ) -> Result> { match predicate { Some(pred) => { - let pred_value = get_value(pred, witness); + let pred_value = get_value(pred, witness)?; + let predicate_is_false = pred_value.is_zero(); if pedantic_solving { - pred_value.and_then(|predicate_value| { - if predicate_value.is_zero() { - Ok(true) - } else if predicate_value.is_one() { - Ok(false) - } else { - let opcode_location = *opcode_location; - Err(OpcodeResolutionError::PredicateLargerThanOne { + // We expect that the predicate should resolve to either 0 or 1. + if !predicate_is_false && !predicate.is_one() { + let opcode_location = *opcode_location; + return Err(OpcodeResolutionError::PredicateLargerThanOne { opcode_location, predicate_value, }) - } - }) - } else { - pred_value.map(|pred_value| pred_value.is_zero()) + } } - } + Ok(predicate_is_false) // If the predicate is `None`, then we treat it as an unconditional `true` None => Ok(false), } From 8796f15e27b13443001a4a161129bd3ef43f2de1 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Wed, 11 Dec 2024 13:59:18 -0500 Subject: [PATCH 11/22] fix small refactor and use single constant for pedantic_solving in memory_op tests --- acvm-repo/acvm/src/pwg/memory_op.rs | 15 +++++++-------- acvm-repo/acvm/src/pwg/mod.rs | 19 ++++++++++--------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/acvm-repo/acvm/src/pwg/memory_op.rs b/acvm-repo/acvm/src/pwg/memory_op.rs index 9409ee36249..1a6519d19c5 100644 --- a/acvm-repo/acvm/src/pwg/memory_op.rs +++ b/acvm-repo/acvm/src/pwg/memory_op.rs @@ -134,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([ @@ -153,9 +156,8 @@ mod tests { block_solver.init(&init, &initial_witness).unwrap(); for op in trace { - let pedantic_solving = true; block_solver - .solve_memory_op(&op, &mut initial_witness, &None, pedantic_solving) + .solve_memory_op(&op, &mut initial_witness, &None, PEDANTIC_SOLVING) .unwrap(); } @@ -181,9 +183,8 @@ mod tests { let mut err = None; for op in invalid_trace { if err.is_none() { - let pedantic_solving = true; err = block_solver - .solve_memory_op(&op, &mut initial_witness, &None, pedantic_solving) + .solve_memory_op(&op, &mut initial_witness, &None, PEDANTIC_SOLVING) .err(); } } @@ -217,13 +218,12 @@ mod tests { let mut err = None; for op in invalid_trace { if err.is_none() { - let pedantic_solving = true; err = block_solver .solve_memory_op( &op, &mut initial_witness, &Some(Expression::zero()), - pedantic_solving, + PEDANTIC_SOLVING, ) .err(); } @@ -255,13 +255,12 @@ mod tests { let mut err = None; for op in invalid_trace { if err.is_none() { - let pedantic_solving = true; err = block_solver .solve_memory_op( &op, &mut initial_witness, &Some(Expression::zero()), - pedantic_solving, + PEDANTIC_SOLVING, ) .err(); } diff --git a/acvm-repo/acvm/src/pwg/mod.rs b/acvm-repo/acvm/src/pwg/mod.rs index 1ee8ce15068..104a15c17cc 100644 --- a/acvm-repo/acvm/src/pwg/mod.rs +++ b/acvm-repo/acvm/src/pwg/mod.rs @@ -144,8 +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: {predicate_value}")] - PredicateLargerThanOne { opcode_location: ErrorLocation, predicate_value: F }, + #[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 { @@ -768,16 +768,17 @@ pub(crate) fn is_predicate_false( 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 && !predicate.is_one() { - let opcode_location = *opcode_location; - return Err(OpcodeResolutionError::PredicateLargerThanOne { - opcode_location, - predicate_value, - }) + // 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), } From 3e89c31faa727ca25d709b56948c9063b20ea9d7 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Wed, 11 Dec 2024 15:14:12 -0500 Subject: [PATCH 12/22] updating lsp usage of solvers, cargo clippy/fmt --- acvm-repo/acvm_js/src/execute.rs | 9 +++------ tooling/lsp/src/lib.rs | 2 +- tooling/lsp/src/requests/mod.rs | 2 +- tooling/lsp/src/test_utils.rs | 2 +- tooling/nargo_cli/benches/criterion.rs | 3 ++- 5 files changed, 8 insertions(+), 10 deletions(-) diff --git a/acvm-repo/acvm_js/src/execute.rs b/acvm-repo/acvm_js/src/execute.rs index 643935b9a79..e4d52063977 100644 --- a/acvm-repo/acvm_js/src/execute.rs +++ b/acvm-repo/acvm_js/src/execute.rs @@ -36,8 +36,7 @@ pub async fn execute_circuit( } /// `execute_circuit` with pedantic ACVM solving -#[wasm_bindgen(js_name = executeCircuitPedantic, skip_jsdoc)] -pub async fn execute_circuit_pedantic( +async fn execute_circuit_pedantic( program: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, @@ -81,8 +80,7 @@ pub async fn execute_circuit_with_return_witness( } /// `executeCircuitWithReturnWitness` with pedantic ACVM execution -#[wasm_bindgen(js_name = executeCircuitWithReturnWitnessPedantic, skip_jsdoc)] -pub async fn execute_circuit_with_return_witness_pedantic( +async fn execute_circuit_with_return_witness_pedantic( program: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, @@ -128,8 +126,7 @@ pub async fn execute_program( } /// `execute_program` with pedantic ACVM solving -#[wasm_bindgen(js_name = executeProgramPedantic, skip_jsdoc)] -pub async fn execute_program_pedantic( +async fn execute_program_pedantic( program: Vec, initial_witness: JsWitnessMap, foreign_call_handler: ForeignCallHandler, diff --git a/tooling/lsp/src/lib.rs b/tooling/lsp/src/lib.rs index a152cc17c7b..97a1c7ad27b 100644 --- a/tooling/lsp/src/lib.rs +++ b/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/tooling/lsp/src/requests/mod.rs b/tooling/lsp/src/requests/mod.rs index 22bdda8d7d7..6442c1ba637 100644 --- a/tooling/lsp/src/requests/mod.rs +++ b/tooling/lsp/src/requests/mod.rs @@ -631,7 +631,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/tooling/lsp/src/test_utils.rs b/tooling/lsp/src/test_utils.rs index c0505107842..c2c19b0efc7 100644 --- a/tooling/lsp/src/test_utils.rs +++ b/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/tooling/nargo_cli/benches/criterion.rs b/tooling/nargo_cli/benches/criterion.rs index 51de97df139..a97239b34f0 100644 --- a/tooling/nargo_cli/benches/criterion.rs +++ b/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"); From 7b5d66e02962553674a3678aab886fc08b07db22 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Wed, 11 Dec 2024 15:38:46 -0500 Subject: [PATCH 13/22] update test error message --- acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index 41950027734..00dcdfc54d6 100644 --- a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs @@ -303,7 +303,7 @@ mod tests { res, Err(BlackBoxResolutionError::Failed( BlackBoxFunc::EmbeddedCurveAdd, - "Infinite point input: infinity".into(), + "Infinite input: embedded_curve_add(infinity, infinity)".into(), )) ); } From fd6085b62f2959444e15bb44e7508061dc0ed8c8 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Thu, 12 Dec 2024 16:54:24 -0500 Subject: [PATCH 14/22] cargo fmt --- tooling/acvm_cli/src/cli/execute_cmd.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/tooling/acvm_cli/src/cli/execute_cmd.rs b/tooling/acvm_cli/src/cli/execute_cmd.rs index 5b68697dd81..83429b3ad85 100644 --- a/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -83,7 +83,6 @@ pub(crate) fn execute_program_from_witness( inputs_map, &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallExecutor::new(PrintOutput::Stdout, None, None, None), - ) .map_err(CliError::CircuitExecutionError) } From a1f824ba72d16aabcc4dd5c330a37f6b0bff0d7f Mon Sep 17 00:00:00 2001 From: Michael J Klein Date: Mon, 16 Dec 2024 12:21:53 -0500 Subject: [PATCH 15/22] Update acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com> --- acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index 00dcdfc54d6..6fb87fbef60 100644 --- a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs @@ -82,7 +82,7 @@ pub fn embedded_curve_add( input2: [FieldElement; 3], pedantic_solving: bool, ) -> Result<(FieldElement, FieldElement, FieldElement), BlackBoxResolutionError> { - if pedantic_solving && input1[2] > FieldElement::one() && input2[2] > FieldElement::one() { + 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") } From 8ea09bad06d0e633f2d45a61d752fb992b8ef792 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Mon, 16 Dec 2024 15:04:53 -0500 Subject: [PATCH 16/22] use '--pedantic-solving' in CI and example programs --- .github/workflows/test-js-packages.yml | 2 +- examples/codegen_verifier/codegen_verifier.sh | 2 +- examples/prove_and_verify/prove_and_verify.sh | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/test-js-packages.yml b/.github/workflows/test-js-packages.yml index d227b4eb00b..2ea34282b1b 100644 --- a/.github/workflows/test-js-packages.yml +++ b/.github/workflows/test-js-packages.yml @@ -592,7 +592,7 @@ jobs: - name: Run nargo test working-directory: ./test-repo/${{ matrix.project.path }} - run: nargo test -q --silence-warnings + run: nargo test -q --silence-warnings --pedantic-solving env: NARGO_IGNORE_TEST_FAILURES_FROM_FOREIGN_CALLS: true diff --git a/examples/codegen_verifier/codegen_verifier.sh b/examples/codegen_verifier/codegen_verifier.sh index f224f74168f..9d1a6251dde 100755 --- a/examples/codegen_verifier/codegen_verifier.sh +++ b/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/examples/prove_and_verify/prove_and_verify.sh b/examples/prove_and_verify/prove_and_verify.sh index df3ec3ff97a..0f0aee9a8ae 100755 --- a/examples/prove_and_verify/prove_and_verify.sh +++ b/examples/prove_and_verify/prove_and_verify.sh @@ -3,7 +3,7 @@ set -eu BACKEND=${BACKEND:-bb} -nargo execute witness +cargo run 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 From 6146477f60eaa2afcf6eccd1d9a93292de96dd4d Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Tue, 17 Dec 2024 11:03:56 -0500 Subject: [PATCH 17/22] revert nargo->cargo testing change --- examples/prove_and_verify/prove_and_verify.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/prove_and_verify/prove_and_verify.sh b/examples/prove_and_verify/prove_and_verify.sh index 0f0aee9a8ae..a7c8eeaf317 100755 --- a/examples/prove_and_verify/prove_and_verify.sh +++ b/examples/prove_and_verify/prove_and_verify.sh @@ -3,7 +3,7 @@ set -eu BACKEND=${BACKEND:-bb} -cargo run execute --pedantic-solving witness +nargo run execute --pedantic-solving witness # TODO: `bb` should create `proofs` directory if it doesn't exist. mkdir -p proofs From 0556d1c68b64e522aace6a68c8f44c32d58c5725 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Tue, 17 Dec 2024 16:16:31 +0000 Subject: [PATCH 18/22] Update examples/prove_and_verify/prove_and_verify.sh --- examples/prove_and_verify/prove_and_verify.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/prove_and_verify/prove_and_verify.sh b/examples/prove_and_verify/prove_and_verify.sh index a7c8eeaf317..411f5258caf 100755 --- a/examples/prove_and_verify/prove_and_verify.sh +++ b/examples/prove_and_verify/prove_and_verify.sh @@ -3,7 +3,7 @@ set -eu BACKEND=${BACKEND:-bb} -nargo run execute --pedantic-solving witness +nargo execute --pedantic-solving witness # TODO: `bb` should create `proofs` directory if it doesn't exist. mkdir -p proofs From 905f4f23dca68d5a28973b7824ef468c92dff53d Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Fri, 3 Jan 2025 16:27:10 -0500 Subject: [PATCH 19/22] fixup after merge --- Cargo.lock | 6 ++++-- acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs | 2 +- tooling/acvm_cli/src/cli/execute_cmd.rs | 1 + 3 files changed, 6 insertions(+), 3 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 008af1915b0..d8ca4e0984b 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2063,6 +2063,8 @@ source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3a9bfc1af68b1726ea47d3d5109de126281def866b33970e10fbab11b5dafab3" dependencies = [ "allocator-api2", + "equivalent", + "foldhash", ] [[package]] @@ -3467,7 +3469,7 @@ checksum = "a5e44f723f1133c9deac646763579fdb3ac745e418f2a7af9cd0c431da1f20b9" dependencies = [ "num-integer", "num-traits", - "rand 0.8.5", + "rand", ] [[package]] @@ -3519,7 +3521,7 @@ dependencies = [ "num-integer", "num-modular", "num-traits", - "rand 0.8.5", + "rand", ] [[package]] diff --git a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index cd0416d96af..63928d3eb66 100644 --- a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs @@ -53,7 +53,7 @@ pub fn multi_scalar_mul( let grumpkin_integer = BigUint::from_bytes_be(&bytes); // Check if this is smaller than the grumpkin modulus - if pedantic_solving && grumpkin_integer >= grumpkin::FrConfig::MODULUS.into() { + 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)), diff --git a/tooling/acvm_cli/src/cli/execute_cmd.rs b/tooling/acvm_cli/src/cli/execute_cmd.rs index 08d0549ae87..9fed8cbd497 100644 --- a/tooling/acvm_cli/src/cli/execute_cmd.rs +++ b/tooling/acvm_cli/src/cli/execute_cmd.rs @@ -81,6 +81,7 @@ pub(crate) fn execute_program_from_witness( execute_program( &program, inputs_map, + &Bn254BlackBoxSolver(pedantic_solving), &mut DefaultForeignCallBuilder { output: PrintOutput::Stdout, ..Default::default() } .build(), ) From 56362d4c516882e26ebd8f039a802727979eb6cd Mon Sep 17 00:00:00 2001 From: "Michael J. Klein" Date: Mon, 6 Jan 2025 12:58:39 -0500 Subject: [PATCH 20/22] allow clippy::too_many_arguments when constructing Elaborator --- compiler/noirc_frontend/src/elaborator/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/compiler/noirc_frontend/src/elaborator/mod.rs b/compiler/noirc_frontend/src/elaborator/mod.rs index ba5ca3ed7a2..9b0b94c39eb 100644 --- a/compiler/noirc_frontend/src/elaborator/mod.rs +++ b/compiler/noirc_frontend/src/elaborator/mod.rs @@ -201,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, From 5adbaaedac6fcfb1ddd31c6a68e50f6626572cf7 Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Tue, 7 Jan 2025 14:06:42 -0500 Subject: [PATCH 21/22] cargo fmt --- compiler/noirc_driver/src/lib.rs | 8 ++++++-- compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs | 7 ++++++- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/compiler/noirc_driver/src/lib.rs b/compiler/noirc_driver/src/lib.rs index d457a22fd6b..87da9cd658e 100644 --- a/compiler/noirc_driver/src/lib.rs +++ b/compiler/noirc_driver/src/lib.rs @@ -317,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(), options.pedantic_solving); + 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/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs index 36a566d24cf..9081cb1cccd 100644 --- a/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs +++ b/compiler/noirc_frontend/src/hir/def_collector/dc_crate.rs @@ -288,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, pedantic_solving)); + 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"); From 75f912ec93badba0f6bc61db2a89c5d031c6cffd Mon Sep 17 00:00:00 2001 From: Michael Klein Date: Tue, 7 Jan 2025 14:49:52 -0500 Subject: [PATCH 22/22] fix after merge --- acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs index 63928d3eb66..ffe7fa50089 100644 --- a/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs +++ b/acvm-repo/bn254_blackbox_solver/src/embedded_curve_ops.rs @@ -207,7 +207,7 @@ mod tests { #[test] fn rejects_grumpkin_modulus_when_pedantic() { - let x = grumpkin::FrConfig::MODULUS.to_bytes_be(); + 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]);