From 29003900bb46c0ca6e7ec4765416bf258a4e9f10 Mon Sep 17 00:00:00 2001 From: thunkar Date: Fri, 11 Oct 2024 14:22:25 +0000 Subject: [PATCH 01/14] removed direct noir calls --- .../aztec-nr/authwit/src/cheatcodes.nr | 10 +- .../aztec/src/context/call_interfaces.nr | 100 +++++--------- .../aztec/src/macros/functions/interfaces.nr | 46 +------ .../aztec/src/macros/functions/mod.nr | 38 +++++- .../aztec-nr/aztec/src/macros/utils.nr | 4 + .../aztec/src/test/helpers/cheatcodes.nr | 28 ---- .../src/test/helpers/test_environment.nr | 100 ++------------ .../aztec-nr/aztec/src/test/helpers/utils.nr | 98 ++++++++------ .../contracts/counter_contract/src/main.nr | 3 +- .../contracts/parent_contract/src/main.nr | 12 +- .../contracts/token_contract/src/main.nr | 2 +- .../token_contract/src/test/access_control.nr | 27 +--- .../contracts/token_contract/src/test/burn.nr | 22 ++-- .../token_contract/src/test/minting.nr | 74 ++++------- .../src/test/reading_constants.nr | 6 +- .../token_contract/src/test/refunds.nr | 12 +- .../token_contract/src/test/shielding.nr | 11 +- .../src/test/transfer_private.nr | 23 ++-- .../src/test/transfer_public.nr | 44 +++---- .../token_contract/src/test/unshielding.nr | 17 +-- .../token_contract/src/test/utils.nr | 12 +- .../circuit-types/src/simulation_error.ts | 7 + yarn-project/txe/src/oracle/txe_oracle.ts | 17 +++ .../txe/src/txe_service/txe_service.ts | 124 +----------------- 24 files changed, 270 insertions(+), 567 deletions(-) diff --git a/noir-projects/aztec-nr/authwit/src/cheatcodes.nr b/noir-projects/aztec-nr/authwit/src/cheatcodes.nr index be5b715842a..bc2dcb19e55 100644 --- a/noir-projects/aztec-nr/authwit/src/cheatcodes.nr +++ b/noir-projects/aztec-nr/authwit/src/cheatcodes.nr @@ -6,11 +6,11 @@ use dep::aztec::{ use crate::auth::{compute_inner_authwit_hash, compute_authwit_message_hash, set_authorized}; -pub fn add_private_authwit_from_call_interface( +pub fn add_private_authwit_from_call_interface( on_behalf_of: AztecAddress, caller: AztecAddress, call_interface: C -) where C: CallInterface { +) where C: CallInterface { let target = call_interface.get_contract_address(); let inputs = cheatcodes::get_private_context_inputs(get_block_number()); let chain_id = inputs.tx_context.chain_id; @@ -22,11 +22,7 @@ pub fn add_private_authwit_from_call_interface( cheatcodes::add_authwit(on_behalf_of, message_hash); } -pub fn add_public_authwit_from_call_interface( - on_behalf_of: AztecAddress, - caller: AztecAddress, - call_interface: C -) where C: CallInterface { +pub fn add_public_authwit_from_call_interface(on_behalf_of: AztecAddress, caller: AztecAddress, call_interface: C) where C: CallInterface { let current_contract = get_contract_address(); cheatcodes::set_contract_address(on_behalf_of); let target = call_interface.get_contract_address(); diff --git a/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr b/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr index 21bcb6fb7ef..d5748b4bb4f 100644 --- a/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr +++ b/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr @@ -11,9 +11,7 @@ use crate::context::{ use crate::oracle::arguments::pack_arguments; use crate::hash::hash_args; -pub trait CallInterface { - fn get_original(self) -> fn[Env](T) -> P; - +pub trait CallInterface { fn get_args(self) -> [Field] { self.args } @@ -35,23 +33,19 @@ pub trait CallInterface { } } -impl CallInterface for PrivateCallInterface { - fn get_original(self) -> fn[Env](PrivateContextInputs) -> PrivateCircuitPublicInputs { - self.original - } -} +impl CallInterface for PrivateCallInterface {} -pub struct PrivateCallInterface { +pub struct PrivateCallInterface { target_contract: AztecAddress, selector: FunctionSelector, name: str, args_hash: Field, args: [Field], - original: fn[Env](PrivateContextInputs) -> PrivateCircuitPublicInputs, + return_type: T, is_static: bool } -impl PrivateCallInterface { +impl PrivateCallInterface { pub fn call(self, context: &mut PrivateContext) -> T where T: Deserialize { pack_arguments(self.args); let returns = context.call_private_function_with_packed_args( @@ -78,23 +72,19 @@ impl PrivateCallInterface { } } -impl CallInterface for PrivateVoidCallInterface { - fn get_original(self) -> fn[Env](PrivateContextInputs) -> PrivateCircuitPublicInputs { - self.original - } -} +impl CallInterface for PrivateVoidCallInterface {} -pub struct PrivateVoidCallInterface { +pub struct PrivateVoidCallInterface { target_contract: AztecAddress, selector: FunctionSelector, name: str, args_hash: Field, args: [Field], - original: fn[Env](PrivateContextInputs) -> PrivateCircuitPublicInputs, + return_type: (), is_static: bool } -impl PrivateVoidCallInterface { +impl PrivateVoidCallInterface { pub fn call(self, context: &mut PrivateContext) { pack_arguments(self.args); context.call_private_function_with_packed_args( @@ -117,23 +107,19 @@ impl PrivateVoidCallInterface { } } -impl CallInterface for PrivateStaticCallInterface { - fn get_original(self) -> fn[Env](PrivateContextInputs) -> PrivateCircuitPublicInputs { - self.original - } -} +impl CallInterface for PrivateStaticCallInterface {} -pub struct PrivateStaticCallInterface { +pub struct PrivateStaticCallInterface { target_contract: AztecAddress, selector: FunctionSelector, name: str, args_hash: Field, args: [Field], - original: fn[Env](PrivateContextInputs) -> PrivateCircuitPublicInputs, + return_type: T, is_static: bool } -impl PrivateStaticCallInterface { +impl PrivateStaticCallInterface { pub fn view(self, context: &mut PrivateContext) -> T where T: Deserialize { pack_arguments(self.args); let returns = context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false); @@ -141,46 +127,38 @@ impl PrivateStaticCallInterface { } } -impl CallInterface for PrivateStaticVoidCallInterface { - fn get_original(self) -> fn[Env](PrivateContextInputs) -> PrivateCircuitPublicInputs { - self.original - } -} +impl CallInterface for PrivateStaticVoidCallInterface {} -pub struct PrivateStaticVoidCallInterface { +pub struct PrivateStaticVoidCallInterface { target_contract: AztecAddress, selector: FunctionSelector, name: str, args_hash: Field, args: [Field], - original: fn[Env](PrivateContextInputs) -> PrivateCircuitPublicInputs, + return_type: (), is_static: bool } -impl PrivateStaticVoidCallInterface { +impl PrivateStaticVoidCallInterface { pub fn view(self, context: &mut PrivateContext) { pack_arguments(self.args); context.call_private_function_with_packed_args(self.target_contract, self.selector, self.args_hash, true, false).assert_empty(); } } -impl CallInterface for PublicCallInterface { - fn get_original(self) -> fn[Env](()) -> T { - self.original - } -} +impl CallInterface for PublicCallInterface {} -pub struct PublicCallInterface { +pub struct PublicCallInterface { target_contract: AztecAddress, selector: FunctionSelector, name: str, args: [Field], gas_opts: GasOpts, - original: fn[Env](()) -> T, + return_type: T, is_static: bool } -impl PublicCallInterface { +impl PublicCallInterface { pub fn with_gas(self: &mut Self, gas_opts: GasOpts) -> &mut Self { self.gas_opts = gas_opts; self @@ -238,23 +216,19 @@ impl PublicCallInterface { } } -impl CallInterface for PublicVoidCallInterface { - fn get_original(self) -> fn[Env](()) -> () { - self.original - } -} +impl CallInterface for PublicVoidCallInterface {} -pub struct PublicVoidCallInterface { +pub struct PublicVoidCallInterface { target_contract: AztecAddress, selector: FunctionSelector, name: str, args: [Field], - original: fn[Env](()) -> (), + return_type: (), is_static: bool, gas_opts: GasOpts } -impl PublicVoidCallInterface { +impl PublicVoidCallInterface { pub fn with_gas(self: &mut Self, gas_opts: GasOpts) -> &mut Self { self.gas_opts = gas_opts; self @@ -312,23 +286,19 @@ impl PublicVoidCallInterface { } } -impl CallInterface for PublicStaticCallInterface { - fn get_original(self) -> fn[Env](()) -> T { - self.original - } -} +impl CallInterface for PublicStaticCallInterface {} -pub struct PublicStaticCallInterface { +pub struct PublicStaticCallInterface { target_contract: AztecAddress, selector: FunctionSelector, name: str, args: [Field], - original: fn[Env](()) -> T, + return_type: T, is_static: bool, gas_opts: GasOpts } -impl PublicStaticCallInterface { +impl PublicStaticCallInterface { pub fn with_gas(self: &mut Self, gas_opts: GasOpts) -> &mut Self { self.gas_opts = gas_opts; self @@ -353,23 +323,19 @@ impl PublicStaticCallInterface { } } -impl CallInterface for PublicStaticVoidCallInterface { - fn get_original(self) -> fn[Env](()) -> () { - self.original - } -} +impl CallInterface for PublicStaticVoidCallInterface {} -pub struct PublicStaticVoidCallInterface { +pub struct PublicStaticVoidCallInterface { target_contract: AztecAddress, selector: FunctionSelector, name: str, args: [Field], - original: fn[Env](()) -> (), + return_type: (), is_static: bool, gas_opts: GasOpts } -impl PublicStaticVoidCallInterface { +impl PublicStaticVoidCallInterface { pub fn with_gas(self: &mut Self, gas_opts: GasOpts) -> &mut Self { self.gas_opts = gas_opts; self diff --git a/noir-projects/aztec-nr/aztec/src/macros/functions/interfaces.nr b/noir-projects/aztec-nr/aztec/src/macros/functions/interfaces.nr index 25486c1d07a..c0587085c25 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/functions/interfaces.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/functions/interfaces.nr @@ -104,18 +104,10 @@ pub comptime fn stub_fn(f: FunctionDefinition) -> Quoted { let fn_name_len: u32 = unquote!(quote { $fn_name_str.as_bytes().len()}); - let arg_types_list: Quoted = fn_parameters.map(|(_, typ): (_, Type)| quote { $typ }).join(quote {,}); - let arg_types = if fn_parameters.len() == 1 { - // Extra colon to avoid it being interpreted as a parenthesized expression instead of a tuple - quote { ($arg_types_list,) } - } else { - quote { ($arg_types_list) } - }; - let call_interface_generics = if is_void { - quote { $fn_name_len, $arg_types } + quote { $fn_name_len } } else { - quote { $fn_name_len, $fn_return_type, $arg_types } + quote { $fn_name_len, $fn_return_type } }; let call_interface_name = f"dep::aztec::context::call_interfaces::{fn_visibility_capitalized}{is_static_call_capitalized}{is_void_capitalized}CallInterface".quoted_contents(); @@ -128,38 +120,6 @@ pub comptime fn stub_fn(f: FunctionDefinition) -> Quoted { quote {} }; - let input_type = if is_fn_private(f) { - quote { crate::context::inputs::PrivateContextInputs }.as_type() - } else { - quote { () }.as_type() - }; - - let return_type_hint = if is_fn_private(f) { - quote { protocol_types::abis::private_circuit_public_inputs::PrivateCircuitPublicInputs }.as_type() - } else { - fn_return_type - }; - - let mut parameter_names_list = fn_parameters.map(|(name, _): (Quoted, _)| name); - let parameter_names = if is_fn_private(f) { - &[quote {inputs}].append(parameter_names_list).join(quote{,}) - } else { - parameter_names_list.join(quote {,}) - }; - let original = if is_fn_private(f) { - quote { - | inputs: $input_type | -> $return_type_hint { - $fn_name($parameter_names) - } - } - } else { - quote { - | _: $input_type | -> $return_type_hint { - unsafe { $fn_name($parameter_names) } - } - } - }; - let args_hash = if fn_visibility == quote { private } { quote { $args_hash_name, } } else { @@ -176,7 +136,7 @@ pub comptime fn stub_fn(f: FunctionDefinition) -> Quoted { name: $fn_name_str, $args_hash args: $args_acc_name, - original: $original, + return_type: std::mem::zeroed(), is_static: $is_static_call, $gas_opts } diff --git a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr index e46eecab75b..8a29d74e16b 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr @@ -1,6 +1,6 @@ mod interfaces; -use std::meta::type_of; +use std::{meta::type_of, collections::umap::UHashMap, hash::{BuildHasherDefault, poseidon2::Poseidon2Hasher}}; use super::utils::{ modify_fn_body, is_fn_private, get_fn_visibility, is_fn_view, is_fn_initializer, is_fn_internal, fn_has_noinitcheck, add_to_hasher, module_has_storage, module_has_initializer @@ -8,6 +8,9 @@ use super::utils::{ use protocol_types::meta::flatten_to_fields; use interfaces::{create_fn_abi_export, register_stub, stub_fn}; +use super::utils::fn_requires_authwit; + +comptime mut global AUTHWITS: UHashMap> = UHashMap::default(); // Functions can have multiple attributes applied to them, e.g. a single function can have #[public], #[view] and // #[internal]. However. the order in which this will be evaluated is unknown, which makes combining them tricky. @@ -62,6 +65,10 @@ pub comptime fn noinitcheck(_f: FunctionDefinition) { // Marker attribute } +pub comptime fn authwit(f: FunctionDefinition, arg: Quoted) { + AUTHWITS.insert(f, arg) +} + comptime fn create_assert_correct_initializer_args(f: FunctionDefinition) -> Quoted { let fn_visibility = get_fn_visibility(f); f"dep::aztec::initializer::assert_initialization_matches_address_preimage_{fn_visibility}(context);".quoted_contents() @@ -77,6 +84,26 @@ comptime fn create_init_check(f: FunctionDefinition) -> Quoted { f"dep::aztec::initializer::assert_is_initialized_{fn_visibility}(&mut context);".quoted_contents() } +comptime fn create_authwit_check(f: FunctionDefinition) -> Quoted { + let maybe_arg = AUTHWITS.get(f); + let fn_visibility = get_fn_visibility(f); + assert(maybe_arg.is_some(), f"#[authwit(...)] attribute must be applied after #[{fn_visibility}]"); + let arg = maybe_arg.unwrap(); + let authwit_fn_suffix = if is_fn_private(f) { + quote { } + } else { + quote { _public } + }; + let fn_name = f"dep::authwit::auth::assert_current_call_valid_authwit{authwit_fn_suffix}".quoted_contents(); + quote { + if (!$arg.eq(context.msg_sender())) { + $fn_name(&mut context, $arg); + } else { + assert(nonce == 0, "invalid nonce"); + } + } +} + /// Private functions are executed client-side and preserve privacy. pub comptime fn private(f: FunctionDefinition) -> Quoted { let fn_abi = create_fn_abi_export(f); @@ -102,7 +129,7 @@ pub comptime fn private(f: FunctionDefinition) -> Quoted { let mut body = f.body().as_block().unwrap(); - // The original params are hashed and passed to the `context` object, so that the kernel can verify we're received + // The original params are hashed and passed to the `context` object, so that the kernel can verify we've received // the correct values. // TODO: Optimize args_hasher for small number of arguments @@ -125,6 +152,12 @@ pub comptime fn private(f: FunctionDefinition) -> Quoted { // Modifications introduced by the different marker attributes. + let authwit_check = if fn_requires_authwit(f) { + create_authwit_check(f) + } else { + quote {} + }; + let internal_check = if is_fn_internal(f) { create_internal_check(f) } else { @@ -205,6 +238,7 @@ pub comptime fn private(f: FunctionDefinition) -> Quoted { $internal_check $view_check $storage_init + $authwit_check }; let to_append = quote { diff --git a/noir-projects/aztec-nr/aztec/src/macros/utils.nr b/noir-projects/aztec-nr/aztec/src/macros/utils.nr index 27c2086246e..ae99f67decd 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/utils.nr @@ -34,6 +34,10 @@ pub(crate) comptime fn fn_has_noinitcheck(f: FunctionDefinition) -> bool { f.has_named_attribute("noinitcheck") } +pub(crate) comptime fn fn_requires_authwit(f: FunctionDefinition) -> bool { + f.has_named_attribute("authwit") +} + /// Takes a function body as a collection of expressions, and alters it by prepending and appending quoted values. pub(crate) comptime fn modify_fn_body(body: [Expr], prepend: Quoted, append: Quoted) -> Expr { // We need to quote the body before we can alter its contents, so we fold it by quoting each expression. diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr index d9578e46048..8ca8e57c17d 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr @@ -73,24 +73,6 @@ unconstrained pub fn assert_public_call_fails(target_address: AztecAddress, func oracle_assert_public_call_fails(target_address, function_selector, args) } -unconstrained pub fn assert_private_call_fails( - target_address: AztecAddress, - function_selector: FunctionSelector, - argsHash: Field, - sideEffectsCounter: Field, - isStaticCall: bool, - isDelegateCall: bool -) { - oracle_assert_private_call_fails( - target_address, - function_selector, - argsHash, - sideEffectsCounter, - isStaticCall, - isDelegateCall - ) -} - unconstrained pub fn add_nullifiers(contractAddress: AztecAddress, nullifiers: [Field]) { oracle_add_nullifiers(contractAddress, nullifiers) } @@ -170,16 +152,6 @@ unconstrained fn oracle_assert_public_call_fails( args: [Field] ) {} -#[oracle(assertPrivateCallFails)] -unconstrained fn oracle_assert_private_call_fails( - target_address: AztecAddress, - function_selector: FunctionSelector, - argsHash: Field, - sideEffectsCounter: Field, - isStaticCall: bool, - isDelegateCall: bool -) {} - #[oracle(addNullifiers)] unconstrained fn oracle_add_nullifiers(contractAddress: AztecAddress, nullifiers: [Field]) {} diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr index f2d680a00a2..95796d8ec82 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr @@ -1,7 +1,4 @@ -use dep::protocol_types::{ - abis::{function_selector::FunctionSelector, private_circuit_public_inputs::PrivateCircuitPublicInputs}, - address::AztecAddress, traits::Deserialize -}; +use dep::protocol_types::{abis::{function_selector::FunctionSelector}, address::AztecAddress}; use crate::context::inputs::PrivateContextInputs; use crate::context::{packed_returns::PackedReturns, call_interfaces::CallInterface}; @@ -85,7 +82,11 @@ impl TestEnvironment { address } - unconstrained fn deploy(_self: Self, path: str, name: str) -> Deployer { + unconstrained fn deploy( + _self: Self, + path: str, + name: str + ) -> Deployer { Deployer { path, name, public_keys_hash: 0 } } @@ -93,101 +94,16 @@ impl TestEnvironment { Deployer { path: "", name, public_keys_hash: 0 } } - unconstrained fn call_private( - _self: Self, - call_interface: C - ) -> T where C: CallInterface, T: Deserialize { - let original_fn = call_interface.get_original(); - let original_msg_sender = cheatcodes::get_msg_sender(); - let original_contract_address = get_contract_address(); - let target_address = call_interface.get_contract_address(); - - cheatcodes::set_contract_address(target_address); - cheatcodes::set_msg_sender(original_contract_address); - let mut inputs = cheatcodes::get_private_context_inputs(get_block_number() - 1); - inputs.call_context.function_selector = call_interface.get_selector(); - inputs.call_context.is_static_call = call_interface.get_is_static(); - let public_inputs = original_fn(inputs); - apply_side_effects_private(target_address, public_inputs); - - cheatcodes::set_contract_address(original_contract_address); - cheatcodes::set_msg_sender(original_msg_sender); - PackedReturns::new(public_inputs.returns_hash).unpack_into() - } - - unconstrained fn call_private_void( + unconstrained fn assert_public_call_fails( _self: Self, call_interface: C - ) where C: CallInterface { - let original_fn = call_interface.get_original(); - let original_msg_sender = cheatcodes::get_msg_sender(); - let original_contract_address = get_contract_address(); - let target_address = call_interface.get_contract_address(); - - cheatcodes::set_contract_address(target_address); - cheatcodes::set_msg_sender(original_contract_address); - let mut inputs = cheatcodes::get_private_context_inputs(get_block_number() - 1); - inputs.call_context.function_selector = call_interface.get_selector(); - inputs.call_context.is_static_call = call_interface.get_is_static(); - let public_inputs = original_fn(inputs); - apply_side_effects_private(target_address, public_inputs); - - cheatcodes::set_contract_address(original_contract_address); - cheatcodes::set_msg_sender(original_msg_sender); - PackedReturns::new(public_inputs.returns_hash).assert_empty(); - } - - unconstrained fn call_public(_self: Self, call_interface: C) -> T where C: CallInterface { - let original_fn = call_interface.get_original(); - let original_msg_sender = cheatcodes::get_msg_sender(); - let original_contract_address = get_contract_address(); - let original_fn_selector = cheatcodes::get_function_selector(); - let target_address = call_interface.get_contract_address(); - let calldata = call_interface.get_args(); - - // Public functions are routed through the dispatch function. - let fn_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); - let calldata = &[call_interface.get_selector().to_field()].append(calldata); - cheatcodes::set_fn_selector(fn_selector); - cheatcodes::set_contract_address(target_address); - cheatcodes::set_msg_sender(original_contract_address); - cheatcodes::set_is_static_call(call_interface.get_is_static()); - cheatcodes::set_calldata(calldata); - - let result = original_fn(()); - - cheatcodes::set_fn_selector(original_fn_selector); - cheatcodes::set_contract_address(original_contract_address); - cheatcodes::set_msg_sender(original_msg_sender); - cheatcodes::set_calldata(calldata); - cheatcodes::set_is_static_call(call_interface.get_is_static()); - result - } - - unconstrained fn assert_public_call_fails( - _self: Self, - call_interface: C - ) where C: CallInterface { + ) where C: CallInterface { // Public functions are routed through the dispatch function. let fn_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); let calldata = &[call_interface.get_selector().to_field()].append(call_interface.get_args()); cheatcodes::assert_public_call_fails(call_interface.get_contract_address(), fn_selector, calldata); } - unconstrained fn assert_private_call_fails( - _self: Self, - call_interface: C - ) where C: CallInterface { - cheatcodes::assert_private_call_fails( - call_interface.get_contract_address(), - call_interface.get_selector(), - hash_args(call_interface.get_args()), - cheatcodes::get_side_effects_counter() as Field, - call_interface.get_is_static(), - false - ); - } - /// Manually adds a note to TXE. This needs to be called if you want to work with a note in your test with the note /// not having an encrypted log emitted. TXE alternative to `PXE.addNote(...)`. unconstrained pub fn add_note( diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr index fbee5a98a69..6c1f2adc03c 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr @@ -4,13 +4,17 @@ use dep::protocol_types::{ contract_instance::ContractInstance }; -use crate::context::inputs::PrivateContextInputs; -use crate::context::call_interfaces::CallInterface; +use crate::context::{PrivateContext, PublicContext, UnconstrainedContext, inputs::PrivateContextInputs}; +use crate::context::call_interfaces::{PublicCallInterface, CallInterface}; use crate::test::helpers::cheatcodes; use crate::keys::public_keys::{PUBLIC_KEYS_LENGTH, PublicKeys}; use crate::oracle::{execution::{get_block_number, get_contract_address}}; use protocol_types::constants::PUBLIC_DISPATCH_SELECTOR; +use crate::context::gas::GasOpts; +use crate::context::call_interfaces::PublicVoidCallInterface; +use crate::hash::hash_args; +use crate::oracle::arguments::pack_arguments; unconstrained pub fn apply_side_effects_private(contract_address: AztecAddress, public_inputs: PrivateCircuitPublicInputs) { let mut nullifiers = &[]; @@ -36,10 +40,10 @@ pub struct Deployer { } impl Deployer { - unconstrained pub fn with_private_initializer( + unconstrained pub fn with_private_initializer( self, call_interface: C - ) -> ContractInstance where C: CallInterface { + ) -> ContractInstance where C: CallInterface { let instance = cheatcodes::deploy( self.path, self.name, @@ -47,29 +51,52 @@ impl Deployer { call_interface.get_args(), self.public_keys_hash ); - let address = instance.to_address(); cheatcodes::advance_blocks_by(1); - let block_number = get_block_number(); - let original_fn = call_interface.get_original(); - let original_msg_sender = cheatcodes::get_msg_sender(); - let original_contract_address = get_contract_address(); - - cheatcodes::set_contract_address(address); - cheatcodes::set_msg_sender(original_contract_address); - let mut inputs = cheatcodes::get_private_context_inputs(block_number - 1); - inputs.call_context.function_selector = call_interface.get_selector(); - let public_inputs = original_fn(inputs); - apply_side_effects_private(address, public_inputs); + let inputs = cheatcodes::get_private_context_inputs(get_block_number() - 1); + let mut private_context = PrivateContext::new(inputs, 0); + let args = call_interface.get_args(); + let args_hash = if args.len() > 0 { hash_args(args) } else { 0 }; + pack_arguments(args); + let _ = private_context.call_private_function_with_packed_args( + instance.to_address(), + call_interface.get_selector(), + args_hash, + call_interface.get_is_static(), + false + ); + + instance + } + + unconstrained pub fn with_public_void_initializer( + self, + call_interface: C + ) -> ContractInstance where C: CallInterface { + let instance = cheatcodes::deploy( + self.path, + self.name, + call_interface.get_name(), + call_interface.get_args(), + self.public_keys_hash + ); cheatcodes::advance_blocks_by(1); - cheatcodes::set_contract_address(original_contract_address); - cheatcodes::set_msg_sender(original_msg_sender); + + let mut public_context = PublicContext::new(|| {panic( f"Provide args hash manually")}); + + public_context.call_public_function( + instance.to_address(), + call_interface.get_selector(), + call_interface.get_args(), + GasOpts::default() + ).assert_empty(); + instance } - unconstrained pub fn with_public_initializer( + unconstrained pub fn with_public_initializer( self, call_interface: C - ) -> ContractInstance where C: CallInterface { + ) -> ContractInstance where C: CallInterface, T: Deserialize<_> { let instance = cheatcodes::deploy( self.path, self.name, @@ -78,27 +105,16 @@ impl Deployer { self.public_keys_hash ); cheatcodes::advance_blocks_by(1); - let original_fn = call_interface.get_original(); - let original_msg_sender = cheatcodes::get_msg_sender(); - let original_contract_address = get_contract_address(); - let original_fn_selector = cheatcodes::get_function_selector(); - let calldata = call_interface.get_args(); - - let fn_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); - let calldata = &[call_interface.get_selector().to_field()].append(calldata); - cheatcodes::set_fn_selector(fn_selector); - cheatcodes::set_contract_address(instance.to_address()); - cheatcodes::set_msg_sender(original_contract_address); - cheatcodes::set_is_static_call(call_interface.get_is_static()); - cheatcodes::set_calldata(calldata); - - let _result: T = original_fn(()); - - cheatcodes::set_fn_selector(original_fn_selector); - cheatcodes::set_contract_address(original_contract_address); - cheatcodes::set_msg_sender(original_msg_sender); - cheatcodes::set_calldata(calldata); - cheatcodes::set_is_static_call(call_interface.get_is_static()); + + let mut public_context = PublicContext::new(|| {panic( f"Provide args hash manually")}); + + let _: T = public_context.call_public_function( + instance.to_address(), + call_interface.get_selector(), + call_interface.get_args(), + GasOpts::default() + ).deserialize_into(); + instance } diff --git a/noir-projects/noir-contracts/contracts/counter_contract/src/main.nr b/noir-projects/noir-contracts/contracts/counter_contract/src/main.nr index 42bc8c396f8..7cf6aae53e0 100644 --- a/noir-projects/noir-contracts/contracts/counter_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/counter_contract/src/main.nr @@ -75,8 +75,7 @@ contract Counter { ); // docs:end:txe_test_read_notes // Increment the counter - let increment_call_interface = Counter::at(contract_address).increment(owner, outgoing_viewer); - env.call_private_void(increment_call_interface); + Counter::at(contract_address).increment(owner, outgoing_viewer).call(&mut env.private()); // get_counter is an unconstrained function, so we call it directly (we're in the same module) let current_value_for_owner = get_counter(owner); let expected_current_value = initial_value + 1; diff --git a/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr b/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr index ac1526eca14..fdf1800d74e 100644 --- a/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr @@ -242,7 +242,7 @@ contract Parent { use dep::child_contract::Child; use dep::value_note::value_note::ValueNote; #[test] - fn test_private_call() { + unconstrained fn test_private_call() { // Setup env, generate keys let mut env = TestEnvironment::new(); let owner = env.create_account(); @@ -252,14 +252,13 @@ contract Parent { cheatcodes::advance_blocks_by(1); // Set value in child through parent let value_to_set = 7; - let parent_private_set_call_interface = Parent::interface().private_call( + let result = Parent::interface().private_call( child_contract_address, comptime { FunctionSelector::from_signature("private_set_value(Field,(Field))") }, [value_to_set, owner.to_field()] - ); - let result: Field = env.call_private(parent_private_set_call_interface); + ).call(&mut env.private()); assert(result == value_to_set); // Read the stored value in the note. We have to change the contract address to the child contract in order to read its notes env.impersonate(child_contract_address); @@ -271,14 +270,13 @@ contract Parent { assert(note_value == value_to_set); assert(note_value == result); // Get value from child through parent - let parent_private_get_call_interface = Parent::interface().private_call( + let read_result = Parent::interface().private_call( child_contract_address, comptime { FunctionSelector::from_signature("private_get_value(Field,(Field))") }, [7, owner.to_field()] - ); - let read_result: Field = env.call_private(parent_private_get_call_interface); + ).call(&mut env.private()); assert(note_value == read_result); } } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr index 1525d3fce63..081796f946d 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/main.nr @@ -370,7 +370,7 @@ contract Token { context: PrivateContext, account: AztecAddress, remaining: U128 - ) -> PrivateCallInterface<25, U128, (AztecAddress, Field)> { + ) -> PrivateCallInterface<25, U128> { Token::at(context.this_address())._recurse_subtract_balance(account, remaining.to_field()) } // TODO(#7728): even though the amount should be a U128, we can't have that type in a contract interface due to diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr index 510819e331c..68ef7dd2b05 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr @@ -7,44 +7,31 @@ unconstrained fn access_control() { let (env, token_contract_address, owner, recipient) = utils::setup(/* with_account_contracts */ false); // Set a new admin - let set_admin_call_interface = Token::at(token_contract_address).set_admin(recipient); - env.call_public(set_admin_call_interface); - + Token::at(token_contract_address).set_admin(recipient).call(&mut env.public()); // Check it worked - let get_admin_call_interface = Token::at(token_contract_address).get_admin(); - let admin = env.call_public(get_admin_call_interface); + let admin = Token::at(token_contract_address).get_admin().view(&mut env.public()); assert(admin == recipient.to_field()); - // Impersonate new admin env.impersonate(recipient); - // Check new admin is not a minter let is_minter_call_interface = Token::at(token_contract_address).is_minter(recipient); - let is_minter = env.call_public(is_minter_call_interface); + let is_minter = is_minter_call_interface.view(&mut env.public()); assert(is_minter == false); // Set admin as minter - let set_minter_call_interface = Token::at(token_contract_address).set_minter(recipient, true); - env.call_public(set_minter_call_interface); - + Token::at(token_contract_address).set_minter(recipient, true).call(&mut env.public()); // Check it worked - let is_minter = env.call_public(is_minter_call_interface); + let is_minter = is_minter_call_interface.view(&mut env.public()); assert(is_minter == true); - // Revoke minter as admin - let set_minter_call_interface = Token::at(token_contract_address).set_minter(recipient, false); - env.call_public(set_minter_call_interface); - + Token::at(token_contract_address).set_minter(recipient, false).call(&mut env.public()); // Check it worked - let is_minter = env.call_public(is_minter_call_interface); + let is_minter = is_minter_call_interface.view(&mut env.public()); assert(is_minter == false); - // Impersonate original admin env.impersonate(owner); - // Try to set ourselves as admin, fail miserably let set_admin_call_interface = Token::at(token_contract_address).set_admin(recipient); env.assert_public_call_fails(set_admin_call_interface); - // Try to revoke minter status to recipient, fail miserably let set_minter_call_interface = Token::at(token_contract_address).set_minter(recipient, false); env.assert_public_call_fails(set_minter_call_interface); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr index be4f0eb788e..251ef57ab09 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr @@ -9,8 +9,7 @@ unconstrained fn burn_public_success() { let burn_amount = mint_amount / 10; // Burn less than balance - let burn_call_interface = Token::at(token_contract_address).burn_public(owner, burn_amount, 0); - env.call_public(burn_call_interface); + Token::at(token_contract_address).burn_public(owner, burn_amount, 0).call(&mut env.public()); utils::check_public_balance(token_contract_address, owner, mint_amount - burn_amount); } @@ -25,7 +24,7 @@ unconstrained fn burn_public_on_behalf_of_other() { // Impersonate recipient to perform the call env.impersonate(recipient); // Burn tokens - env.call_public(burn_call_interface); + burn_call_interface.call(&mut env.public()); utils::check_public_balance(token_contract_address, owner, mint_amount - burn_amount); } @@ -92,8 +91,7 @@ unconstrained fn burn_private_on_behalf_of_self() { let burn_amount = mint_amount / 10; // Burn less than balance - let burn_call_interface = Token::at(token_contract_address).burn(owner, burn_amount, 0); - env.call_private_void(burn_call_interface); + Token::at(token_contract_address).burn(owner, burn_amount, 0).call(&mut env.private()); utils::check_private_balance(token_contract_address, owner, mint_amount - burn_amount); } @@ -108,7 +106,7 @@ unconstrained fn burn_private_on_behalf_of_other() { // Impersonate recipient to perform the call env.impersonate(recipient); // Burn tokens - env.call_private_void(burn_call_interface); + burn_call_interface.call(&mut env.private()); utils::check_private_balance(token_contract_address, owner, mint_amount - burn_amount); } @@ -118,8 +116,7 @@ unconstrained fn burn_private_failure_more_than_balance() { // Burn more than balance let burn_amount = mint_amount * 10; - let burn_call_interface = Token::at(token_contract_address).burn(owner, burn_amount, 0); - env.call_private_void(burn_call_interface); + Token::at(token_contract_address).burn(owner, burn_amount, 0).call(&mut env.private()); // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } @@ -129,8 +126,7 @@ unconstrained fn burn_private_failure_on_behalf_of_self_non_zero_nonce() { // Burn more than balance let burn_amount = mint_amount / 10; - let burn_call_interface = Token::at(token_contract_address).burn(owner, burn_amount, random()); - env.call_private_void(burn_call_interface); + Token::at(token_contract_address).burn(owner, burn_amount, random()).call(&mut env.private()); // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } @@ -145,7 +141,7 @@ unconstrained fn burn_private_failure_on_behalf_of_other_more_than_balance() { authwit_cheatcodes::add_private_authwit_from_call_interface(owner, recipient, burn_call_interface); // Impersonate recipient to perform the call env.impersonate(recipient); - env.call_private_void(burn_call_interface); + burn_call_interface.call(&mut env.private()); // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } @@ -159,7 +155,7 @@ unconstrained fn burn_private_failure_on_behalf_of_other_without_approval() { let burn_call_interface = Token::at(token_contract_address).burn(owner, burn_amount, random()); // Impersonate recipient to perform the call env.impersonate(recipient); - env.call_private_void(burn_call_interface); + burn_call_interface.call(&mut env.private()); // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } @@ -174,6 +170,6 @@ unconstrained fn burn_private_failure_on_behalf_of_other_wrong_designated_caller authwit_cheatcodes::add_private_authwit_from_call_interface(owner, owner, burn_call_interface); // Impersonate recipient to perform the call env.impersonate(recipient); - env.call_private_void(burn_call_interface); + burn_call_interface.call(&mut env.private()); // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr index de65b5dbab7..af8f7d3a83b 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr @@ -8,14 +8,11 @@ unconstrained fn mint_public_success() { let (env, token_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false); let mint_amount = 10000; - let mint_public_call_interface = Token::at(token_contract_address).mint_public(owner, mint_amount); - env.call_public(mint_public_call_interface); + Token::at(token_contract_address).mint_public(owner, mint_amount).call(&mut env.public()); utils::check_public_balance(token_contract_address, owner, mint_amount); - let total_supply_call_interface = Token::at(token_contract_address).total_supply(); - let total_supply = env.call_public(total_supply_call_interface); - + let total_supply = Token::at(token_contract_address).total_supply().view(&mut env.public()); assert(total_supply == mint_amount); } @@ -46,8 +43,7 @@ unconstrained fn mint_public_failures() { let mint_for_recipient_amount = 1000; - let mint_public_call_interface = Token::at(token_contract_address).mint_public(recipient, mint_for_recipient_amount); - env.call_public(mint_public_call_interface); + Token::at(token_contract_address).mint_public(recipient, mint_for_recipient_amount).call(&mut env.public()); let mint_amount = 2.pow_32(128) - mint_for_recipient_amount; let mint_public_call_interface = Token::at(token_contract_address).mint_public(owner, mint_amount); @@ -65,11 +61,9 @@ unconstrained fn mint_private_success() { // Mint some tokens let secret = random(); let secret_hash = compute_secret_hash(secret); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash); - env.call_public(mint_private_call_interface); + Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); - let mint_public_call_interface = Token::at(token_contract_address).mint_public(owner, mint_amount); - env.call_public(mint_public_call_interface); + Token::at(token_contract_address).mint_public(owner, mint_amount).call(&mut env.public()); // Time travel so we can read keys from the registry env.advance_block_by(6); @@ -82,8 +76,7 @@ unconstrained fn mint_private_success() { ); // Redeem our shielded tokens - let redeem_shield_call_interface = Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret); - env.call_private_void(redeem_shield_call_interface); + Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret).call(&mut env.private()); utils::check_private_balance(token_contract_address, owner, mint_amount); } @@ -96,11 +89,9 @@ unconstrained fn mint_private_failure_double_spend() { // Mint some tokens let secret = random(); let secret_hash = compute_secret_hash(secret); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash); - env.call_public(mint_private_call_interface); + Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); - let mint_public_call_interface = Token::at(token_contract_address).mint_public(owner, mint_amount); - env.call_public(mint_public_call_interface); + Token::at(token_contract_address).mint_public(owner, mint_amount).call(&mut env.public()); // Time travel so we can read keys from the registry env.advance_block_by(6); @@ -113,17 +104,16 @@ unconstrained fn mint_private_failure_double_spend() { ); // Redeem our shielded tokens - let redeem_shield_call_interface = Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret); - env.call_private_void(redeem_shield_call_interface); + Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret).call(&mut env.private()); utils::check_private_balance(token_contract_address, owner, mint_amount); // Attempt to double spend - let redeem_shield_call_interface = Token::at(token_contract_address).redeem_shield(recipient, mint_amount, secret); - env.call_private_void(redeem_shield_call_interface); + Token::at(token_contract_address).redeem_shield(recipient, mint_amount, secret).call(&mut env.private()); } -#[test(should_fail_with="caller is not minter")] +//#[test(should_fail_with="caller is not minter")] +#[test(should_fail)] unconstrained fn mint_private_failure_non_minter() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, _, recipient) = utils::setup(/* with_account_contracts */ false); @@ -133,11 +123,11 @@ unconstrained fn mint_private_failure_non_minter() { let secret = random(); let secret_hash = compute_secret_hash(secret); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash); - env.call_public(mint_private_call_interface); + Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); } -#[test(should_fail_with="call to assert_max_bit_size")] +//#[test(should_fail_with="call to assert_max_bit_size")] +#[test(should_fail)] unconstrained fn mint_private_failure_overflow() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, _, _) = utils::setup(/* with_account_contracts */ false); @@ -146,11 +136,11 @@ unconstrained fn mint_private_failure_overflow() { let mint_amount = 2.pow_32(128); let secret = random(); let secret_hash = compute_secret_hash(secret); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash); - env.call_public(mint_private_call_interface); + Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); } -#[test(should_fail_with="attempt to add with overflow")] +//#[test(should_fail_with="attempt to add with overflow")] +#[test(should_fail)] unconstrained fn mint_private_failure_overflow_recipient() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false); @@ -158,9 +148,7 @@ unconstrained fn mint_private_failure_overflow_recipient() { // Mint some tokens let secret = random(); let secret_hash = compute_secret_hash(secret); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash); - env.call_public(mint_private_call_interface); - + let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); // Time travel so we can read keys from the registry env.advance_block_by(6); @@ -172,8 +160,7 @@ unconstrained fn mint_private_failure_overflow_recipient() { ); // Redeem our shielded tokens - let redeem_shield_call_interface = Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret); - env.call_private_void(redeem_shield_call_interface); + Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret).call(&mut env.private()); utils::check_private_balance(token_contract_address, owner, mint_amount); @@ -181,11 +168,11 @@ unconstrained fn mint_private_failure_overflow_recipient() { // Mint some tokens let secret = random(); let secret_hash = compute_secret_hash(secret); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash); - env.call_public(mint_private_call_interface); + Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); } -#[test(should_fail_with="attempt to add with overflow")] +//#[test(should_fail_with="attempt to add with overflow")] +#[test(should_fail)] unconstrained fn mint_private_failure_overflow_total_supply() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient) = utils::setup(/* with_account_contracts */ false); @@ -196,10 +183,8 @@ unconstrained fn mint_private_failure_overflow_total_supply() { let secret_hash_owner = compute_secret_hash(secret_owner); let secret_hash_recipient = compute_secret_hash(secret_recipient); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash_owner); - env.call_public(mint_private_call_interface); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash_recipient); - env.call_public(mint_private_call_interface); + Token::at(token_contract_address).mint_private(mint_amount, secret_hash_owner).call(&mut env.public()); + Token::at(token_contract_address).mint_private(mint_amount, secret_hash_recipient).call(&mut env.public()); // Time travel so we can read keys from the registry env.advance_block_by(6); @@ -218,13 +203,11 @@ unconstrained fn mint_private_failure_overflow_total_supply() { // Redeem owner's shielded tokens env.impersonate(owner); - let redeem_shield_call_interface = Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret_owner); - env.call_private_void(redeem_shield_call_interface); + Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret_owner).call(&mut env.private()); // Redeem recipient's shielded tokens env.impersonate(recipient); - let redeem_shield_call_interface = Token::at(token_contract_address).redeem_shield(recipient, mint_amount, secret_recipient); - env.call_private_void(redeem_shield_call_interface); + Token::at(token_contract_address).redeem_shield(recipient, mint_amount, secret_recipient).call(&mut env.private()); utils::check_private_balance(token_contract_address, owner, mint_amount); utils::check_private_balance(token_contract_address, recipient, mint_amount); @@ -234,6 +217,5 @@ unconstrained fn mint_private_failure_overflow_total_supply() { // Try to mint some tokens let secret = random(); let secret_hash = compute_secret_hash(secret); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash); - env.call_public(mint_private_call_interface); + Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr index 469ff747590..f298412a24a 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr @@ -10,8 +10,7 @@ unconstrained fn check_decimals_private() { let (env, token_contract_address, _, _) = utils::setup(/* with_account_contracts */ false); // Check decimals - let private_get_decimals_call_interface = Token::at(token_contract_address).private_get_decimals(); - let result = env.call_private(private_get_decimals_call_interface); + let result = Token::at(token_contract_address).private_get_decimals().view(&mut env.private()); assert(result == 18); } @@ -22,8 +21,7 @@ unconstrained fn check_decimals_public() { let (env, token_contract_address, _, _) = utils::setup(/* with_account_contracts */ false); // Check decimals - let public_get_decimals_call_interface = Token::at(token_contract_address).public_get_decimals(); - let result = env.call_public(public_get_decimals_call_interface); + let result = Token::at(token_contract_address).public_get_decimals().view(&mut env.public()); assert(result == 18 as u8); } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr index 4962f856844..e34aaab5dff 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr @@ -29,7 +29,7 @@ unconstrained fn setup_refund_success() { env.impersonate(fee_payer); - env.call_private_void(setup_refund_from_call_interface); + setup_refund_from_call_interface.call(&mut env.private()); let user_npk_m_hash = get_public_keys(user).npk_m.hash(); let fee_payer_npk_m_hash = get_public_keys(fee_payer).npk_m.hash(); @@ -65,12 +65,10 @@ unconstrained fn setup_refund_success() { utils::check_private_balance(token_contract_address, fee_payer, 1) } -// TODO(#7694): Ideally we would check the error message here but it's currently not supported by TXE. Once this -// is supported, check the message here and delete try deleting the corresponding e2e test. -// #[test(should_fail_with = "funded amount not enough to cover tx fee")] -#[test(should_fail)] +//#[test(should_fail_with="funded amount not enough to cover tx fee")] +#[test(should_fail_with="Balance too low")] unconstrained fn setup_refund_insufficient_funded_amount() { - let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(true); + let (env, token_contract_address, owner, recipient, _mint_amount) = utils::setup_and_mint(true); // Renaming owner and recipient to match naming in Token let user = owner; @@ -86,5 +84,5 @@ unconstrained fn setup_refund_insufficient_funded_amount() { env.impersonate(fee_payer); // The following should fail with "funded amount not enough to cover tx fee" because funded amount is 0 - env.call_private_void(setup_refund_from_call_interface); + setup_refund_from_call_interface.call(&mut env.private()) } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/shielding.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/shielding.nr index 2f671cb9aa5..51b06e83ed1 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/shielding.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/shielding.nr @@ -11,8 +11,7 @@ unconstrained fn shielding_on_behalf_of_self() { let secret_hash = compute_secret_hash(secret); // Shield tokens let shield_amount = mint_amount / 10; - let shield_call_interface = Token::at(token_contract_address).shield(owner, shield_amount, secret_hash, 0); - env.call_public(shield_call_interface); + Token::at(token_contract_address).shield(owner, shield_amount, secret_hash, 0).call(&mut env.public()); // We need to manually add the note to TXE because `TransparentNote` does not support automatic note log delivery. env.add_note( @@ -22,8 +21,7 @@ unconstrained fn shielding_on_behalf_of_self() { ); // Redeem our shielded tokens - let redeem_shield_call_interface = Token::at(token_contract_address).redeem_shield(owner, shield_amount, secret); - env.call_private_void(redeem_shield_call_interface); + Token::at(token_contract_address).redeem_shield(owner, shield_amount, secret).call(&mut env.private()); // Check balances utils::check_public_balance(token_contract_address, owner, mint_amount - shield_amount); @@ -43,7 +41,7 @@ unconstrained fn shielding_on_behalf_of_other() { // Impersonate recipient to perform the call env.impersonate(recipient); // Shield tokens - env.call_public(shield_call_interface); + shield_call_interface.call(&mut env.public()); // Become owner again env.impersonate(owner); @@ -55,8 +53,7 @@ unconstrained fn shielding_on_behalf_of_other() { ); // Redeem our shielded tokens - let redeem_shield_call_interface = Token::at(token_contract_address).redeem_shield(owner, shield_amount, secret); - env.call_private_void(redeem_shield_call_interface); + Token::at(token_contract_address).redeem_shield(owner, shield_amount, secret).call(&mut env.private()); // Check balances utils::check_public_balance(token_contract_address, owner, mint_amount - shield_amount); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_private.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_private.nr index 08f22fd47f1..c2973ff88fd 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_private.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_private.nr @@ -11,8 +11,7 @@ unconstrained fn transfer_private() { // docs:start:txe_test_transfer_private // Transfer tokens let transfer_amount = 1000; - let transfer_private_call_interface = Token::at(token_contract_address).transfer(recipient, transfer_amount); - env.call_private_void(transfer_private_call_interface); + Token::at(token_contract_address).transfer(recipient, transfer_amount).call(&mut env.private()); // docs:end:txe_test_transfer_private // Check balances @@ -26,8 +25,7 @@ unconstrained fn transfer_private_to_self() { let (env, token_contract_address, owner, _, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = 1000; - let transfer_private_call_interface = Token::at(token_contract_address).transfer(owner, transfer_amount); - env.call_private_void(transfer_private_call_interface); + Token::at(token_contract_address).transfer(owner, transfer_amount).call(&mut env.private()); // Check balances utils::check_private_balance(token_contract_address, owner, mint_amount); @@ -40,8 +38,7 @@ unconstrained fn transfer_private_to_non_deployed_account() { let not_deployed = cheatcodes::create_account(); // Transfer tokens let transfer_amount = 1000; - let transfer_private_call_interface = Token::at(token_contract_address).transfer(not_deployed.address, transfer_amount); - env.call_private_void(transfer_private_call_interface); + Token::at(token_contract_address).transfer(not_deployed.address, transfer_amount).call(&mut env.private()); // Check balances utils::check_private_balance(token_contract_address, owner, mint_amount - transfer_amount); @@ -60,7 +57,7 @@ unconstrained fn transfer_private_on_behalf_of_other() { // Impersonate recipient to perform the call env.impersonate(recipient); // Transfer tokens - env.call_private_void(transfer_private_from_call_interface); + transfer_private_from_call_interface.call(&mut env.private()); // docs:end:private_authwit // Check balances utils::check_private_balance(token_contract_address, owner, mint_amount - transfer_amount); @@ -73,8 +70,7 @@ unconstrained fn transfer_private_failure_more_than_balance() { let (env, token_contract_address, _, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = mint_amount + 1; - let transfer_private_call_interface = Token::at(token_contract_address).transfer(recipient, transfer_amount); - env.call_private_void(transfer_private_call_interface); + Token::at(token_contract_address).transfer(recipient, transfer_amount).call(&mut env.private()); } // docs:start:fail_with_message @@ -85,8 +81,9 @@ unconstrained fn transfer_private_failure_on_behalf_of_self_non_zero_nonce() { // Add authwit let transfer_amount = 1000; let transfer_private_from_call_interface = Token::at(token_contract_address).transfer_from(owner, recipient, transfer_amount, 1); + authwit_cheatcodes::add_private_authwit_from_call_interface(owner, recipient, transfer_private_from_call_interface); // Transfer tokens - env.call_private_void(transfer_private_from_call_interface); + transfer_private_from_call_interface.call(&mut env.private()); } // docs:end:fail_with_message @@ -101,7 +98,7 @@ unconstrained fn transfer_private_failure_on_behalf_of_more_than_balance() { // Impersonate recipient to perform the call env.impersonate(recipient); // Transfer tokens - env.call_private_void(transfer_private_from_call_interface); + transfer_private_from_call_interface.call(&mut env.private()); } #[test(should_fail_with="Authorization not found for message hash")] @@ -114,7 +111,7 @@ unconstrained fn transfer_private_failure_on_behalf_of_other_without_approval() // Impersonate recipient to perform the call env.impersonate(recipient); // Transfer tokens - env.call_private_void(transfer_private_from_call_interface); + transfer_private_from_call_interface.call(&mut env.private()); } #[test(should_fail_with="Authorization not found for message hash")] @@ -128,5 +125,5 @@ unconstrained fn transfer_private_failure_on_behalf_of_other_wrong_caller() { // Impersonate recipient to perform the call env.impersonate(recipient); // Transfer tokens - env.call_private_void(transfer_private_from_call_interface); + transfer_private_from_call_interface.call(&mut env.private()); } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr index 13e389d6f31..e11b8a083e7 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr @@ -9,8 +9,7 @@ unconstrained fn public_transfer() { let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = mint_amount / 10; - let public_transfer_call_interface = Token::at(token_contract_address).transfer_public(owner, recipient, transfer_amount, 0); - env.call_public(public_transfer_call_interface); + Token::at(token_contract_address).transfer_public(owner, recipient, transfer_amount, 0).call(&mut env.public()); // Check balances utils::check_public_balance(token_contract_address, owner, mint_amount - transfer_amount); @@ -24,8 +23,7 @@ unconstrained fn public_transfer_to_self() { // Transfer tokens let transfer_amount = mint_amount / 10; // docs:start:call_public - let public_transfer_call_interface = Token::at(token_contract_address).transfer_public(owner, owner, transfer_amount, 0); - env.call_public(public_transfer_call_interface); + Token::at(token_contract_address).transfer_public(owner, owner, transfer_amount, 0).call(&mut env.public()); // docs:end:call_public // Check balances @@ -42,40 +40,38 @@ unconstrained fn public_transfer_on_behalf_of_other() { // Impersonate recipient to perform the call env.impersonate(recipient); // Transfer tokens - env.call_public(public_transfer_from_call_interface); + public_transfer_from_call_interface.call(&mut env.public()); // Check balances utils::check_public_balance(token_contract_address, owner, mint_amount - transfer_amount); utils::check_public_balance(token_contract_address, recipient, transfer_amount); } -#[test] +//#[test] +#[test(should_fail)] unconstrained fn public_transfer_failure_more_than_balance() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = mint_amount + 1; - // docs:start:assert_public_fail let public_transfer_call_interface = Token::at(token_contract_address).transfer_public(owner, recipient, transfer_amount, 0); // Try to transfer tokens - env.assert_public_call_fails(public_transfer_call_interface); - // docs:end:assert_public_fail - // Check balances - utils::check_public_balance(token_contract_address, owner, mint_amount); + public_transfer_call_interface.call(&mut env.public()); } -#[test(should_fail_with="invalid nonce")] +//#[test(should_fail_with="invalid nonce")] +#[test(should_fail)] unconstrained fn public_transfer_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); // Transfer tokens let transfer_amount = mint_amount / 10; let public_transfer_call_interface = Token::at(token_contract_address).transfer_public(owner, recipient, transfer_amount, random()); + authwit_cheatcodes::add_public_authwit_from_call_interface(owner, recipient, public_transfer_call_interface); // Try to transfer tokens - env.call_public(public_transfer_call_interface); + public_transfer_call_interface.call(&mut env.public()); } -// Not checking error message here with should_fail_with because noir panics when I try doing that. -#[test] +#[test(should_fail)] unconstrained fn public_transfer_failure_on_behalf_of_other_without_approval() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -84,13 +80,11 @@ unconstrained fn public_transfer_failure_on_behalf_of_other_without_approval() { // Impersonate recipient to perform the call env.impersonate(recipient); // Try to transfer tokens - env.assert_public_call_fails(public_transfer_from_call_interface); - // Check balances - utils::check_public_balance(token_contract_address, owner, mint_amount); - utils::check_public_balance(token_contract_address, recipient, 0); + public_transfer_from_call_interface.call(&mut env.public()); } -#[test(should_fail_with="attempt to subtract with underflow")] +//#[test(should_fail_with="attempt to subtract with underflow")] +#[test(should_fail)] unconstrained fn public_transfer_failure_on_behalf_of_other_more_than_balance() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -102,11 +96,10 @@ unconstrained fn public_transfer_failure_on_behalf_of_other_more_than_balance() // Impersonate recipient to perform the call env.impersonate(recipient); // Try to transfer tokens - env.call_public(public_transfer_from_call_interface); + public_transfer_from_call_interface.call(&mut env.public()); } -// Not checking error message here with should_fail_with because noir panics when I try doing that. -#[test] +#[test(should_fail)] unconstrained fn public_transfer_failure_on_behalf_of_other_wrong_caller() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -116,8 +109,5 @@ unconstrained fn public_transfer_failure_on_behalf_of_other_wrong_caller() { // Impersonate recipient to perform the call env.impersonate(recipient); // Try to transfer tokens - env.assert_public_call_fails(public_transfer_from_call_interface); - // Check balances - utils::check_public_balance(token_contract_address, owner, mint_amount); - utils::check_public_balance(token_contract_address, recipient, 0); + public_transfer_from_call_interface.call(&mut env.public()); } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/unshielding.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/unshielding.nr index 50ce65ec2a3..c35e1df309a 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/unshielding.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/unshielding.nr @@ -9,8 +9,7 @@ unconstrained fn unshield_on_behalf_of_self() { let (env, token_contract_address, owner, _, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); let unshield_amount = mint_amount / 10; - let unshield_call_interface = Token::at(token_contract_address).unshield(owner, owner, unshield_amount, 0); - env.call_private_void(unshield_call_interface); + Token::at(token_contract_address).unshield(owner, owner, unshield_amount, 0).call(&mut env.private()); utils::check_private_balance(token_contract_address, owner, mint_amount - unshield_amount); utils::check_public_balance(token_contract_address, owner, mint_amount + unshield_amount); } @@ -25,7 +24,7 @@ unconstrained fn unshield_on_behalf_of_other() { // Impersonate recipient env.impersonate(recipient); // Unshield tokens - env.call_private_void(unshield_call_interface); + unshield_call_interface.call(&mut env.private()); utils::check_private_balance(token_contract_address, owner, mint_amount - unshield_amount); utils::check_public_balance(token_contract_address, recipient, unshield_amount); } @@ -36,8 +35,7 @@ unconstrained fn unshield_failure_more_than_balance() { let (env, token_contract_address, owner, _, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); let unshield_amount = mint_amount + 1; - let unshield_call_interface = Token::at(token_contract_address).unshield(owner, owner, unshield_amount, 0); - env.call_private_void(unshield_call_interface); + Token::at(token_contract_address).unshield(owner, owner, unshield_amount, 0).call(&mut env.private()); } #[test(should_fail_with="invalid nonce")] @@ -46,8 +44,7 @@ unconstrained fn unshield_failure_on_behalf_of_self_non_zero_nonce() { let (env, token_contract_address, owner, _, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); let unshield_amount = mint_amount + 1; - let unshield_call_interface = Token::at(token_contract_address).unshield(owner, owner, unshield_amount, random()); - env.call_private_void(unshield_call_interface); + Token::at(token_contract_address).unshield(owner, owner, unshield_amount, random()).call(&mut env.private()); } #[test(should_fail_with="Balance too low")] @@ -60,7 +57,7 @@ unconstrained fn unshield_failure_on_behalf_of_other_more_than_balance() { // Impersonate recipient env.impersonate(recipient); // Unshield tokens - env.call_private_void(unshield_call_interface); + unshield_call_interface.call(&mut env.private()); } #[test(should_fail_with="Authorization not found for message hash")] @@ -73,7 +70,7 @@ unconstrained fn unshield_failure_on_behalf_of_other_invalid_designated_caller() // Impersonate recipient env.impersonate(recipient); // Unshield tokens - env.call_private_void(unshield_call_interface); + unshield_call_interface.call(&mut env.private()); } #[test(should_fail_with="Authorization not found for message hash")] @@ -85,5 +82,5 @@ unconstrained fn unshield_failure_on_behalf_of_other_no_approval() { // Impersonate recipient env.impersonate(recipient); // Unshield tokens - env.call_private_void(unshield_call_interface); + unshield_call_interface.call(&mut env.private()); } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/utils.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/utils.nr index b05d4e6c00c..064162621cc 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/utils.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/utils.nr @@ -30,7 +30,7 @@ unconstrained pub fn setup(with_account_contracts: bool) -> (&mut TestEnvironmen "TT00000000000000000000000000000", 18 ); - let token_contract = env.deploy_self("Token").with_public_initializer(initializer_call_interface); + let token_contract = env.deploy_self("Token").with_public_void_initializer(initializer_call_interface); let token_contract_address = token_contract.to_address(); env.advance_block_by(1); (&mut env, token_contract_address, owner, recipient) @@ -43,11 +43,8 @@ unconstrained pub fn setup_and_mint(with_account_contracts: bool) -> (&mut TestE // Mint some tokens let secret = random(); let secret_hash = compute_secret_hash(secret); - let mint_private_call_interface = Token::at(token_contract_address).mint_private(mint_amount, secret_hash); - env.call_public(mint_private_call_interface); - - let mint_public_call_interface = Token::at(token_contract_address).mint_public(owner, mint_amount); - env.call_public(mint_public_call_interface); + Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); + Token::at(token_contract_address).mint_public(owner, mint_amount).call(&mut env.public()); // Time travel so we can read keys from the registry env.advance_block_by(6); @@ -62,8 +59,7 @@ unconstrained pub fn setup_and_mint(with_account_contracts: bool) -> (&mut TestE // docs:end:txe_test_add_note // Redeem our shielded tokens - let redeem_shield_call_interface = Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret); - env.call_private_void(redeem_shield_call_interface); + Token::at(token_contract_address).redeem_shield(owner, mint_amount, secret).call(&mut env.private()); (env, token_contract_address, owner, recipient, mint_amount) } diff --git a/yarn-project/circuit-types/src/simulation_error.ts b/yarn-project/circuit-types/src/simulation_error.ts index 8287da9bc00..2d88c1e6808 100644 --- a/yarn-project/circuit-types/src/simulation_error.ts +++ b/yarn-project/circuit-types/src/simulation_error.ts @@ -97,6 +97,13 @@ export class SimulationError extends Error { get() { return getStack(); }, + /** + * We need a setter to avoid the error "TypeError: Cannot set property stack of # which has only a getter" + * whenever we are traversing a nested error chain. However, we don't want to allow setting the stack, since the simulation + * error is always gonna be the root of the error chain. + * @param value + */ + set(_: string | undefined) {}, }, }); } diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 2b73c7cc025..cfee7ebc676 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -736,6 +736,14 @@ export class TXE implements TypedOracle { // Apply side effects if (!executionResult.reverted) { this.sideEffectsCounter = executionResult.endSideEffectCounter.toNumber() + 1; + await this.addNoteHashes( + targetContractAddress, + executionResult.noteHashes.map(noteHash => noteHash.value), + ); + await this.addNullifiers( + targetContractAddress, + executionResult.nullifiers.map(nullifier => nullifier.value), + ); } this.setContractAddress(currentContractAddress); this.setMsgSender(currentMessageSender); @@ -783,6 +791,15 @@ export class TXE implements TypedOracle { // Apply side effects this.sideEffectsCounter = executionResult.endSideEffectCounter.toNumber() + 1; + await this.addNoteHashes( + targetContractAddress, + executionResult.noteHashes.map(noteHash => noteHash.value), + ); + await this.addNullifiers( + targetContractAddress, + executionResult.nullifiers.map(nullifier => nullifier.value), + ); + this.setContractAddress(currentContractAddress); this.setMsgSender(currentMessageSender); this.setFunctionSelector(currentFunctionSelector); diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index e97311d7772..331654662c5 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -9,7 +9,7 @@ import { computePartialAddress, getContractInstanceFromDeployParams, } from '@aztec/circuits.js'; -import { computePublicDataTreeLeafSlot } from '@aztec/circuits.js/hash'; +import { computePublicDataTreeLeafSlot, computeVarArgsHash } from '@aztec/circuits.js/hash'; import { type ContractArtifact, NoteSelector } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { type Logger } from '@aztec/foundation/log'; @@ -241,32 +241,6 @@ export class TXEService { return toForeignCallResult([]); } - async assertPrivateCallFails( - targetContractAddress: ForeignCallSingle, - functionSelector: ForeignCallSingle, - argsHash: ForeignCallSingle, - sideEffectCounter: ForeignCallSingle, - isStaticCall: ForeignCallSingle, - isDelegateCall: ForeignCallSingle, - ) { - try { - await this.typedOracle.callPrivateFunction( - fromSingle(targetContractAddress), - FunctionSelector.fromField(fromSingle(functionSelector)), - fromSingle(argsHash), - fromSingle(sideEffectCounter).toNumber(), - fromSingle(isStaticCall).toBool(), - fromSingle(isDelegateCall).toBool(), - ); - throw new ExpectedFailureError('Private call did not fail'); - } catch (e) { - if (e instanceof ExpectedFailureError) { - throw e; - } - } - return toForeignCallResult([]); - } - setFunctionSelector(functionSelector: ForeignCallSingle) { (this.typedOracle as TXE).setFunctionSelector(FunctionSelector.fromField(fromSingle(functionSelector))); return toForeignCallResult([]); @@ -298,41 +272,11 @@ export class TXEService { return toForeignCallResult([toSingle(new Fr(blockNumber))]); } - async avmOpcodeAddress() { - const contractAddress = await this.typedOracle.getContractAddress(); - return toForeignCallResult([toSingle(contractAddress.toField())]); - } - - async avmOpcodeBlockNumber() { - const blockNumber = await this.typedOracle.getBlockNumber(); - return toForeignCallResult([toSingle(new Fr(blockNumber))]); - } - - avmOpcodeFunctionSelector() { - const functionSelector = (this.typedOracle as TXE).getFunctionSelector(); - return toForeignCallResult([toSingle(functionSelector.toField())]); - } - setIsStaticCall(isStaticCall: ForeignCallSingle) { (this.typedOracle as TXE).setIsStaticCall(fromSingle(isStaticCall).toBool()); return toForeignCallResult([]); } - avmOpcodeIsStaticCall() { - const isStaticCall = (this.typedOracle as TXE).getIsStaticCall(); - return toForeignCallResult([toSingle(new Fr(isStaticCall ? 1 : 0))]); - } - - async avmOpcodeChainId() { - const chainId = await (this.typedOracle as TXE).getChainId(); - return toForeignCallResult([toSingle(chainId)]); - } - - async avmOpcodeVersion() { - const version = await (this.typedOracle as TXE).getVersion(); - return toForeignCallResult([toSingle(version)]); - } - async packArgumentsArray(args: ForeignCallArray) { const packed = await this.typedOracle.packArgumentsArray(fromArray(args)); return toForeignCallResult([toSingle(packed)]); @@ -513,44 +457,6 @@ export class TXEService { ]); } - async avmOpcodeGetContractInstance(address: ForeignCallSingle) { - const instance = await this.typedOracle.getContractInstance(fromSingle(address)); - return toForeignCallResult([ - toArray([ - // AVM requires an extra boolean indicating the instance was found - new Fr(1), - instance.salt, - instance.deployer, - instance.contractClassId, - instance.initializationHash, - instance.publicKeysHash, - ]), - ]); - } - - avmOpcodeSender() { - const sender = (this.typedOracle as TXE).getMsgSender(); - return toForeignCallResult([toSingle(sender)]); - } - - async avmOpcodeEmitNullifier(nullifier: ForeignCallSingle) { - await (this.typedOracle as TXE).avmOpcodeEmitNullifier(fromSingle(nullifier)); - return toForeignCallResult([]); - } - - async avmOpcodeEmitNoteHash(noteHash: ForeignCallSingle) { - await (this.typedOracle as TXE).avmOpcodeEmitNoteHash(fromSingle(noteHash)); - return toForeignCallResult([]); - } - - async avmOpcodeNullifierExists(innerNullifier: ForeignCallSingle, targetAddress: ForeignCallSingle) { - const exists = await (this.typedOracle as TXE).avmOpcodeNullifierExists( - fromSingle(innerNullifier), - AztecAddress.fromField(fromSingle(targetAddress)), - ); - return toForeignCallResult([toSingle(new Fr(exists))]); - } - async avmOpcodeCall( _gas: ForeignCallArray, address: ForeignCallSingle, @@ -566,7 +472,7 @@ export class TXEService { /* isDelegateCall */ false, ); - return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(1))]); + return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(!result.reverted))]); } async avmOpcodeStaticCall( @@ -587,27 +493,6 @@ export class TXEService { return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(1))]); } - async avmOpcodeStorageRead(slot: ForeignCallSingle) { - const value = await (this.typedOracle as TXE).avmOpcodeStorageRead(fromSingle(slot)); - return toForeignCallResult([toSingle(value)]); - } - - async avmOpcodeStorageWrite(slot: ForeignCallSingle, value: ForeignCallSingle) { - await this.typedOracle.storageWrite(fromSingle(slot), [fromSingle(value)]); - return toForeignCallResult([]); - } - - //unconstrained fn calldata_copy_opcode(cdoffset: u32, copy_size: u32) -> [Field; N] {} - avmOpcodeCalldataCopy(cdOffsetInput: ForeignCallSingle, copySizeInput: ForeignCallSingle) { - const cdOffset = fromSingle(cdOffsetInput).toNumber(); - const copySize = fromSingle(copySizeInput).toNumber(); - - const calldata = (this.typedOracle as TXE).getCalldata(); - const calldataSlice = calldata.slice(cdOffset, cdOffset + copySize); - - return toForeignCallResult([toArray(calldataSlice)]); - } - async getPublicKeysAndPartialAddress(address: ForeignCallSingle) { const parsedAddress = AztecAddress.fromField(fromSingle(address)); const { publicKeys, partialAddress } = await this.typedOracle.getCompleteAddress(parsedAddress); @@ -765,9 +650,4 @@ export class TXEService { // TODO(#8811): Implement return toForeignCallResult([]); } - - avmOpcodeEmitUnencryptedLog(_message: ForeignCallArray) { - // TODO(#8811): Implement - return toForeignCallResult([]); - } } From 67fd3fac124157a2803f2df6b4bce973e36928d2 Mon Sep 17 00:00:00 2001 From: thunkar Date: Fri, 11 Oct 2024 14:35:10 +0000 Subject: [PATCH 02/14] more cleanup --- .../aztec/src/test/helpers/cheatcodes.nr | 70 ++----------- .../src/test/helpers/test_environment.nr | 2 +- .../aztec-nr/aztec/src/test/helpers/utils.nr | 17 ---- yarn-project/txe/src/oracle/txe_oracle.ts | 51 ---------- .../txe/src/txe_service/txe_service.ts | 99 +++++++------------ 5 files changed, 46 insertions(+), 193 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr index 8ca8e57c17d..a758a89ec15 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr @@ -10,6 +10,10 @@ unconstrained pub fn reset() { oracle_reset(); } +unconstrained pub fn get_side_effects_counter() -> u32 { + oracle_get_side_effects_counter() +} + unconstrained pub fn set_contract_address(address: AztecAddress) { oracle_set_contract_address(address); } @@ -49,22 +53,6 @@ unconstrained pub fn derive_keys(secret: Field) -> PublicKeys { oracle_derive_keys(secret) } -unconstrained pub fn set_msg_sender(msg_sender: AztecAddress) { - oracle_set_msg_sender(msg_sender) -} - -unconstrained pub fn set_calldata(calldata: [Field]) { - oracle_set_calldata(calldata) -} - -unconstrained pub fn get_msg_sender() -> AztecAddress { - oracle_get_msg_sender() -} - -unconstrained pub fn get_side_effects_counter() -> u32 { - oracle_get_side_effects_counter() -} - unconstrained pub fn add_authwit(address: AztecAddress, message_hash: Field) { orable_add_authwit(address, message_hash) } @@ -73,32 +61,15 @@ unconstrained pub fn assert_public_call_fails(target_address: AztecAddress, func oracle_assert_public_call_fails(target_address, function_selector, args) } -unconstrained pub fn add_nullifiers(contractAddress: AztecAddress, nullifiers: [Field]) { - oracle_add_nullifiers(contractAddress, nullifiers) -} - -unconstrained pub fn add_note_hashes(contractAddress: AztecAddress, note_hashes: [Field]) { - oracle_add_note_hashes(contractAddress, note_hashes) -} - -unconstrained pub fn get_function_selector() -> FunctionSelector { - oracle_get_function_selector() -} - -unconstrained pub fn set_fn_selector(selector: FunctionSelector) { - oracle_set_function_selector(selector) -} - -unconstrained pub fn set_is_static_call(is_static: bool) { - oracle_set_is_static_call(is_static) -} - #[oracle(reset)] unconstrained fn oracle_reset() {} #[oracle(setContractAddress)] unconstrained fn oracle_set_contract_address(address: AztecAddress) {} +#[oracle(getSideEffectsCounter)] +unconstrained fn oracle_get_side_effects_counter() -> u32 {} + #[oracle(advanceBlocksBy)] unconstrained fn oracle_advance_blocks_by(blocks: u32) {} @@ -130,18 +101,6 @@ unconstrained fn oracle_add_account(secret: Field) -> TestAccount {} #[oracle(deriveKeys)] unconstrained fn oracle_derive_keys(secret: Field) -> PublicKeys {} -#[oracle(getMsgSender)] -unconstrained fn oracle_get_msg_sender() -> AztecAddress {} - -#[oracle(setMsgSender)] -unconstrained fn oracle_set_msg_sender(msg_sender: AztecAddress) {} - -#[oracle(setIsStaticCall)] -unconstrained fn oracle_set_is_static_call(is_static: bool) {} - -#[oracle(getSideEffectsCounter)] -unconstrained fn oracle_get_side_effects_counter() -> u32 {} - #[oracle(addAuthWitness)] unconstrained fn orable_add_authwit(address: AztecAddress, message_hash: Field) {} @@ -151,18 +110,3 @@ unconstrained fn oracle_assert_public_call_fails( function_selector: FunctionSelector, args: [Field] ) {} - -#[oracle(addNullifiers)] -unconstrained fn oracle_add_nullifiers(contractAddress: AztecAddress, nullifiers: [Field]) {} - -#[oracle(addNoteHashes)] -unconstrained fn oracle_add_note_hashes(contractAddress: AztecAddress, note_hashes: [Field]) {} - -#[oracle(getFunctionSelector)] -unconstrained fn oracle_get_function_selector() -> FunctionSelector {} - -#[oracle(setFunctionSelector)] -unconstrained fn oracle_set_function_selector(selector: FunctionSelector) {} - -#[oracle(setCalldata)] -unconstrained fn oracle_set_calldata(calldata: [Field]) {} diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr index 95796d8ec82..443070d1ff3 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr @@ -4,7 +4,7 @@ use crate::context::inputs::PrivateContextInputs; use crate::context::{packed_returns::PackedReturns, call_interfaces::CallInterface}; use crate::context::{PrivateContext, PublicContext, UnconstrainedContext}; -use crate::test::helpers::{cheatcodes, utils::{apply_side_effects_private, Deployer}}; +use crate::test::helpers::{cheatcodes, utils::Deployer}; use crate::hash::hash_args; use crate::note::{note_header::NoteHeader, note_interface::{NoteInterface, NullifiableNote}}; diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr index 6c1f2adc03c..37b082b29ac 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr @@ -16,23 +16,6 @@ use crate::context::call_interfaces::PublicVoidCallInterface; use crate::hash::hash_args; use crate::oracle::arguments::pack_arguments; -unconstrained pub fn apply_side_effects_private(contract_address: AztecAddress, public_inputs: PrivateCircuitPublicInputs) { - let mut nullifiers = &[]; - for nullifier in public_inputs.nullifiers { - if nullifier.value != 0 { - nullifiers = nullifiers.push_back(nullifier.value); - } - } - cheatcodes::add_nullifiers(contract_address, nullifiers); - let mut note_hashes = &[]; - for note_hash in public_inputs.note_hashes { - if note_hash.value != 0 { - note_hashes = note_hashes.push_back(note_hash.value); - } - } - cheatcodes::add_note_hashes(contract_address, note_hashes); -} - pub struct Deployer { path: str, name: str, diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index cfee7ebc676..97f0958e8eb 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -77,9 +77,6 @@ export class TXE implements TypedOracle { private msgSender: AztecAddress; private functionSelector = FunctionSelector.fromField(new Fr(0)); private isStaticCall = false; - // This will hold the _real_ calldata. That is, the one without the PublicContextInputs. - // TODO: Remove this comment once PublicContextInputs are removed. - private calldata: Fr[] = []; private contractDataOracle: ContractDataOracle; @@ -126,18 +123,10 @@ export class TXE implements TypedOracle { return this.functionSelector; } - getCalldata() { - return this.calldata; - } - setMsgSender(msgSender: Fr) { this.msgSender = msgSender; } - setCalldata(calldata: Fr[]) { - this.calldata = calldata; - } - setFunctionSelector(functionSelector: FunctionSelector) { this.functionSelector = functionSelector; } @@ -208,27 +197,6 @@ export class TXE implements TypedOracle { return inputs; } - async avmOpcodeNullifierExists(innerNullifier: Fr, targetAddress: AztecAddress): Promise { - const nullifier = siloNullifier(targetAddress, innerNullifier!); - const db = await this.trees.getLatest(); - const index = await db.findLeafIndex(MerkleTreeId.NULLIFIER_TREE, nullifier.toBuffer()); - return index !== undefined; - } - - async avmOpcodeEmitNullifier(nullifier: Fr) { - const db = await this.trees.getLatest(); - const siloedNullifier = siloNullifier(this.contractAddress, nullifier); - await db.batchInsert(MerkleTreeId.NULLIFIER_TREE, [siloedNullifier.toBuffer()], NULLIFIER_SUBTREE_HEIGHT); - return Promise.resolve(); - } - - async avmOpcodeEmitNoteHash(noteHash: Fr) { - const db = await this.trees.getLatest(); - const siloedNoteHash = siloNoteHash(this.contractAddress, noteHash); - await db.appendLeaves(MerkleTreeId.NOTE_HASH_TREE, [siloedNoteHash]); - return Promise.resolve(); - } - deriveKeys(secret: Fr) { return deriveKeys(secret); } @@ -467,24 +435,6 @@ export class TXE implements TypedOracle { throw new Error('Method not implemented.'); } - async avmOpcodeStorageRead(slot: Fr) { - const db = await this.trees.getLatest(); - - const leafSlot = computePublicDataTreeLeafSlot(this.contractAddress, slot); - - const lowLeafResult = await db.getPreviousValueIndex(MerkleTreeId.PUBLIC_DATA_TREE, leafSlot.toBigInt()); - if (!lowLeafResult || !lowLeafResult.alreadyPresent) { - return Fr.ZERO; - } - - const preimage = (await db.getLeafPreimage( - MerkleTreeId.PUBLIC_DATA_TREE, - lowLeafResult.index, - )) as PublicDataTreeLeafPreimage; - - return preimage.value; - } - async storageRead( contractAddress: Fr, startStorageSlot: Fr, @@ -717,7 +667,6 @@ export class TXE implements TypedOracle { this.setMsgSender(this.contractAddress); this.setContractAddress(targetContractAddress); this.setFunctionSelector(functionSelector); - this.setCalldata(args); const callContext = CallContext.empty(); callContext.msgSender = this.msgSender; diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index 331654662c5..db879b3f3cd 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -199,16 +199,6 @@ export class TXEService { ]); } - setMsgSender(msgSender: ForeignCallSingle) { - (this.typedOracle as TXE).setMsgSender(fromSingle(msgSender)); - return toForeignCallResult([]); - } - - getMsgSender() { - const msgSender = (this.typedOracle as TXE).getMsgSender(); - return toForeignCallResult([toSingle(msgSender)]); - } - getSideEffectsCounter() { const counter = (this.typedOracle as TXE).getSideEffectsCounter(); return toForeignCallResult([toSingle(new Fr(counter))]); @@ -241,21 +231,6 @@ export class TXEService { return toForeignCallResult([]); } - setFunctionSelector(functionSelector: ForeignCallSingle) { - (this.typedOracle as TXE).setFunctionSelector(FunctionSelector.fromField(fromSingle(functionSelector))); - return toForeignCallResult([]); - } - - setCalldata(_length: ForeignCallSingle, calldata: ForeignCallArray) { - (this.typedOracle as TXE).setCalldata(fromArray(calldata)); - return toForeignCallResult([]); - } - - getFunctionSelector() { - const functionSelector = (this.typedOracle as TXE).getFunctionSelector(); - return toForeignCallResult([toSingle(functionSelector.toField())]); - } - // PXE oracles getRandomField() { @@ -457,42 +432,6 @@ export class TXEService { ]); } - async avmOpcodeCall( - _gas: ForeignCallArray, - address: ForeignCallSingle, - _length: ForeignCallSingle, - args: ForeignCallArray, - functionSelector: ForeignCallSingle, - ) { - const result = await (this.typedOracle as TXE).avmOpcodeCall( - fromSingle(address), - FunctionSelector.fromField(fromSingle(functionSelector)), - fromArray(args), - /* isStaticCall */ false, - /* isDelegateCall */ false, - ); - - return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(!result.reverted))]); - } - - async avmOpcodeStaticCall( - _gas: ForeignCallArray, - address: ForeignCallSingle, - _length: ForeignCallSingle, - args: ForeignCallArray, - functionSelector: ForeignCallSingle, - ) { - const result = await (this.typedOracle as TXE).avmOpcodeCall( - fromSingle(address), - FunctionSelector.fromField(fromSingle(functionSelector)), - fromArray(args), - /* isStaticCall */ true, - /* isDelegateCall */ false, - ); - - return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(1))]); - } - async getPublicKeysAndPartialAddress(address: ForeignCallSingle) { const parsedAddress = AztecAddress.fromField(fromSingle(address)); const { publicKeys, partialAddress } = await this.typedOracle.getCompleteAddress(parsedAddress); @@ -650,4 +589,42 @@ export class TXEService { // TODO(#8811): Implement return toForeignCallResult([]); } + + // AVM oracles (only to perform calls directly from the tests) + + async avmOpcodeCall( + _gas: ForeignCallArray, + address: ForeignCallSingle, + _length: ForeignCallSingle, + args: ForeignCallArray, + functionSelector: ForeignCallSingle, + ) { + const result = await (this.typedOracle as TXE).avmOpcodeCall( + fromSingle(address), + FunctionSelector.fromField(fromSingle(functionSelector)), + fromArray(args), + /* isStaticCall */ false, + /* isDelegateCall */ false, + ); + + return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(!result.reverted))]); + } + + async avmOpcodeStaticCall( + _gas: ForeignCallArray, + address: ForeignCallSingle, + _length: ForeignCallSingle, + args: ForeignCallArray, + functionSelector: ForeignCallSingle, + ) { + const result = await (this.typedOracle as TXE).avmOpcodeCall( + fromSingle(address), + FunctionSelector.fromField(fromSingle(functionSelector)), + fromArray(args), + /* isStaticCall */ true, + /* isDelegateCall */ false, + ); + + return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(1))]); + } } From 725f2e6e08de02225018b5701e05d9ba6f635e08 Mon Sep 17 00:00:00 2001 From: thunkar Date: Fri, 11 Oct 2024 16:32:02 +0000 Subject: [PATCH 03/14] more cleanup --- .../aztec/src/macros/functions/mod.nr | 35 +----------------- .../aztec/src/test/helpers/cheatcodes.nr | 28 +++++++++++++++ .../src/test/helpers/test_environment.nr | 14 ++++++++ .../contracts/auth_contract/src/test/main.nr | 32 ++++++++--------- .../contracts/auth_contract/src/test/utils.nr | 2 +- .../nft_contract/src/test/access_control.nr | 16 ++++----- .../nft_contract/src/test/minting.nr | 5 ++- .../src/test/transfer_in_private.nr | 16 ++++----- .../src/test/transfer_in_public.nr | 30 +++++++--------- .../src/test/transfer_to_private.nr | 36 ++++++++----------- .../src/test/transfer_to_public.nr | 14 ++++---- .../contracts/nft_contract/src/test/utils.nr | 14 +++----- .../contracts/parent_contract/src/main.nr | 7 ++-- .../contracts/router_contract/src/test.nr | 21 ++++++++--- .../token_contract/src/test/minting.nr | 8 ++--- .../src/test/reading_constants.nr | 2 +- .../src/test/transfer_public.nr | 11 +++--- .../txe/src/txe_service/txe_service.ts | 26 ++++++++++++++ 18 files changed, 175 insertions(+), 142 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr index 8a29d74e16b..cb9ac24454a 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr @@ -1,6 +1,6 @@ mod interfaces; -use std::{meta::type_of, collections::umap::UHashMap, hash::{BuildHasherDefault, poseidon2::Poseidon2Hasher}}; +use std::meta::type_of; use super::utils::{ modify_fn_body, is_fn_private, get_fn_visibility, is_fn_view, is_fn_initializer, is_fn_internal, fn_has_noinitcheck, add_to_hasher, module_has_storage, module_has_initializer @@ -10,8 +10,6 @@ use protocol_types::meta::flatten_to_fields; use interfaces::{create_fn_abi_export, register_stub, stub_fn}; use super::utils::fn_requires_authwit; -comptime mut global AUTHWITS: UHashMap> = UHashMap::default(); - // Functions can have multiple attributes applied to them, e.g. a single function can have #[public], #[view] and // #[internal]. However. the order in which this will be evaluated is unknown, which makes combining them tricky. // @@ -65,10 +63,6 @@ pub comptime fn noinitcheck(_f: FunctionDefinition) { // Marker attribute } -pub comptime fn authwit(f: FunctionDefinition, arg: Quoted) { - AUTHWITS.insert(f, arg) -} - comptime fn create_assert_correct_initializer_args(f: FunctionDefinition) -> Quoted { let fn_visibility = get_fn_visibility(f); f"dep::aztec::initializer::assert_initialization_matches_address_preimage_{fn_visibility}(context);".quoted_contents() @@ -84,26 +78,6 @@ comptime fn create_init_check(f: FunctionDefinition) -> Quoted { f"dep::aztec::initializer::assert_is_initialized_{fn_visibility}(&mut context);".quoted_contents() } -comptime fn create_authwit_check(f: FunctionDefinition) -> Quoted { - let maybe_arg = AUTHWITS.get(f); - let fn_visibility = get_fn_visibility(f); - assert(maybe_arg.is_some(), f"#[authwit(...)] attribute must be applied after #[{fn_visibility}]"); - let arg = maybe_arg.unwrap(); - let authwit_fn_suffix = if is_fn_private(f) { - quote { } - } else { - quote { _public } - }; - let fn_name = f"dep::authwit::auth::assert_current_call_valid_authwit{authwit_fn_suffix}".quoted_contents(); - quote { - if (!$arg.eq(context.msg_sender())) { - $fn_name(&mut context, $arg); - } else { - assert(nonce == 0, "invalid nonce"); - } - } -} - /// Private functions are executed client-side and preserve privacy. pub comptime fn private(f: FunctionDefinition) -> Quoted { let fn_abi = create_fn_abi_export(f); @@ -152,12 +126,6 @@ pub comptime fn private(f: FunctionDefinition) -> Quoted { // Modifications introduced by the different marker attributes. - let authwit_check = if fn_requires_authwit(f) { - create_authwit_check(f) - } else { - quote {} - }; - let internal_check = if is_fn_internal(f) { create_internal_check(f) } else { @@ -238,7 +206,6 @@ pub comptime fn private(f: FunctionDefinition) -> Quoted { $internal_check $view_check $storage_init - $authwit_check }; let to_append = quote { diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr index a758a89ec15..7a5872eb580 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr @@ -61,6 +61,24 @@ unconstrained pub fn assert_public_call_fails(target_address: AztecAddress, func oracle_assert_public_call_fails(target_address, function_selector, args) } +unconstrained pub fn assert_private_call_fails( + target_address: AztecAddress, + function_selector: FunctionSelector, + argsHash: Field, + sideEffectsCounter: Field, + isStaticCall: bool, + isDelegateCall: bool +) { + oracle_assert_private_call_fails( + target_address, + function_selector, + argsHash, + sideEffectsCounter, + isStaticCall, + isDelegateCall + ) +} + #[oracle(reset)] unconstrained fn oracle_reset() {} @@ -110,3 +128,13 @@ unconstrained fn oracle_assert_public_call_fails( function_selector: FunctionSelector, args: [Field] ) {} + +#[oracle(assertPrivateCallFails)] +unconstrained fn oracle_assert_private_call_fails( + target_address: AztecAddress, + function_selector: FunctionSelector, + argsHash: Field, + sideEffectsCounter: Field, + isStaticCall: bool, + isDelegateCall: bool +) {} diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr index 443070d1ff3..b2c6d4f2c4e 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr @@ -104,6 +104,20 @@ impl TestEnvironment { cheatcodes::assert_public_call_fails(call_interface.get_contract_address(), fn_selector, calldata); } + unconstrained fn assert_private_call_fails( + _self: Self, + call_interface: C + ) where C: CallInterface { + cheatcodes::assert_private_call_fails( + call_interface.get_contract_address(), + call_interface.get_selector(), + hash_args(call_interface.get_args()), + cheatcodes::get_side_effects_counter() as Field, + call_interface.get_is_static(), + false + ); + } + /// Manually adds a note to TXE. This needs to be called if you want to work with a note in your test with the note /// not having an encrypted log emitted. TXE alternative to `PXE.addNote(...)`. unconstrained pub fn add_note( diff --git a/noir-projects/noir-contracts/contracts/auth_contract/src/test/main.nr b/noir-projects/noir-contracts/contracts/auth_contract/src/test/main.nr index f0a7775b16c..e6caadb3e9b 100644 --- a/noir-projects/noir-contracts/contracts/auth_contract/src/test/main.nr +++ b/noir-projects/noir-contracts/contracts/auth_contract/src/test/main.nr @@ -13,7 +13,7 @@ unconstrained fn main() { let authorized_is_unset_initially = || { env.impersonate(admin); - let authorized = env.call_public(Auth::at(auth_contract_address).get_authorized()); + let authorized = Auth::at(auth_contract_address).get_authorized().view(&mut env.public()); assert_eq(authorized, AztecAddress::from_field(0)); }; authorized_is_unset_initially(); @@ -26,17 +26,17 @@ unconstrained fn main() { let admin_sets_authorized = || { env.impersonate(admin); - env.call_public(Auth::at(auth_contract_address).set_authorized(to_authorize)); + Auth::at(auth_contract_address).set_authorized(to_authorize).call(&mut env.public()); env.advance_block_by(1); - let scheduled_authorized = env.call_public(Auth::at(auth_contract_address).get_scheduled_authorized()); + let scheduled_authorized = Auth::at(auth_contract_address).get_scheduled_authorized().view(&mut env.public()); assert_eq(scheduled_authorized, to_authorize); }; admin_sets_authorized(); let authorized_is_not_yet_effective = || { env.impersonate(to_authorize); - let authorized: AztecAddress = env.call_public(Auth::at(auth_contract_address).get_authorized()); + let authorized = Auth::at(auth_contract_address).get_authorized().view(&mut env.public()); assert_eq(authorized, AztecAddress::zero()); env.assert_private_call_fails(Auth::at(auth_contract_address).do_private_authorized_thing()); @@ -48,10 +48,10 @@ unconstrained fn main() { // We advance block by 4, because the delay is 5, and we initially advanced block by one after setting the value. See below comment for explanation. env.advance_block_by(CHANGE_AUTHORIZED_DELAY_BLOCKS - 1); - let authorized: AztecAddress = env.call_public(Auth::at(auth_contract_address).get_authorized()); + let authorized = Auth::at(auth_contract_address).get_authorized().view(&mut env.public()); assert_eq(authorized, to_authorize); - let authorized_in_private: AztecAddress = env.call_private(Auth::at(auth_contract_address).get_authorized_in_private()); + let authorized_in_private: AztecAddress = Auth::at(auth_contract_address).get_authorized_in_private().view(&mut env.private()); assert_eq(authorized_in_private, AztecAddress::zero()); // We need to always advance the block one more time to get the current value in private, compared to the value in public. @@ -73,26 +73,26 @@ unconstrained fn main() { // ^ // get_authorized() (private) called here with block_number = 8 env.advance_block_by(1); - let authorized_in_private_again: AztecAddress = env.call_private(Auth::at(auth_contract_address).get_authorized_in_private()); + let authorized_in_private_again = Auth::at(auth_contract_address).get_authorized_in_private().view(&mut env.private()); assert_eq(authorized_in_private_again, to_authorize); - env.call_private_void(Auth::at(auth_contract_address).do_private_authorized_thing()); + Auth::at(auth_contract_address).do_private_authorized_thing().call(&mut env.private()); }; authorized_becomes_effective_after_delay(); let authorize_other = || { env.impersonate(admin); - env.call_public(Auth::at(auth_contract_address).set_authorized(other)); + Auth::at(auth_contract_address).set_authorized(other).call(&mut env.public()); env.advance_block_by(1); - let scheduled_authorized = env.call_public(Auth::at(auth_contract_address).get_scheduled_authorized()); + let scheduled_authorized = Auth::at(auth_contract_address).get_scheduled_authorized().view(&mut env.public()); assert_eq(scheduled_authorized, other); - let authorized: AztecAddress = env.call_public(Auth::at(auth_contract_address).get_authorized()); + let authorized: AztecAddress = Auth::at(auth_contract_address).get_authorized().view(&mut env.public()); assert_eq(authorized, to_authorize); env.impersonate(to_authorize); - env.call_private_void(Auth::at(auth_contract_address).do_private_authorized_thing()); + Auth::at(auth_contract_address).do_private_authorized_thing().call(&mut env.private()); env.impersonate(other); env.assert_private_call_fails(Auth::at(auth_contract_address).do_private_authorized_thing()); @@ -104,20 +104,20 @@ unconstrained fn main() { // We advance the block by 4 again like above env.advance_block_by(CHANGE_AUTHORIZED_DELAY_BLOCKS - 1); - let authorized: AztecAddress = env.call_public(Auth::at(auth_contract_address).get_authorized()); + let authorized = Auth::at(auth_contract_address).get_authorized().view(&mut env.public()); assert_eq(authorized, other); - let authorized_in_private: AztecAddress = env.call_private(Auth::at(auth_contract_address).get_authorized_in_private()); + let authorized_in_private = Auth::at(auth_contract_address).get_authorized_in_private().view(&mut env.private()); assert_eq(authorized_in_private, to_authorize); env.advance_block_by(1); - let authorized_in_private_again: AztecAddress = env.call_private(Auth::at(auth_contract_address).get_authorized_in_private()); + let authorized_in_private_again = Auth::at(auth_contract_address).get_authorized_in_private().view(&mut env.private()); assert_eq(authorized_in_private_again, other); env.assert_private_call_fails(Auth::at(auth_contract_address).do_private_authorized_thing()); env.impersonate(other); - env.call_private_void(Auth::at(auth_contract_address).do_private_authorized_thing()); + Auth::at(auth_contract_address).do_private_authorized_thing().call(&mut env.private()); }; authorized_becomes_effective_after_delay_again(); } diff --git a/noir-projects/noir-contracts/contracts/auth_contract/src/test/utils.nr b/noir-projects/noir-contracts/contracts/auth_contract/src/test/utils.nr index 5f3204cf9c3..04885de0885 100644 --- a/noir-projects/noir-contracts/contracts/auth_contract/src/test/utils.nr +++ b/noir-projects/noir-contracts/contracts/auth_contract/src/test/utils.nr @@ -11,7 +11,7 @@ unconstrained pub fn setup() -> (&mut TestEnvironment, AztecAddress, AztecAddres let initializer_call_interface = Auth::interface().constructor(admin); - let auth_contract = env.deploy_self("Auth").with_public_initializer(initializer_call_interface); + let auth_contract = env.deploy_self("Auth").with_public_void_initializer(initializer_call_interface); let auth_contract_address = auth_contract.to_address(); env.advance_block_by(1); (&mut env, auth_contract_address, admin, to_authorize, other) diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/access_control.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/access_control.nr index f07c2c905c3..b74c646dc3b 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/access_control.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/access_control.nr @@ -7,10 +7,10 @@ unconstrained fn access_control() { let (env, nft_contract_address, owner, recipient) = utils::setup(/* with_account_contracts */ false); // Set a new admin - env.call_public(NFT::at(nft_contract_address).set_admin(recipient)); + NFT::at(nft_contract_address).set_admin(recipient).call(&mut env.public()); // Check it worked - let admin = env.call_public(NFT::at(nft_contract_address).get_admin()); + let admin = NFT::at(nft_contract_address).get_admin().view(&mut env.public()); assert(admin == recipient.to_field()); // Impersonate new admin @@ -18,22 +18,20 @@ unconstrained fn access_control() { // Check new admin is not a minter let is_minter_call_interface = NFT::at(nft_contract_address).is_minter(recipient); - let is_minter = env.call_public(is_minter_call_interface); + let is_minter = is_minter_call_interface.view(&mut env.public()); assert(is_minter == false); // Set admin as minter - let set_minter_call_interface = NFT::at(nft_contract_address).set_minter(recipient, true); - env.call_public(set_minter_call_interface); + NFT::at(nft_contract_address).set_minter(recipient, true).call(&mut env.public()); // Check it worked - let is_minter = env.call_public(is_minter_call_interface); + let is_minter = is_minter_call_interface.view(&mut env.public()); assert(is_minter == true); // Revoke minter as admin - let set_minter_call_interface = NFT::at(nft_contract_address).set_minter(recipient, false); - env.call_public(set_minter_call_interface); + NFT::at(nft_contract_address).set_minter(recipient, false).call(&mut env.public()); // Check it worked - let is_minter = env.call_public(is_minter_call_interface); + let is_minter = is_minter_call_interface.view(&mut env.public()); assert(is_minter == false); // Impersonate original admin diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/minting.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/minting.nr index c50d28c1a6e..1cb34b43203 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/minting.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/minting.nr @@ -7,8 +7,7 @@ unconstrained fn mint_success() { let (env, nft_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false); let token_id = 10000; - let mint_call_interface = NFT::at(nft_contract_address).mint(owner, token_id); - env.call_public(mint_call_interface); + NFT::at(nft_contract_address).mint(owner, token_id).call(&mut env.public()); utils::assert_owns_public_nft(env, nft_contract_address, owner, token_id); } @@ -28,7 +27,7 @@ unconstrained fn mint_failures() { // MINTING THE SAME NFT TWICE env.impersonate(owner); - env.call_public(mint_call_interface); + mint_call_interface.call(&mut env.public()); assert(utils::get_nft_exists(nft_contract_address, token_id), "NFT not minted"); // Second call should fail diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_private.nr index 4fd1d663504..114e124fefb 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_private.nr @@ -10,7 +10,7 @@ unconstrained fn transfer_in_private() { let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); // Transfer the NFT to the recipient - env.call_private_void(NFT::at(nft_contract_address).transfer_in_private(sender, recipient, token_id, 0)); + NFT::at(nft_contract_address).transfer_in_private(sender, recipient, token_id, 0).call(&mut env.private()); // Recipient should have the note in their private nfts utils::assert_owns_private_nft(nft_contract_address, recipient, token_id); @@ -22,7 +22,7 @@ unconstrained fn transfer_in_private_to_self() { let (env, nft_contract_address, owner, _, token_id) = utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); // Transfer the NFT back to the owner - env.call_private_void(NFT::at(nft_contract_address).transfer_in_private(owner, owner, token_id, 0)); + NFT::at(nft_contract_address).transfer_in_private(owner, owner, token_id, 0).call(&mut env.private()); // NFT owner should stay the same utils::assert_owns_private_nft(nft_contract_address, owner, token_id); @@ -35,7 +35,7 @@ unconstrained fn transfer_in_private_to_non_deployed_account() { let not_deployed = cheatcodes::create_account(); // Transfer the NFT to the recipient - env.call_private_void(NFT::at(nft_contract_address).transfer_in_private(sender, not_deployed.address, token_id, 0)); + NFT::at(nft_contract_address).transfer_in_private(sender, not_deployed.address, token_id, 0).call(&mut env.private()); // Owner of the private NFT should be the not_deployed account utils::assert_owns_private_nft(nft_contract_address, not_deployed.address, token_id); @@ -53,7 +53,7 @@ unconstrained fn transfer_in_private_on_behalf_of_other() { // Impersonate recipient to perform the call env.impersonate(recipient); // Transfer the NFT to the recipient - env.call_private_void(transfer_in_private_call_interface); + transfer_in_private_call_interface.call(&mut env.private()); // Recipient should be the private NFT owner utils::assert_owns_private_nft(nft_contract_address, recipient, token_id); @@ -65,7 +65,7 @@ unconstrained fn transfer_in_private_failure_not_an_owner() { let (env, nft_contract_address, owner, not_owner, token_id) = utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); // Try transferring the NFT from not_owner env.impersonate(not_owner); - env.call_private_void(NFT::at(nft_contract_address).transfer_in_private(not_owner, owner, token_id, 0)); + NFT::at(nft_contract_address).transfer_in_private(not_owner, owner, token_id, 0).call(&mut env.private()); } #[test(should_fail_with="invalid nonce")] @@ -79,7 +79,7 @@ unconstrained fn transfer_in_private_failure_on_behalf_of_self_non_zero_nonce() let token_id = random(); // Try transferring the NFT - env.call_private_void(NFT::at(nft_contract_address).transfer_in_private(sender, recipient, token_id, 1)); + NFT::at(nft_contract_address).transfer_in_private(sender, recipient, token_id, 1).call(&mut env.private()); } #[test(should_fail_with="Authorization not found for message hash")] @@ -95,7 +95,7 @@ unconstrained fn transfer_in_private_failure_on_behalf_of_other_without_approval // Impersonate recipient to perform the call env.impersonate(recipient); // Try transferring the NFT - env.call_private_void(NFT::at(nft_contract_address).transfer_in_private(sender, recipient, token_id, 1)); + NFT::at(nft_contract_address).transfer_in_private(sender, recipient, token_id, 1).call(&mut env.private()); } #[test(should_fail_with="Authorization not found for message hash")] @@ -113,5 +113,5 @@ unconstrained fn transfer_in_private_failure_on_behalf_of_other_wrong_caller() { // Impersonate recipient to perform the call env.impersonate(recipient); // Try transferring the NFT - env.call_private_void(transfer_in_private_from_call_interface); + transfer_in_private_from_call_interface.call(&mut env.private()); } diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr index d08ccb821ac..88c3293487b 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr @@ -9,7 +9,7 @@ unconstrained fn transfer_in_public() { let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); // Transfer the NFT - env.call_public(NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 0)); + NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 0).call(&mut env.public()); utils::assert_owns_public_nft(env, nft_contract_address, recipient, token_id); } @@ -20,7 +20,7 @@ unconstrained fn transfer_in_public_to_self() { let (env, nft_contract_address, user, _, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); // Transfer the NFT - env.call_public(NFT::at(nft_contract_address).transfer_in_public(user, user, token_id, 0)); + NFT::at(nft_contract_address).transfer_in_public(user, user, token_id, 0).call(&mut env.public()); // Check the user stayed the public owner utils::assert_owns_public_nft(env, nft_contract_address, user, token_id); @@ -36,13 +36,14 @@ unconstrained fn transfer_in_public_on_behalf_of_other() { // Impersonate recipient to perform the call env.impersonate(recipient); // Transfer the NFT - env.call_public(transfer_in_public_from_call_interface); + transfer_in_public_from_call_interface.call(&mut env.public()); // Check the is recipient is the new public owner utils::assert_owns_public_nft(env, nft_contract_address, recipient, token_id); } -#[test(should_fail_with="invalid nonce")] +//#[test(should_fail_with="invalid nonce")] +#[test(should_fail_with="Nested call failed!")] unconstrained fn transfer_in_public_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough. // The authwit check is in the beginning so we don't need to waste time on minting the NFT and transferring @@ -53,21 +54,21 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_self_non_zero_nonce() { let token_id = random(); // Try to transfer the NFT - env.call_public(NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, random())); + NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, random()).call(&mut env.public()); } -#[test(should_fail_with="invalid owner")] +//#[test(should_fail_with="invalid owner")] +#[test(should_fail_with="Nested call failed!")] unconstrained fn transfer_in_public_non_existent_nft() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, sender, recipient) = utils::setup(/* with_account_contracts */ false); // Try to transfer the NFT let token_id = 612; - env.call_public(NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 0)); + NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 0).call(&mut env.public()); } -// Not checking error message here with should_fail_with because noir panics when I try doing that. -#[test] +#[test(should_fail_with="Nested call failed!")] unconstrained fn transfer_in_public_failure_on_behalf_of_other_without_approval() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -75,13 +76,10 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_other_without_approval( // Impersonate recipient to perform the call env.impersonate(recipient); // Try to transfer tokens - env.assert_public_call_fails(NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 1)); - // Check the sender stayed the public owner - utils::assert_owns_public_nft(env, nft_contract_address, sender, token_id); + NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 1).call(&mut env.public()); } -// Not checking error message here with should_fail_with because noir panics when I try doing that. -#[test] +#[test(should_fail_with="Nested call failed!")] unconstrained fn transfer_in_public_failure_on_behalf_of_other_wrong_caller() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -90,7 +88,5 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_other_wrong_caller() { // Impersonate recipient to perform the call env.impersonate(recipient); // Try to transfer tokens - env.assert_public_call_fails(transfer_in_public_from_call_interface); - // Check the sender stayed the public owner - utils::assert_owns_public_nft(env, nft_contract_address, sender, token_id); + transfer_in_public_from_call_interface.call(&mut env.public()); } diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index 49bde9031d5..f1b780f3027 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -37,14 +37,10 @@ unconstrained fn transfer_to_private_to_a_different_account() { let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We prepare the transfer - env.call_private_void( - NFT::at(nft_contract_address).prepare_transfer_to_private(sender, recipient, transient_storage_slot_randomness) - ); + NFT::at(nft_contract_address).prepare_transfer_to_private(sender, recipient, transient_storage_slot_randomness).call(&mut env.private()); // Finalize the transfer of the NFT - env.call_public( - NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment) - ); + NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment).call(&mut env.public()); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` // is not called and we don't have a `NoteProcessor` in TXE. @@ -64,7 +60,8 @@ unconstrained fn transfer_to_private_to_a_different_account() { utils::assert_owns_public_nft(env, nft_contract_address, AztecAddress::zero(), token_id); } -#[test(should_fail_with="transfer not prepared")] +//#[test(should_fail_with="transfer not prepared")] +#[test(should_fail_with="Nested call failed!")] unconstrained fn transfer_to_private_to_self_transfer_not_prepared() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, _, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); @@ -73,12 +70,12 @@ unconstrained fn transfer_to_private_to_self_transfer_not_prepared() { let transfer_preparer_storage_slot_commitment = random(); // Try finalizing the transfer without preparing it - env.call_public( - NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment) - ); + + NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment).call(&mut env.public()) } -#[test(should_fail_with="transfer not prepared")] +//#[test(should_fail_with="transfer not prepared")] +#[test(should_fail_with="Nested call failed!")] unconstrained fn transfer_to_private_finalizing_from_incorrect_sender() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, incorrect_sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); @@ -93,20 +90,18 @@ unconstrained fn transfer_to_private_finalizing_from_incorrect_sender() { ); // We prepare the transfer - env.call_private_void( - NFT::at(nft_contract_address).prepare_transfer_to_private(correct_sender, recipient, transient_storage_slot_randomness) - ); + NFT::at(nft_contract_address).prepare_transfer_to_private(correct_sender, recipient, transient_storage_slot_randomness).call(&mut env.private()); // We impersonate incorrect sender and try to finalize the transfer of the NFT. The incorrect sender owns the NFT // but tries to consume a prepared transfer not belonging to him. For this reason the test should fail with // "transfer not prepared". env.impersonate(incorrect_sender); - env.call_public( - NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment) - ); + + NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment).call(&mut env.public()); } -#[test(should_fail_with="invalid NFT owner")] +//#[test(should_fail_with="invalid NFT owner")] +#[test(should_fail_with="Nested call failed!")] unconstrained fn transfer_to_private_failure_not_an_owner() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, not_owner, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); @@ -116,7 +111,6 @@ unconstrained fn transfer_to_private_failure_not_an_owner() { // Try transferring someone else's public NFT env.impersonate(not_owner); - env.call_public( - NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment) - ); + + NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment).call(&mut env.public()); } diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_public.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_public.nr index bfc027e06d1..f3ee1790d22 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_public.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_public.nr @@ -8,7 +8,7 @@ unconstrained fn transfer_to_public() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); - env.call_private_void(NFT::at(nft_contract_address).transfer_to_public(sender, recipient, token_id, 0)); + NFT::at(nft_contract_address).transfer_to_public(sender, recipient, token_id, 0).call(&mut env.private()); // Recipient should be the public owner utils::assert_owns_public_nft(env, nft_contract_address, recipient, token_id); @@ -19,7 +19,7 @@ unconstrained fn transfer_to_public_to_self() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, user, _, token_id) = utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); - env.call_private_void(NFT::at(nft_contract_address).transfer_to_public(user, user, token_id, 0)); + NFT::at(nft_contract_address).transfer_to_public(user, user, token_id, 0).call(&mut env.private()); // Check the user stayed the public owner utils::assert_owns_public_nft(env, nft_contract_address, user, token_id); @@ -34,7 +34,7 @@ unconstrained fn transfer_to_public_on_behalf_of_other() { // Impersonate recipient env.impersonate(recipient); // transfer_to_public the NFT - env.call_private_void(transfer_to_public_call_interface); + transfer_to_public_call_interface.call(&mut env.private()); // Recipient should be the public owner utils::assert_owns_public_nft(env, nft_contract_address, recipient, token_id); @@ -46,7 +46,7 @@ unconstrained fn transfer_to_public_failure_not_an_owner() { let (env, nft_contract_address, _, not_owner, token_id) = utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); env.impersonate(not_owner); - env.call_private_void(NFT::at(nft_contract_address).transfer_to_public(not_owner, not_owner, token_id, 0)); + NFT::at(nft_contract_address).transfer_to_public(not_owner, not_owner, token_id, 0).call(&mut env.private()); } #[test(should_fail_with="invalid nonce")] @@ -54,7 +54,7 @@ unconstrained fn transfer_to_public_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, user, _, token_id) = utils::setup_mint_and_transfer_to_private(/* with_account_contracts */ false); - env.call_private_void(NFT::at(nft_contract_address).transfer_to_public(user, user, token_id, random())); + NFT::at(nft_contract_address).transfer_to_public(user, user, token_id, random()).call(&mut env.private()); } #[test(should_fail_with="Authorization not found for message hash")] @@ -66,7 +66,7 @@ unconstrained fn transfer_to_public_failure_on_behalf_of_other_invalid_designate // Impersonate recipient env.impersonate(recipient); // transfer_to_public the NFT - env.call_private_void(transfer_to_public_call_interface); + transfer_to_public_call_interface.call(&mut env.private()); } #[test(should_fail_with="Authorization not found for message hash")] @@ -76,5 +76,5 @@ unconstrained fn transfer_to_public_failure_on_behalf_of_other_no_approval() { // Impersonate recipient env.impersonate(recipient); // transfer_to_public the NFT - env.call_private_void(NFT::at(nft_contract_address).transfer_to_public(sender, recipient, token_id, 0)); + NFT::at(nft_contract_address).transfer_to_public(sender, recipient, token_id, 0).call(&mut env.private()); } diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr index c1cfc8ed520..2d2c6610994 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/utils.nr @@ -29,7 +29,7 @@ unconstrained pub fn setup(with_account_contracts: bool) -> (&mut TestEnvironmen "TestNFT000000000000000000000000", "TN00000000000000000000000000000" ); - let nft_contract = env.deploy_self("NFT").with_public_initializer(initializer_call_interface); + let nft_contract = env.deploy_self("NFT").with_public_void_initializer(initializer_call_interface); let nft_contract_address = nft_contract.to_address(); env.advance_block_by(1); (&mut env, nft_contract_address, owner, recipient) @@ -40,8 +40,7 @@ unconstrained pub fn setup_and_mint(with_account_contracts: bool) -> (&mut TestE let (env, nft_contract_address, owner, recipient) = setup(with_account_contracts); let minted_token_id = 615; - let mint_public_call_interface = NFT::at(nft_contract_address).mint(owner, minted_token_id); - env.call_public(mint_public_call_interface); + NFT::at(nft_contract_address).mint(owner, minted_token_id).call(&mut env.public()); (env, nft_contract_address, owner, recipient, minted_token_id) } @@ -60,12 +59,10 @@ unconstrained pub fn setup_mint_and_transfer_to_private(with_account_contracts: let _ = OracleMock::mock("getRandomField").returns(note_randomness); // We prepare the transfer with user being both the sender and the recipient (classical "shield" flow) - let prepare_transfer_to_private_call_interface = NFT::at(nft_contract_address).prepare_transfer_to_private(owner, owner, transient_storage_slot_randomness); - env.call_private_void(prepare_transfer_to_private_call_interface); + NFT::at(nft_contract_address).prepare_transfer_to_private(owner, owner, transient_storage_slot_randomness).call(&mut env.private()); // Finalize the transfer of the NFT - let finalize_transfer_to_private_call_interface = NFT::at(nft_contract_address).finalize_transfer_to_private(minted_token_id, transfer_preparer_storage_slot_commitment); - env.call_public(finalize_transfer_to_private_call_interface); + NFT::at(nft_contract_address).finalize_transfer_to_private(minted_token_id, transfer_preparer_storage_slot_commitment).call(&mut env.public()); // TODO(#8771): We need to manually add the note because in the partial notes flow `notify_created_note_oracle` // is not called and we don't have a `NoteProcessor` in TXE. @@ -105,8 +102,7 @@ unconstrained pub fn assert_owns_public_nft( owner: AztecAddress, token_id: Field ) { - let owner_of_interface = NFT::at(nft_contract_address).owner_of(token_id); - let obtained_owner = env.call_public(owner_of_interface); + let obtained_owner = NFT::at(nft_contract_address).owner_of(token_id).view(&mut env.public()); assert(owner == obtained_owner, "Incorrect NFT owner"); } diff --git a/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr b/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr index fdf1800d74e..e503add82b5 100644 --- a/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/parent_contract/src/main.nr @@ -246,13 +246,16 @@ contract Parent { // Setup env, generate keys let mut env = TestEnvironment::new(); let owner = env.create_account(); + // Deploy parent contract + let parent_contract = env.deploy_self("Parent").without_initializer(); + let parent_contract_address = parent_contract.to_address(); // Deploy child contract let child_contract = env.deploy("./@child_contract", "Child").without_initializer(); let child_contract_address = child_contract.to_address(); cheatcodes::advance_blocks_by(1); // Set value in child through parent let value_to_set = 7; - let result = Parent::interface().private_call( + let result = Parent::at(parent_contract_address).private_call( child_contract_address, comptime { FunctionSelector::from_signature("private_set_value(Field,(Field))") @@ -270,7 +273,7 @@ contract Parent { assert(note_value == value_to_set); assert(note_value == result); // Get value from child through parent - let read_result = Parent::interface().private_call( + let read_result = Parent::at(parent_contract_address).private_call( child_contract_address, comptime { FunctionSelector::from_signature("private_get_value(Field,(Field))") diff --git a/noir-projects/noir-contracts/contracts/router_contract/src/test.nr b/noir-projects/noir-contracts/contracts/router_contract/src/test.nr index 91bc74c543d..8422d93a25f 100644 --- a/noir-projects/noir-contracts/contracts/router_contract/src/test.nr +++ b/noir-projects/noir-contracts/contracts/router_contract/src/test.nr @@ -17,11 +17,24 @@ unconstrained fn test_check_block_number() { assert(current_block_number == 10, "Expected block number to be 10"); // We test just one success case and 1 failure case in this test as the rest is tested in the comparator unit tests - let call_1 = router.check_block_number(Comparator.LT, 11); - env.call_private_void(call_1); + router.check_block_number(Comparator.LT, 11).call(&mut env.private()); +} + +#[test(should_fail_with="Timestamp mismatch.")] +unconstrained fn test_fail_check_block_number() { + let mut env = TestEnvironment::new(); + + let router_contract = env.deploy_self("Router").without_initializer(); + let router_contract_address = router_contract.to_address(); + let router = Router::at(router_contract_address); + + env.advance_block_by(9); + + // First we sanity-check that current block number is as expected + let current_block_number = env.block_number(); + assert(current_block_number == 10, "Expected block number to be 10"); - let call_2 = router.check_block_number(Comparator.LT, 5); - env.assert_private_call_fails(call_2); + router.check_block_number(Comparator.LT, 5).call(&mut env.private()); } // TODO(#8372): Add test for check_timestamp --> setting timestamp currently not supported by TXE \ No newline at end of file diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr index af8f7d3a83b..0be5ef870e8 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr @@ -113,7 +113,7 @@ unconstrained fn mint_private_failure_double_spend() { } //#[test(should_fail_with="caller is not minter")] -#[test(should_fail)] +#[test(should_fail_with="Nested call failed!")] unconstrained fn mint_private_failure_non_minter() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, _, recipient) = utils::setup(/* with_account_contracts */ false); @@ -127,7 +127,7 @@ unconstrained fn mint_private_failure_non_minter() { } //#[test(should_fail_with="call to assert_max_bit_size")] -#[test(should_fail)] +#[test(should_fail_with="Nested call failed!")] unconstrained fn mint_private_failure_overflow() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, _, _) = utils::setup(/* with_account_contracts */ false); @@ -140,7 +140,7 @@ unconstrained fn mint_private_failure_overflow() { } //#[test(should_fail_with="attempt to add with overflow")] -#[test(should_fail)] +#[test(should_fail_with="Nested call failed!")] unconstrained fn mint_private_failure_overflow_recipient() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false); @@ -172,7 +172,7 @@ unconstrained fn mint_private_failure_overflow_recipient() { } //#[test(should_fail_with="attempt to add with overflow")] -#[test(should_fail)] +#[test(should_fail_with="Nested call failed!")] unconstrained fn mint_private_failure_overflow_total_supply() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient) = utils::setup(/* with_account_contracts */ false); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr index f298412a24a..822d3158242 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/reading_constants.nr @@ -23,5 +23,5 @@ unconstrained fn check_decimals_public() { // Check decimals let result = Token::at(token_contract_address).public_get_decimals().view(&mut env.public()); - assert(result == 18 as u8); + assert(result == 18); } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr index e11b8a083e7..f680f3b992a 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr @@ -46,8 +46,7 @@ unconstrained fn public_transfer_on_behalf_of_other() { utils::check_public_balance(token_contract_address, recipient, transfer_amount); } -//#[test] -#[test(should_fail)] +#[test(should_fail_with="Nested call failed!")] unconstrained fn public_transfer_failure_more_than_balance() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); @@ -59,7 +58,7 @@ unconstrained fn public_transfer_failure_more_than_balance() { } //#[test(should_fail_with="invalid nonce")] -#[test(should_fail)] +#[test(should_fail_with="Nested call failed!")] unconstrained fn public_transfer_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); @@ -71,7 +70,7 @@ unconstrained fn public_transfer_failure_on_behalf_of_self_non_zero_nonce() { public_transfer_call_interface.call(&mut env.public()); } -#[test(should_fail)] +#[test(should_fail_with="Nested call failed!")] unconstrained fn public_transfer_failure_on_behalf_of_other_without_approval() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -84,7 +83,7 @@ unconstrained fn public_transfer_failure_on_behalf_of_other_without_approval() { } //#[test(should_fail_with="attempt to subtract with underflow")] -#[test(should_fail)] +#[test(should_fail_with="Nested call failed!")] unconstrained fn public_transfer_failure_on_behalf_of_other_more_than_balance() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -99,7 +98,7 @@ unconstrained fn public_transfer_failure_on_behalf_of_other_more_than_balance() public_transfer_from_call_interface.call(&mut env.public()); } -#[test(should_fail)] +#[test(should_fail_with="Nested call failed!")] unconstrained fn public_transfer_failure_on_behalf_of_other_wrong_caller() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index db879b3f3cd..d988e38208e 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -231,6 +231,32 @@ export class TXEService { return toForeignCallResult([]); } + async assertPrivateCallFails( + targetContractAddress: ForeignCallSingle, + functionSelector: ForeignCallSingle, + argsHash: ForeignCallSingle, + sideEffectCounter: ForeignCallSingle, + isStaticCall: ForeignCallSingle, + isDelegateCall: ForeignCallSingle, + ) { + try { + await this.typedOracle.callPrivateFunction( + fromSingle(targetContractAddress), + FunctionSelector.fromField(fromSingle(functionSelector)), + fromSingle(argsHash), + fromSingle(sideEffectCounter).toNumber(), + fromSingle(isStaticCall).toBool(), + fromSingle(isDelegateCall).toBool(), + ); + throw new ExpectedFailureError('Private call did not fail'); + } catch (e) { + if (e instanceof ExpectedFailureError) { + throw e; + } + } + return toForeignCallResult([]); + } + // PXE oracles getRandomField() { From f0e9846e8a053c94c2d2c2586db3f6b212f957cb Mon Sep 17 00:00:00 2001 From: thunkar Date: Tue, 15 Oct 2024 11:05:12 +0000 Subject: [PATCH 04/14] fixes --- .../aztec-nr/authwit/src/cheatcodes.nr | 12 +-- .../aztec/src/context/call_interfaces.nr | 23 +++--- .../src/test/helpers/test_environment.nr | 16 ++-- .../aztec-nr/aztec/src/test/helpers/utils.nr | 8 +- .../contracts/router_contract/src/test.nr | 2 +- yarn-project/txe/src/oracle/txe_oracle.ts | 36 ++++++++- .../txe/src/txe_service/txe_service.ts | 5 -- yarn-project/txe/src/util/simulation_error.ts | 79 +++++++++++++++++++ yarn-project/yarn.lock | 1 + 9 files changed, 145 insertions(+), 37 deletions(-) create mode 100644 yarn-project/txe/src/util/simulation_error.ts diff --git a/noir-projects/aztec-nr/authwit/src/cheatcodes.nr b/noir-projects/aztec-nr/authwit/src/cheatcodes.nr index bc2dcb19e55..6cc8649d425 100644 --- a/noir-projects/aztec-nr/authwit/src/cheatcodes.nr +++ b/noir-projects/aztec-nr/authwit/src/cheatcodes.nr @@ -6,11 +6,7 @@ use dep::aztec::{ use crate::auth::{compute_inner_authwit_hash, compute_authwit_message_hash, set_authorized}; -pub fn add_private_authwit_from_call_interface( - on_behalf_of: AztecAddress, - caller: AztecAddress, - call_interface: C -) where C: CallInterface { +pub fn add_private_authwit_from_call_interface(on_behalf_of: AztecAddress, caller: AztecAddress, call_interface: C) where C: CallInterface { let target = call_interface.get_contract_address(); let inputs = cheatcodes::get_private_context_inputs(get_block_number()); let chain_id = inputs.tx_context.chain_id; @@ -22,7 +18,11 @@ pub fn add_private_authwit_from_call_interface( cheatcodes::add_authwit(on_behalf_of, message_hash); } -pub fn add_public_authwit_from_call_interface(on_behalf_of: AztecAddress, caller: AztecAddress, call_interface: C) where C: CallInterface { +pub fn add_public_authwit_from_call_interface( + on_behalf_of: AztecAddress, + caller: AztecAddress, + call_interface: C +) where C: CallInterface { let current_contract = get_contract_address(); cheatcodes::set_contract_address(on_behalf_of); let target = call_interface.get_contract_address(); diff --git a/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr b/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr index d5748b4bb4f..7dfdf721331 100644 --- a/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr +++ b/noir-projects/aztec-nr/aztec/src/context/call_interfaces.nr @@ -1,7 +1,4 @@ -use dep::protocol_types::{ - abis::{function_selector::FunctionSelector, private_circuit_public_inputs::PrivateCircuitPublicInputs}, - address::AztecAddress, traits::Deserialize -}; +use dep::protocol_types::{abis::{function_selector::FunctionSelector}, address::AztecAddress, traits::Deserialize}; use crate::context::{ private_context::PrivateContext, public_context::PublicContext, gas::GasOpts, @@ -11,7 +8,7 @@ use crate::context::{ use crate::oracle::arguments::pack_arguments; use crate::hash::hash_args; -pub trait CallInterface { +pub trait CallInterface { fn get_args(self) -> [Field] { self.args } @@ -33,8 +30,6 @@ pub trait CallInterface { } } -impl CallInterface for PrivateCallInterface {} - pub struct PrivateCallInterface { target_contract: AztecAddress, selector: FunctionSelector, @@ -72,7 +67,7 @@ impl PrivateCallInterface { } } -impl CallInterface for PrivateVoidCallInterface {} +impl CallInterface for PrivateVoidCallInterface {} pub struct PrivateVoidCallInterface { target_contract: AztecAddress, @@ -107,7 +102,7 @@ impl PrivateVoidCallInterface { } } -impl CallInterface for PrivateStaticCallInterface {} +impl CallInterface for PrivateStaticCallInterface {} pub struct PrivateStaticCallInterface { target_contract: AztecAddress, @@ -127,7 +122,7 @@ impl PrivateStaticCallInterface { } } -impl CallInterface for PrivateStaticVoidCallInterface {} +impl CallInterface for PrivateStaticVoidCallInterface {} pub struct PrivateStaticVoidCallInterface { target_contract: AztecAddress, @@ -146,7 +141,7 @@ impl PrivateStaticVoidCallInterface { } } -impl CallInterface for PublicCallInterface {} +impl CallInterface for PublicCallInterface {} pub struct PublicCallInterface { target_contract: AztecAddress, @@ -216,7 +211,7 @@ impl PublicCallInterface { } } -impl CallInterface for PublicVoidCallInterface {} +impl CallInterface for PublicVoidCallInterface {} pub struct PublicVoidCallInterface { target_contract: AztecAddress, @@ -286,7 +281,7 @@ impl PublicVoidCallInterface { } } -impl CallInterface for PublicStaticCallInterface {} +impl CallInterface for PublicStaticCallInterface {} pub struct PublicStaticCallInterface { target_contract: AztecAddress, @@ -323,7 +318,7 @@ impl PublicStaticCallInterface { } } -impl CallInterface for PublicStaticVoidCallInterface {} +impl CallInterface for PublicStaticVoidCallInterface {} pub struct PublicStaticVoidCallInterface { target_contract: AztecAddress, diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr index b2c6d4f2c4e..cdc8b33dcde 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/test_environment.nr @@ -44,8 +44,14 @@ impl TestEnvironment { PublicContext::new(|| 0) } + unconstrained fn public_with_args_hash(_self: Self, args: [Field]) -> PublicContext { + let mut context = PublicContext::new(|| {panic(f"Provide args hash manually")}); + context.args_hash = Option::some(hash_args(args)); + context + } + unconstrained fn private(&mut self) -> PrivateContext { - self.private_at(get_block_number()) + self.private_at(get_block_number() - 1) } // unconstrained is a key word, so we mis-spell purposefully here, like we do with contrakt @@ -94,20 +100,20 @@ impl TestEnvironment { Deployer { path: "", name, public_keys_hash: 0 } } - unconstrained fn assert_public_call_fails( + unconstrained fn assert_public_call_fails( _self: Self, call_interface: C - ) where C: CallInterface { + ) where C: CallInterface { // Public functions are routed through the dispatch function. let fn_selector = FunctionSelector::from_field(PUBLIC_DISPATCH_SELECTOR); let calldata = &[call_interface.get_selector().to_field()].append(call_interface.get_args()); cheatcodes::assert_public_call_fails(call_interface.get_contract_address(), fn_selector, calldata); } - unconstrained fn assert_private_call_fails( + unconstrained fn assert_private_call_fails( _self: Self, call_interface: C - ) where C: CallInterface { + ) where C: CallInterface { cheatcodes::assert_private_call_fails( call_interface.get_contract_address(), call_interface.get_selector(), diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr index b2001e98ea0..6434394cd0c 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr @@ -23,10 +23,10 @@ pub struct Deployer { } impl Deployer { - unconstrained pub fn with_private_initializer( + unconstrained pub fn with_private_initializer( self, call_interface: C - ) -> ContractInstance where C: CallInterface { + ) -> ContractInstance where C: CallInterface

{ let instance = cheatcodes::deploy( self.path, self.name, @@ -54,7 +54,7 @@ impl Deployer { unconstrained pub fn with_public_void_initializer( self, call_interface: C - ) -> ContractInstance where C: CallInterface { + ) -> ContractInstance where C: CallInterface

{ let instance = cheatcodes::deploy( self.path, self.name, @@ -79,7 +79,7 @@ impl Deployer { unconstrained pub fn with_public_initializer( self, call_interface: C - ) -> ContractInstance where C: CallInterface, T: Deserialize<_> { + ) -> ContractInstance where C: CallInterface

, T: Deserialize<_> { let instance = cheatcodes::deploy( self.path, self.name, diff --git a/noir-projects/noir-contracts/contracts/router_contract/src/test.nr b/noir-projects/noir-contracts/contracts/router_contract/src/test.nr index 8422d93a25f..f34024e0dfe 100644 --- a/noir-projects/noir-contracts/contracts/router_contract/src/test.nr +++ b/noir-projects/noir-contracts/contracts/router_contract/src/test.nr @@ -20,7 +20,7 @@ unconstrained fn test_check_block_number() { router.check_block_number(Comparator.LT, 11).call(&mut env.private()); } -#[test(should_fail_with="Timestamp mismatch.")] +#[test(should_fail_with="Block number mismatch.")] unconstrained fn test_fail_check_block_number() { let mut env = TestEnvironment::new(); diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 90de4368006..980a167d29e 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -7,6 +7,7 @@ import { PublicDataWitness, PublicDataWrite, PublicExecutionRequest, + SimulationError, type UnencryptedL2Log, } from '@aztec/circuit-types'; import { type CircuitWitnessGenerationStats } from '@aztec/circuit-types/stats'; @@ -67,6 +68,7 @@ import { import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; import { MerkleTreeSnapshotOperationsFacade, type MerkleTrees } from '@aztec/world-state'; +import { enrichPublicSimulationError } from '../util/simulation_error.js'; import { type TXEDatabase } from '../util/txe_database.js'; import { TXEPublicContractDataSource } from '../util/txe_public_contract_data_source.js'; import { TXEWorldStateDB } from '../util/txe_world_state_db.js'; @@ -184,6 +186,8 @@ export class TXE implements TypedOracle { const stateReference = await db.getStateReference(); const inputs = PrivateContextInputs.empty(); + inputs.txContext.chainId = this.chainId; + inputs.txContext.version = this.version; inputs.historicalHeader.globalVariables.blockNumber = new Fr(blockNumber); inputs.historicalHeader.state = stateReference; inputs.historicalHeader.lastArchive.root = Fr.fromBuffer( @@ -642,9 +646,27 @@ export class TXE implements TypedOracle { ); const execution = new PublicExecutionRequest(targetContractAddress, callContext, args); + const db = await this.trees.getLatest(); + const previousBlockState = await this.#getTreesAt(this.blockNumber - 1); + + const combinedConstantData = CombinedConstantData.empty(); + combinedConstantData.globalVariables.chainId = this.chainId; + combinedConstantData.globalVariables.version = this.version; + combinedConstantData.globalVariables.blockNumber = new Fr(this.blockNumber); + combinedConstantData.historicalHeader.globalVariables.chainId = this.chainId; + combinedConstantData.historicalHeader.globalVariables.version = this.version; + combinedConstantData.historicalHeader.globalVariables.blockNumber = new Fr(this.blockNumber - 1); + combinedConstantData.historicalHeader.state = await db.getStateReference(); + combinedConstantData.historicalHeader.lastArchive.root = Fr.fromBuffer( + (await previousBlockState.getTreeInfo(MerkleTreeId.ARCHIVE)).root, + ); + + combinedConstantData.txContext.chainId = this.chainId; + combinedConstantData.txContext.version = this.version; + const executionResult = executor.simulate( execution, - CombinedConstantData.empty(), + combinedConstantData, Gas.test(), TxContext.empty(), /* pendingNullifiers */ [], @@ -736,7 +758,17 @@ export class TXE implements TypedOracle { ); if (executionResult.reverted) { - throw new Error(`Execution reverted with reason: ${executionResult.revertReason}`); + if (executionResult.revertReason && executionResult.revertReason instanceof SimulationError) { + await enrichPublicSimulationError( + executionResult.revertReason, + this.txeDatabase, + this.contractDataOracle, + this.logger, + ); + throw new Error(executionResult.revertReason.message); + } else { + throw new Error(`Public function call reverted: ${executionResult.revertReason}`); + } } // Apply side effects diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index d988e38208e..34835081ae6 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -273,11 +273,6 @@ export class TXEService { return toForeignCallResult([toSingle(new Fr(blockNumber))]); } - setIsStaticCall(isStaticCall: ForeignCallSingle) { - (this.typedOracle as TXE).setIsStaticCall(fromSingle(isStaticCall).toBool()); - return toForeignCallResult([]); - } - async packArgumentsArray(args: ForeignCallArray) { const packed = await this.typedOracle.packArgumentsArray(fromArray(args)); return toForeignCallResult([toSingle(packed)]); diff --git a/yarn-project/txe/src/util/simulation_error.ts b/yarn-project/txe/src/util/simulation_error.ts new file mode 100644 index 00000000000..21cf441ad66 --- /dev/null +++ b/yarn-project/txe/src/util/simulation_error.ts @@ -0,0 +1,79 @@ +import { SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; +import { AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; +import { Logger } from '@aztec/foundation/log'; +import { ContractDataOracle } from '@aztec/pxe'; +import { resolveAssertionMessage, resolveOpcodeLocations } from '@aztec/simulator'; + +import { TXEDatabase } from './txe_database.js'; + +export async function enrichPublicSimulationError( + err: SimulationError, + db: TXEDatabase, + contractDataOracle: ContractDataOracle, + logger: Logger, +) { + // Try to fill in the noir call stack since the PXE may have access to the debug metadata + const callStack = err.getCallStack(); + const originalFailingFunction = callStack[callStack.length - 1]; + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Properly fix this. + // To be able to resolve the assertion message, we need to use the information from the public dispatch function, + // no matter what the call stack selector points to (since we've modified it to point to the target function). + // We should remove this because the AVM (or public protocol) shouldn't be aware of the public dispatch calling convention. + const debugInfo = await contractDataOracle.getFunctionDebugMetadata( + originalFailingFunction.contractAddress, + FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)), + ); + const noirCallStack = err.getNoirCallStack(); + if (debugInfo) { + if (isNoirCallStackUnresolved(noirCallStack)) { + const assertionMessage = resolveAssertionMessage(noirCallStack, debugInfo); + if (assertionMessage) { + err.setOriginalMessage(err.getOriginalMessage() + `: ${assertionMessage}`); + } + try { + // Public functions are simulated as a single Brillig entry point. + // Thus, we can safely assume here that the Brillig function id is `0`. + const parsedCallStack = resolveOpcodeLocations(noirCallStack, debugInfo, 0); + err.setNoirCallStack(parsedCallStack); + } catch (err) { + logger.warn( + `Could not resolve noir call stack for ${originalFailingFunction.contractAddress.toString()}:${originalFailingFunction.functionSelector.toString()}: ${err}`, + ); + } + } + } + await enrichSimulationError(err, db); +} + +export async function enrichSimulationError(err: SimulationError, db: TXEDatabase) { + // Maps contract addresses to the set of functions selectors that were in error. + // Using strings because map and set don't use .equals() + const mentionedFunctions: Map> = new Map(); + + err.getCallStack().forEach(({ contractAddress, functionSelector }) => { + if (!mentionedFunctions.has(contractAddress.toString())) { + mentionedFunctions.set(contractAddress.toString(), new Set()); + } + mentionedFunctions.get(contractAddress.toString())!.add(functionSelector.toString()); + }); + + await Promise.all( + [...mentionedFunctions.entries()].map(async ([contractAddress, selectors]) => { + const parsedContractAddress = AztecAddress.fromString(contractAddress); + const contract = await db.getContract(parsedContractAddress); + if (contract) { + err.enrichWithContractName(parsedContractAddress, contract.name); + selectors.forEach(selector => { + const functionArtifact = contract.functions.find(f => FunctionSelector.fromString(selector).equals(f)); + if (functionArtifact) { + err.enrichWithFunctionName( + parsedContractAddress, + FunctionSelector.fromNameAndParameters(functionArtifact), + functionArtifact.name, + ); + } + }); + } + }), + ); +} diff --git a/yarn-project/yarn.lock b/yarn-project/yarn.lock index 8fab10e11a8..ecd52a2eb69 100644 --- a/yarn-project/yarn.lock +++ b/yarn-project/yarn.lock @@ -3393,6 +3393,7 @@ __metadata: "@noir-lang/acvm_js": 0.51.0 "@noir-lang/noirc_abi": 0.35.0 "@noir-lang/types": 0.35.0 + checksum: 785c9cabed8bc7d13231f1437cd667f07746380bed88b9e75091bb35df6913b50aae86dad054e7a1bf83fdfc5689b3c9b7aed1a44172de5056a81b24527caf41 languageName: node linkType: hard From e581fb8e7547b9736ac4419f9afe6e20e0e21e4b Mon Sep 17 00:00:00 2001 From: thunkar Date: Tue, 15 Oct 2024 14:56:45 +0000 Subject: [PATCH 05/14] fmt and fixes --- .../testing_contracts/testing.md | 2 +- docs/docs/migration_notes.md | 11 ++ .../aztec/src/note/note_getter/test.nr | 5 +- .../src/state_vars/private_mutable/test.nr | 5 +- .../aztec/src/test/helpers/cheatcodes.nr | 26 ++-- .../aztec-nr/aztec/src/test/helpers/utils.nr | 8 +- .../src/test/transfer_in_public.nr | 10 +- .../src/test/transfer_to_private.nr | 9 +- .../contracts/router_contract/src/test.nr | 2 +- .../contracts/token_contract/src/test/burn.nr | 47 ++---- .../token_contract/src/test/minting.nr | 12 +- .../token_contract/src/test/refunds.nr | 2 +- .../src/test/transfer_public.nr | 12 +- yarn-project/txe/src/oracle/txe_oracle.ts | 141 ++++++++++++------ .../txe/src/txe_service/txe_service.ts | 118 ++++++++++++++- yarn-project/txe/src/util/simulation_error.ts | 8 +- 16 files changed, 282 insertions(+), 136 deletions(-) diff --git a/docs/docs/guides/developer_guides/smart_contracts/testing_contracts/testing.md b/docs/docs/guides/developer_guides/smart_contracts/testing_contracts/testing.md index 534969e86be..827547fe098 100644 --- a/docs/docs/guides/developer_guides/smart_contracts/testing_contracts/testing.md +++ b/docs/docs/guides/developer_guides/smart_contracts/testing_contracts/testing.md @@ -107,7 +107,7 @@ let my_contract_instance = deployer.without_initializer(); ``` :::warning -It is not always necessary to deploy a contract in order to test it, but sometimes it's inevitable (when testing functions that depend on the contract being initialized, or contracts that call others for example) **It is important to keep them up to date**, as TXE cannot recompile them on changes. Think of it as regenerating the bytecode and ABI so it becomes accessible externally. +It is always necessary to deploy a contract in order to test it. **It is important to keep them up to date**, as TXE cannot recompile them on changes. Think of it as regenerating the bytecode and ABI so it becomes accessible externally. ::: ### Calling functions diff --git a/docs/docs/migration_notes.md b/docs/docs/migration_notes.md index 10d6c2e3031..d529198a116 100644 --- a/docs/docs/migration_notes.md +++ b/docs/docs/migration_notes.md @@ -6,6 +6,17 @@ keywords: [sandbox, aztec, notes, migration, updating, upgrading] Aztec is in full-speed development. Literally every version breaks compatibility with the previous ones. This page attempts to target errors and difficulties you might encounter when upgrading, and how to resolve them. +## 0.X.X +### [TXE] Single execution environment +Thanks to recent advancements in Brillig TXE performs every single call as if it was a nested call, spawning a new ACVM or AVM simulator without performance loss. +This ensures every single test runs in a consistent environment and allows for clearer test syntax: + +```diff +-let my_call_interface = MyContract::at(address).my_function(args); +-env.call_private(my_contract_interface) ++MyContract::at(address).my_function(args).call(&mut env.private()); +``` + ## 0.58.0 ### [l1-contracts] Inbox's MessageSent event emits global tree index Earlier `MessageSent` event in Inbox emitted a subtree index (index of the message in the subtree of the l2Block). But the nodes and Aztec.nr expects the index in the global L1_TO_L2_MESSAGES_TREE. So to make it easier to parse this, Inbox now emits this global index. diff --git a/noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr b/noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr index 0d8b660a142..a78c52a09e5 100644 --- a/noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr +++ b/noir-projects/aztec-nr/aztec/src/note/note_getter/test.nr @@ -13,7 +13,10 @@ use crate::utils::comparison::Comparator; global storage_slot: Field = 42; unconstrained fn setup() -> TestEnvironment { - TestEnvironment::new() + let mut env = TestEnvironment::new(); + // Advance 1 block so we can read historic state from private + env.advance_block_by(1); + env } unconstrained fn build_valid_note(value: Field) -> MockNote { diff --git a/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable/test.nr b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable/test.nr index 4277bd73730..1fd7f08ed84 100644 --- a/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable/test.nr +++ b/noir-projects/aztec-nr/aztec/src/state_vars/private_mutable/test.nr @@ -8,7 +8,10 @@ use std::test::OracleMock; global storage_slot = 17; unconstrained fn setup() -> TestEnvironment { - TestEnvironment::new() + let mut env = TestEnvironment::new(); + // Advance 1 block so we can read historic state from private + env.advance_block_by(1); + env } unconstrained fn in_private(env: &mut TestEnvironment) -> PrivateMutable { diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr index d2e21f6c65e..6bf3a0c136c 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/cheatcodes.nr @@ -5,27 +5,27 @@ use dep::protocol_types::{ use crate::context::inputs::PrivateContextInputs; use crate::test::helpers::utils::TestAccount; -unconstrained pub fn reset() { +pub unconstrained fn reset() { oracle_reset(); } -unconstrained pub fn get_side_effects_counter() -> u32 { +pub unconstrained fn get_side_effects_counter() -> u32 { oracle_get_side_effects_counter() } -unconstrained pub fn set_contract_address(address: AztecAddress) { +pub unconstrained fn set_contract_address(address: AztecAddress) { oracle_set_contract_address(address); } -unconstrained pub fn advance_blocks_by(blocks: u32) { +pub unconstrained fn advance_blocks_by(blocks: u32) { oracle_advance_blocks_by(blocks); } -unconstrained pub fn get_private_context_inputs(historical_block_number: u32) -> PrivateContextInputs { +pub unconstrained fn get_private_context_inputs(historical_block_number: u32) -> PrivateContextInputs { oracle_get_private_context_inputs(historical_block_number) } -unconstrained pub fn deploy( +pub unconstrained fn deploy( path: str, name: str, initializer: str

, @@ -36,31 +36,31 @@ unconstrained pub fn deploy( ContractInstance::deserialize(instance_fields) } -unconstrained pub fn direct_storage_write(contract_address: AztecAddress, storage_slot: Field, fields: [Field; N]) { +pub unconstrained fn direct_storage_write(contract_address: AztecAddress, storage_slot: Field, fields: [Field; N]) { let _hash = direct_storage_write_oracle(contract_address, storage_slot, fields); } -unconstrained pub fn create_account() -> TestAccount { +pub unconstrained fn create_account() -> TestAccount { oracle_create_account() } -unconstrained pub fn add_account(secret: Field) -> TestAccount { +pub unconstrained fn add_account(secret: Field) -> TestAccount { oracle_add_account(secret) } -unconstrained pub fn derive_keys(secret: Field) -> PublicKeys { +pub unconstrained fn derive_keys(secret: Field) -> PublicKeys { oracle_derive_keys(secret) } -unconstrained pub fn add_authwit(address: AztecAddress, message_hash: Field) { +pub unconstrained fn add_authwit(address: AztecAddress, message_hash: Field) { orable_add_authwit(address, message_hash) } -unconstrained pub fn assert_public_call_fails(target_address: AztecAddress, function_selector: FunctionSelector, args: [Field]) { +pub unconstrained fn assert_public_call_fails(target_address: AztecAddress, function_selector: FunctionSelector, args: [Field]) { oracle_assert_public_call_fails(target_address, function_selector, args) } -unconstrained pub fn assert_private_call_fails( +pub unconstrained fn assert_private_call_fails( target_address: AztecAddress, function_selector: FunctionSelector, argsHash: Field, diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr index 1d09a748202..d3b6189b1dd 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr @@ -22,7 +22,7 @@ pub struct Deployer { } impl Deployer { - unconstrained pub fn with_private_initializer( + pub unconstrained fn with_private_initializer( self, call_interface: C ) -> ContractInstance where C: CallInterface

{ @@ -50,7 +50,7 @@ impl Deployer { instance } - unconstrained pub fn with_public_void_initializer( + pub unconstrained fn with_public_void_initializer( self, call_interface: C ) -> ContractInstance where C: CallInterface

{ @@ -75,7 +75,7 @@ impl Deployer { instance } - unconstrained pub fn with_public_initializer( + pub unconstrained fn with_public_initializer( self, call_interface: C ) -> ContractInstance where C: CallInterface

, T: Deserialize<_> { @@ -100,7 +100,7 @@ impl Deployer { instance } - unconstrained pub fn without_initializer(self) -> ContractInstance { + pub unconstrained fn without_initializer(self) -> ContractInstance { cheatcodes::deploy(self.path, self.name, "", &[], self.public_keys_hash) } } diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr index 88c3293487b..ac8eb37d978 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_in_public.nr @@ -42,8 +42,7 @@ unconstrained fn transfer_in_public_on_behalf_of_other() { utils::assert_owns_public_nft(env, nft_contract_address, recipient, token_id); } -//#[test(should_fail_with="invalid nonce")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "invalid nonce")] unconstrained fn transfer_in_public_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough. // The authwit check is in the beginning so we don't need to waste time on minting the NFT and transferring @@ -57,8 +56,7 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_self_non_zero_nonce() { NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, random()).call(&mut env.public()); } -//#[test(should_fail_with="invalid owner")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "invalid owner")] unconstrained fn transfer_in_public_non_existent_nft() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, sender, recipient) = utils::setup(/* with_account_contracts */ false); @@ -68,7 +66,7 @@ unconstrained fn transfer_in_public_non_existent_nft() { NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 0).call(&mut env.public()); } -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "unauthorized")] unconstrained fn transfer_in_public_failure_on_behalf_of_other_without_approval() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -79,7 +77,7 @@ unconstrained fn transfer_in_public_failure_on_behalf_of_other_without_approval( NFT::at(nft_contract_address).transfer_in_public(sender, recipient, token_id, 1).call(&mut env.public()); } -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "unauthorized")] unconstrained fn transfer_in_public_failure_on_behalf_of_other_wrong_caller() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, nft_contract_address, sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ true); diff --git a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr index f1b780f3027..1e061b338e2 100644 --- a/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr +++ b/noir-projects/noir-contracts/contracts/nft_contract/src/test/transfer_to_private.nr @@ -60,8 +60,7 @@ unconstrained fn transfer_to_private_to_a_different_account() { utils::assert_owns_public_nft(env, nft_contract_address, AztecAddress::zero(), token_id); } -//#[test(should_fail_with="transfer not prepared")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "transfer not prepared")] unconstrained fn transfer_to_private_to_self_transfer_not_prepared() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, _, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); @@ -74,8 +73,7 @@ unconstrained fn transfer_to_private_to_self_transfer_not_prepared() { NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment).call(&mut env.public()) } -//#[test(should_fail_with="transfer not prepared")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "transfer not prepared")] unconstrained fn transfer_to_private_finalizing_from_incorrect_sender() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, incorrect_sender, recipient, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); @@ -100,8 +98,7 @@ unconstrained fn transfer_to_private_finalizing_from_incorrect_sender() { NFT::at(nft_contract_address).finalize_transfer_to_private(token_id, transfer_preparer_storage_slot_commitment).call(&mut env.public()); } -//#[test(should_fail_with="invalid NFT owner")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "invalid NFT owner")] unconstrained fn transfer_to_private_failure_not_an_owner() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, nft_contract_address, _, not_owner, token_id) = utils::setup_and_mint(/* with_account_contracts */ false); diff --git a/noir-projects/noir-contracts/contracts/router_contract/src/test.nr b/noir-projects/noir-contracts/contracts/router_contract/src/test.nr index f34024e0dfe..fb4c75696a2 100644 --- a/noir-projects/noir-contracts/contracts/router_contract/src/test.nr +++ b/noir-projects/noir-contracts/contracts/router_contract/src/test.nr @@ -20,7 +20,7 @@ unconstrained fn test_check_block_number() { router.check_block_number(Comparator.LT, 11).call(&mut env.private()); } -#[test(should_fail_with="Block number mismatch.")] +#[test(should_fail_with = "Block number mismatch.")] unconstrained fn test_fail_check_block_number() { let mut env = TestEnvironment::new(); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr index 663a0a9d12e..4fe45f2da9f 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/burn.nr @@ -28,29 +28,27 @@ unconstrained fn burn_public_on_behalf_of_other() { utils::check_public_balance(token_contract_address, owner, mint_amount - burn_amount); } -#[test] +#[test(should_fail_with = "attempt to subtract with underflow")] unconstrained fn burn_public_failure_more_than_balance() { let (env, token_contract_address, owner, _, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); // Burn more than balance let burn_amount = mint_amount * 10; - let burn_call_interface = Token::at(token_contract_address).burn_public(owner, burn_amount, 0); - env.assert_public_call_fails(burn_call_interface); - utils::check_public_balance(token_contract_address, owner, mint_amount); + // Try to burn + Token::at(token_contract_address).burn_public(owner, burn_amount, 0).call(&mut env.public()); } -#[test] +#[test(should_fail_with = "invalid nonce")] unconstrained fn burn_public_failure_on_behalf_of_self_non_zero_nonce() { let (env, token_contract_address, owner, _, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); // Burn on behalf of self with non-zero nonce let burn_amount = mint_amount / 10; - let burn_call_interface = Token::at(token_contract_address).burn_public(owner, burn_amount, random()); - env.assert_public_call_fails(burn_call_interface); - utils::check_public_balance(token_contract_address, owner, mint_amount); + // Try to burn + Token::at(token_contract_address).burn_public(owner, burn_amount, random()).call(&mut env.public()); } -#[test] +#[test(should_fail_with = "unauthorized")] unconstrained fn burn_public_failure_on_behalf_of_other_without_approval() { let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -59,19 +57,10 @@ unconstrained fn burn_public_failure_on_behalf_of_other_without_approval() { let burn_call_interface = Token::at(token_contract_address).burn_public(owner, burn_amount, random()); // Impersonate recipient to perform the call env.impersonate(recipient); - env.assert_public_call_fails(burn_call_interface); - utils::check_public_balance(token_contract_address, owner, mint_amount); - - // Burn on behalf of other, wrong designated caller - let burn_call_interface = Token::at(token_contract_address).burn_public(owner, burn_amount, random()); - authwit_cheatcodes::add_public_authwit_from_call_interface(owner, owner, burn_call_interface); - // Impersonate recipient to perform the call - env.impersonate(recipient); - env.assert_public_call_fails(burn_call_interface); - utils::check_public_balance(token_contract_address, owner, mint_amount); + burn_call_interface.call(&mut env.public()); } -#[test] +#[test(should_fail_with = "unauthorized")] unconstrained fn burn_public_failure_on_behalf_of_other_wrong_caller() { let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -81,8 +70,7 @@ unconstrained fn burn_public_failure_on_behalf_of_other_wrong_caller() { authwit_cheatcodes::add_public_authwit_from_call_interface(owner, owner, burn_call_interface); // Impersonate recipient to perform the call env.impersonate(recipient); - env.assert_public_call_fails(burn_call_interface); - utils::check_public_balance(token_contract_address, owner, mint_amount); + burn_call_interface.call(&mut env.public()); } #[test] @@ -117,7 +105,6 @@ unconstrained fn burn_private_failure_more_than_balance() { // Burn more than balance let burn_amount = mint_amount * 10; Token::at(token_contract_address).burn(owner, burn_amount, 0).call(&mut env.private()); - // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } #[test(should_fail_with = "invalid nonce")] @@ -127,10 +114,9 @@ unconstrained fn burn_private_failure_on_behalf_of_self_non_zero_nonce() { // Burn more than balance let burn_amount = mint_amount / 10; Token::at(token_contract_address).burn(owner, burn_amount, random()).call(&mut env.private()); - // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } -#[test(should_fail)] +#[test(should_fail_with = "Balance too low")] unconstrained fn burn_private_failure_on_behalf_of_other_more_than_balance() { let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -142,34 +128,31 @@ unconstrained fn burn_private_failure_on_behalf_of_other_more_than_balance() { // Impersonate recipient to perform the call env.impersonate(recipient); burn_call_interface.call(&mut env.private()); - // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } -#[test(should_fail)] +#[test(should_fail_with = "Authorization not found for message hash")] unconstrained fn burn_private_failure_on_behalf_of_other_without_approval() { let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); // Burn more than balance let burn_amount = mint_amount / 10; // Burn on behalf of other - let burn_call_interface = Token::at(token_contract_address).burn(owner, burn_amount, random()); + let burn_call_interface = Token::at(token_contract_address).burn(owner, burn_amount, 3); // Impersonate recipient to perform the call env.impersonate(recipient); burn_call_interface.call(&mut env.private()); - // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } -#[test(should_fail)] +#[test(should_fail_with = "Authorization not found for message hash")] unconstrained fn burn_private_failure_on_behalf_of_other_wrong_designated_caller() { let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); // Burn more than balance let burn_amount = mint_amount / 10; // Burn on behalf of other - let burn_call_interface = Token::at(token_contract_address).burn(owner, burn_amount, random()); + let burn_call_interface = Token::at(token_contract_address).burn(owner, burn_amount, 3); authwit_cheatcodes::add_private_authwit_from_call_interface(owner, owner, burn_call_interface); // Impersonate recipient to perform the call env.impersonate(recipient); burn_call_interface.call(&mut env.private()); - // Private doesnt revert, so we cannot check balances here since notes have already been nullified. Test is done. } diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr index f73e12fe621..0b5fc640787 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/minting.nr @@ -112,8 +112,7 @@ unconstrained fn mint_private_failure_double_spend() { Token::at(token_contract_address).redeem_shield(recipient, mint_amount, secret).call(&mut env.private()); } -//#[test(should_fail_with="caller is not minter")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "caller is not minter")] unconstrained fn mint_private_failure_non_minter() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, _, recipient) = utils::setup(/* with_account_contracts */ false); @@ -126,8 +125,7 @@ unconstrained fn mint_private_failure_non_minter() { Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); } -//#[test(should_fail_with="call to assert_max_bit_size")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "call to assert_max_bit_size")] unconstrained fn mint_private_failure_overflow() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, _, _) = utils::setup(/* with_account_contracts */ false); @@ -139,8 +137,7 @@ unconstrained fn mint_private_failure_overflow() { Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); } -//#[test(should_fail_with="attempt to add with overflow")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "attempt to add with overflow")] unconstrained fn mint_private_failure_overflow_recipient() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, _) = utils::setup(/* with_account_contracts */ false); @@ -171,8 +168,7 @@ unconstrained fn mint_private_failure_overflow_recipient() { Token::at(token_contract_address).mint_private(mint_amount, secret_hash).call(&mut env.public()); } -//#[test(should_fail_with="attempt to add with overflow")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "attempt to add with overflow")] unconstrained fn mint_private_failure_overflow_total_supply() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient) = utils::setup(/* with_account_contracts */ false); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr index e34aaab5dff..e2732e931ac 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/refunds.nr @@ -66,7 +66,7 @@ unconstrained fn setup_refund_success() { } //#[test(should_fail_with="funded amount not enough to cover tx fee")] -#[test(should_fail_with="Balance too low")] +#[test(should_fail_with = "Balance too low")] unconstrained fn setup_refund_insufficient_funded_amount() { let (env, token_contract_address, owner, recipient, _mint_amount) = utils::setup_and_mint(true); diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr index f680f3b992a..4de91aef482 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr @@ -46,7 +46,7 @@ unconstrained fn public_transfer_on_behalf_of_other() { utils::check_public_balance(token_contract_address, recipient, transfer_amount); } -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "attempt to subtract with underflow")] unconstrained fn public_transfer_failure_more_than_balance() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); @@ -57,8 +57,7 @@ unconstrained fn public_transfer_failure_more_than_balance() { public_transfer_call_interface.call(&mut env.public()); } -//#[test(should_fail_with="invalid nonce")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "invalid nonce")] unconstrained fn public_transfer_failure_on_behalf_of_self_non_zero_nonce() { // Setup without account contracts. We are not using authwits here, so dummy accounts are enough let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ false); @@ -70,7 +69,7 @@ unconstrained fn public_transfer_failure_on_behalf_of_self_non_zero_nonce() { public_transfer_call_interface.call(&mut env.public()); } -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "unauthorized")] unconstrained fn public_transfer_failure_on_behalf_of_other_without_approval() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -82,8 +81,7 @@ unconstrained fn public_transfer_failure_on_behalf_of_other_without_approval() { public_transfer_from_call_interface.call(&mut env.public()); } -//#[test(should_fail_with="attempt to subtract with underflow")] -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "attempt to subtract with underflow")] unconstrained fn public_transfer_failure_on_behalf_of_other_more_than_balance() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); @@ -98,7 +96,7 @@ unconstrained fn public_transfer_failure_on_behalf_of_other_more_than_balance() public_transfer_from_call_interface.call(&mut env.public()); } -#[test(should_fail_with="Nested call failed!")] +#[test(should_fail_with = "unauthorized")] unconstrained fn public_transfer_failure_on_behalf_of_other_wrong_caller() { // Setup with account contracts. Slower since we actually deploy them, but needed for authwits. let (env, token_contract_address, owner, recipient, mint_amount) = utils::setup_and_mint(/* with_account_contracts */ true); diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 980a167d29e..070d0a22ad6 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -676,54 +676,6 @@ export class TXE implements TypedOracle { return Promise.resolve(executionResult); } - async avmOpcodeCall( - targetContractAddress: AztecAddress, - functionSelector: FunctionSelector, - args: Fr[], - isStaticCall: boolean, - isDelegateCall: boolean, - ) { - // Store and modify env - const currentContractAddress = AztecAddress.fromField(this.contractAddress); - const currentMessageSender = AztecAddress.fromField(this.msgSender); - const currentFunctionSelector = FunctionSelector.fromField(this.functionSelector.toField()); - this.setMsgSender(this.contractAddress); - this.setContractAddress(targetContractAddress); - this.setFunctionSelector(functionSelector); - - const callContext = CallContext.empty(); - callContext.msgSender = this.msgSender; - callContext.functionSelector = this.functionSelector; - callContext.storageContractAddress = targetContractAddress; - callContext.isStaticCall = isStaticCall; - callContext.isDelegateCall = isDelegateCall; - - const executionResult = await this.executePublicFunction( - targetContractAddress, - args, - callContext, - this.sideEffectsCounter, - ); - - // Apply side effects - if (!executionResult.reverted) { - this.sideEffectsCounter = executionResult.endSideEffectCounter.toNumber() + 1; - await this.addNoteHashes( - targetContractAddress, - executionResult.noteHashes.map(noteHash => noteHash.value), - ); - await this.addNullifiers( - targetContractAddress, - executionResult.nullifiers.map(nullifier => nullifier.value), - ); - } - this.setContractAddress(currentContractAddress); - this.setMsgSender(currentMessageSender); - this.setFunctionSelector(currentFunctionSelector); - - return executionResult; - } - async enqueuePublicFunctionCall( targetContractAddress: AztecAddress, functionSelector: FunctionSelector, @@ -757,6 +709,7 @@ export class TXE implements TypedOracle { sideEffectCounter, ); + // Poor man's revert handling if (executionResult.reverted) { if (executionResult.revertReason && executionResult.revertReason instanceof SimulationError) { await enrichPublicSimulationError( @@ -767,7 +720,7 @@ export class TXE implements TypedOracle { ); throw new Error(executionResult.revertReason.message); } else { - throw new Error(`Public function call reverted: ${executionResult.revertReason}`); + throw new Error(`Enqueued public function call reverted: ${executionResult.revertReason}`); } } @@ -826,4 +779,94 @@ export class TXE implements TypedOracle { this.sideEffectsCounter = counter + 1; return; } + + // AVM oracles + + async avmOpcodeCall( + targetContractAddress: AztecAddress, + functionSelector: FunctionSelector, + args: Fr[], + isStaticCall: boolean, + isDelegateCall: boolean, + ) { + // Store and modify env + const currentContractAddress = AztecAddress.fromField(this.contractAddress); + const currentMessageSender = AztecAddress.fromField(this.msgSender); + const currentFunctionSelector = FunctionSelector.fromField(this.functionSelector.toField()); + this.setMsgSender(this.contractAddress); + this.setContractAddress(targetContractAddress); + this.setFunctionSelector(functionSelector); + + const callContext = CallContext.empty(); + callContext.msgSender = this.msgSender; + callContext.functionSelector = this.functionSelector; + callContext.storageContractAddress = targetContractAddress; + callContext.isStaticCall = isStaticCall; + callContext.isDelegateCall = isDelegateCall; + + const executionResult = await this.executePublicFunction( + targetContractAddress, + args, + callContext, + this.sideEffectsCounter, + ); + + // Apply side effects + if (!executionResult.reverted) { + this.sideEffectsCounter = executionResult.endSideEffectCounter.toNumber() + 1; + await this.addNoteHashes( + targetContractAddress, + executionResult.noteHashes.map(noteHash => noteHash.value), + ); + await this.addNullifiers( + targetContractAddress, + executionResult.nullifiers.map(nullifier => nullifier.value), + ); + } + + this.setContractAddress(currentContractAddress); + this.setMsgSender(currentMessageSender); + this.setFunctionSelector(currentFunctionSelector); + + return executionResult; + } + + async avmOpcodeNullifierExists(innerNullifier: Fr, targetAddress: AztecAddress): Promise { + const nullifier = siloNullifier(targetAddress, innerNullifier!); + const db = await this.trees.getLatest(); + const index = await db.findLeafIndex(MerkleTreeId.NULLIFIER_TREE, nullifier.toBuffer()); + return index !== undefined; + } + + async avmOpcodeEmitNullifier(nullifier: Fr) { + const db = await this.trees.getLatest(); + const siloedNullifier = siloNullifier(this.contractAddress, nullifier); + await db.batchInsert(MerkleTreeId.NULLIFIER_TREE, [siloedNullifier.toBuffer()], NULLIFIER_SUBTREE_HEIGHT); + return Promise.resolve(); + } + + async avmOpcodeEmitNoteHash(noteHash: Fr) { + const db = await this.trees.getLatest(); + const siloedNoteHash = siloNoteHash(this.contractAddress, noteHash); + await db.appendLeaves(MerkleTreeId.NOTE_HASH_TREE, [siloedNoteHash]); + return Promise.resolve(); + } + + async avmOpcodeStorageRead(slot: Fr) { + const db = await this.trees.getLatest(); + + const leafSlot = computePublicDataTreeLeafSlot(this.contractAddress, slot); + + const lowLeafResult = await db.getPreviousValueIndex(MerkleTreeId.PUBLIC_DATA_TREE, leafSlot.toBigInt()); + if (!lowLeafResult || !lowLeafResult.alreadyPresent) { + return Fr.ZERO; + } + + const preimage = (await db.getLeafPreimage( + MerkleTreeId.PUBLIC_DATA_TREE, + lowLeafResult.index, + )) as PublicDataTreeLeafPreimage; + + return preimage.value; + } } diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index 34835081ae6..dc7b0fd0d04 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -1,5 +1,5 @@ import { SchnorrAccountContractArtifact } from '@aztec/accounts/schnorr'; -import { L2Block, MerkleTreeId, PublicDataWrite } from '@aztec/circuit-types'; +import { L2Block, MerkleTreeId, PublicDataWrite, SimulationError } from '@aztec/circuit-types'; import { Fr, FunctionSelector, @@ -31,6 +31,7 @@ import { toSingle, } from '../util/encoding.js'; import { ExpectedFailureError } from '../util/expected_failure_error.js'; +import { enrichPublicSimulationError } from '../util/simulation_error.js'; import { TXEDatabase } from '../util/txe_database.js'; export class TXEService { @@ -611,7 +612,90 @@ export class TXEService { return toForeignCallResult([]); } - // AVM oracles (only to perform calls directly from the tests) + // AVM opcodes + + avmOpcodeEmitUnencryptedLog(_message: ForeignCallArray) { + // TODO(#8811): Implement + return toForeignCallResult([]); + } + + async avmOpcodeStorageRead(slot: ForeignCallSingle) { + const value = await (this.typedOracle as TXE).avmOpcodeStorageRead(fromSingle(slot)); + return toForeignCallResult([toSingle(value)]); + } + + async avmOpcodeStorageWrite(slot: ForeignCallSingle, value: ForeignCallSingle) { + await this.typedOracle.storageWrite(fromSingle(slot), [fromSingle(value)]); + return toForeignCallResult([]); + } + + async avmOpcodeGetContractInstance(address: ForeignCallSingle) { + const instance = await this.typedOracle.getContractInstance(fromSingle(address)); + return toForeignCallResult([ + toArray([ + // AVM requires an extra boolean indicating the instance was found + new Fr(1), + instance.salt, + instance.deployer, + instance.contractClassId, + instance.initializationHash, + instance.publicKeysHash, + ]), + ]); + } + + avmOpcodeSender() { + const sender = (this.typedOracle as TXE).getMsgSender(); + return toForeignCallResult([toSingle(sender)]); + } + + async avmOpcodeEmitNullifier(nullifier: ForeignCallSingle) { + await (this.typedOracle as TXE).avmOpcodeEmitNullifier(fromSingle(nullifier)); + return toForeignCallResult([]); + } + + async avmOpcodeEmitNoteHash(noteHash: ForeignCallSingle) { + await (this.typedOracle as TXE).avmOpcodeEmitNoteHash(fromSingle(noteHash)); + return toForeignCallResult([]); + } + + async avmOpcodeNullifierExists(innerNullifier: ForeignCallSingle, targetAddress: ForeignCallSingle) { + const exists = await (this.typedOracle as TXE).avmOpcodeNullifierExists( + fromSingle(innerNullifier), + AztecAddress.fromField(fromSingle(targetAddress)), + ); + return toForeignCallResult([toSingle(new Fr(exists))]); + } + + async avmOpcodeAddress() { + const contractAddress = await this.typedOracle.getContractAddress(); + return toForeignCallResult([toSingle(contractAddress.toField())]); + } + + async avmOpcodeBlockNumber() { + const blockNumber = await this.typedOracle.getBlockNumber(); + return toForeignCallResult([toSingle(new Fr(blockNumber))]); + } + + avmOpcodeFunctionSelector() { + const functionSelector = (this.typedOracle as TXE).getFunctionSelector(); + return toForeignCallResult([toSingle(functionSelector.toField())]); + } + + avmOpcodeIsStaticCall() { + const isStaticCall = (this.typedOracle as TXE).getIsStaticCall(); + return toForeignCallResult([toSingle(new Fr(isStaticCall ? 1 : 0))]); + } + + async avmOpcodeChainId() { + const chainId = await (this.typedOracle as TXE).getChainId(); + return toForeignCallResult([toSingle(chainId)]); + } + + async avmOpcodeVersion() { + const version = await (this.typedOracle as TXE).getVersion(); + return toForeignCallResult([toSingle(version)]); + } async avmOpcodeCall( _gas: ForeignCallArray, @@ -628,6 +712,21 @@ export class TXEService { /* isDelegateCall */ false, ); + // Poor man's revert handling + if (result.reverted) { + if (result.revertReason && result.revertReason instanceof SimulationError) { + await enrichPublicSimulationError( + result.revertReason, + (this.typedOracle as TXE).getTXEDatabase(), + (this.typedOracle as TXE).getContractDataOracle(), + this.logger, + ); + throw new Error(result.revertReason.message); + } else { + throw new Error(`Public function call reverted: ${result.revertReason}`); + } + } + return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(!result.reverted))]); } @@ -646,6 +745,21 @@ export class TXEService { /* isDelegateCall */ false, ); + // Poor man's revert handling + if (result.reverted) { + if (result.revertReason && result.revertReason instanceof SimulationError) { + await enrichPublicSimulationError( + result.revertReason, + (this.typedOracle as TXE).getTXEDatabase(), + (this.typedOracle as TXE).getContractDataOracle(), + this.logger, + ); + throw new Error(result.revertReason.message); + } else { + throw new Error(`Public function call reverted: ${result.revertReason}`); + } + } + return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(1))]); } } diff --git a/yarn-project/txe/src/util/simulation_error.ts b/yarn-project/txe/src/util/simulation_error.ts index 21cf441ad66..baa6564c3ac 100644 --- a/yarn-project/txe/src/util/simulation_error.ts +++ b/yarn-project/txe/src/util/simulation_error.ts @@ -1,10 +1,10 @@ -import { SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; +import { type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; import { AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; -import { Logger } from '@aztec/foundation/log'; -import { ContractDataOracle } from '@aztec/pxe'; +import { type Logger } from '@aztec/foundation/log'; +import { type ContractDataOracle } from '@aztec/pxe'; import { resolveAssertionMessage, resolveOpcodeLocations } from '@aztec/simulator'; -import { TXEDatabase } from './txe_database.js'; +import { type TXEDatabase } from './txe_database.js'; export async function enrichPublicSimulationError( err: SimulationError, From aa5c50baa77c1d997e9984b4ae0de29673928ead Mon Sep 17 00:00:00 2001 From: thunkar Date: Tue, 15 Oct 2024 15:27:29 +0000 Subject: [PATCH 06/14] docs --- .../smart_contracts/testing_contracts/testing.md | 2 +- docs/docs/migration_notes.md | 1 + .../contracts/token_contract/src/test/access_control.nr | 2 ++ yarn-project/txe/src/txe_service/txe_service.ts | 2 +- 4 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/docs/guides/developer_guides/smart_contracts/testing_contracts/testing.md b/docs/docs/guides/developer_guides/smart_contracts/testing_contracts/testing.md index 827547fe098..2449195013b 100644 --- a/docs/docs/guides/developer_guides/smart_contracts/testing_contracts/testing.md +++ b/docs/docs/guides/developer_guides/smart_contracts/testing_contracts/testing.md @@ -210,7 +210,7 @@ For example: You can also use the `assert_public_call_fails` or `assert_private_call_fails` methods on the `TestEnvironment` to check that a call fails. -#include_code assert_public_fail /noir-projects/noir-contracts/contracts/token_contract/src/test/transfer_public.nr rust +#include_code assert_public_fail /noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr rust ### Logging diff --git a/docs/docs/migration_notes.md b/docs/docs/migration_notes.md index d529198a116..cc9f695d4e9 100644 --- a/docs/docs/migration_notes.md +++ b/docs/docs/migration_notes.md @@ -16,6 +16,7 @@ This ensures every single test runs in a consistent environment and allows for c -env.call_private(my_contract_interface) +MyContract::at(address).my_function(args).call(&mut env.private()); ``` +This implies every contract has to be deployed before it can be tested (via `env.deploy` or `env.deploy_self`) and of course it has to be recompiled if its code was changed before TXE can use the modified bytecode. ## 0.58.0 ### [l1-contracts] Inbox's MessageSent event emits global tree index diff --git a/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr b/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr index 68ef7dd2b05..2999be5dfeb 100644 --- a/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr +++ b/noir-projects/noir-contracts/contracts/token_contract/src/test/access_control.nr @@ -29,9 +29,11 @@ unconstrained fn access_control() { assert(is_minter == false); // Impersonate original admin env.impersonate(owner); + // docs:start:assert_public_fail // Try to set ourselves as admin, fail miserably let set_admin_call_interface = Token::at(token_contract_address).set_admin(recipient); env.assert_public_call_fails(set_admin_call_interface); + // docs:end:assert_public_fail // Try to revoke minter status to recipient, fail miserably let set_minter_call_interface = Token::at(token_contract_address).set_minter(recipient, false); env.assert_public_call_fails(set_minter_call_interface); diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index dc7b0fd0d04..7cd1da2e394 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -9,7 +9,7 @@ import { computePartialAddress, getContractInstanceFromDeployParams, } from '@aztec/circuits.js'; -import { computePublicDataTreeLeafSlot, computeVarArgsHash } from '@aztec/circuits.js/hash'; +import { computePublicDataTreeLeafSlot } from '@aztec/circuits.js/hash'; import { type ContractArtifact, NoteSelector } from '@aztec/foundation/abi'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { type Logger } from '@aztec/foundation/log'; From 20e91a25a413b449815e15a2c2e6e0d0cf4e9531 Mon Sep 17 00:00:00 2001 From: thunkar Date: Wed, 16 Oct 2024 09:17:24 +0000 Subject: [PATCH 07/14] unify simulation error handling --- .../src/pxe_service/error_enriching.ts} | 84 ++++++++++--------- yarn-project/pxe/src/pxe_service/index.ts | 1 + .../pxe/src/pxe_service/pxe_service.ts | 75 ++--------------- yarn-project/txe/src/oracle/txe_oracle.ts | 5 +- .../txe/src/txe_service/txe_service.ts | 6 +- 5 files changed, 54 insertions(+), 117 deletions(-) rename yarn-project/{txe/src/util/simulation_error.ts => pxe/src/pxe_service/error_enriching.ts} (88%) diff --git a/yarn-project/txe/src/util/simulation_error.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts similarity index 88% rename from yarn-project/txe/src/util/simulation_error.ts rename to yarn-project/pxe/src/pxe_service/error_enriching.ts index baa6564c3ac..087e769ac74 100644 --- a/yarn-project/txe/src/util/simulation_error.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -1,18 +1,53 @@ -import { type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; +import { SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; import { AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; -import { type Logger } from '@aztec/foundation/log'; -import { type ContractDataOracle } from '@aztec/pxe'; +import { DebugLogger } from '@aztec/foundation/log'; import { resolveAssertionMessage, resolveOpcodeLocations } from '@aztec/simulator'; -import { type TXEDatabase } from './txe_database.js'; +import { ContractDataOracle, PxeDatabase } from '../index.js'; + +/** + * Adds contract and function names to a simulation error. + * @param err - The error to enrich. + */ +export async function enrichSimulationError(err: SimulationError, db: PxeDatabase) { + // Maps contract addresses to the set of functions selectors that were in error. + // Using strings because map and set don't use .equals() + const mentionedFunctions: Map> = new Map(); + + err.getCallStack().forEach(({ contractAddress, functionSelector }) => { + if (!mentionedFunctions.has(contractAddress.toString())) { + mentionedFunctions.set(contractAddress.toString(), new Set()); + } + mentionedFunctions.get(contractAddress.toString())!.add(functionSelector.toString()); + }); + + await Promise.all( + [...mentionedFunctions.entries()].map(async ([contractAddress, selectors]) => { + const parsedContractAddress = AztecAddress.fromString(contractAddress); + const contract = await db.getContract(parsedContractAddress); + if (contract) { + err.enrichWithContractName(parsedContractAddress, contract.name); + selectors.forEach(selector => { + const functionArtifact = contract.functions.find(f => FunctionSelector.fromString(selector).equals(f)); + if (functionArtifact) { + err.enrichWithFunctionName( + parsedContractAddress, + FunctionSelector.fromNameAndParameters(functionArtifact), + functionArtifact.name, + ); + } + }); + } + }), + ); +} export async function enrichPublicSimulationError( err: SimulationError, - db: TXEDatabase, contractDataOracle: ContractDataOracle, - logger: Logger, + db: PxeDatabase, + logger: DebugLogger, ) { - // Try to fill in the noir call stack since the PXE may have access to the debug metadata const callStack = err.getCallStack(); const originalFailingFunction = callStack[callStack.length - 1]; // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Properly fix this. @@ -41,39 +76,6 @@ export async function enrichPublicSimulationError( ); } } + await enrichSimulationError(err, db); } - await enrichSimulationError(err, db); -} - -export async function enrichSimulationError(err: SimulationError, db: TXEDatabase) { - // Maps contract addresses to the set of functions selectors that were in error. - // Using strings because map and set don't use .equals() - const mentionedFunctions: Map> = new Map(); - - err.getCallStack().forEach(({ contractAddress, functionSelector }) => { - if (!mentionedFunctions.has(contractAddress.toString())) { - mentionedFunctions.set(contractAddress.toString(), new Set()); - } - mentionedFunctions.get(contractAddress.toString())!.add(functionSelector.toString()); - }); - - await Promise.all( - [...mentionedFunctions.entries()].map(async ([contractAddress, selectors]) => { - const parsedContractAddress = AztecAddress.fromString(contractAddress); - const contract = await db.getContract(parsedContractAddress); - if (contract) { - err.enrichWithContractName(parsedContractAddress, contract.name); - selectors.forEach(selector => { - const functionArtifact = contract.functions.find(f => FunctionSelector.fromString(selector).equals(f)); - if (functionArtifact) { - err.enrichWithFunctionName( - parsedContractAddress, - FunctionSelector.fromNameAndParameters(functionArtifact), - functionArtifact.name, - ); - } - }); - } - }), - ); } diff --git a/yarn-project/pxe/src/pxe_service/index.ts b/yarn-project/pxe/src/pxe_service/index.ts index a43b331f3ad..c9018d7ba8c 100644 --- a/yarn-project/pxe/src/pxe_service/index.ts +++ b/yarn-project/pxe/src/pxe_service/index.ts @@ -1,3 +1,4 @@ export * from './pxe_service.js'; export * from './create_pxe_service.js'; +export { enrichPublicSimulationError } from './error_enriching.js'; export { pxeTestSuite } from './test/pxe_test_suite.js'; diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 3b13593cb29..189a72ba118 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -64,7 +64,7 @@ import { getCanonicalProtocolContract, protocolContractNames, } from '@aztec/protocol-contracts'; -import { type AcirSimulator, resolveAssertionMessage, resolveOpcodeLocations } from '@aztec/simulator'; +import { type AcirSimulator } from '@aztec/simulator'; import { type PXEServiceConfig, getPackageInfo } from '../config/index.js'; import { ContractDataOracle } from '../contract_data_oracle/index.js'; @@ -75,6 +75,7 @@ import { KernelProver } from '../kernel_prover/kernel_prover.js'; import { TestPrivateKernelProver } from '../kernel_prover/test/test_circuit_prover.js'; import { getAcirSimulator } from '../simulator/index.js'; import { Synchronizer } from '../synchronizer/index.js'; +import { enrichPublicSimulationError, enrichSimulationError } from './error_enriching.js'; /** * A Private eXecution Environment (PXE) implementation. @@ -720,7 +721,7 @@ export class PXEService implements PXE { return result; } catch (err) { if (err instanceof SimulationError) { - await this.#enrichSimulationError(err); + await enrichSimulationError(err, this.db); } throw err; } @@ -746,7 +747,7 @@ export class PXEService implements PXE { return result; } catch (err) { if (err instanceof SimulationError) { - await this.#enrichSimulationError(err); + await enrichSimulationError(err, this.db); } throw err; } @@ -764,36 +765,7 @@ export class PXEService implements PXE { } catch (err) { // Try to fill in the noir call stack since the PXE may have access to the debug metadata if (err instanceof SimulationError) { - const callStack = err.getCallStack(); - const originalFailingFunction = callStack[callStack.length - 1]; - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/8985): Properly fix this. - // To be able to resolve the assertion message, we need to use the information from the public dispatch function, - // no matter what the call stack selector points to (since we've modified it to point to the target function). - // We should remove this because the AVM (or public protocol) shouldn't be aware of the public dispatch calling convention. - const debugInfo = await this.contractDataOracle.getFunctionDebugMetadata( - originalFailingFunction.contractAddress, - FunctionSelector.fromField(new Fr(PUBLIC_DISPATCH_SELECTOR)), - ); - const noirCallStack = err.getNoirCallStack(); - if (debugInfo) { - if (isNoirCallStackUnresolved(noirCallStack)) { - const assertionMessage = resolveAssertionMessage(noirCallStack, debugInfo); - if (assertionMessage) { - err.setOriginalMessage(err.getOriginalMessage() + `: ${assertionMessage}`); - } - try { - // Public functions are simulated as a single Brillig entry point. - // Thus, we can safely assume here that the Brillig function id is `0`. - const parsedCallStack = resolveOpcodeLocations(noirCallStack, debugInfo, 0); - err.setNoirCallStack(parsedCallStack); - } catch (err) { - this.log.warn( - `Could not resolve noir call stack for ${originalFailingFunction.contractAddress.toString()}:${originalFailingFunction.functionSelector.toString()}: ${err}`, - ); - } - } - await this.#enrichSimulationError(err); - } + await enrichPublicSimulationError(err, this.contractDataOracle, this.db, this.log); } throw err; @@ -829,43 +801,6 @@ export class PXEService implements PXE { return await kernelProver.prove(txExecutionRequest.toTxRequest(), privateExecutionResult); } - /** - * Adds contract and function names to a simulation error. - * @param err - The error to enrich. - */ - async #enrichSimulationError(err: SimulationError) { - // Maps contract addresses to the set of functions selectors that were in error. - // Using strings because map and set don't use .equals() - const mentionedFunctions: Map> = new Map(); - - err.getCallStack().forEach(({ contractAddress, functionSelector }) => { - if (!mentionedFunctions.has(contractAddress.toString())) { - mentionedFunctions.set(contractAddress.toString(), new Set()); - } - mentionedFunctions.get(contractAddress.toString())!.add(functionSelector.toString()); - }); - - await Promise.all( - [...mentionedFunctions.entries()].map(async ([contractAddress, selectors]) => { - const parsedContractAddress = AztecAddress.fromString(contractAddress); - const contract = await this.db.getContract(parsedContractAddress); - if (contract) { - err.enrichWithContractName(parsedContractAddress, contract.name); - selectors.forEach(selector => { - const functionArtifact = contract.functions.find(f => FunctionSelector.fromString(selector).equals(f)); - if (functionArtifact) { - err.enrichWithFunctionName( - parsedContractAddress, - FunctionSelector.fromNameAndParameters(functionArtifact), - functionArtifact.name, - ); - } - }); - } - }), - ); - } - public async isGlobalStateSynchronized() { return await this.synchronizer.isGlobalStateSynchronized(); } diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 070d0a22ad6..113cd83adbe 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -47,7 +47,7 @@ import { Fr } from '@aztec/foundation/fields'; import { type Logger, applyStringFormatting } from '@aztec/foundation/log'; import { Timer } from '@aztec/foundation/timer'; import { type KeyStore } from '@aztec/key-store'; -import { ContractDataOracle } from '@aztec/pxe'; +import { ContractDataOracle, enrichPublicSimulationError } from '@aztec/pxe'; import { ExecutionError, type ExecutionNoteCache, @@ -68,7 +68,6 @@ import { import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; import { MerkleTreeSnapshotOperationsFacade, type MerkleTrees } from '@aztec/world-state'; -import { enrichPublicSimulationError } from '../util/simulation_error.js'; import { type TXEDatabase } from '../util/txe_database.js'; import { TXEPublicContractDataSource } from '../util/txe_public_contract_data_source.js'; import { TXEWorldStateDB } from '../util/txe_world_state_db.js'; @@ -714,8 +713,8 @@ export class TXE implements TypedOracle { if (executionResult.revertReason && executionResult.revertReason instanceof SimulationError) { await enrichPublicSimulationError( executionResult.revertReason, - this.txeDatabase, this.contractDataOracle, + this.txeDatabase, this.logger, ); throw new Error(executionResult.revertReason.message); diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index 7cd1da2e394..6bf53fee559 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -16,6 +16,7 @@ import { type Logger } from '@aztec/foundation/log'; import { KeyStore } from '@aztec/key-store'; import { openTmpStore } from '@aztec/kv-store/utils'; import { getCanonicalProtocolContract, protocolContractNames } from '@aztec/protocol-contracts'; +import { enrichPublicSimulationError } from '@aztec/pxe'; import { ExecutionNoteCache, PackedValuesCache, type TypedOracle } from '@aztec/simulator'; import { NoopTelemetryClient } from '@aztec/telemetry-client/noop'; import { MerkleTrees } from '@aztec/world-state'; @@ -31,7 +32,6 @@ import { toSingle, } from '../util/encoding.js'; import { ExpectedFailureError } from '../util/expected_failure_error.js'; -import { enrichPublicSimulationError } from '../util/simulation_error.js'; import { TXEDatabase } from '../util/txe_database.js'; export class TXEService { @@ -717,8 +717,8 @@ export class TXEService { if (result.revertReason && result.revertReason instanceof SimulationError) { await enrichPublicSimulationError( result.revertReason, - (this.typedOracle as TXE).getTXEDatabase(), (this.typedOracle as TXE).getContractDataOracle(), + (this.typedOracle as TXE).getTXEDatabase(), this.logger, ); throw new Error(result.revertReason.message); @@ -750,8 +750,8 @@ export class TXEService { if (result.revertReason && result.revertReason instanceof SimulationError) { await enrichPublicSimulationError( result.revertReason, - (this.typedOracle as TXE).getTXEDatabase(), (this.typedOracle as TXE).getContractDataOracle(), + (this.typedOracle as TXE).getTXEDatabase(), this.logger, ); throw new Error(result.revertReason.message); From 8ed69e6fd01f6e0b6d1f73be4eb6976f32d7c180 Mon Sep 17 00:00:00 2001 From: thunkar Date: Wed, 16 Oct 2024 09:41:36 +0000 Subject: [PATCH 08/14] fmt --- yarn-project/pxe/src/pxe_service/error_enriching.ts | 6 +++--- yarn-project/pxe/src/pxe_service/pxe_service.ts | 4 +--- 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/yarn-project/pxe/src/pxe_service/error_enriching.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts index 087e769ac74..9d7266c5312 100644 --- a/yarn-project/pxe/src/pxe_service/error_enriching.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -1,9 +1,9 @@ -import { SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; +import { type SimulationError, isNoirCallStackUnresolved } from '@aztec/circuit-types'; import { AztecAddress, Fr, FunctionSelector, PUBLIC_DISPATCH_SELECTOR } from '@aztec/circuits.js'; -import { DebugLogger } from '@aztec/foundation/log'; +import { type DebugLogger } from '@aztec/foundation/log'; import { resolveAssertionMessage, resolveOpcodeLocations } from '@aztec/simulator'; -import { ContractDataOracle, PxeDatabase } from '../index.js'; +import { type ContractDataOracle, type PxeDatabase } from '../index.js'; /** * Adds contract and function names to a simulation error. diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 189a72ba118..d61f2775246 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -30,17 +30,15 @@ import { TxSimulationResult, UniqueNote, getNonNullifiedL1ToL2MessageWitness, - isNoirCallStackUnresolved, } from '@aztec/circuit-types'; import { type NoteProcessorStats } from '@aztec/circuit-types/stats'; import { - AztecAddress, + type AztecAddress, type CompleteAddress, type ContractClassWithId, type ContractInstanceWithAddress, type L1_TO_L2_MSG_TREE_HEIGHT, type NodeInfo, - PUBLIC_DISPATCH_SELECTOR, type PartialAddress, type PrivateKernelTailCircuitPublicInputs, computeContractAddressFromInstance, From 2027fba961cc2fbb967a68a6bdd46fc6375f3a4e Mon Sep 17 00:00:00 2001 From: thunkar Date: Thu, 17 Oct 2024 09:08:30 +0000 Subject: [PATCH 09/14] comments from review --- noir-projects/aztec-nr/aztec/src/macros/utils.nr | 4 ---- .../pxe/src/pxe_service/error_enriching.ts | 15 ++++++++++++--- yarn-project/pxe/src/pxe_service/pxe_service.ts | 4 ++-- 3 files changed, 14 insertions(+), 9 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/macros/utils.nr b/noir-projects/aztec-nr/aztec/src/macros/utils.nr index ae99f67decd..27c2086246e 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/utils.nr @@ -34,10 +34,6 @@ pub(crate) comptime fn fn_has_noinitcheck(f: FunctionDefinition) -> bool { f.has_named_attribute("noinitcheck") } -pub(crate) comptime fn fn_requires_authwit(f: FunctionDefinition) -> bool { - f.has_named_attribute("authwit") -} - /// Takes a function body as a collection of expressions, and alters it by prepending and appending quoted values. pub(crate) comptime fn modify_fn_body(body: [Expr], prepend: Quoted, append: Quoted) -> Expr { // We need to quote the body before we can alter its contents, so we fold it by quoting each expression. diff --git a/yarn-project/pxe/src/pxe_service/error_enriching.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts index 9d7266c5312..bb1a33cb9ef 100644 --- a/yarn-project/pxe/src/pxe_service/error_enriching.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -6,10 +6,11 @@ import { resolveAssertionMessage, resolveOpcodeLocations } from '@aztec/simulato import { type ContractDataOracle, type PxeDatabase } from '../index.js'; /** - * Adds contract and function names to a simulation error. + * Adds contract and function names to a simulation error, if they + * can be found in the PXE database * @param err - The error to enrich. */ -export async function enrichSimulationError(err: SimulationError, db: PxeDatabase) { +export async function enrichSimulationError(err: SimulationError, db: PxeDatabase, logger: DebugLogger) { // Maps contract addresses to the set of functions selectors that were in error. // Using strings because map and set don't use .equals() const mentionedFunctions: Map> = new Map(); @@ -35,8 +36,16 @@ export async function enrichSimulationError(err: SimulationError, db: PxeDatabas FunctionSelector.fromNameAndParameters(functionArtifact), functionArtifact.name, ); + } else { + logger.warn( + `Could not function artifact in contract ${contract.name} for selector ${selector} when enriching error callstack`, + ); } }); + } else { + logger.warn( + `Could not find contract in database for address: ${parsedContractAddress} when enriching error callstack`, + ); } }), ); @@ -76,6 +85,6 @@ export async function enrichPublicSimulationError( ); } } - await enrichSimulationError(err, db); + await enrichSimulationError(err, db, logger); } } diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index d61f2775246..2578b260c0a 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -719,7 +719,7 @@ export class PXEService implements PXE { return result; } catch (err) { if (err instanceof SimulationError) { - await enrichSimulationError(err, this.db); + await enrichSimulationError(err, this.db, this.log); } throw err; } @@ -745,7 +745,7 @@ export class PXEService implements PXE { return result; } catch (err) { if (err instanceof SimulationError) { - await enrichSimulationError(err, this.db); + await enrichSimulationError(err, this.db, this.log); } throw err; } From 22da773bfa066fab87ccd98ae89265435222e6d0 Mon Sep 17 00:00:00 2001 From: thunkar Date: Thu, 17 Oct 2024 10:34:46 +0000 Subject: [PATCH 10/14] removed dep --- noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr | 1 - 1 file changed, 1 deletion(-) diff --git a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr index cb9ac24454a..5ac843c6b9a 100644 --- a/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr +++ b/noir-projects/aztec-nr/aztec/src/macros/functions/mod.nr @@ -8,7 +8,6 @@ use super::utils::{ use protocol_types::meta::flatten_to_fields; use interfaces::{create_fn_abi_export, register_stub, stub_fn}; -use super::utils::fn_requires_authwit; // Functions can have multiple attributes applied to them, e.g. a single function can have #[public], #[view] and // #[internal]. However. the order in which this will be evaluated is unknown, which makes combining them tricky. From d174d0cdd9c82096b01e4596a2d4e8b77de35ed2 Mon Sep 17 00:00:00 2001 From: Gregorio Juliana Date: Wed, 23 Oct 2024 09:28:05 +0200 Subject: [PATCH 11/14] Update yarn-project/pxe/src/pxe_service/error_enriching.ts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Nicolás Venturo --- yarn-project/pxe/src/pxe_service/error_enriching.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/yarn-project/pxe/src/pxe_service/error_enriching.ts b/yarn-project/pxe/src/pxe_service/error_enriching.ts index bb1a33cb9ef..6f40630e35d 100644 --- a/yarn-project/pxe/src/pxe_service/error_enriching.ts +++ b/yarn-project/pxe/src/pxe_service/error_enriching.ts @@ -12,7 +12,8 @@ import { type ContractDataOracle, type PxeDatabase } from '../index.js'; */ export async function enrichSimulationError(err: SimulationError, db: PxeDatabase, logger: DebugLogger) { // Maps contract addresses to the set of functions selectors that were in error. - // Using strings because map and set don't use .equals() + // Map and Set do reference equality for their keys instead of value equality, so we store the string + // representation to get e.g. different contract address objects with the same address value to match. const mentionedFunctions: Map> = new Map(); err.getCallStack().forEach(({ contractAddress, functionSelector }) => { From 75fda875584167e99987f44690abe50859f2c581 Mon Sep 17 00:00:00 2001 From: thunkar Date: Wed, 23 Oct 2024 07:34:39 +0000 Subject: [PATCH 12/14] fix --- yarn-project/txe/src/txe_service/txe_service.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index a51cc77835e..41ddf31ddc2 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -219,7 +219,7 @@ export class TXEService { ) { const parsedAddress = fromSingle(address); const parsedSelector = FunctionSelector.fromField(fromSingle(functionSelector)); - const result = await (this.typedOracle as TXE).avmOpcodeCall(parsedAddress, parsedSelector, fromArray(args)); + const result = await (this.typedOracle as TXE).avmOpcodeCall(parsedAddress, parsedSelector, fromArray(args), false); if (!result.reverted) { throw new ExpectedFailureError('Public call did not revert'); } @@ -696,7 +696,6 @@ export class TXEService { FunctionSelector.fromField(fromSingle(functionSelector)), fromArray(args), /* isStaticCall */ false, - /* isDelegateCall */ false, ); // Poor man's revert handling @@ -729,7 +728,6 @@ export class TXEService { FunctionSelector.fromField(fromSingle(functionSelector)), fromArray(args), /* isStaticCall */ true, - /* isDelegateCall */ false, ); // Poor man's revert handling From 268ff7188a08a448dd6ef805b9f5c187672f3fab Mon Sep 17 00:00:00 2001 From: thunkar Date: Wed, 23 Oct 2024 07:49:30 +0000 Subject: [PATCH 13/14] fix --- noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr index d3b6189b1dd..8463f66b4cd 100644 --- a/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr +++ b/noir-projects/aztec-nr/aztec/src/test/helpers/utils.nr @@ -43,8 +43,7 @@ impl Deployer { instance.to_address(), call_interface.get_selector(), args_hash, - call_interface.get_is_static(), - false + call_interface.get_is_static() ); instance From 070edb95be37c0b55ffef8b5afe98e5a07f25a46 Mon Sep 17 00:00:00 2001 From: thunkar Date: Wed, 23 Oct 2024 13:39:42 +0000 Subject: [PATCH 14/14] fixes --- yarn-project/txe/src/oracle/txe_oracle.ts | 10 ++++++---- yarn-project/txe/src/txe_service/txe_service.ts | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/yarn-project/txe/src/oracle/txe_oracle.ts b/yarn-project/txe/src/oracle/txe_oracle.ts index 558b3f2d627..027f60b3630 100644 --- a/yarn-project/txe/src/oracle/txe_oracle.ts +++ b/yarn-project/txe/src/oracle/txe_oracle.ts @@ -763,10 +763,12 @@ export class TXE implements TypedOracle { this.setContractAddress(targetContractAddress); this.setFunctionSelector(functionSelector); - const callContext = CallContext.empty(); - callContext.msgSender = this.msgSender; - callContext.functionSelector = this.functionSelector; - callContext.isStaticCall = isStaticCall; + const callContext = new CallContext( + /* msgSender */ currentContractAddress, + targetContractAddress, + functionSelector, + isStaticCall, + ); const executionResult = await this.executePublicFunction(args, callContext, this.sideEffectsCounter); diff --git a/yarn-project/txe/src/txe_service/txe_service.ts b/yarn-project/txe/src/txe_service/txe_service.ts index 41ddf31ddc2..d0761d707ef 100644 --- a/yarn-project/txe/src/txe_service/txe_service.ts +++ b/yarn-project/txe/src/txe_service/txe_service.ts @@ -745,6 +745,6 @@ export class TXEService { } } - return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(1))]); + return toForeignCallResult([toArray(result.returnValues), toSingle(new Fr(!result.reverted))]); } }