From 799911f243614f535c657c37226ff6b4cf9fe730 Mon Sep 17 00:00:00 2001 From: LHerskind Date: Tue, 4 Jun 2024 08:02:44 +0000 Subject: [PATCH 1/2] test: Add test to show-case nullifier issue --- .../escrow/escrow_transfer_private.test.ts | 150 +++++++++++++++++- .../pxe/src/pxe_service/pxe_service.ts | 1 + 2 files changed, 150 insertions(+), 1 deletion(-) diff --git a/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrow/escrow_transfer_private.test.ts b/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrow/escrow_transfer_private.test.ts index 0e86014db196..6fa2bbd3f7a0 100644 --- a/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrow/escrow_transfer_private.test.ts +++ b/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrow/escrow_transfer_private.test.ts @@ -7,6 +7,7 @@ import { SimpleEscrowContract, } from '@aztec/noir-contracts.js'; +import { DUPLICATE_NULLIFIER_ERROR } from '../../fixtures/fixtures.js'; import { setupPXEService } from '../../fixtures/utils.js'; import { EscrowTokenContractTest, toAddressOption } from '../escrowable_token_contract_test.js'; @@ -42,6 +43,28 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { alice = wallets[0]; bob = (await createAccounts(pxeB, 1))[0]; + // https://i.pinimg.com/originals/e0/a7/1f/e0a71f597967638ffd90698f1fd8aa5a.png + { + { + const instance = await alice.getContractInstance(alice.getAddress()); + if (!instance) { + throw new Error('Failed to get contract instance'); + } + await pxeB.registerContract({ + instance: instance, + }); + } + { + const instance = await bob.getContractInstance(bob.getAddress()); + if (!instance) { + throw new Error('Failed to get contract instance'); + } + await alice.registerContract({ + instance: instance, + }); + } + } + await alice.registerRecipient(bob.getCompleteAddress()); await bob.registerRecipient(alice.getCompleteAddress()); @@ -54,6 +77,20 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { escrowAlice = await SimpleEscrowContract.deploy(alice, asset.address, alice.getAddress()).send().deployed(); escrowBob = await SimpleEscrowContract.deploy(bob, asset.address, bob.getAddress()).send().deployed(); + { + { + const extendedNotes = await alice.getNotes({ contractAddress: alice.getAddress() }); + console.log(extendedNotes[0]); + expect(extendedNotes.length).toEqual(1); + await bob.addNote(extendedNotes[0]); + } + { + const extendedNotes = await bob.getNotes({ contractAddress: bob.getAddress() }); + expect(extendedNotes.length).toEqual(1); + await alice.addNote(extendedNotes[0]); + } + } + // Add to the token tokenSim.addAccount(escrowAlice.address); tokenSim.addAccount(escrowBob.address); @@ -71,6 +108,69 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { await t.tokenSim.check(); }); + describe.skip('broken authwit escrow', () => { + /** + * The ability to pass an authwit to someone else such that they can do something on your behalf is BROKEN. + * Unless the actor is yourself, they will not have the keys to nullify or make the proper outgoing + */ + + it('transfer on behalf of other', async () => { + // The only difference between this test and the one in the `transfer_private.test.ts` files, are that this is run + // with wallets connected to different PXE services. This is to show that the authwit is not working as expected. + // The reason that this was not encountered earlier is that this slows down the tests, and was not directly considered. + const balance0 = await asset.methods.balance_of_private(alice.getAddress()).simulate(); + 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(bob) + .methods.transfer( + alice.getAddress(), + bob.getAddress(), + amount, + nonce, + toAddressOption(), + toAddressOption(), + toAddressOption(), + ); + + const witness = await alice.createAuthWit({ caller: bob.getAddress(), action }); + + await bob.addAuthWitness(witness); + + expect(await alice.lookupValidity(alice.getAddress(), { caller: bob.getAddress(), action })).toEqual({ + isValidInPrivate: true, + isValidInPublic: false, + }); + + expect(await bob.lookupValidity(alice.getAddress(), { caller: bob.getAddress(), action })).toEqual({ + isValidInPrivate: true, + isValidInPublic: false, + }); + + // Perform the transfer + await action.send().wait(); + tokenSim.transferPrivate(alice.getAddress(), bob.getAddress(), amount); + + // Perform the transfer again, should fail + const txReplay = asset + .withWallet(bob) + .methods.transfer( + alice.getAddress(), + bob.getAddress(), + amount, + nonce, + toAddressOption(), + toAddressOption(), + toAddressOption(), + ) + .send(); + await expect(txReplay.wait()).rejects.toThrow(DUPLICATE_NULLIFIER_ERROR); + }); + }); + describe('escrowing', () => { describe('simple escrow', () => { it('alice donate to alice escrow, `incoming_viewer_and_nullifier != to`', async () => { @@ -159,7 +259,7 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { }); }); - it('Alice blackmails Bob. She transfer funds to him, but is using herself as viewers and nullifier', async () => { + it.only('Alice blackmails Bob. She transfer funds to him, but is using herself as viewers and nullifier', async () => { // For this test, we are doing something quite strange. // Alice is sending funds to Bob, but setting herself as the `to_incoming_viewer_and_nullifier` // This means that Bob will not be used when encrypting the message, e.g., he will not learn that he received something, @@ -209,6 +309,54 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { .simulate(), ).rejects.toThrow("Assertion failed: Cannot return zero notes 'num_notes != 0'"); } + + console.log('LASSE 2'); + { + // Alice and Bob is colluding on getting a hold of Bobs funds + // Alice will only help if bob gives her half of the amount! + + const toAlice = amount / 2n; + const toBob = amount - toAlice; + const actionBlackmail = asset + .withWallet(alice) + .methods.transfer( + bob.getAddress(), + alice.getAddress(), + toAlice, + 0, + toAddressOption(), + toAddressOption(), + toAddressOption(), + ) + .request(); + console.log('LASSE 3'); + + const actionNonBlackmail = asset + .withWallet(alice) + .methods.transfer( + bob.getAddress(), + bob.getAddress(), + toBob, + 0, + toAddressOption(), + toAddressOption(), + toAddressOption(), + ) + .request(); + + console.log('LASSE 4'); + const appPayload = EntrypointPayload.fromAppExecution([actionBlackmail, actionNonBlackmail]); + // Even though Alice is practically sending it, she is not the sender in the fee option notation. + const feePayload = await EntrypointPayload.fromFeeOptions(bob.getAddress()); + + await alice.addAuthWitness(await bob.createAuthWit(appPayload.hash())); + await alice.addAuthWitness(await bob.createAuthWit(feePayload.hash())); + + console.log('LASSE 5'); + const bobsAccount = await Contract.at(bob.getAddress(), SchnorrAccountContractArtifact, alice); + console.log('LASSE 6'); + await bobsAccount.withWallet(alice).methods['entrypoint'](appPayload, feePayload).send().wait(); + } }); }); }); diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index c04d586cc11c..375a41e177f0 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -317,6 +317,7 @@ export class PXEService implements PXE { } for (const nonce of nonces) { + console.log(`GO FUCK YOURSELF, `, note); const { innerNoteHash, siloedNoteHash, innerNullifier } = await this.simulator.computeNoteHashAndNullifier( note.contractAddress, nonce, From ca164ae1088cdb457053057af40e09842bd7e0b5 Mon Sep 17 00:00:00 2001 From: LHerskind Date: Thu, 6 Jun 2024 10:46:30 +0000 Subject: [PATCH 2/2] extending blackmail case --- .../aztec.js/src/wallet/account_wallet.ts | 4 ++++ .../escrow/escrow_transfer_private.test.ts | 24 ++++++++++++++----- .../escrowable_token_contract_test.ts | 3 +++ .../pxe/src/pxe_service/pxe_service.ts | 1 - 4 files changed, 25 insertions(+), 7 deletions(-) diff --git a/yarn-project/aztec.js/src/wallet/account_wallet.ts b/yarn-project/aztec.js/src/wallet/account_wallet.ts index 93c4114a5985..0bdef16644df 100644 --- a/yarn-project/aztec.js/src/wallet/account_wallet.ts +++ b/yarn-project/aztec.js/src/wallet/account_wallet.ts @@ -16,6 +16,10 @@ export class AccountWallet extends BaseWallet { super(pxe); } + getAccount(): AccountInterface { + return this.account; + } + createTxExecutionRequest(exec: ExecutionRequestInit): Promise { return this.account.createTxExecutionRequest(exec); } diff --git a/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrow/escrow_transfer_private.test.ts b/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrow/escrow_transfer_private.test.ts index 6fa2bbd3f7a0..fc8329b94f64 100644 --- a/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrow/escrow_transfer_private.test.ts +++ b/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrow/escrow_transfer_private.test.ts @@ -1,5 +1,5 @@ import { createAccounts } from '@aztec/accounts/testing'; -import { type AccountWallet, AztecAddress, Contract, Fr, retryUntil } from '@aztec/aztec.js'; +import { AccountWallet, AztecAddress, Contract, Fr, retryUntil } from '@aztec/aztec.js'; import { EntrypointPayload } from '@aztec/aztec.js/entrypoint'; import { EscrowableTokenContract, @@ -15,11 +15,14 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { let teardownB: () => Promise; const t = new EscrowTokenContractTest('transfer_private'); - let { asset, tokenSim, wallets, aztecNode } = t; + let { asset, tokenSim, wallets, aztecNode, pxe } = t; let alice!: AccountWallet; // On pxe a let bob!: AccountWallet; // On pxe b + let doubleTroubleA!: AccountWallet; // On both PXE + let doubleTroubleB!: AccountWallet; // On both PXE + let escrowAlice!: SimpleEscrowContract; let escrowBob!: SimpleEscrowContract; @@ -34,7 +37,7 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { await t.applyBaseSnapshots(); await t.applyMintSnapshot(); await t.setup(); - ({ asset, tokenSim, wallets, aztecNode } = t); + ({ asset, tokenSim, wallets, aztecNode, pxe } = t); // We need a fresh PXE service for the second group of wallets const { pxe: pxeB, teardown: _teardown } = await setupPXEService(aztecNode!, {}, undefined, true); @@ -43,6 +46,14 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { alice = wallets[0]; bob = (await createAccounts(pxeB, 1))[0]; + const secret = Fr.random(); + + // :skull: This is a pain to deal with. + doubleTroubleA = (await createAccounts(pxe, 1, [secret]))[0]; + const acc = doubleTroubleA.getAccount(); + doubleTroubleB = new AccountWallet(pxeB, acc); + await pxeB.registerAccount(secret, doubleTroubleB.getCompleteAddress().partialAddress); + // https://i.pinimg.com/originals/e0/a7/1f/e0a71f597967638ffd90698f1fd8aa5a.png { { @@ -77,10 +88,11 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { escrowAlice = await SimpleEscrowContract.deploy(alice, asset.address, alice.getAddress()).send().deployed(); escrowBob = await SimpleEscrowContract.deploy(bob, asset.address, bob.getAddress()).send().deployed(); + /* { + // We cannot actually do these because the `addNote` is broken! 💀 { const extendedNotes = await alice.getNotes({ contractAddress: alice.getAddress() }); - console.log(extendedNotes[0]); expect(extendedNotes.length).toEqual(1); await bob.addNote(extendedNotes[0]); } @@ -89,7 +101,7 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { expect(extendedNotes.length).toEqual(1); await alice.addNote(extendedNotes[0]); } - } + }*/ // Add to the token tokenSim.addAccount(escrowAlice.address); @@ -259,7 +271,7 @@ describe('e2e_escrowable_token_contract escrow transfer private', () => { }); }); - it.only('Alice blackmails Bob. She transfer funds to him, but is using herself as viewers and nullifier', async () => { + it('Alice blackmails Bob. She transfer funds to him, but is using herself as viewers and nullifier', async () => { // For this test, we are doing something quite strange. // Alice is sending funds to Bob, but setting herself as the `to_incoming_viewer_and_nullifier` // This means that Bob will not be used when encrypting the message, e.g., he will not learn that he received something, diff --git a/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrowable_token_contract_test.ts b/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrowable_token_contract_test.ts index 3ffdcaa94c3d..587f0aca5a92 100644 --- a/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrowable_token_contract_test.ts +++ b/yarn-project/end-to-end/src/e2e_escrowable_token_contract/escrowable_token_contract_test.ts @@ -8,6 +8,7 @@ import { ExtendedNote, Fr, Note, + PXE, type TxHash, createDebugLogger, } from '@aztec/aztec.js'; @@ -78,6 +79,7 @@ export class EscrowTokenContractTest { blacklisted!: AccountWallet; aztecNode!: AztecNode; + pxe!: PXE; constructor(testName: string) { this.logger = createDebugLogger(`aztec:e2e_escrowable_token_contract:${testName}`); @@ -124,6 +126,7 @@ export class EscrowTokenContractTest { this.accounts = await pxe.getRegisteredAccounts(); this.wallets.forEach((w, i) => this.logger.verbose(`Wallet ${i} address: ${w.getAddress()}`)); this.aztecNode = aztecNode; + this.pxe = pxe; }, ); diff --git a/yarn-project/pxe/src/pxe_service/pxe_service.ts b/yarn-project/pxe/src/pxe_service/pxe_service.ts index 375a41e177f0..c04d586cc11c 100644 --- a/yarn-project/pxe/src/pxe_service/pxe_service.ts +++ b/yarn-project/pxe/src/pxe_service/pxe_service.ts @@ -317,7 +317,6 @@ export class PXEService implements PXE { } for (const nonce of nonces) { - console.log(`GO FUCK YOURSELF, `, note); const { innerNoteHash, siloedNoteHash, innerNullifier } = await this.simulator.computeNoteHashAndNullifier( note.contractAddress, nonce,