Report the sending peer in Event::OnionMessageIntercepted

lightning-tests/src/upgrade_downgrade_tests.rs

@@ -17,7 +17,10 @@ use lightning_0_2::ln::channelmanager::PaymentId as PaymentId_0_2;
 use lightning_0_2::ln::channelmanager::RecipientOnionFields as RecipientOnionFields_0_2;
 use lightning_0_2::ln::functional_test_utils as lightning_0_2_utils;
 use lightning_0_2::ln::msgs::ChannelMessageHandler as _;
+use lightning_0_2::ln::msgs::OnionMessage as OnionMessage_0_2;
+use lightning_0_2::onion_message::packet::Packet as Packet_0_2;
 use lightning_0_2::routing::router as router_0_2;
+use lightning_0_2::util::ser::MaybeReadable as MaybeReadable_0_2;
 use lightning_0_2::util::ser::Writeable as _;
 
 use lightning_0_1::commitment_signed_dance as commitment_signed_dance_0_1;
@@ -45,23 +48,29 @@ use lightning_0_0_125::ln::msgs::ChannelMessageHandler as _;
 use lightning_0_0_125::routing::router as router_0_0_125;
 use lightning_0_0_125::util::ser::Writeable as _;
 
+use lightning::blinded_path::message::NextMessageHop;
 use lightning::chain::channelmonitor::{ANTI_REORG_DELAY, HTLC_FAIL_BACK_BUFFER};
 use lightning::events::{ClosureReason, Event, HTLCHandlingFailureType};
 use lightning::ln::functional_test_utils::*;
+use lightning::ln::msgs;
 use lightning::ln::msgs::BaseMessageHandler as _;
 use lightning::ln::msgs::ChannelMessageHandler as _;
 use lightning::ln::msgs::MessageSendEvent;
 use lightning::ln::splicing_tests::*;
 use lightning::ln::types::ChannelId;
+use lightning::onion_message::packet::Packet;
 use lightning::sign::OutputSpender;
+use lightning::util::ser::{MaybeReadable, Writeable};
 use lightning::util::wallet_utils::WalletSourceSync;
 
 use lightning_types::payment::{PaymentHash, PaymentPreimage, PaymentSecret};
 
 use bitcoin::script::Builder;
-use bitcoin::secp256k1::Secp256k1;
+use bitcoin::secp256k1::{PublicKey, Secp256k1, SecretKey};
 use bitcoin::{opcodes, Amount, TxOut};
 
+use lightning::io::Cursor;
+
 use std::sync::Arc;
 
 #[test]
@@ -700,3 +709,113 @@ fn do_upgrade_mid_htlc_forward(test: MidHtlcForwardCase) {
 	expect_payment_claimable!(nodes[2], pay_hash, pay_secret, 1_000_000);
 	claim_payment(&nodes[0], &[&nodes[1], &nodes[2]], pay_preimage);
 }
+
+/// Constructs a dummy `OnionMessage` (current version) for use in serialization tests.
+fn dummy_onion_message() -> msgs::OnionMessage {
+	let pubkey =
+		PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&[42; 32]).unwrap());
+	msgs::OnionMessage {
+		blinding_point: pubkey,
+		onion_routing_packet: Packet {
+			version: 0,
+			public_key: pubkey,
+			hop_data: vec![1; 64],
+			hmac: [2; 32],
+		},
+	}
+}
+
+/// Constructs a dummy `OnionMessage` (0.2 version) for use in serialization tests.
+fn dummy_onion_message_0_2() -> OnionMessage_0_2 {
+	let pubkey = bitcoin::secp256k1::PublicKey::from_secret_key(
+		&Secp256k1::new(),
+		&SecretKey::from_slice(&[42; 32]).unwrap(),
+	);
+	OnionMessage_0_2 {
+		blinding_point: pubkey,
+		onion_routing_packet: Packet_0_2 {
+			version: 0,
+			public_key: pubkey,
+			hop_data: vec![1; 64],
+			hmac: [2; 32],
+		},
+	}
+}
+
+#[test]
+fn test_onion_message_intercepted_upgrade_from_0_2() {
+	// Ensure that an `Event::OnionMessageIntercepted` serialized by LDK 0.2 (which uses
+	// `peer_node_id: PublicKey` in TLV field 0) can be deserialized by the current version,
+	// producing `NextMessageHop::NodeId`.
+	let pubkey =
+		PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&[42; 32]).unwrap());
+
+	let event_0_2 = Event_0_2::OnionMessageIntercepted {
+		peer_node_id: pubkey,
+		message: dummy_onion_message_0_2(),
+	};
+
+	let serialized = lightning_0_2::util::ser::Writeable::encode(&event_0_2);
+
+	let mut reader = Cursor::new(&serialized);
+	let deserialized = <Event as MaybeReadable>::read(&mut reader).unwrap().unwrap();
+
+	match deserialized {
+		Event::OnionMessageIntercepted { prev_hop, next_hop, message } => {
+			// LDK 0.2 did not write a `prev_hop`, so it must default to `None`.
+			assert_eq!(prev_hop, None);
+			assert_eq!(next_hop, NextMessageHop::NodeId(pubkey));
+			assert_eq!(message, dummy_onion_message());
+		},
+		_ => panic!("Expected OnionMessageIntercepted event"),
+	}
+}
+
+#[test]
+fn test_onion_message_intercepted_node_id_downgrade_to_0_2() {
+	// Ensure that an `Event::OnionMessageIntercepted` with a `NodeId` next hop serialized by
+	// the current version can be deserialized by LDK 0.2 (which expects `peer_node_id` in TLV
+	// field 0 and ignores the newer `prev_hop` in TLV field 3).
+	let pubkey =
+		PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&[42; 32]).unwrap());
+	let prev_hop =
+		PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&[43; 32]).unwrap());
+
+	let event = Event::OnionMessageIntercepted {
+		prev_hop: Some(prev_hop),
+		next_hop: NextMessageHop::NodeId(pubkey),
+		message: dummy_onion_message(),
+	};
+
+	let serialized = event.encode();
+
+	let mut reader = Cursor::new(&serialized);
+	let deserialized = <Event_0_2 as MaybeReadable_0_2>::read(&mut reader).unwrap().unwrap();
+
+	match deserialized {
+		Event_0_2::OnionMessageIntercepted { peer_node_id, message } => {
+			assert_eq!(peer_node_id, pubkey);
+			assert_eq!(message, dummy_onion_message_0_2());
+		},
+		_ => panic!("Expected OnionMessageIntercepted event"),
+	}
+}
+
+#[test]
+fn test_onion_message_intercepted_scid_downgrade_to_0_2() {
+	// Ensure that an `Event::OnionMessageIntercepted` with a `ShortChannelId` next hop
+	// serialized by the current version cannot be deserialized by LDK 0.2, since the
+	// `peer_node_id` field (0) is not written for SCID variants and LDK 0.2 requires it.
+	let event = Event::OnionMessageIntercepted {
+		prev_hop: None,
+		next_hop: NextMessageHop::ShortChannelId(42),
+		message: dummy_onion_message(),
+	};
+
+	let serialized = event.encode();
+
+	// LDK 0.2 will try to read field 0 as required. Since it's absent, the read will fail.
+	let mut reader = Cursor::new(&serialized);
+	let result = <Event_0_2 as MaybeReadable_0_2>::read(&mut reader);
+	assert!(result.is_err(), "LDK 0.2 should fail to decode a ShortChannelId variant");
+}

lightning/src/blinded_path/message.rs

@@ -275,6 +275,11 @@ pub enum NextMessageHop {
 	ShortChannelId(u64),
 }
 
+impl_ser_tlv_based_enum!(NextMessageHop,
+	{0, NodeId} => (),
+	{2, ShortChannelId} => (),
+);
+
 /// An intermediate node, and possibly a short channel id leading to the next node.
 ///
 /// Note:

lightning/src/events/mod.rs

@@ -18,7 +18,7 @@ pub mod bump_transaction;
 
 pub use bump_transaction::BumpTransactionEvent;
 
-use crate::blinded_path::message::{BlindedMessagePath, OffersContext};
+use crate::blinded_path::message::{BlindedMessagePath, NextMessageHop, OffersContext};
 use crate::blinded_path::payment::{
 	Bolt12OfferContext, Bolt12RefundContext, PaymentContext, PaymentContextRef,
 };
@@ -1836,9 +1836,13 @@ pub enum Event {
 	/// [`ChannelHandshakeConfig::negotiate_anchor_zero_fee_commitments`]: crate::util::config::ChannelHandshakeConfig::negotiate_anchor_zero_fee_commitments
 	BumpTransaction(BumpTransactionEvent),
 	/// We received an onion message that is intended to be forwarded to a peer
-	/// that is currently offline. This event will only be generated if the
-	/// `OnionMessenger` was initialized with
-	/// [`OnionMessenger::new_with_offline_peer_interception`], see its docs.
+	/// that is currently offline *or* that is intended to be forwarded along a channel with an
+	/// SCID unknown to us.
+	///
+	/// This event will only be generated if the `OnionMessenger` was initialized with
+	/// [`OnionMessenger::new_with_offline_peer_interception`], see its docs. The
+	/// [`NextMessageHop::ShortChannelId`] variant is only generated if `intercept_for_unknown_scids`
+	/// was set when constructing the `OnionMessenger`.
 	///
 	/// The offline peer should be awoken if possible on receipt of this event, such as via the LSPS5
 	/// protocol.
@@ -1852,9 +1856,21 @@ pub enum Event {
 	///
 	/// [`OnionMessenger::new_with_offline_peer_interception`]: crate::onion_message::messenger::OnionMessenger::new_with_offline_peer_interception
 	OnionMessageIntercepted {
-		/// The node id of the offline peer.
-		peer_node_id: PublicKey,
-		/// The onion message intended to be forwarded to `peer_node_id`.
+		/// The node id of the peer that sent the message, if known.
+		///
+		/// This is `None` when the message is sent with
+		/// [`MessageSendInstructions::ForwardedMessage`] (e.g., when calling
+		/// [`OffersMessageFlow::enqueue_invoice_request_to_forward`]) rather than forwarded
+		/// internally by the `OnionMessenger`, as well as for events serialized prior to LDK 0.3.
+		/// Otherwise it is the node we received the message from.
+		///
+		/// [`MessageSendInstructions::ForwardedMessage`]: crate::onion_message::messenger::MessageSendInstructions::ForwardedMessage
+		/// [`OffersMessageFlow::enqueue_invoice_request_to_forward`]: crate::offers::flow::OffersMessageFlow::enqueue_invoice_request_to_forward
+		prev_hop: Option<PublicKey>,
+		/// The next hop (offline peer or unknown SCID).
+		next_hop: NextMessageHop,
+		/// The onion message intended to be forwarded to the offline peer or via the unknown
+		/// channel once established.
 		message: msgs::OnionMessage,
 	},
 	/// Indicates that an onion message supporting peer has come online and any messages previously
@@ -2436,11 +2452,19 @@ impl Writeable for Event {
 				35u8.write(writer)?;
 				// Never write ConnectionNeeded events as buffered onion messages aren't serialized.
 			},
-			&Event::OnionMessageIntercepted { ref peer_node_id, ref message } => {
+			&Event::OnionMessageIntercepted { ref prev_hop, ref next_hop, ref message } => {
 				37u8.write(writer)?;
+				// 0 used to be peer_node_id in LDK v0.2 and prior; we keep writing it when the next
+				// hop is a node id for backwards compatibility.
+				let legacy_peer_node_id = match next_hop {
+					NextMessageHop::NodeId(node_id) => Some(node_id),
+					NextMessageHop::ShortChannelId(_) => None,
+				};
 				write_tlv_fields!(writer, {
-					(0, peer_node_id, required),
+					(0, legacy_peer_node_id, option),
+					(1, next_hop, required),
 					(2, message, required),
+					(3, prev_hop, option),
 				});
 			},
 			&Event::OnionMessagePeerConnected { ref peer_node_id } => {
@@ -3069,11 +3093,18 @@ impl MaybeReadable for Event {
 			37u8 => {
 				let mut f = || {
 					_init_and_read_len_prefixed_tlv_fields!(reader, {
-						(0, peer_node_id, required),
+						(0, peer_node_id, option),
+						(1, next_hop, option),
 						(2, message, required),
+						(3, prev_hop, option),
 					});
+
+					let next_hop = next_hop
+						.or(peer_node_id.map(NextMessageHop::NodeId))
+						.ok_or(msgs::DecodeError::InvalidValue)?;
 					Ok(Some(Event::OnionMessageIntercepted {
-						peer_node_id: peer_node_id.0.unwrap(),
+						prev_hop,
+						next_hop,
 						message: message.0.unwrap(),
 					}))
 				};

lightning/src/ln/functional_test_utils.rs

@@ -4799,6 +4799,7 @@ pub fn create_network<'a, 'b: 'a, 'c: 'b>(
 			&chan_mgrs[i],
 			IgnoringMessageHandler {},
 			IgnoringMessageHandler {},
+			true,
 		);
 		let gossip_sync = P2PGossipSync::new(cfgs[i].network_graph.as_ref(), None, cfgs[i].logger);
 		let wallet_source = Arc::new(test_utils::TestWalletSource::new(