diff --git a/avm-transpiler/Cargo.lock b/avm-transpiler/Cargo.lock index e145398e26e..e922fb18971 100644 --- a/avm-transpiler/Cargo.lock +++ b/avm-transpiler/Cargo.lock @@ -1202,6 +1202,7 @@ dependencies = [ "acvm", "iter-extended", "noirc_frontend", + "noirc_printable_type", "num-bigint", "num-traits", "serde", diff --git a/avm-transpiler/src/transpile.rs b/avm-transpiler/src/transpile.rs index e3141da53cd..1dc82a955e2 100644 --- a/avm-transpiler/src/transpile.rs +++ b/avm-transpiler/src/transpile.rs @@ -251,19 +251,16 @@ pub fn brillig_to_avm(brillig_bytecode: &[BrilligOpcode]) -> Vec { ..Default::default() }); } - BrilligOpcode::Trap { - revert_data_offset, - revert_data_size, - } => { + BrilligOpcode::Trap { revert_data } => { avm_instrs.push(AvmInstruction { opcode: AvmOpcode::REVERT, - indirect: Some(ALL_DIRECT), + indirect: Some(ZEROTH_OPERAND_INDIRECT), operands: vec![ AvmOperand::U32 { - value: *revert_data_offset as u32, + value: revert_data.pointer.0 as u32, }, AvmOperand::U32 { - value: *revert_data_size as u32, + value: revert_data.size as u32, }, ], ..Default::default() 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 09005b29941..e2c9d020d6b 100644 --- a/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp +++ b/barretenberg/cpp/src/barretenberg/dsl/acir_format/serde/acir.hpp @@ -1094,8 +1094,7 @@ struct BrilligOpcode { }; struct Trap { - uint64_t revert_data_offset; - uint64_t revert_data_size; + Program::HeapArray revert_data; friend bool operator==(const Trap&, const Trap&); std::vector bincodeSerialize() const; @@ -1136,6 +1135,56 @@ struct BrilligOpcode { static BrilligOpcode bincodeDeserialize(std::vector); }; +struct ExpressionOrMemory { + + struct Expression { + Program::Expression value; + + friend bool operator==(const Expression&, const Expression&); + std::vector bincodeSerialize() const; + static Expression bincodeDeserialize(std::vector); + }; + + struct Memory { + Program::BlockId value; + + friend bool operator==(const Memory&, const Memory&); + std::vector bincodeSerialize() const; + static Memory bincodeDeserialize(std::vector); + }; + + std::variant value; + + friend bool operator==(const ExpressionOrMemory&, const ExpressionOrMemory&); + std::vector bincodeSerialize() const; + static ExpressionOrMemory bincodeDeserialize(std::vector); +}; + +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; + + friend bool operator==(const AssertionPayload&, const AssertionPayload&); + std::vector bincodeSerialize() const; + static AssertionPayload bincodeDeserialize(std::vector); +}; + struct ExpressionWidth { struct Unbounded { @@ -1200,7 +1249,7 @@ struct Circuit { std::vector private_parameters; Program::PublicInputs public_parameters; Program::PublicInputs return_values; - std::vector> assert_messages; + std::vector> assert_messages; bool recursive; friend bool operator==(const Circuit&, const Circuit&); @@ -1229,6 +1278,150 @@ struct Program { namespace Program { +inline bool operator==(const AssertionPayload& lhs, const AssertionPayload& rhs) +{ + if (!(lhs.value == rhs.value)) { + return false; + } + return true; +} + +inline std::vector AssertionPayload::bincodeSerialize() const +{ + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); +} + +inline AssertionPayload AssertionPayload::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& obj, + Serializer& serializer) +{ + serializer.increase_container_depth(); + serde::Serializable::serialize(obj.value, serializer); + serializer.decrease_container_depth(); +} + +template <> +template +Program::AssertionPayload serde::Deserializable::deserialize(Deserializer& deserializer) +{ + deserializer.increase_container_depth(); + Program::AssertionPayload obj; + obj.value = 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)) { @@ -5980,10 +6173,7 @@ namespace Program { inline bool operator==(const BrilligOpcode::Trap& lhs, const BrilligOpcode::Trap& rhs) { - if (!(lhs.revert_data_offset == rhs.revert_data_offset)) { - return false; - } - if (!(lhs.revert_data_size == rhs.revert_data_size)) { + if (!(lhs.revert_data == rhs.revert_data)) { return false; } return true; @@ -6013,8 +6203,7 @@ template void serde::Serializable::serialize(const Program::BrilligOpcode::Trap& obj, Serializer& serializer) { - serde::Serializable::serialize(obj.revert_data_offset, serializer); - serde::Serializable::serialize(obj.revert_data_size, serializer); + serde::Serializable::serialize(obj.revert_data, serializer); } template <> @@ -6023,8 +6212,7 @@ Program::BrilligOpcode::Trap serde::Deserializable Deserializer& deserializer) { Program::BrilligOpcode::Trap obj; - obj.revert_data_offset = serde::Deserializable::deserialize(deserializer); - obj.revert_data_size = serde::Deserializable::deserialize(deserializer); + obj.revert_data = serde::Deserializable::deserialize(deserializer); return obj; } @@ -6474,6 +6662,150 @@ Program::Expression serde::Deserializable::deserialize(Dese namespace Program { +inline bool operator==(const ExpressionOrMemory& lhs, const ExpressionOrMemory& rhs) +{ + if (!(lhs.value == rhs.value)) { + return false; + } + return true; +} + +inline std::vector ExpressionOrMemory::bincodeSerialize() const +{ + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); +} + +inline ExpressionOrMemory ExpressionOrMemory::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::ExpressionOrMemory& obj, + Serializer& serializer) +{ + serializer.increase_container_depth(); + serde::Serializable::serialize(obj.value, serializer); + serializer.decrease_container_depth(); +} + +template <> +template +Program::ExpressionOrMemory serde::Deserializable::deserialize(Deserializer& deserializer) +{ + deserializer.increase_container_depth(); + Program::ExpressionOrMemory obj; + obj.value = serde::Deserializable::deserialize(deserializer); + deserializer.decrease_container_depth(); + return obj; +} + +namespace Program { + +inline bool operator==(const ExpressionOrMemory::Expression& lhs, const ExpressionOrMemory::Expression& rhs) +{ + if (!(lhs.value == rhs.value)) { + return false; + } + return true; +} + +inline std::vector ExpressionOrMemory::Expression::bincodeSerialize() const +{ + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); +} + +inline ExpressionOrMemory::Expression ExpressionOrMemory::Expression::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::ExpressionOrMemory::Expression& obj, Serializer& serializer) +{ + serde::Serializable::serialize(obj.value, serializer); +} + +template <> +template +Program::ExpressionOrMemory::Expression serde::Deserializable::deserialize( + Deserializer& deserializer) +{ + Program::ExpressionOrMemory::Expression obj; + obj.value = serde::Deserializable::deserialize(deserializer); + return obj; +} + +namespace Program { + +inline bool operator==(const ExpressionOrMemory::Memory& lhs, const ExpressionOrMemory::Memory& rhs) +{ + if (!(lhs.value == rhs.value)) { + return false; + } + return true; +} + +inline std::vector ExpressionOrMemory::Memory::bincodeSerialize() const +{ + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); +} + +inline ExpressionOrMemory::Memory ExpressionOrMemory::Memory::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::ExpressionOrMemory::Memory& obj, + Serializer& serializer) +{ + serde::Serializable::serialize(obj.value, serializer); +} + +template <> +template +Program::ExpressionOrMemory::Memory serde::Deserializable::deserialize( + Deserializer& deserializer) +{ + Program::ExpressionOrMemory::Memory obj; + obj.value = serde::Deserializable::deserialize(deserializer); + return obj; +} + +namespace Program { + inline bool operator==(const ExpressionWidth& lhs, const ExpressionWidth& rhs) { if (!(lhs.value == rhs.value)) { diff --git a/noir/noir-repo/Cargo.lock b/noir/noir-repo/Cargo.lock index 8a8ccfdbf8a..377e3339d7e 100644 --- a/noir/noir-repo/Cargo.lock +++ b/noir/noir-repo/Cargo.lock @@ -3072,6 +3072,7 @@ dependencies = [ "acvm", "iter-extended", "noirc_frontend", + "noirc_printable_type", "num-bigint", "num-traits", "serde", diff --git a/noir/noir-repo/Cargo.toml b/noir/noir-repo/Cargo.toml index 108d179b9ca..8911f6bfccb 100644 --- a/noir/noir-repo/Cargo.toml +++ b/noir/noir-repo/Cargo.toml @@ -109,7 +109,7 @@ chumsky = { git = "https://github.com/jfecher/chumsky", rev = "ad9d312", default criterion = "0.5.0" # Note that using the "frame-pointer" feature breaks framegraphs on linux # https://github.com/tikv/pprof-rs/pull/172 -pprof = { version = "0.13", features = ["flamegraph","criterion"] } +pprof = { version = "0.13", features = ["flamegraph", "criterion"] } dirs = "4" diff --git a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp index 9ce25c6fd94..0ad193fedf6 100644 --- a/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp +++ b/noir/noir-repo/acvm-repo/acir/codegen/acir.cpp @@ -1050,8 +1050,7 @@ namespace Program { }; struct Trap { - uint64_t revert_data_offset; - uint64_t revert_data_size; + Program::HeapArray revert_data; friend bool operator==(const Trap&, const Trap&); std::vector bincodeSerialize() const; @@ -1074,6 +1073,56 @@ namespace Program { static BrilligOpcode bincodeDeserialize(std::vector); }; + struct ExpressionOrMemory { + + struct Expression { + Program::Expression value; + + friend bool operator==(const Expression&, const Expression&); + std::vector bincodeSerialize() const; + static Expression bincodeDeserialize(std::vector); + }; + + struct Memory { + Program::BlockId value; + + friend bool operator==(const Memory&, const Memory&); + std::vector bincodeSerialize() const; + static Memory bincodeDeserialize(std::vector); + }; + + std::variant value; + + friend bool operator==(const ExpressionOrMemory&, const ExpressionOrMemory&); + std::vector bincodeSerialize() const; + static ExpressionOrMemory bincodeDeserialize(std::vector); + }; + + 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; + + friend bool operator==(const AssertionPayload&, const AssertionPayload&); + std::vector bincodeSerialize() const; + static AssertionPayload bincodeDeserialize(std::vector); + }; + struct ExpressionWidth { struct Unbounded { @@ -1138,7 +1187,7 @@ namespace Program { std::vector private_parameters; Program::PublicInputs public_parameters; Program::PublicInputs return_values; - std::vector> assert_messages; + std::vector> assert_messages; bool recursive; friend bool operator==(const Circuit&, const Circuit&); @@ -1166,6 +1215,124 @@ namespace Program { } // end of namespace Program +namespace Program { + + inline bool operator==(const AssertionPayload &lhs, const AssertionPayload &rhs) { + if (!(lhs.value == rhs.value)) { return false; } + return true; + } + + inline std::vector AssertionPayload::bincodeSerialize() const { + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); + } + + inline AssertionPayload AssertionPayload::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 &obj, Serializer &serializer) { + serializer.increase_container_depth(); + serde::Serializable::serialize(obj.value, serializer); + serializer.decrease_container_depth(); +} + +template <> +template +Program::AssertionPayload serde::Deserializable::deserialize(Deserializer &deserializer) { + deserializer.increase_container_depth(); + Program::AssertionPayload obj; + obj.value = 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) { @@ -4947,8 +5114,7 @@ Program::BrilligOpcode::BlackBox serde::Deserializable template void serde::Serializable::serialize(const Program::BrilligOpcode::Trap &obj, Serializer &serializer) { - serde::Serializable::serialize(obj.revert_data_offset, serializer); - serde::Serializable::serialize(obj.revert_data_size, serializer); + serde::Serializable::serialize(obj.revert_data, serializer); } template <> template Program::BrilligOpcode::Trap serde::Deserializable::deserialize(Deserializer &deserializer) { Program::BrilligOpcode::Trap obj; - obj.revert_data_offset = serde::Deserializable::deserialize(deserializer); - obj.revert_data_size = serde::Deserializable::deserialize(deserializer); + obj.revert_data = serde::Deserializable::deserialize(deserializer); return obj; } @@ -5341,6 +5505,124 @@ Program::Expression serde::Deserializable::deserialize(Dese return obj; } +namespace Program { + + inline bool operator==(const ExpressionOrMemory &lhs, const ExpressionOrMemory &rhs) { + if (!(lhs.value == rhs.value)) { return false; } + return true; + } + + inline std::vector ExpressionOrMemory::bincodeSerialize() const { + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); + } + + inline ExpressionOrMemory ExpressionOrMemory::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::ExpressionOrMemory &obj, Serializer &serializer) { + serializer.increase_container_depth(); + serde::Serializable::serialize(obj.value, serializer); + serializer.decrease_container_depth(); +} + +template <> +template +Program::ExpressionOrMemory serde::Deserializable::deserialize(Deserializer &deserializer) { + deserializer.increase_container_depth(); + Program::ExpressionOrMemory obj; + obj.value = serde::Deserializable::deserialize(deserializer); + deserializer.decrease_container_depth(); + return obj; +} + +namespace Program { + + inline bool operator==(const ExpressionOrMemory::Expression &lhs, const ExpressionOrMemory::Expression &rhs) { + if (!(lhs.value == rhs.value)) { return false; } + return true; + } + + inline std::vector ExpressionOrMemory::Expression::bincodeSerialize() const { + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); + } + + inline ExpressionOrMemory::Expression ExpressionOrMemory::Expression::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::ExpressionOrMemory::Expression &obj, Serializer &serializer) { + serde::Serializable::serialize(obj.value, serializer); +} + +template <> +template +Program::ExpressionOrMemory::Expression serde::Deserializable::deserialize(Deserializer &deserializer) { + Program::ExpressionOrMemory::Expression obj; + obj.value = serde::Deserializable::deserialize(deserializer); + return obj; +} + +namespace Program { + + inline bool operator==(const ExpressionOrMemory::Memory &lhs, const ExpressionOrMemory::Memory &rhs) { + if (!(lhs.value == rhs.value)) { return false; } + return true; + } + + inline std::vector ExpressionOrMemory::Memory::bincodeSerialize() const { + auto serializer = serde::BincodeSerializer(); + serde::Serializable::serialize(*this, serializer); + return std::move(serializer).bytes(); + } + + inline ExpressionOrMemory::Memory ExpressionOrMemory::Memory::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::ExpressionOrMemory::Memory &obj, Serializer &serializer) { + serde::Serializable::serialize(obj.value, serializer); +} + +template <> +template +Program::ExpressionOrMemory::Memory serde::Deserializable::deserialize(Deserializer &deserializer) { + Program::ExpressionOrMemory::Memory obj; + obj.value = serde::Deserializable::deserialize(deserializer); + return obj; +} + namespace Program { inline bool operator==(const ExpressionWidth &lhs, const ExpressionWidth &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 d655d136bc8..2cdb59f1b2a 100644 --- a/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs +++ b/noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs @@ -3,7 +3,8 @@ pub mod brillig; pub mod directives; pub mod opcodes; -use crate::native_types::Witness; +use crate::native_types::{Expression, Witness}; +use acir_field::FieldElement; pub use opcodes::Opcode; use thiserror::Error; @@ -15,7 +16,7 @@ use serde::{de::Error as DeserializationError, Deserialize, Deserializer, Serial use std::collections::BTreeSet; -use self::brillig::BrilligBytecode; +use self::{brillig::BrilligBytecode, opcodes::BlockId}; /// Specifies the maximum width of the expressions which will be constrained. /// @@ -59,18 +60,14 @@ pub struct Circuit { pub public_parameters: PublicInputs, /// The set of public inputs calculated within the circuit. pub return_values: PublicInputs, - /// Maps opcode locations to failed assertion messages. - /// These messages are embedded in the circuit to provide useful feedback to users + /// Maps opcode locations to failed assertion payloads. + /// The data in the payload is embedded in the circuit to provide useful feedback to users /// when a constraint in the circuit is not satisfied. /// // Note: This should be a BTreeMap, but serde-reflect is creating invalid // c++ code at the moment when it is, due to OpcodeLocation needing a comparison // implementation which is never generated. - // - // TODO: These are only used for constraints that are explicitly created during code generation (such as index out of bounds on slices) - // TODO: We should move towards having all the checks being evaluated in the same manner - // TODO: as runtime assert messages specified by the user. This will also be a breaking change as the `Circuit` structure will change. - pub assert_messages: Vec<(OpcodeLocation, String)>, + pub assert_messages: Vec<(OpcodeLocation, AssertionPayload)>, /// States whether the backend should use a SNARK recursion friendly prover. /// If implemented by a backend, this means that proofs generated with this circuit @@ -78,15 +75,28 @@ pub struct Circuit { pub recursive: bool, } -impl Circuit { - /// Returns the assert message associated with the provided [`OpcodeLocation`]. - /// Returns `None` if no such assert message exists. - pub fn get_assert_message(&self, opcode_location: OpcodeLocation) -> Option<&str> { - self.assert_messages - .iter() - .find(|(loc, _)| *loc == opcode_location) - .map(|(_, message)| message.as_str()) - } +/// 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: u64 = 0; + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum ExpressionOrMemory { + Expression(Expression), + Memory(BlockId), +} + +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub enum AssertionPayload { + StaticString(String), + Dynamic(/* error_selector */ u64, Vec), +} + +#[derive(Clone, PartialEq, Eq, Debug)] +pub enum ResolvedAssertionPayload { + String(String), + Raw(/*error_selector:*/ u64, Vec), } #[derive(Debug, Copy, Clone)] diff --git a/noir/noir-repo/acvm-repo/acir/src/lib.rs b/noir/noir-repo/acvm-repo/acir/src/lib.rs index d14159f34a1..24f27aae06f 100644 --- a/noir/noir-repo/acvm-repo/acir/src/lib.rs +++ b/noir/noir-repo/acvm-repo/acir/src/lib.rs @@ -42,7 +42,8 @@ mod reflection { brillig::{BrilligInputs, BrilligOutputs}, directives::Directive, opcodes::BlackBoxFuncCall, - Circuit, ExpressionWidth, Opcode, OpcodeLocation, Program, + AssertionPayload, Circuit, ExpressionOrMemory, ExpressionWidth, Opcode, OpcodeLocation, + Program, }, native_types::{Witness, WitnessMap, WitnessStack}, }; @@ -74,6 +75,8 @@ mod reflection { tracer.trace_simple_type::().unwrap(); tracer.trace_simple_type::().unwrap(); tracer.trace_simple_type::().unwrap(); + tracer.trace_simple_type::().unwrap(); + tracer.trace_simple_type::().unwrap(); let registry = tracer.registry().unwrap(); diff --git a/noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs index 6543c70958b..436db648ea8 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/compiler/mod.rs @@ -1,6 +1,6 @@ use std::collections::HashMap; -use acir::circuit::{Circuit, ExpressionWidth, OpcodeLocation}; +use acir::circuit::{AssertionPayload, Circuit, ExpressionWidth, OpcodeLocation}; // The various passes that we can use over ACIR mod optimizers; @@ -54,9 +54,9 @@ impl AcirTransformationMap { } fn transform_assert_messages( - assert_messages: Vec<(OpcodeLocation, String)>, + assert_messages: Vec<(OpcodeLocation, AssertionPayload)>, map: &AcirTransformationMap, -) -> Vec<(OpcodeLocation, String)> { +) -> Vec<(OpcodeLocation, AssertionPayload)> { assert_messages .into_iter() .flat_map(|(location, message)| { diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/arithmetic.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/arithmetic.rs index dc9e13d44b6..b971e4a0efb 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/arithmetic.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/arithmetic.rs @@ -53,6 +53,7 @@ impl ExpressionSolver { if !total_sum.is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Unresolved, + payload: None, }) } else { Ok(()) @@ -81,6 +82,7 @@ impl ExpressionSolver { if !total_sum.is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Unresolved, + payload: None, }) } else { Ok(()) @@ -96,6 +98,7 @@ impl ExpressionSolver { if !(a + b + opcode.q_c).is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Unresolved, + payload: None, }) } else { Ok(()) @@ -113,6 +116,7 @@ impl ExpressionSolver { if !total_sum.is_zero() { Err(OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Unresolved, + payload: None, }) } else { Ok(()) diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/range.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/range.rs index 2afe820b636..aac50b32fc8 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/range.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/blackbox/range.rs @@ -12,6 +12,7 @@ pub(crate) fn solve_range_opcode( if w_value.num_bits() > input.num_bits { return Err(OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Unresolved, + payload: None, }); } Ok(()) 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 b0a79c50aa0..9cf87455acb 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs @@ -5,7 +5,7 @@ use acir::{ circuit::{ brillig::{BrilligInputs, BrilligOutputs}, opcodes::BlockId, - OpcodeLocation, + OpcodeLocation, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, }, native_types::WitnessMap, FieldElement, @@ -160,32 +160,51 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> { VMStatus::Finished { .. } => Ok(BrilligSolverStatus::Finished), VMStatus::InProgress => Ok(BrilligSolverStatus::InProgress), VMStatus::Failure { reason, call_stack } => { - let message = match reason { - FailureReason::RuntimeError { message } => Some(message), + let payload = match reason { + FailureReason::RuntimeError { message } => { + Some(ResolvedAssertionPayload::String(message)) + } FailureReason::Trap { revert_data_offset, revert_data_size } => { // Since noir can only revert with strings currently, we can parse return data as a string if revert_data_size == 0 { None } else { let memory = self.vm.get_memory(); - let bytes = memory + let mut revert_values_iter = memory [revert_data_offset..(revert_data_offset + revert_data_size)] - .iter() - .map(|memory_value| { - memory_value - .try_into() - .expect("Assert message character is not a byte") - }) - .collect(); - Some( - String::from_utf8(bytes) - .expect("Assert message is not valid UTF-8"), - ) + .iter(); + let error_selector = revert_values_iter + .next() + .expect("Incorrect revert data size") + .try_into() + .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( + error_selector, + revert_values_iter.map(|value| value.to_field()).collect(), + )) + } + } } } }; Err(OpcodeResolutionError::BrilligFunctionFailed { - message, + payload, call_stack: call_stack .iter() .map(|brillig_index| OpcodeLocation::Brillig { diff --git a/noir/noir-repo/acvm-repo/acvm/src/pwg/directives/mod.rs b/noir/noir-repo/acvm-repo/acvm/src/pwg/directives/mod.rs index ee544521fc7..db79379a374 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/directives/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/directives/mod.rs @@ -26,6 +26,7 @@ pub(crate) fn solve_directives( if b.len() < decomposed_integer.len() { return Err(OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Unresolved, + payload: None, }); } 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 6a1bebf4ee8..1f4867f7656 100644 --- a/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs +++ b/noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs @@ -4,7 +4,10 @@ use std::collections::HashMap; use acir::{ brillig::ForeignCallResult, - circuit::{brillig::BrilligBytecode, opcodes::BlockId, Opcode, OpcodeLocation}, + circuit::{ + brillig::BrilligBytecode, opcodes::BlockId, AssertionPayload, ExpressionOrMemory, Opcode, + OpcodeLocation, ResolvedAssertionPayload, STRING_ERROR_SELECTOR, + }, native_types::{Expression, Witness, WitnessMap}, BlackBoxFunc, FieldElement, }; @@ -117,13 +120,19 @@ pub enum OpcodeResolutionError { #[error("Cannot solve opcode: {0}")] OpcodeNotSolvable(#[from] OpcodeNotSolvable), #[error("Cannot satisfy constraint")] - UnsatisfiedConstrain { opcode_location: ErrorLocation }, + UnsatisfiedConstrain { + opcode_location: ErrorLocation, + payload: Option, + }, #[error("Index out of bounds, array has size {array_size:?}, but index was {index:?}")] IndexOutOfBounds { opcode_location: ErrorLocation, index: u32, array_size: u32 }, #[error("Failed to solve blackbox function: {0}, reason: {1}")] BlackBoxFunctionFailed(BlackBoxFunc, String), - #[error("Failed to solve brillig function{}", .message.as_ref().map(|m| format!(", reason: {}", m)).unwrap_or_default())] - BrilligFunctionFailed { message: Option, call_stack: Vec }, + #[error("Failed to solve brillig function")] + BrilligFunctionFailed { + call_stack: Vec, + payload: Option, + }, #[error("Attempted to call `main` with a `Call` opcode")] AcirMainCallAttempted { opcode_location: ErrorLocation }, #[error("{results_size:?} result values were provided for {outputs_size:?} call output witnesses, most likely due to bad ACIR codegen")] @@ -168,6 +177,8 @@ pub struct ACVM<'a, B: BlackBoxFunctionSolver> { // Each unconstrained function referenced in the program unconstrained_functions: &'a [BrilligBytecode], + + assertion_payloads: &'a [(OpcodeLocation, AssertionPayload)], } impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { @@ -176,6 +187,7 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { opcodes: &'a [Opcode], initial_witness: WitnessMap, unconstrained_functions: &'a [BrilligBytecode], + assertion_payloads: &'a [(OpcodeLocation, AssertionPayload)], ) -> Self { let status = if opcodes.is_empty() { ACVMStatus::Solved } else { ACVMStatus::InProgress }; ACVM { @@ -190,6 +202,7 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { acir_call_counter: 0, acir_call_results: Vec::default(), unconstrained_functions, + assertion_payloads, } } @@ -362,14 +375,19 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { OpcodeResolutionError::IndexOutOfBounds { opcode_location: opcode_index, .. - } - | OpcodeResolutionError::UnsatisfiedConstrain { - opcode_location: opcode_index, } => { *opcode_index = ErrorLocation::Resolved(OpcodeLocation::Acir( self.instruction_pointer(), )); } + OpcodeResolutionError::UnsatisfiedConstrain { + opcode_location: opcode_index, + payload: assertion_payload, + } => { + let location = OpcodeLocation::Acir(self.instruction_pointer()); + *opcode_index = ErrorLocation::Resolved(location); + *assertion_payload = self.extract_assertion_payload(location); + } // All other errors are thrown normally. _ => (), }; @@ -378,6 +396,61 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { } } + fn extract_assertion_payload( + &self, + location: OpcodeLocation, + ) -> Option { + let (_, found_assertion_payload) = + 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") + })); + } + } + } + + 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(*error_selector, fields) + } + }) + } + } + } + fn solve_brillig_call_opcode( &mut self, ) -> Result, OpcodeResolutionError> { @@ -387,9 +460,9 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { unreachable!("Not executing a BrilligCall opcode"); }; - let witness = &mut self.witness_map; - if is_predicate_false(witness, predicate)? { - return BrilligSolver::::zero_out_brillig_outputs(witness, outputs).map(|_| None); + if is_predicate_false(&self.witness_map, predicate)? { + return BrilligSolver::::zero_out_brillig_outputs(&mut self.witness_map, outputs) + .map(|_| None); } // If we're resuming execution after resolving a foreign call then @@ -397,7 +470,7 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { let mut solver: BrilligSolver<'_, B> = match self.brillig_solver.take() { Some(solver) => solver, None => BrilligSolver::new_call( - witness, + &self.witness_map, &self.block_solvers, inputs, &self.unconstrained_functions[*id as usize].bytecode, @@ -405,7 +478,10 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { self.instruction_pointer, )?, }; - match solver.solve()? { + + let result = solver.solve().map_err(|err| self.map_brillig_error(err))?; + + match result { BrilligSolverStatus::ForeignCallWait(foreign_call) => { // Cache the current state of the solver self.brillig_solver = Some(solver); @@ -416,12 +492,37 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> { } BrilligSolverStatus::Finished => { // Write execution outputs - solver.finalize(witness, outputs)?; + solver.finalize(&mut self.witness_map, outputs)?; Ok(None) } } } + 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, B> { let Opcode::BrilligCall { id, inputs, outputs, predicate } = &self.opcodes[self.instruction_pointer] @@ -559,6 +660,7 @@ pub fn insert_value( if old_value != value_to_insert { return Err(OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Unresolved, + payload: None, }); } diff --git a/noir/noir-repo/acvm-repo/acvm/tests/solver.rs b/noir/noir-repo/acvm-repo/acvm/tests/solver.rs index 93985f97f40..df61083eee4 100644 --- a/noir/noir-repo/acvm-repo/acvm/tests/solver.rs +++ b/noir/noir-repo/acvm-repo/acvm/tests/solver.rs @@ -1,7 +1,7 @@ use std::collections::BTreeMap; use acir::{ - brillig::{BinaryFieldOp, MemoryAddress, Opcode as BrilligOpcode, ValueOrArray}, + brillig::{BinaryFieldOp, HeapArray, MemoryAddress, Opcode as BrilligOpcode, ValueOrArray}, circuit::{ brillig::{BrilligBytecode, BrilligInputs, BrilligOutputs}, opcodes::{BlockId, MemOp}, @@ -107,8 +107,13 @@ fn inversion_brillig_oracle_equivalence() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions); + let mut acvm = ACVM::new( + &StubbedBlackBoxSolver, + &opcodes, + witness_assignments, + &unconstrained_functions, + &[], + ); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -247,8 +252,13 @@ fn double_inversion_brillig_oracle() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions); + let mut acvm = ACVM::new( + &StubbedBlackBoxSolver, + &opcodes, + witness_assignments, + &unconstrained_functions, + &[], + ); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -379,8 +389,13 @@ fn oracle_dependent_execution() { let witness_assignments = BTreeMap::from([(w_x, FieldElement::from(2u128)), (w_y, FieldElement::from(2u128))]).into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions); + let mut acvm = ACVM::new( + &StubbedBlackBoxSolver, + &opcodes, + witness_assignments, + &unconstrained_functions, + &[], + ); // use the partial witness generation solver with our acir program let solver_status = acvm.solve(); @@ -486,8 +501,13 @@ fn brillig_oracle_predicate() { ]) .into(); let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, witness_assignments, &unconstrained_functions); + let mut acvm = ACVM::new( + &StubbedBlackBoxSolver, + &opcodes, + witness_assignments, + &unconstrained_functions, + &[], + ); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved, "should be fully solved"); @@ -522,12 +542,14 @@ fn unsatisfied_opcode_resolved() { let opcodes = vec![Opcode::AssertZero(opcode_a)]; let unconstrained_functions = vec![]; - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, values, &unconstrained_functions); + let mut acvm = + ACVM::new(&StubbedBlackBoxSolver, &opcodes, values, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!( solver_status, ACVMStatus::Failure(OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Resolved(OpcodeLocation::Acir(0)), + payload: None }), "The first opcode is not satisfiable, expected an error indicating this" ); @@ -562,7 +584,7 @@ fn unsatisfied_opcode_resolved_brillig() { let jmp_if_opcode = BrilligOpcode::JumpIf { condition: MemoryAddress::from(2), location: location_of_stop }; - let trap_opcode = BrilligOpcode::Trap { revert_data_offset: 0, revert_data_size: 0 }; + let trap_opcode = BrilligOpcode::Trap { revert_data: HeapArray::default() }; let stop_opcode = BrilligOpcode::Stop { return_data_offset: 0, return_data_size: 0 }; let brillig_bytecode = BrilligBytecode { @@ -610,12 +632,13 @@ fn unsatisfied_opcode_resolved_brillig() { Opcode::AssertZero(opcode_a), ]; let unconstrained_functions = vec![brillig_bytecode]; - let mut acvm = ACVM::new(&StubbedBlackBoxSolver, &opcodes, values, &unconstrained_functions); + let mut acvm = + ACVM::new(&StubbedBlackBoxSolver, &opcodes, values, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!( solver_status, ACVMStatus::Failure(OpcodeResolutionError::BrilligFunctionFailed { - message: None, + payload: None, call_stack: vec![OpcodeLocation::Brillig { acir_index: 0, brillig_index: 3 }] }), "The first opcode is not satisfiable, expected an error indicating this" @@ -655,7 +678,7 @@ fn memory_operations() { let opcodes = vec![init, read_op, expression]; let unconstrained_functions = vec![]; let mut acvm = - ACVM::new(&StubbedBlackBoxSolver, &opcodes, initial_witness, &unconstrained_functions); + ACVM::new(&StubbedBlackBoxSolver, &opcodes, initial_witness, &unconstrained_functions, &[]); let solver_status = acvm.solve(); assert_eq!(solver_status, ACVMStatus::Solved); let witness_map = acvm.finalize(); diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs index 2fab684467e..99d4b4ccb74 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs +++ b/noir/noir-repo/acvm-repo/acvm_js/src/execute.rs @@ -1,6 +1,7 @@ use std::{future::Future, pin::Pin}; use acvm::acir::circuit::brillig::BrilligBytecode; +use acvm::acir::circuit::ResolvedAssertionPayload; use acvm::BlackBoxFunctionSolver; use acvm::{ acir::circuit::{Circuit, Program}, @@ -78,7 +79,7 @@ pub async fn execute_circuit_with_return_witness( console_error_panic_hook::set_once(); let program: Program = Program::deserialize_program(&program) - .map_err(|_| JsExecutionError::new("Failed to deserialize circuit. This is likely due to differing serialization formats between ACVM_JS and your compiler".to_string(), None))?; + .map_err(|_| JsExecutionError::new("Failed to deserialize circuit. This is likely due to differing serialization formats between ACVM_JS and your compiler".to_string(), None, None))?; let mut witness_stack = execute_program_with_native_program_and_return( solver, @@ -93,7 +94,7 @@ pub async fn execute_circuit_with_return_witness( let main_circuit = &program.functions[0]; let return_witness = extract_indices(&solved_witness, main_circuit.return_values.0.iter().copied().collect()) - .map_err(|err| JsExecutionError::new(err, None))?; + .map_err(|err| JsExecutionError::new(err, None, None))?; Ok((solved_witness, return_witness).into()) } @@ -165,7 +166,10 @@ async fn execute_program_with_native_type_return( foreign_call_executor: &ForeignCallHandler, ) -> Result { let program: Program = Program::deserialize_program(&program) - .map_err(|_| JsExecutionError::new("Failed to deserialize circuit. This is likely due to differing serialization formats between ACVM_JS and your compiler".to_string(), None))?; + .map_err(|_| JsExecutionError::new( + "Failed to deserialize circuit. This is likely due to differing serialization formats between ACVM_JS and your compiler".to_string(), + None, + None))?; execute_program_with_native_program_and_return( solver, @@ -239,6 +243,7 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { &circuit.opcodes, initial_witness, self.unconstrained_functions, + &circuit.assert_messages, ); loop { @@ -250,39 +255,48 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { unreachable!("Execution should not stop while in `InProgress` state.") } ACVMStatus::Failure(error) => { - let (assert_message, call_stack): (Option<&str>, _) = match &error { + // Fetch call stack + let call_stack = match &error { OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Resolved(opcode_location), + .. } | OpcodeResolutionError::IndexOutOfBounds { opcode_location: ErrorLocation::Resolved(opcode_location), .. - } => ( - circuit.get_assert_message(*opcode_location), - Some(vec![*opcode_location]), - ), - OpcodeResolutionError::BrilligFunctionFailed { - call_stack, - message, - } => { - let revert_message = message.as_ref().map(String::as_str); - let failing_opcode = call_stack - .last() - .expect("Brillig error call stacks cannot be empty"); - ( - revert_message.or(circuit.get_assert_message(*failing_opcode)), - Some(call_stack.clone()), - ) + } => Some(vec![*opcode_location]), + OpcodeResolutionError::BrilligFunctionFailed { call_stack, .. } => { + Some(call_stack.clone()) } - _ => (None, None), + _ => None, }; - - let error_string = match &assert_message { - Some(assert_message) => format!("Assertion failed: {}", assert_message), - None => error.to_string(), + // If the failed opcode has an assertion message, integrate it into the error message for backwards compatibility. + // Otherwise, pass the raw assertion payload as is. + let (message, raw_assertion_payload) = match &error { + OpcodeResolutionError::UnsatisfiedConstrain { + payload: Some(payload), + .. + } + | OpcodeResolutionError::BrilligFunctionFailed { + payload: Some(payload), + .. + } => match payload { + ResolvedAssertionPayload::Raw(selector, fields) => { + (error.to_string(), Some((*selector, fields.clone()))) + } + ResolvedAssertionPayload::String(message) => { + (format!("Assertion failed: {}", message), None) + } + }, + _ => (error.to_string(), None), }; - return Err(JsExecutionError::new(error_string, call_stack).into()); + return Err(JsExecutionError::new( + message, + call_stack, + raw_assertion_payload, + ) + .into()); } ACVMStatus::RequiresForeignCall(foreign_call) => { let result = @@ -304,7 +318,7 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> { call_resolved_outputs.push(*return_value); } else { // TODO: look at changing this call stack from None - return Err(JsExecutionError::new(format!("Failed to read from solved witness of ACIR call at witness {}", return_witness_index), None).into()); + return Err(JsExecutionError::new(format!("Failed to read from solved witness of ACIR call at witness {}", return_witness_index), None, None).into()); } } acvm.resolve_pending_acir_call(call_resolved_outputs); diff --git a/noir/noir-repo/acvm-repo/acvm_js/src/js_execution_error.rs b/noir/noir-repo/acvm-repo/acvm_js/src/js_execution_error.rs index d91a9425f7e..ae0e0aaa236 100644 --- a/noir/noir-repo/acvm-repo/acvm_js/src/js_execution_error.rs +++ b/noir/noir-repo/acvm-repo/acvm_js/src/js_execution_error.rs @@ -1,11 +1,18 @@ -use acvm::acir::circuit::OpcodeLocation; -use js_sys::{Array, Error, JsString, Reflect}; +use acvm::{acir::circuit::OpcodeLocation, FieldElement}; +use js_sys::{Array, Error, JsString, Map, Object, Reflect}; use wasm_bindgen::prelude::{wasm_bindgen, JsValue}; +use crate::js_witness_map::field_element_to_js_string; + #[wasm_bindgen(typescript_custom_section)] const EXECUTION_ERROR: &'static str = r#" +export type RawAssertionPayload = { + selector: number; + fields: string[]; +}; export type ExecutionError = Error & { callStack?: string[]; + rawAssertionPayload?: RawAssertionPayload; }; "#; @@ -25,7 +32,11 @@ extern "C" { impl JsExecutionError { /// Creates a new execution error with the given call stack. /// Call stacks won't be optional in the future, after removing ErrorLocation in ACVM. - pub fn new(message: String, call_stack: Option>) -> Self { + pub fn new( + message: String, + call_stack: Option>, + assertion_payload: Option<(u64, Vec)>, + ) -> Self { let mut error = JsExecutionError::constructor(JsString::from(message)); let js_call_stack = match call_stack { Some(call_stack) => { @@ -37,8 +48,24 @@ impl JsExecutionError { } None => JsValue::UNDEFINED, }; + let assertion_payload = match assertion_payload { + Some((selector, fields)) => { + let raw_payload_map = Map::new(); + raw_payload_map + .set(&JsValue::from_str("selector"), &JsValue::from(selector.to_string())); + let js_fields = Array::new(); + for field in fields { + js_fields.push(&field_element_to_js_string(&field)); + } + raw_payload_map.set(&JsValue::from_str("fields"), &js_fields.into()); + + Object::from_entries(&raw_payload_map).unwrap().into() + } + None => JsValue::UNDEFINED, + }; error.set_property("callStack", js_call_stack); + error.set_property("rawAssertionPayload", assertion_payload); error } diff --git a/noir/noir-repo/acvm-repo/brillig/src/opcodes.rs b/noir/noir-repo/acvm-repo/brillig/src/opcodes.rs index 468fd88db45..a060aa83d41 100644 --- a/noir/noir-repo/acvm-repo/brillig/src/opcodes.rs +++ b/noir/noir-repo/acvm-repo/brillig/src/opcodes.rs @@ -52,6 +52,12 @@ pub struct HeapArray { pub size: usize, } +impl Default for HeapArray { + fn default() -> Self { + Self { pointer: MemoryAddress(0), size: 0 } + } +} + /// A memory-sized vector passed starting from a Brillig memory location and with a memory-held size #[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize, Copy)] pub struct HeapVector { @@ -179,8 +185,7 @@ pub enum BrilligOpcode { BlackBox(BlackBoxOp), /// Used to denote execution failure, returning data after the offset Trap { - revert_data_offset: usize, - revert_data_size: usize, + revert_data: HeapArray, }, /// Stop execution, returning data after the offset Stop { diff --git a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs index 75299670f94..7901c313596 100644 --- a/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs +++ b/noir/noir-repo/acvm-repo/brillig_vm/src/lib.rs @@ -305,8 +305,12 @@ impl<'a, B: BlackBoxFunctionSolver> VM<'a, B> { } self.increment_program_counter() } - Opcode::Trap { revert_data_offset, revert_data_size } => { - self.trap(*revert_data_offset, *revert_data_size) + Opcode::Trap { revert_data } => { + if revert_data.size > 0 { + self.trap(self.memory.read_ref(revert_data.pointer).0, revert_data.size) + } else { + self.trap(0, 0) + } } Opcode::Stop { return_data_offset, return_data_size } => { self.finish(*return_data_offset, *return_data_size) @@ -715,7 +719,7 @@ mod tests { let jump_opcode = Opcode::Jump { location: 3 }; - let trap_opcode = Opcode::Trap { revert_data_offset: 0, revert_data_size: 0 }; + let trap_opcode = Opcode::Trap { revert_data: HeapArray::default() }; let not_equal_cmp_opcode = Opcode::BinaryFieldOp { op: BinaryFieldOp::Equals, 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 51fe4986845..b1e52dc309d 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs @@ -2,11 +2,11 @@ use std::collections::BTreeMap; use acvm::acir::native_types::Witness; use iter_extended::{btree_map, vecmap}; -use noirc_abi::{Abi, AbiParameter, AbiReturnType, AbiType, AbiValue}; +use noirc_abi::{Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue}; use noirc_frontend::ast::Visibility; use noirc_frontend::{ hir::Context, - hir_def::{expr::HirArrayLiteral, function::Param, stmt::HirPattern}, + hir_def::{expr::HirArrayLiteral, function::Param, stmt::HirPattern, types::Type}, macros_api::{HirExpression, HirLiteral}, node_interner::{FuncId, NodeInterner}, }; @@ -20,12 +20,17 @@ pub(super) fn gen_abi( input_witnesses: Vec, return_witnesses: Vec, return_visibility: Visibility, + error_types: BTreeMap, ) -> Abi { let (parameters, return_type) = compute_function_abi(context, func_id); let param_witnesses = param_witnesses_from_abi_param(¶meters, input_witnesses); let return_type = return_type .map(|typ| AbiReturnType { abi_type: typ, visibility: return_visibility.into() }); - Abi { parameters, return_type, param_witnesses, return_witnesses } + let error_types = error_types + .into_iter() + .map(|(selector, typ)| (selector, AbiErrorType::from_type(context, &typ))) + .collect(); + Abi { parameters, return_type, param_witnesses, return_witnesses, error_types } } pub(super) fn compute_function_abi( diff --git a/noir/noir-repo/compiler/noirc_driver/src/lib.rs b/noir/noir-repo/compiler/noirc_driver/src/lib.rs index 8a554879e9f..135d95b473f 100644 --- a/noir/noir-repo/compiler/noirc_driver/src/lib.rs +++ b/noir/noir-repo/compiler/noirc_driver/src/lib.rs @@ -529,6 +529,7 @@ pub fn compile_no_check( main_input_witnesses, main_return_witnesses, names, + error_types, } = create_program( program, options.show_ssa, @@ -543,6 +544,7 @@ pub fn compile_no_check( main_input_witnesses, main_return_witnesses, visibility, + error_types, ); let file_map = filter_relevant_files(&debug, &context.file_manager); diff --git a/noir/noir-repo/compiler/noirc_errors/Cargo.toml b/noir/noir-repo/compiler/noirc_errors/Cargo.toml index c9cb7e2709f..41b1cd0ff58 100644 --- a/noir/noir-repo/compiler/noirc_errors/Cargo.toml +++ b/noir/noir-repo/compiler/noirc_errors/Cargo.toml @@ -20,4 +20,4 @@ serde_with = "3.2.0" tracing.workspace = true flate2.workspace = true serde_json.workspace = true -base64.workspace = true \ No newline at end of file +base64.workspace = true 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 22407fc5695..93431cb45dc 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 @@ -5,7 +5,7 @@ use crate::brillig::brillig_ir::{ BrilligBinaryOp, BrilligContext, BRILLIG_MEMORY_ADDRESSING_BIT_SIZE, }; use crate::ssa::ir::dfg::CallStack; -use crate::ssa::ir::instruction::{ConstrainError, UserDefinedConstrainError}; +use crate::ssa::ir::instruction::ConstrainError; use crate::ssa::ir::{ basic_block::{BasicBlock, BasicBlockId}, dfg::DataFlowGraph, @@ -248,34 +248,6 @@ impl<'block> BrilligBlock<'block> { self.convert_ssa_binary(binary, dfg, result_var); } Instruction::Constrain(lhs, rhs, assert_message) => { - let (has_revert_data, static_assert_message) = if let Some(error) = assert_message { - match error.as_ref() { - ConstrainError::Intrinsic(string) => (false, Some(string.clone())), - ConstrainError::UserDefined(UserDefinedConstrainError::Static(string)) => { - (true, Some(string.clone())) - } - ConstrainError::UserDefined(UserDefinedConstrainError::Dynamic( - call_instruction, - )) => { - let Instruction::Call { func, arguments } = call_instruction else { - unreachable!("expected a call instruction") - }; - - let Value::Function(func_id) = &dfg[*func] else { - unreachable!("expected a function value") - }; - - self.convert_ssa_function_call(*func_id, arguments, dfg, &[]); - - // Dynamic assert messages are handled in the generated function call. - // We then don't need to attach one to the constrain instruction. - (false, None) - } - } - } else { - (false, None) - }; - let condition = SingleAddrVariable { address: self.brillig_context.allocate_register(), bit_size: 1, @@ -286,11 +258,27 @@ impl<'block> BrilligBlock<'block> { dfg, condition, ); - if has_revert_data { - self.brillig_context - .codegen_constrain_with_revert_data(condition, static_assert_message); - } else { - self.brillig_context.codegen_constrain(condition, static_assert_message); + match assert_message { + Some(ConstrainError::UserDefined(selector, values)) => { + let payload_values = + vecmap(values, |value| self.convert_ssa_value(*value, dfg)); + let payload_as_params = vecmap(values, |value| { + let value_type = dfg.type_of_value(*value); + FunctionContext::ssa_type_to_parameter(&value_type) + }); + self.brillig_context.codegen_constrain_with_revert_data( + condition, + payload_values, + payload_as_params, + selector.to_u64(), + ); + } + Some(ConstrainError::Intrinsic(message)) => { + self.brillig_context.codegen_constrain(condition, Some(message.clone())); + } + None => { + self.brillig_context.codegen_constrain(condition, None); + } } self.brillig_context.deallocate_single_addr(condition); } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs index 7e37e1da434..ded41e02bda 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir.rs @@ -130,7 +130,7 @@ pub(crate) mod tests { use std::vec; use acvm::acir::brillig::{ - ForeignCallParam, ForeignCallResult, HeapVector, MemoryAddress, ValueOrArray, + ForeignCallParam, ForeignCallResult, HeapArray, HeapVector, MemoryAddress, ValueOrArray, }; use acvm::brillig_vm::brillig::HeapValueType; use acvm::brillig_vm::{VMStatus, VM}; @@ -270,7 +270,7 @@ pub(crate) mod tests { // uses unresolved jumps which requires a block to be constructed in SSA and // we don't need this for Brillig IR tests context.push_opcode(BrilligOpcode::JumpIf { condition: r_equality, location: 8 }); - context.push_opcode(BrilligOpcode::Trap { revert_data_offset: 0, revert_data_size: 0 }); + context.push_opcode(BrilligOpcode::Trap { revert_data: HeapArray::default() }); context.stop_instruction(); 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 8a4f469f5c9..dee6c6076f4 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 @@ -29,7 +29,9 @@ 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 + /// 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, /// The set of jumps that need to have their locations /// resolved. 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 f8f39f03df4..d9109646338 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,7 +1,9 @@ -use acvm::acir::brillig::MemoryAddress; +use acvm::acir::brillig::{HeapArray, MemoryAddress}; use super::{ - brillig_variable::SingleAddrVariable, BrilligBinaryOp, BrilligContext, ReservedRegisters, + artifact::BrilligParameter, + brillig_variable::{BrilligVariable, SingleAddrVariable}, + BrilligBinaryOp, BrilligContext, ReservedRegisters, }; impl BrilligContext { @@ -144,25 +146,62 @@ impl BrilligContext { pub(crate) fn codegen_constrain_with_revert_data( &mut self, condition: SingleAddrVariable, - assert_message: Option, + revert_data_items: Vec, + revert_data_types: Vec, + error_selector: u64, ) { assert!(condition.bit_size == 1); self.codegen_if_not(condition.address, |ctx| { - let (revert_data_offset, revert_data_size) = - if let Some(assert_message) = assert_message { - let bytes = assert_message.as_bytes(); - for (i, byte) in bytes.iter().enumerate() { - ctx.const_instruction( - SingleAddrVariable::new(MemoryAddress(i), 8), - (*byte as usize).into(), + let revert_data = HeapArray { + pointer: ctx.allocate_register(), + // + 1 due to the revert data id being the first item returned + size: BrilligContext::flattened_tuple_size(&revert_data_types) + 1, + }; + ctx.codegen_allocate_fixed_length_array(revert_data.pointer, revert_data.size); + + let current_revert_data_pointer = ctx.allocate_register(); + ctx.mov_instruction(current_revert_data_pointer, revert_data.pointer); + let revert_data_id = + ctx.make_usize_constant_instruction((error_selector as u128).into()); + ctx.store_instruction(current_revert_data_pointer, revert_data_id.address); + + ctx.codegen_usize_op_in_place(current_revert_data_pointer, BrilligBinaryOp::Add, 1); + for (revert_variable, revert_param) in + revert_data_items.into_iter().zip(revert_data_types.into_iter()) + { + let flattened_size = BrilligContext::flattened_size(&revert_param); + match revert_param { + BrilligParameter::SingleAddr(_) => { + ctx.store_instruction( + current_revert_data_pointer, + revert_variable.extract_single_addr().address, + ); + } + BrilligParameter::Array(item_type, item_count) => { + let variable_pointer = revert_variable.extract_array().pointer; + + ctx.flatten_array( + &item_type, + item_count, + current_revert_data_pointer, + variable_pointer, ); } - (0, bytes.len()) - } else { - (0, 0) - }; - ctx.trap_instruction(revert_data_offset, revert_data_size); + BrilligParameter::Slice(_, _) => { + unimplemented!("Slices are not supported as revert data") + } + } + ctx.codegen_usize_op_in_place( + current_revert_data_pointer, + BrilligBinaryOp::Add, + flattened_size, + ); + } + ctx.trap_instruction(revert_data); + ctx.deallocate_register(revert_data.pointer); + ctx.deallocate_register(current_revert_data_pointer); + ctx.deallocate_single_addr(revert_data_id); }); } @@ -176,7 +215,7 @@ impl BrilligContext { assert!(condition.bit_size == 1); self.codegen_if_not(condition.address, |ctx| { - ctx.trap_instruction(0, 0); + ctx.trap_instruction(HeapArray::default()); if let Some(assert_message) = assert_message { ctx.obj.add_assert_message_to_last_opcode(assert_message); } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs index 41a6d1873e4..5601bbde877 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/debug_show.rs @@ -114,13 +114,8 @@ impl DebugShow { } /// Emits a `trap` instruction. - pub(crate) fn trap_instruction(&self, revert_data_offset: usize, revert_data_size: usize) { - debug_println!( - self.enable_debug_trace, - " TRAP {}..{}", - revert_data_offset, - revert_data_offset + revert_data_size - ); + pub(crate) fn trap_instruction(&self, revert_data: HeapArray) { + debug_println!(self.enable_debug_trace, " TRAP {}", revert_data); } /// Emits a `mov` instruction. diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs index 88cf987325d..732bd3cbc59 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/entry_point.rs @@ -164,7 +164,7 @@ impl BrilligContext { } /// Computes the size of a parameter if it was flattened - fn flattened_size(param: &BrilligParameter) -> usize { + pub(super) fn flattened_size(param: &BrilligParameter) -> usize { match param { BrilligParameter::SingleAddr(_) => 1, BrilligParameter::Array(item_types, item_count) @@ -176,7 +176,7 @@ impl BrilligContext { } /// Computes the size of a parameter if it was flattened - fn flattened_tuple_size(tuple: &[BrilligParameter]) -> usize { + pub(super) fn flattened_tuple_size(tuple: &[BrilligParameter]) -> usize { tuple.iter().map(BrilligContext::flattened_size).sum() } @@ -369,7 +369,7 @@ impl BrilligContext { } // Flattens an array by recursively copying nested arrays and regular items. - fn flatten_array( + pub(super) fn flatten_array( &mut self, item_type: &[BrilligParameter], item_count: usize, diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs index 901ccc58036..5f4e8c4a171 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/brillig/brillig_ir/instructions.rs @@ -1,6 +1,6 @@ use acvm::{ acir::brillig::{ - BinaryFieldOp, BinaryIntOp, BlackBoxOp, HeapValueType, MemoryAddress, + BinaryFieldOp, BinaryIntOp, BlackBoxOp, HeapArray, HeapValueType, MemoryAddress, Opcode as BrilligOpcode, ValueOrArray, }, FieldElement, @@ -466,10 +466,10 @@ impl BrilligContext { }); } - pub(super) fn trap_instruction(&mut self, revert_data_offset: usize, revert_data_size: usize) { - self.debug_show.trap_instruction(revert_data_offset, revert_data_size); + pub(super) fn trap_instruction(&mut self, revert_data: HeapArray) { + self.debug_show.trap_instruction(revert_data); - self.push_opcode(BrilligOpcode::Trap { revert_data_offset, revert_data_size }); + self.push_opcode(BrilligOpcode::Trap { revert_data }); } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs b/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs index b3e838e708e..1e922060100 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/errors.rs @@ -7,7 +7,7 @@ //! An Error of the former is a user Error //! //! An Error of the latter is an error in the implementation of the compiler -use acvm::{acir::native_types::Expression, FieldElement}; +use acvm::FieldElement; use iter_extended::vecmap; use noirc_errors::{CustomDiagnostic as Diagnostic, FileDiagnostic}; use thiserror::Error; @@ -17,13 +17,6 @@ use serde::{Deserialize, Serialize}; #[derive(Debug, PartialEq, Eq, Clone, Error)] pub enum RuntimeError { - #[error("{}", format_failed_constraint(.assert_message))] - FailedConstraint { - lhs: Box, - rhs: Box, - call_stack: CallStack, - assert_message: Option, - }, #[error(transparent)] InternalError(#[from] InternalError), #[error("Index out of bounds, array has size {array_size}, but index was {index}")] @@ -52,16 +45,6 @@ pub enum RuntimeError { UnconstrainedOracleReturnToConstrained { call_stack: CallStack }, } -// We avoid showing the actual lhs and rhs since most of the time they are just 0 -// and 1 respectively. This would confuse users if a constraint such as -// assert(foo < bar) fails with "failed constraint: 0 = 1." -fn format_failed_constraint(message: &Option) -> String { - match message { - Some(message) => format!("Failed constraint: '{message}'"), - None => "Failed constraint".to_owned(), - } -} - #[derive(Debug, Clone, Serialize, Deserialize)] pub enum SsaReport { Warning(InternalWarning), @@ -129,7 +112,6 @@ impl RuntimeError { | InternalError::UndeclaredAcirVar { call_stack } | InternalError::Unexpected { call_stack, .. }, ) - | RuntimeError::FailedConstraint { call_stack, .. } | RuntimeError::IndexOutOfBounds { call_stack, .. } | RuntimeError::InvalidRangeConstraint { call_stack, .. } | RuntimeError::TypeConversion { call_stack, .. } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs index 3482a8c25c7..92461e1a1b1 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs @@ -7,7 +7,7 @@ //! This module heavily borrows from Cranelift #![allow(dead_code)] -use std::collections::BTreeSet; +use std::collections::{BTreeMap, BTreeSet}; use crate::{ brillig::Brillig, @@ -23,10 +23,16 @@ use acvm::acir::{ use noirc_errors::debug_info::{DebugFunctions, DebugInfo, DebugTypes, DebugVariables}; use noirc_frontend::ast::Visibility; -use noirc_frontend::{hir_def::function::FunctionSignature, monomorphization::ast::Program}; +use noirc_frontend::{ + hir_def::{function::FunctionSignature, types::Type as HirType}, + monomorphization::ast::Program, +}; use tracing::{span, Level}; -use self::{acir_gen::GeneratedAcir, ssa_gen::Ssa}; +use self::{ + acir_gen::{Artifacts, GeneratedAcir}, + ssa_gen::Ssa, +}; mod acir_gen; pub(super) mod function_builder; @@ -45,7 +51,7 @@ pub(crate) fn optimize_into_acir( print_brillig_trace: bool, force_brillig_output: bool, print_timings: bool, -) -> Result<(Vec, Vec), RuntimeError> { +) -> Result { let abi_distinctness = program.return_distinctness; let ssa_gen_span = span!(Level::TRACE, "ssa_generation"); @@ -99,10 +105,14 @@ pub struct SsaProgramArtifact { pub main_input_witnesses: Vec, pub main_return_witnesses: Vec, pub names: Vec, + pub error_types: BTreeMap, } impl SsaProgramArtifact { - fn new(unconstrained_functions: Vec) -> Self { + fn new( + unconstrained_functions: Vec, + error_types: BTreeMap, + ) -> Self { let program = AcirProgram { functions: Vec::default(), unconstrained_functions }; Self { program, @@ -111,6 +121,7 @@ impl SsaProgramArtifact { main_input_witnesses: Vec::default(), main_return_witnesses: Vec::default(), names: Vec::default(), + error_types, } } @@ -145,7 +156,7 @@ pub fn create_program( let func_sigs = program.function_signatures.clone(); let recursive = program.recursive; - let (generated_acirs, generated_brillig) = optimize_into_acir( + let (generated_acirs, generated_brillig, error_types) = optimize_into_acir( program, enable_ssa_logging, enable_brillig_logging, @@ -158,7 +169,12 @@ pub fn create_program( "The generated ACIRs should match the supplied function signatures" ); - let mut program_artifact = SsaProgramArtifact::new(generated_brillig); + let error_types = error_types + .into_iter() + .map(|(error_typ_id, error_typ)| (error_typ_id.to_u64(), error_typ)) + .collect(); + + let mut program_artifact = SsaProgramArtifact::new(generated_brillig, error_types); // For setting up the ABI we need separately specify main's input and return witnesses let mut is_main = true; for (acir, func_sig) in generated_acirs.into_iter().zip(func_sigs) { @@ -201,7 +217,7 @@ fn convert_generated_acir_into_circuit( return_witnesses, locations, input_witnesses, - assert_messages, + assertion_payloads: assert_messages, warnings, name, .. 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 3f5e4129dd0..2d546bc7d86 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 @@ -9,7 +9,7 @@ use crate::ssa::ir::types::Type as SsaType; use crate::ssa::ir::{instruction::Endian, types::NumericType}; use acvm::acir::circuit::brillig::{BrilligInputs, BrilligOutputs}; use acvm::acir::circuit::opcodes::{BlockId, MemOp}; -use acvm::acir::circuit::Opcode; +use acvm::acir::circuit::{AssertionPayload, ExpressionOrMemory, Opcode}; use acvm::blackbox_solver; use acvm::brillig_vm::{MemoryValue, VMStatus, VM}; use acvm::{ @@ -495,7 +495,7 @@ impl AcirContext { &mut self, lhs: AcirVar, rhs: AcirVar, - assert_message: Option, + assert_message: Option, ) -> Result<(), RuntimeError> { let lhs_expr = self.var_to_expression(lhs)?; let rhs_expr = self.var_to_expression(rhs)?; @@ -511,14 +511,38 @@ impl AcirContext { } self.acir_ir.assert_is_zero(diff_expr); - if let Some(message) = assert_message { - self.acir_ir.assert_messages.insert(self.acir_ir.last_acir_opcode_location(), message); + if let Some(payload) = assert_message { + self.acir_ir + .assertion_payloads + .insert(self.acir_ir.last_acir_opcode_location(), payload); } self.mark_variables_equivalent(lhs, rhs)?; Ok(()) } + pub(crate) fn vars_to_expressions_or_memory( + &self, + values: &[AcirValue], + ) -> Result, RuntimeError> { + let mut result = Vec::with_capacity(values.len()); + for value in values { + match value { + AcirValue::Var(var, _) => { + result.push(ExpressionOrMemory::Expression(self.var_to_expression(*var)?)); + } + AcirValue::Array(vars) => { + let vars_as_vec: Vec<_> = vars.iter().cloned().collect(); + result.extend(self.vars_to_expressions_or_memory(&vars_as_vec)?); + } + AcirValue::DynamicArray(AcirDynamicArray { block_id, .. }) => { + result.push(ExpressionOrMemory::Memory(*block_id)); + } + } + } + Ok(result) + } + /// Adds a new Variable to context whose value will /// be constrained to be the division of `lhs` and `rhs` pub(crate) fn div_var( @@ -985,9 +1009,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 - .assert_messages - .insert(self.acir_ir.last_acir_opcode_location(), message); + self.acir_ir.assertion_payloads.insert( + self.acir_ir.last_acir_opcode_location(), + AssertionPayload::StaticString(message.clone()), + ); } } NumericType::NativeField => { 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 0b04d1b63ab..1d3bc3fce57 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 @@ -7,12 +7,11 @@ use crate::{ errors::{InternalError, RuntimeError, SsaReport}, ssa::ir::dfg::CallStack, }; - use acvm::acir::{ circuit::{ brillig::{BrilligInputs, BrilligOutputs}, opcodes::{BlackBoxFuncCall, FunctionInput, Opcode as AcirOpcode}, - OpcodeLocation, + AssertionPayload, OpcodeLocation, }, native_types::Witness, BlackBoxFunc, @@ -61,7 +60,7 @@ pub(crate) struct GeneratedAcir { pub(crate) call_stack: CallStack, /// Correspondence between an opcode index and the error message associated with it. - pub(crate) assert_messages: BTreeMap, + pub(crate) assertion_payloads: BTreeMap, pub(crate) warnings: Vec, @@ -652,12 +651,12 @@ impl GeneratedAcir { ); } for (brillig_index, message) in generated_brillig.assert_messages.iter() { - self.assert_messages.insert( + self.assertion_payloads.insert( OpcodeLocation::Brillig { acir_index: self.opcodes.len() - 1, brillig_index: *brillig_index, }, - message.clone(), + AssertionPayload::StaticString(message.clone()), ); } } 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 ee236df8eac..80dcb5db328 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,8 @@ 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, UserDefinedConstrainError}; +use super::ir::instruction::{ConstrainError, ErrorSelector, ErrorType}; +use super::ir::printer::try_to_extract_string_from_error_payload; use super::{ ir::{ dfg::DataFlowGraph, @@ -31,7 +32,7 @@ use crate::ssa::ir::function::InlineType; pub(crate) use acir_ir::generated_acir::GeneratedAcir; use acvm::acir::circuit::brillig::BrilligBytecode; -use acvm::acir::circuit::OpcodeLocation; +use acvm::acir::circuit::{AssertionPayload, OpcodeLocation}; use acvm::acir::native_types::Witness; use acvm::acir::BlackBoxFunc; use acvm::{ @@ -276,13 +277,16 @@ impl AcirValue { } } +pub(crate) type Artifacts = + (Vec, Vec, BTreeMap); + impl Ssa { #[tracing::instrument(level = "trace", skip_all)] pub(crate) fn into_acir( self, brillig: &Brillig, abi_distinctness: Distinctness, - ) -> Result<(Vec, Vec), RuntimeError> { + ) -> Result { let mut acirs = Vec::new(); // TODO: can we parallelise this? let mut shared_context = SharedContext::default(); @@ -353,7 +357,7 @@ impl Ssa { } Distinctness::DuplicationAllowed => {} } - Ok((acirs, brillig)) + Ok((acirs, brillig, self.error_selector_to_type)) } } @@ -611,24 +615,39 @@ impl<'a> Context<'a> { let lhs = self.convert_numeric_value(*lhs, dfg)?; let rhs = self.convert_numeric_value(*rhs, dfg)?; - let assert_message = if let Some(error) = assert_message { - match error.as_ref() { - ConstrainError::Intrinsic(string) - | ConstrainError::UserDefined(UserDefinedConstrainError::Static(string)) => { - Some(string.clone()) + let assert_payload = if let Some(error) = assert_message { + match error { + ConstrainError::Intrinsic(string) => { + Some(AssertionPayload::StaticString(string.clone())) } - ConstrainError::UserDefined(UserDefinedConstrainError::Dynamic( - call_instruction, - )) => { - self.convert_ssa_call(call_instruction, dfg, ssa, brillig, &[])?; - None + ConstrainError::UserDefined(error_selector, values) => { + if let Some(constant_string) = try_to_extract_string_from_error_payload( + *error_selector, + values, + dfg, + ) { + Some(AssertionPayload::StaticString(constant_string)) + } else { + let acir_vars: Vec<_> = values + .iter() + .map(|value| self.convert_value(*value, dfg)) + .collect(); + + let expressions_or_memory = + self.acir_context.vars_to_expressions_or_memory(&acir_vars)?; + + Some(AssertionPayload::Dynamic( + error_selector.to_u64(), + expressions_or_memory, + )) + } } } } else { None }; - self.acir_context.assert_eq_var(lhs, rhs, assert_message)?; + self.acir_context.assert_eq_var(lhs, rhs, assert_payload)?; } Instruction::Cast(value_id, _) => { let acir_var = self.convert_numeric_value(*value_id, dfg)?; @@ -2683,7 +2702,7 @@ mod test { let ssa = builder.finish(); - let (acir_functions, _) = ssa + let (acir_functions, _, _) = ssa .into_acir(&Brillig::default(), noirc_frontend::ast::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); // Expected result: @@ -2779,7 +2798,7 @@ mod test { let ssa = builder.finish(); - let (acir_functions, _) = ssa + let (acir_functions, _, _) = ssa .into_acir(&Brillig::default(), noirc_frontend::ast::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); // The expected result should look very similar to the abvoe test expect that the input witnesses of the `Call` @@ -2870,7 +2889,7 @@ mod test { let ssa = builder.finish(); - let (acir_functions, _) = ssa + let (acir_functions, _, _) = ssa .into_acir(&Brillig::default(), noirc_frontend::ast::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); @@ -2982,9 +3001,8 @@ mod test { let ssa = builder.finish(); let brillig = ssa.to_brillig(false); - println!("{}", ssa); - let (acir_functions, brillig_functions) = ssa + let (acir_functions, brillig_functions, _) = ssa .into_acir(&brillig, noirc_frontend::ast::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); @@ -3041,7 +3059,7 @@ mod test { // The Brillig bytecode we insert for the stdlib is hardcoded so we do not need to provide any // Brillig artifacts to the ACIR gen pass. - let (acir_functions, brillig_functions) = ssa + let (acir_functions, brillig_functions, _) = ssa .into_acir(&Brillig::default(), noirc_frontend::ast::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); @@ -3113,7 +3131,7 @@ mod test { let brillig = ssa.to_brillig(false); println!("{}", ssa); - let (acir_functions, brillig_functions) = ssa + let (acir_functions, brillig_functions, _) = ssa .into_acir(&brillig, noirc_frontend::ast::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); @@ -3200,7 +3218,7 @@ mod test { let brillig = ssa.to_brillig(false); println!("{}", ssa); - let (acir_functions, brillig_functions) = ssa + let (acir_functions, brillig_functions, _) = ssa .into_acir(&brillig, noirc_frontend::ast::Distinctness::Distinct) .expect("Should compile manually written SSA into ACIR"); 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 75a427397b6..e149769f786 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 @@ -1,6 +1,6 @@ pub(crate) mod data_bus; -use std::{borrow::Cow, rc::Rc}; +use std::{borrow::Cow, collections::BTreeMap, rc::Rc}; use acvm::FieldElement; use noirc_errors::Location; @@ -18,7 +18,7 @@ use super::{ basic_block::BasicBlock, dfg::{CallStack, InsertInstructionResult}, function::{InlineType, RuntimeType}, - instruction::{ConstrainError, InstructionId, Intrinsic}, + instruction::{ConstrainError, ErrorSelector, ErrorType, InstructionId, Intrinsic}, }, ssa_gen::Ssa, }; @@ -35,6 +35,7 @@ pub(crate) struct FunctionBuilder { current_block: BasicBlockId, finished_functions: Vec, call_stack: CallStack, + error_types: BTreeMap, } impl FunctionBuilder { @@ -50,6 +51,7 @@ impl FunctionBuilder { current_function: new_function, finished_functions: Vec::new(), call_stack: CallStack::new(), + error_types: BTreeMap::default(), } } @@ -99,7 +101,7 @@ impl FunctionBuilder { /// Consume the FunctionBuilder returning all the functions it has generated. pub(crate) fn finish(mut self) -> Ssa { self.finished_functions.push(self.current_function); - Ssa::new(self.finished_functions) + Ssa::new(self.finished_functions, self.error_types) } /// Add a parameter to the current function with the given parameter type. @@ -263,7 +265,7 @@ impl FunctionBuilder { &mut self, lhs: ValueId, rhs: ValueId, - assert_message: Option>, + assert_message: Option, ) { self.insert_instruction(Instruction::Constrain(lhs, rhs, assert_message), None); } @@ -474,6 +476,10 @@ impl FunctionBuilder { } } } + + pub(crate) fn record_error_type(&mut self, selector: ErrorSelector, typ: ErrorType) { + self.error_types.insert(selector, typ); + } } impl std::ops::Index for FunctionBuilder { 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 04f33d528cd..582e00b6be2 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 @@ -1,5 +1,12 @@ -use acvm::{acir::BlackBoxFunc, FieldElement}; +use std::hash::{Hash, Hasher}; + +use acvm::{ + acir::{circuit::STRING_ERROR_SELECTOR, BlackBoxFunc}, + FieldElement, +}; +use fxhash::FxHasher; use iter_extended::vecmap; +use noirc_frontend::hir_def::types::Type as HirType; use super::{ basic_block::BasicBlockId, @@ -157,7 +164,7 @@ pub(crate) enum Instruction { Truncate { value: ValueId, bit_size: u32, max_bit_size: u32 }, /// Constrains two values to be equal to one another. - Constrain(ValueId, ValueId, Option>), + Constrain(ValueId, ValueId, Option), /// Range constrain `value` to `max_bit_size` RangeCheck { value: ValueId, max_bit_size: u32, assert_message: Option }, @@ -346,12 +353,12 @@ impl Instruction { // Must map the `lhs` and `rhs` first as the value `f` is moved with the closure let lhs = f(*lhs); let rhs = f(*rhs); - let assert_message = assert_message.as_ref().map(|error| match error.as_ref() { - ConstrainError::UserDefined(UserDefinedConstrainError::Dynamic(call_instr)) => { - let new_instr = call_instr.map_values(f); - Box::new(ConstrainError::UserDefined(UserDefinedConstrainError::Dynamic( - new_instr, - ))) + let assert_message = assert_message.as_ref().map(|error| match error { + ConstrainError::UserDefined(selector, payload_values) => { + ConstrainError::UserDefined( + *selector, + payload_values.iter().map(|&value| f(value)).collect(), + ) } _ => error.clone(), }); @@ -412,13 +419,10 @@ impl Instruction { Instruction::Constrain(lhs, rhs, assert_error) => { f(*lhs); f(*rhs); - if let Some(error) = assert_error.as_ref() { - if let ConstrainError::UserDefined(UserDefinedConstrainError::Dynamic( - call_instr, - )) = error.as_ref() - { - call_instr.for_each_value(f); - } + if let Some(ConstrainError::UserDefined(_, values)) = assert_error.as_ref() { + values.iter().for_each(|&val| { + f(val); + }); } } @@ -599,22 +603,36 @@ impl Instruction { } } +pub(crate) type ErrorType = HirType; + +#[derive(Debug, Copy, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)] +pub(crate) struct ErrorSelector(u64); + +impl ErrorSelector { + pub(crate) fn new(typ: &ErrorType) -> Self { + match typ { + ErrorType::String(_) => Self(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); + Self(hash) + } + } + } + + pub(crate) fn to_u64(self) -> u64 { + self.0 + } +} + #[derive(Debug, PartialEq, Eq, Hash, Clone)] pub(crate) enum ConstrainError { // These are errors which have been hardcoded during SSA gen Intrinsic(String), // These are errors issued by the user - UserDefined(UserDefinedConstrainError), -} - -#[derive(Debug, PartialEq, Eq, Hash, Clone)] -pub(crate) enum UserDefinedConstrainError { - // These are errors which come from static strings specified by a Noir program - Static(String), - // These are errors which come from runtime expressions specified by a Noir program - // We store an `Instruction` as we want this Instruction to be atomic in SSA with - // a constrain instruction, and leave codegen of this instruction to lower level passes. - Dynamic(Instruction), + UserDefined(ErrorSelector, Vec), } impl From for ConstrainError { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/constrain.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/constrain.rs index b4198e2cfec..d844f350927 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/constrain.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction/constrain.rs @@ -7,7 +7,7 @@ use super::{Binary, BinaryOp, ConstrainError, DataFlowGraph, Instruction, Type, pub(super) fn decompose_constrain( lhs: ValueId, rhs: ValueId, - msg: &Option>, + msg: &Option, dfg: &mut DataFlowGraph, ) -> Vec { let lhs = dfg.resolve(lhs); 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 d17d2989341..bbff3cf8acb 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,16 +4,17 @@ use std::{ fmt::{Formatter, Result}, }; +use acvm::acir::circuit::STRING_ERROR_SELECTOR; use iter_extended::vecmap; use super::{ basic_block::BasicBlockId, + dfg::DataFlowGraph, function::Function, instruction::{ - ConstrainError, Instruction, InstructionId, TerminatorInstruction, - UserDefinedConstrainError, + ConstrainError, ErrorSelector, Instruction, InstructionId, TerminatorInstruction, }, - value::ValueId, + value::{Value, ValueId}, }; /// Helper function for Function's Display impl to pretty-print the function with the given formatter. @@ -63,7 +64,6 @@ pub(crate) fn display_block( /// Specialize displaying value ids so that if they refer to a numeric /// constant or a function we print those directly. fn value(function: &Function, id: ValueId) -> String { - use super::value::Value; let id = function.dfg.resolve(id); match &function.dfg[id] { Value::NumericConstant { constant, typ } => { @@ -159,7 +159,6 @@ fn display_instruction_inner( Instruction::Constrain(lhs, rhs, error) => { write!(f, "constrain {} == {}", show(*lhs), show(*rhs))?; if let Some(error) = error { - write!(f, " ")?; display_constrain_error(function, error, f) } else { writeln!(f) @@ -198,18 +197,51 @@ 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, + values: &[ValueId], + dfg: &DataFlowGraph, +) -> Option { + ((error_selector.to_u64() == STRING_ERROR_SELECTOR) && (values.len() == 1)) + .then_some(()) + .and_then(|()| { + let Value::Array { array: values, .. } = &dfg[values[0]] else { + return None; + }; + let fields: Option> = + values.iter().map(|value_id| dfg.get_numeric_constant(*value_id)).collect(); + + fields + }) + .map(|fields| { + fields + .iter() + .map(|field| { + let as_u8 = field.try_to_u64().unwrap_or_default() as u8; + as_u8 as char + }) + .collect() + }) +} + fn display_constrain_error( function: &Function, error: &ConstrainError, f: &mut Formatter, ) -> Result { match error { - ConstrainError::Intrinsic(assert_message_string) - | ConstrainError::UserDefined(UserDefinedConstrainError::Static(assert_message_string)) => { - writeln!(f, "{assert_message_string:?}") - } - ConstrainError::UserDefined(UserDefinedConstrainError::Dynamic(assert_message_call)) => { - display_instruction_inner(function, assert_message_call, f) + ConstrainError::Intrinsic(assert_message_string) => { + writeln!(f, " '{assert_message_string:?}'") + } + ConstrainError::UserDefined(selector, values) => { + if let Some(constant_string) = + try_to_extract_string_from_error_payload(*selector, values, &function.dfg) + { + writeln!(f, " '{}'", constant_string) + } else { + writeln!(f, ", data {}", value_list(function, values)) + } } } } diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs index aa0368cc2dd..cfeb8751f25 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/opt/defunctionalize.rs @@ -14,7 +14,7 @@ use crate::ssa::{ ir::{ basic_block::BasicBlockId, function::{Function, FunctionId, Signature}, - instruction::{BinaryOp, ConstrainError, Instruction, UserDefinedConstrainError}, + instruction::{BinaryOp, Instruction}, types::{NumericType, Type}, value::{Value, ValueId}, }, @@ -90,18 +90,6 @@ impl DefunctionalizationContext { Instruction::Call { func: target_func_id, arguments } => { (*target_func_id, arguments) } - // Constrain instruction potentially hold a call instruction themselves - // thus we need to account for them. - Instruction::Constrain(_, _, Some(constrain_error)) => { - if let ConstrainError::UserDefined(UserDefinedConstrainError::Dynamic( - Instruction::Call { func: target_func_id, arguments }, - )) = constrain_error.as_ref() - { - (*target_func_id, arguments) - } else { - continue; - } - } _ => continue, }; @@ -133,22 +121,7 @@ impl DefunctionalizationContext { } _ => {} } - if let Some(mut new_instruction) = replacement_instruction { - if let Instruction::Constrain(lhs, rhs, constrain_error_call) = instruction { - let new_error_call = if let Some(error) = constrain_error_call { - match error.as_ref() { - ConstrainError::UserDefined( - UserDefinedConstrainError::Dynamic(_), - ) => Some(Box::new(ConstrainError::UserDefined( - UserDefinedConstrainError::Dynamic(new_instruction), - ))), - _ => None, - } - } else { - None - }; - new_instruction = Instruction::Constrain(lhs, rhs, new_error_call); - } + if let Some(new_instruction) = replacement_instruction { func.dfg[instruction_id] = new_instruction; } } 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 79f7cda4ae2..8de09d4309b 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 @@ -8,10 +8,11 @@ use context::SharedContext; use iter_extended::{try_vecmap, vecmap}; use noirc_errors::Location; use noirc_frontend::ast::{UnaryOp, Visibility}; +use noirc_frontend::hir_def::types::Type as HirType; use noirc_frontend::monomorphization::ast::{self, Expression, Program}; use crate::{ - errors::{InternalError, RuntimeError}, + errors::RuntimeError, ssa::{ function_builder::data_bus::DataBusBuilder, ir::{function::InlineType, instruction::Intrinsic}, @@ -23,13 +24,12 @@ use self::{ value::{Tree, Values}, }; +use super::ir::instruction::ErrorSelector; use super::{ function_builder::data_bus::DataBus, ir::{ function::RuntimeType, - instruction::{ - BinaryOp, ConstrainError, Instruction, TerminatorInstruction, UserDefinedConstrainError, - }, + instruction::{BinaryOp, ConstrainError, TerminatorInstruction}, types::Type, value::ValueId, }, @@ -151,8 +151,8 @@ impl<'a> FunctionContext<'a> { } Expression::Call(call) => self.codegen_call(call), Expression::Let(let_expr) => self.codegen_let(let_expr), - Expression::Constrain(expr, location, assert_message) => { - self.codegen_constrain(expr, *location, assert_message) + Expression::Constrain(expr, location, assert_payload) => { + self.codegen_constrain(expr, *location, assert_payload) } Expression::Assign(assign) => self.codegen_assign(assign), Expression::Semi(semi) => self.codegen_semi(semi), @@ -453,7 +453,7 @@ impl<'a> FunctionContext<'a> { self.builder.insert_constrain( is_offset_out_of_bounds, true_const, - Some(Box::new("Index out of bounds".to_owned().into())), + Some("Index out of bounds".to_owned().into()), ); } @@ -680,7 +680,7 @@ impl<'a> FunctionContext<'a> { &mut self, expr: &Expression, location: Location, - assert_message: &Option>, + assert_payload: &Option>, ) -> Result { let expr = self.codegen_non_tuple_expression(expr)?; let true_literal = self.builder.numeric_constant(true, Type::bool()); @@ -688,9 +688,9 @@ impl<'a> FunctionContext<'a> { // Set the location here for any errors that may occur when we codegen the assert message self.builder.set_location(location); - let assert_message = self.codegen_constrain_error(assert_message)?; + let assert_payload = self.codegen_constrain_error(assert_payload)?; - self.builder.insert_constrain(expr, true_literal, assert_message); + self.builder.insert_constrain(expr, true_literal, assert_payload); Ok(Self::unit_value()) } @@ -701,42 +701,23 @@ impl<'a> FunctionContext<'a> { // inserted to the SSA as we want that instruction to be atomic in SSA with a constrain instruction. fn codegen_constrain_error( &mut self, - assert_message: &Option>, - ) -> Result>, RuntimeError> { - let Some(assert_message_expr) = assert_message else { return Ok(None) }; + assert_message: &Option>, + ) -> Result, RuntimeError> { + let Some(assert_message_payload) = assert_message else { return Ok(None) }; + let (assert_message_expression, assert_message_typ) = assert_message_payload.as_ref(); - if let ast::Expression::Literal(ast::Literal::Str(assert_message)) = - assert_message_expr.as_ref() - { - return Ok(Some(Box::new(ConstrainError::UserDefined( - UserDefinedConstrainError::Static(assert_message.to_string()), - )))); - } + let values = self.codegen_expression(assert_message_expression)?.into_value_list(self); + + let error_type_id = ErrorSelector::new(assert_message_typ); - let ast::Expression::Call(call) = assert_message_expr.as_ref() else { - return Err(InternalError::Unexpected { - expected: "Expected a call expression".to_owned(), - found: "Instead found {expr:?}".to_owned(), - call_stack: self.builder.get_call_stack(), + // 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()); } - .into()); }; - - let func = self.codegen_non_tuple_expression(&call.func)?; - let mut arguments = Vec::with_capacity(call.arguments.len()); - - for argument in &call.arguments { - let mut values = self.codegen_expression(argument)?.into_value_list(self); - arguments.append(&mut values); - } - - // If an array is passed as an argument we increase its reference count - for argument in &arguments { - self.builder.increment_array_reference_count(*argument); - } - - let instr = Instruction::Call { func, arguments }; - Ok(Some(Box::new(ConstrainError::UserDefined(UserDefinedConstrainError::Dynamic(instr))))) + Ok(Some(ConstrainError::UserDefined(error_type_id, values))) } fn codegen_assign(&mut self, assign: &ast::Assign) -> Result { diff --git a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs index b05a2cbc741..03d4ac05bd9 100644 --- a/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs +++ b/noir/noir-repo/compiler/noirc_evaluator/src/ssa/ssa_gen/program.rs @@ -4,8 +4,10 @@ use iter_extended::btree_map; use crate::ssa::ir::{ function::{Function, FunctionId, RuntimeType}, + instruction::ErrorSelector, map::AtomicCounter, }; +use noirc_frontend::hir_def::types::Type as HirType; /// Contains the entire SSA representation of the program. pub(crate) struct Ssa { @@ -17,12 +19,16 @@ pub(crate) struct Ssa { /// This mapping is necessary to use the correct function pointer for an ACIR call, /// as the final program artifact will be a list of only entry point functions. pub(crate) entry_point_to_generated_index: BTreeMap, + pub(crate) error_selector_to_type: BTreeMap, } impl Ssa { /// Create a new Ssa object from the given SSA functions. /// The first function in this vector is expected to be the main function. - pub(crate) fn new(functions: Vec) -> Self { + pub(crate) fn new( + functions: Vec, + error_types: BTreeMap, + ) -> Self { let main_id = functions.first().expect("Expected at least 1 SSA function").id(); let mut max_id = main_id; @@ -50,6 +56,7 @@ impl Ssa { main_id, next_id: AtomicCounter::starting_after(max_id), entry_point_to_generated_index, + error_selector_to_type: error_types, } } diff --git a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/resolver.rs b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/resolver.rs index 8e29f8f9fce..2f2677def88 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/resolver.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/hir/resolution/resolver.rs @@ -1205,15 +1205,14 @@ impl<'a> Resolver<'a> { }) } StatementKind::Constrain(constrain_stmt) => { - let span = constrain_stmt.0.span; - let assert_msg_call_expr_id = - self.resolve_assert_message(constrain_stmt.1, span, constrain_stmt.0.clone()); let expr_id = self.resolve_expression(constrain_stmt.0); + let assert_message_expr_id = + constrain_stmt.1.map(|assert_expr_id| self.resolve_expression(assert_expr_id)); HirStatement::Constrain(HirConstrainStatement( expr_id, self.file, - assert_msg_call_expr_id, + assert_message_expr_id, )) } StatementKind::Expression(expr) => { @@ -1279,48 +1278,6 @@ impl<'a> Resolver<'a> { } } - fn resolve_assert_message( - &mut self, - assert_message_expr: Option, - span: Span, - condition: Expression, - ) -> Option { - let assert_message_expr = assert_message_expr?; - - if matches!( - assert_message_expr, - Expression { kind: ExpressionKind::Literal(Literal::Str(..)), .. } - ) { - return Some(self.resolve_expression(assert_message_expr)); - } - - let is_in_stdlib = self.path_resolver.module_id().krate.is_stdlib(); - let assert_msg_call_path = if is_in_stdlib { - ExpressionKind::Variable(Path { - segments: vec![Ident::from("internal"), Ident::from("resolve_assert_message")], - kind: PathKind::Crate, - span, - }) - } else { - ExpressionKind::Variable(Path { - segments: vec![ - Ident::from("std"), - Ident::from("internal"), - Ident::from("resolve_assert_message"), - ], - kind: PathKind::Dep, - span, - }) - }; - let assert_msg_call_args = vec![assert_message_expr.clone(), condition]; - let assert_msg_call_expr = Expression::call( - Expression { kind: assert_msg_call_path, span }, - assert_msg_call_args, - span, - ); - Some(self.resolve_expression(assert_msg_call_expr)) - } - pub fn intern_stmt(&mut self, stmt: Statement) -> StmtId { let hir_stmt = self.resolve_stmt(stmt.kind, stmt.span); let id = self.interner.push_stmt(hir_stmt); diff --git a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/ast.rs b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/ast.rs index 434cc922a05..ef3af57d303 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/ast.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/ast.rs @@ -6,7 +6,7 @@ use noirc_errors::{ }; use crate::ast::{BinaryOpKind, Distinctness, IntegerBitSize, Signedness, Visibility}; -use crate::hir_def::function::FunctionSignature; +use crate::hir_def::{function::FunctionSignature, types::Type as HirType}; /// The monomorphized AST is expression-based, all statements are also /// folded into this expression enum. Compared to the HIR, the monomorphized @@ -33,7 +33,7 @@ pub enum Expression { ExtractTupleField(Box, usize), Call(Call), Let(Let), - Constrain(Box, Location, Option>), + Constrain(Box, Location, Option>), Assign(Assign), Semi(Box), Break, diff --git a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs index b130af9d125..d7213667c48 100644 --- a/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs +++ b/noir/noir-repo/compiler/noirc_frontend/src/monomorphization/mod.rs @@ -593,7 +593,11 @@ impl<'interner> Monomorphizer<'interner> { let location = self.interner.expr_location(&constrain.0); let assert_message = constrain .2 - .map(|assert_msg_expr| self.expr(assert_msg_expr)) + .map(|assert_msg_expr| { + self.expr(assert_msg_expr).map(|expr| { + (expr, self.interner.id_type(assert_msg_expr).follow_bindings()) + }) + }) .transpose()? .map(Box::new); Ok(ast::Expression::Constrain(Box::new(expr), location, assert_message)) @@ -1100,9 +1104,6 @@ impl<'interner> Monomorphizer<'interner> { // The first argument to the `print` oracle is a bool, indicating a newline to be inserted at the end of the input // The second argument is expected to always be an ident self.append_printable_type_info(&hir_arguments[1], &mut arguments); - } else if name.as_str() == "assert_message" { - // The first argument to the `assert_message` oracle is the expression passed as a message to an `assert` or `assert_eq` statement - self.append_printable_type_info(&hir_arguments[0], &mut arguments); } } } diff --git a/noir/noir-repo/noir_stdlib/src/internal.nr b/noir/noir-repo/noir_stdlib/src/internal.nr deleted file mode 100644 index 8d5c01dda7f..00000000000 --- a/noir/noir-repo/noir_stdlib/src/internal.nr +++ /dev/null @@ -1,12 +0,0 @@ -// This file contains functions which should only be used in calls injected by the Noir compiler. -// These functions should not be called manually in user code. -// -// Changes to this file will not be considered breaking. - -#[oracle(assert_message)] -unconstrained fn assert_message_oracle(_input: T) {} -unconstrained pub fn resolve_assert_message(input: T, condition: bool) { - if !condition { - assert_message_oracle(input); - } -} diff --git a/noir/noir-repo/noir_stdlib/src/lib.nr b/noir/noir-repo/noir_stdlib/src/lib.nr index 90c04472066..ebde4b88858 100644 --- a/noir/noir-repo/noir_stdlib/src/lib.nr +++ b/noir/noir-repo/noir_stdlib/src/lib.nr @@ -26,7 +26,6 @@ mod default; mod prelude; mod uint128; mod bigint; -mod internal; // Oracle calls are required to be wrapped in an unconstrained function // Thus, the only argument to the `println` oracle is expected to always be an ident diff --git a/noir/noir-repo/test_programs/noir_test_success/should_fail_with_matches/src/main.nr b/noir/noir-repo/test_programs/noir_test_success/should_fail_with_matches/src/main.nr index d2b7d155a32..56ee5cfa586 100644 --- a/noir/noir-repo/test_programs/noir_test_success/should_fail_with_matches/src/main.nr +++ b/noir/noir-repo/test_programs/noir_test_success/should_fail_with_matches/src/main.nr @@ -17,3 +17,67 @@ fn test_should_fail_with_runtime_match() { fn test_should_fail_without_runtime_match() { assert_eq(dep::std::hash::pedersen_commitment([27]).x, 0); } + +struct InvalidPointError { + point: dep::std::hash::PedersenPoint, +} + +#[test(should_fail_with = "InvalidPointError { point: PedersenPoint { x: 0x1cea3a116d01eb94d568ef04c3dfbc39f96f015ed801ab8958e360d406503ce0, y: 0x2721b237df87234acc36a238b8f231a3d31d18fe3845fff4cc59f0bd873818f8 } }")] +fn test_should_fail_with_struct() { + let hash = dep::std::hash::pedersen_commitment([27]); + assert_eq(hash.x, 0, InvalidPointError { point: hash }); +} + +#[test(should_fail_with = "A: 0x00 is not 1!")] +fn test_should_fail_with_basic_type_fmt_string() { + let a = 0; + let b = 1; + assert_eq(a, b, f"A: {a} is not 1!"); +} + +#[test(should_fail_with = "Invalid hash: PedersenPoint { x: 0x1cea3a116d01eb94d568ef04c3dfbc39f96f015ed801ab8958e360d406503ce0, y: 0x2721b237df87234acc36a238b8f231a3d31d18fe3845fff4cc59f0bd873818f8 }")] +fn test_should_fail_with_struct_fmt_string() { + let hash = dep::std::hash::pedersen_commitment([27]); + assert_eq(hash.x, 0, f"Invalid hash: {hash}"); +} + +// Also test unconstrained versions + +#[test(should_fail_with = "Not equal")] +unconstrained fn unconstrained_test_should_fail_with_match() { + assert_eq(0, 1, "Not equal"); +} + +#[test(should_fail)] +unconstrained fn unconstrained_test_should_fail_without_match() { + assert_eq(0, 1); +} + +#[test(should_fail_with = "Not equal")] +unconstrained fn unconstrained_test_should_fail_with_runtime_match() { + assert_eq(dep::std::hash::pedersen_commitment([27]).x, 0, "Not equal"); +} + +#[test(should_fail)] +unconstrained fn unconstrained_test_should_fail_without_runtime_match() { + assert_eq(dep::std::hash::pedersen_commitment([27]).x, 0); +} + +#[test(should_fail_with = "InvalidPointError { point: PedersenPoint { x: 0x1cea3a116d01eb94d568ef04c3dfbc39f96f015ed801ab8958e360d406503ce0, y: 0x2721b237df87234acc36a238b8f231a3d31d18fe3845fff4cc59f0bd873818f8 } }")] +unconstrained fn unconstrained_test_should_fail_with_struct() { + let hash = dep::std::hash::pedersen_commitment([27]); + assert_eq(hash.x, 0, InvalidPointError { point: hash }); +} + +#[test(should_fail_with = "A: 0x00 is not 1!")] +unconstrained fn unconstrained_test_should_fail_with_basic_type_fmt_string() { + let a = 0; + let b = 1; + assert_eq(a, b, f"A: {a} is not 1!"); +} + +#[test(should_fail_with = "Invalid hash: PedersenPoint { x: 0x1cea3a116d01eb94d568ef04c3dfbc39f96f015ed801ab8958e360d406503ce0, y: 0x2721b237df87234acc36a238b8f231a3d31d18fe3845fff4cc59f0bd873818f8 }")] +unconstrained fn unconstrained_test_should_fail_with_struct_fmt_string() { + let hash = dep::std::hash::pedersen_commitment([27]); + assert_eq(hash.x, 0, f"Invalid hash: {hash}"); +} diff --git a/noir/noir-repo/tooling/debugger/src/context.rs b/noir/noir-repo/tooling/debugger/src/context.rs index a423016eacf..ea32c864a0b 100644 --- a/noir/noir-repo/tooling/debugger/src/context.rs +++ b/noir/noir-repo/tooling/debugger/src/context.rs @@ -2,7 +2,6 @@ use crate::foreign_calls::DebugForeignCallExecutor; use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::acir::circuit::{Circuit, Opcode, OpcodeLocation}; use acvm::acir::native_types::{Witness, WitnessMap}; -use acvm::brillig_vm::brillig::ForeignCallResult; use acvm::brillig_vm::MemoryValue; use acvm::pwg::{ ACVMStatus, BrilligSolver, BrilligSolverStatus, ForeignCallWaitInfo, StepResult, ACVM, @@ -54,6 +53,7 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { &circuit.opcodes, initial_witness, unconstrained_functions, + &circuit.assert_messages, ), brillig_solver: None, foreign_call_executor, @@ -356,9 +356,6 @@ impl<'a, B: BlackBoxFunctionSolver> DebugContext<'a, B> { let foreign_call_result = self.foreign_call_executor.execute(&foreign_call); match foreign_call_result { Ok(foreign_call_result) => { - let foreign_call_result = foreign_call_result - .get_brillig_output() - .unwrap_or(ForeignCallResult::default()); if let Some(mut solver) = self.brillig_solver.take() { solver.resolve_pending_foreign_call(foreign_call_result); self.brillig_solver = Some(solver); diff --git a/noir/noir-repo/tooling/debugger/src/foreign_calls.rs b/noir/noir-repo/tooling/debugger/src/foreign_calls.rs index f11ac22cd75..209439f5f92 100644 --- a/noir/noir-repo/tooling/debugger/src/foreign_calls.rs +++ b/noir/noir-repo/tooling/debugger/src/foreign_calls.rs @@ -5,7 +5,7 @@ use acvm::{ }; use nargo::{ artifacts::debug::{DebugArtifact, DebugVars, StackFrame}, - ops::{DefaultForeignCallExecutor, ForeignCallExecutor, NargoForeignCallResult}, + ops::{DefaultForeignCallExecutor, ForeignCallExecutor}, }; use noirc_errors::debug_info::{DebugFnId, DebugVarId}; use noirc_printable_type::ForeignCallError; @@ -94,7 +94,7 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { fn execute( &mut self, foreign_call: &ForeignCallWaitInfo, - ) -> Result { + ) -> Result { let foreign_call_name = foreign_call.function.as_str(); match DebugForeignCall::lookup(foreign_call_name) { Some(DebugForeignCall::VarAssign) => { @@ -105,7 +105,7 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { foreign_call.inputs[1..].iter().flat_map(|x| x.fields()).collect(); self.debug_vars.assign_var(var_id, &values); } - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } Some(DebugForeignCall::VarDrop) => { let fcp_var_id = &foreign_call.inputs[0]; @@ -113,7 +113,7 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { let var_id = debug_var_id(var_id_value); self.debug_vars.drop_var(var_id); } - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } Some(DebugForeignCall::MemberAssign(arity)) => { if let Some(ForeignCallParam::Single(var_id_value)) = foreign_call.inputs.first() { @@ -141,7 +141,7 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { .collect(); self.debug_vars.assign_field(var_id, indexes, &values); } - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } Some(DebugForeignCall::DerefAssign) => { let fcp_var_id = &foreign_call.inputs[0]; @@ -150,7 +150,7 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { let var_id = debug_var_id(var_id_value); self.debug_vars.assign_deref(var_id, &fcp_value.fields()); } - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } Some(DebugForeignCall::FnEnter) => { let fcp_fn_id = &foreign_call.inputs[0]; @@ -159,11 +159,11 @@ impl ForeignCallExecutor for DefaultDebugForeignCallExecutor { }; let fn_id = debug_fn_id(fn_id_value); self.debug_vars.push_fn(fn_id); - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } Some(DebugForeignCall::FnExit) => { self.debug_vars.pop_fn(); - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } None => self.executor.execute(foreign_call), } diff --git a/noir/noir-repo/tooling/nargo/src/errors.rs b/noir/noir-repo/tooling/nargo/src/errors.rs index ac03330a7c8..4b0c29a9b1b 100644 --- a/noir/noir-repo/tooling/nargo/src/errors.rs +++ b/noir/noir-repo/tooling/nargo/src/errors.rs @@ -1,7 +1,11 @@ +use std::collections::BTreeMap; + use acvm::{ - acir::circuit::{OpcodeLocation, ResolvedOpcodeLocation}, + acir::circuit::{OpcodeLocation, ResolvedAssertionPayload, ResolvedOpcodeLocation}, pwg::{ErrorLocation, OpcodeResolutionError}, + FieldElement, }; +use noirc_abi::{Abi, AbiErrorType}; use noirc_errors::{ debug_info::DebugInfo, reporter::ReportedErrors, CustomDiagnostic, FileDiagnostic, }; @@ -9,7 +13,9 @@ use noirc_errors::{ pub use noirc_errors::Location; use noirc_frontend::graph::CrateName; -use noirc_printable_type::ForeignCallError; +use noirc_printable_type::{ + decode_value, ForeignCallError, PrintableType, PrintableValue, PrintableValueDisplay, +}; use thiserror::Error; /// Errors covering situations where a package cannot be compiled. @@ -53,24 +59,34 @@ impl NargoError { /// /// We want to extract the user defined error so that we can compare it /// in tests to expected failure messages - pub fn user_defined_failure_message(&self) -> Option<&str> { + pub fn user_defined_failure_message( + &self, + error_types: &BTreeMap, + ) -> Option { let execution_error = match self { NargoError::ExecutionError(error) => error, _ => return None, }; match execution_error { - ExecutionError::AssertionFailed(message, _) => Some(message), + ExecutionError::AssertionFailed(payload, _) => match payload { + ResolvedAssertionPayload::String(message) => Some(message.to_string()), + ResolvedAssertionPayload::Raw(error_selector, fields) => { + let abi_type = error_types.get(error_selector)?; + let decoded = prepare_for_display(fields, abi_type.clone()); + Some(decoded.to_string()) + } + }, ExecutionError::SolvingError(error, _) => match error { OpcodeResolutionError::IndexOutOfBounds { .. } | OpcodeResolutionError::OpcodeNotSolvable(_) | OpcodeResolutionError::UnsatisfiedConstrain { .. } | OpcodeResolutionError::AcirMainCallAttempted { .. } + | OpcodeResolutionError::BrilligFunctionFailed { .. } | OpcodeResolutionError::AcirCallOutputsMismatch { .. } => None, - OpcodeResolutionError::BrilligFunctionFailed { message, .. } => { - message.as_ref().map(|s| s.as_str()) + OpcodeResolutionError::BlackBoxFunctionFailed(_, reason) => { + Some(reason.to_string()) } - OpcodeResolutionError::BlackBoxFunctionFailed(_, reason) => Some(reason), }, } } @@ -78,8 +94,8 @@ impl NargoError { #[derive(Debug, Error)] pub enum ExecutionError { - #[error("Failed assertion: '{}'", .0)] - AssertionFailed(String, Vec), + #[error("Failed assertion")] + AssertionFailed(ResolvedAssertionPayload, Vec), #[error("Failed to solve program: '{}'", .0)] SolvingError(OpcodeResolutionError, Option>), @@ -101,7 +117,7 @@ fn extract_locations_from_error( acir_call_stack, ) | ExecutionError::SolvingError( - OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: error_location }, + OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: error_location, .. }, acir_call_stack, ) => match error_location { ErrorLocation::Unresolved => { @@ -144,11 +160,52 @@ fn extract_locations_from_error( ) } -fn extract_message_from_error(nargo_err: &NargoError) -> String { +fn prepare_for_display(fields: &[FieldElement], error_type: AbiErrorType) -> PrintableValueDisplay { + match error_type { + AbiErrorType::FmtString { length, item_types } => { + let mut fields_iter = fields.iter().copied(); + let PrintableValue::String(string) = + decode_value(&mut fields_iter, &PrintableType::String { length }) + else { + unreachable!("Got non-string from string decoding"); + }; + let _length_of_items = fields_iter.next(); + let items = item_types.into_iter().map(|abi_type| { + let printable_typ = (&abi_type).into(); + let decoded = decode_value(&mut fields_iter, &printable_typ); + (decoded, printable_typ) + }); + PrintableValueDisplay::FmtString(string, items.collect()) + } + AbiErrorType::Custom(abi_typ) => { + let printable_type = (&abi_typ).into(); + let decoded = decode_value(&mut fields.iter().copied(), &printable_type); + PrintableValueDisplay::Plain(decoded, printable_type) + } + } +} + +fn extract_message_from_error( + error_types: &BTreeMap, + nargo_err: &NargoError, +) -> String { match nargo_err { - NargoError::ExecutionError(ExecutionError::AssertionFailed(message, _)) => { + NargoError::ExecutionError(ExecutionError::AssertionFailed( + ResolvedAssertionPayload::String(message), + _, + )) => { format!("Assertion failed: '{message}'") } + NargoError::ExecutionError(ExecutionError::AssertionFailed( + ResolvedAssertionPayload::Raw(error_selector, fields), + .., + )) => { + if let Some(error_type) = error_types.get(error_selector) { + format!("Assertion failed: {}", prepare_for_display(fields, error_type.clone())) + } else { + "Assertion failed".to_string() + } + } NargoError::ExecutionError(ExecutionError::SolvingError( OpcodeResolutionError::IndexOutOfBounds { index, array_size, .. }, _, @@ -166,6 +223,7 @@ fn extract_message_from_error(nargo_err: &NargoError) -> String { /// Tries to generate a runtime diagnostic from a nargo error. It will successfully do so if it's a runtime error with a call stack. pub fn try_to_diagnose_runtime_error( nargo_err: &NargoError, + abi: &Abi, debug: &[DebugInfo], ) -> Option { let source_locations = match nargo_err { @@ -177,7 +235,7 @@ pub fn try_to_diagnose_runtime_error( // The location of the error itself will be the location at the top // of the call stack (the last item in the Vec). let location = source_locations.last()?; - let message = extract_message_from_error(nargo_err); + let message = extract_message_from_error(&abi.error_types, nargo_err); Some( CustomDiagnostic::simple_error(message, String::new(), location.span) .in_file(location.file) diff --git a/noir/noir-repo/tooling/nargo/src/ops/execute.rs b/noir/noir-repo/tooling/nargo/src/ops/execute.rs index 34755d14ed2..4a75212ba47 100644 --- a/noir/noir-repo/tooling/nargo/src/ops/execute.rs +++ b/noir/noir-repo/tooling/nargo/src/ops/execute.rs @@ -1,7 +1,6 @@ use acvm::acir::circuit::brillig::BrilligBytecode; use acvm::acir::circuit::{OpcodeLocation, Program, ResolvedOpcodeLocation}; use acvm::acir::native_types::WitnessStack; -use acvm::brillig_vm::brillig::ForeignCallResult; use acvm::pwg::{ACVMStatus, ErrorLocation, OpcodeNotSolvable, OpcodeResolutionError, ACVM}; use acvm::BlackBoxFunctionSolver; use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap}; @@ -9,7 +8,7 @@ use acvm::{acir::circuit::Circuit, acir::native_types::WitnessMap}; use crate::errors::ExecutionError; use crate::NargoError; -use super::foreign_calls::{ForeignCallExecutor, NargoForeignCallResult}; +use super::foreign_calls::ForeignCallExecutor; struct ProgramExecutor<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> { functions: &'a [Circuit], @@ -63,10 +62,9 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, &circuit.opcodes, initial_witness, self.unconstrained_functions, + &circuit.assert_messages, ); - // This message should be resolved by a nargo foreign call only when we have an unsatisfied assertion. - let mut assert_message: Option = None; loop { let solver_status = acvm.solve(); @@ -79,6 +77,7 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, let call_stack = match &error { OpcodeResolutionError::UnsatisfiedConstrain { opcode_location: ErrorLocation::Resolved(opcode_location), + .. } | OpcodeResolutionError::IndexOutOfBounds { opcode_location: ErrorLocation::Resolved(opcode_location), @@ -103,55 +102,25 @@ impl<'a, B: BlackBoxFunctionSolver, F: ForeignCallExecutor> ProgramExecutor<'a, _ => None, }; - return Err(NargoError::ExecutionError(match call_stack { - Some(call_stack) => { - // First check whether we have a runtime assertion message that should be resolved on an ACVM failure - // If we do not have a runtime assertion message, we check whether the error is a brillig error with a user-defined message, - // and finally we should check whether the circuit has any hardcoded messages associated with a specific `OpcodeLocation`. - // Otherwise return the provided opcode resolution error. - if let Some(assert_message) = assert_message { - ExecutionError::AssertionFailed( - assert_message.to_owned(), - call_stack, - ) - } else if let OpcodeResolutionError::BrilligFunctionFailed { - message: Some(message), - .. - } = &error - { - ExecutionError::AssertionFailed(message.to_owned(), call_stack) - } else if let Some(assert_message) = circuit.get_assert_message( - call_stack - .last() - .expect("Call stacks should not be empty") - .opcode_location, - ) { - ExecutionError::AssertionFailed( - assert_message.to_owned(), - call_stack, - ) - } else { - ExecutionError::SolvingError(error, Some(call_stack)) - } + let assertion_payload = match &error { + OpcodeResolutionError::BrilligFunctionFailed { payload, .. } + | OpcodeResolutionError::UnsatisfiedConstrain { payload, .. } => { + payload.clone() } - None => ExecutionError::SolvingError(error, None), + _ => None, + }; + + return Err(NargoError::ExecutionError(match assertion_payload { + Some(payload) => ExecutionError::AssertionFailed( + payload, + call_stack.expect("Should have call stack for an assertion failure"), + ), + None => ExecutionError::SolvingError(error, call_stack), })); } ACVMStatus::RequiresForeignCall(foreign_call) => { let foreign_call_result = self.foreign_call_executor.execute(&foreign_call)?; - match foreign_call_result { - NargoForeignCallResult::BrilligOutput(foreign_call_result) => { - acvm.resolve_pending_foreign_call(foreign_call_result); - } - NargoForeignCallResult::ResolvedAssertMessage(message) => { - if assert_message.is_some() { - unreachable!("Resolving an assert message should happen only once as the VM should have failed"); - } - assert_message = Some(message); - - acvm.resolve_pending_foreign_call(ForeignCallResult::default()); - } - } + acvm.resolve_pending_foreign_call(foreign_call_result); } ACVMStatus::RequiresAcirCall(call_info) => { // Store the parent function index whose context we are currently executing diff --git a/noir/noir-repo/tooling/nargo/src/ops/foreign_calls.rs b/noir/noir-repo/tooling/nargo/src/ops/foreign_calls.rs index 33767314a37..90f6659ad28 100644 --- a/noir/noir-repo/tooling/nargo/src/ops/foreign_calls.rs +++ b/noir/noir-repo/tooling/nargo/src/ops/foreign_calls.rs @@ -10,69 +10,13 @@ pub trait ForeignCallExecutor { fn execute( &mut self, foreign_call: &ForeignCallWaitInfo, - ) -> Result; -} - -#[derive(Debug, PartialEq, Eq, Clone)] -pub enum NargoForeignCallResult { - BrilligOutput(ForeignCallResult), - ResolvedAssertMessage(String), -} - -impl NargoForeignCallResult { - pub fn get_assert_message(self) -> Option { - match self { - Self::ResolvedAssertMessage(msg) => Some(msg), - _ => None, - } - } - - pub fn get_brillig_output(self) -> Option { - match self { - Self::BrilligOutput(foreign_call_result) => Some(foreign_call_result), - _ => None, - } - } -} - -impl From for NargoForeignCallResult { - fn from(value: ForeignCallResult) -> Self { - Self::BrilligOutput(value) - } -} - -impl From for NargoForeignCallResult { - fn from(value: String) -> Self { - Self::ResolvedAssertMessage(value) - } -} - -impl From for NargoForeignCallResult { - fn from(value: FieldElement) -> Self { - let foreign_call_result: ForeignCallResult = value.into(); - foreign_call_result.into() - } -} - -impl From> for NargoForeignCallResult { - fn from(values: Vec) -> Self { - let foreign_call_result: ForeignCallResult = values.into(); - foreign_call_result.into() - } -} - -impl From> for NargoForeignCallResult { - fn from(values: Vec) -> Self { - let foreign_call_result: ForeignCallResult = values.into(); - foreign_call_result.into() - } + ) -> Result; } /// This enumeration represents the Brillig foreign calls that are natively supported by nargo. /// After resolution of a foreign call, nargo will restart execution of the ACVM pub enum ForeignCall { Print, - AssertMessage, CreateMock, SetMockParams, GetMockLastParams, @@ -91,7 +35,6 @@ impl ForeignCall { pub(crate) fn name(&self) -> &'static str { match self { ForeignCall::Print => "print", - ForeignCall::AssertMessage => "assert_message", ForeignCall::CreateMock => "create_mock", ForeignCall::SetMockParams => "set_mock_params", ForeignCall::GetMockLastParams => "get_mock_last_params", @@ -104,7 +47,6 @@ impl ForeignCall { pub(crate) fn lookup(op_name: &str) -> Option { match op_name { "print" => Some(ForeignCall::Print), - "assert_message" => Some(ForeignCall::AssertMessage), "create_mock" => Some(ForeignCall::CreateMock), "set_mock_params" => Some(ForeignCall::SetMockParams), "get_mock_last_params" => Some(ForeignCall::GetMockLastParams), @@ -223,13 +165,6 @@ impl DefaultForeignCallExecutor { Ok(()) } - fn execute_assert_message( - foreign_call_inputs: &[ForeignCallParam], - ) -> Result { - let display_string = Self::format_printable_value(foreign_call_inputs, true)?; - Ok(display_string.into()) - } - fn format_printable_value( foreign_call_inputs: &[ForeignCallParam], skip_newline: bool, @@ -246,16 +181,15 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { fn execute( &mut self, foreign_call: &ForeignCallWaitInfo, - ) -> Result { + ) -> Result { let foreign_call_name = foreign_call.function.as_str(); match ForeignCall::lookup(foreign_call_name) { Some(ForeignCall::Print) => { if self.show_output { Self::execute_print(&foreign_call.inputs)?; } - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } - Some(ForeignCall::AssertMessage) => Self::execute_assert_message(&foreign_call.inputs), Some(ForeignCall::CreateMock) => { let mock_oracle_name = Self::parse_string(&foreign_call.inputs[0]); assert!(ForeignCall::lookup(&mock_oracle_name).is_none()); @@ -271,7 +205,7 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { .unwrap_or_else(|| panic!("Unknown mock id {}", id)) .params = Some(params.to_vec()); - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } Some(ForeignCall::GetMockLastParams) => { let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; @@ -291,7 +225,7 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { .unwrap_or_else(|| panic!("Unknown mock id {}", id)) .result = ForeignCallResult { values: params.to_vec() }; - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } Some(ForeignCall::SetMockTimes) => { let (id, params) = Self::extract_mock_id(&foreign_call.inputs)?; @@ -302,12 +236,12 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { .unwrap_or_else(|| panic!("Unknown mock id {}", id)) .times_left = Some(times); - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } Some(ForeignCall::ClearMock) => { let (id, _) = Self::extract_mock_id(&foreign_call.inputs)?; self.mocked_responses.retain(|response| response.id != id); - Ok(ForeignCallResult::default().into()) + Ok(ForeignCallResult::default()) } None => { let mock_response_position = self @@ -346,7 +280,7 @@ impl ForeignCallExecutor for DefaultForeignCallExecutor { let parsed_response: ForeignCallResult = response.result()?; - Ok(parsed_response.into()) + Ok(parsed_response) } (None, None) => panic!( "No mock for foreign call {}({:?})", @@ -423,7 +357,7 @@ mod tests { }; let result = executor.execute(&foreign_call); - assert_eq!(result.unwrap(), ForeignCallResult { values: foreign_call.inputs }.into()); + assert_eq!(result.unwrap(), ForeignCallResult { values: foreign_call.inputs }); server.close(); } diff --git a/noir/noir-repo/tooling/nargo/src/ops/mod.rs b/noir/noir-repo/tooling/nargo/src/ops/mod.rs index 2f5e6ebb7d4..cada2f0e915 100644 --- a/noir/noir-repo/tooling/nargo/src/ops/mod.rs +++ b/noir/noir-repo/tooling/nargo/src/ops/mod.rs @@ -3,9 +3,7 @@ pub use self::compile::{ compile_workspace, report_errors, }; pub use self::execute::execute_program; -pub use self::foreign_calls::{ - DefaultForeignCallExecutor, ForeignCall, ForeignCallExecutor, NargoForeignCallResult, -}; +pub use self::foreign_calls::{DefaultForeignCallExecutor, ForeignCall, ForeignCallExecutor}; pub use self::optimize::{optimize_contract, optimize_program}; pub use self::transform::{transform_contract, transform_program}; diff --git a/noir/noir-repo/tooling/nargo/src/ops/test.rs b/noir/noir-repo/tooling/nargo/src/ops/test.rs index b216fff827d..86dd8cd7cd5 100644 --- a/noir/noir-repo/tooling/nargo/src/ops/test.rs +++ b/noir/noir-repo/tooling/nargo/src/ops/test.rs @@ -2,9 +2,9 @@ use acvm::{ acir::native_types::{WitnessMap, WitnessStack}, BlackBoxFunctionSolver, }; +use noirc_abi::Abi; use noirc_driver::{compile_no_check, CompileError, CompileOptions}; use noirc_errors::{debug_info::DebugInfo, FileDiagnostic}; -use noirc_evaluator::errors::RuntimeError; use noirc_frontend::hir::{def_map::TestFunction, Context}; use crate::{errors::try_to_diagnose_runtime_error, NargoError}; @@ -44,6 +44,7 @@ pub fn run_test( ); test_status_program_compile_pass( test_function, + compiled_program.abi, compiled_program.debug, circuit_execution, ) @@ -64,18 +65,7 @@ fn test_status_program_compile_fail(err: CompileError, test_function: &TestFunct return TestStatus::CompileError(err.into()); } - // The test has failed compilation, extract the assertion message if present and check if it's expected. - let assert_message = if let CompileError::RuntimeError(RuntimeError::FailedConstraint { - assert_message, - .. - }) = &err - { - assert_message.clone() - } else { - None - }; - - check_expected_failure_message(test_function, assert_message, Some(err.into())) + check_expected_failure_message(test_function, None, Some(err.into())) } /// The test function compiled successfully. @@ -84,6 +74,7 @@ fn test_status_program_compile_fail(err: CompileError, test_function: &TestFunct /// passed/failed to determine the test status. fn test_status_program_compile_pass( test_function: &TestFunction, + abi: Abi, debug: Vec, circuit_execution: Result, ) -> TestStatus { @@ -105,7 +96,7 @@ fn test_status_program_compile_pass( // If we reach here, then the circuit execution failed. // // Check if the function should have passed - let diagnostic = try_to_diagnose_runtime_error(&circuit_execution_err, &debug); + let diagnostic = try_to_diagnose_runtime_error(&circuit_execution_err, &abi, &debug); let test_should_have_passed = !test_function.should_fail(); if test_should_have_passed { return TestStatus::Fail { @@ -116,7 +107,7 @@ fn test_status_program_compile_pass( check_expected_failure_message( test_function, - circuit_execution_err.user_defined_failure_message().map(|s| s.to_string()), + circuit_execution_err.user_defined_failure_message(&abi.error_types), diagnostic, ) } diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs index a353065491f..854ad559012 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/execute_cmd.rs @@ -154,7 +154,9 @@ pub(crate) fn execute_program( warnings: compiled_program.warnings.clone(), }; - if let Some(diagnostic) = try_to_diagnose_runtime_error(&err, &compiled_program.debug) { + if let Some(diagnostic) = + try_to_diagnose_runtime_error(&err, &compiled_program.abi, &compiled_program.debug) + { diagnostic.report(&debug_artifact, false); } diff --git a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/inputs.rs b/noir/noir-repo/tooling/nargo_cli/src/cli/fs/inputs.rs index 023195010ac..bd038c51ad5 100644 --- a/noir/noir-repo/tooling/nargo_cli/src/cli/fs/inputs.rs +++ b/noir/noir-repo/tooling/nargo_cli/src/cli/fs/inputs.rs @@ -107,6 +107,7 @@ mod tests { // Neither of these should be relevant so we leave them empty. param_witnesses: BTreeMap::new(), return_witnesses: Vec::new(), + error_types: BTreeMap::new(), }; let input_map = BTreeMap::from([ ("foo".to_owned(), InputValue::Field(42u128.into())), diff --git a/noir/noir-repo/tooling/noir_js/src/witness_generation.ts b/noir/noir-repo/tooling/noir_js/src/witness_generation.ts index cef1d817d9b..1f233422061 100644 --- a/noir/noir-repo/tooling/noir_js/src/witness_generation.ts +++ b/noir/noir-repo/tooling/noir_js/src/witness_generation.ts @@ -26,12 +26,6 @@ const defaultForeignCallHandler: ForeignCallHandler = async (name: string, args: // // If a user needs to print values then they should provide a custom foreign call handler. return []; - } else if (name == 'assert_message') { - // By default we do not do anything for `assert_message` foreign calls due to a need for formatting, - // however we provide an empty response in order to not halt execution. - // - // If a user needs to use dynamic assertion messages then they should provide a custom foreign call handler. - return []; } throw Error(`Unexpected oracle during execution: ${name}(${args.join(', ')})`); }; diff --git a/noir/noir-repo/tooling/noir_js_backend_barretenberg/test/public_input_deflattening.test.ts b/noir/noir-repo/tooling/noir_js_backend_barretenberg/test/public_input_deflattening.test.ts index 079a1ad268b..cfd43eff250 100644 --- a/noir/noir-repo/tooling/noir_js_backend_barretenberg/test/public_input_deflattening.test.ts +++ b/noir/noir-repo/tooling/noir_js_backend_barretenberg/test/public_input_deflattening.test.ts @@ -55,6 +55,7 @@ const abi: Abi = { visibility: 'public', }, return_witnesses: [2, 13, 13], + error_types: {}, }; it('flattens a witness map in order of its witness indices', async () => { 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 456e5a57f40..664a7c2a457 100644 --- a/noir/noir-repo/tooling/noir_js_types/src/types.ts +++ b/noir/noir-repo/tooling/noir_js_types/src/types.ts @@ -19,6 +19,14 @@ export type AbiParameter = { visibility: Visibility; }; +export type AbiErrorType = + | { + error_kind: 'fmtstring'; + length: number; + item_types: AbiType[]; + } + | ({ error_kind: 'custom' } & AbiType); + // Map from witness index to hex string value of witness. export type WitnessMap = Map; @@ -27,6 +35,7 @@ export type Abi = { param_witnesses: Record; return_type: { abi_type: AbiType; visibility: Visibility } | null; return_witnesses: number[]; + error_types: Record; }; export interface VerifierBackend { diff --git a/noir/noir-repo/tooling/noirc_abi/Cargo.toml b/noir/noir-repo/tooling/noirc_abi/Cargo.toml index 3258ea04c40..040f8a0dc79 100644 --- a/noir/noir-repo/tooling/noirc_abi/Cargo.toml +++ b/noir/noir-repo/tooling/noirc_abi/Cargo.toml @@ -12,6 +12,7 @@ license.workspace = true acvm.workspace = true iter-extended.workspace = true noirc_frontend.workspace = true +noirc_printable_type.workspace = true toml.workspace = true serde_json = "1.0" serde.workspace = true diff --git a/noir/noir-repo/tooling/noirc_abi/src/input_parser/mod.rs b/noir/noir-repo/tooling/noirc_abi/src/input_parser/mod.rs index 4cf66820b8d..9629ddc87ab 100644 --- a/noir/noir-repo/tooling/noirc_abi/src/input_parser/mod.rs +++ b/noir/noir-repo/tooling/noirc_abi/src/input_parser/mod.rs @@ -270,6 +270,7 @@ mod serialization_tests { // These two fields are unused when serializing/deserializing to file. param_witnesses: BTreeMap::new(), return_witnesses: Vec::new(), + error_types: Default::default(), }; let input_map: BTreeMap = BTreeMap::from([ diff --git a/noir/noir-repo/tooling/noirc_abi/src/lib.rs b/noir/noir-repo/tooling/noirc_abi/src/lib.rs index 8ff0154d32c..48c88833bdf 100644 --- a/noir/noir-repo/tooling/noirc_abi/src/lib.rs +++ b/noir/noir-repo/tooling/noirc_abi/src/lib.rs @@ -12,8 +12,9 @@ use input_parser::InputValue; use iter_extended::{try_btree_map, try_vecmap, vecmap}; use noirc_frontend::ast::{Signedness, Visibility}; use noirc_frontend::{hir::Context, Type, TypeBinding, TypeVariableKind}; +use noirc_printable_type::PrintableType; use serde::{Deserialize, Serialize}; -use std::ops::Range; +use std::{borrow::Borrow, ops::Range}; use std::{collections::BTreeMap, str}; // This is the ABI used to bridge the different TOML formats for the initial // witness, the partial witness generator and the interpreter. @@ -203,6 +204,38 @@ impl AbiType { } } +impl From<&AbiType> for PrintableType { + fn from(value: &AbiType) -> Self { + match value { + AbiType::Field => PrintableType::Field, + AbiType::String { length } => PrintableType::String { length: *length }, + AbiType::Tuple { fields } => { + let fields = fields.iter().map(|field| field.into()).collect(); + PrintableType::Tuple { types: fields } + } + AbiType::Array { length, typ } => { + let borrowed: &AbiType = typ.borrow(); + PrintableType::Array { length: Some(*length), typ: Box::new(borrowed.into()) } + } + AbiType::Boolean => PrintableType::Boolean, + AbiType::Struct { path, fields } => { + let fields = + fields.iter().map(|(name, field)| (name.clone(), field.into())).collect(); + PrintableType::Struct { + name: path.split("::").last().unwrap_or_default().to_string(), + fields, + } + } + AbiType::Integer { sign: Sign::Unsigned, width } => { + PrintableType::UnsignedInteger { width: *width } + } + AbiType::Integer { sign: Sign::Signed, width } => { + PrintableType::SignedInteger { width: *width } + } + } + } +} + #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] /// An argument or return value of the circuit's `main` function. pub struct AbiParameter { @@ -223,6 +256,7 @@ pub struct AbiReturnType { pub abi_type: AbiType, pub visibility: AbiVisibility, } + #[derive(Clone, Debug, Serialize, Deserialize)] pub struct Abi { /// An ordered list of the arguments to the program's `main` function, specifying their types and visibility. @@ -232,6 +266,7 @@ pub struct Abi { pub param_witnesses: BTreeMap>>, pub return_type: Option, pub return_witnesses: Vec, + pub error_types: BTreeMap, } impl Abi { @@ -281,6 +316,7 @@ impl Abi { param_witnesses, return_type: self.return_type, return_witnesses: self.return_witnesses, + error_types: self.error_types, } } @@ -547,6 +583,29 @@ fn range_to_vec(ranges: &[Range]) -> Vec { result } +#[derive(Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +#[serde(tag = "error_kind", rename_all = "lowercase")] +pub enum AbiErrorType { + FmtString { length: u64, item_types: Vec }, + Custom(AbiType), +} +impl AbiErrorType { + pub fn from_type(context: &Context, typ: &Type) -> Self { + match typ { + Type::FmtString(len, item_types) => { + let length = len.evaluate_to_u64().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| AbiType::from_type(context, typ)).collect(); + Self::FmtString { length, item_types } + } + _ => Self::Custom(AbiType::from_type(context, typ)), + } + } +} + #[cfg(test)] mod test { use std::collections::BTreeMap; @@ -583,6 +642,7 @@ mod test { visibility: AbiVisibility::Public, }), return_witnesses: vec![Witness(3)], + error_types: BTreeMap::default(), }; // Note we omit return value from inputs diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/abi_encode.ts b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/abi_encode.ts index cb80c6710ba..f4ab8175700 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/abi_encode.ts +++ b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/abi_encode.ts @@ -12,6 +12,7 @@ export const abi: Abi = { param_witnesses: { foo: [{ start: 1, end: 2 }], bar: [{ start: 2, end: 4 }] }, return_type: null, return_witnesses: [], + error_types: {}, }; export const inputs: InputMap = { diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/array_as_field.ts b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/array_as_field.ts index 0cc0035fa68..3698b913c66 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/array_as_field.ts +++ b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/array_as_field.ts @@ -11,6 +11,7 @@ export const abi: Abi = { param_witnesses: { foo: [{ start: 1, end: 3 }] }, return_type: null, return_witnesses: [], + error_types: {}, }; export const inputs: InputMap = { diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/field_as_array.ts b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/field_as_array.ts index 6ae709459de..4e3e2fd12a8 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/field_as_array.ts +++ b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/field_as_array.ts @@ -11,6 +11,7 @@ export const abi: Abi = { param_witnesses: { foo: [{ start: 1, end: 3 }] }, return_type: null, return_witnesses: [], + error_types: {}, }; export const inputs: InputMap = { diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/structs.ts b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/structs.ts index 6614f8f278e..ee666e40e87 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/structs.ts +++ b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/structs.ts @@ -54,6 +54,7 @@ export const abi: Abi = { }, return_type: null, return_witnesses: [], + error_types: {}, }; export const inputs: InputMap = { diff --git a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts index c6e066e2bcd..82a3e3998ca 100644 --- a/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts +++ b/noir/noir-repo/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts @@ -11,6 +11,7 @@ export const abi: Abi = { param_witnesses: { foo: [{ start: 1, end: 2 }] }, return_type: null, return_witnesses: [], + error_types: {}, }; export const inputs: InputMap = { diff --git a/yarn-project/simulator/src/avm/avm_machine_state.ts b/yarn-project/simulator/src/avm/avm_machine_state.ts index 8ebfa6b1e47..4183e1ff9c7 100644 --- a/yarn-project/simulator/src/avm/avm_machine_state.ts +++ b/yarn-project/simulator/src/avm/avm_machine_state.ts @@ -143,7 +143,7 @@ export class AvmMachineState { try { // Try to interpret the output as a text string. revertReason = new Error( - 'Reverted with output: ' + String.fromCharCode(...this.output.map(fr => fr.toNumber())), + 'Reverted with output: ' + String.fromCharCode(...this.output.slice(1).map(fr => fr.toNumber())), ); } catch (e) { revertReason = new Error('Reverted with non-string output'); diff --git a/yarn-project/simulator/src/avm/avm_simulator.test.ts b/yarn-project/simulator/src/avm/avm_simulator.test.ts index 9ea4b20d02d..b8850982f1f 100644 --- a/yarn-project/simulator/src/avm/avm_simulator.test.ts +++ b/yarn-project/simulator/src/avm/avm_simulator.test.ts @@ -113,7 +113,10 @@ describe('AVM simulator: transpiled Noir contracts', () => { expect(results.reverted).toBe(true); expect(results.revertReason?.message).toEqual("Reverted with output: Nullifier doesn't exist!"); - expect(results.output).toEqual([..."Nullifier doesn't exist!"].flatMap(c => new Fr(c.charCodeAt(0)))); + expect(results.output).toEqual([ + new Fr(0), + ...[..."Nullifier doesn't exist!"].flatMap(c => new Fr(c.charCodeAt(0))), + ]); }); describe.each([ diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts index 29c6626fa8d..2c76fc38f83 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.test.ts @@ -311,6 +311,7 @@ describe('External Calls', () => { it('Should return data and revert from the revert opcode', async () => { const returnData = [...'assert message'].flatMap(c => new Field(c.charCodeAt(0))); + returnData.unshift(new Field(0n)); // Prepend an error selector context.machineState.memory.setSlice(0, returnData);