]> git.bitcoin.ninja Git - rust-lightning/commitdiff
Append backwards-compat TLVs to serialization of larger structs
authorMatt Corallo <git@bluematt.me>
Wed, 5 May 2021 22:56:42 +0000 (22:56 +0000)
committerMatt Corallo <git@bluematt.me>
Mon, 24 May 2021 22:05:41 +0000 (22:05 +0000)
Currently our serialization is very compact, and contains version
numbers to indicate which versions the code can read a given
serialized struct. However, if you want to add a new field without
needlessly breaking the ability of previous versions of the code to
read the struct, there is not a good way to do so.

This adds dummy, currently empty, TLVs to the major structs we
serialize out for users, providing an easy place to put new
optional fields without breaking previous versions.

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

index 8526ce51722afefa80d14657f095fbcbfdf41ab2..bbb55fc22e7f81a583fc0b1e7707dce0e93cfc14 100644 (file)
@@ -502,9 +502,6 @@ enum OnchainEvent {
        },
 }
 
-const SERIALIZATION_VERSION: u8 = 1;
-const MIN_SERIALIZATION_VERSION: u8 = 1;
-
 #[cfg_attr(any(test, feature = "fuzztarget", feature = "_test_utils"), derive(PartialEq))]
 #[derive(Clone)]
 pub(crate) enum ChannelMonitorUpdateStep {
@@ -805,17 +802,17 @@ impl<Signer: Sign> PartialEq for ChannelMonitorImpl<Signer> {
 
 impl<Signer: Sign> Writeable for ChannelMonitor<Signer> {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
-               //TODO: We still write out all the serialization here manually instead of using the fancy
-               //serialization framework we have, we should migrate things over to it.
-               writer.write_all(&[SERIALIZATION_VERSION; 1])?;
-               writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
-
                self.inner.lock().unwrap().write(writer)
        }
 }
 
+const SERIALIZATION_VERSION: u8 = 1;
+const MIN_SERIALIZATION_VERSION: u8 = 1;
+
 impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), Error> {
+               write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
+
                self.latest_update_id.write(writer)?;
 
                // Set in initial Channel-object creation, so should always be set by now:
@@ -991,6 +988,8 @@ impl<Signer: Sign> Writeable for ChannelMonitorImpl<Signer> {
                self.lockdown_from_offchain.write(writer)?;
                self.holder_tx_signed.write(writer)?;
 
+               write_tlv_fields!(writer, {});
+
                Ok(())
        }
 }
@@ -2754,11 +2753,7 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
                        }
                }
 
-               let _ver: u8 = Readable::read(reader)?;
-               let min_ver: u8 = Readable::read(reader)?;
-               if min_ver > SERIALIZATION_VERSION {
-                       return Err(DecodeError::UnknownVersion);
-               }
+               let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
                let latest_update_id: u64 = Readable::read(reader)?;
                let commitment_transaction_number_obscure_factor = <U48 as Readable>::read(reader)?.0;
@@ -2979,6 +2974,8 @@ impl<'a, Signer: Sign, K: KeysInterface<Signer = Signer>> ReadableArgs<&'a K>
                let lockdown_from_offchain = Readable::read(reader)?;
                let holder_tx_signed = Readable::read(reader)?;
 
+               read_tlv_fields!(reader, {}, {});
+
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
 
index b940b45f7013e976e0c36d9abe1fde4de1c56ec1..7da38b7aed656cac727c6f29ebfbfb0bb9a798bb 100644 (file)
@@ -4395,8 +4395,7 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                // Note that we write out as if remove_uncommitted_htlcs_and_mark_paused had just been
                // called but include holding cell updates (and obviously we don't modify self).
 
-               writer.write_all(&[SERIALIZATION_VERSION; 1])?;
-               writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
+               write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
 
                self.user_id.write(writer)?;
                self.config.write(writer)?;
@@ -4585,6 +4584,9 @@ impl<Signer: Sign> Writeable for Channel<Signer> {
                self.commitment_secrets.write(writer)?;
 
                self.channel_update_status.write(writer)?;
+
+               write_tlv_fields!(writer, {});
+
                Ok(())
        }
 }
@@ -4593,11 +4595,7 @@ const MAX_ALLOC_SIZE: usize = 64*1024;
 impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
                where K::Target: KeysInterface<Signer = Signer> {
        fn read<R : ::std::io::Read>(reader: &mut R, keys_source: &'a K) -> Result<Self, DecodeError> {
-               let _ver: u8 = Readable::read(reader)?;
-               let min_ver: u8 = Readable::read(reader)?;
-               if min_ver > SERIALIZATION_VERSION {
-                       return Err(DecodeError::UnknownVersion);
-               }
+               let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
                let user_id = Readable::read(reader)?;
                let config: ChannelConfig = Readable::read(reader)?;
@@ -4759,6 +4757,8 @@ impl<'a, Signer: Sign, K: Deref> ReadableArgs<&'a K> for Channel<Signer>
 
                let channel_update_status = Readable::read(reader)?;
 
+               read_tlv_fields!(reader, {}, {});
+
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_source.get_secure_random_bytes());
 
index fe3be2aeca56d9dc089d3385afadd5921b24ebd4..4919a225aacc2133c1623d245c5bc83c4be6c05f 100644 (file)
@@ -4431,8 +4431,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
        fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
                let _consistency_lock = self.total_consistency_lock.write().unwrap();
 
-               writer.write_all(&[SERIALIZATION_VERSION; 1])?;
-               writer.write_all(&[MIN_SERIALIZATION_VERSION; 1])?;
+               write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
 
                self.genesis_hash.write(writer)?;
                {
@@ -4515,6 +4514,8 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> Writeable f
                        session_priv.write(writer)?;
                }
 
+               write_tlv_fields!(writer, {});
+
                Ok(())
        }
 }
@@ -4638,11 +4639,7 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
         L::Target: Logger,
 {
        fn read<R: ::std::io::Read>(reader: &mut R, mut args: ChannelManagerReadArgs<'a, Signer, M, T, K, F, L>) -> Result<Self, DecodeError> {
-               let _ver: u8 = Readable::read(reader)?;
-               let min_ver: u8 = Readable::read(reader)?;
-               if min_ver > SERIALIZATION_VERSION {
-                       return Err(DecodeError::UnknownVersion);
-               }
+               let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
 
                let genesis_hash: BlockHash = Readable::read(reader)?;
                let best_block_height: u32 = Readable::read(reader)?;
@@ -4762,6 +4759,8 @@ impl<'a, Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref>
                        }
                }
 
+               read_tlv_fields!(reader, {}, {});
+
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&args.keys_manager.get_secure_random_bytes());
 
index a51f265135f26cfac08a61f06c6743240b013687..f18f2d406bc688d71550cae400e20197a43566fb 100644 (file)
@@ -308,8 +308,13 @@ pub struct OnchainTxHandler<ChannelSigner: Sign> {
        secp_ctx: Secp256k1<secp256k1::All>,
 }
 
+const SERIALIZATION_VERSION: u8 = 1;
+const MIN_SERIALIZATION_VERSION: u8 = 1;
+
 impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
        pub(crate) fn write<W: Writer>(&self, writer: &mut W) -> Result<(), ::std::io::Error> {
+               write_ver_prefix!(writer, SERIALIZATION_VERSION, MIN_SERIALIZATION_VERSION);
+
                self.destination_script.write(writer)?;
                self.holder_commitment.write(writer)?;
                self.holder_htlc_sigs.write(writer)?;
@@ -355,12 +360,16 @@ impl<ChannelSigner: Sign> OnchainTxHandler<ChannelSigner> {
                        }
                }
                self.latest_height.write(writer)?;
+
+               write_tlv_fields!(writer, {});
                Ok(())
        }
 }
 
 impl<'a, K: KeysInterface> ReadableArgs<&'a K> for OnchainTxHandler<K::Signer> {
        fn read<R: ::std::io::Read>(reader: &mut R, keys_manager: &'a K) -> Result<Self, DecodeError> {
+               let _ver = read_ver_prefix!(reader, SERIALIZATION_VERSION);
+
                let destination_script = Readable::read(reader)?;
 
                let holder_commitment = Readable::read(reader)?;
@@ -421,6 +430,8 @@ impl<'a, K: KeysInterface> ReadableArgs<&'a K> for OnchainTxHandler<K::Signer> {
                }
                let latest_height = Readable::read(reader)?;
 
+               read_tlv_fields!(reader, {}, {});
+
                let mut secp_ctx = Secp256k1::new();
                secp_ctx.seeded_randomize(&keys_manager.get_secure_random_bytes());
 
index 5062bae769cf9d9d752791226875a131ae58895e..f510e0f17224b4ce3bb61f7321c38a317aa45c71 100644 (file)
@@ -9,6 +9,7 @@
 
 macro_rules! encode_tlv {
        ($stream: expr, {$(($type: expr, $field: expr)),*}) => { {
+               #[allow(unused_imports)]
                use util::ser::{BigSize, LengthCalculatingWriter};
                $(
                        BigSize($type).write($stream)?;
@@ -23,6 +24,7 @@ macro_rules! encode_tlv {
 macro_rules! encode_varint_length_prefixed_tlv {
        ($stream: expr, {$(($type: expr, $field: expr)),*}) => { {
                use util::ser::{BigSize, LengthCalculatingWriter};
+               #[allow(unused_mut)]
                let mut len = LengthCalculatingWriter(0);
                {
                        $(
@@ -178,6 +180,64 @@ macro_rules! impl_writeable_len_match {
        }
 }
 
+/// Write out two bytes to indicate the version of this object.
+/// $this_version represents a unique version of this object, incremented whenever the reader may
+///               wish to interpret fields differently based on the version or when we wish to
+///               communicate to previous versions that they can no longer reasonably understand
+///               this serialized object.
+/// $min_version_that_can_read_this is the minimum client version which can reasonably understand
+///                                 this serialized object. Previous versions will simply err with a
+///                                 DecodeError::UnknownVersion.
+///
+/// Updates to either $this_version or $min_version_that_can_read_this should be included in
+/// release notes.
+///
+/// Both version fields can be specific to this type of object.
+macro_rules! write_ver_prefix {
+       ($stream: expr, $this_version: expr, $min_version_that_can_read_this: expr) => {
+               $stream.write_all(&[$this_version; 1])?;
+               $stream.write_all(&[$min_version_that_can_read_this; 1])?;
+       }
+}
+
+/// Writes out a suffix to an object which contains potentially backwards-compatible, optional
+/// fields which old nodes can happily ignore.
+///
+/// It is written out in TLV format and, as with all TLV fields, unknown even fields cause a
+/// DecodeError::UnknownRequiredFeature error, with unknown odd fields ignored.
+///
+/// This is the preferred method of adding new fields which old nodes can ignore and still function
+/// correctly.
+macro_rules! write_tlv_fields {
+       ($stream: expr, {$(($type: expr, $field: expr)),*}) => {
+               encode_varint_length_prefixed_tlv!($stream, {$(($type, $field)),*});
+       }
+}
+
+/// Reads a prefix added by write_ver_prefix!(), above. Takes the current version of the
+/// serialization logic for this object. This is compared against the
+/// $min_version_that_can_read_this added by write_ver_prefix!().
+macro_rules! read_ver_prefix {
+       ($stream: expr, $this_version: expr) => { {
+               let ver: u8 = Readable::read($stream)?;
+               let min_ver: u8 = Readable::read($stream)?;
+               if min_ver > $this_version {
+                       return Err(DecodeError::UnknownVersion);
+               }
+               ver
+       } }
+}
+
+/// 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)?;
+               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)?;
+       } }
+}
+
 #[cfg(test)]
 mod tests {
        use std::io::{Cursor, Read};