diff --git a/noir-projects/noir-contracts/contracts/import_test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/import_test_contract/src/main.nr index 92636928014..bd11a2ac560 100644 --- a/noir-projects/noir-contracts/contracts/import_test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/import_test_contract/src/main.nr @@ -40,20 +40,20 @@ contract ImportTest { Test::at(target).get_this_address().call(&mut context) } - // Calls the create_nullifier_public on the Test contract at the target address + // Calls the emit_nullifier_public on the Test contract at the target address // Used for testing calling an open function // See yarn-project/end-to-end/src/e2e_nested_contract.test.ts #[aztec(private)] fn call_open_fn(target: AztecAddress) { - Test::at(target).create_nullifier_public(1, 2).enqueue(&mut context); + Test::at(target).emit_nullifier_public(1).enqueue(&mut context); } - // Calls the create_nullifier_public on the Test contract at the target address + // Calls the emit_nullifier_public on the Test contract at the target address // Used for testing calling an open function from another open function // See yarn-project/end-to-end/src/e2e_nested_contract.test.ts #[aztec(public)] fn pub_call_open_fn(target: AztecAddress) { - Test::at(target).create_nullifier_public(1, 2).call(&mut context); + Test::at(target).emit_nullifier_public(1).call(&mut context); } } diff --git a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr index 16f8cfe19c9..baa3a451c44 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -226,12 +226,8 @@ contract Test { // Purely exists for testing #[aztec(public)] - fn create_nullifier_public(amount: Field, secret_hash: Field) { - // Create a commitment to the amount - let note = DummyNote::new(amount, secret_hash); - - // Public oracle call to emit new commitment. - context.push_new_nullifier(note.get_commitment(), 0); + fn emit_nullifier_public(nullifier: Field) { + context.push_new_nullifier(nullifier, 0); } // Forcefully emits a nullifier (for testing purposes) diff --git a/yarn-project/end-to-end/package.json b/yarn-project/end-to-end/package.json index 8ed6dcfc2c2..971985b2e8f 100644 --- a/yarn-project/end-to-end/package.json +++ b/yarn-project/end-to-end/package.json @@ -61,6 +61,7 @@ "get-port": "^7.1.0", "glob": "^10.3.10", "jest": "^29.5.0", + "jest-extended": "^4.0.2", "jest-mock-extended": "^3.0.5", "koa": "^2.14.2", "koa-static": "^5.0.0", @@ -89,6 +90,7 @@ "@types/jest": "^29.5.0", "concurrently": "^7.6.0", "jest": "^29.5.0", + "jest-extended": "^4.0.2", "ts-node": "^10.9.1", "typescript": "^5.0.4" }, @@ -103,6 +105,9 @@ }, "jest": { "slowTestThreshold": 300, + "setupFilesAfterEnv": [ + "jest-extended/all" + ], "extensionsToTreatAsEsm": [ ".ts" ], diff --git a/yarn-project/end-to-end/src/e2e_block_building.test.ts b/yarn-project/end-to-end/src/e2e_block_building.test.ts index fce384281b8..0688b129d9d 100644 --- a/yarn-project/end-to-end/src/e2e_block_building.test.ts +++ b/yarn-project/end-to-end/src/e2e_block_building.test.ts @@ -2,24 +2,21 @@ import { getSchnorrAccount } from '@aztec/accounts/schnorr'; import { type AztecAddress, type AztecNode, - BatchCall, ContractDeployer, ContractFunctionInteraction, type DebugLogger, Fr, type PXE, - type SentTx, - type TxReceipt, - TxStatus, type Wallet, deriveKeys, } from '@aztec/aztec.js'; import { times } from '@aztec/foundation/collection'; -import { pedersenHash } from '@aztec/foundation/crypto'; import { StatefulTestContractArtifact } from '@aztec/noir-contracts.js'; import { TestContract } from '@aztec/noir-contracts.js/Test'; import { TokenContract } from '@aztec/noir-contracts.js/Token'; +import 'jest-extended'; + import { TaggedNote } from '../../circuit-types/src/logs/l1_note_payload/tagged_note.js'; import { setup } from './fixtures/utils.js'; @@ -117,6 +114,9 @@ describe('e2e_block_building', () => { describe('double-spends', () => { let contract: TestContract; let teardown: () => Promise; + // TODO(https://github.com/AztecProtocol/aztec-packages/issues/5818): clean up + // Under current public, we expect 'dropped', under the AVM, we expect 'reverted'. + const DUPLICATE_NULLIFIER_ERROR = /dropped|reverted/; beforeAll(async () => { ({ teardown, pxe, logger, wallet: owner } = await setup(1)); @@ -127,63 +127,104 @@ describe('e2e_block_building', () => { afterAll(() => teardown()); // Regressions for https://github.com/AztecProtocol/aztec-packages/issues/2502 - describe('in the same block', () => { - it('drops tx with private nullifier already emitted on the same block', async () => { + // Note that the order in which the TX are processed is not guaranteed. + describe('in the same block, different tx', () => { + it('private <-> private', async () => { const nullifier = Fr.random(); - const calls = times(2, () => contract.methods.emit_nullifier(nullifier)); - for (const call of calls) { - await call.prove(); - } - const [tx1, tx2] = calls.map(call => call.send()); - await expectXorTx(tx1, tx2); + const txs = await sendAndWait([ + contract.methods.emit_nullifier(nullifier), + contract.methods.emit_nullifier(nullifier), + ]); + + // One transaction should succeed, the other should fail, but in any order. + expect(txs).toIncludeSameMembers([ + { status: 'fulfilled', value: expect.anything() }, + { + status: 'rejected', + reason: expect.objectContaining({ message: expect.stringMatching(DUPLICATE_NULLIFIER_ERROR) }), + }, + ]); }); - it('drops tx with public nullifier already emitted on the same block', async () => { - const secret = Fr.random(); - const calls = times(2, () => contract.methods.create_nullifier_public(140n, secret)); - for (const call of calls) { - await call.prove(); - } - const [tx1, tx2] = calls.map(call => call.send()); - await expectXorTx(tx1, tx2); + it('public -> public', async () => { + const nullifier = Fr.random(); + const txs = await sendAndWait([ + contract.methods.emit_nullifier_public(nullifier), + contract.methods.emit_nullifier_public(nullifier), + ]); + + // One transaction should succeed, the other should fail, but in any order. + expect(txs).toIncludeSameMembers([ + { status: 'fulfilled', value: expect.anything() }, + { + status: 'rejected', + reason: expect.objectContaining({ message: expect.stringMatching(DUPLICATE_NULLIFIER_ERROR) }), + }, + ]); }); - it('drops tx with two equal nullifiers', async () => { + it('private -> public', async () => { const nullifier = Fr.random(); - const calls = times(2, () => contract.methods.emit_nullifier(nullifier).request()); - await expect(new BatchCall(owner, calls).send().wait()).rejects.toThrow(/dropped/); + const txs = await sendAndWait([ + contract.methods.emit_nullifier(nullifier), + contract.methods.emit_nullifier_public(nullifier), + ]); + + // One transaction should succeed, the other should fail, but in any order. + expect(txs).toIncludeSameMembers([ + { status: 'fulfilled', value: expect.anything() }, + { + status: 'rejected', + reason: expect.objectContaining({ message: expect.stringMatching(DUPLICATE_NULLIFIER_ERROR) }), + }, + ]); }); - it('drops tx with private nullifier already emitted from public on the same block', async () => { - const secret = Fr.random(); - // See yarn-project/simulator/src/public/index.test.ts 'Should be able to create a nullifier from the public context' - const emittedPublicNullifier = pedersenHash([new Fr(140), secret]); - - const calls = [ - contract.methods.create_nullifier_public(140n, secret), - contract.methods.emit_nullifier(emittedPublicNullifier), - ]; - - for (const call of calls) { - await call.prove(); - } - const [tx1, tx2] = calls.map(call => call.send()); - await expectXorTx(tx1, tx2); + it('public -> private', async () => { + const nullifier = Fr.random(); + const txs = await sendAndWait([ + contract.methods.emit_nullifier_public(nullifier), + contract.methods.emit_nullifier(nullifier), + ]); + + // One transaction should succeed, the other should fail, but in any order. + expect(txs).toIncludeSameMembers([ + { status: 'fulfilled', value: expect.anything() }, + { + status: 'rejected', + reason: expect.objectContaining({ message: expect.stringMatching(DUPLICATE_NULLIFIER_ERROR) }), + }, + ]); }); }); describe('across blocks', () => { - it('drops a tx that tries to spend a nullifier already emitted on a previous block', async () => { - const secret = Fr.random(); - const emittedPublicNullifier = pedersenHash([new Fr(140), secret]); - - await expect(contract.methods.create_nullifier_public(140n, secret).send().wait()).resolves.toEqual( - expect.objectContaining({ - status: TxStatus.MINED, - }), + it('private -> private', async () => { + const nullifier = Fr.random(); + await contract.methods.emit_nullifier(nullifier).send().wait(); + await expect(contract.methods.emit_nullifier(nullifier).send().wait()).rejects.toThrow('dropped'); + }); + + it('public -> public', async () => { + const nullifier = Fr.random(); + await contract.methods.emit_nullifier_public(nullifier).send().wait(); + await expect(contract.methods.emit_nullifier_public(nullifier).send().wait()).rejects.toThrow( + DUPLICATE_NULLIFIER_ERROR, ); + }); + + it('private -> public', async () => { + const nullifier = Fr.random(); + await contract.methods.emit_nullifier(nullifier).send().wait(); + await expect(contract.methods.emit_nullifier_public(nullifier).send().wait()).rejects.toThrow( + DUPLICATE_NULLIFIER_ERROR, + ); + }); - await expect(contract.methods.emit_nullifier(emittedPublicNullifier).send().wait()).rejects.toThrow(/dropped/); + it('public -> private', async () => { + const nullifier = Fr.random(); + await contract.methods.emit_nullifier_public(nullifier).send().wait(); + await expect(contract.methods.emit_nullifier(nullifier).send().wait()).rejects.toThrow('dropped'); }); }); }); @@ -243,17 +284,12 @@ describe('e2e_block_building', () => { }); }); -/** - * Checks that only one of the two provided transactions succeeds. - * @param tx1 - A transaction. - * @param tx2 - Another transaction. - */ -async function expectXorTx(tx1: SentTx, tx2: SentTx) { - const receipts = await Promise.allSettled([tx1.wait(), tx2.wait()]); - const succeeded = receipts.find((r): r is PromiseSettledResult => r.status === 'fulfilled'); - const failed = receipts.find((r): r is PromiseRejectedResult => r.status === 'rejected'); - - expect(succeeded).toBeDefined(); - expect(failed).toBeDefined(); - expect((failed?.reason as Error).message).toMatch(/dropped/); +async function sendAndWait(calls: ContractFunctionInteraction[]) { + return await Promise.allSettled( + calls + // First we send them all. + .map(call => call.send()) + // Onlt then we wait. + .map(p => p.wait()), + ); } diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts index 8d886932f80..9a4c85bdae7 100644 --- a/yarn-project/simulator/src/avm/journal/journal.ts +++ b/yarn-project/simulator/src/avm/journal/journal.ts @@ -253,7 +253,9 @@ export class AvmPersistableStateManager { */ public async writeNullifier(storageAddress: Fr, nullifier: Fr) { // TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit - this.transitionalExecutionResult.newNullifiers.push(new Nullifier(nullifier, this.trace.accessCounter, Fr.ZERO)); + this.transitionalExecutionResult.newNullifiers.push( + new Nullifier(nullifier, this.trace.accessCounter, /*noteHash=*/ Fr.ZERO), + ); this.log.debug(`nullifiers(${storageAddress}) += ${nullifier}.`); // Cache pending nullifiers for later access diff --git a/yarn-project/simulator/src/public/index.test.ts b/yarn-project/simulator/src/public/index.test.ts index 73b6312e171..76131633890 100644 --- a/yarn-project/simulator/src/public/index.test.ts +++ b/yarn-project/simulator/src/public/index.test.ts @@ -405,10 +405,11 @@ describe('ACIR public execution simulator', () => { it('Should be able to create a nullifier from the public context', async () => { const createNullifierPublicArtifact = TestContractArtifact.functions.find( - f => f.name === 'create_nullifier_public', + f => f.name === 'emit_nullifier_public', )!; - const args = encodeArguments(createNullifierPublicArtifact, params); + const nullifier = new Fr(1234); + const args = encodeArguments(createNullifierPublicArtifact, [nullifier]); const callContext = makeCallContext(contractAddress); @@ -417,11 +418,7 @@ describe('ACIR public execution simulator', () => { const execution: PublicExecution = { contractAddress, functionData, args, callContext }; const result = await simulate(execution, globalVariables); - // Assert the l2 to l1 message was created - expect(result.newNullifiers.length).toEqual(1); - - const expectedNewMessageValue = pedersenHash(params); - expect(result.newNullifiers[0].value).toEqual(expectedNewMessageValue); + expect(result.newNullifiers).toEqual([expect.objectContaining({ value: nullifier })]); }); describe('L1 to L2 messages', () => { diff --git a/yarn-project/yarn.lock b/yarn-project/yarn.lock index f3871f72580..f8cd6fc6dc7 100644 --- a/yarn-project/yarn.lock +++ b/yarn-project/yarn.lock @@ -396,6 +396,7 @@ __metadata: get-port: ^7.1.0 glob: ^10.3.10 jest: ^29.5.0 + jest-extended: ^4.0.2 jest-mock-extended: ^3.0.5 koa: ^2.14.2 koa-static: ^5.0.0 @@ -9048,7 +9049,7 @@ __metadata: languageName: node linkType: hard -"jest-diff@npm:^29.7.0": +"jest-diff@npm:^29.0.0, jest-diff@npm:^29.7.0": version: 29.7.0 resolution: "jest-diff@npm:29.7.0" dependencies: @@ -9096,7 +9097,22 @@ __metadata: languageName: node linkType: hard -"jest-get-type@npm:^29.6.3": +"jest-extended@npm:^4.0.2": + version: 4.0.2 + resolution: "jest-extended@npm:4.0.2" + dependencies: + jest-diff: ^29.0.0 + jest-get-type: ^29.0.0 + peerDependencies: + jest: ">=27.2.5" + peerDependenciesMeta: + jest: + optional: true + checksum: afdc255eec7caa173f9e805e94562273d8b8aa4c7ab9b396668f018c18ea5236270a6ac499ca84b8c60e90ccbe9ccb4aebf998daef13aec9542c426df1df6079 + languageName: node + linkType: hard + +"jest-get-type@npm:^29.0.0, jest-get-type@npm:^29.6.3": version: 29.6.3 resolution: "jest-get-type@npm:29.6.3" checksum: 88ac9102d4679d768accae29f1e75f592b760b44277df288ad76ce5bf038c3f5ce3719dea8aa0f035dac30e9eb034b848ce716b9183ad7cc222d029f03e92205