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.

diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 5ebb3b2a8b3ff36c69d2abdb61d4f85d536336f7..617c58c2e4887c2e34a994c80bb326801e37dc10 100644 (file)
--- 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 859b346729625c92f89bec23b509a864531934d1..5cc31f60500922d72058d4f05ccb8cc2dad8d787 100644 (file)
--- 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<ChannelDetails>, inflight_htlcs: IH, entropy_source: &ES, node_signer: &NS,
-               node_id_lookup: &NL, secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32,
-               logger: &L,
+               first_hops: Vec<ChannelDetails>, features: Bolt12InvoiceFeatures, inflight_htlcs: IH,
+               entropy_source: &ES, node_signer: &NS, node_id_lookup: &NL,
+               secp_ctx: &Secp256k1<secp256k1::All>, best_block_height: u32, logger: &L,
                pending_events: &Mutex<VecDeque<(events::Event, Option<EventCompletionAction>)>>,
                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),
                );