From 6d0c5f00ba315981e83994774dc50c5c31d4e5ae Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 18 Jul 2023 14:41:50 -0500 Subject: [PATCH 01/13] Add an InvoiceRequestFailed event When an invoice is requested but either receives an error or never receives a response, surface an event to indicate to the user that the corresponding future payment has failed. --- lightning/src/events/mod.rs | 25 +++++++++++++++++++ lightning/src/ln/channelmanager.rs | 20 +++++++++++---- .../invoice_request_failed_downgrade.txt | 3 +++ 3 files changed, 43 insertions(+), 5 deletions(-) create mode 100644 pending_changelog/invoice_request_failed_downgrade.txt diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index f4f7a7cca9a..bb98e271597 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -508,6 +508,14 @@ pub enum Event { /// serialized prior to LDK version 0.0.117. sender_intended_total_msat: Option, }, + /// Indicates a request for an invoice failed to yield a response in a reasonable amount of time + /// or was explicitly abandoned by [`ChannelManager::abandon_payment`]. + /// + /// [`ChannelManager::abandon_payment`]: crate::ln::channelmanager::ChannelManager::abandon_payment + InvoiceRequestFailed { + /// The `payment_id` to have been associated with payment for the requested invoice. + payment_id: PaymentId, + }, /// Indicates an outbound payment we made succeeded (i.e. it made it all the way to its target /// and we got back the payment preimage for it). /// @@ -1148,6 +1156,12 @@ impl Writeable for Event { (8, funding_txo, required), }); }, + &Event::InvoiceRequestFailed { ref payment_id } => { + 33u8.write(writer)?; + write_tlv_fields!(writer, { + (0, payment_id, required), + }) + }, // Note that, going forward, all new events must only write data inside of // `write_tlv_fields`. Versions 0.0.101+ will ignore odd-numbered events that write // data via `write_tlv_fields`. @@ -1535,6 +1549,17 @@ impl MaybeReadable for Event { }; f() }, + 33u8 => { + let f = || { + _init_and_read_len_prefixed_tlv_fields!(reader, { + (0, payment_id, required), + }); + Ok(Some(Event::InvoiceRequestFailed { + payment_id: payment_id.0.unwrap(), + })) + }; + f() + }, // Versions prior to 0.0.100 did not ignore odd types, instead returning InvalidValue. // Version 0.0.100 failed to properly ignore odd types, possibly resulting in corrupt // reads. diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index cff9837fceb..b0f4ef4e094 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -3381,10 +3381,12 @@ where } - /// Signals that no further retries for the given payment should occur. Useful if you have a + /// Signals that no further attempts for the given payment should occur. Useful if you have a /// pending outbound payment with retries remaining, but wish to stop retrying the payment before /// retries are exhausted. /// + /// # Event Generation + /// /// If no [`Event::PaymentFailed`] event had been generated before, one will be generated as soon /// as there are no remaining pending HTLCs for this payment. /// @@ -3392,11 +3394,19 @@ where /// wait until you receive either a [`Event::PaymentFailed`] or [`Event::PaymentSent`] event to /// determine the ultimate status of a payment. /// - /// If an [`Event::PaymentFailed`] event is generated and we restart without this - /// [`ChannelManager`] having been persisted, another [`Event::PaymentFailed`] may be generated. + /// # Requested Invoices /// - /// [`Event::PaymentFailed`]: events::Event::PaymentFailed - /// [`Event::PaymentSent`]: events::Event::PaymentSent + /// In the case of paying a [`Bolt12Invoice`], abandoning the payment prior to receiving the + /// invoice will result in an [`Event::InvoiceRequestFailed`] and prevent any attempts at paying + /// it once received. The other events may only be generated once the invoice has been received. + /// + /// # Restart Behavior + /// + /// If an [`Event::PaymentFailed`] is generated and we restart without first persisting the + /// [`ChannelManager`], another [`Event::PaymentFailed`] may be generated; likewise for + /// [`Event::InvoiceRequestFailed`]. + /// + /// [`Bolt12Invoice`]: crate::offers::invoice::Bolt12Invoice pub fn abandon_payment(&self, payment_id: PaymentId) { let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); self.pending_outbound_payments.abandon_payment(payment_id, PaymentFailureReason::UserAbandoned, &self.pending_events); diff --git a/pending_changelog/invoice_request_failed_downgrade.txt b/pending_changelog/invoice_request_failed_downgrade.txt new file mode 100644 index 00000000000..d701cef04b5 --- /dev/null +++ b/pending_changelog/invoice_request_failed_downgrade.txt @@ -0,0 +1,3 @@ +## Backwards Compatibility + +* If an `Event::InvoiceRequestFailed` was generated for a BOLT 12 payment (#2371), downgrading will result in the payment silently failing if the event had not been processed yet. From 581ea7c6edef48be22137f85b6e6e09446d58396 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 17 Jul 2023 16:55:22 -0500 Subject: [PATCH 02/13] Add PendingOutboundPayment::AwaitingInvoice When a BOLT 12 invoice has been requested, in order to guarantee invoice payment idempotency the future payment needs to be tracked. Add an AwaitingInvoice variant to PendingOutboundPayment such that only requested invoices are paid only once. Timeout after a few timer ticks if a request has not been received. --- lightning/src/ln/channelmanager.rs | 15 ++++- lightning/src/ln/outbound_payment.rs | 98 +++++++++++++++++++++++----- 2 files changed, 94 insertions(+), 19 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b0f4ef4e094..8f68a1d6812 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1343,7 +1343,7 @@ pub(crate) const MPP_TIMEOUT_TICKS: u8 = 3; /// The number of ticks of [`ChannelManager::timer_tick_occurred`] until we time-out the /// idempotency of payments by [`PaymentId`]. See -/// [`OutboundPayments::remove_stale_resolved_payments`]. +/// [`OutboundPayments::remove_stale_payments`]. pub(crate) const IDEMPOTENCY_TIMEOUT_TICKS: u8 = 7; /// The number of ticks of [`ChannelManager::timer_tick_occurred`] where a peer is disconnected @@ -1688,6 +1688,11 @@ pub enum ChannelShutdownState { /// These include payments that have yet to find a successful path, or have unresolved HTLCs. #[derive(Debug, PartialEq)] pub enum RecentPaymentDetails { + /// When an invoice was requested but not yet received, and thus a payment has not been sent. + AwaitingInvoice { + /// Identifier for the payment to ensure idempotency. + payment_id: PaymentId, + }, /// When a payment is still being sent and awaiting successful delivery. Pending { /// Hash of the payment that is currently being sent but has yet to be fulfilled or @@ -2419,7 +2424,10 @@ where /// [`Event::PaymentSent`]: events::Event::PaymentSent pub fn list_recent_payments(&self) -> Vec { self.pending_outbound_payments.pending_outbound_payments.lock().unwrap().iter() - .filter_map(|(_, pending_outbound_payment)| match pending_outbound_payment { + .filter_map(|(payment_id, pending_outbound_payment)| match pending_outbound_payment { + PendingOutboundPayment::AwaitingInvoice { .. } => { + Some(RecentPaymentDetails::AwaitingInvoice { payment_id: *payment_id }) + }, PendingOutboundPayment::Retryable { payment_hash, total_msat, .. } => { Some(RecentPaymentDetails::Pending { payment_hash: *payment_hash, @@ -4665,7 +4673,7 @@ where let _ = handle_error!(self, err, counterparty_node_id); } - self.pending_outbound_payments.remove_stale_resolved_payments(&self.pending_events); + self.pending_outbound_payments.remove_stale_payments(&self.pending_events); // Technically we don't need to do this here, but if we have holding cell entries in a // channel that need freeing, it's better to do that here and block a background task @@ -8357,6 +8365,7 @@ where session_priv.write(writer)?; } } + PendingOutboundPayment::AwaitingInvoice { .. } => {}, PendingOutboundPayment::Fulfilled { .. } => {}, PendingOutboundPayment::Abandoned { .. } => {}, } diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 67d90d2dbf8..f5341b88405 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -32,12 +32,21 @@ use core::ops::Deref; use crate::prelude::*; use crate::sync::Mutex; +/// 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 { Legacy { session_privs: HashSet<[u8; 32]>, }, + AwaitingInvoice { + timer_ticks_without_response: u8, + }, Retryable { retry_strategy: Option, attempts: PaymentAttempts, @@ -108,6 +117,12 @@ impl PendingOutboundPayment { params.previously_failed_channels.push(scid); } } + fn is_awaiting_invoice(&self) -> bool { + match self { + PendingOutboundPayment::AwaitingInvoice { .. } => true, + _ => false, + } + } pub(super) fn is_fulfilled(&self) -> bool { match self { PendingOutboundPayment::Fulfilled { .. } => true, @@ -130,6 +145,7 @@ impl PendingOutboundPayment { fn payment_hash(&self) -> Option { match self { PendingOutboundPayment::Legacy { .. } => None, + PendingOutboundPayment::AwaitingInvoice { .. } => None, PendingOutboundPayment::Retryable { payment_hash, .. } => Some(*payment_hash), PendingOutboundPayment::Fulfilled { payment_hash, .. } => *payment_hash, PendingOutboundPayment::Abandoned { payment_hash, .. } => Some(*payment_hash), @@ -142,8 +158,11 @@ impl PendingOutboundPayment { PendingOutboundPayment::Legacy { session_privs } | PendingOutboundPayment::Retryable { session_privs, .. } | PendingOutboundPayment::Fulfilled { session_privs, .. } | - PendingOutboundPayment::Abandoned { session_privs, .. } - => session_privs, + PendingOutboundPayment::Abandoned { session_privs, .. } => session_privs, + PendingOutboundPayment::AwaitingInvoice { .. } => { + debug_assert!(false); + return; + }, }); let payment_hash = self.payment_hash(); *self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash, timer_ticks_without_htlcs: 0 }; @@ -169,7 +188,11 @@ impl PendingOutboundPayment { PendingOutboundPayment::Fulfilled { session_privs, .. } | PendingOutboundPayment::Abandoned { session_privs, .. } => { session_privs.remove(session_priv) - } + }, + PendingOutboundPayment::AwaitingInvoice { .. } => { + debug_assert!(false); + false + }, }; if remove_res { if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self { @@ -189,6 +212,10 @@ impl PendingOutboundPayment { PendingOutboundPayment::Retryable { session_privs, .. } => { session_privs.insert(session_priv) } + PendingOutboundPayment::AwaitingInvoice { .. } => { + debug_assert!(false); + false + }, PendingOutboundPayment::Fulfilled { .. } => false, PendingOutboundPayment::Abandoned { .. } => false, }; @@ -210,7 +237,8 @@ impl PendingOutboundPayment { PendingOutboundPayment::Fulfilled { session_privs, .. } | PendingOutboundPayment::Abandoned { session_privs, .. } => { session_privs.len() - } + }, + PendingOutboundPayment::AwaitingInvoice { .. } => 0, } } } @@ -679,7 +707,7 @@ impl OutboundPayments { let mut outbounds = self.pending_outbound_payments.lock().unwrap(); outbounds.retain(|pmt_id, pmt| { let mut retain = true; - if !pmt.is_auto_retryable_now() && pmt.remaining_parts() == 0 { + if !pmt.is_auto_retryable_now() && pmt.remaining_parts() == 0 && !pmt.is_awaiting_invoice() { pmt.mark_abandoned(PaymentFailureReason::RetriesExhausted); if let PendingOutboundPayment::Abandoned { payment_hash, reason, .. } = pmt { pending_events.lock().unwrap().push_back((events::Event::PaymentFailed { @@ -697,7 +725,8 @@ impl OutboundPayments { pub(super) fn needs_abandon(&self) -> bool { let outbounds = self.pending_outbound_payments.lock().unwrap(); outbounds.iter().any(|(_, pmt)| - !pmt.is_auto_retryable_now() && pmt.remaining_parts() == 0 && !pmt.is_fulfilled()) + !pmt.is_auto_retryable_now() && pmt.remaining_parts() == 0 && !pmt.is_fulfilled() && + !pmt.is_awaiting_invoice()) } /// Errors immediately on [`RetryableSendFailure`] error conditions. Otherwise, further errors may @@ -845,6 +874,10 @@ impl OutboundPayments { log_error!(logger, "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102"); return }, + PendingOutboundPayment::AwaitingInvoice { .. } => { + log_error!(logger, "Payment not yet sent"); + return + }, PendingOutboundPayment::Fulfilled { .. } => { log_error!(logger, "Payment already completed"); return @@ -1051,6 +1084,21 @@ impl OutboundPayments { } } + #[allow(unused)] + pub(super) fn add_new_awaiting_invoice(&self, payment_id: PaymentId) -> 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, + }); + + Ok(()) + }, + } + } + fn pay_route_internal( &self, route: &Route, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, keysend_preimage: Option, payment_id: PaymentId, recv_value_msat: Option, @@ -1256,19 +1304,19 @@ impl OutboundPayments { } } - pub(super) fn remove_stale_resolved_payments(&self, - pending_events: &Mutex)>>) + pub(super) fn remove_stale_payments( + &self, pending_events: &Mutex)>>) { - // 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 - // guarantees would be violated. Instead, we wait a few timer ticks to do the actual - // 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. let mut pending_outbound_payments = self.pending_outbound_payments.lock().unwrap(); - let pending_events = pending_events.lock().unwrap(); + let mut pending_events = pending_events.lock().unwrap(); pending_outbound_payments.retain(|payment_id, 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 + // guarantees would be violated. Instead, we wait a few timer ticks to do the actual + // 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 { let mut no_remaining_entries = session_privs.is_empty(); if no_remaining_entries { @@ -1293,6 +1341,16 @@ 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 { + pending_events.push_back( + (events::Event::InvoiceRequestFailed { payment_id: *payment_id }, None) + ); + false + } } else { true } }); } @@ -1438,6 +1496,11 @@ impl OutboundPayments { }, None)); payment.remove(); } + } else if let PendingOutboundPayment::AwaitingInvoice { .. } = payment.get() { + pending_events.lock().unwrap().push_back((events::Event::InvoiceRequestFailed { + payment_id, + }, None)); + payment.remove(); } } } @@ -1501,6 +1564,9 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, (1, reason, option), (2, payment_hash, required), }, + (5, AwaitingInvoice) => { + (0, timer_ticks_without_response, required), + }, ); #[cfg(test)] From 7f4c473c88a0fd43de2b72bb368f244b63492dc6 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 6 Sep 2023 13:56:46 -0500 Subject: [PATCH 03/13] Move IDEMPOTENCY_TIMEOUT_TICKS to where it is used --- lightning/src/ln/channelmanager.rs | 5 ----- lightning/src/ln/outbound_payment.rs | 8 +++++++- lightning/src/ln/payment_tests.rs | 4 ++-- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 8f68a1d6812..85fb7852bfe 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1341,11 +1341,6 @@ const CHECK_CLTV_EXPIRY_SANITY_2: u32 = MIN_CLTV_EXPIRY_DELTA as u32 - LATENCY_G /// The number of ticks of [`ChannelManager::timer_tick_occurred`] until expiry of incomplete MPPs pub(crate) const MPP_TIMEOUT_TICKS: u8 = 3; -/// The number of ticks of [`ChannelManager::timer_tick_occurred`] until we time-out the -/// idempotency of payments by [`PaymentId`]. See -/// [`OutboundPayments::remove_stale_payments`]. -pub(crate) const IDEMPOTENCY_TIMEOUT_TICKS: u8 = 7; - /// The number of ticks of [`ChannelManager::timer_tick_occurred`] where a peer is disconnected /// until we mark the channel disabled and gossip the update. pub(crate) const DISABLE_GOSSIP_TICKS: u8 = 10; diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index f5341b88405..aa92043cf91 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -16,7 +16,7 @@ use bitcoin::secp256k1::{self, Secp256k1, SecretKey}; use crate::sign::{EntropySource, NodeSigner, Recipient}; use crate::events::{self, PaymentFailureReason}; use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; -use crate::ln::channelmanager::{ChannelDetails, EventCompletionAction, HTLCSource, IDEMPOTENCY_TIMEOUT_TICKS, PaymentId}; +use crate::ln::channelmanager::{ChannelDetails, EventCompletionAction, HTLCSource, PaymentId}; use crate::ln::onion_utils::{DecodedOnionFailure, HTLCFailReason}; use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteParameters, Router}; use crate::util::errors::APIError; @@ -32,6 +32,12 @@ use core::ops::Deref; use crate::prelude::*; use crate::sync::Mutex; +/// The number of ticks of [`ChannelManager::timer_tick_occurred`] until we time-out the idempotency +/// of payments by [`PaymentId`]. See [`OutboundPayments::remove_stale_payments`]. +/// +/// [`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. /// diff --git a/lightning/src/ln/payment_tests.rs b/lightning/src/ln/payment_tests.rs index 0c94adb3560..506209df2b5 100644 --- a/lightning/src/ln/payment_tests.rs +++ b/lightning/src/ln/payment_tests.rs @@ -17,11 +17,11 @@ use crate::sign::EntropySource; use crate::chain::transaction::OutPoint; use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEvent, MessageSendEventsProvider, PathFailure, PaymentFailureReason, PaymentPurpose}; use crate::ln::channel::EXPIRE_PREV_CONFIG_TICKS; -use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, IDEMPOTENCY_TIMEOUT_TICKS, RecentPaymentDetails, RecipientOnionFields, HTLCForwardInfo, PendingHTLCRouting, PendingAddHTLCInfo}; +use crate::ln::channelmanager::{BREAKDOWN_TIMEOUT, MPP_TIMEOUT_TICKS, MIN_CLTV_EXPIRY_DELTA, PaymentId, PaymentSendFailure, RecentPaymentDetails, RecipientOnionFields, HTLCForwardInfo, PendingHTLCRouting, PendingAddHTLCInfo}; use crate::ln::features::Bolt11InvoiceFeatures; use crate::ln::{msgs, ChannelId, PaymentSecret, PaymentPreimage}; use crate::ln::msgs::ChannelMessageHandler; -use crate::ln::outbound_payment::Retry; +use crate::ln::outbound_payment::{IDEMPOTENCY_TIMEOUT_TICKS, Retry}; use crate::routing::gossip::{EffectiveCapacity, RoutingFees}; use crate::routing::router::{get_route, Path, PaymentParameters, Route, Router, RouteHint, RouteHintHop, RouteHop, RouteParameters, find_route}; use crate::routing::scoring::ChannelUsage; From fe2244e9cfae35a23accc2f3eb813c5f865d8958 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 30 Aug 2023 20:22:18 -0500 Subject: [PATCH 04/13] Test for removing stale AwaitingInvoice payments --- lightning/src/ln/outbound_payment.rs | 33 +++++++++++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index aa92043cf91..f8989366509 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -1585,7 +1585,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::{OutboundPayments, Retry, RetryableSendFailure}; + use crate::ln::outbound_payment::{INVOICE_REQUEST_TIMEOUT_TICKS, OutboundPayments, Retry, RetryableSendFailure}; use crate::routing::gossip::NetworkGraph; use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteHop, RouteParameters}; use crate::sync::{Arc, Mutex, RwLock}; @@ -1782,4 +1782,35 @@ mod tests { } else { panic!("Unexpected event"); } if let Event::PaymentFailed { .. } = events[1].0 { } else { panic!("Unexpected event"); } } + + #[test] + fn removes_stale_awaiting_invoice() { + let pending_events = Mutex::new(VecDeque::new()); + let outbound_payments = OutboundPayments::new(); + let payment_id = PaymentId([0; 32]); + + assert!(!outbound_payments.has_pending_payments()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.has_pending_payments()); + + for _ in 0..INVOICE_REQUEST_TIMEOUT_TICKS { + outbound_payments.remove_stale_payments(&pending_events); + assert!(outbound_payments.has_pending_payments()); + assert!(pending_events.lock().unwrap().is_empty()); + } + + outbound_payments.remove_stale_payments(&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).is_ok()); + assert!(outbound_payments.has_pending_payments()); + + assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_err()); + } } From cbeaeb708f413b1abadc7a3dadb2f7cf0a531eae Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 6 Sep 2023 15:17:01 -0500 Subject: [PATCH 05/13] Test removing abandoned AwaitingInvoice payments --- lightning/src/ln/outbound_payment.rs | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index f8989366509..e154b3f0d73 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -1813,4 +1813,26 @@ mod tests { assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_err()); } + + #[test] + fn removes_abandoned_awaiting_invoice() { + let pending_events = Mutex::new(VecDeque::new()); + let outbound_payments = OutboundPayments::new(); + let payment_id = PaymentId([0; 32]); + + assert!(!outbound_payments.has_pending_payments()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.has_pending_payments()); + + outbound_payments.abandon_payment( + payment_id, PaymentFailureReason::UserAbandoned, &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()); + } } From b5169301d8d1b0c74c0b796be0e06acc122b766d Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 30 Aug 2023 12:01:15 -0500 Subject: [PATCH 06/13] Add PendingOutboundPayment::InvoiceReceived When a BOLT 12 invoice has been received, a payment attempt is made and any errors result in abandoning the PendingOutboundPayment. This results in generating at PaymentFailed event, which has a PaymentHash. Thus, when receiving an invoice, transition from AwaitingInvoice to a new InvoiceReceived state, the latter of which contains a PaymentHash such the abandon_payment helper can still be used. --- lightning/src/ln/channelmanager.rs | 7 ++++- lightning/src/ln/outbound_payment.rs | 38 ++++++++++++++++++---------- 2 files changed, 30 insertions(+), 15 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 85fb7852bfe..c7aa0dd021a 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1683,7 +1683,7 @@ pub enum ChannelShutdownState { /// These include payments that have yet to find a successful path, or have unresolved HTLCs. #[derive(Debug, PartialEq)] pub enum RecentPaymentDetails { - /// When an invoice was requested but not yet received, and thus a payment has not been sent. + /// When an invoice was requested and thus a payment has not yet been sent. AwaitingInvoice { /// Identifier for the payment to ensure idempotency. payment_id: PaymentId, @@ -2423,6 +2423,10 @@ where PendingOutboundPayment::AwaitingInvoice { .. } => { Some(RecentPaymentDetails::AwaitingInvoice { payment_id: *payment_id }) }, + // InvoiceReceived is an intermediate state and doesn't need to be exposed + PendingOutboundPayment::InvoiceReceived { .. } => { + Some(RecentPaymentDetails::AwaitingInvoice { payment_id: *payment_id }) + }, PendingOutboundPayment::Retryable { payment_hash, total_msat, .. } => { Some(RecentPaymentDetails::Pending { payment_hash: *payment_hash, @@ -8361,6 +8365,7 @@ where } } PendingOutboundPayment::AwaitingInvoice { .. } => {}, + PendingOutboundPayment::InvoiceReceived { .. } => {}, PendingOutboundPayment::Fulfilled { .. } => {}, PendingOutboundPayment::Abandoned { .. } => {}, } diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index e154b3f0d73..cf60c2cf2fa 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -53,6 +53,9 @@ pub(crate) enum PendingOutboundPayment { AwaitingInvoice { timer_ticks_without_response: u8, }, + InvoiceReceived { + payment_hash: PaymentHash, + }, Retryable { retry_strategy: Option, attempts: PaymentAttempts, @@ -152,6 +155,7 @@ impl PendingOutboundPayment { match self { PendingOutboundPayment::Legacy { .. } => None, PendingOutboundPayment::AwaitingInvoice { .. } => None, + PendingOutboundPayment::InvoiceReceived { payment_hash } => Some(*payment_hash), PendingOutboundPayment::Retryable { payment_hash, .. } => Some(*payment_hash), PendingOutboundPayment::Fulfilled { payment_hash, .. } => *payment_hash, PendingOutboundPayment::Abandoned { payment_hash, .. } => Some(*payment_hash), @@ -165,10 +169,8 @@ impl PendingOutboundPayment { PendingOutboundPayment::Retryable { session_privs, .. } | PendingOutboundPayment::Fulfilled { session_privs, .. } | PendingOutboundPayment::Abandoned { session_privs, .. } => session_privs, - PendingOutboundPayment::AwaitingInvoice { .. } => { - debug_assert!(false); - return; - }, + PendingOutboundPayment::AwaitingInvoice { .. } | + PendingOutboundPayment::InvoiceReceived { .. } => { debug_assert!(false); return; }, }); let payment_hash = self.payment_hash(); *self = PendingOutboundPayment::Fulfilled { session_privs, payment_hash, timer_ticks_without_htlcs: 0 }; @@ -183,6 +185,12 @@ impl PendingOutboundPayment { payment_hash: *payment_hash, reason: Some(reason) }; + } else if let PendingOutboundPayment::InvoiceReceived { payment_hash } = self { + *self = PendingOutboundPayment::Abandoned { + session_privs: HashSet::new(), + payment_hash: *payment_hash, + reason: Some(reason) + }; } } @@ -195,10 +203,8 @@ impl PendingOutboundPayment { PendingOutboundPayment::Abandoned { session_privs, .. } => { session_privs.remove(session_priv) }, - PendingOutboundPayment::AwaitingInvoice { .. } => { - debug_assert!(false); - false - }, + PendingOutboundPayment::AwaitingInvoice { .. } | + PendingOutboundPayment::InvoiceReceived { .. } => { debug_assert!(false); false }, }; if remove_res { if let PendingOutboundPayment::Retryable { ref mut pending_amt_msat, ref mut pending_fee_msat, .. } = self { @@ -217,11 +223,9 @@ impl PendingOutboundPayment { PendingOutboundPayment::Legacy { session_privs } | PendingOutboundPayment::Retryable { session_privs, .. } => { session_privs.insert(session_priv) - } - PendingOutboundPayment::AwaitingInvoice { .. } => { - debug_assert!(false); - false - }, + }, + PendingOutboundPayment::AwaitingInvoice { .. } | + PendingOutboundPayment::InvoiceReceived { .. } => { debug_assert!(false); false }, PendingOutboundPayment::Fulfilled { .. } => false, PendingOutboundPayment::Abandoned { .. } => false, }; @@ -245,6 +249,7 @@ impl PendingOutboundPayment { session_privs.len() }, PendingOutboundPayment::AwaitingInvoice { .. } => 0, + PendingOutboundPayment::InvoiceReceived { .. } => 0, } } } @@ -880,7 +885,9 @@ impl OutboundPayments { log_error!(logger, "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102"); return }, - PendingOutboundPayment::AwaitingInvoice { .. } => { + PendingOutboundPayment::AwaitingInvoice { .. } | + PendingOutboundPayment::InvoiceReceived { .. } => + { log_error!(logger, "Payment not yet sent"); return }, @@ -1573,6 +1580,9 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, (5, AwaitingInvoice) => { (0, timer_ticks_without_response, required), }, + (7, InvoiceReceived) => { + (0, payment_hash, required), + }, ); #[cfg(test)] From 283d9b4e03b8b91fe8448ead94f63b0e1750e48e Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 1 Sep 2023 16:23:27 -0500 Subject: [PATCH 07/13] Refactor OutboundPayments::retry_payment_internal Consolidate the creation and insertion of onion_session_privs to the PendingOutboundPayment::Retryable arm. In an upcoming commit, this method will be reused for an initial BOLT 12 invoice payment. However, onion_session_privs are created using a different helper. --- lightning/src/ln/outbound_payment.rs | 53 ++++++++++++++++------------ 1 file changed, 30 insertions(+), 23 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index cf60c2cf2fa..ddf51cdd796 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -839,12 +839,6 @@ impl OutboundPayments { } } - const RETRY_OVERFLOW_PERCENTAGE: u64 = 10; - let mut onion_session_privs = Vec::with_capacity(route.paths.len()); - for _ in 0..route.paths.len() { - onion_session_privs.push(entropy_source.get_secure_random_bytes()); - } - macro_rules! abandon_with_entry { ($payment: expr, $reason: expr) => { $payment.get_mut().mark_abandoned($reason); @@ -860,26 +854,49 @@ impl OutboundPayments { } } } - let (total_msat, recipient_onion, keysend_preimage) = { + let (total_msat, recipient_onion, keysend_preimage, onion_session_privs) = { let mut outbounds = self.pending_outbound_payments.lock().unwrap(); match outbounds.entry(payment_id) { hash_map::Entry::Occupied(mut payment) => { - let res = match payment.get() { + match payment.get() { PendingOutboundPayment::Retryable { total_msat, keysend_preimage, payment_secret, payment_metadata, custom_tlvs, pending_amt_msat, .. } => { + const RETRY_OVERFLOW_PERCENTAGE: u64 = 10; let retry_amt_msat = route.get_total_amount(); if retry_amt_msat + *pending_amt_msat > *total_msat * (100 + RETRY_OVERFLOW_PERCENTAGE) / 100 { log_error!(logger, "retry_amt_msat of {} will put pending_amt_msat (currently: {}) more than 10% over total_payment_amt_msat of {}", retry_amt_msat, pending_amt_msat, total_msat); abandon_with_entry!(payment, PaymentFailureReason::UnexpectedError); return } - (*total_msat, RecipientOnionFields { - payment_secret: *payment_secret, - payment_metadata: payment_metadata.clone(), - custom_tlvs: custom_tlvs.clone(), - }, *keysend_preimage) + + if !payment.get().is_retryable_now() { + log_error!(logger, "Retries exhausted for payment id {}", &payment_id); + abandon_with_entry!(payment, PaymentFailureReason::RetriesExhausted); + return + } + + let total_msat = *total_msat; + let recipient_onion = RecipientOnionFields { + payment_secret: *payment_secret, + payment_metadata: payment_metadata.clone(), + custom_tlvs: custom_tlvs.clone(), + }; + let keysend_preimage = *keysend_preimage; + + let mut onion_session_privs = Vec::with_capacity(route.paths.len()); + for _ in 0..route.paths.len() { + onion_session_privs.push(entropy_source.get_secure_random_bytes()); + } + + for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) { + assert!(payment.get_mut().insert(*session_priv_bytes, path)); + } + + payment.get_mut().increment_attempts(); + + (total_msat, recipient_onion, keysend_preimage, onion_session_privs) }, PendingOutboundPayment::Legacy { .. } => { log_error!(logger, "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102"); @@ -899,17 +916,7 @@ impl OutboundPayments { log_error!(logger, "Payment already abandoned (with some HTLCs still pending)"); return }, - }; - if !payment.get().is_retryable_now() { - log_error!(logger, "Retries exhausted for payment id {}", &payment_id); - abandon_with_entry!(payment, PaymentFailureReason::RetriesExhausted); - return - } - payment.get_mut().increment_attempts(); - for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) { - assert!(payment.get_mut().insert(*session_priv_bytes, path)); } - res }, hash_map::Entry::Vacant(_) => { log_error!(logger, "Payment with ID {} not found", &payment_id); From 2852e4cdef515d5e0a97340fdc45cd6f5651c384 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Wed, 6 Sep 2023 14:31:57 -0500 Subject: [PATCH 08/13] Rename OutboundPayments::retry_payment_internal It will be used for initial attempts at paying BOLT 12 invoices, so rename it something that covers both that and retries. --- lightning/src/ln/outbound_payment.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index ddf51cdd796..d681fd4421a 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -711,7 +711,7 @@ impl OutboundPayments { } core::mem::drop(outbounds); if let Some((payment_hash, payment_id, route_params)) = retry_id_route_params { - self.retry_payment_internal(payment_hash, payment_id, route_params, router, first_hops(), &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, &send_payment_along_path) + self.find_route_and_send_payment(payment_hash, payment_id, route_params, router, first_hops(), &inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, &send_payment_along_path) } else { break } } @@ -797,7 +797,7 @@ impl OutboundPayments { Ok(()) } - fn retry_payment_internal( + fn find_route_and_send_payment( &self, payment_hash: PaymentHash, payment_id: PaymentId, route_params: RouteParameters, router: &R, first_hops: Vec, inflight_htlcs: &IH, entropy_source: &ES, node_signer: &NS, best_block_height: u32, logger: &L, @@ -950,14 +950,14 @@ impl OutboundPayments { match err { PaymentSendFailure::AllFailedResendSafe(errs) => { Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut route_params, route.paths, errs.into_iter().map(|e| Err(e)), logger, pending_events); - self.retry_payment_internal(payment_hash, payment_id, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path); + self.find_route_and_send_payment(payment_hash, payment_id, route_params, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path); }, PaymentSendFailure::PartialFailure { failed_paths_retry: Some(mut retry), results, .. } => { Self::push_path_failed_evs_and_scids(payment_id, payment_hash, &mut retry, route.paths, results.into_iter(), logger, pending_events); // Some paths were sent, even if we failed to send the full MPP value our recipient may // misbehave and claim the funds, at which point we have to consider the payment sent, so // return `Ok()` here, ignoring any retry errors. - self.retry_payment_internal(payment_hash, payment_id, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path); + self.find_route_and_send_payment(payment_hash, payment_id, retry, router, first_hops, inflight_htlcs, entropy_source, node_signer, best_block_height, logger, pending_events, send_payment_along_path); }, PaymentSendFailure::PartialFailure { failed_paths_retry: None, .. } => { // This may happen if we send a payment and some paths fail, but only due to a temporary @@ -1661,7 +1661,7 @@ mod tests { PaymentId([0; 32]), None, &Route { paths: vec![], route_params: None }, Some(Retry::Attempts(1)), Some(expired_route_params.payment_params.clone()), &&keys_manager, 0).unwrap(); - outbound_payments.retry_payment_internal( + outbound_payments.find_route_and_send_payment( PaymentHash([0; 32]), PaymentId([0; 32]), expired_route_params, &&router, vec![], &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, &|_| Ok(())); @@ -1705,7 +1705,7 @@ mod tests { PaymentId([0; 32]), None, &Route { paths: vec![], route_params: None }, Some(Retry::Attempts(1)), Some(route_params.payment_params.clone()), &&keys_manager, 0).unwrap(); - outbound_payments.retry_payment_internal( + outbound_payments.find_route_and_send_payment( PaymentHash([0; 32]), PaymentId([0; 32]), route_params, &&router, vec![], &|| InFlightHtlcs::new(), &&keys_manager, &&keys_manager, 0, &&logger, &pending_events, &|_| Ok(())); From 4e3c031d5647bac1bdd5cbd9a2b20f1f9dec981c Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 1 Sep 2023 14:01:24 -0500 Subject: [PATCH 09/13] Support paying BOLT 12 invoices Add a send_payment_for_bolt12_invoice method to OutboundPayments for initiating payment of a BOLT 12 invoice. This will be called from an OffersMessageHandler, after which any retries are handled using the Retryable logic. --- lightning/src/ln/outbound_payment.rs | 136 +++++++++++++++++++++------ 1 file changed, 108 insertions(+), 28 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index d681fd4421a..83f83eb5d96 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -18,6 +18,7 @@ use crate::events::{self, PaymentFailureReason}; use crate::ln::{PaymentHash, PaymentPreimage, PaymentSecret}; use crate::ln::channelmanager::{ChannelDetails, EventCompletionAction, HTLCSource, PaymentId}; use crate::ln::onion_utils::{DecodedOnionFailure, HTLCFailReason}; +use crate::offers::invoice::Bolt12Invoice; use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteParameters, Router}; use crate::util::errors::APIError; use crate::util::logger::Logger; @@ -440,6 +441,14 @@ pub enum PaymentSendFailure { }, } +/// An error when attempting to pay a BOLT 12 invoice. +pub(super) enum Bolt12PaymentError { + /// The invoice was not requested. + UnexpectedInvoice, + /// Payment for an invoice with the corresponding [`PaymentId`] was already initiated. + DuplicateInvoice, +} + /// Information which is provided, encrypted, to the payment recipient when sending HTLCs. /// /// This should generally be constructed with data communicated to us from the recipient (via a @@ -577,6 +586,8 @@ pub(super) struct SendAlongPathArgs<'a> { pub session_priv_bytes: [u8; 32], } +const BOLT_12_INVOICE_RETRY_STRATEGY: Retry = Retry::Attempts(3); + pub(super) struct OutboundPayments { pub(super) pending_outbound_payments: Mutex>, pub(super) retry_lock: Mutex<()>, @@ -677,6 +688,45 @@ impl OutboundPayments { } } + #[allow(unused)] + pub(super) fn send_payment_for_bolt12_invoice( + &self, invoice: &Bolt12Invoice, payment_id: PaymentId, router: &R, + first_hops: Vec, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, + best_block_height: u32, logger: &L, + pending_events: &Mutex)>>, + send_payment_along_path: SP, + ) -> Result<(), Bolt12PaymentError> + where + R::Target: Router, + ES::Target: EntropySource, + NS::Target: NodeSigner, + L::Target: Logger, + IH: Fn() -> InFlightHtlcs, + SP: Fn(SendAlongPathArgs) -> Result<(), APIError>, + { + let payment_hash = invoice.payment_hash(); + match self.pending_outbound_payments.lock().unwrap().entry(payment_id) { + hash_map::Entry::Occupied(entry) if entry.get().is_awaiting_invoice() => { + *entry.into_mut() = PendingOutboundPayment::InvoiceReceived { payment_hash }; + }, + hash_map::Entry::Occupied(_) => return Err(Bolt12PaymentError::DuplicateInvoice), + 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(), + }; + + self.find_route_and_send_payment( + payment_hash, payment_id, route_params, router, first_hops, &inflight_htlcs, + entropy_source, node_signer, best_block_height, logger, pending_events, + &send_payment_along_path + ); + + Ok(()) + } + pub(super) fn check_retry_payments( &self, router: &R, first_hops: FH, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, best_block_height: u32, @@ -902,12 +952,26 @@ impl OutboundPayments { log_error!(logger, "Unable to retry payments that were initially sent on LDK versions prior to 0.0.102"); return }, - PendingOutboundPayment::AwaitingInvoice { .. } | - PendingOutboundPayment::InvoiceReceived { .. } => - { + PendingOutboundPayment::AwaitingInvoice { .. } => { log_error!(logger, "Payment not yet sent"); return }, + PendingOutboundPayment::InvoiceReceived { payment_hash } => { + let total_amount = route_params.final_value_msat; + let recipient_onion = RecipientOnionFields { + payment_secret: None, + payment_metadata: None, + custom_tlvs: vec![], + }; + let retry_strategy = Some(BOLT_12_INVOICE_RETRY_STRATEGY); + let payment_params = Some(route_params.payment_params.clone()); + let (retryable_payment, onion_session_privs) = self.create_pending_payment( + *payment_hash, recipient_onion.clone(), None, &route, + retry_strategy, payment_params, entropy_source, best_block_height + ); + *payment.into_mut() = retryable_payment; + (total_amount, recipient_onion, None, onion_session_privs) + }, PendingOutboundPayment::Fulfilled { .. } => { log_error!(logger, "Payment already completed"); return @@ -1070,40 +1134,56 @@ impl OutboundPayments { keysend_preimage: Option, route: &Route, retry_strategy: Option, payment_params: Option, entropy_source: &ES, best_block_height: u32 ) -> Result, PaymentSendFailure> where ES::Target: EntropySource { - let mut onion_session_privs = Vec::with_capacity(route.paths.len()); - for _ in 0..route.paths.len() { - onion_session_privs.push(entropy_source.get_secure_random_bytes()); - } - let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap(); match pending_outbounds.entry(payment_id) { hash_map::Entry::Occupied(_) => Err(PaymentSendFailure::DuplicatePayment), hash_map::Entry::Vacant(entry) => { - let payment = entry.insert(PendingOutboundPayment::Retryable { - retry_strategy, - attempts: PaymentAttempts::new(), - payment_params, - session_privs: HashSet::new(), - pending_amt_msat: 0, - pending_fee_msat: Some(0), - payment_hash, - payment_secret: recipient_onion.payment_secret, - payment_metadata: recipient_onion.payment_metadata, - keysend_preimage, - custom_tlvs: recipient_onion.custom_tlvs, - starting_block_height: best_block_height, - total_msat: route.get_total_amount(), - }); - - for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) { - assert!(payment.insert(*session_priv_bytes, path)); - } - + let (payment, onion_session_privs) = self.create_pending_payment( + payment_hash, recipient_onion, keysend_preimage, route, retry_strategy, + payment_params, entropy_source, best_block_height + ); + entry.insert(payment); Ok(onion_session_privs) }, } } + fn create_pending_payment( + &self, payment_hash: PaymentHash, recipient_onion: RecipientOnionFields, + keysend_preimage: Option, route: &Route, retry_strategy: Option, + payment_params: Option, entropy_source: &ES, best_block_height: u32 + ) -> (PendingOutboundPayment, Vec<[u8; 32]>) + where + ES::Target: EntropySource, + { + let mut onion_session_privs = Vec::with_capacity(route.paths.len()); + for _ in 0..route.paths.len() { + onion_session_privs.push(entropy_source.get_secure_random_bytes()); + } + + let mut payment = PendingOutboundPayment::Retryable { + retry_strategy, + attempts: PaymentAttempts::new(), + payment_params, + session_privs: HashSet::new(), + pending_amt_msat: 0, + pending_fee_msat: Some(0), + payment_hash, + payment_secret: recipient_onion.payment_secret, + payment_metadata: recipient_onion.payment_metadata, + keysend_preimage, + custom_tlvs: recipient_onion.custom_tlvs, + starting_block_height: best_block_height, + total_msat: route.get_total_amount(), + }; + + for (path, session_priv_bytes) in route.paths.iter().zip(onion_session_privs.iter()) { + assert!(payment.insert(*session_priv_bytes, path)); + } + + (payment, onion_session_privs) + } + #[allow(unused)] pub(super) fn add_new_awaiting_invoice(&self, payment_id: PaymentId) -> Result<(), ()> { let mut pending_outbounds = self.pending_outbound_payments.lock().unwrap(); From 19c43d0693aa535455010819170a7c5fe93a43a8 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 31 Aug 2023 17:19:29 -0500 Subject: [PATCH 10/13] pub(crate) visibility for offers/test_utils.rs The test utilities for Offers are needed for testing message handling in ChannelManager and OutboundPayments. --- lightning/src/offers/mod.rs | 2 +- lightning/src/offers/test_utils.rs | 20 ++++++++++---------- 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/lightning/src/offers/mod.rs b/lightning/src/offers/mod.rs index c6883abca34..3593b14f1a8 100644 --- a/lightning/src/offers/mod.rs +++ b/lightning/src/offers/mod.rs @@ -24,4 +24,4 @@ mod payer; pub mod refund; pub(crate) mod signer; #[cfg(test)] -mod test_utils; +pub(crate) mod test_utils; diff --git a/lightning/src/offers/test_utils.rs b/lightning/src/offers/test_utils.rs index f1b3c79edc0..39122472eac 100644 --- a/lightning/src/offers/test_utils.rs +++ b/lightning/src/offers/test_utils.rs @@ -20,33 +20,33 @@ use crate::ln::features::BlindedHopFeatures; use crate::offers::invoice::BlindedPayInfo; use crate::offers::merkle::TaggedHash; -pub(super) fn payer_keys() -> KeyPair { +pub(crate) fn payer_keys() -> KeyPair { let secp_ctx = Secp256k1::new(); KeyPair::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()) } -pub(super) fn payer_sign>(message: &T) -> Result { +pub(crate) fn payer_sign>(message: &T) -> Result { let secp_ctx = Secp256k1::new(); let keys = KeyPair::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[42; 32]).unwrap()); Ok(secp_ctx.sign_schnorr_no_aux_rand(message.as_ref().as_digest(), &keys)) } -pub(super) fn payer_pubkey() -> PublicKey { +pub(crate) fn payer_pubkey() -> PublicKey { payer_keys().public_key() } -pub(super) fn recipient_keys() -> KeyPair { +pub(crate) fn recipient_keys() -> KeyPair { let secp_ctx = Secp256k1::new(); KeyPair::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[43; 32]).unwrap()) } -pub(super) fn recipient_sign>(message: &T) -> Result { +pub(crate) fn recipient_sign>(message: &T) -> Result { let secp_ctx = Secp256k1::new(); let keys = KeyPair::from_secret_key(&secp_ctx, &SecretKey::from_slice(&[43; 32]).unwrap()); Ok(secp_ctx.sign_schnorr_no_aux_rand(message.as_ref().as_digest(), &keys)) } -pub(super) fn recipient_pubkey() -> PublicKey { +pub(crate) fn recipient_pubkey() -> PublicKey { recipient_keys().public_key() } @@ -59,7 +59,7 @@ pub(super) fn privkey(byte: u8) -> SecretKey { SecretKey::from_slice(&[byte; 32]).unwrap() } -pub(super) fn payment_paths() -> Vec<(BlindedPayInfo, BlindedPath)> { +pub(crate) fn payment_paths() -> Vec<(BlindedPayInfo, BlindedPath)> { let paths = vec![ BlindedPath { introduction_node_id: pubkey(40), @@ -101,17 +101,17 @@ pub(super) fn payment_paths() -> Vec<(BlindedPayInfo, BlindedPath)> { payinfo.into_iter().zip(paths.into_iter()).collect() } -pub(super) fn payment_hash() -> PaymentHash { +pub(crate) fn payment_hash() -> PaymentHash { PaymentHash([42; 32]) } -pub(super) fn now() -> Duration { +pub(crate) fn now() -> Duration { std::time::SystemTime::now() .duration_since(std::time::SystemTime::UNIX_EPOCH) .expect("SystemTime::now() should come after SystemTime::UNIX_EPOCH") } -pub(super) struct FixedEntropy; +pub(crate) struct FixedEntropy; impl EntropySource for FixedEntropy { fn get_secure_random_bytes(&self) -> [u8; 32] { From 50336b3c7bcfea979c65fc34443bdd24b6fcc372 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 31 Aug 2023 17:22:31 -0500 Subject: [PATCH 11/13] Add tests for send_payment_for_bolt12_invoice --- lightning/src/ln/outbound_payment.rs | 242 ++++++++++++++++++++++++++- lightning/src/offers/invoice.rs | 2 +- 2 files changed, 242 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 83f83eb5d96..712f33e99f2 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -442,6 +442,7 @@ pub enum PaymentSendFailure { } /// An error when attempting to pay a BOLT 12 invoice. +#[derive(Clone, Debug, PartialEq, Eq)] pub(super) enum Bolt12PaymentError { /// The invoice was not requested. UnexpectedInvoice, @@ -1682,7 +1683,10 @@ 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::{INVOICE_REQUEST_TIMEOUT_TICKS, OutboundPayments, Retry, RetryableSendFailure}; + use crate::ln::outbound_payment::{Bolt12PaymentError, INVOICE_REQUEST_TIMEOUT_TICKS, OutboundPayments, Retry, RetryableSendFailure}; + use crate::offers::invoice::DEFAULT_RELATIVE_EXPIRY; + use crate::offers::offer::OfferBuilder; + use crate::offers::test_utils::*; use crate::routing::gossip::NetworkGraph; use crate::routing::router::{InFlightHtlcs, Path, PaymentParameters, Route, RouteHop, RouteParameters}; use crate::sync::{Arc, Mutex, RwLock}; @@ -1932,4 +1936,240 @@ mod tests { ); assert!(pending_events.lock().unwrap().is_empty()); } + + #[cfg(feature = "std")] + #[test] + fn fails_sending_payment_for_expired_bolt12_invoice() { + let logger = test_utils::TestLogger::new(); + let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger)); + let scorer = RwLock::new(test_utils::TestScorer::new()); + let router = test_utils::TestRouter::new(network_graph, &scorer); + let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); + + let pending_events = Mutex::new(VecDeque::new()); + let outbound_payments = OutboundPayments::new(); + let payment_id = PaymentId([0; 32]); + + assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.has_pending_payments()); + + let created_at = now() - DEFAULT_RELATIVE_EXPIRY; + let invoice = OfferBuilder::new("foo".into(), recipient_pubkey()) + .amount_msats(1000) + .build().unwrap() + .request_invoice(vec![1; 32], payer_pubkey()).unwrap() + .build().unwrap() + .sign(payer_sign).unwrap() + .respond_with_no_std(payment_paths(), payment_hash(), created_at).unwrap() + .build().unwrap() + .sign(recipient_sign).unwrap(); + + assert_eq!( + outbound_payments.send_payment_for_bolt12_invoice( + &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, + &&keys_manager, 0, &&logger, &pending_events, |_| panic!() + ), + Ok(()), + ); + assert!(!outbound_payments.has_pending_payments()); + + let payment_hash = invoice.payment_hash(); + let reason = Some(PaymentFailureReason::PaymentExpired); + + assert!(!pending_events.lock().unwrap().is_empty()); + assert_eq!( + pending_events.lock().unwrap().pop_front(), + Some((Event::PaymentFailed { payment_id, payment_hash, reason }, None)), + ); + assert!(pending_events.lock().unwrap().is_empty()); + } + + #[test] + fn fails_finding_route_for_bolt12_invoice() { + let logger = test_utils::TestLogger::new(); + let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger)); + let scorer = RwLock::new(test_utils::TestScorer::new()); + let router = test_utils::TestRouter::new(network_graph, &scorer); + let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); + + let pending_events = Mutex::new(VecDeque::new()); + let outbound_payments = OutboundPayments::new(); + let payment_id = PaymentId([0; 32]); + + assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.has_pending_payments()); + + let invoice = OfferBuilder::new("foo".into(), recipient_pubkey()) + .amount_msats(1000) + .build().unwrap() + .request_invoice(vec![1; 32], payer_pubkey()).unwrap() + .build().unwrap() + .sign(payer_sign).unwrap() + .respond_with_no_std(payment_paths(), payment_hash(), now()).unwrap() + .build().unwrap() + .sign(recipient_sign).unwrap(); + + router.expect_find_route( + RouteParameters { + payment_params: PaymentParameters::from_bolt12_invoice(&invoice), + final_value_msat: invoice.amount_msats(), + }, + Err(LightningError { err: String::new(), action: ErrorAction::IgnoreError }), + ); + + assert_eq!( + outbound_payments.send_payment_for_bolt12_invoice( + &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, + &&keys_manager, 0, &&logger, &pending_events, |_| panic!() + ), + Ok(()), + ); + assert!(!outbound_payments.has_pending_payments()); + + let payment_hash = invoice.payment_hash(); + let reason = Some(PaymentFailureReason::RouteNotFound); + + assert!(!pending_events.lock().unwrap().is_empty()); + assert_eq!( + pending_events.lock().unwrap().pop_front(), + Some((Event::PaymentFailed { payment_id, payment_hash, reason }, None)), + ); + assert!(pending_events.lock().unwrap().is_empty()); + } + + #[test] + fn fails_paying_for_bolt12_invoice() { + let logger = test_utils::TestLogger::new(); + let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger)); + let scorer = RwLock::new(test_utils::TestScorer::new()); + let router = test_utils::TestRouter::new(network_graph, &scorer); + let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); + + let pending_events = Mutex::new(VecDeque::new()); + let outbound_payments = OutboundPayments::new(); + let payment_id = PaymentId([0; 32]); + + assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.has_pending_payments()); + + let invoice = OfferBuilder::new("foo".into(), recipient_pubkey()) + .amount_msats(1000) + .build().unwrap() + .request_invoice(vec![1; 32], payer_pubkey()).unwrap() + .build().unwrap() + .sign(payer_sign).unwrap() + .respond_with_no_std(payment_paths(), payment_hash(), now()).unwrap() + .build().unwrap() + .sign(recipient_sign).unwrap(); + + let route_params = RouteParameters { + payment_params: PaymentParameters::from_bolt12_invoice(&invoice), + final_value_msat: invoice.amount_msats(), + }; + router.expect_find_route( + route_params.clone(), Ok(Route { paths: vec![], route_params: Some(route_params) }) + ); + + assert_eq!( + outbound_payments.send_payment_for_bolt12_invoice( + &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, + &&keys_manager, 0, &&logger, &pending_events, |_| panic!() + ), + Ok(()), + ); + assert!(!outbound_payments.has_pending_payments()); + + let payment_hash = invoice.payment_hash(); + let reason = Some(PaymentFailureReason::UnexpectedError); + + assert!(!pending_events.lock().unwrap().is_empty()); + assert_eq!( + pending_events.lock().unwrap().pop_front(), + Some((Event::PaymentFailed { payment_id, payment_hash, reason }, None)), + ); + assert!(pending_events.lock().unwrap().is_empty()); + } + + #[test] + fn sends_payment_for_bolt12_invoice() { + let logger = test_utils::TestLogger::new(); + let network_graph = Arc::new(NetworkGraph::new(Network::Testnet, &logger)); + let scorer = RwLock::new(test_utils::TestScorer::new()); + let router = test_utils::TestRouter::new(network_graph, &scorer); + let keys_manager = test_utils::TestKeysInterface::new(&[0; 32], Network::Testnet); + + let pending_events = Mutex::new(VecDeque::new()); + let outbound_payments = OutboundPayments::new(); + let payment_id = PaymentId([0; 32]); + + let invoice = OfferBuilder::new("foo".into(), recipient_pubkey()) + .amount_msats(1000) + .build().unwrap() + .request_invoice(vec![1; 32], payer_pubkey()).unwrap() + .build().unwrap() + .sign(payer_sign).unwrap() + .respond_with_no_std(payment_paths(), payment_hash(), now()).unwrap() + .build().unwrap() + .sign(recipient_sign).unwrap(); + + let route_params = RouteParameters { + payment_params: PaymentParameters::from_bolt12_invoice(&invoice), + final_value_msat: invoice.amount_msats(), + }; + router.expect_find_route( + route_params.clone(), + Ok(Route { + paths: vec![ + Path { + hops: vec![ + RouteHop { + pubkey: recipient_pubkey(), + node_features: NodeFeatures::empty(), + short_channel_id: 42, + channel_features: ChannelFeatures::empty(), + fee_msat: invoice.amount_msats(), + cltv_expiry_delta: 0, + } + ], + blinded_tail: None, + } + ], + route_params: Some(route_params), + }) + ); + + assert!(!outbound_payments.has_pending_payments()); + assert_eq!( + outbound_payments.send_payment_for_bolt12_invoice( + &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, + &&keys_manager, 0, &&logger, &pending_events, |_| panic!() + ), + Err(Bolt12PaymentError::UnexpectedInvoice), + ); + assert!(!outbound_payments.has_pending_payments()); + assert!(pending_events.lock().unwrap().is_empty()); + + assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.has_pending_payments()); + + assert_eq!( + outbound_payments.send_payment_for_bolt12_invoice( + &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, + &&keys_manager, 0, &&logger, &pending_events, |_| Ok(()) + ), + Ok(()), + ); + assert!(outbound_payments.has_pending_payments()); + assert!(pending_events.lock().unwrap().is_empty()); + + assert_eq!( + outbound_payments.send_payment_for_bolt12_invoice( + &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, + &&keys_manager, 0, &&logger, &pending_events, |_| panic!() + ), + Err(Bolt12PaymentError::DuplicateInvoice), + ); + assert!(outbound_payments.has_pending_payments()); + assert!(pending_events.lock().unwrap().is_empty()); + } } diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index 06215e2d486..908d2d4bee6 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -129,7 +129,7 @@ use crate::prelude::*; #[cfg(feature = "std")] use std::time::SystemTime; -const DEFAULT_RELATIVE_EXPIRY: Duration = Duration::from_secs(7200); +pub(crate) const DEFAULT_RELATIVE_EXPIRY: Duration = Duration::from_secs(7200); /// Tag for the hash function used when signing a [`Bolt12Invoice`]'s merkle root. pub const SIGNATURE_TAG: &'static str = concat!("lightning", "invoice", "signature"); From 2f6b5d157a00181e8554383653aac48552dec524 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 5 Sep 2023 15:21:35 -0500 Subject: [PATCH 12/13] Use u32 instead of usize in Retry::Attempts An upcoming commit requires serializing Retry, so use a type with a fixed byte length. Otherwise, using eight bytes to serialize a usize would fail to read on 32-bit machines. --- lightning/src/ln/outbound_payment.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 712f33e99f2..a1c6596784e 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -263,7 +263,7 @@ pub enum Retry { /// Each attempt may be multiple HTLCs along multiple paths if the router decides to split up a /// retry, and may retry multiple failed HTLCs at once if they failed around the same time and /// were retried along a route from a single call to [`Router::find_route_with_id`]. - Attempts(usize), + Attempts(u32), #[cfg(not(feature = "no-std"))] /// Time elapsed before abandoning retries for a payment. At least one attempt at payment is made; /// see [`PaymentParameters::expiry_time`] to avoid any attempt at payment after a specific time. @@ -305,7 +305,7 @@ pub(crate) type PaymentAttempts = PaymentAttemptsUsingTime; pub(crate) struct PaymentAttemptsUsingTime { /// This count will be incremented only after the result of the attempt is known. When it's 0, /// it means the result of the first attempt is not known yet. - pub(crate) count: usize, + pub(crate) count: u32, /// This field is only used when retry is `Retry::Timeout` which is only build with feature std #[cfg(not(feature = "no-std"))] first_attempted_at: T, From 44b9c54fa9b4216a7bcb557ed7b1748d8a47f6e3 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 5 Sep 2023 14:32:53 -0500 Subject: [PATCH 13/13] Configure BOLT 12 invoice payment retry strategy Replace a constant three retry attempts for BOLT 12 invoice payments with a retry strategy specified when creating a pending outbound payment. This is configured by users in a later commit when constructing an InvoiceRequest or a Refund. --- lightning/src/ln/outbound_payment.rs | 59 +++++++++++++++++++--------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index a1c6596784e..0cc9e7e0531 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -53,9 +53,11 @@ pub(crate) enum PendingOutboundPayment { }, AwaitingInvoice { timer_ticks_without_response: u8, + retry_strategy: Retry, }, InvoiceReceived { payment_hash: PaymentHash, + retry_strategy: Retry, }, Retryable { retry_strategy: Option, @@ -156,7 +158,7 @@ impl PendingOutboundPayment { match self { PendingOutboundPayment::Legacy { .. } => None, PendingOutboundPayment::AwaitingInvoice { .. } => None, - PendingOutboundPayment::InvoiceReceived { payment_hash } => Some(*payment_hash), + PendingOutboundPayment::InvoiceReceived { payment_hash, .. } => Some(*payment_hash), PendingOutboundPayment::Retryable { payment_hash, .. } => Some(*payment_hash), PendingOutboundPayment::Fulfilled { payment_hash, .. } => *payment_hash, PendingOutboundPayment::Abandoned { payment_hash, .. } => Some(*payment_hash), @@ -186,7 +188,7 @@ impl PendingOutboundPayment { payment_hash: *payment_hash, reason: Some(reason) }; - } else if let PendingOutboundPayment::InvoiceReceived { payment_hash } = self { + } else if let PendingOutboundPayment::InvoiceReceived { payment_hash, .. } = self { *self = PendingOutboundPayment::Abandoned { session_privs: HashSet::new(), payment_hash: *payment_hash, @@ -272,6 +274,19 @@ pub enum Retry { Timeout(core::time::Duration), } +#[cfg(feature = "no-std")] +impl_writeable_tlv_based_enum!(Retry, + ; + (0, Attempts) +); + +#[cfg(not(feature = "no-std"))] +impl_writeable_tlv_based_enum!(Retry, + ; + (0, Attempts), + (2, Timeout) +); + impl Retry { pub(crate) fn is_retryable_now(&self, attempts: &PaymentAttempts) -> bool { match (self, attempts) { @@ -587,8 +602,6 @@ pub(super) struct SendAlongPathArgs<'a> { pub session_priv_bytes: [u8; 32], } -const BOLT_12_INVOICE_RETRY_STRATEGY: Retry = Retry::Attempts(3); - pub(super) struct OutboundPayments { pub(super) pending_outbound_payments: Mutex>, pub(super) retry_lock: Mutex<()>, @@ -707,10 +720,15 @@ impl OutboundPayments { { let payment_hash = invoice.payment_hash(); match self.pending_outbound_payments.lock().unwrap().entry(payment_id) { - hash_map::Entry::Occupied(entry) if entry.get().is_awaiting_invoice() => { - *entry.into_mut() = PendingOutboundPayment::InvoiceReceived { payment_hash }; + hash_map::Entry::Occupied(entry) => match entry.get() { + PendingOutboundPayment::AwaitingInvoice { retry_strategy, .. } => { + *entry.into_mut() = PendingOutboundPayment::InvoiceReceived { + payment_hash, + retry_strategy: *retry_strategy, + }; + }, + _ => return Err(Bolt12PaymentError::DuplicateInvoice), }, - hash_map::Entry::Occupied(_) => return Err(Bolt12PaymentError::DuplicateInvoice), hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice), }; @@ -957,14 +975,14 @@ impl OutboundPayments { log_error!(logger, "Payment not yet sent"); return }, - PendingOutboundPayment::InvoiceReceived { payment_hash } => { + PendingOutboundPayment::InvoiceReceived { payment_hash, retry_strategy } => { let total_amount = route_params.final_value_msat; let recipient_onion = RecipientOnionFields { payment_secret: None, payment_metadata: None, custom_tlvs: vec![], }; - let retry_strategy = Some(BOLT_12_INVOICE_RETRY_STRATEGY); + let retry_strategy = Some(*retry_strategy); let payment_params = Some(route_params.payment_params.clone()); let (retryable_payment, onion_session_privs) = self.create_pending_payment( *payment_hash, recipient_onion.clone(), None, &route, @@ -1186,13 +1204,16 @@ impl OutboundPayments { } #[allow(unused)] - pub(super) fn add_new_awaiting_invoice(&self, payment_id: PaymentId) -> Result<(), ()> { + pub(super) fn add_new_awaiting_invoice( + &self, payment_id: PaymentId, retry_strategy: Retry + ) -> 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, + retry_strategy, }); Ok(()) @@ -1667,9 +1688,11 @@ impl_writeable_tlv_based_enum_upgradable!(PendingOutboundPayment, }, (5, AwaitingInvoice) => { (0, timer_ticks_without_response, required), + (2, retry_strategy, required), }, (7, InvoiceReceived) => { (0, payment_hash, required), + (2, retry_strategy, required), }, ); @@ -1891,7 +1914,7 @@ mod tests { let payment_id = PaymentId([0; 32]); assert!(!outbound_payments.has_pending_payments()); - assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok()); assert!(outbound_payments.has_pending_payments()); for _ in 0..INVOICE_REQUEST_TIMEOUT_TICKS { @@ -1909,10 +1932,10 @@ mod tests { ); assert!(pending_events.lock().unwrap().is_empty()); - assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok()); assert!(outbound_payments.has_pending_payments()); - assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_err()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_err()); } #[test] @@ -1922,7 +1945,7 @@ mod tests { let payment_id = PaymentId([0; 32]); assert!(!outbound_payments.has_pending_payments()); - assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok()); assert!(outbound_payments.has_pending_payments()); outbound_payments.abandon_payment( @@ -1950,7 +1973,7 @@ mod tests { let outbound_payments = OutboundPayments::new(); let payment_id = PaymentId([0; 32]); - assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok()); assert!(outbound_payments.has_pending_payments()); let created_at = now() - DEFAULT_RELATIVE_EXPIRY; @@ -1996,7 +2019,7 @@ mod tests { let outbound_payments = OutboundPayments::new(); let payment_id = PaymentId([0; 32]); - assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok()); assert!(outbound_payments.has_pending_payments()); let invoice = OfferBuilder::new("foo".into(), recipient_pubkey()) @@ -2049,7 +2072,7 @@ mod tests { let outbound_payments = OutboundPayments::new(); let payment_id = PaymentId([0; 32]); - assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok()); assert!(outbound_payments.has_pending_payments()); let invoice = OfferBuilder::new("foo".into(), recipient_pubkey()) @@ -2149,7 +2172,7 @@ mod tests { assert!(!outbound_payments.has_pending_payments()); assert!(pending_events.lock().unwrap().is_empty()); - assert!(outbound_payments.add_new_awaiting_invoice(payment_id).is_ok()); + assert!(outbound_payments.add_new_awaiting_invoice(payment_id, Retry::Attempts(0)).is_ok()); assert!(outbound_payments.has_pending_payments()); assert_eq!(