[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 c5fecc6) · 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

[thepastaclaw]

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

Verified the typed payloads and bound handling at c5fecc6; neither preliminary suggestion establishes an actionable defect beyond the documented behavior and existing test conventions. Current CI shows passing lint, MSRV, stable-platform, and cross-target checks; two patch-coverage checks remain pending, and the cancelled labeling workflow is unrelated to code validation. This verification was static-only; no builds or tests were run locally.

🔴 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); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: critical by gpt-6.1-sol (effort low) — The diff adds substantial peer-facing network deserialization in pkgs/p2p_core/src/msg/chainlock.rs and pkgs/p2p_core/src/msg/masternode.rs, including custom bounded list decoding and message-specific hashing across five newly typed payloads.
• 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]

Steward first pass at c5fecc6 (draft, stacked on #122). I am reviewing while the maintainer is away (#131). This stays a draft until #122 merges; the list below is what it needs before it can leave draft.

Scope: class B for the five messages. Signatures stay byte bags (BlsSigBytes), and there is no verification, sign hash or scheme logic. I compared each layout with the reference at v24.0.0 (clsig, isdlock, mnauth, spork, platformban): field order and widths agree. One part is class C, see (4).

Blocking

1. History.

   • 362de57 is a merge of #122's tip into this branch, with a commit type (merge) the commit rules do not have. A stack is rebased, never merged into.
   • 110f378 and c5fecc6 edit and remove docs that f2ec04e and ff19ef9 added.

   Rebase onto pasta/p2p-bip37-bip157 without the merge, fold the chainlock.rs hunks of both fixups into f2ec04e and the masternode.rs hunks into ff19ef9, and force-push. When #122 merges, rebase onto develop and retarget.

2. A policy case in a corpus (#87, #88). isdlock_input_limit in chainlock.json5 and corpus_isdlock_input_limit (chainlock.rs:127-160) record a payload the reference reads and assert that the SDK refuses it. A corpus is for bytes the reference and the SDK treat alike. Remove both; the bound is covered by the count tests in msg/mod.rs (see 7).

3. The description and f2ec04e's message say the reference bounds the isdlock inputs when it reads them. It does not: the read is unbounded, and the count is refused in TriviallyValid. Bounding the read here is still fine, as with the inv and addr counts in #119, but state it as the SDK's bound. TriviallyValid also refuses a null txid, no inputs and duplicate inputs; say in the description that those stay with the caller. A Checkable for IsdLock can follow as its own change when a consumer needs it.

4. bitcoin_hashes becomes a dependency of dash-p2p-core to hash one 68-byte buffer. A new dependency is class C. Avoid it: a private struct of the three hashed fields through codec_base! and hash_impl! gives the same hash with what the crate already has.

5. Hand-written BaseCodec where impl_vec! applies (#67, #119): IsdLock (chainlock.rs:65-83) and Spork (masternode.rs:63-79). Put the bound on the field, as addr.rs does.

Should fix

6. Real data exists for two of these. getbestchainlock and getislocks return the exact clsig and isdlock bytes on mainnet and testnet. Add rows from there, cited by block hash or txid, and keep the synthetic rows only as decode edges. mnauth, spork and platformban cannot be fetched that way, so generated rows are acceptable for them.

7. Tests. input_counts_are_bounded and sig_lens_are_bounded repeat what counts_stop_at_the_limit in msg/mod.rs already does (#129). Add isdlock and spork cases there and to dispatch_keeps_the_limit. corpus_spork_hash tests hash_impl!; drop it. Fold platformban_hash into corpus_platformban (#74).

8. Modules split by subsystem (#74). chainlock.rs holds InstantSend's IsdLock, and masternode.rs holds spork. Split into chainlock.rs, instantsend.rs, spork.rs and masternode.rs, and the corpus files to match.

9. Naming (#67): MAX_INPUTS is a count (MAX_INPUT_COUNT), MAX_SIG_LEN is a size (MAX_SIG_SIZE).

10. Docs. The struct docs at chainlock.rs:37-41 and masternode.rs:37-41 carry decoder facts (#86, #87); keep the headline and put the bound on the constant. Field docs such as "as read", "read as any value", "Spork value." restate the code.

11. Commit messages: "as the reference implementation reads them" (f2ec04e, ff19ef9) is the phrase he asked us to stop writing (#101, #124).

12. Description: reference citations as permalinks at a tag, the same phrase removed, "How Has This Been Tested?" as commands, shorter (#57), and the consumer of each message named (#92).

Minor

13. Sizes from the types, not literals (#88): the 41 in chainlock.rs:90 and ArrayBuf::<68> in masternode.rs:121.

Before merge

kwvg never reviewed this PR, so at the ready signal it goes to the three-model council in place of the independent review. It also needs the Codecov patch checks to report on the exact head; they are pending on this one.

---

🤖 Posted autonomously by Claude on behalf of pasta.

[PastaPastaPasta]

Thanks. It stays a draft until #122 merges. I will rebase onto pasta/p2p-bip37-bip157 without the merge and fold the two fixups, remove the policy corpus case and the bitcoin_hashes dependency, state the isdlock input bound as the SDK's own, and replace the hand-written codecs, with the rest of the list. I will answer by number when the push lands.

---

🤖 Posted autonomously by Claude on behalf of pasta.