feat!: Do not encode assertion strings in the programs (#8315)

avm-transpiler/Cargo.toml

@@ -9,8 +9,9 @@ license = "MIT OR Apache-2.0"
 
 [dependencies]
 # local
-acvm = { path = "../noir/noir-repo/acvm-repo/acvm", features=["bn254"] }
+acvm = { path = "../noir/noir-repo/acvm-repo/acvm", features = ["bn254"] }
 noirc_errors = { path = "../noir/noir-repo/compiler/noirc_errors" }
+fxhash = "0.2.1"
 
 # external
 base64 = "0.21"

avm-transpiler/src/transpile.rs

@@ -1,6 +1,6 @@
-use std::collections::BTreeMap;
-
 use acvm::acir::brillig::{BitSize, IntegerBitSize, Opcode as BrilligOpcode};
+use fxhash::FxHashMap as HashMap;
+use std::collections::BTreeMap;
 
 use acvm::acir::circuit::BrilligOpcodeLocation;
 use acvm::brillig_vm::brillig::{
@@ -1100,6 +1100,18 @@ pub fn patch_debug_info_pcs(
     debug_infos
 }
 
+/// Patch the assert messages with updated PCs since transpilation injects extra
+/// opcodes into the bytecode.
+pub fn patch_assert_message_pcs(
+    assert_messages: HashMap<usize, String>,
+    brillig_pcs_to_avm_pcs: &[usize],
+) -> HashMap<usize, String> {
+    assert_messages
+        .into_iter()
+        .map(|(brillig_pc, message)| (brillig_pcs_to_avm_pcs[brillig_pc], message))
+        .collect()
+}
+
 /// Compute an array that maps each Brillig pc to an AVM pc.
 /// This must be done before transpiling to properly transpile jump destinations.
 /// This is necessary for two reasons:

avm-transpiler/src/transpile_contract.rs

@@ -8,8 +8,11 @@ use serde::{Deserialize, Serialize};
 use acvm::acir::circuit::Program;
 use noirc_errors::debug_info::ProgramDebugInfo;
 
-use crate::transpile::{brillig_to_avm, map_brillig_pcs_to_avm_pcs, patch_debug_info_pcs};
-use crate::utils::extract_brillig_from_acir_program;
+use crate::transpile::{
+    brillig_to_avm, map_brillig_pcs_to_avm_pcs, patch_assert_message_pcs, patch_debug_info_pcs,
+};
+use crate::utils::{extract_brillig_from_acir_program, extract_static_assert_messages};
+use fxhash::FxHashMap as HashMap;
 
 /// Representation of a contract with some transpiled functions
 #[derive(Debug, Serialize, Deserialize)]
@@ -49,6 +52,7 @@ pub struct AvmContractFunctionArtifact {
         deserialize_with = "ProgramDebugInfo::deserialize_compressed_base64_json"
     )]
     pub debug_symbols: ProgramDebugInfo,
+    pub assert_messages: HashMap<usize, String>,
 }
 
 /// Representation of an ACIR contract function but with
@@ -93,10 +97,15 @@ impl From<CompiledAcirContractArtifact> for TranspiledContractArtifact {
                 // Extract Brillig Opcodes from acir
                 let acir_program = function.bytecode;
                 let brillig_bytecode = extract_brillig_from_acir_program(&acir_program);
+                let assert_messages = extract_static_assert_messages(&acir_program);
 
                 // Map Brillig pcs to AVM pcs (index is Brillig PC, value is AVM PC)
                 let brillig_pcs_to_avm_pcs = map_brillig_pcs_to_avm_pcs(brillig_bytecode);
 
+                // Patch the assert messages with updated PCs
+                let assert_messages =
+                    patch_assert_message_pcs(assert_messages, &brillig_pcs_to_avm_pcs);
+
                 // Transpile to AVM
                 let avm_bytecode = brillig_to_avm(brillig_bytecode, &brillig_pcs_to_avm_pcs);
 
@@ -130,6 +139,7 @@ impl From<CompiledAcirContractArtifact> for TranspiledContractArtifact {
                         abi: function.abi,
                         bytecode: base64::prelude::BASE64_STANDARD.encode(compressed_avm_bytecode),
                         debug_symbols: ProgramDebugInfo { debug_infos },
+                        assert_messages,
                     },
                 ));
             } else {

avm-transpiler/src/utils.rs

@@ -1,9 +1,11 @@
+use fxhash::FxHashMap as HashMap;
+
 use acvm::acir::circuit::brillig::BrilligFunctionId;
 use acvm::FieldElement;
 use log::debug;
 
 use acvm::acir::brillig::Opcode as BrilligOpcode;
-use acvm::acir::circuit::{Opcode, Program};
+use acvm::acir::circuit::{AssertionPayload, Opcode, Program};
 
 use crate::instructions::AvmInstruction;
 
@@ -36,6 +38,33 @@ pub fn extract_brillig_from_acir_program(
     &program.unconstrained_functions[0].bytecode
 }
 
+/// Assertion messages that are static strings are stored in the assert_messages map of the ACIR program.
+pub fn extract_static_assert_messages(program: &Program<FieldElement>) -> HashMap<usize, String> {
+    assert_eq!(
+        program.functions.len(),
+        1,
+        "An AVM program should have only a single ACIR function with a 'BrilligCall'"
+    );
+    let main_function = &program.functions[0];
+    main_function
+        .assert_messages
+        .iter()
+        .filter_map(|(location, payload)| {
+            if let AssertionPayload::StaticString(static_string) = payload {
+                Some((
+                    location
+                        .to_brillig_location()
+                        .expect("Assert message is not for the brillig function")
+                        .0,
+                    static_string.clone(),
+                ))
+            } else {
+                None
+            }
+        })
+        .collect()
+}
+
 /// Print inputs, outputs, and instructions in a Brillig program
 pub fn dbg_print_brillig_program(brillig_bytecode: &[BrilligOpcode<FieldElement>]) {
     debug!("Printing Brillig program...");

noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs

@@ -266,7 +266,7 @@ impl<'block> BrilligBlock<'block> {
                     condition,
                 );
                 match assert_message {
-                    Some(ConstrainError::UserDefined(selector, values)) => {
+                    Some(ConstrainError::Dynamic(selector, values)) => {
                         let payload_values =
                             vecmap(values, |value| self.convert_ssa_value(*value, dfg));
                         let payload_as_params = vecmap(values, |value| {
@@ -280,7 +280,7 @@ impl<'block> BrilligBlock<'block> {
                             selector.as_u64(),
                         );
                     }
-                    Some(ConstrainError::Intrinsic(message)) => {
+                    Some(ConstrainError::StaticString(message)) => {
                         self.brillig_context.codegen_constrain(condition, Some(message.clone()));
                     }
                     None => {

noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs

@@ -647,10 +647,10 @@ impl<'a> Context<'a> {
 
                 let assert_payload = if let Some(error) = assert_message {
                     match error {
-                        ConstrainError::Intrinsic(string) => {
+                        ConstrainError::StaticString(string) => {
                             Some(AssertionPayload::StaticString(string.clone()))
                         }
-                        ConstrainError::UserDefined(error_selector, values) => {
+                        ConstrainError::Dynamic(error_selector, values) => {
                             if let Some(constant_string) = try_to_extract_string_from_error_payload(
                                 *error_selector,
                                 values,

noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs

@@ -468,12 +468,10 @@ impl Instruction {
                 let lhs = f(*lhs);
                 let rhs = f(*rhs);
                 let assert_message = assert_message.as_ref().map(|error| match error {
-                    ConstrainError::UserDefined(selector, payload_values) => {
-                        ConstrainError::UserDefined(
-                            *selector,
-                            payload_values.iter().map(|&value| f(value)).collect(),
-                        )
-                    }
+                    ConstrainError::Dynamic(selector, payload_values) => ConstrainError::Dynamic(
+                        *selector,
+                        payload_values.iter().map(|&value| f(value)).collect(),
+                    ),
                     _ => error.clone(),
                 });
                 Instruction::Constrain(lhs, rhs, assert_message)
@@ -541,7 +539,7 @@ impl Instruction {
             Instruction::Constrain(lhs, rhs, assert_error) => {
                 f(*lhs);
                 f(*rhs);
-                if let Some(ConstrainError::UserDefined(_, values)) = assert_error.as_ref() {
+                if let Some(ConstrainError::Dynamic(_, values)) = assert_error.as_ref() {
                     values.iter().for_each(|&val| {
                         f(val);
                     });
@@ -836,15 +834,15 @@ pub(crate) fn error_selector_from_type(typ: &ErrorType) -> ErrorSelector {
 
 #[derive(Debug, PartialEq, Eq, Hash, Clone, Serialize, Deserialize)]
 pub(crate) enum ConstrainError {
-    // These are errors which have been hardcoded during SSA gen
-    Intrinsic(String),
-    // These are errors issued by the user
-    UserDefined(ErrorSelector, Vec<ValueId>),
+    // Static string errors are not handled inside the program as data for efficiency reasons.
+    StaticString(String),
+    // These errors are handled by the program as data.
+    Dynamic(ErrorSelector, Vec<ValueId>),
 }
 
 impl From<String> for ConstrainError {
     fn from(value: String) -> Self {
-        ConstrainError::Intrinsic(value)
+        ConstrainError::StaticString(value)
     }
 }
 

noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs

@@ -245,10 +245,10 @@ fn display_constrain_error(
     f: &mut Formatter,
 ) -> Result {
     match error {
-        ConstrainError::Intrinsic(assert_message_string) => {
+        ConstrainError::StaticString(assert_message_string) => {
             writeln!(f, " '{assert_message_string:?}'")
         }
-        ConstrainError::UserDefined(selector, values) => {
+        ConstrainError::Dynamic(selector, values) => {
             if let Some(constant_string) =
                 try_to_extract_string_from_error_payload(*selector, values, &function.dfg)
             {

noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs

@@ -701,6 +701,11 @@ impl<'a> FunctionContext<'a> {
         assert_message: &Option<Box<(Expression, HirType)>>,
     ) -> Result<Option<ConstrainError>, RuntimeError> {
         let Some(assert_message_payload) = assert_message else { return Ok(None) };
+
+        if let Expression::Literal(ast::Literal::Str(static_string)) = &assert_message_payload.0 {
+            return Ok(Some(ConstrainError::StaticString(static_string.clone())));
+        }
+
         let (assert_message_expression, assert_message_typ) = assert_message_payload.as_ref();
 
         let values = self.codegen_expression(assert_message_expression)?.into_value_list(self);
@@ -713,7 +718,7 @@ impl<'a> FunctionContext<'a> {
                 self.builder.record_error_type(error_type_id, assert_message_typ.clone());
             }
         };
-        Ok(Some(ConstrainError::UserDefined(error_type_id, values)))
+        Ok(Some(ConstrainError::Dynamic(error_type_id, values)))
     }
 
     fn codegen_assign(&mut self, assign: &ast::Assign) -> Result<Values, RuntimeError> {

yarn-project/bb-prover/src/avm_proving.test.ts

@@ -39,6 +39,7 @@ import {
   initExecutionEnvironment,
   initHostStorage,
   initPersistableStateManager,
+  resolveAvmTestContractAssertionMessage,
 } from '@aztec/simulator/avm/fixtures';
 
 import { jest } from '@jest/globals';
@@ -256,7 +257,8 @@ const proveAndVerifyAvmTestContract = async (
   } else {
     // Explicit revert when an assertion failed.
     expect(avmResult.reverted).toBe(true);
-    expect(avmResult.revertReason?.message).toContain(assertionErrString);
+    expect(avmResult.revertReason).toBeDefined();
+    expect(resolveAvmTestContractAssertionMessage(functionName, avmResult.revertReason!)).toContain(assertionErrString);
   }
 
   const pxResult = trace.toPublicExecutionResult(

yarn-project/circuit-types/src/simulation_error.ts

@@ -108,6 +108,14 @@ export class SimulationError extends Error {
     return this.originalMessage;
   }
 
+  getOriginalMessage() {
+    return this.originalMessage;
+  }
+
+  setOriginalMessage(message: string) {
+    this.originalMessage = message;
+  }
+
   /**
    * Enriches the error with the name of a contract that failed.
    * @param contractAddress - The address of the contract

yarn-project/end-to-end/src/fixtures/fixtures.ts

@@ -5,7 +5,7 @@ export const privateKey2 = Buffer.from('59c6995e998f97a5a0044966f0945389dc9e86da
 /// Common errors
 export const U128_UNDERFLOW_ERROR = "Assertion failed: attempt to subtract with underflow 'hi == high'";
 export const U128_OVERFLOW_ERROR = "Assertion failed: attempt to add with overflow 'hi == high'";
-export const BITSIZE_TOO_BIG_ERROR = "Assertion failed. 'self.__assert_max_bit_size'";
+export const BITSIZE_TOO_BIG_ERROR = "Assertion failed: call to assert_max_bit_size 'self.__assert_max_bit_size'";
 // TODO(https://github.com/AztecProtocol/aztec-packages/issues/5818): Make these a fixed error after transition.
 export const DUPLICATE_NULLIFIER_ERROR = /dropped|duplicate nullifier|reverted/;
 export const NO_L1_TO_L2_MSG_ERROR =

yarn-project/foundation/src/abi/abi.ts

@@ -210,6 +210,10 @@ export interface FunctionArtifact extends FunctionAbi {
   verificationKey?: string;
   /** Maps opcodes to source code pointers */
   debugSymbols: string;
+  /**
+   * Public functions store their static assertion messages externally to the bytecode.
+   */
+  assertMessages?: Record<number, string>;
   /** Debug metadata for the function. */
   debug?: FunctionDebugMetadata;
 }
@@ -369,6 +373,10 @@ export interface FunctionDebugMetadata {
    * Maps the file IDs to the file contents to resolve pointers
    */
   files: DebugFileMap;
+  /**
+   * Public functions store their static assertion messages externally to the bytecode.
+   */
+  assertMessages?: Record<number, string>;
 }
 
 /**
@@ -407,7 +415,11 @@ export function getFunctionDebugMetadata(
     // TODO(https://github.com/AztecProtocol/aztec-packages/issues/5813)
     // We only support handling debug info for the contract function entry point.
     // So for now we simply index into the first debug info.
-    return { debugSymbols: programDebugSymbols.debug_infos[0], files: contractArtifact.fileMap };
+    return {
+      debugSymbols: programDebugSymbols.debug_infos[0],
+      files: contractArtifact.fileMap,
+      assertMessages: functionArtifact.assertMessages,
+    };
   }
   return undefined;
 }