p2p: Wrap V1MessageHeaderDecoderError with V1NetworkMessage Header err
What the developer wrote
Authored by Mitchell Bagot
p2p: Wrap V1MessageHeaderDecoderError with V1NetworkMessage Header err
The V1NetworkMessageDecoderErrorInner has a Header error variant with
no internal information. This makes it less useful to decode a
V1NetworkMessage with the V1NetworkMessageDecoder than a
V1MessageHeaderDecoder in the case of an error. In order that users can
still retrieve meaningful error information, the header's error should
be wrapped by the Header error variant.
Add internal V1MessageHeaderDecoderError for
V1NetworkMessageDecoderErrorInner::Header variant.
What changed, and why it matters
This change improves error messages when decoding Bitcoin peer-to-peer network messages. Previously, if the message header failed to decode, the higher-level decoder would report a generic 'header error' with no details. Now it wraps and preserves the underlying header decoder error so callers can see what specifically went wrong. There is no security vulnerability being fixed here; it is a usability and diagnostics improvement.
No security action required. Treat as a normal code-quality/diagnostics patch. Reviewers may verify that the new error variant is public API-stable if this crate has stability commitments.
Security signals we found
No security-relevant logic change
Error propagation improvement only
No validation bypass or memory safety change
Evidence from the diff
The commit modifies V1NetworkMessageDecoderErrorInner::Header to carry a V1MessageHeaderDecoderError instead of being an empty variant. It updates all construction sites in p2p/src/message.rs to propagate the wrapped error, and adjusts Display and std::error::Error::source implementations to expose the cause. A missing std::error::Error impl for V1MessageHeaderDecoderError is also added. This is a non-breaking API quality change in the error type; it does not alter decoding logic, validation, or behavior beyond error reporting.
Changed components
p2p/src/message.rsV1NetworkMessageDecoderV1NetworkMessageDecoderErrorInnerV1MessageHeaderDecoderErrorInspect captured patch +18 / −10
diff --git a/p2p/src/message.rs b/p2p/src/message.rs
index 0bb375f1..01c6a679 100644
--- a/p2p/src/message.rs
+++ b/p2p/src/message.rs
@@ -1400,8 +1400,8 @@ impl encoding::Decoder for V1NetworkMessageDecoder {
fn push_bytes(&mut self, bytes: &mut &[u8]) -> Result<bool, Self::Error> {
match &mut self.state {
DecoderState::ReadingHeader { header_decoder } => {
- let need_more = header_decoder.push_bytes(bytes).map_err(|_| {
- V1NetworkMessageDecoderError(V1NetworkMessageDecoderErrorInner::Header)
+ let need_more = header_decoder.push_bytes(bytes).map_err(|e| {
+ V1NetworkMessageDecoderError(V1NetworkMessageDecoderErrorInner::Header(e))
})?;
if !need_more {
@@ -1417,8 +1417,8 @@ impl encoding::Decoder for V1NetworkMessageDecoder {
unreachable!("we are in ReadingHeader state")
};
- let header = header_decoder.end().map_err(|_| {
- V1NetworkMessageDecoderError(V1NetworkMessageDecoderErrorInner::Header)
+ let header = header_decoder.end().map_err(|e| {
+ V1NetworkMessageDecoderError(V1NetworkMessageDecoderErrorInner::Header(e))
})?;
let payload_len = usize::try_from(header.length)
.expect("u32 -> usize cast ok for >= 32-bit platforms");
@@ -1449,8 +1449,11 @@ impl encoding::Decoder for V1NetworkMessageDecoder {
fn end(self) -> Result<Self::Output, Self::Error> {
match self.state {
- DecoderState::ReadingHeader { .. } =>
- Err(V1NetworkMessageDecoderError(V1NetworkMessageDecoderErrorInner::Header)),
+ DecoderState::ReadingHeader { header_decoder } => Err(header_decoder
+ .end()
+ .map_err(V1NetworkMessageDecoderErrorInner::Header)
+ .map_err(V1NetworkMessageDecoderError)
+ .expect_err("push_bytes() moves to ReadingPayload on header_decoder completion")),
DecoderState::ReadingPayload { magic, length, checksum, payload_decoder, .. } => {
let payload = payload_decoder.end()?;
let (_, expected_checksum) = sha2_checksum(&payload);
@@ -1985,6 +1988,11 @@ pub mod error {
}
}
+ #[cfg(feature = "std")]
+ impl std::error::Error for V1MessageHeaderDecoderError {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) }
+ }
+
/// An error decoding a [`InventoryPayload`].
///
/// [`InventoryPayload`]: super::InventoryPayload
@@ -2103,7 +2111,7 @@ pub mod error {
#[derive(Debug, Clone, PartialEq, Eq)]
pub(super) enum V1NetworkMessageDecoderErrorInner {
/// Error decoding the message header.
- Header,
+ Header(V1MessageHeaderDecoderError),
/// Payload length exceeds maximum allowed message size.
PayloadTooLarge,
/// Error decoding the message payload.
@@ -2117,8 +2125,8 @@ pub mod error {
impl fmt::Display for V1NetworkMessageDecoderError {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self.0 {
- V1NetworkMessageDecoderErrorInner::Header => {
- write!(f, "error decoding message header")
+ V1NetworkMessageDecoderErrorInner::Header(ref e) => {
+ write_err!(f, "error decoding message header"; e)
}
V1NetworkMessageDecoderErrorInner::PayloadTooLarge => {
write!(f, "payload length exceeds maximum allowed message size")
@@ -2141,7 +2149,7 @@ pub mod error {
impl std::error::Error for V1NetworkMessageDecoderError {
fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
match self.0 {
- V1NetworkMessageDecoderErrorInner::Header => None,
+ V1NetworkMessageDecoderErrorInner::Header(ref e) => Some(e),
V1NetworkMessageDecoderErrorInner::PayloadTooLarge => None,
V1NetworkMessageDecoderErrorInner::Payload => None,
V1NetworkMessageDecoderErrorInner::InvalidChecksum { expected: _, actual: _ } =>
Why this scored 19/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.