diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 30e4df67578..50aa10d11dc 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -8621,8 +8621,10 @@ macro_rules! create_offer_builder { ($self: ident, $builder: ty) => { let entropy = &*$self.entropy_source; let secp_ctx = &$self.secp_ctx; - let path = $self.create_blinded_path_using_absolute_expiry(OffersContext::Unknown {}, absolute_expiry) + let path = $self.create_blinded_paths_using_absolute_expiry(OffersContext::Unknown {}, absolute_expiry) + .and_then(|paths| paths.into_iter().next().ok_or(())) .map_err(|_| Bolt12SemanticError::MissingPaths)?; + let builder = OfferBuilder::deriving_signing_pubkey( node_id, expanded_key, entropy, secp_ctx ) @@ -8694,8 +8696,10 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { let secp_ctx = &$self.secp_ctx; let context = OffersContext::OutboundPayment { payment_id }; - let path = $self.create_blinded_path_using_absolute_expiry(context, Some(absolute_expiry)) + let path = $self.create_blinded_paths_using_absolute_expiry(context, Some(absolute_expiry)) + .and_then(|paths| paths.into_iter().next().ok_or(())) .map_err(|_| Bolt12SemanticError::MissingPaths)?; + let builder = RefundBuilder::deriving_payer_id( node_id, expanded_key, entropy, secp_ctx, amount_msats, payment_id )? @@ -8716,6 +8720,13 @@ macro_rules! create_refund_builder { ($self: ident, $builder: ty) => { } } } +/// Defines the maximum number of [`OffersMessage`] including different reply paths to be sent +/// along different paths. +/// Sending multiple requests increases the chances of successful delivery in case some +/// paths are unavailable. However, only one invoice for a given [`PaymentId`] will be paid, +/// even if multiple invoices are received. +const OFFERS_MESSAGE_REQUEST_LIMIT: usize = 10; + impl ChannelManager where M::Target: chain::Watch<::EcdsaSigner>, @@ -8819,7 +8830,7 @@ where let invoice_request = builder.build_and_sign()?; let context = OffersContext::OutboundPayment { payment_id }; - let reply_path = self.create_blinded_path(context).map_err(|_| Bolt12SemanticError::MissingPaths)?; + let reply_paths = self.create_blinded_paths(context).map_err(|_| Bolt12SemanticError::MissingPaths)?; let _persistence_guard = PersistenceNotifierGuard::notify_on_drop(self); @@ -8832,25 +8843,27 @@ where let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap(); if !offer.paths().is_empty() { - // Send as many invoice requests as there are paths in the offer (with an upper bound). - // Using only one path could result in a failure if the path no longer exists. But only - // one invoice for a given payment id will be paid, even if more than one is received. - const REQUEST_LIMIT: usize = 10; - for path in offer.paths().into_iter().take(REQUEST_LIMIT) { + reply_paths + .iter() + .flat_map(|reply_path| offer.paths().iter().map(move |path| (path, reply_path))) + .take(OFFERS_MESSAGE_REQUEST_LIMIT) + .for_each(|(path, reply_path)| { + let message = new_pending_onion_message( + OffersMessage::InvoiceRequest(invoice_request.clone()), + Destination::BlindedPath(path.clone()), + Some(reply_path.clone()), + ); + pending_offers_messages.push(message); + }); + } else if let Some(signing_pubkey) = offer.signing_pubkey() { + for reply_path in reply_paths { let message = new_pending_onion_message( OffersMessage::InvoiceRequest(invoice_request.clone()), - Destination::BlindedPath(path.clone()), - Some(reply_path.clone()), + Destination::Node(signing_pubkey), + Some(reply_path), ); pending_offers_messages.push(message); } - } else if let Some(signing_pubkey) = offer.signing_pubkey() { - let message = new_pending_onion_message( - OffersMessage::InvoiceRequest(invoice_request), - Destination::Node(signing_pubkey), - Some(reply_path), - ); - pending_offers_messages.push(message); } else { debug_assert!(false); return Err(Bolt12SemanticError::MissingSigningPubkey); @@ -8919,26 +8932,32 @@ where )?; let builder: InvoiceBuilder = builder.into(); let invoice = builder.allow_mpp().build_and_sign(secp_ctx)?; - let reply_path = self.create_blinded_path(OffersContext::Unknown {}) + let reply_paths = self.create_blinded_paths(OffersContext::Unknown {}) .map_err(|_| Bolt12SemanticError::MissingPaths)?; let mut pending_offers_messages = self.pending_offers_messages.lock().unwrap(); if refund.paths().is_empty() { - let message = new_pending_onion_message( - OffersMessage::Invoice(invoice.clone()), - Destination::Node(refund.payer_id()), - Some(reply_path), - ); - pending_offers_messages.push(message); - } else { - for path in refund.paths() { + for reply_path in reply_paths { let message = new_pending_onion_message( OffersMessage::Invoice(invoice.clone()), - Destination::BlindedPath(path.clone()), - Some(reply_path.clone()), + Destination::Node(refund.payer_id()), + Some(reply_path), ); pending_offers_messages.push(message); } + } else { + reply_paths + .iter() + .flat_map(|reply_path| refund.paths().iter().map(move |path| (path, reply_path))) + .take(OFFERS_MESSAGE_REQUEST_LIMIT) + .for_each(|(path, reply_path)| { + let message = new_pending_onion_message( + OffersMessage::Invoice(invoice.clone()), + Destination::BlindedPath(path.clone()), + Some(reply_path.clone()), + ); + pending_offers_messages.push(message); + }); } Ok(invoice) @@ -9045,22 +9064,22 @@ where inbound_payment::get_payment_preimage(payment_hash, payment_secret, &self.inbound_payment_key) } - /// Creates a blinded path by delegating to [`MessageRouter`] based on the path's intended - /// lifetime. + /// Creates a collection of blinded paths by delegating to [`MessageRouter`] based on + /// the path's intended lifetime. /// /// Whether or not the path is compact depends on whether the path is short-lived or long-lived, /// respectively, based on the given `absolute_expiry` as seconds since the Unix epoch. See /// [`MAX_SHORT_LIVED_RELATIVE_EXPIRY`]. - fn create_blinded_path_using_absolute_expiry( + fn create_blinded_paths_using_absolute_expiry( &self, context: OffersContext, absolute_expiry: Option, - ) -> Result { + ) -> Result, ()> { let now = self.duration_since_epoch(); let max_short_lived_absolute_expiry = now.saturating_add(MAX_SHORT_LIVED_RELATIVE_EXPIRY); if absolute_expiry.unwrap_or(Duration::MAX) <= max_short_lived_absolute_expiry { - self.create_compact_blinded_path(context) + self.create_compact_blinded_paths(context) } else { - self.create_blinded_path(context) + self.create_blinded_paths(context) } } @@ -9077,10 +9096,11 @@ where now } - /// Creates a blinded path by delegating to [`MessageRouter::create_blinded_paths`]. + /// Creates a collection of blinded paths by delegating to + /// [`MessageRouter::create_blinded_paths`]. /// - /// Errors if the `MessageRouter` errors or returns an empty `Vec`. - fn create_blinded_path(&self, context: OffersContext) -> Result { + /// Errors if the `MessageRouter` errors. + fn create_blinded_paths(&self, context: OffersContext) -> Result, ()> { let recipient = self.get_our_node_id(); let secp_ctx = &self.secp_ctx; @@ -9094,13 +9114,14 @@ where self.router .create_blinded_paths(recipient, MessageContext::Offers(context), peers, secp_ctx) - .and_then(|paths| paths.into_iter().next().ok_or(())) + .and_then(|paths| (!paths.is_empty()).then(|| paths).ok_or(())) } - /// Creates a blinded path by delegating to [`MessageRouter::create_compact_blinded_paths`]. + /// Creates a collection of blinded paths by delegating to + /// [`MessageRouter::create_compact_blinded_paths`]. /// - /// Errors if the `MessageRouter` errors or returns an empty `Vec`. - fn create_compact_blinded_path(&self, context: OffersContext) -> Result { + /// Errors if the `MessageRouter` errors. + fn create_compact_blinded_paths(&self, context: OffersContext) -> Result, ()> { let recipient = self.get_our_node_id(); let secp_ctx = &self.secp_ctx; @@ -9121,7 +9142,7 @@ where self.router .create_compact_blinded_paths(recipient, MessageContext::Offers(context), peers, secp_ctx) - .and_then(|paths| paths.into_iter().next().ok_or(())) + .and_then(|paths| (!paths.is_empty()).then(|| paths).ok_or(())) } /// Creates multi-hop blinded payment paths for the given `amount_msats` by delegating to diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index 83ff8ce6d5c..cdd78d02ca8 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -205,12 +205,12 @@ fn extract_invoice_request<'a, 'b, 'c>( } } -fn extract_invoice<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, message: &OnionMessage) -> Bolt12Invoice { +fn extract_invoice<'a, 'b, 'c>(node: &Node<'a, 'b, 'c>, message: &OnionMessage) -> (Bolt12Invoice, Option) { match node.onion_messenger.peel_onion_message(message) { - Ok(PeeledOnion::Receive(message, _, _)) => match message { + Ok(PeeledOnion::Receive(message, _, reply_path)) => match message { ParsedOnionMessageContents::Offers(offers_message) => match offers_message { OffersMessage::InvoiceRequest(invoice_request) => panic!("Unexpected invoice_request: {:?}", invoice_request), - OffersMessage::Invoice(invoice) => invoice, + OffersMessage::Invoice(invoice) => (invoice, reply_path), #[cfg(async_payments)] OffersMessage::StaticInvoice(invoice) => panic!("Unexpected static invoice: {:?}", invoice), OffersMessage::InvoiceError(error) => panic!("Unexpected invoice_error: {:?}", error), @@ -566,7 +566,7 @@ fn creates_and_pays_for_offer_using_two_hop_blinded_path() { let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap(); david.onion_messenger.handle_onion_message(&charlie_id, &onion_message); - let invoice = extract_invoice(david, &onion_message); + let (invoice, _) = extract_invoice(david, &onion_message); assert_eq!(invoice.amount_msats(), 10_000_000); assert_ne!(invoice.signing_pubkey(), alice_id); assert!(!invoice.payment_paths().is_empty()); @@ -645,7 +645,7 @@ fn creates_and_pays_for_refund_using_two_hop_blinded_path() { let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap(); david.onion_messenger.handle_onion_message(&charlie_id, &onion_message); - let invoice = extract_invoice(david, &onion_message); + let (invoice, _) = extract_invoice(david, &onion_message); assert_eq!(invoice, expected_invoice); assert_eq!(invoice.amount_msats(), 10_000_000); @@ -712,7 +712,7 @@ fn creates_and_pays_for_offer_using_one_hop_blinded_path() { let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); bob.onion_messenger.handle_onion_message(&alice_id, &onion_message); - let invoice = extract_invoice(bob, &onion_message); + let (invoice, _) = extract_invoice(bob, &onion_message); assert_eq!(invoice.amount_msats(), 10_000_000); assert_ne!(invoice.signing_pubkey(), alice_id); assert!(!invoice.payment_paths().is_empty()); @@ -765,7 +765,7 @@ fn creates_and_pays_for_refund_using_one_hop_blinded_path() { let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); bob.onion_messenger.handle_onion_message(&alice_id, &onion_message); - let invoice = extract_invoice(bob, &onion_message); + let (invoice, _) = extract_invoice(bob, &onion_message); assert_eq!(invoice, expected_invoice); assert_eq!(invoice.amount_msats(), 10_000_000); @@ -827,7 +827,7 @@ fn pays_for_offer_without_blinded_paths() { let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); bob.onion_messenger.handle_onion_message(&alice_id, &onion_message); - let invoice = extract_invoice(bob, &onion_message); + let (invoice, _) = extract_invoice(bob, &onion_message); route_bolt12_payment(bob, &[alice], &invoice); expect_recent_payment!(bob, RecentPaymentDetails::Pending, payment_id); @@ -868,7 +868,7 @@ fn pays_for_refund_without_blinded_paths() { let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); bob.onion_messenger.handle_onion_message(&alice_id, &onion_message); - let invoice = extract_invoice(bob, &onion_message); + let (invoice, _) = extract_invoice(bob, &onion_message); assert_eq!(invoice, expected_invoice); route_bolt12_payment(bob, &[alice], &invoice); @@ -878,6 +878,170 @@ fn pays_for_refund_without_blinded_paths() { expect_recent_payment!(bob, RecentPaymentDetails::Fulfilled, payment_id); } +/// This test checks that when multiple potential introduction nodes are available for the payer, +/// multiple `invoice_request` messages are sent for the offer, each with a different `reply_path`. +#[test] +fn send_invoice_requests_with_distinct_reply_path() { + let mut accept_forward_cfg = test_default_channel_config(); + accept_forward_cfg.accept_forwards_to_priv_channels = true; + + let mut features = channelmanager::provided_init_features(&accept_forward_cfg); + features.set_onion_messages_optional(); + features.set_route_blinding_optional(); + + let chanmon_cfgs = create_chanmon_cfgs(7); + let node_cfgs = create_node_cfgs(7, &chanmon_cfgs); + + *node_cfgs[1].override_init_features.borrow_mut() = Some(features); + + let node_chanmgrs = create_node_chanmgrs( + 7, &node_cfgs, &[None, Some(accept_forward_cfg), None, None, None, None, None] + ); + let nodes = create_network(7, &node_cfgs, &node_chanmgrs); + + create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000); + create_unannounced_chan_between_nodes_with_value(&nodes, 2, 3, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 5, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 5, 10_000_000, 1_000_000_000); + + // Introduce another potential introduction node, node[6], as a candidate + create_unannounced_chan_between_nodes_with_value(&nodes, 3, 6, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 6, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 4, 6, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 5, 6, 10_000_000, 1_000_000_000); + + let (alice, bob, charlie, david) = (&nodes[0], &nodes[1], &nodes[2], &nodes[3]); + let alice_id = alice.node.get_our_node_id(); + let bob_id = bob.node.get_our_node_id(); + let charlie_id = charlie.node.get_our_node_id(); + let david_id = david.node.get_our_node_id(); + + disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5], &nodes[6]]); + disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); + + let offer = alice.node + .create_offer_builder(None) + .unwrap() + .amount_msats(10_000_000) + .build().unwrap(); + assert_ne!(offer.signing_pubkey(), Some(alice_id)); + assert!(!offer.paths().is_empty()); + for path in offer.paths() { + assert_eq!(path.introduction_node, IntroductionNode::NodeId(bob_id)); + } + + let payment_id = PaymentId([1; 32]); + david.node.pay_for_offer(&offer, None, None, None, payment_id, Retry::Attempts(0), None) + .unwrap(); + expect_recent_payment!(david, RecentPaymentDetails::AwaitingInvoice, payment_id); + connect_peers(david, bob); + + // Send, extract and verify the first Invoice Request message + let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); + bob.onion_messenger.handle_onion_message(&david_id, &onion_message); + + connect_peers(alice, charlie); + + let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap(); + alice.onion_messenger.handle_onion_message(&bob_id, &onion_message); + + let (_, reply_path) = extract_invoice_request(alice, &onion_message); + assert_eq!(reply_path.introduction_node, IntroductionNode::NodeId(charlie_id)); + + // Send, extract and verify the second Invoice Request message + let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); + bob.onion_messenger.handle_onion_message(&david_id, &onion_message); + + let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap(); + alice.onion_messenger.handle_onion_message(&bob_id, &onion_message); + + let (_, reply_path) = extract_invoice_request(alice, &onion_message); + assert_eq!(reply_path.introduction_node, IntroductionNode::NodeId(nodes[6].node.get_our_node_id())); +} + +/// This test checks that when multiple potential introduction nodes are available for the payee, +/// multiple `Invoice` messages are sent for the Refund, each with a different `reply_path`. +#[test] +fn send_invoice_for_refund_with_distinct_reply_path() { + let mut accept_forward_cfg = test_default_channel_config(); + accept_forward_cfg.accept_forwards_to_priv_channels = true; + + let mut features = channelmanager::provided_init_features(&accept_forward_cfg); + features.set_onion_messages_optional(); + features.set_route_blinding_optional(); + + let chanmon_cfgs = create_chanmon_cfgs(7); + let node_cfgs = create_node_cfgs(7, &chanmon_cfgs); + + *node_cfgs[1].override_init_features.borrow_mut() = Some(features); + + let node_chanmgrs = create_node_chanmgrs( + 7, &node_cfgs, &[None, Some(accept_forward_cfg), None, None, None, None, None] + ); + let nodes = create_network(7, &node_cfgs, &node_chanmgrs); + + create_unannounced_chan_between_nodes_with_value(&nodes, 0, 1, 10_000_000, 1_000_000_000); + create_unannounced_chan_between_nodes_with_value(&nodes, 2, 3, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 1, 5, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 4, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 5, 10_000_000, 1_000_000_000); + + // Introduce another potential introduction node, node[6], as a candidate + create_unannounced_chan_between_nodes_with_value(&nodes, 3, 6, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 2, 6, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 4, 6, 10_000_000, 1_000_000_000); + create_announced_chan_between_nodes_with_value(&nodes, 5, 6, 10_000_000, 1_000_000_000); + + let (alice, bob, charlie, david) = (&nodes[0], &nodes[1], &nodes[2], &nodes[3]); + let alice_id = alice.node.get_our_node_id(); + let bob_id = bob.node.get_our_node_id(); + let charlie_id = charlie.node.get_our_node_id(); + let david_id = david.node.get_our_node_id(); + + disconnect_peers(alice, &[charlie, david, &nodes[4], &nodes[5], &nodes[6]]); + disconnect_peers(david, &[bob, &nodes[4], &nodes[5]]); + + let absolute_expiry = Duration::from_secs(u64::MAX); + let payment_id = PaymentId([1; 32]); + let refund = alice.node + .create_refund_builder(10_000_000, absolute_expiry, payment_id, Retry::Attempts(0), None) + .unwrap() + .build().unwrap(); + assert_ne!(refund.payer_id(), alice_id); + for path in refund.paths() { + assert_eq!(path.introduction_node, IntroductionNode::NodeId(bob_id)); + } + expect_recent_payment!(alice, RecentPaymentDetails::AwaitingInvoice, payment_id); + + let _expected_invoice = david.node.request_refund_payment(&refund).unwrap(); + + connect_peers(david, bob); + + let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); + bob.onion_messenger.handle_onion_message(&david_id, &onion_message); + + connect_peers(alice, charlie); + + let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap(); + + let (_, reply_path) = extract_invoice(alice, &onion_message); + assert_eq!(reply_path.unwrap().introduction_node, IntroductionNode::NodeId(charlie_id)); + + // Send, extract and verify the second Invoice Request message + let onion_message = david.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); + bob.onion_messenger.handle_onion_message(&david_id, &onion_message); + + let onion_message = bob.onion_messenger.next_onion_message_for_peer(alice_id).unwrap(); + + let (_, reply_path) = extract_invoice(alice, &onion_message); + assert_eq!(reply_path.unwrap().introduction_node, IntroductionNode::NodeId(nodes[6].node.get_our_node_id())); +} + /// Checks that a deferred invoice can be paid asynchronously from an Event::InvoiceReceived. #[test] fn pays_bolt12_invoice_asynchronously() { @@ -1012,7 +1176,7 @@ fn creates_offer_with_blinded_path_using_unannounced_introduction_node() { let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); bob.onion_messenger.handle_onion_message(&alice_id, &onion_message); - let invoice = extract_invoice(bob, &onion_message); + let (invoice, _) = extract_invoice(bob, &onion_message); assert_ne!(invoice.signing_pubkey(), alice_id); assert!(!invoice.payment_paths().is_empty()); for (_, path) in invoice.payment_paths() { @@ -1061,7 +1225,7 @@ fn creates_refund_with_blinded_path_using_unannounced_introduction_node() { let onion_message = alice.onion_messenger.next_onion_message_for_peer(bob_id).unwrap(); - let invoice = extract_invoice(bob, &onion_message); + let (invoice, _) = extract_invoice(bob, &onion_message); assert_eq!(invoice, expected_invoice); assert_ne!(invoice.signing_pubkey(), alice_id); assert!(!invoice.payment_paths().is_empty()); @@ -1525,7 +1689,7 @@ fn fails_paying_invoice_more_than_once() { // David pays the first invoice let payment_context = PaymentContext::Bolt12Refund(Bolt12RefundContext {}); - let invoice1 = extract_invoice(david, &onion_message); + let (invoice1, _) = extract_invoice(david, &onion_message); route_bolt12_payment(david, &[charlie, bob, alice], &invoice1); expect_recent_payment!(david, RecentPaymentDetails::Pending, payment_id); @@ -1547,7 +1711,7 @@ fn fails_paying_invoice_more_than_once() { let onion_message = charlie.onion_messenger.next_onion_message_for_peer(david_id).unwrap(); david.onion_messenger.handle_onion_message(&charlie_id, &onion_message); - let invoice2 = extract_invoice(david, &onion_message); + let (invoice2, _) = extract_invoice(david, &onion_message); assert_eq!(invoice1.payer_metadata(), invoice2.payer_metadata()); // David sends an error instead of paying the second invoice