p2p: Remove legacy encoding from `message_network`
What changed, and why it matters
This commit removes old, unused Bitcoin network message encoding code from the rust-bitcoin p2p library. It deletes fallback implementations that were kept alongside newer encoding logic. There is no direct evidence this fixes an active security bug, but removing redundant legacy code reduces the attack surface and the chance that outdated, potentially unsafe code paths could be accidentally used in the future.
Treat as a normal refactor/cleanup commit. Reviewers should verify that the newer `encoding` module implementations provide equivalent or stronger input validation than the removed legacy implementations, especially for variable-length fields such as user agent bytes and alert payloads. No urgent security action is indicated by the commit itself.
Security signals we found
Removal of legacy encoding code paths reduces duplicate deserialization surface
Deleted macro included an OOM-protection comment for untrusted compact-size vector lengths
No replacement of unsafe logic with equivalent safe logic is visible in the diff
No advisory, CVE, or security-relevant commit message wording is present
Evidence from the diff
The patch deletes the impl_vec_wrapper! macro and several impl_consensus_encoding! invocations for VersionMessage, UserAgent, Reject, and Alert, plus a manual Encodable/Decodable implementation for RejectReason. These were legacy consensus-encoding implementations in p2p/src/message_network.rs and the supporting macro in p2p/src/consensus.rs. The code now relies on the newer encoding module’s Encode/Decode traits. The removed impl_vec_wrapper! included an allocation-capacity cap (8,000 bytes) for untrusted compact-size lengths, but that macro was only used for Alert, which wraps a Vec<u8>. The change appears to be a code-cleanup refactor rather than a targeted vulnerability fix.
Changed components
p2p/src/message_network.rsp2p/src/consensus.rsVersionMessageUserAgentRejectRejectReasonAlertInspect captured patch +0 / −89
diff --git a/p2p/src/consensus.rs b/p2p/src/consensus.rs
index 5bc377d7..6a0a3c2f 100644
--- a/p2p/src/consensus.rs
+++ b/p2p/src/consensus.rs
@@ -55,47 +55,3 @@ macro_rules! impl_consensus_encoding {
);
}
pub(crate) use impl_consensus_encoding;
-
-#[cfg(feature = "std")]
-macro_rules! impl_vec_wrapper {
- ($wrapper: ident, $type: ty) => {
- impl bitcoin::consensus::encode::Encodable for $wrapper {
- #[inline]
- fn consensus_encode<W: io::Write + ?Sized>(
- &self,
- w: &mut W,
- ) -> core::result::Result<usize, io::Error> {
- let mut len = 0;
- len += w.emit_compact_size(self.0.len())?;
- for c in self.0.iter() {
- len += c.consensus_encode(w)?;
- }
- Ok(len)
- }
- }
-
- impl bitcoin::consensus::encode::Decodable for $wrapper {
- #[inline]
- fn consensus_decode_from_finite_reader<R: io::BufRead + ?Sized>(
- r: &mut R,
- ) -> core::result::Result<$wrapper, bitcoin::consensus::encode::Error> {
- let len = r.read_compact_size()?;
- // Limit the initial vec allocation to at most 8,000 bytes, which is
- // sufficient for most use cases. We don't allocate more space upfront
- // than this, since `len` is an untrusted allocation capacity. If the
- // vector does overflow the initial capacity `push` will just reallocate.
- // Note: OOM protection relies on reader eventually running out of
- // data to feed us.
- let max_init_capacity = 8000 / core::mem::size_of::<$type>();
- let mut ret = Vec::with_capacity(core::cmp::min(len as usize, max_init_capacity));
- for _ in 0..len {
- ret.push(Decodable::consensus_decode_from_finite_reader(r)?);
- }
- Ok($wrapper(ret))
- }
- }
- };
-}
-
-#[cfg(feature = "std")]
-pub(crate) use impl_vec_wrapper;
diff --git a/p2p/src/message_network.rs b/p2p/src/message_network.rs
index 3361ed41..2a560bba 100644
--- a/p2p/src/message_network.rs
+++ b/p2p/src/message_network.rs
@@ -12,16 +12,13 @@ use alloc::vec::Vec;
#[cfg(feature = "arbitrary")]
use arbitrary::{Arbitrary, Unstructured};
-use bitcoin::consensus::{encode, Decodable, Encodable, ReadExt, WriteExt};
use encoding::{
ArrayDecoder, ArrayEncoder, ByteVecDecoder, BytesEncoder, CompactSizeEncoder, Decoder4,
Encoder2, Encoder4,
};
use hashes::sha256d;
-use io::{BufRead, Write};
use crate::address::{Address, AddressDecoder};
-use crate::consensus::{impl_consensus_encoding, impl_vec_wrapper};
use crate::{ProtocolVersion, ServiceFlags};
#[rustfmt::skip] // Keep public re-exports separate.
@@ -216,19 +213,6 @@ type VersionMessageInnerDecoder = encoding::Decoder2<
#[derive(Debug, Clone)]
pub struct VersionMessageDecoder(VersionMessageInnerDecoder);
-impl_consensus_encoding!(
- VersionMessage,
- version,
- services,
- timestamp,
- receiver,
- sender,
- nonce,
- user_agent,
- start_height,
- relay
-);
-
/// A bitcoin user agent defined by BIP-0014. The user agent is sent in the version message when a
/// connection between two peers is established. It is intended to advertise client software in a
/// well-defined format.
@@ -289,8 +273,6 @@ impl encoding::Decode for UserAgent {
fn decoder() -> Self::Decoder { UserAgentDecoder(UserAgentInnerDecoder::new()) }
}
-impl_consensus_encoding!(UserAgent, user_agent);
-
impl UserAgent {
const MAX_USER_AGENT_LEN: usize = 256;
@@ -509,29 +491,6 @@ impl encoding::Decode for RejectReason {
fn decoder() -> Self::Decoder { RejectReasonDecoder(ArrayDecoder::new()) }
}
-impl Encodable for RejectReason {
- fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
- w.write_all(&[*self as u8])?;
- Ok(1)
- }
-}
-
-impl Decodable for RejectReason {
- fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
- Ok(match r.read_u8()? {
- 0x01 => Self::Malformed,
- 0x10 => Self::Invalid,
- 0x11 => Self::Obsolete,
- 0x12 => Self::Duplicate,
- 0x40 => Self::NonStandard,
- 0x41 => Self::Dust,
- 0x42 => Self::Fee,
- 0x43 => Self::Checkpoint,
- _ => return Err(crate::consensus::parse_failed_error("unknown reject code")),
- })
- }
-}
-
/// Reject message might be sent by peers rejecting one of our messages
#[derive(PartialEq, Eq, Clone, Debug)]
pub struct Reject {
@@ -623,8 +582,6 @@ impl encoding::Decode for Reject {
}
}
-impl_consensus_encoding!(Reject, message, ccode, reason, hash);
-
/// A deprecated message type that was used to notify users of system changes. Due to a number of
/// vulnerabilities, alerts are no longer used. A final alert was sent as of Bitcoin Core 0.14.0,
/// and is sent to any node that is advertising a potentially vulnerable protocol version.
@@ -695,8 +652,6 @@ impl encoding::Decode for Alert {
fn decoder() -> Self::Decoder { AlertDecoder(AlertInnerDecoder::new()) }
}
-impl_vec_wrapper!(Alert, Vec<u8>);
-
/// Error types for network messages.
pub mod error {
use core::convert::Infallible;
Why this scored 16/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.