From 72f883e0a1a5d2286aa996f6e7bb80a54ca916c0 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 8 Jul 2024 19:21:19 +0000 Subject: [PATCH 1/2] Make `impl_writeable_tlv_based_enum*` actually upgradable In cc78b77c715d6ef62693d4c1bc7190da990ec0fa it was discovered that `impl_writeable_tlv_based_enum_upgradable` wasn't actually upgradable - tuple variants weren't written with length-prefixes, causing downgrades with new tuple variants to be unreadable by older clients as they wouldn't know where to stop reading. This was fixed by simply assuming that any new variants will be non-tuple variants with a length prefix, but no code write-side changes were made, allowing new code to freely continue to use the broken tuple-variant serialization. Here we address this be defining yet more serialization macros which aren't broken, and convert existing usage of the existing macros using non-length-prefixed tuple variants to renamed `*_legacy` macros. Note that this changes the serialization format of `impl_writeable_tlv_based_enum[_upgradable]` when tuple fields are written, and as such deliberately changes the call semantics for such tuples. Only the serialization format of `MessageContext` is changed here which is fine as it has not yet reached a release of LDK. --- lightning/src/blinded_path/message.rs | 8 +- lightning/src/blinded_path/payment.rs | 2 +- lightning/src/chain/channelmonitor.rs | 2 +- lightning/src/chain/package.rs | 2 +- lightning/src/events/mod.rs | 6 +- lightning/src/ln/channel.rs | 4 +- lightning/src/ln/channel_state.rs | 6 +- lightning/src/ln/channelmanager.rs | 10 +- lightning/src/ln/onion_utils.rs | 2 +- lightning/src/ln/outbound_payment.rs | 6 +- lightning/src/ln/script.rs | 2 +- lightning/src/sign/mod.rs | 2 +- lightning/src/util/config.rs | 2 +- lightning/src/util/ser_macros.rs | 218 +++++++++++++++++++++++--- lightning/src/util/sweep.rs | 2 +- 15 files changed, 222 insertions(+), 52 deletions(-) diff --git a/lightning/src/blinded_path/message.rs b/lightning/src/blinded_path/message.rs index 06d535a3527..fe1a8b76681 100644 --- a/lightning/src/blinded_path/message.rs +++ b/lightning/src/blinded_path/message.rs @@ -119,9 +119,9 @@ pub enum OffersContext { }, } -impl_writeable_tlv_based_enum!(MessageContext, ; - (0, Offers), - (1, Custom), +impl_writeable_tlv_based_enum!(MessageContext, + {0, Offers} => (), + {1, Custom} => (), ); impl_writeable_tlv_based_enum!(OffersContext, @@ -129,7 +129,7 @@ impl_writeable_tlv_based_enum!(OffersContext, (1, OutboundPayment) => { (0, payment_id, required), }, -;); +); /// Construct blinded onion message hops for the given `intermediate_nodes` and `recipient_node_id`. pub(super) fn blinded_hops( diff --git a/lightning/src/blinded_path/payment.rs b/lightning/src/blinded_path/payment.rs index 7df7d1c63ed..8c892e896c8 100644 --- a/lightning/src/blinded_path/payment.rs +++ b/lightning/src/blinded_path/payment.rs @@ -431,7 +431,7 @@ impl Readable for PaymentConstraints { } } -impl_writeable_tlv_based_enum!(PaymentContext, +impl_writeable_tlv_based_enum_legacy!(PaymentContext, ; (0, Unknown), (1, Bolt12Offer), diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 0e87f3569e6..13f2ff044a2 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -189,7 +189,7 @@ pub enum MonitorEvent { monitor_update_id: u64, }, } -impl_writeable_tlv_based_enum_upgradable!(MonitorEvent, +impl_writeable_tlv_based_enum_upgradable_legacy!(MonitorEvent, // Note that Completed is currently never serialized to disk as it is generated only in // ChainMonitor. (0, Completed) => { diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index 906e730dbf2..9b40f4b6f56 100644 --- a/lightning/src/chain/package.rs +++ b/lightning/src/chain/package.rs @@ -689,7 +689,7 @@ impl PackageSolvingData { } } -impl_writeable_tlv_based_enum!(PackageSolvingData, ; +impl_writeable_tlv_based_enum_legacy!(PackageSolvingData, ; (0, RevokedOutput), (1, RevokedHTLCOutput), (2, CounterpartyOfferedHTLCOutput), diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index e59a3d18501..acee931138f 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -171,7 +171,7 @@ impl PaymentPurpose { } } -impl_writeable_tlv_based_enum!(PaymentPurpose, +impl_writeable_tlv_based_enum_legacy!(PaymentPurpose, (0, Bolt11InvoicePayment) => { (0, payment_preimage, option), (2, payment_secret, required), @@ -494,7 +494,7 @@ enum InterceptNextHop { impl_writeable_tlv_based_enum!(InterceptNextHop, (0, FakeScid) => { (0, requested_next_hop_scid, required), - }; + }, ); /// The reason the payment failed. Used in [`Event::PaymentFailed`]. @@ -535,7 +535,7 @@ impl_writeable_tlv_based_enum!(PaymentFailureReason, (4, RetriesExhausted) => {}, (6, PaymentExpired) => {}, (8, RouteNotFound) => {}, - (10, UnexpectedError) => {}, ; + (10, UnexpectedError) => {}, ); /// An Event which you should probably take some action in response to. diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index fc75b2ff5c6..a4b496bb7c8 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -130,7 +130,7 @@ impl_writeable_tlv_based_enum!(InboundHTLCResolution, }, (2, Pending) => { (0, update_add_htlc, required), - }; + }, ); enum InboundHTLCState { @@ -8390,7 +8390,7 @@ fn get_initial_channel_type(config: &UserConfig, their_features: &InitFeatures) const SERIALIZATION_VERSION: u8 = 4; const MIN_SERIALIZATION_VERSION: u8 = 3; -impl_writeable_tlv_based_enum!(InboundHTLCRemovalReason,; +impl_writeable_tlv_based_enum_legacy!(InboundHTLCRemovalReason,; (0, FailRelay), (1, FailMalformed), (2, Fulfill), diff --git a/lightning/src/ln/channel_state.rs b/lightning/src/ln/channel_state.rs index 5e71b8fe701..991ba077225 100644 --- a/lightning/src/ln/channel_state.rs +++ b/lightning/src/ln/channel_state.rs @@ -69,7 +69,7 @@ impl_writeable_tlv_based_enum_upgradable!(InboundHTLCStateDetails, (0, AwaitingRemoteRevokeToAdd) => {}, (2, Committed) => {}, (4, AwaitingRemoteRevokeToRemoveFulfill) => {}, - (6, AwaitingRemoteRevokeToRemoveFail) => {}; + (6, AwaitingRemoteRevokeToRemoveFail) => {}, ); /// Exposes details around pending inbound HTLCs. @@ -159,7 +159,7 @@ impl_writeable_tlv_based_enum_upgradable!(OutboundHTLCStateDetails, (0, AwaitingRemoteRevokeToAdd) => {}, (2, Committed) => {}, (4, AwaitingRemoteRevokeToRemoveSuccess) => {}, - (6, AwaitingRemoteRevokeToRemoveFailure) => {}; + (6, AwaitingRemoteRevokeToRemoveFailure) => {}, ); /// Exposes details around pending outbound HTLCs. @@ -701,5 +701,5 @@ impl_writeable_tlv_based_enum!(ChannelShutdownState, (2, ShutdownInitiated) => {}, (4, ResolvingHTLCs) => {}, (6, NegotiatingClosingFee) => {}, - (8, ShutdownComplete) => {}, ; + (8, ShutdownComplete) => {}, ); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 89f967c4f70..edd5c3b0d3c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -476,7 +476,7 @@ impl_writeable_tlv_based_enum!(SentHTLCId, }, (2, OutboundRoute) => { (0, session_priv, required), - }; + }, ); @@ -881,7 +881,7 @@ impl_writeable_tlv_based_enum!(EventCompletionAction, // Note that by the time we get past the required read above, channel_funding_outpoint will be // filled in, so we can safely unwrap it here. (3, channel_id, (default_value, ChannelId::v1_from_funding_outpoint(channel_funding_outpoint.0.unwrap()))), - }; + } ); #[derive(Debug)] @@ -10808,7 +10808,7 @@ impl_writeable_tlv_based_enum!(PendingHTLCRouting, (4, payment_data, option), // Added in 0.0.116 (5, custom_tlvs, optional_vec), }, -;); +); impl_writeable_tlv_based!(PendingHTLCInfo, { (0, routing, required), @@ -10888,14 +10888,14 @@ impl Readable for HTLCFailureMsg { } } -impl_writeable_tlv_based_enum!(PendingHTLCStatus, ; +impl_writeable_tlv_based_enum_legacy!(PendingHTLCStatus, ; (0, Forward), (1, Fail), ); impl_writeable_tlv_based_enum!(BlindedFailure, (0, FromIntroductionNode) => {}, - (2, FromBlindedNode) => {}, ; + (2, FromBlindedNode) => {}, ); impl_writeable_tlv_based!(HTLCPreviousHopData, { diff --git a/lightning/src/ln/onion_utils.rs b/lightning/src/ln/onion_utils.rs index fab73cf73ec..bab81e2de5c 100644 --- a/lightning/src/ln/onion_utils.rs +++ b/lightning/src/ln/onion_utils.rs @@ -953,7 +953,7 @@ impl_writeable_tlv_based_enum!(HTLCFailReasonRepr, (0, failure_code, required), (2, data, required_vec), }, -;); +); impl HTLCFailReason { #[rustfmt::skip] diff --git a/lightning/src/ln/outbound_payment.rs b/lightning/src/ln/outbound_payment.rs index 443a7b2c3a2..3c427da1eb0 100644 --- a/lightning/src/ln/outbound_payment.rs +++ b/lightning/src/ln/outbound_payment.rs @@ -300,13 +300,13 @@ pub enum Retry { } #[cfg(not(feature = "std"))] -impl_writeable_tlv_based_enum!(Retry, +impl_writeable_tlv_based_enum_legacy!(Retry, ; (0, Attempts) ); #[cfg(feature = "std")] -impl_writeable_tlv_based_enum!(Retry, +impl_writeable_tlv_based_enum_legacy!(Retry, ; (0, Attempts), (2, Timeout) @@ -397,7 +397,7 @@ pub(crate) enum StaleExpiration { AbsoluteTimeout(core::time::Duration), } -impl_writeable_tlv_based_enum!(StaleExpiration, +impl_writeable_tlv_based_enum_legacy!(StaleExpiration, ; (0, TimerTicks), (2, AbsoluteTimeout) diff --git a/lightning/src/ln/script.rs b/lightning/src/ln/script.rs index ef75bb581b4..190e243a7a5 100644 --- a/lightning/src/ln/script.rs +++ b/lightning/src/ln/script.rs @@ -53,7 +53,7 @@ impl Readable for ShutdownScript { } } -impl_writeable_tlv_based_enum!(ShutdownScriptImpl, ; +impl_writeable_tlv_based_enum_legacy!(ShutdownScriptImpl, ; (0, Legacy), (1, Bolt2), ); diff --git a/lightning/src/sign/mod.rs b/lightning/src/sign/mod.rs index 885b8840b76..2f9c4b49c57 100644 --- a/lightning/src/sign/mod.rs +++ b/lightning/src/sign/mod.rs @@ -301,7 +301,7 @@ pub enum SpendableOutputDescriptor { StaticPaymentOutput(StaticPaymentOutputDescriptor), } -impl_writeable_tlv_based_enum!(SpendableOutputDescriptor, +impl_writeable_tlv_based_enum_legacy!(SpendableOutputDescriptor, (0, StaticOutput) => { (0, outpoint, required), (1, channel_keys_id, option), diff --git a/lightning/src/util/config.rs b/lightning/src/util/config.rs index 4e124c27fd9..bd8e053899d 100644 --- a/lightning/src/util/config.rs +++ b/lightning/src/util/config.rs @@ -410,7 +410,7 @@ pub enum MaxDustHTLCExposure { FeeRateMultiplier(u64), } -impl_writeable_tlv_based_enum!(MaxDustHTLCExposure, ; +impl_writeable_tlv_based_enum_legacy!(MaxDustHTLCExposure, ; (1, FixedLimitMsat), (3, FeeRateMultiplier), ); diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index a45c5029230..f7a299d9b21 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -996,8 +996,11 @@ macro_rules! tlv_record_ref_type { 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)),* $(,)*) => { + ),* $(,)?; + // $tuple_variant_* are only passed from `impl_writeable_tlv_based_enum_*_legacy` + $(($tuple_variant_id: expr, $tuple_variant_name: ident)),* $(,)?; + // $length_prefixed_* are only passed from `impl_writeable_tlv_based_enum_*` non-`legacy` + $(($length_prefixed_tuple_variant_id: expr, $length_prefixed_tuple_variant_name: ident)),* $(,)?) => { impl $crate::util::ser::Writeable for $st { fn write(&self, writer: &mut W) -> Result<(), $crate::io::Error> { match self { @@ -1013,6 +1016,12 @@ macro_rules! _impl_writeable_tlv_based_enum_common { id.write(writer)?; field.write(writer)?; }),* + $($st::$length_prefixed_tuple_variant_name (ref field) => { + let id: u8 = $length_prefixed_tuple_variant_id; + id.write(writer)?; + $crate::util::ser::BigSize(field.serialized_length() as u64).write(writer)?; + field.write(writer)?; + }),* } Ok(()) } @@ -1022,29 +1031,104 @@ macro_rules! _impl_writeable_tlv_based_enum_common { /// Implement [`Readable`] and [`Writeable`] for an enum, with struct variants stored as TLVs and tuple /// variants stored directly. -/// The format is, for example -/// ```ignore +/// +/// The format is, for example, +/// ``` +/// enum EnumName { +/// StructVariantA { +/// required_variant_field: u64, +/// optional_variant_field: Option, +/// }, +/// StructVariantB { +/// variant_field_a: bool, +/// variant_field_b: u32, +/// variant_vec_field: Vec, +/// }, +/// TupleVariantA(), +/// TupleVariantB(Vec), +/// } +/// # use lightning::impl_writeable_tlv_based_enum; /// 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, optional_vec)}; -/// (2, TupleVariantA), (3, TupleVariantB), +/// (1, StructVariantB) => {(0, variant_field_a, required), (1, variant_field_b, required), (2, variant_vec_field, optional_vec)}, +/// (2, TupleVariantA) => {}, // Note that empty tuple variants have to use the struct syntax due to rust limitations +/// {3, TupleVariantB} => (), /// ); /// ``` -/// The type is written as a single byte, followed by any variant data. +/// +/// The type is written as a single byte, followed by length-prefixed variant data. +/// /// Attempts to read an unknown type byte result in [`DecodeError::UnknownRequiredFeature`]. /// +/// Note that the serialization for tuple variants (as well as the call format) was changed in LDK +/// 0.0.124. +/// /// [`Readable`]: crate::util::ser::Readable /// [`Writeable`]: crate::util::ser::Writeable /// [`DecodeError::UnknownRequiredFeature`]: crate::ln::msgs::DecodeError::UnknownRequiredFeature #[macro_export] 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} => ()),* + $(,)? + ) => { + $crate::_impl_writeable_tlv_based_enum_common!($st, + $(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),* + ;; + $(($tuple_variant_id, $tuple_variant_name)),*); + + impl $crate::util::ser::Readable for $st { + #[allow(unused_mut)] + fn read(mut reader: &mut R) -> Result { + let id: u8 = $crate::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 mut f = || { + $crate::_init_and_read_len_prefixed_tlv_fields!(reader, { + $(($type, $field, $fieldty)),* + }); + Ok($st::$variant_name { + $( + $field: $crate::_init_tlv_based_struct_field!($field, $fieldty) + ),* + }) + }; + f() + }),* + $($tuple_variant_id => { + let length: $crate::util::ser::BigSize = $crate::util::ser::Readable::read(reader)?; + let mut s = $crate::util::ser::FixedLengthReader::new(&mut reader, length.0); + let res = $crate::util::ser::Readable::read(&mut s)?; + if s.bytes_remain() { + s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes + return Err($crate::ln::msgs::DecodeError::InvalidValue); + } + Ok($st::$tuple_variant_name(res)) + }),* + _ => { + Err($crate::ln::msgs::DecodeError::UnknownRequiredFeature) + }, + } + } + } + } +} + +/// See [`impl_writeable_tlv_based_enum`] and use that unless backwards-compatibility with tuple +/// variants is required. +macro_rules! impl_writeable_tlv_based_enum_legacy { ($st: ident, $(($variant_id: expr, $variant_name: ident) => {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*} ),* $(,)*; - $(($tuple_variant_id: expr, $tuple_variant_name: ident)),* $(,)*) => { + $(($tuple_variant_id: expr, $tuple_variant_name: ident)),+ $(,)?) => { $crate::_impl_writeable_tlv_based_enum_common!($st, $(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*; - $(($tuple_variant_id, $tuple_variant_name)),*); + $(($tuple_variant_id, $tuple_variant_name)),+;); impl $crate::util::ser::Readable for $st { fn read(reader: &mut R) -> Result { @@ -1067,7 +1151,7 @@ macro_rules! impl_writeable_tlv_based_enum { }),* $($tuple_variant_id => { Ok($st::$tuple_variant_name($crate::util::ser::Readable::read(reader)?)) - }),* + }),+ _ => { Err($crate::ln::msgs::DecodeError::UnknownRequiredFeature) }, @@ -1085,9 +1169,8 @@ macro_rules! impl_writeable_tlv_based_enum { /// 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. /// -/// Note that only struct and unit variants (not tuple variants) will support downgrading, thus any -/// new odd variants MUST be non-tuple (i.e. described using `$variant_id` and `$variant_name` not -/// `$tuple_variant_id` and `$tuple_variant_name`). +/// Note that the serialization for tuple variants (as well as the call format) was changed in LDK +/// 0.0.124. /// /// [`MaybeReadable`]: crate::util::ser::MaybeReadable /// [`Writeable`]: crate::util::ser::Writeable @@ -1095,19 +1178,23 @@ macro_rules! impl_writeable_tlv_based_enum { /// [`Readable`]: crate::util::ser::Readable #[macro_export] 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)),* $(,)*)? - $(unread_variants: $($unread_variant: ident),*)?) => { + ($st: ident, + $(($variant_id: expr, $variant_name: ident) => + {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*} + ),* + $(, {$tuple_variant_id: expr, $tuple_variant_name: ident} => ())* + $(, unread_variants: $($unread_variant: ident),*)? + $(,)? + ) => { $crate::_impl_writeable_tlv_based_enum_common!($st, $(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),* $(, $((255, $unread_variant) => {}),*)? - ; $($(($tuple_variant_id, $tuple_variant_name)),*)?); + ;; + $(($tuple_variant_id, $tuple_variant_name)),*); impl $crate::util::ser::MaybeReadable for $st { - fn read(reader: &mut R) -> Result, $crate::ln::msgs::DecodeError> { + #[allow(unused_mut)] + fn read(mut reader: &mut R) -> Result, $crate::ln::msgs::DecodeError> { let id: u8 = $crate::util::ser::Readable::read(reader)?; match id { $($variant_id => { @@ -1125,12 +1212,66 @@ macro_rules! impl_writeable_tlv_based_enum_upgradable { }; f() }),* - $($($tuple_variant_id => { - Ok(Some($st::$tuple_variant_name(Readable::read(reader)?))) - }),*)* + $($tuple_variant_id => { + let length: $crate::util::ser::BigSize = $crate::util::ser::Readable::read(reader)?; + let mut s = $crate::util::ser::FixedLengthReader::new(&mut reader, length.0); + let res = $crate::util::ser::Readable::read(&mut s)?; + if s.bytes_remain() { + s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes + return Err($crate::ln::msgs::DecodeError::InvalidValue); + } + Ok(Some($st::$tuple_variant_name(res))) + }),* // Note that we explicitly match 255 here to reserve it for use in // `unread_variants`. 255|_ if id % 2 == 1 => { + let tlv_len: $crate::util::ser::BigSize = $crate::util::ser::Readable::read(reader)?; + let mut rd = $crate::util::ser::FixedLengthReader::new(reader, tlv_len.0); + rd.eat_remaining().map_err(|_| $crate::ln::msgs::DecodeError::ShortRead)?; + Ok(None) + }, + _ => Err($crate::ln::msgs::DecodeError::UnknownRequiredFeature), + } + } + } + } +} + +/// See [`impl_writeable_tlv_based_enum_upgradable`] and use that unless backwards-compatibility +/// with tuple variants is required. +macro_rules! impl_writeable_tlv_based_enum_upgradable_legacy { + ($st: ident, $(($variant_id: expr, $variant_name: ident) => + {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*} + ),* $(,)? + ; + $(($tuple_variant_id: expr, $tuple_variant_name: ident)),+ $(,)?) => { + $crate::_impl_writeable_tlv_based_enum_common!($st, + $(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*; + $(($tuple_variant_id, $tuple_variant_name)),+;); + + impl $crate::util::ser::MaybeReadable for $st { + fn read(reader: &mut R) -> Result, $crate::ln::msgs::DecodeError> { + let id: u8 = $crate::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 mut f = || { + $crate::_init_and_read_len_prefixed_tlv_fields!(reader, { + $(($type, $field, $fieldty)),* + }); + Ok(Some($st::$variant_name { + $( + $field: $crate::_init_tlv_based_struct_field!($field, $fieldty) + ),* + })) + }; + f() + }),* + $($tuple_variant_id => { + Ok(Some($st::$tuple_variant_name(Readable::read(reader)?))) + }),+ + _ if id % 2 == 1 => { // Assume that a $variant_id was written, not a $tuple_variant_id, and read // the length prefix and discard the correct number of bytes. let tlv_len: $crate::util::ser::BigSize = $crate::util::ser::Readable::read(reader)?; @@ -1549,4 +1690,33 @@ mod tests { let decoded_msg: EmptyMsg = Readable::read(&mut encoded_msg_stream).unwrap(); assert_eq!(msg, decoded_msg); } + + #[derive(Debug, PartialEq, Eq)] + enum TuplesOnly { + A(), + B(u64), + } + impl_writeable_tlv_based_enum_upgradable!(TuplesOnly, (2, A) => {}, {3, B} => ()); + + #[test] + fn test_impl_writeable_enum() { + let a = TuplesOnly::A().encode(); + assert_eq!(TuplesOnly::read(&mut Cursor::new(&a)).unwrap(), Some(TuplesOnly::A())); + let b42 = TuplesOnly::B(42).encode(); + assert_eq!(TuplesOnly::read(&mut Cursor::new(&b42)).unwrap(), Some(TuplesOnly::B(42))); + + // Test unknown variants with 0-length data + let unknown_variant = vec![41, 0]; + let mut none_read = Cursor::new(&unknown_variant); + assert_eq!(TuplesOnly::read(&mut none_read).unwrap(), None); + assert_eq!(none_read.position(), unknown_variant.len() as u64); + + TuplesOnly::read(&mut Cursor::new(&vec![42, 0])).unwrap_err(); + + // Test unknown variants with data + let unknown_data_variant = vec![41, 3, 42, 52, 62]; + let mut none_data_read = Cursor::new(&unknown_data_variant); + assert_eq!(TuplesOnly::read(&mut none_data_read).unwrap(), None); + assert_eq!(none_data_read.position(), unknown_data_variant.len() as u64); + } } diff --git a/lightning/src/util/sweep.rs b/lightning/src/util/sweep.rs index dd26a8ef844..1e04eb59bf8 100644 --- a/lightning/src/util/sweep.rs +++ b/lightning/src/util/sweep.rs @@ -315,7 +315,7 @@ impl_writeable_tlv_based_enum!(OutputSpendStatus, (4, latest_spending_tx, required), (6, confirmation_height, required), (8, confirmation_hash, required), - }; + }, ); /// A utility that keeps track of [`SpendableOutputDescriptor`]s, persists them in a given From 1d1f47c45a07b2bd622979d17ce158abfc92e9e9 Mon Sep 17 00:00:00 2001 From: Matt Corallo Date: Mon, 8 Jul 2024 20:43:51 +0000 Subject: [PATCH 2/2] Add pending changelog entry for the previous commit --- pending_changelog/3160-format-change.txt | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 pending_changelog/3160-format-change.txt diff --git a/pending_changelog/3160-format-change.txt b/pending_changelog/3160-format-change.txt new file mode 100644 index 00000000000..80cdc5757b9 --- /dev/null +++ b/pending_changelog/3160-format-change.txt @@ -0,0 +1,4 @@ +## Backwards Compatibility + * `impl_writeable_tlv_based_enum[_upgradable]`'s writing of tuple variants has + changed format. Users of this macro who are reading/writing tuple variants + will need to perform a format-change to update their serialized data (#3160).