From b15bbb231d7e0094cd025e930732c663a990f3e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Thu, 18 Apr 2024 21:25:38 +0000 Subject: [PATCH 1/3] Refactor tests to use simulate --- .../contracts/auth_contract/src/main.nr | 21 ++------ .../contracts/test_contract/src/main.nr | 14 ++--- .../end-to-end/src/e2e_auth_contract.test.ts | 51 ++++++++----------- .../end-to-end/src/e2e_note_getter.test.ts | 26 +--------- 4 files changed, 33 insertions(+), 79 deletions(-) diff --git a/noir-projects/noir-contracts/contracts/auth_contract/src/main.nr b/noir-projects/noir-contracts/contracts/auth_contract/src/main.nr index a8025208c60..36df4cbeb45 100644 --- a/noir-projects/noir-contracts/contracts/auth_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/auth_contract/src/main.nr @@ -33,32 +33,21 @@ contract Auth { } #[aztec(public)] - fn get_authorized() { - // We emit logs because we cannot otherwise return these values - emit_unencrypted_log( - &mut context, - storage.authorized.get_current_value_in_public() - ); + fn get_authorized() -> AztecAddress { + storage.authorized.get_current_value_in_public() } #[aztec(public)] - fn get_scheduled_authorized() { - // We emit logs because we cannot otherwise return these values - emit_unencrypted_log( - &mut context, - storage.authorized.get_scheduled_value_in_public().0 - ); + fn get_scheduled_authorized() -> AztecAddress { + storage.authorized.get_scheduled_value_in_public().0 } #[aztec(private)] - fn do_private_authorized_thing(value: Field) { + fn do_private_authorized_thing() { // Reading a value from authorized in private automatically adds an extra validity condition: the base rollup // circuit will reject this tx if included in a block past the block horizon, which is as far as the circuit can // guarantee the value will not change from some historical value (due to CHANGE_AUTHORIZED_DELAY_BLOCKS). let authorized = storage.authorized.get_current_value_in_private(); assert_eq(authorized, context.msg_sender(), "caller is not authorized"); - - // We emit logs because we cannot otherwise return these values - emit_unencrypted_log_from_private(&mut context, value); } } 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 851b8e59ee6..6cb0f783a7b 100644 --- a/noir-projects/noir-contracts/contracts/test_contract/src/main.nr +++ b/noir-projects/noir-contracts/contracts/test_contract/src/main.nr @@ -96,7 +96,7 @@ contract Test { } #[aztec(private)] - fn call_get_notes(storage_slot: Field, active_or_nullified: bool) { + fn call_get_notes(storage_slot: Field, active_or_nullified: bool) -> Field { assert( storage_slot != storage.example_constant.get_storage_slot(), "this storage slot is reserved for example_constant" ); @@ -108,14 +108,11 @@ contract Test { let opt_notes: [Option; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = get_notes(&mut context, storage_slot, options); - // We can't get the return value of a private function from the outside world in an end to end test, so we - // expose it via an unecrypted log instead. - let value = opt_notes[0].unwrap().value; - emit_unencrypted_log_from_private(&mut context, value); + opt_notes[0].unwrap().value } #[aztec(private)] - fn call_get_notes_many(storage_slot: Field, active_or_nullified: bool) { + fn call_get_notes_many(storage_slot: Field, active_or_nullified: bool) -> [Field; 2] { assert( storage_slot != storage.example_constant.get_storage_slot(), "this storage slot is reserved for example_constant" ); @@ -127,10 +124,7 @@ contract Test { let opt_notes: [Option; MAX_NOTE_HASH_READ_REQUESTS_PER_CALL] = get_notes(&mut context, storage_slot, options); - // We can't get the return value of a private function from the outside world in an end to end test, so we - // expose it via an unecrypted log instead. - emit_unencrypted_log_from_private(&mut context, opt_notes[0].unwrap().value); - emit_unencrypted_log_from_private(&mut context, opt_notes[1].unwrap().value); + [opt_notes[0].unwrap().value, opt_notes[1].unwrap().value] } unconstrained fn call_view_notes(storage_slot: Field, active_or_nullified: bool) -> pub Field { diff --git a/yarn-project/end-to-end/src/e2e_auth_contract.test.ts b/yarn-project/end-to-end/src/e2e_auth_contract.test.ts index b2568365140..84967bee4d9 100644 --- a/yarn-project/end-to-end/src/e2e_auth_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_auth_contract.test.ts @@ -19,7 +19,6 @@ describe('e2e_auth_contract', () => { let contract: AuthContract; - const VALUE = 3; const DELAY = 5; beforeAll(async () => { @@ -48,40 +47,30 @@ describe('e2e_auth_contract', () => { } } - async function assertLoggedAddress(interaction: ContractFunctionInteraction, address: AztecAddress) { - const logs = await pxe.getUnencryptedLogs({ txHash: (await interaction.send().wait()).txHash }); - expect(AztecAddress.fromBuffer(logs.logs[0].log.data)).toEqual(address); - } - - async function assertLoggedNumber(interaction: ContractFunctionInteraction, value: number) { - const logs = await pxe.getUnencryptedLogs({ txHash: (await interaction.send().wait()).txHash }); - expect(Fr.fromBuffer(logs.logs[0].log.data)).toEqual(new Fr(value)); - } - it('authorized is unset initially', async () => { - await assertLoggedAddress(contract.methods.get_authorized(), AztecAddress.ZERO); + expect(await contract.methods.get_authorized().simulate()).toEqual(AztecAddress.ZERO); }); it('admin sets authorized', async () => { await contract.withWallet(admin).methods.set_authorized(authorized.getAddress()).send().wait(); - await assertLoggedAddress(contract.methods.get_scheduled_authorized(), authorized.getAddress()); + expect(await contract.methods.get_scheduled_authorized().simulate()).toEqual(authorized.getAddress()); }); it('authorized is not yet set, cannot use permission', async () => { - await assertLoggedAddress(contract.methods.get_authorized(), AztecAddress.ZERO); + expect(await contract.methods.get_authorized().simulate()).toEqual(AztecAddress.ZERO); - await expect( - contract.withWallet(authorized).methods.do_private_authorized_thing(VALUE).send().wait(), - ).rejects.toThrow('caller is not authorized'); + await expect(contract.withWallet(authorized).methods.do_private_authorized_thing().send().wait()).rejects.toThrow( + 'caller is not authorized', + ); }); it('after a while the scheduled change is effective and can be used with max block restriction', async () => { await mineBlocks(DELAY); // This gets us past the block of change - await assertLoggedAddress(contract.methods.get_authorized(), authorized.getAddress()); + expect(await contract.methods.get_authorized().simulate()).toEqual(authorized.getAddress()); - const interaction = contract.withWallet(authorized).methods.do_private_authorized_thing(VALUE); + const interaction = contract.withWallet(authorized).methods.do_private_authorized_thing(); const tx = await interaction.prove(); @@ -94,32 +83,36 @@ describe('e2e_auth_contract', () => { expect(tx.data.forRollup!.rollupValidationRequests.maxBlockNumber.isSome).toEqual(true); expect(tx.data.forRollup!.rollupValidationRequests.maxBlockNumber.value).toEqual(new Fr(expectedMaxBlockNumber)); - await assertLoggedNumber(interaction, VALUE); + expect((await interaction.send().wait()).status).toEqual('mined'); }); it('a new authorized address is set but not immediately effective, the previous one retains permissions', async () => { await contract.withWallet(admin).methods.set_authorized(other.getAddress()).send().wait(); - await assertLoggedAddress(contract.methods.get_authorized(), authorized.getAddress()); + expect(await contract.methods.get_authorized().simulate()).toEqual(authorized.getAddress()); - await assertLoggedAddress(contract.methods.get_scheduled_authorized(), other.getAddress()); + expect(await contract.methods.get_scheduled_authorized().simulate()).toEqual(other.getAddress()); - await expect(contract.withWallet(other).methods.do_private_authorized_thing(VALUE).send().wait()).rejects.toThrow( + await expect(contract.withWallet(other).methods.do_private_authorized_thing().send().wait()).rejects.toThrow( 'caller is not authorized', ); - await assertLoggedNumber(contract.withWallet(authorized).methods.do_private_authorized_thing(VALUE), VALUE); + expect((await contract.withWallet(authorized).methods.do_private_authorized_thing().send().wait()).status).toEqual( + 'mined', + ); }); it('after some time the scheduled change is made effective', async () => { await mineBlocks(DELAY); // This gets us past the block of change - await assertLoggedAddress(contract.methods.get_authorized(), other.getAddress()); + expect(await contract.methods.get_authorized().simulate()).toEqual(other.getAddress()); - await expect( - contract.withWallet(authorized).methods.do_private_authorized_thing(VALUE).send().wait(), - ).rejects.toThrow('caller is not authorized'); + await expect(contract.withWallet(authorized).methods.do_private_authorized_thing().send().wait()).rejects.toThrow( + 'caller is not authorized', + ); - await assertLoggedNumber(contract.withWallet(other).methods.do_private_authorized_thing(VALUE), VALUE); + expect((await contract.withWallet(other).methods.do_private_authorized_thing().send().wait()).status).toEqual( + 'mined', + ); }); }); diff --git a/yarn-project/end-to-end/src/e2e_note_getter.test.ts b/yarn-project/end-to-end/src/e2e_note_getter.test.ts index 54a2084be58..cb35ac8d21a 100644 --- a/yarn-project/end-to-end/src/e2e_note_getter.test.ts +++ b/yarn-project/end-to-end/src/e2e_note_getter.test.ts @@ -168,7 +168,7 @@ describe('e2e_note_getter', () => { async function assertNoteIsReturned(storageSlot: number, expectedValue: number, activeOrNullified: boolean) { const viewNotesResult = await contract.methods.call_view_notes(storageSlot, activeOrNullified).simulate(); - const getNotesResult = await callGetNotes(storageSlot, activeOrNullified); + const getNotesResult = await contract.methods.call_get_notes(storageSlot, activeOrNullified).simulate(); expect(viewNotesResult).toEqual(getNotesResult); expect(viewNotesResult).toEqual(BigInt(expectedValue)); @@ -183,28 +183,6 @@ describe('e2e_note_getter', () => { ); } - async function callGetNotes(storageSlot: number, activeOrNullified: boolean): Promise { - // call_get_notes exposes the return value via an event since we cannot use simulate() with it. - const tx = contract.methods.call_get_notes(storageSlot, activeOrNullified).send(); - await tx.wait(); - - const logs = (await tx.getUnencryptedLogs()).logs; - expect(logs.length).toBe(1); - - return toBigInt(logs[0].log.data); - } - - async function callGetNotesMany(storageSlot: number, activeOrNullified: boolean): Promise> { - // call_get_notes_many exposes the return values via event since we cannot use simulate() with it. - const tx = contract.methods.call_get_notes_many(storageSlot, activeOrNullified).send(); - await tx.wait(); - - const logs = (await tx.getUnencryptedLogs()).logs; - expect(logs.length).toBe(2); - - return [toBigInt(logs[0].log.data), toBigInt(logs[1].log.data)]; - } - describe('active note only', () => { const activeOrNullified = false; @@ -250,7 +228,7 @@ describe('e2e_note_getter', () => { const viewNotesManyResult = await contract.methods .call_view_notes_many(storageSlot, activeOrNullified) .simulate(); - const getNotesManyResult = await callGetNotesMany(storageSlot, activeOrNullified); + const getNotesManyResult = await contract.methods.call_get_notes_many(storageSlot, activeOrNullified).simulate(); // We can't be sure in which order the notes will be returned, so we simply sort them to test equality. Note // however that both view_notes and get_notes get the exact same result. From aaff5de2da03ea779af77f42d69e9fdafb367c41 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 19 Apr 2024 12:57:04 +0000 Subject: [PATCH 2/3] Fix linter --- yarn-project/end-to-end/src/e2e_note_getter.test.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/yarn-project/end-to-end/src/e2e_note_getter.test.ts b/yarn-project/end-to-end/src/e2e_note_getter.test.ts index cb35ac8d21a..4e6976a5182 100644 --- a/yarn-project/end-to-end/src/e2e_note_getter.test.ts +++ b/yarn-project/end-to-end/src/e2e_note_getter.test.ts @@ -228,7 +228,9 @@ describe('e2e_note_getter', () => { const viewNotesManyResult = await contract.methods .call_view_notes_many(storageSlot, activeOrNullified) .simulate(); - const getNotesManyResult = await contract.methods.call_get_notes_many(storageSlot, activeOrNullified).simulate(); + const getNotesManyResult = await contract.methods + .call_get_notes_many(storageSlot, activeOrNullified) + .simulate(); // We can't be sure in which order the notes will be returned, so we simply sort them to test equality. Note // however that both view_notes and get_notes get the exact same result. From 815bc00b403694e985a8fa3083e00001a0866988 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Nicol=C3=A1s=20Venturo?= Date: Fri, 19 Apr 2024 13:12:38 +0000 Subject: [PATCH 3/3] Remove extra imports --- yarn-project/end-to-end/src/e2e_auth_contract.test.ts | 2 +- yarn-project/end-to-end/src/e2e_note_getter.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_auth_contract.test.ts b/yarn-project/end-to-end/src/e2e_auth_contract.test.ts index 84967bee4d9..099885c1c9a 100644 --- a/yarn-project/end-to-end/src/e2e_auth_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_auth_contract.test.ts @@ -1,4 +1,4 @@ -import { type AccountWallet, AztecAddress, type ContractFunctionInteraction, Fr, type PXE } from '@aztec/aztec.js'; +import { type AccountWallet, AztecAddress, Fr, type PXE } from '@aztec/aztec.js'; import { AuthContract } from '@aztec/noir-contracts.js'; import { jest } from '@jest/globals'; diff --git a/yarn-project/end-to-end/src/e2e_note_getter.test.ts b/yarn-project/end-to-end/src/e2e_note_getter.test.ts index 4e6976a5182..c7deb286a92 100644 --- a/yarn-project/end-to-end/src/e2e_note_getter.test.ts +++ b/yarn-project/end-to-end/src/e2e_note_getter.test.ts @@ -1,4 +1,4 @@ -import { type AztecAddress, Comparator, Fr, type Wallet, toBigInt } from '@aztec/aztec.js'; +import { type AztecAddress, Comparator, Fr, type Wallet } from '@aztec/aztec.js'; import { DocsExampleContract, TestContract } from '@aztec/noir-contracts.js'; import { setup } from './fixtures/utils.js';