X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Futil%2Fser_macros.rs;h=165d1f1edbacea61438ef3426828a1195336a39e;hb=5cfe19ef02cc9746ba664aeaa90921691a75dcd6;hp=45435ecdd64ac5f5f579e168b011896831550709;hpb=583e6ac6f0bf1fd311621ea7584d160bd6d7c192;p=rust-lightning

diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs
index 45435ecd..165d1f1e 100644
--- a/lightning/src/util/ser_macros.rs
+++ b/lightning/src/util/ser_macros.rs
@@ -8,83 +8,168 @@
 // licenses.
 
 macro_rules! encode_tlv {
-	($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { {
+	($stream: expr, $type: expr, $field: expr, (default_value, $default: expr)) => {
+		encode_tlv!($stream, $type, $field, required)
+	};
+	($stream: expr, $type: expr, $field: expr, required) => {
+		BigSize($type).write($stream)?;
+		BigSize($field.serialized_length() as u64).write($stream)?;
+		$field.write($stream)?;
+	};
+	($stream: expr, $type: expr, $field: expr, vec_type) => {
+		encode_tlv!($stream, $type, ::util::ser::VecWriteWrapper(&$field), required);
+	};
+	($stream: expr, $optional_type: expr, $optional_field: expr, option) => {
+		if let Some(ref field) = $optional_field {
+			BigSize($optional_type).write($stream)?;
+			BigSize(field.serialized_length() as u64).write($stream)?;
+			field.write($stream)?;
+		}
+	};
+}
+
+macro_rules! encode_tlv_stream {
+	($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { {
 		#[allow(unused_imports)]
-		use util::ser::BigSize;
-		// 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; }
-		)*
+		use {
+			ln::msgs::DecodeError,
+			util::ser,
+			util::ser::BigSize,
+		};
+
 		$(
-			if $optional_type >= max_field { max_field = $optional_type + 1; }
+			encode_tlv!($stream, $type, $field, $fieldty);
 		)*
-		#[allow(unused_variables)]
-		for i in 0..max_field {
-			$(
-				if i == $type {
-					BigSize($type).write($stream)?;
-					BigSize($field.serialized_length() as u64).write($stream)?;
-					$field.write($stream)?;
-				}
-			)*
+
+		#[allow(unused_mut, unused_variables, unused_assignments)]
+		#[cfg(debug_assertions)]
+		{
+			let mut last_seen: Option<u64> = None;
 			$(
-				if i == $optional_type {
-					if let Some(ref field) = $optional_field {
-						BigSize($optional_type).write($stream)?;
-						BigSize(field.serialized_length() as u64).write($stream)?;
-						field.write($stream)?;
-					}
+				if let Some(t) = last_seen {
+					debug_assert!(t <= $type);
 				}
+				last_seen = Some($type);
 			)*
 		}
 	} }
 }
 
 macro_rules! get_varint_length_prefixed_tlv_length {
-	({$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}) => { {
-		use util::ser::LengthCalculatingWriter;
-		#[allow(unused_mut)]
-		let mut len = LengthCalculatingWriter(0);
-		{
-			$(
-				BigSize($type).write(&mut len).expect("No in-memory data may fail to serialize");
-				let field_len = $field.serialized_length();
-				BigSize(field_len as u64).write(&mut len).expect("No in-memory data may fail to serialize");
-				len.0 += field_len;
-			)*
-			$(
-				if let Some(ref field) = $optional_field {
-					BigSize($optional_type).write(&mut len).expect("No in-memory data may fail to serialize");
-					let field_len = field.serialized_length();
-					BigSize(field_len as u64).write(&mut len).expect("No in-memory data may fail to serialize");
-					len.0 += field_len;
-				}
-			)*
+	($len: expr, $type: expr, $field: expr, (default_value, $default: expr)) => {
+		get_varint_length_prefixed_tlv_length!($len, $type, $field, required)
+	};
+	($len: expr, $type: expr, $field: expr, required) => {
+		BigSize($type).write(&mut $len).expect("No in-memory data may fail to serialize");
+		let field_len = $field.serialized_length();
+		BigSize(field_len as u64).write(&mut $len).expect("No in-memory data may fail to serialize");
+		$len.0 += field_len;
+	};
+	($len: expr, $type: expr, $field: expr, vec_type) => {
+		get_varint_length_prefixed_tlv_length!($len, $type, ::util::ser::VecWriteWrapper(&$field), required);
+	};
+	($len: expr, $optional_type: expr, $optional_field: expr, option) => {
+		if let Some(ref field) = $optional_field {
+			BigSize($optional_type).write(&mut $len).expect("No in-memory data may fail to serialize");
+			let field_len = field.serialized_length();
+			BigSize(field_len as u64).write(&mut $len).expect("No in-memory data may fail to serialize");
+			$len.0 += field_len;
 		}
-		len.0
-	} }
+	};
 }
 
 macro_rules! encode_varint_length_prefixed_tlv {
-	($stream: expr, {$(($type: expr, $field: expr)),*}, {$(($optional_type: expr, $optional_field: expr)),*}) => { {
+	($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),*}) => { {
 		use util::ser::BigSize;
-		let len = get_varint_length_prefixed_tlv_length!({ $(($type, $field)),* }, { $(($optional_type, $optional_field)),* });
+		let len = {
+			#[allow(unused_mut)]
+			let mut len = ::util::ser::LengthCalculatingWriter(0);
+			$(
+				get_varint_length_prefixed_tlv_length!(len, $type, $field, $fieldty);
+			)*
+			len.0
+		};
 		BigSize(len as u64).write($stream)?;
-		encode_tlv!($stream, { $(($type, $field)),* }, { $(($optional_type, $optional_field)),* });
+		encode_tlv_stream!($stream, { $(($type, $field, $fieldty)),* });
 	} }
 }
 
+macro_rules! check_tlv_order {
+	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{
+		#[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true
+		let invalid_order = ($last_seen_type.is_none() || $last_seen_type.unwrap() < $type) && $typ.0 > $type;
+		if invalid_order {
+			$field = $default;
+		}
+	}};
+	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, required) => {{
+		#[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true
+		let invalid_order = ($last_seen_type.is_none() || $last_seen_type.unwrap() < $type) && $typ.0 > $type;
+		if invalid_order {
+			return Err(DecodeError::InvalidValue);
+		}
+	}};
+	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, option) => {{
+		// no-op
+	}};
+	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, vec_type) => {{
+		// no-op
+	}};
+	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, ignorable) => {{
+		// no-op
+	}};
+}
+
+macro_rules! check_missing_tlv {
+	($last_seen_type: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{
+		#[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true
+		let missing_req_type = $last_seen_type.is_none() || $last_seen_type.unwrap() < $type;
+		if missing_req_type {
+			$field = $default;
+		}
+	}};
+	($last_seen_type: expr, $type: expr, $field: ident, required) => {{
+		#[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true
+		let missing_req_type = $last_seen_type.is_none() || $last_seen_type.unwrap() < $type;
+		if missing_req_type {
+			return Err(DecodeError::InvalidValue);
+		}
+	}};
+	($last_seen_type: expr, $type: expr, $field: ident, vec_type) => {{
+		// no-op
+	}};
+	($last_seen_type: expr, $type: expr, $field: ident, option) => {{
+		// no-op
+	}};
+	($last_seen_type: expr, $type: expr, $field: ident, ignorable) => {{
+		// no-op
+	}};
+}
+
 macro_rules! decode_tlv {
-	($stream: expr, {$(($reqtype: expr, $reqfield: ident)),*}, {$(($type: expr, $field: ident)),*}) => { {
+	($reader: expr, $field: ident, (default_value, $default: expr)) => {{
+		decode_tlv!($reader, $field, required)
+	}};
+	($reader: expr, $field: ident, required) => {{
+		$field = ser::Readable::read(&mut $reader)?;
+	}};
+	($reader: expr, $field: ident, vec_type) => {{
+		let f: ::util::ser::VecReadWrapper<_> = ser::Readable::read(&mut $reader)?;
+		$field = Some(f.0);
+	}};
+	($reader: expr, $field: ident, option) => {{
+		$field = Some(ser::Readable::read(&mut $reader)?);
+	}};
+	($reader: expr, $field: ident, ignorable) => {{
+		$field = ser::MaybeReadable::read(&mut $reader)?;
+	}};
+}
+
+macro_rules! decode_tlv_stream {
+	($stream: expr, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { {
 		use ln::msgs::DecodeError;
 		let mut last_seen_type: Option<u64> = None;
+		let mut stream_ref = $stream;
 		'tlv_read: loop {
 			use util::ser;
 
@@ -94,16 +179,16 @@ macro_rules! decode_tlv {
 				// determine whether we should break or return ShortRead if we get an
 				// UnexpectedEof. This should in every case be largely cosmetic, but its nice to
 				// pass the TLV test vectors exactly, which requre this distinction.
-				let mut tracking_reader = ser::ReadTrackingReader::new($stream);
+				let mut tracking_reader = ser::ReadTrackingReader::new(&mut stream_ref);
 				match ser::Readable::read(&mut tracking_reader) {
 					Err(DecodeError::ShortRead) => {
 						if !tracking_reader.have_read {
-							break 'tlv_read
+							break 'tlv_read;
 						} else {
-							Err(DecodeError::ShortRead)?
+							return Err(DecodeError::ShortRead);
 						}
 					},
-					Err(e) => Err(e)?,
+					Err(e) => return Err(e),
 					Ok(t) => t,
 				}
 			};
@@ -111,40 +196,29 @@ macro_rules! decode_tlv {
 			// Types must be unique and monotonically increasing:
 			match last_seen_type {
 				Some(t) if typ.0 <= t => {
-					Err(DecodeError::InvalidValue)?
+					return Err(DecodeError::InvalidValue);
 				},
 				_ => {},
 			}
 			// As we read types, make sure we hit every required type:
 			$({
-				#[allow(unused_comparisons)] // Note that $reqtype may be 0 making the second comparison always true
-				let invalid_order = (last_seen_type.is_none() || last_seen_type.unwrap() < $reqtype) && typ.0 > $reqtype;
-				if invalid_order {
-					Err(DecodeError::InvalidValue)?
-				}
+				check_tlv_order!(last_seen_type, typ, $type, $field, $fieldty);
 			})*
 			last_seen_type = Some(typ.0);
 
 			// Finally, read the length and value itself:
-			let length: ser::BigSize = Readable::read($stream)?;
-			let mut s = ser::FixedLengthReader::new($stream, length.0);
+			let length: ser::BigSize = ser::Readable::read(&mut stream_ref)?;
+			let mut s = ser::FixedLengthReader::new(&mut stream_ref, length.0);
 			match typ.0 {
-				$($reqtype => {
-					$reqfield = ser::Readable::read(&mut s)?;
-					if s.bytes_remain() {
-						s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes
-						Err(DecodeError::InvalidValue)?
-					}
-				},)*
 				$($type => {
-					$field = Some(ser::Readable::read(&mut s)?);
+					decode_tlv!(s, $field, $fieldty);
 					if s.bytes_remain() {
 						s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes
-						Err(DecodeError::InvalidValue)?
+						return Err(DecodeError::InvalidValue);
 					}
 				},)*
 				x if x % 2 == 0 => {
-					Err(DecodeError::UnknownRequiredFeature)?
+					return Err(DecodeError::UnknownRequiredFeature);
 				},
 				_ => {},
 			}
@@ -152,114 +226,57 @@ macro_rules! decode_tlv {
 		}
 		// Make sure we got to each required type after we've read every TLV:
 		$({
-			#[allow(unused_comparisons)] // Note that $reqtype may be 0 making the second comparison always true
-			let missing_req_type = last_seen_type.is_none() || last_seen_type.unwrap() < $reqtype;
-			if missing_req_type {
-				Err(DecodeError::InvalidValue)?
-			}
+			check_missing_tlv!(last_seen_type, $type, $field, $fieldty);
 		})*
 	} }
 }
 
-macro_rules! impl_writeable {
-	($st:ident, $len: expr, {$($field:ident),*}) => {
+macro_rules! impl_writeable_msg {
+	($st:ident, {$($field:ident),* $(,)*}, {$(($type: expr, $tlvfield: ident, $fieldty: tt)),* $(,)*}) => {
 		impl ::util::ser::Writeable for $st {
-			fn write<W: ::util::ser::Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
-				if $len != 0 {
-					w.size_hint($len);
-				}
-				#[cfg(any(test, feature = "fuzztarget"))]
-				{
-					// In tests, assert that the hard-coded length matches the actual one
-					if $len != 0 {
-						use util::ser::LengthCalculatingWriter;
-						let mut len_calc = LengthCalculatingWriter(0);
-						$( self.$field.write(&mut len_calc).expect("No in-memory data may fail to serialize"); )*
-						assert_eq!(len_calc.0, $len);
-						assert_eq!(self.serialized_length(), $len);
-					}
-				}
+			fn write<W: ::util::ser::Writer>(&self, w: &mut W) -> Result<(), $crate::io::Error> {
 				$( self.$field.write(w)?; )*
+				encode_tlv_stream!(w, {$(($type, self.$tlvfield, $fieldty)),*});
 				Ok(())
 			}
-
-			#[inline]
-			fn serialized_length(&self) -> usize {
-				if $len == 0 || cfg!(any(test, feature = "fuzztarget")) {
-					let mut len_calc = 0;
-					$( len_calc += self.$field.serialized_length(); )*
-					if $len != 0 {
-						// In tests, assert that the hard-coded length matches the actual one
-						assert_eq!(len_calc, $len);
-					} else {
-						return len_calc;
-					}
-				}
-				$len
-			}
 		}
-
 		impl ::util::ser::Readable for $st {
-			fn read<R: ::std::io::Read>(r: &mut R) -> Result<Self, ::ln::msgs::DecodeError> {
+			fn read<R: $crate::io::Read>(r: &mut R) -> Result<Self, ::ln::msgs::DecodeError> {
+				$(let $field = ::util::ser::Readable::read(r)?;)*
+				$(init_tlv_field_var!($tlvfield, $fieldty);)*
+				decode_tlv_stream!(r, {$(($type, $tlvfield, $fieldty)),*});
 				Ok(Self {
-					$($field: ::util::ser::Readable::read(r)?),*
+					$($field),*,
+					$($tlvfield),*
 				})
 			}
 		}
 	}
 }
-macro_rules! impl_writeable_len_match {
-	($struct: ident, $cmp: tt, ($calc_len: expr), {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => {
-		impl Writeable for $struct {
-			fn write<W: Writer>(&self, w: &mut W) -> Result<(), ::std::io::Error> {
-				let len = match *self {
-					$($match => $length,)*
-				};
-				w.size_hint(len);
-				#[cfg(any(test, feature = "fuzztarget"))]
-				{
-					// In tests, assert that the hard-coded length matches the actual one
-					use util::ser::LengthCalculatingWriter;
-					let mut len_calc = LengthCalculatingWriter(0);
-					$( self.$field.write(&mut len_calc).expect("No in-memory data may fail to serialize"); )*
-					assert!(len_calc.0 $cmp len);
-					assert_eq!(len_calc.0, self.serialized_length());
-				}
+
+macro_rules! impl_writeable {
+	($st:ident, {$($field:ident),*}) => {
+		impl ::util::ser::Writeable for $st {
+			fn write<W: ::util::ser::Writer>(&self, w: &mut W) -> Result<(), $crate::io::Error> {
 				$( self.$field.write(w)?; )*
 				Ok(())
 			}
 
 			#[inline]
 			fn serialized_length(&self) -> usize {
-				if $calc_len || cfg!(any(test, feature = "fuzztarget")) {
-					let mut len_calc = 0;
-					$( len_calc += self.$field.serialized_length(); )*
-					if !$calc_len {
-						assert_eq!(len_calc, match *self {
-							$($match => $length,)*
-						});
-					}
-					return len_calc
-				}
-				match *self {
-					$($match => $length,)*
-				}
+				let mut len_calc = 0;
+				$( len_calc += self.$field.serialized_length(); )*
+				return len_calc;
 			}
 		}
 
-		impl ::util::ser::Readable for $struct {
-			fn read<R: ::std::io::Read>(r: &mut R) -> Result<Self, DecodeError> {
+		impl ::util::ser::Readable for $st {
+			fn read<R: $crate::io::Read>(r: &mut R) -> Result<Self, ::ln::msgs::DecodeError> {
 				Ok(Self {
-					$($field: Readable::read(r)?),*
+					$($field: ::util::ser::Readable::read(r)?),*
 				})
 			}
 		}
-	};
-	($struct: ident, $cmp: tt, {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => {
-		impl_writeable_len_match!($struct, $cmp, (true), { $({ $match, $length }),* }, { $($field),* });
-	};
-	($struct: ident, {$({$match: pat, $length: expr}),*}, {$($field:ident),*}) => {
-		impl_writeable_len_match!($struct, ==, (false), { $({ $match, $length }),* }, { $($field),* });
 	}
 }
 
@@ -292,8 +309,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)),* $(,)*}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}) => {
-		encode_varint_length_prefixed_tlv!($stream, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*});
+	($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => {
+		encode_varint_length_prefixed_tlv!($stream, {$(($type, $field, $fieldty)),*})
 	}
 }
 
@@ -313,127 +330,220 @@ macro_rules! read_ver_prefix {
 
 /// Reads a suffix added by write_tlv_fields.
 macro_rules! read_tlv_fields {
-	($stream: expr, {$(($reqtype: expr, $reqfield: ident)),* $(,)*}, {$(($type: expr, $field: ident)),* $(,)*}) => { {
-		let tlv_len = ::util::ser::BigSize::read($stream)?;
+	($stream: expr, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { {
+		let tlv_len: ::util::ser::BigSize = ::util::ser::Readable::read($stream)?;
 		let mut rd = ::util::ser::FixedLengthReader::new($stream, tlv_len.0);
-		decode_tlv!(&mut rd, {$(($reqtype, $reqfield)),*}, {$(($type, $field)),*});
-		rd.eat_remaining().map_err(|_| DecodeError::ShortRead)?;
+		decode_tlv_stream!(&mut rd, {$(($type, $field, $fieldty)),*});
+		rd.eat_remaining().map_err(|_| ::ln::msgs::DecodeError::ShortRead)?;
 	} }
 }
 
-// If we naively create a struct in impl_writeable_tlv_based below, we may end up returning
-// `Self { ,,vecfield: vecfield }` which is obviously incorrect. Instead, we have to match here to
-// detect at least one empty field set and skip the potentially-extra comma.
-macro_rules! _init_tlv_based_struct {
-	({}, {$($field: ident),*}, {$($vecfield: ident),*}) => {
-		Ok(Self {
-			$($field),*,
-			$($vecfield: $vecfield.unwrap().0),*
-		})
+macro_rules! init_tlv_based_struct_field {
+	($field: ident, (default_value, $default: expr)) => {
+		$field
 	};
-	({$($reqfield: ident),*}, {}, {$($vecfield: ident),*}) => {
-		Ok(Self {
-			$($reqfield: $reqfield.0.unwrap()),*,
-			$($vecfield: $vecfield.unwrap().0),*
-		})
+	($field: ident, option) => {
+		$field
 	};
-	({$($reqfield: ident),*}, {$($field: ident),*}, {}) => {
-		Ok(Self {
-			$($reqfield: $reqfield.0.unwrap()),*,
-			$($field),*
-		})
+	($field: ident, required) => {
+		$field.0.unwrap()
+	};
+	($field: ident, vec_type) => {
+		$field.unwrap()
 	};
-	({$($reqfield: ident),*}, {$($field: ident),*}, {$($vecfield: ident),*}) => {
-		Ok(Self {
-			$($reqfield: $reqfield.0.unwrap()),*,
-			$($field),*,
-			$($vecfield: $vecfield.unwrap().0),*
-		})
-	}
 }
 
-// If we don't have any optional types below, but do have some vec types, we end up calling
-// `write_tlv_field!($stream, {..}, {, (vec_ty, vec_val)})`, which is obviously broken.
-// Instead, for write and read we match the missing values and skip the extra comma.
-macro_rules! _write_tlv_fields {
-	($stream: expr, {$(($type: expr, $field: expr)),* $(,)*}, {}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}) => {
-		write_tlv_fields!($stream, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*});
+macro_rules! init_tlv_field_var {
+	($field: ident, (default_value, $default: expr)) => {
+		let mut $field = $default;
 	};
-	($stream: expr, {$(($type: expr, $field: expr)),* $(,)*}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}, {$(($optional_type_2: expr, $optional_field_2: expr)),* $(,)*}) => {
-		write_tlv_fields!($stream, {$(($type, $field)),*} , {$(($optional_type, $optional_field)),*, $(($optional_type_2, $optional_field_2)),*});
-	}
-}
-macro_rules! _get_tlv_len {
-	({$(($type: expr, $field: expr)),* $(,)*}, {}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}) => {
-		get_varint_length_prefixed_tlv_length!({$(($type, $field)),*} , {$(($optional_type, $optional_field)),*})
+	($field: ident, required) => {
+		let mut $field = ::util::ser::OptionDeserWrapper(None);
 	};
-	({$(($type: expr, $field: expr)),* $(,)*}, {$(($optional_type: expr, $optional_field: expr)),* $(,)*}, {$(($optional_type_2: expr, $optional_field_2: expr)),* $(,)*}) => {
-		get_varint_length_prefixed_tlv_length!({$(($type, $field)),*} , {$(($optional_type, $optional_field)),*, $(($optional_type_2, $optional_field_2)),*})
-	}
-}
-macro_rules! _read_tlv_fields {
-	($stream: expr, {$(($reqtype: expr, $reqfield: ident)),* $(,)*}, {}, {$(($type: expr, $field: ident)),* $(,)*}) => {
-		read_tlv_fields!($stream, {$(($reqtype, $reqfield)),*}, {$(($type, $field)),*});
+	($field: ident, vec_type) => {
+		let mut $field = Some(Vec::new());
+	};
+	($field: ident, option) => {
+		let mut $field = None;
 	};
-	($stream: expr, {$(($reqtype: expr, $reqfield: ident)),* $(,)*}, {$(($type: expr, $field: ident)),* $(,)*}, {$(($type_2: expr, $field_2: ident)),* $(,)*}) => {
-		read_tlv_fields!($stream, {$(($reqtype, $reqfield)),*}, {$(($type, $field)),*, $(($type_2, $field_2)),*});
-	}
 }
 
 /// Implements Readable/Writeable for a struct storing it as a set of TLVs
-/// First block includes all the required fields including a dummy value which is used during
-/// deserialization but which will never be exposed to other code.
-/// The second block includes optional fields.
-/// The third block includes any Vecs which need to have their individual elements serialized.
+/// If $fieldty is `required`, then $field is a required field that is not an Option nor a Vec.
+/// If $fieldty is `option`, then $field is optional field.
+/// if $fieldty is `vec_type`, then $field is a Vec, which needs to have its individual elements
+/// serialized.
 macro_rules! impl_writeable_tlv_based {
-	($st: ident, {$(($reqtype: expr, $reqfield: ident)),* $(,)*}, {$(($type: expr, $field: ident)),* $(,)*}, {$(($vectype: expr, $vecfield: ident)),* $(,)*}) => {
+	($st: ident, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => {
 		impl ::util::ser::Writeable for $st {
-			fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
-				_write_tlv_fields!(writer, {
-					$(($reqtype, self.$reqfield)),*
-				}, {
-					$(($type, self.$field)),*
-				}, {
-					$(($vectype, Some(::util::ser::VecWriteWrapper(&self.$vecfield)))),*
+			fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), $crate::io::Error> {
+				write_tlv_fields!(writer, {
+					$(($type, self.$field, $fieldty)),*
 				});
 				Ok(())
 			}
 
 			#[inline]
 			fn serialized_length(&self) -> usize {
-				let len = _get_tlv_len!({
-					$(($reqtype, self.$reqfield)),*
-				}, {
-					$(($type, self.$field)),*
-				}, {
-					$(($vectype, Some(::util::ser::VecWriteWrapper(&self.$vecfield)))),*
-				});
-				use util::ser::{BigSize, LengthCalculatingWriter};
-				let mut len_calc = LengthCalculatingWriter(0);
+				use util::ser::BigSize;
+				let len = {
+					#[allow(unused_mut)]
+					let mut len = ::util::ser::LengthCalculatingWriter(0);
+					$(
+						get_varint_length_prefixed_tlv_length!(len, $type, self.$field, $fieldty);
+					)*
+					len.0
+				};
+				let mut len_calc = ::util::ser::LengthCalculatingWriter(0);
 				BigSize(len as u64).write(&mut len_calc).expect("No in-memory data may fail to serialize");
 				len + len_calc.0
 			}
 		}
 
 		impl ::util::ser::Readable for $st {
-			fn read<R: ::std::io::Read>(reader: &mut R) -> Result<Self, ::ln::msgs::DecodeError> {
-				$(
-					let mut $reqfield = ::util::ser::OptionDeserWrapper(None);
-				)*
+			fn read<R: $crate::io::Read>(reader: &mut R) -> Result<Self, ::ln::msgs::DecodeError> {
 				$(
-					let mut $field = None;
+					init_tlv_field_var!($field, $fieldty);
 				)*
-				$(
-					let mut $vecfield = Some(::util::ser::VecReadWrapper(Vec::new()));
-				)*
-				_read_tlv_fields!(reader, {
-					$(($reqtype, $reqfield)),*
-				}, {
-					$(($type, $field)),*
-				}, {
-					$(($vectype, $vecfield)),*
+				read_tlv_fields!(reader, {
+					$(($type, $field, $fieldty)),*
 				});
-				_init_tlv_based_struct!({$($reqfield),*}, {$($field),*}, {$($vecfield),*})
+				Ok(Self {
+					$(
+						$field: init_tlv_based_struct_field!($field, $fieldty)
+					),*
+				})
+			}
+		}
+	}
+}
+
+macro_rules! _impl_writeable_tlv_based_enum_common {
+	($st: ident, $(($variant_id: expr, $variant_name: ident) =>
+		{$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
+	),* $(,)*;
+	$(($tuple_variant_id: expr, $tuple_variant_name: ident)),*  $(,)*) => {
+		impl ::util::ser::Writeable for $st {
+			fn write<W: ::util::ser::Writer>(&self, writer: &mut W) -> Result<(), $crate::io::Error> {
+				match self {
+					$($st::$variant_name { $(ref $field),* } => {
+						let id: u8 = $variant_id;
+						id.write(writer)?;
+						write_tlv_fields!(writer, {
+							$(($type, $field, $fieldty)),*
+						});
+					}),*
+					$($st::$tuple_variant_name (ref field) => {
+						let id: u8 = $tuple_variant_id;
+						id.write(writer)?;
+						field.write(writer)?;
+					}),*
+				}
+				Ok(())
+			}
+		}
+	}
+}
+
+/// Implement MaybeReadable and Writeable for an enum, with struct variants stored as TLVs and
+/// tuple variants stored directly.
+///
+/// This is largely identical to `impl_writeable_tlv_based_enum`, except that odd variants will
+/// return `Ok(None)` instead of `Err(UnknownRequiredFeature)`. It should generally be preferred
+/// when `MaybeReadable` is practical instead of just `Readable` as it provides an upgrade path for
+/// new variants to be added which are simply ignored by existing clients.
+macro_rules! impl_writeable_tlv_based_enum_upgradable {
+	($st: ident, $(($variant_id: expr, $variant_name: ident) =>
+		{$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
+	),* $(,)*
+	$(;
+	$(($tuple_variant_id: expr, $tuple_variant_name: ident)),*  $(,)*)*) => {
+		_impl_writeable_tlv_based_enum_common!($st,
+			$(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*;
+			$($(($tuple_variant_id, $tuple_variant_name)),*)*);
+
+		impl ::util::ser::MaybeReadable for $st {
+			fn read<R: $crate::io::Read>(reader: &mut R) -> Result<Option<Self>, ::ln::msgs::DecodeError> {
+				let id: u8 = ::util::ser::Readable::read(reader)?;
+				match id {
+					$($variant_id => {
+						// Because read_tlv_fields creates a labeled loop, we cannot call it twice
+						// in the same function body. Instead, we define a closure and call it.
+						let f = || {
+							$(
+								init_tlv_field_var!($field, $fieldty);
+							)*
+							read_tlv_fields!(reader, {
+								$(($type, $field, $fieldty)),*
+							});
+							Ok(Some($st::$variant_name {
+								$(
+									$field: init_tlv_based_struct_field!($field, $fieldty)
+								),*
+							}))
+						};
+						f()
+					}),*
+					$($($tuple_variant_id => {
+						Ok(Some($st::$tuple_variant_name(Readable::read(reader)?)))
+					}),*)*
+					_ if id % 2 == 1 => Ok(None),
+					_ => Err(DecodeError::UnknownRequiredFeature),
+				}
+			}
+		}
+
+	}
+}
+
+/// Implement Readable and Writeable for an enum, with struct variants stored as TLVs and tuple
+/// variants stored directly.
+/// The format is, for example
+/// impl_writeable_tlv_based_enum!(EnumName,
+///   (0, StructVariantA) => {(0, required_variant_field, required), (1, optional_variant_field, option)},
+///   (1, StructVariantB) => {(0, variant_field_a, required), (1, variant_field_b, required), (2, variant_vec_field, vec_type)};
+///   (2, TupleVariantA), (3, TupleVariantB),
+/// );
+/// The type is written as a single byte, followed by any variant data.
+/// Attempts to read an unknown type byte result in DecodeError::UnknownRequiredFeature.
+macro_rules! impl_writeable_tlv_based_enum {
+	($st: ident, $(($variant_id: expr, $variant_name: ident) =>
+		{$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
+	),* $(,)*;
+	$(($tuple_variant_id: expr, $tuple_variant_name: ident)),*  $(,)*) => {
+		_impl_writeable_tlv_based_enum_common!($st,
+			$(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*;
+			$(($tuple_variant_id, $tuple_variant_name)),*);
+
+		impl ::util::ser::Readable for $st {
+			fn read<R: $crate::io::Read>(reader: &mut R) -> Result<Self, ::ln::msgs::DecodeError> {
+				let id: u8 = ::util::ser::Readable::read(reader)?;
+				match id {
+					$($variant_id => {
+						// Because read_tlv_fields creates a labeled loop, we cannot call it twice
+						// in the same function body. Instead, we define a closure and call it.
+						let f = || {
+							$(
+								init_tlv_field_var!($field, $fieldty);
+							)*
+							read_tlv_fields!(reader, {
+								$(($type, $field, $fieldty)),*
+							});
+							Ok($st::$variant_name {
+								$(
+									$field: init_tlv_based_struct_field!($field, $fieldty)
+								),*
+							})
+						};
+						f()
+					}),*
+					$($tuple_variant_id => {
+						Ok($st::$tuple_variant_name(Readable::read(reader)?))
+					}),*
+					_ => {
+						Err(DecodeError::UnknownRequiredFeature)
+					},
+				}
 			}
 		}
 	}
@@ -441,10 +551,10 @@ macro_rules! impl_writeable_tlv_based {
 
 #[cfg(test)]
 mod tests {
+	use io::{self, Cursor};
 	use prelude::*;
-	use std::io::{Cursor, Read};
 	use ln::msgs::DecodeError;
-	use util::ser::{Readable, Writeable, HighZeroBytesDroppedVarInt, VecWriter};
+	use util::ser::{Writeable, HighZeroBytesDroppedVarInt, VecWriter};
 	use bitcoin::secp256k1::PublicKey;
 
 	// The BOLT TLV test cases don't include any tests which use our "required-value" logic since
@@ -455,7 +565,7 @@ mod tests {
 		let mut a: u64 = 0;
 		let mut b: u32 = 0;
 		let mut c: Option<u32> = None;
-		decode_tlv!(&mut s, {(2, a), (3, b)}, {(4, c)});
+		decode_tlv_stream!(&mut s, {(2, a, required), (3, b, required), (4, c, option)});
 		Ok((a, b, c))
 	}
 
@@ -512,13 +622,6 @@ mod tests {
 			(0xdeadbeef1badbeef, 0x1bad1dea, Some(0x01020304)));
 	}
 
-	impl Readable for (PublicKey, u64, u64) {
-		#[inline]
-		fn read<R: Read>(reader: &mut R) -> Result<(PublicKey, u64, u64), DecodeError> {
-			Ok((Readable::read(reader)?, Readable::read(reader)?, Readable::read(reader)?))
-		}
-	}
-
 	// BOLT TLV test cases
 	fn tlv_reader_n1(s: &[u8]) -> Result<(Option<HighZeroBytesDroppedVarInt<u64>>, Option<u64>, Option<(PublicKey, u64, u64)>, Option<u16>), DecodeError> {
 		let mut s = Cursor::new(s);
@@ -526,7 +629,7 @@ mod tests {
 		let mut tlv2: Option<u64> = None;
 		let mut tlv3: Option<(PublicKey, u64, u64)> = None;
 		let mut tlv4: Option<u16> = None;
-		decode_tlv!(&mut s, {}, {(1, tlv1), (2, tlv2), (3, tlv3), (254, tlv4)});
+		decode_tlv_stream!(&mut s, {(1, tlv1, option), (2, tlv2, option), (3, tlv3, option), (254, tlv4, option)});
 		Ok((tlv1, tlv2, tlv3, tlv4))
 	}
 
@@ -633,31 +736,31 @@ mod tests {
 		do_test!(concat!("fd00fe", "02", "0226"), None, None, None, Some(550));
 	}
 
-	fn do_simple_test_tlv_write() -> Result<(), ::std::io::Error> {
+	fn do_simple_test_tlv_write() -> Result<(), io::Error> {
 		let mut stream = VecWriter(Vec::new());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { (1, 1u8) }, { (42, None::<u64>) });
+		encode_varint_length_prefixed_tlv!(&mut stream, {(1, 1u8, required), (42, None::<u64>, option)});
 		assert_eq!(stream.0, ::hex::decode("03010101").unwrap());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { }, { (1, Some(1u8)) });
+		encode_varint_length_prefixed_tlv!(&mut stream, {(1, Some(1u8), option)});
 		assert_eq!(stream.0, ::hex::decode("03010101").unwrap());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { (4, 0xabcdu16) }, { (42, None::<u64>) });
+		encode_varint_length_prefixed_tlv!(&mut stream, {(4, 0xabcdu16, required), (42, None::<u64>, option)});
 		assert_eq!(stream.0, ::hex::decode("040402abcd").unwrap());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { (0xff, 0xabcdu16) }, { (42, None::<u64>) });
+		encode_varint_length_prefixed_tlv!(&mut stream, {(42, None::<u64>, option), (0xff, 0xabcdu16, required)});
 		assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { (0, 1u64), (0xff, HighZeroBytesDroppedVarInt(0u64)) }, { (42, None::<u64>) });
+		encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::<u64>, option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 		assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());
 
 		stream.0.clear();
-		encode_varint_length_prefixed_tlv!(&mut stream, { (0xff, HighZeroBytesDroppedVarInt(0u64)) }, { (0, Some(1u64)) });
+		encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedVarInt(0u64), required)});
 		assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap());
 
 		Ok(())