From 5636e438595f1988dd9bf39748e25d46ca6046c7 Mon Sep 17 00:00:00 2001 From: dbanks12 Date: Mon, 17 Jun 2024 19:13:34 +0000 Subject: [PATCH] chore: avm simulator uses regular ContractStorageRead type --- .../circuits.js/src/structs/avm/avm.ts | 9 +++ .../src/structs/contract_storage_read.ts | 28 ++++------ .../simulator/src/avm/journal/journal.ts | 22 ++++---- .../simulator/src/avm/journal/trace.test.ts | 18 +++--- .../simulator/src/avm/journal/trace.ts | 55 +++++++++---------- .../simulator/src/avm/journal/trace_types.ts | 32 +---------- .../src/avm/opcodes/external_calls.ts | 2 +- yarn-project/simulator/src/public/executor.ts | 2 +- .../src/public/transitional_adaptors.ts | 2 +- 9 files changed, 69 insertions(+), 101 deletions(-) diff --git a/yarn-project/circuits.js/src/structs/avm/avm.ts b/yarn-project/circuits.js/src/structs/avm/avm.ts index f33335f800c..907e41ad4f2 100644 --- a/yarn-project/circuits.js/src/structs/avm/avm.ts +++ b/yarn-project/circuits.js/src/structs/avm/avm.ts @@ -243,6 +243,7 @@ export class AvmContractInstanceHint { } } +// TODO(dbanks12): rename AvmCircuitHints export class AvmExecutionHints { public readonly storageValues: Vector; public readonly noteHashExists: Vector; @@ -267,6 +268,14 @@ export class AvmExecutionHints { this.contractInstances = new Vector(contractInstances); } + /** + * Return an empty instance. + * @returns an empty instance. + */ + empty() { + return new AvmExecutionHints([], [], [], [], [], []); + } + /** * Serializes the inputs to a buffer. * @returns - The inputs serialized to a buffer. diff --git a/yarn-project/circuits.js/src/structs/contract_storage_read.ts b/yarn-project/circuits.js/src/structs/contract_storage_read.ts index 5a679a75bf7..56d7f350168 100644 --- a/yarn-project/circuits.js/src/structs/contract_storage_read.ts +++ b/yarn-project/circuits.js/src/structs/contract_storage_read.ts @@ -23,30 +23,24 @@ export class ContractStorageRead { /** * Side effect counter tracking position of this event in tx execution. */ - public readonly sideEffectCounter: number, + public readonly counter: number, + /** + * Contract address whose storage is being read. + */ public contractAddress?: AztecAddress, // TODO: Should not be optional. This is a temporary hack to silo the storage slot with the correct address for nested executions. ) {} static from(args: { - /** - * Storage slot we are reading from. - */ storageSlot: Fr; - /** - * Value read from the storage slot. - */ currentValue: Fr; - /** - * Side effect counter tracking position of this event in tx execution. - */ - sideEffectCounter: number; + counter: number; contractAddress?: AztecAddress; }) { - return new ContractStorageRead(args.storageSlot, args.currentValue, args.sideEffectCounter, args.contractAddress); + return new ContractStorageRead(args.storageSlot, args.currentValue, args.counter, args.contractAddress); } toBuffer() { - return serializeToBuffer(this.storageSlot, this.currentValue, new Fr(this.sideEffectCounter)); + return serializeToBuffer(this.storageSlot, this.currentValue, new Fr(this.counter)); } static fromBuffer(buffer: Buffer | BufferReader) { @@ -59,7 +53,7 @@ export class ContractStorageRead { } isEmpty() { - return this.storageSlot.isZero() && this.currentValue.isZero() && this.sideEffectCounter == 0; + return this.storageSlot.isZero() && this.currentValue.isZero() && this.counter == 0; } toFriendlyJSON() { @@ -67,7 +61,7 @@ export class ContractStorageRead { } toFields(): Fr[] { - const fields = [this.storageSlot, this.currentValue, new Fr(this.sideEffectCounter)]; + const fields = [this.storageSlot, this.currentValue, new Fr(this.counter)]; if (fields.length !== CONTRACT_STORAGE_READ_LENGTH) { throw new Error( `Invalid number of fields for ContractStorageRead. Expected ${CONTRACT_STORAGE_READ_LENGTH}, got ${fields.length}`, @@ -81,8 +75,8 @@ export class ContractStorageRead { const storageSlot = reader.readField(); const currentValue = reader.readField(); - const sideEffectCounter = reader.readField().toNumber(); + const counter = reader.readField().toNumber(); - return new ContractStorageRead(storageSlot, currentValue, sideEffectCounter); + return new ContractStorageRead(storageSlot, currentValue, counter); } } diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts index dd028a63db9..c1495874b42 100644 --- a/yarn-project/simulator/src/avm/journal/journal.ts +++ b/yarn-project/simulator/src/avm/journal/journal.ts @@ -118,7 +118,7 @@ export class AvmPersistableStateManager { l1ToL2MsgReadRequests: [], newNoteHashes: [], newL2ToL1Messages: [], - startSideEffectCounter: this.trace.accessCounter, + startSideEffectCounter: this.trace.counter, newNullifiers: [], contractStorageReads: [], contractStorageUpdateRequests: [], @@ -150,7 +150,7 @@ export class AvmPersistableStateManager { // TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit this.transitionalExecutionResult.contractStorageUpdateRequests.push( - new ContractStorageUpdateRequest(slot, value, this.trace.accessCounter, storageAddress), + new ContractStorageUpdateRequest(slot, value, this.trace.counter, storageAddress), ); // Trace all storage writes (even reverted ones) @@ -172,7 +172,7 @@ export class AvmPersistableStateManager { // TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit this.transitionalExecutionResult.contractStorageReads.push( - new ContractStorageRead(slot, value, this.trace.accessCounter, storageAddress), + new ContractStorageRead(slot, value, this.trace.counter, storageAddress), ); // We want to keep track of all performed reads (even reverted ones) @@ -195,7 +195,7 @@ export class AvmPersistableStateManager { this.log.debug(`noteHashes(${storageAddress})@${noteHash} ?? leafIndex: ${leafIndex}, exists: ${exists}.`); // TODO: include exists here also - This can for sure come from the trace??? - this.transitionalExecutionResult.noteHashReadRequests.push(new ReadRequest(noteHash, this.trace.accessCounter)); + this.transitionalExecutionResult.noteHashReadRequests.push(new ReadRequest(noteHash, this.trace.counter)); this.trace.traceNoteHashCheck(storageAddress, noteHash, exists, leafIndex); return Promise.resolve(exists); @@ -207,7 +207,7 @@ export class AvmPersistableStateManager { */ public writeNoteHash(storageAddress: Fr, noteHash: Fr) { // TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit - this.transitionalExecutionResult.newNoteHashes.push(new NoteHash(noteHash, this.trace.accessCounter)); + this.transitionalExecutionResult.newNoteHashes.push(new NoteHash(noteHash, this.trace.counter)); this.log.debug(`noteHashes(${storageAddress}) += @${noteHash}.`); this.trace.traceNewNoteHash(storageAddress, noteHash); @@ -227,10 +227,10 @@ export class AvmPersistableStateManager { // TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit if (exists) { - this.transitionalExecutionResult.nullifierReadRequests.push(new ReadRequest(nullifier, this.trace.accessCounter)); + this.transitionalExecutionResult.nullifierReadRequests.push(new ReadRequest(nullifier, this.trace.counter)); } else { this.transitionalExecutionResult.nullifierNonExistentReadRequests.push( - new ReadRequest(nullifier, this.trace.accessCounter), + new ReadRequest(nullifier, this.trace.counter), ); } @@ -246,7 +246,7 @@ export class AvmPersistableStateManager { public async writeNullifier(storageAddress: Fr, nullifier: Fr) { // TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit this.transitionalExecutionResult.newNullifiers.push( - new Nullifier(nullifier, this.trace.accessCounter, /*noteHash=*/ Fr.ZERO), + new Nullifier(nullifier, this.trace.counter, /*noteHash=*/ Fr.ZERO), ); this.log.debug(`nullifiers(${storageAddress}) += ${nullifier}.`); @@ -269,7 +269,7 @@ export class AvmPersistableStateManager { `l1ToL2Messages(@${msgLeafIndex}) ?? exists: ${exists}, expected: ${msgHash}, found: ${valueAtIndex}.`, ); - this.transitionalExecutionResult.l1ToL2MsgReadRequests.push(new ReadRequest(msgHash, this.trace.accessCounter)); + this.transitionalExecutionResult.l1ToL2MsgReadRequests.push(new ReadRequest(msgHash, this.trace.counter)); this.trace.traceL1ToL2MessageCheck(msgHash, msgLeafIndex, exists); return Promise.resolve(exists); @@ -305,7 +305,7 @@ export class AvmPersistableStateManager { // this duplicates exactly what happens in the trace just for the purpose of transitional integration with the kernel this.transitionalExecutionResult.unencryptedLogsHashes.push( // TODO(6578): explain magic number 4 here - new LogHash(logHash, this.trace.accessCounter, new Fr(ulog.length + 4)), + new LogHash(logHash, this.trace.counter, new Fr(ulog.length + 4)), ); // TODO(6206): likely need to track this here and not just in the transitional logic. @@ -374,7 +374,7 @@ export class AvmPersistableStateManager { currentStorageValue: this.publicStorage.getCache().cachePerContract, storageReads: this.trace.publicStorageReads, storageWrites: this.trace.publicStorageWrites, - sideEffectCounter: this.trace.accessCounter, + sideEffectCounter: this.trace.counter, }; } } diff --git a/yarn-project/simulator/src/avm/journal/trace.test.ts b/yarn-project/simulator/src/avm/journal/trace.test.ts index a143ce4e3be..eb1b19030bc 100644 --- a/yarn-project/simulator/src/avm/journal/trace.test.ts +++ b/yarn-project/simulator/src/avm/journal/trace.test.ts @@ -31,7 +31,7 @@ describe('world state access trace', () => { leafIndex: leafIndex, }, ]); - expect(trace.getAccessCounter()).toBe(1); + expect(trace.getCounter()).toBe(1); }); it('Should trace note hashes', () => { const contractAddress = new Fr(1); @@ -42,7 +42,7 @@ describe('world state access trace', () => { expect(trace.newNoteHashes).toEqual([ expect.objectContaining({ storageAddress: contractAddress, noteHash: utxo }), ]); - expect(trace.getAccessCounter()).toEqual(1); + expect(trace.getCounter()).toEqual(1); }); it('Should trace nullifier checks', () => { const contractAddress = new Fr(1); @@ -62,7 +62,7 @@ describe('world state access trace', () => { leafIndex: leafIndex, }; expect(trace.nullifierChecks).toEqual([expectedCheck]); - expect(trace.getAccessCounter()).toEqual(1); + expect(trace.getCounter()).toEqual(1); }); it('Should trace nullifiers', () => { const contractAddress = new Fr(1); @@ -76,7 +76,7 @@ describe('world state access trace', () => { // endLifetime: Fr.ZERO, }; expect(trace.newNullifiers).toEqual([expectedNullifier]); - expect(trace.getAccessCounter()).toEqual(1); + expect(trace.getCounter()).toEqual(1); }); it('Should trace L1ToL2 Message checks', () => { const utxo = new Fr(2); @@ -90,13 +90,13 @@ describe('world state access trace', () => { counter: new Fr(0), }; expect(trace.l1ToL2MessageChecks).toEqual([expectedCheck]); - expect(trace.getAccessCounter()).toEqual(1); + expect(trace.getCounter()).toEqual(1); }); it('Should trace get contract instance', () => { const instance = randomTracedContractInstance(); trace.traceGetContractInstance(instance); expect(trace.gotContractInstances).toEqual([instance]); - expect(trace.getAccessCounter()).toEqual(1); + expect(trace.getCounter()).toEqual(1); }); }); @@ -145,7 +145,7 @@ describe('world state access trace', () => { counter++; trace.traceGetContractInstance(instance); counter++; - expect(trace.getAccessCounter()).toEqual(counter); + expect(trace.getCounter()).toEqual(counter); }); it('Should merge two traces together', () => { @@ -216,9 +216,9 @@ describe('world state access trace', () => { childTrace.traceL1ToL2MessageCheck(msgHashT1, msgLeafIndexT1, msgExistsT1); childTrace.traceGetContractInstance(instanceT1); - const childCounterBeforeMerge = childTrace.getAccessCounter(); + const childCounterBeforeMerge = childTrace.getCounter(); trace.acceptAndMerge(childTrace); - expect(trace.getAccessCounter()).toEqual(childCounterBeforeMerge); + expect(trace.getCounter()).toEqual(childCounterBeforeMerge); expect(trace.publicStorageReads).toEqual([ expect.objectContaining({ diff --git a/yarn-project/simulator/src/avm/journal/trace.ts b/yarn-project/simulator/src/avm/journal/trace.ts index 608f738ccc3..ca4d9f508f3 100644 --- a/yarn-project/simulator/src/avm/journal/trace.ts +++ b/yarn-project/simulator/src/avm/journal/trace.ts @@ -7,15 +7,15 @@ import { type TracedNoteHashCheck, type TracedNullifier, type TracedNullifierCheck, - type TracedPublicStorageRead, type TracedPublicStorageWrite, type TracedUnencryptedL2Log, } from './trace_types.js'; +import { AvmExecutionHints, AvmKeyValueHint, AztecAddress, ContractStorageRead } from '@aztec/circuits.js'; export class WorldStateAccessTrace { - public accessCounter: number; + public counter: number; - public publicStorageReads: TracedPublicStorageRead[] = []; + public publicStorageReads: ContractStorageRead[] = []; public publicStorageWrites: TracedPublicStorageWrite[] = []; public noteHashChecks: TracedNoteHashCheck[] = []; @@ -26,32 +26,27 @@ export class WorldStateAccessTrace { public newLogsHashes: TracedUnencryptedL2Log[] = []; public gotContractInstances: TracedContractInstance[] = []; - //public contractCalls: TracedContractCall[] = []; - //public archiveChecks: TracedArchiveLeafCheck[] = []; + // TODO(dbanks12): use this + public circuitHints: AvmExecutionHints; constructor(parentTrace?: WorldStateAccessTrace) { - this.accessCounter = parentTrace ? parentTrace.accessCounter : 0; + this.counter = parentTrace ? parentTrace.counter : 0; // TODO(4805): consider tracking the parent's trace vector lengths so we can enforce limits + this.circuitHints = AvmExecutionHints.empty(); } - public getAccessCounter() { - return this.accessCounter; + public getCounter() { + return this.counter; } - public tracePublicStorageRead(storageAddress: Fr, slot: Fr, value: Fr, exists: boolean, cached: boolean) { + public tracePublicStorageRead(storageAddress: Fr, slot: Fr, value: Fr, _exists: boolean, _cached: boolean) { // TODO(4805): check if some threshold is reached for max storage reads // (need access to parent length, or trace needs to be initialized with parent's contents) - const traced: TracedPublicStorageRead = { - // callPointer: Fr.ZERO, - storageAddress, - slot, - value, - exists, - cached, - counter: new Fr(this.accessCounter), - // endLifetime: Fr.ZERO, - }; - this.publicStorageReads.push(traced); + // NOTE: exists and cached are unused for now but may be used for optimizations or kernel hints later + this.publicStorageReads.push( + new ContractStorageRead(slot, value, this.counter, AztecAddress.fromField(storageAddress)) + ); + this.circuitHints.storageValues.items.push(new AvmKeyValueHint(/*key=*/new Fr(this.counter), /*value=*/value)); this.incrementAccessCounter(); } @@ -60,10 +55,10 @@ export class WorldStateAccessTrace { // (need access to parent length, or trace needs to be initialized with parent's contents) const traced: TracedPublicStorageWrite = { // callPointer: Fr.ZERO, - storageAddress, + contractAddress: storageAddress, slot, value, - counter: new Fr(this.accessCounter), + counter: new Fr(this.counter), // endLifetime: Fr.ZERO, }; this.publicStorageWrites.push(traced); @@ -76,7 +71,7 @@ export class WorldStateAccessTrace { storageAddress, noteHash, exists, - counter: new Fr(this.accessCounter), + counter: new Fr(this.counter), // endLifetime: Fr.ZERO, leafIndex, }; @@ -90,7 +85,7 @@ export class WorldStateAccessTrace { // callPointer: Fr.ZERO, storageAddress, noteHash, - counter: new Fr(this.accessCounter), + counter: new Fr(this.counter), // endLifetime: Fr.ZERO, }; this.newNoteHashes.push(traced); @@ -104,7 +99,7 @@ export class WorldStateAccessTrace { storageAddress, nullifier, exists, - counter: new Fr(this.accessCounter), + counter: new Fr(this.counter), // endLifetime: Fr.ZERO, isPending, leafIndex, @@ -119,7 +114,7 @@ export class WorldStateAccessTrace { // callPointer: Fr.ZERO, storageAddress, nullifier, - counter: new Fr(this.accessCounter), + counter: new Fr(this.counter), // endLifetime: Fr.ZERO, }; this.newNullifiers.push(tracedNullifier); @@ -133,7 +128,7 @@ export class WorldStateAccessTrace { leafIndex: msgLeafIndex, msgHash: msgHash, exists: exists, - counter: new Fr(this.accessCounter), + counter: new Fr(this.counter), //endLifetime: Fr.ZERO, // FIXME }; this.l1ToL2MessageChecks.push(traced); @@ -143,7 +138,7 @@ export class WorldStateAccessTrace { public traceNewLog(logHash: Fr) { const traced: TracedUnencryptedL2Log = { logHash, - counter: new Fr(this.accessCounter), + counter: new Fr(this.counter), }; this.newLogsHashes.push(traced); this.incrementAccessCounter(); @@ -155,7 +150,7 @@ export class WorldStateAccessTrace { } private incrementAccessCounter() { - this.accessCounter++; + this.counter++; } /** @@ -176,6 +171,6 @@ export class WorldStateAccessTrace { this.newLogsHashes.push(...incomingTrace.newLogsHashes); this.gotContractInstances.push(...incomingTrace.gotContractInstances); // it is assumed that the incoming trace was initialized with this as parent, so accept counter - this.accessCounter = incomingTrace.accessCounter; + this.counter = incomingTrace.counter; } } diff --git a/yarn-project/simulator/src/avm/journal/trace_types.ts b/yarn-project/simulator/src/avm/journal/trace_types.ts index db57e53998b..ecabd6033c3 100644 --- a/yarn-project/simulator/src/avm/journal/trace_types.ts +++ b/yarn-project/simulator/src/avm/journal/trace_types.ts @@ -1,27 +1,8 @@ import { type Fr } from '@aztec/foundation/fields'; import { type ContractInstanceWithAddress } from '@aztec/types/contracts'; -//export type TracedContractCall = { -// callPointer: Fr; -// address: Fr; -// storageAddress: Fr; -// endLifetime: Fr; -//}; - -export type TracedPublicStorageRead = { - // callPointer: Fr; - storageAddress: Fr; - exists: boolean; - cached: boolean; - slot: Fr; - value: Fr; - counter: Fr; - // endLifetime: Fr; -}; - export type TracedPublicStorageWrite = { - // callPointer: Fr; - storageAddress: Fr; + contractAddress: Fr; slot: Fr; value: Fr; counter: Fr; @@ -29,7 +10,6 @@ export type TracedPublicStorageWrite = { }; export type TracedNoteHashCheck = { - // callPointer: Fr; storageAddress: Fr; leafIndex: Fr; noteHash: Fr; @@ -39,7 +19,6 @@ export type TracedNoteHashCheck = { }; export type TracedNoteHash = { - // callPointer: Fr; storageAddress: Fr; noteHash: Fr; counter: Fr; @@ -47,7 +26,6 @@ export type TracedNoteHash = { }; export type TracedNullifierCheck = { - // callPointer: Fr; storageAddress: Fr; nullifier: Fr; exists: boolean; @@ -60,7 +38,6 @@ export type TracedNullifierCheck = { }; export type TracedNullifier = { - // callPointer: Fr; storageAddress: Fr; nullifier: Fr; counter: Fr; @@ -68,7 +45,6 @@ export type TracedNullifier = { }; export type TracedL1toL2MessageCheck = { - //callPointer: Fr; leafIndex: Fr; msgHash: Fr; exists: boolean; @@ -77,15 +53,9 @@ export type TracedL1toL2MessageCheck = { }; export type TracedUnencryptedL2Log = { - //callPointer: Fr; logHash: Fr; counter: Fr; //endLifetime: Fr; }; -//export type TracedArchiveLeafCheck = { -// leafIndex: Fr; -// leaf: Fr; -//}; - export type TracedContractInstance = { exists: boolean } & ContractInstanceWithAddress; diff --git a/yarn-project/simulator/src/avm/opcodes/external_calls.ts b/yarn-project/simulator/src/avm/opcodes/external_calls.ts index 20f72557b3c..4ef5860e8e1 100644 --- a/yarn-project/simulator/src/avm/opcodes/external_calls.ts +++ b/yarn-project/simulator/src/avm/opcodes/external_calls.ts @@ -89,7 +89,7 @@ abstract class ExternalCall extends Instruction { callType, FunctionSelector.fromField(functionSelector), ); - const startSideEffectCounter = nestedContext.persistableState.trace.accessCounter; + const startSideEffectCounter = nestedContext.persistableState.trace.counter; const oldStyleExecution = createPublicExecution(startSideEffectCounter, nestedContext.environment, calldata); const simulator = new AvmSimulator(nestedContext); diff --git a/yarn-project/simulator/src/public/executor.ts b/yarn-project/simulator/src/public/executor.ts index 45885d23de6..3640a2b16b4 100644 --- a/yarn-project/simulator/src/public/executor.ts +++ b/yarn-project/simulator/src/public/executor.ts @@ -56,7 +56,7 @@ export class PublicExecutor { for (const nullifier of pendingNullifiers) { worldStateJournal.nullifiers.cache.appendSiloed(nullifier.value); } - worldStateJournal.trace.accessCounter = startSideEffectCounter; + worldStateJournal.trace.counter = startSideEffectCounter; const executionEnv = createAvmExecutionEnvironment( execution, diff --git a/yarn-project/simulator/src/public/transitional_adaptors.ts b/yarn-project/simulator/src/public/transitional_adaptors.ts index 36d0f2ade12..fa81a871ba4 100644 --- a/yarn-project/simulator/src/public/transitional_adaptors.ts +++ b/yarn-project/simulator/src/public/transitional_adaptors.ts @@ -125,7 +125,7 @@ export function convertAvmResultsToPxResult( execution: fromPx, returnValues: avmResult.output, startSideEffectCounter: new Fr(startSideEffectCounter), - endSideEffectCounter: new Fr(endPersistableState.trace.accessCounter), + endSideEffectCounter: new Fr(endPersistableState.trace.counter), unencryptedLogs: new UnencryptedFunctionL2Logs(endPersistableState.transitionalExecutionResult.unencryptedLogs), allUnencryptedLogs: new UnencryptedFunctionL2Logs( endPersistableState.transitionalExecutionResult.allUnencryptedLogs,