Merge pull request #99 from bitwalt/inbound-expired-payments-test

expired inbound payments stuck as Pending: test and fix

Author: zoedberg

openapi.yaml

@@ -1971,6 +1971,9 @@ components:
         updated_at:
           type: integer
           example: 1691162674
+        expires_at:
+          type: integer
+          example: 1691162674
         payee_pubkey:
           type: string
           example: 03b79a4bc1ec365524b4fab9a39eb133753646babb5a1da5c4bc94c53110b7795d

src/ldk.rs

@@ -127,6 +127,7 @@ pub(crate) struct PaymentInfo {
     pub(crate) created_at: u64,
     pub(crate) updated_at: u64,
     pub(crate) payee_pubkey: PublicKey,
+    pub(crate) expires_at: Option<u64>,
 }
 
 impl_writeable_tlv_based!(PaymentInfo, {
@@ -137,6 +138,7 @@ impl_writeable_tlv_based!(PaymentInfo, {
     (8, created_at, required),
     (10, updated_at, required),
     (12, payee_pubkey, required),
+    (14, expires_at, option),
 });
 
 pub(crate) struct InboundPaymentInfoStorage {
@@ -283,6 +285,30 @@ impl UnlockedAppState {
         }
     }
 
+    pub(crate) fn list_updated_inbound_payments(&self) -> LdkHashMap<PaymentHash, PaymentInfo> {
+        let now = get_current_timestamp();
+        let mut inbound = self.get_inbound_payments();
+        let mut failed = false;
+        for (_, payment_info) in inbound
+            .payments
+            .iter_mut()
+            .filter(|(_, i)| matches!(i.status, HTLCStatus::Pending))
+        {
+            if let Some(expires_at) = payment_info.expires_at {
+                if now > expires_at {
+                    payment_info.status = HTLCStatus::Failed;
+                    payment_info.updated_at = now;
+                    failed = true;
+                }
+            }
+        }
+        let payments = inbound.payments.clone();
+        if failed {
+            self.save_inbound_payments(inbound);
+        }
+        payments
+    }
+
     pub(crate) fn inbound_payments(&self) -> LdkHashMap<PaymentHash, PaymentInfo> {
         self.get_inbound_payments().payments.clone()
     }
@@ -334,6 +360,7 @@ impl UnlockedAppState {
                     created_at,
                     updated_at: created_at,
                     payee_pubkey,
+                    expires_at: None,
                 });
             }
         }

src/routes.rs

@@ -1743,7 +1743,7 @@ pub(crate) async fn get_payment(
     }
     let requested_ph = PaymentHash(payment_hash_vec.unwrap().try_into().unwrap());
 
-    let inbound_payments = unlocked_state.inbound_payments();
+    let inbound_payments = unlocked_state.list_updated_inbound_payments();
     let outbound_payments = unlocked_state.outbound_payments();
 
     for (payment_hash, payment_info) in &inbound_payments {
@@ -2074,6 +2074,7 @@ pub(crate) async fn keysend(
                 created_at,
                 updated_at: created_at,
                 payee_pubkey: dest_pubkey,
+                expires_at: None,
             },
         )?;
         if let Some((contract_id, rgb_amount)) = rgb_payment {
@@ -2282,7 +2283,7 @@ pub(crate) async fn list_payments(
     let guard = state.check_unlocked().await?;
     let unlocked_state = guard.as_ref().unwrap();
 
-    let inbound_payments = unlocked_state.inbound_payments();
+    let inbound_payments = unlocked_state.list_updated_inbound_payments();
     let outbound_payments = unlocked_state.outbound_payments();
     let mut payments = vec![];
 
@@ -2564,6 +2565,7 @@ pub(crate) async fn ln_invoice(
                 created_at,
                 updated_at: created_at,
                 payee_pubkey: unlocked_state.channel_manager.get_our_node_id(),
+                expires_at: Some(created_at + payload.expiry_sec as u64),
             },
         );
 
@@ -3501,6 +3503,7 @@ pub(crate) async fn send_payment(
                     created_at,
                     updated_at: created_at,
                     payee_pubkey: offer.issuer_signing_pubkey().ok_or(APIError::InvalidInvoice(s!("missing signing pubkey")))?,
+                    expires_at: None,
                 },
             )?;
 
@@ -3579,6 +3582,7 @@ pub(crate) async fn send_payment(
                     created_at,
                     updated_at: created_at,
                     payee_pubkey: invoice.get_payee_pub_key(),
+                    expires_at: None,
                 },
             )?;
             let payment_hash = PaymentHash(invoice.payment_hash().to_byte_array());

src/test/payment.rs

@@ -3,6 +3,7 @@ use crate::routes::{BitcoinNetwork, TransactionType, TransferKind, TransferStatu
 use super::*;
 
 const TEST_DIR_BASE: &str = "tmp/payment/";
+const SHORT_EXPIRY_SEC: u32 = 1;
 
 #[serial_test::serial]
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
@@ -212,7 +213,7 @@ async fn success() {
 #[serial_test::serial]
 #[tokio::test(flavor = "multi_thread", worker_threads = 1)]
 #[traced_test]
-async fn same_invoice_twice() {
+async fn same_invoice_twice_and_expired_inbound_payments() {
     initialize();
 
     let test_dir_base = format!("{TEST_DIR_BASE}same_invoice_twice/");
@@ -273,4 +274,87 @@ async fn same_invoice_twice() {
 
     let decoded = decode_ln_invoice(node1_addr, &invoice).await;
     wait_for_ln_payment(node1_addr, &decoded.payment_hash, HTLCStatus::Succeeded).await;
+    // create several invoices with a very short expiry that will NOT be paid
+    let LNInvoiceResponse { invoice: invoice1 } =
+        ln_invoice(node2_addr, Some(50000), None, None, SHORT_EXPIRY_SEC).await;
+    let LNInvoiceResponse { invoice: invoice2 } =
+        ln_invoice(node2_addr, Some(100000), None, None, SHORT_EXPIRY_SEC).await;
+    let LNInvoiceResponse { invoice: invoice3 } =
+        ln_invoice(node2_addr, None, None, None, SHORT_EXPIRY_SEC).await;
+
+    let decoded1 = decode_ln_invoice(node2_addr, &invoice1).await;
+    let decoded2 = decode_ln_invoice(node2_addr, &invoice2).await;
+    let decoded3 = decode_ln_invoice(node2_addr, &invoice3).await;
+
+    // verify all three start as Pending on the receiver node
+    let payments_before = list_payments(node2_addr).await;
+    let pending_before: Vec<_> = payments_before
+        .iter()
+        .filter(|p| {
+            p.inbound
+                && matches!(p.status, HTLCStatus::Pending)
+                && [
+                    decoded1.payment_hash.as_str(),
+                    decoded2.payment_hash.as_str(),
+                    decoded3.payment_hash.as_str(),
+                ]
+                .contains(&p.payment_hash.as_str())
+        })
+        .collect();
+    assert_eq!(
+        pending_before.len(),
+        3,
+        "expected all 3 unpaid invoices to be Pending"
+    );
+
+    // wait for the invoices to expire
+    tokio::time::sleep(std::time::Duration::from_secs(SHORT_EXPIRY_SEC as u64 + 1)).await;
+
+    // getting a payment should trigger expiration-based status transition
+    let payment = get_payment(node2_addr, &decoded1.payment_hash).await;
+    assert_eq!(
+        payment.status,
+        HTLCStatus::Failed,
+        "expected expired inbound payment {} to be Failed via getpayment, got {:?}",
+        decoded1.payment_hash,
+        payment.status
+    );
+
+    // listing payments should trigger expiration-based status transition
+    let payments_after = list_payments(node2_addr).await;
+
+    for hash in [
+        decoded2.payment_hash.as_str(),
+        decoded3.payment_hash.as_str(),
+    ] {
+        let payment = payments_after
+            .iter()
+            .find(|p| p.payment_hash == hash)
+            .unwrap_or_else(|| panic!("payment {hash} not found"));
+        assert_eq!(
+            payment.status,
+            HTLCStatus::Failed,
+            "expected expired inbound payment {hash} to be Failed, got {:?}",
+            payment.status
+        );
+    }
+
+    // sanity: no new Pending inbound payments should have appeared
+    let still_pending: Vec<_> = payments_after
+        .iter()
+        .filter(|p| {
+            p.inbound
+                && matches!(p.status, HTLCStatus::Pending)
+                && [
+                    decoded1.payment_hash.as_str(),
+                    decoded2.payment_hash.as_str(),
+                    decoded3.payment_hash.as_str(),
+                ]
+                .contains(&p.payment_hash.as_str())
+        })
+        .collect();
+    assert!(
+        still_pending.is_empty(),
+        "found expired inbound payments still Pending: {still_pending:?}"
+    );
 }