From 4676431ecf18003c6648e914effb1c3087108f0f Mon Sep 17 00:00:00 2001 From: Facundo Date: Tue, 7 May 2024 17:06:13 +0100 Subject: [PATCH] feat(avm-simulator): consider previous pending nullifiers across enqueued calls (#6188) --- .../end-to-end/src/e2e_avm_simulator.test.ts | 12 +++++++++ .../prover-client/src/mocks/test_context.ts | 4 ++- .../simulator/src/avm/journal/nullifiers.ts | 27 +++++++++++++------ .../src/public/abstract_phase_manager.ts | 7 +++++ yarn-project/simulator/src/public/executor.ts | 6 +++++ .../simulator/src/public/index.test.ts | 2 +- .../src/public/public_execution_context.ts | 3 +++ .../src/public/public_processor.test.ts | 1 + 8 files changed, 52 insertions(+), 10 deletions(-) diff --git a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts index ee24f1aad6c..6691ad45c34 100644 --- a/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts +++ b/yarn-project/end-to-end/src/e2e_avm_simulator.test.ts @@ -98,6 +98,18 @@ describe('e2e_avm_simulator', () => { tx = await avmContract.methods.assert_nullifier_exists(nullifier).send().wait(); expect(tx.status).toEqual(TxStatus.MINED); }); + + it('Emit and check in separate enqueued calls but same tx', async () => { + const nullifier = new Fr(123456); + + // This will create 1 tx with 2 public calls in it. + await new BatchCall(wallet, [ + avmContract.methods.new_nullifier(nullifier).request(), + avmContract.methods.assert_nullifier_exists(nullifier).request(), + ]) + .send() + .wait(); + }); }); }); diff --git a/yarn-project/prover-client/src/mocks/test_context.ts b/yarn-project/prover-client/src/mocks/test_context.ts index 75abb24ada6..7dcdcf6ea93 100644 --- a/yarn-project/prover-client/src/mocks/test_context.ts +++ b/yarn-project/prover-client/src/mocks/test_context.ts @@ -1,5 +1,5 @@ import { type BlockProver, type ProcessedTx, type Tx, type TxValidator } from '@aztec/circuit-types'; -import { type Gas, GlobalVariables, Header, type TxContext } from '@aztec/circuits.js'; +import { type Gas, GlobalVariables, Header, type Nullifier, type TxContext } from '@aztec/circuits.js'; import { type Fr } from '@aztec/foundation/fields'; import { type DebugLogger } from '@aztec/foundation/log'; import { openTmpStore } from '@aztec/kv-store/utils'; @@ -129,6 +129,7 @@ export class TestContext { _globalVariables: GlobalVariables, availableGas: Gas, _txContext: TxContext, + _pendingNullifiers: Nullifier[], transactionFee?: Fr, _sideEffectCounter?: number, ) => { @@ -166,6 +167,7 @@ export class TestContext { globalVariables: GlobalVariables, availableGas: Gas, txContext: TxContext, + pendingNullifiers: Nullifier[], transactionFee?: Fr, sideEffectCounter?: number, ) => Promise, diff --git a/yarn-project/simulator/src/avm/journal/nullifiers.ts b/yarn-project/simulator/src/avm/journal/nullifiers.ts index f8374f6f8a7..99d12757e60 100644 --- a/yarn-project/simulator/src/avm/journal/nullifiers.ts +++ b/yarn-project/simulator/src/avm/journal/nullifiers.ts @@ -1,3 +1,4 @@ +import { AztecAddress } from '@aztec/circuits.js'; import { siloNullifier } from '@aztec/circuits.js/hash'; import { Fr } from '@aztec/foundation/fields'; @@ -10,7 +11,7 @@ import type { CommitmentsDB } from '../../index.js'; */ export class Nullifiers { /** Cached nullifiers. */ - private cache: NullifierCache; + public cache: NullifierCache; /** Parent's nullifier cache. Checked on cache-miss. */ private readonly parentCache: NullifierCache | undefined; /** Reference to node storage. Checked on parent cache-miss. */ @@ -95,6 +96,7 @@ export class NullifierCache { * each entry being a nullifier. */ private cachePerContract: Map> = new Map(); + private siloedNullifiers: Set = new Set(); /** * Check whether a nullifier exists in the cache. @@ -104,8 +106,10 @@ export class NullifierCache { * @returns whether the nullifier is found in the cache */ public exists(storageAddress: Fr, nullifier: Fr): boolean { - const exists = this.cachePerContract.get(storageAddress.toBigInt())?.has(nullifier.toBigInt()); - return exists ? true : false; + const exists = + this.cachePerContract.get(storageAddress.toBigInt())?.has(nullifier.toBigInt()) || + this.siloedNullifiers.has(siloNullifier(AztecAddress.fromField(storageAddress), nullifier).toBigInt()); + return !!exists; } /** @@ -115,20 +119,25 @@ export class NullifierCache { * @param nullifier - the nullifier to stage */ public append(storageAddress: Fr, nullifier: Fr) { + if (this.exists(storageAddress, nullifier)) { + throw new NullifierCollisionError( + `Nullifier ${nullifier} at contract ${storageAddress} already exists in cache.`, + ); + } + let nullifiersForContract = this.cachePerContract.get(storageAddress.toBigInt()); // If this contract's nullifier set has no cached nullifiers, create a new Set to store them if (!nullifiersForContract) { nullifiersForContract = new Set(); this.cachePerContract.set(storageAddress.toBigInt(), nullifiersForContract); } - if (nullifiersForContract.has(nullifier.toBigInt())) { - throw new NullifierCollisionError( - `Nullifier ${nullifier} at contract ${storageAddress} already exists in cache.`, - ); - } nullifiersForContract.add(nullifier.toBigInt()); } + public appendSiloed(siloedNullifier: Fr) { + this.siloedNullifiers.add(siloedNullifier.toBigInt()); + } + /** * Merge another cache's nullifiers into this instance's. * @@ -139,6 +148,8 @@ export class NullifierCache { * @param incomingNullifiers - the incoming cached nullifiers to merge into this instance's */ public acceptAndMerge(incomingNullifiers: NullifierCache) { + // Merge siloed nullifiers. + this.siloedNullifiers = new Set([...this.siloedNullifiers, ...incomingNullifiers.siloedNullifiers]); // Iterate over all contracts with staged writes in the child. for (const [incomingAddress, incomingCacheAtContract] of incomingNullifiers.cachePerContract) { const thisCacheAtContract = this.cachePerContract.get(incomingAddress); diff --git a/yarn-project/simulator/src/public/abstract_phase_manager.ts b/yarn-project/simulator/src/public/abstract_phase_manager.ts index e3084193263..f6a7d1848ad 100644 --- a/yarn-project/simulator/src/public/abstract_phase_manager.ts +++ b/yarn-project/simulator/src/public/abstract_phase_manager.ts @@ -249,6 +249,7 @@ export abstract class AbstractPhaseManager { // TODO(6052): Extract correct new counter from nested calls const sideEffectCounter = lastSideEffectCounter(tx) + 1; const availableGas = this.getAvailableGas(tx, kernelOutput); + const pendingNullifiers = this.getSiloedPendingNullifiers(kernelOutput); const result = isExecutionRequest ? await this.publicExecutor.simulate( @@ -256,6 +257,7 @@ export abstract class AbstractPhaseManager { this.globalVariables, availableGas, tx.data.constants.txContext, + pendingNullifiers, transactionFee, sideEffectCounter, ) @@ -323,6 +325,11 @@ export abstract class AbstractPhaseManager { return [publicKernelInputs, kernelOutput, kernelProof, newUnencryptedFunctionLogs, undefined, returns]; } + /** Returns all pending private and public nullifiers. */ + private getSiloedPendingNullifiers(ko: PublicKernelCircuitPublicInputs) { + return [...ko.end.newNullifiers, ...ko.endNonRevertibleData.newNullifiers].filter(n => !n.isEmpty()); + } + protected getAvailableGas(tx: Tx, previousPublicKernelOutput: PublicKernelCircuitPublicInputs) { return tx.data.constants.txContext.gasSettings .getLimits() // No need to subtract teardown limits since they are already included in end.gasUsed diff --git a/yarn-project/simulator/src/public/executor.ts b/yarn-project/simulator/src/public/executor.ts index 10ec1e7ad93..89951608404 100644 --- a/yarn-project/simulator/src/public/executor.ts +++ b/yarn-project/simulator/src/public/executor.ts @@ -4,6 +4,7 @@ import { Gas, type GlobalVariables, type Header, + type Nullifier, PublicCircuitPublicInputs, type TxContext, } from '@aztec/circuits.js'; @@ -78,6 +79,9 @@ async function executeTopLevelPublicFunctionAvm( // or modify the PersistableStateManager to manage rollbacks across enqueued-calls and transactions. const worldStateJournal = new AvmPersistableStateManager(hostStorage); const startSideEffectCounter = executionContext.execution.callContext.sideEffectCounter; + for (const nullifier of executionContext.pendingNullifiers) { + worldStateJournal.nullifiers.cache.appendSiloed(nullifier.value); + } worldStateJournal.trace.accessCounter = startSideEffectCounter; const executionEnv = createAvmExecutionEnvironment( @@ -289,6 +293,7 @@ export class PublicExecutor { globalVariables: GlobalVariables, availableGas: Gas, txContext: TxContext, + pendingNullifiers: Nullifier[], transactionFee: Fr = Fr.ZERO, sideEffectCounter: number = 0, ): Promise { @@ -308,6 +313,7 @@ export class PublicExecutor { availableGas, transactionFee, txContext.gasSettings, + pendingNullifiers, ); const executionResult = await executePublicFunction(context, /*nested=*/ false); diff --git a/yarn-project/simulator/src/public/index.test.ts b/yarn-project/simulator/src/public/index.test.ts index 14995aa727a..81dd93a2712 100644 --- a/yarn-project/simulator/src/public/index.test.ts +++ b/yarn-project/simulator/src/public/index.test.ts @@ -99,7 +99,7 @@ describe('ACIR public execution simulator', () => { }); const simulate = (execution: PublicExecution, globalVariables: GlobalVariables) => - executor.simulate(execution, globalVariables, Gas.test(), makeTxContext(), Fr.ZERO); + executor.simulate(execution, globalVariables, Gas.test(), makeTxContext(), /*pendingNullifiers=*/ [], Fr.ZERO); describe('Token contract', () => { let recipient: AztecAddress; diff --git a/yarn-project/simulator/src/public/public_execution_context.ts b/yarn-project/simulator/src/public/public_execution_context.ts index bcd468aa568..c6279209ecf 100644 --- a/yarn-project/simulator/src/public/public_execution_context.ts +++ b/yarn-project/simulator/src/public/public_execution_context.ts @@ -7,6 +7,7 @@ import { type GasSettings, type GlobalVariables, type Header, + type Nullifier, PublicContextInputs, } from '@aztec/circuits.js'; import { type AztecAddress } from '@aztec/foundation/aztec-address'; @@ -45,6 +46,7 @@ export class PublicExecutionContext extends TypedOracle { public readonly availableGas: Gas, public readonly transactionFee: Fr, public readonly gasSettings: GasSettings, + public readonly pendingNullifiers: Nullifier[], // Unencrypted logs emitted during this call AND any nested calls // Useful for maintaining correct ordering in ts private allUnencryptedLogs: UnencryptedL2Log[] = [], @@ -239,6 +241,7 @@ export class PublicExecutionContext extends TypedOracle { this.availableGas, this.transactionFee, this.gasSettings, + /*pendingNullifiers=*/ [], this.allUnencryptedLogs, this.log, ); diff --git a/yarn-project/simulator/src/public/public_processor.test.ts b/yarn-project/simulator/src/public/public_processor.test.ts index 94e666fd524..33c25cab600 100644 --- a/yarn-project/simulator/src/public/public_processor.test.ts +++ b/yarn-project/simulator/src/public/public_processor.test.ts @@ -764,6 +764,7 @@ describe('public_processor', () => { expect.anything(), // GlobalVariables Gas.from(availableGas), expect.anything(), // TxContext + expect.anything(), // pendingNullifiers new Fr(txFee), expect.anything(), // SideEffectCounter ];