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
14 changes: 9 additions & 5 deletions barretenberg/cpp/pil/vm2/opcodes/emit_notehash.pil
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,20 @@ namespace execution; // this is a virtual gadget that shares rows with the execu
pol REMAINING_NOTE_HASH_WRITES = constants.MAX_NOTE_HASHES_PER_TX -
prev_num_note_hashes_emitted;

// TODO(dbanks12): error if in static context
pol commit sel_reached_max_note_hashes;
sel_reached_max_note_hashes * (1 - sel_reached_max_note_hashes) = 0;

pol commit remaining_note_hashes_inv;
// Opcode errors if REMAINING_NOTE_HASH_WRITES is 0
#[EMIT_NOTEHASH_MAX_NOTE_HASH_WRITES_REACHED]
sel_execute_emit_notehash * (REMAINING_NOTE_HASH_WRITES * (sel_opcode_error * (1 - remaining_note_hashes_inv) + remaining_note_hashes_inv) - 1 + sel_opcode_error) = 0;
// We reached the max note hashes if REMAINING_NOTE_HASH_WRITES is 0
#[MAX_NOTE_HASHES_REACHED]
sel_execute_emit_notehash * (REMAINING_NOTE_HASH_WRITES * (sel_reached_max_note_hashes * (1 - remaining_note_hashes_inv) + remaining_note_hashes_inv) - 1 + sel_reached_max_note_hashes) = 0;

// Opcode errors if we've reached the max note hashes or if we're in a static context
#[OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC]
sel_execute_emit_notehash * ((1 - sel_reached_max_note_hashes) * (1 - is_static) - (1 - sel_opcode_error)) = 0;

// Commited since it's used in the lookup
pol commit sel_write_note_hash;
#[EMIT_NOTEHASH_LIMIT_REACHED]
sel_execute_emit_notehash * ((1 - sel_opcode_error) - sel_write_note_hash) = 0;
Comment on lines 28 to 29

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.

Even if we don't check the constraint explicitly in tests, I like having more constraints named as it makes errors more debuggable. I know this old name was no longer accurate, but just thought I'd mention my preference!

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.

Yeah but this is just the negation of a variable for a lookup, i hope we didn't mess this one up (famous last words)

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 true.... It isn't really a meaningful constraint that needs to be named. Fair!


// =========== OPCODE EXECUTION ===========
Expand Down
24 changes: 13 additions & 11 deletions barretenberg/cpp/pil/vm2/opcodes/emit_nullifier.pil
Original file line number Diff line number Diff line change
Expand Up @@ -30,25 +30,27 @@ namespace execution; // this is a virtual gadget that shares rows with the execu

// =========== VALIDATION ===========

// TODO(dbanks12): error if in static context

pol REMAINING_NULLIFIER_WRITES = constants.MAX_NULLIFIERS_PER_TX -
prev_num_nullifiers_emitted;

pol commit sel_write_nullifier;
// LIMIT_ERROR implies that we do NOT write the nullifier via the lookup later
pol LIMIT_ERROR = 1 - sel_write_nullifier;
pol commit sel_reached_max_nullifiers;
sel_reached_max_nullifiers * (1 - sel_reached_max_nullifiers) = 0;

pol commit remaining_nullifiers_inv;
// Limit error if REMAINING_NULLIFIER_WRITES is 0
#[EMIT_NULLIFIER_MAX_NULLIFIER_WRITES_REACHED]
sel_execute_emit_nullifier * (REMAINING_NULLIFIER_WRITES * (LIMIT_ERROR * (1 - remaining_nullifiers_inv) + remaining_nullifiers_inv) - 1 + LIMIT_ERROR) = 0;
#[MAX_NULLIFIER_WRITES_REACHED]
sel_execute_emit_nullifier * (REMAINING_NULLIFIER_WRITES * (sel_reached_max_nullifiers * (1 - remaining_nullifiers_inv) + remaining_nullifiers_inv) - 1 + sel_reached_max_nullifiers) = 0;

pol commit sel_write_nullifier;
// Validation errors if we've reached the max nullifiers or if we're in a static context
#[VALIDATION_ERROR_DISABLE_WRITE]
sel_execute_emit_nullifier * ((1 - sel_reached_max_nullifiers) * (1 - is_static) - sel_write_nullifier) = 0;

// A limit error must cause an "opcode error".
// if LIMIT_ERROR == 1, sel_opcode_error must be 1
// A validation error must cause an "opcode error".
// if sel_write_nullifier == 0, sel_opcode_error must be 1
// but otherwise, we don't force a value for sel_opcode_error and instead let the lookup below set it.
#[OPCODE_ERROR_IF_LIMIT_ERROR]
sel_execute_emit_nullifier * LIMIT_ERROR * (1 - sel_opcode_error) = 0;
#[OPCODE_ERROR_IF_VALIDATION_ERROR]
sel_execute_emit_nullifier * (1 - sel_write_nullifier) * (1 - sel_opcode_error) = 0;

// =========== OPCODE EXECUTION ===========

Expand Down
8 changes: 4 additions & 4 deletions barretenberg/cpp/pil/vm2/opcodes/sstore.pil
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ namespace execution; // this is a virtual gadget that shares rows with the execu

// =========== VALIDATION ===========

// TODO(dbanks12): error if in static context

pol commit max_data_writes_reached;
max_data_writes_reached * (1 - max_data_writes_reached) = 0;

Expand All @@ -40,8 +38,10 @@ namespace execution; // this is a virtual gadget that shares rows with the execu
// If we are at the maximum number of data writes,
// and the dynamic gas factor is 1 (which means that we haven't written to this slot before),
// the opcode should fail since we can't write to this slot anymore.
#[SSTORE_ERROR_TOO_MANY_WRITES]
sel_execute_sstore * (max_data_writes_reached * dynamic_da_gas_factor - sel_opcode_error) = 0;
// We also should error if we are in a static context.
// Thus, sel_opcode_error = overflow OR static context
#[OPCODE_ERROR_IF_OVERFLOW_OR_STATIC]
sel_execute_sstore * ((1 - max_data_writes_reached * dynamic_da_gas_factor) * (1 - is_static) - (1 - sel_opcode_error)) = 0;

// Commited since it's used in the lookup
// Note: we could perform the work unconditionally here, since the roots will be reverted if sel_opcode_error is one.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,9 +66,10 @@ TEST(EmitNoteHashConstrainingTest, LimitReached)
{ C::execution_sel_execute_emit_notehash, 1 },
{ C::execution_register_0_, /*note_hash=*/42 },
{ C::execution_mem_tag_reg_0_, static_cast<uint8_t>(MemoryTag::FF) },
{ C::execution_sel_reached_max_note_hashes, 1 },
{ C::execution_remaining_note_hashes_inv, 0 },
{ C::execution_sel_write_note_hash, 0 },
{ C::execution_sel_opcode_error, 1 },
{ C::execution_sel_write_note_hash, 0 },
{ C::execution_subtrace_operation_id, AVM_EXEC_OP_ID_EMIT_NOTEHASH },
{ C::execution_prev_note_hash_tree_size, 1 },
{ C::execution_prev_note_hash_tree_root, 27 },
Expand All @@ -79,10 +80,17 @@ TEST(EmitNoteHashConstrainingTest, LimitReached)
} });
check_relation<emit_notehash>(trace);

// Negative test: sel_reached_max_note_hashes must be 1
trace.set(C::execution_sel_reached_max_note_hashes, 0, 0);
EXPECT_THROW_WITH_MESSAGE(check_relation<emit_notehash>(trace, emit_notehash::SR_MAX_NOTE_HASHES_REACHED),
"MAX_NOTE_HASHES_REACHED");
trace.set(C::execution_sel_reached_max_note_hashes, 0, 1);

// Negative test: sel_opcode_error must be on
trace.set(C::execution_sel_opcode_error, 0, 0);
EXPECT_THROW_WITH_MESSAGE(check_relation<emit_notehash>(trace, emit_notehash::SR_EMIT_NOTEHASH_LIMIT_REACHED),
"EMIT_NOTEHASH_LIMIT_REACHED");
EXPECT_THROW_WITH_MESSAGE(
check_relation<emit_notehash>(trace, emit_notehash::SR_OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC),
"OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC");
trace.set(C::execution_sel_opcode_error, 0, 1);

// Negative test: note hash tree root must be the same
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ TEST(EmitNullifierConstrainingTest, LimitReached)
{ C::execution_sel_execute_emit_nullifier, 1 },
{ C::execution_register_0_, /*nullifier=*/42 },
{ C::execution_mem_tag_reg_0_, static_cast<uint8_t>(MemoryTag::FF) },
{ C::execution_sel_reached_max_nullifiers, 1 },
{ C::execution_remaining_nullifiers_inv, 0 },
{ C::execution_sel_write_nullifier, 0 },
{ C::execution_sel_opcode_error, 1 },
Expand All @@ -81,10 +82,17 @@ TEST(EmitNullifierConstrainingTest, LimitReached)
} });
check_relation<emit_nullifier>(trace);

// Negative test: sel_reached_max_nullifiers must be 1
trace.set(C::execution_sel_reached_max_nullifiers, 0, 0);
EXPECT_THROW_WITH_MESSAGE(check_relation<emit_nullifier>(trace, emit_nullifier::SR_MAX_NULLIFIER_WRITES_REACHED),
"MAX_NULLIFIER_WRITES_REACHED");
trace.set(C::execution_sel_reached_max_nullifiers, 0, 1);

// Negative test: sel_opcode_error must be on
trace.set(C::execution_sel_opcode_error, 0, 0);
EXPECT_THROW_WITH_MESSAGE(check_relation<emit_nullifier>(trace, emit_nullifier::SR_OPCODE_ERROR_IF_LIMIT_ERROR),
"OPCODE_ERROR_IF_LIMIT_ERROR");
EXPECT_THROW_WITH_MESSAGE(
check_relation<emit_nullifier>(trace, emit_nullifier::SR_OPCODE_ERROR_IF_VALIDATION_ERROR),
"OPCODE_ERROR_IF_VALIDATION_ERROR");
trace.set(C::execution_sel_opcode_error, 0, 1);

// Negative test: nullifier tree root must be the same
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ TEST(SStoreConstrainingTest, MaxDataWritesReached)
"SSTORE_MAX_DATA_WRITES_REACHED");
}

TEST(SStoreConstrainingTest, ErrorTooManyWrites)
TEST(SStoreConstrainingTest, OpcodeError)
{
TestTraceContainer trace({
{
Expand All @@ -113,19 +113,33 @@ TEST(SStoreConstrainingTest, ErrorTooManyWrites)
{ C::execution_max_data_writes_reached, 1 },
{ C::execution_sel_opcode_error, 1 },
},
{
{ C::execution_sel_execute_sstore, 1 },
{ C::execution_dynamic_da_gas_factor, 0 },
{ C::execution_max_data_writes_reached, 0 },
{ C::execution_is_static, 1 },
{ C::execution_sel_opcode_error, 1 },
},
{
{ C::execution_sel_execute_sstore, 1 },
{ C::execution_dynamic_da_gas_factor, 0 },
{ C::execution_max_data_writes_reached, 1 },
{ C::execution_sel_opcode_error, 0 },
},
});
check_relation<sstore>(trace, sstore::SR_SSTORE_ERROR_TOO_MANY_WRITES);
check_relation<sstore>(trace, sstore::SR_OPCODE_ERROR_IF_OVERFLOW_OR_STATIC);

trace.set(C::execution_dynamic_da_gas_factor, 0, 0);

EXPECT_THROW_WITH_MESSAGE(check_relation<sstore>(trace, sstore::SR_SSTORE_ERROR_TOO_MANY_WRITES),
"SSTORE_ERROR_TOO_MANY_WRITES");
EXPECT_THROW_WITH_MESSAGE(check_relation<sstore>(trace, sstore::SR_OPCODE_ERROR_IF_OVERFLOW_OR_STATIC),
"OPCODE_ERROR_IF_OVERFLOW_OR_STATIC");

trace.set(C::execution_dynamic_da_gas_factor, 0, 1);

trace.set(C::execution_is_static, 1, 0);

EXPECT_THROW_WITH_MESSAGE(check_relation<sstore>(trace, sstore::SR_OPCODE_ERROR_IF_OVERFLOW_OR_STATIC),
"OPCODE_ERROR_IF_OVERFLOW_OR_STATIC");
}

TEST(SStoreConstrainingTest, TreeStateNotChangedOnError)
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 @@ -121,10 +121,10 @@ namespace bb::avm2 {

struct AvmFlavorVariables {
static constexpr size_t NUM_PRECOMPUTED_ENTITIES = 126;
static constexpr size_t NUM_WITNESS_ENTITIES = 2656;
static constexpr size_t NUM_WITNESS_ENTITIES = 2658;
static constexpr size_t NUM_SHIFTED_ENTITIES = 287;
static constexpr size_t NUM_WIRES = NUM_WITNESS_ENTITIES + NUM_PRECOMPUTED_ENTITIES;
static constexpr size_t NUM_ALL_ENTITIES = 3069;
static constexpr size_t NUM_ALL_ENTITIES = 3071;

// Need to be templated for recursive verifier
template <typename FF_>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ template <typename FF_> class emit_notehashImpl {
public:
using FF = FF_;

static constexpr std::array<size_t, 5> SUBRELATION_PARTIAL_LENGTHS = { 5, 3, 4, 3, 3 };
static constexpr std::array<size_t, 7> SUBRELATION_PARTIAL_LENGTHS = { 3, 5, 4, 3, 4, 3, 3 };

template <typename AllEntities> inline static bool skip(const AllEntities& in)
{
Expand All @@ -37,47 +37,63 @@ template <typename FF_> class emit_notehashImpl {
const auto execution_REMAINING_NOTE_HASH_WRITES =
(constants_MAX_NOTE_HASHES_PER_TX - in.get(C::execution_prev_num_note_hashes_emitted));

{ // EMIT_NOTEHASH_MAX_NOTE_HASH_WRITES_REACHED
{
using Accumulator = typename std::tuple_element_t<0, ContainerOverSubrelations>;
auto tmp = in.get(C::execution_sel_reached_max_note_hashes) *
(FF(1) - in.get(C::execution_sel_reached_max_note_hashes));
tmp *= scaling_factor;
std::get<0>(evals) += typename Accumulator::View(tmp);
}
{ // MAX_NOTE_HASHES_REACHED
using Accumulator = typename std::tuple_element_t<1, ContainerOverSubrelations>;
auto tmp =
in.get(C::execution_sel_execute_emit_notehash) *
((execution_REMAINING_NOTE_HASH_WRITES * (in.get(C::execution_sel_opcode_error) *
((execution_REMAINING_NOTE_HASH_WRITES * (in.get(C::execution_sel_reached_max_note_hashes) *
(FF(1) - in.get(C::execution_remaining_note_hashes_inv)) +
in.get(C::execution_remaining_note_hashes_inv)) -
FF(1)) +
in.get(C::execution_sel_opcode_error));
in.get(C::execution_sel_reached_max_note_hashes));
tmp *= scaling_factor;
std::get<0>(evals) += typename Accumulator::View(tmp);
std::get<1>(evals) += typename Accumulator::View(tmp);
}
{ // EMIT_NOTEHASH_LIMIT_REACHED
using Accumulator = typename std::tuple_element_t<1, ContainerOverSubrelations>;
{ // OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC
using Accumulator = typename std::tuple_element_t<2, ContainerOverSubrelations>;
auto tmp =
in.get(C::execution_sel_execute_emit_notehash) *
((FF(1) - in.get(C::execution_sel_reached_max_note_hashes)) * (FF(1) - in.get(C::execution_is_static)) -
(FF(1) - in.get(C::execution_sel_opcode_error)));
tmp *= scaling_factor;
std::get<2>(evals) += typename Accumulator::View(tmp);
}
{
using Accumulator = typename std::tuple_element_t<3, ContainerOverSubrelations>;
auto tmp = in.get(C::execution_sel_execute_emit_notehash) *
((FF(1) - in.get(C::execution_sel_opcode_error)) - in.get(C::execution_sel_write_note_hash));
tmp *= scaling_factor;
std::get<1>(evals) += typename Accumulator::View(tmp);
std::get<3>(evals) += typename Accumulator::View(tmp);
}
{ // EMIT_NOTEHASH_TREE_ROOT_NOT_CHANGED
using Accumulator = typename std::tuple_element_t<2, ContainerOverSubrelations>;
using Accumulator = typename std::tuple_element_t<4, ContainerOverSubrelations>;
auto tmp = in.get(C::execution_sel_execute_emit_notehash) * in.get(C::execution_sel_opcode_error) *
(in.get(C::execution_prev_note_hash_tree_root) - in.get(C::execution_note_hash_tree_root));
tmp *= scaling_factor;
std::get<2>(evals) += typename Accumulator::View(tmp);
std::get<4>(evals) += typename Accumulator::View(tmp);
}
{ // EMIT_NOTEHASH_TREE_SIZE_INCREASE
using Accumulator = typename std::tuple_element_t<3, ContainerOverSubrelations>;
using Accumulator = typename std::tuple_element_t<5, ContainerOverSubrelations>;
auto tmp = in.get(C::execution_sel_execute_emit_notehash) *
((in.get(C::execution_prev_note_hash_tree_size) + in.get(C::execution_sel_write_note_hash)) -
in.get(C::execution_note_hash_tree_size));
tmp *= scaling_factor;
std::get<3>(evals) += typename Accumulator::View(tmp);
std::get<5>(evals) += typename Accumulator::View(tmp);
}
{ // EMIT_NOTEHASH_NUM_NOTE_HASHES_EMITTED_INCREASE
using Accumulator = typename std::tuple_element_t<4, ContainerOverSubrelations>;
using Accumulator = typename std::tuple_element_t<6, ContainerOverSubrelations>;
auto tmp = in.get(C::execution_sel_execute_emit_notehash) *
((in.get(C::execution_prev_num_note_hashes_emitted) + in.get(C::execution_sel_write_note_hash)) -
in.get(C::execution_num_note_hashes_emitted));
tmp *= scaling_factor;
std::get<4>(evals) += typename Accumulator::View(tmp);
std::get<6>(evals) += typename Accumulator::View(tmp);
}
}
};
Expand All @@ -89,26 +105,26 @@ template <typename FF> class emit_notehash : public Relation<emit_notehashImpl<F
static std::string get_subrelation_label(size_t index)
{
switch (index) {
case 0:
return "EMIT_NOTEHASH_MAX_NOTE_HASH_WRITES_REACHED";
case 1:
return "EMIT_NOTEHASH_LIMIT_REACHED";
return "MAX_NOTE_HASHES_REACHED";
case 2:
return "OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC";
case 4:
return "EMIT_NOTEHASH_TREE_ROOT_NOT_CHANGED";
case 3:
case 5:
return "EMIT_NOTEHASH_TREE_SIZE_INCREASE";
case 4:
case 6:
return "EMIT_NOTEHASH_NUM_NOTE_HASHES_EMITTED_INCREASE";
}
return std::to_string(index);
}

// Subrelation indices constants, to be used in tests.
static constexpr size_t SR_EMIT_NOTEHASH_MAX_NOTE_HASH_WRITES_REACHED = 0;
static constexpr size_t SR_EMIT_NOTEHASH_LIMIT_REACHED = 1;
static constexpr size_t SR_EMIT_NOTEHASH_TREE_ROOT_NOT_CHANGED = 2;
static constexpr size_t SR_EMIT_NOTEHASH_TREE_SIZE_INCREASE = 3;
static constexpr size_t SR_EMIT_NOTEHASH_NUM_NOTE_HASHES_EMITTED_INCREASE = 4;
static constexpr size_t SR_MAX_NOTE_HASHES_REACHED = 1;
static constexpr size_t SR_OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC = 2;
static constexpr size_t SR_EMIT_NOTEHASH_TREE_ROOT_NOT_CHANGED = 4;
static constexpr size_t SR_EMIT_NOTEHASH_TREE_SIZE_INCREASE = 5;
static constexpr size_t SR_EMIT_NOTEHASH_NUM_NOTE_HASHES_EMITTED_INCREASE = 6;
};

} // namespace bb::avm2
Loading
Loading