From 87a96635e5f873fcb4918fc05a3f6fbe66986ad7 Mon Sep 17 00:00:00 2001 From: Facundo Date: Thu, 8 Feb 2024 19:54:36 +0000 Subject: [PATCH] chore(avm): remove field support for comparators and bitwise ops (#4516) --- .../src/avm/avm_memory_types.test.ts | 7 ----- .../simulator/src/avm/avm_memory_types.ts | 15 +++++---- yarn-project/simulator/src/avm/errors.ts | 12 +++++-- .../src/avm/opcodes/comparators.test.ts | 31 +++++-------------- .../simulator/src/avm/opcodes/comparators.ts | 11 ++++--- .../docs/public-vm/gen/_InstructionSet.mdx | 16 +++++----- .../InstructionSet/InstructionSet.js | 17 +++++----- 7 files changed, 51 insertions(+), 58 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 9aeccaa73b9..cdb581467de 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.test.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.test.ts @@ -175,13 +175,6 @@ describe('Field', () => { expect(field1.equals(field3)).toBe(false); }); - it(`Should check if one Field is less than another correctly`, () => { - const field1 = new Field(5); - const field2 = new Field(10); - expect(field1.lt(field2)).toBe(true); - expect(field2.lt(field1)).toBe(false); - }); - it(`Should convert Field to BigInt correctly`, () => { const field = new Field(5); expect(field.toBigInt()).toStrictEqual(5n); diff --git a/yarn-project/simulator/src/avm/avm_memory_types.ts b/yarn-project/simulator/src/avm/avm_memory_types.ts index 12df18223c1..f4fb0b80e9c 100644 --- a/yarn-project/simulator/src/avm/avm_memory_types.ts +++ b/yarn-project/simulator/src/avm/avm_memory_types.ts @@ -11,7 +11,6 @@ export abstract class MemoryValue { public abstract div(rhs: MemoryValue): MemoryValue; public abstract equals(rhs: MemoryValue): boolean; - public abstract lt(rhs: MemoryValue): boolean; // We need this to be able to build an instance of the subclasses. public abstract build(n: bigint): MemoryValue; @@ -32,6 +31,8 @@ export abstract class IntegralValue extends MemoryValue { public abstract or(rhs: IntegralValue): IntegralValue; public abstract xor(rhs: IntegralValue): IntegralValue; public abstract not(): IntegralValue; + + public abstract lt(rhs: MemoryValue): boolean; } // TODO: Optimize calculation of mod, etc. Can only do once per class? @@ -200,10 +201,6 @@ export class Field extends MemoryValue { return this.rep.equals(rhs.rep); } - public lt(rhs: Field): boolean { - return this.rep.lt(rhs.rep); - } - public toBigInt(): bigint { return this.rep.toBigInt(); } @@ -284,7 +281,13 @@ export class TaggedMemory { */ public checkTag(tag: TypeTag, offset: number) { if (this.getTag(offset) !== tag) { - throw new TagCheckError(offset, TypeTag[this.getTag(offset)], TypeTag[tag]); + throw TagCheckError.forOffset(offset, TypeTag[this.getTag(offset)], TypeTag[tag]); + } + } + + public static checkIsIntegralTag(tag: TypeTag) { + if (![TypeTag.UINT8, TypeTag.UINT16, TypeTag.UINT32, TypeTag.UINT64, TypeTag.UINT128].includes(tag)) { + throw TagCheckError.forTag(TypeTag[tag], 'integral'); } } diff --git a/yarn-project/simulator/src/avm/errors.ts b/yarn-project/simulator/src/avm/errors.ts index 735abb33df5..24d52eea026 100644 --- a/yarn-project/simulator/src/avm/errors.ts +++ b/yarn-project/simulator/src/avm/errors.ts @@ -42,8 +42,16 @@ export class InstructionExecutionError extends AvmExecutionError { * Error thrown on failed AVM memory tag check. */ export class TagCheckError extends AvmExecutionError { - constructor(offset: number, gotTag: string, expectedTag: string) { - super(`Memory offset ${offset} has tag ${gotTag}, expected ${expectedTag}`); + public static forOffset(offset: number, gotTag: string, expectedTag: string): TagCheckError { + return new TagCheckError(`Tag mismatch at offset ${offset}, got ${gotTag}, expected ${expectedTag}`); + } + + public static forTag(gotTag: string, expectedTag: string): TagCheckError { + return new TagCheckError(`Tag mismatch, got ${gotTag}, expected ${expectedTag}`); + } + + constructor(message: string) { + super(message); this.name = 'TagCheckError'; } } diff --git a/yarn-project/simulator/src/avm/opcodes/comparators.test.ts b/yarn-project/simulator/src/avm/opcodes/comparators.test.ts index c8001c7759f..591c657239e 100644 --- a/yarn-project/simulator/src/avm/opcodes/comparators.test.ts +++ b/yarn-project/simulator/src/avm/opcodes/comparators.test.ts @@ -110,19 +110,11 @@ describe('Comparators', () => { expect(actual).toEqual([new Uint32(0), new Uint32(1), new Uint32(0)]); }); - it('Works on field elements', async () => { - context.machineState.memory.setSlice(0, [new Field(1), new Field(2), new Field(0)]); - - [ - new Lt(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 0, /*dstOffset=*/ 10), - new Lt(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 11), - new Lt(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 2, /*dstOffset=*/ 12), - ].forEach(i => i.execute(context)); - - const actual = context.machineState.memory.getSlice(/*offset=*/ 10, /*size=*/ 4); - expect(actual).toEqual([new Field(0), new Field(1), new Field(0)]); + it('Does not work on field elements', async () => { + await expect(() => + new Lt(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 10).execute(context), + ).rejects.toThrow(TagCheckError); }); - it('InTag is checked', async () => { context.machineState.memory.setSlice(0, [new Field(1), new Uint32(2), new Uint16(3)]); @@ -174,17 +166,10 @@ describe('Comparators', () => { expect(actual).toEqual([new Uint32(1), new Uint32(1), new Uint32(0)]); }); - it('Works on field elements', async () => { - context.machineState.memory.setSlice(0, [new Field(1), new Field(2), new Field(0)]); - - [ - new Lte(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 0, /*dstOffset=*/ 10), - new Lte(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 11), - new Lte(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 2, /*dstOffset=*/ 12), - ].forEach(i => i.execute(context)); - - const actual = context.machineState.memory.getSlice(/*offset=*/ 10, /*size=*/ 4); - expect(actual).toEqual([new Field(1), new Field(1), new Field(0)]); + it('Does not work on field elements', async () => { + await expect(() => + new Lte(/*indirect=*/ 0, TypeTag.FIELD, /*aOffset=*/ 0, /*bOffset=*/ 1, /*dstOffset=*/ 10).execute(context), + ).rejects.toThrow(TagCheckError); }); it('InTag is checked', async () => { diff --git a/yarn-project/simulator/src/avm/opcodes/comparators.ts b/yarn-project/simulator/src/avm/opcodes/comparators.ts index 3f896248738..cff0b9baa38 100644 --- a/yarn-project/simulator/src/avm/opcodes/comparators.ts +++ b/yarn-project/simulator/src/avm/opcodes/comparators.ts @@ -1,4 +1,5 @@ import type { AvmContext } from '../avm_context.js'; +import { IntegralValue, TaggedMemory } from '../avm_memory_types.js'; import { Opcode } from '../serialization/instruction_serialization.js'; import { ThreeOperandInstruction } from './instruction_impl.js'; @@ -34,9 +35,10 @@ export class Lt extends ThreeOperandInstruction { async execute(context: AvmContext): Promise { context.machineState.memory.checkTags(this.inTag, this.aOffset, this.bOffset); + TaggedMemory.checkIsIntegralTag(this.inTag); - const a = context.machineState.memory.get(this.aOffset); - const b = context.machineState.memory.get(this.bOffset); + const a = context.machineState.memory.getAs(this.aOffset); + const b = context.machineState.memory.getAs(this.bOffset); // Result will be of the same type as 'a'. const dest = a.build(a.lt(b) ? 1n : 0n); @@ -56,9 +58,10 @@ export class Lte extends ThreeOperandInstruction { async execute(context: AvmContext): Promise { context.machineState.memory.checkTags(this.inTag, this.aOffset, this.bOffset); + TaggedMemory.checkIsIntegralTag(this.inTag); - const a = context.machineState.memory.get(this.aOffset); - const b = context.machineState.memory.get(this.bOffset); + const a = context.machineState.memory.getAs(this.aOffset); + const b = context.machineState.memory.getAs(this.bOffset); // Result will be of the same type as 'a'. const dest = a.build(a.equals(b) || a.lt(b) ? 1n : 0n); diff --git a/yellow-paper/docs/public-vm/gen/_InstructionSet.mdx b/yellow-paper/docs/public-vm/gen/_InstructionSet.mdx index 0e7456c8a16..d11198262d2 100644 --- a/yellow-paper/docs/public-vm/gen/_InstructionSet.mdx +++ b/yellow-paper/docs/public-vm/gen/_InstructionSet.mdx @@ -576,7 +576,7 @@ Less-than check (a < b) - **Category**: Compute - Comparators - **Flags**: - **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`. - - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. + - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction. - **Args**: - **aOffset**: memory offset of the operation's left input - **bOffset**: memory offset of the operation's right input @@ -597,7 +597,7 @@ Less-than-or-equals check (a <= b) - **Category**: Compute - Comparators - **Flags**: - **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`. - - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. + - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction. - **Args**: - **aOffset**: memory offset of the operation's left input - **bOffset**: memory offset of the operation's right input @@ -618,7 +618,7 @@ Bitwise AND (a & b) - **Category**: Compute - Bitwise - **Flags**: - **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`. - - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. + - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction. - **Args**: - **aOffset**: memory offset of the operation's left input - **bOffset**: memory offset of the operation's right input @@ -639,7 +639,7 @@ Bitwise OR (a | b) - **Category**: Compute - Bitwise - **Flags**: - **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`. - - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. + - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction. - **Args**: - **aOffset**: memory offset of the operation's left input - **bOffset**: memory offset of the operation's right input @@ -660,7 +660,7 @@ Bitwise XOR (a ^ b) - **Category**: Compute - Bitwise - **Flags**: - **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`. - - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. + - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction. - **Args**: - **aOffset**: memory offset of the operation's left input - **bOffset**: memory offset of the operation's right input @@ -681,7 +681,7 @@ Bitwise NOT (inversion) - **Category**: Compute - Bitwise - **Flags**: - **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`. - - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. + - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction. - **Args**: - **aOffset**: memory offset of the operation's input - **dstOffset**: memory offset specifying where to store operation's result @@ -701,7 +701,7 @@ Bitwise leftward shift (a << b) - **Category**: Compute - Bitwise - **Flags**: - **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`. - - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. + - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction. - **Args**: - **aOffset**: memory offset of the operation's left input - **bOffset**: memory offset of the operation's right input @@ -722,7 +722,7 @@ Bitwise rightward shift (a >> b) - **Category**: Compute - Bitwise - **Flags**: - **indirect**: Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`. - - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. + - **inTag**: The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with. `field` type is NOT supported for this instruction. - **Args**: - **aOffset**: memory offset of the operation's left input - **bOffset**: memory offset of the operation's right input diff --git a/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js b/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js index 01014150f3e..ae297566cb4 100644 --- a/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js +++ b/yellow-paper/src/preprocess/InstructionSet/InstructionSet.js @@ -8,6 +8,7 @@ const TOPICS_IN_SECTIONS = [ ]; const IN_TAG_DESCRIPTION = "The [tag/size](./state-model#tags-and-tagged-memory) to check inputs against and tag the destination with."; +const IN_TAG_DESCRIPTION_NO_FIELD = IN_TAG_DESCRIPTION + " `field` type is NOT supported for this instruction."; const DST_TAG_DESCRIPTION = "The [tag/size](./state-model#tags-and-tagged-memory) to tag the destination with but not to check inputs against."; const INDIRECT_FLAG_DESCRIPTION = "Toggles whether each memory-offset argument is an indirect offset. Rightmost bit corresponds to 0th offset arg, etc. Indirect offsets result in memory accesses like `M[M[offset]]` instead of the more standard `M[offset]`."; @@ -113,7 +114,7 @@ const INSTRUCTION_SET_RAW = [ "Category": "Compute - Comparators", "Flags": [ {"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION}, - {"name": "inTag", "description": IN_TAG_DESCRIPTION}, + {"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD}, ], "Args": [ {"name": "aOffset", "description": "memory offset of the operation's left input"}, @@ -132,7 +133,7 @@ const INSTRUCTION_SET_RAW = [ "Category": "Compute - Comparators", "Flags": [ {"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION}, - {"name": "inTag", "description": IN_TAG_DESCRIPTION}, + {"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD}, ], "Args": [ {"name": "aOffset", "description": "memory offset of the operation's left input"}, @@ -151,7 +152,7 @@ const INSTRUCTION_SET_RAW = [ "Category": "Compute - Bitwise", "Flags": [ {"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION}, - {"name": "inTag", "description": IN_TAG_DESCRIPTION}, + {"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD}, ], "Args": [ {"name": "aOffset", "description": "memory offset of the operation's left input"}, @@ -170,7 +171,7 @@ const INSTRUCTION_SET_RAW = [ "Category": "Compute - Bitwise", "Flags": [ {"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION}, - {"name": "inTag", "description": IN_TAG_DESCRIPTION}, + {"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD}, ], "Args": [ {"name": "aOffset", "description": "memory offset of the operation's left input"}, @@ -189,7 +190,7 @@ const INSTRUCTION_SET_RAW = [ "Category": "Compute - Bitwise", "Flags": [ {"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION}, - {"name": "inTag", "description": IN_TAG_DESCRIPTION}, + {"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD}, ], "Args": [ {"name": "aOffset", "description": "memory offset of the operation's left input"}, @@ -208,7 +209,7 @@ const INSTRUCTION_SET_RAW = [ "Category": "Compute - Bitwise", "Flags": [ {"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION}, - {"name": "inTag", "description": IN_TAG_DESCRIPTION}, + {"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD}, ], "Args": [ {"name": "aOffset", "description": "memory offset of the operation's input"}, @@ -226,7 +227,7 @@ const INSTRUCTION_SET_RAW = [ "Category": "Compute - Bitwise", "Flags": [ {"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION}, - {"name": "inTag", "description": IN_TAG_DESCRIPTION}, + {"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD}, ], "Args": [ {"name": "aOffset", "description": "memory offset of the operation's left input"}, @@ -245,7 +246,7 @@ const INSTRUCTION_SET_RAW = [ "Category": "Compute - Bitwise", "Flags": [ {"name": "indirect", "description": INDIRECT_FLAG_DESCRIPTION}, - {"name": "inTag", "description": IN_TAG_DESCRIPTION}, + {"name": "inTag", "description": IN_TAG_DESCRIPTION_NO_FIELD}, ], "Args": [ {"name": "aOffset", "description": "memory offset of the operation's left input"},