Skip to content

Commit cc78b77

Browse files
committed
Fix unknown handling in impl_writeable_tlv_based_enum_upgradable
`impl_writeable_tlv_based_enum_upgradable` professed to supporting upgrades by returning `None` from `MaybeReadable` when unknown variants written by newer versions of LDK were read. However, it generally didn't support this as it didn't discard bytes for unknown types, resulting in corrupt reading. This is fixed here for enum variants written as a TLV stream, however we don't have a length prefix for tuple enum variants, so the documentation on the macro is updated to mention that downgrades are not supported for tuple variants.
1 parent b8b1ef3 commit cc78b77

File tree

1 file changed

+12
-1
lines changed

1 file changed

+12
-1
lines changed

lightning/src/util/ser_macros.rs

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1065,6 +1065,10 @@ macro_rules! impl_writeable_tlv_based_enum {
10651065
/// when [`MaybeReadable`] is practical instead of just [`Readable`] as it provides an upgrade path for
10661066
/// new variants to be added which are simply ignored by existing clients.
10671067
///
1068+
/// Note that only struct and unit variants (not tuple variants) will support downgrading, thus any
1069+
/// new odd variants MUST be non-tuple (i.e. described using `$variant_id` and `$variant_name` not
1070+
/// `$tuple_variant_id` and `$tuple_variant_name`).
1071+
///
10681072
/// [`MaybeReadable`]: crate::util::ser::MaybeReadable
10691073
/// [`Writeable`]: crate::util::ser::Writeable
10701074
/// [`DecodeError::UnknownRequiredFeature`]: crate::ln::msgs::DecodeError::UnknownRequiredFeature
@@ -1102,7 +1106,14 @@ macro_rules! impl_writeable_tlv_based_enum_upgradable {
11021106
$($($tuple_variant_id => {
11031107
Ok(Some($st::$tuple_variant_name(Readable::read(reader)?)))
11041108
}),*)*
1105-
_ if id % 2 == 1 => Ok(None),
1109+
_ if id % 2 == 1 => {
1110+
// Assume that a $variant_id was written, not a $tuple_variant_id, and read
1111+
// the length prefix and discard the correct number of bytes.
1112+
let tlv_len: $crate::util::ser::BigSize = $crate::util::ser::Readable::read(reader)?;
1113+
let mut rd = $crate::util::ser::FixedLengthReader::new(reader, tlv_len.0);
1114+
rd.eat_remaining().map_err(|_| $crate::ln::msgs::DecodeError::ShortRead)?;
1115+
Ok(None)
1116+
},
11061117
_ => Err($crate::ln::msgs::DecodeError::UnknownRequiredFeature),
11071118
}
11081119
}

0 commit comments

Comments
 (0)