p2p: remove explicit compact size limit
What the developer wrote
Authored by Nick Johnson
p2p: remove explicit compact size limit
These limit overrides are unnecessary because both types,
`PrefilledTransaction.idx` and `Offset`, are u16's under the hood. The
decoding will fail on too large of values when the type conversion
is attempted in `::end()`.
What changed, and why it matters
This commit removes an explicit 4-million-item size limit when decoding two compact-size numbers in Bitcoin compact block (BIP-152) messages. The author argues the limit is unnecessary because the decoded values are ultimately stored as 16-bit unsigned integers (u16), so any value larger than 65,535 will already be rejected later in the decoding process. The change is a code simplification rather than a clear security fix, but it slightly shifts where and how oversized inputs are rejected.
Verify that CompactSizeDecoder::new() without a limit still rejects values above the u16 range (0..=65535) cleanly via the decoder's ::end() or equivalent conversion, and that error handling surfaces the failure rather than truncating or panicking. Consider adding a regression test for values 65536..=4000000 to confirm the intended behavior. No urgent patch is indicated unless verification shows the downstream check is missing or unsafe.
Security signals we found
Removal of an explicit anti-DoS decoding limit
Reliance on downstream type conversion (u16) to enforce the effective bound
Change in defense-in-depth: single enforcement point instead of layered limits
Evidence from the diff
In p2p/src/bip152.rs, the patch deletes the MAX_VEC_SIZE constant and changes CompactSizeDecoder::new_with_limit(4_000_000) to CompactSizeDecoder::new() for PrefilledTransaction.idx and Offset. The commit message states the 4,000,000 limit is redundant because both fields are u16 internally, and decoding will fail on overflow when the value is converted in ::end(). This is a behavior-preserving refactor if the downstream conversion is indeed strict and correctly reports errors. If the downstream conversion were permissive or silently truncated, removing the limit could allow larger values to be accepted, but the commit message asserts that does not happen.
Changed components
p2p/src/bip152.rsPrefilledTransaction decodingOffset decodingCompactSizeDecoder usageInspect captured patch +2 / −10
diff --git a/p2p/src/bip152.rs b/p2p/src/bip152.rs
index b4805233..52c6ccb5 100644
--- a/p2p/src/bip152.rs
+++ b/p2p/src/bip152.rs
@@ -25,12 +25,6 @@ use io::{BufRead, Write};
use primitives::block::{BlockHashDecoder, BlockHashEncoder, Header, HeaderDecoder, HeaderEncoder};
use primitives::transaction::{TransactionDecoder, TransactionEncoder};
-/// Maximum number of elements in a vector.
-///
-/// This is an anti-DoS limit which won't possibly reject any block,
-/// or part of a block, on the network.
-const MAX_VEC_SIZE: usize = 4_000_000;
-
/// A BIP-0152 error
#[derive(Debug, Clone, PartialEq, Eq)]
#[non_exhaustive]
@@ -144,7 +138,7 @@ impl encoding::Decodable for PrefilledTransaction {
fn decoder() -> Self::Decoder {
PrefilledTransactionDecoder(Decoder2::new(
- CompactSizeDecoder::new_with_limit(MAX_VEC_SIZE),
+ CompactSizeDecoder::new(),
TransactionDecoder::new(),
))
}
@@ -636,9 +630,7 @@ impl encoding::Decoder for OffsetDecoder {
impl encoding::Decodable for Offset {
type Decoder = OffsetDecoder;
- fn decoder() -> Self::Decoder {
- OffsetDecoder(CompactSizeDecoder::new_with_limit(MAX_VEC_SIZE))
- }
+ fn decoder() -> Self::Decoder { OffsetDecoder(CompactSizeDecoder::new()) }
}
/// A [`BlockTransactionsRequest`] structure is used to list transaction indexes
Why this scored 29/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.