From 708e8b538d81bfb34986a65b4cd8e8cf2895f46a Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 6 Jan 2025 14:10:57 -0500 Subject: [PATCH 1/6] Tests: DRY failing a blinded HTLC backwards Blinded HTLCs are always failed back with the same error, so DRY the test code that fails them backwards. This util will also be used for async payments testing in upcoming commits. --- lightning/src/ln/blinded_payment_tests.rs | 70 +++++++++++------------ 1 file changed, 34 insertions(+), 36 deletions(-) diff --git a/lightning/src/ln/blinded_payment_tests.rs b/lightning/src/ln/blinded_payment_tests.rs index 813ee1dcf07..9708dbd6d88 100644 --- a/lightning/src/ln/blinded_payment_tests.rs +++ b/lightning/src/ln/blinded_payment_tests.rs @@ -105,6 +105,37 @@ pub fn get_blinded_route_parameters( ) } +pub fn fail_blinded_htlc_backwards( + payment_hash: PaymentHash, intro_node_idx: usize, nodes: &[&Node], +) { + for i in (0..nodes.len()).rev() { + match i { + 0 => { + let mut payment_failed_conditions = PaymentFailedConditions::new() + .expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32]); + expect_payment_failed_conditions(&nodes[0], payment_hash, false, payment_failed_conditions); + }, + i if i <= intro_node_idx => { + let unblinded_node_updates = get_htlc_update_msgs!(nodes[i], nodes[i-1].node.get_our_node_id()); + assert_eq!(unblinded_node_updates.update_fail_htlcs.len(), 1); + nodes[i-1].node.handle_update_fail_htlc( + nodes[i].node.get_our_node_id(), &unblinded_node_updates.update_fail_htlcs[i-1] + ); + do_commitment_signed_dance(&nodes[i-1], &nodes[i], &unblinded_node_updates.commitment_signed, false, false); + }, + _ => { + let blinded_node_updates = get_htlc_update_msgs!(nodes[i], nodes[i-1].node.get_our_node_id()); + assert_eq!(blinded_node_updates.update_fail_malformed_htlcs.len(), 1); + let update_malformed = &blinded_node_updates.update_fail_malformed_htlcs[0]; + assert_eq!(update_malformed.sha256_of_onion, [0; 32]); + assert_eq!(update_malformed.failure_code, INVALID_ONION_BLINDING); + nodes[i-1].node.handle_update_fail_malformed_htlc(nodes[i].node.get_our_node_id(), update_malformed); + do_commitment_signed_dance(&nodes[i-1], &nodes[i], &blinded_node_updates.commitment_signed, true, false); + } + } + } +} + #[test] fn one_hop_blinded_path() { do_one_hop_blinded_path(true); @@ -572,13 +603,8 @@ fn do_forward_fail_in_process_pending_htlc_fwds(check: ProcessPendingHTLCsCheck, if intro_fails { cause_error!(nodes[0], nodes[1], nodes[2], chan_id_1_2, chan_upd_1_2.short_channel_id); - let mut updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); - nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]); check_added_monitors!(nodes[1], 1); - do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false); - - expect_payment_failed_conditions(&nodes[0], payment_hash, false, - PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32])); + fail_blinded_htlc_backwards(payment_hash, 1, &[&nodes[0], &nodes[1]]); return } @@ -669,14 +695,8 @@ fn do_blinded_intercept_payment(intercept_node_fails: bool) { nodes[1].node.fail_intercepted_htlc(intercept_id).unwrap(); expect_pending_htlcs_forwardable_and_htlc_handling_failed_ignore!(nodes[1], vec![HTLCDestination::UnknownNextHop { requested_forward_scid: intercept_scid }]); nodes[1].node.process_pending_htlc_forwards(); - let update_fail = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); check_added_monitors!(&nodes[1], 1); - assert!(update_fail.update_fail_htlcs.len() == 1); - let fail_msg = update_fail.update_fail_htlcs[0].clone(); - nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &fail_msg); - commitment_signed_dance!(nodes[0], nodes[1], update_fail.commitment_signed, false); - expect_payment_failed_conditions(&nodes[0], payment_hash, false, - PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32])); + fail_blinded_htlc_backwards(payment_hash, 1, &[&nodes[0], &nodes[1]]); return } @@ -782,29 +802,7 @@ fn three_hop_blinded_path_fail() { ); nodes[3].node.process_pending_htlc_forwards(); check_added_monitors!(nodes[3], 1); - - let updates_3_2 = get_htlc_update_msgs!(nodes[3], nodes[2].node.get_our_node_id()); - assert_eq!(updates_3_2.update_fail_malformed_htlcs.len(), 1); - let update_malformed = &updates_3_2.update_fail_malformed_htlcs[0]; - assert_eq!(update_malformed.sha256_of_onion, [0; 32]); - assert_eq!(update_malformed.failure_code, INVALID_ONION_BLINDING); - nodes[2].node.handle_update_fail_malformed_htlc(nodes[3].node.get_our_node_id(), update_malformed); - do_commitment_signed_dance(&nodes[2], &nodes[3], &updates_3_2.commitment_signed, true, false); - - let updates_2_1 = get_htlc_update_msgs!(nodes[2], nodes[1].node.get_our_node_id()); - assert_eq!(updates_2_1.update_fail_malformed_htlcs.len(), 1); - let update_malformed = &updates_2_1.update_fail_malformed_htlcs[0]; - assert_eq!(update_malformed.sha256_of_onion, [0; 32]); - assert_eq!(update_malformed.failure_code, INVALID_ONION_BLINDING); - nodes[1].node.handle_update_fail_malformed_htlc(nodes[2].node.get_our_node_id(), update_malformed); - do_commitment_signed_dance(&nodes[1], &nodes[2], &updates_2_1.commitment_signed, true, false); - - let updates_1_0 = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); - assert_eq!(updates_1_0.update_fail_htlcs.len(), 1); - nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates_1_0.update_fail_htlcs[0]); - do_commitment_signed_dance(&nodes[0], &nodes[1], &updates_1_0.commitment_signed, false, false); - expect_payment_failed_conditions(&nodes[0], payment_hash, false, - PaymentFailedConditions::new().expected_htlc_error_data(INVALID_ONION_BLINDING, &[0; 32])); + fail_blinded_htlc_backwards(payment_hash, 1, &[&nodes[0], &nodes[1], &nodes[2], &nodes[3]]); } #[derive(PartialEq)] From da8cb9a71de41017f9121575077ecf4aa424a3ab Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 7 Jan 2025 23:36:25 -0500 Subject: [PATCH 2/6] Tests: DRY static invoice creation --- lightning/src/ln/async_payments_tests.rs | 94 ++++++++++-------------- 1 file changed, 37 insertions(+), 57 deletions(-) diff --git a/lightning/src/ln/async_payments_tests.rs b/lightning/src/ln/async_payments_tests.rs index 424d76da6c2..01135f625cd 100644 --- a/lightning/src/ln/async_payments_tests.rs +++ b/lightning/src/ln/async_payments_tests.rs @@ -20,6 +20,8 @@ use crate::ln::offers_tests; use crate::ln::onion_utils::INVALID_ONION_BLINDING; use crate::ln::outbound_payment::Retry; use crate::offers::nonce::Nonce; +use crate::offers::offer::Offer; +use crate::offers::static_invoice::StaticInvoice; use crate::onion_message::async_payments::{ AsyncPaymentsMessage, AsyncPaymentsMessageHandler, ReleaseHeldHtlc, }; @@ -32,11 +34,39 @@ use crate::sign::NodeSigner; use crate::types::features::Bolt12InvoiceFeatures; use crate::types::payment::{PaymentPreimage, PaymentSecret}; use crate::util::config::UserConfig; +use bitcoin::secp256k1; use bitcoin::secp256k1::Secp256k1; use core::convert::Infallible; use core::time::Duration; +fn create_static_invoice( + always_online_counterparty: &Node, recipient: &Node, relative_expiry: Option, + secp_ctx: &Secp256k1, +) -> (Offer, StaticInvoice) { + let blinded_paths_to_always_online_node = always_online_counterparty + .message_router + .create_blinded_paths( + always_online_counterparty.node.get_our_node_id(), + MessageContext::Offers(OffersContext::InvoiceRequest { nonce: Nonce([42; 16]) }), + Vec::new(), + &secp_ctx, + ) + .unwrap(); + let (offer_builder, offer_nonce) = recipient + .node + .create_async_receive_offer_builder(blinded_paths_to_always_online_node) + .unwrap(); + let offer = offer_builder.build().unwrap(); + let static_invoice = recipient + .node + .create_static_invoice_builder(&offer, offer_nonce, relative_expiry) + .unwrap() + .build_and_sign(&secp_ctx) + .unwrap(); + (offer, static_invoice) +} + #[test] fn blinded_keysend() { let chanmon_cfgs = create_chanmon_cfgs(3); @@ -362,20 +392,8 @@ fn ignore_unexpected_static_invoice() { create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0); // Initiate payment to the sender's intended offer. - let blinded_paths_to_always_online_node = nodes[1] - .message_router - .create_blinded_paths( - nodes[1].node.get_our_node_id(), - MessageContext::Offers(OffersContext::InvoiceRequest { nonce: Nonce([42; 16]) }), - Vec::new(), - &secp_ctx, - ) - .unwrap(); - let (offer_builder, offer_nonce) = nodes[2] - .node - .create_async_receive_offer_builder(blinded_paths_to_always_online_node.clone()) - .unwrap(); - let offer = offer_builder.build().unwrap(); + let (offer, valid_static_invoice) = + create_static_invoice(&nodes[1], &nodes[2], None, &secp_ctx); let amt_msat = 5000; let payment_id = PaymentId([1; 32]); nodes[0] @@ -393,20 +411,7 @@ fn ignore_unexpected_static_invoice() { // Create a static invoice to be sent over the reply path containing the original payment_id, but // the static invoice corresponds to a different offer than was originally paid. - let unexpected_static_invoice = { - let (offer_builder, nonce) = nodes[2] - .node - .create_async_receive_offer_builder(blinded_paths_to_always_online_node) - .unwrap(); - let sender_unintended_offer = offer_builder.build().unwrap(); - - nodes[2] - .node - .create_static_invoice_builder(&sender_unintended_offer, nonce, None) - .unwrap() - .build_and_sign(&secp_ctx) - .unwrap() - }; + let unexpected_static_invoice = create_static_invoice(&nodes[1], &nodes[2], None, &secp_ctx).1; // Check that we'll ignore the unexpected static invoice. nodes[1] @@ -433,13 +438,6 @@ fn ignore_unexpected_static_invoice() { // A valid static invoice corresponding to the correct offer will succeed and cause us to send a // held_htlc_available onion message. - let valid_static_invoice = nodes[2] - .node - .create_static_invoice_builder(&offer, offer_nonce, None) - .unwrap() - .build_and_sign(&secp_ctx) - .unwrap(); - nodes[1] .onion_messenger .send_onion_message( @@ -499,32 +497,14 @@ fn pays_static_invoice() { create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0); - let blinded_paths_to_always_online_node = nodes[1] - .message_router - .create_blinded_paths( - nodes[1].node.get_our_node_id(), - MessageContext::Offers(OffersContext::InvoiceRequest { nonce: Nonce([42; 16]) }), - Vec::new(), - &secp_ctx, - ) - .unwrap(); - let (offer_builder, offer_nonce) = nodes[2] - .node - .create_async_receive_offer_builder(blinded_paths_to_always_online_node) - .unwrap(); - let offer = offer_builder.build().unwrap(); - let amt_msat = 5000; - let payment_id = PaymentId([1; 32]); let relative_expiry = Duration::from_secs(1000); - let static_invoice = nodes[2] - .node - .create_static_invoice_builder(&offer, offer_nonce, Some(relative_expiry)) - .unwrap() - .build_and_sign(&secp_ctx) - .unwrap(); + let (offer, static_invoice) = + create_static_invoice(&nodes[1], &nodes[2], Some(relative_expiry), &secp_ctx); assert!(static_invoice.invoice_features().supports_basic_mpp()); assert_eq!(static_invoice.relative_expiry(), relative_expiry); + let amt_msat = 5000; + let payment_id = PaymentId([1; 32]); nodes[0] .node .pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), None) From 0611e6065b3a1c8323abdc542f7767e8debac69e Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Wed, 4 Dec 2024 14:00:22 -0500 Subject: [PATCH 3/6] Add handle_held_htlc_available MessageContext param Needed to authenticate that the held_htlc_available message is being sent over a reply path that we originally created and that isn't expired before we reply with release_held_htlc. This context will be used in upcoming commits when we add support for async receive. --- fuzz/src/onion_message.rs | 3 ++- lightning/src/ln/channelmanager.rs | 3 ++- lightning/src/ln/peer_handler.rs | 3 ++- lightning/src/onion_message/async_payments.rs | 3 ++- lightning/src/onion_message/functional_tests.rs | 3 ++- lightning/src/onion_message/messenger.rs | 10 +++++++++- 6 files changed, 19 insertions(+), 6 deletions(-) diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index 94da4d09be5..10a667fb594 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -122,7 +122,8 @@ struct TestAsyncPaymentsMessageHandler {} impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler { fn handle_held_htlc_available( - &self, _message: HeldHtlcAvailable, responder: Option, + &self, _message: HeldHtlcAvailable, _context: AsyncPaymentsContext, + responder: Option, ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)> { let responder = match responder { Some(resp) => resp, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 99b7c0586d5..0bf250c67fc 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -12166,7 +12166,8 @@ where L::Target: Logger, { fn handle_held_htlc_available( - &self, _message: HeldHtlcAvailable, _responder: Option + &self, _message: HeldHtlcAvailable, _context: AsyncPaymentsContext, + _responder: Option ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)> { None } diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index dbce9ca0498..80b92cec1bd 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -149,7 +149,8 @@ impl OffersMessageHandler for IgnoringMessageHandler { } impl AsyncPaymentsMessageHandler for IgnoringMessageHandler { fn handle_held_htlc_available( - &self, _message: HeldHtlcAvailable, _responder: Option, + &self, _message: HeldHtlcAvailable, _context: AsyncPaymentsContext, + _responder: Option, ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)> { None } diff --git a/lightning/src/onion_message/async_payments.rs b/lightning/src/onion_message/async_payments.rs index d81010e5d5f..7a473c90e8f 100644 --- a/lightning/src/onion_message/async_payments.rs +++ b/lightning/src/onion_message/async_payments.rs @@ -28,7 +28,8 @@ pub trait AsyncPaymentsMessageHandler { /// Handle a [`HeldHtlcAvailable`] message. A [`ReleaseHeldHtlc`] should be returned to release /// the held funds. fn handle_held_htlc_available( - &self, message: HeldHtlcAvailable, responder: Option, + &self, message: HeldHtlcAvailable, context: AsyncPaymentsContext, + responder: Option, ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)>; /// Handle a [`ReleaseHeldHtlc`] message. If authentication of the message succeeds, an HTLC diff --git a/lightning/src/onion_message/functional_tests.rs b/lightning/src/onion_message/functional_tests.rs index f9d73f05ff3..57a7bc8afd5 100644 --- a/lightning/src/onion_message/functional_tests.rs +++ b/lightning/src/onion_message/functional_tests.rs @@ -85,7 +85,8 @@ struct TestAsyncPaymentsMessageHandler {} impl AsyncPaymentsMessageHandler for TestAsyncPaymentsMessageHandler { fn handle_held_htlc_available( - &self, _message: HeldHtlcAvailable, _responder: Option, + &self, _message: HeldHtlcAvailable, _context: AsyncPaymentsContext, + _responder: Option, ) -> Option<(ReleaseHeldHtlc, ResponseInstruction)> { None } diff --git a/lightning/src/onion_message/messenger.rs b/lightning/src/onion_message/messenger.rs index f076e6a9da4..7b0bae82ba8 100644 --- a/lightning/src/onion_message/messenger.rs +++ b/lightning/src/onion_message/messenger.rs @@ -1646,8 +1646,16 @@ where }, #[cfg(async_payments)] ParsedOnionMessageContents::AsyncPayments(AsyncPaymentsMessage::HeldHtlcAvailable(msg)) => { + let context = match context { + Some(MessageContext::AsyncPayments(context)) => context, + Some(_) => { + debug_assert!(false, "Checked in peel_onion_message"); + return + }, + None => return, + }; let response_instructions = self.async_payments_handler.handle_held_htlc_available( - msg, responder + msg, context, responder ); if let Some((msg, instructions)) = response_instructions { let _ = self.handle_onion_message_response(msg, instructions); From 135f7578e6910756fbabeae052b1144ef0b2d86d Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 6 Jan 2025 17:24:50 -0500 Subject: [PATCH 4/6] Fail earlier on expired static invoice Prior to this patch, if we received an expired static invoice we would delay surfacing the payment failure until after the recipient had come online and sent the release_held_htlc OM, which could be a long time later. Now, we'll detect that the invoice is expired as soon as it's received. --- lightning/src/events/mod.rs | 4 +- lightning/src/ln/async_payments_tests.rs | 71 ++++++++++++++++++++++++ lightning/src/ln/channelmanager.rs | 4 +- lightning/src/ln/outbound_payment.rs | 7 ++- 4 files changed, 81 insertions(+), 5 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 1a212c7c4da..07a97759f3b 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -561,8 +561,8 @@ pub enum PaymentFailureReason { #[cfg_attr(feature = "std", doc = "")] #[cfg_attr(feature = "std", doc = "[`Retry::Timeout`]: crate::ln::channelmanager::Retry::Timeout")] RetriesExhausted, - /// The payment expired while retrying, based on the provided - /// [`PaymentParameters::expiry_time`]. + /// Either the BOLT 12 invoice was expired by the time we received it or the payment expired while + /// retrying based on the provided [`PaymentParameters::expiry_time`]. /// /// Also used for [`InvoiceRequestExpired`] when downgrading to version prior to 0.0.124. /// diff --git a/lightning/src/ln/async_payments_tests.rs b/lightning/src/ln/async_payments_tests.rs index 01135f625cd..a1d907a1b63 100644 --- a/lightning/src/ln/async_payments_tests.rs +++ b/lightning/src/ln/async_payments_tests.rs @@ -579,3 +579,74 @@ fn pays_static_invoice() { .handle_onion_message(nodes[2].node.get_our_node_id(), &release_held_htlc_om); assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); } + +#[cfg_attr(feature = "std", ignore)] +#[test] +fn expired_static_invoice_fail() { + // Test that if we receive an expired static invoice we'll fail the payment. + let secp_ctx = Secp256k1::new(); + let chanmon_cfgs = create_chanmon_cfgs(3); + let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); + let nodes = create_network(3, &node_cfgs, &node_chanmgrs); + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + create_unannounced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0); + + const INVOICE_EXPIRY_SECS: u32 = 10; + let relative_expiry = Duration::from_secs(INVOICE_EXPIRY_SECS as u64); + let (offer, static_invoice) = + create_static_invoice(&nodes[1], &nodes[2], Some(relative_expiry), &secp_ctx); + + let amt_msat = 5000; + let payment_id = PaymentId([1; 32]); + nodes[0] + .node + .pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(0), None) + .unwrap(); + + let invreq_om = nodes[0] + .onion_messenger + .next_onion_message_for_peer(nodes[1].node.get_our_node_id()) + .unwrap(); + let invreq_reply_path = offers_tests::extract_invoice_request(&nodes[1], &invreq_om).1; + // TODO: update to not manually send here when we add support for being the recipient's + // always-online counterparty + nodes[1] + .onion_messenger + .send_onion_message( + ParsedOnionMessageContents::::Offers(OffersMessage::StaticInvoice( + static_invoice, + )), + MessageSendInstructions::WithoutReplyPath { + destination: Destination::BlindedPath(invreq_reply_path), + }, + ) + .unwrap(); + let static_invoice_om = nodes[1] + .onion_messenger + .next_onion_message_for_peer(nodes[0].node.get_our_node_id()) + .unwrap(); + + // Wait until the static invoice expires before providing it to the sender. + let block = create_dummy_block( + nodes[0].best_block_hash(), + nodes[0].node.duration_since_epoch().as_secs() as u32 + INVOICE_EXPIRY_SECS + 1, + Vec::new(), + ); + connect_block(&nodes[0], &block); + nodes[0] + .onion_messenger + .handle_onion_message(nodes[1].node.get_our_node_id(), &static_invoice_om); + + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 1); + match events[0] { + Event::PaymentFailed { payment_id: ev_payment_id, reason, .. } => { + assert_eq!(reason.unwrap(), PaymentFailureReason::PaymentExpired); + assert_eq!(ev_payment_id, payment_id); + }, + _ => panic!(), + } + // The sender doesn't reply with InvoiceError right now because the always-online node doesn't + // currently provide them with a reply path to do so. +} diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 0bf250c67fc..373ffe5a091 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4729,8 +4729,8 @@ where let best_block_height = self.best_block.read().unwrap().height; let features = self.bolt12_invoice_features(); let outbound_pmts_res = self.pending_outbound_payments.static_invoice_received( - invoice, payment_id, features, best_block_height, &*self.entropy_source, - &self.pending_events + invoice, payment_id, features, best_block_height, self.duration_since_epoch(), + &*self.entropy_source, &self.pending_events ); match outbound_pmts_res { Ok(()) => {}, diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 9c28e43ff51..9996f081a73 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -1011,7 +1011,7 @@ impl OutboundPayments { #[cfg(async_payments)] pub(super) fn static_invoice_received( &self, invoice: &StaticInvoice, payment_id: PaymentId, features: Bolt12InvoiceFeatures, - best_block_height: u32, entropy_source: ES, + best_block_height: u32, duration_since_epoch: Duration, entropy_source: ES, pending_events: &Mutex)>> ) -> Result<(), Bolt12PaymentError> where ES::Target: EntropySource { macro_rules! abandon_with_entry { @@ -1045,6 +1045,11 @@ impl OutboundPayments { abandon_with_entry!(entry, PaymentFailureReason::UnknownRequiredFeatures); return Err(Bolt12PaymentError::UnknownRequiredFeatures) } + if duration_since_epoch > invoice.created_at().saturating_add(invoice.relative_expiry()) { + abandon_with_entry!(entry, PaymentFailureReason::PaymentExpired); + return Err(Bolt12PaymentError::SendingFailed(RetryableSendFailure::PaymentExpired)) + } + let amount_msat = match InvoiceBuilder::::amount_msats(invreq) { Ok(amt) => amt, Err(_) => { From 8331791f71acd25b50448942bf901100ba79f61b Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 6 Jan 2025 14:52:37 -0500 Subject: [PATCH 5/6] Fail out-of-PaymentContext inbound keysends Here we bubble up the payment context into PendingHTLCRouting::ReceiveKeysend and check it when receiving a spontaneous payment prior to generating a claimable event. Prior to this patch, we would have accepted out-of-context keysends sent over blinded paths taken from our BOLT 12 invoices. As a side effect of this, our blinded keysend success test cases now fail, so those tests are now removed. Their coverage is re-added in future commits when we add support for async receive, meaning we're able to receive blinded keysends in the correct payment context. While we could avoid storing the payment context for the purposes of this bugfix, we go ahead and store it now because it will be needed when support for receiving async payments is added. --- lightning/src/ln/async_payments_tests.rs | 169 +---------------------- lightning/src/ln/channelmanager.rs | 17 ++- lightning/src/ln/offers_tests.rs | 67 ++++++++- lightning/src/ln/onion_payment.rs | 1 + 4 files changed, 82 insertions(+), 172 deletions(-) diff --git a/lightning/src/ln/async_payments_tests.rs b/lightning/src/ln/async_payments_tests.rs index a1d907a1b63..a62f7b5936a 100644 --- a/lightning/src/ln/async_payments_tests.rs +++ b/lightning/src/ln/async_payments_tests.rs @@ -9,11 +9,9 @@ use crate::blinded_path::message::{MessageContext, OffersContext}; use crate::events::{Event, HTLCDestination, MessageSendEventsProvider, PaymentFailureReason}; -use crate::ln::blinded_payment_tests::{blinded_payment_path, get_blinded_route_parameters}; -use crate::ln::channelmanager; +use crate::ln::blinded_payment_tests::get_blinded_route_parameters; use crate::ln::channelmanager::{PaymentId, RecipientOnionFields}; use crate::ln::functional_test_utils::*; -use crate::ln::inbound_payment; use crate::ln::msgs::ChannelMessageHandler; use crate::ln::msgs::OnionMessageHandler; use crate::ln::offers_tests; @@ -29,11 +27,8 @@ use crate::onion_message::messenger::{Destination, MessageRouter, MessageSendIns use crate::onion_message::offers::OffersMessage; use crate::onion_message::packet::ParsedOnionMessageContents; use crate::prelude::*; -use crate::routing::router::{PaymentParameters, RouteParameters}; -use crate::sign::NodeSigner; use crate::types::features::Bolt12InvoiceFeatures; use crate::types::payment::{PaymentPreimage, PaymentSecret}; -use crate::util::config::UserConfig; use bitcoin::secp256k1; use bitcoin::secp256k1::Secp256k1; @@ -67,168 +62,6 @@ fn create_static_invoice( (offer, static_invoice) } -#[test] -fn blinded_keysend() { - let chanmon_cfgs = create_chanmon_cfgs(3); - let node_cfgs = create_node_cfgs(3, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(3, &node_cfgs, &[None, None, None]); - let mut nodes = create_network(3, &node_cfgs, &node_chanmgrs); - create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); - let chan_upd_1_2 = - create_announced_chan_between_nodes_with_value(&nodes, 1, 2, 1_000_000, 0).0.contents; - - let inbound_payment_key = nodes[2].keys_manager.get_inbound_payment_key(); - let payment_secret = inbound_payment::create_for_spontaneous_payment( - &inbound_payment_key, - None, - u32::MAX, - nodes[2].node.duration_since_epoch().as_secs(), - None, - ) - .unwrap(); - - let amt_msat = 5000; - let keysend_preimage = PaymentPreimage([42; 32]); - let route_params = get_blinded_route_parameters( - amt_msat, - payment_secret, - 1, - 1_0000_0000, - nodes.iter().skip(1).map(|n| n.node.get_our_node_id()).collect(), - &[&chan_upd_1_2], - &chanmon_cfgs[2].keys_manager, - ); - - let payment_hash = nodes[0] - .node - .send_spontaneous_payment( - Some(keysend_preimage), - RecipientOnionFields::spontaneous_empty(), - PaymentId(keysend_preimage.0), - route_params, - Retry::Attempts(0), - ) - .unwrap(); - check_added_monitors(&nodes[0], 1); - - let expected_route: &[&[&Node]] = &[&[&nodes[1], &nodes[2]]]; - let mut events = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 1); - - let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); - pass_along_path( - &nodes[0], - expected_route[0], - amt_msat, - payment_hash, - Some(payment_secret), - ev.clone(), - true, - Some(keysend_preimage), - ); - claim_payment_along_route(ClaimAlongRouteArgs::new( - &nodes[0], - expected_route, - keysend_preimage, - )); -} - -#[test] -fn blinded_mpp_keysend() { - let chanmon_cfgs = create_chanmon_cfgs(4); - let node_cfgs = create_node_cfgs(4, &chanmon_cfgs); - let node_chanmgrs = create_node_chanmgrs(4, &node_cfgs, &[None, None, None, None]); - let nodes = create_network(4, &node_cfgs, &node_chanmgrs); - - create_announced_chan_between_nodes(&nodes, 0, 1); - create_announced_chan_between_nodes(&nodes, 0, 2); - let chan_1_3 = create_announced_chan_between_nodes(&nodes, 1, 3); - let chan_2_3 = create_announced_chan_between_nodes(&nodes, 2, 3); - - let inbound_payment_key = nodes[3].keys_manager.get_inbound_payment_key(); - let payment_secret = inbound_payment::create_for_spontaneous_payment( - &inbound_payment_key, - None, - u32::MAX, - nodes[3].node.duration_since_epoch().as_secs(), - None, - ) - .unwrap(); - - let amt_msat = 15_000_000; - let keysend_preimage = PaymentPreimage([42; 32]); - let route_params = { - let pay_params = PaymentParameters::blinded(vec![ - blinded_payment_path( - payment_secret, - 1, - 1_0000_0000, - vec![nodes[1].node.get_our_node_id(), nodes[3].node.get_our_node_id()], - &[&chan_1_3.0.contents], - &chanmon_cfgs[3].keys_manager, - ), - blinded_payment_path( - payment_secret, - 1, - 1_0000_0000, - vec![nodes[2].node.get_our_node_id(), nodes[3].node.get_our_node_id()], - &[&chan_2_3.0.contents], - &chanmon_cfgs[3].keys_manager, - ), - ]) - .with_bolt12_features(channelmanager::provided_bolt12_invoice_features( - &UserConfig::default(), - )) - .unwrap(); - RouteParameters::from_payment_params_and_value(pay_params, amt_msat) - }; - - let payment_hash = nodes[0] - .node - .send_spontaneous_payment( - Some(keysend_preimage), - RecipientOnionFields::spontaneous_empty(), - PaymentId(keysend_preimage.0), - route_params, - Retry::Attempts(0), - ) - .unwrap(); - check_added_monitors!(nodes[0], 2); - - let expected_route: &[&[&Node]] = &[&[&nodes[1], &nodes[3]], &[&nodes[2], &nodes[3]]]; - let mut events = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(events.len(), 2); - - let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); - pass_along_path( - &nodes[0], - expected_route[0], - amt_msat, - payment_hash.clone(), - Some(payment_secret), - ev.clone(), - false, - Some(keysend_preimage), - ); - - let ev = remove_first_msg_event_to_node(&nodes[2].node.get_our_node_id(), &mut events); - pass_along_path( - &nodes[0], - expected_route[1], - amt_msat, - payment_hash.clone(), - Some(payment_secret), - ev.clone(), - true, - Some(keysend_preimage), - ); - claim_payment_along_route(ClaimAlongRouteArgs::new( - &nodes[0], - expected_route, - keysend_preimage, - )); -} - #[test] fn invalid_keysend_payment_secret() { let chanmon_cfgs = create_chanmon_cfgs(3); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 373ffe5a091..89795b00601 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -240,6 +240,11 @@ pub enum PendingHTLCRouting { /// [`PaymentSecret`] and should verify it using our /// [`NodeSigner::get_inbound_payment_key`]. has_recipient_created_payment_secret: bool, + /// The context of the payment included by the recipient in a blinded path, or `None` if a + /// blinded path was not used. + /// + /// Used in part to determine the [`events::PaymentPurpose`]. + payment_context: Option, }, } @@ -6023,7 +6028,7 @@ where PendingHTLCRouting::ReceiveKeysend { payment_data, payment_preimage, payment_metadata, incoming_cltv_expiry, custom_tlvs, requires_blinded_error: _, - has_recipient_created_payment_secret, + has_recipient_created_payment_secret, payment_context, } => { let onion_fields = RecipientOnionFields { payment_secret: payment_data.as_ref().map(|data| data.payment_secret), @@ -6031,7 +6036,8 @@ where custom_tlvs, }; (incoming_cltv_expiry, OnionPayload::Spontaneous(payment_preimage), - payment_data, None, None, onion_fields, has_recipient_created_payment_secret) + payment_data, payment_context, None, onion_fields, + has_recipient_created_payment_secret) }, _ => { panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive"); @@ -6228,6 +6234,12 @@ where check_total_value!(purpose); }, OnionPayload::Spontaneous(preimage) => { + if payment_context.is_some() { + if !matches!(payment_context, Some(PaymentContext::AsyncBolt12Offer(_))) { + log_trace!(self.logger, "Failing new HTLC with payment_hash {}: received a keysend payment to a non-async payments context {:#?}", payment_hash, payment_context); + } + fail_htlc!(claimable_htlc, payment_hash); + } let purpose = events::PaymentPurpose::SpontaneousPayment(preimage); check_total_value!(purpose); } @@ -12377,6 +12389,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting, (4, payment_data, option), // Added in 0.0.116 (5, custom_tlvs, optional_vec), (7, has_recipient_created_payment_secret, (default_value, false)), + (9, payment_context, option), }, ); diff --git a/lightning/src/ln/offers_tests.rs b/lightning/src/ln/offers_tests.rs index ba059d55e4c..803441f09fe 100644 --- a/lightning/src/ln/offers_tests.rs +++ b/lightning/src/ln/offers_tests.rs @@ -47,8 +47,8 @@ use crate::blinded_path::IntroductionNode; use crate::blinded_path::message::BlindedMessagePath; use crate::blinded_path::payment::{Bolt12OfferContext, Bolt12RefundContext, PaymentContext}; use crate::blinded_path::message::{MessageContext, OffersContext}; -use crate::events::{ClosureReason, Event, MessageSendEventsProvider, PaymentFailureReason, PaymentPurpose}; -use crate::ln::channelmanager::{Bolt12PaymentError, MAX_SHORT_LIVED_RELATIVE_EXPIRY, PaymentId, RecentPaymentDetails, Retry, self}; +use crate::events::{ClosureReason, Event, HTLCDestination, MessageSendEventsProvider, PaymentFailureReason, PaymentPurpose}; +use crate::ln::channelmanager::{Bolt12PaymentError, MAX_SHORT_LIVED_RELATIVE_EXPIRY, PaymentId, RecentPaymentDetails, RecipientOnionFields, Retry, self}; use crate::types::features::Bolt12InvoiceFeatures; use crate::ln::functional_test_utils::*; use crate::ln::msgs::{ChannelMessageHandler, Init, NodeAnnouncement, OnionMessage, OnionMessageHandler, RoutingMessageHandler, SocketAddress, UnsignedGossipMessage, UnsignedNodeAnnouncement}; @@ -62,6 +62,7 @@ use crate::onion_message::messenger::{Destination, PeeledOnion, MessageSendInstr use crate::onion_message::offers::OffersMessage; use crate::onion_message::packet::ParsedOnionMessageContents; use crate::routing::gossip::{NodeAlias, NodeId}; +use crate::routing::router::{PaymentParameters, RouteParameters}; use crate::sign::{NodeSigner, Recipient}; use crate::util::ser::Writeable; @@ -2255,6 +2256,68 @@ fn fails_paying_invoice_with_unknown_required_features() { } } +#[test] +fn rejects_keysend_to_non_static_invoice_path() { + // Test that we'll fail a keysend payment that was sent over a non-static BOLT 12 invoice path. + let chanmon_cfgs = create_chanmon_cfgs(2); + let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); + let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]); + let nodes = create_network(2, &node_cfgs, &node_chanmgrs); + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 1_000_000, 0); + + // First pay the offer and save the payment preimage and invoice. + let offer = nodes[1].node.create_offer_builder(None).unwrap().build().unwrap(); + let amt_msat = 5000; + let payment_id = PaymentId([1; 32]); + nodes[0].node.pay_for_offer(&offer, None, Some(amt_msat), None, payment_id, Retry::Attempts(1), None).unwrap(); + let invreq_om = nodes[0].onion_messenger.next_onion_message_for_peer(nodes[1].node.get_our_node_id()).unwrap(); + nodes[1].onion_messenger.handle_onion_message(nodes[0].node.get_our_node_id(), &invreq_om); + let invoice_om = nodes[1].onion_messenger.next_onion_message_for_peer(nodes[0].node.get_our_node_id()).unwrap(); + let invoice = extract_invoice(&nodes[0], &invoice_om).0; + nodes[0].onion_messenger.handle_onion_message(nodes[1].node.get_our_node_id(), &invoice_om); + + route_bolt12_payment(&nodes[0], &[&nodes[1]], &invoice); + expect_recent_payment!(nodes[0], RecentPaymentDetails::Pending, payment_id); + + let payment_preimage = match get_event!(nodes[1], Event::PaymentClaimable) { + Event::PaymentClaimable { purpose, .. } => purpose.preimage().unwrap(), + _ => panic!() + }; + + claim_payment(&nodes[0], &[&nodes[1]], payment_preimage); + expect_recent_payment!(&nodes[0], RecentPaymentDetails::Fulfilled, payment_id); + + // Time out the payment from recent payments so we can attempt to pay it again via keysend. + for _ in 0..=IDEMPOTENCY_TIMEOUT_TICKS { + nodes[0].node.timer_tick_occurred(); + nodes[1].node.timer_tick_occurred(); + } + + // Pay the invoice via keysend now that we have the preimage and make sure the recipient fails it + // due to incorrect payment context. + let pay_params = PaymentParameters::from_bolt12_invoice(&invoice); + let route_params = RouteParameters::from_payment_params_and_value(pay_params, amt_msat); + let keysend_payment_id = PaymentId([2; 32]); + let payment_hash = nodes[0].node.send_spontaneous_payment( + Some(payment_preimage), RecipientOnionFields::spontaneous_empty(), keysend_payment_id, + route_params, Retry::Attempts(0) + ).unwrap(); + check_added_monitors!(nodes[0], 1); + let mut events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(events.len(), 1); + let ev = remove_first_msg_event_to_node(&nodes[1].node.get_our_node_id(), &mut events); + let route: &[&[&Node]] = &[&[&nodes[1]]]; + + let args = PassAlongPathArgs::new(&nodes[0], route[0], amt_msat, payment_hash, ev) + .with_payment_preimage(payment_preimage) + .expect_failure(HTLCDestination::FailedPayment { payment_hash }); + do_pass_along_path(args); + let mut updates = get_htlc_update_msgs!(nodes[1], nodes[0].node.get_our_node_id()); + nodes[0].node.handle_update_fail_htlc(nodes[1].node.get_our_node_id(), &updates.update_fail_htlcs[0]); + do_commitment_signed_dance(&nodes[0], &nodes[1], &updates.commitment_signed, false, false); + expect_payment_failed_conditions(&nodes[0], payment_hash, true, PaymentFailedConditions::new()); +} + #[test] fn no_double_pay_with_stale_channelmanager() { // This tests the following bug: diff --git a/lightning/src/ln/onion_payment.rs b/lightning/src/ln/onion_payment.rs index 193cdd1582a..203a528a8fb 100644 --- a/lightning/src/ln/onion_payment.rs +++ b/lightning/src/ln/onion_payment.rs @@ -235,6 +235,7 @@ pub(super) fn create_recv_pending_htlc_info( custom_tlvs, requires_blinded_error, has_recipient_created_payment_secret, + payment_context, } } else if let Some(data) = payment_data { PendingHTLCRouting::Receive { From f799e6b97ca1a3cb0c128029f59d9bf1bdc031da Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 24 Jan 2025 14:34:21 -0500 Subject: [PATCH 6/6] Tweak RetryableSendFailure::PaymentExpired docs This error variant is also used when manually sending to BOLT 12 invoices is enabled, so document that. --- lightning/src/ln/outbound_payment.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 9996f081a73..0c02af49763 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -458,11 +458,14 @@ impl_writeable_tlv_based_enum_legacy!(StaleExpiration, /// [`Event::PaymentFailed`]: crate::events::Event::PaymentFailed #[derive(Clone, Debug, PartialEq, Eq)] pub enum RetryableSendFailure { - /// The provided [`PaymentParameters::expiry_time`] indicated that the payment has expired. + /// The provided [`PaymentParameters::expiry_time`] indicated that the payment has expired or + /// the BOLT 12 invoice paid to via [`ChannelManager::send_payment_for_bolt12_invoice`] was + /// expired. #[cfg_attr(feature = "std", doc = "")] #[cfg_attr(feature = "std", doc = "Note that this error is *not* caused by [`Retry::Timeout`].")] /// /// [`PaymentParameters::expiry_time`]: crate::routing::router::PaymentParameters::expiry_time + /// [`ChannelManager::send_payment_for_bolt12_invoice`]: crate::ln::channelmanager::ChannelManager::send_payment_for_bolt12_invoice PaymentExpired, /// We were unable to find a route to the destination. RouteNotFound,