From a3cec7b8a50939cc268b3e5a7b64a803c0351404 Mon Sep 17 00:00:00 2001 From: meryacine Date: Thu, 16 Jun 2022 00:41:50 +0200 Subject: [PATCH 1/8] Exposing minimal macro set Some structs had to become public so the macros can be used when exposed This change doesn't add docs and works around the missing docs by adding `msgs_macros` feature --- lightning/Cargo.toml | 2 ++ lightning/src/lib.rs | 2 +- lightning/src/util/mod.rs | 5 +++++ lightning/src/util/ser.rs | 4 ++-- lightning/src/util/ser_macros.rs | 4 ++++ 5 files changed, 14 insertions(+), 3 deletions(-) diff --git a/lightning/Cargo.toml b/lightning/Cargo.toml index b3e2af8c88e..87b3b8d7c71 100644 --- a/lightning/Cargo.toml +++ b/lightning/Cargo.toml @@ -17,6 +17,8 @@ rustdoc-args = ["--cfg", "docsrs"] [features] # Internal test utilities exposed to other repo crates _test_utils = ["hex", "regex", "bitcoin/bitcoinconsensus"] +msgs_macros = [] + # Unlog messages superior at targeted level. max_level_off = [] max_level_error = [] diff --git a/lightning/src/lib.rs b/lightning/src/lib.rs index ba6d6bc7191..9fff817f5a7 100644 --- a/lightning/src/lib.rs +++ b/lightning/src/lib.rs @@ -37,7 +37,7 @@ //! * `max_level_debug` //! * `max_level_trace` -#![cfg_attr(not(any(test, fuzzing, feature = "_test_utils")), deny(missing_docs))] +#![cfg_attr(not(any(test, fuzzing, feature = "_test_utils", feature = "msgs_macros")), deny(missing_docs))] #![cfg_attr(not(any(test, fuzzing, feature = "_test_utils")), forbid(unsafe_code))] #![deny(broken_intra_doc_links)] diff --git a/lightning/src/util/mod.rs b/lightning/src/util/mod.rs index c6181ab269a..d1d3fb42dd1 100644 --- a/lightning/src/util/mod.rs +++ b/lightning/src/util/mod.rs @@ -12,6 +12,11 @@ #[macro_use] pub(crate) mod fuzz_wrappers; +#[cfg(feature = "msgs_macros")] +#[macro_use] +pub mod ser_macros; + +#[cfg(not(feature = "msgs_macros"))] #[macro_use] pub(crate) mod ser_macros; diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 428adbc5e66..d74eb3f2841 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -91,7 +91,7 @@ impl Writer for LengthCalculatingWriter { /// Essentially std::io::Take but a bit simpler and with a method to walk the underlying stream /// forward to ensure we always consume exactly the fixed length specified. -pub(crate) struct FixedLengthReader { +pub struct FixedLengthReader { read: R, bytes_read: u64, total_bytes: u64, @@ -136,7 +136,7 @@ impl Read for FixedLengthReader { /// A Read which tracks whether any bytes have been read at all. This allows us to distinguish /// between "EOF reached before we started" and "EOF reached mid-read". -pub(crate) struct ReadTrackingReader { +pub struct ReadTrackingReader { read: R, pub have_read: bool, } diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 165d1f1edba..f0557aa2cae 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -28,6 +28,7 @@ macro_rules! encode_tlv { }; } +#[macro_export] macro_rules! encode_tlv_stream { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { { #[allow(unused_imports)] @@ -165,6 +166,7 @@ macro_rules! decode_tlv { }}; } +#[macro_export] macro_rules! decode_tlv_stream { ($stream: expr, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { { use ln::msgs::DecodeError; @@ -231,6 +233,7 @@ macro_rules! decode_tlv_stream { } } } +#[macro_export] macro_rules! impl_writeable_msg { ($st:ident, {$($field:ident),* $(,)*}, {$(($type: expr, $tlvfield: ident, $fieldty: tt)),* $(,)*}) => { impl ::util::ser::Writeable for $st { @@ -254,6 +257,7 @@ macro_rules! impl_writeable_msg { } } +#[macro_export] macro_rules! impl_writeable { ($st:ident, {$($field:ident),*}) => { impl ::util::ser::Writeable for $st { From 64de58efcc9d7465fd254e6051b25005550d7606 Mon Sep 17 00:00:00 2001 From: meryacine Date: Thu, 16 Jun 2022 10:06:50 +0200 Subject: [PATCH 2/8] Prefixing imports with `$crate::` in ser/deser macros so they work with any rust edition Previously, imports or usage of structs from different modules/submodules were using the `::module` syntax, which wouldn't work as expected if the macros are used in a rust edition > 2015 Mainly, the two macros `impl_writeable` and `impl_writeable_msg` are now exported and any other macro that was used inside of them. This also exposes some structs that the macros *might* construct for some macro arguments. This also adds some docs to the exposed structs which are missing docs + remove the `msgs_macros` feature which was used to workaround missing docs, though docs for the macros have not been added yet. --- lightning/Cargo.toml | 2 - lightning/src/lib.rs | 2 +- lightning/src/util/mod.rs | 5 -- lightning/src/util/ser.rs | 12 +++- lightning/src/util/ser_macros.rs | 95 +++++++++++++++++++------------- 5 files changed, 66 insertions(+), 50 deletions(-) diff --git a/lightning/Cargo.toml b/lightning/Cargo.toml index 87b3b8d7c71..b3e2af8c88e 100644 --- a/lightning/Cargo.toml +++ b/lightning/Cargo.toml @@ -17,8 +17,6 @@ rustdoc-args = ["--cfg", "docsrs"] [features] # Internal test utilities exposed to other repo crates _test_utils = ["hex", "regex", "bitcoin/bitcoinconsensus"] -msgs_macros = [] - # Unlog messages superior at targeted level. max_level_off = [] max_level_error = [] diff --git a/lightning/src/lib.rs b/lightning/src/lib.rs index 9fff817f5a7..ba6d6bc7191 100644 --- a/lightning/src/lib.rs +++ b/lightning/src/lib.rs @@ -37,7 +37,7 @@ //! * `max_level_debug` //! * `max_level_trace` -#![cfg_attr(not(any(test, fuzzing, feature = "_test_utils", feature = "msgs_macros")), deny(missing_docs))] +#![cfg_attr(not(any(test, fuzzing, feature = "_test_utils")), deny(missing_docs))] #![cfg_attr(not(any(test, fuzzing, feature = "_test_utils")), forbid(unsafe_code))] #![deny(broken_intra_doc_links)] diff --git a/lightning/src/util/mod.rs b/lightning/src/util/mod.rs index d1d3fb42dd1..301ba6afe72 100644 --- a/lightning/src/util/mod.rs +++ b/lightning/src/util/mod.rs @@ -12,14 +12,9 @@ #[macro_use] pub(crate) mod fuzz_wrappers; -#[cfg(feature = "msgs_macros")] #[macro_use] pub mod ser_macros; -#[cfg(not(feature = "msgs_macros"))] -#[macro_use] -pub(crate) mod ser_macros; - pub mod events; pub mod errors; pub mod ser; diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index d74eb3f2841..6b519b9eb1f 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -97,15 +97,18 @@ pub struct FixedLengthReader { total_bytes: u64, } impl FixedLengthReader { + /// Returns a new FixedLengthReader. pub fn new(read: R, total_bytes: u64) -> Self { Self { read, bytes_read: 0, total_bytes } } + /// Returns whether there are remaining bytes or not. #[inline] pub fn bytes_remain(&mut self) -> bool { self.bytes_read != self.total_bytes } + /// Consume the remaning bytes. #[inline] pub fn eat_remaining(&mut self) -> Result<(), DecodeError> { copy(self, &mut sink()).unwrap(); @@ -138,9 +141,11 @@ impl Read for FixedLengthReader { /// between "EOF reached before we started" and "EOF reached mid-read". pub struct ReadTrackingReader { read: R, + /// Tells whether we have read from this reader or not yet. pub have_read: bool, } impl ReadTrackingReader { + /// Returns a new ReadTrackingReader. pub fn new(read: R) -> Self { Self { read, have_read: false } } @@ -237,7 +242,8 @@ impl MaybeReadable for T { } } -pub(crate) struct OptionDeserWrapper(pub Option); +/// Wrapper to read a required (non-optional) TLV record. +pub struct OptionDeserWrapper(pub Option); impl Readable for OptionDeserWrapper { #[inline] fn read(reader: &mut R) -> Result { @@ -246,7 +252,7 @@ impl Readable for OptionDeserWrapper { } /// Wrapper to write each element of a Vec with no length prefix -pub(crate) struct VecWriteWrapper<'a, T: Writeable>(pub &'a Vec); +pub struct VecWriteWrapper<'a, T: Writeable>(pub &'a Vec); impl<'a, T: Writeable> Writeable for VecWriteWrapper<'a, T> { #[inline] fn write(&self, writer: &mut W) -> Result<(), io::Error> { @@ -258,7 +264,7 @@ 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(pub Vec); +pub struct VecReadWrapper(pub Vec); impl Readable for VecReadWrapper { #[inline] fn read(mut reader: &mut R) -> Result { diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index f0557aa2cae..4c342b971f1 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -7,6 +7,11 @@ // You may not use this file except in accordance with one or both of these // licenses. +//! Some macros that implement Readable/Writeable traits for lightning messages. +//! They also handle serialization and deserialization of TLVs. + +/// doc +#[macro_export] macro_rules! encode_tlv { ($stream: expr, $type: expr, $field: expr, (default_value, $default: expr)) => { encode_tlv!($stream, $type, $field, required) @@ -17,7 +22,7 @@ macro_rules! encode_tlv { $field.write($stream)?; }; ($stream: expr, $type: expr, $field: expr, vec_type) => { - encode_tlv!($stream, $type, ::util::ser::VecWriteWrapper(&$field), required); + encode_tlv!($stream, $type, ser::VecWriteWrapper(&$field), required); }; ($stream: expr, $optional_type: expr, $optional_field: expr, option) => { if let Some(ref field) = $optional_field { @@ -28,14 +33,15 @@ macro_rules! encode_tlv { }; } +/// doc #[macro_export] macro_rules! encode_tlv_stream { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { { #[allow(unused_imports)] use { - ln::msgs::DecodeError, - util::ser, - util::ser::BigSize, + $crate::ln::msgs::DecodeError, + $crate::util::ser, + $crate::util::ser::BigSize, }; $( @@ -67,7 +73,7 @@ macro_rules! get_varint_length_prefixed_tlv_length { $len.0 += field_len; }; ($len: expr, $type: expr, $field: expr, vec_type) => { - get_varint_length_prefixed_tlv_length!($len, $type, ::util::ser::VecWriteWrapper(&$field), required); + get_varint_length_prefixed_tlv_length!($len, $type, $crate::util::ser::VecWriteWrapper(&$field), required); }; ($len: expr, $optional_type: expr, $optional_field: expr, option) => { if let Some(ref field) = $optional_field { @@ -81,10 +87,10 @@ macro_rules! get_varint_length_prefixed_tlv_length { macro_rules! encode_varint_length_prefixed_tlv { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),*}) => { { - use util::ser::BigSize; + use $crate::util::ser::BigSize; let len = { #[allow(unused_mut)] - let mut len = ::util::ser::LengthCalculatingWriter(0); + let mut len = $crate::util::ser::LengthCalculatingWriter(0); $( get_varint_length_prefixed_tlv_length!(len, $type, $field, $fieldty); )* @@ -95,6 +101,8 @@ macro_rules! encode_varint_length_prefixed_tlv { } } } +/// doc +#[macro_export] macro_rules! check_tlv_order { ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{ #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true @@ -121,6 +129,8 @@ macro_rules! check_tlv_order { }}; } +/// doc +#[macro_export] macro_rules! check_missing_tlv { ($last_seen_type: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{ #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true @@ -147,6 +157,8 @@ macro_rules! check_missing_tlv { }}; } +/// doc +#[macro_export] macro_rules! decode_tlv { ($reader: expr, $field: ident, (default_value, $default: expr)) => {{ decode_tlv!($reader, $field, required) @@ -155,7 +167,7 @@ macro_rules! decode_tlv { $field = ser::Readable::read(&mut $reader)?; }}; ($reader: expr, $field: ident, vec_type) => {{ - let f: ::util::ser::VecReadWrapper<_> = ser::Readable::read(&mut $reader)?; + let f: $crate::util::ser::VecReadWrapper<_> = ser::Readable::read(&mut $reader)?; $field = Some(f.0); }}; ($reader: expr, $field: ident, option) => {{ @@ -166,14 +178,15 @@ macro_rules! decode_tlv { }}; } +/// doc #[macro_export] macro_rules! decode_tlv_stream { ($stream: expr, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { { - use ln::msgs::DecodeError; + use $crate::ln::msgs::DecodeError; let mut last_seen_type: Option = None; let mut stream_ref = $stream; 'tlv_read: loop { - use util::ser; + use $crate::util::ser; // First decode the type of this TLV: let typ: ser::BigSize = { @@ -233,19 +246,20 @@ macro_rules! decode_tlv_stream { } } } +/// doc #[macro_export] macro_rules! impl_writeable_msg { ($st:ident, {$($field:ident),* $(,)*}, {$(($type: expr, $tlvfield: ident, $fieldty: tt)),* $(,)*}) => { - impl ::util::ser::Writeable for $st { - fn write(&self, w: &mut W) -> Result<(), $crate::io::Error> { + impl $crate::util::ser::Writeable for $st { + fn write(&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: &mut R) -> Result { - $(let $field = ::util::ser::Readable::read(r)?;)* + impl $crate::util::ser::Readable for $st { + fn read(r: &mut R) -> Result { + $(let $field = $crate::util::ser::Readable::read(r)?;)* $(init_tlv_field_var!($tlvfield, $fieldty);)* decode_tlv_stream!(r, {$(($type, $tlvfield, $fieldty)),*}); Ok(Self { @@ -257,11 +271,12 @@ macro_rules! impl_writeable_msg { } } +/// doc #[macro_export] macro_rules! impl_writeable { ($st:ident, {$($field:ident),*}) => { - impl ::util::ser::Writeable for $st { - fn write(&self, w: &mut W) -> Result<(), $crate::io::Error> { + impl $crate::util::ser::Writeable for $st { + fn write(&self, w: &mut W) -> Result<(), $crate::io::Error> { $( self.$field.write(w)?; )* Ok(()) } @@ -274,10 +289,10 @@ macro_rules! impl_writeable { } } - impl ::util::ser::Readable for $st { - fn read(r: &mut R) -> Result { + impl $crate::util::ser::Readable for $st { + fn read(r: &mut R) -> Result { Ok(Self { - $($field: ::util::ser::Readable::read(r)?),* + $($field: $crate::util::ser::Readable::read(r)?),* }) } } @@ -335,10 +350,10 @@ 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 = ::util::ser::Readable::read($stream)?; - let mut rd = ::util::ser::FixedLengthReader::new($stream, tlv_len.0); + let tlv_len: $crate::util::ser::BigSize = $crate::util::ser::Readable::read($stream)?; + let mut rd = $crate::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)?; + rd.eat_remaining().map_err(|_| $crate::ln::msgs::DecodeError::ShortRead)?; } } } @@ -357,12 +372,14 @@ macro_rules! init_tlv_based_struct_field { }; } +/// doc +#[macro_export] macro_rules! init_tlv_field_var { ($field: ident, (default_value, $default: expr)) => { let mut $field = $default; }; ($field: ident, required) => { - let mut $field = ::util::ser::OptionDeserWrapper(None); + let mut $field = $crate::util::ser::OptionDeserWrapper(None); }; ($field: ident, vec_type) => { let mut $field = Some(Vec::new()); @@ -379,8 +396,8 @@ macro_rules! init_tlv_field_var { /// serialized. macro_rules! impl_writeable_tlv_based { ($st: ident, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { - impl ::util::ser::Writeable for $st { - fn write(&self, writer: &mut W) -> Result<(), $crate::io::Error> { + impl $crate::util::ser::Writeable for $st { + fn write(&self, writer: &mut W) -> Result<(), $crate::io::Error> { write_tlv_fields!(writer, { $(($type, self.$field, $fieldty)),* }); @@ -389,23 +406,23 @@ macro_rules! impl_writeable_tlv_based { #[inline] fn serialized_length(&self) -> usize { - use util::ser::BigSize; + use $crate::util::ser::BigSize; let len = { #[allow(unused_mut)] - let mut len = ::util::ser::LengthCalculatingWriter(0); + let mut len = $crate::util::ser::LengthCalculatingWriter(0); $( get_varint_length_prefixed_tlv_length!(len, $type, self.$field, $fieldty); )* len.0 }; - let mut len_calc = ::util::ser::LengthCalculatingWriter(0); + let mut len_calc = $crate::util::ser::LengthCalculatingWriter(0); BigSize(len as u64).write(&mut len_calc).expect("No in-memory data may fail to serialize"); len + len_calc.0 } } - impl ::util::ser::Readable for $st { - fn read(reader: &mut R) -> Result { + impl $crate::util::ser::Readable for $st { + fn read(reader: &mut R) -> Result { $( init_tlv_field_var!($field, $fieldty); )* @@ -427,8 +444,8 @@ macro_rules! _impl_writeable_tlv_based_enum_common { {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*} ),* $(,)*; $(($tuple_variant_id: expr, $tuple_variant_name: ident)),* $(,)*) => { - impl ::util::ser::Writeable for $st { - fn write(&self, writer: &mut W) -> Result<(), $crate::io::Error> { + impl $crate::util::ser::Writeable for $st { + fn write(&self, writer: &mut W) -> Result<(), $crate::io::Error> { match self { $($st::$variant_name { $(ref $field),* } => { let id: u8 = $variant_id; @@ -466,9 +483,9 @@ macro_rules! impl_writeable_tlv_based_enum_upgradable { $(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*; $($(($tuple_variant_id, $tuple_variant_name)),*)*); - impl ::util::ser::MaybeReadable for $st { - fn read(reader: &mut R) -> Result, ::ln::msgs::DecodeError> { - let id: u8 = ::util::ser::Readable::read(reader)?; + 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 @@ -519,9 +536,9 @@ macro_rules! impl_writeable_tlv_based_enum { $(($variant_id, $variant_name) => {$(($type, $field, $fieldty)),*}),*; $(($tuple_variant_id, $tuple_variant_name)),*); - impl ::util::ser::Readable for $st { - fn read(reader: &mut R) -> Result { - let id: u8 = ::util::ser::Readable::read(reader)?; + impl $crate::util::ser::Readable for $st { + fn read(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 From 5c8807db5fcf4e8653aac9a4373f8d6bbc0c3a1a Mon Sep 17 00:00:00 2001 From: meryacine Date: Fri, 17 Jun 2022 11:03:53 +0200 Subject: [PATCH 3/8] TLV types MUST be strictly increasing (< and not <=) See: https://github.com/lightning/bolts/blob/master/01-messaging.md#requirements --- lightning/src/util/ser_macros.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 4c342b971f1..3ac944635df 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -54,7 +54,8 @@ macro_rules! encode_tlv_stream { let mut last_seen: Option = None; $( if let Some(t) = last_seen { - debug_assert!(t <= $type); + #[allow(unused_comparisons)] // Note that $type may be 0 making the following comparison always true + (debug_assert!($type > t)) } last_seen = Some($type); )* From 319de94db5c43af802e79e8e0cc8f1219efae3c0 Mon Sep 17 00:00:00 2001 From: meryacine Date: Fri, 17 Jun 2022 11:06:30 +0200 Subject: [PATCH 4/8] Fix a typo --- lightning/src/ln/msgs.rs | 2 +- lightning/src/util/ser_macros.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 0e5b2e07e7a..d40de9f705f 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -790,7 +790,7 @@ pub struct CommitmentUpdate { /// Messages could have optional fields to use with extended features /// As we wish to serialize these differently from Options (Options get a tag byte, but -/// OptionalFeild simply gets Present if there are enough bytes to read into it), we have a +/// OptionalField simply gets Present if there are enough bytes to read into it), we have a /// separate enum type for them. /// (C-not exported) due to a free generic in T #[derive(Clone, Debug, PartialEq)] diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 3ac944635df..2cd3ae30380 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -194,7 +194,7 @@ macro_rules! decode_tlv_stream { // We track whether any bytes were read during the consensus_decode call to // determine whether we should break or return ShortRead if we get an // UnexpectedEof. This should in every case be largely cosmetic, but its nice to - // pass the TLV test vectors exactly, which requre this distinction. + // pass the TLV test vectors exactly, which require this distinction. let mut tracking_reader = ser::ReadTrackingReader::new(&mut stream_ref); match ser::Readable::read(&mut tracking_reader) { Err(DecodeError::ShortRead) => { From 402aeb4ae1c7522d05db75dd0d13f32a4f3016f5 Mon Sep 17 00:00:00 2001 From: meryacine Date: Fri, 17 Jun 2022 19:46:30 +0200 Subject: [PATCH 5/8] Add docs for exposed (de)serialization macros --- lightning/src/util/ser_macros.rs | 43 +++++++++++++++++++++++++------- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 2cd3ae30380..f11c5f9fb11 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -10,7 +10,7 @@ //! Some macros that implement Readable/Writeable traits for lightning messages. //! They also handle serialization and deserialization of TLVs. -/// doc +/// Implements serialization for a single TLV record. #[macro_export] macro_rules! encode_tlv { ($stream: expr, $type: expr, $field: expr, (default_value, $default: expr)) => { @@ -33,7 +33,7 @@ macro_rules! encode_tlv { }; } -/// doc +/// Implements the TLVs serialization part in a Writeable implementation of a struct. #[macro_export] macro_rules! encode_tlv_stream { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { { @@ -102,7 +102,7 @@ macro_rules! encode_varint_length_prefixed_tlv { } } } -/// doc +/// Asserts that the type of the last seen TLV is strictly less than the type of the TLV currently being processed. #[macro_export] macro_rules! check_tlv_order { ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{ @@ -130,7 +130,8 @@ macro_rules! check_tlv_order { }}; } -/// doc +/// Checks for required missing TLV records. If a required TLV is missing and has no default value, +/// an error is returned. #[macro_export] macro_rules! check_missing_tlv { ($last_seen_type: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{ @@ -158,7 +159,7 @@ macro_rules! check_missing_tlv { }}; } -/// doc +/// Implements deserialization for a single TLV record. #[macro_export] macro_rules! decode_tlv { ($reader: expr, $field: ident, (default_value, $default: expr)) => {{ @@ -179,7 +180,7 @@ macro_rules! decode_tlv { }}; } -/// doc +/// Implements the TLVs deserialization part in a Readable implementation of a struct. #[macro_export] macro_rules! decode_tlv_stream { ($stream: expr, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { { @@ -247,7 +248,30 @@ macro_rules! decode_tlv_stream { } } } -/// doc +/// Implements Readable/Writeable for a struct. This macro also handles (de)serialization of TLV records. +/// # Example +/// ``` +/// use lightning::*; +/// +/// #[derive(Debug)] +/// pub struct LightningMessage { +/// pub to: String, +/// pub note: String, +/// pub secret_number: u64, +/// // TLV records +/// pub nick_name: Option, +/// pub street_number: Option, +/// } +/// +/// impl_writeable_msg!(LightningMessage, { +/// to, +/// note, +/// secret_number, +/// }, { +/// (1, nick_name, option), +/// (3, street_number, option), +/// }); +/// ``` #[macro_export] macro_rules! impl_writeable_msg { ($st:ident, {$($field:ident),* $(,)*}, {$(($type: expr, $tlvfield: ident, $fieldty: tt)),* $(,)*}) => { @@ -272,7 +296,8 @@ macro_rules! impl_writeable_msg { } } -/// doc +/// Implements Readable/Writeable for a struct. Note that this macro doesn't handle messages +/// containing TLV records. If your message contains TLVs, use `impl_writeable_msg!` instead. #[macro_export] macro_rules! impl_writeable { ($st:ident, {$($field:ident),*}) => { @@ -373,7 +398,7 @@ macro_rules! init_tlv_based_struct_field { }; } -/// doc +/// Initializes the variables we are going to read the TLVs into. #[macro_export] macro_rules! init_tlv_field_var { ($field: ident, (default_value, $default: expr)) => { From 4bab16e81c174eaea3963cc5709b0b990c851057 Mon Sep 17 00:00:00 2001 From: meryacine Date: Sat, 18 Jun 2022 07:45:58 +0200 Subject: [PATCH 6/8] Fix some macro docs I wrote these comments before while misunderstanding what these macros do. Updated them. --- lightning/src/util/ser_macros.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index f11c5f9fb11..0497babbbd5 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -102,7 +102,7 @@ macro_rules! encode_varint_length_prefixed_tlv { } } } -/// Asserts that the type of the last seen TLV is strictly less than the type of the TLV currently being processed. +/// Errors if there are missing required TLV types between the last seen type and the type currently being processed. #[macro_export] macro_rules! check_tlv_order { ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{ @@ -130,8 +130,7 @@ macro_rules! check_tlv_order { }}; } -/// Checks for required missing TLV records. If a required TLV is missing and has no default value, -/// an error is returned. +/// Errors if there are missing required TLV types after the last seen type. #[macro_export] macro_rules! check_missing_tlv { ($last_seen_type: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{ @@ -217,7 +216,7 @@ macro_rules! decode_tlv_stream { }, _ => {}, } - // As we read types, make sure we hit every required type: + // As we read types, make sure we hit every required type between last_seen_type and typ: $({ check_tlv_order!(last_seen_type, typ, $type, $field, $fieldty); })* From 594c1013c636c4b18aad385e7f3cedd717667060 Mon Sep 17 00:00:00 2001 From: meryacine Date: Mon, 20 Jun 2022 09:39:31 +0200 Subject: [PATCH 7/8] Prefix usage of exported macros with `$crate` so users don't have to `use lightning::*;` when using them --- lightning/src/util/ser_macros.rs | 24 +++++++++++------------- 1 file changed, 11 insertions(+), 13 deletions(-) diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 0497babbbd5..198843ec763 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -14,7 +14,7 @@ #[macro_export] macro_rules! encode_tlv { ($stream: expr, $type: expr, $field: expr, (default_value, $default: expr)) => { - encode_tlv!($stream, $type, $field, required) + $crate::encode_tlv!($stream, $type, $field, required) }; ($stream: expr, $type: expr, $field: expr, required) => { BigSize($type).write($stream)?; @@ -22,7 +22,7 @@ macro_rules! encode_tlv { $field.write($stream)?; }; ($stream: expr, $type: expr, $field: expr, vec_type) => { - encode_tlv!($stream, $type, ser::VecWriteWrapper(&$field), required); + $crate::encode_tlv!($stream, $type, ser::VecWriteWrapper(&$field), required); }; ($stream: expr, $optional_type: expr, $optional_field: expr, option) => { if let Some(ref field) = $optional_field { @@ -45,7 +45,7 @@ macro_rules! encode_tlv_stream { }; $( - encode_tlv!($stream, $type, $field, $fieldty); + $crate::encode_tlv!($stream, $type, $field, $fieldty); )* #[allow(unused_mut, unused_variables, unused_assignments)] @@ -162,7 +162,7 @@ macro_rules! check_missing_tlv { #[macro_export] macro_rules! decode_tlv { ($reader: expr, $field: ident, (default_value, $default: expr)) => {{ - decode_tlv!($reader, $field, required) + $crate::decode_tlv!($reader, $field, required) }}; ($reader: expr, $field: ident, required) => {{ $field = ser::Readable::read(&mut $reader)?; @@ -218,7 +218,7 @@ macro_rules! decode_tlv_stream { } // As we read types, make sure we hit every required type between last_seen_type and typ: $({ - check_tlv_order!(last_seen_type, typ, $type, $field, $fieldty); + $crate::check_tlv_order!(last_seen_type, typ, $type, $field, $fieldty); })* last_seen_type = Some(typ.0); @@ -227,7 +227,7 @@ macro_rules! decode_tlv_stream { let mut s = ser::FixedLengthReader::new(&mut stream_ref, length.0); match typ.0 { $($type => { - decode_tlv!(s, $field, $fieldty); + $crate::decode_tlv!(s, $field, $fieldty); if s.bytes_remain() { s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes return Err(DecodeError::InvalidValue); @@ -242,7 +242,7 @@ macro_rules! decode_tlv_stream { } // Make sure we got to each required type after we've read every TLV: $({ - check_missing_tlv!(last_seen_type, $type, $field, $fieldty); + $crate::check_missing_tlv!(last_seen_type, $type, $field, $fieldty); })* } } } @@ -250,8 +250,6 @@ macro_rules! decode_tlv_stream { /// Implements Readable/Writeable for a struct. This macro also handles (de)serialization of TLV records. /// # Example /// ``` -/// use lightning::*; -/// /// #[derive(Debug)] /// pub struct LightningMessage { /// pub to: String, @@ -262,7 +260,7 @@ macro_rules! decode_tlv_stream { /// pub street_number: Option, /// } /// -/// impl_writeable_msg!(LightningMessage, { +/// lightning::impl_writeable_msg!(LightningMessage, { /// to, /// note, /// secret_number, @@ -277,15 +275,15 @@ macro_rules! impl_writeable_msg { impl $crate::util::ser::Writeable for $st { fn write(&self, w: &mut W) -> Result<(), $crate::io::Error> { $( self.$field.write(w)?; )* - encode_tlv_stream!(w, {$(($type, self.$tlvfield, $fieldty)),*}); + $crate::encode_tlv_stream!(w, {$(($type, self.$tlvfield, $fieldty)),*}); Ok(()) } } impl $crate::util::ser::Readable for $st { fn read(r: &mut R) -> Result { $(let $field = $crate::util::ser::Readable::read(r)?;)* - $(init_tlv_field_var!($tlvfield, $fieldty);)* - decode_tlv_stream!(r, {$(($type, $tlvfield, $fieldty)),*}); + $($crate::init_tlv_field_var!($tlvfield, $fieldty);)* + $crate::decode_tlv_stream!(r, {$(($type, $tlvfield, $fieldty)),*}); Ok(Self { $($field),*, $($tlvfield),* From 3cab63482ea326abee1e29131eb79707eae2a7a8 Mon Sep 17 00:00:00 2001 From: meryacine Date: Tue, 21 Jun 2022 06:35:53 +0200 Subject: [PATCH 8/8] These comparisons are always false 0 > u64 is redundant because a u64 can't be less than zero --- lightning/src/util/ser_macros.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 198843ec763..7fda9d20ed3 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -54,7 +54,7 @@ macro_rules! encode_tlv_stream { let mut last_seen: Option = None; $( if let Some(t) = last_seen { - #[allow(unused_comparisons)] // Note that $type may be 0 making the following comparison always true + #[allow(unused_comparisons)] // Note that $type may be 0 making the following comparison always false (debug_assert!($type > t)) } last_seen = Some($type); @@ -106,14 +106,14 @@ macro_rules! encode_varint_length_prefixed_tlv { #[macro_export] macro_rules! check_tlv_order { ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{ - #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true + #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always false let invalid_order = ($last_seen_type.is_none() || $last_seen_type.unwrap() < $type) && $typ.0 > $type; if invalid_order { $field = $default; } }}; ($last_seen_type: expr, $typ: expr, $type: expr, $field: ident, required) => {{ - #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true + #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always false let invalid_order = ($last_seen_type.is_none() || $last_seen_type.unwrap() < $type) && $typ.0 > $type; if invalid_order { return Err(DecodeError::InvalidValue); @@ -134,14 +134,14 @@ macro_rules! check_tlv_order { #[macro_export] macro_rules! check_missing_tlv { ($last_seen_type: expr, $type: expr, $field: ident, (default_value, $default: expr)) => {{ - #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true + #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always false let missing_req_type = $last_seen_type.is_none() || $last_seen_type.unwrap() < $type; if missing_req_type { $field = $default; } }}; ($last_seen_type: expr, $type: expr, $field: ident, required) => {{ - #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always true + #[allow(unused_comparisons)] // Note that $type may be 0 making the second comparison always false let missing_req_type = $last_seen_type.is_none() || $last_seen_type.unwrap() < $type; if missing_req_type { return Err(DecodeError::InvalidValue);