From 3d08551739c8a11c9cc35abf4f43c63a345140dc Mon Sep 17 00:00:00 2001 From: LHerskind Date: Tue, 27 Feb 2024 15:48:28 +0000 Subject: [PATCH] feat: initial authwit cancellation support --- .../resources/common_patterns/authwit.md | 15 ++-- .../resources/common_patterns/main.md | 2 +- .../tutorials/uniswap/l2_contract_setup.md | 2 +- noir-projects/aztec-nr/authwit/src/account.nr | 48 +++++++++---- noir-projects/aztec-nr/authwit/src/auth.nr | 68 +++++++------------ .../ecdsa_account_contract/src/main.nr | 21 +++--- .../schnorr_account_contract/src/main.nr | 25 ++++--- .../src/main.nr | 24 ++++--- .../src/main.nr | 27 +++++--- .../contracts/uniswap_contract/src/main.nr | 15 +++- noir/aztec_macros/src/lib.rs | 3 + .../aztec.js/src/wallet/account_wallet.ts | 38 +++++++++-- .../src/e2e_authwit_token_contract.test.ts | 33 +++++++++ .../end-to-end/src/e2e_token_contract.test.ts | 48 +++++++++++++ .../end-to-end/src/shared/uniswap_l1_l2.ts | 4 +- .../src/sequencer/abstract_phase_manager.ts | 2 +- .../src/sequencer/public_processor.ts | 2 +- 17 files changed, 263 insertions(+), 114 deletions(-) create mode 100644 yarn-project/end-to-end/src/e2e_authwit_token_contract.test.ts diff --git a/docs/docs/developers/contracts/resources/common_patterns/authwit.md b/docs/docs/developers/contracts/resources/common_patterns/authwit.md index f2dc1bc723ce..f9a679658d80 100644 --- a/docs/docs/developers/contracts/resources/common_patterns/authwit.md +++ b/docs/docs/developers/contracts/resources/common_patterns/authwit.md @@ -70,10 +70,10 @@ As part of `AuthWit` we are assuming that the `on_behalf_of` implements the priv ```rust #[aztec(private)] -fn is_valid(message_hash: Field) -> Field; +fn spend_private_authwit(caller: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field; #[aztec(public)] -fn is_valid_public(message_hash: Field) -> Field; +fn spend_public_authwit(caller: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field; ``` Both return the value `0xe86ab4ff` (`is_valid` selector) for a successful authentication, and `0x00000000` for a failed authentication. You might be wondering why we are expecting the return value to be a selector instead of a boolean. This is mainly to account for a case of selector collisions where the same selector is used for different functions, and we don't want an account to mistakenly allow a different function to be called on its behalf - it is hard to return the selector by mistake, but you might have other functions returning a bool. @@ -102,11 +102,10 @@ As you can see above, this function takes a `caller` and a `request`. The `reque For private calls where we allow execution on behalf of others, we generally want to check if the current call is authenticated by `on_behalf_of`. To easily do so, we can use the `assert_current_call_valid_authwit` which fetches information from the current context without us needing to provide much beyond the `on_behalf_of`. -This function computes the message hash, and then forwards the call to the more generic `assert_valid_authwit`. This validating function will then: - -- make a call to `on_behalf_of` to validate that the call is authenticated -- emit a nullifier for the action to prevent replay attacks -- throw if the action is not authenticated by `on_behalf_of` +This function will then make a to `on_behalf_of` to execute the `spend_private_authwit` function which validates that the call is authenticated. +The `on_behalf_of` should assert that we are indeed authenticated and then emit a nullifier when we are spending the authwit to prevent replay attacks. +If the return value is not as expected, we throw an error. +This is to cover the case where the `on_behalf_of` might implemented some function with the same selector as the `spend_private_authwit` that could be used to authenticate unintentionally. #### Example @@ -176,7 +175,7 @@ In the snippet below, this is done as a separate contract call, but can also be We have cases where we need a non-wallet contract to approve an action to be executed by another contract. One of the cases could be when making more complex defi where funds are passed along. When doing so, we need the intermediate contracts to support approving of actions on their behalf. -To support this, we must implement the `is_valid_public` function as seen in the snippet below. +To support this, we must implement the `spend_public_authwit` function as seen in the snippet below. #include_code authwit_uniswap_get /noir-projects/noir-contracts/contracts/uniswap_contract/src/main.nr rust diff --git a/docs/docs/developers/contracts/resources/common_patterns/main.md b/docs/docs/developers/contracts/resources/common_patterns/main.md index c6c31296a0ee..e00f97703970 100644 --- a/docs/docs/developers/contracts/resources/common_patterns/main.md +++ b/docs/docs/developers/contracts/resources/common_patterns/main.md @@ -38,7 +38,7 @@ E.g. you don't want a user to subscribe once they have subscribed already. Or yo Emit a nullifier in your function. By adding this nullifier into the tree, you prevent another nullifier from being added again. This is also why in authwit, we emit a nullifier, to prevent someone from reusing their approval. -#include_code assert_valid_authwit_public /noir-projects/aztec-nr/authwit/src/auth.nr rust +#include_code spend_private_authwit /noir-projects/aztec-nr/authwit/src/account.nr rust Note be careful to ensure that the nullifier is not deterministic and that no one could do a preimage analysis attack. More in [the anti pattern section on deterministic nullifiers](#deterministic-nullifiers) diff --git a/docs/docs/developers/tutorials/uniswap/l2_contract_setup.md b/docs/docs/developers/tutorials/uniswap/l2_contract_setup.md index a7b54b17975a..26ff39e7c488 100644 --- a/docs/docs/developers/tutorials/uniswap/l2_contract_setup.md +++ b/docs/docs/developers/tutorials/uniswap/l2_contract_setup.md @@ -22,7 +22,7 @@ Next, paste this function: #include_code authwit_uniswap_get noir-projects/noir-contracts/contracts/uniswap_contract/src/main.nr rust -In this function, the token contract calls the Uniswap contract to check if Uniswap has indeed done the approval. The token contract expects a `is_valid()` function to exit for private approvals and `is_valid_public()` for public approvals. If the action is indeed approved, it expects that the contract would return the function selector for `is_valid()`  in both cases. The Aztec.nr library exposes this constant for ease of use. The token contract also emits a nullifier for this message so that this approval (with the nonce) can’t be used again. +In this function, the token contract calls the Uniswap contract to check if Uniswap has indeed done the approval. The token contract expects a `spend_private_authwit()` function to exit for private approvals and `spend_public_authwit()` for public approvals. If the action is indeed approved, it expects that the contract would return the function selector for `is_valid()`  in both cases. The Aztec.nr library exposes this constant for ease of use. The token contract also emits a nullifier for this message so that this approval (with the nonce) can’t be used again. This is similar to the [Authwit flow](../../contracts/resources/common_patterns/authwit.md). diff --git a/noir-projects/aztec-nr/authwit/src/account.nr b/noir-projects/aztec-nr/authwit/src/account.nr index 20816c6ae61b..4cedfe3571a8 100644 --- a/noir-projects/aztec-nr/authwit/src/account.nr +++ b/noir-projects/aztec-nr/authwit/src/account.nr @@ -1,8 +1,12 @@ use dep::aztec::context::{PrivateContext, PublicContext, Context}; use dep::aztec::state_vars::{Map, PublicMutable}; +use dep::aztec::protocol_types::{ + address::AztecAddress, abis::function_selector::FunctionSelector, + constants::{GENERATOR_INDEX__AUTHWIT}, hash::{pedersen_hash} +}; use crate::entrypoint::{app::AppPayload, fee::FeePayload}; -use crate::auth::IS_VALID_SELECTOR; +use crate::auth::{IS_VALID_SELECTOR, inner_compute_authwit_message_hash}; struct AccountActions { context: Context, @@ -69,21 +73,41 @@ impl AccountActions { } // docs:end:entrypoint - pub fn is_valid(self, message_hash: Field) -> Field { + // docs:start:spend_private_authwit + pub fn spend_private_authwit( + self, + caller: AztecAddress, + selector: FunctionSelector, + args_hash: Field + ) -> Field { + let context = self.context.private.unwrap(); + let message_hash = inner_compute_authwit_message_hash(caller, context.msg_sender(), selector, args_hash); let valid_fn = self.is_valid_impl; - if (valid_fn(self.context.private.unwrap(), message_hash)) { - IS_VALID_SELECTOR - } else { - 0 - } + assert(valid_fn(context, message_hash) == true, "Message not authorized by account"); + context.push_new_nullifier(message_hash, 0); + IS_VALID_SELECTOR } + // docs:end:spend_private_authwit - pub fn is_valid_public(self, message_hash: Field) -> Field { - let value = self.approved_action.at(message_hash).read(); - if (value) { IS_VALID_SELECTOR } else { 0 } + // docs:start:spend_public_authwit + pub fn spend_public_authwit( + self, + caller: AztecAddress, + selector: FunctionSelector, + args_hash: Field + ) -> Field { + let context = self.context.public.unwrap(); + let message_hash = inner_compute_authwit_message_hash(caller, context.msg_sender(), selector, args_hash); + let is_valid = self.approved_action.at(message_hash).read(); + assert(is_valid == true, "Message not authorized by account"); + context.push_new_nullifier(message_hash, 0); + IS_VALID_SELECTOR } + // docs:end:spend_public_authwit - pub fn internal_set_is_valid_storage(self, message_hash: Field, value: bool) { - self.approved_action.at(message_hash).write(value); + // docs:start:approve_public_authwit + pub fn approve_public_authwit(self, message_hash: Field) { + self.approved_action.at(message_hash).write(true); } + // docs:end:approve_public_authwit } diff --git a/noir-projects/aztec-nr/authwit/src/auth.nr b/noir-projects/aztec-nr/authwit/src/auth.nr index 34af8aa8bf91..052887feb4f2 100644 --- a/noir-projects/aztec-nr/authwit/src/auth.nr +++ b/noir-projects/aztec-nr/authwit/src/auth.nr @@ -5,59 +5,30 @@ use dep::aztec::protocol_types::{ use dep::aztec::context::{PrivateContext, PublicContext, Context}; global IS_VALID_SELECTOR = 0xe86ab4ff; -global IS_VALID_PUBLIC_SELECTOR = 0xf3661153; - -// @todo #2676 Should use different generator than the payload to limit probability of collisions. - -// docs:start:assert_valid_authwit -// Assert that `on_behalf_of` have authorized `message_hash` with a valid authentication witness -pub fn assert_valid_authwit( - context: &mut PrivateContext, - on_behalf_of: AztecAddress, - message_hash: Field -) { - let is_valid_selector = FunctionSelector::from_field(IS_VALID_SELECTOR); - let result = context.call_private_function(on_behalf_of, is_valid_selector, [message_hash])[0]; - context.push_new_nullifier(message_hash, 0); - assert(result == IS_VALID_SELECTOR, "Message not authorized by account"); -} -// docs:end:assert_valid_authwit // docs:start:assert_current_call_valid_authwit // Assert that `on_behalf_of` have authorized the current call with a valid authentication witness pub fn assert_current_call_valid_authwit(context: &mut PrivateContext, on_behalf_of: AztecAddress) { - // message_hash = H(caller, contract_this, selector, args_hash) - let message_hash = pedersen_hash( - [ - context.msg_sender().to_field(), context.this_address().to_field(), context.selector().to_field(), context.args_hash - ], - GENERATOR_INDEX__AUTHWIT - ); - assert_valid_authwit(context, on_behalf_of, message_hash); -} -// docs:end:assert_current_call_valid_authwit - -// docs:start:assert_valid_authwit_public -// Assert that `on_behalf_of` have authorized `message_hash` in a public context -pub fn assert_valid_authwit_public(context: &mut PublicContext, on_behalf_of: AztecAddress, message_hash: Field) { - let is_valid_public_selector = FunctionSelector::from_field(IS_VALID_PUBLIC_SELECTOR); - let result = context.call_public_function(on_behalf_of, is_valid_public_selector, [message_hash])[0]; - context.push_new_nullifier(message_hash, 0); + let function_selector = FunctionSelector::from_signature("spend_private_authwit((Field),(u32),Field)"); + let result = context.call_private_function( + on_behalf_of, + function_selector, + [context.msg_sender().to_field(), context.selector().to_field(), context.args_hash] + )[0]; assert(result == IS_VALID_SELECTOR, "Message not authorized by account"); } -// docs:end:assert_valid_authwit_public +// docs:end:assert_current_call_valid_authwit // docs:start:assert_current_call_valid_authwit_public // Assert that `on_behalf_of` have authorized the current call in a public context pub fn assert_current_call_valid_authwit_public(context: &mut PublicContext, on_behalf_of: AztecAddress) { - // message_hash = H(caller, contract_this, selector, args_hash) - let message_hash = pedersen_hash( - [ - context.msg_sender().to_field(), context.this_address().to_field(), context.selector().to_field(), context.args_hash - ], - GENERATOR_INDEX__AUTHWIT - ); - assert_valid_authwit_public(context, on_behalf_of, message_hash); + let function_selector = FunctionSelector::from_signature("spend_public_authwit((Field),(u32),Field)"); + let result = context.call_public_function( + on_behalf_of, + function_selector, + [context.msg_sender().to_field(), context.selector().to_field(), context.args_hash] + )[0]; + assert(result == IS_VALID_SELECTOR, "Message not authorized by account"); } // docs:end:assert_current_call_valid_authwit_public @@ -70,9 +41,18 @@ pub fn compute_authwit_message_hash( args: [Field; N] ) -> Field { let args_hash = hash_args(args); + inner_compute_authwit_message_hash(caller, target, selector, args_hash) +} +// docs:end:compute_authwit_message_hash + +pub fn inner_compute_authwit_message_hash( + caller: AztecAddress, + target: AztecAddress, + selector: FunctionSelector, + args_hash: Field +) -> Field { pedersen_hash( [caller.to_field(), target.to_field(), selector.to_field(), args_hash], GENERATOR_INDEX__AUTHWIT ) } -// docs:end:compute_authwit_message_hash diff --git a/noir-projects/noir-contracts/contracts/ecdsa_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/ecdsa_account_contract/src/main.nr index f9dac6425b8e..45010cf14c0d 100644 --- a/noir-projects/noir-contracts/contracts/ecdsa_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/ecdsa_account_contract/src/main.nr @@ -3,7 +3,7 @@ mod ecdsa_public_key_note; // Account contract that uses ECDSA signatures for authentication on the same curve as Ethereum. // The signing key is stored in an immutable private note and should be different from the signing key. contract EcdsaAccount { - use dep::aztec::protocol_types::{abis::call_context::CallContext, address::AztecAddress}; + use dep::aztec::protocol_types::{abis::{call_context::CallContext, function_selector::FunctionSelector}, address::AztecAddress}; use dep::std; use dep::std::option::Option; use dep::aztec::{ @@ -40,25 +40,30 @@ contract EcdsaAccount { } #[aztec(private)] - fn is_valid(message_hash: Field) -> Field { + fn spend_private_authwit(caller: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field { let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.is_valid(message_hash) + actions.spend_private_authwit(caller, selector, args_hash) } #[aztec(public)] - fn is_valid_public(message_hash: Field) -> Field { + fn spend_public_authwit(caller: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field { let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.is_valid_public(message_hash) + actions.spend_public_authwit(caller, selector, args_hash) + } + + #[aztec(private)] + internal fn cancel_authwit(message_hash: Field) { + context.push_new_nullifier(message_hash, 0); } #[aztec(public)] - internal fn set_is_valid_storage(message_hash: Field, value: bool) { + internal fn approve_public_authwit(message_hash: Field) { let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.internal_set_is_valid_storage(message_hash, value) + actions.approve_public_authwit(message_hash) } #[contract_library_method] - fn is_valid_impl(context: &mut PrivateContext, message_field: Field) -> pub bool { + fn is_valid_impl(context: &mut PrivateContext, message_field: Field) -> bool { // Load public key from storage let storage = Storage::init(Context::private(context)); let public_key = storage.public_key.get_note(); diff --git a/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr index 6e4eec425c99..ff6c66905c52 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_account_contract/src/main.nr @@ -6,7 +6,7 @@ contract SchnorrAccount { use dep::std; use dep::std::option::Option; - use dep::aztec::protocol_types::address::AztecAddress; + use dep::aztec::protocol_types::{address::AztecAddress, abis::function_selector::FunctionSelector}; use dep::aztec::{ context::{PrivateContext, Context}, note::{note_header::NoteHeader, utils as note_utils}, @@ -45,25 +45,34 @@ contract SchnorrAccount { } #[aztec(private)] - fn is_valid(message_hash: Field) -> Field { + fn spend_private_authwit( + caller: AztecAddress, + selector: FunctionSelector, + args_hash: Field + ) -> Field { let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.is_valid(message_hash) + actions.spend_private_authwit(caller, selector, args_hash) } #[aztec(public)] - fn is_valid_public(message_hash: Field) -> Field { + fn spend_public_authwit(caller: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field { let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.is_valid_public(message_hash) + actions.spend_public_authwit(caller, selector, args_hash) + } + + #[aztec(private)] + internal fn cancel_authwit(message_hash: Field) { + context.push_new_nullifier(message_hash, 0); } #[aztec(public)] - internal fn set_is_valid_storage(message_hash: Field, value: bool) { + internal fn approve_public_authwit(message_hash: Field) { let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.internal_set_is_valid_storage(message_hash, value) + actions.approve_public_authwit(message_hash) } #[contract_library_method] - fn is_valid_impl(context: &mut PrivateContext, message_hash: Field) -> pub bool { + fn is_valid_impl(context: &mut PrivateContext, message_hash: Field) -> bool { // docs:start:entrypoint // Load public key from storage let storage = Storage::init(Context::private(context)); diff --git a/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr index 88f6b7cf5e73..2c3557d3d77c 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_hardcoded_account_contract/src/main.nr @@ -2,7 +2,10 @@ // Account contract that uses Schnorr signatures for authentication using a hardcoded public key. contract SchnorrHardcodedAccount { use dep::std; - use dep::aztec::{context::PrivateContext, protocol_types::address::AztecAddress}; + use dep::aztec::{ + context::PrivateContext, + protocol_types::{address::AztecAddress, abis::function_selector::FunctionSelector} + }; use dep::authwit::{ entrypoint::{app::AppPayload, fee::FeePayload}, account::AccountActions, @@ -25,26 +28,31 @@ contract SchnorrHardcodedAccount { } #[aztec(private)] - fn is_valid(message_hash: Field) -> Field { + fn spend_private_authwit(caller: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field { let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.is_valid(message_hash) + actions.spend_private_authwit(caller, selector, args_hash) } #[aztec(public)] - fn is_valid_public(message_hash: Field) -> Field { + fn spend_public_authwit(caller: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field { let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.is_valid_public(message_hash) + actions.spend_public_authwit(caller, selector, args_hash) + } + + #[aztec(private)] + internal fn cancel_authwit(message_hash: Field) { + context.push_new_nullifier(message_hash, 0); } #[aztec(public)] - internal fn set_is_valid_storage(message_hash: Field, value: bool) { + internal fn approve_public_authwit(message_hash: Field) { let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.internal_set_is_valid_storage(message_hash, value) + actions.approve_public_authwit(message_hash) } // docs:start:is-valid #[contract_library_method] - fn is_valid_impl(_context: &mut PrivateContext, message_hash: Field) -> pub bool { + fn is_valid_impl(_context: &mut PrivateContext, message_hash: Field) -> bool { // Load auth witness and format as an u8 array let witness: [Field; 64] = get_auth_witness(message_hash); let mut signature: [u8; 64] = [0; 64]; diff --git a/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr b/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr index b5633b70009d..843645ed8006 100644 --- a/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/schnorr_single_key_account_contract/src/main.nr @@ -2,12 +2,15 @@ mod util; mod auth_oracle; contract SchnorrSingleKeyAccount { - use dep::std::{option::Option}; - use dep::aztec::{context::{PrivateContext, PublicContext, Context}, protocol_types::address::AztecAddress}; + use dep::aztec::{ + context::{PrivateContext}, + protocol_types::{address::AztecAddress, abis::function_selector::FunctionSelector} + }; use dep::authwit::{entrypoint::{app::AppPayload, fee::FeePayload}, account::AccountActions}; use crate::{util::recover_address, auth_oracle::get_auth_witness}; + use dep::aztec::oracle::debug_log::debug_log_format; global ACCOUNT_ACTIONS_STORAGE_SLOT = 1; @@ -20,27 +23,31 @@ contract SchnorrSingleKeyAccount { let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); actions.entrypoint(app_payload, fee_payload); } - #[aztec(private)] - fn is_valid(message_hash: Field) -> Field { + fn spend_private_authwit(caller: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field { let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.is_valid(message_hash) + actions.spend_private_authwit(caller, selector, args_hash) } #[aztec(public)] - fn is_valid_public(message_hash: Field) -> Field { + fn spend_public_authwit(caller: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field { let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.is_valid_public(message_hash) + actions.spend_public_authwit(caller, selector, args_hash) + } + + #[aztec(private)] + internal fn cancel_authwit(message_hash: Field) { + context.push_new_nullifier(message_hash, 0); } #[aztec(public)] - internal fn set_is_valid_storage(message_hash: Field, value: bool) { + internal fn approve_public_authwit(message_hash: Field) { let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); - actions.internal_set_is_valid_storage(message_hash, value) + actions.approve_public_authwit(message_hash) } #[contract_library_method] - fn is_valid_impl(context: &mut PrivateContext, message_hash: Field) -> pub bool { + fn is_valid_impl(context: &mut PrivateContext, message_hash: Field) -> bool { let witness = get_auth_witness(message_hash); assert(recover_address(message_hash, witness).eq(context.this_address())); true diff --git a/noir-projects/noir-contracts/contracts/uniswap_contract/src/main.nr b/noir-projects/noir-contracts/contracts/uniswap_contract/src/main.nr index f92dddff2f28..1f806e6f2d43 100644 --- a/noir-projects/noir-contracts/contracts/uniswap_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/uniswap_contract/src/main.nr @@ -9,7 +9,10 @@ contract Uniswap { use dep::aztec::protocol_types::{abis::function_selector::FunctionSelector, address::{AztecAddress, EthAddress}}; use dep::aztec::{oracle::{context::get_portal_address}, state_vars::{Map, PublicMutable}}; - use dep::authwit::auth::{IS_VALID_SELECTOR, assert_current_call_valid_authwit_public, compute_authwit_message_hash}; + use dep::authwit::auth::{ + IS_VALID_SELECTOR, assert_current_call_valid_authwit_public, compute_authwit_message_hash, + inner_compute_authwit_message_hash + }; use crate::interfaces::{Token, TokenBridge}; use crate::util::{compute_swap_private_content_hash, compute_swap_public_content_hash}; @@ -173,9 +176,15 @@ contract Uniswap { // implementation is similar to how account contracts validate public approvals. // if valid, it returns the IS_VALID selector which is expected by token contract #[aztec(public)] - fn is_valid_public(message_hash: Field) -> Field { + fn spend_public_authwit(token_bridge: AztecAddress, selector: FunctionSelector, args_hash: Field) -> Field { + let message_hash = inner_compute_authwit_message_hash(token_bridge, context.msg_sender(), selector, args_hash); let value = storage.approved_action.at(message_hash).read(); - if (value) { IS_VALID_SELECTOR } else { 0 } + if (value) { + context.push_new_nullifier(message_hash, 0); + IS_VALID_SELECTOR + } else { + 0 + } } // docs:end:authwit_uniswap_get diff --git a/noir/aztec_macros/src/lib.rs b/noir/aztec_macros/src/lib.rs index 156ba1d5b08b..807615e1d482 100644 --- a/noir/aztec_macros/src/lib.rs +++ b/noir/aztec_macros/src/lib.rs @@ -648,6 +648,9 @@ fn transform_function( // Abstract return types such that they get added to the kernel's return_values if let Some(return_values) = abstract_return_values(func) { + // Delete the old return statement + func.def.body.0.pop(); + // Add the new return statement func.def.body.0.push(return_values); } diff --git a/yarn-project/aztec.js/src/wallet/account_wallet.ts b/yarn-project/aztec.js/src/wallet/account_wallet.ts index 428d4b46fd08..b8db6836ff49 100644 --- a/yarn-project/aztec.js/src/wallet/account_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/account_wallet.ts @@ -33,8 +33,21 @@ export class AccountWallet extends BaseWallet { * @returns - A function interaction. */ public setPublicAuth(message: Fr | Buffer, authorized: boolean): ContractFunctionInteraction { - const args = [message, authorized]; - return new ContractFunctionInteraction(this, this.getAddress(), this.getSetIsValidStorageAbi(), args); + if (authorized) { + return new ContractFunctionInteraction(this, this.getAddress(), this.getApprovePublicAuthwitAbi(), [message]); + } else { + return this.cancelAuthWit(message); + } + } + + /** + * Returns a function interaction to cancel a message hash as authorized in this account. + * @param message - Message hash to authorize. + * @returns - A function interaction. + */ + public cancelAuthWit(message: Fr | Buffer): ContractFunctionInteraction { + const args = [message]; + return new ContractFunctionInteraction(this, this.getAddress(), this.getCancelAuthwitAbi(), args); } /** Returns the complete address of the account that implements this wallet. */ @@ -47,10 +60,10 @@ export class AccountWallet extends BaseWallet { return this.getCompleteAddress().address; } - private getSetIsValidStorageAbi(): FunctionAbi { + private getApprovePublicAuthwitAbi(): FunctionAbi { return { - name: 'set_is_valid_storage', - functionType: 'open' as FunctionType, + name: 'approve_public_authwit', + functionType: FunctionType.OPEN, isInternal: true, parameters: [ { @@ -58,9 +71,20 @@ export class AccountWallet extends BaseWallet { type: { kind: 'field' }, visibility: 'private' as ABIParameterVisibility, }, + ], + returnTypes: [], + }; + } + + private getCancelAuthwitAbi(): FunctionAbi { + return { + name: 'cancel_authwit', + functionType: FunctionType.SECRET, + isInternal: true, + parameters: [ { - name: 'value', - type: { kind: 'boolean' }, + name: 'message_hash', + type: { kind: 'field' }, visibility: 'private' as ABIParameterVisibility, }, ], diff --git a/yarn-project/end-to-end/src/e2e_authwit_token_contract.test.ts b/yarn-project/end-to-end/src/e2e_authwit_token_contract.test.ts new file mode 100644 index 000000000000..6b7fbaad7cd6 --- /dev/null +++ b/yarn-project/end-to-end/src/e2e_authwit_token_contract.test.ts @@ -0,0 +1,33 @@ +import { AccountWallet, DebugLogger, Fr, FunctionSelector } from '@aztec/aztec.js'; +import { decodeFunctionSignature } from '@aztec/foundation/abi'; +import { SchnorrAccountContract } from '@aztec/noir-contracts.js'; + +import { jest } from '@jest/globals'; + +import { setup } from './fixtures/utils.js'; + +const TIMEOUT = 90_000; + +describe('e2e_token_contract', () => { + jest.setTimeout(TIMEOUT); + + let wallets: AccountWallet[]; + let logger: DebugLogger; + + beforeAll(async () => { + ({ logger, wallets } = await setup(1)); + }, 100_000); + + it('Testing shit', async () => { + const address = wallets[0].getAddress(); + const w = await SchnorrAccountContract.at(address, wallets[0]); + // await w.methods.spend_private_authwit( new Fr(420)).send().wait(); + + SchnorrAccountContract.artifact.functions.forEach(fn => { + const sig = decodeFunctionSignature(fn.name, fn.parameters); + logger(`Function ${sig} and the selector: ${FunctionSelector.fromNameAndParameters(fn.name, fn.parameters)}`); + }); + }); + + describe('failure cases', () => {}); +}); diff --git a/yarn-project/end-to-end/src/e2e_token_contract.test.ts b/yarn-project/end-to-end/src/e2e_token_contract.test.ts index f8cfb7db8ef2..99bd9f51c4bf 100644 --- a/yarn-project/end-to-end/src/e2e_token_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_token_contract.test.ts @@ -488,6 +488,29 @@ describe('e2e_token_contract', () => { expect(await asset.methods.balance_of_public(accounts[1].address).view()).toEqual(balance1); }); + it('transfer on behalf of other, cancelled authwit', async () => { + const balance0 = await asset.methods.balance_of_public(accounts[0].address).view(); + const amount = balance0 / 2n; + expect(amount).toBeGreaterThan(0n); + const nonce = Fr.random(); + + const action = asset + .withWallet(wallets[1]) + .methods.transfer_public(accounts[0].address, accounts[1].address, amount, nonce); + const messageHash = computeAuthWitMessageHash(accounts[1].address, action.request()); + + await wallets[0].setPublicAuth(messageHash, true).send().wait(); + + await wallets[0].cancelAuthWit(messageHash).send().wait(); + + // Check that the message hash is no longer valid. Need to try to send since nullifiers are handled by sequencer. + const txCancelledAuthwit = asset + .withWallet(wallets[1]) + .methods.transfer_public(accounts[0].address, accounts[1].address, amount, nonce) + .send(); + await expect(txCancelledAuthwit.wait()).rejects.toThrowError('Transaction '); + }); + it.skip('transfer into account to overflow', () => { // This should already be covered by the mint case earlier. e.g., since we cannot mint to overflow, there is not // a way to get funds enough to overflow. @@ -640,6 +663,31 @@ describe('e2e_token_contract', () => { ); expect(await asset.methods.balance_of_private(accounts[0].address).view()).toEqual(balance0); }); + + it('transfer on behalf of other, cancelled approval', async () => { + const balance0 = await asset.methods.balance_of_private(accounts[0].address).view(); + const amount = balance0 / 2n; + const nonce = Fr.random(); + expect(amount).toBeGreaterThan(0n); + + // We need to compute the message we want to sign and add it to the wallet as approved + const action = asset + .withWallet(wallets[1]) + .methods.transfer(accounts[0].address, accounts[1].address, amount, nonce); + const messageHash = computeAuthWitMessageHash(accounts[1].address, action.request()); + + const witness = await wallets[0].createAuthWitness(messageHash); + await wallets[1].addAuthWitness(witness); + + await wallets[0].cancelAuthWit(messageHash).send().wait(); + + // Perform the transfer, should fail because nullifier already emitted + const txCancelledAuthwit = asset + .withWallet(wallets[1]) + .methods.transfer(accounts[0].address, accounts[1].address, amount, nonce) + .send(); + await expect(txCancelledAuthwit.wait()).rejects.toThrowError('Transaction '); + }); }); }); }); diff --git a/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts b/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts index 5a27cd0d53a2..704fb0532f07 100644 --- a/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts +++ b/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts @@ -588,7 +588,7 @@ export const uniswapL1L2TestSuite = ( // Swap! await expect(action.simulate()).rejects.toThrowError( - "Assertion failed: Message not authorized by account 'result == IS_VALID_SELECTOR'", + "Assertion failed: Message not authorized by account 'is_valid == true'", ); }); @@ -622,7 +622,7 @@ export const uniswapL1L2TestSuite = ( Fr.ZERO, ) .simulate(), - ).rejects.toThrowError(`Assertion failed: Message not authorized by account 'result == IS_VALID_SELECTOR'`); + ).rejects.toThrowError(`Assertion failed: Message not authorized by account 'is_valid == true'`); }); // tests when trying to mix private and public flows: diff --git a/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts b/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts index 980f5de1e8fb..33fd74e3669d 100644 --- a/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts +++ b/yarn-project/sequencer-client/src/sequencer/abstract_phase_manager.ts @@ -230,7 +230,7 @@ export abstract class AbstractPhaseManager { newUnencryptedFunctionLogs.push(result.unencryptedLogs); const functionSelector = result.execution.functionData.selector.toString(); this.log.debug( - `Running public kernel circuit for ${functionSelector}@${result.execution.contractAddress.toString()}`, + `Running public kernel circuit for ${result.execution.contractAddress.toString()}:${functionSelector}`, ); executionStack.push(...result.nestedExecutions); const callData = await this.getPublicCallData(result, isExecutionRequest); diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.ts index 99189c376a1b..1cacdd2d5e86 100644 --- a/yarn-project/sequencer-client/src/sequencer/public_processor.ts +++ b/yarn-project/sequencer-client/src/sequencer/public_processor.ts @@ -125,7 +125,7 @@ export class PublicProcessor { const processedTransaction = makeProcessedTx(tx, publicKernelPublicInput, publicKernelProof); result.push(processedTransaction); - this.log(`Processed public part of ${tx.data.endNonRevertibleData.newNullifiers[0]}`, { + this.log(`Processed public part of ${tx.data.endNonRevertibleData.newNullifiers[0].value}`, { eventName: 'tx-sequencer-processing', duration: timer.ms(), publicDataUpdateRequests: