Skip to content

chore(avm): remove check_interaction from tests #13136

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fcarreiro merged 2 commits into from
Mar 28, 2025
Merged

chore(avm): remove check_interaction from tests #13136

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
7ec3b84
chore(avm): remove check_interaction from tests
fcarreiro Mar 28, 2025
1ed81ae
Simplify instruction fetching constr. unit tests
jeanmon Mar 28, 2025
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
11 changes: 0 additions & 11 deletions barretenberg/cpp/src/barretenberg/vm2/constraining/relations/address_derivation.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,17 +157,6 @@ TEST(AddressDerivationConstrainingTest, WithInteractions)
LookupIntoDynamicTableSequential<lookup_address_ecadd::Settings>().process(trace);

check_relation<address_derivation_relation>(trace);
check_interaction<lookup_salted_initialization_hash_poseidon2_0>(trace);
check_interaction<lookup_salted_initialization_hash_poseidon2_1>(trace);
check_interaction<lookup_partial_address_poseidon2>(trace);
check_interaction<lookup_public_keys_hash_poseidon2_0>(trace);
check_interaction<lookup_public_keys_hash_poseidon2_1>(trace);
check_interaction<lookup_public_keys_hash_poseidon2_2>(trace);
check_interaction<lookup_public_keys_hash_poseidon2_3>(trace);
check_interaction<lookup_public_keys_hash_poseidon2_4>(trace);
check_interaction<lookup_public_preaddress_poseidon2>(trace);
check_interaction<lookup_public_preaddress_scalar_mul>(trace);
check_interaction<lookup_address_ecadd>(trace);
}

} // namespace
Expand Down
3 changes: 0 additions & 3 deletions barretenberg/cpp/src/barretenberg/vm2/constraining/relations/bc_hashing.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,6 @@ TEST(BytecodeHashingConstrainingTest, PoseidonInteractions)
tracegen::LookupIntoDynamicTableSequential<bc_hashing_lookup_relation::Settings>().process(trace);

check_relation<bc_hashing>(trace);
check_interaction<bc_hashing_lookup_relation>(trace);
}

TEST(BytecodeHashingConstrainingTest, BytecodeInteractions)
Expand All @@ -118,8 +117,6 @@ TEST(BytecodeHashingConstrainingTest, BytecodeInteractions)
tracegen::LookupIntoDynamicTableSequential<length_iv_relation::Settings>().process(trace);

check_relation<bc_hashing>(trace);
check_interaction<bc_decomp_lookup_relation>(trace);
check_interaction<length_iv_relation>(trace);
}

TEST(BytecodeHashingConstrainingTest, NegativeInvalidStartAfterLatch)
Expand Down
2 changes: 0 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm2/constraining/relations/bitwise.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -350,8 +350,6 @@ TEST(BitwiseConstrainingTest, MixedOperationsInteractions)
LookupIntoIndexedByClk<lookup_bitwise_integral_tag_length::Settings>().process(trace);

check_relation<bitwise>(trace);
check_interaction<lookup_bitwise_byte_operations>(trace);
check_interaction<lookup_bitwise_integral_tag_length>(trace);
}

} // namespace
Expand Down
4 changes: 0 additions & 4 deletions barretenberg/cpp/src/barretenberg/vm2/constraining/relations/class_id_derivation.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,9 +99,6 @@ TEST(ClassIdDerivationConstrainingTest, WithHashInteraction)

LookupIntoDynamicTableSequential<lookup_poseidon2_hash_0::Settings>().process(trace);
LookupIntoDynamicTableSequential<lookup_poseidon2_hash_1::Settings>().process(trace);

check_interaction<lookup_poseidon2_hash_0>(trace);
check_interaction<lookup_poseidon2_hash_1>(trace);
}

// TODO: This should probably be refined and moved to bc_retrieval test file once that exists
Expand Down Expand Up @@ -139,7 +136,6 @@ TEST(ClassIdDerivationConstrainingTest, WithRetrievalInteraction)
trace);

LookupIntoDynamicTableSequential<lookup_bc_retrieval::Settings>().process(trace);
check_interaction<lookup_bc_retrieval>(trace);
}

} // namespace
Expand Down
16 changes: 0 additions & 16 deletions barretenberg/cpp/src/barretenberg/vm2/constraining/relations/ecc.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -564,12 +564,6 @@ TEST(ScalarMulConstrainingTest, MulInteractions)
LookupIntoDynamicTableGeneric<lookup_scalar_mul_double_relation::Settings>().process(trace);
LookupIntoDynamicTableGeneric<lookup_scalar_mul_add_relation::Settings>().process(trace);
LookupIntoDynamicTableGeneric<lookup_scalar_mul_to_radix_relation::Settings>().process(trace);

check_relation<scalar_mul>(trace);
check_relation<ecc>(trace);
check_interaction<lookup_scalar_mul_double_relation>(trace);
check_interaction<lookup_scalar_mul_add_relation>(trace);
check_interaction<lookup_scalar_mul_to_radix_relation>(trace);
}

TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinity)
Expand Down Expand Up @@ -597,8 +591,6 @@ TEST(ScalarMulConstrainingTest, MulAddInteractionsInfinity)

check_relation<scalar_mul>(trace);
check_relation<ecc>(trace);
check_interaction<lookup_scalar_mul_double_relation>(trace);
check_interaction<lookup_scalar_mul_add_relation>(trace);
}

TEST(ScalarMulConstrainingTest, NegativeMulAddInteractions)
Expand All @@ -625,12 +617,6 @@ TEST(ScalarMulConstrainingTest, NegativeMulAddInteractions)
"Failed.*SCALAR_MUL_DOUBLE. Could not find tuple in destination.");
EXPECT_THROW_WITH_MESSAGE(LookupIntoDynamicTableGeneric<lookup_scalar_mul_add_relation::Settings>().process(trace),
"Failed.*SCALAR_MUL_ADD. Could not find tuple in destination.");

check_relation<scalar_mul>(trace);
EXPECT_THROW_WITH_MESSAGE(check_interaction<lookup_scalar_mul_double_relation>(trace),
"Relation.*SCALAR_MUL_DOUBLE.* ACCUMULATION.* is non-zero");
EXPECT_THROW_WITH_MESSAGE(check_interaction<lookup_scalar_mul_add_relation>(trace),
"Relation.*SCALAR_MUL_ADD.* ACCUMULATION.* is non-zero");
}

TEST(ScalarMulConstrainingTest, NegativeMulRadixInteractions)
Expand All @@ -657,8 +643,6 @@ TEST(ScalarMulConstrainingTest, NegativeMulRadixInteractions)
"Failed.*SCALAR_MUL_TO_RADIX. Could not find tuple in destination.");

check_relation<scalar_mul>(trace);
EXPECT_THROW_WITH_MESSAGE(check_interaction<lookup_scalar_mul_to_radix_relation>(trace),
"Relation.*SCALAR_MUL_TO_RADIX.* ACCUMULATION.* is non-zero");
}

TEST(ScalarMulConstrainingTest, NegativeDisableSel)
Expand Down
2 changes: 0 additions & 2 deletions barretenberg/cpp/src/barretenberg/vm2/constraining/relations/field_gt.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,6 @@ TEST_P(InteractionsTests, InteractionsWithRangeCheck)
LookupIntoDynamicTableSequential<lookup_a_lo_range::Settings>().process(trace);

check_relation<ff_gt>(trace);
check_interaction<lookup_a_hi_range>(trace);
check_interaction<lookup_a_lo_range>(trace);
}

INSTANTIATE_TEST_SUITE_P(FieldGreaterThanConstrainingTest, InteractionsTests, ::testing::ValuesIn(comparison_tests));
Expand Down
100 changes: 39 additions & 61 deletions barretenberg/cpp/src/barretenberg/vm2/constraining/relations/instr_fetching.test.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,6 @@ TEST(InstrFetchingConstrainingTest, WireInstructionSpecInteractions)
EXPECT_EQ(trace.get_num_rows(), 1 << 8); // 2^8 for selector against wire_instruction_spec

check_relation<instr_fetching>(trace);
check_interaction<wire_instr_spec_lookup>(trace);
}

std::vector<RangeCheckEvent> gen_range_check_events(const std::vector<InstructionFetchingEvent>& instr_events)
Expand All @@ -250,34 +249,6 @@ std::vector<RangeCheckEvent> gen_range_check_events(const std::vector<Instructio
return range_check_events;
}

// Positive test for interaction with range checks using same events as for the test
// EachOpcodeWithTraceGen, i.e., one event/row is generated per wire opcode.
TEST(InstrFetchingConstrainingTest, RangeCheckInteractions)
{
TestTraceContainer trace;
BytecodeTraceBuilder bytecode_builder;
PrecomputedTraceBuilder precomputed_builder;
RangeCheckTraceBuilder range_check_builder;

const auto instr_fetch_events = gen_instr_events_each_opcode();
const auto range_check_events = gen_range_check_events(instr_fetch_events);

precomputed_builder.process_sel_range_8(trace);
precomputed_builder.process_sel_range_16(trace);
bytecode_builder.process_instruction_fetching(instr_fetch_events, trace);
range_check_builder.process(range_check_events, trace);
precomputed_builder.process_misc(trace, trace.get_num_rows()); // Limit to the number of rows we need.

LookupIntoIndexedByClk<instr_abs_diff_positive_lookup::Settings>().process(trace);
LookupIntoDynamicTableGeneric<pc_abs_diff_positive_lookup::Settings>().process(trace);

EXPECT_EQ(trace.get_num_rows(), 1 << 16); // 2^16 for range checks

check_relation<instr_fetching>(trace);
check_interaction<instr_abs_diff_positive_lookup>(trace);
check_interaction<pc_abs_diff_positive_lookup>(trace);
}

// Positive test for the interaction with bytecode decomposition table.
// One event/row is generated per wire opcode (same as for test WireInstructionSpecInteractions).
TEST(InstrFetchingConstrainingTest, BcDecompositionInteractions)
Expand All @@ -302,8 +273,6 @@ TEST(InstrFetchingConstrainingTest, BcDecompositionInteractions)
EXPECT_EQ(trace.get_num_rows(), instr_fetch_events.at(0).bytecode->size() + 1);

check_relation<instr_fetching>(trace);
check_interaction<bc_decomposition_lookup>(trace);
check_interaction<bytecode_size_bc_decomposition_lookup>(trace);
}

void check_all(const std::vector<InstructionFetchingEvent>& instr_events,
Expand Down Expand Up @@ -334,12 +303,31 @@ void check_all(const std::vector<InstructionFetchingEvent>& instr_events,
EXPECT_EQ(trace.get_num_rows(), 1 << 16); // 2^16 for range checks

check_relation<instr_fetching>(trace);
check_interaction<instr_abs_diff_positive_lookup>(trace);
check_interaction<pc_abs_diff_positive_lookup>(trace);
check_interaction<wire_instr_spec_lookup>(trace);
check_interaction<bc_decomposition_lookup>(trace);
check_interaction<bytecode_size_bc_decomposition_lookup>(trace);
check_interaction<tag_validation_lookup>(trace);
}

void check_without_range_check(const std::vector<InstructionFetchingEvent>& instr_events,
const std::vector<BytecodeDecompositionEvent>& decomposition_events)
{
TestTraceContainer trace;
BytecodeTraceBuilder bytecode_builder;
PrecomputedTraceBuilder precomputed_builder;

precomputed_builder.process_wire_instruction_spec(trace);
precomputed_builder.process_sel_range_8(trace);
precomputed_builder.process_memory_tag_range(trace);
bytecode_builder.process_instruction_fetching(instr_events, trace);
bytecode_builder.process_decomposition(decomposition_events, trace);
precomputed_builder.process_misc(trace, trace.get_num_rows()); // Limit to the number of rows we need.

LookupIntoIndexedByClk<instr_abs_diff_positive_lookup::Settings>().process(trace);
LookupIntoIndexedByClk<wire_instr_spec_lookup::Settings>().process(trace);
LookupIntoIndexedByClk<tag_validation_lookup::Settings>().process(trace);
LookupIntoDynamicTableGeneric<bc_decomposition_lookup::Settings>().process(trace);
LookupIntoDynamicTableGeneric<bytecode_size_bc_decomposition_lookup::Settings>().process(trace);

EXPECT_EQ(trace.get_num_rows(), 1 << 8); // 2^8 for range checks

check_relation<instr_fetching>(trace);
}

// Positive test with 5 five bytecodes and bytecode_id = 0,1,2,3,4
Expand Down Expand Up @@ -413,7 +401,7 @@ TEST(InstrFetchingConstrainingTest, SingleInstructionOutOfRange)
},
};

check_all(instr_events, gen_range_check_events(instr_events), decomposition_events);
check_without_range_check(instr_events, decomposition_events);
}

// Positive test with one single instruction (SET_FF) with error INSTRUCTION_OUT_OF_RANGE.
Expand Down Expand Up @@ -450,7 +438,7 @@ TEST(InstrFetchingConstrainingTest, SingleInstructionOutOfRangeSplitOperand)
},
};

check_all(instr_events, gen_range_check_events(instr_events), decomposition_events);
check_without_range_check(instr_events, decomposition_events);
}

// Positive test with error case PC_OUT_OF_RANGE. We pass a pc which is out of range.
Expand Down Expand Up @@ -529,7 +517,7 @@ TEST(InstrFetchingConstrainingTest, SingleInstructionOpcodeOutOfRange)
},
};

check_all(instr_events, gen_range_check_events(instr_events), decomposition_events);
check_without_range_check(instr_events, decomposition_events);
}

// Positive test with one single instruction (SET_16) with error TAG_OUT_OF_RANGE.
Expand Down Expand Up @@ -563,11 +551,12 @@ TEST(InstrFetchingConstrainingTest, SingleInstructionTagOutOfRange)
},
};

check_all(instr_events, gen_range_check_events(instr_events), decomposition_events);
check_without_range_check(instr_events, decomposition_events);
}

// TODO(jeanmon): Reconsider this test after #13140
// Negative interaction test with some values not matching the instruction spec table.
TEST(InstrFetchingConstrainingTest, NegativeWrongWireInstructionSpecInteractions)
TEST(InstrFetchingConstrainingTest, DISABLED_NegativeWrongWireInstructionSpecInteractions)
{
BytecodeTraceBuilder bytecode_builder;
PrecomputedTraceBuilder precomputed_builder;
Expand All @@ -592,7 +581,6 @@ TEST(InstrFetchingConstrainingTest, NegativeWrongWireInstructionSpecInteractions
LookupIntoIndexedByClk<wire_instr_spec_lookup::Settings>().process(trace);

ASSERT_EQ(trace.get(C::lookup_instr_fetching_wire_instruction_info_counts, static_cast<uint32_t>(opcode)), 1);
check_interaction<wire_instr_spec_lookup>(trace);

constexpr std::array<C, 22> mutated_cols = {
C::instr_fetching_exec_opcode, C::instr_fetching_instr_size, C::instr_fetching_sel_has_tag,
Expand All @@ -614,9 +602,8 @@ TEST(InstrFetchingConstrainingTest, NegativeWrongWireInstructionSpecInteractions
// We do not need to re-run LookupIntoIndexedByClk<wire_instr_spec_lookup::Settings>().process(trace);
Copy link
Contributor Author

@fcarreiro fcarreiro Mar 28, 2025

Choose a reason for hiding this comment

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

@jeanmon I disabled 2 tests because of a comment like this. I'll leave it to you to think if the test still makes sense.

// because we never mutate the indexing column for this lookup (clk) and for this lookup
// find_in_dst only uses column C::instr_fetching_bd0 mapped to (clk). So, the counts are still valid.

EXPECT_THROW_WITH_MESSAGE(check_interaction<wire_instr_spec_lookup>(mutated_trace),
"Relation.*WIRE_INSTRUCTION_INFO.* ACCUMULATION.* is non-zero");
// EXPECT_THROW_WITH_MESSAGE(check_interaction<wire_instr_spec_lookup>(mutated_trace),
// "Relation.*WIRE_INSTRUCTION_INFO.* ACCUMULATION.* is non-zero");
}
}
}
Expand Down Expand Up @@ -650,7 +637,6 @@ TEST(InstrFetchingConstrainingTest, NegativeWrongBcDecompositionInteractions)

auto valid_trace = trace; // Keep original trace before lookup processing
LookupIntoDynamicTableSequential<bc_decomposition_lookup::Settings>().process(valid_trace);
check_interaction<bc_decomposition_lookup>(valid_trace);

constexpr std::array<C, 39> mutated_cols = {
C::instr_fetching_pc, C::instr_fetching_bytecode_id, C::instr_fetching_bd0, C::instr_fetching_bd1,
Expand All @@ -671,14 +657,11 @@ TEST(InstrFetchingConstrainingTest, NegativeWrongBcDecompositionInteractions)
const FF mutated_value = trace.get(col, 1) + 1; // Mutate to value + 1
mutated_trace.set(col, 1, mutated_value);

// This sets the length of the inverse polynomial via SetDummyInverses, so we still need to call this
// even though we know it will fail.
// This sets the length of the inverse polynomial via SetDummyInverses, so we
// still need to call this even though we know it will fail.
EXPECT_THROW_WITH_MESSAGE(
LookupIntoDynamicTableSequential<bc_decomposition_lookup::Settings>().process(mutated_trace),
"Failed.*BYTES_FROM_BC_DEC. Could not find tuple in destination.");

EXPECT_THROW_WITH_MESSAGE(check_interaction<bc_decomposition_lookup>(mutated_trace),
"Relation.*BYTES_FROM_BC_DEC.* ACCUMULATION.* is non-zero");
}
}
}
Expand Down Expand Up @@ -724,7 +707,6 @@ TEST(InstrFetchingConstrainingTest, NegativeWrongBytecodeSizeBcDecompositionInte

auto valid_trace = trace; // Keep original trace before lookup processing
LookupIntoDynamicTableSequential<bytecode_size_bc_decomposition_lookup::Settings>().process(valid_trace);
check_interaction<bytecode_size_bc_decomposition_lookup>(valid_trace);

auto mutated_trace = trace;
const FF mutated_value = trace.get(C::instr_fetching_bytecode_size, 1) + 1; // Mutate to value + 1
Expand All @@ -735,14 +717,12 @@ TEST(InstrFetchingConstrainingTest, NegativeWrongBytecodeSizeBcDecompositionInte
EXPECT_THROW_WITH_MESSAGE(
LookupIntoDynamicTableSequential<bytecode_size_bc_decomposition_lookup::Settings>().process(mutated_trace),
"Failed.*BYTECODE_SIZE_FROM_BC_DEC. Could not find tuple in destination.");

EXPECT_THROW_WITH_MESSAGE(check_interaction<bytecode_size_bc_decomposition_lookup>(mutated_trace),
"Relation.*BYTECODE_SIZE_FROM_BC_DEC.* ACCUMULATION.* is non-zero");
}
}

// TODO(jeanmon): Reconsider this test after #13140
// Negative interaction test for #[TAG_VALUE_VALIDATION] where tag_out_of_range is wrongly mutated
TEST(InstrFetchingConstrainingTest, NegativeWrongTagValidationInteractions)
TEST(InstrFetchingConstrainingTest, DISABLED_NegativeWrongTagValidationInteractions)
{
TestTraceContainer trace;
BytecodeTraceBuilder bytecode_builder;
Expand All @@ -768,7 +748,6 @@ TEST(InstrFetchingConstrainingTest, NegativeWrongTagValidationInteractions)
LookupIntoIndexedByClk<tag_validation_lookup::Settings>().process(trace);

auto valid_trace = trace; // Keep original trace before lookup processing
check_interaction<tag_validation_lookup>(valid_trace);

// Mutate tag out-of-range error
auto mutated_trace = trace;
Expand All @@ -778,9 +757,8 @@ TEST(InstrFetchingConstrainingTest, NegativeWrongTagValidationInteractions)
// We do not need to re-run LookupIntoIndexedByClk<tag_validation_lookup::Settings>().process(trace);
// because we never mutate the indexing column for this lookup (clk) and for this lookup
// find_in_dst only uses column C::instr_fetching_tag_value mapped to (clk). So, the counts are still valid.

EXPECT_THROW_WITH_MESSAGE(check_interaction<tag_validation_lookup>(mutated_trace),
"Relation.*TAG_VALUE_VALIDATION.* ACCUMULATION.* is non-zero");
// EXPECT_THROW_WITH_MESSAGE(check_interaction<tag_validation_lookup>(mutated_trace),
// "Relation.*TAG_VALUE_VALIDATION.* ACCUMULATION.* is non-zero");
}
}

Expand Down
Loading