Skip to content

Commit

Permalink
chore: error on false constraint (#5890)
Browse files Browse the repository at this point in the history
# Description

## Problem\*

Report a bug during compilation when a constraint is false

## Summary\*

It is done in acir-gen which ensures that all side effects are resolved.


## Additional Context

The reported bug does not fail the compilation, because it will mess
around compile/run-time failures and also for the Noir tests. We want a
consistent handling of assert, whether it fails at compile-time,
run-time, or test-time.

## Documentation\*

Check one:
- [X] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** 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: jfecher <jake@aztecprotocol.com>
  • Loading branch information
guipublic and jfecher authored Sep 5, 2024
1 parent af3db4b commit 416b293
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 1 deletion.
3 changes: 3 additions & 0 deletions compiler/noirc_evaluator/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ impl From<SsaReport> for FileDiagnostic {
InternalBug::IndependentSubgraph { call_stack } => {
("There is no path from the output of this brillig call to either return values or inputs of the circuit, which creates an independent subgraph. This is quite likely a soundness vulnerability".to_string(),call_stack)
}
InternalBug::AssertFailed { call_stack } => ("As a result, the compiled circuit is ensured to fail. Other assertions may also fail during execution".to_string(), call_stack)
};
let call_stack = vecmap(call_stack, |location| location);
let file_id = call_stack.last().map(|location| location.file).unwrap_or_default();
Expand All @@ -111,6 +112,8 @@ pub enum InternalWarning {
pub enum InternalBug {
#[error("Input to brillig function is in a separate subgraph to output")]
IndependentSubgraph { call_stack: CallStack },
#[error("Assertion is always false")]
AssertFailed { call_stack: CallStack },
}

#[derive(Debug, PartialEq, Eq, Clone, Error)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use super::big_int::BigIntContext;
use super::generated_acir::{BrilligStdlibFunc, GeneratedAcir, PLACEHOLDER_BRILLIG_INDEX};
use crate::brillig::brillig_gen::brillig_directive;
use crate::brillig::brillig_ir::artifact::GeneratedBrillig;
use crate::errors::{InternalError, RuntimeError, SsaReport};
use crate::errors::{InternalBug, InternalError, RuntimeError, SsaReport};
use crate::ssa::acir_gen::{AcirDynamicArray, AcirValue};
use crate::ssa::ir::dfg::CallStack;
use crate::ssa::ir::types::Type as SsaType;
Expand Down Expand Up @@ -126,6 +126,8 @@ pub(crate) struct AcirContext<F: AcirField> {
big_int_ctx: BigIntContext,

expression_width: ExpressionWidth,

pub(crate) warnings: Vec<SsaReport>,
}

impl<F: AcirField> AcirContext<F> {
Expand Down Expand Up @@ -518,6 +520,12 @@ impl<F: AcirField> AcirContext<F> {
self.mark_variables_equivalent(lhs, rhs)?;
return Ok(());
}
if diff_expr.is_const() {
// Constraint is always false
self.warnings.push(SsaReport::Bug(InternalBug::AssertFailed {
call_stack: self.get_call_stack(),
}));
}

self.acir_ir.assert_is_zero(diff_expr);
if let Some(payload) = assert_message {
Expand Down
1 change: 1 addition & 0 deletions compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ impl<'a> Context<'a> {
}

warnings.extend(return_warnings);
warnings.extend(self.acir_context.warnings.clone());

// Add the warnings from the alter Ssa passes
Ok(self.acir_context.finish(input_witness, return_witnesses, warnings))
Expand Down

0 comments on commit 416b293

Please sign in to comment.