chore: addressing remaining feedback in PR 12182

barretenberg/cpp/pil/vm2/instr_fetching.pil

@@ -30,6 +30,8 @@ pol commit op1, op2, op3, op4, op5, op6, op7;
 // Wire to execution opcodes translation.
 pol commit exec_opcode;
 
+pol commit instr_size_in_bytes;
+
 // Bring in the bytes from the bytecode columns.
 #[BYTES_FROM_BC_DEC]
 sel {
@@ -82,6 +84,7 @@ pol commit sel_op_dc_17;
 sel {
      bd0,
      exec_opcode,
+     instr_size_in_bytes,
      sel_op_dc_0,  sel_op_dc_1,  sel_op_dc_2,  sel_op_dc_3,
      sel_op_dc_4,  sel_op_dc_5,  sel_op_dc_6,  sel_op_dc_7,
      sel_op_dc_8,  sel_op_dc_9,  sel_op_dc_10, sel_op_dc_11,
@@ -91,6 +94,7 @@ sel {
 precomputed.sel_range_wire_opcode {
     precomputed.clk,
     precomputed.exec_opcode,
+    precomputed.instr_size_in_bytes,
     precomputed.sel_op_dc_0,  precomputed.sel_op_dc_1,  precomputed.sel_op_dc_2,  precomputed.sel_op_dc_3,
     precomputed.sel_op_dc_4,  precomputed.sel_op_dc_5,  precomputed.sel_op_dc_6,  precomputed.sel_op_dc_7,
     precomputed.sel_op_dc_8,  precomputed.sel_op_dc_9,  precomputed.sel_op_dc_10, precomputed.sel_op_dc_11,
@@ -103,11 +107,19 @@ precomputed.sel_range_wire_opcode {
 // Remark: Upper-casing the alias needs to be edited manually (not code-generated)!
 pol SEL_OP_DC_18 = sel_op_dc_2 + sel_op_dc_6;
 
+#[INDIRECT_BYTES_DECOMPOSITION]
 indirect = sel_op_dc_0 * (bd1 * 2**8 + bd2 * 2**0) + SEL_OP_DC_18 * (bd1 * 2**0);
+#[OP1_BYTES_DECOMPOSITION]
 op1 = sel_op_dc_0 * (bd3 * 2**8 + bd4 * 2**0) + sel_op_dc_2 * (bd2 * 2**8 + bd3 * 2**0) + sel_op_dc_6 * (bd2 * 2**0) + sel_op_dc_15 * (bd1 * 2**24 + bd2 * 2**16 + bd3 * 2**8 + bd4 * 2**0);
+#[OP2_BYTES_DECOMPOSITION]
 op2 = sel_op_dc_0 * (bd5 * 2**8 + bd6 * 2**0) + sel_op_dc_3 * (bd4 * 2**8 + bd5 * 2**0) + sel_op_dc_6 * (bd3 * 2**0) + sel_op_dc_8 * (bd4 * 2**0) + sel_op_dc_16 * (bd4 * 2**24 + bd5 * 2**16 + bd6 * 2**8 + bd7 * 2**0);
+#[OP3_BYTES_DECOMPOSITION]
 op3 = sel_op_dc_0 * (bd7 * 2**8 + bd8 * 2**0) + sel_op_dc_4 * (bd6 * 2**8 + bd7 * 2**0) + sel_op_dc_9 * (bd5 * 2**248 + bd6 * 2**240 + bd7 * 2**232 + bd8 * 2**224 + bd9 * 2**216 + bd10 * 2**208 + bd11 * 2**200 + bd12 * 2**192 + bd13 * 2**184 + bd14 * 2**176 + bd15 * 2**168 + bd16 * 2**160 + bd17 * 2**152 + bd18 * 2**144 + bd19 * 2**136 + bd20 * 2**128 + bd21 * 2**120 + bd22 * 2**112 + bd23 * 2**104 + bd24 * 2**96 + bd25 * 2**88 + bd26 * 2**80 + bd27 * 2**72 + bd28 * 2**64 + bd29 * 2**56 + bd30 * 2**48 + bd31 * 2**40 + bd32 * 2**32 + bd33 * 2**24 + bd34 * 2**16 + bd35 * 2**8 + bd36 * 2**0) + sel_op_dc_10 * (bd5 * 2**120 + bd6 * 2**112 + bd7 * 2**104 + bd8 * 2**96 + bd9 * 2**88 + bd10 * 2**80 + bd11 * 2**72 + bd12 * 2**64 + bd13 * 2**56 + bd14 * 2**48 + bd15 * 2**40 + bd16 * 2**32 + bd17 * 2**24 + bd18 * 2**16 + bd19 * 2**8 + bd20 * 2**0) + sel_op_dc_11 * (bd5 * 2**56 + bd6 * 2**48 + bd7 * 2**40 + bd8 * 2**32 + bd9 * 2**24 + bd10 * 2**16 + bd11 * 2**8 + bd12 * 2**0) + sel_op_dc_12 * (bd5 * 2**24 + bd6 * 2**16 + bd7 * 2**8 + bd8 * 2**0) + sel_op_dc_13 * (bd5 * 2**8 + bd6 * 2**0) + sel_op_dc_14 * (bd4 * 2**0) + sel_op_dc_17 * (bd6 * 2**0);
+#[OP4_BYTES_DECOMPOSITION]
 op4 = sel_op_dc_0 * (bd9 * 2**8 + bd10 * 2**0) + sel_op_dc_5 * (bd8 * 2**8 + bd9 * 2**0) + sel_op_dc_7 * (bd8 * 2**0);
+#[OP5_BYTES_DECOMPOSITION]
 op5 = sel_op_dc_0 * (bd11 * 2**8 + bd12 * 2**0);
+#[OP6_BYTES_DECOMPOSITION]
 op6 = sel_op_dc_1 * (bd13 * 2**8 + bd14 * 2**0);
+#[OP7_BYTES_DECOMPOSITION]
 op7 = sel_op_dc_1 * (bd15 * 2**8 + bd16 * 2**0);

barretenberg/cpp/pil/vm2/precomputed.pil

@@ -75,6 +75,7 @@ pol constant sel_op_dc_16;
 pol constant sel_op_dc_17;
 
 pol constant exec_opcode;
+pol constant instr_size_in_bytes;
 
 // Toggle the rows which index (clk) is equal to a wire opcode
 // Is used to lookup into the wire instruction spec table which contains the operand decomposition

barretenberg/cpp/src/barretenberg/vm2/common/instruction_spec.hpp

@@ -28,6 +28,7 @@ struct ExecInstructionSpec {
 
 struct WireInstructionSpec {
     ExecutionOpCode exec_opcode;
+    uint32_t size_in_bytes;
     std::array<uint8_t, NUM_OP_DC_SELECTORS> op_dc_selectors;
 
     bool operator==(const WireInstructionSpec& other) const = default;

barretenberg/cpp/src/barretenberg/vm2/common/instruction_spec.test.cpp

@@ -0,0 +1,39 @@
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+#include "barretenberg/vm2/common/instruction_spec.hpp"
+#include "barretenberg/vm2/common/opcodes.hpp"
+#include "barretenberg/vm2/simulation/lib/serialization.hpp"
+
+namespace bb::avm2 {
+namespace {
+
+size_t compute_instruction_size(WireOpCode wire_opcode,
+                                const std::unordered_map<WireOpCode, std::vector<simulation::OperandType>>& wire_format,
+                                const std::unordered_map<simulation::OperandType, uint32_t>& operand_type_sizes)
+{
+    size_t instr_size = 1; // Take into account the opcode byte
+    for (const auto& operand_type : wire_format.at(wire_opcode)) {
+        instr_size += operand_type_sizes.at(operand_type);
+    }
+
+    return instr_size;
+}
+
+// Test checking that the hardcoded size for each instruction specified in WIRE_INSTRUCTION_SPEC
+// is correct. This test would fail only when we change the wire format of an instruction.
+TEST(InstructionSpecTest, CheckAllInstructionSizes)
+{
+    const auto& wire_format = simulation::testonly::get_instruction_wire_formats();
+    const auto& operand_type_sizes = simulation::testonly::get_operand_type_sizes();
+
+    for (int i = 0; i < static_cast<int>(WireOpCode::LAST_OPCODE_SENTINEL); i++) {
+        const auto wire_opcode = static_cast<WireOpCode>(i);
+        const auto computed_size = compute_instruction_size(wire_opcode, wire_format, operand_type_sizes);
+        EXPECT_EQ(WIRE_INSTRUCTION_SPEC.at(wire_opcode).size_in_bytes, computed_size)
+            << "Incorrect size_in_bytes field for " << wire_opcode << " in WIRE_INSTRUCTION_SPEC.";
+    }
+}
+
+} // namespace
+} // namespace bb::avm2

barretenberg/cpp/src/barretenberg/vm2/common/memory_types.hpp

@@ -6,6 +6,7 @@
 
 namespace bb::avm2 {
 
+// Adapt NUM_MEMORY_TAGS in fixtures.cpp if this enum is modified.
 enum class MemoryTag {
     FF,
     U1,

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/bc_decomposition.test.cpp

@@ -34,11 +34,7 @@ void init_trace(TestTraceContainer& trace)
 
 TEST(BytecodeDecompositionConstrainingTest, EmptyRow)
 {
-    TestTraceContainer trace({
-        { { C::precomputed_first_row, 1 } },
-    });
-
-    check_relation<bc_decomposition>(trace);
+    check_relation<bc_decomposition>(testing::empty_trace());
 }
 
 TEST(BytecodeDecompositionConstrainingTest, SingleBytecode)

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/bc_hashing.test.cpp

@@ -41,11 +41,7 @@ using length_iv_relation = bb::avm2::lookup_bc_hashing_iv_is_len_relation<FF>;
 
 TEST(BytecodeHashingConstrainingTest, EmptyRow)
 {
-    TestTraceContainer trace({
-        { { C::precomputed_first_row, 1 } },
-    });
-
-    check_relation<bc_hashing>(trace);
+    check_relation<bc_hashing>(testing::empty_trace());
 }
 
 TEST(BytecodeHashingConstrainingTest, SingleBytecodeHash)

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/bitwise.test.cpp

@@ -8,6 +8,7 @@
 #include "barretenberg/vm2/constraining/testing/check_relation.hpp"
 #include "barretenberg/vm2/generated/flavor_settings.hpp"
 #include "barretenberg/vm2/generated/relations/bitwise.hpp"
+#include "barretenberg/vm2/testing/fixtures.hpp"
 #include "barretenberg/vm2/testing/macros.hpp"
 #include "barretenberg/vm2/tracegen/bitwise_trace.hpp"
 #include "barretenberg/vm2/tracegen/test_trace_container.hpp"
@@ -23,11 +24,7 @@ using bitwise = bb::avm2::bitwise<FF>;
 
 TEST(BitwiseConstrainingTest, EmptyRow)
 {
-    TestTraceContainer trace({
-        { { C::precomputed_first_row, 1 } },
-    });
-
-    check_relation<bitwise>(trace);
+    check_relation<bitwise>(testing::empty_trace());
 }
 
 // Testing a positive AND operation for each integral type (U1, U8, ... U128)

barretenberg/cpp/src/barretenberg/vm2/constraining/relations/class_id_derivation.test.cpp

@@ -54,12 +54,7 @@ ContractClass generate_contract_class()
 
 TEST(ClassIdDerivationConstrainingTest, EmptyRow)
 {
-    TestTraceContainer trace({
-        { { C::precomputed_first_row, 1 } },
-        { { C::precomputed_clk, 0 } },
-    });
-
-    check_relation<class_id_derivation_relation>(trace);
+    check_relation<class_id_derivation_relation>(testing::empty_trace());
 }
 
 TEST(ClassIdDerivationConstrainingTest, Basic)