diff --git a/.circleci/config.yml b/.circleci/config.yml index 9d9c26a0842..7d83ab35e40 100644 --- a/.circleci/config.yml +++ b/.circleci/config.yml @@ -833,6 +833,18 @@ jobs: command: ./scripts/cond_run_script end-to-end $JOB_NAME ./scripts/run_tests_local e2e_pending_commitments_contract.test.ts working_directory: yarn-project/end-to-end + e2e-ordering: + machine: + image: ubuntu-2004:202010-01 + resource_class: large + steps: + - *checkout + - *setup_env + - run: + name: "Test" + command: ./scripts/cond_run_script end-to-end $JOB_NAME ./scripts/run_tests_local e2e_ordering.test.ts + working_directory: yarn-project/end-to-end + uniswap-trade-on-l1-from-l2: machine: image: ubuntu-2004:202010-01 @@ -1312,6 +1324,7 @@ workflows: - e2e-account-contracts: *e2e_test - e2e-escrow-contract: *e2e_test - e2e-pending-commitments-contract: *e2e_test + - e2e-ordering: *e2e_test - uniswap-trade-on-l1-from-l2: *e2e_test - integration-l1-publisher: *e2e_test - integration-archiver-l1-to-l2: *e2e_test @@ -1338,6 +1351,7 @@ workflows: - e2e-account-contracts - e2e-escrow-contract - e2e-pending-commitments-contract + - e2e-ordering - uniswap-trade-on-l1-from-l2 - integration-l1-publisher - integration-archiver-l1-to-l2 diff --git a/yarn-project/acir-simulator/src/public/index.test.ts b/yarn-project/acir-simulator/src/public/index.test.ts index bd22df610fd..cf45331d690 100644 --- a/yarn-project/acir-simulator/src/public/index.test.ts +++ b/yarn-project/acir-simulator/src/public/index.test.ts @@ -199,7 +199,7 @@ describe('ACIR public execution simulator', () => { ); const childContractAddress = AztecAddress.random(); - const childValueFn = ChildContractAbi.functions.find(f => f.name === 'pubValue')!; + const childValueFn = ChildContractAbi.functions.find(f => f.name === 'pubGetValue')!; const childValueFnSelector = generateFunctionSelector(childValueFn.name, childValueFn.parameters); const initialValue = 3n; diff --git a/yarn-project/aztec.js/src/utils/abi_types.ts b/yarn-project/aztec.js/src/utils/abi_types.ts index bc99ae735d8..fdebe26003a 100644 --- a/yarn-project/aztec.js/src/utils/abi_types.ts +++ b/yarn-project/aztec.js/src/utils/abi_types.ts @@ -1,4 +1,4 @@ import { Fr } from '@aztec/circuits.js'; /** Any type that can be converted into a field for a contract call. */ -export type FieldLike = Fr | bigint | number | { /** Converts to field */ toField: () => Fr }; +export type FieldLike = Fr | Buffer | bigint | number | { /** Converts to field */ toField: () => Fr }; diff --git a/yarn-project/end-to-end/src/e2e_2_rpc_servers.test.ts b/yarn-project/end-to-end/src/e2e_2_rpc_servers.test.ts index 7d6ecc9b9f5..64017816f1d 100644 --- a/yarn-project/end-to-end/src/e2e_2_rpc_servers.test.ts +++ b/yarn-project/end-to-end/src/e2e_2_rpc_servers.test.ts @@ -189,7 +189,7 @@ describe('e2e_2_rpc_servers', () => { const newValueToSet = 256n; const childContractWithWalletB = await ChildContract.at(childAddress, walletB); - const tx = childContractWithWalletB.methods.pubStoreValue(newValueToSet).send({ origin: userB.address }); + const tx = childContractWithWalletB.methods.pubIncValue(newValueToSet).send({ origin: userB.address }); await tx.isMined({ interval: 0.1 }); const receipt = await tx.getReceipt(); diff --git a/yarn-project/end-to-end/src/e2e_account_contracts.test.ts b/yarn-project/end-to-end/src/e2e_account_contracts.test.ts index 1093ac91bb0..d74da4ad91c 100644 --- a/yarn-project/end-to-end/src/e2e_account_contracts.test.ts +++ b/yarn-project/end-to-end/src/e2e_account_contracts.test.ts @@ -47,7 +47,7 @@ function itShouldBehaveLikeAnAccountContract(getAccountContract: (encryptionKey: it('calls a public function', async () => { const { logger, aztecRpcServer } = context; logger('Calling public function...'); - const tx = child.methods.pubStoreValue(42).send(); + const tx = child.methods.pubIncValue(42).send(); expect(await tx.isMined({ interval: 0.1 })).toBeTruthy(); expect(toBigInt((await aztecRpcServer.getPublicStorageAt(child.address, new Fr(1)))!)).toEqual(42n); }, 60_000); diff --git a/yarn-project/end-to-end/src/e2e_nested_contract.test.ts b/yarn-project/end-to-end/src/e2e_nested_contract.test.ts index ff152ecec1e..54f4bd9d918 100644 --- a/yarn-project/end-to-end/src/e2e_nested_contract.test.ts +++ b/yarn-project/end-to-end/src/e2e_nested_contract.test.ts @@ -73,7 +73,7 @@ describe('e2e_nested_contract', () => { it('performs public nested calls', async () => { const tx = parentContract.methods - .pubEntryPoint(childContract.address, Fr.fromBuffer(childContract.methods.pubValue.selector), 42n) + .pubEntryPoint(childContract.address, Fr.fromBuffer(childContract.methods.pubGetValue.selector), 42n) .send({ origin: sender }); await tx.isMined({ interval: 0.1 }); @@ -84,7 +84,7 @@ describe('e2e_nested_contract', () => { it('enqueues a single public call', async () => { const tx = parentContract.methods - .enqueueCallToChild(childContract.address, Fr.fromBuffer(childContract.methods.pubStoreValue.selector), 42n) + .enqueueCallToChild(childContract.address, Fr.fromBuffer(childContract.methods.pubIncValue.selector), 42n) .send({ origin: sender }); await tx.isMined({ interval: 0.1 }); @@ -101,7 +101,7 @@ describe('e2e_nested_contract', () => { const tx = parentContract.methods .enqueueCallToChildTwice( addressToField(childContract.address), - Fr.fromBuffer(childContract.methods.pubStoreValue.selector).value, + Fr.fromBuffer(childContract.methods.pubIncValue.selector).value, 42n, ) .send({ origin: sender }); @@ -117,7 +117,7 @@ describe('e2e_nested_contract', () => { const tx = parentContract.methods .enqueueCallToPubEntryPoint( childContract.address, - Fr.fromBuffer(childContract.methods.pubStoreValue.selector), + Fr.fromBuffer(childContract.methods.pubIncValue.selector), 42n, ) .send({ origin: sender }); @@ -136,7 +136,7 @@ describe('e2e_nested_contract', () => { const tx = parentContract.methods .enqueueCallsToPubEntryPoint( childContract.address, - Fr.fromBuffer(childContract.methods.pubStoreValue.selector), + Fr.fromBuffer(childContract.methods.pubIncValue.selector), 42n, ) .send({ origin: sender }); @@ -156,7 +156,7 @@ describe('e2e_nested_contract', () => { const tx = parentContract.methods .pubEntryPointTwice( addressToField(childContract.address), - Fr.fromBuffer(childContract.methods.pubStoreValue.selector).value, + Fr.fromBuffer(childContract.methods.pubIncValue.selector).value, 42n, ) .send({ origin: sender }); diff --git a/yarn-project/end-to-end/src/e2e_ordering.test.ts b/yarn-project/end-to-end/src/e2e_ordering.test.ts new file mode 100644 index 00000000000..971f22f3ec8 --- /dev/null +++ b/yarn-project/end-to-end/src/e2e_ordering.test.ts @@ -0,0 +1,66 @@ +// Test suite for testing proper ordering of side effects +import { AztecNodeService } from '@aztec/aztec-node'; +import { AztecRPCServer } from '@aztec/aztec-rpc'; +import { BatchCall, 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, FunctionCall, L2BlockL2Logs } from '@aztec/types'; + +import { setup } from './fixtures/utils.js'; + +// See https://github.com/AztecProtocol/aztec-packages/issues/1601 +describe('e2e_ordering', () => { + let aztecNode: AztecNodeService | undefined; + let aztecRpcServer: AztecRPC; + let wallet: Wallet; + + beforeEach(async () => { + ({ aztecNode, aztecRpcServer, wallet } = await setup()); + }, 100_000); + + afterEach(async () => { + await aztecNode?.stop(); + if (aztecRpcServer instanceof AztecRPCServer) { + await aztecRpcServer?.stop(); + } + }); + + describe('with parent and child contract', () => { + let parent: ParentContract; + let child: ChildContract; + + beforeEach(async () => { + parent = await ParentContract.deploy(wallet).send().deployed(); + child = await ChildContract.deploy(wallet).send().deployed(); + }); + + const getChildStoredValue = () => + aztecRpcServer.getPublicStorageAt(child.address, new Fr(1)).then(x => toBigInt(x!)); + + // Fails since current value at the end of execution is 10, not 20 + it.skip('orders public function execution requests when nested call is last', async () => { + const actions: FunctionCall[] = [ + child.methods.pubSetValue(10).request(), + parent.methods.enqueueCallToChild(child.address, child.methods.pubSetValue.selector, 20).request(), + ]; + + await new BatchCall(wallet, actions).send().wait(); + expect(await getChildStoredValue()).toEqual(20n); + const logs = await aztecRpcServer.getUnencryptedLogs(1, 10).then(L2BlockL2Logs.unrollLogs); + expect(logs).toEqual([[10], [20]].map(Buffer.from)); + }); + + it('orders public function execution requests when nested call is first', async () => { + const actions: FunctionCall[] = [ + parent.methods.enqueueCallToChild(child.address, child.methods.pubSetValue.selector, 10).request(), + child.methods.pubSetValue(20).request(), + ]; + + await new BatchCall(wallet, actions).send().wait(); + expect(await getChildStoredValue()).toEqual(20n); + const logs = await aztecRpcServer.getUnencryptedLogs(1, 10).then(L2BlockL2Logs.unrollLogs); + expect(logs).toEqual([[10], [20]].map(Buffer.from)); + }); + }); +}); diff --git a/yarn-project/foundation/src/abi/encoder.ts b/yarn-project/foundation/src/abi/encoder.ts index acf0a41aad1..5966f435af9 100644 --- a/yarn-project/foundation/src/abi/encoder.ts +++ b/yarn-project/foundation/src/abi/encoder.ts @@ -27,7 +27,9 @@ class ArgumentEncoder { } else if (typeof arg === 'bigint') { this.flattened.push(new Fr(arg)); } else if (typeof arg === 'object') { - if (typeof arg.toField === 'function') { + if (Buffer.isBuffer(arg)) { + this.flattened.push(Fr.fromBuffer(arg)); + } else if (typeof arg.toField === 'function') { this.flattened.push(arg.toField()); } else if (arg instanceof Fr) { this.flattened.push(arg); 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 56e2a1e87e0..7eb6a969bd9 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 @@ -10,6 +10,7 @@ contract Child { PublicContext }; use crate::storage::Storage; + use dep::aztec::oracle::logs::emit_unencrypted_log; fn constructor( inputs: PrivateContextInputs, @@ -32,7 +33,7 @@ contract Child { } // Returns base_value + 42. - open fn pubValue(inputs: PublicContextInputs, base_value: Field) -> pub abi::PublicCircuitPublicInputs { + 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(); @@ -41,20 +42,26 @@ contract Child { context.finish() } - // Increments `current_value` by `new_value` and returns `new_value` + 1. - open fn pubStoreValue(inputs: PublicContextInputs, new_value: Field) -> pub abi::PublicCircuitPublicInputs { + // Sets `current_value` to `new_value` + open fn pubSetValue(inputs: PublicContextInputs, new_value: Field) -> pub abi::PublicCircuitPublicInputs { + let mut context = PublicContext::new(inputs, abi::hash_args([new_value])); + + let storage = Storage::init(); + storage.current_value.write(new_value); + let _hash = emit_unencrypted_log(new_value); + context.return_values.push(new_value); + context.finish() + } + + // Increments `current_value` by `new_value` + open fn pubIncValue(inputs: PublicContextInputs, new_value: Field) -> pub abi::PublicCircuitPublicInputs { let mut context = PublicContext::new(inputs, abi::hash_args([new_value])); let storage = Storage::init(); let old_value = storage.current_value.read(); - // Compiler complains if we don't assign the result to anything - let _void1 = storage.current_value.write(old_value + new_value); - // Compiler fails with "we do not allow private ABI inputs to be returned as public outputs" if we try to - // return new_value as-is, but then it also complains if we add `pub` to `new_value` in the args, so we - // just assign it to another variable and tweak it so it's not the same value, and the compiler is happy. - let return_value = new_value + 1; - - context.return_values.push(return_value); + storage.current_value.write(old_value + new_value); + let _hash = emit_unencrypted_log(new_value); + context.return_values.push(new_value); context.finish() } } diff --git a/yarn-project/sequencer-client/src/sequencer/public_processor.ts b/yarn-project/sequencer-client/src/sequencer/public_processor.ts index eacdac37579..8574a9083fd 100644 --- a/yarn-project/sequencer-client/src/sequencer/public_processor.ts +++ b/yarn-project/sequencer-client/src/sequencer/public_processor.ts @@ -138,8 +138,7 @@ export class PublicProcessor { this.log(`Executing enqueued public calls for tx ${await tx.getTxHash()}`); if (!tx.enqueuedPublicFunctionCalls) throw new Error(`Missing preimages for enqueued public calls`); - // We execute the requests in order, which means reversing the input as the stack pops from the end of the array - const executionStack: (PublicExecution | PublicExecutionResult)[] = [...tx.enqueuedPublicFunctionCalls].reverse(); + const executionStack: (PublicExecution | PublicExecutionResult)[] = [...tx.enqueuedPublicFunctionCalls]; let kernelOutput = tx.data; let kernelProof = tx.proof;