-
Notifications
You must be signed in to change notification settings - Fork 405
Implement Readable for LengthReadable #3579
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -344,8 +344,8 @@ where | |
fn read<R: Read>(reader: &mut R, params: P) -> Result<Self, DecodeError>; | ||
} | ||
|
||
/// A [`std::io::Read`] that also provides the total bytes available to be read. | ||
pub(crate) trait LengthRead: Read { | ||
/// A [`io::Read`] that also provides the total bytes available to be read. | ||
pub trait LengthRead: Read { | ||
/// The total number of bytes available to be read. | ||
fn total_bytes(&self) -> u64; | ||
} | ||
|
@@ -361,14 +361,24 @@ where | |
fn read<R: LengthRead>(reader: &mut R, params: P) -> Result<Self, DecodeError>; | ||
} | ||
|
||
/// A trait that various higher-level LDK types implement allowing them to be read in | ||
/// from a [`Read`], requiring the implementer to provide the total length of the read. | ||
pub(crate) trait LengthReadable | ||
/// A trait that allows the implementer to be read in from a [`LengthRead`], requiring | ||
/// the reader to provide the total length of the read. | ||
/// | ||
/// Any type that implements [`Readable`] also automatically has a [`LengthReadable`] | ||
/// implementation, but some types, most notably onion packets, only implement [`LengthReadable`]. | ||
pub trait LengthReadable | ||
where | ||
Self: Sized, | ||
{ | ||
/// Reads a `Self` in from the given [`LengthRead`]. | ||
fn read<R: LengthRead>(reader: &mut R) -> Result<Self, DecodeError>; | ||
fn read_from_fixed_length_buffer<R: LengthRead>(reader: &mut R) -> Result<Self, DecodeError>; | ||
} | ||
|
||
impl<T: Readable> LengthReadable for T { | ||
arik-so marked this conversation as resolved.
Show resolved
Hide resolved
|
||
#[inline] | ||
fn read_from_fixed_length_buffer<R: LengthRead>(reader: &mut R) -> Result<T, DecodeError> { | ||
Readable::read(reader) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should this fail if the expected length wasn't read? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given that without resorting to assert_eq, it would produce pretty ubiquitous overhead, we agreed not to |
||
} | ||
} | ||
|
||
/// A trait that various LDK types implement allowing them to (maybe) be read in from a [`Read`]. | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hello! ($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)
};
}}; Would love to understand it. Thanks! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I think this is the last missing one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. CI is currently running with it added There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. squashed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably could update the docs since its now pub