Absolute expiry or timer tick payment expiration
authorJeffrey Czyz <jkczyz@gmail.com>
Thu, 19 Oct 2023 19:38:16 +0000 (14:38 -0500)
committerJeffrey Czyz <jkczyz@gmail.com>
Fri, 20 Oct 2023 14:49:56 +0000 (09:49 -0500)
Pending outbound payments use an absolute expiry to determine when they
are considered stale and should be fail. In `no-std`, this may result in
long timeouts as the highest seen block time is used. Instead, allow for
expiration based on timer ticks. This will be use in an upcoming commit
for invoice request expiration.

lightning/src/ln/channelmanager.rs
lightning/src/ln/outbound_payment.rs

index ffe8ba2b16b2fd318f43994b12cdeec09f34055d..74fef11e9132c65e05818d07874baa33952d7f53 100644 (file)
@@ -54,7 +54,7 @@ use crate::ln::onion_utils::HTLCFailReason;
 use crate::ln::msgs::{ChannelMessageHandler, DecodeError, LightningError};
 #[cfg(test)]
 use crate::ln::outbound_payment;
-use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment, SendAlongPathArgs};
+use crate::ln::outbound_payment::{OutboundPayments, PaymentAttempts, PendingOutboundPayment, SendAlongPathArgs, StaleExpiration};
 use crate::ln::wire::Encode;
 use crate::offers::offer::{DerivedMetadata, OfferBuilder};
 use crate::offers::parse::Bolt12SemanticError;
@@ -7349,9 +7349,10 @@ where
                        .absolute_expiry(absolute_expiry)
                        .path(path);
 
+               let expiration = StaleExpiration::AbsoluteTimeout(absolute_expiry);
                self.pending_outbound_payments
                        .add_new_awaiting_invoice(
-                               payment_id, absolute_expiry, retry_strategy, max_total_routing_fee_msat,
+                               payment_id, expiration, retry_strategy, max_total_routing_fee_msat,
                        )
                        .map_err(|_| Bolt12SemanticError::DuplicatePaymentId)?;
 
index 19faad07bbd518f4de55a442c52067905a5959cd..2adabebe7e3eca165ec40a3a9f3302c5cf2b47a9 100644 (file)
@@ -47,7 +47,7 @@ pub(crate) enum PendingOutboundPayment {
                session_privs: HashSet<[u8; 32]>,
        },
        AwaitingInvoice {
-               absolute_expiry: Duration,
+               expiration: StaleExpiration,
                retry_strategy: Retry,
                max_total_routing_fee_msat: Option<u64>,
        },
@@ -378,6 +378,22 @@ impl<T: Time> Display for PaymentAttemptsUsingTime<T> {
        }
 }
 
+/// How long before a [`PendingOutboundPayment::AwaitingInvoice`] should be considered stale and
+/// candidate for removal in [`OutboundPayments::remove_stale_payments`].
+#[derive(Clone, Copy)]
+pub(crate) enum StaleExpiration {
+       /// Number of times [`OutboundPayments::remove_stale_payments`] is called.
+       TimerTicks(u64),
+       /// Duration since the Unix epoch.
+       AbsoluteTimeout(core::time::Duration),
+}
+
+impl_writeable_tlv_based_enum!(StaleExpiration,
+       ;
+       (0, TimerTicks),
+       (2, AbsoluteTimeout)
+);
+
 /// Indicates an immediate error on [`ChannelManager::send_payment`]. Further errors may be
 /// surfaced later via [`Event::PaymentPathFailed`] and [`Event::PaymentFailed`].
 ///
@@ -1269,7 +1285,7 @@ impl OutboundPayments {
        }
 
        pub(super) fn add_new_awaiting_invoice(
-               &self, payment_id: PaymentId, absolute_expiry: Duration, retry_strategy: Retry,
+               &self, payment_id: PaymentId, expiration: StaleExpiration, retry_strategy: Retry,
                max_total_routing_fee_msat: Option<u64>
        ) -> Result<(), ()> {
                let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap();
@@ -1277,7 +1293,7 @@ impl OutboundPayments {
                        hash_map::Entry::Occupied(_) => Err(()),
                        hash_map::Entry::Vacant(entry) => {
                                entry.insert(PendingOutboundPayment::AwaitingInvoice {
-                                       absolute_expiry,
+                                       expiration,
                                        retry_strategy,
                                        max_total_routing_fee_msat,
                                });
@@ -1547,15 +1563,28 @@ impl OutboundPayments {
                                        true
                                }
                        },
-                       PendingOutboundPayment::AwaitingInvoice { absolute_expiry, ..  } => {
-                               if duration_since_epoch < *absolute_expiry {
-                                       true
-                               } else {
+                       PendingOutboundPayment::AwaitingInvoice { expiration, .. } => {
+                               let is_stale = match expiration {
+                                       StaleExpiration::AbsoluteTimeout(absolute_expiry) => {
+                                               *absolute_expiry <= duration_since_epoch
+                                       },
+                                       StaleExpiration::TimerTicks(timer_ticks_remaining) => {
+                                               if *timer_ticks_remaining > 0 {
+                                                       *timer_ticks_remaining -= 1;
+                                                       false
+                                               } else {
+                                                       true
+                                               }
+                                       },
+                               };
+                               if is_stale {
                                        #[cfg(invreqfailed)]
                                        pending_events.push_back(
                                                (events::Event::InvoiceRequestFailed { payment_id: *payment_id }, None)
                                        );
                                        false
+                               } else {
+                                       true
                                }
                        },
                        _ => true,
@@ -1775,7 +1804,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
                (2, payment_hash, required),
        },
        (5, AwaitingInvoice) => {
-               (0, absolute_expiry, required),
+               (0, expiration, required),
                (2, retry_strategy, required),
                (4, max_total_routing_fee_msat, option),
        },
@@ -1798,7 +1827,7 @@ mod tests {
        use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
        use crate::ln::features::{ChannelFeatures, NodeFeatures};
        use crate::ln::msgs::{ErrorAction, LightningError};
-       use crate::ln::outbound_payment::{Bolt12PaymentError, OutboundPayments, Retry, RetryableSendFailure};
+       use crate::ln::outbound_payment::{Bolt12PaymentError, OutboundPayments, Retry, RetryableSendFailure, StaleExpiration};
        use crate::offers::invoice::DEFAULT_RELATIVE_EXPIRY;
        use crate::offers::offer::OfferBuilder;
        use crate::offers::test_utils::*;
@@ -2004,17 +2033,18 @@ mod tests {
 
        #[test]
        #[cfg(invreqfailed)]
-       fn removes_stale_awaiting_invoice() {
+       fn removes_stale_awaiting_invoice_using_absolute_timeout() {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
                let absolute_expiry = 100;
                let tick_interval = 10;
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(absolute_expiry));
 
                assert!(!outbound_payments.has_pending_payments());
                assert!(
                        outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Duration::from_secs(absolute_expiry), Retry::Attempts(0), None
+                               payment_id, expiration, Retry::Attempts(0), None
                        ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());
@@ -2040,14 +2070,64 @@ mod tests {
 
                assert!(
                        outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Duration::from_secs(absolute_expiry + 1), Retry::Attempts(0), None
+                               payment_id, expiration, Retry::Attempts(0), None
                        ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());
 
                assert!(
                        outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Duration::from_secs(absolute_expiry + 1), Retry::Attempts(0), None
+                               payment_id, expiration, Retry::Attempts(0), None
+                       ).is_err()
+               );
+       }
+
+       #[test]
+       #[cfg(invreqfailed)]
+       fn removes_stale_awaiting_invoice_using_timer_ticks() {
+               let pending_events = Mutex::new(VecDeque::new());
+               let outbound_payments = OutboundPayments::new();
+               let payment_id = PaymentId([0; 32]);
+               let timer_ticks = 3;
+               let expiration = StaleExpiration::TimerTicks(timer_ticks);
+
+               assert!(!outbound_payments.has_pending_payments());
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0), None
+                       ).is_ok()
+               );
+               assert!(outbound_payments.has_pending_payments());
+
+               for i in 0..timer_ticks {
+                       let duration_since_epoch = Duration::from_secs(i * 60);
+                       outbound_payments.remove_stale_payments(duration_since_epoch, &pending_events);
+
+                       assert!(outbound_payments.has_pending_payments());
+                       assert!(pending_events.lock().unwrap().is_empty());
+               }
+
+               let duration_since_epoch = Duration::from_secs(timer_ticks * 60);
+               outbound_payments.remove_stale_payments(duration_since_epoch, &pending_events);
+
+               assert!(!outbound_payments.has_pending_payments());
+               assert!(!pending_events.lock().unwrap().is_empty());
+               assert_eq!(
+                       pending_events.lock().unwrap().pop_front(),
+                       Some((Event::InvoiceRequestFailed { payment_id }, None)),
+               );
+               assert!(pending_events.lock().unwrap().is_empty());
+
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0), None
+                       ).is_ok()
+               );
+               assert!(outbound_payments.has_pending_payments());
+
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0), None
                        ).is_err()
                );
        }
@@ -2058,12 +2138,12 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
-               let absolute_expiry = 100;
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                assert!(!outbound_payments.has_pending_payments());
                assert!(
                        outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Duration::from_secs(absolute_expiry), Retry::Attempts(0), None
+                               payment_id, expiration, Retry::Attempts(0), None
                        ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());
@@ -2092,11 +2172,11 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
-               let absolute_expiry = 100;
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                assert!(
                        outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Duration::from_secs(absolute_expiry), Retry::Attempts(0), None
+                               payment_id, expiration, Retry::Attempts(0), None
                        ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());
@@ -2143,7 +2223,7 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
-               let absolute_expiry = 100;
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
                        .amount_msats(1000)
@@ -2157,7 +2237,7 @@ mod tests {
 
                assert!(
                        outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Duration::from_secs(absolute_expiry), Retry::Attempts(0),
+                               payment_id, expiration, Retry::Attempts(0),
                                Some(invoice.amount_msats() / 100 + 50_000)
                        ).is_ok()
                );
@@ -2202,7 +2282,7 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
-               let absolute_expiry = 100;
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
                        .amount_msats(1000)
@@ -2216,7 +2296,7 @@ mod tests {
 
                assert!(
                        outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Duration::from_secs(absolute_expiry), Retry::Attempts(0),
+                               payment_id, expiration, Retry::Attempts(0),
                                Some(invoice.amount_msats() / 100 + 50_000)
                        ).is_ok()
                );
@@ -2261,7 +2341,7 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
-               let absolute_expiry = 100;
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
                        .amount_msats(1000)
@@ -2314,7 +2394,7 @@ mod tests {
 
                assert!(
                        outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Duration::from_secs(absolute_expiry), Retry::Attempts(0), Some(1234)
+                               payment_id, expiration, Retry::Attempts(0), Some(1234)
                        ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());