Add WithoutLength wrapper

When serializing variable-length types as part of a TLV stream, the
length does not need to be serialized as it is already encoded in TLV
records. Add a WithoutLength wrapper for this encoding. Replace
VecReadWrapper and VecWriteWrapper with this single type to avoid
redundant encoders.

Author: jkczyz

lightning/src/ln/channelmanager.rs

@@ -6620,7 +6620,7 @@ impl Writeable for HTLCSource {
 					(1, payment_id_opt, option),
 					(2, first_hop_htlc_msat, required),
 					(3, payment_secret, option),
-					(4, path, vec_type),
+					(4, *path, vec_type),
 					(5, payment_params, option),
 				 });
 			}

lightning/src/onion_message/packet.rs

@@ -164,15 +164,15 @@ impl<T: CustomOnionMessageContents> Writeable for (Payload<T>, [u8; 32]) {
 		match &self.0 {
 			Payload::Forward(ForwardControlTlvs::Blinded(encrypted_bytes)) => {
 				encode_varint_length_prefixed_tlv!(w, {
-					(4, encrypted_bytes, vec_type)
+					(4, *encrypted_bytes, vec_type)
 				})
 			},
 			Payload::Receive {
 				control_tlvs: ReceiveControlTlvs::Blinded(encrypted_bytes), reply_path, message,
 			} => {
 				encode_varint_length_prefixed_tlv!(w, {
 					(2, reply_path, option),
-					(4, encrypted_bytes, vec_type),
+					(4, *encrypted_bytes, vec_type),
 					(message.tlv_type(), message, required)
 				})
 			},

lightning/src/util/events.rs

@@ -24,7 +24,7 @@ use crate::ln::msgs;
 use crate::ln::msgs::DecodeError;
 use crate::ln::{PaymentPreimage, PaymentHash, PaymentSecret};
 use crate::routing::gossip::NetworkUpdate;
-use crate::util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Readable, VecReadWrapper, VecWriteWrapper, OptionDeserWrapper};
+use crate::util::ser::{BigSize, FixedLengthReader, Writeable, Writer, MaybeReadable, Readable, WithoutLength, OptionDeserWrapper};
 use crate::routing::router::{RouteHop, RouteParameters};
 
 use bitcoin::{PackedLockTime, Transaction};
@@ -785,7 +785,7 @@ impl Writeable for Event {
 					(1, network_update, option),
 					(2, payment_failed_permanently, required),
 					(3, all_paths_failed, required),
-					(5, path, vec_type),
+					(5, *path, vec_type),
 					(7, short_channel_id, option),
 					(9, retry, option),
 					(11, payment_id, option),
@@ -799,7 +799,7 @@ impl Writeable for Event {
 			&Event::SpendableOutputs { ref outputs } => {
 				5u8.write(writer)?;
 				write_tlv_fields!(writer, {
-					(0, VecWriteWrapper(outputs), required),
+					(0, WithoutLength(outputs), required),
 				});
 			},
 			&Event::PaymentForwarded { fee_earned_msat, prev_channel_id, claim_from_onchain_tx, next_channel_id } => {
@@ -831,7 +831,7 @@ impl Writeable for Event {
 				write_tlv_fields!(writer, {
 					(0, payment_id, required),
 					(2, payment_hash, option),
-					(4, path, vec_type)
+					(4, *path, vec_type)
 				})
 			},
 			&Event::PaymentFailed { ref payment_id, ref payment_hash } => {
@@ -859,15 +859,15 @@ impl Writeable for Event {
 				write_tlv_fields!(writer, {
 					(0, payment_id, required),
 					(2, payment_hash, required),
-					(4, path, vec_type)
+					(4, *path, vec_type)
 				})
 			},
 			&Event::ProbeFailed { ref payment_id, ref payment_hash, ref path, ref short_channel_id } => {
 				23u8.write(writer)?;
 				write_tlv_fields!(writer, {
 					(0, payment_id, required),
 					(2, payment_hash, required),
-					(4, path, vec_type),
+					(4, *path, vec_type),
 					(6, short_channel_id, option),
 				})
 			},
@@ -1007,7 +1007,7 @@ impl MaybeReadable for Event {
 			4u8 => Ok(None),
 			5u8 => {
 				let f = || {
-					let mut outputs = VecReadWrapper(Vec::new());
+					let mut outputs = WithoutLength(Vec::new());
 					read_tlv_fields!(reader, {
 						(0, outputs, required),
 					});

lightning/src/util/ser.rs

@@ -283,39 +283,6 @@ impl<T: Readable> From<T> for OptionDeserWrapper<T> {
 	fn from(t: T) -> OptionDeserWrapper<T> { OptionDeserWrapper(Some(t)) }
 }
 
-/// Wrapper to write each element of a Vec with no length prefix
-pub(crate) struct VecWriteWrapper<'a, T: Writeable>(pub &'a Vec<T>);
-impl<'a, T: Writeable> Writeable for VecWriteWrapper<'a, T> {
-	#[inline]
-	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
-		for ref v in self.0.iter() {
-			v.write(writer)?;
-		}
-		Ok(())
-	}
-}
-
-/// Wrapper to read elements from a given stream until it reaches the end of the stream.
-pub(crate) struct VecReadWrapper<T>(pub Vec<T>);
-impl<T: MaybeReadable> Readable for VecReadWrapper<T> {
-	#[inline]
-	fn read<R: Read>(mut reader: &mut R) -> Result<Self, DecodeError> {
-		let mut values = Vec::new();
-		loop {
-			let mut track_read = ReadTrackingReader::new(&mut reader);
-			match MaybeReadable::read(&mut track_read) {
-				Ok(Some(v)) => { values.push(v); },
-				Ok(None) => { },
-				// If we failed to read any bytes at all, we reached the end of our TLV
-				// stream and have simply exhausted all entries.
-				Err(ref e) if e == &DecodeError::ShortRead && !track_read.have_read => break,
-				Err(e) => return Err(e),
-			}
-		}
-		Ok(Self(values))
-	}
-}
-
 pub(crate) struct U48(pub u64);
 impl Writeable for U48 {
 	#[inline]
@@ -547,6 +514,42 @@ impl Readable for [u16; 8] {
 	}
 }
 
+/// For variable-length values within TLV record where the length is encoded as part of the record.
+/// Used to prevent encoding the length twice.
+pub(crate) struct WithoutLength<T>(pub T);
+
+impl<'a, T: Writeable> Writeable for WithoutLength<&'a Vec<T>> {
+	#[inline]
+	fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+		for ref v in self.0.iter() {
+			v.write(writer)?;
+		}
+		Ok(())
+	}
+}
+
+impl<T: MaybeReadable> Readable for WithoutLength<Vec<T>> {
+	#[inline]
+	fn read<R: Read>(mut reader: &mut R) -> Result<Self, DecodeError> {
+		let mut values = Vec::new();
+		loop {
+			let mut track_read = ReadTrackingReader::new(&mut reader);
+			match MaybeReadable::read(&mut track_read) {
+				Ok(Some(v)) => { values.push(v); },
+				Ok(None) => { },
+				// If we failed to read any bytes at all, we reached the end of our TLV
+				// stream and have simply exhausted all entries.
+				Err(ref e) if e == &DecodeError::ShortRead && !track_read.have_read => break,
+				Err(e) => return Err(e),
+			}
+		}
+		Ok(Self(values))
+	}
+}
+impl<'a, T> From<&'a Vec<T>> for WithoutLength<&'a Vec<T>> {
+	fn from(v: &'a Vec<T>) -> Self { Self(v) }
+}
+
 // HashMap
 impl<K, V> Writeable for HashMap<K, V>
 	where K: Writeable + Eq + Hash,

lightning/src/util/ser_macros.rs

@@ -17,7 +17,7 @@ macro_rules! encode_tlv {
 		$field.write($stream)?;
 	};
 	($stream: expr, $type: expr, $field: expr, vec_type) => {
-		encode_tlv!($stream, $type, $crate::util::ser::VecWriteWrapper(&$field), required);
+		encode_tlv!($stream, $type, $crate::util::ser::WithoutLength(&$field), required);
 	};
 	($stream: expr, $optional_type: expr, $optional_field: expr, option) => {
 		if let Some(ref field) = $optional_field {
@@ -66,7 +66,7 @@ macro_rules! get_varint_length_prefixed_tlv_length {
 		$len.0 += field_len;
 	};
 	($len: expr, $type: expr, $field: expr, vec_type) => {
-		get_varint_length_prefixed_tlv_length!($len, $type, $crate::util::ser::VecWriteWrapper(&$field), required);
+		get_varint_length_prefixed_tlv_length!($len, $type, $crate::util::ser::WithoutLength(&$field), required);
 	};
 	($len: expr, $optional_type: expr, $optional_field: expr, option) => {
 		if let Some(ref field) = $optional_field {
@@ -160,7 +160,7 @@ macro_rules! decode_tlv {
 		$field = $crate::util::ser::Readable::read(&mut $reader)?;
 	}};
 	($reader: expr, $field: ident, vec_type) => {{
-		let f: $crate::util::ser::VecReadWrapper<_> = $crate::util::ser::Readable::read(&mut $reader)?;
+		let f: $crate::util::ser::WithoutLength<Vec<_>> = $crate::util::ser::Readable::read(&mut $reader)?;
 		$field = Some(f.0);
 	}};
 	($reader: expr, $field: ident, option) => {{
@@ -453,7 +453,7 @@ macro_rules! _impl_writeable_tlv_based_enum_common {
 						let id: u8 = $variant_id;
 						id.write(writer)?;
 						write_tlv_fields!(writer, {
-							$(($type, $field, $fieldty)),*
+							$(($type, *$field, $fieldty)),*
 						});
 					}),*
 					$($st::$tuple_variant_name (ref field) => {