From 34bdf224891dfc42b50e09e29f40e0633b2aba25 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 19 Oct 2023 14:38:16 -0500 Subject: [PATCH] Absolute expiry or timer tick payment expiration 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 | 5 +- lightning/src/ln/outbound_payment.rs | 126 ++++++++++++++++++++++----- 2 files changed, 106 insertions(+), 25 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index ffe8ba2b1..74fef11e9 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -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)?; diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 19faad07b..2adabebe7 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -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, }, @@ -378,6 +378,22 @@ impl Display for PaymentAttemptsUsingTime { } } +/// 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 ) -> 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()); -- 2.39.5