chore(avm): vm2 followup cleanup (#11186)

barretenberg/cpp/pil/vm2/addressing.pil

@@ -1,8 +1,8 @@
 // This is a virtual gadget, which is part of the execution trace.
-namespace execution(256);
+namespace execution;
 
-pol commit stack_pointer_val;
-pol commit stack_pointer_tag;
+pol commit base_address_val;
+pol commit base_address_tag;
 pol commit sel_addressing_error;  // true if any error type
 pol commit addressing_error_kind;  // TODO: might need to be selectors
 pol commit addressing_error_idx;  // operand index for error, if any

barretenberg/cpp/pil/vm2/alu.pil

@@ -1,4 +1,4 @@
-namespace alu(256);
+namespace alu;
 
 pol commit sel_op_add;
 pol commit ia;

barretenberg/cpp/pil/vm2/execution.pil

@@ -2,7 +2,7 @@ include "alu.pil";
 include "addressing.pil";
 include "precomputed.pil";
 
-namespace execution(256);
+namespace execution;
 
 pol commit sel; // subtrace selector
 

barretenberg/cpp/pil/vm2/precomputed.pil

@@ -1,5 +1,5 @@
 // General/shared precomputed columns.
-namespace precomputed(256);
+namespace precomputed;
 
 // From 0 and incrementing up to the size of the circuit (2^21).
 pol constant clk;

barretenberg/cpp/src/barretenberg/vm2/constraining/check_circuit.hpp

@@ -7,7 +7,7 @@
 namespace bb::avm2::constraining {
 
 // This is a version of check circuit that runs on the prover polynomials.
-// Better versions could be done, but since this is for debugging only, it is enough for now.
+// It is the closest to "real proving" that we can get without actually running the prover.
 void run_check_circuit(AvmFlavor::ProverPolynomials& polys, size_t num_rows);
 
 } // namespace bb::avm2::constraining

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

@@ -8,7 +8,7 @@ namespace bb::avm2 {
 // The entities that will be used in the flavor.
 // clang-format off
 #define AVM2_PRECOMPUTED_ENTITIES precomputed_bitwise_input_a, precomputed_bitwise_input_b, precomputed_bitwise_op_id, precomputed_bitwise_output, precomputed_clk, precomputed_first_row, precomputed_sel_bitwise
-#define AVM2_WIRE_ENTITIES execution_input, alu_dst_addr, alu_ia, alu_ia_addr, alu_ib, alu_ib_addr, alu_ic, alu_op, alu_sel_op_add, execution_addressing_error_idx, execution_addressing_error_kind, execution_clk, execution_ex_opcode, execution_indirect, execution_last, execution_op1, execution_op1_after_relative, execution_op2, execution_op2_after_relative, execution_op3, execution_op3_after_relative, execution_op4, execution_op4_after_relative, execution_pc, execution_rop1, execution_rop2, execution_rop3, execution_rop4, execution_sel, execution_sel_addressing_error, execution_sel_op1_is_address, execution_sel_op2_is_address, execution_sel_op3_is_address, execution_sel_op4_is_address, execution_stack_pointer_tag, execution_stack_pointer_val, lookup_dummy_precomputed_counts, lookup_dummy_dynamic_counts
+#define AVM2_WIRE_ENTITIES execution_input, alu_dst_addr, alu_ia, alu_ia_addr, alu_ib, alu_ib_addr, alu_ic, alu_op, alu_sel_op_add, execution_addressing_error_idx, execution_addressing_error_kind, execution_base_address_tag, execution_base_address_val, execution_clk, execution_ex_opcode, execution_indirect, execution_last, execution_op1, execution_op1_after_relative, execution_op2, execution_op2_after_relative, execution_op3, execution_op3_after_relative, execution_op4, execution_op4_after_relative, execution_pc, execution_rop1, execution_rop2, execution_rop3, execution_rop4, execution_sel, execution_sel_addressing_error, execution_sel_op1_is_address, execution_sel_op2_is_address, execution_sel_op3_is_address, execution_sel_op4_is_address, lookup_dummy_precomputed_counts, lookup_dummy_dynamic_counts
 #define AVM2_DERIVED_WITNESS_ENTITIES perm_dummy_dynamic_inv, lookup_dummy_precomputed_inv, lookup_dummy_dynamic_inv
 #define AVM2_SHIFTED_ENTITIES execution_sel_shift
 #define AVM2_TO_BE_SHIFTED(e) e.execution_sel

barretenberg/cpp/src/barretenberg/vm2/generated/flavor.cpp

@@ -23,31 +23,31 @@ AvmFlavor::AllConstRefValues::AllConstRefValues(
     , alu_sel_op_add(il[15])
     , execution_addressing_error_idx(il[16])
     , execution_addressing_error_kind(il[17])
-    , execution_clk(il[18])
-    , execution_ex_opcode(il[19])
-    , execution_indirect(il[20])
-    , execution_last(il[21])
-    , execution_op1(il[22])
-    , execution_op1_after_relative(il[23])
-    , execution_op2(il[24])
-    , execution_op2_after_relative(il[25])
-    , execution_op3(il[26])
-    , execution_op3_after_relative(il[27])
-    , execution_op4(il[28])
-    , execution_op4_after_relative(il[29])
-    , execution_pc(il[30])
-    , execution_rop1(il[31])
-    , execution_rop2(il[32])
-    , execution_rop3(il[33])
-    , execution_rop4(il[34])
-    , execution_sel(il[35])
-    , execution_sel_addressing_error(il[36])
-    , execution_sel_op1_is_address(il[37])
-    , execution_sel_op2_is_address(il[38])
-    , execution_sel_op3_is_address(il[39])
-    , execution_sel_op4_is_address(il[40])
-    , execution_stack_pointer_tag(il[41])
-    , execution_stack_pointer_val(il[42])
+    , execution_base_address_tag(il[18])
+    , execution_base_address_val(il[19])
+    , execution_clk(il[20])
+    , execution_ex_opcode(il[21])
+    , execution_indirect(il[22])
+    , execution_last(il[23])
+    , execution_op1(il[24])
+    , execution_op1_after_relative(il[25])
+    , execution_op2(il[26])
+    , execution_op2_after_relative(il[27])
+    , execution_op3(il[28])
+    , execution_op3_after_relative(il[29])
+    , execution_op4(il[30])
+    , execution_op4_after_relative(il[31])
+    , execution_pc(il[32])
+    , execution_rop1(il[33])
+    , execution_rop2(il[34])
+    , execution_rop3(il[35])
+    , execution_rop4(il[36])
+    , execution_sel(il[37])
+    , execution_sel_addressing_error(il[38])
+    , execution_sel_op1_is_address(il[39])
+    , execution_sel_op2_is_address(il[40])
+    , execution_sel_op3_is_address(il[41])
+    , execution_sel_op4_is_address(il[42])
     , lookup_dummy_precomputed_counts(il[43])
     , lookup_dummy_dynamic_counts(il[44])
     , perm_dummy_dynamic_inv(il[45])
@@ -88,6 +88,8 @@ AvmFlavor::AllConstRefValues AvmFlavor::ProverPolynomials::get_row(size_t row_id
                      alu_sel_op_add[row_idx],
                      execution_addressing_error_idx[row_idx],
                      execution_addressing_error_kind[row_idx],
+                     execution_base_address_tag[row_idx],
+                     execution_base_address_val[row_idx],
                      execution_clk[row_idx],
                      execution_ex_opcode[row_idx],
                      execution_indirect[row_idx],
@@ -111,8 +113,6 @@ AvmFlavor::AllConstRefValues AvmFlavor::ProverPolynomials::get_row(size_t row_id
                      execution_sel_op2_is_address[row_idx],
                      execution_sel_op3_is_address[row_idx],
                      execution_sel_op4_is_address[row_idx],
-                     execution_stack_pointer_tag[row_idx],
-                     execution_stack_pointer_val[row_idx],
                      lookup_dummy_precomputed_counts[row_idx],
                      lookup_dummy_dynamic_counts[row_idx],
                      perm_dummy_dynamic_inv[row_idx],
@@ -141,6 +141,8 @@ AvmFlavor::CommitmentLabels::CommitmentLabels()
     Base::alu_sel_op_add = "ALU_SEL_OP_ADD";
     Base::execution_addressing_error_idx = "EXECUTION_ADDRESSING_ERROR_IDX";
     Base::execution_addressing_error_kind = "EXECUTION_ADDRESSING_ERROR_KIND";
+    Base::execution_base_address_tag = "EXECUTION_BASE_ADDRESS_TAG";
+    Base::execution_base_address_val = "EXECUTION_BASE_ADDRESS_VAL";
     Base::execution_clk = "EXECUTION_CLK";
     Base::execution_ex_opcode = "EXECUTION_EX_OPCODE";
     Base::execution_indirect = "EXECUTION_INDIRECT";
@@ -164,8 +166,6 @@ AvmFlavor::CommitmentLabels::CommitmentLabels()
     Base::execution_sel_op2_is_address = "EXECUTION_SEL_OP2_IS_ADDRESS";
     Base::execution_sel_op3_is_address = "EXECUTION_SEL_OP3_IS_ADDRESS";
     Base::execution_sel_op4_is_address = "EXECUTION_SEL_OP4_IS_ADDRESS";
-    Base::execution_stack_pointer_tag = "EXECUTION_STACK_POINTER_TAG";
-    Base::execution_stack_pointer_val = "EXECUTION_STACK_POINTER_VAL";
     Base::perm_dummy_dynamic_inv = "PERM_DUMMY_DYNAMIC_INV";
     Base::lookup_dummy_precomputed_inv = "LOOKUP_DUMMY_PRECOMPUTED_INV";
     Base::lookup_dummy_dynamic_inv = "LOOKUP_DUMMY_DYNAMIC_INV";

barretenberg/cpp/src/barretenberg/vm2/generated/full_row.cpp

@@ -37,6 +37,8 @@ template <typename FF> std::vector<std::string> AvmFullRow<FF>::names()
              "alu_sel_op_add",
              "execution_addressing_error_idx",
              "execution_addressing_error_kind",
+             "execution_base_address_tag",
+             "execution_base_address_val",
              "execution_clk",
              "execution_ex_opcode",
              "execution_indirect",
@@ -60,8 +62,6 @@ template <typename FF> std::vector<std::string> AvmFullRow<FF>::names()
              "execution_sel_op2_is_address",
              "execution_sel_op3_is_address",
              "execution_sel_op4_is_address",
-             "execution_stack_pointer_tag",
-             "execution_stack_pointer_val",
              "perm_dummy_dynamic_inv",
              "lookup_dummy_precomputed_inv",
              "lookup_dummy_dynamic_inv",
@@ -90,6 +90,8 @@ template <typename FF> RefVector<const FF> AvmFullRow<FF>::as_vector() const
         alu_sel_op_add,
         execution_addressing_error_idx,
         execution_addressing_error_kind,
+        execution_base_address_tag,
+        execution_base_address_val,
         execution_clk,
         execution_ex_opcode,
         execution_indirect,
@@ -113,8 +115,6 @@ template <typename FF> RefVector<const FF> AvmFullRow<FF>::as_vector() const
         execution_sel_op2_is_address,
         execution_sel_op3_is_address,
         execution_sel_op4_is_address,
-        execution_stack_pointer_tag,
-        execution_stack_pointer_val,
         perm_dummy_dynamic_inv,
         lookup_dummy_precomputed_inv,
         lookup_dummy_dynamic_inv,

barretenberg/cpp/src/barretenberg/vm2/generated/full_row.hpp

@@ -29,7 +29,7 @@ template <typename FF_> struct AvmFullRow {
     const FF& get_column(ColumnAndShifts col) const
     {
         static_assert(sizeof(*this) == sizeof(FF) * static_cast<size_t>(ColumnAndShifts::NUM_COLUMNS));
-        return reinterpret_cast<FF*>(this)[static_cast<size_t>(col)];
+        return reinterpret_cast<const FF*>(this)[static_cast<size_t>(col)];
     }
 };
 

barretenberg/cpp/src/barretenberg/vm2/generated/relations/execution.hpp

@@ -34,7 +34,7 @@ template <typename FF_> class executionImpl {
         {
             using Accumulator = typename std::tuple_element_t<2, ContainerOverSubrelations>;
             auto tmp =
-                (new_term.execution_sel * ((FF(1) - new_term.execution_sel_shift) * (FF(1) - new_term.execution_last)));
+                ((new_term.execution_sel * (FF(1) - new_term.execution_sel_shift)) * (FF(1) - new_term.execution_last));
             tmp *= scaling_factor;
             std::get<2>(evals) += typename Accumulator::View(tmp);
         }

barretenberg/cpp/src/barretenberg/vm2/simulation/addressing.cpp

@@ -32,20 +32,20 @@ std::vector<Operand> Addressing::resolve(const Instruction& instruction, MemoryI
         // We retrieve, cache first because this is probably what we'll do in the circuit.
         // However, we can't check the value and tag yet! This should be done only if it's used.
         // This is because the first few instructions might not YET have a valid stack pointer.
-        auto stack_pointer = memory.get(0);
-        event.stack_pointer_tag = stack_pointer.tag;
-        event.stack_pointer_val = stack_pointer.value;
+        auto base_address = memory.get(0);
+        event.base_address_tag = base_address.tag;
+        event.base_address_val = base_address.value;
 
         // First process relative addressing for all the addresses.
         event.after_relative = instruction.operands;
         for (size_t i = 0; i < spec.num_addresses; ++i) {
             if ((instruction.indirect >> i) & 1) {
-                if (!memory.is_valid_address(stack_pointer)) {
-                    throw AddressingException(AddressingEventError::STACK_POINTER_INVALID_ADDRESS, i);
+                if (!memory.is_valid_address(base_address)) {
+                    throw AddressingException(AddressingEventError::BASE_ADDRESS_INVALID_ADDRESS, i);
                 }
 
                 MemoryValue offset(event.after_relative[i]);
-                offset += stack_pointer.value;
+                offset += base_address.value;
                 event.after_relative[i] = Operand::ff(offset);
                 if (!memory.is_valid_address(offset)) {
                     throw AddressingException(AddressingEventError::RELATIVE_COMPUTATION_OOB, i);

barretenberg/cpp/src/barretenberg/vm2/simulation/events/addressing_event.hpp

@@ -12,7 +12,7 @@
 namespace bb::avm2::simulation {
 
 enum class AddressingEventError {
-    STACK_POINTER_INVALID_ADDRESS,
+    BASE_ADDRESS_INVALID_ADDRESS,
     RELATIVE_COMPUTATION_OOB,
     INDIRECT_INVALID_ADDRESS,
     FINAL_ADDRESS_INVALID,
@@ -35,8 +35,8 @@ struct AddressingEvent {
     Instruction instruction;
     std::vector<Operand> after_relative;
     std::vector<Operand> resolved_operands;
-    MemoryValue stack_pointer_val;
-    MemoryTag stack_pointer_tag;
+    MemoryValue base_address_val;
+    MemoryTag base_address_tag;
     const InstructionSpec* spec = nullptr;
     std::optional<AddressingException> error;
 };

barretenberg/cpp/src/barretenberg/vm2/simulation/events/bytecode_events.hpp

@@ -12,7 +12,8 @@
 
 namespace bb::avm2::simulation {
 
-// Question: ideally we'd avoid exploding the whole thing here, but we could if needed to.
+// TODO: Implement tracegen for this. This event might need to change. Ideally we'd
+// avoid having an event for each iteration of the hashing.
 // It really depends on how we want to separate the concerns between simulation and tracegen.
 // And wether we want to allow events to explode vertically in tracegen.
 struct BytecodeHashingEvent {

barretenberg/cpp/src/barretenberg/vm2/simulation/lib/raw_data_db.cpp

@@ -24,14 +24,12 @@ ContractInstance HintedRawDataDB::get_contract_instance(const AztecAddress& addr
         .contract_class_id = contract_instance_hint.contractClassId,
         .initialisation_hash = contract_instance_hint.initializationHash,
         .public_keys =
-            [](const auto& pk) {
-                return PublicKeys{
-                    .nullifier_key = pk.masterNullifierPublicKey,
-                    .incoming_viewing_key = pk.masterIncomingViewingPublicKey,
-                    .outgoing_viewing_key = pk.masterOutgoingViewingPublicKey,
-                    .tagging_key = pk.masterTaggingPublicKey,
-                };
-            }(contract_instance_hint.publicKeys),
+            PublicKeys{
+                .nullifier_key = contract_instance_hint.publicKeys.masterNullifierPublicKey,
+                .incoming_viewing_key = contract_instance_hint.publicKeys.masterIncomingViewingPublicKey,
+                .outgoing_viewing_key = contract_instance_hint.publicKeys.masterOutgoingViewingPublicKey,
+                .tagging_key = contract_instance_hint.publicKeys.masterTaggingPublicKey,
+            },
     };
 }
 

barretenberg/cpp/src/barretenberg/vm2/tracegen/execution_trace.cpp

@@ -65,8 +65,8 @@ void ExecutionTraceBuilder::process(
         trace.set(
             row,
             { {
-                { C::execution_stack_pointer_val, addr_event.stack_pointer_val },
-                { C::execution_stack_pointer_tag, static_cast<size_t>(addr_event.stack_pointer_tag) },
+                { C::execution_base_address_val, addr_event.base_address_val },
+                { C::execution_base_address_tag, static_cast<size_t>(addr_event.base_address_tag) },
                 { C::execution_sel_addressing_error, addr_event.error.has_value() ? 1 : 0 },
                 { C::execution_addressing_error_idx, addr_event.error.has_value() ? addr_event.error->operand_idx : 0 },
                 { C::execution_addressing_error_kind,

barretenberg/cpp/src/barretenberg/vm2/tracegen/lib/README.md

@@ -3,7 +3,7 @@
 **Current state**
 
 - Both lookups and permutations are supported.
-  - When you lookup into a precomputed table, you need to define a fast way to find the row for a tuple. See bitwise example.
+  - When you lookup into a precomputed table, you need to define a fast way to find the row for a tuple (`find_dst_row`). See bitwise example.
   - When you lookup into a non-precomputed (dynamic) table, you can use the class `LookupIntoDynamicTable`. There is an example.
   - For permutations you need to use the `PermutationBuilder` class.
 - Lookups and permutations work but you need to manually create a LookupInto class and add it to the tracehelper. You can use the autogenerated `lookup_settings` class to specify the columns, etc. See examples.

barretenberg/cpp/src/barretenberg/vm2/tracegen/lib/lookup_builder.hpp

@@ -1,6 +1,7 @@
 #pragma once
 
 #include <array>
+#include <bit>
 #include <cstddef>
 #include <stdexcept>
 
@@ -58,6 +59,7 @@ template <typename LookupSettings_> class LookupIntoDynamicTable : public BaseLo
 
   protected:
     using LookupSettings = LookupSettings_;
+    using ArrayTuple = std::array<FF, LookupSettings::LOOKUP_TUPLE_SIZE>;
 
     void init(TraceContainer& trace) override
     {
@@ -67,13 +69,13 @@ template <typename LookupSettings_> class LookupIntoDynamicTable : public BaseLo
             (void)dst_sel_value; // Avoid GCC complaining of unused parameter when asserts are disabled.
 
             auto dst_values = trace.get_multiple(LookupSettings::DST_COLUMNS, row);
-            row_idx.insert({ get_key(dst_values), row });
+            row_idx.insert({ dst_values, row });
         });
     }
 
-    uint32_t find_in_dst(const std::array<FF, LookupSettings::LOOKUP_TUPLE_SIZE>& tup) const override
+    uint32_t find_in_dst(const ArrayTuple& tup) const override
     {
-        auto it = row_idx.find(get_key(tup));
+        auto it = row_idx.find(tup);
         if (it != row_idx.end()) {
             return it->second;
         }
@@ -83,20 +85,21 @@ template <typename LookupSettings_> class LookupIntoDynamicTable : public BaseLo
     }
 
   private:
-    FF get_key(const std::array<FF, LookupSettings::LOOKUP_TUPLE_SIZE>& tup) const
+    // TODO: Using the whole tuple as the key is not memory efficient.
+    unordered_flat_map<ArrayTuple, uint32_t> row_idx;
+};
+
+} // namespace bb::avm2::tracegen
+
+// Define a hash function for std::array so that it can be used as a key in a std::unordered_map.
+template <typename T, size_t SIZE> struct std::hash<std::array<T, SIZE>> {
+    std::size_t operator()(const std::array<T, SIZE>& arr) const noexcept
     {
-        FF acc = 0;
-        for (const auto& el : tup) {
-            acc = acc * beta + el;
+        std::size_t hash = 0;
+        for (const auto& elem : arr) {
+            hash = std::rotl(hash, 1);
+            hash ^= std::hash<T>{}(elem);
         }
-        return acc + gamma;
+        return hash;
     }
-
-    // We use an RLC for the key instead of the tuple, to save memory.
-    // FIXME: reconsider, what if beta is 0.
-    unordered_flat_map<FF, uint32_t> row_idx;
-    FF beta = FF::random_element();
-    FF gamma = FF::random_element();
-};
-
-} // namespace bb::avm2::tracegen
+};