fix self payment route issue

src/fiber/channel.rs

@@ -548,6 +548,7 @@ where
                         "begin to remove tlc from previous channel: {:?}",
                         &previous_tlc
                     );
+                    assert!(previous_channel_id != state.get_id());
                     let (send, recv) = oneshot::channel::<Result<(), String>>();
                     let port = RpcReplyPort::from(send);
                     self.network
@@ -773,10 +774,8 @@ where
             } else if let Some(preimage) = self.store.get_invoice_preimage(&tlc.payment_hash) {
                 preimage
             } else {
-                error!(
-                    "No preimage found for payment hash: {:?}",
-                    &tlc.payment_hash
-                );
+                // here maybe the tlc is not the last hop, we can not settle down it now.
+                // maybe we should exclude it from the settle down list.
                 continue;
             };
             let command = RemoveTlcCommand {
@@ -935,12 +934,9 @@ where
                 ),
             ))
             .expect(ASSUME_NETWORK_ACTOR_ALIVE);
-        let res = match recv.await.expect("expect command replied") {
-            Ok(tlc_id) => Ok(tlc_id),
-            Err(e) => Err(e),
-        };
+
         // If we failed to forward the onion packet, we should remove the tlc.
-        if let Err(res) = res {
+        if let Err(res) = recv.await.expect("expect command replied") {
             error!("Error forwarding onion packet: {:?}", res);
             let (send, recv) = oneshot::channel::<Result<(), String>>();
             let port = RpcReplyPort::from(send);
@@ -3848,7 +3844,6 @@ impl ChannelActorState {
     // will have the second pubkey.
     // This tlc must have valid local_committed_at and remote_committed_at fields.
     pub fn get_tlc_pubkeys(&self, tlc: &DetailedTLCInfo, local: bool) -> (Pubkey, Pubkey) {
-        debug!("Getting tlc pubkeys for tlc: {:?}", tlc);
         let is_offered = tlc.tlc.is_offered();
         let CommitmentNumbers {
             local: local_commitment_number,
@@ -3896,7 +3891,6 @@ impl ChannelActorState {
 
     fn get_active_htlcs(&self, local: bool) -> Vec<u8> {
         // Build a sorted array of TLC so that both party can generate the same commitment transaction.
-        debug!("All tlcs: {:?}", self.tlcs);
         let tlcs = {
             let (mut received_tlcs, mut offered_tlcs) = (
                 self.get_active_received_tlc_with_pubkeys(local)

src/fiber/graph.rs

@@ -12,7 +12,7 @@ use ckb_jsonrpc_types::JsonBytes;
 use ckb_types::packed::{OutPoint, Script};
 use serde::{Deserialize, Serialize};
 use serde_with::serde_as;
-use std::collections::HashMap;
+use std::collections::{HashMap, HashSet};
 use tentacle::secio::PeerId;
 use thiserror::Error;
 use tracing::log::error;
@@ -646,9 +646,11 @@ where
             incoming_htlc_expiry: 0,
         });
         let route_to_self = source == target;
+        let mut last_hop_channels = HashSet::new();
         while let Some(cur_hop) = nodes_heap.pop() {
             nodes_visited += 1;
 
+            let mut cur_hop_channels = HashSet::new();
             for (from, channel_info, channel_update) in self.get_node_inbounds(cur_hop.node_id) {
                 if from == target && !route_to_self {
                     continue;
@@ -658,6 +660,12 @@ where
                     continue;
                 }
 
+                // if the channel is already visited in the last hop, skip it
+                if last_hop_channels.contains(&channel_info.out_point()) {
+                    error!("debug skip the same channel {:?}", channel_info.out_point());
+                    continue;
+                }
+
                 edges_expanded += 1;
 
                 let fee_rate = channel_update.fee_rate;
@@ -716,7 +724,6 @@ where
                     debug!("probability is too low: {:?}", probability);
                     continue;
                 }
-                debug!("probability: {:?}", probability);
                 let agg_weight =
                     self.edge_weight(amount_to_send, fee, channel_update.htlc_expiry_delta);
                 let weight = cur_hop.weight + agg_weight;
@@ -727,7 +734,7 @@ where
                         continue;
                     }
                 }
-                let node: NodeHeapElement = NodeHeapElement {
+                let node = NodeHeapElement {
                     node_id: from,
                     weight,
                     distance,
@@ -737,9 +744,11 @@ where
                     probability,
                     next_hop: Some((cur_hop.node_id, channel_info.out_point())),
                 };
+                cur_hop_channels.insert(channel_info.out_point());
                 distances.insert(node.node_id, node.clone());
                 nodes_heap.push_or_fix(node);
             }
+            last_hop_channels = cur_hop_channels.clone();
         }
 
         let mut current = source_node.node_id;
@@ -760,10 +769,11 @@ where
         }
 
         info!(
-            "get_route: nodes visited: {}, edges expanded: {}, time: {:?}",
+            "get_route: nodes visited: {}, edges expanded: {}, time: {:?}, result: {:?}",
             nodes_visited,
             edges_expanded,
-            started_time.elapsed()
+            started_time.elapsed(),
+            result
         );
         if result.is_empty() || current != target {
             return Err(GraphError::PathFind("no path found".to_string()));

src/fiber/network.rs

@@ -597,12 +597,6 @@ pub struct FiberMessageWithSessionId {
     pub message: FiberMessage,
 }
 
-#[derive(Debug)]
-pub struct FiberMessageWithChannelId {
-    pub channel_id: Hash256,
-    pub message: FiberMessage,
-}
-
 pub struct NetworkActor<S> {
     // An event emitter to notify ourside observers.
     event_sender: mpsc::Sender<NetworkServiceEvent>,
@@ -2186,7 +2180,7 @@ where
                 return unknown_next_peer(reply);
             }
             Err(err) => {
-                // must be some error fron tentacle, set it as temporary node failure
+                // must be some error from tentacle, set it as temporary node failure
                 error!(
                     "Failed to send onion packet to channel: {:?} with err: {:?}",
                     channel_id, err

src/fiber/tests/graph.rs

@@ -811,6 +811,36 @@ fn test_graph_payment_pay_single_path() {
     network.build_route_with_expect(&payment_data, vec![2, 4, 5, 6]);
 }
 
+#[test]
+fn test_graph_payment_pay_self_with_one_node() {
+    let mut network = MockNetworkGraph::new(9);
+    network.add_edge(0, 2, Some(500), Some(2));
+    network.add_edge(2, 0, Some(500), Some(2));
+
+    let node0 = network.keys[0];
+
+    // node0 is the source node
+    let command = SendPaymentCommand {
+        target_pubkey: Some(network.keys[0].into()),
+        amount: Some(100),
+        payment_hash: Some(Hash256::default()),
+        final_htlc_expiry_delta: Some(100),
+        invoice: None,
+        timeout: Some(10),
+        max_fee_amount: Some(1000),
+        max_parts: None,
+        keysend: Some(false),
+        udt_type_script: None,
+        allow_self_payment: true,
+    };
+    let payment_data = SendPaymentData::new(command, node0.into());
+    assert!(payment_data.is_ok());
+    let payment_data = payment_data.unwrap();
+
+    let route = network.graph.build_route(&payment_data);
+    assert!(route.is_ok());
+}
+
 #[test]
 fn test_graph_payment_pay_self_will_ok() {
     let mut network = MockNetworkGraph::new(9);

tests/bruno/e2e/router-pay/11-node2-list-channels.bru

@@ -34,5 +34,5 @@ assert {
 
 script:post-response {
   await new Promise(r => setTimeout(r, 5000));
-  console.log("list channels: ", res.body.result.channels[0]);
+  console.log("step 11 list channels: ", res.body.result.channels);
 }

tests/bruno/e2e/router-pay/11-node2-list-graph-channels.bru

@@ -0,0 +1,40 @@
+meta {
+  name: get channels from node2
+  type: http
+  seq: 11
+}
+
+post {
+  url: {{NODE2_RPC_URL}}
+  body: json
+  auth: none
+}
+
+headers {
+  Content-Type: application/json
+  Accept: application/json
+}
+
+body:json {
+  {
+    "id": 42,
+    "jsonrpc": "2.0",
+    "method": "graph_channels",
+    "params": [
+      { }
+    ]
+  }
+}
+
+
+assert {
+  res.status: eq 200
+}
+
+script:post-response {
+  await new Promise(r => setTimeout(r, 1000));
+  console.log("step 11 list graph channels: ", res.body.result.channels);
+  if (res.body.result.channels.length != 2) {
+    throw new Error("graph channels length is not right");
+  }
+}

tests/bruno/e2e/router-pay/24-node1-gen-invoice-for-self.bru

@@ -0,0 +1,66 @@
+meta {
+  name: generate a invoice
+  type: http
+  seq: 24
+}
+
+post {
+  url: {{NODE1_RPC_URL}}
+  body: json
+  auth: none
+}
+
+headers {
+  Content-Type: application/json
+  Accept: application/json
+}
+
+body:json {
+  {
+    "id": "42",
+    "jsonrpc": "2.0",
+    "method": "new_invoice",
+    "params": [
+      {
+        "amount": "0x613ae650",
+        "currency": "Fibd",
+        "description": "test invoice generated by node1",
+        "expiry": "0xe10",
+        "final_expiry_delta": "0x28",
+        "payment_preimage": "{{payment_preimage}}"
+      }
+    ]
+  }
+}
+
+assert {
+  res.body.error: isUndefined
+  res.body.result: isDefined
+}
+
+script:pre-request {
+  // generate random preimage
+  function generateRandomPreimage() {
+    let hash = '0x';
+    for (let i = 0; i < 64; i++) {
+        hash += Math.floor(Math.random() * 16).toString(16);
+    }
+    return hash;
+  }
+  const payment_preimage = generateRandomPreimage();
+  bru.setVar("payment_preimage", payment_preimage);
+  let hash_algorithm = bru.getEnvVar("HASH_ALGORITHM");
+  if (hash_algorithm !== null) {
+    let body = req.getBody();
+    body.params[0].hash_algorithm = hash_algorithm;
+    req.setBody(body);
+  }
+}
+
+script:post-response {
+  // Sleep for sometime to make sure current operation finishes before next request starts.
+  await new Promise(r => setTimeout(r, 100));
+  bru.setVar("payment_hash_self", res.body.result.invoice.data.payment_hash);
+  console.log("generated result: ", res.body.result);
+  bru.setVar("encoded_invoice_self", res.body.result.invoice_address);
+}

tests/bruno/e2e/router-pay/25-node1-pay-self-with-node2-err.bru

@@ -0,0 +1,45 @@
+
+meta {
+  name: Node1 send payment with router
+  type: http
+  seq: 25
+}
+
+post {
+  url: {{NODE1_RPC_URL}}
+  body: json
+  auth: none
+}
+
+headers {
+  Content-Type: application/json
+  Accept: application/json
+}
+
+body:json {
+  {
+    "id": "42",
+    "jsonrpc": "2.0",
+    "method": "send_payment",
+    "params": [
+      {
+        "invoice": "{{encoded_invoice_self}}",
+        "allow_self_payment": true
+      }
+    ]
+  }
+}
+
+assert {
+  res.body.error: isDefined
+}
+
+script:post-response {
+  // Sleep for sometime to make sure current operation finishes before next request starts.
+  await new Promise(r => setTimeout(r, 100));
+  console.log("25 step result: ", res.body);
+  // for pay self router A -> B -> A, can not use the same channel from A -> B and B -> A
+  if (!(res.body.error.message.includes("Failed to build route"))) {
+    throw new Error("Assertion failed: error message is not right");
+  }
+}

tests/bruno/e2e/router-pay/26-node2-node1-open-channel.bru

@@ -0,0 +1,46 @@
+meta {
+  name: Node2 open a channel to Node1
+  type: http
+  seq: 26
+}
+
+post {
+  url: {{NODE2_RPC_URL}}
+  body: json
+  auth: none
+}
+
+headers {
+  Content-Type: application/json
+  Accept: application/json
+}
+
+body:json {
+  {
+    "id": "42",
+    "jsonrpc": "2.0",
+    "method": "open_channel",
+    "params": [
+      {
+        "peer_id": "{{NODE1_PEERID}}",
+        "funding_amount": "0x277aab54d000",
+        "public": true,
+        "tlc_fee_proportional_millionths": "0x4B0"
+      }
+    ]
+  }
+}
+
+assert {
+  res.body.error: isUndefined
+  res.body.result.temporary_channel_id: isDefined
+}
+
+script:pre-request {
+  await new Promise(r => setTimeout(r, 1000));
+}
+
+script:post-response {
+  await new Promise(r => setTimeout(r, 1000));
+  console.log("26 step result: ", res.body);
+}