5466: First unit test and various bugs fixing and improvements

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_alu_trace.cpp

@@ -667,12 +667,37 @@ FF AvmAluTraceBuilder::op_lte(FF const& a, FF const& b, AvmMemoryTag in_tag, uin
  */
 FF AvmAluTraceBuilder::op_cast(FF const& a, AvmMemoryTag in_tag, uint32_t clk)
 {
-    uint256_t a_256{ a };
+    FF c;
+
+    switch (in_tag) {
+    case AvmMemoryTag::U8:
+        c = FF(uint8_t(a));
+        break;
+    case AvmMemoryTag::U16:
+        c = FF(uint16_t(a));
+        break;
+    case AvmMemoryTag::U32:
+        c = FF(uint32_t(a));
+        break;
+    case AvmMemoryTag::U64:
+        c = FF(uint64_t(a));
+        break;
+    case AvmMemoryTag::U128:
+        c = FF(uint256_t::from_uint128(uint128_t(a)));
+        break;
+    case AvmMemoryTag::FF:
+        c = a;
+        break;
+    default:
+        c = 0;
+        break;
+    }
+
     // Get the decomposition of a
-    auto [a_lo, a_hi] = decompose(a_256);
+    auto [a_lo, a_hi] = decompose(uint256_t(a));
     // Decomposition of p-a
-    auto [p_sub_a_lo, p_sub_a_hi, p_a_borrow] = gt_witness(FF::modulus, a_256);
-    auto [u8_r0, u8_r1, u16_reg] = to_alu_slice_registers(a_256);
+    auto [p_sub_a_lo, p_sub_a_hi, p_a_borrow] = gt_witness(FF::modulus, uint256_t(a));
+    auto [u8_r0, u8_r1, u16_reg] = to_alu_slice_registers(uint256_t(a));
 
     alu_trace.push_back(AvmAluTraceBuilder::AluTraceEntry{
         .alu_clk = clk,
@@ -684,7 +709,7 @@ FF AvmAluTraceBuilder::op_cast(FF const& a, AvmMemoryTag in_tag, uint32_t clk)
         .alu_u64_tag = in_tag == AvmMemoryTag::U64,
         .alu_u128_tag = in_tag == AvmMemoryTag::U128,
         .alu_ia = a,
-        .alu_ic = 0,
+        .alu_ic = c,
         .alu_u8_r0 = u8_r0,
         .alu_u8_r1 = u8_r1,
         .alu_u16_reg = u16_reg,
@@ -703,32 +728,6 @@ FF AvmAluTraceBuilder::op_cast(FF const& a, AvmMemoryTag in_tag, uint32_t clk)
         .hi_lo_limbs = { p_sub_a_lo, p_sub_a_hi },
     });
 
-    FF c;
-
-    switch (in_tag) {
-    case AvmMemoryTag::U8:
-        c = FF(uint8_t(a));
-        break;
-    case AvmMemoryTag::U16:
-        c = FF(uint16_t(a));
-        break;
-    case AvmMemoryTag::U32:
-        c = FF(uint32_t(a));
-        break;
-    case AvmMemoryTag::U64:
-        c = FF(uint64_t(a));
-        break;
-    case AvmMemoryTag::U128:
-        c = FF(uint256_t::from_uint128(uint128_t(a)));
-        break;
-    case AvmMemoryTag::FF:
-        c = a;
-        break;
-    default:
-        c = 0;
-        break;
-    }
-
     return c;
 }
 

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_common.hpp

@@ -1,6 +1,5 @@
 #pragma once
 
-#include "barretenberg/relations/generated/avm/avm_binary.hpp"
 #include "barretenberg/stdlib_circuit_builders/circuit_builder_base.hpp"
 #include "barretenberg/vm/generated/avm_circuit_builder.hpp"
 #include <cstdint>

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_mem_trace.cpp

@@ -179,10 +179,10 @@ void AvmMemTraceBuilder::store_in_mem_trace(
 }
 
 /**
- * @brief Handle a read memory operation without any tag check. Load the corresponding
+ * @brief Handle a read memory operation specific to MOV opcode. Load the corresponding
  *        value to the intermediate register ia. A memory trace entry for the load
  *        operation is added. It is permissive in the sense that we do not enforce tag
- *        matching with against any instruction tag. Optionally, a specific selector
+ *        matching against any instruction tag. In addition, the specific selector
  *        for MOV opcode is enabled.
  *
  * @param clk Main clock
@@ -191,9 +191,7 @@ void AvmMemTraceBuilder::store_in_mem_trace(
  * @return Result of the read operation containing the value and the tag of the memory cell
  *         at the supplied address.
  */
-AvmMemTraceBuilder::MemEntry AvmMemTraceBuilder::read_and_load_no_tag_check(uint32_t const clk,
-                                                                            uint32_t const addr,
-                                                                            bool is_mov)
+AvmMemTraceBuilder::MemEntry AvmMemTraceBuilder::read_and_load_mov_opcode(uint32_t const clk, uint32_t const addr)
 {
     MemEntry mem_entry = memory.contains(addr) ? memory.at(addr) : MemEntry{};
 
@@ -205,12 +203,27 @@ AvmMemTraceBuilder::MemEntry AvmMemTraceBuilder::read_and_load_no_tag_check(uint
         .m_tag = mem_entry.tag,
         .r_in_tag = mem_entry.tag,
         .w_in_tag = mem_entry.tag,
-        .m_sel_mov_a = is_mov,
+        .m_sel_mov_a = true,
     });
 
     return mem_entry;
 }
 
+/**
+ * @brief Handle a read memory operation specific to CMOV opcode. Load the corresponding
+ *        values to the intermediate register ia, ib, id. Three memory trace entries for
+ *        these load operations are added. They are permissive in the sense that we do not
+ *        enforce tag matching against any instruction tag. In addition, the specific selector
+ *        for CMOV opcode is enabled.
+ *
+ * @param clk Main clock
+ * @param a_addr Memory address of the first value candidate a.
+ * @param b_addr Memory address of the second value candidate b.
+ * @param cond_addr Memory address of the conditional value.
+ *
+ * @return Result of the read operation containing the value and the tag of the memory cell
+ *         at the supplied address.
+ */
 std::array<AvmMemTraceBuilder::MemEntry, 3> AvmMemTraceBuilder::read_and_load_cmov_opcode(uint32_t clk,
                                                                                           uint32_t a_addr,
                                                                                           uint32_t b_addr,
@@ -262,6 +275,39 @@ std::array<AvmMemTraceBuilder::MemEntry, 3> AvmMemTraceBuilder::read_and_load_cm
     return { a_mem_entry, b_mem_entry, cond_mem_entry };
 }
 
+/**
+ * @brief Handle a read memory operation specific to CAST opcode. Load the corresponding
+ *        value to the intermediate register ia. A memory trace entry for the load
+ *        operation is added. It is permissive in the sense that we do not enforce tag
+ *        matching against any instruction tag. The write instruction tag w_in_tag
+ *        is passed and added in the memory trace entry.
+ *
+ * @param clk Main clock
+ * @param addr Memory address of the source offset
+ * @param w_in_tag Write instruction instruction tag (tag the value is casted to)
+ *
+ * @return Result of the read operation containing the value and the tag of the memory cell
+ *         at the supplied address.
+ */
+AvmMemTraceBuilder::MemEntry AvmMemTraceBuilder::read_and_load_cast_opcode(uint32_t clk,
+                                                                           uint32_t addr,
+                                                                           AvmMemoryTag w_in_tag)
+{
+    MemEntry mem_entry = memory.contains(addr) ? memory.at(addr) : MemEntry{};
+
+    mem_trace.emplace_back(MemoryTraceEntry{
+        .m_clk = clk,
+        .m_sub_clk = SUB_CLK_LOAD_A,
+        .m_addr = addr,
+        .m_val = mem_entry.val,
+        .m_tag = mem_entry.tag,
+        .r_in_tag = mem_entry.tag,
+        .w_in_tag = w_in_tag,
+    });
+
+    return mem_entry;
+}
+
 /**
  * @brief Handle a read memory operation and load the corresponding value to the
  *        supplied intermediate register. A memory trace entry for the load operation

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_mem_trace.hpp

@@ -87,11 +87,12 @@ class AvmMemTraceBuilder {
 
     std::vector<MemoryTraceEntry> finalize();
 
-    MemEntry read_and_load_no_tag_check(uint32_t clk, uint32_t addr, bool is_mov);
+    MemEntry read_and_load_mov_opcode(uint32_t clk, uint32_t addr);
     std::array<MemEntry, 3> read_and_load_cmov_opcode(uint32_t clk,
                                                       uint32_t a_addr,
                                                       uint32_t b_addr,
                                                       uint32_t cond_addr);
+    MemEntry read_and_load_cast_opcode(uint32_t clk, uint32_t addr, AvmMemoryTag w_in_tag);
     MemRead read_and_load_from_memory(
         uint32_t clk, IntermRegister interm_reg, uint32_t addr, AvmMemoryTag r_in_tag, AvmMemoryTag w_in_tag);
     MemRead indirect_read_and_load_from_memory(uint32_t clk, IndirectRegister ind_reg, uint32_t addr);

barretenberg/cpp/src/barretenberg/vm/avm_trace/avm_trace.cpp

@@ -814,7 +814,7 @@ void AvmTraceBuilder::op_mov(uint8_t indirect, uint32_t src_offset, uint32_t dst
     }
 
     // Reading from memory and loading into ia without tag check.
-    auto const [val, tag] = mem_trace_builder.read_and_load_no_tag_check(clk, direct_src_offset, true);
+    auto const [val, tag] = mem_trace_builder.read_and_load_mov_opcode(clk, direct_src_offset);
 
     // Write into memory from intermediate register ic.
     mem_trace_builder.write_into_memory(clk, IntermRegister::IC, direct_dst_offset, val, tag, tag);
@@ -982,7 +982,7 @@ void AvmTraceBuilder::op_cast(uint8_t indirect, uint32_t a_offset, uint32_t dst_
     }
 
     // Reading from memory and loading into ia
-    auto memEntry = mem_trace_builder.read_and_load_no_tag_check(clk, direct_a_offset, false);
+    auto memEntry = mem_trace_builder.read_and_load_cast_opcode(clk, direct_a_offset, dst_tag);
     FF a = memEntry.val;
 
     // In case of a memory tag error, we do not perform the computation.
@@ -1614,7 +1614,7 @@ std::vector<Row> AvmTraceBuilder::finalize()
 
         if ((r.avm_main_sel_op_add == FF(1) || r.avm_main_sel_op_sub == FF(1) || r.avm_main_sel_op_mul == FF(1) ||
              r.avm_main_sel_op_eq == FF(1) || r.avm_main_sel_op_not == FF(1) || r.avm_main_sel_op_lt ||
-             r.avm_main_sel_op_lte) &&
+             r.avm_main_sel_op_lte || r.avm_main_sel_op_cast == FF(1)) &&
             r.avm_main_tag_err == FF(0)) {
             r.avm_main_alu_sel = FF(1);
         }

barretenberg/cpp/src/barretenberg/vm/tests/avm_cast.test.cpp

@@ -0,0 +1,32 @@
+#include "avm_common.test.hpp"
+#include "barretenberg/vm/avm_trace/avm_common.hpp"
+#include "barretenberg/vm/tests/helpers.test.hpp"
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+namespace tests_avm {
+using namespace bb::avm_trace;
+using namespace testing;
+
+class AvmCastTests : public ::testing::Test {
+  public:
+    AvmTraceBuilder trace_builder;
+
+  protected:
+    std::vector<Row> trace;
+
+    // TODO(640): The Standard Honk on Grumpkin test suite fails unless the SRS is initialised for every test.
+    void SetUp() override { srs::init_crs_factory("../srs_db/ignition"); };
+};
+
+TEST_F(AvmCastTests, basicU8ToU16)
+{
+    trace_builder.op_set(0, 237, 0, AvmMemoryTag::U8);
+    trace_builder.op_cast(0, 0, 1, AvmMemoryTag::U16);
+    trace_builder.return_op(0, 0, 0);
+    auto trace = trace_builder.finalize();
+
+    validate_trace_check_circuit(std::move(trace));
+}
+
+} // namespace tests_avm