fix: preserve empty items when merging checkpoints (#19288)

The L1 contract processes fees in the array one by one, assuming each
entry corresponds to the checkpoint at the same position, and uses the
index to get the fee header. But the circuits allow zero fee and
recipient. So when merging fees from two checkpoints' public inputs,
their indexes need to be preserved, we can't skip the empty items.

Author: LeilaWang

noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_merge/tests/consecutive_checkpoint_rollups_tests.nr

@@ -1,4 +1,8 @@
 use super::TestBuilder;
+use types::{
+    abis::fee_recipient::FeeRecipient, address::EthAddress,
+    constants::CHECKPOINT_MERGE_ROLLUP_VK_INDEX, tests::utils::pad_end, traits::Empty,
+};
 
 #[test]
 fn correct_default_inputs() {
@@ -7,6 +11,102 @@ fn correct_default_inputs() {
     builder.assert_expected_public_inputs(pi);
 }
 
+// --- fees ---
+
+fn mock_fee(recipient: Field, value: Field) -> FeeRecipient {
+    FeeRecipient { recipient: EthAddress::from_field(recipient), value }
+}
+
+#[test]
+fn accumulated_fees_from_both_side() {
+    let mut builder = TestBuilder::new(
+        CHECKPOINT_MERGE_ROLLUP_VK_INDEX,
+        2,
+        CHECKPOINT_MERGE_ROLLUP_VK_INDEX,
+        2,
+    );
+
+    let fees = [mock_fee(11, 23), mock_fee(456, 7), mock_fee(8, 90), mock_fee(1234, 56789)];
+
+    builder.left_rollup.fees[0] = fees[0];
+    builder.left_rollup.fees[1] = fees[1];
+    builder.right_rollup.fees[0] = fees[2];
+    builder.right_rollup.fees[1] = fees[3];
+
+    let pi = builder.execute();
+    builder.assert_expected_public_inputs(pi);
+
+    assert_eq(pi.fees, pad_end(fees));
+}
+
+#[test]
+fn preserve_empty_fees_from_left() {
+    let mut builder = TestBuilder::new(
+        CHECKPOINT_MERGE_ROLLUP_VK_INDEX,
+        2,
+        CHECKPOINT_MERGE_ROLLUP_VK_INDEX,
+        2,
+    );
+
+    let fees = [mock_fee(0, 0), mock_fee(11, 23), mock_fee(456, 7), mock_fee(8, 90)];
+
+    builder.left_rollup.fees[0] = FeeRecipient::empty();
+    builder.left_rollup.fees[1] = fees[1];
+    builder.right_rollup.fees[0] = fees[2];
+    builder.right_rollup.fees[1] = fees[3];
+
+    let pi = builder.execute();
+    builder.assert_expected_public_inputs(pi);
+
+    assert_eq(pi.fees, pad_end(fees));
+}
+
+#[test]
+fn preserve_empty_fees_from_right() {
+    let mut builder = TestBuilder::new(
+        CHECKPOINT_MERGE_ROLLUP_VK_INDEX,
+        2,
+        CHECKPOINT_MERGE_ROLLUP_VK_INDEX,
+        2,
+    );
+
+    let fees = [mock_fee(11, 23), mock_fee(456, 7), mock_fee(0, 0), mock_fee(8, 90)];
+
+    builder.left_rollup.fees[0] = fees[0];
+    builder.left_rollup.fees[1] = fees[1];
+    builder.right_rollup.fees[0] = FeeRecipient::empty();
+    builder.right_rollup.fees[1] = fees[3];
+
+    let pi = builder.execute();
+    builder.assert_expected_public_inputs(pi);
+
+    assert_eq(pi.fees, pad_end(fees));
+}
+
+#[test]
+fn preserve_empty_fees_from_both_sides() {
+    let mut builder = TestBuilder::new(
+        CHECKPOINT_MERGE_ROLLUP_VK_INDEX,
+        2,
+        CHECKPOINT_MERGE_ROLLUP_VK_INDEX,
+        2,
+    );
+
+    let fees = [mock_fee(11, 23), mock_fee(0, 0), mock_fee(0, 0), mock_fee(456, 7)];
+
+    builder.left_rollup.fees[0] = fees[0];
+    builder.left_rollup.fees[1] = FeeRecipient::empty();
+    builder.right_rollup.fees[0] = FeeRecipient::empty();
+    builder.right_rollup.fees[1] = fees[3];
+
+    let pi = builder.execute();
+    builder.assert_expected_public_inputs(pi);
+
+    assert_eq(pi.fees, pad_end(fees));
+}
+
+// -- constants ---
+
 #[test(should_fail_with = "Mismatched constants in checkpoint rollups")]
 fn mismatch_chain_id() {
     let mut builder = TestBuilder::default();
@@ -69,6 +169,8 @@ fn mismatch_prover_id() {
     builder.execute_and_fail();
 }
 
+// --- archives ---
+
 #[test(should_fail_with = "Mismatched archives: expected right.previous_archive to match left.new_archive")]
 fn non_consecutive_archive_roots() {
     let mut builder = TestBuilder::default();
@@ -89,6 +191,8 @@ fn non_consecutive_archive_next_available_leaf_indices() {
     builder.execute_and_fail();
 }
 
+// --- blob accumulators ---
+
 #[test(should_fail_with = "Mismatched blob accumulators: expected right.start_blob_accumulator to match left.end_blob_accumulator")]
 fn non_consecutive_blob_accumulators() {
     let mut builder = TestBuilder::default();

noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_merge/tests/mod.nr

@@ -109,14 +109,12 @@ impl TestBuilder {
             expected_header_hashes[i] = left.checkpoint_header_hashes[i];
             expected_fees[i] = left.fees[i];
             assert(left.checkpoint_header_hashes[i] != 0);
-            assert(!left.fees[i].is_empty());
         }
         let offset = self.num_left_checkpoints as u32;
         for i in 0..self.num_right_checkpoints as u32 {
             expected_header_hashes[i + offset] = right.checkpoint_header_hashes[i];
             expected_fees[i + offset] = right.fees[i];
             assert(right.checkpoint_header_hashes[i] != 0);
-            assert(!right.fees[i].is_empty());
         }
         assert_eq(pi.checkpoint_header_hashes, expected_header_hashes);
         assert_eq(pi.fees, expected_fees);

noir-projects/noir-protocol-circuits/crates/rollup-lib/src/checkpoint_merge/utils/merge_checkpoint_rollups.nr

@@ -1,25 +1,43 @@
 use crate::abis::CheckpointRollupPublicInputs;
-use types::{constants::AZTEC_MAX_EPOCH_DURATION, utils::arrays::array_merge};
+use types::{constants::AZTEC_MAX_EPOCH_DURATION, utils::arrays::copy_items_into_array};
 
 pub fn merge_checkpoint_rollups(
     left: CheckpointRollupPublicInputs,
     right: CheckpointRollupPublicInputs,
 ) -> CheckpointRollupPublicInputs {
+    let num_left_checkpoints = left.num_checkpoints() as u32;
+    let num_right_checkpoints = right.num_checkpoints() as u32;
+
     // Make sure that the total number of checkpoints does not exceed the maximum allowed in an epoch, preventing the
     // merged arrays (`checkpoint_header_hashes`, `fees`) from being truncated.
-    let num_checkpoints = left.num_checkpoints() + right.num_checkpoints();
+    let num_checkpoints = num_left_checkpoints + num_right_checkpoints;
     assert(
-        num_checkpoints <= AZTEC_MAX_EPOCH_DURATION as u16,
+        num_checkpoints <= AZTEC_MAX_EPOCH_DURATION,
         "total number of checkpoints exceeds max allowed in an epoch",
     );
 
-    let checkpoint_header_hashes = array_merge(
+    // We use `copy_items_into_array` below because we know exactly how many items should be taken from each side when
+    // merging checkpoints.
+    // It is especially critical for `fees` to copy the exact amount of items from both checkpoints. Because the L1
+    // contracts will process fees by index, assuming each entry corresponds to the checkpoint at the same position.
+    // However, the existence of zero fees or recipients is not prohibited by the circuit. Therefore, preserving empty
+    // values is important.
+    // Using this helper (instead of helper like `array_merge`) ensures those empty entries are copied over.
+
+    let checkpoint_header_hashes = copy_items_into_array(
         left.checkpoint_header_hashes,
+        num_left_checkpoints,
         right.checkpoint_header_hashes,
+        num_right_checkpoints,
     );
 
     // We can't merge fees for the same recipient since the l1 contract iterates per checkpoint.
-    let fees = array_merge(left.fees, right.fees);
+    let fees = copy_items_into_array(
+        left.fees,
+        num_left_checkpoints,
+        right.fees,
+        num_right_checkpoints,
+    );
 
     CheckpointRollupPublicInputs {
         constants: left.constants,

noir-projects/noir-protocol-circuits/crates/types/src/utils/arrays.nr

@@ -1,9 +1,11 @@
 pub(crate) mod assert_trailing_zeros;
+pub(crate) mod copy_items_into_array;
 pub(crate) mod find_index;
 pub(crate) mod get_sorted_tuples;
 
 // Re-exports.
 pub use assert_trailing_zeros::assert_trailing_zeros;
+pub use copy_items_into_array::copy_items_into_array;
 pub use find_index::{find_first_index, find_last_index};
 pub use get_sorted_tuples::{get_sorted_tuples, SortedTuple};