fix remote_nonces and add tests for two directions

src/fiber/channel.rs

@@ -2626,6 +2626,7 @@ pub struct ChannelActorState {
     #[serde_as(as = "Option<PubNonceAsBytes>")]
     pub last_used_nonce_in_commitment_signed: Option<PubNonce>,
 
+    // The nonces that are sent by the counterparty, the length is at most 2
     #[serde_as(as = "Vec<(U64Hex, PubNonceAsBytes)>")]
     pub remote_nonces: Vec<(u64, PubNonce)>,
 
@@ -2635,6 +2636,7 @@ pub struct ChannelActorState {
 
     // All the commitment point that are sent from the counterparty.
     // We need to save all these points to derive the keys for the commitment transactions.
+    // The length of this vector is at most the maximum number of flighting tlcs.
     pub remote_commitment_points: Vec<(u64, Pubkey)>,
     pub remote_channel_public_keys: Option<ChannelBasePublicKeys>,
 
@@ -3912,6 +3914,7 @@ impl ChannelActorState {
             "Getting remote nonce: commitment number {}, current nonces: {:?}",
             comitment_number, &self.remote_nonces
         );
+        assert!(self.remote_nonces.len() <= 2);
         self.remote_nonces
             .iter()
             .rev()
@@ -3922,27 +3925,22 @@ impl ChannelActorState {
                     None
                 }
             })
-            .unwrap_or_else(|| {
-                self.remote_nonces
-                    .last()
-                    .expect("expect remote nonce")
-                    .1
-                    .clone()
-            })
+            .expect("get_remote_nonce")
     }
 
     fn save_remote_nonce(&mut self, nonce: PubNonce) {
         debug!(
             "Saving remote nonce: new nonce {:?}, current nonces {:?}, commitment numbers {:?}",
             &nonce, &self.remote_nonces, self.commitment_numbers
         );
-        self.remote_nonces
-            .push((self.get_remote_commitment_number() + 1, nonce));
-        loop {
-            let len = self.remote_nonces.len();
-            if len <= 2 {
-                break;
-            }
+
+        let next_remote_number = if self.remote_nonces.is_empty() {
+            0
+        } else {
+            self.get_remote_commitment_number() + 1
+        };
+        self.remote_nonces.push((next_remote_number, nonce));
+        if self.remote_nonces.len() > 2 {
             self.remote_nonces.remove(0);
         }
     }
@@ -5253,7 +5251,7 @@ impl ChannelActorState {
             .push((self.get_local_commitment_number(), commitment_point));
 
         let len = self.remote_commitment_points.len();
-        if len > 5 {
+        if len > (self.max_tlc_number_in_flight + 1) as usize {
             let min_remote_commitment = self
                 .tlc_state
                 .all_tlcs()
@@ -5263,6 +5261,9 @@ impl ChannelActorState {
             self.remote_commitment_points
                 .retain(|(num, _)| *num >= min_remote_commitment);
         }
+        assert!(
+            self.remote_commitment_points.len() <= (self.max_tlc_number_in_flight + 1) as usize
+        );
     }
 
     fn handle_revoke_and_ack_message(

src/fiber/tests/channel.rs

@@ -33,6 +33,7 @@ use ckb_types::{
     prelude::{AsTransactionBuilder, Builder, Entity, IntoTransactionView, Pack, Unpack},
 };
 use ractor::call;
+use rand::Rng;
 use secp256k1::Secp256k1;
 use std::collections::HashSet;
 
@@ -2146,7 +2147,7 @@ async fn do_test_add_tlc_waiting_ack() {
 #[tokio::test]
 async fn do_test_add_tlc_number_limit() {
     let node_a_funding_amount = 100000000000;
-    let node_b_funding_amount = 6200000000;
+    let node_b_funding_amount = 100000000000;
 
     let [mut node_a, mut node_b] = NetworkNode::new_n_interconnected_nodes().await;
 
@@ -2163,7 +2164,10 @@ async fn do_test_add_tlc_number_limit() {
     .await;
 
     let tlc_amount = 1000000000;
+    let rand_in_100 = rand::thread_rng().gen_range(1..=100);
 
+    // both tlc sent from a -> b or b -> a will be charged as tlc numbers
+    // TODO: we should consider the tlc number limit for both direction
     for i in 1..=max_tlc_number + 1 {
         std::thread::sleep(std::time::Duration::from_millis(400));
         let add_tlc_command = AddTlcCommand {
@@ -2174,15 +2178,16 @@ async fn do_test_add_tlc_number_limit() {
             onion_packet: None,
             previous_tlc: None,
         };
-        let add_tlc_result = call!(node_a.network_actor, |rpc_reply| {
+        let source_node = if rand_in_100 > 50 { &node_a } else { &node_b };
+        let add_tlc_result = call!(source_node.network_actor, |rpc_reply| {
             NetworkActorMessage::Command(NetworkActorCommand::ControlFiberChannel(
                 ChannelCommandWithId {
                     channel_id: new_channel_id,
                     command: ChannelCommand::AddTlc(add_tlc_command, rpc_reply),
                 },
             ))
         })
-        .expect("node_b alive");
+        .expect("source node alive");
         tokio::time::sleep(tokio::time::Duration::from_millis(300)).await;
         if i == max_tlc_number + 1 {
             assert!(add_tlc_result.is_err());