Skip to content

Commit e8bfa98

Browse files
sirasistantsirasistant
authored
feat(avm)!: Tree opcodes fail in static context (#16158)
Small PR to make emit notehash, emit nullifier and sstore fail on static contexts
1 parent 7df9fff commit e8bfa98

File tree

12 files changed

+172
-95
lines changed

12 files changed

+172
-95
lines changed

‎barretenberg/cpp/pil/vm2/opcodes/emit_notehash.pil‎

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,20 @@ namespace execution; // this is a virtual gadget that shares rows with the execu
1212
pol REMAINING_NOTE_HASH_WRITES = constants.MAX_NOTE_HASHES_PER_TX -
1313
prev_num_note_hashes_emitted;
1414

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

1718
pol commit remaining_note_hashes_inv;
18-
// Opcode errors if REMAINING_NOTE_HASH_WRITES is 0
19-
#[EMIT_NOTEHASH_MAX_NOTE_HASH_WRITES_REACHED]
20-
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;
19+
// We reached the max note hashes if REMAINING_NOTE_HASH_WRITES is 0
20+
#[MAX_NOTE_HASHES_REACHED]
21+
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;
22+
23+
// Opcode errors if we've reached the max note hashes or if we're in a static context
24+
#[OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC]
25+
sel_execute_emit_notehash * ((1 - sel_reached_max_note_hashes) * (1 - is_static) - (1 - sel_opcode_error)) = 0;
2126

2227
// Commited since it's used in the lookup
2328
pol commit sel_write_note_hash;
24-
#[EMIT_NOTEHASH_LIMIT_REACHED]
2529
sel_execute_emit_notehash * ((1 - sel_opcode_error) - sel_write_note_hash) = 0;
2630

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

‎barretenberg/cpp/pil/vm2/opcodes/emit_nullifier.pil‎

Lines changed: 13 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -30,25 +30,27 @@ namespace execution; // this is a virtual gadget that shares rows with the execu
3030

3131
// =========== VALIDATION ===========
3232

33-
// TODO(dbanks12): error if in static context
34-
3533
pol REMAINING_NULLIFIER_WRITES = constants.MAX_NULLIFIERS_PER_TX -
3634
prev_num_nullifiers_emitted;
3735

38-
pol commit sel_write_nullifier;
39-
// LIMIT_ERROR implies that we do NOT write the nullifier via the lookup later
40-
pol LIMIT_ERROR = 1 - sel_write_nullifier;
36+
pol commit sel_reached_max_nullifiers;
37+
sel_reached_max_nullifiers * (1 - sel_reached_max_nullifiers) = 0;
4138

4239
pol commit remaining_nullifiers_inv;
4340
// Limit error if REMAINING_NULLIFIER_WRITES is 0
44-
#[EMIT_NULLIFIER_MAX_NULLIFIER_WRITES_REACHED]
45-
sel_execute_emit_nullifier * (REMAINING_NULLIFIER_WRITES * (LIMIT_ERROR * (1 - remaining_nullifiers_inv) + remaining_nullifiers_inv) - 1 + LIMIT_ERROR) = 0;
41+
#[MAX_NULLIFIER_WRITES_REACHED]
42+
sel_execute_emit_nullifier * (REMAINING_NULLIFIER_WRITES * (sel_reached_max_nullifiers * (1 - remaining_nullifiers_inv) + remaining_nullifiers_inv) - 1 + sel_reached_max_nullifiers) = 0;
43+
44+
pol commit sel_write_nullifier;
45+
// Validation errors if we've reached the max nullifiers or if we're in a static context
46+
#[VALIDATION_ERROR_DISABLE_WRITE]
47+
sel_execute_emit_nullifier * ((1 - sel_reached_max_nullifiers) * (1 - is_static) - sel_write_nullifier) = 0;
4648

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

5355
// =========== OPCODE EXECUTION ===========
5456

‎barretenberg/cpp/pil/vm2/opcodes/sstore.pil‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,6 @@ namespace execution; // this is a virtual gadget that shares rows with the execu
2323

2424
// =========== VALIDATION ===========
2525

26-
// TODO(dbanks12): error if in static context
27-
2826
pol commit max_data_writes_reached;
2927
max_data_writes_reached * (1 - max_data_writes_reached) = 0;
3028

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

4646
// Commited since it's used in the lookup
4747
// Note: we could perform the work unconditionally here, since the roots will be reverted if sel_opcode_error is one.

‎barretenberg/cpp/src/barretenberg/vm2/constraining/relations/emit_notehash.test.cpp‎

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -66,9 +66,10 @@ TEST(EmitNoteHashConstrainingTest, LimitReached)
6666
{ C::execution_sel_execute_emit_notehash, 1 },
6767
{ C::execution_register_0_, /*note_hash=*/42 },
6868
{ C::execution_mem_tag_reg_0_, static_cast<uint8_t>(MemoryTag::FF) },
69+
{ C::execution_sel_reached_max_note_hashes, 1 },
6970
{ C::execution_remaining_note_hashes_inv, 0 },
70-
{ C::execution_sel_write_note_hash, 0 },
7171
{ C::execution_sel_opcode_error, 1 },
72+
{ C::execution_sel_write_note_hash, 0 },
7273
{ C::execution_subtrace_operation_id, AVM_EXEC_OP_ID_EMIT_NOTEHASH },
7374
{ C::execution_prev_note_hash_tree_size, 1 },
7475
{ C::execution_prev_note_hash_tree_root, 27 },
@@ -79,10 +80,17 @@ TEST(EmitNoteHashConstrainingTest, LimitReached)
7980
} });
8081
check_relation<emit_notehash>(trace);
8182

83+
// Negative test: sel_reached_max_note_hashes must be 1
84+
trace.set(C::execution_sel_reached_max_note_hashes, 0, 0);
85+
EXPECT_THROW_WITH_MESSAGE(check_relation<emit_notehash>(trace, emit_notehash::SR_MAX_NOTE_HASHES_REACHED),
86+
"MAX_NOTE_HASHES_REACHED");
87+
trace.set(C::execution_sel_reached_max_note_hashes, 0, 1);
88+
8289
// Negative test: sel_opcode_error must be on
8390
trace.set(C::execution_sel_opcode_error, 0, 0);
84-
EXPECT_THROW_WITH_MESSAGE(check_relation<emit_notehash>(trace, emit_notehash::SR_EMIT_NOTEHASH_LIMIT_REACHED),
85-
"EMIT_NOTEHASH_LIMIT_REACHED");
91+
EXPECT_THROW_WITH_MESSAGE(
92+
check_relation<emit_notehash>(trace, emit_notehash::SR_OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC),
93+
"OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC");
8694
trace.set(C::execution_sel_opcode_error, 0, 1);
8795

8896
// Negative test: note hash tree root must be the same

‎barretenberg/cpp/src/barretenberg/vm2/constraining/relations/emit_nullifier.test.cpp‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,6 +70,7 @@ TEST(EmitNullifierConstrainingTest, LimitReached)
7070
{ C::execution_sel_execute_emit_nullifier, 1 },
7171
{ C::execution_register_0_, /*nullifier=*/42 },
7272
{ C::execution_mem_tag_reg_0_, static_cast<uint8_t>(MemoryTag::FF) },
73+
{ C::execution_sel_reached_max_nullifiers, 1 },
7374
{ C::execution_remaining_nullifiers_inv, 0 },
7475
{ C::execution_sel_write_nullifier, 0 },
7576
{ C::execution_sel_opcode_error, 1 },
@@ -81,10 +82,17 @@ TEST(EmitNullifierConstrainingTest, LimitReached)
8182
} });
8283
check_relation<emit_nullifier>(trace);
8384

85+
// Negative test: sel_reached_max_nullifiers must be 1
86+
trace.set(C::execution_sel_reached_max_nullifiers, 0, 0);
87+
EXPECT_THROW_WITH_MESSAGE(check_relation<emit_nullifier>(trace, emit_nullifier::SR_MAX_NULLIFIER_WRITES_REACHED),
88+
"MAX_NULLIFIER_WRITES_REACHED");
89+
trace.set(C::execution_sel_reached_max_nullifiers, 0, 1);
90+
8491
// Negative test: sel_opcode_error must be on
8592
trace.set(C::execution_sel_opcode_error, 0, 0);
86-
EXPECT_THROW_WITH_MESSAGE(check_relation<emit_nullifier>(trace, emit_nullifier::SR_OPCODE_ERROR_IF_LIMIT_ERROR),
87-
"OPCODE_ERROR_IF_LIMIT_ERROR");
93+
EXPECT_THROW_WITH_MESSAGE(
94+
check_relation<emit_nullifier>(trace, emit_nullifier::SR_OPCODE_ERROR_IF_VALIDATION_ERROR),
95+
"OPCODE_ERROR_IF_VALIDATION_ERROR");
8896
trace.set(C::execution_sel_opcode_error, 0, 1);
8997

9098
// Negative test: nullifier tree root must be the same

‎barretenberg/cpp/src/barretenberg/vm2/constraining/relations/storage_write.test.cpp‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@ TEST(SStoreConstrainingTest, MaxDataWritesReached)
104104
"SSTORE_MAX_DATA_WRITES_REACHED");
105105
}
106106

107-
TEST(SStoreConstrainingTest, ErrorTooManyWrites)
107+
TEST(SStoreConstrainingTest, OpcodeError)
108108
{
109109
TestTraceContainer trace({
110110
{
@@ -113,19 +113,33 @@ TEST(SStoreConstrainingTest, ErrorTooManyWrites)
113113
{ C::execution_max_data_writes_reached, 1 },
114114
{ C::execution_sel_opcode_error, 1 },
115115
},
116+
{
117+
{ C::execution_sel_execute_sstore, 1 },
118+
{ C::execution_dynamic_da_gas_factor, 0 },
119+
{ C::execution_max_data_writes_reached, 0 },
120+
{ C::execution_is_static, 1 },
121+
{ C::execution_sel_opcode_error, 1 },
122+
},
116123
{
117124
{ C::execution_sel_execute_sstore, 1 },
118125
{ C::execution_dynamic_da_gas_factor, 0 },
119126
{ C::execution_max_data_writes_reached, 1 },
120127
{ C::execution_sel_opcode_error, 0 },
121128
},
122129
});
123-
check_relation<sstore>(trace, sstore::SR_SSTORE_ERROR_TOO_MANY_WRITES);
130+
check_relation<sstore>(trace, sstore::SR_OPCODE_ERROR_IF_OVERFLOW_OR_STATIC);
124131

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

127-
EXPECT_THROW_WITH_MESSAGE(check_relation<sstore>(trace, sstore::SR_SSTORE_ERROR_TOO_MANY_WRITES),
128-
"SSTORE_ERROR_TOO_MANY_WRITES");
134+
EXPECT_THROW_WITH_MESSAGE(check_relation<sstore>(trace, sstore::SR_OPCODE_ERROR_IF_OVERFLOW_OR_STATIC),
135+
"OPCODE_ERROR_IF_OVERFLOW_OR_STATIC");
136+
137+
trace.set(C::execution_dynamic_da_gas_factor, 0, 1);
138+
139+
trace.set(C::execution_is_static, 1, 0);
140+
141+
EXPECT_THROW_WITH_MESSAGE(check_relation<sstore>(trace, sstore::SR_OPCODE_ERROR_IF_OVERFLOW_OR_STATIC),
142+
"OPCODE_ERROR_IF_OVERFLOW_OR_STATIC");
129143
}
130144

131145
TEST(SStoreConstrainingTest, TreeStateNotChangedOnError)

‎barretenberg/cpp/src/barretenberg/vm2/generated/columns.hpp‎

Lines changed: 3 additions & 3 deletions
Large diffs are not rendered by default.

‎barretenberg/cpp/src/barretenberg/vm2/generated/flavor_variables.hpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -121,10 +121,10 @@ namespace bb::avm2 {
121121

122122
struct AvmFlavorVariables {
123123
static constexpr size_t NUM_PRECOMPUTED_ENTITIES = 126;
124-
static constexpr size_t NUM_WITNESS_ENTITIES = 2656;
124+
static constexpr size_t NUM_WITNESS_ENTITIES = 2658;
125125
static constexpr size_t NUM_SHIFTED_ENTITIES = 287;
126126
static constexpr size_t NUM_WIRES = NUM_WITNESS_ENTITIES + NUM_PRECOMPUTED_ENTITIES;
127-
static constexpr size_t NUM_ALL_ENTITIES = 3069;
127+
static constexpr size_t NUM_ALL_ENTITIES = 3071;
128128

129129
// Need to be templated for recursive verifier
130130
template <typename FF_>

‎barretenberg/cpp/src/barretenberg/vm2/generated/relations/emit_notehash.hpp‎

Lines changed: 40 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ template <typename FF_> class emit_notehashImpl {
1414
public:
1515
using FF = FF_;
1616

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

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

40-
{ // EMIT_NOTEHASH_MAX_NOTE_HASH_WRITES_REACHED
40+
{
4141
using Accumulator = typename std::tuple_element_t<0, ContainerOverSubrelations>;
42+
auto tmp = in.get(C::execution_sel_reached_max_note_hashes) *
43+
(FF(1) - in.get(C::execution_sel_reached_max_note_hashes));
44+
tmp *= scaling_factor;
45+
std::get<0>(evals) += typename Accumulator::View(tmp);
46+
}
47+
{ // MAX_NOTE_HASHES_REACHED
48+
using Accumulator = typename std::tuple_element_t<1, ContainerOverSubrelations>;
4249
auto tmp =
4350
in.get(C::execution_sel_execute_emit_notehash) *
44-
((execution_REMAINING_NOTE_HASH_WRITES * (in.get(C::execution_sel_opcode_error) *
51+
((execution_REMAINING_NOTE_HASH_WRITES * (in.get(C::execution_sel_reached_max_note_hashes) *
4552
(FF(1) - in.get(C::execution_remaining_note_hashes_inv)) +
4653
in.get(C::execution_remaining_note_hashes_inv)) -
4754
FF(1)) +
48-
in.get(C::execution_sel_opcode_error));
55+
in.get(C::execution_sel_reached_max_note_hashes));
4956
tmp *= scaling_factor;
50-
std::get<0>(evals) += typename Accumulator::View(tmp);
57+
std::get<1>(evals) += typename Accumulator::View(tmp);
5158
}
52-
{ // EMIT_NOTEHASH_LIMIT_REACHED
53-
using Accumulator = typename std::tuple_element_t<1, ContainerOverSubrelations>;
59+
{ // OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC
60+
using Accumulator = typename std::tuple_element_t<2, ContainerOverSubrelations>;
61+
auto tmp =
62+
in.get(C::execution_sel_execute_emit_notehash) *
63+
((FF(1) - in.get(C::execution_sel_reached_max_note_hashes)) * (FF(1) - in.get(C::execution_is_static)) -
64+
(FF(1) - in.get(C::execution_sel_opcode_error)));
65+
tmp *= scaling_factor;
66+
std::get<2>(evals) += typename Accumulator::View(tmp);
67+
}
68+
{
69+
using Accumulator = typename std::tuple_element_t<3, ContainerOverSubrelations>;
5470
auto tmp = in.get(C::execution_sel_execute_emit_notehash) *
5571
((FF(1) - in.get(C::execution_sel_opcode_error)) - in.get(C::execution_sel_write_note_hash));
5672
tmp *= scaling_factor;
57-
std::get<1>(evals) += typename Accumulator::View(tmp);
73+
std::get<3>(evals) += typename Accumulator::View(tmp);
5874
}
5975
{ // EMIT_NOTEHASH_TREE_ROOT_NOT_CHANGED
60-
using Accumulator = typename std::tuple_element_t<2, ContainerOverSubrelations>;
76+
using Accumulator = typename std::tuple_element_t<4, ContainerOverSubrelations>;
6177
auto tmp = in.get(C::execution_sel_execute_emit_notehash) * in.get(C::execution_sel_opcode_error) *
6278
(in.get(C::execution_prev_note_hash_tree_root) - in.get(C::execution_note_hash_tree_root));
6379
tmp *= scaling_factor;
64-
std::get<2>(evals) += typename Accumulator::View(tmp);
80+
std::get<4>(evals) += typename Accumulator::View(tmp);
6581
}
6682
{ // EMIT_NOTEHASH_TREE_SIZE_INCREASE
67-
using Accumulator = typename std::tuple_element_t<3, ContainerOverSubrelations>;
83+
using Accumulator = typename std::tuple_element_t<5, ContainerOverSubrelations>;
6884
auto tmp = in.get(C::execution_sel_execute_emit_notehash) *
6985
((in.get(C::execution_prev_note_hash_tree_size) + in.get(C::execution_sel_write_note_hash)) -
7086
in.get(C::execution_note_hash_tree_size));
7187
tmp *= scaling_factor;
72-
std::get<3>(evals) += typename Accumulator::View(tmp);
88+
std::get<5>(evals) += typename Accumulator::View(tmp);
7389
}
7490
{ // EMIT_NOTEHASH_NUM_NOTE_HASHES_EMITTED_INCREASE
75-
using Accumulator = typename std::tuple_element_t<4, ContainerOverSubrelations>;
91+
using Accumulator = typename std::tuple_element_t<6, ContainerOverSubrelations>;
7692
auto tmp = in.get(C::execution_sel_execute_emit_notehash) *
7793
((in.get(C::execution_prev_num_note_hashes_emitted) + in.get(C::execution_sel_write_note_hash)) -
7894
in.get(C::execution_num_note_hashes_emitted));
7995
tmp *= scaling_factor;
80-
std::get<4>(evals) += typename Accumulator::View(tmp);
96+
std::get<6>(evals) += typename Accumulator::View(tmp);
8197
}
8298
}
8399
};
@@ -89,26 +105,26 @@ template <typename FF> class emit_notehash : public Relation<emit_notehashImpl<F
89105
static std::string get_subrelation_label(size_t index)
90106
{
91107
switch (index) {
92-
case 0:
93-
return "EMIT_NOTEHASH_MAX_NOTE_HASH_WRITES_REACHED";
94108
case 1:
95-
return "EMIT_NOTEHASH_LIMIT_REACHED";
109+
return "MAX_NOTE_HASHES_REACHED";
96110
case 2:
111+
return "OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC";
112+
case 4:
97113
return "EMIT_NOTEHASH_TREE_ROOT_NOT_CHANGED";
98-
case 3:
114+
case 5:
99115
return "EMIT_NOTEHASH_TREE_SIZE_INCREASE";
100-
case 4:
116+
case 6:
101117
return "EMIT_NOTEHASH_NUM_NOTE_HASHES_EMITTED_INCREASE";
102118
}
103119
return std::to_string(index);
104120
}
105121

106122
// Subrelation indices constants, to be used in tests.
107-
static constexpr size_t SR_EMIT_NOTEHASH_MAX_NOTE_HASH_WRITES_REACHED = 0;
108-
static constexpr size_t SR_EMIT_NOTEHASH_LIMIT_REACHED = 1;
109-
static constexpr size_t SR_EMIT_NOTEHASH_TREE_ROOT_NOT_CHANGED = 2;
110-
static constexpr size_t SR_EMIT_NOTEHASH_TREE_SIZE_INCREASE = 3;
111-
static constexpr size_t SR_EMIT_NOTEHASH_NUM_NOTE_HASHES_EMITTED_INCREASE = 4;
123+
static constexpr size_t SR_MAX_NOTE_HASHES_REACHED = 1;
124+
static constexpr size_t SR_OPCODE_ERROR_IF_MAX_NOTE_HASHES_REACHED_OR_STATIC = 2;
125+
static constexpr size_t SR_EMIT_NOTEHASH_TREE_ROOT_NOT_CHANGED = 4;
126+
static constexpr size_t SR_EMIT_NOTEHASH_TREE_SIZE_INCREASE = 5;
127+
static constexpr size_t SR_EMIT_NOTEHASH_NUM_NOTE_HASHES_EMITTED_INCREASE = 6;
112128
};
113129

114130
} // namespace bb::avm2

0 commit comments

Comments
 (0)