fix(avm)!: handle zero copy size (#16567)

Handles the situation when `copy_size = 0` for `cd_copy` or `rd_copy`.
This is not an error but effectively a no-op.

Adds tests for this as well

Author: IlyasRidhuan

barretenberg/cpp/pil/vm2/data_copy.pil

@@ -118,9 +118,6 @@ namespace data_copy;
     pol commit sel_rd_copy;
     sel_rd_copy * (1 - sel_rd_copy) = 0;
 
-    // Two varieties depending of if we gate by error
-    pol SEL_NO_ERR = SEL * (1 - err);
-
     pol commit clk;
 
     // Things are range checked to 32 bits
@@ -226,11 +223,21 @@ namespace data_copy;
     #[START_AFTER_END]
     (sel_cd_copy' + sel_rd_copy') * sel_end * (sel_start' - 1) = 0;
 
+    pol commit sel_write_count_is_zero;
+    pol commit write_count_zero_inv; // Could optimise by using the existing write_count_minus_on_inv
+    // sel_write_count_is_zero = 1 IFF copy_size = 0 && sel_start = 1 (and there are no errors)
+    #[ZERO_SIZED_WRITE]
+    sel_start_no_err * (copy_size * (sel_write_count_is_zero * (1 - write_count_zero_inv) + write_count_zero_inv) - 1 + sel_write_count_is_zero) = 0;
+    #[END_IF_WRITE_IS_ZERO]
+    sel_start_no_err * sel_write_count_is_zero * (sel_end - 1) = 0;
+
+    pol SEL_PERFORM_COPY = SEL * (1 - err) * (1 - sel_write_count_is_zero);
+
     pol WRITE_COUNT_MINUS_ONE = copy_size - 1;
     pol commit write_count_minus_one_inv;
     // sel_end = 1 IFF copy_size - 1 = 0;
     #[END_WRITE_CONDITION]
-    SEL_NO_ERR * (WRITE_COUNT_MINUS_ONE * (sel_end * (1 - write_count_minus_one_inv) + write_count_minus_one_inv) - 1 + sel_end) = 0;
+    SEL_PERFORM_COPY * (WRITE_COUNT_MINUS_ONE * (sel_end * (1 - write_count_minus_one_inv) + write_count_minus_one_inv) - 1 + sel_end) = 0;
 
     #[END_ON_ERR] // sel_end = 1 if error
     err * (sel_end - 1) = 0;
@@ -248,15 +255,15 @@ namespace data_copy;
     // If sel_offset_gt_max_read = 1 (i.e. when offset > MAX_READ_INDEX, reads_left = 0)
     // otherwise, reads_left = MAX_READ_INDEX - offset
     #[INIT_READS_LEFT]
-    sel_start_no_err * ( reads_left - (max_read_index - offset) * (1 - offset_gt_max_read_index)) = 0;
+    sel_start_no_err * (1 - sel_write_count_is_zero) * (reads_left - (max_read_index - offset) * (1 - offset_gt_max_read_index)) = 0;
 
     //////////////////////////////
     // Execute Data Copy
     //////////////////////////////
     // Most of these relations are either gated explicitly by an err or by sel_end (which is 1 when err = 1)
     // ===== Writing to dst_context_id =====
     pol commit sel_mem_write;
-    sel_mem_write = SEL_NO_ERR; // We write if there is no error
+    sel_mem_write = SEL_PERFORM_COPY; // We write if there is no error and copy_size != 0
     // Data copy size decrements for each row until we end
     #[DECR_COPY_SIZE]
     SEL * (1 - sel_end) * (copy_size' - copy_size + 1) = 0;
@@ -271,8 +278,8 @@ namespace data_copy;
 
     // ===== Reading for nested call =====
     pol commit read_addr;  // The addr to start reading the data from: src_addr + offset;
-    #[INIT_READ_ADDR]
-    SEL * sel_start_no_err * (read_addr - src_addr - offset) = 0;
+    #[INIT_READ_ADDR] // Only occurs at the start if we have not errored
+    sel_start_no_err * (1 - sel_write_count_is_zero) * (read_addr - src_addr - offset) = 0;
     // Subsequent read addrs are incremented by 1 unless this is a padding row
     #[INCR_READ_ADDR]
     SEL * (1 - padding) * (1 - sel_end) * (read_addr' - read_addr - 1) = 0;
@@ -283,16 +290,16 @@ namespace data_copy;
     pol commit padding; // Padding = 1 if reads_left = 0
     pol commit reads_left_inv;
     #[PADDING_CONDITION]
-    SEL_NO_ERR * (reads_left * (padding * (1 - reads_left_inv) + reads_left_inv) - 1 + padding) = 0;
+    SEL_PERFORM_COPY * (reads_left * (padding * (1 - reads_left_inv) + reads_left_inv) - 1 + padding) = 0;
 
     // Read from memory if we are not the top level call and not a padding row
     pol commit sel_mem_read; // If the current row is a memory op read
-    sel_mem_read = SEL_NO_ERR * (1 - is_top_level) * (1 - padding);
+    sel_mem_read = SEL_PERFORM_COPY * (1 - is_top_level) * (1 - padding);
 
     // === Value Padding ===
     pol commit value;
     #[PAD_VALUE]
-    SEL_NO_ERR * padding * value = 0;
+    SEL_PERFORM_COPY * padding * value = 0;
 
     #[MEM_READ]
     sel_mem_read { clk, read_addr, value, /*mem_tag=*/precomputed.zero/*FF*/, /*rw=*/precomputed.zero/*(read)*/, src_context_id }
@@ -305,7 +312,7 @@ namespace data_copy;
     // After calldata hashing
     pol commit cd_copy_col_read;
     #[CD_COPY_COLUMN]
-    cd_copy_col_read = SEL_NO_ERR * (1 - padding) * is_top_level * sel_cd_copy;
+    cd_copy_col_read = SEL_PERFORM_COPY * (1 - padding) * is_top_level * sel_cd_copy;
 
     #[COL_READ]
     cd_copy_col_read { value, dst_context_id, read_addr }
@@ -357,3 +364,4 @@ namespace data_copy;
         src_addr, src_data_size,
         sel_rd_copy, err
     };
+

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

@@ -74,6 +74,29 @@ class NestedCdConstrainingBuilderTest : public DataCopyConstrainingBuilderTest {
     }
 };
 
+TEST_F(NestedCdConstrainingBuilderTest, CdZeroCopy)
+{
+    uint32_t copy_size = 0;
+    uint32_t cd_offset = 0; // Offset into calldata
+
+    EXPECT_CALL(context, get_calldata(cd_offset, copy_size)).WillOnce(::testing::Return(std::vector<FF>{}));
+
+    copy_data.cd_copy(context, copy_size, cd_offset, dst_addr);
+
+    tracegen::DataCopyTraceBuilder builder;
+    builder.process(event_emitter.dump_events(), trace);
+
+    tracegen::GreaterThanTraceBuilder gt_builder;
+    gt_builder.process(gt_event_emitter.dump_events(), trace);
+
+    check_relation<data_copy>(trace);
+    check_interaction<DataCopyTraceBuilder,
+                      lookup_data_copy_max_read_index_gt_settings,
+                      lookup_data_copy_offset_gt_max_read_index_settings,
+                      lookup_data_copy_check_src_addr_in_range_settings,
+                      lookup_data_copy_check_dst_addr_in_range_settings>(trace);
+}
+
 TEST_F(NestedCdConstrainingBuilderTest, SimpleNestedCdCopy)
 {
     uint32_t copy_size = static_cast<uint32_t>(data.size());
@@ -194,6 +217,29 @@ class EnqueuedCdConstrainingBuilderTest : public DataCopyConstrainingBuilderTest
     }
 };
 
+TEST_F(EnqueuedCdConstrainingBuilderTest, CdZeroCopy)
+{
+    uint32_t copy_size = 0;
+    uint32_t cd_offset = 0; // Offset into calldata
+
+    EXPECT_CALL(context, get_calldata(cd_offset, copy_size)).WillOnce(::testing::Return(std::vector<FF>{}));
+
+    copy_data.cd_copy(context, copy_size, cd_offset, dst_addr);
+
+    tracegen::DataCopyTraceBuilder builder;
+    builder.process(event_emitter.dump_events(), trace);
+
+    tracegen::GreaterThanTraceBuilder gt_builder;
+    gt_builder.process(gt_event_emitter.dump_events(), trace);
+
+    check_relation<data_copy>(trace);
+    check_interaction<DataCopyTraceBuilder,
+                      lookup_data_copy_max_read_index_gt_settings,
+                      lookup_data_copy_offset_gt_max_read_index_settings,
+                      lookup_data_copy_check_src_addr_in_range_settings,
+                      lookup_data_copy_check_dst_addr_in_range_settings>(trace);
+}
+
 TEST_F(EnqueuedCdConstrainingBuilderTest, SimpleEnqueuedCdCopy)
 {
     auto copy_size = static_cast<uint32_t>(data.size());
@@ -343,6 +389,42 @@ TEST(DataCopyWithExecutionPerm, CdCopy)
                       perm_data_copy_dispatch_rd_copy_settings>(trace);
 }
 
+class NestedRdConstrainingBuilderTest : public DataCopyConstrainingBuilderTest {
+  protected:
+    NestedRdConstrainingBuilderTest()
+    {
+        // Set up parent context
+        EXPECT_CALL(context, has_parent).WillRepeatedly(Return(true));
+        EXPECT_CALL(context, get_last_child_id).WillRepeatedly(Return(2));
+        EXPECT_CALL(context, get_context_id).WillRepeatedly(Return(2));
+        EXPECT_CALL(context, get_last_rd_size).WillRepeatedly(Return(data.size()));
+        EXPECT_CALL(context, get_last_rd_addr).WillRepeatedly(Return(0));
+    }
+};
+
+TEST_F(NestedRdConstrainingBuilderTest, RdZeroCopy)
+{
+    uint32_t copy_size = 0;
+    uint32_t rd_offset = 0; // Offset into calldata
+
+    EXPECT_CALL(context, get_returndata(rd_offset, copy_size)).WillOnce(::testing::Return(std::vector<FF>{}));
+
+    copy_data.rd_copy(context, copy_size, rd_offset, dst_addr);
+
+    tracegen::DataCopyTraceBuilder builder;
+    builder.process(event_emitter.dump_events(), trace);
+
+    tracegen::GreaterThanTraceBuilder gt_builder;
+    gt_builder.process(gt_event_emitter.dump_events(), trace);
+
+    check_relation<data_copy>(trace);
+    check_interaction<DataCopyTraceBuilder,
+                      lookup_data_copy_max_read_index_gt_settings,
+                      lookup_data_copy_offset_gt_max_read_index_settings,
+                      lookup_data_copy_check_src_addr_in_range_settings,
+                      lookup_data_copy_check_dst_addr_in_range_settings>(trace);
+}
+
 TEST(DataCopyWithExecutionPerm, RdCopy)
 {
     // Current Context

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

@@ -133,10 +133,10 @@ namespace bb::avm2 {
 
 struct AvmFlavorVariables {
     static constexpr size_t NUM_PRECOMPUTED_ENTITIES = 133;
-    static constexpr size_t NUM_WITNESS_ENTITIES = 2934;
+    static constexpr size_t NUM_WITNESS_ENTITIES = 2936;
     static constexpr size_t NUM_SHIFTED_ENTITIES = 316;
     static constexpr size_t NUM_WIRES = NUM_WITNESS_ENTITIES + NUM_PRECOMPUTED_ENTITIES;
-    static constexpr size_t NUM_ALL_ENTITIES = 3383;
+    static constexpr size_t NUM_ALL_ENTITIES = 3385;
 
     // Need to be templated for recursive verifier
     template <typename FF_>

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

@@ -14,9 +14,9 @@ template <typename FF_> class data_copyImpl {
   public:
     using FF = FF_;
 
-    static constexpr std::array<size_t, 33> SUBRELATION_PARTIAL_LENGTHS = { 3, 3, 3, 3, 3, 3, 5, 3, 4, 3, 3,
-                                                                            3, 3, 3, 3, 3, 3, 4, 6, 3, 4, 3,
-                                                                            4, 5, 4, 5, 5, 6, 5, 5, 6, 3, 3 };
+    static constexpr std::array<size_t, 35> SUBRELATION_PARTIAL_LENGTHS = { 3, 3, 3, 3, 3, 3, 5, 3, 4, 3, 3, 3,
+                                                                            3, 3, 3, 3, 3, 4, 5, 4, 7, 3, 5, 4,
+                                                                            4, 5, 4, 5, 5, 7, 6, 6, 7, 3, 3 };
 
     template <typename AllEntities> inline static bool skip(const AllEntities& in)
     {
@@ -46,26 +46,30 @@ template <typename FF> class data_copy : public Relation<data_copyImpl<FF>> {
         case 17:
             return "START_AFTER_END";
         case 18:
-            return "END_WRITE_CONDITION";
+            return "ZERO_SIZED_WRITE";
         case 19:
-            return "END_ON_ERR";
+            return "END_IF_WRITE_IS_ZERO";
         case 20:
-            return "INIT_READS_LEFT";
+            return "END_WRITE_CONDITION";
+        case 21:
+            return "END_ON_ERR";
         case 22:
+            return "INIT_READS_LEFT";
+        case 24:
             return "DECR_COPY_SIZE";
-        case 23:
+        case 25:
             return "INCR_WRITE_ADDR";
-        case 24:
+        case 26:
             return "INIT_READ_ADDR";
-        case 25:
+        case 27:
             return "INCR_READ_ADDR";
-        case 26:
+        case 28:
             return "DECR_READ_COUNT";
-        case 27:
-            return "PADDING_CONDITION";
         case 29:
+            return "PADDING_CONDITION";
+        case 31:
             return "PAD_VALUE";
-        case 30:
+        case 32:
             return "CD_COPY_COLUMN";
         }
         return std::to_string(index);
@@ -74,17 +78,19 @@ template <typename FF> class data_copy : public Relation<data_copyImpl<FF>> {
     // Subrelation indices constants, to be used in tests.
     static constexpr size_t SR_TOP_LEVEL_COND = 6;
     static constexpr size_t SR_START_AFTER_END = 17;
-    static constexpr size_t SR_END_WRITE_CONDITION = 18;
-    static constexpr size_t SR_END_ON_ERR = 19;
-    static constexpr size_t SR_INIT_READS_LEFT = 20;
-    static constexpr size_t SR_DECR_COPY_SIZE = 22;
-    static constexpr size_t SR_INCR_WRITE_ADDR = 23;
-    static constexpr size_t SR_INIT_READ_ADDR = 24;
-    static constexpr size_t SR_INCR_READ_ADDR = 25;
-    static constexpr size_t SR_DECR_READ_COUNT = 26;
-    static constexpr size_t SR_PADDING_CONDITION = 27;
-    static constexpr size_t SR_PAD_VALUE = 29;
-    static constexpr size_t SR_CD_COPY_COLUMN = 30;
+    static constexpr size_t SR_ZERO_SIZED_WRITE = 18;
+    static constexpr size_t SR_END_IF_WRITE_IS_ZERO = 19;
+    static constexpr size_t SR_END_WRITE_CONDITION = 20;
+    static constexpr size_t SR_END_ON_ERR = 21;
+    static constexpr size_t SR_INIT_READS_LEFT = 22;
+    static constexpr size_t SR_DECR_COPY_SIZE = 24;
+    static constexpr size_t SR_INCR_WRITE_ADDR = 25;
+    static constexpr size_t SR_INIT_READ_ADDR = 26;
+    static constexpr size_t SR_INCR_READ_ADDR = 27;
+    static constexpr size_t SR_DECR_READ_COUNT = 28;
+    static constexpr size_t SR_PADDING_CONDITION = 29;
+    static constexpr size_t SR_PAD_VALUE = 31;
+    static constexpr size_t SR_CD_COPY_COLUMN = 32;
 };
 
 } // namespace bb::avm2