Merge pull request #2727 from TheBlueMatt/2023-11-better-bolt11-utils
[rust-lightning] / lightning / src / ln / outbound_payment.rs
index 025a197348ad18c616ffc0ea114ecaa6c3f116f5..0fbb0f5eaf4ccdeffd32d2a7c9d2aea20888273c 100644 (file)
@@ -29,6 +29,7 @@ use crate::util::ser::ReadableArgs;
 
 use core::fmt::{self, Display, Formatter};
 use core::ops::Deref;
+use core::time::Duration;
 
 use crate::prelude::*;
 use crate::sync::Mutex;
@@ -39,12 +40,6 @@ use crate::sync::Mutex;
 /// [`ChannelManager::timer_tick_occurred`]: crate::ln::channelmanager::ChannelManager::timer_tick_occurred
 pub(crate) const IDEMPOTENCY_TIMEOUT_TICKS: u8 = 7;
 
-/// The number of ticks of [`ChannelManager::timer_tick_occurred`] until an invoice request without
-/// a response is timed out.
-///
-/// [`ChannelManager::timer_tick_occurred`]: crate::ln::channelmanager::ChannelManager::timer_tick_occurred
-const INVOICE_REQUEST_TIMEOUT_TICKS: u8 = 3;
-
 /// Stores the session_priv for each part of a payment that is still pending. For versions 0.0.102
 /// and later, also stores information for retrying the payment.
 pub(crate) enum PendingOutboundPayment {
@@ -52,7 +47,7 @@ pub(crate) enum PendingOutboundPayment {
                session_privs: HashSet<[u8; 32]>,
        },
        AwaitingInvoice {
-               timer_ticks_without_response: u8,
+               expiration: StaleExpiration,
                retry_strategy: Retry,
                max_total_routing_fee_msat: Option<u64>,
        },
@@ -383,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`].
 ///
@@ -594,10 +605,26 @@ impl RecipientOnionFields {
        /// Note that if this field is non-empty, it will contain strictly increasing TLVs, each
        /// represented by a `(u64, Vec<u8>)` for its type number and serialized value respectively.
        /// This is validated when setting this field using [`Self::with_custom_tlvs`].
+       #[cfg(not(c_bindings))]
        pub fn custom_tlvs(&self) -> &Vec<(u64, Vec<u8>)> {
                &self.custom_tlvs
        }
 
+       /// Gets the custom TLVs that will be sent or have been received.
+       ///
+       /// Custom TLVs allow sending extra application-specific data with a payment. They provide
+       /// additional flexibility on top of payment metadata, as while other implementations may
+       /// require `payment_metadata` to reflect metadata provided in an invoice, custom TLVs
+       /// do not have this restriction.
+       ///
+       /// Note that if this field is non-empty, it will contain strictly increasing TLVs, each
+       /// represented by a `(u64, Vec<u8>)` for its type number and serialized value respectively.
+       /// This is validated when setting this field using [`Self::with_custom_tlvs`].
+       #[cfg(c_bindings)]
+       pub fn custom_tlvs(&self) -> Vec<(u64, Vec<u8>)> {
+               self.custom_tlvs.clone()
+       }
+
        /// When we have received some HTLC(s) towards an MPP payment, as we receive further HTLC(s) we
        /// have to make sure that some fields match exactly across the parts. For those that aren't
        /// required to match, if they don't match we should remove them so as to not expose data
@@ -735,7 +762,6 @@ impl OutboundPayments {
                }
        }
 
-       #[allow(unused)]
        pub(super) fn send_payment_for_bolt12_invoice<R: Deref, ES: Deref, NS: Deref, IH, SP, L: Deref>(
                &self, invoice: &Bolt12Invoice, payment_id: PaymentId, router: &R,
                first_hops: Vec<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS,
@@ -752,7 +778,7 @@ impl OutboundPayments {
                SP: Fn(SendAlongPathArgs) -> Result<(), APIError>,
        {
                let payment_hash = invoice.payment_hash();
-               let mut max_total_routing_fee_msat = None;
+               let max_total_routing_fee_msat;
                match self.pending_outbound_payments.lock().unwrap().entry(payment_id) {
                        hash_map::Entry::Occupied(entry) => match entry.get() {
                                PendingOutboundPayment::AwaitingInvoice { retry_strategy, max_total_routing_fee_msat: max_total_fee, .. } => {
@@ -768,11 +794,12 @@ impl OutboundPayments {
                        hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice),
                };
 
-               let route_params = RouteParameters {
-                       payment_params: PaymentParameters::from_bolt12_invoice(&invoice),
-                       final_value_msat: invoice.amount_msats(),
-                       max_total_routing_fee_msat,
-               };
+               let pay_params = PaymentParameters::from_bolt12_invoice(&invoice);
+               let amount_msat = invoice.amount_msats();
+               let mut route_params = RouteParameters::from_payment_params_and_value(pay_params, amount_msat);
+               if let Some(max_fee_msat) = max_total_routing_fee_msat {
+                       route_params.max_total_routing_fee_msat = Some(max_fee_msat);
+               }
 
                self.find_route_and_send_payment(
                        payment_hash, payment_id, route_params, router, first_hops, &inflight_htlcs,
@@ -875,7 +902,7 @@ impl OutboundPayments {
                        }
                }
 
-               let route = router.find_route_with_id(
+               let mut route = router.find_route_with_id(
                        &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params,
                        Some(&first_hops.iter().collect::<Vec<_>>()), inflight_htlcs(),
                        payment_hash, payment_id,
@@ -885,6 +912,12 @@ impl OutboundPayments {
                        RetryableSendFailure::RouteNotFound
                })?;
 
+               if route.route_params.as_ref() != Some(&route_params) {
+                       debug_assert!(false,
+                               "Routers are expected to return a Route which includes the requested RouteParameters");
+                       route.route_params = Some(route_params.clone());
+               }
+
                let onion_session_privs = self.add_new_pending_payment(payment_hash,
                        recipient_onion.clone(), payment_id, keysend_preimage, &route, Some(retry_strategy),
                        Some(route_params.payment_params.clone()), entropy_source, best_block_height)
@@ -926,7 +959,7 @@ impl OutboundPayments {
                        }
                }
 
-               let route = match router.find_route_with_id(
+               let mut route = match router.find_route_with_id(
                        &node_signer.get_node_id(Recipient::Node).unwrap(), &route_params,
                        Some(&first_hops.iter().collect::<Vec<_>>()), inflight_htlcs(),
                        payment_hash, payment_id,
@@ -938,6 +971,13 @@ impl OutboundPayments {
                                return
                        }
                };
+
+               if route.route_params.as_ref() != Some(&route_params) {
+                       debug_assert!(false,
+                               "Routers are expected to return a Route which includes the requested RouteParameters");
+                       route.route_params = Some(route_params.clone());
+               }
+
                for path in route.paths.iter() {
                        if path.hops.len() == 0 {
                                log_error!(logger, "Unusable path in route (path.hops.len() must be at least 1");
@@ -1244,16 +1284,16 @@ impl OutboundPayments {
                (payment, onion_session_privs)
        }
 
-       #[allow(unused)]
        pub(super) fn add_new_awaiting_invoice(
-               &self, payment_id: PaymentId, retry_strategy: Retry, max_total_routing_fee_msat: Option<u64>
+               &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();
                match pending_outbounds.entry(payment_id) {
                        hash_map::Entry::Occupied(_) => Err(()),
                        hash_map::Entry::Vacant(entry) => {
                                entry.insert(PendingOutboundPayment::AwaitingInvoice {
-                                       timer_ticks_without_response: 0,
+                                       expiration,
                                        retry_strategy,
                                        max_total_routing_fee_msat,
                                });
@@ -1337,12 +1377,14 @@ impl OutboundPayments {
                }
                let mut has_ok = false;
                let mut has_err = false;
-               let mut pending_amt_unsent = 0;
+               let mut has_unsent = false;
                let mut total_ok_fees_msat = 0;
+               let mut total_ok_amt_sent_msat = 0;
                for (res, path) in results.iter().zip(route.paths.iter()) {
                        if res.is_ok() {
                                has_ok = true;
                                total_ok_fees_msat += path.fee_msat();
+                               total_ok_amt_sent_msat += path.final_value_msat();
                        }
                        if res.is_err() { has_err = true; }
                        if let &Err(APIError::MonitorUpdateInProgress) = res {
@@ -1351,23 +1393,27 @@ impl OutboundPayments {
                                has_err = true;
                                has_ok = true;
                                total_ok_fees_msat += path.fee_msat();
+                               total_ok_amt_sent_msat += path.final_value_msat();
                        } else if res.is_err() {
-                               pending_amt_unsent += path.final_value_msat();
+                               has_unsent = true;
                        }
                }
                if has_err && has_ok {
                        Err(PaymentSendFailure::PartialFailure {
                                results,
                                payment_id,
-                               failed_paths_retry: if pending_amt_unsent != 0 {
+                               failed_paths_retry: if has_unsent {
                                        if let Some(route_params) = &route.route_params {
                                                let mut route_params = route_params.clone();
                                                // We calculate the leftover fee budget we're allowed to spend by
                                                // subtracting the used fee from the total fee budget.
                                                route_params.max_total_routing_fee_msat = route_params
                                                        .max_total_routing_fee_msat.map(|m| m.saturating_sub(total_ok_fees_msat));
-                                               route_params.final_value_msat = pending_amt_unsent;
 
+                                               // We calculate the remaining target amount by subtracting the succeded
+                                               // path values.
+                                               route_params.final_value_msat = route_params.final_value_msat
+                                                       .saturating_sub(total_ok_amt_sent_msat);
                                                Some(route_params)
                                        } else { None }
                                } else { None },
@@ -1476,11 +1522,12 @@ impl OutboundPayments {
        }
 
        pub(super) fn remove_stale_payments(
-               &self, pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>)
+               &self, duration_since_epoch: Duration,
+               pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>)
        {
                let mut pending_outbound_payments = self.pending_outbound_payments.lock().unwrap();
                let mut pending_events = pending_events.lock().unwrap();
-               pending_outbound_payments.retain(|payment_id, payment| {
+               pending_outbound_payments.retain(|payment_id, payment| match payment {
                        // If an outbound payment was completed, and no pending HTLCs remain, we should remove it
                        // from the map. However, if we did that immediately when the last payment HTLC is claimed,
                        // this could race the user making a duplicate send_payment call and our idempotency
@@ -1488,7 +1535,7 @@ impl OutboundPayments {
                        // removal. This should be more than sufficient to ensure the idempotency of any
                        // `send_payment` calls that were made at the same time the `PaymentSent` event was being
                        // processed.
-                       if let PendingOutboundPayment::Fulfilled { session_privs, timer_ticks_without_htlcs, .. } = payment {
+                       PendingOutboundPayment::Fulfilled { session_privs, timer_ticks_without_htlcs, .. } => {
                                let mut no_remaining_entries = session_privs.is_empty();
                                if no_remaining_entries {
                                        for (ev, _) in pending_events.iter() {
@@ -1512,17 +1559,31 @@ impl OutboundPayments {
                                        *timer_ticks_without_htlcs = 0;
                                        true
                                }
-                       } else if let PendingOutboundPayment::AwaitingInvoice { timer_ticks_without_response, .. } = payment {
-                               *timer_ticks_without_response += 1;
-                               if *timer_ticks_without_response <= INVOICE_REQUEST_TIMEOUT_TICKS {
-                                       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 {
                                        pending_events.push_back(
                                                (events::Event::InvoiceRequestFailed { payment_id: *payment_id }, None)
                                        );
                                        false
+                               } else {
+                                       true
                                }
-                       } else { true }
+                       },
+                       _ => true,
                });
        }
 
@@ -1738,7 +1799,7 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment,
                (2, payment_hash, required),
        },
        (5, AwaitingInvoice) => {
-               (0, timer_ticks_without_response, required),
+               (0, expiration, required),
                (2, retry_strategy, required),
                (4, max_total_routing_fee_msat, option),
        },
@@ -1754,12 +1815,15 @@ mod tests {
        use bitcoin::network::constants::Network;
        use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
 
+       use core::time::Duration;
+
        use crate::events::{Event, PathFailure, PaymentFailureReason};
        use crate::ln::PaymentHash;
        use crate::ln::channelmanager::{PaymentId, RecipientOnionFields};
        use crate::ln::features::{ChannelFeatures, NodeFeatures};
        use crate::ln::msgs::{ErrorAction, LightningError};
-       use crate::ln::outbound_payment::{Bolt12PaymentError, INVOICE_REQUEST_TIMEOUT_TICKS, OutboundPayments, Retry, RetryableSendFailure};
+       use crate::ln::outbound_payment::{Bolt12PaymentError, OutboundPayments, Retry, RetryableSendFailure, StaleExpiration};
+       #[cfg(feature = "std")]
        use crate::offers::invoice::DEFAULT_RELATIVE_EXPIRY;
        use crate::offers::offer::OfferBuilder;
        use crate::offers::test_utils::*;
@@ -1912,7 +1976,9 @@ mod tests {
                router.expect_find_route(route_params.clone(), Ok(route.clone()));
                let mut route_params_w_failed_scid = route_params.clone();
                route_params_w_failed_scid.payment_params.previously_failed_channels.push(failed_scid);
-               router.expect_find_route(route_params_w_failed_scid, Ok(route.clone()));
+               let mut route_w_failed_scid = route.clone();
+               route_w_failed_scid.route_params = Some(route_params_w_failed_scid.clone());
+               router.expect_find_route(route_params_w_failed_scid, Ok(route_w_failed_scid));
                router.expect_find_route(route_params.clone(), Ok(route.clone()));
                router.expect_find_route(route_params.clone(), Ok(route.clone()));
 
@@ -1962,24 +2028,82 @@ mod tests {
        }
 
        #[test]
-       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, expiration, Retry::Attempts(0), None
+                       ).is_ok()
+               );
+               assert!(outbound_payments.has_pending_payments());
+
+               for seconds_since_epoch in (0..absolute_expiry).step_by(tick_interval) {
+                       let duration_since_epoch = Duration::from_secs(seconds_since_epoch);
+                       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(absolute_expiry);
+               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()
+               );
+       }
+
+       #[test]
+       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, Retry::Attempts(0), None).is_ok()
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0), None
+                       ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());
 
-               for _ in 0..INVOICE_REQUEST_TIMEOUT_TICKS {
-                       outbound_payments.remove_stale_payments(&pending_events);
+               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());
                }
 
-               outbound_payments.remove_stale_payments(&pending_events);
+               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!(
@@ -1989,13 +2113,16 @@ mod tests {
                assert!(pending_events.lock().unwrap().is_empty());
 
                assert!(
-                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+                       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, Retry::Attempts(0), None)
-                               .is_err()
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0), None
+                       ).is_err()
                );
        }
 
@@ -2004,10 +2131,13 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                assert!(!outbound_payments.has_pending_payments());
                assert!(
-                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0), None
+                       ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());
 
@@ -2035,9 +2165,12 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                assert!(
-                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), None).is_ok()
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0), None
+                       ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());
 
@@ -2083,6 +2216,7 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
                        .amount_msats(1000)
@@ -2094,9 +2228,11 @@ mod tests {
                        .build().unwrap()
                        .sign(recipient_sign).unwrap();
 
-               assert!(outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Retry::Attempts(0), Some(invoice.amount_msats() / 100 + 50_000))
-                       .is_ok()
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0),
+                               Some(invoice.amount_msats() / 100 + 50_000)
+                       ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());
 
@@ -2139,6 +2275,7 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
                        .amount_msats(1000)
@@ -2150,9 +2287,11 @@ mod tests {
                        .build().unwrap()
                        .sign(recipient_sign).unwrap();
 
-               assert!(outbound_payments.add_new_awaiting_invoice(
-                               payment_id, Retry::Attempts(0), Some(invoice.amount_msats() / 100 + 50_000))
-                       .is_ok()
+               assert!(
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0),
+                               Some(invoice.amount_msats() / 100 + 50_000)
+                       ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());
 
@@ -2195,6 +2334,7 @@ mod tests {
                let pending_events = Mutex::new(VecDeque::new());
                let outbound_payments = OutboundPayments::new();
                let payment_id = PaymentId([0; 32]);
+               let expiration = StaleExpiration::AbsoluteTimeout(Duration::from_secs(100));
 
                let invoice = OfferBuilder::new("foo".into(), recipient_pubkey())
                        .amount_msats(1000)
@@ -2246,7 +2386,9 @@ mod tests {
                assert!(pending_events.lock().unwrap().is_empty());
 
                assert!(
-                       outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0), Some(1234)).is_ok()
+                       outbound_payments.add_new_awaiting_invoice(
+                               payment_id, expiration, Retry::Attempts(0), Some(1234)
+                       ).is_ok()
                );
                assert!(outbound_payments.has_pending_payments());