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.

Author: valentinewallace

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<T: secp256k1::Signing + secp256k1::Verification>(
 	(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);

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<PaymentContext>,
 	},
 }
 
@@ -6023,15 +6028,16 @@ 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),
 											payment_metadata,
 											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),
 	},
 );
 

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:

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 {