From fae4eadfedbf42ae73610c3475158072a183b329 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Wed, 7 Feb 2024 19:17:52 +0000 Subject: [PATCH] fix: ensure that unconstrained entrypoint functions don't generate constraints (#4292) # Description ## Problem\* Resolves ## Summary\* This PR enforces that if we're generating an unconstrained function that we should only emit a single brillig opcode. ## Additional Context ## Documentation\* Check one: - [x] No documentation needed. - [ ] Documentation included in this PR. - [ ] **[Exceptional Case]** Documentation to be submitted in a separate PR. # PR Checklist\* - [x] I have tested the changes locally. - [x] I have formatted the changes with [Prettier](https://prettier.io/) and/or `cargo fmt` on default settings. --------- Co-authored-by: kevaundray --- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 12 +++++++++--- .../src/ssa/acir_gen/acir_ir/generated_acir.rs | 4 ++++ compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs | 14 ++++++++++++-- 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index d246374001d..aef6f0a2e96 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -319,6 +319,7 @@ impl AcirContext { vec![AcirValue::Var(var, AcirType::field())], vec![AcirType::field()], true, + false, )?; let inverted_var = Self::expect_one_var(results); @@ -706,6 +707,7 @@ impl AcirContext { ], vec![AcirType::unsigned(max_q_bits), AcirType::unsigned(max_rhs_bits)], true, + false, )? .try_into() .expect("quotient only returns two values"); @@ -1438,6 +1440,7 @@ impl AcirContext { inputs: Vec, outputs: Vec, attempt_execution: bool, + unsafe_return_values: bool, ) -> Result, RuntimeError> { let b_inputs = try_vecmap(inputs, |i| -> Result<_, InternalError> { match i { @@ -1512,10 +1515,13 @@ impl AcirContext { Ok(()) } - for output_var in &outputs_var { - range_constraint_value(self, output_var)?; + // This is a hack to ensure that if we're compiling a brillig entrypoint function then + // we don't also add a number of range constraints. + if !unsafe_return_values { + for output_var in &outputs_var { + range_constraint_value(self, output_var)?; + } } - Ok(outputs_var) } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 2330b19207e..e241c0d8ea8 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -74,6 +74,10 @@ impl GeneratedAcir { } } + pub(crate) fn opcodes(&self) -> &[AcirOpcode] { + &self.opcodes + } + pub(crate) fn take_opcodes(&mut self) -> Vec { std::mem::take(&mut self.opcodes) } diff --git a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 413c29e860b..2c151ed1e68 100644 --- a/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -270,6 +270,7 @@ impl Context { inputs, outputs, false, + true, )?; let output_vars: Vec<_> = output_values .iter() @@ -280,7 +281,16 @@ impl Context { for acir_var in output_vars { self.acir_context.return_var(acir_var)?; } - Ok(self.acir_context.finish(witness_inputs, Vec::new())) + + let generated_acir = self.acir_context.finish(witness_inputs, Vec::new()); + + assert_eq!( + generated_acir.opcodes().len(), + 1, + "Unconstrained programs should only generate a single opcode but multiple were emitted" + ); + + Ok(generated_acir) } /// Adds and binds `AcirVar`s for each numeric block parameter or block parameter array element. @@ -523,7 +533,7 @@ impl Context { let outputs: Vec = vecmap(result_ids, |result_id| dfg.type_of_value(*result_id).into()); - let output_values = self.acir_context.brillig(self.current_side_effects_enabled_var, code, inputs, outputs, true)?; + let output_values = self.acir_context.brillig(self.current_side_effects_enabled_var, code, inputs, outputs, true, false)?; // Compiler sanity check assert_eq!(result_ids.len(), output_values.len(), "ICE: The number of Brillig output values should match the result ids in SSA");