p2p: Implement `encoding` traits for `PartialMerkleTree`
What changed, and why it matters
This commit adds new serialization/deserialization code for PartialMerkleTree, a data structure used in Bitcoin peer-to-peer messages. It re-implements the existing bit-packing logic using a new internal encoding framework. There is no direct evidence of a security bug, but the change touches low-level binary parsing and could introduce subtle correctness issues if the new decoder does not exactly match the old behavior.
Review whether the new decoder's output is bit-for-bit semantically equivalent to the old consensus deserialization path, especially whether trailing padding bits in the final byte affect PartialMerkleTree validation logic. If the old path is being deprecated, ensure it is removed to avoid inconsistent behavior.
Security signals we found
Low-level binary deserialization of network data
Bit expansion loop does not truncate to original bit count; decoded `bits` length is always a multiple of 8
New decoder uses `ByteVecDecoder` with `CompactSizeEncoder` length prefix, matching Bitcoin consensus serialization
Old consensus_encode implementation still present; behavior divergence possible if both paths are used
Evidence from the diff
The patch implements the new encoding::Encodable/Decodable traits for PartialMerkleTree in p2p/src/merkle_tree.rs. It introduces a BitVecEncoder to pack the Vec<bool> into a compressed byte vector and a PartialMerkleTreeDecoder that unpacks it. The decoder reads a 4-byte little-endian transaction count, a length-prefixed vector of TxMerkleNode hashes, and a length-prefixed byte vector of packed bits, then expands each byte into 8 booleans. The old Encodable implementation remains in place below the new code, so this appears to be an additive migration rather than a replacement.
Changed components
p2p/src/merkle_tree.rsPartialMerkleTreeBitVecEncoderPartialMerkleTreeDecoderInspect captured patch +134 / −0
diff --git a/p2p/src/merkle_tree.rs b/p2p/src/merkle_tree.rs
index 597999ff..22306d02 100644
--- a/p2p/src/merkle_tree.rs
+++ b/p2p/src/merkle_tree.rs
@@ -17,7 +17,9 @@ use core::fmt;
#[cfg(feature = "arbitrary")]
use arbitrary::{Arbitrary, Unstructured};
use bitcoin::consensus::encode::{self, Decodable, Encodable, ReadExt, WriteExt, MAX_VEC_SIZE};
+use encoding::{ArrayDecoder, ArrayEncoder, ByteVecDecoder, CompactSizeEncoder, Decoder3, Encoder2, Encoder3, SliceEncoder, VecDecoder};
use internals::ToU64 as _;
+use internals::write_err;
use io::{BufRead, Write};
use primitives::block::{self, Block, Checked};
use primitives::merkle_tree::TxMerkleNode;
@@ -409,6 +411,138 @@ impl PartialMerkleTree {
}
}
+struct BitVecEncoder {
+ buffer: Vec<u8>,
+ exhausted: bool,
+}
+
+impl BitVecEncoder {
+ fn new(bits: &[bool]) -> Self {
+ let mut buffer = Vec::with_capacity(bits.len().div_ceil(8));
+ for chunk in bits.chunks(8) {
+ let mut byte = 0u8;
+ for (i, bit) in chunk.iter().enumerate() {
+ byte |= u8::from(*bit) << i;
+ }
+ buffer.push(byte);
+ }
+ Self {
+ buffer,
+ exhausted: false,
+ }
+ }
+}
+
+impl encoding::Encoder for BitVecEncoder {
+ fn current_chunk(&self) -> &[u8] {
+ if self.exhausted {
+ &[]
+ } else {
+ &self.buffer
+ }
+ }
+
+ fn advance(&mut self) -> bool {
+ self.exhausted = true;
+ false
+ }
+}
+
+encoding::encoder_newtype! {
+ /// The encoder for a [`PartialMerkleTree`].
+ pub struct PartialMerkleTreeEncoder<'e>(
+ Encoder3<
+ ArrayEncoder<4>,
+ Encoder2<CompactSizeEncoder, SliceEncoder<'e, TxMerkleNode>>,
+ Encoder2<CompactSizeEncoder, BitVecEncoder>,
+ >
+ );
+}
+
+impl encoding::Encodable for PartialMerkleTree {
+ type Encoder<'e> = PartialMerkleTreeEncoder<'e>;
+
+ fn encoder(&self) -> Self::Encoder<'_> {
+ PartialMerkleTreeEncoder::new(
+ Encoder3::new(
+ ArrayEncoder::without_length_prefix(self.num_transactions.to_le_bytes()),
+ Encoder2::new(
+ CompactSizeEncoder::new(self.hashes.len()),
+ SliceEncoder::without_length_prefix(&self.hashes),
+ ),
+ Encoder2::new(
+ CompactSizeEncoder::new(self.bits.len().div_ceil(8)),
+ BitVecEncoder::new(&self.bits)
+ ),
+ )
+ )
+ }
+}
+
+type PartialMerkleTreeInnerDecoder = Decoder3<ArrayDecoder<4>, VecDecoder<TxMerkleNode>, ByteVecDecoder>;
+
+/// The decoder type for a [`PartialMerkleTree`].
+pub struct PartialMerkleTreeDecoder(PartialMerkleTreeInnerDecoder);
+
+impl encoding::Decoder for PartialMerkleTreeDecoder {
+ type Output = PartialMerkleTree;
+ type Error = PartialMerkleTreeDecoderError;
+
+ #[inline]
+ fn push_bytes(&mut self, bytes: &mut &[u8]) -> Result<bool, Self::Error> {
+ self.0.push_bytes(bytes).map_err(PartialMerkleTreeDecoderError)
+ }
+
+ #[inline]
+ fn end(self) -> Result<Self::Output, Self::Error> {
+ let (num_transactions, hashes, compress_bit_vec) = self.0.end().map_err(PartialMerkleTreeDecoderError)?;
+ let num_transactions = u32::from_le_bytes(num_transactions);
+ let mut bits = Vec::with_capacity(compress_bit_vec.len());
+ for byte in compress_bit_vec {
+ for i in 0..8 {
+ bits.push((byte & (1 << i)) != 0);
+ }
+ }
+ Ok(PartialMerkleTree {
+ num_transactions,
+ bits,
+ hashes
+ })
+ }
+
+ #[inline]
+ fn read_limit(&self) -> usize { self.0.read_limit() }
+}
+
+impl encoding::Decodable for PartialMerkleTree {
+ type Decoder = PartialMerkleTreeDecoder;
+
+ fn decoder() -> Self::Decoder {
+ PartialMerkleTreeDecoder(
+ Decoder3::new(ArrayDecoder::new(),VecDecoder::new(), ByteVecDecoder::new())
+ )
+ }
+}
+
+/// An error occuring when decoding a [`PartialMerkleTree`].
+#[derive(Debug, Clone, PartialEq, Eq)]
+pub struct PartialMerkleTreeDecoderError(<PartialMerkleTreeInnerDecoder as encoding::Decoder>::Error);
+
+impl From<Infallible> for PartialMerkleTreeDecoderError {
+ fn from(never: Infallible) -> Self { match never {} }
+}
+
+impl fmt::Display for PartialMerkleTreeDecoderError {
+ fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
+ write_err!(f, "partial merkletree error"; self.0)
+ }
+}
+
+#[cfg(feature = "std")]
+impl std::error::Error for PartialMerkleTreeDecoderError {
+ fn source(&self) -> Option<&(dyn std::error::Error + 'static)> { Some(&self.0) }
+}
+
impl Encodable for PartialMerkleTree {
fn consensus_encode<W: Write + ?Sized>(&self, w: &mut W) -> Result<usize, io::Error> {
let mut ret = self.num_transactions.consensus_encode(w)?;
Why this scored 28/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.