Receive payment onions as new InboundPayload instead of OnionHopData

To support route blinding, we want to split OnionHopData into two separate
structs, one for inbound onions and one for outbound onions. This is because
blinded payloads change the fields present in the onion hop data struct based
on whether we're sending vs receiving (outbound onions include encrypted blobs,
inbound onions can decrypt those blobs and contain the decrypted fields
themselves).

In upcoming commits, we'll add variants for blinded payloads to the new
InboundPayload enum.

Author: valentinewallace

fuzz/src/bin/gen_target.sh

@@ -20,6 +20,7 @@ GEN_TEST refund_deser
 GEN_TEST router
 GEN_TEST zbase32
 GEN_TEST indexedmap
+GEN_TEST onion_hop_data
 
 GEN_TEST msg_accept_channel msg_targets::
 GEN_TEST msg_announcement_signatures msg_targets::
@@ -51,7 +52,6 @@ GEN_TEST msg_update_add_htlc msg_targets::
 GEN_TEST msg_error_message msg_targets::
 GEN_TEST msg_channel_update msg_targets::
 
-GEN_TEST msg_onion_hop_data msg_targets::
 GEN_TEST msg_ping msg_targets::
 GEN_TEST msg_pong msg_targets::
 

fuzz/src/bin/msg_onion_hop_data_target.rs renamed to fuzz/src/bin/onion_hop_data_target.rs

@@ -16,14 +16,14 @@
 compile_error!("Fuzz targets need cfg=fuzzing");
 
 extern crate lightning_fuzz;
-use lightning_fuzz::msg_targets::msg_onion_hop_data::*;
+use lightning_fuzz::onion_hop_data::*;
 
 #[cfg(feature = "afl")]
 #[macro_use] extern crate afl;
 #[cfg(feature = "afl")]
 fn main() {
 	fuzz!(|data| {
-		msg_onion_hop_data_run(data.as_ptr(), data.len());
+		onion_hop_data_run(data.as_ptr(), data.len());
 	});
 }
 
@@ -33,7 +33,7 @@ fn main() {
 fn main() {
 	loop {
 		fuzz!(|data| {
-			msg_onion_hop_data_run(data.as_ptr(), data.len());
+			onion_hop_data_run(data.as_ptr(), data.len());
 		});
 	}
 }
@@ -42,7 +42,7 @@ fn main() {
 #[macro_use] extern crate libfuzzer_sys;
 #[cfg(feature = "libfuzzer_fuzz")]
 fuzz_target!(|data: &[u8]| {
-	msg_onion_hop_data_run(data.as_ptr(), data.len());
+	onion_hop_data_run(data.as_ptr(), data.len());
 });
 
 #[cfg(feature = "stdin_fuzz")]
@@ -51,7 +51,7 @@ fn main() {
 
 	let mut data = Vec::with_capacity(8192);
 	std::io::stdin().read_to_end(&mut data).unwrap();
-	msg_onion_hop_data_run(data.as_ptr(), data.len());
+	onion_hop_data_run(data.as_ptr(), data.len());
 }
 
 #[test]
@@ -63,11 +63,11 @@ fn run_test_cases() {
 	use std::sync::{atomic, Arc};
 	{
 		let data: Vec<u8> = vec![0];
-		msg_onion_hop_data_run(data.as_ptr(), data.len());
+		onion_hop_data_run(data.as_ptr(), data.len());
 	}
 	let mut threads = Vec::new();
 	let threads_running = Arc::new(atomic::AtomicUsize::new(0));
-	if let Ok(tests) = fs::read_dir("test_cases/msg_onion_hop_data") {
+	if let Ok(tests) = fs::read_dir("test_cases/onion_hop_data") {
 		for test in tests {
 			let mut data: Vec<u8> = Vec::new();
 			let path = test.unwrap().path();
@@ -82,7 +82,7 @@ fn run_test_cases() {
 
 					let panic_logger = string_logger.clone();
 					let res = if ::std::panic::catch_unwind(move || {
-						msg_onion_hop_data_test(&data, panic_logger);
+						onion_hop_data_test(&data, panic_logger);
 					}).is_err() {
 						Some(string_logger.into_string())
 					} else { None };

fuzz/src/lib.rs

@@ -28,5 +28,6 @@ pub mod process_network_graph;
 pub mod refund_deser;
 pub mod router;
 pub mod zbase32;
+pub mod onion_hop_data;
 
 pub mod msg_targets;

fuzz/src/msg_targets/mod.rs

@@ -8,7 +8,6 @@ pub mod msg_channel_ready;
 pub mod msg_funding_signed;
 pub mod msg_gossip_timestamp_filter;
 pub mod msg_init;
-pub mod msg_onion_hop_data;
 pub mod msg_open_channel;
 pub mod msg_ping;
 pub mod msg_pong;

fuzz/src/msg_targets/msg_onion_hop_data.rs renamed to fuzz/src/onion_hop_data.rs

@@ -10,16 +10,19 @@
 // This file is auto-generated by gen_target.sh based on msg_target_template.txt
 // To modify it, modify msg_target_template.txt and run gen_target.sh instead.
 
-use crate::msg_targets::utils::VecWriter;
 use crate::utils::test_logger;
 
 #[inline]
-pub fn msg_onion_hop_data_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
-	test_msg_simple!(lightning::ln::msgs::OnionHopData, data);
+pub fn onion_hop_data_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
+	use lightning::util::ser::Readable;
+	let mut r = ::std::io::Cursor::new(data);
+	let _ =  <lightning::ln::msgs::InboundPayload as Readable>::read(&mut r);
 }
 
 #[no_mangle]
-pub extern "C" fn msg_onion_hop_data_run(data: *const u8, datalen: usize) {
+pub extern "C" fn onion_hop_data_run(data: *const u8, datalen: usize) {
+	use lightning::util::ser::Readable;
 	let data = unsafe { std::slice::from_raw_parts(data, datalen) };
-	test_msg_simple!(lightning::ln::msgs::OnionHopData, data);
+	let mut r = ::std::io::Cursor::new(data);
+	let _ =  <lightning::ln::msgs::InboundPayload as Readable>::read(&mut r);
 }

fuzz/targets.h

@@ -13,6 +13,7 @@ void refund_deser_run(const unsigned char* data, size_t data_len);
 void router_run(const unsigned char* data, size_t data_len);
 void zbase32_run(const unsigned char* data, size_t data_len);
 void indexedmap_run(const unsigned char* data, size_t data_len);
+void onion_hop_data_run(const unsigned char* data, size_t data_len);
 void msg_accept_channel_run(const unsigned char* data, size_t data_len);
 void msg_announcement_signatures_run(const unsigned char* data, size_t data_len);
 void msg_channel_reestablish_run(const unsigned char* data, size_t data_len);
@@ -40,7 +41,6 @@ void msg_gossip_timestamp_filter_run(const unsigned char* data, size_t data_len)
 void msg_update_add_htlc_run(const unsigned char* data, size_t data_len);
 void msg_error_message_run(const unsigned char* data, size_t data_len);
 void msg_channel_update_run(const unsigned char* data, size_t data_len);
-void msg_onion_hop_data_run(const unsigned char* data, size_t data_len);
 void msg_ping_run(const unsigned char* data, size_t data_len);
 void msg_pong_run(const unsigned char* data, size_t data_len);
 void msg_channel_details_run(const unsigned char* data, size_t data_len);

lightning/src/ln/channelmanager.rs

@@ -2617,7 +2617,7 @@ where
 	}
 
 	fn construct_fwd_pending_htlc_info(
-		&self, msg: &msgs::UpdateAddHTLC, hop_data: msgs::OnionHopData, hop_hmac: [u8; 32],
+		&self, msg: &msgs::UpdateAddHTLC, hop_data: msgs::InboundPayload, hop_hmac: [u8; 32],
 		new_packet_bytes: [u8; onion_utils::ONION_DATA_LEN], shared_secret: [u8; 32],
 		next_packet_pubkey_opt: Option<Result<PublicKey, secp256k1::Error>>
 	) -> Result<PendingHTLCInfo, InboundOnionErr> {
@@ -2626,42 +2626,44 @@ where
 			version: 0,
 			public_key: next_packet_pubkey_opt.unwrap_or(Err(secp256k1::Error::InvalidPublicKey)),
 			hop_data: new_packet_bytes,
-			hmac: hop_hmac.clone(),
+			hmac: hop_hmac,
 		};
 
-		let short_channel_id = match hop_data.format {
-			msgs::OnionHopDataFormat::NonFinalNode { short_channel_id } => short_channel_id,
-			msgs::OnionHopDataFormat::FinalNode { .. } => {
+		let (short_channel_id, amt_to_forward, outgoing_cltv_value) = match hop_data {
+			msgs::InboundPayload::Forward { short_channel_id, amt_to_forward, outgoing_cltv_value } =>
+				(short_channel_id, amt_to_forward, outgoing_cltv_value),
+			msgs::InboundPayload::Receive { .. } =>
 				return Err(InboundOnionErr {
 					msg: "Final Node OnionHopData provided for us as an intermediary node",
 					err_code: 0x4000 | 22,
 					err_data: Vec::new(),
-				})
-			},
+				}),
 		};
 
 		Ok(PendingHTLCInfo {
 			routing: PendingHTLCRouting::Forward {
 				onion_packet: outgoing_packet,
 				short_channel_id,
 			},
-			payment_hash: msg.payment_hash.clone(),
+			payment_hash: msg.payment_hash,
 			incoming_shared_secret: shared_secret,
 			incoming_amt_msat: Some(msg.amount_msat),
-			outgoing_amt_msat: hop_data.amt_to_forward,
-			outgoing_cltv_value: hop_data.outgoing_cltv_value,
+			outgoing_amt_msat: amt_to_forward,
+			outgoing_cltv_value,
 			skimmed_fee_msat: None,
 		})
 	}
 
 	fn construct_recv_pending_htlc_info(
-		&self, hop_data: msgs::OnionHopData, shared_secret: [u8; 32], payment_hash: PaymentHash,
+		&self, hop_data: msgs::InboundPayload, shared_secret: [u8; 32], payment_hash: PaymentHash,
 		amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>, allow_underpay: bool,
 		counterparty_skimmed_fee_msat: Option<u64>,
 	) -> Result<PendingHTLCInfo, InboundOnionErr> {
-		let (payment_data, keysend_preimage, payment_metadata) = match hop_data.format {
-			msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage, payment_metadata } =>
-				(payment_data, keysend_preimage, payment_metadata),
+		let (payment_data, keysend_preimage, onion_amt_msat, outgoing_cltv_value, payment_metadata) = match hop_data {
+			msgs::InboundPayload::Receive {
+				payment_data, keysend_preimage, amt_msat, outgoing_cltv_value, payment_metadata, ..
+			} =>
+				(payment_data, keysend_preimage, amt_msat, outgoing_cltv_value, payment_metadata),
 			_ =>
 				return Err(InboundOnionErr {
 					err_code: 0x4000|22,
@@ -2670,7 +2672,7 @@ where
 				}),
 		};
 		// final_incorrect_cltv_expiry
-		if hop_data.outgoing_cltv_value > cltv_expiry {
+		if outgoing_cltv_value > cltv_expiry {
 			return Err(InboundOnionErr {
 				msg: "Upstream node set CLTV to less than the CLTV set by the sender",
 				err_code: 18,
@@ -2685,7 +2687,7 @@ where
 		// payment logic has enough time to fail the HTLC backward before our onchain logic triggers a
 		// channel closure (see HTLC_FAIL_BACK_BUFFER rationale).
 		let current_height: u32 = self.best_block.read().unwrap().height();
-		if (hop_data.outgoing_cltv_value as u64) <= current_height as u64 + HTLC_FAIL_BACK_BUFFER as u64 + 1 {
+		if (outgoing_cltv_value as u64) <= current_height as u64 + HTLC_FAIL_BACK_BUFFER as u64 + 1 {
 			let mut err_data = Vec::with_capacity(12);
 			err_data.extend_from_slice(&amt_msat.to_be_bytes());
 			err_data.extend_from_slice(&current_height.to_be_bytes());
@@ -2694,8 +2696,8 @@ where
 				msg: "The final CLTV expiry is too soon to handle",
 			});
 		}
-		if (!allow_underpay && hop_data.amt_to_forward > amt_msat) ||
-			(allow_underpay && hop_data.amt_to_forward >
+		if (!allow_underpay && onion_amt_msat > amt_msat) ||
+			(allow_underpay && onion_amt_msat >
 			 amt_msat.saturating_add(counterparty_skimmed_fee_msat.unwrap_or(0)))
 		{
 			return Err(InboundOnionErr {
@@ -2731,13 +2733,13 @@ where
 				payment_data,
 				payment_preimage,
 				payment_metadata,
-				incoming_cltv_expiry: hop_data.outgoing_cltv_value,
+				incoming_cltv_expiry: outgoing_cltv_value,
 			}
 		} else if let Some(data) = payment_data {
 			routing = PendingHTLCRouting::Receive {
 				payment_data: data,
 				payment_metadata,
-				incoming_cltv_expiry: hop_data.outgoing_cltv_value,
+				incoming_cltv_expiry: outgoing_cltv_value,
 				phantom_shared_secret,
 			}
 		} else {
@@ -2752,8 +2754,8 @@ where
 			payment_hash,
 			incoming_shared_secret: shared_secret,
 			incoming_amt_msat: Some(amt_msat),
-			outgoing_amt_msat: hop_data.amt_to_forward,
-			outgoing_cltv_value: hop_data.outgoing_cltv_value,
+			outgoing_amt_msat: onion_amt_msat,
+			outgoing_cltv_value,
 			skimmed_fee_msat: counterparty_skimmed_fee_msat,
 		})
 	}
@@ -2817,9 +2819,8 @@ where
 		};
 		let (outgoing_scid, outgoing_amt_msat, outgoing_cltv_value, next_packet_pk_opt) = match next_hop {
 			onion_utils::Hop::Forward {
-				next_hop_data: msgs::OnionHopData {
-					format: msgs::OnionHopDataFormat::NonFinalNode { short_channel_id }, amt_to_forward,
-					outgoing_cltv_value,
+				next_hop_data: msgs::InboundPayload::Forward {
+					short_channel_id, amt_to_forward, outgoing_cltv_value
 				}, ..
 			} => {
 				let next_pk = onion_utils::next_hop_packet_pubkey(&self.secp_ctx,
@@ -2829,9 +2830,7 @@ where
 			// We'll do receive checks in [`Self::construct_pending_htlc_info`] so we have access to the
 			// inbound channel's state.
 			onion_utils::Hop::Receive { .. } => return Ok((next_hop, shared_secret, None)),
-			onion_utils::Hop::Forward {
-				next_hop_data: msgs::OnionHopData { format: msgs::OnionHopDataFormat::FinalNode { .. }, .. }, ..
-			} => {
+			onion_utils::Hop::Forward { next_hop_data: msgs::InboundPayload::Receive { .. }, .. } => {
 				return_err!("Final Node OnionHopData provided for us as an intermediary node", 0x4000 | 22, &[0; 0]);
 			}
 		};
@@ -10021,16 +10020,14 @@ mod tests {
 		let node = create_network(1, &node_cfg, &node_chanmgr);
 		let sender_intended_amt_msat = 100;
 		let extra_fee_msat = 10;
-		let hop_data = msgs::OnionHopData {
-			amt_to_forward: 100,
+		let hop_data = msgs::InboundPayload::Receive {
+			amt_msat: 100,
 			outgoing_cltv_value: 42,
-			format: msgs::OnionHopDataFormat::FinalNode {
-				keysend_preimage: None,
-				payment_metadata: None,
-				payment_data: Some(msgs::FinalOnionHopData {
-					payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat,
-				}),
-			}
+			payment_metadata: None,
+			keysend_preimage: None,
+			payment_data: Some(msgs::FinalOnionHopData {
+				payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat,
+			}),
 		};
 		// Check that if the amount we received + the penultimate hop extra fee is less than the sender
 		// intended amount, we fail the payment.
@@ -10042,16 +10039,14 @@ mod tests {
 		} else { panic!(); }
 
 		// If amt_received + extra_fee is equal to the sender intended amount, we're fine.
-		let hop_data = msgs::OnionHopData { // This is the same hop_data as above, OnionHopData doesn't implement Clone
-			amt_to_forward: 100,
+		let hop_data = msgs::InboundPayload::Receive { // This is the same payload as above, InboundPayload doesn't implement Clone
+			amt_msat: 100,
 			outgoing_cltv_value: 42,
-			format: msgs::OnionHopDataFormat::FinalNode {
-				keysend_preimage: None,
-				payment_metadata: None,
-				payment_data: Some(msgs::FinalOnionHopData {
-					payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat,
-				}),
-			}
+			payment_metadata: None,
+			keysend_preimage: None,
+			payment_data: Some(msgs::FinalOnionHopData {
+				payment_secret: PaymentSecret([0; 32]), total_msat: sender_intended_amt_msat,
+			}),
 		};
 		assert!(node[0].node.construct_recv_pending_htlc_info(hop_data, [0; 32], PaymentHash([0; 32]),
 			sender_intended_amt_msat - extra_fee_msat, 42, None, true, Some(extra_fee_msat)).is_ok());