Skip to content

Commit

Permalink
feat: Parsing non-string assertion payloads in noir js (AztecProtocol…
Browse files Browse the repository at this point in the history
…#6079)

This PR adds a noirc_abi_wasm entrypoint for decoding ABI error types.
This entrypoint is used by noir_js when an execution error is detected
to try to decode the error using the ABI.
If the result from decoding the error is a string (such as a fmt string
assertion, that previously weren't decoded at all) it'll patch the error
message to include the formatted string.
If the result from decoding the error is a non-string payload (structs,
tuples, arrays, etc) it'll add the decodedPayload property to the error
so users can get the data back from the circuit.
Also, this PR refactors ErrorSelector so it's a shared struct, instead
of being casted to u64 outside of the compiler backend.
  • Loading branch information
sirasistant authored May 3, 2024
1 parent 6785512 commit fbd78fd
Show file tree
Hide file tree
Showing 28 changed files with 455 additions and 141 deletions.
53 changes: 46 additions & 7 deletions noir/noir-repo/acvm-repo/acir/src/circuit/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,6 @@ pub struct Circuit {
pub recursive: bool,
}

/// 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),
Expand All @@ -93,10 +87,55 @@ pub enum AssertionPayload {
Dynamic(/* error_selector */ u64, Vec<ExpressionOrMemory>),
}

#[derive(Debug, Copy, PartialEq, Eq, Hash, Clone, PartialOrd, Ord)]
pub struct ErrorSelector(u64);

impl ErrorSelector {
pub fn new(integer: u64) -> Self {
ErrorSelector(integer)
}

pub fn as_u64(&self) -> u64 {
self.0
}
}

impl Serialize for ErrorSelector {
fn serialize<S>(&self, serializer: S) -> Result<S::Ok, S::Error>
where
S: serde::Serializer,
{
self.0.to_string().serialize(serializer)
}
}

impl<'de> Deserialize<'de> for ErrorSelector {
fn deserialize<D>(deserializer: D) -> Result<Self, D::Error>
where
D: serde::Deserializer<'de>,
{
let s: String = Deserialize::deserialize(deserializer)?;
let as_u64 = s.parse().map_err(serde::de::Error::custom)?;
Ok(ErrorSelector(as_u64))
}
}

/// This selector indicates that the payload is a string.
/// This is used to parse any error with a string payload directly,
/// to avoid users having to parse the error externally to the ACVM.
/// Only non-string errors need to be parsed externally to the ACVM using the circuit ABI.
pub const STRING_ERROR_SELECTOR: ErrorSelector = ErrorSelector(0);

#[derive(Clone, PartialEq, Eq, Debug, Serialize, Deserialize)]
pub struct RawAssertionPayload {
pub selector: ErrorSelector,
pub data: Vec<FieldElement>,
}

#[derive(Clone, PartialEq, Eq, Debug)]
pub enum ResolvedAssertionPayload {
String(String),
Raw(/*error_selector:*/ u64, Vec<FieldElement>),
Raw(RawAssertionPayload),
}

#[derive(Debug, Copy, Clone)]
Expand Down
25 changes: 15 additions & 10 deletions noir/noir-repo/acvm-repo/acvm/src/pwg/brillig.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ use acir::{
circuit::{
brillig::{BrilligInputs, BrilligOutputs},
opcodes::BlockId,
OpcodeLocation, ResolvedAssertionPayload, STRING_ERROR_SELECTOR,
ErrorSelector, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload,
STRING_ERROR_SELECTOR,
},
native_types::WitnessMap,
FieldElement,
Expand Down Expand Up @@ -173,11 +174,13 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> {
let mut revert_values_iter = memory
[revert_data_offset..(revert_data_offset + revert_data_size)]
.iter();
let error_selector = revert_values_iter
.next()
.expect("Incorrect revert data size")
.try_into()
.expect("Error selector is not u64");
let error_selector = ErrorSelector::new(
revert_values_iter
.next()
.expect("Incorrect revert data size")
.try_into()
.expect("Error selector is not u64"),
);

match error_selector {
STRING_ERROR_SELECTOR => {
Expand All @@ -194,10 +197,12 @@ impl<'b, B: BlackBoxFunctionSolver> BrilligSolver<'b, B> {
}
_ => {
// 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(),
))
Some(ResolvedAssertionPayload::Raw(RawAssertionPayload {
selector: error_selector,
data: revert_values_iter
.map(|value| value.to_field())
.collect(),
}))
}
}
}
Expand Down
13 changes: 9 additions & 4 deletions noir/noir-repo/acvm-repo/acvm/src/pwg/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,9 @@ use std::collections::HashMap;
use acir::{
brillig::ForeignCallResult,
circuit::{
brillig::BrilligBytecode, opcodes::BlockId, AssertionPayload, ExpressionOrMemory, Opcode,
OpcodeLocation, ResolvedAssertionPayload, STRING_ERROR_SELECTOR,
brillig::BrilligBytecode, opcodes::BlockId, AssertionPayload, ErrorSelector,
ExpressionOrMemory, Opcode, OpcodeLocation, RawAssertionPayload, ResolvedAssertionPayload,
STRING_ERROR_SELECTOR,
},
native_types::{Expression, Witness, WitnessMap},
BlackBoxFunc, FieldElement,
Expand Down Expand Up @@ -425,8 +426,9 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
}
}
}
let error_selector = ErrorSelector::new(*error_selector);

Some(match *error_selector {
Some(match error_selector {
STRING_ERROR_SELECTOR => {
// If the error selector is 0, it means the error is a string
let string = fields
Expand All @@ -444,7 +446,10 @@ impl<'a, B: BlackBoxFunctionSolver> ACVM<'a, B> {
}
_ => {
// If the error selector is not 0, it means the error is a custom error
ResolvedAssertionPayload::Raw(*error_selector, fields)
ResolvedAssertionPayload::Raw(RawAssertionPayload {
selector: error_selector,
data: fields,
})
}
})
}
Expand Down
6 changes: 3 additions & 3 deletions noir/noir-repo/acvm-repo/acvm_js/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -272,7 +272,7 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> {
};
// 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 {
let (message, raw_assertion_payload) = match error {
OpcodeResolutionError::UnsatisfiedConstrain {
payload: Some(payload),
..
Expand All @@ -281,8 +281,8 @@ impl<'a, B: BlackBoxFunctionSolver> ProgramExecutor<'a, B> {
payload: Some(payload),
..
} => match payload {
ResolvedAssertionPayload::Raw(selector, fields) => {
(error.to_string(), Some((*selector, fields.clone())))
ResolvedAssertionPayload::Raw(raw_payload) => {
("Assertion failed".to_string(), Some(raw_payload))
}
ResolvedAssertionPayload::String(message) => {
(format!("Assertion failed: {}", message), None)
Expand Down
27 changes: 8 additions & 19 deletions noir/noir-repo/acvm-repo/acvm_js/src/js_execution_error.rs
Original file line number Diff line number Diff line change
@@ -1,14 +1,13 @@
use acvm::{acir::circuit::OpcodeLocation, FieldElement};
use js_sys::{Array, Error, JsString, Map, Object, Reflect};
use acvm::acir::circuit::{OpcodeLocation, RawAssertionPayload};
use gloo_utils::format::JsValueSerdeExt;
use js_sys::{Array, Error, JsString, 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[];
selector: string;
data: string[];
};
export type ExecutionError = Error & {
callStack?: string[];
Expand All @@ -35,7 +34,7 @@ impl JsExecutionError {
pub fn new(
message: String,
call_stack: Option<Vec<OpcodeLocation>>,
assertion_payload: Option<(u64, Vec<FieldElement>)>,
assertion_payload: Option<RawAssertionPayload>,
) -> Self {
let mut error = JsExecutionError::constructor(JsString::from(message));
let js_call_stack = match call_stack {
Expand All @@ -49,18 +48,8 @@ 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()
}
Some(raw) => <wasm_bindgen::JsValue as JsValueSerdeExt>::from_serde(&raw)
.expect("Cannot serialize assertion payload"),
None => JsValue::UNDEFINED,
};

Expand Down
3 changes: 2 additions & 1 deletion noir/noir-repo/compiler/noirc_driver/src/abi_gen.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use std::collections::BTreeMap;

use acvm::acir::circuit::ErrorSelector;
use acvm::acir::native_types::Witness;
use iter_extended::{btree_map, vecmap};
use noirc_abi::{Abi, AbiErrorType, AbiParameter, AbiReturnType, AbiType, AbiValue};
Expand All @@ -20,7 +21,7 @@ pub(super) fn gen_abi(
input_witnesses: Vec<Witness>,
return_witnesses: Vec<Witness>,
return_visibility: Visibility,
error_types: BTreeMap<u64, Type>,
error_types: BTreeMap<ErrorSelector, Type>,
) -> Abi {
let (parameters, return_type) = compute_function_abi(context, func_id);
let param_witnesses = param_witnesses_from_abi_param(&parameters, input_witnesses);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -270,7 +270,7 @@ impl<'block> BrilligBlock<'block> {
condition,
payload_values,
payload_as_params,
selector.to_u64(),
selector.as_u64(),
);
}
Some(ConstrainError::Intrinsic(message)) => {
Expand Down
12 changes: 4 additions & 8 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@ use std::collections::{BTreeMap, BTreeSet};
use crate::errors::{RuntimeError, SsaReport};
use acvm::acir::{
circuit::{
brillig::BrilligBytecode, Circuit, ExpressionWidth, Program as AcirProgram, PublicInputs,
brillig::BrilligBytecode, Circuit, ErrorSelector, ExpressionWidth, Program as AcirProgram,
PublicInputs,
},
native_types::Witness,
};
Expand Down Expand Up @@ -103,13 +104,13 @@ pub struct SsaProgramArtifact {
pub main_input_witnesses: Vec<Witness>,
pub main_return_witnesses: Vec<Witness>,
pub names: Vec<String>,
pub error_types: BTreeMap<u64, HirType>,
pub error_types: BTreeMap<ErrorSelector, HirType>,
}

impl SsaProgramArtifact {
fn new(
unconstrained_functions: Vec<BrilligBytecode>,
error_types: BTreeMap<u64, HirType>,
error_types: BTreeMap<ErrorSelector, HirType>,
) -> Self {
let program = AcirProgram { functions: Vec::default(), unconstrained_functions };
Self {
Expand Down Expand Up @@ -167,11 +168,6 @@ pub fn create_program(
"The generated ACIRs should match the supplied function signatures"
);

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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ use self::acir_ir::generated_acir::BrilligStdlibFunc;
use super::function_builder::data_bus::DataBus;
use super::ir::dfg::CallStack;
use super::ir::function::FunctionId;
use super::ir::instruction::{ConstrainError, ErrorSelector, ErrorType};
use super::ir::instruction::{ConstrainError, ErrorType};
use super::ir::printer::try_to_extract_string_from_error_payload;
use super::{
ir::{
Expand All @@ -32,7 +32,7 @@ pub(crate) use acir_ir::generated_acir::GeneratedAcir;
use noirc_frontend::monomorphization::ast::InlineType;

use acvm::acir::circuit::brillig::BrilligBytecode;
use acvm::acir::circuit::{AssertionPayload, OpcodeLocation};
use acvm::acir::circuit::{AssertionPayload, ErrorSelector, OpcodeLocation};
use acvm::acir::native_types::Witness;
use acvm::acir::BlackBoxFunc;
use acvm::{
Expand Down Expand Up @@ -639,7 +639,7 @@ impl<'a> Context<'a> {
self.acir_context.vars_to_expressions_or_memory(&acir_vars)?;

Some(AssertionPayload::Dynamic(
error_selector.to_u64(),
error_selector.as_u64(),
expressions_or_memory,
))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ pub(crate) mod data_bus;

use std::{borrow::Cow, collections::BTreeMap, rc::Rc};

use acvm::FieldElement;
use acvm::{acir::circuit::ErrorSelector, FieldElement};
use noirc_errors::Location;
use noirc_frontend::monomorphization::ast::InlineType;

use crate::ssa::ir::{
basic_block::BasicBlockId,
function::{Function, FunctionId},
instruction::{Binary, BinaryOp, ErrorSelector, Instruction, TerminatorInstruction},
instruction::{Binary, BinaryOp, Instruction, TerminatorInstruction},
types::Type,
value::{Value, ValueId},
};
Expand Down
32 changes: 13 additions & 19 deletions noir/noir-repo/compiler/noirc_evaluator/src/ssa/ir/instruction.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
use std::hash::{Hash, Hasher};

use acvm::{
acir::{circuit::STRING_ERROR_SELECTOR, BlackBoxFunc},
acir::{
circuit::{ErrorSelector, STRING_ERROR_SELECTOR},
BlackBoxFunc,
},
FieldElement,
};
use fxhash::FxHasher;
Expand Down Expand Up @@ -605,26 +608,17 @@ 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 error_selector_from_type(typ: &ErrorType) -> ErrorSelector {
match typ {
ErrorType::String(_) => STRING_ERROR_SELECTOR,
_ => {
let mut hasher = FxHasher::default();
typ.hash(&mut hasher);
let hash = hasher.finish();
assert!(hash != 0, "ICE: Error type {} collides with the string error type", typ);
ErrorSelector::new(hash)
}
}

pub(crate) fn to_u64(self) -> u64 {
self.0
}
}

#[derive(Debug, PartialEq, Eq, Hash, Clone)]
Expand Down
Loading

0 comments on commit fbd78fd

Please sign in to comment.