From 7c24870a0a446b8725ca4e8ac46b1004bea6c8f3 Mon Sep 17 00:00:00 2001 From: Lasse Herskind <16536249+LHerskind@users.noreply.github.com> Date: Thu, 21 Mar 2024 12:09:13 +0000 Subject: [PATCH] feat(Authwit): lookup the validity of authwits (#5316) Fixes #4822. The current solution is pretty ugly (I think). The issues encountered is mainly that: - There are no context so we need to pass stuff that seems quite redundant (address of the contract itself and block number) - Fetching witness data with oracles explodes if you try to fetch something that does not exists. - The implementation here is a lookup from "outside" a transaction. The latter is the main trouble. It can be somewhat mitigated by having a function on the typescript side of things that try doing the lookup in its local storage first to not encounter that case, but then we need to be able to split the check so seems a bit meh to fix it that way. This means that the unconstrained cannot really be used nicely as what @spalladino did at the offsite, but mainly is an off-chain thing for the user who is unsure if something was approved and want to check it. I'm not really sure if I like to have the implementation this way where we are doing things in both ts and noir to get it somewhat working, but seems like the thing that is making it the easiest to get a query going fast. --- noir-projects/aztec-nr/aztec/src/hash.nr | 9 +- .../schnorr_account_contract/src/main.nr | 81 +++++++++++++-- .../aztec.js/src/wallet/account_wallet.ts | 98 ++++++++++++++++--- .../aztec.js/src/wallet/base_wallet.ts | 3 + .../circuit-types/src/interfaces/pxe.ts | 7 ++ .../end-to-end/src/e2e_authwit.test.ts | 67 +++++++++++++ .../end-to-end/src/e2e_token_contract.test.ts | 20 ++++ .../pxe/src/pxe_service/pxe_service.ts | 4 + 8 files changed, 267 insertions(+), 22 deletions(-) diff --git a/noir-projects/aztec-nr/aztec/src/hash.nr b/noir-projects/aztec-nr/aztec/src/hash.nr index d59bdd9a7c3..05234149c8a 100644 --- a/noir-projects/aztec-nr/aztec/src/hash.nr +++ b/noir-projects/aztec-nr/aztec/src/hash.nr @@ -1,6 +1,13 @@ -use dep::protocol_types::{constants::GENERATOR_INDEX__L1_TO_L2_MESSAGE_SECRET, hash::pedersen_hash}; +use dep::protocol_types::{ + address::AztecAddress, + constants::GENERATOR_INDEX__L1_TO_L2_MESSAGE_SECRET, + hash::{pedersen_hash, silo_nullifier}}; pub fn compute_secret_hash(secret: Field) -> Field { // TODO(#1205) This is probably not the right index to use pedersen_hash([secret], GENERATOR_INDEX__L1_TO_L2_MESSAGE_SECRET) } + +pub fn compute_siloed_nullifier(address: AztecAddress, nullifier: Field) -> Field { + silo_nullifier(address, nullifier) +} \ No newline at end of file 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 0f5355b1978..b2bd8351604 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,12 +6,14 @@ contract SchnorrAccount { use dep::std; use dep::aztec::prelude::{AztecAddress, FunctionSelector, NoteHeader, PrivateContext, PrivateImmutable}; - + use dep::aztec::state_vars::{Map, PublicMutable}; use dep::aztec::{context::Context, oracle::get_public_key::get_public_key}; use dep::authwit::{ entrypoint::{app::AppPayload, fee::FeePayload}, account::AccountActions, auth_witness::get_auth_witness }; + use dep::aztec::hash::compute_siloed_nullifier; + use dep::aztec::oracle::get_nullifier_membership_witness::get_low_nullifier_membership_witness; use crate::public_key_note::{PublicKeyNote, PUBLIC_KEY_NOTE_LEN}; @@ -19,10 +21,9 @@ contract SchnorrAccount { // docs:start:storage signing_public_key: PrivateImmutable, // docs:end:storage + approved_actions: Map>, } - global ACCOUNT_ACTIONS_STORAGE_SLOT = 2; - // Constructs the contract #[aztec(private)] #[aztec(initializer)] @@ -37,19 +38,31 @@ contract SchnorrAccount { // Note: If you globally change the entrypoint signature don't forget to update default_entrypoint.ts file #[aztec(private)] fn entrypoint(app_payload: pub AppPayload, fee_payload: pub FeePayload) { - let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); + let actions = AccountActions::private( + &mut context, + storage.approved_actions.storage_slot, + is_valid_impl + ); actions.entrypoint(app_payload, fee_payload); } #[aztec(private)] fn spend_private_authwit(inner_hash: Field) -> Field { - let actions = AccountActions::private(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); + let actions = AccountActions::private( + &mut context, + storage.approved_actions.storage_slot, + is_valid_impl + ); actions.spend_private_authwit(inner_hash) } #[aztec(public)] fn spend_public_authwit(inner_hash: Field) -> Field { - let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); + let actions = AccountActions::public( + &mut context, + storage.approved_actions.storage_slot, + is_valid_impl + ); actions.spend_public_authwit(inner_hash) } @@ -62,7 +75,11 @@ contract SchnorrAccount { #[aztec(public)] #[aztec(internal)] fn approve_public_authwit(outer_hash: Field) { - let actions = AccountActions::public(&mut context, ACCOUNT_ACTIONS_STORAGE_SLOT, is_valid_impl); + let actions = AccountActions::public( + &mut context, + storage.approved_actions.storage_slot, + is_valid_impl + ); actions.approve_public_authwit(outer_hash) } @@ -92,4 +109,54 @@ contract SchnorrAccount { // docs:end:entrypoint true } + + /** + * @notice Helper function to check the existing and validity of authwitnesses + * @dev TODO: myself and block_number should be removed and passed from a context + * @param myself The address of the contract + * @param block_number The block number to check the nullifier against + * @param check_private Whether to check the validity of the authwitness in private state or not + * @param message_hash The message hash of the message to check the validity + * @return An array of two booleans, the first is the validity of the authwitness in the private state, + * the second is the validity of the authwitness in the public state + * Both values will be `false` if the nullifier is spent + */ + unconstrained fn lookup_validity( + myself: AztecAddress, + block_number: u32, + check_private: bool, + message_hash: Field + ) -> pub [bool; 2] { + let valid_in_private = if check_private { + let public_key = storage.signing_public_key.view_note(); + let witness: [Field; 64] = get_auth_witness(message_hash); + let mut signature: [u8; 64] = [0; 64]; + for i in 0..64 { + signature[i] = witness[i] as u8; + } + std::schnorr::verify_signature( + public_key.x, + public_key.y, + signature, + message_hash.to_be_bytes(32) + ) + } else { + false + }; + + let valid_in_public = storage.approved_actions.at(message_hash).read(); + + // Compute the nullifier and check if it is spent + // This will BLINDLY TRUST the oracle, but the oracle is us, and + // it is not as part of execution of the contract, so we are good. + let siloed_nullifier = compute_siloed_nullifier(myself, message_hash); + let lower_wit = get_low_nullifier_membership_witness(block_number, siloed_nullifier); + let is_spent = lower_wit.leaf_preimage.nullifier == siloed_nullifier; + + if is_spent { + [false, false] + } else { + [valid_in_private, valid_in_public] + } + } } diff --git a/yarn-project/aztec.js/src/wallet/account_wallet.ts b/yarn-project/aztec.js/src/wallet/account_wallet.ts index 10383dcea4c..9c388f5b953 100644 --- a/yarn-project/aztec.js/src/wallet/account_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/account_wallet.ts @@ -43,6 +43,33 @@ export class AccountWallet extends BaseWallet { return witness; } + /** + * Returns a function interaction to set a message hash as authorized or revoked in this account. + * Public calls can then consume this authorization. + * @param messageHashOrIntent - The message or the caller and action to authorize/revoke + * @param authorized - True to authorize, false to revoke authorization. + * @returns - A function interaction. + */ + public setPublicAuthWit( + messageHashOrIntent: + | Fr + | Buffer + | { + /** The caller to approve */ + caller: AztecAddress; + /** The action to approve */ + action: ContractFunctionInteraction | FunctionCall; + }, + authorized: boolean, + ): ContractFunctionInteraction { + const message = this.getMessageHash(messageHashOrIntent); + if (authorized) { + return new ContractFunctionInteraction(this, this.getAddress(), this.getApprovePublicAuthwitAbi(), [message]); + } else { + return this.cancelAuthWit(message); + } + } + /** * Returns the message hash for the given message or authwit input. * @param messageHashOrIntent - The message hash or the caller and action to authorize @@ -70,13 +97,14 @@ export class AccountWallet extends BaseWallet { } /** - * Returns a function interaction to set a message hash as authorized or revoked in this account. - * Public calls can then consume this authorization. - * @param messageHashOrIntent - The message or the caller and action to authorize/revoke - * @param authorized - True to authorize, false to revoke authorization. - * @returns - A function interaction. + * Lookup the validity of an authwit in private and public contexts. + * If the authwit have been consumed already (nullifier spent), will return false in both contexts. + * @param target - The target contract address + * @param messageHashOrIntent - The message hash or the caller and action to authorize/revoke + * @returns - A struct containing the validity of the authwit in private and public contexts. */ - public setPublicAuthWit( + async lookupValidity( + target: AztecAddress, messageHashOrIntent: | Fr | Buffer @@ -86,14 +114,24 @@ export class AccountWallet extends BaseWallet { /** The action to approve */ action: ContractFunctionInteraction | FunctionCall; }, - authorized: boolean, - ): ContractFunctionInteraction { - const message = this.getMessageHash(messageHashOrIntent); - if (authorized) { - return new ContractFunctionInteraction(this, this.getAddress(), this.getApprovePublicAuthwitAbi(), [message]); - } else { - return this.cancelAuthWit(message); - } + ): Promise<{ + /** boolean flag indicating if the authwit is valid in private context */ + isValidInPrivate: boolean; + /** boolean flag indicating if the authwit is valid in public context */ + isValidInPublic: boolean; + }> { + const messageHash = this.getMessageHash(messageHashOrIntent); + const witness = await this.getAuthWitness(messageHash); + const blockNumber = await this.getBlockNumber(); + const interaction = new ContractFunctionInteraction(this, target, this.getLookupValidityAbi(), [ + target, + blockNumber, + witness != undefined, + messageHash, + ]); + + const [isValidInPrivate, isValidInPublic] = await interaction.view(); + return { isValidInPrivate, isValidInPublic }; } /** @@ -160,4 +198,36 @@ export class AccountWallet extends BaseWallet { returnTypes: [], }; } + + private getLookupValidityAbi(): FunctionAbi { + return { + name: 'lookup_validity', + isInitializer: false, + functionType: FunctionType.UNCONSTRAINED, + isInternal: false, + parameters: [ + { + name: 'myself', + type: { + kind: 'struct', + path: 'authwit::aztec::protocol_types::address::aztec_address::AztecAddress', + fields: [{ name: 'inner', type: { kind: 'field' } }], + }, + visibility: 'private' as ABIParameterVisibility, + }, + { + name: 'block_number', + type: { kind: 'integer', sign: 'unsigned', width: 32 }, + visibility: 'private' as ABIParameterVisibility, + }, + { + name: 'check_private', + type: { kind: 'boolean' }, + visibility: 'private' as ABIParameterVisibility, + }, + { name: 'message_hash', type: { kind: 'field' }, visibility: 'private' as ABIParameterVisibility }, + ], + returnTypes: [{ kind: 'array', length: 2, type: { kind: 'boolean' } }], + }; + } } diff --git a/yarn-project/aztec.js/src/wallet/base_wallet.ts b/yarn-project/aztec.js/src/wallet/base_wallet.ts index 1df06b661e9..6ce1d9fa34d 100644 --- a/yarn-project/aztec.js/src/wallet/base_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/base_wallet.ts @@ -139,6 +139,9 @@ export abstract class BaseWallet implements Wallet { addAuthWitness(authWitness: AuthWitness) { return this.pxe.addAuthWitness(authWitness); } + getAuthWitness(messageHash: Fr) { + return this.pxe.getAuthWitness(messageHash); + } isContractClassPubliclyRegistered(id: Fr): Promise { return this.pxe.isContractClassPubliclyRegistered(id); } diff --git a/yarn-project/circuit-types/src/interfaces/pxe.ts b/yarn-project/circuit-types/src/interfaces/pxe.ts index 94cc7e3a712..384ccdb1437 100644 --- a/yarn-project/circuit-types/src/interfaces/pxe.ts +++ b/yarn-project/circuit-types/src/interfaces/pxe.ts @@ -35,6 +35,13 @@ export interface PXE { */ addAuthWitness(authWitness: AuthWitness): Promise; + /** + * Fetches the serialized auth witness for a given message hash or returns undefined if not found. + * @param messageHash - The hash of the message for which to get the auth witness. + * @returns The serialized auth witness for the given message hash. + */ + getAuthWitness(messageHash: Fr): Promise; + /** * Adding a capsule to the capsule dispenser. * @param capsule - An array of field elements representing the capsule. diff --git a/yarn-project/end-to-end/src/e2e_authwit.test.ts b/yarn-project/end-to-end/src/e2e_authwit.test.ts index 8b9d57f5ab4..d8487288bef 100644 --- a/yarn-project/end-to-end/src/e2e_authwit.test.ts +++ b/yarn-project/end-to-end/src/e2e_authwit.test.ts @@ -27,8 +27,25 @@ describe('e2e_authwit_tests', () => { const witness = await wallets[0].createAuthWit(outerHash); await wallets[1].addAuthWitness(witness); + // Check that the authwit is valid in private for wallets[0] + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: true, + isValidInPublic: false, + }); + + // Check that the authwit is NOT valid in private for wallets[1] + expect(await wallets[0].lookupValidity(wallets[1].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + const c = await SchnorrAccountContract.at(wallets[0].getAddress(), wallets[0]); await c.withWallet(wallets[1]).methods.spend_private_authwit(innerHash).send().wait(); + + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); }); describe('failure case', () => { @@ -36,12 +53,32 @@ describe('e2e_authwit_tests', () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0xbeef')]); const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + const witness = await wallets[0].createAuthWit(outerHash); await wallets[1].addAuthWitness(witness); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: true, + isValidInPublic: false, + }); await wallets[0].cancelAuthWit(outerHash).send().wait(); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + const c = await SchnorrAccountContract.at(wallets[0].getAddress(), wallets[0]); const txCancelledAuthwit = c.withWallet(wallets[1]).methods.spend_private_authwit(innerHash).send(); + + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + // The transaction should be dropped because of a cancelled authwit (duplicate nullifier) await expect(txCancelledAuthwit.wait()).rejects.toThrow('Transaction '); }); @@ -55,10 +92,25 @@ describe('e2e_authwit_tests', () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0x01')]); const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + await wallets[0].setPublicAuthWit(outerHash, true).send().wait(); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: true, + }); + const c = await SchnorrAccountContract.at(wallets[0].getAddress(), wallets[0]); await c.withWallet(wallets[1]).methods.spend_public_authwit(innerHash).send().wait(); + + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); }); describe('failure case', () => { @@ -66,10 +118,25 @@ describe('e2e_authwit_tests', () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0x02')]); const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + await wallets[0].setPublicAuthWit(outerHash, true).send().wait(); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: true, + }); + await wallets[0].cancelAuthWit(outerHash).send().wait(); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + const c = await SchnorrAccountContract.at(wallets[0].getAddress(), wallets[0]); const txCancelledAuthwit = c.withWallet(wallets[1]).methods.spend_public_authwit(innerHash).send(); // The transaction should be dropped because of a cancelled authwit (duplicate nullifier) 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 04420c05075..a64490a6571 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 @@ -395,9 +395,23 @@ describe('e2e_token_contract', () => { .withWallet(wallets[1]) .methods.transfer_public(accounts[0].address, accounts[1].address, amount, nonce); + expect( + await wallets[0].lookupValidity(wallets[0].getAddress(), { caller: accounts[1].address, action }), + ).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + // We need to compute the message we want to sign and add it to the wallet as approved await wallets[0].setPublicAuthWit({ caller: accounts[1].address, action }, true).send().wait(); + expect( + await wallets[0].lookupValidity(wallets[0].getAddress(), { caller: accounts[1].address, action }), + ).toEqual({ + isValidInPrivate: false, + isValidInPublic: true, + }); + // Perform the transfer await expect(action.simulate()).rejects.toThrow(U128_UNDERFLOW_ERROR); @@ -567,6 +581,12 @@ describe('e2e_token_contract', () => { const witness = await wallets[0].createAuthWit({ caller: accounts[1].address, action }); await wallets[1].addAuthWitness(witness); + expect( + await wallets[0].lookupValidity(wallets[0].getAddress(), { caller: accounts[1].address, action }), + ).toEqual({ + isValidInPrivate: true, + isValidInPublic: false, + }); // docs:end:authwit_transfer_example // Perform the transfer diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 1f25b401f07..de2c170513a 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -147,6 +147,10 @@ export class PXEService implements PXE { return this.db.addAuthWitness(witness.requestHash, witness.witness); } + public getAuthWitness(messageHash: Fr): Promise { + return this.db.getAuthWitness(messageHash); + } + public addCapsule(capsule: Fr[]) { return this.db.addCapsule(capsule); }