From 0d2b06f577629eee75f26728189d70adccb658e7 Mon Sep 17 00:00:00 2001 From: Tom French <15848336+TomAFrench@users.noreply.github.com> Date: Mon, 25 Sep 2023 20:11:03 +0100 Subject: [PATCH] feat(abi): throw errors rather than returning string from `noirc_abi_wasm` (#2817) --- tooling/noirc_abi_wasm/src/errors.rs | 47 +++++++++++++++++++ tooling/noirc_abi_wasm/src/lib.rs | 37 +++++++-------- .../test/browser/errors.test.ts | 30 ++++++++++++ .../noirc_abi_wasm/test/node/errors.test.ts | 26 ++++++++++ .../test/shared/array_as_field.ts | 16 +++++++ .../test/shared/field_as_array.ts | 16 +++++++ .../test/shared/uint_overflow.ts | 16 +++++++ 7 files changed, 168 insertions(+), 20 deletions(-) create mode 100644 tooling/noirc_abi_wasm/src/errors.rs create mode 100644 tooling/noirc_abi_wasm/test/browser/errors.test.ts create mode 100644 tooling/noirc_abi_wasm/test/node/errors.test.ts create mode 100644 tooling/noirc_abi_wasm/test/shared/array_as_field.ts create mode 100644 tooling/noirc_abi_wasm/test/shared/field_as_array.ts create mode 100644 tooling/noirc_abi_wasm/test/shared/uint_overflow.ts diff --git a/tooling/noirc_abi_wasm/src/errors.rs b/tooling/noirc_abi_wasm/src/errors.rs new file mode 100644 index 00000000000..14ee2d5fd8e --- /dev/null +++ b/tooling/noirc_abi_wasm/src/errors.rs @@ -0,0 +1,47 @@ +use js_sys::{Error, JsString}; +use noirc_abi::errors::{AbiError, InputParserError}; +use wasm_bindgen::prelude::wasm_bindgen; + +#[wasm_bindgen(typescript_custom_section)] +const ABI_ERROR: &'static str = r#" +export type ABIError = Error; +"#; + +/// JsAbiError is a raw js error. +/// It'd be ideal that ABI error was a subclass of Error, but for that we'd need to use JS snippets or a js module. +/// Currently JS snippets don't work with a nodejs target. And a module would be too much for just a custom error type. +#[wasm_bindgen] +extern "C" { + #[wasm_bindgen(extends = Error, js_name = "AbiError", typescript_type = "AbiError")] + #[derive(Clone, Debug, PartialEq, Eq)] + pub type JsAbiError; + + #[wasm_bindgen(constructor, js_class = "Error")] + fn constructor(message: JsString) -> JsAbiError; +} + +impl JsAbiError { + /// 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) -> Self { + JsAbiError::constructor(JsString::from(message)) + } +} + +impl From for JsAbiError { + fn from(value: String) -> Self { + JsAbiError::new(value) + } +} + +impl From for JsAbiError { + fn from(value: AbiError) -> Self { + JsAbiError::new(value.to_string()) + } +} + +impl From for JsAbiError { + fn from(value: InputParserError) -> Self { + JsAbiError::new(value.to_string()) + } +} diff --git a/tooling/noirc_abi_wasm/src/lib.rs b/tooling/noirc_abi_wasm/src/lib.rs index 91ed724a101..2b1fc672fc4 100644 --- a/tooling/noirc_abi_wasm/src/lib.rs +++ b/tooling/noirc_abi_wasm/src/lib.rs @@ -14,9 +14,11 @@ use std::collections::BTreeMap; use gloo_utils::format::JsValueSerdeExt; use wasm_bindgen::{prelude::wasm_bindgen, JsValue}; +mod errors; mod js_witness_map; mod temp; +use errors::JsAbiError; use js_witness_map::JsWitnessMap; use temp::{input_value_from_json_type, JsonTypes}; @@ -25,7 +27,7 @@ pub fn abi_encode( abi: JsValue, inputs: JsValue, return_value: JsValue, -) -> Result { +) -> Result { console_error_panic_hook::set_once(); let abi: Abi = JsValueSerdeExt::into_serde(&abi).map_err(|err| err.to_string())?; let inputs: BTreeMap = @@ -36,14 +38,11 @@ pub fn abi_encode( } else { let toml_return_value = JsValueSerdeExt::into_serde(&return_value).expect("could not decode return value"); - Some( - input_value_from_json_type( - toml_return_value, - abi.return_type.as_ref().unwrap(), - MAIN_RETURN_NAME, - ) - .map_err(|err| err.to_string())?, - ) + Some(input_value_from_json_type( + toml_return_value, + abi.return_type.as_ref().unwrap(), + MAIN_RETURN_NAME, + )?) }; let abi_map = abi.to_btree_map(); @@ -55,32 +54,30 @@ pub fn abi_encode( .ok_or_else(|| InputParserError::MissingArgument(arg_name.clone()))?; input_value_from_json_type(value.clone(), &abi_type, &arg_name) .map(|input_value| (arg_name, input_value)) - }) - .map_err(|err| err.to_string())?; + })?; - let witness_map = abi.encode(&parsed_inputs, return_value).map_err(|err| err.to_string())?; + let witness_map = abi.encode(&parsed_inputs, return_value)?; Ok(witness_map.into()) } #[wasm_bindgen(js_name = abiDecode)] -pub fn abi_decode(abi: JsValue, witness_map: JsWitnessMap) -> Result { +pub fn abi_decode(abi: JsValue, witness_map: JsWitnessMap) -> Result { console_error_panic_hook::set_once(); let abi: Abi = JsValueSerdeExt::into_serde(&abi).map_err(|err| err.to_string())?; let witness_map = WitnessMap::from(witness_map); - let (inputs, return_value) = abi.decode(&witness_map).map_err(|err| err.to_string())?; + let (inputs, return_value) = abi.decode(&witness_map)?; let abi_types = abi.to_btree_map(); let inputs_map: BTreeMap = try_btree_map(inputs, |(key, value)| { JsonTypes::try_from_input_value(&value, &abi_types[&key]).map(|value| (key, value)) - }) - .map_err(|err| err.to_string())?; - let return_value = return_value.map(|value| { - JsonTypes::try_from_input_value(&value, &abi.return_type.unwrap()) - .expect("could not decode return value") - }); + })?; + + let return_value = return_value + .map(|value| JsonTypes::try_from_input_value(&value, &abi.return_type.unwrap())) + .transpose()?; #[derive(Serialize)] struct InputsAndReturn { diff --git a/tooling/noirc_abi_wasm/test/browser/errors.test.ts b/tooling/noirc_abi_wasm/test/browser/errors.test.ts new file mode 100644 index 00000000000..6cfb3d6b192 --- /dev/null +++ b/tooling/noirc_abi_wasm/test/browser/errors.test.ts @@ -0,0 +1,30 @@ +import { expect } from "@esm-bundle/chai"; +import initNoirAbi, { abiEncode } from "@noir-lang/noirc_abi"; + +beforeEach(async () => { + await initNoirAbi(); +}); + +it("errors when an integer input overflows", async () => { + const { abi, inputs } = await import("../shared/uint_overflow"); + + expect(() => abiEncode(abi, inputs, null)).to.throw( + "The parameter foo is expected to be a Integer { sign: Unsigned, width: 32 } but found incompatible value Field(2³⁸)", + ); +}); + +it("errors when passing a field in place of an array", async () => { + const { abi, inputs } = await import("../shared/field_as_array"); + + expect(() => abiEncode(abi, inputs, null)).to.throw( + "cannot parse value into Array { length: 2, typ: Field }", + ); +}); + +it("errors when passing an array in place of a field", async () => { + const { abi, inputs } = await import("../shared/array_as_field"); + + expect(() => abiEncode(abi, inputs, null)).to.throw( + "cannot parse value into Field", + ); +}); diff --git a/tooling/noirc_abi_wasm/test/node/errors.test.ts b/tooling/noirc_abi_wasm/test/node/errors.test.ts new file mode 100644 index 00000000000..a1bda73763f --- /dev/null +++ b/tooling/noirc_abi_wasm/test/node/errors.test.ts @@ -0,0 +1,26 @@ +import { expect } from "chai"; +import { abiEncode } from "@noir-lang/noirc_abi"; + +it("errors when an integer input overflows", async () => { + const { abi, inputs } = await import("../shared/uint_overflow"); + + expect(() => abiEncode(abi, inputs, null)).to.throw( + "The parameter foo is expected to be a Integer { sign: Unsigned, width: 32 } but found incompatible value Field(2³⁸)", + ); +}); + +it("errors when passing a field in place of an array", async () => { + const { abi, inputs } = await import("../shared/field_as_array"); + + expect(() => abiEncode(abi, inputs, null)).to.throw( + "cannot parse value into Array { length: 2, typ: Field }", + ); +}); + +it("errors when passing an array in place of a field", async () => { + const { abi, inputs } = await import("../shared/array_as_field"); + + expect(() => abiEncode(abi, inputs, null)).to.throw( + "cannot parse value into Field", + ); +}); diff --git a/tooling/noirc_abi_wasm/test/shared/array_as_field.ts b/tooling/noirc_abi_wasm/test/shared/array_as_field.ts new file mode 100644 index 00000000000..ff62ec75259 --- /dev/null +++ b/tooling/noirc_abi_wasm/test/shared/array_as_field.ts @@ -0,0 +1,16 @@ +export const abi = { + parameters: [ + { + name: "foo", + type: { kind: "field" }, + visibility: "private", + }, + ], + param_witnesses: { foo: [1, 2] }, + return_type: null, + return_witnesses: [], +}; + +export const inputs = { + foo: ["1", "2"], +}; diff --git a/tooling/noirc_abi_wasm/test/shared/field_as_array.ts b/tooling/noirc_abi_wasm/test/shared/field_as_array.ts new file mode 100644 index 00000000000..c669154962a --- /dev/null +++ b/tooling/noirc_abi_wasm/test/shared/field_as_array.ts @@ -0,0 +1,16 @@ +export const abi = { + parameters: [ + { + name: "foo", + type: { kind: "array", length: 2, type: { kind: "field" } }, + visibility: "private", + }, + ], + param_witnesses: { foo: [1, 2] }, + return_type: null, + return_witnesses: [], +}; + +export const inputs = { + foo: "1", +}; diff --git a/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts b/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts new file mode 100644 index 00000000000..97bfe79e926 --- /dev/null +++ b/tooling/noirc_abi_wasm/test/shared/uint_overflow.ts @@ -0,0 +1,16 @@ +export const abi = { + parameters: [ + { + name: "foo", + type: { kind: "integer", sign: "unsigned", width: 32 }, + visibility: "private", + }, + ], + param_witnesses: { foo: [1] }, + return_type: null, + return_witnesses: [], +}; + +export const inputs = { + foo: `0x${(1n << 38n).toString(16)}`, +};