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
1 change: 1 addition & 0 deletions barretenberg/cpp/pil/vm2/execution.pil
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ include "class_id_derivation.pil";
include "range_check.pil";
include "bitwise.pil";
include "merkle_check.pil";
include "memory.pil";
include "precomputed.pil";
include "sha256.pil";
include "ecc.pil";
Expand Down
19 changes: 19 additions & 0 deletions barretenberg/cpp/pil/vm2/memory.pil
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
include "range_check.pil";

namespace memory;

pol commit sel;
pol commit address;
pol commit value;
pol commit tag;
pol commit rw;
pol commit space_id;

#[skippable_if]
sel = 0;

sel * (sel - 1) = 0;
rw * (1 - rw) = 0;

// TODO: consider tag-value consistency checking.
// TODO: consider address range checking.
23 changes: 23 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm2/common/tagged_value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,29 @@ template <typename Op> struct UnaryOperationVisitor {

} // namespace

uint8_t get_tag_bits(ValueTag tag)
{
switch (tag) {
case ValueTag::U1:
return 1;
case ValueTag::U8:
return 8;
case ValueTag::U16:
return 16;
case ValueTag::U32:
return 32;
case ValueTag::U64:
return 64;
case ValueTag::U128:
return 128;
case ValueTag::FF:
return 254;
}

assert(false && "Invalid tag");
return 0;
}

// Constructor
TaggedValue::TaggedValue(TaggedValue::value_type value_)
: value(value_)
Expand Down
2 changes: 2 additions & 0 deletions barretenberg/cpp/src/barretenberg/vm2/common/tagged_value.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,8 @@ template <typename T> ValueTag tag_for_type()
}
}

uint8_t get_tag_bits(ValueTag tag);

class TaggedValue {
public:
// We are using variant to avoid heap allocations at the cost of a bigger memory footprint.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,7 @@ using ::testing::ReturnRef;
using ::testing::StrictMock;

using simulation::EventEmitter;
using simulation::Memory;
using simulation::MemoryEvent;
using simulation::NoopEventEmitter;
using simulation::MemoryStore;
using simulation::Sha256;
using simulation::Sha256CompressionEvent;

Expand All @@ -53,8 +51,7 @@ TEST(Sha256ConstrainingTest, EmptyRow)
// TOOD: Replace this with a hardcoded test vector and write a negative test
TEST(Sha256ConstrainingTest, Basic)
{
NoopEventEmitter<MemoryEvent> emitter;
Memory mem(/*space_id=*/0, emitter);
MemoryStore mem;
StrictMock<simulation::MockContext> context;
EXPECT_CALL(context, get_memory()).WillRepeatedly(ReturnRef(mem));

Expand Down Expand Up @@ -89,8 +86,7 @@ TEST(Sha256ConstrainingTest, Basic)

TEST(Sha256ConstrainingTest, Interaction)
{
NoopEventEmitter<MemoryEvent> emitter;
Memory mem(/*space_id=*/0, emitter);
MemoryStore mem;
StrictMock<simulation::MockContext> context;
EXPECT_CALL(context, get_memory()).WillRepeatedly(ReturnRef(mem));

Expand Down
6 changes: 3 additions & 3 deletions barretenberg/cpp/src/barretenberg/vm2/generated/columns.hpp

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
#include "relations/execution.hpp"
#include "relations/ff_gt.hpp"
#include "relations/instr_fetching.hpp"
#include "relations/memory.hpp"
#include "relations/merkle_check.hpp"
#include "relations/nullifier_check.hpp"
#include "relations/poseidon2_hash.hpp"
Expand Down Expand Up @@ -49,10 +50,10 @@ namespace bb::avm2 {

struct AvmFlavorVariables {
static constexpr size_t NUM_PRECOMPUTED_ENTITIES = 45;
static constexpr size_t NUM_WITNESS_ENTITIES = 1020;
static constexpr size_t NUM_WITNESS_ENTITIES = 1026;
static constexpr size_t NUM_SHIFTED_ENTITIES = 135;
static constexpr size_t NUM_WIRES = NUM_WITNESS_ENTITIES + NUM_PRECOMPUTED_ENTITIES;
static constexpr size_t NUM_ALL_ENTITIES = 1200;
static constexpr size_t NUM_ALL_ENTITIES = 1206;

// Need to be templated for recursive verifier
template <typename FF_>
Expand All @@ -71,6 +72,7 @@ struct AvmFlavorVariables {
avm2::execution<FF_>,
avm2::ff_gt<FF_>,
avm2::instr_fetching<FF_>,
avm2::memory<FF_>,
avm2::merkle_check<FF_>,
avm2::nullifier_check<FF_>,
avm2::poseidon2_hash<FF_>,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
// AUTOGENERATED FILE
#pragma once

#include <string_view>

#include "barretenberg/relations/relation_parameters.hpp"
#include "barretenberg/relations/relation_types.hpp"

namespace bb::avm2 {

template <typename FF_> class memoryImpl {
public:
using FF = FF_;

static constexpr std::array<size_t, 2> SUBRELATION_PARTIAL_LENGTHS = { 3, 3 };

template <typename AllEntities> inline static bool skip(const AllEntities& in)
{
const auto& new_term = in;
return (new_term.memory_sel).is_zero();
}

template <typename ContainerOverSubrelations, typename AllEntities>
void static accumulate(ContainerOverSubrelations& evals,
const AllEntities& new_term,
[[maybe_unused]] const RelationParameters<FF>&,
[[maybe_unused]] const FF& scaling_factor)
{

{
using Accumulator = typename std::tuple_element_t<0, ContainerOverSubrelations>;
auto tmp = new_term.memory_sel * (new_term.memory_sel - FF(1));
tmp *= scaling_factor;
std::get<0>(evals) += typename Accumulator::View(tmp);
}
{
using Accumulator = typename std::tuple_element_t<1, ContainerOverSubrelations>;
auto tmp = new_term.memory_rw * (FF(1) - new_term.memory_rw);
tmp *= scaling_factor;
std::get<1>(evals) += typename Accumulator::View(tmp);
}
}
};

template <typename FF> class memory : public Relation<memoryImpl<FF>> {
public:
static constexpr const std::string_view NAME = "memory";

static std::string get_subrelation_label(size_t index)
{
switch (index) {}
return std::to_string(index);
}
};

} // namespace bb::avm2
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ std::unique_ptr<ContextInterface> ExecutionComponentsProvider::make_nested_conte
msg_sender,
is_static,
std::make_unique<BytecodeManager>(address, tx_bytecode_manager),
std::make_unique<Memory>(context_id, memory_events),
std::make_unique<Memory>(context_id, range_check, memory_events),
parent_context,
cd_offset_address,
cd_size_address);
Expand All @@ -37,7 +37,7 @@ std::unique_ptr<ContextInterface> ExecutionComponentsProvider::make_enqueued_con
msg_sender,
is_static,
std::make_unique<BytecodeManager>(address, tx_bytecode_manager),
std::make_unique<Memory>(context_id, memory_events),
std::make_unique<Memory>(context_id, range_check, memory_events),
calldata);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include "barretenberg/vm2/simulation/events/event_emitter.hpp"
#include "barretenberg/vm2/simulation/events/memory_event.hpp"
#include "barretenberg/vm2/simulation/memory.hpp"
#include "barretenberg/vm2/simulation/range_check.hpp"

namespace bb::avm2::simulation {

Expand All @@ -39,9 +40,11 @@ class ExecutionComponentsProviderInterface {
class ExecutionComponentsProvider : public ExecutionComponentsProviderInterface {
public:
ExecutionComponentsProvider(TxBytecodeManagerInterface& tx_bytecode_manager,
RangeCheckInterface& range_check,
EventEmitterInterface<MemoryEvent>& memory_events,
const InstructionInfoDBInterface& instruction_info_db)
: tx_bytecode_manager(tx_bytecode_manager)
, range_check(range_check)
, memory_events(memory_events)
, instruction_info_db(instruction_info_db)
{}
Expand All @@ -61,6 +64,7 @@ class ExecutionComponentsProvider : public ExecutionComponentsProviderInterface
uint32_t next_context_id = 0;

TxBytecodeManagerInterface& tx_bytecode_manager;
RangeCheckInterface& range_check;
EventEmitterInterface<MemoryEvent>& memory_events;
const InstructionInfoDBInterface& instruction_info_db;

Expand Down
19 changes: 18 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm2/simulation/memory.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
#include <memory>

#include "barretenberg/common/log.hpp"
#include "barretenberg/numeric/uint128/uint128.hpp"
#include "barretenberg/vm2/common/memory_types.hpp"

namespace bb::avm2::simulation {
Expand All @@ -21,14 +22,17 @@ bool MemoryInterface::is_valid_address(const MemoryValue& address)

void Memory::set(MemoryAddress index, MemoryValue value)
{
// TODO: validate tag-value makes sense.
// TODO: validate address?
// TODO: reconsider tag validation.
validate_tag(value);
memory[index] = value;
debug("Memory write: ", index, " <- ", value.to_string());
events.emit({ .mode = MemoryMode::WRITE, .addr = index, .value = value, .space_id = space_id });
}

const MemoryValue& Memory::get(MemoryAddress index) const
{
// TODO: validate address?
static const auto default_value = MemoryValue::from<FF>(0);

auto it = memory.find(index);
Expand All @@ -39,4 +43,17 @@ const MemoryValue& Memory::get(MemoryAddress index) const
return vt;
}

// Sadly this is circuit leaking. In simulation we know the tag-value is consistent.
// But the circuit does need to force a range check.
void Memory::validate_tag(const MemoryValue& value) const
{
if (value.get_tag() == MemoryTag::FF) {
return;
}

uint128_t value_as_uint128 = static_cast<uint128_t>(value.as_ff());
auto tag_bits = get_tag_bits(value.get_tag());
range_check.assert_range(value_as_uint128, tag_bits);
}

} // namespace bb::avm2::simulation
30 changes: 29 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm2/simulation/memory.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#include "barretenberg/vm2/common/memory_types.hpp"
#include "barretenberg/vm2/simulation/events/event_emitter.hpp"
#include "barretenberg/vm2/simulation/events/memory_event.hpp"
#include "barretenberg/vm2/simulation/range_check.hpp"

namespace bb::avm2::simulation {

Expand All @@ -26,8 +27,9 @@ class MemoryInterface {

class Memory : public MemoryInterface {
public:
Memory(uint32_t space_id, EventEmitterInterface<MemoryEvent>& event_emitter)
Memory(uint32_t space_id, RangeCheckInterface& range_check, EventEmitterInterface<MemoryEvent>& event_emitter)
: space_id(space_id)
, range_check(range_check)
, events(event_emitter)
{}

Expand All @@ -39,7 +41,33 @@ class Memory : public MemoryInterface {
private:
uint32_t space_id;
unordered_flat_map<size_t, MemoryValue> memory;

RangeCheckInterface& range_check;
// TODO: consider a deduplicating event emitter (within the same clk).
EventEmitterInterface<MemoryEvent>& events;

void validate_tag(const MemoryValue& value) const;
};

// Just a map that doesn't emit events or do anything else.
class MemoryStore : public MemoryInterface {
public:
MemoryStore(uint32_t space_id = 0)
: space_id(space_id)
{}
Comment on lines +52 to +57

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.

For actual memory that generates events (so i guess Memory defined above this), if there is a memory instance per-space-id, does that mean that each space-id's events get generated separately? And I guess that's not problematic for tracegen?

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.

Ah I guess all memory instances (space-ids) will get an instance of the same memory event emitter? So ultimately the tracegen component will receive all of the memory events for all space-ids. And then it'll be responsible for doing any sorting, etc

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yes! you got it :)


const MemoryValue& get(MemoryAddress index) const override
{
static const auto default_value = MemoryValue::from<FF>(0);
auto it = memory.find(index);
return it != memory.end() ? it->second : default_value;
}
void set(MemoryAddress index, MemoryValue value) override { memory[index] = value; }
uint32_t get_space_id() const override { return space_id; }

private:
uint32_t space_id;
unordered_flat_map<size_t, MemoryValue> memory;
};

} // namespace bb::avm2::simulation
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
#include <gmock/gmock.h>
#include <gtest/gtest.h>

#include "barretenberg/crypto/merkle_tree/memory_store.hpp"
#include "barretenberg/vm2/common/memory_types.hpp"
#include "barretenberg/vm2/simulation/events/event_emitter.hpp"
#include "barretenberg/vm2/simulation/events/memory_event.hpp"
Expand All @@ -18,8 +19,7 @@ using ::testing::StrictMock;

TEST(Sha256CompressionSimulationTest, Sha256Compression)
{
NoopEventEmitter<MemoryEvent> emitter;
Memory mem(/*space_id=*/0, emitter);
MemoryStore mem;
StrictMock<MockContext> context;
EXPECT_CALL(context, get_memory()).WillRepeatedly(ReturnRef(mem));

Expand Down
3 changes: 2 additions & 1 deletion barretenberg/cpp/src/barretenberg/vm2/simulation_helper.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,8 @@ template <typename S> EventsContainer AvmSimulationHelper::simulate_with_setting
bytecode_retrieval_emitter,
bytecode_decomposition_emitter,
instruction_fetching_emitter);
ExecutionComponentsProvider execution_components(bytecode_manager, memory_emitter, instruction_info_db);
ExecutionComponentsProvider execution_components(
bytecode_manager, range_check, memory_emitter, instruction_info_db);

Alu alu(alu_emitter);
Execution execution(alu, execution_components, instruction_info_db, execution_emitter, context_stack_emitter);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
#include "barretenberg/common/utils.hpp"
#include "barretenberg/vm2/common/field.hpp"
#include "barretenberg/vm2/common/map.hpp"
#include "barretenberg/vm2/common/stringify.hpp"
#include "barretenberg/vm2/generated/columns.hpp"
#include "barretenberg/vm2/tracegen/lib/interaction_builder.hpp"
#include "barretenberg/vm2/tracegen/trace_container.hpp"
Expand Down Expand Up @@ -74,6 +75,14 @@ class LookupIntoDynamicTableGeneric : public BaseLookupTraceBuilder<LookupSettin
if (it != row_idx.end()) {
return it->second;
}
vinfo(
"Failed computing counts for ",
std::string(LookupSettings::NAME),
" with src tuple: {",
[&tup]<size_t... Is>(std::index_sequence<Is...>) {
return ((field_to_string(tup[Is]) + ", ") + ...);
}(std::make_index_sequence<LookupSettings::LOOKUP_TUPLE_SIZE>()),
"}");
throw std::runtime_error("Failed computing counts for " + std::string(LookupSettings::NAME) +
". Could not find tuple in destination.");
}
Expand Down
Loading