Skip to content

Require LengthLimitedRead for structs that always drain reader #3640

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Mar 13, 2025
Merged

Require LengthLimitedRead for structs that always drain reader #3640

Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
f5fc13b
Refactor LengthRead to also limit number of bytes read
valentinewallace Mar 6, 2025
16fdd33
Implement LengthLimitedRead for &[u8]
valentinewallace Mar 7, 2025
e9399d7
Require length limited reader in impl_writeable_msg
valentinewallace Mar 3, 2025
50745bf
Rustfmt wire.rs
valentinewallace Mar 7, 2025
6f96d02
Require length limited reader for LN gossip messages
valentinewallace Mar 11, 2025
7b1bb4a
Require length limited reader for all LN messages
valentinewallace Mar 11, 2025
7e5c80f
Require length limited reader for custom LN messages
valentinewallace Mar 11, 2025
94f8952
Require length limiting reader for offers messages
valentinewallace Mar 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
130 changes: 92 additions & 38 deletions lightning/src/ln/wire.rs
View file
Open in desktop
Copy link
Contributor

@arik-so arik-so Mar 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this commit remain or be squashed out pre-merge?

Copy link
Collaborator

@TheBlueMatt TheBlueMatt Mar 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you're referring to the rustfmtone? No reason to squash, IMO. Anything that helps a reviewer now helps a reviewer trying to understand the code in three years and it doesn't violate our "each commit should, on its own, build and pass tests" rule.

Original file line number Diff line number Diff line change
Expand Up @@ -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)?))
},
Expand All @@ -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))
Expand Down