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.

Also gets rid of some indentation in construct_recv_pending_htlc_info

Author: valentinewallace

fuzz/src/msg_targets/msg_onion_hop_data.rs

@@ -13,13 +13,44 @@
 use crate::msg_targets::utils::VecWriter;
 use crate::utils::test_logger;
 
+macro_rules! test_onion_payload_ser_roundtrip {
+	($data: ident) => {
+		{
+			use lightning::util::ser::{Writeable, Readable};
+			let mut r = ::std::io::Cursor::new($data);
+			if let Ok(inbound_payload_1) = <lightning::ln::msgs::InboundPayload as Readable>::read(&mut r) {
+				let outbound_payload_1 = match inbound_payload_1 {
+					lightning::ln::msgs::InboundPayload::Forward { short_channel_id, amt_to_forward, outgoing_cltv_value } =>
+						lightning::ln::msgs::OnionHopData { format: lightning::ln::msgs::OnionHopDataFormat::NonFinalNode { short_channel_id }, amt_to_forward, outgoing_cltv_value },
+					lightning::ln::msgs::InboundPayload::Receive { payment_data, keysend_preimage, amt_msat, outgoing_cltv_value } =>
+						lightning::ln::msgs::OnionHopData { format: lightning::ln::msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage }, amt_to_forward: amt_msat, outgoing_cltv_value },
+				};
+				let mut w = VecWriter(Vec::new());
+				outbound_payload_1.write(&mut w).unwrap();
+				assert_eq!(outbound_payload_1.serialized_length(), w.0.len());
+
+				let inbound_payload_2 = <lightning::ln::msgs::InboundPayload as Readable>::read(&mut ::std::io::Cursor::new(&w.0)).unwrap();
+				let outbound_payload_2 = match inbound_payload_2 {
+					lightning::ln::msgs::InboundPayload::Forward { short_channel_id, amt_to_forward, outgoing_cltv_value } =>
+						lightning::ln::msgs::OnionHopData { format: lightning::ln::msgs::OnionHopDataFormat::NonFinalNode { short_channel_id }, amt_to_forward, outgoing_cltv_value },
+					lightning::ln::msgs::InboundPayload::Receive { payment_data, keysend_preimage, amt_msat, outgoing_cltv_value } =>
+						lightning::ln::msgs::OnionHopData { format: lightning::ln::msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage }, amt_to_forward: amt_msat, outgoing_cltv_value },
+				};
+				let mut w_two = VecWriter(Vec::new());
+				outbound_payload_2 .write(&mut w_two).unwrap();
+				assert_eq!(&w.0[..], &w_two.0[..]);
+			}
+		}
+	}
+}
+
 #[inline]
 pub fn msg_onion_hop_data_test<Out: test_logger::Output>(data: &[u8], _out: Out) {
-	test_msg_simple!(lightning::ln::msgs::OnionHopData, data);
+	test_onion_payload_ser_roundtrip!(data);
 }
 
 #[no_mangle]
 pub extern "C" fn msg_onion_hop_data_run(data: *const u8, datalen: usize) {
 	let data = unsafe { std::slice::from_raw_parts(data, datalen) };
-	test_msg_simple!(lightning::ln::msgs::OnionHopData, data);
+	test_onion_payload_ser_roundtrip!(data);
 }

lightning/src/ln/channelmanager.rs

@@ -2087,46 +2087,57 @@ 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],
 	) -> Result<PendingHTLCInfo, InboundOnionErr> {
 		let new_pubkey = msg.onion_routing_packet.public_key.unwrap();
 		let outgoing_packet = msgs::OnionPacket {
 			version: 0,
 			public_key: onion_utils::next_hop_packet_pubkey(&self.secp_ctx, new_pubkey, &shared_secret),
 			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,
 		})
 	}
 
-	fn construct_recv_pending_htlc_info(&self, hop_data: msgs::OnionHopData, shared_secret: [u8; 32],
-		payment_hash: PaymentHash, amt_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>) -> Result<PendingHTLCInfo, InboundOnionErr>
+	fn construct_recv_pending_htlc_info(&self, hop_data: msgs::InboundPayload, shared_secret: [u8; 32],
+		payment_hash: PaymentHash, amount_msat: u64, cltv_expiry: u32, phantom_shared_secret: Option<[u8; 32]>) -> Result<PendingHTLCInfo, InboundOnionErr>
 	{
+		let (payment_data, keysend_preimage, amt_msat, outgoing_cltv_value) = match hop_data {
+			msgs::InboundPayload::Receive { payment_data, keysend_preimage, amt_msat, outgoing_cltv_value, .. } =>
+				(payment_data, keysend_preimage, amt_msat, outgoing_cltv_value),
+			_ =>
+				return Err(InboundOnionErr {
+					err_code: 0x4000|22,
+					err_data: Vec::new(),
+					msg: "Got non final data with an HMAC of 0",
+				}),
+		};
+
 		// 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 the wrong value",
 				err_code: 18,
@@ -2141,7 +2152,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());
@@ -2150,70 +2161,60 @@ where
 				msg: "The final CLTV expiry is too soon to handle",
 			});
 		}
-		if hop_data.amt_to_forward > amt_msat {
+		if amt_msat > amount_msat {
 			return Err(InboundOnionErr {
 				err_code: 19,
 				err_data: amt_msat.to_be_bytes().to_vec(),
 				msg: "Upstream node sent less than we were supposed to receive in payment",
 			});
 		}
 
-		let routing = match hop_data.format {
-			msgs::OnionHopDataFormat::NonFinalNode { .. } => {
+		let routing;
+		if payment_data.is_some() && keysend_preimage.is_some() {
+			return Err(InboundOnionErr {
+				err_code: 0x4000|22,
+				err_data: Vec::new(),
+				msg: "We don't support MPP keysend payments",
+			});
+		} else if let Some(data) = payment_data {
+			routing = PendingHTLCRouting::Receive {
+				payment_data: data,
+				incoming_cltv_expiry: outgoing_cltv_value,
+				phantom_shared_secret,
+			}
+		} else if let Some(payment_preimage) = keysend_preimage {
+			// We need to check that the sender knows the keysend preimage before processing this
+			// payment further. Otherwise, an intermediary routing hop forwarding non-keysend-HTLC X
+			// could discover the final destination of X, by probing the adjacent nodes on the route
+			// with a keysend payment of identical payment hash to X and observing the processing
+			// time discrepancies due to a hash collision with X.
+			let hashed_preimage = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
+			if hashed_preimage != payment_hash {
 				return Err(InboundOnionErr {
 					err_code: 0x4000|22,
 					err_data: Vec::new(),
-					msg: "Got non final data with an HMAC of 0",
+					msg: "Payment preimage didn't match payment hash",
 				});
-			},
-			msgs::OnionHopDataFormat::FinalNode { payment_data, keysend_preimage } => {
-				if payment_data.is_some() && keysend_preimage.is_some() {
-					return Err(InboundOnionErr {
-						err_code: 0x4000|22,
-						err_data: Vec::new(),
-						msg: "We don't support MPP keysend payments",
-					});
-				} else if let Some(data) = payment_data {
-					PendingHTLCRouting::Receive {
-						payment_data: data,
-						incoming_cltv_expiry: hop_data.outgoing_cltv_value,
-						phantom_shared_secret,
-					}
-				} else if let Some(payment_preimage) = keysend_preimage {
-					// We need to check that the sender knows the keysend preimage before processing this
-					// payment further. Otherwise, an intermediary routing hop forwarding non-keysend-HTLC X
-					// could discover the final destination of X, by probing the adjacent nodes on the route
-					// with a keysend payment of identical payment hash to X and observing the processing
-					// time discrepancies due to a hash collision with X.
-					let hashed_preimage = PaymentHash(Sha256::hash(&payment_preimage.0).into_inner());
-					if hashed_preimage != payment_hash {
-						return Err(InboundOnionErr {
-							err_code: 0x4000|22,
-							err_data: Vec::new(),
-							msg: "Payment preimage didn't match payment hash",
-						});
-					}
+			}
 
-					PendingHTLCRouting::ReceiveKeysend {
-						payment_preimage,
-						incoming_cltv_expiry: hop_data.outgoing_cltv_value,
-					}
-				} else {
-					return Err(InboundOnionErr {
-						err_code: 0x4000|0x2000|3,
-						err_data: Vec::new(),
-						msg: "We require payment_secrets",
-					});
-				}
-			},
-		};
+			routing = PendingHTLCRouting::ReceiveKeysend {
+				payment_preimage,
+				incoming_cltv_expiry: outgoing_cltv_value,
+			};
+		} else {
+			return Err(InboundOnionErr {
+				err_code: 0x4000|0x2000|3,
+				err_data: Vec::new(),
+				msg: "We require payment_secrets",
+			});
+		}
 		Ok(PendingHTLCInfo {
 			routing,
 			payment_hash,
 			incoming_shared_secret: shared_secret,
-			incoming_amt_msat: Some(amt_msat),
-			outgoing_amt_msat: amt_msat,
-			outgoing_cltv_value: hop_data.outgoing_cltv_value,
+			incoming_amt_msat: Some(amount_msat),
+			outgoing_amt_msat: amount_msat,
+			outgoing_cltv_value,
 		})
 	}