X-Git-Url: http://git.bitcoin.ninja/index.cgi?a=blobdiff_plain;f=lightning%2Fsrc%2Futil%2Fser_macros.rs;h=e1f4762ecbb8dde0b2600cd9b5fe5a85aa453fdc;hb=refs%2Fheads%2F2024-03-fix-upgradable-enum;hp=41185b179ffa815fee9d785fab9c18a2ab1711da;hpb=4a4163fcf4842d7531e8730a6327764277aadf99;p=rust-lightning diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 41185b17..e1f4762e 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -143,6 +143,9 @@ macro_rules! encode_tlv_stream { #[macro_export] macro_rules! _encode_tlv_stream { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}) => { { + $crate::_encode_tlv_stream!($stream, { $(($type, $field, $fieldty)),* }, &[]) + } }; + ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),* $(,)*}, $extra_tlvs: expr) => { { #[allow(unused_imports)] use $crate::{ ln::msgs::DecodeError, @@ -154,6 +157,10 @@ macro_rules! _encode_tlv_stream { $( $crate::_encode_tlv!($stream, $type, $field, $fieldty); )* + for tlv in $extra_tlvs { + let (typ, value): &(u64, Vec) = tlv; + $crate::_encode_tlv!($stream, *typ, *value, required_vec); + } #[allow(unused_mut, unused_variables, unused_assignments)] #[cfg(debug_assertions)] @@ -162,18 +169,8 @@ macro_rules! _encode_tlv_stream { $( $crate::_check_encoded_tlv_order!(last_seen, $type, $fieldty); )* - } - } }; - ($stream: expr, $tlvs: expr) => { { - for tlv in $tlvs { - let (typ, value): &&(u64, Vec) = tlv; - $crate::_encode_tlv!($stream, *typ, *value, required_vec); - } - - #[cfg(debug_assertions)] { - let mut last_seen: Option = None; - for tlv in $tlvs { - let (typ, _): &&(u64, Vec) = tlv; + for tlv in $extra_tlvs { + let (typ, _): &(u64, Vec) = tlv; $crate::_check_encoded_tlv_order!(last_seen, *typ, required_vec); } } @@ -234,9 +231,10 @@ macro_rules! _get_varint_length_prefixed_tlv_length { #[macro_export] macro_rules! _encode_varint_length_prefixed_tlv { ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),*}) => { { - _encode_varint_length_prefixed_tlv!($stream, {$(($type, $field, $fieldty)),*}, &[]) + $crate::_encode_varint_length_prefixed_tlv!($stream, {$(($type, $field, $fieldty)),*}, &[]) } }; ($stream: expr, {$(($type: expr, $field: expr, $fieldty: tt)),*}, $extra_tlvs: expr) => { { + extern crate alloc; use $crate::util::ser::BigSize; use alloc::vec::Vec; let len = { @@ -246,14 +244,13 @@ macro_rules! _encode_varint_length_prefixed_tlv { $crate::_get_varint_length_prefixed_tlv_length!(len, $type, $field, $fieldty); )* for tlv in $extra_tlvs { - let (typ, value): &&(u64, Vec) = tlv; + let (typ, value): &(u64, Vec) = tlv; $crate::_get_varint_length_prefixed_tlv_length!(len, *typ, *value, required_vec); } len.0 }; BigSize(len as u64).write($stream)?; - $crate::_encode_tlv_stream!($stream, { $(($type, $field, $fieldty)),* }); - $crate::_encode_tlv_stream!($stream, $extra_tlvs); + $crate::_encode_tlv_stream!($stream, { $(($type, $field, $fieldty)),* }, $extra_tlvs); } }; } @@ -357,25 +354,25 @@ macro_rules! _check_missing_tlv { #[doc(hidden)] #[macro_export] macro_rules! _decode_tlv { - ($reader: expr, $field: ident, (default_value, $default: expr)) => {{ - $crate::_decode_tlv!($reader, $field, required) + ($outer_reader: expr, $reader: expr, $field: ident, (default_value, $default: expr)) => {{ + $crate::_decode_tlv!($outer_reader, $reader, $field, required) }}; - ($reader: expr, $field: ident, (static_value, $value: expr)) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, (static_value, $value: expr)) => {{ }}; - ($reader: expr, $field: ident, required) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, required) => {{ $field = $crate::util::ser::Readable::read(&mut $reader)?; }}; - ($reader: expr, $field: ident, (required: $trait: ident $(, $read_arg: expr)?)) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, (required: $trait: ident $(, $read_arg: expr)?)) => {{ $field = $trait::read(&mut $reader $(, $read_arg)*)?; }}; - ($reader: expr, $field: ident, required_vec) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, required_vec) => {{ let f: $crate::util::ser::WithoutLength> = $crate::util::ser::Readable::read(&mut $reader)?; $field = f.0; }}; - ($reader: expr, $field: ident, option) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, option) => {{ $field = Some($crate::util::ser::Readable::read(&mut $reader)?); }}; - ($reader: expr, $field: ident, optional_vec) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, optional_vec) => {{ let f: $crate::util::ser::WithoutLength> = $crate::util::ser::Readable::read(&mut $reader)?; $field = Some(f.0); }}; @@ -383,32 +380,52 @@ macro_rules! _decode_tlv { // without backwards compat. We'll error if the field is missing, and return `Ok(None)` if the // field is present but we can no longer understand it. // Note that this variant can only be used within a `MaybeReadable` read. - ($reader: expr, $field: ident, upgradable_required) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, upgradable_required) => {{ $field = match $crate::util::ser::MaybeReadable::read(&mut $reader)? { Some(res) => res, - _ => return Ok(None) + None => { + // If we successfully read a value but we don't know how to parse it, we give up + // and immediately return `None`. However, we need to make sure we read the correct + // number of bytes for this TLV stream, which is implicitly the end of the stream. + // Thus, we consume everything left in the `$outer_reader` here, ensuring that if + // we're being read as a part of another TLV stream we don't spuriously fail to + // deserialize the outer object due to a TLV length mismatch. + $crate::io_extras::copy($outer_reader, &mut $crate::io_extras::sink()).unwrap(); + return Ok(None) + }, }; }}; // `upgradable_option` indicates we're reading an Option-al TLV that may have been upgraded // without backwards compat. $field will be None if the TLV is missing or if the field is present // but we can no longer understand it. - ($reader: expr, $field: ident, upgradable_option) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, upgradable_option) => {{ $field = $crate::util::ser::MaybeReadable::read(&mut $reader)?; + if $field.is_none() { + #[cfg(not(debug_assertions))] { + // In general, MaybeReadable implementations are required to consume all the bytes + // of the object even if they don't understand it, but due to a bug in the + // serialization format for `impl_writeable_tlv_based_enum_upgradable` we sometimes + // don't know how many bytes that is. In such cases, we'd like to spuriously allow + // TLV length mismatches, which we do here by calling `eat_remaining` so that the + // `s.bytes_remain()` check in `_decode_tlv_stream_range` doesn't fail. + $reader.eat_remaining()?; + } + } }}; - ($reader: expr, $field: ident, (option: $trait: ident $(, $read_arg: expr)?)) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, (option: $trait: ident $(, $read_arg: expr)?)) => {{ $field = Some($trait::read(&mut $reader $(, $read_arg)*)?); }}; - ($reader: expr, $field: ident, (option, encoding: ($fieldty: ty, $encoding: ident, $encoder:ty))) => {{ - $crate::_decode_tlv!($reader, $field, (option, encoding: ($fieldty, $encoding))); + ($outer_reader: expr, $reader: expr, $field: ident, (option, encoding: ($fieldty: ty, $encoding: ident, $encoder:ty))) => {{ + $crate::_decode_tlv!($outer_reader, $reader, $field, (option, encoding: ($fieldty, $encoding))); }}; - ($reader: expr, $field: ident, (option, encoding: ($fieldty: ty, $encoding: ident))) => {{ + ($outer_reader: expr, $reader: expr, $field: ident, (option, encoding: ($fieldty: ty, $encoding: ident))) => {{ $field = { let field: $encoding<$fieldty> = ser::Readable::read(&mut $reader)?; Some(field.0) }; }}; - ($reader: expr, $field: ident, (option, encoding: $fieldty: ty)) => {{ - $crate::_decode_tlv!($reader, $field, option); + ($outer_reader: expr, $reader: expr, $field: ident, (option, encoding: $fieldty: ty)) => {{ + $crate::_decode_tlv!($outer_reader, $reader, $field, option); }}; } @@ -542,7 +559,7 @@ macro_rules! _decode_tlv_stream_range { let mut s = ser::FixedLengthReader::new(&mut stream_ref, length.0); match typ.0 { $(_t if $crate::_decode_tlv_stream_match_check!(_t, $type, $fieldty) => { - $crate::_decode_tlv!(s, $field, $fieldty); + $crate::_decode_tlv!($stream, s, $field, $fieldty); if s.bytes_remain() { s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes return Err(DecodeError::InvalidValue); @@ -794,10 +811,13 @@ macro_rules! _init_tlv_field_var { /// Equivalent to running [`_init_tlv_field_var`] then [`read_tlv_fields`]. /// +/// If any unused values are read, their type MUST be specified or else `rustc` will read them as an +/// `i64`. +/// /// This is exported for use by other exported macros, do not use directly. #[doc(hidden)] #[macro_export] -macro_rules! _init_and_read_tlv_fields { +macro_rules! _init_and_read_len_prefixed_tlv_fields { ($reader: ident, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { $( $crate::_init_tlv_field_var!($field, $fieldty); @@ -809,6 +829,22 @@ macro_rules! _init_and_read_tlv_fields { } } +/// Equivalent to running [`_init_tlv_field_var`] then [`decode_tlv_stream`]. +/// +/// If any unused values are read, their type MUST be specified or else `rustc` will read them as an +/// `i64`. +macro_rules! _init_and_read_tlv_stream { + ($reader: ident, {$(($type: expr, $field: ident, $fieldty: tt)),* $(,)*}) => { + $( + $crate::_init_tlv_field_var!($field, $fieldty); + )* + + $crate::decode_tlv_stream!($reader, { + $(($type, $field, $fieldty)),* + }); + } +} + /// Implements [`Readable`]/[`Writeable`] for a struct storing it as a set of TLVs /// If `$fieldty` is `required`, then `$field` is a required field that is not an [`Option`] nor a [`Vec`]. /// If `$fieldty` is `(default_value, $default)`, then `$field` will be set to `$default` if not present. @@ -818,8 +854,7 @@ macro_rules! _init_and_read_tlv_fields { /// /// For example, /// ``` -/// # use lightning::{impl_writeable_tlv_based, _encode_varint_length_prefixed_tlv}; -/// # extern crate alloc; +/// # use lightning::impl_writeable_tlv_based; /// struct LightningMessage { /// tlv_integer: u32, /// tlv_default_integer: u32, @@ -867,7 +902,7 @@ macro_rules! impl_writeable_tlv_based { impl $crate::util::ser::Readable for $st { fn read(reader: &mut R) -> Result { - $crate::_init_and_read_tlv_fields!(reader, { + $crate::_init_and_read_len_prefixed_tlv_fields!(reader, { $(($type, $field, $fieldty)),* }); Ok(Self { @@ -902,7 +937,7 @@ macro_rules! tlv_stream { #[cfg_attr(test, derive(PartialEq))] #[derive(Debug)] - pub(super) struct $nameref<'a> { + pub(crate) struct $nameref<'a> { $( pub(super) $field: Option, )* @@ -1019,7 +1054,7 @@ macro_rules! impl_writeable_tlv_based_enum { // 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 = || { - $crate::_init_and_read_tlv_fields!(reader, { + $crate::_init_and_read_len_prefixed_tlv_fields!(reader, { $(($type, $field, $fieldty)),* }); Ok($st::$variant_name { @@ -1050,6 +1085,10 @@ 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`). +/// /// [`MaybeReadable`]: crate::util::ser::MaybeReadable /// [`Writeable`]: crate::util::ser::Writeable /// [`DecodeError::UnknownRequiredFeature`]: crate::ln::msgs::DecodeError::UnknownRequiredFeature @@ -1073,7 +1112,7 @@ macro_rules! impl_writeable_tlv_based_enum_upgradable { // 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 = || { - $crate::_init_and_read_tlv_fields!(reader, { + $crate::_init_and_read_len_prefixed_tlv_fields!(reader, { $(($type, $field, $fieldty)),* }); Ok(Some($st::$variant_name { @@ -1087,7 +1126,14 @@ macro_rules! impl_writeable_tlv_based_enum_upgradable { $($($tuple_variant_id => { Ok(Some($st::$tuple_variant_name(Readable::read(reader)?))) }),*)* - _ if id % 2 == 1 => Ok(None), + _ 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)?; + 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), } } @@ -1101,6 +1147,7 @@ mod tests { use crate::prelude::*; use crate::ln::msgs::DecodeError; use crate::util::ser::{Writeable, HighZeroBytesDroppedBigSize, VecWriter}; + use bitcoin::hashes::hex::FromHex; use bitcoin::secp256k1::PublicKey; // The BOLT TLV test cases don't include any tests which use our "required-value" logic since @@ -1118,7 +1165,7 @@ mod tests { #[test] fn tlv_v_short_read() { // We only expect a u32 for type 3 (which we are given), but the L says its 8 bytes. - if let Err(DecodeError::ShortRead) = tlv_reader(&::hex::decode( + if let Err(DecodeError::ShortRead) = tlv_reader(&>::from_hex( concat!("0100", "0208deadbeef1badbeef", "0308deadbeef") ).unwrap()[..]) { } else { panic!(); } @@ -1126,12 +1173,12 @@ mod tests { #[test] fn tlv_types_out_of_order() { - if let Err(DecodeError::InvalidValue) = tlv_reader(&::hex::decode( + if let Err(DecodeError::InvalidValue) = tlv_reader(&>::from_hex( concat!("0100", "0304deadbeef", "0208deadbeef1badbeef") ).unwrap()[..]) { } else { panic!(); } // ...even if its some field we don't understand - if let Err(DecodeError::InvalidValue) = tlv_reader(&::hex::decode( + if let Err(DecodeError::InvalidValue) = tlv_reader(&>::from_hex( concat!("0208deadbeef1badbeef", "0100", "0304deadbeef") ).unwrap()[..]) { } else { panic!(); } @@ -1140,17 +1187,17 @@ mod tests { #[test] fn tlv_req_type_missing_or_extra() { // It's also bad if they included even fields we don't understand - if let Err(DecodeError::UnknownRequiredFeature) = tlv_reader(&::hex::decode( + if let Err(DecodeError::UnknownRequiredFeature) = tlv_reader(&>::from_hex( concat!("0100", "0208deadbeef1badbeef", "0304deadbeef", "0600") ).unwrap()[..]) { } else { panic!(); } // ... or if they're missing fields we need - if let Err(DecodeError::InvalidValue) = tlv_reader(&::hex::decode( + if let Err(DecodeError::InvalidValue) = tlv_reader(&>::from_hex( concat!("0100", "0208deadbeef1badbeef") ).unwrap()[..]) { } else { panic!(); } // ... even if that field is even - if let Err(DecodeError::InvalidValue) = tlv_reader(&::hex::decode( + if let Err(DecodeError::InvalidValue) = tlv_reader(&>::from_hex( concat!("0304deadbeef", "0500") ).unwrap()[..]) { } else { panic!(); } @@ -1158,11 +1205,11 @@ mod tests { #[test] fn tlv_simple_good_cases() { - assert_eq!(tlv_reader(&::hex::decode( + assert_eq!(tlv_reader(&>::from_hex( concat!("0208deadbeef1badbeef", "03041bad1dea") ).unwrap()[..]).unwrap(), (0xdeadbeef1badbeef, 0x1bad1dea, None)); - assert_eq!(tlv_reader(&::hex::decode( + assert_eq!(tlv_reader(&>::from_hex( concat!("0208deadbeef1badbeef", "03041bad1dea", "040401020304") ).unwrap()[..]).unwrap(), (0xdeadbeef1badbeef, 0x1bad1dea, Some(0x01020304))); @@ -1186,12 +1233,12 @@ mod tests { #[test] fn upgradable_tlv_simple_good_cases() { - assert_eq!(upgradable_tlv_reader(&::hex::decode( + assert_eq!(upgradable_tlv_reader(&>::from_hex( concat!("0204deadbeef", "03041bad1dea", "0404deadbeef") ).unwrap()[..]).unwrap(), Some(TestUpgradable { a: 0xdeadbeef, b: 0x1bad1dea, c: Some(0xdeadbeef) })); - assert_eq!(upgradable_tlv_reader(&::hex::decode( + assert_eq!(upgradable_tlv_reader(&>::from_hex( concat!("0204deadbeef", "03041bad1dea") ).unwrap()[..]).unwrap(), Some(TestUpgradable { a: 0xdeadbeef, b: 0x1bad1dea, c: None})); @@ -1199,11 +1246,11 @@ mod tests { #[test] fn missing_required_upgradable() { - if let Err(DecodeError::InvalidValue) = upgradable_tlv_reader(&::hex::decode( + if let Err(DecodeError::InvalidValue) = upgradable_tlv_reader(&>::from_hex( concat!("0100", "0204deadbeef") ).unwrap()[..]) { } else { panic!(); } - if let Err(DecodeError::InvalidValue) = upgradable_tlv_reader(&::hex::decode( + if let Err(DecodeError::InvalidValue) = upgradable_tlv_reader(&>::from_hex( concat!("0100", "03041bad1dea") ).unwrap()[..]) { } else { panic!(); } @@ -1224,7 +1271,7 @@ mod tests { fn bolt_tlv_bogus_stream() { macro_rules! do_test { ($stream: expr, $reason: ident) => { - if let Err(DecodeError::$reason) = tlv_reader_n1(&::hex::decode($stream).unwrap()[..]) { + if let Err(DecodeError::$reason) = tlv_reader_n1(&>::from_hex($stream).unwrap()[..]) { } else { panic!(); } } } @@ -1249,7 +1296,7 @@ mod tests { fn bolt_tlv_bogus_n1_stream() { macro_rules! do_test { ($stream: expr, $reason: ident) => { - if let Err(DecodeError::$reason) = tlv_reader_n1(&::hex::decode($stream).unwrap()[..]) { + if let Err(DecodeError::$reason) = tlv_reader_n1(&>::from_hex($stream).unwrap()[..]) { } else { panic!(); } } } @@ -1289,7 +1336,7 @@ mod tests { fn bolt_tlv_valid_n1_stream() { macro_rules! do_test { ($stream: expr, $tlv1: expr, $tlv2: expr, $tlv3: expr, $tlv4: expr) => { - if let Ok((tlv1, tlv2, tlv3, tlv4)) = tlv_reader_n1(&::hex::decode($stream).unwrap()[..]) { + if let Ok((tlv1, tlv2, tlv3, tlv4)) = tlv_reader_n1(&>::from_hex($stream).unwrap()[..]) { assert_eq!(tlv1.map(|v| v.0), $tlv1); assert_eq!(tlv2, $tlv2); assert_eq!(tlv3, $tlv3); @@ -1318,7 +1365,7 @@ mod tests { do_test!(concat!("02", "08", "0000000000000226"), None, Some((0 << 30) | (0 << 5) | (550 << 0)), None, None); do_test!(concat!("03", "31", "023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb00000000000000010000000000000002"), None, None, Some(( - PublicKey::from_slice(&::hex::decode("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]).unwrap(), 1, 2)), + PublicKey::from_slice(&>::from_hex("023da092f6980e58d2c037173180e9a465476026ee50f96695963e8efe436f54eb").unwrap()[..]).unwrap(), 1, 2)), None); do_test!(concat!("fd00fe", "02", "0226"), None, None, None, Some(550)); } @@ -1328,27 +1375,27 @@ mod tests { stream.0.clear(); _encode_varint_length_prefixed_tlv!(&mut stream, {(1, 1u8, required), (42, None::, option)}); - assert_eq!(stream.0, ::hex::decode("03010101").unwrap()); + assert_eq!(stream.0, >::from_hex("03010101").unwrap()); stream.0.clear(); _encode_varint_length_prefixed_tlv!(&mut stream, {(1, Some(1u8), option)}); - assert_eq!(stream.0, ::hex::decode("03010101").unwrap()); + assert_eq!(stream.0, >::from_hex("03010101").unwrap()); stream.0.clear(); _encode_varint_length_prefixed_tlv!(&mut stream, {(4, 0xabcdu16, required), (42, None::, option)}); - assert_eq!(stream.0, ::hex::decode("040402abcd").unwrap()); + assert_eq!(stream.0, >::from_hex("040402abcd").unwrap()); stream.0.clear(); _encode_varint_length_prefixed_tlv!(&mut stream, {(42, None::, option), (0xff, 0xabcdu16, required)}); - assert_eq!(stream.0, ::hex::decode("06fd00ff02abcd").unwrap()); + assert_eq!(stream.0, >::from_hex("06fd00ff02abcd").unwrap()); stream.0.clear(); _encode_varint_length_prefixed_tlv!(&mut stream, {(0, 1u64, required), (42, None::, option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)}); - assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap()); + assert_eq!(stream.0, >::from_hex("0e00080000000000000001fd00ff00").unwrap()); stream.0.clear(); _encode_varint_length_prefixed_tlv!(&mut stream, {(0, Some(1u64), option), (0xff, HighZeroBytesDroppedBigSize(0u64), required)}); - assert_eq!(stream.0, ::hex::decode("0e00080000000000000001fd00ff00").unwrap()); + assert_eq!(stream.0, >::from_hex("0e00080000000000000001fd00ff00").unwrap()); Ok(()) }