handler: retry pay_invoice on failure for each blinded path

Before this commit we used the first blinded path offered in the
invoice to send the payment. If it failed because network dynamics
we would fail the payment for the end user.

With this commit if we fail to send the payment through the route
we will try with a new one until we don't have options to do the
payment.

src/offers/handler.rs

@@ -231,7 +231,7 @@ impl OfferHandler {
     /// Sends an invoice request and waits for an invoice to be sent back to us.
     /// Reminder that if this method returns an error after create_invoice_request is called, we
     /// *must* remove the payment_id from self.active_payments.
-    pub(crate) async fn pay_invoice(
+    pub async fn pay_invoice(
         &self,
         client: Client,
         amount: u64,
@@ -239,58 +239,76 @@ impl OfferHandler {
         payment_id: PaymentId,
         fee_limit: Option<FeeLimit>,
     ) -> Result<Payment, OfferError> {
-        let payment_hash = invoice.payment_hash();
-        let payment_path = &invoice.payment_paths()[0];
-
-        let payment_hash = payment_hash.0;
-        let params = SendPaymentParams {
-            path: payment_path.clone(),
-            cltv_expiry_delta: payment_path.payinfo.cltv_expiry_delta,
-            fee_base_msat: payment_path.payinfo.fee_base_msat,
-            fee_ppm: payment_path.payinfo.fee_proportional_millionths,
-            payment_hash,
-            msats: amount,
-            payment_id,
-            fee_limit,
-        };
-
-        let intro_node_id = match params.path.introduction_node() {
-            IntroductionNode::NodeId(node_id) => Some(node_id.to_string()),
-            IntroductionNode::DirectedShortChannelId(direction, scid) => {
-                let node_id_pub = get_node_id_from_scid(client.clone(), *scid, *direction).await?;
-                Some(node_id_pub)
-            }
-        };
+        let payment_hash = invoice.payment_hash().0;
+        let mut last_error: Option<OfferError> = None;
         debug!(
-            "Attempting to pay invoice with introduction node {:?}",
-            intro_node_id
+            "Payment paths found for invoice with payment_id {payment_id:?}: {:?}",
+            invoice.payment_paths().len()
         );
 
-        send_payment(client.clone(), params)
-            .await
-            .inspect_err(|_| {
-                let mut active_payments = self.active_payments.lock().unwrap();
-                active_payments.remove(&payment_id);
-            })?;
+        // We'll try each path until we find one that works.
+        for payment_path in invoice.payment_paths() {
+            let params = SendPaymentParams {
+                path: payment_path.clone(),
+                cltv_expiry_delta: payment_path.payinfo.cltv_expiry_delta,
+                fee_base_msat: payment_path.payinfo.fee_base_msat,
+                fee_ppm: payment_path.payinfo.fee_proportional_millionths,
+                payment_hash,
+                msats: amount,
+                payment_id,
+                fee_limit,
+            };
+
+            let intro_node_id = match params.path.introduction_node() {
+                IntroductionNode::NodeId(node_id) => Some(node_id.to_string()),
+                IntroductionNode::DirectedShortChannelId(direction, scid) => {
+                    let node_id_pub =
+                        get_node_id_from_scid(client.clone(), *scid, *direction).await?;
+                    Some(node_id_pub)
+                }
+            };
+            debug!(
+                "Attempting to pay invoice with introduction node {:?}",
+                intro_node_id
+            );
+
+            if let Err(e) = send_payment(client.clone(), params).await {
+                error!(
+                    "Failed to send payment for payment_id {payment_id:?} through path {:?}",
+                    payment_path
+                );
+                last_error = Some(e);
+                continue;
+            }
+
+            {
+                let mut active_payments = self.active_payments.lock().unwrap();
+                active_payments
+                    .entry(payment_id)
+                    .and_modify(|entry| entry.state = PaymentState::PaymentDispatched);
+            }
+
+            // We'll track the payment until it settles.
+            let payment = match track_payment(client.clone(), payment_hash).await {
+                Ok(payment) => payment,
+                Err(e) => {
+                    error!(
+                        "Failed to track payment for payment_id {payment_id:?} through path {:?}",
+                        payment_path
+                    );
+                    last_error = Some(e);
+                    continue;
+                }
+            };
 
-        {
             let mut active_payments = self.active_payments.lock().unwrap();
-            active_payments
-                .entry(payment_id)
-                .and_modify(|entry| entry.state = PaymentState::PaymentDispatched);
+            active_payments.remove(&payment_id);
+            return Ok(payment);
         }
 
-        // We'll track the payment until it settles.
-        track_payment(client, payment_hash)
-            .await
-            .inspect(|_| {
-                let mut active_payments = self.active_payments.lock().unwrap();
-                active_payments.remove(&payment_id);
-            })
-            .inspect_err(|_| {
-                let mut active_payments = self.active_payments.lock().unwrap();
-                active_payments.remove(&payment_id);
-            })
+        let mut active_payments = self.active_payments.lock().unwrap();
+        active_payments.remove(&payment_id);
+        Err(last_error.unwrap_or(OfferError::PaymentFailure))
     }
 
     /// wait_for_invoice waits for the offer creator to respond with an invoice.

tests/common/mod.rs

@@ -207,6 +207,40 @@ pub async fn setup_lndk(
     return (lndk_cfg, handler, messenger, shutdown);
 }
 
+pub async fn isolate_node(ldk_node: &LdkNode, bitcoind: &BitcoindNode) {
+    let channels_info = ldk_node.list_channels().await;
+    let address = bitcoind.node.client.new_address().unwrap();
+
+    log::info!("Closing channels...");
+
+    for channel in channels_info {
+        ldk_node.close_channel(channel.0, channel.1).await.unwrap();
+
+        // We need to generate a block so we avoid that transaction output is unspendable.
+        bitcoind
+            .node
+            .client
+            .generate_to_address(1, &address)
+            .unwrap();
+    }
+
+    log::info!("Waiting for list channels to be empty...");
+
+    match timeout(Duration::from_secs(100), async {
+        loop {
+            let channels_info = ldk_node.list_channels().await;
+            if channels_info.len() == 0 {
+                break;
+            }
+            sleep(Duration::from_secs(2)).await;
+        }
+    })
+    .await
+    {
+        Err(_) => panic!("timeout before channel closed"),
+        _ => {}
+    };
+}
 // Sets up /tmp/lndk-tests folder where we'll store the bins, data directories, and logs needed
 // for our tests.
 //

tests/integration_tests.rs

@@ -1,9 +1,11 @@
 #![cfg(itest)]
 
 mod common;
+use common::isolate_node;
 use futures::future::try_join_all;
 use lightning::bitcoin::constants::ChainHash;
 use lightning::blinded_path::message::{MessageContext, OffersContext};
+use lightning::blinded_path::IntroductionNode;
 use lightning::ln::channelmanager::PaymentId;
 use lightning::offers::nonce::Nonce;
 use lightning::offers::offer::{Amount, Offer};
@@ -21,6 +23,7 @@ use lndk::offers::create_reply_path;
 use lndk::offers::handler::{CreateOfferParams, OfferHandler, PayOfferParams};
 use lndk::onion_messenger::MessengerUtilities;
 use lndk::{setup_logger, LifecycleSignals};
+use std::net::SocketAddr;
 use std::path::PathBuf;
 use std::str::FromStr;
 use std::sync::Arc;
@@ -562,7 +565,7 @@ async fn test_reply_path_announced_peers() {
 // Here we test that we're able to fully pay an offer.
 async fn test_check_lndk_pay_offer_with_reconnection() {
     let test_name = "lndk_pay_offer_with_reconnection";
-    let (bitcoind, mut lnd, ldk1, ldk2, lndk_dir) =
+    let (bitcoind, mut lnd, ldk1, ldk2, lndk_dir, _) =
         common::setup_test_infrastructure(test_name).await;
 
     let (ldk1_pubkey, ldk2_pubkey, _) =
@@ -615,6 +618,253 @@ async fn test_check_lndk_pay_offer_with_reconnection() {
     };
 }
 
+pub async fn test_payment_with_first_route_down(
+    handler: &Arc<OfferHandler>,
+    pay_cfg: &PayOfferParams,
+    ldk3: &LdkNode,
+    ldk4: &LdkNode,
+    bitcoind: &common::BitcoindNode,
+) -> Result<(), Box<dyn std::error::Error>> {
+    // We first get the invoice and make sure we have 2 paths.
+    let (invoice, amount, payment_id) = handler
+        .get_invoice(pay_cfg.clone())
+        .await
+        .map_err(|e| format!("Failed to get invoice: {:?}", e))?;
+
+    assert!(invoice.payment_paths().len() == 2);
+
+    let first_path = invoice
+        .payment_paths()
+        .first()
+        .ok_or("No payment paths found in invoice")?;
+
+    let introduction_node = match first_path.introduction_node() {
+        IntroductionNode::NodeId(pubkey) => *pubkey,
+        IntroductionNode::DirectedShortChannelId(_, _) => {
+            return Err("Introduction node is a short channel ID, not a public key".into());
+        }
+    };
+
+    let node_to_isolate = if introduction_node == ldk3.get_node_info().0 {
+        ldk3
+    } else if introduction_node == ldk4.get_node_info().0 {
+        ldk4
+    } else {
+        return Err("Introduction node is not a valid node".into());
+    };
+
+    isolate_node(&node_to_isolate, &bitcoind).await;
+    log::info!("Attempting to pay invoice with first route down...");
+
+    // Finally, we try to pay the invoice and make sure it succeeds.
+    let payment_result = handler
+        .pay_invoice(pay_cfg.client.clone(), amount, &invoice, payment_id, None)
+        .await;
+
+    match payment_result {
+        Ok(_) => {
+            log::info!("Payment succeeded despite first route being down!");
+            Ok(())
+        }
+        Err(e) => {
+            log::error!("Payment failed: {:?}", e);
+            Err(format!("Payment failed: {:?}", e).into())
+        }
+    }
+}
+
+#[tokio::test(flavor = "multi_thread")]
+// We test the case that one of the paths fails for some network reason and we
+// retry the payment.
+async fn test_lndk_pay_offer_with_retry() {
+    let test_name = "lndk_pay_offer_with_retry";
+    let (bitcoind, mut lnd, ldk1, ldk2, lndk_dir, ldk_test_dir) =
+        common::setup_test_infrastructure(test_name).await;
+
+    let log_file = Some(lndk_dir.join(format!("lndk-logs.txt")));
+    setup_logger(None, log_file).unwrap();
+    let (ldk1_pubkey, ldk2_pubkey, _) =
+        common::connect_network(&ldk1, &ldk2, false, true, &mut lnd, &bitcoind).await;
+
+    // Second blinded path.
+    let ldk3 = common::setup_ldk_node(&bitcoind, 3, &ldk_test_dir, test_name).await;
+    // Blinded paths on ldk requires minimum 3 announced channels for a peer to be used.
+    // So we add a new node to make sure we have 3 announced channels.
+    //
+    // Network topology with 4 LDK nodes and 1 LND node:
+    //
+    //         LDK2 ───unannounced───── LND
+    //         /                      /|\
+    //        /                      / | \
+    //    LDK1 ──── LDK3 ────────────  |  \
+    //      \       /                  |   \
+    //       \     /                   |    \
+    //        LDK4 ──────────────────────────
+    //
+    // This creates multiple paths for blinded payments with redundancy.
+    // When one path fails, the payment can retry through alternate paths.
+    // Note that LDK3 and LDK4 has 3 announced channels, so invoice will be created
+    // with 2 blinded paths.
+    // We are going to isolate either LDK3 or LDK4 closing their channels to LDK1.
+    // Then, we will try to pay the invoice and make sure it succeeds.
+    let ldk4 = common::setup_ldk_node(&bitcoind, 4, &ldk_test_dir, test_name).await;
+    let (ldk3_pubkey, addr_3) = ldk3.get_node_info();
+    let (ldk4_pubkey, addr_4) = ldk4.get_node_info();
+    let (_, addr_1) = ldk1.get_node_info();
+    let lnd_info = lnd.get_info().await;
+    let lnd_pubkey = PublicKey::from_str(&lnd_info.identity_pubkey).unwrap();
+
+    ldk1.connect_to_peer(ldk4_pubkey, addr_4).await.unwrap();
+    ldk1.connect_to_peer(ldk3_pubkey, addr_3).await.unwrap();
+    lnd.connect_to_peer(ldk3_pubkey, addr_3).await;
+    lnd.connect_to_peer(ldk4_pubkey, addr_4).await;
+
+    let ldk3_fund_addr = ldk3.bitcoind_client.get_new_address().await;
+    let ldk3_addr_string = ldk3_fund_addr.to_string();
+    let ldk3_addr = bitcoincore_rpc::bitcoin::Address::from_str(&ldk3_addr_string)
+        .unwrap()
+        .require_network(bitcoincore_rpc::bitcoin::Network::Regtest)
+        .unwrap();
+
+    let ldk4_fund_addr = ldk4.bitcoind_client.get_new_address().await;
+    let ldk4_addr_string = ldk4_fund_addr.to_string();
+    let ldk4_addr = bitcoincore_rpc::bitcoin::Address::from_str(&ldk4_addr_string)
+        .unwrap()
+        .require_network(bitcoincore_rpc::bitcoin::Network::Regtest)
+        .unwrap();
+
+    bitcoind
+        .node
+        .client
+        .generate_to_address(6, &ldk3_addr)
+        .unwrap();
+
+    bitcoind
+        .node
+        .client
+        .generate_to_address(6, &ldk4_addr)
+        .unwrap();
+    lnd.wait_for_chain_sync().await;
+
+    ldk3.open_channel(ldk1_pubkey, addr_1, 200000, 10000000, true)
+        .await
+        .unwrap();
+
+    ldk4.open_channel(ldk1_pubkey, addr_1, 200000, 10000000, true)
+        .await
+        .unwrap();
+
+    lnd.wait_for_graph_sync().await;
+
+    bitcoind
+        .node
+        .client
+        .generate_to_address(6, &ldk3_addr)
+        .unwrap();
+
+    lnd.wait_for_chain_sync().await;
+
+    let lnd_addr = lnd
+        .address
+        .replace("localhost", "127.0.0.1")
+        .replace("https://", "");
+
+    ldk3.open_channel(
+        lnd_pubkey,
+        SocketAddr::from_str(&lnd_addr).unwrap(),
+        200000,
+        10000000,
+        true,
+    )
+    .await
+    .unwrap();
+
+    ldk4.open_channel(
+        lnd_pubkey,
+        SocketAddr::from_str(&lnd_addr).unwrap(),
+        200000,
+        10000000,
+        true,
+    )
+    .await
+    .unwrap();
+
+    lnd.wait_for_graph_sync().await;
+
+    bitcoind
+        .node
+        .client
+        .generate_to_address(6, &ldk3_addr)
+        .unwrap();
+
+    lnd.wait_for_chain_sync().await;
+
+    ldk3.open_channel(ldk4_pubkey, addr_4, 200000, 10000000, true)
+        .await
+        .unwrap();
+
+    lnd.wait_for_graph_sync().await;
+
+    bitcoind
+        .node
+        .client
+        .generate_to_address(20, &ldk3_addr)
+        .unwrap();
+
+    lnd.wait_for_chain_sync().await;
+    lnd.wait_for_nodes_addresses(&[&ldk1, &ldk2, &ldk3, &ldk4])
+        .await;
+
+    let path_pubkeys = vec![ldk2_pubkey, ldk1_pubkey];
+    let expiration = SystemTime::now() + Duration::from_secs(24 * 60 * 60);
+    let offer = ldk1
+        .create_offer(
+            &path_pubkeys,
+            Network::Regtest,
+            20_000,
+            Quantity::One,
+            expiration,
+        )
+        .await
+        .expect("should create offer");
+
+    let (lndk_cfg, handler, messenger, shutdown) = common::setup_lndk(
+        &lnd.cert_path,
+        &lnd.macaroon_path,
+        lnd.address.clone(),
+        lndk_dir,
+    )
+    .await;
+
+    let client = lnd.client.clone().unwrap();
+    let blinded_path = offer.paths()[0].clone();
+
+    let pay_cfg = PayOfferParams {
+        offer: offer.clone(),
+        amount: Some(20_000),
+        payer_note: Some("".to_string()),
+        network: Network::Regtest,
+        client: client.clone(),
+        destination: Destination::BlindedPath(blinded_path.clone()),
+        reply_path: None,
+        response_invoice_timeout: None,
+        fee_limit: None,
+    };
+
+    select! {
+        val = messenger.run(lndk_cfg.clone(), Arc::clone(&handler)) => {
+            panic!("lndk should not have completed first {:?}", val);
+        },
+        res = test_payment_with_first_route_down(&handler, &pay_cfg, &ldk3, &ldk4, &bitcoind) => {
+            log::info!("res: {:?}", res);
+            assert!(res.is_ok());
+            shutdown.trigger();
+            ldk1.stop().await;
+            ldk2.stop().await;
+        }
+    };
+}
+
 async fn check_pay_offer_with_reconnection(
     handler: Arc<OfferHandler>,
     pay_cfg: PayOfferParams,
@@ -663,7 +913,7 @@ async fn check_pay_offer_with_reconnection(
 // Test that we can create an offer and that the offer is valid.
 async fn test_create_offer() {
     let test_name = "lndk_create_offer";
-    let (bitcoind, mut lnd, ldk1, ldk2, lndk_dir) =
+    let (bitcoind, mut lnd, ldk1, ldk2, lndk_dir, _) =
         common::setup_test_infrastructure(test_name).await;
 
     let (_, _ldk2_pubkey, _lnd_pubkey) =
@@ -721,7 +971,7 @@ async fn pay_offer_and_wait_for_payment(
 // Test that we can receive a payment from an offer.
 async fn test_receive_payment_from_offer() {
     let test_name = "lndk_receive_payment_from_offer";
-    let (bitcoind, mut lnd, ldk1, ldk2, lndk_dir) =
+    let (bitcoind, mut lnd, ldk1, ldk2, lndk_dir, _) =
         common::setup_test_infrastructure(test_name).await;
 
     let (ldk1_pubkey, _ldk2_pubkey, _lnd_pubkey) =
@@ -784,5 +1034,5 @@ async fn test_receive_payment_from_offer() {
             ldk1.stop().await;
             ldk2.stop().await;
         }
-    }
+    };
 }