Support serializing TLV fields which may or may not be present

Author: TheBlueMatt

lightning/src/chain/channelmonitor.rs

@@ -988,7 +988,7 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
 		self.lockdown_from_offchain.write(writer)?;
 		self.holder_tx_signed.write(writer)?;
 
-		write_tlv_fields!(writer, {});
+		write_tlv_fields!(writer, {}, {});
 
 		Ok(())
 	}

lightning/src/ln/channel.rs

@@ -4565,7 +4565,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
 
 		self.channel_update_status.write(writer)?;
 
-		write_tlv_fields!(writer, {});
+		write_tlv_fields!(writer, {}, {});
 
 		Ok(())
 	}

lightning/src/ln/channelmanager.rs

@@ -4651,7 +4651,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
 			session_priv.write(writer)?;
 		}
 
-		write_tlv_fields!(writer, {});
+		write_tlv_fields!(writer, {}, {});
 
 		Ok(())
 	}

lightning/src/ln/msgs.rs

@@ -1285,20 +1285,17 @@ impl Writeable for OnionHopData {
 					(2, HighZeroBytesDroppedVarInt(self.amt_to_forward)),
 					(4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)),
 					(6, short_channel_id)
-				});
+				}, { });
 			},
-			OnionHopDataFormat::FinalNode { payment_data: Some(ref final_data) } => {
-				if final_data.total_msat > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); }
-				encode_varint_length_prefixed_tlv!(w, {
-					(2, HighZeroBytesDroppedVarInt(self.amt_to_forward)),
-					(4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value)),
-					(8, final_data)
-				});
-			},
-			OnionHopDataFormat::FinalNode { payment_data: None } => {
+			OnionHopDataFormat::FinalNode { ref payment_data } => {
+				if let Some(final_data) = payment_data {
+					if final_data.total_msat > MAX_VALUE_MSAT { panic!("We should never be sending infinite/overflow onion payments"); }
+				}
 				encode_varint_length_prefixed_tlv!(w, {
 					(2, HighZeroBytesDroppedVarInt(self.amt_to_forward)),
 					(4, HighZeroBytesDroppedVarInt(self.outgoing_cltv_value))
+				}, {
+					(8, payment_data)
 				});
 			},
 		}

lightning/src/ln/onchaintx.rs

@@ -361,7 +361,7 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
 		}
 		self.latest_height.write(writer)?;
 
-		write_tlv_fields!(writer, {});
+		write_tlv_fields!(writer, {}, {});
 		Ok(())
 	}
 }

lightning/src/util/ser_macros.rs

@@ -8,21 +8,51 @@
 // licenses.
 
 macro_rules! encode_tlv {
-	($stream: expr, {$(($type: expr, $field: expr)),*}) => { {
+	($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { {
 		#[allow(unused_imports)]
 		use util::ser::{BigSize, LengthCalculatingWriter};
+		// Fields must be serialized in order, so we have to potentially switch between optional
+		// fields and normal fields while serializing. Thus, we end up having to loop over the type
+		// counts.
+		// Sadly, while LLVM does appear smart enough to make `max_field` a constant, it appears to
+		// refuse to unroll the loop. If we have enough entries that this is slow we can revisit
+		// this design in the future.
+		#[allow(unused_mut)]
+		let mut max_field: u64 = 0;
+		$(
+			if $type >= max_field { max_field = $type + 1; }
+		)*
 		$(
-			BigSize($type).write($stream)?;
-			let mut len_calc = LengthCalculatingWriter(0);
-			$field.write(&mut len_calc)?;
-			BigSize(len_calc.0 as u64).write($stream)?;
-			$field.write($stream)?;
+			if $optional_type >= max_field { max_field = $optional_type + 1; }
 		)*
+		#[allow(unused_variables)]
+		for i in 0..max_field {
+			$(
+				if i == $type {
+					BigSize($type).write($stream)?;
+					let mut len_calc = LengthCalculatingWriter(0);
+					$field.write(&mut len_calc)?;
+					BigSize(len_calc.0 as u64).write($stream)?;
+					$field.write($stream)?;
+				}
+			)*
+			$(
+				if i == $optional_type {
+					if let Some(ref field) = $optional_field {
+						BigSize($optional_type).write($stream)?;
+						let mut len_calc = LengthCalculatingWriter(0);
+						field.write(&mut len_calc)?;
+						BigSize(len_calc.0 as u64).write($stream)?;
+						field.write($stream)?;
+					}
+				}
+			)*
+		}
 	} }
 }
 
 macro_rules! encode_varint_length_prefixed_tlv {
-	($stream: expr, {$(($type: expr, $field: expr)),*}) => { {
+	($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { {
 		use util::ser::{BigSize, LengthCalculatingWriter};
 		#[allow(unused_mut)]
 		let mut len = LengthCalculatingWriter(0);
@@ -34,12 +64,19 @@ macro_rules! encode_varint_length_prefixed_tlv {
 				BigSize(field_len.0 as u64).write(&mut len)?;
 				len.0 += field_len.0;
 			)*
+			$(
+				if let Some(ref field) = $optional_field {
+					BigSize($optional_type).write(&mut len)?;
+					let mut field_len = LengthCalculatingWriter(0);
+					field.write(&mut field_len)?;
+					BigSize(field_len.0 as u64).write(&mut len)?;
+					len.0 += field_len.0;
+				}
+			)*
 		}
 
 		BigSize(len.0 as u64).write($stream)?;
-		encode_tlv!($stream, {
-			$(($type, $field)),*
-		});
+		encode_tlv!($stream, { $(($type, $field)),* }, { $(($optional_type, $optional_field)),* });
 	} }
 }
 
@@ -209,8 +246,8 @@ macro_rules! write_ver_prefix {
 /// This is the preferred method of adding new fields that old nodes can ignore and still function
 /// correctly.
 macro_rules! write_tlv_fields {
-	($stream: expr, {$(($type: expr, $field: expr)),*}) => {
-		encode_varint_length_prefixed_tlv!($stream, {$(($type, $field)),*});
+	($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => {
+		encode_varint_length_prefixed_tlv!($stream, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*});
 	}
 }
 
@@ -435,19 +472,27 @@ mod tests {
 		let mut stream = VecWriter(Vec::new());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { (1, 1u8) });
+		encode_varint_length_prefixed_tlv!(&mut stream, { (1, 1u8) }, { (42, None::<u64>) });
 		assert_eq!(stream.0, ::hex::decode("03010101").unwrap());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { (4, 0xabcdu16) });
+		encode_varint_length_prefixed_tlv!(&mut stream, { }, { (1, Some(1u8)) });
+		assert_eq!(stream.0, ::hex::decode("03010101").unwrap());
+
+		stream.0.clear();
+		encode_varint_length_prefixed_tlv!(&mut stream, { (4, 0xabcdu16) }, { (42, None::<u64>) });
 		assert_eq!(stream.0, ::hex::decode("040402abcd").unwrap());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { (0xff, 0xabcdu16) });
+		encode_varint_length_prefixed_tlv!(&mut stream, { (0xff, 0xabcdu16) }, { (42, None::<u64>) });
 		assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { (0, 1u64), (0xff, HighZeroBytesDroppedVarInt(0u64)) });
+		encode_varint_length_prefixed_tlv!(&mut stream, { (0, 1u64), (0xff, HighZeroBytesDroppedVarInt(0u64)) }, { (42, None::<u64>) });
+		assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());
+
+		stream.0.clear();
+		encode_varint_length_prefixed_tlv!(&mut stream, { (0xff, HighZeroBytesDroppedVarInt(0u64)) }, { (0, Some(1u64)) });
 		assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());
 
 		Ok(())