p2p: Implement `encoding` traits for `HeaderAndShortIds`
What the developer wrote
Authored by rustaceanrob
p2p: Implement `encoding` traits for `HeaderAndShortIds`
The overflow check is preserved in the decoding, which I see was added
recently. Although, I don't claim to fully understand why this implies the
individual differences between indexes also overflow.
Clippy complained about a complex type so I introduced an alias.
What changed, and why it matters
This commit adds new encoding and decoding machinery for a Bitcoin compact-block-relay data structure called HeaderAndShortIds. It preserves an existing safety check that prevents the combined number of short transaction IDs and prefilled transactions from overflowing a 16-bit limit. The change is mostly a refactor to use newer generic encoder traits; it does not appear to introduce a new vulnerability, but the author notes they do not fully understand why the overflow check guarantees safety of individual index differences.
Treat as a low-risk refactor. Review the legacy `Decodable` implementation to ensure both old and new decoding paths enforce identical limits. Consider adding unit tests that exercise the `IndexOverflow` boundary and clarify in code comments why the combined-length check prevents per-index-difference overflow, addressing the author's noted uncertainty.
Security signals we found
Preserved overflow check on combined short_ids + prefilled_txs length during decoding
New explicit IndexOverflow error variant for decoder failures
Author self-reports incomplete understanding of why overflow check is sufficient
Legacy consensus_decode implementation still present; patch is additive, not a full replacement
No removal of existing validation logic observed in the diff
Evidence from the diff
The patch implements the new Encodable/Decodable traits for HeaderAndShortIds in p2p/src/bip152.rs using Encoder4/Decoder4 combinators. Encoding writes the block header, an 8-byte nonce, a compact-size prefixed slice of short IDs, and a compact-size prefixed slice of prefilled transactions. Decoding mirrors this and retains the existing short_ids.len().checked_add(prefilled_txs.len()) overflow guard, returning a new IndexOverflow error if the sum exceeds u16::MAX. The legacy Decodable implementation remains below the diff cut-off and is not removed. The commit message explicitly states the author does not fully understand the security implication of the overflow check.
Changed components
p2p/src/bip152.rsHeaderAndShortIdsHeaderAndShortIdsEncoderHeaderAndShortIdsDecoderBIP-152 compact block relay decoding pathInspect captured patch +124 / −2
diff --git a/p2p/src/bip152.rs b/p2p/src/bip152.rs
index c2a26909..e44db477 100644
--- a/p2p/src/bip152.rs
+++ b/p2p/src/bip152.rs
@@ -14,12 +14,12 @@ use std::error;
use arbitrary::{Arbitrary, Unstructured};
use bitcoin::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt};
use bitcoin::{block, Block, BlockChecked, BlockHash, Transaction};
-use encoding::{ArrayDecoder, ArrayEncoder, CompactSizeDecoder, CompactSizeEncoder, Decoder2, Encoder2, SliceEncoder, VecDecoder};
+use encoding::{ArrayDecoder, ArrayEncoder, CompactSizeDecoder, CompactSizeEncoder, Decoder2, Decoder4, Encoder2, Encoder4, SliceEncoder, VecDecoder};
use hashes::{sha256, siphash24};
use internals::array::ArrayExt as _;
use internals::write_err;
use io::{BufRead, Write};
-use primitives::block::{BlockHashDecoder, BlockHashEncoder};
+use primitives::block::{Header, BlockHashDecoder, BlockHashEncoder, HeaderDecoder, HeaderEncoder};
use primitives::transaction::{TransactionDecoder, TransactionEncoder};
/// A BIP-0152 error
@@ -361,6 +361,128 @@ pub struct HeaderAndShortIds {
pub prefilled_txs: Vec<PrefilledTransaction>,
}
+type HeaderAndShortIdsInnerEncoder<'e> = Encoder4<
+ HeaderEncoder<'e>,
+ ArrayEncoder<8>,
+ Encoder2<CompactSizeEncoder, SliceEncoder<'e, ShortId>>,
+ Encoder2<CompactSizeEncoder, SliceEncoder<'e, PrefilledTransaction>>,
+ >;
+
+encoding::encoder_newtype! {
+ /// Encoder type for a [`HeaderAndShortIds`] message.
+ pub struct HeaderAndShortIdsEncoder<'e>(
+ HeaderAndShortIdsInnerEncoder<'e>
+ );
+}
+
+impl encoding::Encodable for HeaderAndShortIds {
+ type Encoder<'e> = HeaderAndShortIdsEncoder<'e>
+ where
+ Self: 'e;
+
+ fn encoder(&self) -> Self::Encoder<'_> {
+ HeaderAndShortIdsEncoder::new(
+ Encoder4::new(
+ self.header.encoder(),
+ ArrayEncoder::without_length_prefix(self.nonce.to_le_bytes()),
+ Encoder2::new(
+ CompactSizeEncoder::new(self.short_ids.len()),
+ SliceEncoder::without_length_prefix(&self.short_ids)),
+ Encoder2::new(
+ CompactSizeEncoder::new(self.prefilled_txs.len()),
+ SliceEncoder::without_length_prefix(&self.prefilled_txs),
+ )
+ )
+ )
+ }
+}
+
+type HeaderAndShortIdsInnerDecoder = Decoder4<HeaderDecoder, ArrayDecoder<8>, VecDecoder<ShortId>, VecDecoder<PrefilledTransaction>>;
+
+/// Decoder type for the [`HeaderAndShortIds`] message.
+pub struct HeaderAndShortIdsDecoder(HeaderAndShortIdsInnerDecoder);
+
+impl HeaderAndShortIdsDecoder {
+ fn err_from_inner(inner: <HeaderAndShortIdsInnerDecoder as encoding::Decoder>::Error) -> HeaderAndShortIdsDecoderError {
+ HeaderAndShortIdsDecoderError::Decoder(inner)
+ }
+}
+
+impl encoding::Decoder for HeaderAndShortIdsDecoder {
+ type Output = HeaderAndShortIds;
+ type Error = HeaderAndShortIdsDecoderError;
+
+ #[inline]
+ fn push_bytes(&mut self, bytes: &mut &[u8]) -> Result<bool, Self::Error> {
+ self.0.push_bytes(bytes).map_err(Self::err_from_inner)
+ }
+
+ #[inline]
+ fn end(self) -> Result<Self::Output, Self::Error> {
+ let (header, nonce, short_ids, prefilled_txs) = self.0.end().map_err(Self::err_from_inner)?;
+ let overflow_check = short_ids.len().checked_add(prefilled_txs.len()).ok_or(HeaderAndShortIdsDecoderError::IndexOverflow)?;
+ if overflow_check > u16::MAX.into() {
+ return Err(HeaderAndShortIdsDecoderError::IndexOverflow);
+ }
+ Ok(HeaderAndShortIds {
+ header,
+ nonce: u64::from_le_bytes(nonce),
+ short_ids,
+ prefilled_txs
+ })
+ }
+
+ #[inline]
+ fn read_limit(&self) -> usize { self.0.read_limit() }
+}
+
+impl encoding::Decodable for HeaderAndShortIds {
+ type Decoder = HeaderAndShortIdsDecoder;
+
+ fn decoder() -> Self::Decoder {
+ HeaderAndShortIdsDecoder(
+ Decoder4::new(
+ Header::decoder(),
+ ArrayDecoder::new(),
+ VecDecoder::<ShortId>::new(),
+ VecDecoder::<PrefilledTransaction>::new(),
+ )
+ )
+ }
+}
+
+/// Errors occuring when decoding a [`HeaderAndShortIds`] message.
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub enum HeaderAndShortIdsDecoderError {
+ /// Inner decoder error.
+ Decoder(<HeaderAndShortIdsInnerDecoder as encoding::Decoder>::Error),
+ /// Block indexes overflowed.
+ IndexOverflow,
+}
+
+impl From<Infallible> for HeaderAndShortIdsDecoderError {
+ fn from(never: Infallible) -> Self { match never {} }
+}
+
+impl fmt::Display for HeaderAndShortIdsDecoderError {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ match self {
+ Self::Decoder(d) => write_err!(f, "headerandshortids error"; d),
+ Self::IndexOverflow => write!(f, "block index overflowed"),
+ }
+ }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for HeaderAndShortIdsDecoderError {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> {
+ match self {
+ Self::Decoder(d) => Some(d),
+ Self::IndexOverflow => None,
+ }
+ }
+}
+
impl Decodable for HeaderAndShortIds {
fn consensus_decode<R: BufRead + ?Sized>(r: &mut R) -> Result<Self, encode::Error> {
let header_short_ids = Self {
Why this scored 24/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.