Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions yarn-project/simulator/src/public/avm/opcodes/memory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,42 +13,42 @@ export class Set extends Instruction {
OperandType.UINT8, // opcode
OperandType.UINT8, // indirect
OperandType.UINT8, // dstOffset
OperandType.UINT8, // tag
OperandType.TAG, // tag
OperandType.UINT8, // const (value)
];
public static readonly wireFormat16: OperandType[] = [
OperandType.UINT8, // opcode
OperandType.UINT8, // indirect
OperandType.UINT16, // dstOffset
OperandType.UINT8, // tag
OperandType.TAG, // tag
OperandType.UINT16, // const (value)
];
public static readonly wireFormat32: OperandType[] = [
OperandType.UINT8, // opcode
OperandType.UINT8, // indirect
OperandType.UINT16, // dstOffset
OperandType.UINT8, // tag
OperandType.TAG, // tag
OperandType.UINT32, // const (value)
];
public static readonly wireFormat64: OperandType[] = [
OperandType.UINT8, // opcode
OperandType.UINT8, // indirect
OperandType.UINT16, // dstOffset
OperandType.UINT8, // tag
OperandType.TAG, // tag
OperandType.UINT64, // const (value)
];
public static readonly wireFormat128: OperandType[] = [
OperandType.UINT8, // opcode
OperandType.UINT8, // indirect
OperandType.UINT16, // dstOffset
OperandType.UINT8, // tag
OperandType.TAG, // tag
OperandType.UINT128, // const (value)
];
public static readonly wireFormatFF: OperandType[] = [
OperandType.UINT8, // opcode
OperandType.UINT8, // indirect
OperandType.UINT16, // dstOffset
OperandType.UINT8, // tag
OperandType.TAG, // tag
OperandType.FF, // const (value)
];

Expand All @@ -59,7 +59,6 @@ export class Set extends Instruction {
private value: bigint | number,
) {
super();
TaggedMemory.checkIsValidTag(inTag);
}

public async execute(context: AvmContext): Promise<void> {
Expand All @@ -85,19 +84,18 @@ export class Cast extends Instruction {
OperandType.UINT8,
OperandType.UINT8,
OperandType.UINT8,
OperandType.UINT8,
OperandType.TAG,
];
static readonly wireFormat16 = [
OperandType.UINT8,
OperandType.UINT8,
OperandType.UINT16,
OperandType.UINT16,
OperandType.UINT8,
OperandType.TAG,
];

constructor(private indirect: number, private srcOffset: number, private dstOffset: number, private dstTag: number) {
super();
TaggedMemory.checkIsValidTag(dstTag);
}

public async execute(context: AvmContext): Promise<void> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,22 @@ describe('Bytecode Serialization', () => {
}
});

it('Should throw an AvmParsingError while deserializing an incomplete instruction and a wrong tag value', () => {
const decodeIncomplete = (truncated: Buffer) => {
return () => decodeFromBytecode(truncated);
};

const bufSet16Truncated = Buffer.from([
Opcode.SET_16, //opcode
0x02, // indirect
...Buffer.from('3456', 'hex'), // dstOffset
0x09, //tag (invalid)
// We should have a 16-bit value here (truncation)
]);

expect(decodeIncomplete(bufSet16Truncated)).toThrow(AvmParsingError);
});

it('Should throw an InvalidTagValueError while deserializing a tag value out of range', () => {
const decodeInvalidTag = (buf: Buffer) => {
return () => decodeFromBytecode(buf);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import { strict as assert } from 'assert';

import { TaggedMemory } from '../avm_memory_types.js';
import { BufferCursor } from './buffer_cursor.js';

/**
Expand Down Expand Up @@ -103,8 +104,12 @@ export enum OperandType {
UINT64,
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Re: lines +96 to +99]

arg, missed this

See this comment inline on Graphite.

UINT128,
FF,
TAG,
}

// Define a type that represents the possible types of the deserialized values.
type DeserializedValue = number | bigint;

type OperandNativeType = number | bigint;
type OperandWriter = (value: any) => void;

Expand All @@ -116,6 +121,7 @@ const OPERAND_SPEC = new Map<OperandType, [number, (offset: number) => OperandNa
[OperandType.UINT64, [8, Buffer.prototype.readBigInt64BE, Buffer.prototype.writeBigInt64BE]],
[OperandType.UINT128, [16, readBigInt128BE, writeBigInt128BE]],
[OperandType.FF, [32, readBigInt254BE, writeBigInt254BE]],
[OperandType.TAG, [1, Buffer.prototype.readUint8, Buffer.prototype.writeUint8]],
]);

function readBigInt254BE(this: Buffer, offset: number): bigint {
Expand Down Expand Up @@ -160,19 +166,30 @@ function writeBigInt128BE(this: Buffer, value: bigint): void {
* @param operands Specification of the operand types.
* @returns An array as big as {@code operands}, with the converted TS values.
*/
export function deserialize(cursor: BufferCursor | Buffer, operands: OperandType[]): (number | bigint)[] {
const argValues = [];
export function deserialize(cursor: BufferCursor | Buffer, operands: OperandType[]): DeserializedValue[] {
const argValues: DeserializedValue[] = [];
if (Buffer.isBuffer(cursor)) {
cursor = new BufferCursor(cursor);
}

for (const op of operands) {
const opType = op;
for (const opType of operands) {
const [sizeBytes, reader, _writer] = OPERAND_SPEC.get(opType)!;
argValues.push(reader.call(cursor.buffer(), cursor.position()));
const value = reader.call(cursor.buffer(), cursor.position());
argValues.push(value);
cursor.advance(sizeBytes);
}

// We first want to detect other parsing errors (e.g., instruction size
// is longer than remaining bytes) first and therefore tag validation is done after completion
// of parsing above. Order of errors need to match with circuit.
for (let i = 0; i < operands.length; i++) {
if (operands[i] === OperandType.TAG) {
// We parsed a single byte (readUInt8()) and therefore value is of number type (not bigint)
// We need to cast to number because checkIsValidTag expects a number.
TaggedMemory.checkIsValidTag(Number(argValues[i]) ?? 0);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that this defensive expression is not really helping. You might be calling Number with undefined, I think I had written Number(argValues[i] ?? 0) for a reason :)

}
}

return argValues;
}

Expand Down