Flatten OnionHopData struct with the Realm0 struct.

Previously OnionHopData contained a OnionRealm0HopData field however
instead of bumping the realm number, it has been replaced with a
length, used to indicte the length of a TLV-formatted object.

Because a TLV-formatted hop data can contain the same information as
a realm-0 hop data, we flatten the field and simply keep track of
what format it was in.

Author: TheBlueMatt

lightning/src/ln/channelmanager.rs

@@ -943,11 +943,11 @@ impl<ChanSigner: ChannelKeys, M: Deref> ChannelManager<ChanSigner, M> where M::T
 					return_err!("The final CLTV expiry is too soon to handle", 17, &[0;0]);
 				}
 				// final_incorrect_htlc_amount
-				if next_hop_data.data.amt_to_forward > msg.amount_msat {
+				if next_hop_data.amt_to_forward > msg.amount_msat {
 					return_err!("Upstream node sent less than we were supposed to receive in payment", 19, &byte_utils::be64_to_array(msg.amount_msat));
 				}
 				// final_incorrect_cltv_expiry
-				if next_hop_data.data.outgoing_cltv_value != msg.cltv_expiry {
+				if next_hop_data.outgoing_cltv_value != msg.cltv_expiry {
 					return_err!("Upstream node set CLTV to the wrong value", 18, &byte_utils::be32_to_array(msg.cltv_expiry));
 				}
 
@@ -961,8 +961,8 @@ impl<ChanSigner: ChannelKeys, M: Deref> ChannelManager<ChanSigner, M> where M::T
 					payment_hash: msg.payment_hash.clone(),
 					short_channel_id: 0,
 					incoming_shared_secret: shared_secret,
-					amt_to_forward: next_hop_data.data.amt_to_forward,
-					outgoing_cltv_value: next_hop_data.data.outgoing_cltv_value,
+					amt_to_forward: next_hop_data.amt_to_forward,
+					outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
 				})
 			} else {
 				let mut new_packet_data = [0; 20*65];
@@ -992,10 +992,10 @@ impl<ChanSigner: ChannelKeys, M: Deref> ChannelManager<ChanSigner, M> where M::T
 				PendingHTLCStatus::Forward(PendingForwardHTLCInfo {
 					onion_packet: Some(outgoing_packet),
 					payment_hash: msg.payment_hash.clone(),
-					short_channel_id: next_hop_data.data.short_channel_id,
+					short_channel_id: next_hop_data.short_channel_id,
 					incoming_shared_secret: shared_secret,
-					amt_to_forward: next_hop_data.data.amt_to_forward,
-					outgoing_cltv_value: next_hop_data.data.outgoing_cltv_value,
+					amt_to_forward: next_hop_data.amt_to_forward,
+					outgoing_cltv_value: next_hop_data.outgoing_cltv_value,
 				})
 			};
 

lightning/src/ln/functional_tests.rs

@@ -5001,7 +5001,7 @@ fn test_onion_failure() {
 		let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
 		let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route, &session_priv).unwrap();
 		let (mut onion_payloads, _htlc_msat, _htlc_cltv) = onion_utils::build_onion_payloads(&route, cur_height).unwrap();
-		onion_payloads[0].realm = 3;
+		onion_payloads[0].format = msgs::OnionHopDataFormat::BogusRealm(3);
 		msg.onion_routing_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
 	}, ||{}, true, Some(PERM|1), Some(msgs::HTLCFailChannelUpdate::ChannelClosed{short_channel_id: channels[1].0.contents.short_channel_id, is_permanent: true}));//XXX incremented channels idx here
 
@@ -5011,7 +5011,7 @@ fn test_onion_failure() {
 		let cur_height = nodes[0].node.latest_block_height.load(Ordering::Acquire) as u32 + 1;
 		let onion_keys = onion_utils::construct_onion_keys(&Secp256k1::new(), &route, &session_priv).unwrap();
 		let (mut onion_payloads, _htlc_msat, _htlc_cltv) = onion_utils::build_onion_payloads(&route, cur_height).unwrap();
-		onion_payloads[1].realm = 3;
+		onion_payloads[1].format = msgs::OnionHopDataFormat::BogusRealm(3);
 		msg.onion_routing_packet = onion_utils::construct_onion_packet(onion_payloads, onion_keys, [0; 32], &payment_hash);
 	}, ||{}, false, Some(PERM|1), Some(msgs::HTLCFailChannelUpdate::ChannelClosed{short_channel_id: channels[1].0.contents.short_channel_id, is_permanent: true}));
 

lightning/src/ln/msgs.rs

@@ -606,21 +606,25 @@ pub trait RoutingMessageHandler : Send + Sync {
 	fn get_next_node_announcements(&self, starting_point: Option<&PublicKey>, batch_amount: u8) -> Vec<NodeAnnouncement>;
 }
 
-pub(crate) struct OnionRealm0HopData {
-	pub(crate) short_channel_id: u64,
-	pub(crate) amt_to_forward: u64,
-	pub(crate) outgoing_cltv_value: u32,
-	// 12 bytes of 0-padding
-}
-
 mod fuzzy_internal_msgs {
 	// These types aren't intended to be pub, but are exposed for direct fuzzing (as we deserialize
 	// them from untrusted input):
 
-	use super::OnionRealm0HopData;
+	pub(crate) enum OnionHopDataFormat {
+		Legacy, // aka Realm-0
+		// Some tests expect to be able to generate bogus non-deserializable OnionHopDatas. In the
+		// future we can use bogus TLV attributes, but for now we have to expose a "bogus realm"
+		// option.
+		#[cfg(test)]
+		BogusRealm(u8),
+	}
+
 	pub struct OnionHopData {
-		pub(crate) realm: u8,
-		pub(crate) data: OnionRealm0HopData,
+		pub(crate) format: OnionHopDataFormat,
+		pub(crate) short_channel_id: u64,
+		pub(crate) amt_to_forward: u64,
+		pub(crate) outgoing_cltv_value: u32,
+		// 12 bytes of 0-padding
 		pub(crate) hmac: [u8; 32],
 	}
 
@@ -958,36 +962,18 @@ impl_writeable!(UpdateAddHTLC, 32+8+8+32+4+1366, {
 	onion_routing_packet
 });
 
-impl Writeable for OnionRealm0HopData {
+impl Writeable for OnionHopData {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
-		w.size_hint(32);
+		w.size_hint(65);
+		match self.format {
+			OnionHopDataFormat::Legacy => 0u8.write(w)?,
+			#[cfg(test)]
+			OnionHopDataFormat::BogusRealm(v) => v.write(w)?,
+		}
 		self.short_channel_id.write(w)?;
 		self.amt_to_forward.write(w)?;
 		self.outgoing_cltv_value.write(w)?;
 		w.write_all(&[0;12])?;
-		Ok(())
-	}
-}
-
-impl<R: Read> Readable<R> for OnionRealm0HopData {
-	fn read(r: &mut R) -> Result<Self, DecodeError> {
-		Ok(OnionRealm0HopData {
-			short_channel_id: Readable::read(r)?,
-			amt_to_forward: Readable::read(r)?,
-			outgoing_cltv_value: {
-				let v: u32 = Readable::read(r)?;
-				r.read_exact(&mut [0; 12])?;
-				v
-			}
-		})
-	}
-}
-
-impl Writeable for OnionHopData {
-	fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
-		w.size_hint(65);
-		self.realm.write(w)?;
-		self.data.write(w)?;
 		self.hmac.write(w)?;
 		Ok(())
 	}
@@ -996,14 +982,20 @@ impl Writeable for OnionHopData {
 impl<R: Read> Readable<R> for OnionHopData {
 	fn read(r: &mut R) -> Result<Self, DecodeError> {
 		Ok(OnionHopData {
-			realm: {
+			format: {
 				let r: u8 = Readable::read(r)?;
 				if r != 0 {
 					return Err(DecodeError::UnknownVersion);
 				}
-				r
+				OnionHopDataFormat::Legacy
+			},
+			short_channel_id: Readable::read(r)?,
+			amt_to_forward: Readable::read(r)?,
+			outgoing_cltv_value: {
+				let v: u32 = Readable::read(r)?;
+				r.read_exact(&mut [0; 12])?;
+				v
 			},
-			data: Readable::read(r)?,
 			hmac: Readable::read(r)?,
 		})
 	}

lightning/src/ln/onion_utils.rs

@@ -121,12 +121,10 @@ pub(super) fn build_onion_payloads(route: &Route, starting_htlc_offset: u32) ->
 		let value_msat = if cur_value_msat == 0 { hop.fee_msat } else { cur_value_msat };
 		let cltv = if cur_cltv == starting_htlc_offset { hop.cltv_expiry_delta + starting_htlc_offset } else { cur_cltv };
 		res.insert(0, msgs::OnionHopData {
-			realm: 0,
-			data: msgs::OnionRealm0HopData {
-				short_channel_id: last_short_channel_id,
-				amt_to_forward: value_msat,
-				outgoing_cltv_value: cltv,
-			},
+			format: msgs::OnionHopDataFormat::Legacy,
+			short_channel_id: last_short_channel_id,
+			amt_to_forward: value_msat,
+			outgoing_cltv_value: cltv,
 			hmac: [0; 32],
 		});
 		cur_value_msat += hop.fee_msat;
@@ -516,48 +514,38 @@ mod tests {
 		// Test vectors below are flat-out wrong: they claim to set outgoing_cltv_value to non-0 :/
 		let payloads = vec!(
 			msgs::OnionHopData {
-				realm: 0,
-				data: msgs::OnionRealm0HopData {
-					short_channel_id: 0,
-					amt_to_forward: 0,
-					outgoing_cltv_value: 0,
-				},
+				format: msgs::OnionHopDataFormat::Legacy,
+				short_channel_id: 0,
+				amt_to_forward: 0,
+				outgoing_cltv_value: 0,
 				hmac: [0; 32],
 			},
 			msgs::OnionHopData {
-				realm: 0,
-				data: msgs::OnionRealm0HopData {
-					short_channel_id: 0x0101010101010101,
-					amt_to_forward: 0x0100000001,
-					outgoing_cltv_value: 0,
-				},
+				format: msgs::OnionHopDataFormat::Legacy,
+				short_channel_id: 0x0101010101010101,
+				amt_to_forward: 0x0100000001,
+				outgoing_cltv_value: 0,
 				hmac: [0; 32],
 			},
 			msgs::OnionHopData {
-				realm: 0,
-				data: msgs::OnionRealm0HopData {
-					short_channel_id: 0x0202020202020202,
-					amt_to_forward: 0x0200000002,
-					outgoing_cltv_value: 0,
-				},
+				format: msgs::OnionHopDataFormat::Legacy,
+				short_channel_id: 0x0202020202020202,
+				amt_to_forward: 0x0200000002,
+				outgoing_cltv_value: 0,
 				hmac: [0; 32],
 			},
 			msgs::OnionHopData {
-				realm: 0,
-				data: msgs::OnionRealm0HopData {
-					short_channel_id: 0x0303030303030303,
-					amt_to_forward: 0x0300000003,
-					outgoing_cltv_value: 0,
-				},
+				format: msgs::OnionHopDataFormat::Legacy,
+				short_channel_id: 0x0303030303030303,
+				amt_to_forward: 0x0300000003,
+				outgoing_cltv_value: 0,
 				hmac: [0; 32],
 			},
 			msgs::OnionHopData {
-				realm: 0,
-				data: msgs::OnionRealm0HopData {
-					short_channel_id: 0x0404040404040404,
-					amt_to_forward: 0x0400000004,
-					outgoing_cltv_value: 0,
-				},
+				format: msgs::OnionHopDataFormat::Legacy,
+				short_channel_id: 0x0404040404040404,
+				amt_to_forward: 0x0400000004,
+				outgoing_cltv_value: 0,
 				hmac: [0; 32],
 			},
 		);