[PastaPastaPasta]

Stacked on #122; retargets to develop when it merges.

Motivation

dash-p2p-core declares clsig, isdlock, mnauth, spork and platformban in P2pMsg without typed payloads, so P2pMsg::decode_payload hands back a stub and a caller has to parse the bytes itself. This pull request types each of the five payloads, decoding the fields in the order and widths the reference implementation reads them at v24.0.0. It also bounds the two lists the reference implementation bounds in these messages. The decoder checks each count before it reads any entry.

Additional Information

Description	Notes
ClSig { height, block_hash, sig }	i32, 32-byte block hash, 96-byte BlsSigBytes (src/chainlock/chainlock.h:23-45, read at :43, at v24.0.0). Generated with codec_p2p!, so the hash is the double SHA-256 of the whole payload.
IsdLock { version, inputs, txid, cycle_hash, sig }	u8, a CompactSize list of outpoints, two 32-byte hashes, 96-byte BlsSigBytes (src/instantsend/lock.h:21-52, read at :41-48). Hand-written BaseCodec for the input bound, with hash_impl! for the default hash.
MnAuth { pro_reg_tx_hash, sig }	32-byte hash, 96-byte BlsSigBytes (src/evo/mnauth.h:46-67, read at :54). Generated with codec_p2p!.
Spork { spork_id, value, time_signed, sig }	i32, i64, i64, CompactSize byte vector (src/spork.h:93-141, read at :113-117, widths at :34 and :57). The hash is the double SHA-256 of the whole payload (src/spork.cpp:264-267), as hash_impl! gives.
PlatformBan { pro_tx_hash, requested_height, quorum_hash, sig }	32-byte hash, i32, 32-byte quorum hash, 96-byte BlsSigBytes (src/masternode/meta.h:166-185, read at :176-182). Its Hashable is the double SHA-256 of the first three fields, since the reference implementation leaves the signature out of the hashed bytes (src/masternode/meta.h:179, src/masternode/meta.cpp:266, src/hash.h:229).
Bound isdlock at 48 780 inputs	IsdLock::MAX_INPUTS, one input per 41 bytes of a 2 000 000-byte block (src/instantsend/lock.h:31). The decoder reads the list with decode_vec(data, IsdLock::MAX_INPUTS).
Bound the spork signature at 65 bytes	Spork::MAX_SIG_LEN, 65: dash-pkc's ECDSA_REC_SIG_LEN, equal to the reference implementation's compact signature size (src/pubkey.h:41). The reference implementation reads the signature as a length-limited vector (src/spork.h:116, src/serialize.h:1026-1032) and writes one of any length (src/serialize.h:1020-1024). The decoder reads it with decode_vec(data, Spork::MAX_SIG_LEN); the encoder writes any length.
No other bounds	clsig, mnauth and platformban carry no lists. The BLS signatures are fixed 96-byte fields read as bytes.
Corpora	corpus/chainlock.json5 and corpus/masternode.json5. Each section embeds the net_tests program that printed it at v24.0.0 (938fba5028). The 48 780-input lock would take about 8 MB as a raw vector, so its section records the reference implementation's payload size and hash for a lock built by a stated recipe, which the test checks against the library's own encoding and Hashable.
bitcoin_hashes dependency	dash-p2p-core now depends on bitcoin_hashes for the PlatformBan hash, as dash-primitives and dash-pkc do. MODULE.bazel.lock is updated to match.

Breaking Changes

P2pMsg::Spork, ClSig, IsdLock, MnAuth and PlatformBan move from stubs to variants carrying their typed payloads. P2pMsg::decode_payload now refuses an isdlock with more than 48 780 inputs and a spork whose signature is longer than 65 bytes.

How Has This Been Tested?

• corpus_clsig, corpus_isdlock, corpus_mnauth, corpus_spork and corpus_platformban decode each corpus vector, compare it with its details, encode it back to the same bytes, dispatch it through P2pMsg::decode_payload and check the serde round trip.
• corpus_spork_hash and corpus_platformban_hash check Hashable against the reference implementation's hash. The platformban vectors include pairs that differ only in the signature and share a hash.
• corpus_isdlock_input_limit builds the 48 780 and 48 781-input locks, matches each one's size and hash against the reference implementation, and checks that the first decodes and the second fails with CompactSizeExceedsLimit. corpus_spork_sig_len decodes the reference implementation's spork payloads with 0, 65 and 66-byte signatures: the library reads the first two and refuses the third with CompactSizeExceedsLimit, as the reference implementation reads and refuses them.
• input_counts_are_bounded and sig_lens_are_bounded check each bound: at the limit it decodes, one over fails with CompactSizeExceedsLimit, and that count with no entry bytes after it gives the same error.
• Every corpus section was regenerated from its embedded program in a v24.0.0 build and matches the committed file byte for byte.
• Each of ten planted mutants fails a test: two field-order swaps (spork, clsig), two wrong widths (clsig height, isdlock version), a platformban hash over the signature, a platformban hash missing the height, both bounds off by one, and both bounds removed.
• contrib/git_filter.py --fast-fail origin/develop HEAD -- <script>, where <script> runs cargo clippy -p dash-p2p-core --all-features --all-targets -- -D warnings, cargo nextest run -p dash-p2p-core --all-features and the same nextest run with default features, passes on each commit.
• cargo nextest run --workspace --all-features, RUSTDOCFLAGS='-D warnings' cargo doc -p dash-p2p-core --all-features --no-deps and cargo fmt --check pass on the tip.
• The rust, markdown, python, bazel, nix, just, symlinks, javascript and clang lints, lint_bazel.py graph, lint_commit.py over the range, semgrep and cargo-deny pass on the tip. lint_codeql.py check and lint_codeql.py run for rust, c and cpp pass on both commits; the one local c finding is in contrib/probes/hello.c, which this pull request does not touch.

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, corpus programs, 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]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

• 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

[thepastaclaw]

✅ Final review complete — no blockers (commit 110f378) · triage: critical

[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 validation — Phase 1 + Phase 2

Static verification of ff19ef9 confirms the new payload layouts, bounds, hashing and dispatch follow the inspected Core definitions and existing SDK patterns. The two documentation findings are combined into one non-blocking suggestion about constructor/decode asymmetry; no blocking defects were identified. Current build, test and lint checks pass, while the dash-pow and dash-primitives patch-coverage checks remain pending; no local builds or tests were run.

🟡 1 suggestion(s)

Approved: nothing here blocks the merge. The non-blocking findings are optional: fix them forward, in this PR or a follow-up, without waiting for another review round.

Review provenance

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

• Triage: critical by gpt-6.1-sol (effort low) — The diff changes peer-facing network deserialization in P2pMsg::decode_payload, including bounded variable-length lists and cryptographic signature/hash fields in chainlock.rs and masternode.rs.
• 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)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — general (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6-astra — security-auditor (completed, effort xhigh); agent phase2-reviewer
• Model comparison: every Phase-2 reviewer also ran on gpt-6-astra; the verifier weighed both sets without knowing which model wrote which

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review (PastaClaw). How to work through them:
- Verify each finding against the code; fix only what is real, as narrowly as the finding.
- If a finding is wrong, decline it on its thread in one or two sentences; the bot answers once and stops.
- To defer one, link a tracking issue on its thread (or reply asking the bot to file one).
- Suggestions are optional: fix them forward in a later PR rather than pushing another round just for them.
- Address every current finding in one push: self-review the full diff first, and don't push piecemeal.
- Keep the PR's scope; don't grow it to satisfy hardening suggestions.
- Carried items were raised in an earlier round: read the thread first, and don't re-fix what you already fixed, declined or deferred there. Disputed and deferred items are already handled and are not listed.

In `pkgs/p2p_core/src/msg/chainlock.rs`:
- [SUGGESTION] pkgs/p2p_core/src/msg/chainlock.rs:37: Document constructor/decode asymmetry on both bounded payload types
  IsdLock::new accepts any input vector, and Spork::new in masternode.rs accepts any signature vector. Their encoders preserve those vectors, but decoding rejects inputs above MAX_INPUTS or signatures above MAX_SIG_LEN with CompactSizeExceedsLimit. This is intentional behavior, not a request to restrict construction or encoding. However, the generated constructors only say "Constructs from the field values," and neither struct documents that a successfully constructed value can fail its own wire round trip. Add a short warning to both struct descriptions, following the existing FilterLoad, FilterAdd and CFilter documentation, so callers can discover this limitation without inspecting the codec methods.

[thepastaclaw]

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

Verified the complete change at 110f378 against the source and pinned Dash Core serializers; no actionable in-scope defects remain in the payload layouts, bounds, hashing, or dispatch. This was a static review; no builds or tests were run locally. Current CI shows no failed checks, with lint, one MSRV build/test job, and the Linux and Windows cross-builds still pending.

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

Review provenance

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

• Triage: critical by gpt-6.1-sol (effort low) — The diff changes peer-facing network deserialization in P2pMsg::decode_payload for ChainLock, InstantSend, masternode-authentication, spork, and platform-ban messages, including cryptographic signatures, hashes, and attacker-controlled list bounds.
• 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)
• Single stage: Phase 1 and Phase 2 reviewed this head side by side, with no blocker gate between them (triage tier)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — rust-quality (completed, effort xhigh); agent phase2-reviewer, gpt-6.1-sol — security-auditor (completed, effort xhigh); 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.

[PastaPastaPasta]

/self-reviewed 110f378