Skip to content

feat: add Platform Tenderdash chain id to CChainParams - #7764

Open
PastaPastaPasta wants to merge 1 commit into
dashpay:developfrom
PastaPastaPasta:feat/chainparams-platform-chain-id
Open

PastaPastaPasta wants to merge 1 commit into
dashpay:developfrom
PastaPastaPasta:feat/chainparams-platform-chain-id

Conversation

@PastaPastaPasta

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

A Dash Platform client verifies that signed response metadata comes from the Platform chain it expects. The chain id is part of what the quorum signs. The DashPay GUI in dash-qt (tracking issue #7512) needs each network's Platform chain id. Core already carries the other Platform network parameters (nDefaultPlatformP2PPort, nDefaultPlatformHTTPPort and the bech32 HRP), so this PR puts the chain id next to them.

Earlier, the value was proposed for dash-sdk's own configuration (dashpay/platform#4963). The maintainer closed that PR, so the Platform shell keeps the check, and Core supplies the value from CChainParams. This answers knst's question about where the chain id should live, asked on the earlier composite branch.

What was done?

  • CChainParams::PlatformChainId() returns "evo1" on mainnet and "dash-testnet-51" on testnet. Both match the genesis chain_id in dashmate's mainnet and testnet config defaults. Devnet and regtest have no canonical Platform chain, so their id is empty.
  • The chain id is a network parameter, not a consensus rule, so it stays out of Consensus::Params.
  • No command-line options are added. The GUI adds a GUI-only -platformchainid override for testnet and devnets, which is refused on mainnet.

How Has This Been Tested?

chainparams_platform_tests checks the value for every network. The test ran on this branch alone on top of develop (0f876369f9c7), on aarch64-apple-darwin.

The DashPay GUI verified every live testnet response against dash-testnet-51 from this parameter. -platformchainid=bogus produced an explicit chain-id mismatch (see #7512).

Breaking Changes

None.

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

Expose the Tenderdash chain id of each network's Dash Platform through
CChainParams::PlatformChainId(), beside the existing Platform ports and
bech32 HRP: "evo1" on mainnet and "dash-testnet-51" on testnet,
matching the genesis chain_id in dashmate's mainnet and testnet config
defaults. Devnet and regtest have no canonical Platform chain and carry
an empty id.

This is a network parameter a Platform client verifies signed response
metadata against; it is not a consensus rule, so it stays out of
Consensus::Params. No new command-line options are added.

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Potential PR merge conflicts

This is advisory only. It does not block CI, but it marks PRs that will likely need a rebase depending on merge order.

If this PR merges first

These open PRs will likely need a rebase:

If these PRs merge first

This PR will likely need a rebase:

@thepastaclaw

thepastaclaw commented Sep 28, 2026 •

Copy link
Copy Markdown
Collaborator

✅ Final review complete — no blockers (commit bf7ad04) · triage: low

@coderabbitai

coderabbitai Bot commented Sep 28, 2026

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

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: 5a9f5b80-f827-4a54-a828-139d6fe0f8ef

📥 Commits

Reviewing files that changed from the base of the PR and between 3ba0805 and bf7ad04.

📒 Files selected for processing (5)
  • src/Makefile.test.include
  • src/chainparams.cpp
  • src/chainparams.h
  • src/test/chainparams_platform_tests.cpp
  • test/util/data/non-backported.txt

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


Walkthrough

CChainParams now exposes a PlatformChainId() accessor. Mainnet uses evo1, testnet uses dash-testnet-51, and devnet and regtest use an empty string. A test checks these values, and the test source is added to the test build.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~8 minutes

Merge Risk: ⚪ Minimal · up to bf7ad

This adds per-network Platform ID access. No concrete behavior mismatch or merge-blocking issue was established in the reviewed code.

Security Architecture Review

Security architecture risk: 🔵 Low · up to bf7ad

The new values are fixed per network and tested. This change does not add a response verifier or expose a new network entrypoint, but the downstream verification behavior is outside the reviewed change.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

  • inferred — The visible change exposes network-selected constants to callers, without adding an attacker-controlled endpoint or a response-processing path.

Trust Boundaries and Controls

  • inferred — Whether downstream verification rejects mismatched or empty expected chain IDs cannot be determined from the added accessor and value-comparison test.

Hardening Proposals

  • proposed — At the downstream verifier boundary, require an explicit expected ID or an intentional network-specific override, and reject mismatches rather than treating an empty ID as a disabled check.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (2 skipped: 2… Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and concisely describes the main change: adding the Platform Tenderdash chain ID to CChainParams.
Description check ✅ Passed The description is directly related to the changes. It explains the network values, design decisions, testing, and intended GUI use.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 7 functions across 3 files. (2 skipped: 2 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR

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

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

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Final validation — Phase 1 + Phase 2

Verified the complete single-commit change at bf7ad04 and found no actionable defects: all four network values match the PR description, the accessor is additive, and the new test is registered in both required lists. The unit-test target built successfully; chainparams_platform_tests, pow_tests, llmq_params_tests, and evo_netinfo_tests passed, git diff --check passed, and the worktree remains clean. External Platform genesis values and the reported mutation check were not independently verified; the claimed chainparams_tests suite was not present in the executed binary.

Review provenance

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

  • Triage: low by gpt-6-astra (effort low) — The diff adds a small, self-contained network parameter and accessor with straightforward per-network tests and build registration, without changing consensus, signature verification, or other critical logic.
  • Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — dash-core-commit-history (completed, effort high); agent phase1-reviewer
  • Phase 1 model: glm-5.3-flash — zai quota: 5h 89% left, weekly 94% left; passed over gemini-3.8-flash-high (antigravity below 15% reserve: weekly 13% left, 5h 100% left)
  • Fresh verifier: gpt-6-astra — final-verifier; agent astra-verifier
  • Phase 2 reviewers: gpt-6-astra — general (completed, effort medium); agent phase2-reviewer, gpt-6-astra — dash-core-commit-history (completed, effort medium); agent phase2-reviewer

@thepastaclaw thepastaclaw added the pastaclaw:approved thepastaclaw's latest review approved this PR label Sep 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pastaclaw:approved thepastaclaw's latest review approved this PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants