From d72f791ac47c3e6677040991a5c9677b7115edcb Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Oct 2024 15:51:55 +0000 Subject: [PATCH 01/16] feat: encode static error strings in the ABI --- avm-transpiler/src/transpile_contract.rs | 14 +- avm-transpiler/src/utils.rs | 31 +---- .../dsl/acir_format/serde/acir.hpp | 126 ++---------------- .../noir-protocol-circuits/bootstrap.sh | 4 +- .../noir-repo/acvm-repo/acir/codegen/acir.cpp | 105 ++------------- .../acvm-repo/acir/src/circuit/mod.rs | 12 +- .../acvm-repo/acvm/src/pwg/brillig.rs | 24 +--- noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs | 100 ++++---------- .../compiler/noirc_driver/src/abi_gen.rs | 29 ++-- .../src/brillig/brillig_gen/brillig_block.rs | 6 +- .../brillig/brillig_gen/brillig_directive.rs | 4 +- .../src/brillig/brillig_ir/artifact.rs | 24 ++-- .../brillig_ir/codegen_control_flow.rs | 39 ++++-- .../compiler/noirc_evaluator/src/lib.rs | 2 + .../compiler/noirc_evaluator/src/ssa.rs | 19 ++- .../src/ssa/acir_gen/acir_ir/acir_variable.rs | 15 ++- .../ssa/acir_gen/acir_ir/generated_acir.rs | 35 +++-- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 29 ++-- .../src/ssa/function_builder/mod.rs | 7 +- .../noirc_evaluator/src/ssa/ir/instruction.rs | 46 +++---- .../noirc_evaluator/src/ssa/ir/printer.rs | 11 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 37 ++--- .../tooling/noir_js_types/src/types.ts | 1 + noir/noir-repo/tooling/noirc_abi/src/lib.rs | 8 ++ .../tooling/noirc_abi_wasm/src/lib.rs | 1 + .../test/shared/decode_error.ts | 6 + .../aztec.js/src/contract/contract.test.ts | 3 + .../default_multi_call_entrypoint.ts | 1 + .../aztec.js/src/wallet/account_wallet.ts | 3 + .../src/contract/contract_address.test.ts | 1 + .../entrypoints/src/account_entrypoint.ts | 1 + .../entrypoints/src/dapp_entrypoint.ts | 1 + yarn-project/foundation/src/abi/abi.ts | 22 ++- .../foundation/src/abi/encoder.test.ts | 8 ++ .../src/protocol_contract_data.ts | 8 +- .../pxe/src/pxe_service/error_enriching.ts | 8 +- yarn-project/simulator/src/acvm/acvm.ts | 42 +++--- .../types/src/abi/contract_artifact.ts | 2 +- yarn-project/types/src/noir/index.ts | 12 +- 39 files changed, 314 insertions(+), 533 deletions(-) diff --git a/avm-transpiler/src/transpile_contract.rs b/avm-transpiler/src/transpile_contract.rs index 10b9367a4abd..0c38dbe1e0b9 100644 --- a/avm-transpiler/src/transpile_contract.rs +++ b/avm-transpiler/src/transpile_contract.rs @@ -6,11 +6,8 @@ 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_assert_message_pcs, patch_debug_info_pcs, -}; -use crate::utils::{extract_brillig_from_acir_program, extract_static_assert_messages}; -use fxhash::FxHashMap as HashMap; +use crate::transpile::{brillig_to_avm, map_brillig_pcs_to_avm_pcs, patch_debug_info_pcs}; +use crate::utils::extract_brillig_from_acir_program; /// Representation of a contract with some transpiled functions #[derive(Debug, Serialize, Deserialize)] @@ -51,7 +48,6 @@ pub struct AvmContractFunctionArtifact { )] pub debug_symbols: ProgramDebugInfo, pub brillig_names: Vec, - pub assert_messages: HashMap, } /// Representation of an ACIR contract function but with @@ -96,16 +92,11 @@ impl From 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); info!("Extracted Brillig program has {} instructions", brillig_bytecode.len()); // 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); @@ -132,7 +123,6 @@ impl From for TranspiledContractArtifact { bytecode: base64::prelude::BASE64_STANDARD.encode(avm_bytecode), debug_symbols: ProgramDebugInfo { debug_infos }, brillig_names: function.brillig_names, - assert_messages, }, )); } else { diff --git a/avm-transpiler/src/utils.rs b/avm-transpiler/src/utils.rs index 3f5269f5daa8..018276e4b6d9 100644 --- a/avm-transpiler/src/utils.rs +++ b/avm-transpiler/src/utils.rs @@ -1,11 +1,9 @@ -use fxhash::FxHashMap as HashMap; - use acvm::acir::circuit::brillig::BrilligFunctionId; use acvm::{AcirField, FieldElement}; use log::{debug, info, trace}; use acvm::acir::brillig::Opcode as BrilligOpcode; -use acvm::acir::circuit::{AssertionPayload, Opcode, Program}; +use acvm::acir::circuit::{Opcode, Program}; use crate::instructions::{AvmInstruction, AvmOperand}; use crate::opcodes::AvmOpcode; @@ -39,33 +37,6 @@ 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) -> HashMap { - 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]) { trace!("Printing Brillig program..."); diff --git a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp index 7fd958cc60fa..9116fac30a46 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp @@ -1,6 +1,5 @@ #pragma once -#include "barretenberg/common/throw_or_abort.hpp" #include "bincode.hpp" #include "serde.hpp" @@ -1276,24 +1275,8 @@ struct ExpressionOrMemory { }; struct AssertionPayload { - - struct StaticString { - std::string value; - - friend bool operator==(const StaticString&, const StaticString&); - std::vector bincodeSerialize() const; - static StaticString bincodeDeserialize(std::vector); - }; - - struct Dynamic { - std::tuple> value; - - friend bool operator==(const Dynamic&, const Dynamic&); - std::vector bincodeSerialize() const; - static Dynamic bincodeDeserialize(std::vector); - }; - - std::variant value; + uint64_t error_selector; + std::vector payload; friend bool operator==(const AssertionPayload&, const AssertionPayload&); std::vector bincodeSerialize() const; @@ -1395,7 +1378,10 @@ namespace Program { inline bool operator==(const AssertionPayload& lhs, const AssertionPayload& rhs) { - if (!(lhs.value == rhs.value)) { + if (!(lhs.error_selector == rhs.error_selector)) { + return false; + } + if (!(lhs.payload == rhs.payload)) { return false; } return true; @@ -1426,7 +1412,8 @@ void serde::Serializable::serialize(const Program::As Serializer& serializer) { serializer.increase_container_depth(); - serde::Serializable::serialize(obj.value, serializer); + serde::Serializable::serialize(obj.error_selector, serializer); + serde::Serializable::serialize(obj.payload, serializer); serializer.decrease_container_depth(); } @@ -1436,107 +1423,14 @@ Program::AssertionPayload serde::Deserializable::dese { deserializer.increase_container_depth(); Program::AssertionPayload obj; - obj.value = serde::Deserializable::deserialize(deserializer); + obj.error_selector = serde::Deserializable::deserialize(deserializer); + obj.payload = serde::Deserializable::deserialize(deserializer); deserializer.decrease_container_depth(); return obj; } namespace Program { -inline bool operator==(const AssertionPayload::StaticString& lhs, const AssertionPayload::StaticString& rhs) -{ - if (!(lhs.value == rhs.value)) { - return false; - } - return true; -} - -inline std::vector AssertionPayload::StaticString::bincodeSerialize() const -{ - auto serializer = serde::BincodeSerializer(); - serde::Serializable::serialize(*this, serializer); - return std::move(serializer).bytes(); -} - -inline AssertionPayload::StaticString AssertionPayload::StaticString::bincodeDeserialize(std::vector input) -{ - auto deserializer = serde::BincodeDeserializer(input); - auto value = serde::Deserializable::deserialize(deserializer); - if (deserializer.get_buffer_offset() < input.size()) { - throw_or_abort("Some input bytes were not read"); - } - return value; -} - -} // end of namespace Program - -template <> -template -void serde::Serializable::serialize( - const Program::AssertionPayload::StaticString& obj, Serializer& serializer) -{ - serde::Serializable::serialize(obj.value, serializer); -} - -template <> -template -Program::AssertionPayload::StaticString serde::Deserializable::deserialize( - Deserializer& deserializer) -{ - Program::AssertionPayload::StaticString obj; - obj.value = serde::Deserializable::deserialize(deserializer); - return obj; -} - -namespace Program { - -inline bool operator==(const AssertionPayload::Dynamic& lhs, const AssertionPayload::Dynamic& rhs) -{ - if (!(lhs.value == rhs.value)) { - return false; - } - return true; -} - -inline std::vector AssertionPayload::Dynamic::bincodeSerialize() const -{ - auto serializer = serde::BincodeSerializer(); - serde::Serializable::serialize(*this, serializer); - return std::move(serializer).bytes(); -} - -inline AssertionPayload::Dynamic AssertionPayload::Dynamic::bincodeDeserialize(std::vector input) -{ - auto deserializer = serde::BincodeDeserializer(input); - auto value = serde::Deserializable::deserialize(deserializer); - if (deserializer.get_buffer_offset() < input.size()) { - throw_or_abort("Some input bytes were not read"); - } - return value; -} - -} // end of namespace Program - -template <> -template -void serde::Serializable::serialize(const Program::AssertionPayload::Dynamic& obj, - Serializer& serializer) -{ - serde::Serializable::serialize(obj.value, serializer); -} - -template <> -template -Program::AssertionPayload::Dynamic serde::Deserializable::deserialize( - Deserializer& deserializer) -{ - Program::AssertionPayload::Dynamic obj; - obj.value = serde::Deserializable::deserialize(deserializer); - return obj; -} - -namespace Program { - inline bool operator==(const BinaryFieldOp& lhs, const BinaryFieldOp& rhs) { if (!(lhs.value == rhs.value)) { diff --git a/noir-projects/noir-protocol-circuits/bootstrap.sh b/noir-projects/noir-protocol-circuits/bootstrap.sh index c90b7e00328c..8a4a33987d0b 100755 --- a/noir-projects/noir-protocol-circuits/bootstrap.sh +++ b/noir-projects/noir-protocol-circuits/bootstrap.sh @@ -45,7 +45,9 @@ PARALLEL_VK=${PARALLEL_VK:-true} if [[ AVAILABLE_MEMORY -gt MIN_PARALLEL_VK_GENERATION_MEMORY ]] && [[ $PARALLEL_VK == "true" ]]; then echo "Generating vks in parallel..." for pathname in "./target"/*.json; do - BB_HASH=$BB_HASH node ../scripts/generate_vk_json.js "$pathname" "./target/keys" & + if [[ $pathname != *"_simulated_"* ]]; then + BB_HASH=$BB_HASH node ../scripts/generate_vk_json.js "$pathname" "./target/keys" & + fi done for job in $(jobs -p); do diff --git a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp index 0880b5a0cbef..d228258d4654 100644 --- a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp +++ b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp @@ -1216,24 +1216,8 @@ namespace Program { }; struct AssertionPayload { - - struct StaticString { - std::string value; - - friend bool operator==(const StaticString&, const StaticString&); - std::vector bincodeSerialize() const; - static StaticString bincodeDeserialize(std::vector); - }; - - struct Dynamic { - std::tuple> value; - - friend bool operator==(const Dynamic&, const Dynamic&); - std::vector bincodeSerialize() const; - static Dynamic bincodeDeserialize(std::vector); - }; - - std::variant value; + uint64_t error_selector; + std::vector payload; friend bool operator==(const AssertionPayload&, const AssertionPayload&); std::vector bincodeSerialize() const; @@ -1335,7 +1319,8 @@ namespace Program { namespace Program { inline bool operator==(const AssertionPayload &lhs, const AssertionPayload &rhs) { - if (!(lhs.value == rhs.value)) { return false; } + if (!(lhs.error_selector == rhs.error_selector)) { return false; } + if (!(lhs.payload == rhs.payload)) { return false; } return true; } @@ -1360,7 +1345,8 @@ template <> template void serde::Serializable::serialize(const Program::AssertionPayload &obj, Serializer &serializer) { serializer.increase_container_depth(); - serde::Serializable::serialize(obj.value, serializer); + serde::Serializable::serialize(obj.error_selector, serializer); + serde::Serializable::serialize(obj.payload, serializer); serializer.decrease_container_depth(); } @@ -1369,87 +1355,12 @@ template Program::AssertionPayload serde::Deserializable::deserialize(Deserializer &deserializer) { deserializer.increase_container_depth(); Program::AssertionPayload obj; - obj.value = serde::Deserializable::deserialize(deserializer); + obj.error_selector = serde::Deserializable::deserialize(deserializer); + obj.payload = serde::Deserializable::deserialize(deserializer); deserializer.decrease_container_depth(); return obj; } -namespace Program { - - inline bool operator==(const AssertionPayload::StaticString &lhs, const AssertionPayload::StaticString &rhs) { - if (!(lhs.value == rhs.value)) { return false; } - return true; - } - - inline std::vector AssertionPayload::StaticString::bincodeSerialize() const { - auto serializer = serde::BincodeSerializer(); - serde::Serializable::serialize(*this, serializer); - return std::move(serializer).bytes(); - } - - inline AssertionPayload::StaticString AssertionPayload::StaticString::bincodeDeserialize(std::vector input) { - auto deserializer = serde::BincodeDeserializer(input); - auto value = serde::Deserializable::deserialize(deserializer); - if (deserializer.get_buffer_offset() < input.size()) { - throw serde::deserialization_error("Some input bytes were not read"); - } - return value; - } - -} // end of namespace Program - -template <> -template -void serde::Serializable::serialize(const Program::AssertionPayload::StaticString &obj, Serializer &serializer) { - serde::Serializable::serialize(obj.value, serializer); -} - -template <> -template -Program::AssertionPayload::StaticString serde::Deserializable::deserialize(Deserializer &deserializer) { - Program::AssertionPayload::StaticString obj; - obj.value = serde::Deserializable::deserialize(deserializer); - return obj; -} - -namespace Program { - - inline bool operator==(const AssertionPayload::Dynamic &lhs, const AssertionPayload::Dynamic &rhs) { - if (!(lhs.value == rhs.value)) { return false; } - return true; - } - - inline std::vector AssertionPayload::Dynamic::bincodeSerialize() const { - auto serializer = serde::BincodeSerializer(); - serde::Serializable::serialize(*this, serializer); - return std::move(serializer).bytes(); - } - - inline AssertionPayload::Dynamic AssertionPayload::Dynamic::bincodeDeserialize(std::vector input) { - auto deserializer = serde::BincodeDeserializer(input); - auto value = serde::Deserializable::deserialize(deserializer); - if (deserializer.get_buffer_offset() < input.size()) { - throw serde::deserialization_error("Some input bytes were not read"); - } - return value; - } - -} // end of namespace Program - -template <> -template -void serde::Serializable::serialize(const Program::AssertionPayload::Dynamic &obj, Serializer &serializer) { - serde::Serializable::serialize(obj.value, serializer); -} - -template <> -template -Program::AssertionPayload::Dynamic serde::Deserializable::deserialize(Deserializer &deserializer) { - Program::AssertionPayload::Dynamic obj; - obj.value = serde::Deserializable::deserialize(deserializer); - return obj; -} - namespace Program { inline bool operator==(const BinaryFieldOp &lhs, const BinaryFieldOp &rhs) { diff --git a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs index f700fefe0cdd..c1f9c6d09ace 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs @@ -82,9 +82,9 @@ pub enum ExpressionOrMemory { } #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] -pub enum AssertionPayload { - StaticString(String), - Dynamic(/* error_selector */ u64, Vec>), +pub struct AssertionPayload { + pub error_selector: u64, + pub payload: Vec>, } #[derive(Debug, Copy, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)] @@ -120,12 +120,6 @@ impl<'de> Deserialize<'de> for ErrorSelector { } } -/// This selector indicates that the payload is a string. -/// This is used to parse any error with a string payload directly, -/// to avoid users having to parse the error externally to the ACVM. -/// Only non-string errors need to be parsed externally to the ACVM using the circuit ABI. -pub const STRING_ERROR_SELECTOR: ErrorSelector = ErrorSelector(0); - #[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)] pub struct RawAssertionPayload { pub selector: ErrorSelector, diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs index dffa45dbd7a5..a5f5783478ed 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs @@ -6,7 +6,6 @@ use acir::{ brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}, opcodes::BlockId, ErrorSelector, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload, - STRING_ERROR_SELECTOR, }, native_types::WitnessMap, AcirField, @@ -307,25 +306,10 @@ fn extract_failure_payload_from_memory( .expect("Error selector is not u64"), ); - match error_selector { - STRING_ERROR_SELECTOR => { - // If the error selector is 0, it means the error is a string - let string = revert_values_iter - .map(|&memory_value| { - let as_u8: u8 = memory_value.try_into().expect("String item is not u8"); - as_u8 as char - }) - .collect(); - Some(ResolvedAssertionPayload::String(string)) - } - _ => { - // If the error selector is not 0, it means the error is a custom error - Some(ResolvedAssertionPayload::Raw(RawAssertionPayload { - selector: error_selector, - data: revert_values_iter.map(|value| value.to_field()).collect(), - })) - } - } + Some(ResolvedAssertionPayload::Raw(RawAssertionPayload { + selector: error_selector, + data: revert_values_iter.map(|value| value.to_field()).collect(), + })) } } diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs index fa3498da6138..6b5984770182 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs @@ -10,7 +10,7 @@ use acir::{ AcirFunctionId, BlockId, ConstantOrWitnessEnum, FunctionInput, InvalidInputBitSize, }, AssertionPayload, ErrorSelector, ExpressionOrMemory, Opcode, OpcodeLocation, - RawAssertionPayload, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, + RawAssertionPayload, ResolvedAssertionPayload, }, native_types::{Expression, Witness, WitnessMap}, AcirField, BlackBoxFunc, @@ -445,59 +445,32 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { &self, location: OpcodeLocation, ) -> Option> { - let (_, found_assertion_payload) = + let (_, assertion_descriptor) = self.assertion_payloads.iter().find(|(loc, _)| location == *loc)?; - match found_assertion_payload { - AssertionPayload::StaticString(string) => { - Some(ResolvedAssertionPayload::String(string.clone())) - } - AssertionPayload::Dynamic(error_selector, expression) => { - let mut fields = vec![]; - for expr in expression { - match expr { - ExpressionOrMemory::Expression(expr) => { - let value = get_value(expr, &self.witness_map).ok()?; - fields.push(value); - } - ExpressionOrMemory::Memory(block_id) => { - let memory_block = self.block_solvers.get(block_id)?; - fields.extend((0..memory_block.block_len).map(|memory_index| { - *memory_block - .block_value - .get(&memory_index) - .expect("All memory is initialized on creation") - })); - } - } + let mut fields = vec![]; + for expr in assertion_descriptor.payload.iter() { + match expr { + ExpressionOrMemory::Expression(expr) => { + let value = get_value(expr, &self.witness_map).ok()?; + fields.push(value); + } + ExpressionOrMemory::Memory(block_id) => { + let memory_block = self.block_solvers.get(block_id)?; + fields.extend((0..memory_block.block_len).map(|memory_index| { + *memory_block + .block_value + .get(&memory_index) + .expect("All memory is initialized on creation") + })); } - let error_selector = ErrorSelector::new(*error_selector); - - Some(match error_selector { - STRING_ERROR_SELECTOR => { - // If the error selector is 0, it means the error is a string - let string = fields - .iter() - .map(|field| { - let as_u8: u8 = field - .try_to_u64() - .expect("String character doesn't fit in u64") - .try_into() - .expect("String character doesn't fit in u8"); - as_u8 as char - }) - .collect(); - ResolvedAssertionPayload::String(string) - } - _ => { - // If the error selector is not 0, it means the error is a custom error - ResolvedAssertionPayload::Raw(RawAssertionPayload { - selector: error_selector, - data: fields, - }) - } - }) } } + let error_selector = ErrorSelector::new(assertion_descriptor.error_selector); + + Some(ResolvedAssertionPayload::Raw(RawAssertionPayload { + selector: error_selector, + data: fields, + })) } fn solve_brillig_call_opcode( @@ -530,7 +503,7 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { )?, }; - let result = solver.solve().map_err(|err| self.map_brillig_error(err))?; + let result = solver.solve()?; match result { BrilligSolverStatus::ForeignCallWait(foreign_call) => { @@ -570,31 +543,6 @@ impl<'a, F: AcirField, B: BlackBoxFunctionSolver> ACVM<'a, F, B> { } } - fn map_brillig_error(&self, mut err: OpcodeResolutionError) -> OpcodeResolutionError { - match &mut err { - OpcodeResolutionError::BrilligFunctionFailed { call_stack, payload, .. } => { - // Some brillig errors have static strings as payloads, we can resolve them here - let last_location = - call_stack.last().expect("Call stacks should have at least one item"); - let assertion_descriptor = - self.assertion_payloads.iter().find_map(|(loc, payload)| { - if loc == last_location { - Some(payload) - } else { - None - } - }); - - if let Some(AssertionPayload::StaticString(string)) = assertion_descriptor { - *payload = Some(ResolvedAssertionPayload::String(string.clone())); - } - - err - } - _ => err, - } - } - pub fn step_into_brillig(&mut self) -> StepResult<'a, F, B> { let Opcode::BrilligCall { id, inputs, outputs, predicate } = &self.opcodes[self.instruction_pointer] diff --git a/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs b/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs index ad54d0f7d6bb..bf64c9c920c7 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs @@ -6,6 +6,7 @@ use iter_extended::vecmap; use noirc_abi::{ Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue, AbiVisibility, Sign, }; +use noirc_evaluator::ErrorType; use noirc_frontend::ast::{Signedness, Visibility}; use noirc_frontend::TypeBinding; use noirc_frontend::{ @@ -25,7 +26,7 @@ pub(super) fn gen_abi( context: &Context, func_id: &FuncId, return_visibility: Visibility, - error_types: BTreeMap, + error_types: BTreeMap, ) -> Abi { let (parameters, return_type) = compute_function_abi(context, func_id); let return_type = return_type.map(|typ| AbiReturnType { @@ -34,23 +35,27 @@ pub(super) fn gen_abi( }); let error_types = error_types .into_iter() - .map(|(selector, typ)| (selector, build_abi_error_type(context, &typ))) + .map(|(selector, typ)| (selector, build_abi_error_type(context, typ))) .collect(); Abi { parameters, return_type, error_types } } -fn build_abi_error_type(context: &Context, typ: &Type) -> AbiErrorType { +fn build_abi_error_type(context: &Context, typ: ErrorType) -> AbiErrorType { match typ { - Type::FmtString(len, item_types) => { - let length = len.evaluate_to_u32().expect("Cannot evaluate fmt length"); - let Type::Tuple(item_types) = item_types.as_ref() else { - unreachable!("FmtString items must be a tuple") - }; - let item_types = - item_types.iter().map(|typ| abi_type_from_hir_type(context, typ)).collect(); - AbiErrorType::FmtString { length, item_types } + ErrorType::Dynamic(typ) => { + if let Type::FmtString(len, item_types) = typ { + let length = len.evaluate_to_u32().expect("Cannot evaluate fmt length"); + let Type::Tuple(item_types) = item_types.as_ref() else { + unreachable!("FmtString items must be a tuple") + }; + let item_types = + item_types.iter().map(|typ| abi_type_from_hir_type(context, typ)).collect(); + AbiErrorType::FmtString { length, item_types } + } else { + AbiErrorType::Custom(abi_type_from_hir_type(context, &typ)) + } } - _ => AbiErrorType::Custom(abi_type_from_hir_type(context, typ)), + ErrorType::String(string) => AbiErrorType::String { string }, } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index f3b2f4a91521..721e92cdc02b 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -259,7 +259,7 @@ impl<'block> BrilligBlock<'block> { }; match assert_message { - Some(ConstrainError::Dynamic(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| { @@ -270,10 +270,10 @@ impl<'block> BrilligBlock<'block> { condition, payload_values, payload_as_params, - selector.as_u64(), + *selector, ); } - Some(ConstrainError::StaticString(message)) => { + Some(ConstrainError::StaticString(_, message)) => { self.brillig_context.codegen_constrain(condition, Some(message.clone())); } None => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs index f066d967e0d2..469f0b430d6d 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_directive.rs @@ -67,7 +67,7 @@ pub(crate) fn directive_invert() -> GeneratedBrillig { }, BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 1 }, ], - assert_messages: Default::default(), + error_types: Default::default(), locations: Default::default(), name: "directive_invert".to_string(), } @@ -132,7 +132,7 @@ pub(crate) fn directive_quotient() -> GeneratedBrillig { }, BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 2 }, ], - assert_messages: Default::default(), + error_types: Default::default(), locations: Default::default(), name: "directive_integer_quotient".to_string(), } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index d0bd91e185cb..e0ede8541dc6 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -1,8 +1,10 @@ use acvm::acir::brillig::Opcode as BrilligOpcode; +use acvm::acir::circuit::ErrorSelector; use std::collections::{BTreeMap, HashMap}; use crate::brillig::brillig_ir::procedures::ProcedureId; use crate::ssa::ir::{basic_block::BasicBlockId, dfg::CallStack, function::FunctionId}; +use crate::ErrorType; /// Represents a parameter or a return value of an entry point function. #[derive(Debug, Clone, Eq, PartialEq, Hash, PartialOrd, Ord)] @@ -22,7 +24,7 @@ pub(crate) enum BrilligParameter { pub(crate) struct GeneratedBrillig { pub(crate) byte_code: Vec>, pub(crate) locations: BTreeMap, - pub(crate) assert_messages: BTreeMap, + pub(crate) error_types: BTreeMap, pub(crate) name: String, } @@ -31,10 +33,7 @@ pub(crate) struct GeneratedBrillig { /// It includes the bytecode of the function and all the metadata that allows linking with other functions. pub(crate) struct BrilligArtifact { pub(crate) byte_code: Vec>, - /// A map of bytecode positions to assertion messages. - /// Some error messages (compiler intrinsics) are not emitted via revert data, - /// instead, they are handled externally so they don't add size to user programs. - pub(crate) assert_messages: BTreeMap, + pub(crate) error_types: BTreeMap, /// The set of jumps that need to have their locations /// resolved. unresolved_jumps: Vec<(JumpInstructionPosition, UnresolvedJumpLocation)>, @@ -147,7 +146,7 @@ impl BrilligArtifact { GeneratedBrillig { byte_code: self.byte_code, locations: self.locations, - assert_messages: self.assert_messages, + error_types: self.error_types, name: self.name, } } @@ -166,6 +165,10 @@ impl BrilligArtifact { // Add the unresolved jumps of the linked function to this artifact. self.add_unresolved_jumps_and_calls(obj); + for (error_selector, error_type) in &obj.error_types { + self.error_types.insert(*error_selector, error_type.clone()); + } + let mut byte_code = obj.byte_code.clone(); // Replace STOP with RETURN because this is not the end of the program now. @@ -211,10 +214,6 @@ impl BrilligArtifact { self.unresolved_external_call_labels.push((position_in_bytecode + offset, *label_id)); } - 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()); } @@ -326,9 +325,4 @@ 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); - } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs index 6935ebb0f530..d92dc8c4c0f5 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs @@ -1,8 +1,13 @@ use acvm::{ - acir::brillig::{HeapVector, MemoryAddress}, + acir::{ + brillig::{HeapVector, MemoryAddress}, + circuit::ErrorSelector, + }, AcirField, }; +use crate::{ssa::ir::instruction::error_selector_from_type, ErrorType}; + use super::{ artifact::BrilligParameter, brillig_variable::{BrilligArray, BrilligVariable, SingleAddrVariable}, @@ -152,7 +157,7 @@ impl BrilligContext< condition: SingleAddrVariable, revert_data_items: Vec, revert_data_types: Vec, - error_selector: u64, + error_selector: ErrorSelector, ) { assert!(condition.bit_size == 1); @@ -169,7 +174,7 @@ impl BrilligContext< ctx.indirect_const_instruction( current_revert_data_pointer, 64, - (error_selector as u128).into(), + (error_selector.as_u64() as u128).into(), ); ctx.codegen_usize_op_in_place(current_revert_data_pointer, BrilligBinaryOp::Add, 1); @@ -224,15 +229,27 @@ impl BrilligContext< assert!(condition.bit_size == 1); self.codegen_if_not(condition.address, |ctx| { - let revert_data_size_var = ctx.make_usize_constant_instruction(F::zero()); - ctx.trap_instruction(HeapVector { - pointer: MemoryAddress::direct(0), - size: revert_data_size_var.address, - }); - ctx.deallocate_single_addr(revert_data_size_var); if let Some(assert_message) = assert_message { - ctx.obj.add_assert_message_to_last_opcode(assert_message); - } + let error_type = ErrorType::String(assert_message); + let error_selector = error_selector_from_type(&error_type); + ctx.obj.error_types.insert(error_selector, error_type); + ctx.indirect_const_instruction( + ReservedRegisters::free_memory_pointer(), + 64, + (error_selector.as_u64() as u128).into(), + ); + ctx.trap_instruction(HeapVector { + pointer: ReservedRegisters::free_memory_pointer(), + size: ReservedRegisters::usize_one(), + }); + } else { + let revert_data = HeapVector { + pointer: ReservedRegisters::free_memory_pointer(), + size: ctx.make_usize_constant_instruction(0_usize.into()).address, + }; + ctx.trap_instruction(revert_data); + ctx.deallocate_register(revert_data.size); + }; }); } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/lib.rs b/noir/noir-repo/compiler/noirc_evaluator/src/lib.rs index 543769633381..3fddd9da0564 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/lib.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/lib.rs @@ -12,3 +12,5 @@ pub mod ssa; pub mod brillig; pub use ssa::create_program; + +pub use ssa::ir::instruction::ErrorType; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs index ea41b0cfb320..d87f9fc912de 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs @@ -26,13 +26,11 @@ use acvm::{ FieldElement, }; +use ir::instruction::ErrorType; use noirc_errors::debug_info::{DebugFunctions, DebugInfo, DebugTypes, DebugVariables}; use noirc_frontend::ast::Visibility; -use noirc_frontend::{ - hir_def::{function::FunctionSignature, types::Type as HirType}, - monomorphization::ast::Program, -}; +use noirc_frontend::{hir_def::function::FunctionSignature, monomorphization::ast::Program}; use tracing::{span, Level}; use self::{ @@ -171,13 +169,13 @@ pub struct SsaProgramArtifact { pub main_return_witnesses: Vec, pub names: Vec, pub brillig_names: Vec, - pub error_types: BTreeMap, + pub error_types: BTreeMap, } impl SsaProgramArtifact { fn new( unconstrained_functions: Vec>, - error_types: BTreeMap, + error_types: BTreeMap, ) -> Self { let program = AcirProgram { functions: Vec::default(), unconstrained_functions }; Self { @@ -201,6 +199,7 @@ impl SsaProgramArtifact { self.main_return_witnesses = circuit_artifact.return_witnesses; } self.names.push(circuit_artifact.name); + self.error_types.extend(circuit_artifact.error_types); } fn add_warnings(&mut self, mut warnings: Vec) { @@ -240,6 +239,12 @@ pub fn create_program( "The generated ACIRs should match the supplied function signatures" ); } + + let error_types = error_types + .into_iter() + .map(|(selector, hir_type)| (selector, ErrorType::Dynamic(hir_type))) + .collect(); + let mut program_artifact = SsaProgramArtifact::new(generated_brillig, error_types); // Add warnings collected at the Ssa stage @@ -271,6 +276,7 @@ pub struct SsaCircuitArtifact { warnings: Vec, input_witnesses: Vec, return_witnesses: Vec, + error_types: BTreeMap, } fn convert_generated_acir_into_circuit( @@ -342,6 +348,7 @@ fn convert_generated_acir_into_circuit( warnings, input_witnesses, return_witnesses, + error_types: generated_acir.error_types, } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs index 811b728a9d5f..e7d298558c45 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/acir_variable.rs @@ -1108,10 +1108,10 @@ impl AcirContext { let witness = self.var_to_witness(witness_var)?; self.acir_ir.range_constraint(witness, *bit_size)?; if let Some(message) = message { - self.acir_ir.assertion_payloads.insert( - self.acir_ir.last_acir_opcode_location(), - AssertionPayload::StaticString(message.clone()), - ); + let payload = self.generate_assertion_message_payload(message.clone()); + self.acir_ir + .assertion_payloads + .insert(self.acir_ir.last_acir_opcode_location(), payload); } } NumericType::NativeField => { @@ -2068,6 +2068,13 @@ impl AcirContext { self.acir_ir.push_opcode(Opcode::Call { id, inputs, outputs, predicate }); Ok(results) } + + pub(crate) fn generate_assertion_message_payload( + &mut self, + message: String, + ) -> AssertionPayload { + self.acir_ir.generate_assertion_message_payload(message) + } } /// Enum representing the possible values that a diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index 01fcaef90425..e9d6ec49d0bd 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -5,13 +5,16 @@ use std::{collections::BTreeMap, u32}; use crate::{ brillig::{brillig_gen::brillig_directive, brillig_ir::artifact::GeneratedBrillig}, errors::{InternalError, RuntimeError, SsaReport}, - ssa::ir::dfg::CallStack, + ssa::ir::{ + dfg::CallStack, + instruction::{error_selector_from_type, ErrorType}, + }, }; use acvm::acir::{ circuit::{ brillig::{BrilligFunctionId, BrilligInputs, BrilligOutputs}, opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode}, - AssertionPayload, BrilligOpcodeLocation, OpcodeLocation, + AssertionPayload, BrilligOpcodeLocation, ErrorSelector, OpcodeLocation, }, native_types::Witness, BlackBoxFunc, @@ -62,6 +65,9 @@ pub(crate) struct GeneratedAcir { /// Correspondence between an opcode index and the error message associated with it. pub(crate) assertion_payloads: BTreeMap>, + /// Correspondence between error selectors and types associated with them. + pub(crate) error_types: BTreeMap, + pub(crate) warnings: Vec, /// Name for the corresponding entry point represented by this Acir-gen output. @@ -576,15 +582,8 @@ impl GeneratedAcir { return; } - // TODO(https://github.com/noir-lang/noir/issues/5792) - for (brillig_index, message) in generated_brillig.assert_messages.iter() { - self.assertion_payloads.insert( - OpcodeLocation::Brillig { - acir_index: self.opcodes.len() - 1, - brillig_index: *brillig_index, - }, - AssertionPayload::StaticString(message.clone()), - ); + for (error_selector, error_type) in generated_brillig.error_types.iter() { + self.record_error_type(*error_selector, error_type.clone()); } if inserted_func_before { @@ -619,6 +618,20 @@ impl GeneratedAcir { pub(crate) fn last_acir_opcode_location(&self) -> OpcodeLocation { OpcodeLocation::Acir(self.opcodes.len() - 1) } + + pub(crate) fn record_error_type(&mut self, selector: ErrorSelector, typ: ErrorType) { + self.error_types.insert(selector, typ); + } + + pub(crate) fn generate_assertion_message_payload( + &mut self, + message: String, + ) -> AssertionPayload { + let error_type = ErrorType::String(message); + let error_selector = error_selector_from_type(&error_type); + self.record_error_type(error_selector, error_type); + AssertionPayload { error_selector: error_selector.as_u64(), payload: Vec::new() } + } } /// This function will return the number of inputs that a blackbox function diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index 50a895df2371..e93af6b2ce05 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -9,7 +9,7 @@ use self::acir_ir::generated_acir::BrilligStdlibFunc; use super::function_builder::data_bus::DataBus; use super::ir::dfg::CallStack; use super::ir::function::FunctionId; -use super::ir::instruction::{ConstrainError, ErrorType}; +use super::ir::instruction::ConstrainError; use super::ir::printer::try_to_extract_string_from_error_payload; use super::{ ir::{ @@ -40,6 +40,7 @@ use acvm::{acir::circuit::opcodes::BlockId, acir::AcirField, FieldElement}; use fxhash::FxHashMap as HashMap; use im::Vector; use iter_extended::{try_vecmap, vecmap}; +use noirc_frontend::Type as HirType; #[derive(Default)] struct SharedContext { @@ -283,7 +284,7 @@ pub(crate) type Artifacts = ( Vec>, Vec>, Vec, - BTreeMap, + BTreeMap, ); impl Ssa { @@ -296,6 +297,7 @@ impl Ssa { let mut acirs = Vec::new(); // TODO: can we parallelize this? let mut shared_context = SharedContext::default(); + for function in self.functions.values() { let context = Context::new(&mut shared_context, expression_width); if let Some(mut generated_acir) = @@ -674,17 +676,18 @@ impl<'a> Context<'a> { let rhs = self.convert_numeric_value(*rhs, dfg)?; let assert_payload = if let Some(error) = assert_message { - match error { - ConstrainError::StaticString(string) => { - Some(AssertionPayload::StaticString(string.clone())) + Some(match error { + ConstrainError::StaticString(_, string) => { + self.acir_context.generate_assertion_message_payload(string.clone()) } - ConstrainError::Dynamic(error_selector, values) => { + ConstrainError::Dynamic(error_selector, is_string_type, values) => { if let Some(constant_string) = try_to_extract_string_from_error_payload( - *error_selector, + *is_string_type, values, dfg, ) { - Some(AssertionPayload::StaticString(constant_string)) + self.acir_context + .generate_assertion_message_payload(constant_string) } else { let acir_vars: Vec<_> = values .iter() @@ -694,13 +697,13 @@ impl<'a> Context<'a> { let expressions_or_memory = self.acir_context.vars_to_expressions_or_memory(&acir_vars)?; - Some(AssertionPayload::Dynamic( - error_selector.as_u64(), - expressions_or_memory, - )) + AssertionPayload { + error_selector: error_selector.as_u64(), + payload: expressions_or_memory, + } } } - } + }) } else { None }; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs index f810b65d105e..7b39f7041e45 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/function_builder/mod.rs @@ -4,6 +4,7 @@ use std::{borrow::Cow, collections::BTreeMap, sync::Arc}; use acvm::{acir::circuit::ErrorSelector, FieldElement}; use noirc_errors::Location; +use noirc_frontend::hir_def::types::Type as HirType; use noirc_frontend::monomorphization::ast::InlineType; use crate::ssa::ir::{ @@ -19,7 +20,7 @@ use super::{ basic_block::BasicBlock, dfg::{CallStack, InsertInstructionResult}, function::RuntimeType, - instruction::{ConstrainError, ErrorType, InstructionId, Intrinsic}, + instruction::{ConstrainError, InstructionId, Intrinsic}, }, ssa_gen::Ssa, }; @@ -36,7 +37,7 @@ pub(crate) struct FunctionBuilder { current_block: BasicBlockId, finished_functions: Vec, call_stack: CallStack, - error_types: BTreeMap, + error_types: BTreeMap, } impl FunctionBuilder { @@ -461,7 +462,7 @@ impl FunctionBuilder { } } - pub(crate) fn record_error_type(&mut self, selector: ErrorSelector, typ: ErrorType) { + pub(crate) fn record_error_type(&mut self, selector: ErrorSelector, typ: HirType) { self.error_types.insert(selector, typ); } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index f187a279b9bb..27816e70087e 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -4,10 +4,7 @@ use std::hash::{Hash, Hasher}; use acvm::{ acir::AcirField, - acir::{ - circuit::{ErrorSelector, STRING_ERROR_SELECTOR}, - BlackBoxFunc, - }, + acir::{circuit::ErrorSelector, BlackBoxFunc}, FieldElement, }; use fxhash::FxHasher; @@ -473,10 +470,13 @@ impl Instruction { let lhs = f(*lhs); let rhs = f(*rhs); let assert_message = assert_message.as_ref().map(|error| match error { - ConstrainError::Dynamic(selector, payload_values) => ConstrainError::Dynamic( - *selector, - payload_values.iter().map(|&value| f(value)).collect(), - ), + ConstrainError::Dynamic(selector, is_string, payload_values) => { + ConstrainError::Dynamic( + *selector, + *is_string, + payload_values.iter().map(|&value| f(value)).collect(), + ) + } _ => error.clone(), }); Instruction::Constrain(lhs, rhs, assert_message) @@ -544,7 +544,7 @@ impl Instruction { Instruction::Constrain(lhs, rhs, assert_error) => { f(*lhs); f(*rhs); - if let Some(ConstrainError::Dynamic(_, values)) = assert_error.as_ref() { + if let Some(ConstrainError::Dynamic(_, _, values)) = assert_error.as_ref() { values.iter().for_each(|&val| { f(val); }); @@ -915,32 +915,32 @@ fn try_optimize_array_set_from_previous_get( SimplifyResult::None } -pub(crate) type ErrorType = HirType; +#[derive(Debug, PartialEq, Eq, Hash, Clone)] +pub enum ErrorType { + String(String), + Dynamic(HirType), +} pub(crate) fn error_selector_from_type(typ: &ErrorType) -> ErrorSelector { - match typ { - ErrorType::String(_) => STRING_ERROR_SELECTOR, - _ => { - let mut hasher = FxHasher::default(); - typ.hash(&mut hasher); - let hash = hasher.finish(); - assert!(hash != 0, "ICE: Error type {} collides with the string error type", typ); - ErrorSelector::new(hash) - } - } + let mut hasher = FxHasher::default(); + typ.hash(&mut hasher); + let hash = hasher.finish(); + ErrorSelector::new(hash) } #[derive(Debug, PartialEq, Eq, Hash, Clone, Serialize, Deserialize)] pub(crate) enum ConstrainError { // Static string errors are not handled inside the program as data for efficiency reasons. - StaticString(String), + StaticString(ErrorSelector, String), // These errors are handled by the program as data. - Dynamic(ErrorSelector, Vec), + // We use a boolean to indicate if the error is a string for printing purposes. + Dynamic(ErrorSelector, /* is_string */ bool, Vec), } impl From for ConstrainError { fn from(value: String) -> Self { - ConstrainError::StaticString(value) + let error_type = ErrorType::String(value.clone()); + ConstrainError::StaticString(error_selector_from_type(&error_type), value) } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 2b564c14aa7c..7296ab5a41eb 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -4,7 +4,6 @@ use std::{ fmt::{Formatter, Result}, }; -use acvm::acir::circuit::{ErrorSelector, STRING_ERROR_SELECTOR}; use acvm::acir::AcirField; use iter_extended::vecmap; @@ -213,11 +212,11 @@ fn display_instruction_inner( /// Tries to extract a constant string from an error payload. pub(crate) fn try_to_extract_string_from_error_payload( - error_selector: ErrorSelector, + is_string_type: bool, values: &[ValueId], dfg: &DataFlowGraph, ) -> Option { - ((error_selector == STRING_ERROR_SELECTOR) && (values.len() == 1)) + (is_string_type && (values.len() == 1)) .then_some(()) .and_then(|()| { let Value::Array { array: values, .. } = &dfg[values[0]] else { @@ -245,12 +244,12 @@ fn display_constrain_error( f: &mut Formatter, ) -> Result { match error { - ConstrainError::StaticString(assert_message_string) => { + ConstrainError::StaticString(_, assert_message_string) => { writeln!(f, " '{assert_message_string:?}'") } - ConstrainError::Dynamic(selector, values) => { + ConstrainError::Dynamic(_, is_string, values) => { if let Some(constant_string) = - try_to_extract_string_from_error_payload(*selector, values, &function.dfg) + try_to_extract_string_from_error_payload(*is_string, values, &function.dfg) { writeln!(f, " '{}'", constant_string) } else { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 8bf3a740b3a7..65ffa2f20dca 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -21,7 +21,7 @@ use self::{ value::{Tree, Values}, }; -use super::ir::instruction::error_selector_from_type; +use super::ir::instruction::{error_selector_from_type, ErrorType}; use super::{ function_builder::data_bus::DataBus, ir::{ @@ -701,24 +701,29 @@ impl<'a> FunctionContext<'a> { assert_message: &Option>, ) -> Result, 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); + Ok(Some( + if let Expression::Literal(ast::Literal::Str(static_string)) = assert_message_expression + { + let error_type = ErrorType::String(static_string.clone()); + let selector = error_selector_from_type(&error_type); + ConstrainError::StaticString(selector, static_string.clone()) + } else { + let error_type = ErrorType::Dynamic(assert_message_typ.clone()); + let selector = error_selector_from_type(&error_type); + let values = + self.codegen_expression(assert_message_expression)?.into_value_list(self); + let is_string_type = matches!(assert_message_typ, HirType::String(_)); + // Record custom types in the builder, outside of SSA instructions + // This is made to avoid having Hir types in the SSA code. + if !is_string_type { + self.builder.record_error_type(selector, assert_message_typ.clone()); + } - let error_type_id = error_selector_from_type(assert_message_typ); - // Do not record string errors in the ABI - match assert_message_typ { - HirType::String(_) => {} - _ => { - self.builder.record_error_type(error_type_id, assert_message_typ.clone()); - } - }; - Ok(Some(ConstrainError::Dynamic(error_type_id, values))) + ConstrainError::Dynamic(selector, is_string_type, values) + }, + )) } fn codegen_assign(&mut self, assign: &ast::Assign) -> Result { diff --git a/noir/noir-repo/tooling/noir_js_types/src/types.ts b/noir/noir-repo/tooling/noir_js_types/src/types.ts index dd6548ddb6c7..12c4607962fa 100644 --- a/noir/noir-repo/tooling/noir_js_types/src/types.ts +++ b/noir/noir-repo/tooling/noir_js_types/src/types.ts @@ -20,6 +20,7 @@ export type AbiParameter = { }; export type AbiErrorType = + | { error_kind: 'string'; string: string } | { error_kind: 'fmtstring'; length: number; diff --git a/noir/noir-repo/tooling/noirc_abi/src/lib.rs b/noir/noir-repo/tooling/noirc_abi/src/lib.rs index 39f2e935618a..b1b199727c2a 100644 --- a/noir/noir-repo/tooling/noirc_abi/src/lib.rs +++ b/noir/noir-repo/tooling/noirc_abi/src/lib.rs @@ -464,6 +464,7 @@ pub enum AbiValue { pub enum AbiErrorType { FmtString { length: u32, item_types: Vec }, Custom(AbiType), + String { string: String }, } pub fn display_abi_error( @@ -491,6 +492,13 @@ pub fn display_abi_error( let decoded = printable_type_decode_value(&mut fields.iter().copied(), &printable_type); PrintableValueDisplay::Plain(decoded, printable_type) } + AbiErrorType::String { string } => { + let length = string.len() as u32; + PrintableValueDisplay::Plain( + PrintableValue::String(string), + PrintableType::String { length }, + ) + } } } diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/src/lib.rs b/noir/noir-repo/tooling/noirc_abi_wasm/src/lib.rs index b0c11979d176..79aecafa7426 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/src/lib.rs +++ b/noir/noir-repo/tooling/noirc_abi_wasm/src/lib.rs @@ -157,5 +157,6 @@ pub fn abi_decode_error( ::from_serde(&json_types) .map_err(|err| err.to_string().into()) } + AbiErrorType::String { string } => Ok(JsValue::from_str(&string)), } } diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/decode_error.ts b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/decode_error.ts index 662e0acb4169..461cedd26941 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/decode_error.ts +++ b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/decode_error.ts @@ -4,8 +4,10 @@ export const FAKE_FIELD_SELECTOR = '1'; export const FAKE_TUPLE_SELECTOR = '2'; export const FAKE_FMT_STRING_SELECTOR = '3'; export const FAKE_STRUCT_SELECTOR = '4'; +export const FAKE_STRING_SELECTOR = '5'; export const SAMPLE_FMT_STRING = 'hello {a}'; +export const SAMPLE_STRING = 'hello world'; export const abi: Abi = { parameters: [ @@ -40,5 +42,9 @@ export const abi: Abi = { { name: 'b', type: { kind: 'field' } }, ], }, + [FAKE_STRING_SELECTOR]: { + error_kind: 'string', + string: SAMPLE_STRING, + }, }, }; diff --git a/yarn-project/aztec.js/src/contract/contract.test.ts b/yarn-project/aztec.js/src/contract/contract.test.ts index a9fcd9e5dccc..751571bc537b 100644 --- a/yarn-project/aztec.js/src/contract/contract.test.ts +++ b/yarn-project/aztec.js/src/contract/contract.test.ts @@ -87,6 +87,7 @@ describe('Contract Class', () => { }, ], returnTypes: [], + errorTypes: {}, bytecode: Buffer.alloc(8, 0xfa), }, { @@ -97,6 +98,7 @@ describe('Contract Class', () => { isInternal: false, parameters: [], returnTypes: [], + errorTypes: {}, bytecode: Buffer.alloc(8, 0xfb), debugSymbols: '', }, @@ -124,6 +126,7 @@ describe('Contract Class', () => { ], bytecode: Buffer.alloc(8, 0xfc), debugSymbols: '', + errorTypes: {}, }, ], outputs: { diff --git a/yarn-project/aztec.js/src/entrypoint/default_multi_call_entrypoint.ts b/yarn-project/aztec.js/src/entrypoint/default_multi_call_entrypoint.ts index c3825e846faf..c4c3a653b733 100644 --- a/yarn-project/aztec.js/src/entrypoint/default_multi_call_entrypoint.ts +++ b/yarn-project/aztec.js/src/entrypoint/default_multi_call_entrypoint.ts @@ -86,6 +86,7 @@ export class DefaultMultiCallEntrypoint implements EntrypointInterface { }, ], returnTypes: [], + errorTypes: {}, } as FunctionAbi; } } diff --git a/yarn-project/aztec.js/src/wallet/account_wallet.ts b/yarn-project/aztec.js/src/wallet/account_wallet.ts index 84a21a5f5734..1a891160850c 100644 --- a/yarn-project/aztec.js/src/wallet/account_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/account_wallet.ts @@ -191,6 +191,7 @@ export class AccountWallet extends BaseWallet { }, ], returnTypes: [], + errorTypes: {}, }; } @@ -203,6 +204,7 @@ export class AccountWallet extends BaseWallet { isStatic: false, parameters: [{ name: 'message_hash', type: { kind: 'field' }, visibility: 'private' as ABIParameterVisibility }], returnTypes: [{ kind: 'boolean' }], + errorTypes: {}, }; } @@ -226,6 +228,7 @@ export class AccountWallet extends BaseWallet { { name: 'message_hash', type: { kind: 'field' }, visibility: 'private' as ABIParameterVisibility }, ], returnTypes: [{ kind: 'boolean' }], + errorTypes: {}, }; } } diff --git a/yarn-project/circuits.js/src/contract/contract_address.test.ts b/yarn-project/circuits.js/src/contract/contract_address.test.ts index a23519948ef6..c8918ea0d3e1 100644 --- a/yarn-project/circuits.js/src/contract/contract_address.test.ts +++ b/yarn-project/circuits.js/src/contract/contract_address.test.ts @@ -40,6 +40,7 @@ describe('ContractAddress', () => { name: 'fun', parameters: [{ name: 'param1', type: { kind: 'boolean' }, visibility: 'private' }], returnTypes: [], + errorTypes: {}, }; const mockArgs: any[] = [true]; const result = computeInitializationHash(mockInitFn, mockArgs); diff --git a/yarn-project/entrypoints/src/account_entrypoint.ts b/yarn-project/entrypoints/src/account_entrypoint.ts index 497955901ac1..08a14fc07464 100644 --- a/yarn-project/entrypoints/src/account_entrypoint.ts +++ b/yarn-project/entrypoints/src/account_entrypoint.ts @@ -146,6 +146,7 @@ export class DefaultAccountEntrypoint implements EntrypointInterface { { name: 'cancellable', type: { kind: 'boolean' } }, ], returnTypes: [], + errorTypes: {}, } as FunctionAbi; } } diff --git a/yarn-project/entrypoints/src/dapp_entrypoint.ts b/yarn-project/entrypoints/src/dapp_entrypoint.ts index e05a1ec3148c..65ac61f55c27 100644 --- a/yarn-project/entrypoints/src/dapp_entrypoint.ts +++ b/yarn-project/entrypoints/src/dapp_entrypoint.ts @@ -119,6 +119,7 @@ export class DefaultDappEntrypoint implements EntrypointInterface { }, ], returnTypes: [], + errorTypes: {}, } as FunctionAbi; } } diff --git a/yarn-project/foundation/src/abi/abi.ts b/yarn-project/foundation/src/abi/abi.ts index 160be402bff1..5c721d1baa67 100644 --- a/yarn-project/foundation/src/abi/abi.ts +++ b/yarn-project/foundation/src/abi/abi.ts @@ -4,6 +4,18 @@ import { type Fr } from '../fields/fields.js'; import { type FunctionSelector } from './function_selector.js'; import { type NoteSelector } from './note_selector.js'; +/** + * An error could be a custom error of any regular type or a string error. + */ +export type AbiErrorType = + | { error_kind: 'string'; string: string } + | { + error_kind: 'fmtstring'; + length: number; + item_types: AbiType[]; + } + | ({ error_kind: 'custom' } & AbiType); + /** * A basic value. */ @@ -194,6 +206,8 @@ export interface FunctionAbi { * The types of the return values. */ returnTypes: AbiType[]; + + errorTypes: Partial>; /** * Whether the function is flagged as an initializer. */ @@ -210,10 +224,6 @@ 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; /** Debug metadata for the function. */ debug?: FunctionDebugMetadata; } @@ -390,7 +400,7 @@ export interface FunctionDebugMetadata { /** * Public functions store their static assertion messages externally to the bytecode. */ - assertMessages?: Record; + errorTypes: Partial>; } /** @@ -432,7 +442,7 @@ export function getFunctionDebugMetadata( return { debugSymbols: programDebugSymbols.debug_infos[0], files: contractArtifact.fileMap, - assertMessages: functionArtifact.assertMessages, + errorTypes: functionArtifact.errorTypes, }; } return undefined; diff --git a/yarn-project/foundation/src/abi/encoder.test.ts b/yarn-project/foundation/src/abi/encoder.test.ts index 1053db02af09..f0fd1a4e72ef 100644 --- a/yarn-project/foundation/src/abi/encoder.test.ts +++ b/yarn-project/foundation/src/abi/encoder.test.ts @@ -22,6 +22,7 @@ describe('abi/encoder', () => { }, ], returnTypes: [], + errorTypes: {}, }; const field = Fr.random(); @@ -47,6 +48,7 @@ describe('abi/encoder', () => { }, ], returnTypes: [], + errorTypes: {}, }; const arr = [Fr.random(), Fr.random()]; @@ -71,6 +73,7 @@ describe('abi/encoder', () => { }, ], returnTypes: [], + errorTypes: {}, }; const str = 'abc'; @@ -103,6 +106,7 @@ describe('abi/encoder', () => { }, ], returnTypes: [], + errorTypes: {}, }; const address = AztecAddress.random(); @@ -139,6 +143,7 @@ describe('abi/encoder', () => { }, ], returnTypes: [], + errorTypes: {}, }; const value = Fr.random(); @@ -164,6 +169,7 @@ describe('abi/encoder', () => { }, ], returnTypes: [], + errorTypes: {}, }; const args = ['garbage']; @@ -189,6 +195,7 @@ describe('abi/encoder', () => { }, ], returnTypes: [], + errorTypes: {}, }; const args = ['garbage']; expect(() => encodeArguments(testFunctionAbi, args)).toThrow(`Cannot convert garbage to a BigInt`); @@ -211,6 +218,7 @@ describe('abi/encoder', () => { }, ], returnTypes: [], + errorTypes: {}, }; const args = [ { diff --git a/yarn-project/protocol-contracts/src/protocol_contract_data.ts b/yarn-project/protocol-contracts/src/protocol_contract_data.ts index 11810f96b373..c82f5423180f 100644 --- a/yarn-project/protocol-contracts/src/protocol_contract_data.ts +++ b/yarn-project/protocol-contracts/src/protocol_contract_data.ts @@ -50,14 +50,14 @@ export const ProtocolContractAddress: Record }; export const ProtocolContractLeaf = { - AuthRegistry: Fr.fromString('0x305c48d9d087f24b1618a7cc92c5081db4672f6855816af7ac89ea7e873245cd'), + AuthRegistry: Fr.fromString('0x2e3f5370adceb5d29e33bcf8764f5e3bcf7f5494ad4a53d15fc2b1b96fcda706'), ContractInstanceDeployer: Fr.fromString('0x04a661c9d4d295fc485a7e0f3de40c09b35366343bce8ad229106a8ef4076fe5'), ContractClassRegisterer: Fr.fromString('0x147ba3294403576dbad10f86d3ffd4eb83fb230ffbcd5c8b153dd02942d0611f'), MultiCallEntrypoint: Fr.fromString('0x154b701b41d6cf6da7204fef36b2ee9578b449d21b3792a9287bf45eba48fd26'), - FeeJuice: Fr.fromString('0x21c9ab2e339c9b3394e4e1ff3b7cf37be4e0fc0bc177a192d287d98963b9b254'), - Router: Fr.fromString('0x2779d7e4ccba389da358a0c9362364d0c65e14cd4d9df929e6722187e808e068'), + FeeJuice: Fr.fromString('0x263a408b759d0b798b839c346bd7e370435053a69ed2b43250e35fcc3b79f474'), + Router: Fr.fromString('0x0834dc2118971185cf364414bbc260cb53d8e86fda6e65cc7bed4c80e55b1918'), }; export const protocolContractTreeRoot = Fr.fromString( - '0x00724e4de088411c873c3d6975491eb48889bfa51bc854744a4fcc307ee9abd8', + '0x10838cb488729bcf81b8162a899430a917a9cf89145f4be95ef08bfb093ef786', ); diff --git a/yarn-project/pxe/src/pxe_service/error_enriching.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts index 6f40630e35d8..052fb384128a 100644 --- a/yarn-project/pxe/src/pxe_service/error_enriching.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -71,10 +71,10 @@ export async function enrichPublicSimulationError( const noirCallStack = err.getNoirCallStack(); if (debugInfo) { if (isNoirCallStackUnresolved(noirCallStack)) { - const assertionMessage = resolveAssertionMessage(noirCallStack, debugInfo); - if (assertionMessage) { - err.setOriginalMessage(err.getOriginalMessage() + `: ${assertionMessage}`); - } + // const assertionMessage = resolveAssertionMessage(noirCallStack, debugInfo); + // if (assertionMessage) { + // err.setOriginalMessage(err.getOriginalMessage() + `: ${assertionMessage}`); + // } try { // Public functions are simulated as a single Brillig entry point. // Thus, we can safely assume here that the Brillig function id is `0`. diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts index ba7b3aa05366..e1406be2ea60 100644 --- a/yarn-project/simulator/src/acvm/acvm.ts +++ b/yarn-project/simulator/src/acvm/acvm.ts @@ -103,27 +103,27 @@ export function resolveOpcodeLocations( ); } -/** - * Extracts the source code locations for an array of opcode locations - * @param opcodeLocations - The opcode locations that caused the error. - * @param debug - The debug metadata of the function. - * @returns The source code locations. - */ -export function resolveAssertionMessage( - opcodeLocations: OpcodeLocation[], - debug: FunctionDebugMetadata, -): string | undefined { - if (opcodeLocations.length === 0) { - return undefined; - } - - const lastLocation = extractBrilligLocation(opcodeLocations[opcodeLocations.length - 1]); - if (!lastLocation) { - return undefined; - } - - return debug.assertMessages?.[parseInt(lastLocation, 10)]; -} +// /** +// * Extracts the source code locations for an array of opcode locations +// * @param opcodeLocations - The opcode locations that caused the error. +// * @param debug - The debug metadata of the function. +// * @returns The source code locations. +// */ +// export function resolveAssertionMessage( +// opcodeLocations: OpcodeLocation[], +// debug: FunctionDebugMetadata, +// ): string | undefined { +// if (opcodeLocations.length === 0) { +// return undefined; +// } + +// const lastLocation = extractBrilligLocation(opcodeLocations[opcodeLocations.length - 1]); +// if (!lastLocation) { +// return undefined; +// } + +// return debug.assertMessages?.[parseInt(lastLocation, 10)]; +// } /** * The function call that executes an ACIR. diff --git a/yarn-project/types/src/abi/contract_artifact.ts b/yarn-project/types/src/abi/contract_artifact.ts index a474920eddf3..73896129f2f7 100644 --- a/yarn-project/types/src/abi/contract_artifact.ts +++ b/yarn-project/types/src/abi/contract_artifact.ts @@ -182,7 +182,7 @@ function generateFunctionArtifact(fn: NoirCompiledContractFunction, contract: No bytecode: Buffer.from(fn.bytecode, 'base64'), verificationKey: mockVerificationKey, debugSymbols: fn.debug_symbols, - assertMessages: fn.assert_messages, + errorTypes: fn.abi.error_types, }; } diff --git a/yarn-project/types/src/noir/index.ts b/yarn-project/types/src/noir/index.ts index ca8d028bc228..bfe1519977af 100644 --- a/yarn-project/types/src/noir/index.ts +++ b/yarn-project/types/src/noir/index.ts @@ -1,6 +1,7 @@ import { type ABIParameter, type ABIParameterVisibility, + type AbiErrorType, type AbiType, type AbiValue, type DebugFileMap, @@ -13,17 +14,6 @@ export const AZTEC_INTERNAL_ATTRIBUTE = 'internal'; export const AZTEC_INITIALIZER_ATTRIBUTE = 'initializer'; export const AZTEC_VIEW_ATTRIBUTE = 'view'; -/** - * An error could be a custom error of any regular type or a formatted string error. - */ -export type AbiErrorType = - | { - error_kind: 'fmtstring'; - length: number; - item_types: AbiType[]; - } - | ({ error_kind: 'custom' } & AbiType); - /** The ABI of an Aztec.nr function. */ export interface NoirFunctionAbi { /** The parameters of the function. */ From eee0ce1687db3bc28d1ed5b154987d382a21f7e9 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Tue, 29 Oct 2024 17:17:25 +0000 Subject: [PATCH 02/16] use wasm to decode errors --- .../bb-prover/src/avm_proving.test.ts | 4 +- .../src/avm_integration.test.ts | 4 +- yarn-project/simulator/src/acvm/acvm.ts | 51 +++++++++++-------- .../simulator/src/avm/avm_simulator.test.ts | 24 ++++----- .../simulator/src/avm/fixtures/index.ts | 15 +++++- .../simulator/src/client/private_execution.ts | 4 +- .../src/client/unconstrained_execution.ts | 4 +- yarn-project/txe/src/oracle/txe_oracle.ts | 3 +- 8 files changed, 68 insertions(+), 41 deletions(-) diff --git a/yarn-project/bb-prover/src/avm_proving.test.ts b/yarn-project/bb-prover/src/avm_proving.test.ts index 935a6f448fc2..69b9d4f96e7d 100644 --- a/yarn-project/bb-prover/src/avm_proving.test.ts +++ b/yarn-project/bb-prover/src/avm_proving.test.ts @@ -131,7 +131,9 @@ const proveAndVerifyAvmTestContract = async ( // Explicit revert when an assertion failed. expect(avmResult.reverted).toBe(true); expect(avmResult.revertReason).toBeDefined(); - expect(resolveAvmTestContractAssertionMessage(functionName, avmResult.revertReason!)).toContain(assertionErrString); + expect(resolveAvmTestContractAssertionMessage(functionName, avmResult.revertReason!, avmResult.output)).toContain( + assertionErrString, + ); } const pxResult = trace.toPublicExecutionResult( diff --git a/yarn-project/ivc-integration/src/avm_integration.test.ts b/yarn-project/ivc-integration/src/avm_integration.test.ts index e34049159728..8e045b1f99b2 100644 --- a/yarn-project/ivc-integration/src/avm_integration.test.ts +++ b/yarn-project/ivc-integration/src/avm_integration.test.ts @@ -221,7 +221,9 @@ const proveAvmTestContract = async ( // Explicit revert when an assertion failed. expect(avmResult.reverted).toBe(true); expect(avmResult.revertReason).toBeDefined(); - expect(resolveAvmTestContractAssertionMessage(functionName, avmResult.revertReason!)).toContain(assertionErrString); + expect(resolveAvmTestContractAssertionMessage(functionName, avmResult.revertReason!, avmResult.output)).toContain( + assertionErrString, + ); } const pxResult = trace.toPublicExecutionResult( diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts index e1406be2ea60..e3a4767a3c13 100644 --- a/yarn-project/simulator/src/acvm/acvm.ts +++ b/yarn-project/simulator/src/acvm/acvm.ts @@ -6,8 +6,10 @@ import { type ExecutionError, type ForeignCallInput, type ForeignCallOutput, + type RawAssertionPayload, executeCircuitWithReturnWitness, } from '@noir-lang/acvm_js'; +import { abiDecodeError } from '@noir-lang/noirc_abi'; import { traverseCauseChain } from '../common/errors.js'; import { type ACVMWitness } from './acvm_types.js'; @@ -103,27 +105,34 @@ export function resolveOpcodeLocations( ); } -// /** -// * Extracts the source code locations for an array of opcode locations -// * @param opcodeLocations - The opcode locations that caused the error. -// * @param debug - The debug metadata of the function. -// * @returns The source code locations. -// */ -// export function resolveAssertionMessage( -// opcodeLocations: OpcodeLocation[], -// debug: FunctionDebugMetadata, -// ): string | undefined { -// if (opcodeLocations.length === 0) { -// return undefined; -// } - -// const lastLocation = extractBrilligLocation(opcodeLocations[opcodeLocations.length - 1]); -// if (!lastLocation) { -// return undefined; -// } - -// return debug.assertMessages?.[parseInt(lastLocation, 10)]; -// } +export function resolveAssertionMessage( + errorPayload: RawAssertionPayload, + debug: FunctionDebugMetadata, +): string | undefined { + const decoded = abiDecodeError( + { parameters: [], error_types: debug.errorTypes }, // eslint-disable-line camelcase + errorPayload, + ); + + console.log(decoded); + + if (typeof decoded === 'string') { + return decoded; + } else { + return JSON.stringify(decoded); + } +} + +export function resolveAssertionMessageFromError(err: Error, debug?: FunctionDebugMetadata): string { + if (debug && typeof err === 'object' && err !== null && 'rawAssertionPayload' in err && err.rawAssertionPayload) { + return `Circuit execution failed: ${resolveAssertionMessage( + err.rawAssertionPayload as RawAssertionPayload, + debug, + )}`; + } else { + return err.message; + } +} /** * The function call that executes an ACIR. diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 613c7bf68a07..02319b805696 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -287,9 +287,9 @@ describe('AVM simulator: transpiled Noir contracts', () => { const results = await new AvmSimulator(initContext()).executeBytecode(bytecode); expect(results.reverted).toBe(true); expect(results.revertReason).toBeDefined(); - expect(resolveAvmTestContractAssertionMessage('u128_addition_overflow', results.revertReason!)).toMatch( - 'attempt to add with overflow', - ); + expect( + resolveAvmTestContractAssertionMessage('u128_addition_overflow', results.revertReason!, results.output), + ).toMatch('attempt to add with overflow'); }); it('Expect failure on U128::from_integer() overflow', async () => { @@ -297,9 +297,9 @@ describe('AVM simulator: transpiled Noir contracts', () => { const results = await new AvmSimulator(initContext()).executeBytecode(bytecode); expect(results.reverted).toBe(true); expect(results.revertReason).toBeDefined(); - expect(resolveAvmTestContractAssertionMessage('u128_from_integer_overflow', results.revertReason!)).toMatch( - 'call to assert_max_bit_size', - ); + expect( + resolveAvmTestContractAssertionMessage('u128_from_integer_overflow', results.revertReason!, results.output), + ).toMatch('call to assert_max_bit_size'); }); }); @@ -321,9 +321,9 @@ describe('AVM simulator: transpiled Noir contracts', () => { expect(results.reverted).toBe(true); expect(results.revertReason).toBeDefined(); - expect(resolveAvmTestContractAssertionMessage('assert_nullifier_exists', results.revertReason!)).toMatch( - "Nullifier doesn't exist!", - ); + expect( + resolveAvmTestContractAssertionMessage('assert_nullifier_exists', results.revertReason!, results.output), + ).toMatch("Nullifier doesn't exist!"); expect(results.output).toEqual([]); }); @@ -1011,9 +1011,9 @@ describe('AVM simulator: transpiled Noir contracts', () => { const results = await new AvmSimulator(context).executeBytecode(callBytecode); expect(results.reverted).toBe(true); // The outer call should revert. expect(results.revertReason).toBeDefined(); - expect(resolveAvmTestContractAssertionMessage('public_dispatch', results.revertReason!)).toMatch( - 'Values are not equal', - ); + expect( + resolveAvmTestContractAssertionMessage('public_dispatch', results.revertReason!, results.output), + ).toMatch('Values are not equal'); }); }); diff --git a/yarn-project/simulator/src/avm/fixtures/index.ts b/yarn-project/simulator/src/avm/fixtures/index.ts index 3b6495b52a6f..3ddc5b07ec76 100644 --- a/yarn-project/simulator/src/avm/fixtures/index.ts +++ b/yarn-project/simulator/src/avm/fixtures/index.ts @@ -136,6 +136,7 @@ export function getAvmTestContractBytecode(functionName: string): Buffer { export function resolveAvmTestContractAssertionMessage( functionName: string, revertReason: AvmRevertReason, + output: Fr[], ): string | undefined { const functionArtifact = AvmTestContractArtifact.functions.find(f => f.name === functionName)!; @@ -152,5 +153,17 @@ export function resolveAvmTestContractAssertionMessage( return undefined; } - return resolveAssertionMessage(revertReason.noirCallStack, debugMetadata); + if (output.length == 0) { + return undefined; + } + + const [errorSelector, ...errorData] = output; + + return resolveAssertionMessage( + { + selector: errorSelector.toBigInt().toString(), + data: errorData.map(f => f.toString()), + }, + debugMetadata, + ); } diff --git a/yarn-project/simulator/src/client/private_execution.ts b/yarn-project/simulator/src/client/private_execution.ts index 662cfd13222a..559621e780df 100644 --- a/yarn-project/simulator/src/client/private_execution.ts +++ b/yarn-project/simulator/src/client/private_execution.ts @@ -12,7 +12,7 @@ import { createDebugLogger } from '@aztec/foundation/log'; import { Timer } from '@aztec/foundation/timer'; import { fromACVMField, witnessMapToFields } from '../acvm/deserialize.js'; -import { type ACVMWitness, Oracle, acvm, extractCallStack } from '../acvm/index.js'; +import { type ACVMWitness, Oracle, acvm, extractCallStack, resolveAssertionMessageFromError } from '../acvm/index.js'; import { ExecutionError } from '../common/errors.js'; import { type ClientExecutionContext } from './client_execution_context.js'; @@ -34,7 +34,7 @@ export async function executePrivateFunction( const timer = new Timer(); const acirExecutionResult = await acvm(acir, initialWitness, acvmCallback).catch((err: Error) => { throw new ExecutionError( - err.message, + resolveAssertionMessageFromError(err, artifact.debug), { contractAddress, functionSelector, diff --git a/yarn-project/simulator/src/client/unconstrained_execution.ts b/yarn-project/simulator/src/client/unconstrained_execution.ts index 7fc622c6987f..1d223fba0b4c 100644 --- a/yarn-project/simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/simulator/src/client/unconstrained_execution.ts @@ -4,7 +4,7 @@ import { type Fr } from '@aztec/foundation/fields'; import { createDebugLogger } from '@aztec/foundation/log'; import { witnessMapToFields } from '../acvm/deserialize.js'; -import { Oracle, acvm, extractCallStack, toACVMWitness } from '../acvm/index.js'; +import { Oracle, acvm, extractCallStack, resolveAssertionMessageFromError, toACVMWitness } from '../acvm/index.js'; import { ExecutionError } from '../common/errors.js'; import { type ViewDataOracle } from './view_data_oracle.js'; @@ -26,7 +26,7 @@ export async function executeUnconstrainedFunction( const initialWitness = toACVMWitness(0, args); const acirExecutionResult = await acvm(acir, initialWitness, new Oracle(oracle)).catch((err: Error) => { throw new ExecutionError( - err.message, + resolveAssertionMessageFromError(err, artifact.debug), { contractAddress, functionSelector, diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 3823d6d940d4..933c9d3437fc 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -64,6 +64,7 @@ import { extractCallStack, extractPrivateCircuitPublicInputs, pickNotes, + resolveAssertionMessageFromError, toACVMWitness, witnessMapToFields, } from '@aztec/simulator'; @@ -530,7 +531,7 @@ export class TXE implements TypedOracle { try { const acirExecutionResult = await acvm(acir, initialWitness, acvmCallback).catch((err: Error) => { const execError = new ExecutionError( - err.message, + resolveAssertionMessageFromError(err, artifact.debug), { contractAddress: targetContractAddress, functionSelector, From 66e506f9b1f406c076dde6540f2a49c380516a25 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 30 Oct 2024 11:16:19 +0000 Subject: [PATCH 03/16] fix simulator tests --- .../protocol-contracts/src/protocol_contract_data.ts | 8 ++++---- yarn-project/simulator/src/acvm/acvm.ts | 7 +------ yarn-project/simulator/src/avm/avm_simulator.test.ts | 2 +- yarn-project/simulator/src/avm/avm_simulator.ts | 7 ++++++- yarn-project/simulator/src/avm/errors.ts | 11 ++++++++++- .../simulator/src/avm/opcodes/external_calls.ts | 6 +++++- .../simulator/src/client/private_execution.ts | 3 ++- .../simulator/src/client/unconstrained_execution.ts | 3 ++- yarn-project/txe/src/oracle/txe_oracle.ts | 4 +++- 9 files changed, 34 insertions(+), 17 deletions(-) diff --git a/yarn-project/protocol-contracts/src/protocol_contract_data.ts b/yarn-project/protocol-contracts/src/protocol_contract_data.ts index 8b1c4f4c09ab..c41976aefa8d 100644 --- a/yarn-project/protocol-contracts/src/protocol_contract_data.ts +++ b/yarn-project/protocol-contracts/src/protocol_contract_data.ts @@ -50,14 +50,14 @@ export const ProtocolContractAddress: Record }; export const ProtocolContractLeaf = { - AuthRegistry: Fr.fromString('0x13794ed6c957a68bc852fe4c2a161019a53011b08331d8eb0287483a7845d334'), + AuthRegistry: Fr.fromString('0x1e5520651fa82fedb5b88ea1fa3c581cd439dcdc76faf01c5725718f0c979bab'), ContractInstanceDeployer: Fr.fromString('0x04a661c9d4d295fc485a7e0f3de40c09b35366343bce8ad229106a8ef4076fe5'), ContractClassRegisterer: Fr.fromString('0x147ba3294403576dbad10f86d3ffd4eb83fb230ffbcd5c8b153dd02942d0611f'), MultiCallEntrypoint: Fr.fromString('0x154b701b41d6cf6da7204fef36b2ee9578b449d21b3792a9287bf45eba48fd26'), - FeeJuice: Fr.fromString('0x0191fe64a9d9efca55572a5190479698b8a3b296295f0f2d917b91fcb5486251'), - Router: Fr.fromString('0x19e9ec99aedfe3ea69ba91b862b815df7d1796fa802985a154159cd739fe4817'), + FeeJuice: Fr.fromString('0x0e80947a2a9ad9b19f758a6efc41201429d2e742c803997a7628fc32ef8f75e9'), + Router: Fr.fromString('0x0c45661bad250826b56afb5a1ea8aaf29ca4ef78a624403aff45c463b66d2187'), }; export const protocolContractTreeRoot = Fr.fromString( - '0x158c0b725f29c56278203f9d49c503c14bcf22888684ee73a4826e2edf2a56a8', + '0x08d81a87b084fcd5d4612c2ea4b77d39ad65a9968c2df44f45c5b58a38ae5ee2', ); diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts index e3a4767a3c13..e33919a1f901 100644 --- a/yarn-project/simulator/src/acvm/acvm.ts +++ b/yarn-project/simulator/src/acvm/acvm.ts @@ -114,8 +114,6 @@ export function resolveAssertionMessage( errorPayload, ); - console.log(decoded); - if (typeof decoded === 'string') { return decoded; } else { @@ -125,10 +123,7 @@ export function resolveAssertionMessage( export function resolveAssertionMessageFromError(err: Error, debug?: FunctionDebugMetadata): string { if (debug && typeof err === 'object' && err !== null && 'rawAssertionPayload' in err && err.rawAssertionPayload) { - return `Circuit execution failed: ${resolveAssertionMessage( - err.rawAssertionPayload as RawAssertionPayload, - debug, - )}`; + return `Assertion failed: ${resolveAssertionMessage(err.rawAssertionPayload as RawAssertionPayload, debug)}`; } else { return err.message; } diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 0a8e2b30a06d..0bb1fabce161 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -324,7 +324,7 @@ describe('AVM simulator: transpiled Noir contracts', () => { expect( resolveAvmTestContractAssertionMessage('assert_nullifier_exists', results.revertReason!, results.output), ).toMatch("Nullifier doesn't exist!"); - expect(results.output).toEqual([]); + expect(results.output).toHaveLength(1); // Error selector for static string error }); describe.each([ diff --git a/yarn-project/simulator/src/avm/avm_simulator.ts b/yarn-project/simulator/src/avm/avm_simulator.ts index 582912a91e63..de8478283efa 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.ts @@ -12,6 +12,7 @@ import { AvmExecutionError, InvalidProgramCounterError, NoBytecodeForContractError, + revertDataFromExceptionalHalt, revertReasonFromExceptionalHalt, revertReasonFromExplicitRevert, } from './errors.js'; @@ -137,7 +138,11 @@ export class AvmSimulator { const revertReason = revertReasonFromExceptionalHalt(err, this.context); // Note: "exceptional halts" cannot return data, hence [] - const results = new AvmContractCallResult(/*reverted=*/ true, /*output=*/ [], revertReason); + const results = new AvmContractCallResult( + /*reverted=*/ true, + /*output=*/ revertDataFromExceptionalHalt(err), + revertReason, + ); this.log.debug(`Context execution results: ${results.toString()}`); this.printOpcodeTallies(); diff --git a/yarn-project/simulator/src/avm/errors.ts b/yarn-project/simulator/src/avm/errors.ts index cafd6f92af00..9a311440f807 100644 --- a/yarn-project/simulator/src/avm/errors.ts +++ b/yarn-project/simulator/src/avm/errors.ts @@ -84,7 +84,7 @@ export class StaticCallAlterationError extends InstructionExecutionError { * @param nestedError - the revert reason of the nested call */ export class RethrownError extends AvmExecutionError { - constructor(message: string, public nestedError: AvmRevertReason) { + constructor(message: string, public nestedError: AvmRevertReason, public revertData: Fr[]) { super(message); this.name = 'RethrownError'; } @@ -141,6 +141,15 @@ export function revertReasonFromExceptionalHalt(haltingError: AvmExecutionError, return createRevertReason(haltingError.message, context, nestedError); } +/** + * Extracts revert data from an exceptional halt. Currently this is only used to manually bubble up revertadata. + * @param haltingError - the lower-level error causing the exceptional halt + * @returns the revert data for the exceptional halt + */ +export function revertDataFromExceptionalHalt(haltingError: AvmExecutionError): Fr[] { + return haltingError instanceof RethrownError ? haltingError.revertData : []; +} + /** * Create a "revert reason" error for an explicit revert (a root cause). * diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.ts index db6b658dade5..3a3aa66b6c8b 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -83,7 +83,11 @@ abstract class ExternalCall extends Instruction { throw new Error('A reverted nested call should be assigned a revert reason in the AVM execution loop'); } // The nested call's revertReason will be used to track the stack of error causes down to the root. - throw new RethrownError(nestedCallResults.revertReason.message, nestedCallResults.revertReason); + throw new RethrownError( + nestedCallResults.revertReason.message, + nestedCallResults.revertReason, + nestedCallResults.output, + ); } // Save return/revert data for later. diff --git a/yarn-project/simulator/src/client/private_execution.ts b/yarn-project/simulator/src/client/private_execution.ts index 559621e780df..8d2bb5349c21 100644 --- a/yarn-project/simulator/src/client/private_execution.ts +++ b/yarn-project/simulator/src/client/private_execution.ts @@ -33,8 +33,9 @@ export async function executePrivateFunction( const acvmCallback = new Oracle(context); const timer = new Timer(); const acirExecutionResult = await acvm(acir, initialWitness, acvmCallback).catch((err: Error) => { + err.message = resolveAssertionMessageFromError(err, artifact.debug); throw new ExecutionError( - resolveAssertionMessageFromError(err, artifact.debug), + err.message, { contractAddress, functionSelector, diff --git a/yarn-project/simulator/src/client/unconstrained_execution.ts b/yarn-project/simulator/src/client/unconstrained_execution.ts index 1d223fba0b4c..4da981bbcb57 100644 --- a/yarn-project/simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/simulator/src/client/unconstrained_execution.ts @@ -25,8 +25,9 @@ export async function executeUnconstrainedFunction( const acir = artifact.bytecode; const initialWitness = toACVMWitness(0, args); const acirExecutionResult = await acvm(acir, initialWitness, new Oracle(oracle)).catch((err: Error) => { + err.message = resolveAssertionMessageFromError(err, artifact.debug); throw new ExecutionError( - resolveAssertionMessageFromError(err, artifact.debug), + err.message, { contractAddress, functionSelector, diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index b8367badf9fe..893ab4a59c48 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -533,8 +533,10 @@ export class TXE implements TypedOracle { const timer = new Timer(); try { const acirExecutionResult = await acvm(acir, initialWitness, acvmCallback).catch((err: Error) => { + err.message = resolveAssertionMessageFromError(err, artifact.debug); + const execError = new ExecutionError( - resolveAssertionMessageFromError(err, artifact.debug), + err.message, { contractAddress: targetContractAddress, functionSelector, From 43e2dde667c1654b025e3541b538e2d529a7ac0c Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 30 Oct 2024 11:28:41 +0000 Subject: [PATCH 04/16] fixes --- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 36 +++++++++---------- .../tooling/noir_js_types/src/types.ts | 2 +- yarn-project/types/src/noir/index.ts | 2 +- 3 files changed, 18 insertions(+), 22 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 65ffa2f20dca..64cb8d0aa582 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -703,27 +703,23 @@ impl<'a> FunctionContext<'a> { let Some(assert_message_payload) = assert_message else { return Ok(None) }; let (assert_message_expression, assert_message_typ) = assert_message_payload.as_ref(); - Ok(Some( - if let Expression::Literal(ast::Literal::Str(static_string)) = assert_message_expression - { - let error_type = ErrorType::String(static_string.clone()); - let selector = error_selector_from_type(&error_type); - ConstrainError::StaticString(selector, static_string.clone()) - } else { - let error_type = ErrorType::Dynamic(assert_message_typ.clone()); - let selector = error_selector_from_type(&error_type); - let values = - self.codegen_expression(assert_message_expression)?.into_value_list(self); - let is_string_type = matches!(assert_message_typ, HirType::String(_)); - // Record custom types in the builder, outside of SSA instructions - // This is made to avoid having Hir types in the SSA code. - if !is_string_type { - self.builder.record_error_type(selector, assert_message_typ.clone()); - } + if let Expression::Literal(ast::Literal::Str(static_string)) = assert_message_expression { + let error_type = ErrorType::String(static_string.clone()); + let selector = error_selector_from_type(&error_type); + Ok(Some(ConstrainError::StaticString(selector, static_string.clone()))) + } else { + let error_type = ErrorType::Dynamic(assert_message_typ.clone()); + let selector = error_selector_from_type(&error_type); + let values = self.codegen_expression(assert_message_expression)?.into_value_list(self); + let is_string_type = matches!(assert_message_typ, HirType::String(_)); + // Record custom types in the builder, outside of SSA instructions + // This is made to avoid having Hir types in the SSA code. + if !is_string_type { + self.builder.record_error_type(selector, assert_message_typ.clone()); + } - ConstrainError::Dynamic(selector, is_string_type, values) - }, - )) + Ok(Some(ConstrainError::Dynamic(selector, is_string_type, values))) + } } fn codegen_assign(&mut self, assign: &ast::Assign) -> Result { diff --git a/noir/noir-repo/tooling/noir_js_types/src/types.ts b/noir/noir-repo/tooling/noir_js_types/src/types.ts index 12c4607962fa..03e2a93f8517 100644 --- a/noir/noir-repo/tooling/noir_js_types/src/types.ts +++ b/noir/noir-repo/tooling/noir_js_types/src/types.ts @@ -40,7 +40,7 @@ export type WitnessMap = Map; export type Abi = { parameters: AbiParameter[]; return_type: { abi_type: AbiType; visibility: Visibility } | null; - error_types: Record; + error_types: Partial>; }; export interface VerifierBackend { diff --git a/yarn-project/types/src/noir/index.ts b/yarn-project/types/src/noir/index.ts index bfe1519977af..bfe1d3959071 100644 --- a/yarn-project/types/src/noir/index.ts +++ b/yarn-project/types/src/noir/index.ts @@ -30,7 +30,7 @@ export interface NoirFunctionAbi { visibility: ABIParameterVisibility; }; /** Mapping of error selector => error type */ - error_types: Record; + error_types: Partial>; } /** From ba0c36110e1b3311e98f5c3ff57adb42347812fa Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 30 Oct 2024 13:31:01 +0000 Subject: [PATCH 05/16] fix type --- yarn-project/simulator/src/acvm/acvm.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts index e33919a1f901..5c0bdc1dea02 100644 --- a/yarn-project/simulator/src/acvm/acvm.ts +++ b/yarn-project/simulator/src/acvm/acvm.ts @@ -110,7 +110,7 @@ export function resolveAssertionMessage( debug: FunctionDebugMetadata, ): string | undefined { const decoded = abiDecodeError( - { parameters: [], error_types: debug.errorTypes }, // eslint-disable-line camelcase + { parameters: [], error_types: debug.errorTypes, return_type: null }, // eslint-disable-line camelcase errorPayload, ); From daeb27f4f80e248b404def738171cca7075afc44 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 30 Oct 2024 15:38:59 +0000 Subject: [PATCH 06/16] fix: recovering original errors --- .../noir-protocol-circuits/bootstrap.sh | 6 +++--- .../aztec-node/src/aztec-node/server.ts | 11 ++++------ .../src/protocol_contract_data.ts | 8 ++++---- .../pxe/src/pxe_service/error_enriching.ts | 11 +++++----- .../pxe/src/pxe_service/pxe_service.ts | 20 ++++++++++--------- yarn-project/simulator/src/acvm/acvm.ts | 20 +++++++++++++++++++ .../simulator/src/avm/fixtures/index.ts | 16 ++------------- .../src/public/enqueued_call_simulator.ts | 4 +++- .../src/public/enqueued_calls_processor.ts | 2 +- 9 files changed, 54 insertions(+), 44 deletions(-) diff --git a/noir-projects/noir-protocol-circuits/bootstrap.sh b/noir-projects/noir-protocol-circuits/bootstrap.sh index 8a4a33987d0b..349f0848ca8b 100755 --- a/noir-projects/noir-protocol-circuits/bootstrap.sh +++ b/noir-projects/noir-protocol-circuits/bootstrap.sh @@ -40,12 +40,12 @@ esac # This value may be too low. # If vk generation fail with an amount of free memory greater than this value then it should be increased. MIN_PARALLEL_VK_GENERATION_MEMORY=500000000 -PARALLEL_VK=${PARALLEL_VK:-true} +PARALLEL_VK=${PARALLEL_VK:-false} if [[ AVAILABLE_MEMORY -gt MIN_PARALLEL_VK_GENERATION_MEMORY ]] && [[ $PARALLEL_VK == "true" ]]; then echo "Generating vks in parallel..." for pathname in "./target"/*.json; do - if [[ $pathname != *"_simulated_"* ]]; then + if [[ $pathname != *"_simulated"* ]]; then BB_HASH=$BB_HASH node ../scripts/generate_vk_json.js "$pathname" "./target/keys" & fi done @@ -55,7 +55,7 @@ if [[ AVAILABLE_MEMORY -gt MIN_PARALLEL_VK_GENERATION_MEMORY ]] && [[ $PARALLEL_ done else - echo "System does not have enough memory for parallel vk generation, falling back to sequential" + echo "Generating VKs sequentially..." for pathname in "./target"/*.json; do BB_HASH=$BB_HASH node ../scripts/generate_vk_json.js "$pathname" "./target/keys" diff --git a/yarn-project/aztec-node/src/aztec-node/server.ts b/yarn-project/aztec-node/src/aztec-node/server.ts index a22e5ce6f8a6..cdbaa5271c9d 100644 --- a/yarn-project/aztec-node/src/aztec-node/server.ts +++ b/yarn-project/aztec-node/src/aztec-node/server.ts @@ -735,18 +735,15 @@ export class AztecNodeService implements AztecNode { // REFACTOR: Consider merging ProcessReturnValues into ProcessedTx const [processedTxs, failedTxs, returns] = await processor.process([tx]); - // REFACTOR: Consider returning the error/revert rather than throwing + // REFACTOR: Consider returning the error rather than throwing if (failedTxs.length) { this.log.warn(`Simulated tx ${tx.getTxHash()} fails: ${failedTxs[0].error}`); throw failedTxs[0].error; } - const { reverted } = partitionReverts(processedTxs); - if (reverted.length) { - this.log.warn(`Simulated tx ${tx.getTxHash()} reverts: ${reverted[0].revertReason}`); - throw reverted[0].revertReason; - } - this.log.debug(`Simulated tx ${tx.getTxHash()} succeeds`); + const [processedTx] = processedTxs; + this.log.debug(`Simulated tx ${tx.getTxHash()} ${processedTx.revertReason ? 'Reverts' : 'Succeeds'}`); + return new PublicSimulationOutput( processedTx.encryptedLogs, processedTx.unencryptedLogs, diff --git a/yarn-project/protocol-contracts/src/protocol_contract_data.ts b/yarn-project/protocol-contracts/src/protocol_contract_data.ts index c41976aefa8d..445ba7c543ea 100644 --- a/yarn-project/protocol-contracts/src/protocol_contract_data.ts +++ b/yarn-project/protocol-contracts/src/protocol_contract_data.ts @@ -50,14 +50,14 @@ export const ProtocolContractAddress: Record }; export const ProtocolContractLeaf = { - AuthRegistry: Fr.fromString('0x1e5520651fa82fedb5b88ea1fa3c581cd439dcdc76faf01c5725718f0c979bab'), + AuthRegistry: Fr.fromString('0x10775f6cb2902660200d33914e042e6a44bbc785b192b87caafa2c05ab0a1395'), ContractInstanceDeployer: Fr.fromString('0x04a661c9d4d295fc485a7e0f3de40c09b35366343bce8ad229106a8ef4076fe5'), ContractClassRegisterer: Fr.fromString('0x147ba3294403576dbad10f86d3ffd4eb83fb230ffbcd5c8b153dd02942d0611f'), MultiCallEntrypoint: Fr.fromString('0x154b701b41d6cf6da7204fef36b2ee9578b449d21b3792a9287bf45eba48fd26'), - FeeJuice: Fr.fromString('0x0e80947a2a9ad9b19f758a6efc41201429d2e742c803997a7628fc32ef8f75e9'), - Router: Fr.fromString('0x0c45661bad250826b56afb5a1ea8aaf29ca4ef78a624403aff45c463b66d2187'), + FeeJuice: Fr.fromString('0x0ba92f4271a84407885a54710c9da0e9e2a4a34f2fa270f6c4e96bb2119acebc'), + Router: Fr.fromString('0x2d521442e10fbda1f9c5b25532e46eafa2b3f91eb95dacc2df3c46d1eb6fb356'), }; export const protocolContractTreeRoot = Fr.fromString( - '0x08d81a87b084fcd5d4612c2ea4b77d39ad65a9968c2df44f45c5b58a38ae5ee2', + '0x0b8f3a8f75d2ac8538d45a4f9b042138cbeb47e435823bce0e653f1b98339fea', ); diff --git a/yarn-project/pxe/src/pxe_service/error_enriching.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts index 052fb384128a..b4894b1899ac 100644 --- a/yarn-project/pxe/src/pxe_service/error_enriching.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -1,7 +1,7 @@ import { type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; import { AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; import { type DebugLogger } from '@aztec/foundation/log'; -import { resolveAssertionMessage, resolveOpcodeLocations } from '@aztec/simulator'; +import { resolveAssertionMessageFromRevertData, resolveOpcodeLocations } from '@aztec/simulator'; import { type ContractDataOracle, type PxeDatabase } from '../index.js'; @@ -54,6 +54,7 @@ export async function enrichSimulationError(err: SimulationError, db: PxeDatabas export async function enrichPublicSimulationError( err: SimulationError, + revertData: Fr[], contractDataOracle: ContractDataOracle, db: PxeDatabase, logger: DebugLogger, @@ -70,11 +71,11 @@ export async function enrichPublicSimulationError( ); const noirCallStack = err.getNoirCallStack(); if (debugInfo) { + const assertionMessage = resolveAssertionMessageFromRevertData(revertData, debugInfo); + if (assertionMessage) { + err.setOriginalMessage(err.getOriginalMessage() + `${assertionMessage}`); + } if (isNoirCallStackUnresolved(noirCallStack)) { - // const assertionMessage = resolveAssertionMessage(noirCallStack, debugInfo); - // if (assertionMessage) { - // err.setOriginalMessage(err.getOriginalMessage() + `: ${assertionMessage}`); - // } try { // Public functions are simulated as a single Brillig entry point. // Thus, we can safely assume here that the Brillig function id is `0`. diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index c2bea391e5d9..7142eed0c776 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -759,16 +759,18 @@ export class PXEService implements PXE { * @param tx - The transaction to be simulated. */ async #simulatePublicCalls(tx: Tx) { - try { - return await this.node.simulatePublicCalls(tx); - } catch (err) { - // Try to fill in the noir call stack since the PXE may have access to the debug metadata - if (err instanceof SimulationError) { - await enrichPublicSimulationError(err, this.contractDataOracle, this.db, this.log); - } - - throw err; + const result = await this.node.simulatePublicCalls(tx); + if (result.revertReason) { + await enrichPublicSimulationError( + result.revertReason, + result.publicReturnValues[0].values || [], + this.contractDataOracle, + this.db, + this.log, + ); + throw result.revertReason; } + return result; } /** diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts index 5c0bdc1dea02..d04395210e84 100644 --- a/yarn-project/simulator/src/acvm/acvm.ts +++ b/yarn-project/simulator/src/acvm/acvm.ts @@ -1,4 +1,5 @@ import { type NoirCallStack, type SourceCodeLocation } from '@aztec/circuit-types'; +import { type Fr } from '@aztec/circuits.js'; import type { BrilligFunctionId, FunctionDebugMetadata, OpcodeLocation } from '@aztec/foundation/abi'; import { createDebugLogger } from '@aztec/foundation/log'; @@ -121,6 +122,25 @@ export function resolveAssertionMessage( } } +export function resolveAssertionMessageFromRevertData( + revertData: Fr[], + debug: FunctionDebugMetadata, +): string | undefined { + if (revertData.length == 0) { + return undefined; + } + + const [errorSelector, ...errorData] = revertData; + + return resolveAssertionMessage( + { + selector: errorSelector.toBigInt().toString(), + data: errorData.map(f => f.toString()), + }, + debug, + ); +} + export function resolveAssertionMessageFromError(err: Error, debug?: FunctionDebugMetadata): string { if (debug && typeof err === 'object' && err !== null && 'rawAssertionPayload' in err && err.rawAssertionPayload) { return `Assertion failed: ${resolveAssertionMessage(err.rawAssertionPayload as RawAssertionPayload, debug)}`; diff --git a/yarn-project/simulator/src/avm/fixtures/index.ts b/yarn-project/simulator/src/avm/fixtures/index.ts index 3ddc5b07ec76..ec50bdbebf14 100644 --- a/yarn-project/simulator/src/avm/fixtures/index.ts +++ b/yarn-project/simulator/src/avm/fixtures/index.ts @@ -10,7 +10,7 @@ import { strict as assert } from 'assert'; import { mock } from 'jest-mock-extended'; import merge from 'lodash.merge'; -import { type WorldStateDB, resolveAssertionMessage, traverseCauseChain } from '../../index.js'; +import { type WorldStateDB, resolveAssertionMessageFromRevertData, traverseCauseChain } from '../../index.js'; import { type PublicSideEffectTraceInterface } from '../../public/side_effect_trace_interface.js'; import { AvmContext } from '../avm_context.js'; import { AvmExecutionEnvironment } from '../avm_execution_environment.js'; @@ -153,17 +153,5 @@ export function resolveAvmTestContractAssertionMessage( return undefined; } - if (output.length == 0) { - return undefined; - } - - const [errorSelector, ...errorData] = output; - - return resolveAssertionMessage( - { - selector: errorSelector.toBigInt().toString(), - data: errorData.map(f => f.toString()), - }, - debugMetadata, - ); + return resolveAssertionMessageFromRevertData(output, debugMetadata); } diff --git a/yarn-project/simulator/src/public/enqueued_call_simulator.ts b/yarn-project/simulator/src/public/enqueued_call_simulator.ts index e33679b7e23a..c5c501ef551e 100644 --- a/yarn-project/simulator/src/public/enqueued_call_simulator.ts +++ b/yarn-project/simulator/src/public/enqueued_call_simulator.ts @@ -218,7 +218,9 @@ export class EnqueuedCallSimulator { avmProvingRequest, kernelOutput, newUnencryptedLogs: UnencryptedFunctionL2Logs.empty(), - returnValues: NestedProcessReturnValues.empty(), + // TODO: Following the above comment, we should continue when a frame reverts and collect all the return/revert data of all frames. + // For now, since we stop at the first revert, we can only return the revert reason of the first frame that reverts. + returnValues: new NestedProcessReturnValues(result.returnValues), gasUsed, revertReason: result.revertReason, }; diff --git a/yarn-project/simulator/src/public/enqueued_calls_processor.ts b/yarn-project/simulator/src/public/enqueued_calls_processor.ts index f76be43b1549..6d0d1616d358 100644 --- a/yarn-project/simulator/src/public/enqueued_calls_processor.ts +++ b/yarn-project/simulator/src/public/enqueued_calls_processor.ts @@ -300,7 +300,7 @@ export class EnqueuedCallsProcessor { publicKernelOutput, durationMs: phaseTimer.ms(), gasUsed, - returnValues: revertReason ? [] : returnValues, + returnValues: returnValues, revertReason, }; } From ba067d8b3d32f33de70d9f3a505d0ffd644a41a1 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 30 Oct 2024 16:38:43 +0000 Subject: [PATCH 07/16] fix txe --- yarn-project/txe/src/oracle/txe_oracle.ts | 1 + yarn-project/txe/src/txe_service/txe_service.ts | 2 ++ 2 files changed, 3 insertions(+) diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 893ab4a59c48..28ec52faa521 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -691,6 +691,7 @@ export class TXE implements TypedOracle { if (executionResult.revertReason && executionResult.revertReason instanceof SimulationError) { await enrichPublicSimulationError( executionResult.revertReason, + executionResult.returnValues, this.contractDataOracle, this.txeDatabase, this.logger, diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index 52ceb74f06f5..c5d36e100f38 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -743,6 +743,7 @@ export class TXEService { if (result.revertReason && result.revertReason instanceof SimulationError) { await enrichPublicSimulationError( result.revertReason, + result.returnValues, (this.typedOracle as TXE).getContractDataOracle(), (this.typedOracle as TXE).getTXEDatabase(), this.logger, @@ -773,6 +774,7 @@ export class TXEService { if (result.revertReason && result.revertReason instanceof SimulationError) { await enrichPublicSimulationError( result.revertReason, + result.returnValues, (this.typedOracle as TXE).getContractDataOracle(), (this.typedOracle as TXE).getTXEDatabase(), this.logger, From c881515f8478803b7132d36521dd9a53c22441ba Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 30 Oct 2024 17:35:11 +0000 Subject: [PATCH 08/16] dont require debug metadata for resolution --- yarn-project/foundation/src/abi/abi.ts | 5 ----- .../pxe/src/pxe_service/error_enriching.ts | 16 ++++++++------ yarn-project/simulator/src/acvm/acvm.ts | 22 +++++++------------ .../simulator/src/avm/fixtures/index.ts | 7 +----- .../simulator/src/client/private_execution.ts | 2 +- .../src/client/unconstrained_execution.ts | 2 +- yarn-project/txe/src/oracle/txe_oracle.ts | 2 +- 7 files changed, 21 insertions(+), 35 deletions(-) diff --git a/yarn-project/foundation/src/abi/abi.ts b/yarn-project/foundation/src/abi/abi.ts index 5c721d1baa67..779448d743cd 100644 --- a/yarn-project/foundation/src/abi/abi.ts +++ b/yarn-project/foundation/src/abi/abi.ts @@ -397,10 +397,6 @@ 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. - */ - errorTypes: Partial>; } /** @@ -442,7 +438,6 @@ export function getFunctionDebugMetadata( return { debugSymbols: programDebugSymbols.debug_infos[0], files: contractArtifact.fileMap, - errorTypes: functionArtifact.errorTypes, }; } return undefined; diff --git a/yarn-project/pxe/src/pxe_service/error_enriching.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts index b4894b1899ac..d70895841e46 100644 --- a/yarn-project/pxe/src/pxe_service/error_enriching.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -65,21 +65,23 @@ export async function enrichPublicSimulationError( // To be able to resolve the assertion message, we need to use the information from the public dispatch function, // no matter what the call stack selector points to (since we've modified it to point to the target function). // We should remove this because the AVM (or public protocol) shouldn't be aware of the public dispatch calling convention. - const debugInfo = await contractDataOracle.getFunctionDebugMetadata( + + const artifact = await contractDataOracle.getFunctionArtifact( originalFailingFunction.contractAddress, FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)), ); + const assertionMessage = resolveAssertionMessageFromRevertData(revertData, artifact); + if (assertionMessage) { + err.setOriginalMessage(err.getOriginalMessage() + `${assertionMessage}`); + } + const noirCallStack = err.getNoirCallStack(); - if (debugInfo) { - const assertionMessage = resolveAssertionMessageFromRevertData(revertData, debugInfo); - if (assertionMessage) { - err.setOriginalMessage(err.getOriginalMessage() + `${assertionMessage}`); - } + if (artifact.debug) { if (isNoirCallStackUnresolved(noirCallStack)) { try { // Public functions are simulated as a single Brillig entry point. // Thus, we can safely assume here that the Brillig function id is `0`. - const parsedCallStack = resolveOpcodeLocations(noirCallStack, debugInfo, 0); + const parsedCallStack = resolveOpcodeLocations(noirCallStack, artifact.debug, 0); err.setNoirCallStack(parsedCallStack); } catch (err) { logger.warn( diff --git a/yarn-project/simulator/src/acvm/acvm.ts b/yarn-project/simulator/src/acvm/acvm.ts index d04395210e84..bbd031eb315d 100644 --- a/yarn-project/simulator/src/acvm/acvm.ts +++ b/yarn-project/simulator/src/acvm/acvm.ts @@ -1,6 +1,6 @@ import { type NoirCallStack, type SourceCodeLocation } from '@aztec/circuit-types'; import { type Fr } from '@aztec/circuits.js'; -import type { BrilligFunctionId, FunctionDebugMetadata, OpcodeLocation } from '@aztec/foundation/abi'; +import type { BrilligFunctionId, FunctionAbi, FunctionDebugMetadata, OpcodeLocation } from '@aztec/foundation/abi'; import { createDebugLogger } from '@aztec/foundation/log'; import { @@ -106,12 +106,9 @@ export function resolveOpcodeLocations( ); } -export function resolveAssertionMessage( - errorPayload: RawAssertionPayload, - debug: FunctionDebugMetadata, -): string | undefined { +export function resolveAssertionMessage(errorPayload: RawAssertionPayload, abi: FunctionAbi): string | undefined { const decoded = abiDecodeError( - { parameters: [], error_types: debug.errorTypes, return_type: null }, // eslint-disable-line camelcase + { parameters: [], error_types: abi.errorTypes, return_type: null }, // eslint-disable-line camelcase errorPayload, ); @@ -122,10 +119,7 @@ export function resolveAssertionMessage( } } -export function resolveAssertionMessageFromRevertData( - revertData: Fr[], - debug: FunctionDebugMetadata, -): string | undefined { +export function resolveAssertionMessageFromRevertData(revertData: Fr[], abi: FunctionAbi): string | undefined { if (revertData.length == 0) { return undefined; } @@ -137,13 +131,13 @@ export function resolveAssertionMessageFromRevertData( selector: errorSelector.toBigInt().toString(), data: errorData.map(f => f.toString()), }, - debug, + abi, ); } -export function resolveAssertionMessageFromError(err: Error, debug?: FunctionDebugMetadata): string { - if (debug && typeof err === 'object' && err !== null && 'rawAssertionPayload' in err && err.rawAssertionPayload) { - return `Assertion failed: ${resolveAssertionMessage(err.rawAssertionPayload as RawAssertionPayload, debug)}`; +export function resolveAssertionMessageFromError(err: Error, abi: FunctionAbi): string { + if (typeof err === 'object' && err !== null && 'rawAssertionPayload' in err && err.rawAssertionPayload) { + return `Assertion failed: ${resolveAssertionMessage(err.rawAssertionPayload as RawAssertionPayload, abi)}`; } else { return err.message; } diff --git a/yarn-project/simulator/src/avm/fixtures/index.ts b/yarn-project/simulator/src/avm/fixtures/index.ts index ec50bdbebf14..98fb546e8387 100644 --- a/yarn-project/simulator/src/avm/fixtures/index.ts +++ b/yarn-project/simulator/src/avm/fixtures/index.ts @@ -148,10 +148,5 @@ export function resolveAvmTestContractAssertionMessage( return undefined; } - const debugMetadata = getFunctionDebugMetadata(AvmTestContractArtifact, functionArtifact); - if (!debugMetadata) { - return undefined; - } - - return resolveAssertionMessageFromRevertData(output, debugMetadata); + return resolveAssertionMessageFromRevertData(output, functionArtifact); } diff --git a/yarn-project/simulator/src/client/private_execution.ts b/yarn-project/simulator/src/client/private_execution.ts index 8d2bb5349c21..d27db358c567 100644 --- a/yarn-project/simulator/src/client/private_execution.ts +++ b/yarn-project/simulator/src/client/private_execution.ts @@ -33,7 +33,7 @@ export async function executePrivateFunction( const acvmCallback = new Oracle(context); const timer = new Timer(); const acirExecutionResult = await acvm(acir, initialWitness, acvmCallback).catch((err: Error) => { - err.message = resolveAssertionMessageFromError(err, artifact.debug); + err.message = resolveAssertionMessageFromError(err, artifact); throw new ExecutionError( err.message, { diff --git a/yarn-project/simulator/src/client/unconstrained_execution.ts b/yarn-project/simulator/src/client/unconstrained_execution.ts index 4da981bbcb57..f926b29bdca2 100644 --- a/yarn-project/simulator/src/client/unconstrained_execution.ts +++ b/yarn-project/simulator/src/client/unconstrained_execution.ts @@ -25,7 +25,7 @@ export async function executeUnconstrainedFunction( const acir = artifact.bytecode; const initialWitness = toACVMWitness(0, args); const acirExecutionResult = await acvm(acir, initialWitness, new Oracle(oracle)).catch((err: Error) => { - err.message = resolveAssertionMessageFromError(err, artifact.debug); + err.message = resolveAssertionMessageFromError(err, artifact); throw new ExecutionError( err.message, { diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 28ec52faa521..55ee023a9594 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -533,7 +533,7 @@ export class TXE implements TypedOracle { const timer = new Timer(); try { const acirExecutionResult = await acvm(acir, initialWitness, acvmCallback).catch((err: Error) => { - err.message = resolveAssertionMessageFromError(err, artifact.debug); + err.message = resolveAssertionMessageFromError(err, artifact); const execError = new ExecutionError( err.message, From e0b0c8cb12edf4d64fadb8352dc256d4d24050e8 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Wed, 30 Oct 2024 17:59:42 +0000 Subject: [PATCH 09/16] cleanups --- .../src/brillig/brillig_gen/brillig_block.rs | 2 +- .../brillig_ir/codegen_control_flow.rs | 4 ++-- .../ssa/acir_gen/acir_ir/generated_acir.rs | 7 ++----- .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 20 ++++++++++--------- .../noirc_evaluator/src/ssa/ir/instruction.rs | 17 ++++++++-------- .../noirc_evaluator/src/ssa/ir/printer.rs | 2 +- .../noirc_evaluator/src/ssa/ssa_gen/mod.rs | 8 +++----- yarn-project/foundation/src/abi/abi.ts | 4 +++- 8 files changed, 32 insertions(+), 32 deletions(-) diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs index a5667bb0685e..48914c6219de 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_gen/brillig_block.rs @@ -273,7 +273,7 @@ impl<'block> BrilligBlock<'block> { *selector, ); } - Some(ConstrainError::StaticString(_, message)) => { + Some(ConstrainError::StaticString(message)) => { self.brillig_context.codegen_constrain(condition, Some(message.clone())); } None => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs index 3084874d2a1e..23e89cfcbddb 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/codegen_control_flow.rs @@ -6,7 +6,7 @@ use acvm::{ AcirField, }; -use crate::{ssa::ir::instruction::error_selector_from_type, ErrorType}; +use crate::ErrorType; use super::{ artifact::BrilligParameter, @@ -266,7 +266,7 @@ impl BrilligContext< self.codegen_if_not(condition.address, |ctx| { if let Some(assert_message) = assert_message { let error_type = ErrorType::String(assert_message); - let error_selector = error_selector_from_type(&error_type); + let error_selector = error_type.selector(); ctx.obj.error_types.insert(error_selector, error_type); ctx.indirect_const_instruction( ReservedRegisters::free_memory_pointer(), diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs index e9d6ec49d0bd..bb42d682798e 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/acir_ir/generated_acir.rs @@ -5,10 +5,7 @@ use std::{collections::BTreeMap, u32}; use crate::{ brillig::{brillig_gen::brillig_directive, brillig_ir::artifact::GeneratedBrillig}, errors::{InternalError, RuntimeError, SsaReport}, - ssa::ir::{ - dfg::CallStack, - instruction::{error_selector_from_type, ErrorType}, - }, + ssa::ir::{dfg::CallStack, instruction::ErrorType}, }; use acvm::acir::{ circuit::{ @@ -628,7 +625,7 @@ impl GeneratedAcir { message: String, ) -> AssertionPayload { let error_type = ErrorType::String(message); - let error_selector = error_selector_from_type(&error_type); + let error_selector = error_type.selector(); self.record_error_type(error_selector, error_type); AssertionPayload { error_selector: error_selector.as_u64(), payload: Vec::new() } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs index e93af6b2ce05..e01c22a4044c 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/acir_gen/mod.rs @@ -676,18 +676,20 @@ impl<'a> Context<'a> { let rhs = self.convert_numeric_value(*rhs, dfg)?; let assert_payload = if let Some(error) = assert_message { - Some(match error { - ConstrainError::StaticString(_, string) => { - self.acir_context.generate_assertion_message_payload(string.clone()) - } + match error { + ConstrainError::StaticString(string) => Some( + self.acir_context.generate_assertion_message_payload(string.clone()), + ), ConstrainError::Dynamic(error_selector, is_string_type, values) => { if let Some(constant_string) = try_to_extract_string_from_error_payload( *is_string_type, values, dfg, ) { - self.acir_context - .generate_assertion_message_payload(constant_string) + Some( + self.acir_context + .generate_assertion_message_payload(constant_string), + ) } else { let acir_vars: Vec<_> = values .iter() @@ -697,13 +699,13 @@ impl<'a> Context<'a> { let expressions_or_memory = self.acir_context.vars_to_expressions_or_memory(&acir_vars)?; - AssertionPayload { + Some(AssertionPayload { error_selector: error_selector.as_u64(), payload: expressions_or_memory, - } + }) } } - }) + } } else { None }; diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs index 27816e70087e..c8e6936890c5 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs @@ -921,17 +921,19 @@ pub enum ErrorType { Dynamic(HirType), } -pub(crate) fn error_selector_from_type(typ: &ErrorType) -> ErrorSelector { - let mut hasher = FxHasher::default(); - typ.hash(&mut hasher); - let hash = hasher.finish(); - ErrorSelector::new(hash) +impl ErrorType { + pub fn selector(&self) -> ErrorSelector { + let mut hasher = FxHasher::default(); + self.hash(&mut hasher); + let hash = hasher.finish(); + ErrorSelector::new(hash) + } } #[derive(Debug, PartialEq, Eq, Hash, Clone, Serialize, Deserialize)] pub(crate) enum ConstrainError { // Static string errors are not handled inside the program as data for efficiency reasons. - StaticString(ErrorSelector, String), + StaticString(String), // These errors are handled by the program as data. // We use a boolean to indicate if the error is a string for printing purposes. Dynamic(ErrorSelector, /* is_string */ bool, Vec), @@ -939,8 +941,7 @@ pub(crate) enum ConstrainError { impl From for ConstrainError { fn from(value: String) -> Self { - let error_type = ErrorType::String(value.clone()); - ConstrainError::StaticString(error_selector_from_type(&error_type), value) + ConstrainError::StaticString(value) } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs index 7296ab5a41eb..93d69f255c37 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/printer.rs @@ -244,7 +244,7 @@ fn display_constrain_error( f: &mut Formatter, ) -> Result { match error { - ConstrainError::StaticString(_, assert_message_string) => { + ConstrainError::StaticString(assert_message_string) => { writeln!(f, " '{assert_message_string:?}'") } ConstrainError::Dynamic(_, is_string, values) => { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs index 64cb8d0aa582..96e779482a40 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/mod.rs @@ -21,7 +21,7 @@ use self::{ value::{Tree, Values}, }; -use super::ir::instruction::{error_selector_from_type, ErrorType}; +use super::ir::instruction::ErrorType; use super::{ function_builder::data_bus::DataBus, ir::{ @@ -704,12 +704,10 @@ impl<'a> FunctionContext<'a> { let (assert_message_expression, assert_message_typ) = assert_message_payload.as_ref(); if let Expression::Literal(ast::Literal::Str(static_string)) = assert_message_expression { - let error_type = ErrorType::String(static_string.clone()); - let selector = error_selector_from_type(&error_type); - Ok(Some(ConstrainError::StaticString(selector, static_string.clone()))) + Ok(Some(ConstrainError::StaticString(static_string.clone()))) } else { let error_type = ErrorType::Dynamic(assert_message_typ.clone()); - let selector = error_selector_from_type(&error_type); + let selector = error_type.selector(); let values = self.codegen_expression(assert_message_expression)?.into_value_list(self); let is_string_type = matches!(assert_message_typ, HirType::String(_)); // Record custom types in the builder, outside of SSA instructions diff --git a/yarn-project/foundation/src/abi/abi.ts b/yarn-project/foundation/src/abi/abi.ts index 779448d743cd..a1d9c6aaab87 100644 --- a/yarn-project/foundation/src/abi/abi.ts +++ b/yarn-project/foundation/src/abi/abi.ts @@ -206,7 +206,9 @@ export interface FunctionAbi { * The types of the return values. */ returnTypes: AbiType[]; - + /** + * The types of the errors that the function can throw. + */ errorTypes: Partial>; /** * Whether the function is flagged as an initializer. From 8c2bbaf2f8daf6e29b03722a5f72f4bb755e75a2 Mon Sep 17 00:00:00 2001 From: sirasistant Date: Thu, 31 Oct 2024 10:39:26 +0000 Subject: [PATCH 10/16] try to use procedures to deduplicate constant array reverts --- .../compiler/noirc_driver/src/lib.rs | 1 + .../src/brillig/brillig_ir/artifact.rs | 17 +++++----- .../brillig_ir/codegen_control_flow.rs | 34 ++++++++++++------- .../src/brillig/brillig_ir/instructions.rs | 2 +- .../src/brillig/brillig_ir/procedures/mod.rs | 10 ++++-- .../procedures/revert_with_string.rs | 23 +++++++++++++ .../noirc_evaluator/src/ssa/acir_gen/mod.rs | 2 +- 7 files changed, 64 insertions(+), 25 deletions(-) create mode 100644 noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/procedures/revert_with_string.rs diff --git a/noir/noir-repo/compiler/noirc_driver/src/lib.rs b/noir/noir-repo/compiler/noirc_driver/src/lib.rs index c4e2491a5fe6..1578e84dbb6d 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/lib.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/lib.rs @@ -462,6 +462,7 @@ fn compile_contract_inner( _ => None, }) .collect(); + println!("Compiled function: {}", name); functions.push(ContractFunction { name, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs index e0ede8541dc6..fa98f960dc60 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/artifact.rs @@ -57,7 +57,7 @@ pub(crate) struct BrilligArtifact { /// A pointer to a location in the opcode. pub(crate) type OpcodeLocation = usize; -#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] +#[derive(Debug, Clone, Eq, PartialEq, Hash)] pub(crate) enum LabelType { /// Labels for the entry point bytecode Entrypoint, @@ -87,7 +87,7 @@ impl std::fmt::Display for LabelType { /// /// It is assumed that an entity will keep a map /// of labels to Opcode locations. -#[derive(Debug, Clone, Copy, Eq, PartialEq, Hash)] +#[derive(Debug, Clone, Eq, PartialEq, Hash)] pub(crate) struct Label { pub(crate) label_type: LabelType, pub(crate) section: Option, @@ -95,7 +95,7 @@ pub(crate) struct Label { impl Label { pub(crate) fn add_section(&self, section: usize) -> Self { - Label { label_type: self.label_type, section: Some(section) } + Label { label_type: self.label_type.clone(), section: Some(section) } } pub(crate) fn function(func_id: FunctionId) -> Self { @@ -153,7 +153,7 @@ impl BrilligArtifact { /// Gets the first unresolved function call of this artifact. pub(crate) fn first_unresolved_function_call(&self) -> Option