fix: preserve empty items when merging checkpoints

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};