From be2d7c9c6d302ef564f6f985775a518cc831e3c5 Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Fri, 18 Aug 2023 18:00:12 +0000 Subject: [PATCH 1/2] test that checks public state update order --- .../src/client/execution_result.ts | 3 +- .../end-to-end/src/e2e_ordering.test.ts | 62 ++++++++++++++++--- .../src/contracts/child_contract/src/main.nr | 32 +++++++++- 3 files changed, 87 insertions(+), 10 deletions(-) diff --git a/yarn-project/acir-simulator/src/client/execution_result.ts b/yarn-project/acir-simulator/src/client/execution_result.ts index f4b07dfff6b..80aa97d8fbf 100644 --- a/yarn-project/acir-simulator/src/client/execution_result.ts +++ b/yarn-project/acir-simulator/src/client/execution_result.ts @@ -100,7 +100,8 @@ export function collectUnencryptedLogs(execResult: ExecutionResult): FunctionL2L * @returns All enqueued public function calls. */ export function collectEnqueuedPublicFunctionCalls(execResult: ExecutionResult): PublicCallRequest[] { - // without the reverse sort, the logs will be in a queue like fashion which is wrong as the kernel processes it like a stack. + // without the reverse sort, the logs will be in a queue like fashion which is wrong + // as the kernel processes it like a stack, popping items off and pushing them to output return [ ...execResult.enqueuedPublicFunctionCalls, ...[...execResult.nestedExecutions].flatMap(collectEnqueuedPublicFunctionCalls), diff --git a/yarn-project/end-to-end/src/e2e_ordering.test.ts b/yarn-project/end-to-end/src/e2e_ordering.test.ts index 1978797c968..94ce78dd90a 100644 --- a/yarn-project/end-to-end/src/e2e_ordering.test.ts +++ b/yarn-project/end-to-end/src/e2e_ordering.test.ts @@ -5,7 +5,9 @@ import { Wallet } from '@aztec/aztec.js'; import { Fr } from '@aztec/circuits.js'; import { toBigInt } from '@aztec/foundation/serialize'; import { ChildContract, ParentContract } from '@aztec/noir-contracts/types'; -import { AztecRPC, L2BlockL2Logs } from '@aztec/types'; +import { AztecRPC, L2BlockL2Logs, TxStatus } from '@aztec/types'; + +import { toBigIntBE } from '@aztec/foundation/bigint-buffer'; import { setup } from './fixtures/utils.js'; @@ -14,9 +16,19 @@ describe('e2e_ordering', () => { let aztecNode: AztecNodeService | undefined; let aztecRpcServer: AztecRPC; let wallet: Wallet; + //let logger: DebugLogger; + + const expectLogsFromLastBlockToBe = async (logMessages: bigint[]) => { + const l2BlockNum = await aztecRpcServer.getBlockNum(); + const unencryptedLogs = await aztecRpcServer.getUnencryptedLogs(l2BlockNum, 1); + const unrolledLogs = L2BlockL2Logs.unrollLogs(unencryptedLogs); + const bigintLogs = unrolledLogs.map((log: Buffer) => toBigIntBE(log)); + + expect(bigintLogs).toStrictEqual(logMessages); + }; beforeEach(async () => { - ({ aztecNode, aztecRpcServer, wallet } = await setup()); + ({ aztecNode, aztecRpcServer, wallet, /*logger*/ } = await setup()); }, 100_000); afterEach(async () => { @@ -42,8 +54,8 @@ describe('e2e_ordering', () => { const directValue = 20n; const expectedOrders = { - enqueueCallsToChildWithNestedFirst: [nestedValue, directValue], - enqueueCallsToChildWithNestedLast: [directValue, nestedValue], + enqueueCallsToChildWithNestedFirst: [nestedValue, directValue] as bigint[], + enqueueCallsToChildWithNestedLast: [directValue, nestedValue] as bigint[], } as const; it.each(['enqueueCallsToChildWithNestedFirst', 'enqueueCallsToChildWithNestedLast'] as const)( @@ -66,15 +78,49 @@ describe('e2e_ordering', () => { expect(enqueuedPublicCalls.map(c => c.args[0].toBigInt())).toEqual([...expectedOrder].reverse()); // Logs are emitted in the expected order - const logs = await aztecRpcServer.getUnencryptedLogs(1, 10).then(L2BlockL2Logs.unrollLogs); - const expectedLogs = expectedOrder.map(x => Buffer.from([Number(x)])); - expect(logs).toEqual(expectedLogs); + await expectLogsFromLastBlockToBe(expectedOrder); // The final value of the child is the last one set const value = await aztecRpcServer.getPublicStorageAt(child.address, new Fr(1)).then(x => toBigInt(x!)); - expect(value).toEqual(expectedOrder[1]); + expect(value).toEqual(expectedOrder[1]); // final state should match last value set }, ); }); + + describe('public state update ordering, and final state value check', () => { + const nestedValue = 10n; + const directValue = 20n; + + const expectedOrders = { + setValueTwiceWithNestedFirst: [nestedValue, directValue] as bigint[], + setValueTwiceWithNestedLast: [directValue, nestedValue] as bigint[], + } as const; + + // TODO(dbanks12): implement public state ordering, or... + // TODO(#1622): hack around this error by removing public kernel checks + // that public state updates are in valid sequence + // Full explanation: + // Setting public storage twice (first in a nested call, then directly) leads + // to an error in public kernel because it sees the public state updates + // in reverse order. More info in this thread: https://discourse.aztec.network/t/identifying-the-ordering-of-state-access-across-contract-calls/382/12#transition-counters-for-private-calls-2 + // + // Once fixed, re-include the `setValueTwiceWithNestedFirst` test + //it.each(['setValueTwiceWithNestedFirst', 'setValueTwiceWithNestedLast'] as const)( + it.each(['setValueTwiceWithNestedLast'] as const)( + 'orders public state updates in %s (and ensures final state value is correct)', + async method => { + const expectedOrder = expectedOrders[method]; + + const tx = child.methods[method]().send(); + const receipt = await tx.wait(); + expect(receipt.status).toBe(TxStatus.MINED); + + // Logs are emitted in the expected order + await expectLogsFromLastBlockToBe(expectedOrder); + + const value = await aztecRpcServer.getPublicStorageAt(child.address, new Fr(1)).then(x => toBigInt(x!)); + expect(value).toEqual(expectedOrder[1]); // final state should match last value set + }); + }); }); }); diff --git a/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr b/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr index 7eb6a969bd9..222090acc6c 100644 --- a/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr +++ b/yarn-project/noir-contracts/src/contracts/child_contract/src/main.nr @@ -35,7 +35,7 @@ contract Child { // Returns base_value + 42. open fn pubGetValue(inputs: PublicContextInputs, base_value: Field) -> pub abi::PublicCircuitPublicInputs { let mut context = PublicContext::new(inputs, abi::hash_args([base_value])); - + let returnValue = base_value + context.chain_id() + context.version() + context.block_number() + context.timestamp(); context.return_values.push(returnValue); @@ -64,4 +64,34 @@ contract Child { context.return_values.push(new_value); context.finish() } + + open fn setValueTwiceWithNestedFirst( + inputs: PublicContextInputs, + ) -> pub abi::PublicCircuitPublicInputs { + let mut context = PublicContext::new(inputs, abi::hash_args([])); + + let pubSetValueSelector = 0x5b0f91b0; + let _ret = context.call_public_function(context.this_address(), pubSetValueSelector, [10]); + + let storage = Storage::init(); + storage.current_value.write(20); + let _hash = emit_unencrypted_log(20); + + context.finish() + } + + open fn setValueTwiceWithNestedLast( + inputs: PublicContextInputs, + ) -> pub abi::PublicCircuitPublicInputs { + let mut context = PublicContext::new(inputs, abi::hash_args([])); + + let storage = Storage::init(); + storage.current_value.write(20); + let _hash = emit_unencrypted_log(20); + + let pubSetValueSelector = 0x5b0f91b0; + let _ret = context.call_public_function(context.this_address(), pubSetValueSelector, [10]); + + context.finish() + } } From 15b546ea729e9939f1880437c83734c4291d39a1 Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Fri, 18 Aug 2023 18:04:04 +0000 Subject: [PATCH 2/2] format --- yarn-project/end-to-end/src/e2e_ordering.test.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_ordering.test.ts b/yarn-project/end-to-end/src/e2e_ordering.test.ts index 94ce78dd90a..c512e078b56 100644 --- a/yarn-project/end-to-end/src/e2e_ordering.test.ts +++ b/yarn-project/end-to-end/src/e2e_ordering.test.ts @@ -3,12 +3,11 @@ import { AztecNodeService } from '@aztec/aztec-node'; import { AztecRPCServer } from '@aztec/aztec-rpc'; import { Wallet } from '@aztec/aztec.js'; import { Fr } from '@aztec/circuits.js'; +import { toBigIntBE } from '@aztec/foundation/bigint-buffer'; import { toBigInt } from '@aztec/foundation/serialize'; import { ChildContract, ParentContract } from '@aztec/noir-contracts/types'; import { AztecRPC, L2BlockL2Logs, TxStatus } from '@aztec/types'; -import { toBigIntBE } from '@aztec/foundation/bigint-buffer'; - import { setup } from './fixtures/utils.js'; // See https://github.com/AztecProtocol/aztec-packages/issues/1601 @@ -28,7 +27,7 @@ describe('e2e_ordering', () => { }; beforeEach(async () => { - ({ aztecNode, aztecRpcServer, wallet, /*logger*/ } = await setup()); + ({ aztecNode, aztecRpcServer, wallet /*, logger*/ } = await setup()); }, 100_000); afterEach(async () => { @@ -120,7 +119,8 @@ describe('e2e_ordering', () => { const value = await aztecRpcServer.getPublicStorageAt(child.address, new Fr(1)).then(x => toBigInt(x!)); expect(value).toEqual(expectedOrder[1]); // final state should match last value set - }); + }, + ); }); }); });