From f5fc13bd493ea34a3d6a50b892a6ed7dc94671aa Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Thu, 6 Mar 2025 11:27:13 -0500 Subject: [PATCH 1/8] Refactor LengthRead to also limit number of bytes read In general, the codebase currently tends to use the Readable trait with two different semantics -- some objects know when to stop reading because they have a length prefix and some automatically read to the end of the reader. This differing usage can lead to bugs. For objects that read-to-end, it would be much safer if the compiler enforced that they could only be read from a length-limiting reader. We already have a LengthRead trait that requires the underlying reader to provide the length, which is similar to what we want. So rather than adding an additional trait, here we rename LengthRead and update its semantics to require a length limiting reader. Upcoming commits will switch read-to-end structs that are currently read with Readable to use LengthReadable with LengthLimitedRead instead. --- lightning/src/crypto/streams.rs | 10 ++++---- lightning/src/ln/msgs.rs | 4 +-- lightning/src/onion_message/packet.rs | 4 +-- lightning/src/util/ser.rs | 36 ++++++++++++++++----------- 4 files changed, 31 insertions(+), 23 deletions(-) diff --git a/lightning/src/crypto/streams.rs b/lightning/src/crypto/streams.rs index 9c7df314685..35f13c1f0e1 100644 --- a/lightning/src/crypto/streams.rs +++ b/lightning/src/crypto/streams.rs @@ -4,7 +4,7 @@ use crate::crypto::chacha20poly1305rfc::ChaCha20Poly1305RFC; use crate::io::{self, Read, Write}; use crate::ln::msgs::DecodeError; use crate::util::ser::{ - FixedLengthReader, LengthRead, LengthReadableArgs, Readable, Writeable, Writer, + FixedLengthReader, LengthLimitedRead, LengthReadableArgs, Readable, Writeable, Writer, }; pub(crate) struct ChaChaReader<'a, R: io::Read> { @@ -56,10 +56,10 @@ pub(crate) struct ChaChaPolyReadAdapter { } impl LengthReadableArgs<[u8; 32]> for ChaChaPolyReadAdapter { - // Simultaneously read and decrypt an object from a LengthRead, storing it in Self::readable. - // LengthRead must be used instead of std::io::Read because we need the total length to separate - // out the tag at the end. - fn read(r: &mut R, secret: [u8; 32]) -> Result { + // Simultaneously read and decrypt an object from a LengthLimitedRead storing it in + // Self::readable. LengthLimitedRead must be used instead of std::io::Read because we need the + // total length to separate out the tag at the end. + fn read(r: &mut R, secret: [u8; 32]) -> Result { if r.total_bytes() < 16 { return Err(DecodeError::InvalidValue); } diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 64181ffa929..6609c37cd87 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -59,7 +59,7 @@ use crate::io_extras::read_to_end; use crate::crypto::streams::ChaChaPolyReadAdapter; use crate::util::logger; -use crate::util::ser::{BigSize, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname, LengthRead, LengthReadable, LengthReadableArgs, Readable, ReadableArgs, TransactionU16LenLimited, WithoutLength, Writeable, Writer}; +use crate::util::ser::{BigSize, FixedLengthReader, HighZeroBytesDroppedBigSize, Hostname, LengthLimitedRead, LengthReadable, LengthReadableArgs, Readable, ReadableArgs, TransactionU16LenLimited, WithoutLength, Writeable, Writer}; use crate::util::base32; use crate::routing::gossip::{NodeAlias, NodeId}; @@ -2323,7 +2323,7 @@ impl Writeable for TrampolineOnionPacket { } impl LengthReadable for TrampolineOnionPacket { - fn read_from_fixed_length_buffer(r: &mut R) -> Result { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let version = Readable::read(r)?; let public_key = Readable::read(r)?; diff --git a/lightning/src/onion_message/packet.rs b/lightning/src/onion_message/packet.rs index 8cc95461b73..66f03e6d40f 100644 --- a/lightning/src/onion_message/packet.rs +++ b/lightning/src/onion_message/packet.rs @@ -23,7 +23,7 @@ use crate::ln::msgs::DecodeError; use crate::ln::onion_utils; use crate::util::logger::Logger; use crate::util::ser::{ - BigSize, FixedLengthReader, LengthRead, LengthReadable, LengthReadableArgs, Readable, + BigSize, FixedLengthReader, LengthLimitedRead, LengthReadable, LengthReadableArgs, Readable, ReadableArgs, Writeable, Writer, }; @@ -83,7 +83,7 @@ impl Writeable for Packet { } impl LengthReadable for Packet { - fn read_from_fixed_length_buffer(r: &mut R) -> Result { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { const READ_BUFFER_SIZE: usize = 4096; let version = Readable::read(r)?; diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 21997c09c1a..f53865a6a5a 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -226,7 +226,7 @@ impl<'a, R: Read> Read for FixedLengthReader<'a, R> { } } -impl<'a, R: Read> LengthRead for FixedLengthReader<'a, R> { +impl<'a, R: Read> LengthLimitedRead for FixedLengthReader<'a, R> { #[inline] fn total_bytes(&self) -> u64 { self.total_bytes @@ -344,25 +344,27 @@ where fn read(reader: &mut R, params: P) -> Result; } -/// A [`io::Read`] that also provides the total bytes available to be read. -pub trait LengthRead: Read { +/// A [`io::Read`] that limits the amount of bytes that can be read. Implementations should ensure +/// that the object being read will only consume a fixed number of bytes from the underlying +/// [`io::Read`], see [`FixedLengthReader`] for an example. +pub trait LengthLimitedRead: Read { /// The total number of bytes available to be read. fn total_bytes(&self) -> u64; } -/// A trait that various higher-level LDK types implement allowing them to be read in -/// from a Read given some additional set of arguments which is required to deserialize, requiring -/// the implementer to provide the total length of the read. +/// Similar to [`LengthReadable`]. Useful when an additional set of arguments is required to +/// deserialize. pub(crate) trait LengthReadableArgs

where Self: Sized, { - /// Reads a `Self` in from the given [`LengthRead`]. - fn read(reader: &mut R, params: P) -> Result; + /// Reads a `Self` in from the given [`LengthLimitedRead`]. + fn read(reader: &mut R, params: P) -> Result; } -/// A trait that allows the implementer to be read in from a [`LengthRead`], requiring -/// the reader to provide the total length of the read. +/// A trait that allows the implementer to be read in from a [`LengthLimitedRead`], requiring the +/// reader to limit the number of total bytes read from its underlying [`Read`]. Useful for structs +/// that will always consume the entire provided [`Read`] when deserializing. /// /// Any type that implements [`Readable`] also automatically has a [`LengthReadable`] /// implementation, but some types, most notably onion packets, only implement [`LengthReadable`]. @@ -370,13 +372,17 @@ pub trait LengthReadable where Self: Sized, { - /// Reads a `Self` in from the given [`LengthRead`]. - fn read_from_fixed_length_buffer(reader: &mut R) -> Result; + /// Reads a `Self` in from the given [`LengthLimitedRead`]. + fn read_from_fixed_length_buffer( + reader: &mut R, + ) -> Result; } impl LengthReadable for T { #[inline] - fn read_from_fixed_length_buffer(reader: &mut R) -> Result { + fn read_from_fixed_length_buffer( + reader: &mut R, + ) -> Result { Readable::read(reader) } } @@ -405,7 +411,9 @@ impl MaybeReadable for T { pub struct RequiredWrapper(pub Option); impl LengthReadable for RequiredWrapper { #[inline] - fn read_from_fixed_length_buffer(reader: &mut R) -> Result { + fn read_from_fixed_length_buffer( + reader: &mut R, + ) -> Result { Ok(Self(Some(LengthReadable::read_from_fixed_length_buffer(reader)?))) } } From 16fdd336e56d3f88f80dd92579115c3f9fa1502c Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 7 Mar 2025 11:25:43 -0500 Subject: [PATCH 2/8] Implement LengthLimitedRead for &[u8] We want to slowly phase out the use of Cursor in the codebase, which means we want to be able to call LengthLimitedRead::read(&mut &slice_of_bytes[..]) instead of wrapping the slice in a Cursor. However, this breaks the current LengthLimitedRead::total_bytes method because the underlying Read implementation for slice advances the slice as it is read, so slice.len() can't be used to get the total bytes after any part of the struct is read. Therefore here we also switch the ::total_bytes method to ::remaining_bytes, which seems like a more sensible method anyway since the reader is being consumed. --- lightning/src/crypto/streams.rs | 4 ++-- lightning/src/ln/msgs.rs | 3 ++- lightning/src/onion_message/packet.rs | 2 +- lightning/src/util/ser.rs | 16 ++++++++++++---- 4 files changed, 17 insertions(+), 8 deletions(-) diff --git a/lightning/src/crypto/streams.rs b/lightning/src/crypto/streams.rs index 35f13c1f0e1..82680131f1d 100644 --- a/lightning/src/crypto/streams.rs +++ b/lightning/src/crypto/streams.rs @@ -60,12 +60,12 @@ impl LengthReadableArgs<[u8; 32]> for ChaChaPolyReadAdapter { // Self::readable. LengthLimitedRead must be used instead of std::io::Read because we need the // total length to separate out the tag at the end. fn read(r: &mut R, secret: [u8; 32]) -> Result { - if r.total_bytes() < 16 { + if r.remaining_bytes() < 16 { return Err(DecodeError::InvalidValue); } let mut chacha = ChaCha20Poly1305RFC::new(&secret, &[0; 12], &[]); - let decrypted_len = r.total_bytes() - 16; + let decrypted_len = r.remaining_bytes() - 16; let s = FixedLengthReader::new(r, decrypted_len); let mut chacha_stream = ChaChaPolyReader { chacha: &mut chacha, read: s }; let readable: T = Readable::read(&mut chacha_stream)?; diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 6609c37cd87..43d4b1eb9ee 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -2324,10 +2324,11 @@ impl Writeable for TrampolineOnionPacket { impl LengthReadable for TrampolineOnionPacket { fn read_from_fixed_length_buffer(r: &mut R) -> Result { + let hop_data_len = r.remaining_bytes().saturating_sub(66); // 1 (version) + 33 (pubkey) + 32 (HMAC) = 66 + let version = Readable::read(r)?; let public_key = Readable::read(r)?; - let hop_data_len = r.total_bytes().saturating_sub(66); // 1 (version) + 33 (pubkey) + 32 (HMAC) = 66 let mut rd = FixedLengthReader::new(r, hop_data_len); let hop_data = WithoutLength::>::read(&mut rd)?.0; diff --git a/lightning/src/onion_message/packet.rs b/lightning/src/onion_message/packet.rs index 66f03e6d40f..33061f344fa 100644 --- a/lightning/src/onion_message/packet.rs +++ b/lightning/src/onion_message/packet.rs @@ -85,12 +85,12 @@ impl Writeable for Packet { impl LengthReadable for Packet { fn read_from_fixed_length_buffer(r: &mut R) -> Result { const READ_BUFFER_SIZE: usize = 4096; + let hop_data_len = r.remaining_bytes().saturating_sub(66) as usize; // 1 (version) + 33 (pubkey) + 32 (HMAC) = 66 let version = Readable::read(r)?; let public_key = Readable::read(r)?; let mut hop_data = Vec::new(); - let hop_data_len = r.total_bytes().saturating_sub(66) as usize; // 1 (version) + 33 (pubkey) + 32 (HMAC) = 66 let mut read_idx = 0; while read_idx < hop_data_len { let mut read_buffer = [0; READ_BUFFER_SIZE]; diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index f53865a6a5a..bea4a3f28ad 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -228,8 +228,8 @@ impl<'a, R: Read> Read for FixedLengthReader<'a, R> { impl<'a, R: Read> LengthLimitedRead for FixedLengthReader<'a, R> { #[inline] - fn total_bytes(&self) -> u64 { - self.total_bytes + fn remaining_bytes(&self) -> u64 { + self.total_bytes.saturating_sub(self.bytes_read) } } @@ -348,8 +348,16 @@ where /// that the object being read will only consume a fixed number of bytes from the underlying /// [`io::Read`], see [`FixedLengthReader`] for an example. pub trait LengthLimitedRead: Read { - /// The total number of bytes available to be read. - fn total_bytes(&self) -> u64; + /// The number of bytes remaining to be read. + fn remaining_bytes(&self) -> u64; +} + +impl LengthLimitedRead for &[u8] { + fn remaining_bytes(&self) -> u64 { + // The underlying `Read` implementation for slice updates the slice to point to the yet unread + // part. + self.len() as u64 + } } /// Similar to [`LengthReadable`]. Useful when an additional set of arguments is required to From e9399d743636eccd4ff50a52ae949bf146d9544a Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Mon, 3 Mar 2025 16:58:28 -0500 Subject: [PATCH 3/8] Require length limited reader in impl_writeable_msg See prior two commits. When deserializing objects via this macro, there is no length prefix so the deser code will read the provided reader until it runs out of bytes. Readable is not an appropriate trait for this situation because it should only be used for structs that are prefixed with a length and know when to stop reading. LengthReadable instead requires that the caller supply only the bytes that are reserved for this struct. --- fuzz/src/chanmon_consistency.rs | 4 +- fuzz/src/msg_targets/utils.rs | 9 +-- lightning/src/ln/channelmanager.rs | 6 +- lightning/src/ln/msgs.rs | 25 ++++--- lightning/src/ln/peer_handler.rs | 6 +- lightning/src/ln/wire.rs | 110 +++++++++++++---------------- lightning/src/util/ser.rs | 4 +- lightning/src/util/ser_macros.rs | 18 ++--- 8 files changed, 88 insertions(+), 94 deletions(-) diff --git a/fuzz/src/chanmon_consistency.rs b/fuzz/src/chanmon_consistency.rs index 8b8f09fdca3..1869dfb0d1c 100644 --- a/fuzz/src/chanmon_consistency.rs +++ b/fuzz/src/chanmon_consistency.rs @@ -67,7 +67,7 @@ use lightning::types::payment::{PaymentHash, PaymentPreimage, PaymentSecret}; use lightning::util::config::UserConfig; use lightning::util::hash_tables::*; use lightning::util::logger::Logger; -use lightning::util::ser::{Readable, ReadableArgs, Writeable, Writer}; +use lightning::util::ser::{LengthReadable, ReadableArgs, Writeable, Writer}; use lightning::util::test_channel_signer::{EnforcementState, TestChannelSigner}; use lightning_invoice::RawBolt11Invoice; @@ -1103,7 +1103,7 @@ pub fn do_test(data: &[u8], underlying_out: Out, anchors: bool) { // update_fail_htlc as we do when we reject a payment. let mut msg_ser = update_add.encode(); msg_ser[1000] ^= 0xff; - let new_msg = UpdateAddHTLC::read(&mut Cursor::new(&msg_ser)).unwrap(); + let new_msg = UpdateAddHTLC::read_from_fixed_length_buffer(&mut &msg_ser[..]).unwrap(); dest.handle_update_add_htlc(nodes[$node].get_our_node_id(), &new_msg); } } diff --git a/fuzz/src/msg_targets/utils.rs b/fuzz/src/msg_targets/utils.rs index 8bf776193e6..b2bc220afb0 100644 --- a/fuzz/src/msg_targets/utils.rs +++ b/fuzz/src/msg_targets/utils.rs @@ -49,15 +49,16 @@ macro_rules! test_msg { #[macro_export] macro_rules! test_msg_simple { ($MsgType: path, $data: ident) => {{ - use lightning::util::ser::{Readable, Writeable}; - let mut r = ::lightning::io::Cursor::new($data); - if let Ok(msg) = <$MsgType as Readable>::read(&mut r) { + use lightning::util::ser::{LengthReadable, Writeable}; + if let Ok(msg) = + <$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &$data[..]) + { let mut w = VecWriter(Vec::new()); msg.write(&mut w).unwrap(); assert_eq!(msg.serialized_length(), w.0.len()); let msg = - <$MsgType as Readable>::read(&mut ::lightning::io::Cursor::new(&w.0)).unwrap(); + <$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &w.0[..]).unwrap(); let mut w_two = VecWriter(Vec::new()); msg.write(&mut w_two).unwrap(); assert_eq!(&w.0[..], &w_two.0[..]); diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index f96d376af1e..3f4420e755e 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -81,7 +81,7 @@ use crate::util::config::{ChannelConfig, ChannelConfigUpdate, ChannelConfigOverr use crate::util::wakers::{Future, Notifier}; use crate::util::scid_utils::fake_scid; use crate::util::string::UntrustedString; -use crate::util::ser::{BigSize, FixedLengthReader, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter}; +use crate::util::ser::{BigSize, FixedLengthReader, LengthReadable, Readable, ReadableArgs, MaybeReadable, Writeable, Writer, VecWriter}; use crate::util::logger::{Level, Logger, WithContext}; use crate::util::errors::APIError; #[cfg(async_payments)] use { @@ -12843,14 +12843,14 @@ impl Readable for HTLCFailureMsg { 2 => { let length: BigSize = Readable::read(reader)?; let mut s = FixedLengthReader::new(reader, length.0); - let res = Readable::read(&mut s)?; + let res = LengthReadable::read_from_fixed_length_buffer(&mut s)?; s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes Ok(HTLCFailureMsg::Relay(res)) }, 3 => { let length: BigSize = Readable::read(reader)?; let mut s = FixedLengthReader::new(reader, length.0); - let res = Readable::read(&mut s)?; + let res = LengthReadable::read_from_fixed_length_buffer(&mut s)?; s.eat_remaining()?; // Return ShortRead if there's actually not enough bytes Ok(HTLCFailureMsg::Malformed(res)) }, diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 43d4b1eb9ee..17a58801db7 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -2330,7 +2330,7 @@ impl LengthReadable for TrampolineOnionPacket { let public_key = Readable::read(r)?; let mut rd = FixedLengthReader::new(r, hop_data_len); - let hop_data = WithoutLength::>::read(&mut rd)?.0; + let hop_data = WithoutLength::>::read_from_fixed_length_buffer(&mut rd)?.0; let hmac = Readable::read(r)?; @@ -3937,7 +3937,7 @@ mod tests { use crate::ln::msgs::{self, FinalOnionHopData, OnionErrorPacket, CommonOpenChannelFields, CommonAcceptChannelFields, OutboundTrampolinePayload, TrampolineOnionPacket, InboundOnionForwardPayload, InboundOnionReceivePayload}; use crate::ln::msgs::SocketAddress; use crate::routing::gossip::{NodeAlias, NodeId}; - use crate::util::ser::{BigSize, FixedLengthReader, Hostname, LengthReadable, Readable, ReadableArgs, TransactionU16LenLimited, Writeable}; + use crate::util::ser::{BigSize, Hostname, LengthReadable, Readable, ReadableArgs, TransactionU16LenLimited, Writeable}; use crate::util::test_utils; use bitcoin::hex::FromHex; @@ -4876,7 +4876,7 @@ mod tests { let encoded_value = closing_signed.encode(); let target_value = >::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034dd977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a").unwrap(); assert_eq!(encoded_value, target_value); - assert_eq!(msgs::ClosingSigned::read(&mut Cursor::new(&target_value)).unwrap(), closing_signed); + assert_eq!(msgs::ClosingSigned::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap(), closing_signed); let closing_signed_with_range = msgs::ClosingSigned { channel_id: ChannelId::from_bytes([2; 32]), @@ -4890,7 +4890,7 @@ mod tests { let encoded_value_with_range = closing_signed_with_range.encode(); let target_value_with_range = >::from_hex("020202020202020202020202020202020202020202020202020202020202020200083a840000034dd977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a011000000000deadbeef1badcafe01234567").unwrap(); assert_eq!(encoded_value_with_range, target_value_with_range); - assert_eq!(msgs::ClosingSigned::read(&mut Cursor::new(&target_value_with_range)).unwrap(), + assert_eq!(msgs::ClosingSigned::read_from_fixed_length_buffer(&mut &target_value_with_range[..]).unwrap(), closing_signed_with_range); } @@ -5054,7 +5054,7 @@ mod tests { let encoded_value = init_msg.encode(); let target_value = >::from_hex("0000000001206fe28c0ab6f1b372c1a6a246ae63f74f931e8365e15a089c68d61900000000000307017f00000103e8").unwrap(); assert_eq!(encoded_value, target_value); - assert_eq!(msgs::Init::read(&mut Cursor::new(&target_value)).unwrap(), init_msg); + assert_eq!(msgs::Init::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap(), init_msg); } #[test] @@ -5289,9 +5289,10 @@ mod tests { assert_eq!(encoded_trampoline_packet.len(), 716); { // verify that a codec round trip works - let mut reader = Cursor::new(&encoded_trampoline_packet); - let mut trampoline_packet_reader = FixedLengthReader::new(&mut reader, encoded_trampoline_packet.len() as u64); - let decoded_trampoline_packet: TrampolineOnionPacket = ::read_from_fixed_length_buffer(&mut trampoline_packet_reader).unwrap(); + let decoded_trampoline_packet: TrampolineOnionPacket = + ::read_from_fixed_length_buffer( + &mut &encoded_trampoline_packet[..] + ).unwrap(); assert_eq!(decoded_trampoline_packet.encode(), encoded_trampoline_packet); } @@ -5417,7 +5418,7 @@ mod tests { let target_value = >::from_hex("06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f000186a0000005dc").unwrap(); assert_eq!(encoded_value, target_value); - query_channel_range = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); + query_channel_range = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap(); assert_eq!(query_channel_range.first_blocknum, 100000); assert_eq!(query_channel_range.number_of_blocks, 1500); } @@ -5501,7 +5502,7 @@ mod tests { let target_value = >::from_hex("06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f01").unwrap(); assert_eq!(encoded_value, target_value); - reply_short_channel_ids_end = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); + reply_short_channel_ids_end = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap(); assert_eq!(reply_short_channel_ids_end.chain_hash, expected_chain_hash); assert_eq!(reply_short_channel_ids_end.full_information, true); } @@ -5518,7 +5519,9 @@ mod tests { let target_value = >::from_hex("06226e46111a0b59caaf126043eb5bbf28c34f3a5e332a1fc7b2b73cf188910f5ec57980ffffffff").unwrap(); assert_eq!(encoded_value, target_value); - gossip_timestamp_filter = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); + gossip_timestamp_filter = LengthReadable::read_from_fixed_length_buffer( + &mut &target_value[..] + ).unwrap(); assert_eq!(gossip_timestamp_filter.chain_hash, expected_chain_hash); assert_eq!(gossip_timestamp_filter.first_timestamp, 1590000000); assert_eq!(gossip_timestamp_filter.timestamp_range, 0xffff_ffff); diff --git a/lightning/src/ln/peer_handler.rs b/lightning/src/ln/peer_handler.rs index 336ce40cf11..b4383210802 100644 --- a/lightning/src/ln/peer_handler.rs +++ b/lightning/src/ln/peer_handler.rs @@ -1541,8 +1541,10 @@ impl 8192 { peer.pending_read_buffer = Vec::new(); } diff --git a/lightning/src/ln/wire.rs b/lightning/src/ln/wire.rs index d3442674b1c..7b08d75a824 100644 --- a/lightning/src/ln/wire.rs +++ b/lightning/src/ln/wire.rs @@ -14,7 +14,7 @@ use crate::io; use crate::ln::msgs; -use crate::util::ser::{Readable, Writeable, Writer}; +use crate::util::ser::{LengthLimitedRead, LengthReadable, Readable, Writeable, Writer}; /// Trait to be implemented by custom message (unrelated to the channel/gossip LN layers) /// decoders. @@ -238,7 +238,7 @@ impl Message { /// # Errors /// /// Returns an error if the message payload could not be decoded as the specified type. -pub(crate) fn read( +pub(crate) fn read( buffer: &mut R, custom_reader: H, ) -> Result, (msgs::DecodeError, Option)> where @@ -249,7 +249,7 @@ where do_read(buffer, message_type, custom_reader).map_err(|e| (e, Some(message_type))) } -fn do_read( +fn do_read( buffer: &mut R, message_type: u16, custom_reader: H, ) -> Result, msgs::DecodeError> where @@ -262,48 +262,48 @@ where msgs::WarningMessage::TYPE => Ok(Message::Warning(Readable::read(buffer)?)), msgs::Ping::TYPE => Ok(Message::Ping(Readable::read(buffer)?)), msgs::Pong::TYPE => Ok(Message::Pong(Readable::read(buffer)?)), - msgs::PeerStorage::TYPE => Ok(Message::PeerStorage(Readable::read(buffer)?)), + msgs::PeerStorage::TYPE => Ok(Message::PeerStorage(LengthReadable::read_from_fixed_length_buffer(buffer)?)), msgs::PeerStorageRetrieval::TYPE => { - Ok(Message::PeerStorageRetrieval(Readable::read(buffer)?)) + Ok(Message::PeerStorageRetrieval(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, msgs::OpenChannel::TYPE => Ok(Message::OpenChannel(Readable::read(buffer)?)), msgs::OpenChannelV2::TYPE => Ok(Message::OpenChannelV2(Readable::read(buffer)?)), msgs::AcceptChannel::TYPE => Ok(Message::AcceptChannel(Readable::read(buffer)?)), msgs::AcceptChannelV2::TYPE => Ok(Message::AcceptChannelV2(Readable::read(buffer)?)), - msgs::FundingCreated::TYPE => Ok(Message::FundingCreated(Readable::read(buffer)?)), - msgs::FundingSigned::TYPE => Ok(Message::FundingSigned(Readable::read(buffer)?)), + msgs::FundingCreated::TYPE => Ok(Message::FundingCreated(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::FundingSigned::TYPE => Ok(Message::FundingSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)), #[cfg(splicing)] - msgs::SpliceInit::TYPE => Ok(Message::SpliceInit(Readable::read(buffer)?)), - msgs::Stfu::TYPE => Ok(Message::Stfu(Readable::read(buffer)?)), + msgs::SpliceInit::TYPE => Ok(Message::SpliceInit(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::Stfu::TYPE => Ok(Message::Stfu(LengthReadable::read_from_fixed_length_buffer(buffer)?)), #[cfg(splicing)] - msgs::SpliceAck::TYPE => Ok(Message::SpliceAck(Readable::read(buffer)?)), + msgs::SpliceAck::TYPE => Ok(Message::SpliceAck(LengthReadable::read_from_fixed_length_buffer(buffer)?)), #[cfg(splicing)] - msgs::SpliceLocked::TYPE => Ok(Message::SpliceLocked(Readable::read(buffer)?)), - msgs::TxAddInput::TYPE => Ok(Message::TxAddInput(Readable::read(buffer)?)), - msgs::TxAddOutput::TYPE => Ok(Message::TxAddOutput(Readable::read(buffer)?)), - msgs::TxRemoveInput::TYPE => Ok(Message::TxRemoveInput(Readable::read(buffer)?)), - msgs::TxRemoveOutput::TYPE => Ok(Message::TxRemoveOutput(Readable::read(buffer)?)), - msgs::TxComplete::TYPE => Ok(Message::TxComplete(Readable::read(buffer)?)), - msgs::TxSignatures::TYPE => Ok(Message::TxSignatures(Readable::read(buffer)?)), - msgs::TxInitRbf::TYPE => Ok(Message::TxInitRbf(Readable::read(buffer)?)), - msgs::TxAckRbf::TYPE => Ok(Message::TxAckRbf(Readable::read(buffer)?)), - msgs::TxAbort::TYPE => Ok(Message::TxAbort(Readable::read(buffer)?)), - msgs::ChannelReady::TYPE => Ok(Message::ChannelReady(Readable::read(buffer)?)), - msgs::Shutdown::TYPE => Ok(Message::Shutdown(Readable::read(buffer)?)), - msgs::ClosingSigned::TYPE => Ok(Message::ClosingSigned(Readable::read(buffer)?)), + msgs::SpliceLocked::TYPE => Ok(Message::SpliceLocked(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::TxAddInput::TYPE => Ok(Message::TxAddInput(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::TxAddOutput::TYPE => Ok(Message::TxAddOutput(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::TxRemoveInput::TYPE => Ok(Message::TxRemoveInput(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::TxRemoveOutput::TYPE => Ok(Message::TxRemoveOutput(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::TxComplete::TYPE => Ok(Message::TxComplete(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::TxSignatures::TYPE => Ok(Message::TxSignatures(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::TxInitRbf::TYPE => Ok(Message::TxInitRbf(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::TxAckRbf::TYPE => Ok(Message::TxAckRbf(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::TxAbort::TYPE => Ok(Message::TxAbort(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::ChannelReady::TYPE => Ok(Message::ChannelReady(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::Shutdown::TYPE => Ok(Message::Shutdown(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::ClosingSigned::TYPE => Ok(Message::ClosingSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)), msgs::OnionMessage::TYPE => Ok(Message::OnionMessage(Readable::read(buffer)?)), - msgs::UpdateAddHTLC::TYPE => Ok(Message::UpdateAddHTLC(Readable::read(buffer)?)), - msgs::UpdateFulfillHTLC::TYPE => Ok(Message::UpdateFulfillHTLC(Readable::read(buffer)?)), - msgs::UpdateFailHTLC::TYPE => Ok(Message::UpdateFailHTLC(Readable::read(buffer)?)), + msgs::UpdateAddHTLC::TYPE => Ok(Message::UpdateAddHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::UpdateFulfillHTLC::TYPE => Ok(Message::UpdateFulfillHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::UpdateFailHTLC::TYPE => Ok(Message::UpdateFailHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)), msgs::UpdateFailMalformedHTLC::TYPE => { - Ok(Message::UpdateFailMalformedHTLC(Readable::read(buffer)?)) + Ok(Message::UpdateFailMalformedHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, - msgs::CommitmentSigned::TYPE => Ok(Message::CommitmentSigned(Readable::read(buffer)?)), - msgs::RevokeAndACK::TYPE => Ok(Message::RevokeAndACK(Readable::read(buffer)?)), - msgs::UpdateFee::TYPE => Ok(Message::UpdateFee(Readable::read(buffer)?)), - msgs::ChannelReestablish::TYPE => Ok(Message::ChannelReestablish(Readable::read(buffer)?)), + msgs::CommitmentSigned::TYPE => Ok(Message::CommitmentSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::RevokeAndACK::TYPE => Ok(Message::RevokeAndACK(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::UpdateFee::TYPE => Ok(Message::UpdateFee(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::ChannelReestablish::TYPE => Ok(Message::ChannelReestablish(LengthReadable::read_from_fixed_length_buffer(buffer)?)), msgs::AnnouncementSignatures::TYPE => { - Ok(Message::AnnouncementSignatures(Readable::read(buffer)?)) + Ok(Message::AnnouncementSignatures(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, msgs::ChannelAnnouncement::TYPE => { Ok(Message::ChannelAnnouncement(Readable::read(buffer)?)) @@ -314,12 +314,12 @@ where Ok(Message::QueryShortChannelIds(Readable::read(buffer)?)) }, msgs::ReplyShortChannelIdsEnd::TYPE => { - Ok(Message::ReplyShortChannelIdsEnd(Readable::read(buffer)?)) + Ok(Message::ReplyShortChannelIdsEnd(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, - msgs::QueryChannelRange::TYPE => Ok(Message::QueryChannelRange(Readable::read(buffer)?)), + msgs::QueryChannelRange::TYPE => Ok(Message::QueryChannelRange(LengthReadable::read_from_fixed_length_buffer(buffer)?)), msgs::ReplyChannelRange::TYPE => Ok(Message::ReplyChannelRange(Readable::read(buffer)?)), msgs::GossipTimestampFilter::TYPE => { - Ok(Message::GossipTimestampFilter(Readable::read(buffer)?)) + Ok(Message::GossipTimestampFilter(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, _ => { if let Some(custom) = custom_reader.read(message_type, buffer)? { @@ -590,36 +590,31 @@ mod tests { #[test] fn read_empty_buffer() { let buffer = []; - let mut reader = io::Cursor::new(buffer); - assert!(read(&mut reader, &IgnoringMessageHandler {}).is_err()); + assert!(read(&mut &buffer[..], &IgnoringMessageHandler {}).is_err()); } #[test] fn read_incomplete_type() { let buffer = &ENCODED_PONG[..1]; - let mut reader = io::Cursor::new(buffer); - assert!(read(&mut reader, &IgnoringMessageHandler {}).is_err()); + assert!(read(&mut &buffer[..], &IgnoringMessageHandler {}).is_err()); } #[test] fn read_empty_payload() { let buffer = &ENCODED_PONG[..2]; - let mut reader = io::Cursor::new(buffer); - assert!(read(&mut reader, &IgnoringMessageHandler {}).is_err()); + assert!(read(&mut &buffer[..], &IgnoringMessageHandler {}).is_err()); } #[test] fn read_invalid_message() { let buffer = &ENCODED_PONG[..4]; - let mut reader = io::Cursor::new(buffer); - assert!(read(&mut reader, &IgnoringMessageHandler {}).is_err()); + assert!(read(&mut &buffer[..], &IgnoringMessageHandler {}).is_err()); } #[test] fn read_known_message() { let buffer = &ENCODED_PONG[..]; - let mut reader = io::Cursor::new(buffer); - let message = read(&mut reader, &IgnoringMessageHandler {}).unwrap(); + let message = read(&mut &buffer[..], &IgnoringMessageHandler {}).unwrap(); match message { Message::Pong(_) => (), _ => panic!("Expected pong message; found message type: {}", message.type_id()), @@ -629,8 +624,7 @@ mod tests { #[test] fn read_unknown_message() { let buffer = &::core::u16::MAX.to_be_bytes(); - let mut reader = io::Cursor::new(buffer); - let message = read(&mut reader, &IgnoringMessageHandler {}).unwrap(); + let message = read(&mut &buffer[..], &IgnoringMessageHandler {}).unwrap(); match message { Message::Unknown(::core::u16::MAX) => (), _ => panic!("Expected message type {}; found: {}", ::core::u16::MAX, message.type_id()), @@ -655,8 +649,7 @@ mod tests { let mut buffer = Vec::new(); assert!(write(&message, &mut buffer).is_ok()); - let mut reader = io::Cursor::new(buffer); - let decoded_message = read(&mut reader, &IgnoringMessageHandler {}).unwrap(); + let decoded_message = read(&mut &buffer[..], &IgnoringMessageHandler {}).unwrap(); match decoded_message { Message::Pong(msgs::Pong { byteslen: 2u16 }) => (), Message::Pong(msgs::Pong { byteslen }) => { @@ -697,8 +690,7 @@ mod tests { } fn check_init_msg(buffer: Vec, expect_unknown: bool) { - let mut reader = io::Cursor::new(buffer); - let decoded_msg = read(&mut reader, &IgnoringMessageHandler {}).unwrap(); + let decoded_msg = read(&mut &buffer[..], &IgnoringMessageHandler {}).unwrap(); match decoded_msg { Message::Init(msgs::Init { features, .. }) => { assert!(features.supports_variable_length_onion()); @@ -725,8 +717,7 @@ mod tests { 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 7, 1, 172, 21, 0, 2, 38, 7, ]; - let mut reader = io::Cursor::new(buffer); - let decoded_msg = read(&mut reader, &IgnoringMessageHandler {}).unwrap(); + let decoded_msg = read(&mut &buffer[..], &IgnoringMessageHandler {}).unwrap(); match decoded_msg { Message::NodeAnnouncement(msgs::NodeAnnouncement { contents: msgs::UnsignedNodeAnnouncement { features, .. }, @@ -771,8 +762,7 @@ mod tests { 68, 88, 28, 15, 207, 21, 179, 151, 56, 226, 158, 148, 3, 120, 113, 177, 243, 184, 17, 173, 37, 46, 222, 16, ]; - let mut reader = io::Cursor::new(buffer); - let decoded_msg = read(&mut reader, &IgnoringMessageHandler {}).unwrap(); + let decoded_msg = read(&mut &buffer[..], &IgnoringMessageHandler {}).unwrap(); match decoded_msg { Message::ChannelAnnouncement(msgs::ChannelAnnouncement { contents: msgs::UnsignedChannelAnnouncement { features, .. }, @@ -821,8 +811,7 @@ mod tests { #[test] fn read_custom_message() { let buffer = vec![35, 40]; - let mut reader = io::Cursor::new(buffer); - let decoded_msg = read(&mut reader, &TestCustomMessageReader {}).unwrap(); + let decoded_msg = read(&mut &buffer[..], &TestCustomMessageReader {}).unwrap(); match decoded_msg { Message::Custom(custom) => { assert_eq!(custom.type_id(), CUSTOM_MESSAGE_TYPE); @@ -835,8 +824,7 @@ mod tests { #[test] fn read_with_custom_reader_unknown_message_type() { let buffer = vec![35, 42]; - let mut reader = io::Cursor::new(buffer); - let decoded_msg = read(&mut reader, &TestCustomMessageReader {}).unwrap(); + let decoded_msg = read(&mut &buffer[..], &TestCustomMessageReader {}).unwrap(); match decoded_msg { Message::Unknown(_) => {}, _ => panic!("Expected unknown message, found message type: {}", decoded_msg.type_id()), @@ -846,8 +834,8 @@ mod tests { #[test] fn custom_reader_unknown_message_type() { let buffer = Vec::new(); - let mut reader = io::Cursor::new(buffer); - let res = TestCustomMessageReader {}.read(CUSTOM_MESSAGE_TYPE + 1, &mut reader).unwrap(); + let res = + TestCustomMessageReader {}.read(CUSTOM_MESSAGE_TYPE + 1, &mut &buffer[..]).unwrap(); assert!(res.is_none()); } } diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index bea4a3f28ad..d3998026513 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1028,9 +1028,7 @@ macro_rules! impl_readable_for_vec_with_element_length_prefix { for _ in 0..len.0 { let elem_len: CollectionLength = Readable::read(r)?; let mut elem_reader = FixedLengthReader::new(r, elem_len.0); - if let Some(val) = MaybeReadable::read(&mut elem_reader)? { - ret.push(val); - } + ret.push(LengthReadable::read_from_fixed_length_buffer(&mut elem_reader)?); } Ok(ret) } diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 0b5bdc397e8..7497b3d4d13 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -646,7 +646,7 @@ macro_rules! _decode_tlv_stream_range { } } } -/// Implements [`Readable`]/[`Writeable`] for a message struct that may include non-TLV and +/// Implements [`LengthReadable`]/[`Writeable`] for a message struct that may include non-TLV and /// TLV-encoded parts. /// /// This is useful to implement a [`CustomMessageReader`]. @@ -672,7 +672,7 @@ macro_rules! _decode_tlv_stream_range { /// }); /// ``` /// -/// [`Readable`]: crate::util::ser::Readable +/// [`LengthReadable`]: crate::util::ser::LengthReadable /// [`Writeable`]: crate::util::ser::Writeable /// [`CustomMessageReader`]: crate::ln::wire::CustomMessageReader #[macro_export] @@ -685,8 +685,10 @@ macro_rules! impl_writeable_msg { Ok(()) } } - impl $crate::util::ser::Readable for $st { - fn read(r: &mut R) -> Result { + impl $crate::util::ser::LengthReadable for $st { + fn read_from_fixed_length_buffer( + r: &mut R + ) -> Result { $(let $field = $crate::util::ser::Readable::read(r)?;)* $($crate::_init_tlv_field_var!($tlvfield, $fieldty);)* $crate::decode_tlv_stream!(r, {$(($type, $tlvfield, $fieldty)),*}); @@ -1368,7 +1370,7 @@ mod tests { use crate::io::{self, Cursor}; use crate::ln::msgs::DecodeError; use crate::util::ser::{ - HighZeroBytesDroppedBigSize, MaybeReadable, Readable, VecWriter, Writeable, + HighZeroBytesDroppedBigSize, LengthReadable, MaybeReadable, Readable, VecWriter, Writeable, }; use bitcoin::hex::FromHex; use bitcoin::secp256k1::PublicKey; @@ -1814,10 +1816,10 @@ mod tests { #[test] fn impl_writeable_msg_empty() { let msg = EmptyMsg {}; - let mut encoded_msg = msg.encode(); + let encoded_msg = msg.encode(); assert!(encoded_msg.is_empty()); - let mut encoded_msg_stream = Cursor::new(&mut encoded_msg); - let decoded_msg: EmptyMsg = Readable::read(&mut encoded_msg_stream).unwrap(); + let decoded_msg: EmptyMsg = + LengthReadable::read_from_fixed_length_buffer(&mut &encoded_msg[..]).unwrap(); assert_eq!(msg, decoded_msg); } From 50745bf5b5d0a635735e827854b790d9d12fc6ef Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Fri, 7 Mar 2025 14:14:58 -0500 Subject: [PATCH 4/8] Rustfmt wire.rs Easier to review the previous commit this way. --- lightning/src/ln/wire.rs | 130 +++++++++++++++++++++++++++------------ 1 file changed, 92 insertions(+), 38 deletions(-) diff --git a/lightning/src/ln/wire.rs b/lightning/src/ln/wire.rs index 7b08d75a824..96db1136d7c 100644 --- a/lightning/src/ln/wire.rs +++ b/lightning/src/ln/wire.rs @@ -262,49 +262,101 @@ where msgs::WarningMessage::TYPE => Ok(Message::Warning(Readable::read(buffer)?)), msgs::Ping::TYPE => Ok(Message::Ping(Readable::read(buffer)?)), msgs::Pong::TYPE => Ok(Message::Pong(Readable::read(buffer)?)), - msgs::PeerStorage::TYPE => Ok(Message::PeerStorage(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::PeerStorageRetrieval::TYPE => { - Ok(Message::PeerStorageRetrieval(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + msgs::PeerStorage::TYPE => { + Ok(Message::PeerStorage(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, + msgs::PeerStorageRetrieval::TYPE => Ok(Message::PeerStorageRetrieval( + LengthReadable::read_from_fixed_length_buffer(buffer)?, + )), msgs::OpenChannel::TYPE => Ok(Message::OpenChannel(Readable::read(buffer)?)), msgs::OpenChannelV2::TYPE => Ok(Message::OpenChannelV2(Readable::read(buffer)?)), msgs::AcceptChannel::TYPE => Ok(Message::AcceptChannel(Readable::read(buffer)?)), msgs::AcceptChannelV2::TYPE => Ok(Message::AcceptChannelV2(Readable::read(buffer)?)), - msgs::FundingCreated::TYPE => Ok(Message::FundingCreated(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::FundingSigned::TYPE => Ok(Message::FundingSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::FundingCreated::TYPE => { + Ok(Message::FundingCreated(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::FundingSigned::TYPE => { + Ok(Message::FundingSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, #[cfg(splicing)] - msgs::SpliceInit::TYPE => Ok(Message::SpliceInit(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::Stfu::TYPE => Ok(Message::Stfu(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::SpliceInit::TYPE => { + Ok(Message::SpliceInit(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::Stfu::TYPE => { + Ok(Message::Stfu(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, #[cfg(splicing)] - msgs::SpliceAck::TYPE => Ok(Message::SpliceAck(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::SpliceAck::TYPE => { + Ok(Message::SpliceAck(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, #[cfg(splicing)] - msgs::SpliceLocked::TYPE => Ok(Message::SpliceLocked(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::TxAddInput::TYPE => Ok(Message::TxAddInput(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::TxAddOutput::TYPE => Ok(Message::TxAddOutput(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::TxRemoveInput::TYPE => Ok(Message::TxRemoveInput(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::TxRemoveOutput::TYPE => Ok(Message::TxRemoveOutput(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::TxComplete::TYPE => Ok(Message::TxComplete(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::TxSignatures::TYPE => Ok(Message::TxSignatures(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::TxInitRbf::TYPE => Ok(Message::TxInitRbf(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::TxAckRbf::TYPE => Ok(Message::TxAckRbf(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::TxAbort::TYPE => Ok(Message::TxAbort(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::ChannelReady::TYPE => Ok(Message::ChannelReady(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::Shutdown::TYPE => Ok(Message::Shutdown(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::ClosingSigned::TYPE => Ok(Message::ClosingSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)), + msgs::SpliceLocked::TYPE => { + Ok(Message::SpliceLocked(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::TxAddInput::TYPE => { + Ok(Message::TxAddInput(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::TxAddOutput::TYPE => { + Ok(Message::TxAddOutput(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::TxRemoveInput::TYPE => { + Ok(Message::TxRemoveInput(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::TxRemoveOutput::TYPE => { + Ok(Message::TxRemoveOutput(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::TxComplete::TYPE => { + Ok(Message::TxComplete(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::TxSignatures::TYPE => { + Ok(Message::TxSignatures(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::TxInitRbf::TYPE => { + Ok(Message::TxInitRbf(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::TxAckRbf::TYPE => { + Ok(Message::TxAckRbf(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::TxAbort::TYPE => { + Ok(Message::TxAbort(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::ChannelReady::TYPE => { + Ok(Message::ChannelReady(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::Shutdown::TYPE => { + Ok(Message::Shutdown(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::ClosingSigned::TYPE => { + Ok(Message::ClosingSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, msgs::OnionMessage::TYPE => Ok(Message::OnionMessage(Readable::read(buffer)?)), - msgs::UpdateAddHTLC::TYPE => Ok(Message::UpdateAddHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::UpdateFulfillHTLC::TYPE => Ok(Message::UpdateFulfillHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::UpdateFailHTLC::TYPE => Ok(Message::UpdateFailHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::UpdateFailMalformedHTLC::TYPE => { - Ok(Message::UpdateFailMalformedHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + msgs::UpdateAddHTLC::TYPE => { + Ok(Message::UpdateAddHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, - msgs::CommitmentSigned::TYPE => Ok(Message::CommitmentSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::RevokeAndACK::TYPE => Ok(Message::RevokeAndACK(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::UpdateFee::TYPE => Ok(Message::UpdateFee(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::ChannelReestablish::TYPE => Ok(Message::ChannelReestablish(LengthReadable::read_from_fixed_length_buffer(buffer)?)), - msgs::AnnouncementSignatures::TYPE => { - Ok(Message::AnnouncementSignatures(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + msgs::UpdateFulfillHTLC::TYPE => { + Ok(Message::UpdateFulfillHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, + msgs::UpdateFailHTLC::TYPE => { + Ok(Message::UpdateFailHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::UpdateFailMalformedHTLC::TYPE => Ok(Message::UpdateFailMalformedHTLC( + LengthReadable::read_from_fixed_length_buffer(buffer)?, + )), + msgs::CommitmentSigned::TYPE => { + Ok(Message::CommitmentSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::RevokeAndACK::TYPE => { + Ok(Message::RevokeAndACK(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::UpdateFee::TYPE => { + Ok(Message::UpdateFee(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::ChannelReestablish::TYPE => { + Ok(Message::ChannelReestablish(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::AnnouncementSignatures::TYPE => Ok(Message::AnnouncementSignatures( + LengthReadable::read_from_fixed_length_buffer(buffer)?, + )), msgs::ChannelAnnouncement::TYPE => { Ok(Message::ChannelAnnouncement(Readable::read(buffer)?)) }, @@ -313,14 +365,16 @@ where msgs::QueryShortChannelIds::TYPE => { Ok(Message::QueryShortChannelIds(Readable::read(buffer)?)) }, - msgs::ReplyShortChannelIdsEnd::TYPE => { - Ok(Message::ReplyShortChannelIdsEnd(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + msgs::ReplyShortChannelIdsEnd::TYPE => Ok(Message::ReplyShortChannelIdsEnd( + LengthReadable::read_from_fixed_length_buffer(buffer)?, + )), + msgs::QueryChannelRange::TYPE => { + Ok(Message::QueryChannelRange(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, - msgs::QueryChannelRange::TYPE => Ok(Message::QueryChannelRange(LengthReadable::read_from_fixed_length_buffer(buffer)?)), msgs::ReplyChannelRange::TYPE => Ok(Message::ReplyChannelRange(Readable::read(buffer)?)), - msgs::GossipTimestampFilter::TYPE => { - Ok(Message::GossipTimestampFilter(LengthReadable::read_from_fixed_length_buffer(buffer)?)) - }, + msgs::GossipTimestampFilter::TYPE => Ok(Message::GossipTimestampFilter( + LengthReadable::read_from_fixed_length_buffer(buffer)?, + )), _ => { if let Some(custom) = custom_reader.read(message_type, buffer)? { Ok(Message::Custom(custom)) From 6f96d025ce8d04e8444b723360eb8adca2ce71ba Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 11 Mar 2025 16:39:32 -0400 Subject: [PATCH 5/8] Require length limited reader for LN gossip messages Continuing the work of the last few commits, here we require that some LN gossip messages be read from a length limiting reader. The messages' updates are separated into their own commit because they definitely should use a length limited reader but they also require breaking out of the ser macros. We can't use the macros because each message contains a non-TLV field that will always consume the rest of the reader, while the ser macros only support reading non-TLV fields that know when to stop reading. --- fuzz/src/msg_targets/utils.rs | 9 ++-- fuzz/src/router.rs | 9 ++-- lightning/src/ln/msgs.rs | 81 ++++++++++++++++++++++++--------- lightning/src/ln/wire.rs | 10 ++-- lightning/src/routing/gossip.rs | 5 +- lightning/src/util/ser.rs | 4 +- 6 files changed, 82 insertions(+), 36 deletions(-) diff --git a/fuzz/src/msg_targets/utils.rs b/fuzz/src/msg_targets/utils.rs index b2bc220afb0..17e1318ee3d 100644 --- a/fuzz/src/msg_targets/utils.rs +++ b/fuzz/src/msg_targets/utils.rs @@ -71,12 +71,13 @@ macro_rules! test_msg_simple { #[macro_export] macro_rules! test_msg_exact { ($MsgType: path, $data: ident) => {{ - use lightning::util::ser::{Readable, Writeable}; - let mut r = ::lightning::io::Cursor::new($data); - if let Ok(msg) = <$MsgType as Readable>::read(&mut r) { + use lightning::util::ser::{LengthReadable, Writeable}; + if let Ok(msg) = + <$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &$data[..]) + { let mut w = VecWriter(Vec::new()); msg.write(&mut w).unwrap(); - assert_eq!(&r.into_inner()[..], &w.0[..]); + assert_eq!(&$data[..], &w.0[..]); assert_eq!(msg.serialized_length(), w.0.len()); } }}; diff --git a/fuzz/src/router.rs b/fuzz/src/router.rs index 6bf9e300fa2..fd97d084f25 100644 --- a/fuzz/src/router.rs +++ b/fuzz/src/router.rs @@ -30,7 +30,7 @@ use lightning::routing::utxo::{UtxoFuture, UtxoLookup, UtxoLookupError, UtxoResu use lightning::types::features::{BlindedHopFeatures, Bolt12InvoiceFeatures}; use lightning::util::config::UserConfig; use lightning::util::hash_tables::*; -use lightning::util::ser::Readable; +use lightning::util::ser::LengthReadable; use bitcoin::hashes::Hash; use bitcoin::network::Network; @@ -146,10 +146,11 @@ pub fn do_test(data: &[u8], out: Out) { macro_rules! decode_msg { ($MsgType: path, $len: expr) => {{ - let mut reader = ::lightning::io::Cursor::new(get_slice!($len)); - match <$MsgType>::read(&mut reader) { + let data = get_slice!($len); + let mut reader = &data[..]; + match <$MsgType>::read_from_fixed_length_buffer(&mut reader) { Ok(msg) => { - assert_eq!(reader.position(), $len as u64); + assert_eq!(reader.len(), $len); msg }, Err(e) => match e { diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 17a58801db7..308daa55a6b 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -3572,8 +3572,8 @@ impl Writeable for UnsignedChannelAnnouncement { } } -impl Readable for UnsignedChannelAnnouncement { - fn read(r: &mut R) -> Result { +impl LengthReadable for UnsignedChannelAnnouncement { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { Ok(Self { features: Readable::read(r)?, chain_hash: Readable::read(r)?, @@ -3587,13 +3587,28 @@ impl Readable for UnsignedChannelAnnouncement { } } -impl_writeable!(ChannelAnnouncement, { - node_signature_1, - node_signature_2, - bitcoin_signature_1, - bitcoin_signature_2, - contents -}); +impl Writeable for ChannelAnnouncement { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.node_signature_1.write(w)?; + self.node_signature_2.write(w)?; + self.bitcoin_signature_1.write(w)?; + self.bitcoin_signature_2.write(w)?; + self.contents.write(w)?; + Ok(()) + } +} + +impl LengthReadable for ChannelAnnouncement { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { + Ok(Self { + node_signature_1: Readable::read(r)?, + node_signature_2: Readable::read(r)?, + bitcoin_signature_1: Readable::read(r)?, + bitcoin_signature_2: Readable::read(r)?, + contents: LengthReadable::read_from_fixed_length_buffer(r)?, + }) + } +} impl Writeable for UnsignedChannelUpdate { fn write(&self, w: &mut W) -> Result<(), io::Error> { @@ -3614,8 +3629,8 @@ impl Writeable for UnsignedChannelUpdate { } } -impl Readable for UnsignedChannelUpdate { - fn read(r: &mut R) -> Result { +impl LengthReadable for UnsignedChannelUpdate { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let res = Self { chain_hash: Readable::read(r)?, short_channel_id: Readable::read(r)?, @@ -3639,10 +3654,22 @@ impl Readable for UnsignedChannelUpdate { } } -impl_writeable!(ChannelUpdate, { - signature, - contents -}); +impl Writeable for ChannelUpdate { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.signature.write(w)?; + self.contents.write(w)?; + Ok(()) + } +} + +impl LengthReadable for ChannelUpdate { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { + Ok(Self { + signature: Readable::read(r)?, + contents: LengthReadable::read_from_fixed_length_buffer(r)?, + }) + } +} impl Writeable for ErrorMessage { fn write(&self, w: &mut W) -> Result<(), io::Error> { @@ -3720,8 +3747,8 @@ impl Writeable for UnsignedNodeAnnouncement { } } -impl Readable for UnsignedNodeAnnouncement { - fn read(r: &mut R) -> Result { +impl LengthReadable for UnsignedNodeAnnouncement { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let features: NodeFeatures = Readable::read(r)?; let timestamp: u32 = Readable::read(r)?; let node_id: NodeId = Readable::read(r)?; @@ -3782,10 +3809,22 @@ impl Readable for UnsignedNodeAnnouncement { } } -impl_writeable!(NodeAnnouncement, { - signature, - contents -}); +impl Writeable for NodeAnnouncement { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + self.signature.write(w)?; + self.contents.write(w)?; + Ok(()) + } +} + +impl LengthReadable for NodeAnnouncement { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { + Ok(Self { + signature: Readable::read(r)?, + contents: LengthReadable::read_from_fixed_length_buffer(r)?, + }) + } +} impl Readable for QueryShortChannelIds { fn read(r: &mut R) -> Result { diff --git a/lightning/src/ln/wire.rs b/lightning/src/ln/wire.rs index 96db1136d7c..03a5b51e15e 100644 --- a/lightning/src/ln/wire.rs +++ b/lightning/src/ln/wire.rs @@ -358,10 +358,14 @@ where LengthReadable::read_from_fixed_length_buffer(buffer)?, )), msgs::ChannelAnnouncement::TYPE => { - Ok(Message::ChannelAnnouncement(Readable::read(buffer)?)) + Ok(Message::ChannelAnnouncement(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::NodeAnnouncement::TYPE => { + Ok(Message::NodeAnnouncement(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::ChannelUpdate::TYPE => { + Ok(Message::ChannelUpdate(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, - msgs::NodeAnnouncement::TYPE => Ok(Message::NodeAnnouncement(Readable::read(buffer)?)), - msgs::ChannelUpdate::TYPE => Ok(Message::ChannelUpdate(Readable::read(buffer)?)), msgs::QueryShortChannelIds::TYPE => { Ok(Message::QueryShortChannelIds(Readable::read(buffer)?)) }, diff --git a/lightning/src/routing/gossip.rs b/lightning/src/routing/gossip.rs index 91f6869067b..c8b18af28a8 100644 --- a/lightning/src/routing/gossip.rs +++ b/lightning/src/routing/gossip.rs @@ -2730,7 +2730,7 @@ pub(crate) mod tests { use crate::types::features::InitFeatures; use crate::util::config::UserConfig; use crate::util::scid_utils::scid_from_parts; - use crate::util::ser::{Hostname, Readable, ReadableArgs, Writeable}; + use crate::util::ser::{Hostname, LengthReadable, Readable, ReadableArgs, Writeable}; use crate::util::test_utils; use super::STALE_CHANNEL_UPDATE_AGE_LIMIT_SECS; @@ -4301,7 +4301,8 @@ pub(crate) mod tests { // 1. Check we can read a valid NodeAnnouncementInfo and fail on an invalid one let announcement_message = >::from_hex("d977cb9b53d93a6ff64bb5f1e158b4094b66e798fb12911168a3ccdf80a83096340a6a95da0ae8d9f776528eecdbb747eb6b545495a4319ed5378e35b21e073a000122013413a7031b84c5567b126440995d3ed5aaba0565d71e1834604819ff9c17f5e9d5dd078f2020201010101010101010101010101010101010101010101010101010101010101010000701fffefdfc2607").unwrap(); let announcement_message = - NodeAnnouncement::read(&mut announcement_message.as_slice()).unwrap(); + NodeAnnouncement::read_from_fixed_length_buffer(&mut announcement_message.as_slice()) + .unwrap(); let valid_node_ann_info = NodeAnnouncementInfo::Relayed(announcement_message); let mut encoded_valid_node_ann_info = Vec::new(); diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index d3998026513..181eb5189ac 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -1330,14 +1330,14 @@ impl Writeable for Option { } } -impl Readable for Option { +impl Readable for Option { fn read(r: &mut R) -> Result { let len: BigSize = Readable::read(r)?; match len.0 { 0 => Ok(None), len => { let mut reader = FixedLengthReader::new(r, len - 1); - Ok(Some(Readable::read(&mut reader)?)) + Ok(Some(LengthReadable::read_from_fixed_length_buffer(&mut reader)?)) }, } } From 7b1bb4a30340dc2e6dc845f9aef002b6ed7ff00c Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 11 Mar 2025 17:19:16 -0400 Subject: [PATCH 6/8] Require length limited reader for all LN messages When deserializing lightning messages, many consume the reader until it is out of bytes, or could in the future if new fields or TLVs are added. Therefore, it's safer if they are only provided with readers that limit how much of the byte stream will be read. Many of the relevant LN messages were updated in a prior commit when we transitioned impl_writeable_msg to require length limiting readers, here we finish the job. --- fuzz/src/msg_targets/utils.rs | 21 ++++++------- fuzz/src/onion_message.rs | 9 ++++-- lightning/src/ln/msgs.rs | 56 +++++++++++++++++------------------ lightning/src/ln/wire.rs | 50 ++++++++++++++++++++++--------- 4 files changed, 81 insertions(+), 55 deletions(-) diff --git a/fuzz/src/msg_targets/utils.rs b/fuzz/src/msg_targets/utils.rs index 17e1318ee3d..00474b17684 100644 --- a/fuzz/src/msg_targets/utils.rs +++ b/fuzz/src/msg_targets/utils.rs @@ -30,16 +30,16 @@ impl Writer for VecWriter { #[macro_export] macro_rules! test_msg { ($MsgType: path, $data: ident) => {{ - use lightning::util::ser::{Readable, Writeable}; - let mut r = ::lightning::io::Cursor::new($data); - if let Ok(msg) = <$MsgType as Readable>::read(&mut r) { - let p = r.position() as usize; + use lightning::util::ser::{LengthReadable, Writeable}; + let mut r = &$data[..]; + if let Ok(msg) = <$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut r) { + let p = $data.len() - r.len() as usize; let mut w = VecWriter(Vec::new()); msg.write(&mut w).unwrap(); assert_eq!(w.0.len(), p); assert_eq!(msg.serialized_length(), p); - assert_eq!(&r.into_inner()[..p], &w.0[..p]); + assert_eq!(&$data[..p], &w.0[..p]); } }}; } @@ -88,17 +88,18 @@ macro_rules! test_msg_exact { #[macro_export] macro_rules! test_msg_hole { ($MsgType: path, $data: ident, $hole: expr, $hole_len: expr) => {{ - use lightning::util::ser::{Readable, Writeable}; - let mut r = ::lightning::io::Cursor::new($data); - if let Ok(msg) = <$MsgType as Readable>::read(&mut r) { + use lightning::util::ser::{LengthReadable, Writeable}; + if let Ok(msg) = + <$MsgType as LengthReadable>::read_from_fixed_length_buffer(&mut &$data[..]) + { let mut w = VecWriter(Vec::new()); msg.write(&mut w).unwrap(); let p = w.0.len() as usize; assert_eq!(msg.serialized_length(), p); assert_eq!(w.0.len(), p); - assert_eq!(&r.get_ref()[..$hole], &w.0[..$hole]); - assert_eq!(&r.get_ref()[$hole + $hole_len..p], &w.0[$hole + $hole_len..]); + assert_eq!(&$data[..$hole], &w.0[..$hole]); + assert_eq!(&$data[$hole + $hole_len..p], &w.0[$hole + $hole_len..]); } }}; } diff --git a/fuzz/src/onion_message.rs b/fuzz/src/onion_message.rs index 71836995017..abdd588f1c1 100644 --- a/fuzz/src/onion_message.rs +++ b/fuzz/src/onion_message.rs @@ -26,20 +26,23 @@ use lightning::onion_message::packet::OnionMessageContents; use lightning::sign::{EntropySource, NodeSigner, Recipient, SignerProvider}; use lightning::types::features::InitFeatures; use lightning::util::logger::Logger; -use lightning::util::ser::{Readable, Writeable, Writer}; +use lightning::util::ser::{LengthReadable, Writeable, Writer}; use lightning::util::test_channel_signer::TestChannelSigner; use lightning_invoice::RawBolt11Invoice; use crate::utils::test_logger; -use lightning::io::{self, Cursor}; +use lightning::io; use std::sync::atomic::{AtomicU64, Ordering}; #[inline] /// Actual fuzz test, method signature and name are fixed pub fn do_test(data: &[u8], logger: &L) { - if let Ok(msg) = ::read(&mut Cursor::new(data)) { + let mut reader = &data[..]; + if let Ok(msg) = + ::read_from_fixed_length_buffer(&mut reader) + { let mut secret_bytes = [1; 32]; secret_bytes[31] = 2; let secret = SecretKey::from_slice(&secret_bytes).unwrap(); diff --git a/lightning/src/ln/msgs.rs b/lightning/src/ln/msgs.rs index 308daa55a6b..f4fc710857c 100644 --- a/lightning/src/ln/msgs.rs +++ b/lightning/src/ln/msgs.rs @@ -2412,8 +2412,8 @@ impl Writeable for AcceptChannel { } } -impl Readable for AcceptChannel { - fn read(r: &mut R) -> Result { +impl LengthReadable for AcceptChannel { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let temporary_channel_id: ChannelId = Readable::read(r)?; let dust_limit_satoshis: u64 = Readable::read(r)?; let max_htlc_value_in_flight_msat: u64 = Readable::read(r)?; @@ -2497,8 +2497,8 @@ impl Writeable for AcceptChannelV2 { } } -impl Readable for AcceptChannelV2 { - fn read(r: &mut R) -> Result { +impl LengthReadable for AcceptChannelV2 { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let temporary_channel_id: ChannelId = Readable::read(r)?; let funding_satoshis: u64 = Readable::read(r)?; let dust_limit_satoshis: u64 = Readable::read(r)?; @@ -2760,8 +2760,8 @@ impl Writeable for Init { } } -impl Readable for Init { - fn read(r: &mut R) -> Result { +impl LengthReadable for Init { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let global_features: InitFeatures = Readable::read(r)?; let features: InitFeatures = Readable::read(r)?; let mut remote_network_address: Option = None; @@ -2806,8 +2806,8 @@ impl Writeable for OpenChannel { } } -impl Readable for OpenChannel { - fn read(r: &mut R) -> Result { +impl LengthReadable for OpenChannel { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let chain_hash: ChainHash = Readable::read(r)?; let temporary_channel_id: ChannelId = Readable::read(r)?; let funding_satoshis: u64 = Readable::read(r)?; @@ -2890,8 +2890,8 @@ impl Writeable for OpenChannelV2 { } } -impl Readable for OpenChannelV2 { - fn read(r: &mut R) -> Result { +impl LengthReadable for OpenChannelV2 { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let chain_hash: ChainHash = Readable::read(r)?; let temporary_channel_id: ChannelId = Readable::read(r)?; let funding_feerate_sat_per_1000_weight: u32 = Readable::read(r)?; @@ -3052,8 +3052,8 @@ impl_writeable_msg!(UpdateAddHTLC, { (65537, skimmed_fee_msat, option) }); -impl Readable for OnionMessage { - fn read(r: &mut R) -> Result { +impl LengthReadable for OnionMessage { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let blinding_point: PublicKey = Readable::read(r)?; let len: u16 = Readable::read(r)?; let mut packet_reader = FixedLengthReader::new(r, len as u64); @@ -3526,8 +3526,8 @@ impl Writeable for Ping { } } -impl Readable for Ping { - fn read(r: &mut R) -> Result { +impl LengthReadable for Ping { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { Ok(Ping { ponglen: Readable::read(r)?, byteslen: { @@ -3546,8 +3546,8 @@ impl Writeable for Pong { } } -impl Readable for Pong { - fn read(r: &mut R) -> Result { +impl LengthReadable for Pong { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { Ok(Pong { byteslen: { let byteslen = Readable::read(r)?; @@ -3680,8 +3680,8 @@ impl Writeable for ErrorMessage { } } -impl Readable for ErrorMessage { - fn read(r: &mut R) -> Result { +impl LengthReadable for ErrorMessage { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { Ok(Self { channel_id: Readable::read(r)?, data: { @@ -3707,8 +3707,8 @@ impl Writeable for WarningMessage { } } -impl Readable for WarningMessage { - fn read(r: &mut R) -> Result { +impl LengthReadable for WarningMessage { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { Ok(Self { channel_id: Readable::read(r)?, data: { @@ -3826,8 +3826,8 @@ impl LengthReadable for NodeAnnouncement { } } -impl Readable for QueryShortChannelIds { - fn read(r: &mut R) -> Result { +impl LengthReadable for QueryShortChannelIds { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let chain_hash: ChainHash = Readable::read(r)?; let encoding_len: u16 = Readable::read(r)?; @@ -3902,8 +3902,8 @@ impl_writeable_msg!(QueryChannelRange, { number_of_blocks }, {}); -impl Readable for ReplyChannelRange { - fn read(r: &mut R) -> Result { +impl LengthReadable for ReplyChannelRange { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { let chain_hash: ChainHash = Readable::read(r)?; let first_blocknum: u32 = Readable::read(r)?; let number_of_blocks: u32 = Readable::read(r)?; @@ -5484,7 +5484,7 @@ mod tests { let encoded_value = reply_channel_range.encode(); assert_eq!(encoded_value, target_value); - reply_channel_range = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); + reply_channel_range = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap(); assert_eq!(reply_channel_range.chain_hash, expected_chain_hash); assert_eq!(reply_channel_range.first_blocknum, 756230); assert_eq!(reply_channel_range.number_of_blocks, 1500); @@ -5494,7 +5494,7 @@ mod tests { assert_eq!(reply_channel_range.short_channel_ids[2], 0x000000000045a6c4); } else { target_value.append(&mut >::from_hex("001601789c636000833e08659309a65878be010010a9023a").unwrap()); - let result: Result = Readable::read(&mut Cursor::new(&target_value[..])); + let result: Result = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]); assert!(result.is_err(), "Expected decode failure with unsupported zlib encoding"); } } @@ -5518,14 +5518,14 @@ mod tests { let encoded_value = query_short_channel_ids.encode(); assert_eq!(encoded_value, target_value); - query_short_channel_ids = Readable::read(&mut Cursor::new(&target_value[..])).unwrap(); + query_short_channel_ids = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]).unwrap(); assert_eq!(query_short_channel_ids.chain_hash, expected_chain_hash); assert_eq!(query_short_channel_ids.short_channel_ids[0], 0x000000000000008e); assert_eq!(query_short_channel_ids.short_channel_ids[1], 0x0000000000003c69); assert_eq!(query_short_channel_ids.short_channel_ids[2], 0x000000000045a6c4); } else { target_value.append(&mut >::from_hex("001601789c636000833e08659309a65878be010010a9023a").unwrap()); - let result: Result = Readable::read(&mut Cursor::new(&target_value[..])); + let result: Result = LengthReadable::read_from_fixed_length_buffer(&mut &target_value[..]); assert!(result.is_err(), "Expected decode failure with unsupported zlib encoding"); } } diff --git a/lightning/src/ln/wire.rs b/lightning/src/ln/wire.rs index 03a5b51e15e..a80e5048a7e 100644 --- a/lightning/src/ln/wire.rs +++ b/lightning/src/ln/wire.rs @@ -257,21 +257,39 @@ where H::Target: CustomMessageReader, { match message_type { - msgs::Init::TYPE => Ok(Message::Init(Readable::read(buffer)?)), - msgs::ErrorMessage::TYPE => Ok(Message::Error(Readable::read(buffer)?)), - msgs::WarningMessage::TYPE => Ok(Message::Warning(Readable::read(buffer)?)), - msgs::Ping::TYPE => Ok(Message::Ping(Readable::read(buffer)?)), - msgs::Pong::TYPE => Ok(Message::Pong(Readable::read(buffer)?)), + msgs::Init::TYPE => { + Ok(Message::Init(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::ErrorMessage::TYPE => { + Ok(Message::Error(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::WarningMessage::TYPE => { + Ok(Message::Warning(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::Ping::TYPE => { + Ok(Message::Ping(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::Pong::TYPE => { + Ok(Message::Pong(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, msgs::PeerStorage::TYPE => { Ok(Message::PeerStorage(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, msgs::PeerStorageRetrieval::TYPE => Ok(Message::PeerStorageRetrieval( LengthReadable::read_from_fixed_length_buffer(buffer)?, )), - msgs::OpenChannel::TYPE => Ok(Message::OpenChannel(Readable::read(buffer)?)), - msgs::OpenChannelV2::TYPE => Ok(Message::OpenChannelV2(Readable::read(buffer)?)), - msgs::AcceptChannel::TYPE => Ok(Message::AcceptChannel(Readable::read(buffer)?)), - msgs::AcceptChannelV2::TYPE => Ok(Message::AcceptChannelV2(Readable::read(buffer)?)), + msgs::OpenChannel::TYPE => { + Ok(Message::OpenChannel(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::OpenChannelV2::TYPE => { + Ok(Message::OpenChannelV2(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::AcceptChannel::TYPE => { + Ok(Message::AcceptChannel(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, + msgs::AcceptChannelV2::TYPE => { + Ok(Message::AcceptChannelV2(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, msgs::FundingCreated::TYPE => { Ok(Message::FundingCreated(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, @@ -329,7 +347,9 @@ where msgs::ClosingSigned::TYPE => { Ok(Message::ClosingSigned(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, - msgs::OnionMessage::TYPE => Ok(Message::OnionMessage(Readable::read(buffer)?)), + msgs::OnionMessage::TYPE => { + Ok(Message::OnionMessage(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, msgs::UpdateAddHTLC::TYPE => { Ok(Message::UpdateAddHTLC(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, @@ -366,16 +386,18 @@ where msgs::ChannelUpdate::TYPE => { Ok(Message::ChannelUpdate(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, - msgs::QueryShortChannelIds::TYPE => { - Ok(Message::QueryShortChannelIds(Readable::read(buffer)?)) - }, + msgs::QueryShortChannelIds::TYPE => Ok(Message::QueryShortChannelIds( + LengthReadable::read_from_fixed_length_buffer(buffer)?, + )), msgs::ReplyShortChannelIdsEnd::TYPE => Ok(Message::ReplyShortChannelIdsEnd( LengthReadable::read_from_fixed_length_buffer(buffer)?, )), msgs::QueryChannelRange::TYPE => { Ok(Message::QueryChannelRange(LengthReadable::read_from_fixed_length_buffer(buffer)?)) }, - msgs::ReplyChannelRange::TYPE => Ok(Message::ReplyChannelRange(Readable::read(buffer)?)), + msgs::ReplyChannelRange::TYPE => { + Ok(Message::ReplyChannelRange(LengthReadable::read_from_fixed_length_buffer(buffer)?)) + }, msgs::GossipTimestampFilter::TYPE => Ok(Message::GossipTimestampFilter( LengthReadable::read_from_fixed_length_buffer(buffer)?, )), From 7e5c80f00abc46babdb88b43e3645372d5d40e23 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 11 Mar 2025 13:37:07 -0400 Subject: [PATCH 7/8] Require length limited reader for custom LN messages When reading a raw LSP message specifically, it will consume the provided reader until it is out of bytes. Readable is not an appropriate trait for this situation, and over the past few commits we've been transitioning any struct that always reads-to-end to require a length limited reader. This is less error-prone because there is no risk of a bug being introduced because a custom message read more data than it was supposed to from the provided reader. --- lightning-custom-message/src/lib.rs | 10 +++++----- lightning-liquidity/src/lsps0/ser.rs | 11 ++++++----- lightning-liquidity/src/manager.rs | 8 +++++--- lightning/src/ln/wire.rs | 2 +- 4 files changed, 17 insertions(+), 14 deletions(-) diff --git a/lightning-custom-message/src/lib.rs b/lightning-custom-message/src/lib.rs index fb0ba191cd3..32d5a9e4389 100644 --- a/lightning-custom-message/src/lib.rs +++ b/lightning-custom-message/src/lib.rs @@ -24,7 +24,7 @@ //! use lightning::ln::peer_handler::CustomMessageHandler; //! use lightning::ln::wire::{CustomMessageReader, self}; //! # use lightning::types::features::{InitFeatures, NodeFeatures}; -//! use lightning::util::ser::Writeable; +//! use lightning::util::ser::{LengthLimitedRead, Writeable}; //! # use lightning::util::ser::Writer; //! //! // Assume that `FooHandler` and `BarHandler` are defined in one crate and `BazHandler` is @@ -52,7 +52,7 @@ //! impl CustomMessageReader for FooHandler { //! // ... //! # type CustomMessage = Foo; -//! # fn read( +//! # fn read( //! # &self, _message_type: u16, _buffer: &mut R //! # ) -> Result, DecodeError> { //! # unimplemented!() @@ -104,7 +104,7 @@ //! impl CustomMessageReader for BarHandler { //! // ... //! # type CustomMessage = Bar; -//! # fn read( +//! # fn read( //! # &self, _message_type: u16, _buffer: &mut R //! # ) -> Result, DecodeError> { //! # unimplemented!() @@ -156,7 +156,7 @@ //! impl CustomMessageReader for BazHandler { //! // ... //! # type CustomMessage = Baz; -//! # fn read( +//! # fn read( //! # &self, _message_type: u16, _buffer: &mut R //! # ) -> Result, DecodeError> { //! # unimplemented!() @@ -340,7 +340,7 @@ macro_rules! composite_custom_message_handler { impl $crate::lightning::ln::wire::CustomMessageReader for $handler { type CustomMessage = $message; - fn read( + fn read( &self, message_type: u16, buffer: &mut R ) -> Result, $crate::lightning::ln::msgs::DecodeError> { match message_type { diff --git a/lightning-liquidity/src/lsps0/ser.rs b/lightning-liquidity/src/lsps0/ser.rs index a0cb6bfb143..36b31540058 100644 --- a/lightning-liquidity/src/lsps0/ser.rs +++ b/lightning-liquidity/src/lsps0/ser.rs @@ -18,9 +18,9 @@ use crate::lsps2::msgs::{ }; use crate::prelude::{HashMap, String}; -use lightning::ln::msgs::LightningError; +use lightning::ln::msgs::{DecodeError, LightningError}; use lightning::ln::wire; -use lightning::util::ser::WithoutLength; +use lightning::util::ser::{LengthLimitedRead, LengthReadable, WithoutLength}; use bitcoin::secp256k1::PublicKey; @@ -168,9 +168,10 @@ impl lightning::util::ser::Writeable for RawLSPSMessage { } } -impl lightning::util::ser::Readable for RawLSPSMessage { - fn read(r: &mut R) -> Result { - let payload_without_length = WithoutLength::read(r)?; +impl LengthReadable for RawLSPSMessage { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { + let payload_without_length: WithoutLength = + LengthReadable::read_from_fixed_length_buffer(r)?; Ok(Self { payload: payload_without_length.0 }) } } diff --git a/lightning-liquidity/src/manager.rs b/lightning-liquidity/src/manager.rs index afaac25cdd2..63726cc1f77 100644 --- a/lightning-liquidity/src/manager.rs +++ b/lightning-liquidity/src/manager.rs @@ -27,7 +27,7 @@ use lightning::ln::peer_handler::CustomMessageHandler; use lightning::ln::wire::CustomMessageReader; use lightning::sign::EntropySource; use lightning::util::logger::Level; -use lightning::util::ser::Readable; +use lightning::util::ser::{LengthLimitedRead, LengthReadable}; use lightning_types::features::{InitFeatures, NodeFeatures}; @@ -449,11 +449,13 @@ where { type CustomMessage = RawLSPSMessage; - fn read( + fn read( &self, message_type: u16, buffer: &mut RD, ) -> Result, lightning::ln::msgs::DecodeError> { match message_type { - LSPS_MESSAGE_TYPE_ID => Ok(Some(RawLSPSMessage::read(buffer)?)), + LSPS_MESSAGE_TYPE_ID => { + Ok(Some(RawLSPSMessage::read_from_fixed_length_buffer(buffer)?)) + }, _ => Ok(None), } } diff --git a/lightning/src/ln/wire.rs b/lightning/src/ln/wire.rs index a80e5048a7e..1bb7c7448a8 100644 --- a/lightning/src/ln/wire.rs +++ b/lightning/src/ln/wire.rs @@ -25,7 +25,7 @@ pub trait CustomMessageReader { /// implementation and the message could be decoded, must return `Ok(Some(message))`. If the /// message type is unknown to the implementation, must return `Ok(None)`. If a decoding error /// occur, must return `Err(DecodeError::X)` where `X` details the encountered error. - fn read( + fn read( &self, message_type: u16, buffer: &mut R, ) -> Result, msgs::DecodeError>; } From 94f89521ace71b38661b3fc49abdb3dd0db5d433 Mon Sep 17 00:00:00 2001 From: Valentine Wallace Date: Tue, 11 Mar 2025 17:55:29 -0400 Subject: [PATCH 8/8] Require length limiting reader for offers messages Continuing the work over the past few commits, we want to transition any structs that always consume the entire provided reader when being deserialized to require a length-limiting reader. To do this we actually require all the offers' underlying ser wrappers to be read from a length-limiting reader as well, since these wrappers will always read-to-end in general. Some of the ser macros needed updating for this, which is fine because all TLV values were already read from a length-limiting reader. --- lightning/src/offers/invoice.rs | 12 +++++++----- lightning/src/offers/invoice_request.rs | 9 +++++---- lightning/src/offers/offer.rs | 11 +++++++---- lightning/src/offers/refund.rs | 12 ++++++++---- lightning/src/util/ser.rs | 24 +++++++++++++----------- lightning/src/util/ser_macros.rs | 4 ++-- 6 files changed, 42 insertions(+), 30 deletions(-) diff --git a/lightning/src/offers/invoice.rs b/lightning/src/offers/invoice.rs index be3886346c5..5d49f1a7be3 100644 --- a/lightning/src/offers/invoice.rs +++ b/lightning/src/offers/invoice.rs @@ -148,8 +148,8 @@ use crate::offers::signer::{self, Metadata}; use crate::types::features::{Bolt12InvoiceFeatures, InvoiceRequestFeatures, OfferFeatures}; use crate::types::payment::PaymentHash; use crate::util::ser::{ - CursorReadable, HighZeroBytesDroppedBigSize, Iterable, Readable, WithoutLength, Writeable, - Writer, + CursorReadable, HighZeroBytesDroppedBigSize, Iterable, LengthLimitedRead, LengthReadable, + WithoutLength, Writeable, Writer, }; use crate::util::string::PrintableString; use bitcoin::address::Address; @@ -1398,9 +1398,11 @@ impl Writeable for Bolt12Invoice { } } -impl Readable for Bolt12Invoice { - fn read(reader: &mut R) -> Result { - let bytes: WithoutLength> = Readable::read(reader)?; +impl LengthReadable for Bolt12Invoice { + fn read_from_fixed_length_buffer( + reader: &mut R, + ) -> Result { + let bytes: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(reader)?; Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue) } } diff --git a/lightning/src/offers/invoice_request.rs b/lightning/src/offers/invoice_request.rs index a374a055e70..ff64f85f46a 100644 --- a/lightning/src/offers/invoice_request.rs +++ b/lightning/src/offers/invoice_request.rs @@ -86,7 +86,8 @@ use crate::onion_message::dns_resolution::HumanReadableName; use crate::types::features::InvoiceRequestFeatures; use crate::types::payment::PaymentHash; use crate::util::ser::{ - CursorReadable, HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer, + CursorReadable, HighZeroBytesDroppedBigSize, LengthLimitedRead, LengthReadable, Readable, + WithoutLength, Writeable, Writer, }; use crate::util::string::{PrintableString, UntrustedString}; use bitcoin::constants::ChainHash; @@ -1119,9 +1120,9 @@ impl Writeable for InvoiceRequestContents { } } -impl Readable for InvoiceRequest { - fn read(reader: &mut R) -> Result { - let bytes: WithoutLength> = Readable::read(reader)?; +impl LengthReadable for InvoiceRequest { + fn read_from_fixed_length_buffer(r: &mut R) -> Result { + let bytes: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(r)?; Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue) } } diff --git a/lightning/src/offers/offer.rs b/lightning/src/offers/offer.rs index e9a2d9428e2..52aa782da1d 100644 --- a/lightning/src/offers/offer.rs +++ b/lightning/src/offers/offer.rs @@ -88,7 +88,8 @@ use crate::offers::parse::{Bech32Encode, Bolt12ParseError, Bolt12SemanticError, use crate::offers::signer::{self, Metadata, MetadataMaterial}; use crate::types::features::OfferFeatures; use crate::util::ser::{ - CursorReadable, HighZeroBytesDroppedBigSize, Readable, WithoutLength, Writeable, Writer, + CursorReadable, HighZeroBytesDroppedBigSize, LengthLimitedRead, LengthReadable, Readable, + WithoutLength, Writeable, Writer, }; use crate::util::string::PrintableString; use bitcoin::constants::ChainHash; @@ -1033,9 +1034,11 @@ impl OfferContents { } } -impl Readable for Offer { - fn read(reader: &mut R) -> Result { - let bytes: WithoutLength> = Readable::read(reader)?; +impl LengthReadable for Offer { + fn read_from_fixed_length_buffer( + reader: &mut R, + ) -> Result { + let bytes: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(reader)?; Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue) } } diff --git a/lightning/src/offers/refund.rs b/lightning/src/offers/refund.rs index 56f3cfa01cf..c5c4f498002 100644 --- a/lightning/src/offers/refund.rs +++ b/lightning/src/offers/refund.rs @@ -102,7 +102,9 @@ use crate::offers::signer::{self, Metadata, MetadataMaterial}; use crate::sign::EntropySource; use crate::types::features::InvoiceRequestFeatures; use crate::types::payment::PaymentHash; -use crate::util::ser::{CursorReadable, Readable, WithoutLength, Writeable, Writer}; +use crate::util::ser::{ + CursorReadable, LengthLimitedRead, LengthReadable, WithoutLength, Writeable, Writer, +}; use crate::util::string::PrintableString; use bitcoin::constants::ChainHash; use bitcoin::network::Network; @@ -822,9 +824,11 @@ impl RefundContents { } } -impl Readable for Refund { - fn read(reader: &mut R) -> Result { - let bytes: WithoutLength> = Readable::read(reader)?; +impl LengthReadable for Refund { + fn read_from_fixed_length_buffer( + reader: &mut R, + ) -> Result { + let bytes: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(reader)?; Self::try_from(bytes.0).map_err(|_| DecodeError::InvalidValue) } } diff --git a/lightning/src/util/ser.rs b/lightning/src/util/ser.rs index 181eb5189ac..8a706a8cbdf 100644 --- a/lightning/src/util/ser.rs +++ b/lightning/src/util/ser.rs @@ -740,10 +740,10 @@ impl Writeable for WithoutLength<&String> { w.write_all(self.0.as_bytes()) } } -impl Readable for WithoutLength { +impl LengthReadable for WithoutLength { #[inline] - fn read(r: &mut R) -> Result { - let v: WithoutLength> = Readable::read(r)?; + fn read_from_fixed_length_buffer(r: &mut R) -> Result { + let v: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(r)?; Ok(Self(String::from_utf8(v.0).map_err(|_| DecodeError::InvalidValue)?)) } } @@ -772,10 +772,10 @@ impl Writeable for WithoutLength<&UntrustedString> { WithoutLength(&self.0 .0).write(w) } } -impl Readable for WithoutLength { +impl LengthReadable for WithoutLength { #[inline] - fn read(r: &mut R) -> Result { - let s: WithoutLength = Readable::read(r)?; + fn read_from_fixed_length_buffer(r: &mut R) -> Result { + let s: WithoutLength = LengthReadable::read_from_fixed_length_buffer(r)?; Ok(Self(UntrustedString(s.0))) } } @@ -808,9 +808,11 @@ impl Writeable for WithoutLength { } } -impl Readable for WithoutLength> { +impl LengthReadable for WithoutLength> { #[inline] - fn read(reader: &mut R) -> Result { + fn read_from_fixed_length_buffer( + reader: &mut R, + ) -> Result { let mut values = Vec::new(); loop { let mut track_read = ReadTrackingReader::new(reader); @@ -841,10 +843,10 @@ impl Writeable for WithoutLength<&ScriptBuf> { } } -impl Readable for WithoutLength { +impl LengthReadable for WithoutLength { #[inline] - fn read(r: &mut R) -> Result { - let v: WithoutLength> = Readable::read(r)?; + fn read_from_fixed_length_buffer(r: &mut R) -> Result { + let v: WithoutLength> = LengthReadable::read_from_fixed_length_buffer(r)?; Ok(WithoutLength(script::Builder::from(v.0).into_script())) } } diff --git a/lightning/src/util/ser_macros.rs b/lightning/src/util/ser_macros.rs index 7497b3d4d13..56779e24a05 100644 --- a/lightning/src/util/ser_macros.rs +++ b/lightning/src/util/ser_macros.rs @@ -409,7 +409,7 @@ macro_rules! _decode_tlv { $field = $trait::read(&mut $reader $(, $read_arg)*)?; }}; ($outer_reader: expr, $reader: expr, $field: ident, required_vec) => {{ - let f: $crate::util::ser::WithoutLength> = $crate::util::ser::Readable::read(&mut $reader)?; + let f: $crate::util::ser::WithoutLength> = $crate::util::ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?; $field = f.0; }}; ($outer_reader: expr, $reader: expr, $field: ident, option) => {{ @@ -427,7 +427,7 @@ macro_rules! _decode_tlv { _decode_tlv!($outer_reader, $reader, $field, required); }}; ($outer_reader: expr, $reader: expr, $field: ident, optional_vec) => {{ - let f: $crate::util::ser::WithoutLength> = $crate::util::ser::Readable::read(&mut $reader)?; + let f: $crate::util::ser::WithoutLength> = $crate::util::ser::LengthReadable::read_from_fixed_length_buffer(&mut $reader)?; $field = Some(f.0); }}; // `upgradable_required` indicates we're reading a required TLV that may have been upgraded