Split only-receive/forward data out of PendingHTLCInfo into an enum

This should avoid blowing up the size of the struct when we add
additional data that is only relevant for receive.

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -68,12 +68,20 @@ use std::ops::Deref;
 // Alternatively, we can fill an outbound HTLC with a HTLCSource::OutboundRoute indicating this is
 // our payment, which we can use to decode errors or inform the user that the payment was sent.
 
+#[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
+enum PendingHTLCRouting {
+	Forward {
+		onion_packet: msgs::OnionPacket,
+		short_channel_id: u64, // This should be NonZero<u64> eventually when we bump MSRV
+	},
+	Receive {},
+}
+
 #[derive(Clone)] // See Channel::revoke_and_ack for why, tl;dr: Rust bug
 pub(super) struct PendingHTLCInfo {
-	onion_packet: Option<msgs::OnionPacket>,
+	routing: PendingHTLCRouting,
 	incoming_shared_secret: [u8; 32],
 	payment_hash: PaymentHash,
-	short_channel_id: u64,
 	pub(super) amt_to_forward: u64,
 	pub(super) outgoing_cltv_value: u32,
 }
@@ -1005,9 +1013,8 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
 				// delay) once they've send us a commitment_signed!
 
 				PendingHTLCStatus::Forward(PendingHTLCInfo {
-					onion_packet: None,
+					routing: PendingHTLCRouting::Receive {},
 					payment_hash: msg.payment_hash.clone(),
-					short_channel_id: 0,
 					incoming_shared_secret: shared_secret,
 					amt_to_forward: next_hop_data.amt_to_forward,
 					outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
@@ -1051,24 +1058,29 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
 				let short_channel_id = match next_hop_data.format {
 					msgs::OnionHopDataFormat::Legacy { short_channel_id } => short_channel_id,
 					msgs::OnionHopDataFormat::NonFinalNode { short_channel_id } => short_channel_id,
-					msgs::OnionHopDataFormat::FinalNode => {
+					msgs::OnionHopDataFormat::FinalNode { .. } => {
 						return_err!("Final Node OnionHopData provided for us as an intermediary node", 0x4000 | 22, &[0;0]);
 					},
 				};
 
 				PendingHTLCStatus::Forward(PendingHTLCInfo {
-					onion_packet: Some(outgoing_packet),
+					routing: PendingHTLCRouting::Forward {
+						onion_packet: outgoing_packet,
+						short_channel_id: short_channel_id,
+					},
 					payment_hash: msg.payment_hash.clone(),
-					short_channel_id: short_channel_id,
 					incoming_shared_secret: shared_secret,
 					amt_to_forward: next_hop_data.amt_to_forward,
 					outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
 				})
 			};
 
 		channel_state = Some(self.channel_state.lock().unwrap());
-		if let &PendingHTLCStatus::Forward(PendingHTLCInfo { ref onion_packet, ref short_channel_id, ref amt_to_forward, ref outgoing_cltv_value, .. }) = &pending_forward_info {
-			if onion_packet.is_some() { // If short_channel_id is 0 here, we'll reject them in the body here
+		if let &PendingHTLCStatus::Forward(PendingHTLCInfo { ref routing, ref amt_to_forward, ref outgoing_cltv_value, .. }) = &pending_forward_info {
+			// If short_channel_id is 0 here, we'll reject the HTLC as there cannot be a channel
+			// with a short_channel_id of 0. This is important as various things later assume
+			// short_channel_id is non-0 in any ::Forward.
+			if let &PendingHTLCRouting::Forward { ref short_channel_id, .. } = routing {
 				let id_option = channel_state.as_ref().unwrap().short_to_id.get(&short_channel_id).cloned();
 				let forwarding_id = match id_option {
 					None => { // unknown_next_peer
@@ -1450,22 +1462,25 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
 						let mut fail_htlc_msgs = Vec::new();
 						for forward_info in pending_forwards.drain(..) {
 							match forward_info {
-								HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info } => {
-									log_trace!(self, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", log_bytes!(forward_info.payment_hash.0), prev_short_channel_id, short_chan_id);
+								HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
+										routing: PendingHTLCRouting::Forward {
+											onion_packet, ..
+										}, incoming_shared_secret, payment_hash, amt_to_forward, outgoing_cltv_value }, } => {
+									log_trace!(self, "Adding HTLC from short id {} with payment_hash {} to channel with short id {} after delay", log_bytes!(payment_hash.0), prev_short_channel_id, short_chan_id);
 									let htlc_source = HTLCSource::PreviousHopData(HTLCPreviousHopData {
 										short_channel_id: prev_short_channel_id,
 										htlc_id: prev_htlc_id,
-										incoming_packet_shared_secret: forward_info.incoming_shared_secret,
+										incoming_packet_shared_secret: incoming_shared_secret,
 									});
-									match chan.get_mut().send_htlc(forward_info.amt_to_forward, forward_info.payment_hash, forward_info.outgoing_cltv_value, htlc_source.clone(), forward_info.onion_packet.unwrap()) {
+									match chan.get_mut().send_htlc(amt_to_forward, payment_hash, outgoing_cltv_value, htlc_source.clone(), onion_packet) {
 										Err(e) => {
 											if let ChannelError::Ignore(msg) = e {
-												log_trace!(self, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(forward_info.payment_hash.0), msg);
+												log_trace!(self, "Failed to forward HTLC with payment_hash {}: {}", log_bytes!(payment_hash.0), msg);
 											} else {
 												panic!("Stated return value requirements in send_htlc() were not met");
 											}
 											let chan_update = self.get_channel_update(chan.get()).unwrap();
-											failed_forwards.push((htlc_source, forward_info.payment_hash, 0x1000 | 7, Some(chan_update)));
+											failed_forwards.push((htlc_source, payment_hash, 0x1000 | 7, Some(chan_update)));
 											continue;
 										},
 										Ok(update_add) => {
@@ -1484,6 +1499,9 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
 										}
 									}
 								},
+								HTLCForwardInfo::AddHTLC { .. } => {
+									panic!("short_channel_id != 0 should imply any pending_forward entries are of type Forward");
+								},
 								HTLCForwardInfo::FailHTLC { htlc_id, err_packet } => {
 									log_trace!(self, "Failing HTLC back to channel with short id {} after delay", short_chan_id);
 									match chan.get_mut().get_update_fail_htlc(htlc_id, err_packet) {
@@ -1561,21 +1579,26 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
 				} else {
 					for forward_info in pending_forwards.drain(..) {
 						match forward_info {
-							HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info } => {
+							HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info: PendingHTLCInfo {
+									routing: PendingHTLCRouting::Receive { },
+									incoming_shared_secret, payment_hash, amt_to_forward, .. }, } => {
 								let prev_hop_data = HTLCPreviousHopData {
 									short_channel_id: prev_short_channel_id,
 									htlc_id: prev_htlc_id,
-									incoming_packet_shared_secret: forward_info.incoming_shared_secret,
+									incoming_packet_shared_secret: incoming_shared_secret,
 								};
-								match channel_state.claimable_htlcs.entry(forward_info.payment_hash) {
-									hash_map::Entry::Occupied(mut entry) => entry.get_mut().push((forward_info.amt_to_forward, prev_hop_data)),
-									hash_map::Entry::Vacant(entry) => { entry.insert(vec![(forward_info.amt_to_forward, prev_hop_data)]); },
+								match channel_state.claimable_htlcs.entry(payment_hash) {
+									hash_map::Entry::Occupied(mut entry) => entry.get_mut().push((amt_to_forward, prev_hop_data)),
+									hash_map::Entry::Vacant(entry) => { entry.insert(vec![(amt_to_forward, prev_hop_data)]); },
 								};
 								new_events.push(events::Event::PaymentReceived {
-									payment_hash: forward_info.payment_hash,
-									amt: forward_info.amt_to_forward,
+									payment_hash: payment_hash,
+									amt: amt_to_forward,
 								});
 							},
+							HTLCForwardInfo::AddHTLC { .. } => {
+								panic!("short_channel_id == 0 should imply any pending_forward entries are of type Receive");
+							},
 							HTLCForwardInfo::FailHTLC { .. } => {
 								panic!("Got pending fail of our own HTLC");
 							}
@@ -2388,7 +2411,10 @@ impl<ChanSigner: ChannelKeys, M: Deref, T: Deref, K: Deref, F: Deref> ChannelMan
 					forward_event = Some(Duration::from_millis(MIN_HTLC_RELAY_HOLDING_CELL_MILLIS))
 				}
 				for (forward_info, prev_htlc_id) in pending_forwards.drain(..) {
-					match channel_state.forward_htlcs.entry(forward_info.short_channel_id) {
+					match channel_state.forward_htlcs.entry(match forward_info.routing {
+							PendingHTLCRouting::Forward { short_channel_id, .. } => short_channel_id,
+							PendingHTLCRouting::Receive { .. } => 0,
+					}) {
 						hash_map::Entry::Occupied(mut entry) => {
 							entry.get_mut().push(HTLCForwardInfo::AddHTLC { prev_short_channel_id, prev_htlc_id, forward_info });
 						},
@@ -3066,10 +3092,18 @@ const MIN_SERIALIZATION_VERSION: u8 = 1;
 
 impl Writeable for PendingHTLCInfo {
 	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
-		self.onion_packet.write(writer)?;
+		match &self.routing {
+			&PendingHTLCRouting::Forward { ref onion_packet, ref short_channel_id } => {
+				0u8.write(writer)?;
+				onion_packet.write(writer)?;
+				short_channel_id.write(writer)?;
+			},
+			&PendingHTLCRouting::Receive { } => {
+				1u8.write(writer)?;
+			},
+		}
 		self.incoming_shared_secret.write(writer)?;
 		self.payment_hash.write(writer)?;
-		self.short_channel_id.write(writer)?;
 		self.amt_to_forward.write(writer)?;
 		self.outgoing_cltv_value.write(writer)?;
 		Ok(())
@@ -3079,10 +3113,16 @@ impl Writeable for PendingHTLCInfo {
 impl Readable for PendingHTLCInfo {
 	fn read<R: ::std::io::Read>(reader: &mut R) -> Result<PendingHTLCInfo, DecodeError> {
 		Ok(PendingHTLCInfo {
-			onion_packet: Readable::read(reader)?,
+			routing: match Readable::read(reader)? {
+				0u8 => PendingHTLCRouting::Forward {
+					onion_packet: Readable::read(reader)?,
+					short_channel_id: Readable::read(reader)?,
+				},
+				1u8 => PendingHTLCRouting::Receive { },
+				_ => return Err(DecodeError::InvalidValue),
+			},
 			incoming_shared_secret: Readable::read(reader)?,
 			payment_hash: Readable::read(reader)?,
-			short_channel_id: Readable::read(reader)?,
 			amt_to_forward: Readable::read(reader)?,
 			outgoing_cltv_value: Readable::read(reader)?,
 		})