From 128841ad89447c66726def1a887cb1f0a2fbc93e Mon Sep 17 00:00:00 2001 From: jeanmon Date: Fri, 13 Dec 2024 17:05:09 +0000 Subject: [PATCH 1/2] 10370: Migrate simuluator memory to a map and align the range of addresses with the AVM circuit --- .../src/avm/avm_memory_types.test.ts | 4 +- .../simulator/src/avm/avm_memory_types.ts | 59 +++++++++++-------- 2 files changed, 35 insertions(+), 28 deletions(-) diff --git a/yarn-project/simulator/src/avm/avm_memory_types.test.ts b/yarn-project/simulator/src/avm/avm_memory_types.test.ts index 18c3651a2ed8..cacb5da184b9 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.test.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.test.ts @@ -28,13 +28,13 @@ describe('TaggedMemory', () => { expect(mem.get(10)).toStrictEqual(new Field(5)); }); - it(`Should fail getSlice on unset elements`, () => { + it(`Slice should get Field(0) on unset elements`, () => { const mem = new TaggedMemory(); mem.set(10, new Field(10)); mem.set(12, new Field(12)); - expect(() => mem.getSlice(10, /*size=*/ 4)).toThrow(/size/); + expect(mem.getSlice(10, /*size=*/ 4)).toStrictEqual([new Field(10), new Field(0), new Field(12), new Field(0)]); }); it(`Should set and get slices`, () => { diff --git a/yarn-project/simulator/src/avm/avm_memory_types.ts b/yarn-project/simulator/src/avm/avm_memory_types.ts index bc9aa66e4685..de5a9d0ce747 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.ts @@ -232,13 +232,14 @@ export class TaggedMemory implements TaggedMemoryInterface { // Whether to track and validate memory accesses for each instruction. static readonly TRACK_MEMORY_ACCESSES = process.env.NODE_ENV === 'test'; - // FIXME: memory should be 2^32, but TS max array size is: 2^32 - 1 - static readonly MAX_MEMORY_SIZE = Number((1n << 32n) - 1n); - private _mem: MemoryValue[]; + // Memory is modelled by a map with key type being number. + // We however restrict the keys to be non-negative integers smaller than + // MAX_MEMORY_SIZE. + static readonly MAX_MEMORY_SIZE = Number(1n << 32n); + private _mem: Map; constructor() { - // We do not initialize memory size here because otherwise tests blow up when diffing. - this._mem = []; + this._mem = new Map(); } public getMaxMemorySize(): number { @@ -257,8 +258,9 @@ export class TaggedMemory implements TaggedMemoryInterface { } public getAs(offset: number): T { + assert(Number.isInteger(offset)); assert(offset < TaggedMemory.MAX_MEMORY_SIZE); - const word = this._mem[offset]; + const word = this._mem.get(offset); TaggedMemory.log.trace(`get(${offset}) = ${word}`); if (word === undefined) { TaggedMemory.log.debug(`WARNING: Memory at offset ${offset} is undefined!`); @@ -268,46 +270,51 @@ export class TaggedMemory implements TaggedMemoryInterface { } public getSlice(offset: number, size: number): MemoryValue[] { + assert(Number.isInteger(offset) && Number.isInteger(size)); assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE); - const value = this._mem.slice(offset, offset + size); - TaggedMemory.log.trace(`getSlice(${offset}, ${size}) = ${value}`); - for (let i = 0; i < value.length; i++) { - if (value[i] === undefined) { - value[i] = new Field(0); + const slice = new Array(size); + + for (let i = 0; i < size; i++) { + const value = this._mem.get(offset + i); + if (value === undefined) { + slice[i] = new Field(0); + } else { + slice[i] = value; } } - assert(value.length === size, `Expected slice of size ${size}, got ${value.length}.`); - return value; + + TaggedMemory.log.trace(`getSlice(${offset}, ${size}) = ${slice}`); + return slice; } public getSliceAs(offset: number, size: number): T[] { - assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE); return this.getSlice(offset, size) as T[]; } public getSliceTags(offset: number, size: number): TypeTag[] { - assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE); - return this._mem.slice(offset, offset + size).map(TaggedMemory.getTag); + return this.getSlice(offset, size).map(TaggedMemory.getTag); } public set(offset: number, v: MemoryValue) { + assert(Number.isInteger(offset)); assert(offset < TaggedMemory.MAX_MEMORY_SIZE); - this._mem[offset] = v; + this._mem.set(offset, v); TaggedMemory.log.trace(`set(${offset}, ${v})`); } - public setSlice(offset: number, vs: MemoryValue[]) { - assert(offset + vs.length <= TaggedMemory.MAX_MEMORY_SIZE); - // We may need to extend the memory size, otherwise splice doesn't insert. - if (offset + vs.length > this._mem.length) { - this._mem.length = offset + vs.length; - } - this._mem.splice(offset, vs.length, ...vs); - TaggedMemory.log.trace(`setSlice(${offset}, ${vs})`); + public setSlice(offset: number, slice: MemoryValue[]) { + assert(Number.isInteger(offset)); + assert(offset + slice.length <= TaggedMemory.MAX_MEMORY_SIZE); + slice.forEach((element, idx) => { + this._mem.set(offset + idx, element); + }); + TaggedMemory.log.trace(`setSlice(${offset}, ${slice})`); } public getTag(offset: number): TypeTag { - return TaggedMemory.getTag(this._mem[offset]); + assert(Number.isInteger(offset)); + assert(offset < TaggedMemory.MAX_MEMORY_SIZE); + return TaggedMemory.getTag(this._mem.get(offset)); } /** From 8a040cab05a69281410f6ea94b536b2aa3236725 Mon Sep 17 00:00:00 2001 From: jeanmon Date: Mon, 16 Dec 2024 10:24:12 +0000 Subject: [PATCH 2/2] 10370: address review comments --- .../src/avm/avm_memory_types.test.ts | 20 +++++++++++++++++++ .../simulator/src/avm/avm_memory_types.ts | 7 +------ 2 files changed, 21 insertions(+), 6 deletions(-) diff --git a/yarn-project/simulator/src/avm/avm_memory_types.test.ts b/yarn-project/simulator/src/avm/avm_memory_types.test.ts index cacb5da184b9..8d1146d1021d 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.test.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.test.ts @@ -1,3 +1,5 @@ +import { AssertionError } from 'assert'; + import { Field, MeteredTaggedMemory, @@ -45,6 +47,24 @@ describe('TaggedMemory', () => { expect(mem.getSlice(10, /*size=*/ 2)).toStrictEqual(val); }); + + it(`Should access and write in last index of memory`, () => { + const last = TaggedMemory.MAX_MEMORY_SIZE - 1; + const mem = new TaggedMemory(); + const val = new Uint64(42); + mem.set(last, val); + expect(mem.get(last)).toStrictEqual(val); + }); + + it(`Should not access beyond memory last index`, () => { + const mem = new TaggedMemory(); + + expect(() => mem.set(TaggedMemory.MAX_MEMORY_SIZE, new Field(1))).toThrow(AssertionError); + expect(() => mem.get(TaggedMemory.MAX_MEMORY_SIZE)).toThrow(AssertionError); + + expect(() => mem.set(TaggedMemory.MAX_MEMORY_SIZE + 15, new Field(1))).toThrow(AssertionError); + expect(() => mem.get(TaggedMemory.MAX_MEMORY_SIZE + 7)).toThrow(AssertionError); + }); }); describe('MeteredTaggedMemory', () => { diff --git a/yarn-project/simulator/src/avm/avm_memory_types.ts b/yarn-project/simulator/src/avm/avm_memory_types.ts index de5a9d0ce747..444af4376a20 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.ts @@ -275,12 +275,7 @@ export class TaggedMemory implements TaggedMemoryInterface { const slice = new Array(size); for (let i = 0; i < size; i++) { - const value = this._mem.get(offset + i); - if (value === undefined) { - slice[i] = new Field(0); - } else { - slice[i] = value; - } + slice[i] = this._mem.get(offset + i) ?? new Field(0); } TaggedMemory.log.trace(`getSlice(${offset}, ${size}) = ${slice}`);