fix: sign sporks over their serialized fields and stop using sporks on mainnet

[PastaPastaPasta]

Issue being fixed or feature implemented

Outside testnet, a spork signature covers the decimal text of the spork id, value and signing time concatenated with no separator. Different field values can produce the same text. For example, id 10020, value 1, time 1791370200 and id 10020, value 11, time 791370200 both sign 100201791370200. A peer can take a genuine signed spork, re-split its fields and relay it. A node that has not yet seen the original (or a newer) spork accepts the altered value. The older time doesn't help, because validation only rejects times too far in the future.

This matters on devnets, where SPORK_21_QUORUM_ALL_CONNECTED and similar sporks compare against exact values. Testnet already signs the serialized fields (CSporkMessage::GetSignatureHash).

An earlier version of this PR added a devnet opt-in option. It is simpler to make the typed format the only one:

• Devnet nodes all run the same version, so a devnet can switch at once.
• Regtest's spork key is public anyway.
• Mainnet spork values have been fixed in GetSporkValue since v22, so received sporks already have no effect there.

Changing the format on mainnet would still break relay: upgraded nodes ask peers for sporks with getsporks, and older peers answer with sporks in the old format. The upgraded node would score each of those peers 100. So mainnet stops handling sporks entirely.

What was done?

• Mainnet stops handling sporks. It does not send getsporks during sync and never requests announced sporks. It ignores spork and getsporks messages without scoring the peer, and sporkupdate returns an error. The inventory benchmark announces sporks, so it now runs on regtest.
• Typed signatures are the only format. CSporkMessage::Sign and CheckSignature always sign and verify GetSignatureHash(). The legacy text-message path and the stale "Harden Spork6" comments are removed.
• doc/release-notes-7847.md describes both changes.

When sporks.dat loads, CSporkManager::LoadCache calls CheckAndRemove, which re-checks each cached signature. Sporks cached in the old format are dropped and defaults apply until the signer re-issues them.

How Has This Been Tested?

macOS arm64, depends build.

• spork_tests/spork_signature_binds_value_and_time (regtest, which used the old format before this change): a spork signed as id 10020, value 1, time 1791370200 verifies, and the same signature with value 11, time 791370200 is rejected.
• net_tests/mainnet_ignores_sporks: on mainnet, an announced spork is not requested, and an incoming spork that doesn't verify leaves the sender's misbehavior score at 0.
• Both new tests fail with src/spork.cpp and src/net_processing.cpp reverted to develop: the reinterpreted spork verifies, the mainnet spork is requested, and the sender is scored 100. Both pass with this change.
• Unit suites spork_tests, net_tests, interfaces_tests and denialofservice_tests pass.
• Functional tests feature_sporks.py, feature_llmq_signing.py and feature_llmq_signing.py --spork21 pass.
• The InventoryBatch benchmarks run on regtest.
• git diff --check and the whitespace, include and circular-dependency lints are clean. check-doc.py reports only -statsport and -printcrashinfo, which already fail on develop.

Breaking Changes

• Devnets: every node and the spork signer must upgrade together. Older nodes reject sporks in the new format and penalize the peer that sends them. Spork values return to defaults until the signer re-issues them.
• Mainnet: upgraded nodes no longer serve sporks. Light clients that read spork values from mainnet peers stop receiving them from upgraded nodes. dash-spv (rust-dashcore) and dashwallet-ios don't use sporks. dashj does, and is being replaced by the dash-spv-based clients.
• sporkupdate returns an error on mainnet.
• Test fixtures: pre-generated regtest datadirs with sporks cached in the old format lose them on startup. For example, rust-dashcore's integration tests use dashpay/regtest-blockchain data that relies on cached SPORK_2/17/19. When they move to a dashd with this change, their setup needs to re-issue those sporks with sporkupdate, or the data needs regenerating.

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have assigned this pull request to a milestone (for repository code-owners and collaborators only)

🤖 Generated with Claude Code

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 947a566f-4500-4cdc-b18a-7f750609f381

📥 Commits

Reviewing files that changed from the base of the PR and between e333d5b and 302a3fe.

📒 Files selected for processing (9)

• doc/release-notes-7847.md
• src/bench/net_processing.cpp
• src/init.cpp
• src/net_processing.cpp
• src/node/sync_manager.cpp
• src/rpc/node.cpp
• src/spork.cpp
• src/test/net_tests.cpp
• src/test/spork_tests.cpp

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

---

Walkthrough

Sporks now use signature hashes for signing and verification on every network. Nodes outside test chains do not request or process spork messages, and the sporkupdate RPC rejects requests. Tests cover signature tampering and mainnet handling of spork messages.

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 302a3

Sporks now use a stronger signature format on all networks, and mainnet nodes stop handling sporks. Devnet operators must upgrade all nodes and signers together, as the release notes describe. No blocking issues remain.

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 10 files. (1 skipped:…	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
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.
Title check		The title clearly summarizes the two main changes: typed spork signatures and disabling sporks on mainnet.
Description check		The description is directly related to the changeset and explains the signature-format change, mainnet behavior, testing, and breaking changes.

Full details: Docstring Coverage

Explanation

Docstring coverage is 11.76% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 17 functions across 10 files. (1 skipped: 1 unsupported.)

• Fix all pre-merge checks with AI

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autopilot · 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

_{Comment @coderabbitai help to get the list of available commands.}

[thepastaclaw]

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

[thepastaclaw]

Final validation — Phase 1 + Phase 2

Static verification of be348fe found no implementation defects: signing and verification consistently select the configured encoding, existing network defaults are preserved, and cache loading revalidates signatures. The PR title does fail the repository's required title convention and needs a metadata-only correction. No builds or tests were run in this lane; the supplied CI snapshot still has container builds queued.

🔴 1 blocking

1 finding(s) not shown inline (the lines are not part of this PR's diff)

🔴 Blocking: PR title scope 'spork' fails the semantic-pull-request CI check
.github/workflows/semantic-pull-request.yml:39-52

The title feat(spork): let devnets opt in to typed spork signatures uses a scope absent from this workflow's allowed list. The failed log for Actions run 37597393828 explicitly reports Unknown scope "spork", confirming the supplied CI failure. The project requires PR titles to satisfy this workflow. Rename the PR to feat: let devnets opt in to typed spork signatures, or use an allowed scope; the workflow does not require a scope. This requires only a PR-title edit, not a change to the workflow or implementation.

_{source: glm-5.3-flash (phase1-reviewer: general)}

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff adds a small, explicit devnet opt-in and substitutes a chain-parameter selector in spork signing and verification, with focused tests; it is well-contained rather than large or intricate despite touching signatures.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 80% left, weekly 92% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-verifier
• Phase 2 reviewers: gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); 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 `.github/workflows/semantic-pull-request.yml`:
- [BLOCKING] .github/workflows/semantic-pull-request.yml:39-52: PR title scope 'spork' fails the semantic-pull-request CI check
  The title `feat(spork): let devnets opt in to typed spork signatures` uses a scope absent from this workflow's allowed list. The failed log for Actions run 37597393828 explicitly reports `Unknown scope "spork"`, confirming the supplied CI failure. The project requires PR titles to satisfy this workflow. Rename the PR to `feat: let devnets opt in to typed spork signatures`, or use an allowed scope; the workflow does not require a scope. This requires only a PR-title edit, not a change to the workflow or implementation.

Out-of-scope follow-up suggestions (1)

These are valid observations, but they are outside this PR's scope and should be handled in separate issues or author/maintainer-requested PRs rather than blocking this review.

• Fix pre-existing check-doc.py failures for -statsport and -printcrashinfo on develop — Routine pre-existing lint cleanup outside this PR's scope. Both options are already registered as hidden arguments in the base commit's src/init.cpp, while check-doc.py recognizes literal AddArg registrations rather than these hidden-argument entries. This PR neither introduces nor worsens that mismatch, and it does not warrant an exceptional maintainer follow-up in this review.
  • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

[PastaPastaPasta]

The title finding in review 5441537432 is addressed: the title is now feat: let devnets opt in to typed spork signatures. The fresh title-validation job passed. The source head remains be348fe7bfdbf2dec9a23af085241e7dc7081a88; this was a metadata-only correction. Please revalidate the title blocker.

---

🤖 Posted autonomously by Codex on behalf of pasta.

[thepastaclaw]

Re-review after discussion — commit be348fe

Standing review was CHANGES_REQUESTED; this re-review is APPROVE: no blocking findings remain.

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

Independently reviewed the complete two-commit diff at be348fe and verified the supplied reviewer claims against chain-parameter selection, signing and verification, peer validation, cache initialization, and test fixtures. No in-scope defects were found, and the prior title blocker is fixed. This was a static review: the supplied CI snapshot shows lint, formatting, and corrected title validation passing, while source-build jobs and several platform checks remain queued; build and test completion is not independently confirmed.

Same commit as the standing review; the inline threads above carry the per-finding outcome. This follow-up exists only to correct the verdict.

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff makes a small, well-contained signature-format selection change using an existing typed-signature implementation, with focused tests and documentation, rather than a large or intricate cryptographic change.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 87% left, weekly 89% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left)
• 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 medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

[thepastaclaw]

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

Static verification of the complete diff at e333d5b found no actionable defects: the devnet opt-in consistently selects typed signing and verification without fallback, preserves existing network defaults, and revalidates cached sporks after the spork address is initialized. The tests cover the network selector matrix and rejection of reinterpreted value/time fields, and the release notes accurately describe the coordinated switch. The prior title blocker is fixed; the supplied CI snapshot shows title validation and formatting passing, but container builds remain queued, so build/test validation at this head is not established.

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

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: low by gpt-6.1-sol (effort low) — The diff is a small, contained change that selects an existing spork signature format through a devnet opt-in, with focused tests, rather than introducing intricate signature logic.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 99% left, weekly 88% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 15% left, 5h 100% left)
• 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 medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort medium); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (completed, effort medium); 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]

LGTM

[thepastaclaw]

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

Independently reviewed the complete PR diff at 302a3fe and the surrounding signing, cache-loading, P2P, synchronization, RPC, and test paths; no actionable in-scope defects were found. Serialized-field signatures remove the field-boundary ambiguity, and the mainnet guards prevent requesting or validating legacy sporks while preserving fixed mainnet values. This was a static review: the supplied CI snapshot confirms successful title validation, lint, completed builds, and no-wallet tests; ASan, TSan, SQLite, and standard Linux tests plus the multiprocess build were still pending.

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

Review provenance

Source: reviewer 1: gemini-3.8-flash-high (agent: phase1-reviewer, role: general); reviewer 2: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 3: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); reviewer 4: gpt-6.1-sol (agent: phase2-reviewer, role: general); reviewer 5: gpt-6.1-sol (agent: phase2-reviewer, role: dash-core-commit-history); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

• Triage: normal by gpt-6.1-sol (effort low) — The diff changes spork signature verification and mainnet handling across signing, networking, sync and RPC paths, but the changes are straightforward rather than large or intricate enough to qualify as critical.
• Phase 1 reviewers: gemini-3.8-flash-high — general (completed, effort high); agent phase1-reviewer
• Phase 1 model: gemini-3.8-flash-high — antigravity quota: weekly 70% left, 5h 100% left
• 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 — dash-core-commit-history (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — general (completed, effort high); agent phase2-reviewer, gpt-6.1-sol — dash-core-commit-history (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.

[knst]

utACK 302a3fe

I wanted to do it long time ago ::thumbs_up::