Skip to content

Commit

Permalink
9131: Addressing review feedback
Browse files Browse the repository at this point in the history
  • Loading branch information
jeanmon committed Nov 22, 2024
1 parent 8f69116 commit 7452e0d
Show file tree
Hide file tree
Showing 8 changed files with 168 additions and 156 deletions.
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
#pragma once

#include "barretenberg/vm/avm/trace/common.hpp"
#include "barretenberg/vm/avm/trace/errors.hpp"
#include "barretenberg/vm/avm/trace/mem_trace.hpp"
#include <cstdint>

Expand Down
12 changes: 0 additions & 12 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/common.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -49,18 +49,6 @@ enum class AvmMemoryTag : uint32_t {

static const uint32_t MAX_MEM_TAG = MEM_TAG_U128;

enum class AvmError : uint32_t {
NO_ERROR,
TAG_ERROR,
ADDR_RES_TAG_ERROR,
REL_ADDR_OUT_OF_RANGE,
DIV_ZERO,
PARSING_ERROR,
ENV_VAR_UNKNOWN,
CONTRACT_INST_MEM_UNKNOWN,
RADIX_OUT_OF_BOUNDS,
};

static const size_t NUM_MEM_SPACES = 256;
static const uint8_t INTERNAL_CALL_SPACE_ID = 255;
static const uint32_t MAX_SIZE_INTERNAL_STACK = 1 << 16;
Expand Down
19 changes: 19 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/errors.hpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
#pragma once

#include <cstdint>

namespace bb::avm_trace {

enum class AvmError : uint32_t {
NO_ERROR,
TAG_ERROR,
ADDR_RES_TAG_ERROR,
REL_ADDR_OUT_OF_RANGE,
DIV_ZERO,
PARSING_ERROR,
ENV_VAR_UNKNOWN,
CONTRACT_INST_MEM_UNKNOWN,
RADIX_OUT_OF_BOUNDS,
};

} // namespace bb::avm_trace
4 changes: 3 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -118,13 +118,15 @@ std::string to_name(AvmError error)
return "ENVIRONMENT VARIABLE UNKNOWN";
case AvmError::CONTRACT_INST_MEM_UNKNOWN:
return "CONTRACT INSTANCE MEMBER UNKNOWN";
case AvmError::RADIX_OUT_OF_BOUNDS:
return "RADIX OUT OF BOUNDS";
default:
throw std::runtime_error("Invalid error type");
break;
}
}

bool is_valid(AvmError error)
bool is_ok(AvmError error)
{
return error == AvmError::NO_ERROR;
}
Expand Down
2 changes: 1 addition & 1 deletion barretenberg/cpp/src/barretenberg/vm/avm/trace/helper.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ std::string to_hex(bb::avm_trace::AvmMemoryTag tag);
std::string to_name(bb::avm_trace::AvmMemoryTag tag);

std::string to_name(AvmError error);
bool is_valid(AvmError error);
bool is_ok(AvmError error);

// Mutate the inputs
void inject_end_gas_values(VmPublicInputs& public_inputs, std::vector<Row>& trace);
Expand Down
258 changes: 129 additions & 129 deletions barretenberg/cpp/src/barretenberg/vm/avm/trace/trace.cpp

Large diffs are not rendered by default.

24 changes: 14 additions & 10 deletions yarn-project/simulator/src/avm/avm_memory_types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,15 +232,19 @@ 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 doesn't allow for arrays that big.
static readonly MAX_MEMORY_SIZE = Number((1n << 32n) - 2n);
// 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[];

constructor() {
// We do not initialize memory size here because otherwise tests blow up when diffing.
this._mem = [];
}

public getMaxMemorySize(): number {
return TaggedMemory.MAX_MEMORY_SIZE;
}

/** Returns a MeteredTaggedMemory instance to track the number of reads and writes if TRACK_MEMORY_ACCESSES is set. */
public track(type: string = 'instruction'): TaggedMemoryInterface {
return TaggedMemory.TRACK_MEMORY_ACCESSES ? new MeteredTaggedMemory(this, type) : this;
Expand All @@ -264,8 +268,7 @@ export class TaggedMemory implements TaggedMemoryInterface {
}

public getSlice(offset: number, size: number): MemoryValue[] {
assert(offset < TaggedMemory.MAX_MEMORY_SIZE);
assert(offset + size < TaggedMemory.MAX_MEMORY_SIZE);
assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE);
const value = this._mem.slice(offset, offset + size);
TaggedMemory.log.debug(`getSlice(${offset}, ${size}) = ${value}`);
for (let i = 0; i < value.length; i++) {
Expand All @@ -278,14 +281,12 @@ export class TaggedMemory implements TaggedMemoryInterface {
}

public getSliceAs<T>(offset: number, size: number): T[] {
assert(offset < TaggedMemory.MAX_MEMORY_SIZE);
assert(offset + size < TaggedMemory.MAX_MEMORY_SIZE);
assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE);
return this.getSlice(offset, size) as T[];
}

public getSliceTags(offset: number, size: number): TypeTag[] {
assert(offset < TaggedMemory.MAX_MEMORY_SIZE);
assert(offset + size < TaggedMemory.MAX_MEMORY_SIZE);
assert(offset + size <= TaggedMemory.MAX_MEMORY_SIZE);
return this._mem.slice(offset, offset + size).map(TaggedMemory.getTag);
}

Expand All @@ -296,8 +297,7 @@ export class TaggedMemory implements TaggedMemoryInterface {
}

public setSlice(offset: number, vs: MemoryValue[]) {
assert(offset < TaggedMemory.MAX_MEMORY_SIZE);
assert(offset + vs.length < TaggedMemory.MAX_MEMORY_SIZE);
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;
Expand Down Expand Up @@ -475,6 +475,10 @@ export class MeteredTaggedMemory implements TaggedMemoryInterface {
}
}

public getMaxMemorySize(): number {
return this.wrapped.getMaxMemorySize();
}

public track(type: string = 'instruction'): MeteredTaggedMemory {
return new MeteredTaggedMemory(this.wrapped, type);
}
Expand Down
3 changes: 1 addition & 2 deletions yarn-project/simulator/src/avm/opcodes/addressing_mode.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { strict as assert } from 'assert';
import { maxUint32 } from 'viem';

import { type TaggedMemoryInterface } from '../avm_memory_types.js';
import { AddressOutOfRangeError } from '../errors.js';
Expand Down Expand Up @@ -67,7 +66,7 @@ export class Addressing {
mem.checkIsValidMemoryOffsetTag(0);
const baseAddr = Number(mem.get(0).toBigInt());
resolved[i] += baseAddr;
if (resolved[i] > maxUint32) {
if (resolved[i] >= mem.getMaxMemorySize()) {
throw new AddressOutOfRangeError(baseAddr, offset);
}
}
Expand Down

0 comments on commit 7452e0d

Please sign in to comment.