Skip to content

Commit

Permalink
fix: ensure that unconstrained entrypoint functions don't generate co…
Browse files Browse the repository at this point in the history
…nstraints (#4292)

# Description

## Problem\*

Resolves <!-- Link to GitHub Issue -->

## 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 <[email protected]>
  • Loading branch information
TomAFrench and kevaundray authored Feb 7, 2024
1 parent ed697bb commit fae4ead
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -1438,6 +1440,7 @@ impl AcirContext {
inputs: Vec<AcirValue>,
outputs: Vec<AcirType>,
attempt_execution: bool,
unsafe_return_values: bool,
) -> Result<Vec<AcirValue>, RuntimeError> {
let b_inputs = try_vecmap(inputs, |i| -> Result<_, InternalError> {
match i {
Expand Down Expand Up @@ -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)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,10 @@ impl GeneratedAcir {
}
}

pub(crate) fn opcodes(&self) -> &[AcirOpcode] {
&self.opcodes
}

pub(crate) fn take_opcodes(&mut self) -> Vec<AcirOpcode> {
std::mem::take(&mut self.opcodes)
}
Expand Down
14 changes: 12 additions & 2 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ impl Context {
inputs,
outputs,
false,
true,
)?;
let output_vars: Vec<_> = output_values
.iter()
Expand All @@ -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.
Expand Down Expand Up @@ -523,7 +533,7 @@ impl Context {

let outputs: Vec<AcirType> = 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");
Expand Down

0 comments on commit fae4ead

Please sign in to comment.