From 59981d492d3ac7634b2981697ef30829ac7b3f1a Mon Sep 17 00:00:00 2001 From: pasta Date: Fri, 9 Oct 2026 07:10:50 +0000 Subject: [PATCH 01/17] p2p_core%fix!: make `ProtocolVersion` signed The reference implementation reads the version as a signed 32-bit integer, so a version with the top bit set is below every minimum. --- pkgs/p2p_core/src/version.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/pkgs/p2p_core/src/version.rs b/pkgs/p2p_core/src/version.rs index 3d07d65e9..8b23bb0d3 100644 --- a/pkgs/p2p_core/src/version.rs +++ b/pkgs/p2p_core/src/version.rs @@ -11,7 +11,7 @@ use dash_types::make_num; make_num! { /// Protocol version exchanged during the handshake. - ProtocolVersion, u32, 4 + ProtocolVersion, i32, 4 } hash_impl!(ProtocolVersion); @@ -34,3 +34,19 @@ impl ProtocolVersion { /// Chainlock signatures in MN list diff boundary. pub const MNLISTDIFF_CHAINLOCKS: Self = Self(70230); } + +#[cfg(test)] +mod tests { + use super::*; + + use dash_types::codec::BaseCodec; + use rstest::rstest; + + /// A set top bit reads as a version below every minimum. + #[rstest] + fn top_bit_reads_below_every_minimum() { + let decoded = ProtocolVersion::decode(&mut &[0xFF, 0xFF, 0xFF, 0xFF][..]); + assert_eq!(decoded, Ok(ProtocolVersion(-1))); + assert!(ProtocolVersion(-1) < ProtocolVersion::MIN_PEER); + } +} From e12661ae3b2c2fabd6035c2b5e720147fda90842 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 15:01:14 +0000 Subject: [PATCH 02/17] p2p_core%refac!: carry the decode error in `P2pDecodeError` Callers can tell a short read from trailing bytes. Messages decode through `BaseCodec`, whose errors carry that detail. --- pkgs/p2p_core/src/bip324.rs | 24 ++++++------------------ pkgs/p2p_core/src/error.rs | 14 +++----------- pkgs/p2p_core/src/macros.rs | 22 ++++++++++++++-------- pkgs/p2p_core/tests/decode_payload.rs | 21 +++++++++++++++++++++ 4 files changed, 44 insertions(+), 37 deletions(-) create mode 100644 pkgs/p2p_core/tests/decode_payload.rs diff --git a/pkgs/p2p_core/src/bip324.rs b/pkgs/p2p_core/src/bip324.rs index da0ca071e..ae3ca02bd 100644 --- a/pkgs/p2p_core/src/bip324.rs +++ b/pkgs/p2p_core/src/bip324.rs @@ -12,6 +12,8 @@ use crate::prelude::*; use crate::short_id::ShortId; use crate::P2pDecodeError; +use dash_types::codec::take; + /// Encodes a `P2pMsg` into V2 framed bytes. pub fn encode_v2(msg: &P2pMsg, buf: &mut Vec) { match msg.short_id() { @@ -30,27 +32,13 @@ pub fn encode_v2(msg: &P2pMsg, buf: &mut Vec) { /// Decodes V2 framed bytes into a `P2pMsg`. pub fn decode_v2(payload: &[u8]) -> Result { - if payload.is_empty() { - return Err(P2pDecodeError::Consensus(String::from( - "unexpected eof: needed 1 byte, 0 remaining", - ))); - } - - let short_id = payload[0]; - let rest = &payload[1..]; + let mut rest = payload; + let [short_id] = take(&mut rest)?; if short_id == 0 { // Long format: next 12 bytes are the command string. - if rest.len() < 12 { - return Err(P2pDecodeError::Consensus(format!( - "unexpected eof: needed 12 bytes, {} remaining", - rest.len() - ))); - } - let mut cmd_bytes = [0u8; 12]; - cmd_bytes.copy_from_slice(&rest[..12]); - let cmd = CommandString::from_bytes(cmd_bytes); - P2pMsg::decode_payload(&cmd, &rest[12..]) + let cmd = CommandString::from_bytes(take(&mut rest)?); + P2pMsg::decode_payload(&cmd, rest) } else { // Short ID: resolve to command, then decode payload. let sid = ShortId(short_id); diff --git a/pkgs/p2p_core/src/error.rs b/pkgs/p2p_core/src/error.rs index be821dfc4..b15f1968b 100644 --- a/pkgs/p2p_core/src/error.rs +++ b/pkgs/p2p_core/src/error.rs @@ -6,8 +6,6 @@ //! P2P-layer decoding errors. -use crate::prelude::*; - use dash_types::codec::DecodeError; use dash_types::type_id::Unencodable; @@ -16,9 +14,9 @@ use core::fmt; /// An error encountered during P2P message decoding. #[derive(Clone, Debug, Eq, Hash, PartialEq, thiserror::Error, Unencodable)] pub enum P2pDecodeError { - /// Underlying consensus decoding error. - #[error("{0}")] - Consensus(String), + /// A field failed to decode. + #[error(transparent)] + Consensus(#[from] DecodeError), /// Unrecognised 12-byte command string. #[error(fmt = fmt_unknown_command)] UnknownCommand { @@ -57,12 +55,6 @@ pub enum P2pDecodeError { }, } -impl From for P2pDecodeError { - fn from(e: DecodeError) -> Self { - Self::Consensus(format!("{e}")) - } -} - /// Formats `P2pDecodeError::UnknownCommand`, escaping non-printable bytes. fn fmt_unknown_command(bytes: &[u8; 12], f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("unknown command: ")?; diff --git a/pkgs/p2p_core/src/macros.rs b/pkgs/p2p_core/src/macros.rs index 7bf787470..7164e7533 100644 --- a/pkgs/p2p_core/src/macros.rs +++ b/pkgs/p2p_core/src/macros.rs @@ -7,17 +7,23 @@ //! Shared macro definitions. use crate::codec::MAX_P2P_PAYLOAD_SIZE; -use crate::prelude::*; use crate::P2pDecodeError; -use bitcoin_consensus_encoding::{decode_from_slice, Decode, Decoder}; +use dash_types::codec::{BaseCodec, DecodeError}; -/// Decode from slice, mapping the error. -pub(crate) fn decode_msg(payload: &[u8]) -> Result -where - ::Error: core::fmt::Display, -{ - decode_from_slice(payload).map_err(|e| P2pDecodeError::Consensus(format!("{e}"))) +/// Decodes a whole payload, failing on trailing bytes. +pub(crate) fn decode_msg(payload: &[u8]) -> Result { + let mut cursor = payload; + let msg = T::decode(&mut cursor)?; + if !cursor.is_empty() { + return Err( + DecodeError::TrailingBytes { + remaining: cursor.len(), + } + .into(), + ); + } + Ok(msg) } /// Reject payloads that exceed the protocol limit. diff --git a/pkgs/p2p_core/tests/decode_payload.rs b/pkgs/p2p_core/tests/decode_payload.rs new file mode 100644 index 000000000..2b7230728 --- /dev/null +++ b/pkgs/p2p_core/tests/decode_payload.rs @@ -0,0 +1,21 @@ +// +// Copyright (c) 2026-present, The Dash Core developers +// SPDX-License-Identifier: MIT +// See the accompanying file LICENSE or https://opensource.org/license/MIT +// + +//! Checks applied by `P2pMsg::decode_payload`. + +use dash_p2p_core::{CommandString, P2pDecodeError, P2pMsg}; +use dash_types::codec::DecodeError; +use rstest::rstest; + +#[rstest] +#[case::short(&[0u8; 7], DecodeError::Eof { needed: 8, remaining: 7 })] +#[case::trailing(&[0u8; 9], DecodeError::TrailingBytes { remaining: 1 })] +fn decode_errors_are_typed(#[case] payload: &[u8], #[case] err: DecodeError) { + assert_eq!( + P2pMsg::decode_payload(&CommandString::from_static("ping"), payload), + Err(P2pDecodeError::Consensus(err)) + ); +} From 8ca2966d5972a044a35f59d351a2f993a819cd27 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 16:32:03 +0000 Subject: [PATCH 03/17] p2p_core%fix!: bound every command by the 3 MiB message limit The reference implementation refuses a larger message before it reads its command. `PayloadTooLarge` now carries the `CommandString`. --- pkgs/p2p_core/corpus/transport.json5 | 46 +++++++++++++++++++++++++++ pkgs/p2p_core/src/error.rs | 24 ++++++++++++-- pkgs/p2p_core/src/macros.rs | 22 +++++-------- pkgs/p2p_core/src/msg/mod.rs | 30 +++++++++++++++++ pkgs/p2p_core/tests/decode_payload.rs | 11 +++++++ 5 files changed, 116 insertions(+), 17 deletions(-) create mode 100644 pkgs/p2p_core/corpus/transport.json5 diff --git a/pkgs/p2p_core/corpus/transport.json5 b/pkgs/p2p_core/corpus/transport.json5 new file mode 100644 index 000000000..d689de75a --- /dev/null +++ b/pkgs/p2p_core/corpus/transport.json5 @@ -0,0 +1,46 @@ +// +// Transport vectors. Generated with the reference implementation +// (dashpay/dash v24.0.0-rc.3, 0364ed626e). +// +// Each section comes with the program used to generate the vectors. +// +// To run a program, paste it at the end of the `net_tests` suite in +// `src/test/net_tests.cpp` at the specified revision, build it with +// `make -C src test/test_dash` and run +// `./src/test/test_dash --run_test=net_tests/`. The output should match +// the section. +// + +{ +/* +BOOST_AUTO_TEST_CASE(transport_v1_size) +{ + using namespace std::literals; + for (const auto name : {"ping"sv, "verack"sv, "tx"sv, "nosuchcmd"sv, "pi\x1fng"sv}) { + for (const uint32_t size : {MAX_PROTOCOL_MESSAGE_LENGTH, MAX_PROTOCOL_MESSAGE_LENGTH + 1}) { + V1Transport transport{0, SER_NETWORK, INIT_PROTO_VERSION}; + CMessageHeader hdr{Params().MessageStart(), "", size}; + std::copy(name.begin(), name.end(), hdr.pchCommand); + CDataStream ss{SER_NETWORK, PROTOCOL_VERSION}; + ss << hdr; + Span bytes{UCharCast(ss.data()), ss.size()}; + const bool kept{transport.ReceivedBytes(bytes)}; + std::printf(" { \"command\": \"%s\", \"size\": %u, \"disconnect\": %s },\n", + HexStr(Span{hdr.pchCommand}).c_str(), size, kept ? "false" : "true"); + } + } +} +*/ + "v1_size": [ + { "command": "70696e670000000000000000", "size": 3145728, "disconnect": false }, + { "command": "70696e670000000000000000", "size": 3145729, "disconnect": true }, + { "command": "76657261636b000000000000", "size": 3145728, "disconnect": false }, + { "command": "76657261636b000000000000", "size": 3145729, "disconnect": true }, + { "command": "747800000000000000000000", "size": 3145728, "disconnect": false }, + { "command": "747800000000000000000000", "size": 3145729, "disconnect": true }, + { "command": "6e6f73756368636d64000000", "size": 3145728, "disconnect": false }, + { "command": "6e6f73756368636d64000000", "size": 3145729, "disconnect": true }, + { "command": "70691f6e6700000000000000", "size": 3145728, "disconnect": false }, + { "command": "70691f6e6700000000000000", "size": 3145729, "disconnect": true }, + ], +} diff --git a/pkgs/p2p_core/src/error.rs b/pkgs/p2p_core/src/error.rs index b15f1968b..7ae7427ce 100644 --- a/pkgs/p2p_core/src/error.rs +++ b/pkgs/p2p_core/src/error.rs @@ -6,6 +6,8 @@ //! P2P-layer decoding errors. +use crate::command::CommandString; + use dash_types::codec::DecodeError; use dash_types::type_id::Unencodable; @@ -30,10 +32,10 @@ pub enum P2pDecodeError { id: u8, }, /// Message payload exceeds the allowed size. - #[error("{command} payload too large: {size} bytes, max {max}")] + #[error(fmt = fmt_payload_too_large)] PayloadTooLarge { - /// Wire command name. - command: &'static str, + /// The message's command. + command: CommandString, /// Size of the raw payload, in bytes. size: usize, /// Maximum allowed size, in bytes. @@ -58,6 +60,22 @@ pub enum P2pDecodeError { /// Formats `P2pDecodeError::UnknownCommand`, escaping non-printable bytes. fn fmt_unknown_command(bytes: &[u8; 12], f: &mut fmt::Formatter<'_>) -> fmt::Result { f.write_str("unknown command: ")?; + write_command(bytes, f) +} + +/// Formats `P2pDecodeError::PayloadTooLarge`, escaping non-printable bytes. +fn fmt_payload_too_large( + command: &CommandString, + size: &usize, + max: &usize, + f: &mut fmt::Formatter<'_>, +) -> fmt::Result { + write!(f, "payload too large: {size} bytes, max {max}, command ")?; + write_command(command.as_bytes(), f) +} + +/// Writes command bytes up to the first NUL, escaping non-printable bytes. +fn write_command(bytes: &[u8; 12], f: &mut fmt::Formatter<'_>) -> fmt::Result { for &b in bytes.iter().take_while(|&&b| b != 0) { if b.is_ascii_graphic() || b == b' ' { write!(f, "{}", b as char)?; diff --git a/pkgs/p2p_core/src/macros.rs b/pkgs/p2p_core/src/macros.rs index 7164e7533..a5917dc28 100644 --- a/pkgs/p2p_core/src/macros.rs +++ b/pkgs/p2p_core/src/macros.rs @@ -7,6 +7,7 @@ //! Shared macro definitions. use crate::codec::MAX_P2P_PAYLOAD_SIZE; +use crate::command::CommandString; use crate::P2pDecodeError; use dash_types::codec::{BaseCodec, DecodeError}; @@ -27,10 +28,10 @@ pub(crate) fn decode_msg(payload: &[u8]) -> Result Result<(), P2pDecodeError> { +pub(crate) fn check_payload(command: &CommandString, payload: &[u8]) -> Result<(), P2pDecodeError> { if payload.len() > MAX_P2P_PAYLOAD_SIZE { return Err(P2pDecodeError::PayloadTooLarge { - command, + command: *command, size: payload.len(), max: MAX_P2P_PAYLOAD_SIZE, }); @@ -164,30 +165,23 @@ macro_rules! define_p2p { } /// Decodes a message from its command string and raw payload. + /// + /// Every command, known or not, is bounded by the 3 MiB limit. pub fn decode_payload( cmd: &CommandString, payload: &[u8], ) -> Result { + crate::macros::check_payload(cmd, payload)?; let raw = || Vec::from(payload); let msg = match *cmd { - $( - CommandString::$p_cmd => { - crate::macros::check_payload($p_wire, payload)?; - Self::$p_variant(crate::macros::decode_msg(payload)?) - } - )* + $( CommandString::$p_cmd => Self::$p_variant(crate::macros::decode_msg(payload)?), )* $( CommandString::$pe_cmd => { crate::macros::check_empty($pe_wire, payload)?; Self::$pe_variant } )* - $( - CommandString::$s_cmd => { - crate::macros::check_payload($s_wire, payload)?; - Self::$s_variant(raw()) - } - )* + $( CommandString::$s_cmd => Self::$s_variant(raw()), )* $( CommandString::$se_cmd => { crate::macros::check_empty($se_wire, payload)?; diff --git a/pkgs/p2p_core/src/msg/mod.rs b/pkgs/p2p_core/src/msg/mod.rs index 03108aeb5..7a69c857f 100644 --- a/pkgs/p2p_core/src/msg/mod.rs +++ b/pkgs/p2p_core/src/msg/mod.rs @@ -193,3 +193,33 @@ define_p2p! { QWatch => QWATCH "qwatch" @ 151, } } + +#[cfg(all(test, feature = "serde"))] +#[expect(clippy::unwrap_used, reason = "test code")] +mod tests { + use super::*; + use crate::P2pDecodeError; + + use dash_dev::Corpus; + use hex_conservative::decode_to_array; + use rstest::rstest; + use serde::Deserialize; + + #[derive(Deserialize)] + struct SizeVector { + command: String, + size: usize, + disconnect: bool, + } + + #[rstest] + fn payload_size_matches_vectors() { + let corpus = Corpus::open(env!("CARGO_MANIFEST_DIR"), "transport"); + for v in corpus.vectors::("v1_size") { + let cmd = CommandString::from_bytes(decode_to_array(&v.command).unwrap()); + let result = P2pMsg::decode_payload(&cmd, &vec![0; v.size]); + let too_large = matches!(result, Err(P2pDecodeError::PayloadTooLarge { .. })); + assert_eq!(too_large, v.disconnect, "{} of {} bytes", v.command, v.size); + } + } +} diff --git a/pkgs/p2p_core/tests/decode_payload.rs b/pkgs/p2p_core/tests/decode_payload.rs index 2b7230728..c4bf49e7b 100644 --- a/pkgs/p2p_core/tests/decode_payload.rs +++ b/pkgs/p2p_core/tests/decode_payload.rs @@ -19,3 +19,14 @@ fn decode_errors_are_typed(#[case] payload: &[u8], #[case] err: DecodeError) { Err(P2pDecodeError::Consensus(err)) ); } + +#[rstest] +fn oversized_commands_are_escaped() { + let cmd = CommandString::from_bytes(*b"pi\nng\0\0\0\0\0\0\0"); + assert_eq!( + P2pMsg::decode_payload(&cmd, &vec![0; 3 * 1024 * 1024 + 1]).map_err(|e| e.to_string()), + Err(String::from( + "payload too large: 3145729 bytes, max 3145728, command pi\\x0ang" + )) + ); +} From 34136817e2ad66219e924a9684828ab8c6e2c3ce Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 16:27:48 +0000 Subject: [PATCH 04/17] p2p_core%fix!: bound V2 contents before reading the type The reference implementation bounds the type and payload together by 1 + 12 + 3 MiB, then refuses a long-form type outside 0x20 to 0x7f. --- pkgs/p2p_core/corpus/transport.json5 | 70 ++++++++++++++++++++++++ pkgs/p2p_core/src/bip324.rs | 81 +++++++++++++++++++++++----- pkgs/p2p_core/src/command.rs | 24 +++++++++ pkgs/p2p_core/src/error.rs | 14 ++++- pkgs/p2p_core/src/macros.rs | 8 +++ 5 files changed, 182 insertions(+), 15 deletions(-) diff --git a/pkgs/p2p_core/corpus/transport.json5 b/pkgs/p2p_core/corpus/transport.json5 index d689de75a..1fb6136de 100644 --- a/pkgs/p2p_core/corpus/transport.json5 +++ b/pkgs/p2p_core/corpus/transport.json5 @@ -43,4 +43,74 @@ BOOST_AUTO_TEST_CASE(transport_v1_size) { "command": "70691f6e6700000000000000", "size": 3145728, "disconnect": false }, { "command": "70691f6e6700000000000000", "size": 3145729, "disconnect": true }, ], +/* +BOOST_AUTO_TEST_CASE(transport_v2) +{ + using namespace std::literals; + const auto send = [](Span contents) { + V2TransportTester tester(true); + auto ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.SendKey(); + tester.SendGarbage(); + tester.ReceiveKey(); + tester.SendGarbageTerm(); + tester.SendVersion(); + ret = tester.Interact(); + BOOST_REQUIRE(ret && ret->empty()); + tester.ReceiveGarbage(); + tester.ReceiveVersion(); + tester.SendPacket(contents); + ret = tester.Interact(); + if (!ret) return "disconnect"; + BOOST_REQUIRE(ret->size() == 1); + return (*ret)[0] ? "accept" : "reject"; + }; + const auto long_form = [](std::string_view name) { + std::vector prefix(1 + CMessageHeader::COMMAND_SIZE); + std::copy(name.begin(), name.end(), prefix.begin() + 1); + return prefix; + }; + std::vector, size_t>> cases; + for (const auto name : {"ping"sv, "abcdefghijkl"sv, ""sv, "pi\x7fng"sv, "pi\x1fng"sv, "pi\x80ng"sv, + "pi\0ng"sv, "ping\0\0\0\0\0\0\0\x01"sv}) { + cases.emplace_back(long_form(name), 1 + CMessageHeader::COMMAND_SIZE); + } + const size_t max_contents{1 + CMessageHeader::COMMAND_SIZE + MAX_PROTOCOL_MESSAGE_LENGTH}; + for (const size_t len : {size_t{1}, max_contents, max_contents + 1}) { + cases.emplace_back(std::vector{21}, len); + cases.emplace_back(std::vector{0xff}, len); + } + for (const size_t len : {max_contents, max_contents + 1}) { + cases.emplace_back(long_form("ping"sv), len); + cases.emplace_back(long_form("pi\0ng"sv), len); + } + for (const auto& [prefix, len] : cases) { + std::vector contents{prefix}; + contents.resize(len); + std::printf(" { \"prefix\": \"%s\", \"len\": %zu, \"result\": \"%s\" },\n", HexStr(prefix).c_str(), len, + send(contents)); + } +} +*/ + "v2": [ + { "prefix": "0070696e670000000000000000", "len": 13, "result": "accept" }, + { "prefix": "006162636465666768696a6b6c", "len": 13, "result": "accept" }, + { "prefix": "00000000000000000000000000", "len": 13, "result": "accept" }, + { "prefix": "0070697f6e6700000000000000", "len": 13, "result": "accept" }, + { "prefix": "0070691f6e6700000000000000", "len": 13, "result": "reject" }, + { "prefix": "007069806e6700000000000000", "len": 13, "result": "reject" }, + { "prefix": "007069006e6700000000000000", "len": 13, "result": "reject" }, + { "prefix": "0070696e670000000000000001", "len": 13, "result": "reject" }, + { "prefix": "15", "len": 1, "result": "accept" }, + { "prefix": "ff", "len": 1, "result": "reject" }, + { "prefix": "15", "len": 3145741, "result": "accept" }, + { "prefix": "ff", "len": 3145741, "result": "reject" }, + { "prefix": "15", "len": 3145742, "result": "disconnect" }, + { "prefix": "ff", "len": 3145742, "result": "disconnect" }, + { "prefix": "0070696e670000000000000000", "len": 3145741, "result": "accept" }, + { "prefix": "007069006e6700000000000000", "len": 3145741, "result": "reject" }, + { "prefix": "0070696e670000000000000000", "len": 3145742, "result": "disconnect" }, + { "prefix": "007069006e6700000000000000", "len": 3145742, "result": "disconnect" }, + ], } diff --git a/pkgs/p2p_core/src/bip324.rs b/pkgs/p2p_core/src/bip324.rs index ae3ca02bd..7ffa01ae3 100644 --- a/pkgs/p2p_core/src/bip324.rs +++ b/pkgs/p2p_core/src/bip324.rs @@ -6,6 +6,7 @@ //! BIP324 V2 message framing. +use crate::codec::MAX_P2P_PAYLOAD_SIZE; use crate::command::CommandString; use crate::msg::P2pMsg; use crate::prelude::*; @@ -30,21 +31,73 @@ pub fn encode_v2(msg: &P2pMsg, buf: &mut Vec) { msg.encode_payload(buf); } -/// Decodes V2 framed bytes into a `P2pMsg`. -pub fn decode_v2(payload: &[u8]) -> Result { - let mut rest = payload; - let [short_id] = take(&mut rest)?; +/// The type byte, a 12-byte command and the 3 MiB message limit. +const MAX_V2_CONTENTS: usize = 1 + 12 + MAX_P2P_PAYLOAD_SIZE; - if short_id == 0 { - // Long format: next 12 bytes are the command string. - let cmd = CommandString::from_bytes(take(&mut rest)?); - P2pMsg::decode_payload(&cmd, rest) +/// Decodes V2 message contents into a `P2pMsg`. +/// +/// The size bound applies to the whole contents and comes before any check +/// of the type, so a short-ID payload may be 12 bytes larger than a V1 +/// payload. +/// +/// # Errors +/// +/// Returns `PayloadTooLarge` above the bound, with an empty command for an +/// unknown short ID. Returns `UnknownShortId` or `InvalidCommand` for a type +/// the transport refuses, and otherwise the error decoding the message. +pub fn decode_v2(contents: &[u8]) -> Result { + let mut payload = contents; + let [short_id] = take(&mut payload)?; + let id = ShortId(short_id); + let cmd = if id == ShortId::LONG_FORMAT { + Some(CommandString::from_bytes(take(&mut payload)?)) } else { - // Short ID: resolve to command, then decode payload. - let sid = ShortId(short_id); - let cmd = sid - .to_command() - .ok_or(P2pDecodeError::UnknownShortId { id: short_id })?; - P2pMsg::decode_payload(&cmd, rest) + id.to_command() + }; + + if contents.len() > MAX_V2_CONTENTS { + return Err(P2pDecodeError::PayloadTooLarge { + command: cmd.unwrap_or(CommandString::from_static("")), + size: contents.len(), + max: MAX_V2_CONTENTS, + }); + } + let cmd = cmd.ok_or(P2pDecodeError::UnknownShortId { id: short_id })?; + if id == ShortId::LONG_FORMAT && !cmd.is_valid_v2() { + return Err(P2pDecodeError::InvalidCommand { command: cmd }); + } + P2pMsg::decode_unbounded(&cmd, payload) +} + +#[cfg(all(test, feature = "serde"))] +#[expect(clippy::unwrap_used, reason = "test code")] +mod tests { + use super::*; + + use dash_dev::Corpus; + use hex_conservative::decode_to_vec; + use rstest::rstest; + use serde::Deserialize; + + #[derive(Deserialize)] + struct ContentsVector { + prefix: String, + len: usize, + result: String, + } + + #[rstest] + fn decode_v2_matches_vectors() { + let corpus = Corpus::open(env!("CARGO_MANIFEST_DIR"), "transport"); + for v in corpus.vectors::("v2") { + let mut contents = decode_to_vec(&v.prefix).unwrap(); + contents.resize(v.len, 0); + let result = match decode_v2(&contents) { + Err(P2pDecodeError::PayloadTooLarge { .. }) => "disconnect", + Err(P2pDecodeError::InvalidCommand { .. } | P2pDecodeError::UnknownShortId { .. }) => "reject", + _ => "accept", + }; + assert_eq!(result, v.result, "{} of {} bytes", v.prefix, v.len); + } } } diff --git a/pkgs/p2p_core/src/command.rs b/pkgs/p2p_core/src/command.rs index f3cc75dc4..bd8c69db8 100644 --- a/pkgs/p2p_core/src/command.rs +++ b/pkgs/p2p_core/src/command.rs @@ -57,6 +57,30 @@ impl CommandString { // validated on decode, so this conversion is sound. core::str::from_utf8(&self.0[..end]).unwrap_or("") } + + /// Whether a V2 transport accepts the command as a long-form type. + /// + /// Bytes before the first NUL must be in 0x20 to 0x7f, and every byte after + /// it NUL. + pub const fn is_valid_v2(&self) -> bool { + self.is_valid_up_to(0x7f) + } + + /// Bytes in 0x20 to `max` up to the first NUL, then only NULs. + const fn is_valid_up_to(&self, max: u8) -> bool { + let mut padding = false; + let mut i = 0; + while i < self.0.len() { + let b = self.0[i]; + if b == 0 { + padding = true; + } else if padding || b < b' ' || b > max { + return false; + } + i += 1; + } + true + } } type_cvrt!(From<[u8; 12]> for CommandString, |bytes| Self(*bytes)); diff --git a/pkgs/p2p_core/src/error.rs b/pkgs/p2p_core/src/error.rs index 7ae7427ce..48d90a003 100644 --- a/pkgs/p2p_core/src/error.rs +++ b/pkgs/p2p_core/src/error.rs @@ -25,6 +25,12 @@ pub enum P2pDecodeError { /// The raw command bytes. bytes: [u8; 12], }, + /// A V2 long-form command that fails [`CommandString::is_valid_v2`]. + #[error(fmt = fmt_invalid_command)] + InvalidCommand { + /// The command as received. + command: CommandString, + }, /// V2 short ID does not map to a known message. #[error("unknown v2 short id: {id}")] UnknownShortId { @@ -36,7 +42,7 @@ pub enum P2pDecodeError { PayloadTooLarge { /// The message's command. command: CommandString, - /// Size of the raw payload, in bytes. + /// Size of the payload, or of the whole contents for V2, in bytes. size: usize, /// Maximum allowed size, in bytes. max: usize, @@ -63,6 +69,12 @@ fn fmt_unknown_command(bytes: &[u8; 12], f: &mut fmt::Formatter<'_>) -> fmt::Res write_command(bytes, f) } +/// Formats `P2pDecodeError::InvalidCommand`, escaping non-printable bytes. +fn fmt_invalid_command(command: &CommandString, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.write_str("invalid command: ")?; + write_command(command.as_bytes(), f) +} + /// Formats `P2pDecodeError::PayloadTooLarge`, escaping non-printable bytes. fn fmt_payload_too_large( command: &CommandString, diff --git a/pkgs/p2p_core/src/macros.rs b/pkgs/p2p_core/src/macros.rs index a5917dc28..f62d2357d 100644 --- a/pkgs/p2p_core/src/macros.rs +++ b/pkgs/p2p_core/src/macros.rs @@ -172,6 +172,14 @@ macro_rules! define_p2p { payload: &[u8], ) -> Result { crate::macros::check_payload(cmd, payload)?; + Self::decode_unbounded(cmd, payload) + } + + /// Decodes a payload whose transport has already bounded its size. + pub(crate) fn decode_unbounded( + cmd: &CommandString, + payload: &[u8], + ) -> Result { let raw = || Vec::from(payload); let msg = match *cmd { $( CommandString::$p_cmd => Self::$p_variant(crate::macros::decode_msg(payload)?), )* From 49ef62aa1d3e79495c32bfbfc442acdacf4d3aea Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 16:28:55 +0000 Subject: [PATCH 05/17] p2p_core%fix!: take V2 message types as the reference transport does Contents that end before the type give `MissingCommand`. An unassigned short ID within the reference implementation's tables decodes as an empty command, which is ignored rather than rejected. --- pkgs/p2p_core/corpus/transport.json5 | 264 ++++++++++++++++++++++++++- pkgs/p2p_core/src/bip324.rs | 28 ++- pkgs/p2p_core/src/error.rs | 5 +- pkgs/p2p_core/src/short_id.rs | 11 ++ 4 files changed, 299 insertions(+), 9 deletions(-) diff --git a/pkgs/p2p_core/corpus/transport.json5 b/pkgs/p2p_core/corpus/transport.json5 index 1fb6136de..338a0f69a 100644 --- a/pkgs/p2p_core/corpus/transport.json5 +++ b/pkgs/p2p_core/corpus/transport.json5 @@ -76,8 +76,14 @@ BOOST_AUTO_TEST_CASE(transport_v2) "pi\0ng"sv, "ping\0\0\0\0\0\0\0\x01"sv}) { cases.emplace_back(long_form(name), 1 + CMessageHeader::COMMAND_SIZE); } + for (const size_t len : {size_t{0}, size_t{1}, CMessageHeader::COMMAND_SIZE}) { + cases.emplace_back(std::vector(std::min(len, 1)), len); + } + for (int id = 1; id <= 0xff; ++id) { + cases.emplace_back(std::vector{static_cast(id)}, 1); + } const size_t max_contents{1 + CMessageHeader::COMMAND_SIZE + MAX_PROTOCOL_MESSAGE_LENGTH}; - for (const size_t len : {size_t{1}, max_contents, max_contents + 1}) { + for (const size_t len : {max_contents, max_contents + 1}) { cases.emplace_back(std::vector{21}, len); cases.emplace_back(std::vector{0xff}, len); } @@ -102,7 +108,263 @@ BOOST_AUTO_TEST_CASE(transport_v2) { "prefix": "007069806e6700000000000000", "len": 13, "result": "reject" }, { "prefix": "007069006e6700000000000000", "len": 13, "result": "reject" }, { "prefix": "0070696e670000000000000001", "len": 13, "result": "reject" }, + { "prefix": "", "len": 0, "result": "reject" }, + { "prefix": "00", "len": 1, "result": "reject" }, + { "prefix": "00", "len": 12, "result": "reject" }, + { "prefix": "01", "len": 1, "result": "accept" }, + { "prefix": "02", "len": 1, "result": "accept" }, + { "prefix": "03", "len": 1, "result": "accept" }, + { "prefix": "04", "len": 1, "result": "accept" }, + { "prefix": "05", "len": 1, "result": "accept" }, + { "prefix": "06", "len": 1, "result": "accept" }, + { "prefix": "07", "len": 1, "result": "accept" }, + { "prefix": "08", "len": 1, "result": "accept" }, + { "prefix": "09", "len": 1, "result": "accept" }, + { "prefix": "0a", "len": 1, "result": "accept" }, + { "prefix": "0b", "len": 1, "result": "accept" }, + { "prefix": "0c", "len": 1, "result": "accept" }, + { "prefix": "0d", "len": 1, "result": "accept" }, + { "prefix": "0e", "len": 1, "result": "accept" }, + { "prefix": "0f", "len": 1, "result": "accept" }, + { "prefix": "10", "len": 1, "result": "accept" }, + { "prefix": "11", "len": 1, "result": "accept" }, + { "prefix": "12", "len": 1, "result": "accept" }, + { "prefix": "13", "len": 1, "result": "accept" }, + { "prefix": "14", "len": 1, "result": "accept" }, { "prefix": "15", "len": 1, "result": "accept" }, + { "prefix": "16", "len": 1, "result": "accept" }, + { "prefix": "17", "len": 1, "result": "accept" }, + { "prefix": "18", "len": 1, "result": "accept" }, + { "prefix": "19", "len": 1, "result": "accept" }, + { "prefix": "1a", "len": 1, "result": "accept" }, + { "prefix": "1b", "len": 1, "result": "accept" }, + { "prefix": "1c", "len": 1, "result": "accept" }, + { "prefix": "1d", "len": 1, "result": "accept" }, + { "prefix": "1e", "len": 1, "result": "accept" }, + { "prefix": "1f", "len": 1, "result": "accept" }, + { "prefix": "20", "len": 1, "result": "accept" }, + { "prefix": "21", "len": 1, "result": "reject" }, + { "prefix": "22", "len": 1, "result": "reject" }, + { "prefix": "23", "len": 1, "result": "reject" }, + { "prefix": "24", "len": 1, "result": "reject" }, + { "prefix": "25", "len": 1, "result": "reject" }, + { "prefix": "26", "len": 1, "result": "reject" }, + { "prefix": "27", "len": 1, "result": "reject" }, + { "prefix": "28", "len": 1, "result": "reject" }, + { "prefix": "29", "len": 1, "result": "reject" }, + { "prefix": "2a", "len": 1, "result": "reject" }, + { "prefix": "2b", "len": 1, "result": "reject" }, + { "prefix": "2c", "len": 1, "result": "reject" }, + { "prefix": "2d", "len": 1, "result": "reject" }, + { "prefix": "2e", "len": 1, "result": "reject" }, + { "prefix": "2f", "len": 1, "result": "reject" }, + { "prefix": "30", "len": 1, "result": "reject" }, + { "prefix": "31", "len": 1, "result": "reject" }, + { "prefix": "32", "len": 1, "result": "reject" }, + { "prefix": "33", "len": 1, "result": "reject" }, + { "prefix": "34", "len": 1, "result": "reject" }, + { "prefix": "35", "len": 1, "result": "reject" }, + { "prefix": "36", "len": 1, "result": "reject" }, + { "prefix": "37", "len": 1, "result": "reject" }, + { "prefix": "38", "len": 1, "result": "reject" }, + { "prefix": "39", "len": 1, "result": "reject" }, + { "prefix": "3a", "len": 1, "result": "reject" }, + { "prefix": "3b", "len": 1, "result": "reject" }, + { "prefix": "3c", "len": 1, "result": "reject" }, + { "prefix": "3d", "len": 1, "result": "reject" }, + { "prefix": "3e", "len": 1, "result": "reject" }, + { "prefix": "3f", "len": 1, "result": "reject" }, + { "prefix": "40", "len": 1, "result": "reject" }, + { "prefix": "41", "len": 1, "result": "reject" }, + { "prefix": "42", "len": 1, "result": "reject" }, + { "prefix": "43", "len": 1, "result": "reject" }, + { "prefix": "44", "len": 1, "result": "reject" }, + { "prefix": "45", "len": 1, "result": "reject" }, + { "prefix": "46", "len": 1, "result": "reject" }, + { "prefix": "47", "len": 1, "result": "reject" }, + { "prefix": "48", "len": 1, "result": "reject" }, + { "prefix": "49", "len": 1, "result": "reject" }, + { "prefix": "4a", "len": 1, "result": "reject" }, + { "prefix": "4b", "len": 1, "result": "reject" }, + { "prefix": "4c", "len": 1, "result": "reject" }, + { "prefix": "4d", "len": 1, "result": "reject" }, + { "prefix": "4e", "len": 1, "result": "reject" }, + { "prefix": "4f", "len": 1, "result": "reject" }, + { "prefix": "50", "len": 1, "result": "reject" }, + { "prefix": "51", "len": 1, "result": "reject" }, + { "prefix": "52", "len": 1, "result": "reject" }, + { "prefix": "53", "len": 1, "result": "reject" }, + { "prefix": "54", "len": 1, "result": "reject" }, + { "prefix": "55", "len": 1, "result": "reject" }, + { "prefix": "56", "len": 1, "result": "reject" }, + { "prefix": "57", "len": 1, "result": "reject" }, + { "prefix": "58", "len": 1, "result": "reject" }, + { "prefix": "59", "len": 1, "result": "reject" }, + { "prefix": "5a", "len": 1, "result": "reject" }, + { "prefix": "5b", "len": 1, "result": "reject" }, + { "prefix": "5c", "len": 1, "result": "reject" }, + { "prefix": "5d", "len": 1, "result": "reject" }, + { "prefix": "5e", "len": 1, "result": "reject" }, + { "prefix": "5f", "len": 1, "result": "reject" }, + { "prefix": "60", "len": 1, "result": "reject" }, + { "prefix": "61", "len": 1, "result": "reject" }, + { "prefix": "62", "len": 1, "result": "reject" }, + { "prefix": "63", "len": 1, "result": "reject" }, + { "prefix": "64", "len": 1, "result": "reject" }, + { "prefix": "65", "len": 1, "result": "reject" }, + { "prefix": "66", "len": 1, "result": "reject" }, + { "prefix": "67", "len": 1, "result": "reject" }, + { "prefix": "68", "len": 1, "result": "reject" }, + { "prefix": "69", "len": 1, "result": "reject" }, + { "prefix": "6a", "len": 1, "result": "reject" }, + { "prefix": "6b", "len": 1, "result": "reject" }, + { "prefix": "6c", "len": 1, "result": "reject" }, + { "prefix": "6d", "len": 1, "result": "reject" }, + { "prefix": "6e", "len": 1, "result": "reject" }, + { "prefix": "6f", "len": 1, "result": "reject" }, + { "prefix": "70", "len": 1, "result": "reject" }, + { "prefix": "71", "len": 1, "result": "reject" }, + { "prefix": "72", "len": 1, "result": "reject" }, + { "prefix": "73", "len": 1, "result": "reject" }, + { "prefix": "74", "len": 1, "result": "reject" }, + { "prefix": "75", "len": 1, "result": "reject" }, + { "prefix": "76", "len": 1, "result": "reject" }, + { "prefix": "77", "len": 1, "result": "reject" }, + { "prefix": "78", "len": 1, "result": "reject" }, + { "prefix": "79", "len": 1, "result": "reject" }, + { "prefix": "7a", "len": 1, "result": "reject" }, + { "prefix": "7b", "len": 1, "result": "reject" }, + { "prefix": "7c", "len": 1, "result": "reject" }, + { "prefix": "7d", "len": 1, "result": "reject" }, + { "prefix": "7e", "len": 1, "result": "reject" }, + { "prefix": "7f", "len": 1, "result": "reject" }, + { "prefix": "80", "len": 1, "result": "accept" }, + { "prefix": "81", "len": 1, "result": "accept" }, + { "prefix": "82", "len": 1, "result": "accept" }, + { "prefix": "83", "len": 1, "result": "accept" }, + { "prefix": "84", "len": 1, "result": "accept" }, + { "prefix": "85", "len": 1, "result": "accept" }, + { "prefix": "86", "len": 1, "result": "accept" }, + { "prefix": "87", "len": 1, "result": "accept" }, + { "prefix": "88", "len": 1, "result": "accept" }, + { "prefix": "89", "len": 1, "result": "accept" }, + { "prefix": "8a", "len": 1, "result": "accept" }, + { "prefix": "8b", "len": 1, "result": "accept" }, + { "prefix": "8c", "len": 1, "result": "accept" }, + { "prefix": "8d", "len": 1, "result": "accept" }, + { "prefix": "8e", "len": 1, "result": "accept" }, + { "prefix": "8f", "len": 1, "result": "accept" }, + { "prefix": "90", "len": 1, "result": "accept" }, + { "prefix": "91", "len": 1, "result": "accept" }, + { "prefix": "92", "len": 1, "result": "accept" }, + { "prefix": "93", "len": 1, "result": "accept" }, + { "prefix": "94", "len": 1, "result": "accept" }, + { "prefix": "95", "len": 1, "result": "accept" }, + { "prefix": "96", "len": 1, "result": "accept" }, + { "prefix": "97", "len": 1, "result": "accept" }, + { "prefix": "98", "len": 1, "result": "accept" }, + { "prefix": "99", "len": 1, "result": "accept" }, + { "prefix": "9a", "len": 1, "result": "accept" }, + { "prefix": "9b", "len": 1, "result": "accept" }, + { "prefix": "9c", "len": 1, "result": "accept" }, + { "prefix": "9d", "len": 1, "result": "accept" }, + { "prefix": "9e", "len": 1, "result": "accept" }, + { "prefix": "9f", "len": 1, "result": "accept" }, + { "prefix": "a0", "len": 1, "result": "accept" }, + { "prefix": "a1", "len": 1, "result": "accept" }, + { "prefix": "a2", "len": 1, "result": "accept" }, + { "prefix": "a3", "len": 1, "result": "accept" }, + { "prefix": "a4", "len": 1, "result": "accept" }, + { "prefix": "a5", "len": 1, "result": "accept" }, + { "prefix": "a6", "len": 1, "result": "accept" }, + { "prefix": "a7", "len": 1, "result": "accept" }, + { "prefix": "a8", "len": 1, "result": "accept" }, + { "prefix": "a9", "len": 1, "result": "reject" }, + { "prefix": "aa", "len": 1, "result": "reject" }, + { "prefix": "ab", "len": 1, "result": "reject" }, + { "prefix": "ac", "len": 1, "result": "reject" }, + { "prefix": "ad", "len": 1, "result": "reject" }, + { "prefix": "ae", "len": 1, "result": "reject" }, + { "prefix": "af", "len": 1, "result": "reject" }, + { "prefix": "b0", "len": 1, "result": "reject" }, + { "prefix": "b1", "len": 1, "result": "reject" }, + { "prefix": "b2", "len": 1, "result": "reject" }, + { "prefix": "b3", "len": 1, "result": "reject" }, + { "prefix": "b4", "len": 1, "result": "reject" }, + { "prefix": "b5", "len": 1, "result": "reject" }, + { "prefix": "b6", "len": 1, "result": "reject" }, + { "prefix": "b7", "len": 1, "result": "reject" }, + { "prefix": "b8", "len": 1, "result": "reject" }, + { "prefix": "b9", "len": 1, "result": "reject" }, + { "prefix": "ba", "len": 1, "result": "reject" }, + { "prefix": "bb", "len": 1, "result": "reject" }, + { "prefix": "bc", "len": 1, "result": "reject" }, + { "prefix": "bd", "len": 1, "result": "reject" }, + { "prefix": "be", "len": 1, "result": "reject" }, + { "prefix": "bf", "len": 1, "result": "reject" }, + { "prefix": "c0", "len": 1, "result": "reject" }, + { "prefix": "c1", "len": 1, "result": "reject" }, + { "prefix": "c2", "len": 1, "result": "reject" }, + { "prefix": "c3", "len": 1, "result": "reject" }, + { "prefix": "c4", "len": 1, "result": "reject" }, + { "prefix": "c5", "len": 1, "result": "reject" }, + { "prefix": "c6", "len": 1, "result": "reject" }, + { "prefix": "c7", "len": 1, "result": "reject" }, + { "prefix": "c8", "len": 1, "result": "reject" }, + { "prefix": "c9", "len": 1, "result": "reject" }, + { "prefix": "ca", "len": 1, "result": "reject" }, + { "prefix": "cb", "len": 1, "result": "reject" }, + { "prefix": "cc", "len": 1, "result": "reject" }, + { "prefix": "cd", "len": 1, "result": "reject" }, + { "prefix": "ce", "len": 1, "result": "reject" }, + { "prefix": "cf", "len": 1, "result": "reject" }, + { "prefix": "d0", "len": 1, "result": "reject" }, + { "prefix": "d1", "len": 1, "result": "reject" }, + { "prefix": "d2", "len": 1, "result": "reject" }, + { "prefix": "d3", "len": 1, "result": "reject" }, + { "prefix": "d4", "len": 1, "result": "reject" }, + { "prefix": "d5", "len": 1, "result": "reject" }, + { "prefix": "d6", "len": 1, "result": "reject" }, + { "prefix": "d7", "len": 1, "result": "reject" }, + { "prefix": "d8", "len": 1, "result": "reject" }, + { "prefix": "d9", "len": 1, "result": "reject" }, + { "prefix": "da", "len": 1, "result": "reject" }, + { "prefix": "db", "len": 1, "result": "reject" }, + { "prefix": "dc", "len": 1, "result": "reject" }, + { "prefix": "dd", "len": 1, "result": "reject" }, + { "prefix": "de", "len": 1, "result": "reject" }, + { "prefix": "df", "len": 1, "result": "reject" }, + { "prefix": "e0", "len": 1, "result": "reject" }, + { "prefix": "e1", "len": 1, "result": "reject" }, + { "prefix": "e2", "len": 1, "result": "reject" }, + { "prefix": "e3", "len": 1, "result": "reject" }, + { "prefix": "e4", "len": 1, "result": "reject" }, + { "prefix": "e5", "len": 1, "result": "reject" }, + { "prefix": "e6", "len": 1, "result": "reject" }, + { "prefix": "e7", "len": 1, "result": "reject" }, + { "prefix": "e8", "len": 1, "result": "reject" }, + { "prefix": "e9", "len": 1, "result": "reject" }, + { "prefix": "ea", "len": 1, "result": "reject" }, + { "prefix": "eb", "len": 1, "result": "reject" }, + { "prefix": "ec", "len": 1, "result": "reject" }, + { "prefix": "ed", "len": 1, "result": "reject" }, + { "prefix": "ee", "len": 1, "result": "reject" }, + { "prefix": "ef", "len": 1, "result": "reject" }, + { "prefix": "f0", "len": 1, "result": "reject" }, + { "prefix": "f1", "len": 1, "result": "reject" }, + { "prefix": "f2", "len": 1, "result": "reject" }, + { "prefix": "f3", "len": 1, "result": "reject" }, + { "prefix": "f4", "len": 1, "result": "reject" }, + { "prefix": "f5", "len": 1, "result": "reject" }, + { "prefix": "f6", "len": 1, "result": "reject" }, + { "prefix": "f7", "len": 1, "result": "reject" }, + { "prefix": "f8", "len": 1, "result": "reject" }, + { "prefix": "f9", "len": 1, "result": "reject" }, + { "prefix": "fa", "len": 1, "result": "reject" }, + { "prefix": "fb", "len": 1, "result": "reject" }, + { "prefix": "fc", "len": 1, "result": "reject" }, + { "prefix": "fd", "len": 1, "result": "reject" }, + { "prefix": "fe", "len": 1, "result": "reject" }, { "prefix": "ff", "len": 1, "result": "reject" }, { "prefix": "15", "len": 3145741, "result": "accept" }, { "prefix": "ff", "len": 3145741, "result": "reject" }, diff --git a/pkgs/p2p_core/src/bip324.rs b/pkgs/p2p_core/src/bip324.rs index 7ffa01ae3..2cc0fb616 100644 --- a/pkgs/p2p_core/src/bip324.rs +++ b/pkgs/p2p_core/src/bip324.rs @@ -43,16 +43,20 @@ const MAX_V2_CONTENTS: usize = 1 + 12 + MAX_P2P_PAYLOAD_SIZE; /// # Errors /// /// Returns `PayloadTooLarge` above the bound, with an empty command for an -/// unknown short ID. Returns `UnknownShortId` or `InvalidCommand` for a type -/// the transport refuses, and otherwise the error decoding the message. +/// unknown short ID. Returns `MissingCommand`, `UnknownShortId` or +/// `InvalidCommand` for a type the transport refuses, and otherwise the error +/// decoding the message. pub fn decode_v2(contents: &[u8]) -> Result { let mut payload = contents; - let [short_id] = take(&mut payload)?; + let [short_id] = take(&mut payload).map_err(|_| P2pDecodeError::MissingCommand)?; let id = ShortId(short_id); let cmd = if id == ShortId::LONG_FORMAT { - Some(CommandString::from_bytes(take(&mut payload)?)) + let bytes = take(&mut payload).map_err(|_| P2pDecodeError::MissingCommand)?; + Some(CommandString::from_bytes(bytes)) + } else if id.is_in_table() { + Some(id.to_command().unwrap_or(CommandString::from_static(""))) } else { - id.to_command() + None }; if contents.len() > MAX_V2_CONTENTS { @@ -94,8 +98,18 @@ mod tests { contents.resize(v.len, 0); let result = match decode_v2(&contents) { Err(P2pDecodeError::PayloadTooLarge { .. }) => "disconnect", - Err(P2pDecodeError::InvalidCommand { .. } | P2pDecodeError::UnknownShortId { .. }) => "reject", - _ => "accept", + Err( + P2pDecodeError::MissingCommand + | P2pDecodeError::InvalidCommand { .. } + | P2pDecodeError::UnknownShortId { .. }, + ) => "reject", + Ok(_) + | Err( + P2pDecodeError::UnknownCommand { .. } + | P2pDecodeError::Consensus(_) + | P2pDecodeError::PayloadNotEmpty { .. } + | P2pDecodeError::InvalidValue { .. }, + ) => "accept", }; assert_eq!(result, v.result, "{} of {} bytes", v.prefix, v.len); } diff --git a/pkgs/p2p_core/src/error.rs b/pkgs/p2p_core/src/error.rs index 48d90a003..5ff7de026 100644 --- a/pkgs/p2p_core/src/error.rs +++ b/pkgs/p2p_core/src/error.rs @@ -25,13 +25,16 @@ pub enum P2pDecodeError { /// The raw command bytes. bytes: [u8; 12], }, + /// V2 contents end before the message type does. + #[error("missing command")] + MissingCommand, /// A V2 long-form command that fails [`CommandString::is_valid_v2`]. #[error(fmt = fmt_invalid_command)] InvalidCommand { /// The command as received. command: CommandString, }, - /// V2 short ID does not map to a known message. + /// V2 short ID outside the short ID tables. #[error("unknown v2 short id: {id}")] UnknownShortId { /// The short ID byte. diff --git a/pkgs/p2p_core/src/short_id.rs b/pkgs/p2p_core/src/short_id.rs index 8502e5d71..f6f99fd93 100644 --- a/pkgs/p2p_core/src/short_id.rs +++ b/pkgs/p2p_core/src/short_id.rs @@ -10,6 +10,12 @@ use crate::command::CommandString; use dash_types::type_id::Unencodable; +/// Number of short IDs from 0 that a V2 transport takes. +const LOW_TABLE_LEN: u8 = 33; + +/// Number of short IDs from 128 that a V2 transport takes. +const HIGH_TABLE_LEN: u8 = 41; + /// A resolved V2 short ID. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Unencodable)] #[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))] @@ -24,6 +30,11 @@ impl ShortId { self.to_command_str().is_some() } + /// Whether a V2 transport takes the ID as a message type, assigned or not. + pub const fn is_in_table(self) -> bool { + self.0 < LOW_TABLE_LEN || (self.0 >= 128 && self.0 - 128 < HIGH_TABLE_LEN) + } + /// Resolves the short ID to a `CommandString`, if known. pub fn to_command(self) -> Option { self.to_command_str().map(CommandString::from_static) From 8cfb0a3ba27cde703330a899455a79093dff97d8 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 16:29:56 +0000 Subject: [PATCH 06/17] p2p_core%fix!: refuse commands the reference V1 transport refuses `P2pMsg::decode_payload` gives `InvalidCommand` when a command fails `CommandString::is_valid_v1`: printable bytes up to the first NUL, then only NULs. --- pkgs/p2p_core/corpus/transport.json5 | 36 ++++++++++++++++++++++++++++ pkgs/p2p_core/src/command.rs | 12 ++++++++-- pkgs/p2p_core/src/error.rs | 3 ++- pkgs/p2p_core/src/macros.rs | 6 ++++- pkgs/p2p_core/src/msg/mod.rs | 17 +++++++++++++ 5 files changed, 70 insertions(+), 4 deletions(-) diff --git a/pkgs/p2p_core/corpus/transport.json5 b/pkgs/p2p_core/corpus/transport.json5 index 338a0f69a..c7ad28ed0 100644 --- a/pkgs/p2p_core/corpus/transport.json5 +++ b/pkgs/p2p_core/corpus/transport.json5 @@ -44,6 +44,42 @@ BOOST_AUTO_TEST_CASE(transport_v1_size) { "command": "70691f6e6700000000000000", "size": 3145729, "disconnect": true }, ], /* +BOOST_AUTO_TEST_CASE(transport_v1_command) +{ + using namespace std::literals; + for (const auto name : {"ping"sv, "abcdefghijkl"sv, "a b"sv, ""sv, " "sv, "~"sv, "pi\x7fng"sv, "pi\x1fng"sv, + "pi\x80ng"sv, "\xffping"sv, "pi\0ng"sv, "ping\0\0\0\0\0\0\0\x01"sv}) { + V1Transport transport{0, SER_NETWORK, INIT_PROTO_VERSION}; + CMessageHeader hdr{Params().MessageStart(), "", 0}; + std::copy(name.begin(), name.end(), hdr.pchCommand); + const uint256 hash{Hash(Span{})}; + std::copy(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE, hdr.pchChecksum); + CDataStream ss{SER_NETWORK, PROTOCOL_VERSION}; + ss << hdr; + Span bytes{UCharCast(ss.data()), ss.size()}; + BOOST_REQUIRE(transport.ReceivedBytes(bytes) && transport.ReceivedMessageComplete()); + bool reject{false}; + transport.GetReceivedMessage({}, reject); + std::printf(" { \"command\": \"%s\", \"valid\": %s },\n", HexStr(Span{hdr.pchCommand}).c_str(), + reject ? "false" : "true"); + } +} +*/ + "v1_command": [ + { "command": "70696e670000000000000000", "valid": true }, + { "command": "6162636465666768696a6b6c", "valid": true }, + { "command": "612062000000000000000000", "valid": true }, + { "command": "000000000000000000000000", "valid": true }, + { "command": "200000000000000000000000", "valid": true }, + { "command": "7e0000000000000000000000", "valid": true }, + { "command": "70697f6e6700000000000000", "valid": false }, + { "command": "70691f6e6700000000000000", "valid": false }, + { "command": "7069806e6700000000000000", "valid": false }, + { "command": "ff70696e6700000000000000", "valid": false }, + { "command": "7069006e6700000000000000", "valid": false }, + { "command": "70696e670000000000000001", "valid": false }, + ], +/* BOOST_AUTO_TEST_CASE(transport_v2) { using namespace std::literals; diff --git a/pkgs/p2p_core/src/command.rs b/pkgs/p2p_core/src/command.rs index bd8c69db8..59843716a 100644 --- a/pkgs/p2p_core/src/command.rs +++ b/pkgs/p2p_core/src/command.rs @@ -51,13 +51,21 @@ impl CommandString { } /// Returns the command as a `&str` (trimmed of null padding). + /// + /// A command that is not UTF-8 up to its first NUL gives the empty string. pub fn as_str(&self) -> &str { let end = self.0.iter().position(|&b| b == 0).unwrap_or(12); - // The bytes are always valid ASCII written by from_static or - // validated on decode, so this conversion is sound. core::str::from_utf8(&self.0[..end]).unwrap_or("") } + /// Whether a V1 transport accepts the command. + /// + /// Bytes before the first NUL must be in 0x20 to 0x7e, and every byte after + /// it NUL. + pub const fn is_valid_v1(&self) -> bool { + self.is_valid_up_to(0x7e) + } + /// Whether a V2 transport accepts the command as a long-form type. /// /// Bytes before the first NUL must be in 0x20 to 0x7f, and every byte after diff --git a/pkgs/p2p_core/src/error.rs b/pkgs/p2p_core/src/error.rs index 5ff7de026..e8ff56140 100644 --- a/pkgs/p2p_core/src/error.rs +++ b/pkgs/p2p_core/src/error.rs @@ -28,7 +28,8 @@ pub enum P2pDecodeError { /// V2 contents end before the message type does. #[error("missing command")] MissingCommand, - /// A V2 long-form command that fails [`CommandString::is_valid_v2`]. + /// A command its transport refuses, per [`CommandString::is_valid_v1`] or + /// [`CommandString::is_valid_v2`]. #[error(fmt = fmt_invalid_command)] InvalidCommand { /// The command as received. diff --git a/pkgs/p2p_core/src/macros.rs b/pkgs/p2p_core/src/macros.rs index f62d2357d..d31731f7e 100644 --- a/pkgs/p2p_core/src/macros.rs +++ b/pkgs/p2p_core/src/macros.rs @@ -166,12 +166,16 @@ macro_rules! define_p2p { /// Decodes a message from its command string and raw payload. /// - /// Every command, known or not, is bounded by the 3 MiB limit. + /// Every command, known or not, is bounded by the 3 MiB limit. A command + /// that fails [`CommandString::is_valid_v1`] gives `InvalidCommand`. pub fn decode_payload( cmd: &CommandString, payload: &[u8], ) -> Result { crate::macros::check_payload(cmd, payload)?; + if !cmd.is_valid_v1() { + return Err(crate::P2pDecodeError::InvalidCommand { command: *cmd }); + } Self::decode_unbounded(cmd, payload) } diff --git a/pkgs/p2p_core/src/msg/mod.rs b/pkgs/p2p_core/src/msg/mod.rs index 7a69c857f..e8e5dcdf9 100644 --- a/pkgs/p2p_core/src/msg/mod.rs +++ b/pkgs/p2p_core/src/msg/mod.rs @@ -222,4 +222,21 @@ mod tests { assert_eq!(too_large, v.disconnect, "{} of {} bytes", v.command, v.size); } } + + #[derive(Deserialize)] + struct CommandVector { + command: String, + valid: bool, + } + + #[rstest] + fn invalid_commands_match_vectors() { + let corpus = Corpus::open(env!("CARGO_MANIFEST_DIR"), "transport"); + for v in corpus.vectors::("v1_command") { + let cmd = CommandString::from_bytes(decode_to_array(&v.command).unwrap()); + let result = P2pMsg::decode_payload(&cmd, &[]); + let invalid = matches!(result, Err(P2pDecodeError::InvalidCommand { .. })); + assert_eq!(invalid, !v.valid, "{}", v.command); + } + } } From 8626fe1caa87b076ee34e84b46e31ab2f34c1057 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 16:57:29 +0000 Subject: [PATCH 07/17] p2p_core%feat!: name every inventory type and map it to its command `InvType` names each type the reference implementation defines. Its `command` gives the message that carries the object, a `tx` for an asset unlock, and `is_tx` and `is_block` group the types. --- pkgs/p2p_core/corpus/inv.json5 | 61 +++++++++++++++++++++++- pkgs/p2p_core/src/msg/inv.rs | 86 ++++++++++++++++++++++++++++++++++ 2 files changed, 146 insertions(+), 1 deletion(-) diff --git a/pkgs/p2p_core/corpus/inv.json5 b/pkgs/p2p_core/corpus/inv.json5 index feb82db62..a7638c5fd 100644 --- a/pkgs/p2p_core/corpus/inv.json5 +++ b/pkgs/p2p_core/corpus/inv.json5 @@ -52,5 +52,64 @@ ] } }, - } + }, + // Generated with the reference implementation (dashpay/dash v24.0.0-rc.3, + // 0364ed626e). To run the program, paste it in `src/bitcoin-util.cpp` at the + // specified revision, build it with `make -C src dash-util` and run + // `./src/dash-util`. The output should match the section. +/* +#include + +#include + +int main() +{ + for (uint32_t type = 0; type <= 36; ++type) { + const CInv inv{type, uint256{}}; + std::printf(" { \"type\": %u, \"command\": %s%s%s, \"tx\": %s, \"block\": %s },\n", type, + inv.IsKnownType() ? "\"" : "", inv.IsKnownType() ? inv.GetCommand().c_str() : "null", + inv.IsKnownType() ? "\"" : "", inv.IsGenTxMsg() ? "true" : "false", + inv.IsGenBlkMsg() ? "true" : "false"); + } +} +*/ + "inv_type": [ + { "type": 0, "command": null, "tx": false, "block": false }, + { "type": 1, "command": "tx", "tx": true, "block": false }, + { "type": 2, "command": "block", "tx": false, "block": true }, + { "type": 3, "command": "merkleblock", "tx": false, "block": true }, + { "type": 4, "command": null, "tx": false, "block": false }, + { "type": 5, "command": null, "tx": false, "block": false }, + { "type": 6, "command": "spork", "tx": false, "block": false }, + { "type": 7, "command": null, "tx": false, "block": false }, + { "type": 8, "command": null, "tx": false, "block": false }, + { "type": 9, "command": null, "tx": false, "block": false }, + { "type": 10, "command": null, "tx": false, "block": false }, + { "type": 11, "command": null, "tx": false, "block": false }, + { "type": 12, "command": null, "tx": false, "block": false }, + { "type": 13, "command": null, "tx": false, "block": false }, + { "type": 14, "command": null, "tx": false, "block": false }, + { "type": 15, "command": null, "tx": false, "block": false }, + { "type": 16, "command": "dstx", "tx": true, "block": false }, + { "type": 17, "command": "govobj", "tx": false, "block": false }, + { "type": 18, "command": "govobjvote", "tx": false, "block": false }, + { "type": 19, "command": null, "tx": false, "block": false }, + { "type": 20, "command": "cmpctblock", "tx": false, "block": true }, + { "type": 21, "command": "qfcommit", "tx": false, "block": false }, + { "type": 22, "command": null, "tx": false, "block": false }, + { "type": 23, "command": "qcontrib", "tx": false, "block": false }, + { "type": 24, "command": "qcomplaint", "tx": false, "block": false }, + { "type": 25, "command": "qjustify", "tx": false, "block": false }, + { "type": 26, "command": "qpcommit", "tx": false, "block": false }, + { "type": 27, "command": null, "tx": false, "block": false }, + { "type": 28, "command": "qsigrec", "tx": false, "block": false }, + { "type": 29, "command": "clsig", "tx": false, "block": false }, + { "type": 30, "command": null, "tx": false, "block": false }, + { "type": 31, "command": "isdlock", "tx": false, "block": false }, + { "type": 32, "command": "dsq", "tx": false, "block": false }, + { "type": 33, "command": "platformban", "tx": false, "block": false }, + { "type": 34, "command": "tx", "tx": true, "block": false }, + { "type": 35, "command": null, "tx": false, "block": false }, + { "type": 36, "command": null, "tx": false, "block": false }, + ], } diff --git a/pkgs/p2p_core/src/msg/inv.rs b/pkgs/p2p_core/src/msg/inv.rs index c48fb0ace..256e488fd 100644 --- a/pkgs/p2p_core/src/msg/inv.rs +++ b/pkgs/p2p_core/src/msg/inv.rs @@ -7,6 +7,7 @@ //! Inventory messages and types. use crate::codec::codec_p2p; +use crate::command::CommandString; use crate::prelude::*; use dash_num::Hash256; @@ -38,12 +39,38 @@ enum_map! { Block = 2 => "block", /// Filtered block (BIP37). FilteredBlock = 3 => "filtered_block", + /// Spork. + Spork = 6 => "spork", + /// CoinJoin broadcast transaction. + Dstx = 16 => "dstx", /// Governance object. GovernanceObject = 17 => "governance_object", /// Governance object vote. GovernanceObjectVote = 18 => "governance_object_vote", /// Compact block (BIP152). CompactBlock = 20 => "compact_block", + /// LLMQ final commitment. + QuorumFinalCommitment = 21 => "quorum_final_commitment", + /// LLMQ contribution. + QuorumContrib = 23 => "quorum_contrib", + /// LLMQ complaint. + QuorumComplaint = 24 => "quorum_complaint", + /// LLMQ justification. + QuorumJustification = 25 => "quorum_justification", + /// LLMQ premature commitment. + QuorumPrematureCommitment = 26 => "quorum_premature_commitment", + /// LLMQ recovered signature. + QuorumRecoveredSig = 28 => "quorum_recovered_sig", + /// ChainLock signature. + ClSig = 29 => "cl_sig", + /// InstantSend deterministic lock. + IsdLock = 31 => "isd_lock", + /// CoinJoin queue entry. + Dsq = 32 => "dsq", + /// Platform ban (DIP-0031). + PlatformBan = 33 => "platform_ban", + /// Version 2 asset unlock, announced by its instance hash. + AssetUnlock = 34 => "asset_unlock", } } @@ -51,6 +78,44 @@ impl_num!(InvType, u32); hash_impl!(InvType); +impl InvType { + /// The command of the message that carries the object, if the type is + /// known. + pub fn command(self) -> Option { + Some(match self { + Self::Tx | Self::AssetUnlock => CommandString::TX, + Self::Block => CommandString::BLOCK, + Self::FilteredBlock => CommandString::MERKLEBLOCK, + Self::CompactBlock => CommandString::CMPCTBLOCK, + Self::Spork => CommandString::SPORK, + Self::Dstx => CommandString::DSTX, + Self::GovernanceObject => CommandString::GOVOBJ, + Self::GovernanceObjectVote => CommandString::GOVOBJVOTE, + Self::QuorumFinalCommitment => CommandString::QFCOMMIT, + Self::QuorumContrib => CommandString::QCONTRIB, + Self::QuorumComplaint => CommandString::QCOMPLAINT, + Self::QuorumJustification => CommandString::QJUSTIFY, + Self::QuorumPrematureCommitment => CommandString::QPCOMMIT, + Self::QuorumRecoveredSig => CommandString::QSIGREC, + Self::ClSig => CommandString::CLSIG, + Self::IsdLock => CommandString::ISDLOCK, + Self::Dsq => CommandString::DSQ, + Self::PlatformBan => CommandString::PLATFORMBAN, + Self::Error | Self::Unknown(_) => return None, + }) + } + + /// Whether the object is a transaction. + pub fn is_tx(self) -> bool { + matches!(self, Self::Tx | Self::Dstx | Self::AssetUnlock) + } + + /// Whether the object is a block. + pub fn is_block(self) -> bool { + matches!(self, Self::Block | Self::FilteredBlock | Self::CompactBlock) + } +} + /// An inventory vector: a typed 32-byte hash. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, TypeId)] #[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))] @@ -96,6 +161,16 @@ mod tests { use dash_dev::{assert_serde_rt, check_wire, Corpus}; use rstest::rstest; + use serde::Deserialize; + + #[derive(Deserialize)] + struct TypeVector { + #[serde(rename = "type")] + inv_type: u32, + command: Option, + tx: bool, + block: bool, + } #[rstest] fn corpus_inv() { @@ -103,4 +178,15 @@ mod tests { let items = corpus.entries::("inv", check_wire); assert_serde_rt("inv", &items); } + + #[rstest] + fn inv_type_matches_vectors() { + let corpus = Corpus::open(env!("CARGO_MANIFEST_DIR"), "inv"); + for v in corpus.vectors::("inv_type") { + let t = InvType::new(v.inv_type); + let command = t.command(); + assert_eq!(command.as_ref().map(CommandString::as_str), v.command.as_deref(), "{t}"); + assert_eq!((t.is_tx(), t.is_block()), (v.tx, v.block), "{t}"); + } + } } From a222647f46bda51accba2b88db609cfeeafdf5df Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 15:13:28 +0000 Subject: [PATCH 08/17] p2p_core%fix: accept up to 8000 headers in `headers2` A `headers2` reply carries up to 8000 headers. `Headers::MAX_COUNT` and `Headers2::MAX_COUNT` export the most each message carries. --- pkgs/p2p_core/src/msg/headers.rs | 10 +++++--- pkgs/p2p_core/src/msg/headers2.rs | 41 ++++++++++++++++++++++++++++--- 2 files changed, 43 insertions(+), 8 deletions(-) diff --git a/pkgs/p2p_core/src/msg/headers.rs b/pkgs/p2p_core/src/msg/headers.rs index 92d6183d3..8b82242b8 100644 --- a/pkgs/p2p_core/src/msg/headers.rs +++ b/pkgs/p2p_core/src/msg/headers.rs @@ -15,9 +15,6 @@ use dash_types::codec::{BaseCodec, DecodeError, EncodeBuf}; use dash_types::type_id::TypeId; use dash_types::CompactSize; -/// Maximum headers per message. -const MAX_HEADERS: usize = 2_000; - /// Requests block headers starting from a locator. #[derive(Clone, Debug, Eq, Hash, PartialEq, TypeId)] #[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))] @@ -51,7 +48,7 @@ impl_p2p!(Headers); impl BaseCodec for Headers { fn decode(data: &mut &[u8]) -> Result { - let count = CompactSize::decode(data)?.into_len(MAX_HEADERS)?; + let count = CompactSize::decode(data)?.into_len(Self::MAX_COUNT)?; let mut headers = Vec::with_capacity(count); for _ in 0..count { headers.push(BlockHeader { @@ -83,3 +80,8 @@ impl BaseCodec for Headers { } hash_impl!(Headers); + +impl Headers { + /// The most headers a `headers` message carries. + pub const MAX_COUNT: usize = 2_000; +} diff --git a/pkgs/p2p_core/src/msg/headers2.rs b/pkgs/p2p_core/src/msg/headers2.rs index c519821da..16ec4f70c 100644 --- a/pkgs/p2p_core/src/msg/headers2.rs +++ b/pkgs/p2p_core/src/msg/headers2.rs @@ -15,9 +15,6 @@ use dash_types::codec::{BaseCodec, DecodeError, EncodeBuf}; use dash_types::type_id::{TypeId, Unencodable}; use dash_types::{CompactSize, Hashable}; -/// Maximum headers per message. -const MAX_HEADERS: usize = 2_000; - /// Maximum entries in the MRU version cache. const MAX_VERSION_CACHE: usize = 7; @@ -255,7 +252,7 @@ impl_p2p!(Headers2); impl BaseCodec for Headers2 { fn decode(data: &mut &[u8]) -> Result { - let count = CompactSize::decode(data)?.into_len(MAX_HEADERS)?; + let count = CompactSize::decode(data)?.into_len(Self::MAX_COUNT)?; let mut state = CompressionState::new(); let mut headers = Vec::with_capacity(count); for _ in 0..count { @@ -275,11 +272,17 @@ impl BaseCodec for Headers2 { hash_impl!(Headers2); +impl Headers2 { + /// The most headers a `headers2` message carries. + pub const MAX_COUNT: usize = 8_000; +} + #[cfg(all(test, feature = "serde"))] mod tests { use super::*; use dash_dev::{assert_serde_rt, check_wire, Corpus}; + use dash_types::Numeric; use rstest::rstest; #[rstest] @@ -295,4 +298,34 @@ mod tests { let items = corpus.entries::("headers2", check_wire); assert_serde_rt("headers2", &items); } + + #[rstest] + fn counts_past_2000_decode() { + let headers = (0..2_001) + .map(|i| BlockHeader { + version: 0x2000_0000, + prev_hash: BlockHash::from_lendian([0; 32]), + merkle_root: MerkleRoot::from_lendian([1; 32]), + time: 1_700_000_000 + i, + bits: 0x1e0f_ffff, + nonce: i, + }) + .collect(); + let msg = Headers2 { headers }; + let mut buf = Vec::new(); + msg.encode(&mut buf); + assert_eq!(Headers2::decode(&mut &buf[..]), Ok(msg)); + } + + #[rstest] + fn counts_past_the_limit_fail_before_any_header() { + let payload = [0xfd, 0x41, 0x1f]; + assert_eq!( + Headers2::decode(&mut &payload[..]), + Err(DecodeError::CompactSizeExceedsLimit { + limit: Headers2::MAX_COUNT, + value: 8_001, + }) + ); + } } From b04e311d932b17e65e883a9e74d405664dfbcff3 Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 15:14:52 +0000 Subject: [PATCH 09/17] p2p_core%fix: bound header counts by the remaining input A count the input cannot hold at the smallest entry size is reported as `Eof` before anything is allocated for it. --- pkgs/p2p_core/src/msg/headers.rs | 36 +++++++++++++++++++++++++++++-- pkgs/p2p_core/src/msg/headers2.rs | 19 +++++++++++++++- 2 files changed, 52 insertions(+), 3 deletions(-) diff --git a/pkgs/p2p_core/src/msg/headers.rs b/pkgs/p2p_core/src/msg/headers.rs index 8b82242b8..22a56fd40 100644 --- a/pkgs/p2p_core/src/msg/headers.rs +++ b/pkgs/p2p_core/src/msg/headers.rs @@ -11,10 +11,23 @@ use crate::prelude::*; use crate::version::ProtocolVersion; use dash_primitives::{hash_impl, BlockHash, BlockHeader, MerkleRoot}; -use dash_types::codec::{BaseCodec, DecodeError, EncodeBuf}; +use dash_types::codec::{self, BaseCodec, DecodeError, EncodeBuf}; use dash_types::type_id::TypeId; use dash_types::CompactSize; +/// Smallest `headers` entry, an 80-byte header and a one-byte tx count. +const MIN_HEADERS_ENTRY: usize = 81; + +/// Reads an entry count of at most `limit`. +/// +/// A count the remaining input cannot hold at `min_entry` bytes each is +/// [`DecodeError::Eof`], reported before anything is allocated for it. +pub(super) fn decode_count(data: &mut &[u8], limit: usize, min_entry: usize) -> Result { + let count = CompactSize::decode(data)?.into_len(limit)?; + codec::ensure(data, count * min_entry)?; + Ok(count) +} + /// Requests block headers starting from a locator. #[derive(Clone, Debug, Eq, Hash, PartialEq, TypeId)] #[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))] @@ -48,7 +61,7 @@ impl_p2p!(Headers); impl BaseCodec for Headers { fn decode(data: &mut &[u8]) -> Result { - let count = CompactSize::decode(data)?.into_len(Self::MAX_COUNT)?; + let count = decode_count(data, Self::MAX_COUNT, MIN_HEADERS_ENTRY)?; let mut headers = Vec::with_capacity(count); for _ in 0..count { headers.push(BlockHeader { @@ -85,3 +98,22 @@ impl Headers { /// The most headers a `headers` message carries. pub const MAX_COUNT: usize = 2_000; } + +#[cfg(test)] +mod tests { + use super::*; + + use rstest::rstest; + + #[rstest] + fn counts_past_the_input_fail_before_any_header() { + let payload = [&[2][..], &[0; MIN_HEADERS_ENTRY]].concat(); + assert_eq!( + Headers::decode(&mut &payload[..]), + Err(DecodeError::Eof { + needed: 2 * MIN_HEADERS_ENTRY, + remaining: MIN_HEADERS_ENTRY, + }) + ); + } +} diff --git a/pkgs/p2p_core/src/msg/headers2.rs b/pkgs/p2p_core/src/msg/headers2.rs index 16ec4f70c..54e46e42b 100644 --- a/pkgs/p2p_core/src/msg/headers2.rs +++ b/pkgs/p2p_core/src/msg/headers2.rs @@ -6,6 +6,7 @@ //! Compressed header messages. +use super::headers::decode_count; use crate::codec::{codec_p2p, impl_p2p}; use crate::prelude::*; use crate::version::ProtocolVersion; @@ -15,6 +16,10 @@ use dash_types::codec::{BaseCodec, DecodeError, EncodeBuf}; use dash_types::type_id::{TypeId, Unencodable}; use dash_types::{CompactSize, Hashable}; +/// Smallest compressed header, made of flags, a merkle root, a time delta +/// and a nonce. +const MIN_HEADERS2_ENTRY: usize = 39; + /// Maximum entries in the MRU version cache. const MAX_VERSION_CACHE: usize = 7; @@ -252,7 +257,7 @@ impl_p2p!(Headers2); impl BaseCodec for Headers2 { fn decode(data: &mut &[u8]) -> Result { - let count = CompactSize::decode(data)?.into_len(Self::MAX_COUNT)?; + let count = decode_count(data, Self::MAX_COUNT, MIN_HEADERS2_ENTRY)?; let mut state = CompressionState::new(); let mut headers = Vec::with_capacity(count); for _ in 0..count { @@ -328,4 +333,16 @@ mod tests { }) ); } + + #[rstest] + fn counts_past_the_input_fail_before_any_header() { + let payload = [&[2][..], &[0; MIN_HEADERS2_ENTRY]].concat(); + assert_eq!( + Headers2::decode(&mut &payload[..]), + Err(DecodeError::Eof { + needed: 2 * MIN_HEADERS2_ENTRY, + remaining: MIN_HEADERS2_ENTRY, + }) + ); + } } From cfbc11909eba9a135cd9763a88f888b2b01fc72a Mon Sep 17 00:00:00 2001 From: pasta Date: Thu, 15 Oct 2026 15:15:26 +0000 Subject: [PATCH 10/17] p2p_core%feat: add `Headers2::max_count` for the negotiated limit `headers2` carries up to 8000 headers only from protocol version 70235, `ProtocolVersion::INCREASE_MAX_HEADERS2`; below it, 2000. --- pkgs/p2p_core/src/msg/headers2.rs | 19 ++++++++++++++++++- pkgs/p2p_core/src/version.rs | 2 ++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/pkgs/p2p_core/src/msg/headers2.rs b/pkgs/p2p_core/src/msg/headers2.rs index 54e46e42b..0a50107ac 100644 --- a/pkgs/p2p_core/src/msg/headers2.rs +++ b/pkgs/p2p_core/src/msg/headers2.rs @@ -6,7 +6,7 @@ //! Compressed header messages. -use super::headers::decode_count; +use super::headers::{decode_count, Headers}; use crate::codec::{codec_p2p, impl_p2p}; use crate::prelude::*; use crate::version::ProtocolVersion; @@ -280,6 +280,16 @@ hash_impl!(Headers2); impl Headers2 { /// The most headers a `headers2` message carries. pub const MAX_COUNT: usize = 8_000; + + /// The most headers a peer may send in one `headers2` message, given the + /// protocol version both peers share. + pub fn max_count(common: ProtocolVersion) -> usize { + if common >= ProtocolVersion::INCREASE_MAX_HEADERS2 { + Self::MAX_COUNT + } else { + Headers::MAX_COUNT + } + } } #[cfg(all(test, feature = "serde"))] @@ -345,4 +355,11 @@ mod tests { }) ); } + + #[rstest] + #[case::below(70234, 2_000)] + #[case::at(70235, 8_000)] + fn max_count_follows_the_common_version(#[case] common: i32, #[case] max: usize) { + assert_eq!(Headers2::max_count(ProtocolVersion(common)), max); + } } diff --git a/pkgs/p2p_core/src/version.rs b/pkgs/p2p_core/src/version.rs index 8b23bb0d3..57ec01a42 100644 --- a/pkgs/p2p_core/src/version.rs +++ b/pkgs/p2p_core/src/version.rs @@ -33,6 +33,8 @@ impl ProtocolVersion { pub const MNLISTDIFF_VERSION_ORDER: Self = Self(70229); /// Chainlock signatures in MN list diff boundary. pub const MNLISTDIFF_CHAINLOCKS: Self = Self(70230); + /// `headers2` limit raised to 8000 boundary. + pub const INCREASE_MAX_HEADERS2: Self = Self(70235); } #[cfg(test)] From 606d44dca22c6e4a6fe8c3166195f658a6eb9217 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 19 Oct 2026 12:19:20 -0500 Subject: [PATCH 11/17] p2p_core%refac: decode the V2 message type through `BaseCodec` `decode_v2` reads the short ID as a `u8` and the long-form type as a `CommandString` instead of calling `codec::take`. --- pkgs/p2p_core/src/bip324.rs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/pkgs/p2p_core/src/bip324.rs b/pkgs/p2p_core/src/bip324.rs index 2cc0fb616..5447dabe1 100644 --- a/pkgs/p2p_core/src/bip324.rs +++ b/pkgs/p2p_core/src/bip324.rs @@ -13,7 +13,7 @@ use crate::prelude::*; use crate::short_id::ShortId; use crate::P2pDecodeError; -use dash_types::codec::take; +use dash_types::codec::BaseCodec; /// Encodes a `P2pMsg` into V2 framed bytes. pub fn encode_v2(msg: &P2pMsg, buf: &mut Vec) { @@ -48,11 +48,10 @@ const MAX_V2_CONTENTS: usize = 1 + 12 + MAX_P2P_PAYLOAD_SIZE; /// decoding the message. pub fn decode_v2(contents: &[u8]) -> Result { let mut payload = contents; - let [short_id] = take(&mut payload).map_err(|_| P2pDecodeError::MissingCommand)?; + let short_id = u8::decode(&mut payload).map_err(|_| P2pDecodeError::MissingCommand)?; let id = ShortId(short_id); let cmd = if id == ShortId::LONG_FORMAT { - let bytes = take(&mut payload).map_err(|_| P2pDecodeError::MissingCommand)?; - Some(CommandString::from_bytes(bytes)) + Some(CommandString::decode(&mut payload).map_err(|_| P2pDecodeError::MissingCommand)?) } else if id.is_in_table() { Some(id.to_command().unwrap_or(CommandString::from_static(""))) } else { From 06c68036a3b2fe93680fb5cd2937e180d5b7c5c6 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 19 Oct 2026 12:20:18 -0500 Subject: [PATCH 12/17] p2p_core%fix!: check commands against the V2 rule only `p2p_core` only speaks V2, so `CommandString::is_valid_v1` and the V1 vectors go. `P2pMsg::decode_payload` refuses what `is_valid_v2` refuses. --- pkgs/p2p_core/corpus/transport.json5 | 67 --------------------------- pkgs/p2p_core/src/bip324.rs | 4 +- pkgs/p2p_core/src/command.rs | 15 +----- pkgs/p2p_core/src/error.rs | 6 +-- pkgs/p2p_core/src/macros.rs | 4 +- pkgs/p2p_core/src/msg/mod.rs | 47 ------------------- pkgs/p2p_core/tests/decode_payload.rs | 14 ++++++ 7 files changed, 22 insertions(+), 135 deletions(-) diff --git a/pkgs/p2p_core/corpus/transport.json5 b/pkgs/p2p_core/corpus/transport.json5 index c7ad28ed0..aea8868c6 100644 --- a/pkgs/p2p_core/corpus/transport.json5 +++ b/pkgs/p2p_core/corpus/transport.json5 @@ -13,73 +13,6 @@ { /* -BOOST_AUTO_TEST_CASE(transport_v1_size) -{ - using namespace std::literals; - for (const auto name : {"ping"sv, "verack"sv, "tx"sv, "nosuchcmd"sv, "pi\x1fng"sv}) { - for (const uint32_t size : {MAX_PROTOCOL_MESSAGE_LENGTH, MAX_PROTOCOL_MESSAGE_LENGTH + 1}) { - V1Transport transport{0, SER_NETWORK, INIT_PROTO_VERSION}; - CMessageHeader hdr{Params().MessageStart(), "", size}; - std::copy(name.begin(), name.end(), hdr.pchCommand); - CDataStream ss{SER_NETWORK, PROTOCOL_VERSION}; - ss << hdr; - Span bytes{UCharCast(ss.data()), ss.size()}; - const bool kept{transport.ReceivedBytes(bytes)}; - std::printf(" { \"command\": \"%s\", \"size\": %u, \"disconnect\": %s },\n", - HexStr(Span{hdr.pchCommand}).c_str(), size, kept ? "false" : "true"); - } - } -} -*/ - "v1_size": [ - { "command": "70696e670000000000000000", "size": 3145728, "disconnect": false }, - { "command": "70696e670000000000000000", "size": 3145729, "disconnect": true }, - { "command": "76657261636b000000000000", "size": 3145728, "disconnect": false }, - { "command": "76657261636b000000000000", "size": 3145729, "disconnect": true }, - { "command": "747800000000000000000000", "size": 3145728, "disconnect": false }, - { "command": "747800000000000000000000", "size": 3145729, "disconnect": true }, - { "command": "6e6f73756368636d64000000", "size": 3145728, "disconnect": false }, - { "command": "6e6f73756368636d64000000", "size": 3145729, "disconnect": true }, - { "command": "70691f6e6700000000000000", "size": 3145728, "disconnect": false }, - { "command": "70691f6e6700000000000000", "size": 3145729, "disconnect": true }, - ], -/* -BOOST_AUTO_TEST_CASE(transport_v1_command) -{ - using namespace std::literals; - for (const auto name : {"ping"sv, "abcdefghijkl"sv, "a b"sv, ""sv, " "sv, "~"sv, "pi\x7fng"sv, "pi\x1fng"sv, - "pi\x80ng"sv, "\xffping"sv, "pi\0ng"sv, "ping\0\0\0\0\0\0\0\x01"sv}) { - V1Transport transport{0, SER_NETWORK, INIT_PROTO_VERSION}; - CMessageHeader hdr{Params().MessageStart(), "", 0}; - std::copy(name.begin(), name.end(), hdr.pchCommand); - const uint256 hash{Hash(Span{})}; - std::copy(hash.begin(), hash.begin() + CMessageHeader::CHECKSUM_SIZE, hdr.pchChecksum); - CDataStream ss{SER_NETWORK, PROTOCOL_VERSION}; - ss << hdr; - Span bytes{UCharCast(ss.data()), ss.size()}; - BOOST_REQUIRE(transport.ReceivedBytes(bytes) && transport.ReceivedMessageComplete()); - bool reject{false}; - transport.GetReceivedMessage({}, reject); - std::printf(" { \"command\": \"%s\", \"valid\": %s },\n", HexStr(Span{hdr.pchCommand}).c_str(), - reject ? "false" : "true"); - } -} -*/ - "v1_command": [ - { "command": "70696e670000000000000000", "valid": true }, - { "command": "6162636465666768696a6b6c", "valid": true }, - { "command": "612062000000000000000000", "valid": true }, - { "command": "000000000000000000000000", "valid": true }, - { "command": "200000000000000000000000", "valid": true }, - { "command": "7e0000000000000000000000", "valid": true }, - { "command": "70697f6e6700000000000000", "valid": false }, - { "command": "70691f6e6700000000000000", "valid": false }, - { "command": "7069806e6700000000000000", "valid": false }, - { "command": "ff70696e6700000000000000", "valid": false }, - { "command": "7069006e6700000000000000", "valid": false }, - { "command": "70696e670000000000000001", "valid": false }, - ], -/* BOOST_AUTO_TEST_CASE(transport_v2) { using namespace std::literals; diff --git a/pkgs/p2p_core/src/bip324.rs b/pkgs/p2p_core/src/bip324.rs index 5447dabe1..daca0eeb8 100644 --- a/pkgs/p2p_core/src/bip324.rs +++ b/pkgs/p2p_core/src/bip324.rs @@ -37,8 +37,8 @@ const MAX_V2_CONTENTS: usize = 1 + 12 + MAX_P2P_PAYLOAD_SIZE; /// Decodes V2 message contents into a `P2pMsg`. /// /// The size bound applies to the whole contents and comes before any check -/// of the type, so a short-ID payload may be 12 bytes larger than a V1 -/// payload. +/// of the type. A short-ID payload may thus be 12 bytes larger than a +/// long-format payload. /// /// # Errors /// diff --git a/pkgs/p2p_core/src/command.rs b/pkgs/p2p_core/src/command.rs index 59843716a..d4c13cb6f 100644 --- a/pkgs/p2p_core/src/command.rs +++ b/pkgs/p2p_core/src/command.rs @@ -58,31 +58,18 @@ impl CommandString { core::str::from_utf8(&self.0[..end]).unwrap_or("") } - /// Whether a V1 transport accepts the command. - /// - /// Bytes before the first NUL must be in 0x20 to 0x7e, and every byte after - /// it NUL. - pub const fn is_valid_v1(&self) -> bool { - self.is_valid_up_to(0x7e) - } - /// Whether a V2 transport accepts the command as a long-form type. /// /// Bytes before the first NUL must be in 0x20 to 0x7f, and every byte after /// it NUL. pub const fn is_valid_v2(&self) -> bool { - self.is_valid_up_to(0x7f) - } - - /// Bytes in 0x20 to `max` up to the first NUL, then only NULs. - const fn is_valid_up_to(&self, max: u8) -> bool { let mut padding = false; let mut i = 0; while i < self.0.len() { let b = self.0[i]; if b == 0 { padding = true; - } else if padding || b < b' ' || b > max { + } else if padding || b < b' ' || b > 0x7f { return false; } i += 1; diff --git a/pkgs/p2p_core/src/error.rs b/pkgs/p2p_core/src/error.rs index e8ff56140..75f173fb7 100644 --- a/pkgs/p2p_core/src/error.rs +++ b/pkgs/p2p_core/src/error.rs @@ -28,8 +28,7 @@ pub enum P2pDecodeError { /// V2 contents end before the message type does. #[error("missing command")] MissingCommand, - /// A command its transport refuses, per [`CommandString::is_valid_v1`] or - /// [`CommandString::is_valid_v2`]. + /// A command that fails [`CommandString::is_valid_v2`]. #[error(fmt = fmt_invalid_command)] InvalidCommand { /// The command as received. @@ -46,7 +45,8 @@ pub enum P2pDecodeError { PayloadTooLarge { /// The message's command. command: CommandString, - /// Size of the payload, or of the whole contents for V2, in bytes. + /// Size of the payload, or of the whole contents for `decode_v2`, in + /// bytes. size: usize, /// Maximum allowed size, in bytes. max: usize, diff --git a/pkgs/p2p_core/src/macros.rs b/pkgs/p2p_core/src/macros.rs index d31731f7e..d7931f811 100644 --- a/pkgs/p2p_core/src/macros.rs +++ b/pkgs/p2p_core/src/macros.rs @@ -167,13 +167,13 @@ macro_rules! define_p2p { /// Decodes a message from its command string and raw payload. /// /// Every command, known or not, is bounded by the 3 MiB limit. A command - /// that fails [`CommandString::is_valid_v1`] gives `InvalidCommand`. + /// that fails [`CommandString::is_valid_v2`] gives `InvalidCommand`. pub fn decode_payload( cmd: &CommandString, payload: &[u8], ) -> Result { crate::macros::check_payload(cmd, payload)?; - if !cmd.is_valid_v1() { + if !cmd.is_valid_v2() { return Err(crate::P2pDecodeError::InvalidCommand { command: *cmd }); } Self::decode_unbounded(cmd, payload) diff --git a/pkgs/p2p_core/src/msg/mod.rs b/pkgs/p2p_core/src/msg/mod.rs index e8e5dcdf9..03108aeb5 100644 --- a/pkgs/p2p_core/src/msg/mod.rs +++ b/pkgs/p2p_core/src/msg/mod.rs @@ -193,50 +193,3 @@ define_p2p! { QWatch => QWATCH "qwatch" @ 151, } } - -#[cfg(all(test, feature = "serde"))] -#[expect(clippy::unwrap_used, reason = "test code")] -mod tests { - use super::*; - use crate::P2pDecodeError; - - use dash_dev::Corpus; - use hex_conservative::decode_to_array; - use rstest::rstest; - use serde::Deserialize; - - #[derive(Deserialize)] - struct SizeVector { - command: String, - size: usize, - disconnect: bool, - } - - #[rstest] - fn payload_size_matches_vectors() { - let corpus = Corpus::open(env!("CARGO_MANIFEST_DIR"), "transport"); - for v in corpus.vectors::("v1_size") { - let cmd = CommandString::from_bytes(decode_to_array(&v.command).unwrap()); - let result = P2pMsg::decode_payload(&cmd, &vec![0; v.size]); - let too_large = matches!(result, Err(P2pDecodeError::PayloadTooLarge { .. })); - assert_eq!(too_large, v.disconnect, "{} of {} bytes", v.command, v.size); - } - } - - #[derive(Deserialize)] - struct CommandVector { - command: String, - valid: bool, - } - - #[rstest] - fn invalid_commands_match_vectors() { - let corpus = Corpus::open(env!("CARGO_MANIFEST_DIR"), "transport"); - for v in corpus.vectors::("v1_command") { - let cmd = CommandString::from_bytes(decode_to_array(&v.command).unwrap()); - let result = P2pMsg::decode_payload(&cmd, &[]); - let invalid = matches!(result, Err(P2pDecodeError::InvalidCommand { .. })); - assert_eq!(invalid, !v.valid, "{}", v.command); - } - } -} diff --git a/pkgs/p2p_core/tests/decode_payload.rs b/pkgs/p2p_core/tests/decode_payload.rs index c4bf49e7b..fd2f1a378 100644 --- a/pkgs/p2p_core/tests/decode_payload.rs +++ b/pkgs/p2p_core/tests/decode_payload.rs @@ -20,6 +20,20 @@ fn decode_errors_are_typed(#[case] payload: &[u8], #[case] err: DecodeError) { ); } +#[rstest] +#[case::empty(*b"\0\0\0\0\0\0\0\0\0\0\0\0", true)] +#[case::del(*b"pi\x7fng\0\0\0\0\0\0\0", true)] +#[case::control(*b"pi\x1fng\0\0\0\0\0\0\0", false)] +#[case::high(*b"pi\x80ng\0\0\0\0\0\0\0", false)] +#[case::inner_nul(*b"pi\0ng\0\0\0\0\0\0\0", false)] +#[case::padding(*b"ping\0\0\0\0\0\0\0\x01", false)] +fn commands_follow_the_v2_rule(#[case] bytes: [u8; 12], #[case] valid: bool) { + let cmd = CommandString::from_bytes(bytes); + let invalid = Err(P2pDecodeError::InvalidCommand { command: cmd }); + assert_eq!(cmd.is_valid_v2(), valid); + assert_eq!(P2pMsg::decode_payload(&cmd, &[]) == invalid, !valid); +} + #[rstest] fn oversized_commands_are_escaped() { let cmd = CommandString::from_bytes(*b"pi\nng\0\0\0\0\0\0\0"); From 07a739e339c948a12ec9476893167ca6882dfbc5 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 19 Oct 2026 12:21:00 -0500 Subject: [PATCH 13/17] p2p_core%test: check the V2 transport vectors from `tests` The reference transport vectors run against the public `decode_v2` as a standalone test, so they no longer need the `serde` feature. --- pkgs/p2p_core/Cargo.toml | 1 + pkgs/p2p_core/src/bip324.rs | 43 ------------------------------ pkgs/p2p_core/tests/decode_v2.rs | 45 ++++++++++++++++++++++++++++++++ 3 files changed, 46 insertions(+), 43 deletions(-) create mode 100644 pkgs/p2p_core/tests/decode_v2.rs diff --git a/pkgs/p2p_core/Cargo.toml b/pkgs/p2p_core/Cargo.toml index a8abbe373..c4d231b5e 100644 --- a/pkgs/p2p_core/Cargo.toml +++ b/pkgs/p2p_core/Cargo.toml @@ -57,6 +57,7 @@ thiserror = { workspace = true } dash-dev = { version = "0.0.0", path = "../dev", features = ["full"] } hex-conservative = { workspace = true, features = ["std"] } rstest = { workspace = true } +serde = { workspace = true, features = ["derive", "std"] } [lints] workspace = true diff --git a/pkgs/p2p_core/src/bip324.rs b/pkgs/p2p_core/src/bip324.rs index daca0eeb8..cde0aea2a 100644 --- a/pkgs/p2p_core/src/bip324.rs +++ b/pkgs/p2p_core/src/bip324.rs @@ -71,46 +71,3 @@ pub fn decode_v2(contents: &[u8]) -> Result { } P2pMsg::decode_unbounded(&cmd, payload) } - -#[cfg(all(test, feature = "serde"))] -#[expect(clippy::unwrap_used, reason = "test code")] -mod tests { - use super::*; - - use dash_dev::Corpus; - use hex_conservative::decode_to_vec; - use rstest::rstest; - use serde::Deserialize; - - #[derive(Deserialize)] - struct ContentsVector { - prefix: String, - len: usize, - result: String, - } - - #[rstest] - fn decode_v2_matches_vectors() { - let corpus = Corpus::open(env!("CARGO_MANIFEST_DIR"), "transport"); - for v in corpus.vectors::("v2") { - let mut contents = decode_to_vec(&v.prefix).unwrap(); - contents.resize(v.len, 0); - let result = match decode_v2(&contents) { - Err(P2pDecodeError::PayloadTooLarge { .. }) => "disconnect", - Err( - P2pDecodeError::MissingCommand - | P2pDecodeError::InvalidCommand { .. } - | P2pDecodeError::UnknownShortId { .. }, - ) => "reject", - Ok(_) - | Err( - P2pDecodeError::UnknownCommand { .. } - | P2pDecodeError::Consensus(_) - | P2pDecodeError::PayloadNotEmpty { .. } - | P2pDecodeError::InvalidValue { .. }, - ) => "accept", - }; - assert_eq!(result, v.result, "{} of {} bytes", v.prefix, v.len); - } - } -} diff --git a/pkgs/p2p_core/tests/decode_v2.rs b/pkgs/p2p_core/tests/decode_v2.rs new file mode 100644 index 000000000..f78059c0e --- /dev/null +++ b/pkgs/p2p_core/tests/decode_v2.rs @@ -0,0 +1,45 @@ +// +// Copyright (c) 2026-present, The Dash Core developers +// SPDX-License-Identifier: MIT +// See the accompanying file LICENSE or https://opensource.org/license/MIT +// + +//! V2 transport outcomes of `decode_v2` against the reference vectors. + +#![expect(clippy::unwrap_used, reason = "test code")] + +use dash_dev::Corpus; +use dash_p2p_core::{decode_v2, P2pDecodeError}; +use hex_conservative::decode_to_vec; +use rstest::rstest; +use serde::Deserialize; + +#[derive(Deserialize)] +struct ContentsVector { + prefix: String, + len: usize, + result: String, +} + +#[rstest] +fn decode_v2_matches_vectors() { + let corpus = Corpus::open(env!("CARGO_MANIFEST_DIR"), "transport"); + for v in corpus.vectors::("v2") { + let mut contents = decode_to_vec(&v.prefix).unwrap(); + contents.resize(v.len, 0); + let result = match decode_v2(&contents) { + Err(P2pDecodeError::PayloadTooLarge { .. }) => "disconnect", + Err( + P2pDecodeError::MissingCommand | P2pDecodeError::InvalidCommand { .. } | P2pDecodeError::UnknownShortId { .. }, + ) => "reject", + Ok(_) + | Err( + P2pDecodeError::UnknownCommand { .. } + | P2pDecodeError::Consensus(_) + | P2pDecodeError::PayloadNotEmpty { .. } + | P2pDecodeError::InvalidValue { .. }, + ) => "accept", + }; + assert_eq!(result, v.result, "{} of {} bytes", v.prefix, v.len); + } +} From ebc5c06568419104f0cf3b687a05839e24e44d00 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 19 Oct 2026 12:22:14 -0500 Subject: [PATCH 14/17] p2p_core%feat: export `MAX_P2P_PAYLOAD_SIZE` and link limits in docs Comments and tests name the limit constants instead of their values, so they cannot go stale. Public docs link the newly exported constant. --- pkgs/p2p_core/src/bip324.rs | 2 +- pkgs/p2p_core/src/codec.rs | 4 ++-- pkgs/p2p_core/src/lib.rs | 1 + pkgs/p2p_core/src/macros.rs | 6 ++++-- pkgs/p2p_core/src/msg/headers2.rs | 5 +++-- pkgs/p2p_core/src/version.rs | 2 +- pkgs/p2p_core/tests/decode_payload.rs | 9 +++++---- 7 files changed, 17 insertions(+), 12 deletions(-) diff --git a/pkgs/p2p_core/src/bip324.rs b/pkgs/p2p_core/src/bip324.rs index cde0aea2a..553f86ddb 100644 --- a/pkgs/p2p_core/src/bip324.rs +++ b/pkgs/p2p_core/src/bip324.rs @@ -31,7 +31,7 @@ pub fn encode_v2(msg: &P2pMsg, buf: &mut Vec) { msg.encode_payload(buf); } -/// The type byte, a 12-byte command and the 3 MiB message limit. +/// The type byte, a 12-byte command and a [`MAX_P2P_PAYLOAD_SIZE`] payload. const MAX_V2_CONTENTS: usize = 1 + 12 + MAX_P2P_PAYLOAD_SIZE; /// Decodes V2 message contents into a `P2pMsg`. diff --git a/pkgs/p2p_core/src/codec.rs b/pkgs/p2p_core/src/codec.rs index b9624cc93..9ab0c56e9 100644 --- a/pkgs/p2p_core/src/codec.rs +++ b/pkgs/p2p_core/src/codec.rs @@ -6,8 +6,8 @@ //! Codec helpers for P2P message types. -/// Maximum buffered P2P message payload. -pub(crate) const MAX_P2P_PAYLOAD_SIZE: usize = 3_145_728; +/// Largest P2P message payload, in bytes. +pub const MAX_P2P_PAYLOAD_SIZE: usize = 3_145_728; /// Generates `Encode` + `Decode` with P2P payload size limit. macro_rules! impl_p2p { diff --git a/pkgs/p2p_core/src/lib.rs b/pkgs/p2p_core/src/lib.rs index a494f72c2..f911758c0 100644 --- a/pkgs/p2p_core/src/lib.rs +++ b/pkgs/p2p_core/src/lib.rs @@ -31,6 +31,7 @@ pub mod __private { pub use dash_types; } pub use bip324::{decode_v2, encode_v2}; +pub use codec::MAX_P2P_PAYLOAD_SIZE; pub use command::CommandString; pub use error::P2pDecodeError; pub use magic::Magic; diff --git a/pkgs/p2p_core/src/macros.rs b/pkgs/p2p_core/src/macros.rs index d7931f811..955a2262b 100644 --- a/pkgs/p2p_core/src/macros.rs +++ b/pkgs/p2p_core/src/macros.rs @@ -166,8 +166,10 @@ macro_rules! define_p2p { /// Decodes a message from its command string and raw payload. /// - /// Every command, known or not, is bounded by the 3 MiB limit. A command - /// that fails [`CommandString::is_valid_v2`] gives `InvalidCommand`. + /// Every command, known or not, is bounded by the + /// [`MAX_P2P_PAYLOAD_SIZE`](crate::MAX_P2P_PAYLOAD_SIZE) limit. A + /// command that fails [`CommandString::is_valid_v2`] gives + /// `InvalidCommand`. pub fn decode_payload( cmd: &CommandString, payload: &[u8], diff --git a/pkgs/p2p_core/src/msg/headers2.rs b/pkgs/p2p_core/src/msg/headers2.rs index 0a50107ac..da114b03f 100644 --- a/pkgs/p2p_core/src/msg/headers2.rs +++ b/pkgs/p2p_core/src/msg/headers2.rs @@ -315,8 +315,9 @@ mod tests { } #[rstest] - fn counts_past_2000_decode() { - let headers = (0..2_001) + fn counts_past_the_headers_limit_decode() { + let headers = (0..) + .take(Headers::MAX_COUNT + 1) .map(|i| BlockHeader { version: 0x2000_0000, prev_hash: BlockHash::from_lendian([0; 32]), diff --git a/pkgs/p2p_core/src/version.rs b/pkgs/p2p_core/src/version.rs index 57ec01a42..e23467c27 100644 --- a/pkgs/p2p_core/src/version.rs +++ b/pkgs/p2p_core/src/version.rs @@ -33,7 +33,7 @@ impl ProtocolVersion { pub const MNLISTDIFF_VERSION_ORDER: Self = Self(70229); /// Chainlock signatures in MN list diff boundary. pub const MNLISTDIFF_CHAINLOCKS: Self = Self(70230); - /// `headers2` limit raised to 8000 boundary. + /// Raised [`Headers2::MAX_COUNT`](crate::Headers2::MAX_COUNT) boundary. pub const INCREASE_MAX_HEADERS2: Self = Self(70235); } diff --git a/pkgs/p2p_core/tests/decode_payload.rs b/pkgs/p2p_core/tests/decode_payload.rs index fd2f1a378..58e5cf28e 100644 --- a/pkgs/p2p_core/tests/decode_payload.rs +++ b/pkgs/p2p_core/tests/decode_payload.rs @@ -6,7 +6,7 @@ //! Checks applied by `P2pMsg::decode_payload`. -use dash_p2p_core::{CommandString, P2pDecodeError, P2pMsg}; +use dash_p2p_core::{CommandString, P2pDecodeError, P2pMsg, MAX_P2P_PAYLOAD_SIZE}; use dash_types::codec::DecodeError; use rstest::rstest; @@ -37,10 +37,11 @@ fn commands_follow_the_v2_rule(#[case] bytes: [u8; 12], #[case] valid: bool) { #[rstest] fn oversized_commands_are_escaped() { let cmd = CommandString::from_bytes(*b"pi\nng\0\0\0\0\0\0\0"); + let size = MAX_P2P_PAYLOAD_SIZE + 1; assert_eq!( - P2pMsg::decode_payload(&cmd, &vec![0; 3 * 1024 * 1024 + 1]).map_err(|e| e.to_string()), - Err(String::from( - "payload too large: 3145729 bytes, max 3145728, command pi\\x0ang" + P2pMsg::decode_payload(&cmd, &vec![0; size]).map_err(|e| e.to_string()), + Err(format!( + "payload too large: {size} bytes, max {MAX_P2P_PAYLOAD_SIZE}, command pi\\x0ang" )) ); } From 46e470499f0cbcd58b4b27cba5fafd40d8bb88f5 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 19 Oct 2026 12:22:43 -0500 Subject: [PATCH 15/17] p2p_core%refac: suffix size constants with `_SIZE` `MIN_HEADERS_SIZE`, `MIN_HEADERS2_SIZE` and `MAX_V2_CONTENTS_SIZE` are sizes in bytes, while `MAX_COUNT` and `max_count` stay counts. --- pkgs/p2p_core/src/bip324.rs | 6 +++--- pkgs/p2p_core/src/msg/headers.rs | 23 ++++++++++++----------- pkgs/p2p_core/src/msg/headers2.rs | 14 +++++++------- 3 files changed, 22 insertions(+), 21 deletions(-) diff --git a/pkgs/p2p_core/src/bip324.rs b/pkgs/p2p_core/src/bip324.rs index 553f86ddb..e802f1d93 100644 --- a/pkgs/p2p_core/src/bip324.rs +++ b/pkgs/p2p_core/src/bip324.rs @@ -32,7 +32,7 @@ pub fn encode_v2(msg: &P2pMsg, buf: &mut Vec) { } /// The type byte, a 12-byte command and a [`MAX_P2P_PAYLOAD_SIZE`] payload. -const MAX_V2_CONTENTS: usize = 1 + 12 + MAX_P2P_PAYLOAD_SIZE; +const MAX_V2_CONTENTS_SIZE: usize = 1 + 12 + MAX_P2P_PAYLOAD_SIZE; /// Decodes V2 message contents into a `P2pMsg`. /// @@ -58,11 +58,11 @@ pub fn decode_v2(contents: &[u8]) -> Result { None }; - if contents.len() > MAX_V2_CONTENTS { + if contents.len() > MAX_V2_CONTENTS_SIZE { return Err(P2pDecodeError::PayloadTooLarge { command: cmd.unwrap_or(CommandString::from_static("")), size: contents.len(), - max: MAX_V2_CONTENTS, + max: MAX_V2_CONTENTS_SIZE, }); } let cmd = cmd.ok_or(P2pDecodeError::UnknownShortId { id: short_id })?; diff --git a/pkgs/p2p_core/src/msg/headers.rs b/pkgs/p2p_core/src/msg/headers.rs index 22a56fd40..0aab1b4ed 100644 --- a/pkgs/p2p_core/src/msg/headers.rs +++ b/pkgs/p2p_core/src/msg/headers.rs @@ -15,16 +15,17 @@ use dash_types::codec::{self, BaseCodec, DecodeError, EncodeBuf}; use dash_types::type_id::TypeId; use dash_types::CompactSize; -/// Smallest `headers` entry, an 80-byte header and a one-byte tx count. -const MIN_HEADERS_ENTRY: usize = 81; +/// Size of the smallest `headers` entry, an 80-byte header and a one-byte +/// tx count. +const MIN_HEADERS_SIZE: usize = 81; -/// Reads an entry count of at most `limit`. +/// Reads an entry count of at most `max_count`. /// -/// A count the remaining input cannot hold at `min_entry` bytes each is +/// A count the remaining input cannot hold at `min_size` bytes each is /// [`DecodeError::Eof`], reported before anything is allocated for it. -pub(super) fn decode_count(data: &mut &[u8], limit: usize, min_entry: usize) -> Result { - let count = CompactSize::decode(data)?.into_len(limit)?; - codec::ensure(data, count * min_entry)?; +pub(super) fn decode_count(data: &mut &[u8], max_count: usize, min_size: usize) -> Result { + let count = CompactSize::decode(data)?.into_len(max_count)?; + codec::ensure(data, count * min_size)?; Ok(count) } @@ -61,7 +62,7 @@ impl_p2p!(Headers); impl BaseCodec for Headers { fn decode(data: &mut &[u8]) -> Result { - let count = decode_count(data, Self::MAX_COUNT, MIN_HEADERS_ENTRY)?; + let count = decode_count(data, Self::MAX_COUNT, MIN_HEADERS_SIZE)?; let mut headers = Vec::with_capacity(count); for _ in 0..count { headers.push(BlockHeader { @@ -107,12 +108,12 @@ mod tests { #[rstest] fn counts_past_the_input_fail_before_any_header() { - let payload = [&[2][..], &[0; MIN_HEADERS_ENTRY]].concat(); + let payload = [&[2][..], &[0; MIN_HEADERS_SIZE]].concat(); assert_eq!( Headers::decode(&mut &payload[..]), Err(DecodeError::Eof { - needed: 2 * MIN_HEADERS_ENTRY, - remaining: MIN_HEADERS_ENTRY, + needed: 2 * MIN_HEADERS_SIZE, + remaining: MIN_HEADERS_SIZE, }) ); } diff --git a/pkgs/p2p_core/src/msg/headers2.rs b/pkgs/p2p_core/src/msg/headers2.rs index da114b03f..a97d7a418 100644 --- a/pkgs/p2p_core/src/msg/headers2.rs +++ b/pkgs/p2p_core/src/msg/headers2.rs @@ -16,9 +16,9 @@ use dash_types::codec::{BaseCodec, DecodeError, EncodeBuf}; use dash_types::type_id::{TypeId, Unencodable}; use dash_types::{CompactSize, Hashable}; -/// Smallest compressed header, made of flags, a merkle root, a time delta -/// and a nonce. -const MIN_HEADERS2_ENTRY: usize = 39; +/// Size of the smallest compressed header, made of flags, a merkle root, a +/// time delta and a nonce. +const MIN_HEADERS2_SIZE: usize = 39; /// Maximum entries in the MRU version cache. const MAX_VERSION_CACHE: usize = 7; @@ -257,7 +257,7 @@ impl_p2p!(Headers2); impl BaseCodec for Headers2 { fn decode(data: &mut &[u8]) -> Result { - let count = decode_count(data, Self::MAX_COUNT, MIN_HEADERS2_ENTRY)?; + let count = decode_count(data, Self::MAX_COUNT, MIN_HEADERS2_SIZE)?; let mut state = CompressionState::new(); let mut headers = Vec::with_capacity(count); for _ in 0..count { @@ -347,12 +347,12 @@ mod tests { #[rstest] fn counts_past_the_input_fail_before_any_header() { - let payload = [&[2][..], &[0; MIN_HEADERS2_ENTRY]].concat(); + let payload = [&[2][..], &[0; MIN_HEADERS2_SIZE]].concat(); assert_eq!( Headers2::decode(&mut &payload[..]), Err(DecodeError::Eof { - needed: 2 * MIN_HEADERS2_ENTRY, - remaining: MIN_HEADERS2_ENTRY, + needed: 2 * MIN_HEADERS2_SIZE, + remaining: MIN_HEADERS2_SIZE, }) ); } From 8b43b25a4a4001b7e48b10e3be7754aa106fb617 Mon Sep 17 00:00:00 2001 From: pasta Date: Mon, 19 Oct 2026 12:23:43 -0500 Subject: [PATCH 16/17] p2p_core%refac: derive the V2 short ID table counts from the messages `define_p2p` lists the short IDs the tables hold without a message, so `LOW_TABLE_COUNT` and `HIGH_TABLE_COUNT` follow each new assignment. --- pkgs/p2p_core/src/macros.rs | 13 ++++++++++++ pkgs/p2p_core/src/msg/mod.rs | 7 ++++++ pkgs/p2p_core/src/short_id.rs | 40 ++++++++++++++++++++++++++++++++--- 3 files changed, 57 insertions(+), 3 deletions(-) diff --git a/pkgs/p2p_core/src/macros.rs b/pkgs/p2p_core/src/macros.rs index 955a2262b..c02dfe012 100644 --- a/pkgs/p2p_core/src/macros.rs +++ b/pkgs/p2p_core/src/macros.rs @@ -81,6 +81,10 @@ macro_rules! define_p2p { $se_variant:ident => $se_cmd:ident $se_wire:literal $(@ $se_sid:literal)? ),* $(,)? } + // Short IDs the V2 tables hold without a message. + unassigned { + $( $u_sid:literal ),* $(,)? + } ) => { impl CommandString { $( @@ -102,6 +106,15 @@ macro_rules! define_p2p { } impl ShortId { + /// Every short ID the V2 tables hold, assigned or not. + pub(crate) const TABLE: &'static [u8] = &[ + $( $( $p_sid, )? )* + $( $( $pe_sid, )? )* + $( $( $s_sid, )? )* + $( $( $se_sid, )? )* + $( $u_sid, )* + ]; + /// Resolves the short ID to its command name, if one is assigned. pub const fn to_command_str(self) -> Option<&'static str> { match self.0 { diff --git a/pkgs/p2p_core/src/msg/mod.rs b/pkgs/p2p_core/src/msg/mod.rs index 03108aeb5..2e7a7a449 100644 --- a/pkgs/p2p_core/src/msg/mod.rs +++ b/pkgs/p2p_core/src/msg/mod.rs @@ -192,4 +192,11 @@ define_p2p! { /// LLMQ: watch quorums. QWatch => QWATCH "qwatch" @ 151, } + + unassigned { + // `feefilter`, which Dash does not implement. + 5, + // Assigned in BIP324, not implemented. + 29, 30, 31, 32, + } } diff --git a/pkgs/p2p_core/src/short_id.rs b/pkgs/p2p_core/src/short_id.rs index f6f99fd93..691154bbb 100644 --- a/pkgs/p2p_core/src/short_id.rs +++ b/pkgs/p2p_core/src/short_id.rs @@ -11,10 +11,24 @@ use crate::command::CommandString; use dash_types::type_id::Unencodable; /// Number of short IDs from 0 that a V2 transport takes. -const LOW_TABLE_LEN: u8 = 33; +const LOW_TABLE_COUNT: u8 = table_count(0); /// Number of short IDs from 128 that a V2 transport takes. -const HIGH_TABLE_LEN: u8 = 41; +const HIGH_TABLE_COUNT: u8 = table_count(128); + +/// Number of short IDs from `start` up to the last one in its table. +const fn table_count(start: u8) -> u8 { + let mut count = 0; + let mut i = 0; + while i < ShortId::TABLE.len() { + let id = ShortId::TABLE[i]; + if id >= start && id - start < 128 && id - start >= count { + count = id - start + 1; + } + i += 1; + } + count +} /// A resolved V2 short ID. #[derive(Clone, Copy, Debug, Eq, Hash, PartialEq, Unencodable)] @@ -32,7 +46,7 @@ impl ShortId { /// Whether a V2 transport takes the ID as a message type, assigned or not. pub const fn is_in_table(self) -> bool { - self.0 < LOW_TABLE_LEN || (self.0 >= 128 && self.0 - 128 < HIGH_TABLE_LEN) + self.0 < LOW_TABLE_COUNT || (self.0 >= 128 && self.0 - 128 < HIGH_TABLE_COUNT) } /// Resolves the short ID to a `CommandString`, if known. @@ -40,3 +54,23 @@ impl ShortId { self.to_command_str().map(CommandString::from_static) } } + +#[cfg(test)] +mod tests { + use super::*; + + use rstest::rstest; + + /// The reference tables have no gaps, so every short ID up to the last one + /// in a table is assigned or listed as unassigned, and only once. + #[rstest] + fn tables_are_contiguous() { + let held = |id: u8| ShortId::TABLE.iter().filter(|&&t| t == id).count() == 1; + assert!((1..LOW_TABLE_COUNT).all(held)); + assert!((128..128 + HIGH_TABLE_COUNT).all(held)); + assert_eq!( + ShortId::TABLE.len(), + usize::from(LOW_TABLE_COUNT - 1 + HIGH_TABLE_COUNT) + ); + } +} From ab11ad1f6e33a8ee5ec458a5e9475d229a2f509a Mon Sep 17 00:00:00 2001 From: pasta Date: Tue, 20 Oct 2026 18:03:19 +0000 Subject: [PATCH 17/17] p2p_core%test: decode a full batch of minimally compressed headers Pin the 39-byte compressed header floor: a maximal batch of entries that all drop the previous hash, nbits and version must decode in full. --- pkgs/p2p_core/src/msg/headers2.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/pkgs/p2p_core/src/msg/headers2.rs b/pkgs/p2p_core/src/msg/headers2.rs index a97d7a418..08b8317cb 100644 --- a/pkgs/p2p_core/src/msg/headers2.rs +++ b/pkgs/p2p_core/src/msg/headers2.rs @@ -333,6 +333,32 @@ mod tests { assert_eq!(Headers2::decode(&mut &buf[..]), Ok(msg)); } + #[rstest] + fn a_full_batch_of_minimal_entries_decodes() { + let mut prev_hash = BlockHash::from_lendian([0; 32]); + let headers = (0..Headers2::MAX_COUNT as u32) + .map(|i| { + let header = BlockHeader { + version: 0x2000_0000, + prev_hash, + merkle_root: MerkleRoot::from_lendian([1; 32]), + time: 1_700_000_000 + i, + bits: 0x1e0f_ffff, + nonce: i, + }; + prev_hash = header.hash(); + header + }) + .collect(); + let msg = Headers2 { headers }; + let mut buf = Vec::new(); + msg.encode(&mut buf); + assert_eq!(buf.len(), 3 + 81 + 7_999 * 39); + let mut data = &buf[..]; + assert_eq!(Headers2::decode(&mut data), Ok(msg)); + assert!(data.is_empty()); + } + #[rstest] fn counts_past_the_limit_fail_before_any_header() { let payload = [0xfd, 0x41, 0x1f];