Convert most P2P msg serialization to a new macro with TLV suffixes

The network serialization format for all messages was changed some
time ago to include a TLV suffix for all messages, however we never
bothered to implement it as there isn't a lot of use validating a
TLV stream with nothing to do with it. However, messages are
increasingly utilizing the TLV suffix feature, and there are some
compatibility concerns with messages written as a part of other
structs having their format changed (see previous commit).

Thus, here we go ahead and convert most message serialization to a
new macro which includes a TLV suffix after a series of fields,
simplifying several serialization implementations in the process.

diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs
index 34ccc8adf7ad540ef3cdfa982159e36ae7523f30..4517afb223072aeefaaa9d67ef9296a8819f07d3 100644 (file)
--- a/lightning/src/ln/msgs.rs
+++ b/lightning/src/ln/msgs.rs
@@ -1053,7 +1053,7 @@ impl Readable for OptionalField<u64> {
 }
 
 
-impl_writeable!(AcceptChannel, {
+impl_writeable_msg!(AcceptChannel, {
        temporary_channel_id,
        dust_limit_satoshis,
        max_htlc_value_in_flight_msat,
@@ -1069,14 +1069,14 @@ impl_writeable!(AcceptChannel, {
        htlc_basepoint,
        first_per_commitment_point,
        shutdown_scriptpubkey
-});
+}, {});
 
-impl_writeable!(AnnouncementSignatures, {
+impl_writeable_msg!(AnnouncementSignatures, {
        channel_id,
        short_channel_id,
        node_signature,
        bitcoin_signature
-});
+}, {});
 
 impl Writeable for ChannelReestablish {
        fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
@@ -1115,41 +1115,21 @@ impl Readable for ChannelReestablish{
        }
 }
 
-impl Writeable for ClosingSigned {
-       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
-               self.channel_id.write(w)?;
-               self.fee_satoshis.write(w)?;
-               self.signature.write(w)?;
-               encode_tlv_stream!(w, {
-                       (1, self.fee_range, option),
-               });
-               Ok(())
-       }
-}
-
-impl Readable for ClosingSigned {
-       fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
-               let channel_id = Readable::read(r)?;
-               let fee_satoshis = Readable::read(r)?;
-               let signature = Readable::read(r)?;
-               let mut fee_range = None;
-               decode_tlv_stream!(r, {
-                       (1, fee_range, option),
-               });
-               Ok(Self { channel_id, fee_satoshis, signature, fee_range })
-       }
-}
+impl_writeable_msg!(ClosingSigned,
+       { channel_id, fee_satoshis, signature },
+       { (1, fee_range, option) }
+);
 
 impl_writeable!(ClosingSignedFeeRange, {
        min_fee_satoshis,
        max_fee_satoshis
 });
 
-impl_writeable!(CommitmentSigned, {
+impl_writeable_msg!(CommitmentSigned, {
        channel_id,
        signature,
        htlc_signatures
-});
+}, {});
 
 impl_writeable!(DecodedOnionErrorPacket, {
        hmac,
@@ -1157,22 +1137,22 @@ impl_writeable!(DecodedOnionErrorPacket, {
        pad
 });
 
-impl_writeable!(FundingCreated, {
+impl_writeable_msg!(FundingCreated, {
        temporary_channel_id,
        funding_txid,
        funding_output_index,
        signature
-});
+}, {});
 
-impl_writeable!(FundingSigned, {
+impl_writeable_msg!(FundingSigned, {
        channel_id,
        signature
-});
+}, {});
 
-impl_writeable!(FundingLocked, {
+impl_writeable_msg!(FundingLocked, {
        channel_id,
-       next_per_commitment_point
-});
+       next_per_commitment_point,
+}, {});
 
 impl Writeable for Init {
        fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
@@ -1193,7 +1173,7 @@ impl Readable for Init {
        }
 }
 
-impl_writeable!(OpenChannel, {
+impl_writeable_msg!(OpenChannel, {
        chain_hash,
        temporary_channel_id,
        funding_satoshis,
@@ -1213,47 +1193,53 @@ impl_writeable!(OpenChannel, {
        first_per_commitment_point,
        channel_flags,
        shutdown_scriptpubkey
-});
+}, {});
 
-impl_writeable!(RevokeAndACK, {
+impl_writeable_msg!(RevokeAndACK, {
        channel_id,
        per_commitment_secret,
        next_per_commitment_point
-});
+}, {});
 
-impl_writeable!(Shutdown, {
+impl_writeable_msg!(Shutdown, {
        channel_id,
        scriptpubkey
-});
+}, {});
 
-impl_writeable!(UpdateFailHTLC, {
+impl_writeable_msg!(UpdateFailHTLC, {
        channel_id,
        htlc_id,
        reason
-});
+}, {});
 
-impl_writeable!(UpdateFailMalformedHTLC, {
+impl_writeable_msg!(UpdateFailMalformedHTLC, {
        channel_id,
        htlc_id,
        sha256_of_onion,
        failure_code
-});
+}, {});
 
-impl_writeable!(UpdateFee, {
+impl_writeable_msg!(UpdateFee, {
        channel_id,
        feerate_per_kw
-});
+}, {});
 
-impl_writeable!(UpdateFulfillHTLC, {
+impl_writeable_msg!(UpdateFulfillHTLC, {
        channel_id,
        htlc_id,
        payment_preimage
-});
+}, {});
 
+// Note that this is written as a part of ChannelManager objects, and thus cannot change its
+// serialization format in a way which assumes we know the total serialized length/message end
+// position.
 impl_writeable!(OnionErrorPacket, {
        data
 });
 
+// Note that this is written as a part of ChannelManager objects, and thus cannot change its
+// serialization format in a way which assumes we know the total serialized length/message end
+// position.
 impl Writeable for OnionPacket {
        fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
                self.version.write(w)?;
@@ -1282,14 +1268,14 @@ impl Readable for OnionPacket {
        }
 }
 
-impl_writeable!(UpdateAddHTLC, {
+impl_writeable_msg!(UpdateAddHTLC, {
        channel_id,
        htlc_id,
        amount_msat,
        payment_hash,
        cltv_expiry,
        onion_routing_packet
-});
+}, {});
 
 impl Writeable for FinalOnionHopData {
        fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
@@ -1700,24 +1686,10 @@ impl Writeable for QueryShortChannelIds {
        }
 }
 
-impl Readable for ReplyShortChannelIdsEnd {
-       fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
-               let chain_hash: BlockHash = Readable::read(r)?;
-               let full_information: bool = Readable::read(r)?;
-               Ok(ReplyShortChannelIdsEnd {
-                       chain_hash,
-                       full_information,
-               })
-       }
-}
-
-impl Writeable for ReplyShortChannelIdsEnd {
-       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
-               self.chain_hash.write(w)?;
-               self.full_information.write(w)?;
-               Ok(())
-       }
-}
+impl_writeable_msg!(ReplyShortChannelIdsEnd, {
+       chain_hash,
+       full_information,
+}, {});
 
 impl QueryChannelRange {
        /**
@@ -1732,27 +1704,11 @@ impl QueryChannelRange {
        }
 }
 
-impl Readable for QueryChannelRange {
-       fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
-               let chain_hash: BlockHash = Readable::read(r)?;
-               let first_blocknum: u32 = Readable::read(r)?;
-               let number_of_blocks: u32 = Readable::read(r)?;
-               Ok(QueryChannelRange {
-                       chain_hash,
-                       first_blocknum,
-                       number_of_blocks
-               })
-       }
-}
-
-impl Writeable for QueryChannelRange {
-       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
-               self.chain_hash.write(w)?;
-               self.first_blocknum.write(w)?;
-               self.number_of_blocks.write(w)?;
-               Ok(())
-       }
-}
+impl_writeable_msg!(QueryChannelRange, {
+       chain_hash,
+       first_blocknum,
+       number_of_blocks
+}, {});
 
 impl Readable for ReplyChannelRange {
        fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
@@ -1812,28 +1768,11 @@ impl Writeable for ReplyChannelRange {
        }
 }
 
-impl Readable for GossipTimestampFilter {
-       fn read<R: Read>(r: &mut R) -> Result<Self, DecodeError> {
-               let chain_hash: BlockHash = Readable::read(r)?;
-               let first_timestamp: u32 = Readable::read(r)?;
-               let timestamp_range: u32 = Readable::read(r)?;
-               Ok(GossipTimestampFilter {
-                       chain_hash,
-                       first_timestamp,
-                       timestamp_range,
-               })
-       }
-}
-
-impl Writeable for GossipTimestampFilter {
-       fn write<W: Writer>(&self, w: &mut W) -> Result<(), io::Error> {
-               self.chain_hash.write(w)?;
-               self.first_timestamp.write(w)?;
-               self.timestamp_range.write(w)?;
-               Ok(())
-       }
-}
-
+impl_writeable_msg!(GossipTimestampFilter, {
+       chain_hash,
+       first_timestamp,
+       timestamp_range,
+}, {});
 
 #[cfg(test)]
 mod tests {

diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs
index b229383b283ce1c35e53cf290214a46d2b6e7a2d..1ddb7e017eb900aa80ccd09696d5ad90824284b0 100644 (file)
--- a/lightning/src/util/ser_macros.rs
+++ b/lightning/src/util/ser_macros.rs
@@ -230,6 +230,29 @@ macro_rules! decode_tlv_stream {
        } }
 }
 
+macro_rules! impl_writeable_msg {
+       ($st:ident, {$($field:ident),* $(,)*}, {$(($type: expr, $tlvfield: ident, $fieldty: tt)),* $(,)*}) => {
+               impl ::util::ser::Writeable for $st {
+                       fn write<W: ::util::ser::Writer>(&self, w: &mut W) -> Result<(), $crate::io::Error> {
+                               $( self.$field.write(w)?; )*
+                               encode_tlv_stream!(w, {$(($type, self.$tlvfield, $fieldty)),*});
+                               Ok(())
+                       }
+               }
+               impl ::util::ser::Readable for $st {
+                       fn read<R: $crate::io::Read>(r: &mut R) -> Result<Self, ::ln::msgs::DecodeError> {
+                               $(let $field = ::util::ser::Readable::read(r)?;)*
+                               $(init_tlv_field_var!($tlvfield, $fieldty);)*
+                               decode_tlv_stream!(r, {$(($type, $tlvfield, $fieldty)),*});
+                               Ok(Self {
+                                       $($field),*,
+                                       $($tlvfield),*
+                               })
+                       }
+               }
+       }
+}
+
 macro_rules! impl_writeable {
        ($st:ident, {$($field:ident),*}) => {
                impl ::util::ser::Writeable for $st {