test: add tests that check ordering of public state updates

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),

yarn-project/end-to-end/src/e2e_ordering.test.ts

@@ -3,9 +3,10 @@ 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 } from '@aztec/types';
+import { AztecRPC, L2BlockL2Logs, TxStatus } from '@aztec/types';
 
 import { setup } from './fixtures/utils.js';
 
@@ -14,9 +15,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 +53,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,13 +77,48 @@ 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
         },
       );
     });

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()
+    }
 }