[PastaPastaPasta]

Motivation

dash-p2p-core decodes the lists in inv, getdata, notfound, getblocks, getheaders, getheaders2, addr and addrv2 up to MAX_SER_SIZE entries, far more than the reference implementation's limit for any of them. A caller has to check each limit after decoding, by which point the decoder has already read and allocated the whole list. This pull request enforces each limit in the message's own decoder, on the count, before any entry is read.

Additional Information

Description	Notes
Bound inv, getdata and notfound at 50 000 entries	Inv::MAX_COUNT, the reference implementation's limit (src/net.h:80 at v24.0.0-rc.3). GetData::MAX_COUNT and NotFound::MAX_COUNT take the same value.
Bound the locator of getblocks, getheaders and getheaders2 at 101 hashes	GetHeaders::MAX_LOCATOR_COUNT, the reference implementation's limit (src/net_processing.cpp:150 at v24.0.0-rc.3). GetBlocks::MAX_LOCATOR_COUNT and GetHeaders2::MAX_LOCATOR_COUNT take the same value.
Bound addr and addrv2 at 1 000 addresses	Addr::MAX_COUNT, the reference implementation's limit (src/net_processing.cpp:213 at v24.0.0-rc.3). AddrV2Msg::MAX_COUNT takes the same value.
Add impl_vec! to dash-types	An exported macro under the codec feature, beside impl_type! and impl_bytes!. It generates BaseCodec over the named fields of a struct, in the order listed. A field written field: limit is read with codec::decode_vec(data, limit), which checks the CompactSize count with CompactSize::decode_len and fails with CompactSizeExceedsLimit before it reads or allocates an entry. Every other field reads with its own BaseCodec, and every field encodes with its own BaseCodec. It generates only the codec: the caller adds Encode and Decode under its own size cap, and Hashable.
Read the eight messages with impl_vec!	Each message keeps its limit as an associated constant and uses impl_vec! with impl_p2p! and hash_impl!, the pattern headers, pong and version already follow. For example, impl_vec!(GetHeaders, { version, locator_hashes: Self::MAX_LOCATOR_COUNT, hash_stop }). Every field encodes as codec_p2p! wrote it, so a message within the limit encodes and decodes to the same bytes as before. No codec macro is added to dash-p2p-core.
Build codec_base! on impl_vec!	The field codec of dash-primitives' codec_base! is now impl_vec! with no limited field, followed by impl_type! as before. It generates the same code, so every type built with codec_base!, codec_type!, codec_payload! or codec_p2p! decodes and encodes as before.
Keep headers and headers2 as they are	They already bound their count (Headers::MAX_COUNT, Headers2::MAX_COUNT) through decode_count, before any entry is read. Their entries are not decoded by the field type's own codec: a headers entry carries a transaction count after the header that is read and dropped, and a headers2 entry is restored from the entries before it. decode_count also fails with Eof on a count the input cannot hold at the smallest entry size. impl_vec! would change how their entries are read and which error a short input gives, so they stay on decode_count.

A field can take field: limit when it is a plain struct field, read through its own codec, that the reference implementation bounds at decode. No such field in dash-primitives is bounded today, so none takes a field: limit in this pull request. The ones checked:

• The masternode share count (read in provider/proregtx.rs) is a one-byte count, and its 8-share limit (provider/share.rs) is a validity rule checked after decode, with its own error.
• The network info entry count (types/netinfo.rs) is limited to 4 per purpose by a validity rule checked after decode, with its own error. Its purpose and entry counts are read against MAX_SER_SIZE only.
• Payout and signature lists are read with one-byte counts, which cannot exceed 255.
• The governance vote signature (gov/vote.rs), the compact provider signature (provider/sig.rs), the addrv2 address (types/addrv2.rs) and the network info domain (types/netinfo.rs) are already bounded at decode. Each is read by a hand-written codec in which the length picks a variant or sits inside one, so they stay as they are.
• The special transaction payload is a byte string that the reference implementation reads up to its size cap only.
• The governance object signature is read up to MAX_SER_SIZE bytes, while the reference implementation refuses one longer than 96 bytes before reading it (src/governance/common.h:37 at v24.0.0). It is the one field found that fits field: limit, but bounding it is a new refusal in dash-primitives, so it is left for a follow-up.

Outside these crates, the only other count checked at decode is the recipient count of BlsIesMultiBytes in dash-pkc, whose decoder grows the list as entries arrive rather than reserving it up front, which decode_vec would change. It stays as it is.

Breaking Changes

inv, getdata, notfound, getblocks, getheaders, getheaders2, addr and addrv2 refuse lists longer than their limit, which the crate decoded before.

How Has This Been Tested?

• impl_vec! has its own tests in dash-types, on a struct with a limited list between two other fields. A list at the limit decodes. One more entry fails with CompactSizeExceedsLimit, and so does that count with no entry bytes after it. A list field with no limit decodes as its own BaseCodec does, past the other field's limit. The struct encodes as its fields do, one after another.
• counts_stop_at_the_limit covers each of the eight messages. A list at the limit decodes and encodes back to the same bytes. One more entry fails with CompactSizeExceedsLimit. That count with no entry bytes after it gives the same error rather than Eof, so the count is checked before any entry is read.
• dispatch_keeps_the_limit checks the same error through P2pMsg::decode_payload for each of the eight commands.
• The existing corpus and wire vectors for inv, getblocks, getheaders2, addr and addrv2 still decode and encode to the same bytes, and the headers and headers2 limit tests pass unchanged.
• Each of six planted mutants fails the tests: impl_vec! reading its limited fields with no limit, impl_vec! encoding the fields in reverse order, the inv limit one too high, the locator limit one too low, and the dash-types test limit one too high and one too low.
• contrib/git_filter.py --fast-fail <base> HEAD -- bash -c '<cmds>', where <cmds> is cargo clippy -p dash-types -p dash-primitives -p dash-p2p-core --all-features --all-targets -- -D warnings, cargo nextest run for the same crates with --all-features, and the same clippy and nextest runs with default features, passes on each commit.
• cargo nextest run --workspace --all-features and RUSTDOCFLAGS='-D warnings' cargo doc -p dash-types -p dash-primitives -p dash-p2p-core --all-features --no-deps pass on the tip.
• lint_all.py (without CodeQL), lint_bazel.py graph, lint_commit.py over the range, semgrep and cargo-deny pass on the tip. CodeQL was not run locally.
• A trial merge with the current develop passes the same clippy and nextest runs for the three crates.

Checklist

• I have performed a self-review of my own code
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional tests
• I have made corresponding changes to the documentation
• I did not use agentic assistance (i.e. LLMs, AI) except as disclosed and can answer questions,
  comments and concerns without said assistance. I assume authorship of the code submitted.
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

Agent disclosure

Code, commit messages and this description were generated with Claude Code using Claude Opus 5.5, running autonomously as an agent on behalf of the author. The change was reviewed by a Claude Code agent using Opus 5.5.

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review.

Summary

Summary by CodeRabbit

• Bug Fixes
  • Peer-to-peer messages now enforce maximum sizes for address lists, block and header locators, and inventory lists. Messages exceeding these limits are rejected, while messages within the limits continue to be processed.
• Tests
  • Added coverage for messages at their maximum allowed size and for rejection of oversized messages.

Selected address, inventory, and block locator messages now use explicit BaseCodec implementations. Their decoders enforce configured vector count limits, and their encoders serialize the corresponding fields. Tests cover round trips at the limits, rejection above the limits, and dispatch-level CompactSizeExceedsLimit errors.

Sequence Diagram(s)

sequenceDiagram
  participant Payload as P2P payload
  participant Dispatch as Payload dispatch
  participant Codec as BaseCodec
  Payload->>Dispatch: Supply encoded message
  Dispatch->>Codec: Decode message with vector count limit
  Codec-->>Dispatch: Return decoded message or CompactSizeExceedsLimit
  Dispatch-->>Payload: Return message or decode error

Loading

Suggested reviewers: thepastaclaw

Merge Risk: Low · up to 85ef0

Oversized outbound messages would be rejected on decoding, but no current send path is known to produce them. This does not appear to block merging; document the outbound limit or address it in the public API.

Pre-merge checks: 4

Check name	Status	Explanation
Title check		The title clearly and concisely describes the main change: adding bounds for inventory, locator, and address counts in p2p_core.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Description check		The description clearly explains the decoder count limits, implementation changes, compatibility goals, and tests for the messages modified in the pull request.

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

Recent review info

Run configuration

• Source mode: Auto
• Repository: dashpay/base-sdk
• Branch: pasta/p2p-v1-framing
• File: .coderabbit.yml
• Commit: 85ef0c9
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: fe57f403-e25e-4f51-a1a5-da3c42d90350

Commits

Reviewing files that changed from the base of the PR and between 06d7b19 and 85ef0c9.

Files selected for processing: 6

• pkgs/p2p_core/src/msg/addr.rs
• pkgs/p2p_core/src/msg/blocks.rs
• pkgs/p2p_core/src/msg/headers.rs
• pkgs/p2p_core/src/msg/headers2.rs
• pkgs/p2p_core/src/msg/inv.rs
• pkgs/p2p_core/src/msg/mod.rs

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

[github-actions]

Warning

This pull request may have conflicts, please coordinate with the authors of these pull requests.

Potential conflicts

• p2p_core%feat!: type the BIP37 and BIP157 messages #122

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ All tests successful. No failed tests found.

📢 Thoughts on this report? Let us know!

🚀 New features to boost your workflow:

• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[thepastaclaw]

✅ Final review complete — no blockers (commit b9859b6) · triage: normal

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Verified the supplied findings against head 06d7b19 and compared the framing checks and constants with the pinned Dash Core source; no actionable in-scope issues remain. The payload-length concern violates an explicit caller precondition, while the diagnostic coverage observation does not identify incorrect behavior. This was a static-only review: current CI reports successful build, test, and cross-compilation checks, with Lint still pending and no failed code checks reported.

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — Although V1Header::verify_header and verify_payload introduce peer-facing framing validation, the implementation is compact and straightforward rather than large or intricate enough to meet the critical tier.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Independently reviewed the complete diff at 85ef0c9 and found no actionable defects. All eight decoders enforce the Dash Core count limits before allocating or decoding entries, preserve existing encoding and trait behavior, and have boundary and dispatch coverage. Validation was static only; current CI reports passing lint, relevant MSRV build/tests, stable-platform checks and cross-builds, with no pending or failing checks.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The diff changes peer-facing deserialization but consists of straightforward, repeated bounded-vector decoding across eight messages, so it is not sufficiently large or intricate to qualify as critical.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Reviewed the complete diff at a740455: all eight count limits match Dash Core, and the decoders enforce them before entry allocation or decoding while preserving valid wire encodings. No functional or security blocker was found; two non-blocking suggestions concern shared codec ownership and source citations in documentation. Validation was static only; current CI reports successful applicable MSRV and completed stable-platform checks, no failed checks, and pending lint and additional cross-platform jobs.

🟡 2 suggestion(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The diff changes peer-facing deserialization in pkgs/p2p_core/src/codec.rs and eight message decoders, but the shared bounded-vector macro and repetitive limit substitutions are contained rather than large or intricate.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `pkgs/p2p_core/src/codec.rs`:
- [SUGGESTION] pkgs/p2p_core/src/codec.rs:28-49: Separate shared bounded-field codec generation from P2P wiring
  The new macro repeats the field-by-field BaseCodec generation in dash_primitives::codec_base!, adding optional decode_vec calls, then combines that generic mechanism with the P2P payload limit and Hashable implementation. Keeping the bounded-field generator private to dash-p2p-core prevents lower-level crates such as dash-primitives from reusing it without reversing the dependency direction or adding another implementation. Extract the transport-independent field codec generator into dash-types, alongside BaseCodec and decode_vec, and retain payload-limit selection and hash_impl! composition in a thin P2P wrapper. The current decoder behavior is correct; this is a non-blocking ownership and duplication concern introduced by the new macro.

In `pkgs/p2p_core/src/msg/headers.rs`:
- [SUGGESTION] pkgs/p2p_core/src/msg/headers.rs:53-54: Keep version-specific Core citations in the PR description
  The new constant documentation embeds a Core source path and release version here, with equivalent citations on Inv::MAX_COUNT and Addr::MAX_COUNT. These references become stale independently of the constants and conflict with the project's stated convention of keeping reference-implementation citations in PR descriptions rather than code comments. Keep the descriptions of what each constant limits, remove the source/version parentheticals from all three comments, and retain the Core references in the PR description.

[thepastaclaw]

Re-review — Final validation — Phase 1 + Phase 2

Verified the complete diff at b9859b6: all eight message decoders enforce Dash Core's count limits before allocating or decoding entries, and the shared codec extraction preserves field order and encoding behavior. Both prior findings are fixed, with no remaining actionable findings. Validation was static only; current CI shows successful Linux cross-build and Windows stable checks, no failed checks, and pending MSRV builds/tests, lint, documentation, and other platform jobs.

🔴 0 blocking | 🟡 0 suggestion(s) | 💬 0 nitpick(s)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); reviewer 2: muse-spark-1.3-contributor (agent: phase1-reviewer, role: rust-quality); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); reviewer 6: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 7: gpt-6.1-sol (agent: phase2-reviewer, role: rust-quality); reviewer 8: gpt-6.1-sol (agent: phase2-reviewer, role: security-auditor); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — Although the diff changes peer-facing network deserialization, the count checks and small shared codec macro are straightforward rather than large or intricate enough to meet the critical tier.
• Phase 1 reviewers: muse-spark-1.3-contributor — general (completed, effort xhigh); agent phase1-reviewer, muse-spark-1.3-contributor — rust-quality (completed, effort xhigh); agent phase1-reviewer
• Phase 1 model: muse-spark-1.3-contributor — not quota-gated; passed over gemini-3.8-flash-high (antigravity quota lookup failed), glm-5.3-flash (not used above high effort; tier asks max)
• Fresh final gate: an independent Phase-2 review ran after iterative findings were reconciled
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort high); agent phase2-reviewer

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify the current code and confirm that no unresolved issues remain.

No unresolved findings remain from the prior review on this head.

[kwvg]

utACK b9859b6