ser_macros: encoding wrapper for required_vec

For context, many of our higher level ser macros such as impl_writeable_msg
support the syntax:

impl_writeable_msg!(StructName, { .. }, {
   (42, tlv_field, (option, encoding: (Vec<u8>, WithoutLength>),
})

i.e. specifying an encoding wrapper around an optional field being serialized.

Here we add add similar support for an encoding wrapper around required_vecs
that are being serialized, eg (42, tlv_field, (required_vec: (encoding: (..)))).

One difficulty here is that some ser macros such as impl_writeable_msg and tlv_stream
will call the underlying macro _encode_tlv_stream with each struct field passed in as
a reference, whereas others such as impl_writeable_tlv_based will call _encode_tlv_stream
with each struct field passed in as a value.

This difference led to compilation issues because the WithoutLength ser wrapper
expects to wrap a reference to a Vec, not a value.

As a result, here we also modify the macros that were previously passing in
struct values as values to pass in references instead.

Author: valentinewallace

lightning/src/ln/msgs.rs

@@ -3144,7 +3144,7 @@ impl<'a> Writeable for OutboundOnionPayload<'a> {
 			},
 			Self::BlindedForward { encrypted_tlvs, intro_node_blinding_point } => {
 				_encode_varint_length_prefixed_tlv!(w, {
-					(10, **encrypted_tlvs, required_vec),
+					(10, *encrypted_tlvs, required_vec),
 					(12, intro_node_blinding_point, option)
 				});
 			},
@@ -3165,7 +3165,7 @@ impl<'a> Writeable for OutboundOnionPayload<'a> {
 				_encode_varint_length_prefixed_tlv!(w, {
 					(2, HighZeroBytesDroppedBigSize(*sender_intended_htlc_amt_msat), required),
 					(4, HighZeroBytesDroppedBigSize(*cltv_expiry_height), required),
-					(10, **encrypted_tlvs, required_vec),
+					(10, *encrypted_tlvs, required_vec),
 					(12, intro_node_blinding_point, option),
 					(18, HighZeroBytesDroppedBigSize(*total_msat), required)
 				}, custom_tlvs.iter());
@@ -3206,15 +3206,15 @@ impl<'a> Writeable for OutboundTrampolinePayload<'a> {
 			},
 			Self::BlindedForward { encrypted_tlvs, intro_node_blinding_point} => {
 				_encode_varint_length_prefixed_tlv!(w, {
-					(10, **encrypted_tlvs, required_vec),
+					(10, *encrypted_tlvs, required_vec),
 					(12, intro_node_blinding_point, option)
 				});
 			},
 			Self::BlindedReceive { sender_intended_htlc_amt_msat, total_msat, cltv_expiry_height, encrypted_tlvs, intro_node_blinding_point, keysend_preimage, custom_tlvs } => {
 				_encode_varint_length_prefixed_tlv!(w, {
 					(2, HighZeroBytesDroppedBigSize(*sender_intended_htlc_amt_msat), required),
 					(4, HighZeroBytesDroppedBigSize(*cltv_expiry_height), required),
-					(10, **encrypted_tlvs, required_vec),
+					(10, *encrypted_tlvs, required_vec),
 					(12, intro_node_blinding_point, option),
 					(18, HighZeroBytesDroppedBigSize(*total_msat), required),
 					(20, keysend_preimage, option)
@@ -5592,7 +5592,7 @@ mod tests {
 		let test_bytes = vec![42u8; 1000];
 		if let msgs::OutboundOnionPayload::Forward { short_channel_id, amt_to_forward, outgoing_cltv_value } = payload {
 			_encode_varint_length_prefixed_tlv!(&mut encoded_payload, {
-				(1, test_bytes, required_vec),
+				(1, &test_bytes, required_vec),
 				(2, HighZeroBytesDroppedBigSize(amt_to_forward), required),
 				(4, HighZeroBytesDroppedBigSize(outgoing_cltv_value), required),
 				(6, short_channel_id, required)

lightning/src/onion_message/packet.rs

@@ -217,7 +217,7 @@ impl<T: OnionMessageContents> Writeable for (Payload<T>, [u8; 32]) {
 	fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
 		match &self.0 {
 			Payload::Forward(ForwardControlTlvs::Blinded(encrypted_bytes)) => {
-				_encode_varint_length_prefixed_tlv!(w, { (4, *encrypted_bytes, required_vec) })
+				_encode_varint_length_prefixed_tlv!(w, { (4, encrypted_bytes, required_vec) })
 			},
 			Payload::Receive {
 				control_tlvs: ReceiveControlTlvs::Blinded(encrypted_bytes),
@@ -226,7 +226,7 @@ impl<T: OnionMessageContents> Writeable for (Payload<T>, [u8; 32]) {
 			} => {
 				_encode_varint_length_prefixed_tlv!(w, {
 					(2, reply_path, option),
-					(4, *encrypted_bytes, required_vec),
+					(4, encrypted_bytes, required_vec),
 					(message.tlv_type(), message, required)
 				})
 			},

lightning/src/util/ser_macros.rs

@@ -33,7 +33,10 @@ macro_rules! _encode_tlv {
 		$crate::_encode_tlv!($stream, $type, $field, required);
 	};
 	($stream: expr, $type: expr, $field: expr, required_vec $(, $self: ident)?) => {
-		$crate::_encode_tlv!($stream, $type, $crate::util::ser::WithoutLength(&$field), required);
+		$crate::_encode_tlv!($stream, $type, $crate::util::ser::WithoutLength($field), required);
+	};
+	($stream: expr, $type: expr, $field: expr, (required_vec, encoding: ($fieldty: ty, $encoding: ident)) $(, $self: ident)?) => {
+		$crate::_encode_tlv!($stream, $type, $encoding($field), required);
 	};
 	($stream: expr, $optional_type: expr, $optional_field: expr, option $(, $self: ident)?) => {
 		if let Some(ref field) = $optional_field {
@@ -166,7 +169,7 @@ macro_rules! _encode_tlv_stream {
 		)*
 		for tlv in $extra_tlvs {
 			let (typ, value): &(u64, Vec<u8>) = tlv;
-			$crate::_encode_tlv!($stream, *typ, *value, required_vec);
+			$crate::_encode_tlv!($stream, *typ, value, required_vec);
 		}
 
 		#[allow(unused_mut, unused_variables, unused_assignments)]
@@ -207,11 +210,15 @@ macro_rules! _get_varint_length_prefixed_tlv_length {
 		$crate::_get_varint_length_prefixed_tlv_length!($len, $type, $field, required);
 	};
 	($len: expr, $type: expr, $field: expr, required_vec $(, $self: ident)?) => {
-		let field = $crate::util::ser::WithoutLength(&$field);
+		let field = $crate::util::ser::WithoutLength($field);
+		$crate::_get_varint_length_prefixed_tlv_length!($len, $type, field, required);
+	};
+	($len: expr, $type: expr, $field: expr, (required_vec, encoding: ($fieldty: ty, $encoding: ident)) $(, $self: ident)?) => {
+		let field = $encoding($field);
 		$crate::_get_varint_length_prefixed_tlv_length!($len, $type, field, required);
 	};
 	($len: expr, $optional_type: expr, $optional_field: expr, option $(, $self: ident)?) => {
-		if let Some(ref field) = $optional_field {
+		if let Some(ref field) = $optional_field.as_ref() {
 			BigSize($optional_type)
 				.write(&mut $len)
 				.expect("No in-memory data may fail to serialize");
@@ -265,7 +272,7 @@ macro_rules! _encode_varint_length_prefixed_tlv {
 			)*
 			for tlv in $extra_tlvs {
 				let (typ, value): &(u64, Vec<u8>) = tlv;
-				$crate::_get_varint_length_prefixed_tlv_length!(len, *typ, *value, required_vec);
+				$crate::_get_varint_length_prefixed_tlv_length!(len, *typ, value, required_vec);
 			}
 			len.0
 		};
@@ -316,6 +323,9 @@ macro_rules! _check_decoded_tlv_order {
 	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, required_vec) => {{
 		$crate::_check_decoded_tlv_order!($last_seen_type, $typ, $type, $field, required);
 	}};
+	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (required_vec, encoding: $encoding: tt)) => {{
+		// no-op
+	}};
 	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, optional_vec) => {{
 		// no-op
 	}};
@@ -363,6 +373,10 @@ macro_rules! _check_missing_tlv {
 	($last_seen_type: expr, $type: expr, $field: ident, required_vec) => {{
 		$crate::_check_missing_tlv!($last_seen_type, $type, $field, required);
 	}};
+	($last_seen_type: expr, $type: expr, $field: ident, (required_vec, encoding: $encoding: tt)) => {{
+		// TODO: added this
+		$crate::_check_missing_tlv!($last_seen_type, $type, $field, required);
+	}};
 	($last_seen_type: expr, $type: expr, $field: ident, option) => {{
 		// no-op
 	}};
@@ -412,6 +426,12 @@ macro_rules! _decode_tlv {
 		let f: $crate::util::ser::WithoutLength<Vec<_>> = $crate::util::ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?;
 		$field = f.0;
 	}};
+	($outer_reader: expr, $reader: expr, $field: ident, (required_vec, encoding: ($fieldty: ty, $encoding: ident))) => {{
+		$field = {
+			let field: $encoding<$fieldty> = ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?;
+			$crate::util::ser::RequiredWrapper(Some(field.0))
+		};
+	}};
 	($outer_reader: expr, $reader: expr, $field: ident, option) => {{
 		$field = Some($crate::util::ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?);
 	}};
@@ -762,7 +782,7 @@ macro_rules! write_ver_prefix {
 #[macro_export]
 macro_rules! write_tlv_fields {
 	($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => {
-		$crate::_encode_varint_length_prefixed_tlv!($stream, {$(($type, $field, $fieldty)),*})
+		$crate::_encode_varint_length_prefixed_tlv!($stream, {$(($type, &$field, $fieldty)),*})
 	}
 }
 
@@ -830,6 +850,9 @@ macro_rules! _init_tlv_based_struct_field {
 	($field: ident, required_vec) => {
 		$field
 	};
+	($field: ident, (required_vec, encoding: ($fieldty: ty, $encoding: ident))) => {
+		$crate::_init_tlv_based_struct_field!($field, required)
+	};
 	($field: ident, optional_vec) => {
 		$field.unwrap()
 	};
@@ -856,6 +879,9 @@ macro_rules! _init_tlv_field_var {
 	($field: ident, required_vec) => {
 		let mut $field = Vec::new();
 	};
+	($field: ident, (required_vec, encoding: ($fieldty: ty, $encoding: ident))) => {
+		$crate::_init_tlv_field_var!($field, required);
+	};
 	($field: ident, option) => {
 		let mut $field = None;
 	};
@@ -983,7 +1009,7 @@ macro_rules! impl_writeable_tlv_based {
 		impl $crate::util::ser::Writeable for $st {
 			fn write<W: $crate::util::ser::Writer>(&self, writer: &mut W) -> Result<(), $crate::io::Error> {
 				$crate::_encode_varint_length_prefixed_tlv!(writer, {
-					$(($type, self.$field, $fieldty, self)),*
+					$(($type, &self.$field, $fieldty, self)),*
 				});
 				Ok(())
 			}
@@ -995,7 +1021,7 @@ macro_rules! impl_writeable_tlv_based {
 					#[allow(unused_mut)]
 					let mut len = $crate::util::ser::LengthCalculatingWriter(0);
 					$(
-						$crate::_get_varint_length_prefixed_tlv_length!(len, $type, self.$field, $fieldty, self);
+						$crate::_get_varint_length_prefixed_tlv_length!(len, $type, &self.$field, $fieldty, self);
 					)*
 					len.0
 				};
@@ -1112,7 +1138,7 @@ macro_rules! _impl_writeable_tlv_based_enum_common {
 						let id: u8 = $variant_id;
 						id.write(writer)?;
 						$crate::_encode_varint_length_prefixed_tlv!(writer, {
-							$(($type, *$field, $fieldty, self)),*
+							$(($type, $field, $fieldty, self)),*
 						});
 					}),*
 					$($st::$tuple_variant_name (ref field) => {
@@ -1370,7 +1396,8 @@ mod tests {
 	use crate::io::{self, Cursor};
 	use crate::ln::msgs::DecodeError;
 	use crate::util::ser::{
-		HighZeroBytesDroppedBigSize, LengthReadable, MaybeReadable, Readable, VecWriter, Writeable,
+		FixedLengthReader, HighZeroBytesDroppedBigSize, LengthReadable, MaybeReadable, Readable,
+		VecWriter, WithoutLength, Writeable,
 	};
 	use bitcoin::hex::FromHex;
 	use bitcoin::secp256k1::PublicKey;
@@ -1879,4 +1906,25 @@ mod tests {
 		let read = <ExpandedField as Readable>::read(&mut &encoded[..]).unwrap();
 		assert_eq!(read, ExpandedField { new_field: (42, 0) });
 	}
+
+	#[test]
+	fn required_vec_with_encoding() {
+		// Ensure that serializing a required vec with a specified encoding will survive a ser round
+		// trip.
+		#[derive(PartialEq, Eq, Debug)]
+		struct MyCustomStruct {
+			tlv_field: Vec<u8>,
+		}
+		impl_writeable_tlv_based!(MyCustomStruct, {
+			(0, tlv_field, (required_vec, encoding: (Vec<u8>, WithoutLength))),
+		});
+
+		let instance = MyCustomStruct { tlv_field: vec![42; 32] };
+		let encoded = instance.encode();
+		let mut buffer = &encoded[..];
+		let mut reader = FixedLengthReader::new(&mut buffer, encoded.len() as u64);
+		let decoded: MyCustomStruct =
+			LengthReadable::read_from_fixed_length_buffer(&mut reader).unwrap();
+		assert_eq!(decoded, instance);
+	}
 }