Merge pull request #1045 from TheBlueMatt/2021-08-chanmon-ser-upgradability
authorMatt Corallo <649246+TheBlueMatt@users.noreply.github.com>
Mon, 16 Aug 2021 18:30:36 +0000 (18:30 +0000)
committerGitHub <noreply@github.com>
Mon, 16 Aug 2021 18:30:36 +0000 (18:30 +0000)
Make `ChannelMonitor` serialization slightly more upgradable

lightning/src/chain/channelmonitor.rs
lightning/src/chain/onchaintx.rs
lightning/src/ln/channelmanager.rs
lightning/src/util/ser.rs
lightning/src/util/ser_macros.rs

index 3c75ec9e99ecd68baa51be9d1f60eb8e39becb2f..9c381a350309f1f94d099203f6926156aceca9d9 100644 (file)
@@ -106,7 +106,9 @@ impl Readable for ChannelMonitorUpdate {
                let len: u64 = Readable::read(r)?;
                let mut updates = Vec::with_capacity(cmp::min(len as usize, MAX_ALLOC_SIZE / ::core::mem::size_of::<ChannelMonitorUpdateStep>()));
                for _ in 0..len {
-                       updates.push(Readable::read(r)?);
+                       if let Some(upd) = MaybeReadable::read(r)? {
+                               updates.push(upd);
+                       }
                }
                read_tlv_fields!(r, {});
                Ok(Self { update_id, updates })
@@ -394,13 +396,36 @@ enum OnchainEvent {
        },
 }
 
-impl_writeable_tlv_based!(OnchainEventEntry, {
-       (0, txid, required),
-       (2, height, required),
-       (4, event, required),
-});
+impl Writeable for OnchainEventEntry {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               write_tlv_fields!(writer, {
+                       (0, self.txid, required),
+                       (2, self.height, required),
+                       (4, self.event, required),
+               });
+               Ok(())
+       }
+}
 
-impl_writeable_tlv_based_enum!(OnchainEvent,
+impl MaybeReadable for OnchainEventEntry {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
+               let mut txid = Default::default();
+               let mut height = 0;
+               let mut event = None;
+               read_tlv_fields!(reader, {
+                       (0, txid, required),
+                       (2, height, required),
+                       (4, event, ignorable),
+               });
+               if let Some(ev) = event {
+                       Ok(Some(Self { txid, height, event: ev }))
+               } else {
+                       Ok(None)
+               }
+       }
+}
+
+impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
        (0, HTLCUpdate) => {
                (0, source, required),
                (1, onchain_value_satoshis, option),
@@ -409,7 +434,7 @@ impl_writeable_tlv_based_enum!(OnchainEvent,
        (1, MaturingOutput) => {
                (0, descriptor, required),
        },
-;);
+);
 
 #[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
@@ -443,7 +468,7 @@ pub(crate) enum ChannelMonitorUpdateStep {
        },
 }
 
-impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
+impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
        (0, LatestHolderCommitmentTXInfo) => {
                (0, commitment_tx, required),
                (2, htlc_outputs, vec_type),
@@ -467,7 +492,7 @@ impl_writeable_tlv_based_enum!(ChannelMonitorUpdateStep,
        (5, ShutdownScript) => {
                (0, scriptpubkey, required),
        },
-;);
+);
 
 /// A ChannelMonitor handles chain events (blocks connected and disconnected) and generates
 /// on-chain transactions to ensure no loss of funds occurs.
@@ -2731,7 +2756,9 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
                let waiting_threshold_conf_len: u64 = Readable::read(reader)?;
                let mut onchain_events_awaiting_threshold_conf = Vec::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128));
                for _ in 0..waiting_threshold_conf_len {
-                       onchain_events_awaiting_threshold_conf.push(Readable::read(reader)?);
+                       if let Some(val) = MaybeReadable::read(reader)? {
+                               onchain_events_awaiting_threshold_conf.push(val);
+                       }
                }
 
                let outputs_to_watch_len: u64 = Readable::read(reader)?;
index 44bbfc2d60ee54e8282de82eabdd18da17a39fcf..d6777cc5c8cfd494409f5832f29efb3699bc21ab 100644 (file)
@@ -29,7 +29,7 @@ use chain::channelmonitor::{ANTI_REORG_DELAY, CLTV_SHARED_CLAIM_BUFFER};
 use chain::keysinterface::{Sign, KeysInterface};
 use chain::package::PackageTemplate;
 use util::logger::Logger;
-use util::ser::{Readable, ReadableArgs, Writer, Writeable, VecWriter};
+use util::ser::{Readable, ReadableArgs, MaybeReadable, Writer, Writeable, VecWriter};
 use util::byte_utils;
 
 use io;
@@ -79,20 +79,43 @@ enum OnchainEvent {
        }
 }
 
-impl_writeable_tlv_based!(OnchainEventEntry, {
-       (0, txid, required),
-       (2, height, required),
-       (4, event, required),
-});
+impl Writeable for OnchainEventEntry {
+       fn write<W: Writer>(&self, writer: &mut W) -> Result<(), io::Error> {
+               write_tlv_fields!(writer, {
+                       (0, self.txid, required),
+                       (2, self.height, required),
+                       (4, self.event, required),
+               });
+               Ok(())
+       }
+}
+
+impl MaybeReadable for OnchainEventEntry {
+       fn read<R: io::Read>(reader: &mut R) -> Result<Option<Self>, DecodeError> {
+               let mut txid = Default::default();
+               let mut height = 0;
+               let mut event = None;
+               read_tlv_fields!(reader, {
+                       (0, txid, required),
+                       (2, height, required),
+                       (4, event, ignorable),
+               });
+               if let Some(ev) = event {
+                       Ok(Some(Self { txid, height, event: ev }))
+               } else {
+                       Ok(None)
+               }
+       }
+}
 
-impl_writeable_tlv_based_enum!(OnchainEvent,
+impl_writeable_tlv_based_enum_upgradable!(OnchainEvent,
        (0, Claim) => {
                (0, claim_request, required),
        },
        (1, ContentiousOutpoint) => {
                (0, package, required),
        },
-;);
+);
 
 impl Readable for Option<Vec<Option<(usize, Signature)>>> {
        fn read<R: io::Read>(reader: &mut R) -> Result<Self, DecodeError> {
@@ -296,7 +319,9 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> for OnchainTxHandler<K::Signer> {
                let waiting_threshold_conf_len: u64 = Readable::read(reader)?;
                let mut onchain_events_awaiting_threshold_conf = Vec::with_capacity(cmp::min(waiting_threshold_conf_len as usize, MAX_ALLOC_SIZE / 128));
                for _ in 0..waiting_threshold_conf_len {
-                       onchain_events_awaiting_threshold_conf.push(Readable::read(reader)?);
+                       if let Some(val) = MaybeReadable::read(reader)? {
+                               onchain_events_awaiting_threshold_conf.push(val);
+                       }
                }
 
                read_tlv_fields!(reader, {});
index 7304c698ab185b0dfe6c4b90b6b18c9895a0765c..38e2172ed409c0eb684c5b0b1290820c47c23c49 100644 (file)
@@ -1477,8 +1477,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 
                let mut chacha = ChaCha20::new(&rho, &[0u8; 8]);
                let mut chacha_stream = ChaChaReader { chacha: &mut chacha, read: Cursor::new(&msg.onion_routing_packet.hop_data[..]) };
-               let (next_hop_data, next_hop_hmac) = {
-                       match msgs::OnionHopData::read(&mut chacha_stream) {
+               let (next_hop_data, next_hop_hmac): (msgs::OnionHopData, _) = {
+                       match <msgs::OnionHopData as Readable>::read(&mut chacha_stream) {
                                Err(err) => {
                                        let error_code = match err {
                                                msgs::DecodeError::UnknownVersion => 0x4000 | 1, // unknown realm byte
index b27cf4b348c3d98fc805dd7478dbe640910d80f4..a7f69fcfcb583b13c47179e8cd9f8ad12f9d2573 100644 (file)
@@ -241,6 +241,13 @@ pub trait MaybeReadable
        fn read<R: Read>(reader: &mut R) -> Result<Option<Self>, DecodeError>;
 }
 
+impl<T: Readable> MaybeReadable for T {
+       #[inline]
+       fn read<R: Read>(reader: &mut R) -> Result<Option<T>, DecodeError> {
+               Ok(Some(Readable::read(reader)?))
+       }
+}
+
 pub(crate) struct OptionDeserWrapper<T: Readable>(pub Option<T>);
 impl<T: Readable> Readable for OptionDeserWrapper<T> {
        #[inline]
@@ -262,15 +269,16 @@ impl<'a, T: Writeable> Writeable for VecWriteWrapper<'a, T> {
 }
 
 /// Wrapper to read elements from a given stream until it reaches the end of the stream.
-pub(crate) struct VecReadWrapper<T: Readable>(pub Vec<T>);
-impl<T: Readable> Readable for VecReadWrapper<T> {
+pub(crate) struct VecReadWrapper<T>(pub Vec<T>);
+impl<T: MaybeReadable> Readable for VecReadWrapper<T> {
        #[inline]
        fn read<R: Read>(mut reader: &mut R) -> Result<Self, DecodeError> {
                let mut values = Vec::new();
                loop {
                        let mut track_read = ReadTrackingReader::new(&mut reader);
-                       match Readable::read(&mut track_read) {
-                               Ok(v) => { values.push(v); },
+                       match MaybeReadable::read(&mut track_read) {
+                               Ok(Some(v)) => { values.push(v); },
+                               Ok(None) => { },
                                // If we failed to read any bytes at all, we reached the end of our TLV
                                // stream and have simply exhausted all entries.
                                Err(ref e) if e == &DecodeError::ShortRead && !track_read.have_read => break,
@@ -561,7 +569,7 @@ impl Readable for Vec<Signature> {
                        return Err(DecodeError::BadLengthDescriptor);
                }
                let mut ret = Vec::with_capacity(len as usize);
-               for _ in 0..len { ret.push(Signature::read(r)?); }
+               for _ in 0..len { ret.push(Readable::read(r)?); }
                Ok(ret)
        }
 }
@@ -726,7 +734,8 @@ impl<T: Writeable> Writeable for Option<T> {
 impl<T: Readable> Readable for Option<T>
 {
        fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
-               match BigSize::read(r)?.0 {
+               let len: BigSize = Readable::read(r)?;
+               match len.0 {
                        0 => Ok(None),
                        len => {
                                let mut reader = FixedLengthReader::new(r, len - 1);
index 960fae45d26181d806e95c3da1884aa7be3f930a..af1bebe680b3db9324be291c257f293f8d6f90a8 100644 (file)
@@ -115,6 +115,9 @@ 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, ignorable) => {{
+               // no-op
+       }};
 }
 
 macro_rules! check_missing_tlv {
@@ -138,6 +141,9 @@ macro_rules! check_missing_tlv {
        ($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 {
@@ -153,6 +159,9 @@ macro_rules! decode_tlv {
        ($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 {
@@ -371,7 +380,7 @@ macro_rules! read_ver_prefix {
 /// Reads a suffix added by write_tlv_fields.
 macro_rules! read_tlv_fields {
        ($stream: expr, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { {
-               let tlv_len = ::util::ser::BigSize::read($stream)?;
+               let tlv_len: ::util::ser::BigSize = ::util::ser::Readable::read($stream)?;
                let mut rd = ::util::ser::FixedLengthReader::new($stream, tlv_len.0);
                decode_tlv_stream!(&mut rd, {$(($type, $field, $fieldty)),*});
                rd.eat_remaining().map_err(|_| ::ln::msgs::DecodeError::ShortRead)?;
@@ -405,7 +414,7 @@ macro_rules! init_tlv_field_var {
        };
        ($field: ident, option) => {
                let mut $field = None;
-       }
+       };
 }
 
 /// Implements Readable/Writeable for a struct storing it as a set of TLVs
@@ -458,17 +467,7 @@ macro_rules! impl_writeable_tlv_based {
        }
 }
 
-/// 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 {
+macro_rules! _impl_writeable_tlv_based_enum_common {
        ($st: ident, $(($variant_id: expr, $variant_name: ident) =>
                {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}
        ),* $(,)*;
@@ -492,6 +491,72 @@ macro_rules! impl_writeable_tlv_based_enum {
                                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)),* $(,)*}
+       ),* $(,)*) => {
+               _impl_writeable_tlv_based_enum_common!($st,
+                       $(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*; );
+
+               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()
+                                       }),*
+                                       _ 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> {