From bc67592ea59a8cc37352847f5485414eac79b4ca Mon Sep 17 00:00:00 2001 From: LHerskind Date: Wed, 20 Mar 2024 12:38:42 +0000 Subject: [PATCH 1/3] feat(AuthWit): chain_id and version in hash --- noir-projects/aztec-nr/authwit/src/account.nr | 4 +- noir-projects/aztec-nr/authwit/src/auth.nr | 25 ++++++-- .../contracts/uniswap_contract/src/main.nr | 4 +- .../src/defaults/account_interface.ts | 12 ++++ .../src/single_key/account_contract.ts | 15 ++++- .../aztec.js/src/account/interface.ts | 12 ++++ .../src/fee/private_fee_payment_method.ts | 15 +++-- .../src/fee/public_fee_payment_method.ts | 22 ++++--- yarn-project/aztec.js/src/utils/authwit.ts | 16 +++-- .../aztec.js/src/wallet/account_wallet.ts | 40 ++++++++++++- .../aztec.js/src/wallet/base_wallet.ts | 8 +++ yarn-project/aztec.js/src/wallet/index.ts | 1 + .../aztec.js/src/wallet/signerless_wallet.ts | 8 +++ .../end-to-end/src/e2e_authwit.test.ts | 59 +++++++++++++++++-- .../src/e2e_blacklist_token_contract.test.ts | 35 +++++++++-- .../src/e2e_cross_chain_messaging.test.ts | 2 + .../src/e2e_crowdfunding_and_claim.test.ts | 4 +- yarn-project/end-to-end/src/e2e_fees.test.ts | 42 ++++++++----- .../src/e2e_lending_contract.test.ts | 2 + .../e2e_public_cross_chain_messaging.test.ts | 2 + .../end-to-end/src/e2e_token_contract.test.ts | 43 +++++++++++--- .../end-to-end/src/shared/uniswap_l1_l2.ts | 26 +++++++- .../entrypoints/src/dapp_entrypoint.ts | 7 ++- 23 files changed, 335 insertions(+), 69 deletions(-) diff --git a/noir-projects/aztec-nr/authwit/src/account.nr b/noir-projects/aztec-nr/authwit/src/account.nr index 2a25420feaf..b8a62fb6653 100644 --- a/noir-projects/aztec-nr/authwit/src/account.nr +++ b/noir-projects/aztec-nr/authwit/src/account.nr @@ -76,7 +76,7 @@ impl AccountActions { // The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can // consume the message. // This ensures that contracts cannot consume messages that are not intended for them. - let message_hash = compute_outer_authwit_hash(context.msg_sender(), inner_hash); + let message_hash = compute_outer_authwit_hash(context.msg_sender(), context.chain_id(), context.version(), inner_hash); let valid_fn = self.is_valid_impl; assert(valid_fn(context, message_hash) == true, "Message not authorized by account"); context.push_new_nullifier(message_hash, 0); @@ -90,7 +90,7 @@ impl AccountActions { // The `inner_hash` is "siloed" with the `msg_sender` to ensure that only it can // consume the message. // This ensures that contracts cannot consume messages that are not intended for them. - let message_hash = compute_outer_authwit_hash(context.msg_sender(), inner_hash); + let message_hash = compute_outer_authwit_hash(context.msg_sender(), context.chain_id(), context.version(), inner_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); diff --git a/noir-projects/aztec-nr/authwit/src/auth.nr b/noir-projects/aztec-nr/authwit/src/auth.nr index e098fdd5fd7..90972c94d7b 100644 --- a/noir-projects/aztec-nr/authwit/src/auth.nr +++ b/noir-projects/aztec-nr/authwit/src/auth.nr @@ -29,10 +29,17 @@ pub fn assert_current_call_valid_authwit_public(context: &mut PublicContext, on_ // docs:start:compute_call_authwit_hash // Compute the message hash to be used by an authentication witness -pub fn compute_call_authwit_hash(caller: AztecAddress, consumer: AztecAddress, selector: FunctionSelector, args: [Field; N]) -> Field { +pub fn compute_call_authwit_hash( + caller: AztecAddress, + consumer: AztecAddress, + chain_id: Field, + version: Field, + selector: FunctionSelector, + args: [Field; N] +) -> Field { let args_hash = hash_args(args); let inner_hash = compute_inner_authwit_hash([caller.to_field(), selector.to_field(), args_hash]); - compute_outer_authwit_hash(consumer, inner_hash) + compute_outer_authwit_hash(consumer, chain_id, version, inner_hash) } // docs:end:compute_call_authwit_hash @@ -40,9 +47,19 @@ pub fn compute_inner_authwit_hash(args: [Field; N]) -> Field { pedersen_hash(args, GENERATOR_INDEX__AUTHWIT_INNER) } -pub fn compute_outer_authwit_hash(consumer: AztecAddress, inner_hash: Field) -> Field { +pub fn compute_outer_authwit_hash( + consumer: AztecAddress, + chain_id: Field, + version: Field, + inner_hash: Field +) -> Field { pedersen_hash( - [consumer.to_field(), inner_hash], + [ + consumer.to_field(), + chain_id, + version, + inner_hash + ], GENERATOR_INDEX__AUTHWIT_OUTER ) } 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 c092d6bf081..c6a08d9c2bd 100644 --- a/noir-projects/noir-contracts/contracts/uniswap_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/uniswap_contract/src/main.nr @@ -166,7 +166,7 @@ contract Uniswap { // if valid, it returns the IS_VALID selector which is expected by token contract #[aztec(public)] fn spend_public_authwit(inner_hash: Field) -> Field { - let message_hash = compute_outer_authwit_hash(context.msg_sender(), inner_hash); + let message_hash = compute_outer_authwit_hash(context.msg_sender(), context.chain_id(), context.version(), inner_hash); let value = storage.approved_action.at(message_hash).read(); if (value) { context.push_new_nullifier(message_hash, 0); @@ -192,6 +192,8 @@ contract Uniswap { let message_hash = compute_call_authwit_hash( token_bridge, token, + context.chain_id(), + context.version(), selector, [context.this_address().to_field(), amount, nonce_for_burn_approval] ); diff --git a/yarn-project/accounts/src/defaults/account_interface.ts b/yarn-project/accounts/src/defaults/account_interface.ts index f37833ea9a7..6563a308f1b 100644 --- a/yarn-project/accounts/src/defaults/account_interface.ts +++ b/yarn-project/accounts/src/defaults/account_interface.ts @@ -10,6 +10,8 @@ import { NodeInfo } from '@aztec/types/interfaces'; */ export class DefaultAccountInterface implements AccountInterface { private entrypoint: EntrypointInterface; + private chainId: Fr; + private version: Fr; constructor( private authWitnessProvider: AuthWitnessProvider, @@ -22,6 +24,8 @@ export class DefaultAccountInterface implements AccountInterface { nodeInfo.chainId, nodeInfo.protocolVersion, ); + this.chainId = new Fr(nodeInfo.chainId); + this.version = new Fr(nodeInfo.protocolVersion); } createTxExecutionRequest(executions: FunctionCall[], fee?: FeeOptions): Promise { @@ -39,4 +43,12 @@ export class DefaultAccountInterface implements AccountInterface { getAddress(): AztecAddress { return this.address.address; } + + getChainId(): Fr { + return this.chainId; + } + + getVersion(): Fr { + return this.version; + } } diff --git a/yarn-project/accounts/src/single_key/account_contract.ts b/yarn-project/accounts/src/single_key/account_contract.ts index c790340266f..01cf889f3bd 100644 --- a/yarn-project/accounts/src/single_key/account_contract.ts +++ b/yarn-project/accounts/src/single_key/account_contract.ts @@ -35,11 +35,20 @@ export class SingleKeyAccountContract extends DefaultAccountContract { class SingleKeyAuthWitnessProvider implements AuthWitnessProvider { constructor(private privateKey: GrumpkinPrivateKey, private partialAddress: PartialAddress) {} - createAuthWit(message: Fr): Promise { + /** + * Signs a message hash using the private key and includes the public key and partial address + * in the witness. + * + * Beware that this ONLY signs and rely on the wrapping `wallet_account` to construct the message hash. + * + * @param messageHash - The message hash to sign. + * @returns The authentication witness + */ + createAuthWit(messageHash: Fr): Promise { const schnorr = new Schnorr(); - const signature = schnorr.constructSignature(message.toBuffer(), this.privateKey); + const signature = schnorr.constructSignature(messageHash.toBuffer(), this.privateKey); const publicKey = generatePublicKey(this.privateKey); const witness = [...publicKey.toFields(), ...signature.toBuffer(), this.partialAddress]; - return Promise.resolve(new AuthWitness(message, witness)); + return Promise.resolve(new AuthWitness(messageHash, witness)); } } diff --git a/yarn-project/aztec.js/src/account/interface.ts b/yarn-project/aztec.js/src/account/interface.ts index 7da5226b749..5cbe13430bf 100644 --- a/yarn-project/aztec.js/src/account/interface.ts +++ b/yarn-project/aztec.js/src/account/interface.ts @@ -23,6 +23,8 @@ export interface AuthWitnessProvider { * If a message hash is provided, it will create a witness for that directly. * Otherwise, it will compute the message hash using the caller and the action of the intent. * @param messageHashOrIntent - The message hash or the intent (caller and action) to approve + * @param chainId - The chain id for the message, will default to the current chain id + * @param version - The version for the message, will default to the current protocol version * @returns The authentication witness */ createAuthWit( @@ -34,6 +36,10 @@ export interface AuthWitnessProvider { caller: AztecAddress; /** The action to approve */ action: ContractFunctionInteraction | FunctionCall; + /** The chain id to approve */ + chainId?: Fr; + /** The version to approve */ + version?: Fr; }, ): Promise; } @@ -59,5 +65,11 @@ export interface AccountInterface extends AuthWitnessProvider, EntrypointInterfa /** Returns the address for this account. */ getAddress(): AztecAddress; + + /** Returns the chain id for this account */ + getChainId(): Fr; + + /** Returns the rollup version for this account */ + getVersion(): Fr; } // docs:end:account-interface diff --git a/yarn-project/aztec.js/src/fee/private_fee_payment_method.ts b/yarn-project/aztec.js/src/fee/private_fee_payment_method.ts index 20be7695ccb..bc4c998ce7d 100644 --- a/yarn-project/aztec.js/src/fee/private_fee_payment_method.ts +++ b/yarn-project/aztec.js/src/fee/private_fee_payment_method.ts @@ -58,11 +58,16 @@ export class PrivateFeePaymentMethod implements FeePaymentMethod { */ async getFunctionCalls(maxFee: Fr): Promise { const nonce = Fr.random(); - const messageHash = computeAuthWitMessageHash(this.paymentContract, { - args: [this.wallet.getCompleteAddress().address, this.paymentContract, maxFee, nonce], - functionData: new FunctionData(FunctionSelector.fromSignature('unshield((Field),(Field),Field,Field)'), true), - to: this.asset, - }); + const messageHash = computeAuthWitMessageHash( + this.paymentContract, + this.wallet.getChainId(), + this.wallet.getVersion(), + { + args: [this.wallet.getCompleteAddress().address, this.paymentContract, maxFee, nonce], + functionData: new FunctionData(FunctionSelector.fromSignature('unshield((Field),(Field),Field,Field)'), true), + to: this.asset, + }, + ); await this.wallet.createAuthWit(messageHash); const secretHashForRebate = computeMessageSecretHash(this.rebateSecret); diff --git a/yarn-project/aztec.js/src/fee/public_fee_payment_method.ts b/yarn-project/aztec.js/src/fee/public_fee_payment_method.ts index bbc601d1505..ed44beb91eb 100644 --- a/yarn-project/aztec.js/src/fee/public_fee_payment_method.ts +++ b/yarn-project/aztec.js/src/fee/public_fee_payment_method.ts @@ -51,14 +51,20 @@ export class PublicFeePaymentMethod implements FeePaymentMethod { */ getFunctionCalls(maxFee: Fr): Promise { const nonce = Fr.random(); - const messageHash = computeAuthWitMessageHash(this.paymentContract, { - args: [this.wallet.getAddress(), this.paymentContract, maxFee, nonce], - functionData: new FunctionData( - FunctionSelector.fromSignature('transfer_public((Field),(Field),Field,Field)'), - false, - ), - to: this.asset, - }); + + const messageHash = computeAuthWitMessageHash( + this.paymentContract, + this.wallet.getChainId(), + this.wallet.getVersion(), + { + args: [this.wallet.getAddress(), this.paymentContract, maxFee, nonce], + functionData: new FunctionData( + FunctionSelector.fromSignature('transfer_public((Field),(Field),Field,Field)'), + false, + ), + to: this.asset, + }, + ); return Promise.resolve([ this.wallet.setPublicAuthWit(messageHash, true).request(), diff --git a/yarn-project/aztec.js/src/utils/authwit.ts b/yarn-project/aztec.js/src/utils/authwit.ts index 2087730ca81..b7fd074d300 100644 --- a/yarn-project/aztec.js/src/utils/authwit.ts +++ b/yarn-project/aztec.js/src/utils/authwit.ts @@ -5,19 +5,23 @@ import { pedersenHash } from '@aztec/foundation/crypto'; // docs:start:authwit_computeAuthWitMessageHash /** * Compute an authentication witness message hash from a caller and a request - * H(target: AztecAddress, H(caller: AztecAddress, selector: Field, args_hash: Field)) + * H(target: AztecAddress, chainId: Field, version: Field, H(caller: AztecAddress, selector: Field, args_hash: Field)) * Example usage would be `bob` authenticating `alice` to perform a transfer of `10` * tokens from his account to herself: - * H(token, H(alice, transfer_selector, H(bob, alice, 10, nonce))) + * H(token, 1, 1, H(alice, transfer_selector, H(bob, alice, 10, nonce))) * `bob` then signs the message hash and gives it to `alice` who can then perform the * action. * @param caller - The caller approved to make the call + * @param chainId - The chain id for the message + * @param version - The version for the message * @param action - The request to be made (function call) * @returns The message hash for the witness */ -export const computeAuthWitMessageHash = (caller: AztecAddress, action: FunctionCall) => { +export const computeAuthWitMessageHash = (caller: AztecAddress, chainId: Fr, version: Fr, action: FunctionCall) => { return computeOuterAuthWitHash( action.to.toField(), + chainId, + version, computeInnerAuthWitHash([ caller.toField(), action.functionData.selector.toField(), @@ -51,12 +55,14 @@ export const computeInnerAuthWitHash = (args: Fr[]) => { * It is used as part of the `computeAuthWitMessageHash` but can also be used * in case the message is not a "call" to a function, but arbitrary data. * @param consumer - The address that can "consume" the authwit + * @param chainId - The chain id that can "consume" the authwit + * @param version - The version that can "consume" the authwit * @param innerHash - The inner hash for the witness * @returns The outer hash for the witness */ -export const computeOuterAuthWitHash = (consumer: AztecAddress, innerHash: Fr) => { +export const computeOuterAuthWitHash = (consumer: AztecAddress, chainId: Fr, version: Fr, innerHash: Fr) => { return pedersenHash( - [consumer.toField(), innerHash].map(fr => fr.toBuffer()), + [consumer.toField(), chainId, version, innerHash].map(fr => fr.toBuffer()), GeneratorIndex.AUTHWIT_OUTER, ); }; diff --git a/yarn-project/aztec.js/src/wallet/account_wallet.ts b/yarn-project/aztec.js/src/wallet/account_wallet.ts index 9c388f5b953..bc32d96c5ab 100644 --- a/yarn-project/aztec.js/src/wallet/account_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/account_wallet.ts @@ -19,6 +19,14 @@ export class AccountWallet extends BaseWallet { return this.account.createTxExecutionRequest(execs, fee); } + getChainId(): Fr { + return this.account.getChainId(); + } + + getVersion(): Fr { + return this.account.getVersion(); + } + /** * Computes an authentication witness from either a message or a caller and an action. * If a message is provided, it will create a witness for the message directly. @@ -35,6 +43,10 @@ export class AccountWallet extends BaseWallet { caller: AztecAddress; /** The action to approve */ action: ContractFunctionInteraction | FunctionCall; + /** The chain id to approve */ + chainId?: Fr; + /** The version to approve */ + version?: Fr; }, ): Promise { const messageHash = this.getMessageHash(messageHashOrIntent); @@ -59,6 +71,10 @@ export class AccountWallet extends BaseWallet { caller: AztecAddress; /** The action to approve */ action: ContractFunctionInteraction | FunctionCall; + /** The chain id to approve */ + chainId?: Fr; + /** The version to approve */ + version?: Fr; }, authorized: boolean, ): ContractFunctionInteraction { @@ -84,16 +100,26 @@ export class AccountWallet extends BaseWallet { caller: AztecAddress; /** The action to approve */ action: ContractFunctionInteraction | FunctionCall; + /** The chain id to approve */ + chainId?: Fr; + /** The version to approve */ + version?: Fr; }, ): Fr { if (Buffer.isBuffer(messageHashOrIntent)) { return Fr.fromBuffer(messageHashOrIntent); } else if (messageHashOrIntent instanceof Fr) { return messageHashOrIntent; - } else if (messageHashOrIntent.action instanceof ContractFunctionInteraction) { - return computeAuthWitMessageHash(messageHashOrIntent.caller, messageHashOrIntent.action.request()); + } else { + return computeAuthWitMessageHash( + messageHashOrIntent.caller, + messageHashOrIntent.chainId || this.getChainId(), + messageHashOrIntent.version || this.getVersion(), + messageHashOrIntent.action instanceof ContractFunctionInteraction + ? messageHashOrIntent.action.request() + : messageHashOrIntent.action, + ); } - return computeAuthWitMessageHash(messageHashOrIntent.caller, messageHashOrIntent.action); } /** @@ -113,6 +139,10 @@ export class AccountWallet extends BaseWallet { caller: AztecAddress; /** The action to approve */ action: ContractFunctionInteraction | FunctionCall; + /** The chain id to approve */ + chainId?: Fr; + /** The version to approve */ + version?: Fr; }, ): Promise<{ /** boolean flag indicating if the authwit is valid in private context */ @@ -148,6 +178,10 @@ export class AccountWallet extends BaseWallet { caller: AztecAddress; /** The action to approve */ action: ContractFunctionInteraction | FunctionCall; + /** The chain id to approve */ + chainId?: Fr; + /** The version to approve */ + version?: Fr; }, ): ContractFunctionInteraction { const message = this.getMessageHash(messageHashOrIntent); diff --git a/yarn-project/aztec.js/src/wallet/base_wallet.ts b/yarn-project/aztec.js/src/wallet/base_wallet.ts index 6ce1d9fa34d..759e9180093 100644 --- a/yarn-project/aztec.js/src/wallet/base_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/base_wallet.ts @@ -31,6 +31,10 @@ export abstract class BaseWallet implements Wallet { abstract getCompleteAddress(): CompleteAddress; + abstract getChainId(): Fr; + + abstract getVersion(): Fr; + abstract createTxExecutionRequest(execs: FunctionCall[], fee?: FeeOptions): Promise; abstract createAuthWit( @@ -42,6 +46,10 @@ export abstract class BaseWallet implements Wallet { caller: AztecAddress; /** The action to approve */ action: ContractFunctionInteraction | FunctionCall; + /** The chain id to approve */ + chainId?: Fr; + /** The version to approve */ + version?: Fr; }, ): Promise; diff --git a/yarn-project/aztec.js/src/wallet/index.ts b/yarn-project/aztec.js/src/wallet/index.ts index 18fcfe3514c..492116b38c3 100644 --- a/yarn-project/aztec.js/src/wallet/index.ts +++ b/yarn-project/aztec.js/src/wallet/index.ts @@ -1,4 +1,5 @@ import { PXE } from '@aztec/circuit-types'; +import { Fr } from '@aztec/circuits.js'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { AccountContract } from '../account/contract.js'; diff --git a/yarn-project/aztec.js/src/wallet/signerless_wallet.ts b/yarn-project/aztec.js/src/wallet/signerless_wallet.ts index e99e383a757..100cc73dc65 100644 --- a/yarn-project/aztec.js/src/wallet/signerless_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/signerless_wallet.ts @@ -27,6 +27,14 @@ export class SignerlessWallet extends BaseWallet { ); } + getChainId(): Fr { + throw new Error('Method not implemented.'); + } + + getVersion(): Fr { + throw new Error('Method not implemented.'); + } + getCompleteAddress(): CompleteAddress { throw new Error('Method not implemented.'); } 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 d8487288bef..706ee6502ad 100644 --- a/yarn-project/end-to-end/src/e2e_authwit.test.ts +++ b/yarn-project/end-to-end/src/e2e_authwit.test.ts @@ -13,16 +13,23 @@ describe('e2e_authwit_tests', () => { let wallets: AccountWallet[]; let accounts: CompleteAddress[]; + let chainId: Fr; + let version: Fr; + beforeAll(async () => { ({ wallets, accounts } = await setup(2)); await publicDeployAccounts(wallets[0], accounts.slice(0, 2)); + + const nodeInfo = await wallets[0].getNodeInfo(); + chainId = new Fr(nodeInfo.chainId); + version = new Fr(nodeInfo.protocolVersion); }, 100_000); describe('Private', () => { describe('arbitrary data', () => { it('happy path', async () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead')]); - const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash, chainId, version); const witness = await wallets[0].createAuthWit(outerHash); await wallets[1].addAuthWitness(witness); @@ -51,7 +58,7 @@ describe('e2e_authwit_tests', () => { describe('failure case', () => { it('cancel before usage', async () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0xbeef')]); - const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash, chainId, version); expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ isValidInPrivate: false, @@ -82,6 +89,50 @@ describe('e2e_authwit_tests', () => { // The transaction should be dropped because of a cancelled authwit (duplicate nullifier) await expect(txCancelledAuthwit.wait()).rejects.toThrow('Transaction '); }); + + it('invalid chain id', async () => { + const invalidChainId = Fr.random(); + + const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0xbeef')]); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), invalidChainId, version, innerHash); + const outerCorrectHash = computeOuterAuthWitHash(wallets[1].getAddress(), chainId, version, innerHash); + + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerCorrectHash)).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, + }); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerCorrectHash)).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: true, + isValidInPublic: false, + }); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerCorrectHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + + // The transaction should be dropped because of a cancelled authwit (duplicate nullifier) + await expect(txCancelledAuthwit.wait()).rejects.toThrow('Transaction '); + }); }); }); }); @@ -90,7 +141,7 @@ describe('e2e_authwit_tests', () => { describe('arbitrary data', () => { it('happy path', async () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0x01')]); - const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash, chainId, version); expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ isValidInPrivate: false, @@ -116,7 +167,7 @@ describe('e2e_authwit_tests', () => { describe('failure case', () => { it('cancel before usage', async () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0x02')]); - const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash, chainId, version); expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ isValidInPrivate: false, diff --git a/yarn-project/end-to-end/src/e2e_blacklist_token_contract.test.ts b/yarn-project/end-to-end/src/e2e_blacklist_token_contract.test.ts index 8d6e232fbb1..8b12abaada6 100644 --- a/yarn-project/end-to-end/src/e2e_blacklist_token_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_blacklist_token_contract.test.ts @@ -740,7 +740,12 @@ describe('e2e_blacklist_token_contract', () => { 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 messageHash = computeAuthWitMessageHash( + accounts[1].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); await wallets[1].addCapsule( getMembershipCapsule(await getMembershipProof(accounts[1].address.toBigInt(), true)), ); @@ -763,7 +768,12 @@ describe('e2e_blacklist_token_contract', () => { const action = asset .withWallet(wallets[2]) .methods.transfer(accounts[0].address, accounts[1].address, amount, nonce); - const expectedMessageHash = computeAuthWitMessageHash(accounts[2].address, action.request()); + const expectedMessageHash = computeAuthWitMessageHash( + accounts[2].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); const witness = await wallets[0].createAuthWit({ caller: accounts[1].address, action }); await wallets[2].addAuthWitness(witness); @@ -1047,7 +1057,12 @@ describe('e2e_blacklist_token_contract', () => { const action = asset .withWallet(wallets[2]) .methods.unshield(accounts[0].address, accounts[1].address, amount, nonce); - const expectedMessageHash = computeAuthWitMessageHash(accounts[2].address, action.request()); + const expectedMessageHash = computeAuthWitMessageHash( + accounts[2].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); // Both wallets are connected to same node and PXE so we could just insert directly // But doing it in two actions to show the flow. @@ -1282,7 +1297,12 @@ describe('e2e_blacklist_token_contract', () => { getMembershipCapsule(await getMembershipProof(accounts[0].address.toBigInt(), true)), ); const action = asset.withWallet(wallets[1]).methods.burn(accounts[0].address, amount, nonce); - const messageHash = computeAuthWitMessageHash(accounts[1].address, action.request()); + const messageHash = computeAuthWitMessageHash( + accounts[1].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); await expect(action.simulate()).rejects.toThrow( `Unknown auth witness for message hash ${messageHash.toString()}`, @@ -1300,7 +1320,12 @@ describe('e2e_blacklist_token_contract', () => { getMembershipCapsule(await getMembershipProof(accounts[0].address.toBigInt(), true)), ); const action = asset.withWallet(wallets[2]).methods.burn(accounts[0].address, amount, nonce); - const expectedMessageHash = computeAuthWitMessageHash(accounts[2].address, action.request()); + const expectedMessageHash = computeAuthWitMessageHash( + accounts[2].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); const witness = await wallets[0].createAuthWit({ caller: accounts[1].address, action }); await wallets[2].addAuthWitness(witness); diff --git a/yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts b/yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts index 34ce2da58d0..01883265441 100644 --- a/yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts +++ b/yarn-project/end-to-end/src/e2e_cross_chain_messaging.test.ts @@ -205,6 +205,8 @@ describe('e2e_cross_chain_messaging', () => { const nonce = Fr.random(); const expectedBurnMessageHash = computeAuthWitMessageHash( l2Bridge.address, + user1Wallet.getChainId(), + user1Wallet.getVersion(), l2Token.methods.burn(user1Wallet.getAddress(), withdrawAmount, nonce).request(), ); // Should fail as owner has not given approval to bridge burn their funds. diff --git a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts index 1f2bd159972..eaa666f525c 100644 --- a/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts +++ b/yarn-project/end-to-end/src/e2e_crowdfunding_and_claim.test.ts @@ -9,7 +9,6 @@ import { Note, PXE, TxHash, - computeAuthWitMessageHash, computeMessageSecretHash, generatePublicKey, } from '@aztec/aztec.js'; @@ -264,8 +263,7 @@ describe('e2e_crowdfunding_and_claim', () => { const action = donationToken .withWallet(donorWallets[1]) .methods.transfer(donorWallets[1].getAddress(), crowdfundingContract.address, donationAmount, 0); - const messageHash = computeAuthWitMessageHash(crowdfundingContract.address, action.request()); - const witness = await donorWallets[1].createAuthWit(messageHash); + const witness = await donorWallets[1].createAuthWit({ caller: crowdfundingContract.address, action }); await donorWallets[1].addAuthWitness(witness); } diff --git a/yarn-project/end-to-end/src/e2e_fees.test.ts b/yarn-project/end-to-end/src/e2e_fees.test.ts index 0d87a44df87..9dbbc4825e9 100644 --- a/yarn-project/end-to-end/src/e2e_fees.test.ts +++ b/yarn-project/end-to-end/src/e2e_fees.test.ts @@ -687,14 +687,19 @@ describe('e2e_fees', () => { class BuggedSetupFeePaymentMethod extends PublicFeePaymentMethod { getFunctionCalls(maxFee: Fr): Promise { const nonce = Fr.random(); - const messageHash = computeAuthWitMessageHash(this.paymentContract, { - args: [this.wallet.getAddress(), this.paymentContract, maxFee, nonce], - functionData: new FunctionData( - FunctionSelector.fromSignature('transfer_public((Field),(Field),Field,Field)'), - false, - ), - to: this.asset, - }); + const messageHash = computeAuthWitMessageHash( + this.paymentContract, + this.wallet.getChainId(), + this.wallet.getVersion(), + { + args: [this.wallet.getAddress(), this.paymentContract, maxFee, nonce], + functionData: new FunctionData( + FunctionSelector.fromSignature('transfer_public((Field),(Field),Field,Field)'), + false, + ), + to: this.asset, + }, + ); const tooMuchFee = new Fr(maxFee.toBigInt() * 2n); @@ -716,14 +721,19 @@ class BuggedTeardownFeePaymentMethod extends PublicFeePaymentMethod { async getFunctionCalls(maxFee: Fr): Promise { // authorize the FPC to take the max fee from Alice const nonce = Fr.random(); - const messageHash1 = computeAuthWitMessageHash(this.paymentContract, { - args: [this.wallet.getAddress(), this.paymentContract, maxFee, nonce], - functionData: new FunctionData( - FunctionSelector.fromSignature('transfer_public((Field),(Field),Field,Field)'), - false, - ), - to: this.asset, - }); + const messageHash1 = computeAuthWitMessageHash( + this.paymentContract, + this.wallet.getChainId(), + this.wallet.getVersion(), + { + args: [this.wallet.getAddress(), this.paymentContract, maxFee, nonce], + functionData: new FunctionData( + FunctionSelector.fromSignature('transfer_public((Field),(Field),Field,Field)'), + false, + ), + to: this.asset, + }, + ); // authorize the FPC to take the maxFee // do this first because we only get 2 feepayload calls diff --git a/yarn-project/end-to-end/src/e2e_lending_contract.test.ts b/yarn-project/end-to-end/src/e2e_lending_contract.test.ts index 1a29f947882..b349bfb39c3 100644 --- a/yarn-project/end-to-end/src/e2e_lending_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_lending_contract.test.ts @@ -328,6 +328,8 @@ describe('e2e_lending_contract', () => { const nonce = Fr.random(); const messageHash = computeAuthWitMessageHash( lendingContract.address, + wallet.getChainId(), + wallet.getVersion(), stableCoin.methods.burn_public(lendingAccount.address, repayAmount, nonce).request(), ); diff --git a/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts b/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts index 34bc3259b4b..e4b3b99a33c 100644 --- a/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts +++ b/yarn-project/end-to-end/src/e2e_public_cross_chain_messaging.test.ts @@ -108,6 +108,8 @@ describe('e2e_public_cross_chain_messaging', () => { const nonce = Fr.random(); const burnMessageHash = computeAuthWitMessageHash( l2Bridge.address, + wallets[0].getChainId(), + wallets[0].getVersion(), l2Token.methods.burn_public(ownerAddress, withdrawAmount, nonce).request(), ); await user1Wallet.setPublicAuthWit(burnMessageHash, true).send().wait(); 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 a64490a6571..bd6ccfade1e 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 @@ -513,7 +513,12 @@ describe('e2e_token_contract', () => { 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()); + const messageHash = computeAuthWitMessageHash( + accounts[1].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); await wallets[0].setPublicAuthWit(messageHash, true).send().wait(); @@ -574,7 +579,6 @@ describe('e2e_token_contract', () => { // We need to compute the message we want to sign and add it to the wallet as approved // docs:start:authwit_transfer_example - // docs:start:authwit_computeAuthWitMessageHash const action = asset .withWallet(wallets[1]) .methods.transfer(accounts[0].address, accounts[1].address, amount, nonce); @@ -661,7 +665,12 @@ describe('e2e_token_contract', () => { 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 messageHash = computeAuthWitMessageHash( + accounts[1].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); await expect(action.simulate()).rejects.toThrow( `Unknown auth witness for message hash ${messageHash.toString()}`, @@ -678,7 +687,12 @@ describe('e2e_token_contract', () => { const action = asset .withWallet(wallets[2]) .methods.transfer(accounts[0].address, accounts[1].address, amount, nonce); - const expectedMessageHash = computeAuthWitMessageHash(accounts[2].address, action.request()); + const expectedMessageHash = computeAuthWitMessageHash( + accounts[2].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); const witness = await wallets[0].createAuthWit({ caller: accounts[1].address, action }); await wallets[2].addAuthWitness(witness); @@ -955,7 +969,12 @@ describe('e2e_token_contract', () => { const action = asset .withWallet(wallets[2]) .methods.unshield(accounts[0].address, accounts[1].address, amount, nonce); - const expectedMessageHash = computeAuthWitMessageHash(accounts[2].address, action.request()); + const expectedMessageHash = computeAuthWitMessageHash( + accounts[2].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); // Both wallets are connected to same node and PXE so we could just insert directly // But doing it in two actions to show the flow. @@ -1133,7 +1152,12 @@ describe('e2e_token_contract', () => { // 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.burn(accounts[0].address, amount, nonce); - const messageHash = computeAuthWitMessageHash(accounts[1].address, action.request()); + const messageHash = computeAuthWitMessageHash( + accounts[1].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); await expect(action.simulate()).rejects.toThrow( `Unknown auth witness for message hash ${messageHash.toString()}`, @@ -1148,7 +1172,12 @@ describe('e2e_token_contract', () => { // We need to compute the message we want to sign and add it to the wallet as approved const action = asset.withWallet(wallets[2]).methods.burn(accounts[0].address, amount, nonce); - const expectedMessageHash = computeAuthWitMessageHash(accounts[2].address, action.request()); + const expectedMessageHash = computeAuthWitMessageHash( + accounts[2].address, + wallets[0].getChainId(), + wallets[0].getVersion(), + action.request(), + ); const witness = await wallets[0].createAuthWit({ caller: accounts[1].address, action }); await wallets[2].addAuthWitness(witness); 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 41727806769..69a8c191944 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 @@ -438,6 +438,8 @@ export const uniswapL1L2TestSuite = ( const nonceForWETHTransferApproval = new Fr(1n); const transferMessageHash = computeAuthWitMessageHash( uniswapL2Contract.address, + ownerWallet.getChainId(), + ownerWallet.getVersion(), wethCrossChainHarness.l2Token.methods .transfer_public(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHTransferApproval) .request(), @@ -469,7 +471,12 @@ export const uniswapL1L2TestSuite = ( ownerEthAddress, nonceForSwap, ); - const swapMessageHash = computeAuthWitMessageHash(sponsorAddress, action.request()); + const swapMessageHash = computeAuthWitMessageHash( + sponsorAddress, + ownerWallet.getChainId(), + ownerWallet.getVersion(), + action.request(), + ); await ownerWallet.setPublicAuthWit(swapMessageHash, true).send().wait(); // 4.2 Call swap_public from user2 on behalf of owner @@ -635,6 +642,9 @@ export const uniswapL1L2TestSuite = ( const expectedMessageHash = computeAuthWitMessageHash( uniswapL2Contract.address, + ownerWallet.getChainId(), + ownerWallet.getVersion(), + wethCrossChainHarness.l2Token.methods .unshield(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHUnshieldApproval) .request(), @@ -708,6 +718,9 @@ export const uniswapL1L2TestSuite = ( const nonceForWETHTransferApproval = new Fr(2n); const transferMessageHash = computeAuthWitMessageHash( uniswapL2Contract.address, + ownerWallet.getChainId(), + ownerWallet.getVersion(), + wethCrossChainHarness.l2Token.methods .transfer_public(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHTransferApproval) .request(), @@ -759,7 +772,12 @@ export const uniswapL1L2TestSuite = ( ownerEthAddress, nonceForSwap, ); - const swapMessageHash = computeAuthWitMessageHash(approvedUser, action.request()); + const swapMessageHash = computeAuthWitMessageHash( + approvedUser, + ownerWallet.getChainId(), + ownerWallet.getVersion(), + action.request(), + ); await ownerWallet.setPublicAuthWit(swapMessageHash, true).send().wait(); // Swap! @@ -774,6 +792,8 @@ export const uniswapL1L2TestSuite = ( const transferMessageHash = computeAuthWitMessageHash( uniswapL2Contract.address, + ownerWallet.getChainId(), + ownerWallet.getVersion(), wethCrossChainHarness.l2Token.methods .transfer_public(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHTransferApproval) .request(), @@ -954,6 +974,8 @@ export const uniswapL1L2TestSuite = ( const nonceForWETHTransferApproval = new Fr(5n); const transferMessageHash = computeAuthWitMessageHash( uniswapL2Contract.address, + ownerWallet.getChainId(), + ownerWallet.getVersion(), wethCrossChainHarness.l2Token.methods .transfer_public(ownerAddress, uniswapL2Contract.address, wethAmountToBridge, nonceForWETHTransferApproval) .request(), diff --git a/yarn-project/entrypoints/src/dapp_entrypoint.ts b/yarn-project/entrypoints/src/dapp_entrypoint.ts index 877ba2a4a3d..afeb1bb17a3 100644 --- a/yarn-project/entrypoints/src/dapp_entrypoint.ts +++ b/yarn-project/entrypoints/src/dapp_entrypoint.ts @@ -32,7 +32,12 @@ export class DefaultDappEntrypoint implements EntrypointInterface { const functionData = FunctionData.fromAbi(abi); const innerHash = computeInnerAuthWitHash([Fr.ZERO, functionData.selector.toField(), entrypointPackedArgs.hash]); - const outerHash = computeOuterAuthWitHash(this.dappEntrypointAddress, innerHash); + const outerHash = computeOuterAuthWitHash( + this.dappEntrypointAddress, + innerHash, + new Fr(this.chainId), + new Fr(this.version), + ); const authWitness = await this.userAuthWitnessProvider.createAuthWit(outerHash); From 6df471b43cb1c26d97c928c2cd0ae9f7f228d1a7 Mon Sep 17 00:00:00 2001 From: LHerskind Date: Wed, 20 Mar 2024 14:54:19 +0000 Subject: [PATCH 2/3] fix: fix wrong ordering on compute outher authwit hash --- yarn-project/aztec.js/src/wallet/index.ts | 1 - .../end-to-end/src/e2e_authwit.test.ts | 54 +++++++++++++++++-- .../entrypoints/src/dapp_entrypoint.ts | 2 +- 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/yarn-project/aztec.js/src/wallet/index.ts b/yarn-project/aztec.js/src/wallet/index.ts index 492116b38c3..18fcfe3514c 100644 --- a/yarn-project/aztec.js/src/wallet/index.ts +++ b/yarn-project/aztec.js/src/wallet/index.ts @@ -1,5 +1,4 @@ import { PXE } from '@aztec/circuit-types'; -import { Fr } from '@aztec/circuits.js'; import { AztecAddress } from '@aztec/foundation/aztec-address'; import { AccountContract } from '../account/contract.js'; 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 706ee6502ad..0dee52288fd 100644 --- a/yarn-project/end-to-end/src/e2e_authwit.test.ts +++ b/yarn-project/end-to-end/src/e2e_authwit.test.ts @@ -29,7 +29,7 @@ describe('e2e_authwit_tests', () => { describe('arbitrary data', () => { it('happy path', async () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead')]); - const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash, chainId, version); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), chainId, version, innerHash); const witness = await wallets[0].createAuthWit(outerHash); await wallets[1].addAuthWitness(witness); @@ -58,7 +58,7 @@ describe('e2e_authwit_tests', () => { describe('failure case', () => { it('cancel before usage', async () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0xbeef')]); - const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash, chainId, version); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), chainId, version, innerHash); expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ isValidInPrivate: false, @@ -130,7 +130,51 @@ describe('e2e_authwit_tests', () => { isValidInPublic: false, }); - // The transaction should be dropped because of a cancelled authwit (duplicate nullifier) + // The transaction should be dropped because of the invalid chain id + await expect(txCancelledAuthwit.wait()).rejects.toThrow('Transaction '); + }); + + it('invalid chain id', async () => { + const invalidVersion = Fr.random(); + + const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0xbeef')]); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), chainId, invalidVersion, innerHash); + const outerCorrectHash = computeOuterAuthWitHash(wallets[1].getAddress(), chainId, version, innerHash); + + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerCorrectHash)).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, + }); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerCorrectHash)).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: true, + isValidInPublic: false, + }); + expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerCorrectHash)).toEqual({ + isValidInPrivate: false, + isValidInPublic: false, + }); + + // The transaction should be dropped because of the invalid version await expect(txCancelledAuthwit.wait()).rejects.toThrow('Transaction '); }); }); @@ -141,7 +185,7 @@ describe('e2e_authwit_tests', () => { describe('arbitrary data', () => { it('happy path', async () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0x01')]); - const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash, chainId, version); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), chainId, version, innerHash); expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ isValidInPrivate: false, @@ -167,7 +211,7 @@ describe('e2e_authwit_tests', () => { describe('failure case', () => { it('cancel before usage', async () => { const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0x02')]); - const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), innerHash, chainId, version); + const outerHash = computeOuterAuthWitHash(wallets[1].getAddress(), chainId, version, innerHash); expect(await wallets[0].lookupValidity(wallets[0].getAddress(), outerHash)).toEqual({ isValidInPrivate: false, diff --git a/yarn-project/entrypoints/src/dapp_entrypoint.ts b/yarn-project/entrypoints/src/dapp_entrypoint.ts index afeb1bb17a3..0cb82b372b2 100644 --- a/yarn-project/entrypoints/src/dapp_entrypoint.ts +++ b/yarn-project/entrypoints/src/dapp_entrypoint.ts @@ -34,9 +34,9 @@ export class DefaultDappEntrypoint implements EntrypointInterface { const innerHash = computeInnerAuthWitHash([Fr.ZERO, functionData.selector.toField(), entrypointPackedArgs.hash]); const outerHash = computeOuterAuthWitHash( this.dappEntrypointAddress, - innerHash, new Fr(this.chainId), new Fr(this.version), + innerHash, ); const authWitness = await this.userAuthWitnessProvider.createAuthWit(outerHash); From 078df075baf45fc71e67fd23a534c740b9a8ba3b Mon Sep 17 00:00:00 2001 From: LHerskind Date: Thu, 21 Mar 2024 11:17:40 +0000 Subject: [PATCH 3/3] chore: address comments --- yarn-project/accounts/src/defaults/account_interface.ts | 4 ++-- yarn-project/accounts/src/ecdsa/account_contract.ts | 6 +++--- yarn-project/accounts/src/schnorr/account_contract.ts | 6 +++--- yarn-project/accounts/src/single_key/account_contract.ts | 9 --------- yarn-project/aztec.js/src/wallet/signerless_wallet.ts | 2 +- yarn-project/end-to-end/src/e2e_authwit.test.ts | 2 +- .../src/guides/writing_an_account_contract.test.ts | 6 +++--- 7 files changed, 13 insertions(+), 22 deletions(-) diff --git a/yarn-project/accounts/src/defaults/account_interface.ts b/yarn-project/accounts/src/defaults/account_interface.ts index 6563a308f1b..e3a952f1540 100644 --- a/yarn-project/accounts/src/defaults/account_interface.ts +++ b/yarn-project/accounts/src/defaults/account_interface.ts @@ -32,8 +32,8 @@ export class DefaultAccountInterface implements AccountInterface { return this.entrypoint.createTxExecutionRequest(executions, fee); } - createAuthWit(message: Fr): Promise { - return this.authWitnessProvider.createAuthWit(message); + createAuthWit(messageHash: Fr): Promise { + return this.authWitnessProvider.createAuthWit(messageHash); } getCompleteAddress(): CompleteAddress { diff --git a/yarn-project/accounts/src/ecdsa/account_contract.ts b/yarn-project/accounts/src/ecdsa/account_contract.ts index 22c28d699a4..7919ecf62a3 100644 --- a/yarn-project/accounts/src/ecdsa/account_contract.ts +++ b/yarn-project/accounts/src/ecdsa/account_contract.ts @@ -30,9 +30,9 @@ export class EcdsaAccountContract extends DefaultAccountContract { class EcdsaAuthWitnessProvider implements AuthWitnessProvider { constructor(private signingPrivateKey: Buffer) {} - createAuthWit(message: Fr): Promise { + createAuthWit(messageHash: Fr): Promise { const ecdsa = new Ecdsa(); - const signature = ecdsa.constructSignature(message.toBuffer(), this.signingPrivateKey); - return Promise.resolve(new AuthWitness(message, [...signature.r, ...signature.s])); + const signature = ecdsa.constructSignature(messageHash.toBuffer(), this.signingPrivateKey); + return Promise.resolve(new AuthWitness(messageHash, [...signature.r, ...signature.s])); } } diff --git a/yarn-project/accounts/src/schnorr/account_contract.ts b/yarn-project/accounts/src/schnorr/account_contract.ts index 8e3f19c9154..aa651a073f1 100644 --- a/yarn-project/accounts/src/schnorr/account_contract.ts +++ b/yarn-project/accounts/src/schnorr/account_contract.ts @@ -30,9 +30,9 @@ export class SchnorrAccountContract extends DefaultAccountContract { class SchnorrAuthWitnessProvider implements AuthWitnessProvider { constructor(private signingPrivateKey: GrumpkinPrivateKey) {} - createAuthWit(message: Fr): Promise { + createAuthWit(messageHash: Fr): Promise { const schnorr = new Schnorr(); - const signature = schnorr.constructSignature(message.toBuffer(), this.signingPrivateKey).toBuffer(); - return Promise.resolve(new AuthWitness(message, [...signature])); + const signature = schnorr.constructSignature(messageHash.toBuffer(), this.signingPrivateKey).toBuffer(); + return Promise.resolve(new AuthWitness(messageHash, [...signature])); } } diff --git a/yarn-project/accounts/src/single_key/account_contract.ts b/yarn-project/accounts/src/single_key/account_contract.ts index 01cf889f3bd..1334c6791dc 100644 --- a/yarn-project/accounts/src/single_key/account_contract.ts +++ b/yarn-project/accounts/src/single_key/account_contract.ts @@ -35,15 +35,6 @@ export class SingleKeyAccountContract extends DefaultAccountContract { class SingleKeyAuthWitnessProvider implements AuthWitnessProvider { constructor(private privateKey: GrumpkinPrivateKey, private partialAddress: PartialAddress) {} - /** - * Signs a message hash using the private key and includes the public key and partial address - * in the witness. - * - * Beware that this ONLY signs and rely on the wrapping `wallet_account` to construct the message hash. - * - * @param messageHash - The message hash to sign. - * @returns The authentication witness - */ createAuthWit(messageHash: Fr): Promise { const schnorr = new Schnorr(); const signature = schnorr.constructSignature(messageHash.toBuffer(), this.privateKey); diff --git a/yarn-project/aztec.js/src/wallet/signerless_wallet.ts b/yarn-project/aztec.js/src/wallet/signerless_wallet.ts index 100cc73dc65..e49febbd4e6 100644 --- a/yarn-project/aztec.js/src/wallet/signerless_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/signerless_wallet.ts @@ -39,7 +39,7 @@ export class SignerlessWallet extends BaseWallet { throw new Error('Method not implemented.'); } - createAuthWit(_message: Fr): Promise { + createAuthWit(_messageHash: Fr): Promise { throw new Error('Method not implemented.'); } } 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 0dee52288fd..09e4acf53fe 100644 --- a/yarn-project/end-to-end/src/e2e_authwit.test.ts +++ b/yarn-project/end-to-end/src/e2e_authwit.test.ts @@ -134,7 +134,7 @@ describe('e2e_authwit_tests', () => { await expect(txCancelledAuthwit.wait()).rejects.toThrow('Transaction '); }); - it('invalid chain id', async () => { + it('invalid version', async () => { const invalidVersion = Fr.random(); const innerHash = computeInnerAuthWitHash([Fr.fromString('0xdead'), Fr.fromString('0xbeef')]); diff --git a/yarn-project/end-to-end/src/guides/writing_an_account_contract.test.ts b/yarn-project/end-to-end/src/guides/writing_an_account_contract.test.ts index 5501c8d9875..604f5edf920 100644 --- a/yarn-project/end-to-end/src/guides/writing_an_account_contract.test.ts +++ b/yarn-project/end-to-end/src/guides/writing_an_account_contract.test.ts @@ -34,10 +34,10 @@ class SchnorrHardcodedKeyAccountContract extends DefaultAccountContract { getAuthWitnessProvider(_address: CompleteAddress): AuthWitnessProvider { const privateKey = this.privateKey; return { - createAuthWit(message: Fr): Promise { + createAuthWit(messageHash: Fr): Promise { const signer = new Schnorr(); - const signature = signer.constructSignature(message.toBuffer(), privateKey); - return Promise.resolve(new AuthWitness(message, [...signature.toBuffer()])); + const signature = signer.constructSignature(messageHash.toBuffer(), privateKey); + return Promise.resolve(new AuthWitness(messageHash, [...signature.toBuffer()])); }, }; }