From 47a17de80c8ec6c6f3995130aa73f9c5e469c598 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 30 Jan 2024 22:16:46 +0000 Subject: [PATCH 1/5] feat(avm): keep history of reads and writes in journal --- .../src/avm/journal/journal.test.ts | 27 ++++- .../acir-simulator/src/avm/journal/journal.ts | 106 +++++++++++++++--- 2 files changed, 110 insertions(+), 23 deletions(-) diff --git a/yarn-project/acir-simulator/src/avm/journal/journal.test.ts b/yarn-project/acir-simulator/src/avm/journal/journal.test.ts index 58b0b3345b90..93d77e30f6e1 100644 --- a/yarn-project/acir-simulator/src/avm/journal/journal.test.ts +++ b/yarn-project/acir-simulator/src/avm/journal/journal.test.ts @@ -61,7 +61,7 @@ describe('journal', () => { expect(cachedResult).toEqual(cachedValue); }); - it('When reading from storage, should check the cache first', async () => { + it('When reading from storage, should check the cache first, and be appended to read journal', async () => { // Store a different value in storage vs the cache, and make sure the cache is returned const contractAddress = new Fr(1); const key = new Fr(2); @@ -80,6 +80,12 @@ describe('journal', () => { // Get the storage value const cachedResult = await journal.readStorage(contractAddress, key); expect(cachedResult).toEqual(cachedValue); + + // We expect the journal to store the access in [storedVal, cachedVal] - [time0, time1] + const { storageReads }: JournalData = journal.flush(); + const contractReads = storageReads.get(contractAddress.toBigInt()); + const keyReads = contractReads?.get(key.toBigInt()); + expect(keyReads).toEqual([storedValue, cachedValue]); }); }); @@ -127,17 +133,19 @@ describe('journal', () => { const logsT1 = [new Fr(3), new Fr(4)]; journal.writeStorage(contractAddress, key, value); + await journal.readStorage(contractAddress, key); journal.writeNoteHash(commitment); journal.writeLog(logs); journal.writeL1Message(logs); journal.writeNullifier(commitment); const journal1 = new AvmJournal(journal.hostStorage, journal); - journal.writeStorage(contractAddress, key, valueT1); - journal.writeNoteHash(commitmentT1); - journal.writeLog(logsT1); - journal.writeL1Message(logsT1); - journal.writeNullifier(commitmentT1); + journal1.writeStorage(contractAddress, key, valueT1); + await journal1.readStorage(contractAddress, key); + journal1.writeNoteHash(commitmentT1); + journal1.writeLog(logsT1); + journal1.writeL1Message(logsT1); + journal1.writeNullifier(commitmentT1); journal1.mergeWithParent(); @@ -147,6 +155,13 @@ describe('journal', () => { // Check that the UTXOs are merged const journalUpdates: JournalData = journal.flush(); + + // Check storage reads order is preserved upon merge + // We first read value from t0, then value from t1 + const contractReads = journalUpdates.storageReads.get(contractAddress.toBigInt()); + const slotReads = contractReads?.get(key.toBigInt()); + expect(slotReads).toEqual([value, valueT1]); + expect(journalUpdates.newNoteHashes).toEqual([commitment, commitmentT1]); expect(journalUpdates.newLogs).toEqual([logs, logsT1]); expect(journalUpdates.newL1Messages).toEqual([logs, logsT1]); diff --git a/yarn-project/acir-simulator/src/avm/journal/journal.ts b/yarn-project/acir-simulator/src/avm/journal/journal.ts index 6168ce2cdf26..761a5fa47faa 100644 --- a/yarn-project/acir-simulator/src/avm/journal/journal.ts +++ b/yarn-project/acir-simulator/src/avm/journal/journal.ts @@ -3,6 +3,12 @@ import { Fr } from '@aztec/foundation/fields'; import { RootJournalCannotBeMerged } from './errors.js'; import { HostStorage } from './host_storage.js'; +type StorageWriteMap = Map; +type ContractWriteMap = Map; + +type StorageReadMap = Map>; +type ContractReadMap = Map; + /** * Data held within the journal */ @@ -12,7 +18,9 @@ export type JournalData = { newL1Messages: Fr[][]; newLogs: Fr[][]; /** contract address -\> key -\> value */ - storageWrites: Map>; + storageWrites: ContractWriteMap; + /** contract address -\> key -\> value[] (stored in order of access) */ + storageReads: ContractReadMap; }; /** @@ -28,9 +36,8 @@ export class AvmJournal { public readonly hostStorage: HostStorage; // Reading state - must be tracked for vm execution - // contract address -> key -> value - // TODO(https://github.com/AztecProtocol/aztec-packages/issues/3999) - private storageReads: Map> = new Map(); + // contract address -> key -> value[] (array stored in order of reads) + private storageReads: Map>> = new Map(); // New written state private newNoteHashes: Fr[] = []; @@ -90,15 +97,45 @@ export class AvmJournal { * @param key - * @returns current value */ - public readStorage(contractAddress: Fr, key: Fr): Promise { - const cachedValue = this.storageWrites.get(contractAddress.toBigInt())?.get(key.toBigInt()); - if (cachedValue) { - return Promise.resolve(cachedValue); + public async readStorage(contractAddress: Fr, key: Fr): Promise { + // - We first try this journal's storage cache ( if written to before in this call frame ) + // - Then we try the parent journal's storage cache ( if it exists ) ( written to earlier in this block ) + // - Finally we try the host storage ( a trip to the database ) + + // Do not early return as we want to keep track of reads in this.storageReads + let value = this.storageWrites.get(contractAddress.toBigInt())?.get(key.toBigInt()); + if (!value && this.parentJournal) { + value = await this.parentJournal?.readStorage(contractAddress, key); + } + if (!value) { + value = await this.hostStorage.publicStateDb.storageRead(contractAddress, key); + } + + this.journalRead(contractAddress, key, value); + return Promise.resolve(value); + } + + /** + * We want to keep track of all performed reads in the journal + * This information is hinted to the avm circuit + + * @param contractAddress - + * @param key - + * @param value - + */ + journalRead(contractAddress: Fr, key: Fr, value: Fr): void { + let contractMap = this.storageReads.get(contractAddress.toBigInt()); + if (!contractMap) { + contractMap = new Map>(); + this.storageReads.set(contractAddress.toBigInt(), contractMap); } - if (this.parentJournal) { - return this.parentJournal?.readStorage(contractAddress, key); + + let accessArray = contractMap.get(key.toBigInt()); + if (!accessArray) { + accessArray = new Array(); + contractMap.set(key.toBigInt(), accessArray); } - return this.hostStorage.publicStateDb.storageRead(contractAddress, key); + accessArray.push(value); } public writeNoteHash(noteHash: Fr) { @@ -131,9 +168,11 @@ export class AvmJournal { this.parentJournal.newNoteHashes = this.parentJournal.newNoteHashes.concat(this.newNoteHashes); this.parentJournal.newL1Messages = this.parentJournal.newL1Messages.concat(this.newL1Messages); this.parentJournal.newNullifiers = this.parentJournal.newNullifiers.concat(this.newNullifiers); + this.parentJournal.newLogs = this.parentJournal.newLogs.concat(this.newLogs); // Merge Public State - mergeContractMaps(this.parentJournal.storageWrites, this.storageWrites); + mergeContractWriteMaps(this.parentJournal.storageWrites, this.storageWrites); + mergeContractReadMaps(this.parentJournal.storageReads, this.storageReads); } /** @@ -148,12 +187,13 @@ export class AvmJournal { newL1Messages: this.newL1Messages, newLogs: this.newLogs, storageWrites: this.storageWrites, + storageReads: this.storageReads, }; } } /** - * Merges two contract maps together + * Merges two contract write maps together * Where childMap keys will take precedent over the hostMap * The assumption being that the child map is created at a later time * And thus contains more up to date information @@ -161,24 +201,56 @@ export class AvmJournal { * @param hostMap - The map to be merged into * @param childMap - The map to be merged from */ -function mergeContractMaps(hostMap: Map>, childMap: Map>) { +function mergeContractWriteMaps(hostMap: ContractWriteMap, childMap: ContractWriteMap) { for (const [key, value] of childMap) { const map1Value = hostMap.get(key); if (!map1Value) { hostMap.set(key, value); } else { - mergeStorageMaps(map1Value, value); + mergeStorageWriteMaps(map1Value, value); } } } /** - * * @param hostMap - The map to be merge into * @param childMap - The map to be merged from */ -function mergeStorageMaps(hostMap: Map, childMap: Map) { +function mergeStorageWriteMaps(hostMap: StorageWriteMap, childMap: StorageWriteMap) { for (const [key, value] of childMap) { hostMap.set(key, value); } } + +/** + * Merges two contract read maps together + * For read maps, we just append the childMap into the host map, as the order of reads is important + * + * @param hostMap - The map to be merged into + * @param childMap - The map to be merged from + */ +function mergeContractReadMaps(hostMap: ContractReadMap, childMap: ContractReadMap) { + for (const [key, value] of childMap) { + const map1Value = hostMap.get(key); + if (!map1Value) { + hostMap.set(key, value); + } else { + mergeStorageReadMaps(map1Value, value); + } + } +} + +/** + * @param hostMap - The map to be merge into + * @param childMap - The map to be merged from + */ +function mergeStorageReadMaps(hostMap: StorageReadMap, childMap: StorageReadMap) { + for (const [key, value] of childMap) { + const readArr = hostMap.get(key); + if (!readArr) { + hostMap.set(key, value); + } else { + readArr?.concat(value); + } + } +} From 0d95cbedd3cd554877058bf0a4002bac542d46cd Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 30 Jan 2024 22:25:20 +0000 Subject: [PATCH 2/5] chore: add journaling for contract writes --- .../src/avm/journal/journal.test.ts | 2 +- .../acir-simulator/src/avm/journal/journal.ts | 59 ++++++++++++------- .../src/avm/opcodes/external_calls.test.ts | 2 +- 3 files changed, 41 insertions(+), 22 deletions(-) diff --git a/yarn-project/acir-simulator/src/avm/journal/journal.test.ts b/yarn-project/acir-simulator/src/avm/journal/journal.test.ts index 93d77e30f6e1..4eee36fd3591 100644 --- a/yarn-project/acir-simulator/src/avm/journal/journal.test.ts +++ b/yarn-project/acir-simulator/src/avm/journal/journal.test.ts @@ -29,7 +29,7 @@ describe('journal', () => { journal.writeStorage(contractAddress, key, value); const journalUpdates: JournalData = journal.flush(); - expect(journalUpdates.storageWrites.get(contractAddress.toBigInt())?.get(key.toBigInt())).toEqual(value); + expect(journalUpdates.currentStorageValue.get(contractAddress.toBigInt())?.get(key.toBigInt())).toEqual(value); }); it('When reading from storage, should check the parent first', async () => { diff --git a/yarn-project/acir-simulator/src/avm/journal/journal.ts b/yarn-project/acir-simulator/src/avm/journal/journal.ts index 761a5fa47faa..6c0b591522ea 100644 --- a/yarn-project/acir-simulator/src/avm/journal/journal.ts +++ b/yarn-project/acir-simulator/src/avm/journal/journal.ts @@ -3,11 +3,13 @@ import { Fr } from '@aztec/foundation/fields'; import { RootJournalCannotBeMerged } from './errors.js'; import { HostStorage } from './host_storage.js'; -type StorageWriteMap = Map; -type ContractWriteMap = Map; +// Keeps track of the current value +type StorageValueMap = Map; +type ContractValueMap = Map; -type StorageReadMap = Map>; -type ContractReadMap = Map; +// Keeps track of all previous values +type StorageJournalMap = Map>; +type ContractJournalMap = Map; /** * Data held within the journal @@ -18,9 +20,12 @@ export type JournalData = { newL1Messages: Fr[][]; newLogs: Fr[][]; /** contract address -\> key -\> value */ - storageWrites: ContractWriteMap; + currentStorageValue: ContractValueMap; + + /** contract address -\> key -\> value[] (stored in order of access) */ + storageWrites: ContractJournalMap; /** contract address -\> key -\> value[] (stored in order of access) */ - storageReads: ContractReadMap; + storageReads: ContractJournalMap; }; /** @@ -37,7 +42,8 @@ export class AvmJournal { // Reading state - must be tracked for vm execution // contract address -> key -> value[] (array stored in order of reads) - private storageReads: Map>> = new Map(); + private storageReads: ContractJournalMap = new Map(); + private storageWrites: ContractJournalMap = new Map(); // New written state private newNoteHashes: Fr[] = []; @@ -47,8 +53,9 @@ export class AvmJournal { private newL1Messages: Fr[][] = []; private newLogs: Fr[][] = []; + // TODO: maybe i dont really want to use these aliases????????????????? // contract address -> key -> value - private storageWrites: Map> = new Map(); + private currentStorageValue: ContractValueMap = new Map(); private parentJournal: AvmJournal | undefined; @@ -81,12 +88,15 @@ export class AvmJournal { * @param value - */ public writeStorage(contractAddress: Fr, key: Fr, value: Fr) { - let contractMap = this.storageWrites.get(contractAddress.toBigInt()); + let contractMap = this.currentStorageValue.get(contractAddress.toBigInt()); if (!contractMap) { contractMap = new Map(); - this.storageWrites.set(contractAddress.toBigInt(), contractMap); + this.currentStorageValue.set(contractAddress.toBigInt(), contractMap); } contractMap.set(key.toBigInt(), value); + + // We want to keep track of all performed writes in the journal + this.journalWrite(contractAddress, key, value); } /** @@ -103,7 +113,7 @@ export class AvmJournal { // - Finally we try the host storage ( a trip to the database ) // Do not early return as we want to keep track of reads in this.storageReads - let value = this.storageWrites.get(contractAddress.toBigInt())?.get(key.toBigInt()); + let value = this.currentStorageValue.get(contractAddress.toBigInt())?.get(key.toBigInt()); if (!value && this.parentJournal) { value = await this.parentJournal?.readStorage(contractAddress, key); } @@ -123,8 +133,8 @@ export class AvmJournal { * @param key - * @param value - */ - journalRead(contractAddress: Fr, key: Fr, value: Fr): void { - let contractMap = this.storageReads.get(contractAddress.toBigInt()); + journalUpdate(map: ContractJournalMap, contractAddress: Fr, key: Fr, value: Fr): void { + let contractMap = map.get(contractAddress.toBigInt()); if (!contractMap) { contractMap = new Map>(); this.storageReads.set(contractAddress.toBigInt(), contractMap); @@ -138,6 +148,11 @@ export class AvmJournal { accessArray.push(value); } + // Create an instance of journalUpdate that appends to the read array + private journalRead = this.journalUpdate.bind(this, this.storageReads); + // Create an instance of journalUpdate that appends to the writes array + private journalWrite = this.journalUpdate.bind(this, this.storageWrites); + public writeNoteHash(noteHash: Fr) { this.newNoteHashes.push(noteHash); } @@ -171,8 +186,11 @@ export class AvmJournal { this.parentJournal.newLogs = this.parentJournal.newLogs.concat(this.newLogs); // Merge Public State - mergeContractWriteMaps(this.parentJournal.storageWrites, this.storageWrites); - mergeContractReadMaps(this.parentJournal.storageReads, this.storageReads); + mergeCurrentValueMaps(this.parentJournal.currentStorageValue, this.currentStorageValue); + + // Merge storage read and write journals + mergeContractJournalMaps(this.parentJournal.storageReads, this.storageReads); + mergeContractJournalMaps(this.parentJournal.storageWrites, this.storageWrites); } /** @@ -186,8 +204,9 @@ export class AvmJournal { newNullifiers: this.newNullifiers, newL1Messages: this.newL1Messages, newLogs: this.newLogs, - storageWrites: this.storageWrites, + currentStorageValue: this.currentStorageValue, storageReads: this.storageReads, + storageWrites: this.storageWrites, }; } } @@ -201,7 +220,7 @@ export class AvmJournal { * @param hostMap - The map to be merged into * @param childMap - The map to be merged from */ -function mergeContractWriteMaps(hostMap: ContractWriteMap, childMap: ContractWriteMap) { +function mergeCurrentValueMaps(hostMap: ContractValueMap, childMap: ContractValueMap) { for (const [key, value] of childMap) { const map1Value = hostMap.get(key); if (!map1Value) { @@ -216,7 +235,7 @@ function mergeContractWriteMaps(hostMap: ContractWriteMap, childMap: ContractWri * @param hostMap - The map to be merge into * @param childMap - The map to be merged from */ -function mergeStorageWriteMaps(hostMap: StorageWriteMap, childMap: StorageWriteMap) { +function mergeStorageWriteMaps(hostMap: StorageValueMap, childMap: StorageValueMap) { for (const [key, value] of childMap) { hostMap.set(key, value); } @@ -229,7 +248,7 @@ function mergeStorageWriteMaps(hostMap: StorageWriteMap, childMap: StorageWriteM * @param hostMap - The map to be merged into * @param childMap - The map to be merged from */ -function mergeContractReadMaps(hostMap: ContractReadMap, childMap: ContractReadMap) { +function mergeContractJournalMaps(hostMap: ContractJournalMap, childMap: ContractJournalMap) { for (const [key, value] of childMap) { const map1Value = hostMap.get(key); if (!map1Value) { @@ -244,7 +263,7 @@ function mergeContractReadMaps(hostMap: ContractReadMap, childMap: ContractReadM * @param hostMap - The map to be merge into * @param childMap - The map to be merged from */ -function mergeStorageReadMaps(hostMap: StorageReadMap, childMap: StorageReadMap) { +function mergeStorageReadMaps(hostMap: StorageJournalMap, childMap: StorageJournalMap) { for (const [key, value] of childMap) { const readArr = hostMap.get(key); if (!readArr) { diff --git a/yarn-project/acir-simulator/src/avm/opcodes/external_calls.test.ts b/yarn-project/acir-simulator/src/avm/opcodes/external_calls.test.ts index 1d91c3bc4054..e725b2233720 100644 --- a/yarn-project/acir-simulator/src/avm/opcodes/external_calls.test.ts +++ b/yarn-project/acir-simulator/src/avm/opcodes/external_calls.test.ts @@ -78,7 +78,7 @@ describe('External Calls', () => { expect(retValue).toEqual([new Field(1n), new Field(2n)]); // Check that the storage call has been merged into the parent journal - const { storageWrites } = journal.flush(); + const { currentStorageValue: storageWrites } = journal.flush(); expect(storageWrites.size).toEqual(1); const nestedContractWrites = storageWrites.get(addr.toBigInt()); From 8323dd339f88c4966453047577b3226bd75b2c31 Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 30 Jan 2024 22:59:38 +0000 Subject: [PATCH 3/5] fix: update testys --- .../src/avm/journal/journal.test.ts | 13 +++++++++-- .../acir-simulator/src/avm/journal/journal.ts | 22 +++++++++---------- 2 files changed, 22 insertions(+), 13 deletions(-) diff --git a/yarn-project/acir-simulator/src/avm/journal/journal.test.ts b/yarn-project/acir-simulator/src/avm/journal/journal.test.ts index 4eee36fd3591..1588712e1c73 100644 --- a/yarn-project/acir-simulator/src/avm/journal/journal.test.ts +++ b/yarn-project/acir-simulator/src/avm/journal/journal.test.ts @@ -61,7 +61,7 @@ describe('journal', () => { expect(cachedResult).toEqual(cachedValue); }); - it('When reading from storage, should check the cache first, and be appended to read journal', async () => { + it('When reading from storage, should check the cache first, and be appended to read/write journal', async () => { // Store a different value in storage vs the cache, and make sure the cache is returned const contractAddress = new Fr(1); const key = new Fr(2); @@ -82,10 +82,14 @@ describe('journal', () => { expect(cachedResult).toEqual(cachedValue); // We expect the journal to store the access in [storedVal, cachedVal] - [time0, time1] - const { storageReads }: JournalData = journal.flush(); + const { storageReads, storageWrites }: JournalData = journal.flush(); const contractReads = storageReads.get(contractAddress.toBigInt()); const keyReads = contractReads?.get(key.toBigInt()); expect(keyReads).toEqual([storedValue, cachedValue]); + + const contractWrites = storageWrites.get(contractAddress.toBigInt()); + const keyWrites = contractWrites?.get(key.toBigInt()); + expect(keyWrites).toEqual([cachedValue]); }); }); @@ -162,6 +166,11 @@ describe('journal', () => { const slotReads = contractReads?.get(key.toBigInt()); expect(slotReads).toEqual([value, valueT1]); + // We first write value from t0, then value from t1 + const contractWrites = journalUpdates.storageReads.get(contractAddress.toBigInt()); + const slotWrites = contractWrites?.get(key.toBigInt()); + expect(slotWrites).toEqual([value, valueT1]); + expect(journalUpdates.newNoteHashes).toEqual([commitment, commitmentT1]); expect(journalUpdates.newLogs).toEqual([logs, logsT1]); expect(journalUpdates.newL1Messages).toEqual([logs, logsT1]); diff --git a/yarn-project/acir-simulator/src/avm/journal/journal.ts b/yarn-project/acir-simulator/src/avm/journal/journal.ts index 6c0b591522ea..b1b330d94604 100644 --- a/yarn-project/acir-simulator/src/avm/journal/journal.ts +++ b/yarn-project/acir-simulator/src/avm/journal/journal.ts @@ -94,7 +94,7 @@ export class AvmJournal { this.currentStorageValue.set(contractAddress.toBigInt(), contractMap); } contractMap.set(key.toBigInt(), value); - + // We want to keep track of all performed writes in the journal this.journalWrite(contractAddress, key, value); } @@ -137,7 +137,7 @@ export class AvmJournal { let contractMap = map.get(contractAddress.toBigInt()); if (!contractMap) { contractMap = new Map>(); - this.storageReads.set(contractAddress.toBigInt(), contractMap); + map.set(contractAddress.toBigInt(), contractMap); } let accessArray = contractMap.get(key.toBigInt()); @@ -149,9 +149,9 @@ export class AvmJournal { } // Create an instance of journalUpdate that appends to the read array - private journalRead = this.journalUpdate.bind(this, this.storageReads); + private journalRead = this.journalUpdate.bind(this, this.storageReads); // Create an instance of journalUpdate that appends to the writes array - private journalWrite = this.journalUpdate.bind(this, this.storageWrites); + private journalWrite = this.journalUpdate.bind(this, this.storageWrites); public writeNoteHash(noteHash: Fr) { this.newNoteHashes.push(noteHash); @@ -212,7 +212,7 @@ export class AvmJournal { } /** - * Merges two contract write maps together + * Merges two contract current value together * Where childMap keys will take precedent over the hostMap * The assumption being that the child map is created at a later time * And thus contains more up to date information @@ -226,7 +226,7 @@ function mergeCurrentValueMaps(hostMap: ContractValueMap, childMap: ContractValu if (!map1Value) { hostMap.set(key, value); } else { - mergeStorageWriteMaps(map1Value, value); + mergeStorageCurrentValueMaps(map1Value, value); } } } @@ -235,15 +235,15 @@ function mergeCurrentValueMaps(hostMap: ContractValueMap, childMap: ContractValu * @param hostMap - The map to be merge into * @param childMap - The map to be merged from */ -function mergeStorageWriteMaps(hostMap: StorageValueMap, childMap: StorageValueMap) { +function mergeStorageCurrentValueMaps(hostMap: StorageValueMap, childMap: StorageValueMap) { for (const [key, value] of childMap) { hostMap.set(key, value); } } /** - * Merges two contract read maps together - * For read maps, we just append the childMap into the host map, as the order of reads is important + * Merges two contract journalling maps together + * For read maps, we just append the childMap arrays into the host map arrays, as the order is important * * @param hostMap - The map to be merged into * @param childMap - The map to be merged from @@ -254,7 +254,7 @@ function mergeContractJournalMaps(hostMap: ContractJournalMap, childMap: Contrac if (!map1Value) { hostMap.set(key, value); } else { - mergeStorageReadMaps(map1Value, value); + mergeStorageJournalMaps(map1Value, value); } } } @@ -263,7 +263,7 @@ function mergeContractJournalMaps(hostMap: ContractJournalMap, childMap: Contrac * @param hostMap - The map to be merge into * @param childMap - The map to be merged from */ -function mergeStorageReadMaps(hostMap: StorageJournalMap, childMap: StorageJournalMap) { +function mergeStorageJournalMaps(hostMap: StorageJournalMap, childMap: StorageJournalMap) { for (const [key, value] of childMap) { const readArr = hostMap.get(key); if (!readArr) { From 107fd5b904fc445618d5ab6bba22932ae031049c Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 30 Jan 2024 23:01:02 +0000 Subject: [PATCH 4/5] fix: find and replace thingy --- .../acir-simulator/src/avm/opcodes/external_calls.test.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/yarn-project/acir-simulator/src/avm/opcodes/external_calls.test.ts b/yarn-project/acir-simulator/src/avm/opcodes/external_calls.test.ts index e725b2233720..50c29128ec55 100644 --- a/yarn-project/acir-simulator/src/avm/opcodes/external_calls.test.ts +++ b/yarn-project/acir-simulator/src/avm/opcodes/external_calls.test.ts @@ -78,10 +78,10 @@ describe('External Calls', () => { expect(retValue).toEqual([new Field(1n), new Field(2n)]); // Check that the storage call has been merged into the parent journal - const { currentStorageValue: storageWrites } = journal.flush(); - expect(storageWrites.size).toEqual(1); + const { currentStorageValue } = journal.flush(); + expect(currentStorageValue.size).toEqual(1); - const nestedContractWrites = storageWrites.get(addr.toBigInt()); + const nestedContractWrites = currentStorageValue.get(addr.toBigInt()); expect(nestedContractWrites).toBeDefined(); const slotNumber = 1n; From 199fefa0a7147102ea45502df9e6bc8ccf1eb56d Mon Sep 17 00:00:00 2001 From: Maddiaa0 <47148561+Maddiaa0@users.noreply.github.com> Date: Tue, 30 Jan 2024 23:44:55 +0000 Subject: [PATCH 5/5] chore: remove aliases - longer but clear --- .../acir-simulator/src/avm/journal/journal.ts | 32 +++++++------------ 1 file changed, 12 insertions(+), 20 deletions(-) diff --git a/yarn-project/acir-simulator/src/avm/journal/journal.ts b/yarn-project/acir-simulator/src/avm/journal/journal.ts index b1b330d94604..699d6b93f00d 100644 --- a/yarn-project/acir-simulator/src/avm/journal/journal.ts +++ b/yarn-project/acir-simulator/src/avm/journal/journal.ts @@ -3,14 +3,6 @@ import { Fr } from '@aztec/foundation/fields'; import { RootJournalCannotBeMerged } from './errors.js'; import { HostStorage } from './host_storage.js'; -// Keeps track of the current value -type StorageValueMap = Map; -type ContractValueMap = Map; - -// Keeps track of all previous values -type StorageJournalMap = Map>; -type ContractJournalMap = Map; - /** * Data held within the journal */ @@ -19,13 +11,14 @@ export type JournalData = { newNullifiers: Fr[]; newL1Messages: Fr[][]; newLogs: Fr[][]; + /** contract address -\> key -\> value */ - currentStorageValue: ContractValueMap; + currentStorageValue: Map>; /** contract address -\> key -\> value[] (stored in order of access) */ - storageWrites: ContractJournalMap; + storageWrites: Map>; /** contract address -\> key -\> value[] (stored in order of access) */ - storageReads: ContractJournalMap; + storageReads: Map>; }; /** @@ -42,8 +35,8 @@ export class AvmJournal { // Reading state - must be tracked for vm execution // contract address -> key -> value[] (array stored in order of reads) - private storageReads: ContractJournalMap = new Map(); - private storageWrites: ContractJournalMap = new Map(); + private storageReads: Map> = new Map(); + private storageWrites: Map> = new Map(); // New written state private newNoteHashes: Fr[] = []; @@ -53,9 +46,8 @@ export class AvmJournal { private newL1Messages: Fr[][] = []; private newLogs: Fr[][] = []; - // TODO: maybe i dont really want to use these aliases????????????????? // contract address -> key -> value - private currentStorageValue: ContractValueMap = new Map(); + private currentStorageValue: Map> = new Map(); private parentJournal: AvmJournal | undefined; @@ -133,7 +125,7 @@ export class AvmJournal { * @param key - * @param value - */ - journalUpdate(map: ContractJournalMap, contractAddress: Fr, key: Fr, value: Fr): void { + journalUpdate(map: Map>, contractAddress: Fr, key: Fr, value: Fr): void { let contractMap = map.get(contractAddress.toBigInt()); if (!contractMap) { contractMap = new Map>(); @@ -220,7 +212,7 @@ export class AvmJournal { * @param hostMap - The map to be merged into * @param childMap - The map to be merged from */ -function mergeCurrentValueMaps(hostMap: ContractValueMap, childMap: ContractValueMap) { +function mergeCurrentValueMaps(hostMap: Map>, childMap: Map>) { for (const [key, value] of childMap) { const map1Value = hostMap.get(key); if (!map1Value) { @@ -235,7 +227,7 @@ function mergeCurrentValueMaps(hostMap: ContractValueMap, childMap: ContractValu * @param hostMap - The map to be merge into * @param childMap - The map to be merged from */ -function mergeStorageCurrentValueMaps(hostMap: StorageValueMap, childMap: StorageValueMap) { +function mergeStorageCurrentValueMaps(hostMap: Map, childMap: Map) { for (const [key, value] of childMap) { hostMap.set(key, value); } @@ -248,7 +240,7 @@ function mergeStorageCurrentValueMaps(hostMap: StorageValueMap, childMap: Storag * @param hostMap - The map to be merged into * @param childMap - The map to be merged from */ -function mergeContractJournalMaps(hostMap: ContractJournalMap, childMap: ContractJournalMap) { +function mergeContractJournalMaps(hostMap: Map>, childMap: Map>) { for (const [key, value] of childMap) { const map1Value = hostMap.get(key); if (!map1Value) { @@ -263,7 +255,7 @@ function mergeContractJournalMaps(hostMap: ContractJournalMap, childMap: Contrac * @param hostMap - The map to be merge into * @param childMap - The map to be merged from */ -function mergeStorageJournalMaps(hostMap: StorageJournalMap, childMap: StorageJournalMap) { +function mergeStorageJournalMaps(hostMap: Map, childMap: Map) { for (const [key, value] of childMap) { const readArr = hostMap.get(key); if (!readArr) {