diff --git a/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts b/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts index 7b434878eb9f..58c80bbd266b 100644 --- a/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts +++ b/yarn-project/end-to-end/src/shared/uniswap_l1_l2.ts @@ -940,7 +940,7 @@ export const uniswapL1L2TestSuite = ( uniswapPortal.simulate.swapPublic(swapArgs, { account: ownerEthAddress.toString(), } as any), - ).rejects.toThrow('The contract function "swapPublic" reverted.'); + ).rejects.toThrow(/The contract function "swapPublic" reverted/); }); it("can't call swap_private on L1 if called swap_public on L2", async () => { diff --git a/yarn-project/simulator/src/avm/journal/journal.ts b/yarn-project/simulator/src/avm/journal/journal.ts index 9a4c85bdae7c..944338778729 100644 --- a/yarn-project/simulator/src/avm/journal/journal.ts +++ b/yarn-project/simulator/src/avm/journal/journal.ts @@ -141,7 +141,7 @@ export class AvmPersistableStateManager { * @param value - the value being written to the slot */ public writeStorage(storageAddress: Fr, slot: Fr, value: Fr) { - this.log.debug(`storage(${storageAddress})@${slot} <- ${value}`); + this.log.info(`Storage write (address=${storageAddress}, slot=${slot}): value=${value}`); // Cache storage writes for later reference/reads this.publicStorage.write(storageAddress, slot, value); @@ -172,7 +172,9 @@ export class AvmPersistableStateManager { */ public async readStorage(storageAddress: Fr, slot: Fr): Promise { const { value, exists, cached } = await this.publicStorage.read(storageAddress, slot); - this.log.debug(`storage(${storageAddress})@${slot} ?? value: ${value}, exists: ${exists}, cached: ${cached}.`); + this.log.info( + `Storage read (address=${storageAddress}, slot=${slot}): value=${value}, exists=${exists}, cached=${cached}`, + ); // TRANSITIONAL: This should be removed once the kernel handles and entire enqueued call per circuit // The current info to the kernel kernel does not consider cached reads. diff --git a/yarn-project/simulator/src/avm/journal/nullifiers.test.ts b/yarn-project/simulator/src/avm/journal/nullifiers.test.ts index 16f4d359a53e..f8cec85bd92b 100644 --- a/yarn-project/simulator/src/avm/journal/nullifiers.test.ts +++ b/yarn-project/simulator/src/avm/journal/nullifiers.test.ts @@ -64,6 +64,21 @@ describe('avm nullifier caching', () => { expect(isPending).toEqual(true); expect(gotIndex).toEqual(Fr.ZERO); }); + it('Existence check works on fallback to grandparent (gets value, exists, is pending)', async () => { + const contractAddress = new Fr(1); + const nullifier = new Fr(2); + const childNullifiers = new Nullifiers(commitmentsDb, nullifiers); + const grandChildNullifiers = new Nullifiers(commitmentsDb, childNullifiers); + + // Write to parent cache + await nullifiers.append(contractAddress, nullifier); + // Get from child cache + const [exists, isPending, gotIndex] = await grandChildNullifiers.checkExists(contractAddress, nullifier); + // exists (in parent), isPending, index is zero (not in tree) + expect(exists).toEqual(true); + expect(isPending).toEqual(true); + expect(gotIndex).toEqual(Fr.ZERO); + }); }); describe('Nullifier collision failures', () => { diff --git a/yarn-project/simulator/src/avm/journal/nullifiers.ts b/yarn-project/simulator/src/avm/journal/nullifiers.ts index 99d12757e60e..7e7b0a540555 100644 --- a/yarn-project/simulator/src/avm/journal/nullifiers.ts +++ b/yarn-project/simulator/src/avm/journal/nullifiers.ts @@ -12,21 +12,20 @@ import type { CommitmentsDB } from '../../index.js'; export class Nullifiers { /** Cached nullifiers. */ 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. */ - private readonly hostNullifiers: CommitmentsDB; - constructor(hostNullifiers: CommitmentsDB, parent?: Nullifiers) { - this.hostNullifiers = hostNullifiers; - this.parentCache = parent?.cache; + constructor( + /** Reference to node storage. Checked on parent cache-miss. */ + private readonly hostNullifiers: CommitmentsDB, + /** Parent's nullifiers. Checked on this' cache-miss. */ + private readonly parent?: Nullifiers | undefined, + ) { this.cache = new NullifierCache(); } /** * Get a nullifier's existence status. * 1. Check cache. - * 2. Check parent's cache. + * 2. Check parent. * 3. Fall back to the host state. * 4. Not found! Nullifier does not exist. * @@ -41,10 +40,12 @@ export class Nullifiers { nullifier: Fr, ): Promise<[/*exists=*/ boolean, /*isPending=*/ boolean, /*leafIndex=*/ Fr]> { // First check this cache - let existsAsPending = this.cache.exists(storageAddress, nullifier); - // Then check parent's cache - if (!existsAsPending && this.parentCache) { - existsAsPending = this.parentCache?.exists(storageAddress, nullifier); + const existsAsPending = this.cache.exists(storageAddress, nullifier); + // Then try parent's nullifier cache + if (!existsAsPending && this.parent) { + // Note: this will recurse to grandparent/etc until a cache-hit is encountered. + // Otherwise it will fall back to host state in the top-most [grand]parent. + return await this.parent.checkExists(storageAddress, nullifier); } // Finally try the host's Aztec state (a trip to the database) // If the value is found in the database, it will be associated with a leaf index! diff --git a/yarn-project/simulator/src/avm/journal/public_storage.test.ts b/yarn-project/simulator/src/avm/journal/public_storage.test.ts index 54633e40f96b..1d6359caef95 100644 --- a/yarn-project/simulator/src/avm/journal/public_storage.test.ts +++ b/yarn-project/simulator/src/avm/journal/public_storage.test.ts @@ -67,6 +67,21 @@ describe('avm public storage', () => { expect(cached).toEqual(true); }); + it('Reading works on fallback to grandparent (gets value & exists)', async () => { + const contractAddress = new Fr(1); + const slot = new Fr(2); + const value = new Fr(3); + const childStorage = new PublicStorage(publicDb, publicStorage); + const grandChildStorage = new PublicStorage(publicDb, childStorage); + + publicStorage.write(contractAddress, slot, value); + const { exists, value: gotValue, cached } = await grandChildStorage.read(contractAddress, slot); + // exists because it was previously written! + expect(exists).toEqual(true); + expect(gotValue).toEqual(value); + expect(cached).toEqual(true); + }); + it('When reading from storage, should check cache, then parent, then host', async () => { // Store a different value in storage vs the cache, and make sure the cache is returned const contractAddress = new Fr(1); diff --git a/yarn-project/simulator/src/avm/journal/public_storage.ts b/yarn-project/simulator/src/avm/journal/public_storage.ts index 30c59e421b31..3cf8b5751ecf 100644 --- a/yarn-project/simulator/src/avm/journal/public_storage.ts +++ b/yarn-project/simulator/src/avm/journal/public_storage.ts @@ -17,14 +17,13 @@ type PublicStorageReadResult = { export class PublicStorage { /** Cached storage writes. */ private cache: PublicStorageCache; - /** Parent's storage cache. Checked on cache-miss. */ - private readonly parentCache: PublicStorageCache | undefined; - /** Reference to node storage. Checked on parent cache-miss. */ - private readonly hostPublicStorage: PublicStateDB; - constructor(hostPublicStorage: PublicStateDB, parent?: PublicStorage) { - this.hostPublicStorage = hostPublicStorage; - this.parentCache = parent?.cache; + constructor( + /** Reference to node storage. Checked on parent cache-miss. */ + private readonly hostPublicStorage: PublicStateDB, + /** Parent's storage. Checked on this' cache-miss. */ + private readonly parent?: PublicStorage, + ) { this.cache = new PublicStorageCache(); } @@ -38,7 +37,7 @@ export class PublicStorage { /** * Read a value from storage. * 1. Check cache. - * 2. Check parent's cache. + * 2. Check parent. * 3. Fall back to the host state. * 4. Not found! Value has never been written to before. Flag it as non-existent and return value zero. * @@ -50,9 +49,11 @@ export class PublicStorage { let cached = false; // First try check this storage cache let value = this.cache.read(storageAddress, slot); - // Then try parent's storage cache (if it exists / written to earlier in this TX) - if (!value && this.parentCache) { - value = this.parentCache?.read(storageAddress, slot); + // Then try parent's storage cache + if (!value && this.parent) { + // Note: this will recurse to grandparent/etc until a cache-hit is encountered. + // Otherwise it will fall back to host state in the top-most [grand]parent. + return await this.parent.read(storageAddress, slot); } // Finally try the host's Aztec state (a trip to the database) if (!value) { @@ -73,8 +74,8 @@ export class PublicStorage { * @param slot - the slot in the contract's storage being written to * @param value - the value being written to the slot */ - public write(storageAddress: Fr, key: Fr, value: Fr) { - this.cache.write(storageAddress, key, value); + public write(storageAddress: Fr, slot: Fr, value: Fr) { + this.cache.write(storageAddress, slot, value); } /**