From e4ca166e6298eb57d82d635854ca7cdee0d13279 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Mon, 29 Jul 2024 16:10:43 -0500 Subject: [PATCH] Move BOLT12 invoice features check When handling a BOLT12 invoice, and invoice error is sent if the invoice contains unknown required features. However, since the payment is still in state AwaitingInvoice, abandoning it results in losing the reason since an InvoiceRequestFailed event would be generated. Move the check to PendingOutboundPayments such that the payment is first moved to state InvoiceReceived so that a PaymentFailed event is generated instead. --- lightning/src/ln/channelmanager.rs | 57 +++++++++++----------------- lightning/src/ln/outbound_payment.rs | 48 +++++++++++++---------- 2 files changed, 51 insertions(+), 54 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 5ebb3b2a8..617c58c2e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4238,9 +4238,10 @@ where fn send_payment_for_verified_bolt12_invoice(&self, invoice: &Bolt12Invoice, payment_id: PaymentId) -> Result<(), Bolt12PaymentError> { let best_block_height = self.best_block.read().unwrap().height; let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); + let features = self.bolt12_invoice_features(); self.pending_outbound_payments .send_payment_for_bolt12_invoice( - invoice, payment_id, &self.router, self.list_usable_channels(), + invoice, payment_id, &self.router, self.list_usable_channels(), features, || self.compute_inflight_htlcs(), &self.entropy_source, &self.node_signer, &self, &self.secp_ctx, best_block_height, &self.logger, &self.pending_events, |args| self.send_payment_along_path(args) @@ -10845,27 +10846,6 @@ where &self.logger, None, None, Some(invoice.payment_hash()), ); - let features = self.bolt12_invoice_features(); - if invoice.invoice_features().requires_unknown_bits_from(&features) { - log_trace!( - logger, "Invoice requires unknown features: {:?}", - invoice.invoice_features(), - ); - self.abandon_payment_with_reason( - payment_id, PaymentFailureReason::UnknownRequiredFeatures, - ); - - let error = InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures); - let response = match responder { - Some(responder) => responder.respond(OffersMessage::InvoiceError(error)), - None => { - log_trace!(logger, "No reply path to send error: {:?}", error); - ResponseInstruction::NoResponse - }, - }; - return response; - } - if self.default_configuration.manually_handle_bolt12_invoices { let event = Event::InvoiceReceived { payment_id, invoice, context, responder, @@ -10874,24 +10854,31 @@ where return ResponseInstruction::NoResponse; } - match self.send_payment_for_verified_bolt12_invoice(&invoice, payment_id) { - // Payments with SendingFailed error will already have been abandoned. + let error = match self.send_payment_for_verified_bolt12_invoice( + &invoice, payment_id, + ) { + Err(Bolt12PaymentError::UnknownRequiredFeatures) => { + log_trace!( + logger, "Invoice requires unknown features: {:?}", + invoice.invoice_features() + ); + InvoiceError::from(Bolt12SemanticError::UnknownRequiredFeatures) + }, Err(Bolt12PaymentError::SendingFailed(e)) => { log_trace!(logger, "Failed paying invoice: {:?}", e); - - let err = InvoiceError::from_string(format!("{:?}", e)); - match responder { - Some(responder) => responder.respond(OffersMessage::InvoiceError(err)), - None => { - log_trace!(logger, "No reply path to send error: {:?}", err); - ResponseInstruction::NoResponse - }, - } + InvoiceError::from_string(format!("{:?}", e)) }, - // Otherwise, don't abandon unknown, pending, or successful payments. Err(Bolt12PaymentError::UnexpectedInvoice) | Err(Bolt12PaymentError::DuplicateInvoice) - | Ok(()) => ResponseInstruction::NoResponse, + | Ok(()) => return ResponseInstruction::NoResponse, + }; + + match responder { + Some(responder) => responder.respond(OffersMessage::InvoiceError(error)), + None => { + log_trace!(logger, "No reply path to send error: {:?}", error); + ResponseInstruction::NoResponse + }, } }, #[cfg(async_payments)] diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 859b34672..5cc31f605 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -19,6 +19,7 @@ use crate::events::{self, PaymentFailureReason}; use crate::ln::types::{PaymentHash, PaymentPreimage, PaymentSecret}; use crate::ln::channel_state::ChannelDetails; use crate::ln::channelmanager::{EventCompletionAction, HTLCSource, PaymentId}; +use crate::ln::features::Bolt12InvoiceFeatures; use crate::ln::onion_utils; use crate::ln::onion_utils::{DecodedOnionFailure, HTLCFailReason}; use crate::offers::invoice::Bolt12Invoice; @@ -510,6 +511,8 @@ pub enum Bolt12PaymentError { UnexpectedInvoice, /// Payment for an invoice with the corresponding [`PaymentId`] was already initiated. DuplicateInvoice, + /// The invoice was valid for the corresponding [`PaymentId`], but required unknown features. + UnknownRequiredFeatures, /// The invoice was valid for the corresponding [`PaymentId`], but sending the payment failed. SendingFailed(RetryableSendFailure), } @@ -783,9 +786,9 @@ impl OutboundPayments { R: Deref, ES: Deref, NS: Deref, NL: Deref, IH, SP, L: Deref >( &self, invoice: &Bolt12Invoice, payment_id: PaymentId, router: &R, - first_hops: Vec, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS, - node_id_lookup: &NL, secp_ctx: &Secp256k1, best_block_height: u32, - logger: &L, + first_hops: Vec, features: Bolt12InvoiceFeatures, inflight_htlcs: IH, + entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL, + secp_ctx: &Secp256k1, best_block_height: u32, logger: &L, pending_events: &Mutex)>>, send_payment_along_path: SP, ) -> Result<(), Bolt12PaymentError> @@ -819,6 +822,13 @@ impl OutboundPayments { hash_map::Entry::Vacant(_) => return Err(Bolt12PaymentError::UnexpectedInvoice), } + if invoice.invoice_features().requires_unknown_bits_from(&features) { + self.abandon_payment( + payment_id, PaymentFailureReason::UnknownRequiredFeatures, pending_events, + ); + return Err(Bolt12PaymentError::UnknownRequiredFeatures); + } + let mut payment_params = PaymentParameters::from_bolt12_invoice(&invoice); // Advance any blinded path where the introduction node is our node. @@ -1951,7 +1961,7 @@ mod tests { use crate::events::{Event, PathFailure, PaymentFailureReason}; use crate::ln::types::PaymentHash; use crate::ln::channelmanager::{PaymentId, RecipientOnionFields}; - use crate::ln::features::{ChannelFeatures, NodeFeatures}; + use crate::ln::features::{Bolt12InvoiceFeatures, ChannelFeatures, NodeFeatures}; use crate::ln::msgs::{ErrorAction, LightningError}; use crate::ln::outbound_payment::{Bolt12PaymentError, OutboundPayments, Retry, RetryableSendFailure, StaleExpiration}; #[cfg(feature = "std")] @@ -2319,9 +2329,9 @@ mod tests { assert_eq!( outbound_payments.send_payment_for_bolt12_invoice( - &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, - &&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events, - |_| panic!() + &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), + || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, + &secp_ctx, 0, &&logger, &pending_events, |_| panic!() ), Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::PaymentExpired)), ); @@ -2380,9 +2390,9 @@ mod tests { assert_eq!( outbound_payments.send_payment_for_bolt12_invoice( - &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, - &&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events, - |_| panic!() + &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), + || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, + &secp_ctx, 0, &&logger, &pending_events, |_| panic!() ), Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::RouteNotFound)), ); @@ -2454,9 +2464,9 @@ mod tests { 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, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events, - |_| panic!() + &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), + || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, + &secp_ctx, 0, &&logger, &pending_events, |_| panic!() ), Err(Bolt12PaymentError::UnexpectedInvoice), ); @@ -2472,9 +2482,9 @@ mod tests { assert_eq!( outbound_payments.send_payment_for_bolt12_invoice( - &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, - &&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events, - |_| Ok(()) + &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), + || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, + &secp_ctx, 0, &&logger, &pending_events, |_| Ok(()) ), Ok(()), ); @@ -2483,9 +2493,9 @@ mod tests { assert_eq!( outbound_payments.send_payment_for_bolt12_invoice( - &invoice, payment_id, &&router, vec![], || InFlightHtlcs::new(), &&keys_manager, - &&keys_manager, &EmptyNodeIdLookUp {}, &secp_ctx, 0, &&logger, &pending_events, - |_| panic!() + &invoice, payment_id, &&router, vec![], Bolt12InvoiceFeatures::empty(), + || InFlightHtlcs::new(), &&keys_manager, &&keys_manager, &EmptyNodeIdLookUp {}, + &secp_ctx, 0, &&logger, &pending_events, |_| panic!() ), Err(Bolt12PaymentError::DuplicateInvoice), ); -- 2.39.5