Require FixedLengthReader in LengthReadable

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 LengthReadable 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 refactor LengthReadable to require a fixed length
reader.

Upcoming commits will switch read-to-end structs that are currently read with
Readable to use LengthReadable instead.

Author: valentinewallace

lightning/src/ln/msgs.rs

@@ -2006,7 +2006,7 @@ impl Writeable for TrampolineOnionPacket {
 }
 
 impl LengthReadable for TrampolineOnionPacket {
-	fn read_from_fixed_length_buffer<R: LengthRead>(r: &mut R) -> Result<Self, DecodeError> {
+	fn read_from_fixed_length_buffer<'a, R: Read>(r: &mut FixedLengthReader<'a, R>) -> Result<Self, DecodeError> {
 		let version = Readable::read(r)?;
 		let public_key = Readable::read(r)?;
 

lightning/src/onion_message/packet.rs

@@ -84,7 +84,9 @@ impl Writeable for Packet {
 }
 
 impl LengthReadable for Packet {
-	fn read_from_fixed_length_buffer<R: LengthRead>(r: &mut R) -> Result<Self, DecodeError> {
+	fn read_from_fixed_length_buffer<'a, R: Read>(
+		r: &mut FixedLengthReader<'a, R>,
+	) -> Result<Self, DecodeError> {
 		const READ_BUFFER_SIZE: usize = 4096;
 
 		let version = Readable::read(r)?;

lightning/src/util/ser.rs

@@ -361,7 +361,7 @@ where
 	fn read<R: LengthRead>(reader: &mut R, params: P) -> Result<Self, DecodeError>;
 }
 
-/// A trait that allows the implementer to be read in from a [`LengthRead`], requiring
+/// A trait that allows the implementer to be read in from a [`FixedLengthReader`], requiring
 /// the reader to provide the total length of the read.
 ///
 /// Any type that implements [`Readable`] also automatically has a [`LengthReadable`]
@@ -370,13 +370,17 @@ pub trait LengthReadable
 where
 	Self: Sized,
 {
-	/// Reads a `Self` in from the given [`LengthRead`].
-	fn read_from_fixed_length_buffer<R: LengthRead>(reader: &mut R) -> Result<Self, DecodeError>;
+	/// Reads a `Self` in from the given [`FixedLengthReader`].
+	fn read_from_fixed_length_buffer<'a, R: Read>(
+		reader: &mut FixedLengthReader<'a, R>,
+	) -> Result<Self, DecodeError>;
 }
 
 impl<T: Readable> LengthReadable for T {
 	#[inline]
-	fn read_from_fixed_length_buffer<R: LengthRead>(reader: &mut R) -> Result<T, DecodeError> {
+	fn read_from_fixed_length_buffer<'a, R: Read>(
+		reader: &mut FixedLengthReader<'a, R>,
+	) -> Result<T, DecodeError> {
 		Readable::read(reader)
 	}
 }
@@ -405,7 +409,9 @@ impl<T: Readable> MaybeReadable for T {
 pub struct RequiredWrapper<T>(pub Option<T>);
 impl<T: LengthReadable> LengthReadable for RequiredWrapper<T> {
 	#[inline]
-	fn read_from_fixed_length_buffer<R: LengthRead>(reader: &mut R) -> Result<Self, DecodeError> {
+	fn read_from_fixed_length_buffer<'a, R: Read>(
+		reader: &mut FixedLengthReader<'a, R>,
+	) -> Result<Self, DecodeError> {
 		Ok(Self(Some(LengthReadable::read_from_fixed_length_buffer(reader)?)))
 	}
 }