feat: Support for optional assertion messages (#2491)

Co-authored-by: kevaundray <kevtheappdev@gmail.com>

crates/nargo/src/errors.rs

@@ -1,4 +1,4 @@
-use acvm::pwg::OpcodeResolutionError;
+use acvm::{acir::circuit::OpcodeLocation, pwg::OpcodeResolutionError};
 use noirc_printable_type::ForeignCallError;
 use thiserror::Error;
 
@@ -8,10 +8,20 @@ pub enum NargoError {
     #[error("Failed to compile circuit")]
     CompilationError,
 
-    /// ACIR circuit solving error
+    /// ACIR circuit execution error
     #[error(transparent)]
-    SolvingError(#[from] OpcodeResolutionError),
+    ExecutionError(#[from] ExecutionError),
 
+    /// Oracle handling error
     #[error(transparent)]
     ForeignCallError(#[from] ForeignCallError),
 }
+
+#[derive(Debug, Error)]
+pub enum ExecutionError {
+    #[error("Failed assertion: '{}'", .0)]
+    AssertionFailed(String, OpcodeLocation),
+
+    #[error(transparent)]
+    SolvingError(#[from] OpcodeResolutionError),
+}

crates/nargo/src/lib.rs

@@ -9,7 +9,7 @@
 
 pub mod artifacts;
 pub mod constants;
-mod errors;
+pub mod errors;
 pub mod ops;
 pub mod package;
 pub mod workspace;

crates/nargo/src/ops/execute.rs

@@ -1,7 +1,8 @@
-use acvm::pwg::{ACVMStatus, ACVM};
+use acvm::pwg::{ACVMStatus, ErrorLocation, OpcodeResolutionError, ACVM};
 use acvm::BlackBoxFunctionSolver;
 use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap};
 
+use crate::errors::ExecutionError;
 use crate::NargoError;
 
 use super::foreign_calls::ForeignCall;
@@ -14,6 +15,15 @@ pub fn execute_circuit<B: BlackBoxFunctionSolver>(
 ) -> Result<WitnessMap, NargoError> {
     let mut acvm = ACVM::new(blackbox_solver, circuit.opcodes, initial_witness);
 
+    // Assert messages are not a map due to https://github.com/noir-lang/acvm/issues/522
+    let get_assert_message = |opcode_location| {
+        circuit
+            .assert_messages
+            .iter()
+            .find(|(loc, _)| loc == opcode_location)
+            .map(|(_, message)| message.clone())
+    };
+
     loop {
         let solver_status = acvm.solve();
 
@@ -22,7 +32,19 @@ pub fn execute_circuit<B: BlackBoxFunctionSolver>(
             ACVMStatus::InProgress => {
                 unreachable!("Execution should not stop while in `InProgress` state.")
             }
-            ACVMStatus::Failure(error) => return Err(error.into()),
+            ACVMStatus::Failure(error) => {
+                return Err(NargoError::ExecutionError(match error {
+                    OpcodeResolutionError::UnsatisfiedConstrain {
+                        opcode_location: ErrorLocation::Resolved(opcode_location),
+                    } => match get_assert_message(&opcode_location) {
+                        Some(assert_message) => {
+                            ExecutionError::AssertionFailed(assert_message, opcode_location)
+                        }
+                        None => ExecutionError::SolvingError(error),
+                    },
+                    _ => ExecutionError::SolvingError(error),
+                }))
+            }
             ACVMStatus::RequiresForeignCall(foreign_call) => {
                 let foreign_call_result = ForeignCall::execute(&foreign_call, show_output)?;
                 acvm.resolve_pending_foreign_call(foreign_call_result);

crates/nargo_cli/src/cli/execute_cmd.rs

@@ -1,10 +1,10 @@
 use acvm::acir::circuit::OpcodeLocation;
 use acvm::acir::{circuit::Circuit, native_types::WitnessMap};
-use acvm::pwg::ErrorLocation;
+use acvm::pwg::{ErrorLocation, OpcodeResolutionError};
 use clap::Args;
 use nargo::constants::PROVER_INPUT_FILE;
+use nargo::errors::{ExecutionError, NargoError};
 use nargo::package::Package;
-use nargo::NargoError;
 use nargo_toml::{get_package_manifest, resolve_workspace_from_toml, PackageSelection};
 use noirc_abi::input_parser::{Format, InputValue};
 use noirc_abi::{Abi, InputMap};
@@ -95,24 +95,25 @@ fn execute_package(
 /// If the location has been resolved we return the contained [OpcodeLocation].
 fn extract_opcode_error_from_nargo_error(
     nargo_err: &NargoError,
-) -> Option<(OpcodeLocation, &acvm::pwg::OpcodeResolutionError)> {
-    let solving_err = match nargo_err {
-        nargo::NargoError::SolvingError(err) => err,
+) -> Option<(OpcodeLocation, &ExecutionError)> {
+    let execution_error = match nargo_err {
+        NargoError::ExecutionError(err) => err,
         _ => return None,
     };
 
-    match solving_err {
-        acvm::pwg::OpcodeResolutionError::IndexOutOfBounds {
+    match execution_error {
+        ExecutionError::AssertionFailed(_, location) => Some((*location, execution_error)),
+        ExecutionError::SolvingError(OpcodeResolutionError::IndexOutOfBounds {
             opcode_location: error_location,
             ..
-        }
-        | acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain {
+        })
+        | ExecutionError::SolvingError(OpcodeResolutionError::UnsatisfiedConstrain {
             opcode_location: error_location,
-        } => match error_location {
+        }) => match error_location {
             ErrorLocation::Unresolved => {
                 unreachable!("Cannot resolve index for unsatisfied constraint")
             }
-            ErrorLocation::Resolved(opcode_location) => Some((*opcode_location, solving_err)),
+            ErrorLocation::Resolved(opcode_location) => Some((*opcode_location, execution_error)),
         },
         _ => None,
     }
@@ -122,7 +123,7 @@ fn extract_opcode_error_from_nargo_error(
 /// to determine an opcode's call stack. Then report the error using the resolved
 /// call stack and any other relevant error information returned from the ACVM.
 fn report_error_with_opcode_location(
-    opcode_err_info: Option<(OpcodeLocation, &acvm::pwg::OpcodeResolutionError)>,
+    opcode_err_info: Option<(OpcodeLocation, &ExecutionError)>,
     debug: &DebugInfo,
     context: &Context,
 ) {
@@ -132,18 +133,21 @@ fn report_error_with_opcode_location(
             // of the call stack (the last item in the Vec).
             if let Some(location) = locations.last() {
                 let message = match opcode_err {
-                    acvm::pwg::OpcodeResolutionError::IndexOutOfBounds {
+                    ExecutionError::AssertionFailed(message, _) => {
+                        format!("Assertion failed: '{message}'")
+                    }
+                    ExecutionError::SolvingError(OpcodeResolutionError::IndexOutOfBounds {
                         index,
                         array_size,
                         ..
-                    } => {
+                    }) => {
                         format!(
                             "Index out of bounds, array has size {array_size:?}, but index was {index:?}"
                         )
                     }
-                    acvm::pwg::OpcodeResolutionError::UnsatisfiedConstrain { .. } => {
-                        "Failed constraint".into()
-                    }
+                    ExecutionError::SolvingError(OpcodeResolutionError::UnsatisfiedConstrain {
+                        ..
+                    }) => "Failed constraint".into(),
                     _ => {
                         // All other errors that do not have corresponding opcode locations
                         // should not be reported in this method.

crates/nargo_cli/tests/execution_success/assert_statement/src/main.nr

@@ -2,5 +2,6 @@
 // 
 // The features being tested is assertion
 fn main(x : Field, y : Field)  {
-    assert(x == y);
+    assert(x == y, "x and y are not equal");
+    assert_eq(x, y, "x and y are not equal");
 }

crates/nargo_cli/tests/execution_success/brillig_assert/src/main.nr

@@ -6,6 +6,7 @@ fn main(x: Field)  {
 }
 
 unconstrained fn conditional(x : bool) -> Field {
-    assert(x);
+    assert(x, "x is false");
+    assert_eq(x, true, "x is false");
     1
 }

crates/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -214,7 +214,7 @@ impl<'block> BrilligBlock<'block> {
                 );
                 self.convert_ssa_binary(binary, dfg, result_register);
             }
-            Instruction::Constrain(lhs, rhs) => {
+            Instruction::Constrain(lhs, rhs, assert_message) => {
                 let condition = self.brillig_context.allocate_register();
 
                 self.convert_ssa_binary(
@@ -223,7 +223,7 @@ impl<'block> BrilligBlock<'block> {
                     condition,
                 );
 
-                self.brillig_context.constrain_instruction(condition);
+                self.brillig_context.constrain_instruction(condition, assert_message.clone());
                 self.brillig_context.deallocate_register(condition);
             }
             Instruction::Allocate => {

crates/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs

@@ -33,6 +33,7 @@ pub(crate) fn directive_invert() -> GeneratedBrillig {
             },
             BrilligOpcode::Stop,
         ],
+        assert_messages: Default::default(),
         locations: Default::default(),
     }
 }
@@ -101,6 +102,7 @@ pub(crate) fn directive_quotient(bit_size: u32) -> GeneratedBrillig {
             },
             BrilligOpcode::Stop,
         ],
+        assert_messages: Default::default(),
         locations: Default::default(),
     }
 }

crates/noirc_evaluator/src/brillig/brillig_ir.rs

@@ -361,13 +361,20 @@ impl BrilligContext {
 impl BrilligContext {
     /// Emits brillig bytecode to jump to a trap condition if `condition`
     /// is false.
-    pub(crate) fn constrain_instruction(&mut self, condition: RegisterIndex) {
+    pub(crate) fn constrain_instruction(
+        &mut self,
+        condition: RegisterIndex,
+        assert_message: Option<String>,
+    ) {
         self.debug_show.constrain_instruction(condition);
         self.add_unresolved_jump(
             BrilligOpcode::JumpIf { condition, location: 0 },
             self.next_section_label(),
         );
         self.push_opcode(BrilligOpcode::Trap);
+        if let Some(assert_message) = assert_message {
+            self.obj.add_assert_message_to_last_opcode(assert_message);
+        }
         self.enter_next_section();
     }
 

crates/noirc_evaluator/src/brillig/brillig_ir/artifact.rs

@@ -17,13 +17,16 @@ pub(crate) enum BrilligParameter {
 pub(crate) struct GeneratedBrillig {
     pub(crate) byte_code: Vec<BrilligOpcode>,
     pub(crate) locations: BTreeMap<OpcodeLocation, CallStack>,
+    pub(crate) assert_messages: BTreeMap<OpcodeLocation, String>,
 }
 
 #[derive(Default, Debug, Clone)]
 /// Artifacts resulting from the compilation of a function into brillig byte code.
-/// Currently it is just the brillig bytecode of the function.
+/// It includes the bytecode of the function and all the metadata that allows linking with other functions.
 pub(crate) struct BrilligArtifact {
-    byte_code: Vec<BrilligOpcode>,
+    pub(crate) byte_code: Vec<BrilligOpcode>,
+    /// A map of bytecode positions to assertion messages
+    pub(crate) assert_messages: BTreeMap<OpcodeLocation, String>,
     /// The set of jumps that need to have their locations
     /// resolved.
     unresolved_jumps: Vec<(JumpInstructionPosition, UnresolvedJumpLocation)>,
@@ -68,7 +71,11 @@ impl BrilligArtifact {
     /// Resolves all jumps and generates the final bytecode
     pub(crate) fn finish(mut self) -> GeneratedBrillig {
         self.resolve_jumps();
-        GeneratedBrillig { byte_code: self.byte_code, locations: self.locations }
+        GeneratedBrillig {
+            byte_code: self.byte_code,
+            locations: self.locations,
+            assert_messages: self.assert_messages,
+        }
     }
 
     /// Gets the first unresolved function call of this artifact.
@@ -131,6 +138,10 @@ impl BrilligArtifact {
                 .push((position_in_bytecode + offset, label_id.clone()));
         }
 
+        for (position_in_bytecode, message) in &obj.assert_messages {
+            self.assert_messages.insert(position_in_bytecode + offset, message.clone());
+        }
+
         for (position_in_bytecode, call_stack) in obj.locations.iter() {
             self.locations.insert(position_in_bytecode + offset, call_stack.clone());
         }
@@ -242,4 +253,9 @@ impl BrilligArtifact {
     pub(crate) fn set_call_stack(&mut self, call_stack: CallStack) {
         self.call_stack = call_stack;
     }
+
+    pub(crate) fn add_assert_message_to_last_opcode(&mut self, message: String) {
+        let position = self.index_of_next_opcode() - 1;
+        self.assert_messages.insert(position, message);
+    }
 }

crates/noirc_evaluator/src/errors.rs

@@ -16,11 +16,13 @@ use crate::ssa::ir::dfg::CallStack;
 
 #[derive(Debug, PartialEq, Eq, Clone, Error)]
 pub enum RuntimeError {
-    // We avoid showing the actual lhs and rhs since most of the time they are just 0
-    // and 1 respectively. This would confuse users if a constraint such as
-    // assert(foo < bar) fails with "failed constraint: 0 = 1."
-    #[error("Failed constraint")]
-    FailedConstraint { lhs: Box<Expression>, rhs: Box<Expression>, call_stack: CallStack },
+    #[error("{}", format_failed_constraint(.assert_message))]
+    FailedConstraint {
+        lhs: Box<Expression>,
+        rhs: Box<Expression>,
+        call_stack: CallStack,
+        assert_message: Option<String>,
+    },
     #[error(transparent)]
     InternalError(#[from] InternalError),
     #[error("Index out of bounds, array has size {index:?}, but index was {array_size:?}")]
@@ -39,6 +41,16 @@ pub enum RuntimeError {
     AssertConstantFailed { call_stack: CallStack },
 }
 
+// We avoid showing the actual lhs and rhs since most of the time they are just 0
+// and 1 respectively. This would confuse users if a constraint such as
+// assert(foo < bar) fails with "failed constraint: 0 = 1."
+fn format_failed_constraint(message: &Option<String>) -> String {
+    match message {
+        Some(message) => format!("Failed constraint: '{}'", message),
+        None => "Failed constraint".to_owned(),
+    }
+}
+
 #[derive(Debug, PartialEq, Eq, Clone, Error)]
 pub enum InternalError {
     #[error("ICE: Both expressions should have degree<=1")]

crates/noirc_evaluator/src/ssa.rs

@@ -78,7 +78,12 @@ pub fn create_circuit(
         optimize_into_acir(program, enable_ssa_logging, enable_brillig_logging)?;
     let opcodes = generated_acir.take_opcodes();
     let GeneratedAcir {
-        current_witness_index, return_witnesses, locations, input_witnesses, ..
+        current_witness_index,
+        return_witnesses,
+        locations,
+        input_witnesses,
+        assert_messages,
+        ..
     } = generated_acir;
 
     let abi = gen_abi(context, func_sig, &input_witnesses, return_witnesses.clone());
@@ -99,7 +104,7 @@ pub fn create_circuit(
         private_parameters,
         public_parameters,
         return_values,
-        assert_messages: Default::default(),
+        assert_messages: assert_messages.into_iter().collect(),
     };
 
     // This converts each im::Vector in the BTreeMap to a Vec