f tweaks from jeff

Author: TheBlueMatt

lightning/src/chain/channelmonitor.rs

@@ -415,7 +415,7 @@ impl MaybeReadable for OnchainEventEntry {
 		read_tlv_fields!(reader, {
 			(0, txid, required),
 			(2, height, required),
-			(4, event, maybe_readable),
+			(4, event, ignorable),
 		});
 		if let Some(ev) = event {
 			Ok(Some(Self { txid, height, event: ev }))

lightning/src/chain/onchaintx.rs

@@ -98,7 +98,7 @@ impl MaybeReadable for OnchainEventEntry {
 		read_tlv_fields!(reader, {
 			(0, txid, required),
 			(2, height, required),
-			(4, event, maybe_readable),
+			(4, event, ignorable),
 		});
 		if let Some(ev) = event {
 			Ok(Some(Self { txid, height, event: ev }))

lightning/src/util/ser_macros.rs

@@ -115,7 +115,7 @@ macro_rules! check_tlv_order {
 	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, vec_type) => {{
 		// no-op
 	}};
-	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, maybe_readable) => {{
+	($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, ignorable) => {{
 		// no-op
 	}};
 }
@@ -141,7 +141,7 @@ macro_rules! check_missing_tlv {
 	($last_seen_type: expr, $type: expr, $field: ident, option) => {{
 		// no-op
 	}};
-	($last_seen_type: expr, $type: expr, $field: ident, maybe_readable) => {{
+	($last_seen_type: expr, $type: expr, $field: ident, ignorable) => {{
 		// no-op
 	}};
 }
@@ -159,7 +159,7 @@ macro_rules! decode_tlv {
 	($reader: expr, $field: ident, option) => {{
 		$field = Some(ser::Readable::read(&mut $reader)?);
 	}};
-	($reader: expr, $field: ident, maybe_readable) => {{
+	($reader: expr, $field: ident, ignorable) => {{
 		$field = ser::MaybeReadable::read(&mut $reader)?;
 	}};
 }
@@ -496,6 +496,11 @@ macro_rules! _impl_writeable_tlv_based_enum_common {
 
 /// 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)),* $(,)*}
@@ -526,7 +531,7 @@ macro_rules! impl_writeable_tlv_based_enum_upgradable {
 						f()
 					}),*
 					_ if id % 2 == 1 => { Ok(None) }
-					_ => { Err(DecodeError::UnknownRequiredFeature)? },
+					_ => { return Err(DecodeError::UnknownRequiredFeature); },
 				}
 			}
 		}