What the developer wrote
Authored by rustaceanrob
p2p: Remove BIP-152 indirection
The types of the `bip152` module already express what these messages are
conveying, and the enumeration name matches the command string.
Especially because we will have to maintain `encoding` traits for any
type in `NetworkMessage`, I think this indirection should be removed and
the `bip152` types should be passed through.
What changed, and why it matters
This commit is a straightforward internal code cleanup in the Bitcoin peer-to-peer message handling code. It removes three thin wrapper types (CmpctBlock, GetBlockTxn, BlockTxn) and uses the underlying BIP-152 types directly in the main NetworkMessage enum. There is no functional change to how messages are parsed, serialized, or validated, and no security fix or vulnerability is present in the diff.
No security action required. Treat as a normal API/refactoring change during review; verify downstream consumers that may have referenced the removed wrapper types.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The patch refactors rust-bitcoin’s p2p message module to eliminate indirection. NetworkMessage variants for BIP-152 compact block messages now hold bip152::HeaderAndShortIds, bip152::BlockTransactionsRequest, and bip152::BlockTransactions directly, instead of wrapping them in single-field structs in message_compact_blocks.rs. The consensus encoding macro calls move from the wrappers to the underlying types, preserving identical on-wire serialization. Tests are updated to construct the variants without the wrapper structs. SendCmpct remains unchanged because it is not a simple pass-through type.
Changed components
p2p/src/message.rsp2p/src/message_compact_blocks.rsInspect captured patch +8 / −62
diff --git a/p2p/src/message.rs b/p2p/src/message.rs
index 555c5a38..51a929bb 100644
--- a/p2p/src/message.rs
+++ b/p2p/src/message.rs
@@ -26,8 +26,7 @@ use units::FeeRate;
use crate::address::{AddrV2Message, Address};
use crate::consensus::{impl_consensus_encoding, impl_vec_wrapper};
use crate::{
- message_blockdata, message_bloom, message_compact_blocks, message_filter, message_network,
- Magic,
+ bip152, message_blockdata, message_bloom, message_compact_blocks, message_filter, message_network, Magic
};
/// The maximum number of [`super::message_blockdata::Inventory`] items in an `inv` message.
@@ -473,11 +472,11 @@ pub enum NetworkMessage {
/// BIP-0152 sendcmpct
SendCmpct(message_compact_blocks::SendCmpct),
/// BIP-0152 cmpctblock
- CmpctBlock(message_compact_blocks::CmpctBlock),
+ CmpctBlock(bip152::HeaderAndShortIds),
/// BIP-0152 getblocktxn
- GetBlockTxn(message_compact_blocks::GetBlockTxn),
+ GetBlockTxn(bip152::BlockTransactionsRequest),
/// BIP-0152 blocktxn
- BlockTxn(message_compact_blocks::BlockTxn),
+ BlockTxn(bip152::BlockTransactions),
/// `alert`
Alert(message_network::Alert),
/// `reject`
@@ -1671,7 +1670,7 @@ mod test {
use crate::bip152::BlockTransactionsRequest;
use crate::message_blockdata::{GetBlocksMessage, GetHeadersMessage, Inventory};
use crate::message_bloom::{BloomFlags, FilterAdd, FilterLoad};
- use crate::message_compact_blocks::{GetBlockTxn, SendCmpct};
+ use crate::message_compact_blocks::SendCmpct;
use crate::message_filter::{
CFCheckpt, CFHeaders, CFilter, FilterHash, FilterHeader, GetCFCheckpt, GetCFHeaders,
GetCFilters,
@@ -1798,11 +1797,9 @@ mod test {
}])),
NetworkMessage::SendAddrV2,
NetworkMessage::CmpctBlock(cmptblock),
- NetworkMessage::GetBlockTxn(GetBlockTxn {
- txs_request: BlockTransactionsRequest {
- block_hash: BlockHash::from_byte_array(hash([11u8; 32]).to_byte_array()),
- indexes: vec![0, 1, 2, 3, 10, 3002],
- },
+ NetworkMessage::GetBlockTxn(BlockTransactionsRequest {
+ block_hash: BlockHash::from_byte_array(hash([11u8; 32]).to_byte_array()),
+ indexes: vec![0, 1, 2, 3, 10, 3002],
}),
NetworkMessage::BlockTxn(blocktxn),
NetworkMessage::SendCmpct(SendCmpct { send_compact: true, version: 8333 }),
diff --git a/p2p/src/message_compact_blocks.rs b/p2p/src/message_compact_blocks.rs
index 685696ff..b81db352 100644
--- a/p2p/src/message_compact_blocks.rs
+++ b/p2p/src/message_compact_blocks.rs
@@ -6,7 +6,6 @@
#[cfg(feature = "arbitrary")]
use arbitrary::{Arbitrary, Unstructured};
-use crate::bip152;
use crate::consensus::impl_consensus_encoding;
/// sendcmpct message
@@ -19,59 +18,9 @@ pub struct SendCmpct {
}
impl_consensus_encoding!(SendCmpct, send_compact, version);
-/// cmpctblock message
-///
-/// Note that the rules for validation before relaying compact blocks is
-/// different from headers and regular block messages. Thus, you shouldn't use
-/// compact blocks when relying on an upstream full node to have validated data
-/// being forwarded to you.
-#[derive(PartialEq, Eq, Clone, Debug, PartialOrd, Ord, Hash)]
-pub struct CmpctBlock {
- /// The Compact Block.
- pub compact_block: bip152::HeaderAndShortIds,
-}
-impl_consensus_encoding!(CmpctBlock, compact_block);
-
-/// getblocktxn message
-#[derive(PartialEq, Eq, Clone, Debug, PartialOrd, Ord, Hash)]
-pub struct GetBlockTxn {
- /// The block transactions request.
- pub txs_request: bip152::BlockTransactionsRequest,
-}
-impl_consensus_encoding!(GetBlockTxn, txs_request);
-
-/// blocktxn message
-#[derive(PartialEq, Eq, Clone, Debug, PartialOrd, Ord, Hash)]
-pub struct BlockTxn {
- /// The requested block transactions.
- pub transactions: bip152::BlockTransactions,
-}
-impl_consensus_encoding!(BlockTxn, transactions);
-
#[cfg(feature = "arbitrary")]
impl<'a> Arbitrary<'a> for SendCmpct {
fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
Ok(Self { send_compact: u.arbitrary()?, version: u.arbitrary()? })
}
}
-
-#[cfg(feature = "arbitrary")]
-impl<'a> Arbitrary<'a> for CmpctBlock {
- fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
- Ok(Self { compact_block: u.arbitrary()? })
- }
-}
-
-#[cfg(feature = "arbitrary")]
-impl<'a> Arbitrary<'a> for GetBlockTxn {
- fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
- Ok(Self { txs_request: u.arbitrary()? })
- }
-}
-
-#[cfg(feature = "arbitrary")]
-impl<'a> Arbitrary<'a> for BlockTxn {
- fn arbitrary(u: &mut Unstructured<'a>) -> arbitrary::Result<Self> {
- Ok(Self { transactions: u.arbitrary()? })
- }
-}
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.