[shumkov]

Issue being fixed or feature implemented

The book had one line each for "verify chain lock" and "update core info" in the block pipeline. It did not explain how drive-abci reads Dash Core deterministically, or why every validator's Core answers about the same block. It also did not cover the Core fork activation record that #5681 added at protocol version 15. Contributors adding a Core read, or a consumer of a Core fork, had no rules to follow.

What was done?

architecture/component-pipeline.md: a new section, "Syncing with Dash Core", which expands steps 4 and 5 of run_block_proposal:

• A short primer on evonodes, quorums and chain locks, and the rule this rests on: consensus code reads Core at the block's chain-locked core height c, never at Core's tip.
• How c moves. Only a block with a chain lock update moves it (Tenderdash enforces this, and next_block_to checks it never decreases). The proposer takes the chain lock from its own Core. Init chain starts at the genesis core height.
• Chain lock verification and syncing Core to it: the signature check (locally or through Core), submitchainlock, the almost-synced wait, and the ChainLockedBlockNotKnownByCore and InvalidChainLock refusals.
• Why every node's Core answers about the same block, including a gap stated plainly: Core accepts a chain lock before it has the block.
  • In that state, a read at c usually fails on that node.
  • On a competing branch, the node's results can differ, which shows as an app hash mismatch for anything written to GroveDB.
  • The text notes that checking Core's block hash at c would close the gap, and that drive-abci does not do it.
• What Platform reads from Core and where each result is kept (GroveDB versus Platform state):
  • Core fork activations, the masternode list and quorums in update_core_info.
  • The credit pool balance scan, asset unlock statuses and chain asset lock proofs.
• What each step does on a Core error: step 4 rejects the proposal, a chain asset lock proof fails its transition, and everywhere else the error stops the node. Also why a block that keeps c does not depend on Core.
• The startup wait (wait_for_core_to_sync).
• Rules for adding a Core read:
  • go through CoreRPCLike and read at c;
  • keep nothing between blocks outside state;
  • a Core error fails the block;
  • store in GroveDB what later blocks depend on;
  • prefer blocks that move c;
  • add the method to dashmate's drive_consensus whitelist;
  • treat the read as consensus behaviour.
• Step 5 in the list now reads "Core fork activations, masternode list, quorums".

architecture/core-fork-activations.md: a new page, listed under Architecture in SUMMARY.md:

• The record: Misc (104) / [b'F', id] -> u32 big endian, written once and covered by the app hash. Also the CoreFork enum and the fetch_core_fork_activation_height read API (pass the block's transaction, and None means "not seen active").
• When Core is asked: init chain for chains born at protocol version 15 or later, and every block that moves c until the fork is recorded. Includes a mermaid flowchart and the argument that every node records the same height.
• The window after the protocol upgrade that starts tracking a fork, and the rule for every consumer.
• V24's signal: coinbase payload version 4 or higher, enforced by Core's CheckCbTx.
• Consumers. The operator-key logic still asks Core directly; it gets the same answer because it runs after the record is updated.
• Adding a fork: a CoreFork variant, its key and structure node, and a new update_core_info generation.
• Forks without a block-enforced signal, such as evo_shares: getdeploymentinfo at getblockhash(c - 1) and the three preconditions for tracking one.
• A possible table-driven evolution (CoreForkSignal), marked as not implemented.

contributing/coding-conventions.md: a short rule, "Consensus code reads Core at the chain-locked height", next to "Consensus code reads the version from state". It links to both chapters.

The asset unlock payload version 2 consumer is not described here: that code arrives with #5634, which will add its own paragraph to the Consumers section.

How Has This Been Tested?

Documentation only; no code changes.

• Every file path and symbol the text cites was checked against v5.1-dev (after feat(drive-abci)!: record Core fork activation in state (PV15) #5681 merged).
• Claims about other software were checked against their sources:
  • Tenderdash v1.8-dev: ValidateBlockChainLock, and how the prevoter and socket client handle errors.
  • Dash Core develop, v24.0.x and v23.0.x: submitchainlock, UpdateBestChainlock, CheckCbTx, the coinbase versions and getdeploymentinfo.
• mdbook is not installed on the machine used. Instead, a script checked all 139 relative links and heading anchors in the changed files, and the SUMMARY.md entry points to an existing file.
• Two independent reviews (accuracy against the code, and clarity for a new contributor) were folded in. The accuracy review corrected how Core errors are handled in step 4 and for chain asset lock proofs.

Breaking Changes

None.

Checklist:

• I have performed a self-review of my own code
• My Rust, FFI, WASM, Swift or Kotlin changes follow the coding conventions
• I have commented my code, particularly in hard-to-understand areas
• I have added or updated relevant unit/integration/functional/e2e tests
• I have added "!" to the title and described breaking changes in the corresponding section if my code contains any
• I have made corresponding changes to the documentation if needed
• If I added or changed GroveDB structure, I described it in the area's structure.rs, regenerated grovedb-structure.json, and checked the structure viewer link posted on this pull request
• If I added or changed a system contract's document schemas, I checked the contract viewer link posted on this pull request

For repository code-owners and collaborators only

• I have assigned this pull request to a milestone

🤖 Generated with Claude Code

PR Hygiene · 16f8f4e

• Bots — thepastaclaw ✓
• Self-review — post /self-reviewed
• Build green
• Approvals — you own every area touched; none needed

When every merge requirement is met, the PR Hygiene check passes. Reviewer limits do not block merging; other required GitHub checks and protections still apply.

[coderabbitai]

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 41 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

Run configuration

• Source mode: Auto
• Repository: dashpay/platform
• Branch: shumkov/book-core-reads
• File: .coderabbit.yaml
• Commit: 16f8f4e
• Layers: Repository YAML
• Review profile: CHILL
• Plan: Advanced
• Run ID: 1402ff6d-4c65-47b5-8c9f-42f51e44e892

Commits

Reviewing files that changed from the base of the PR and between b90c412 and 16f8f4e.

Files selected for processing: 4

• book/src/SUMMARY.md
• book/src/architecture/component-pipeline.md
• book/src/architecture/core-fork-activations.md
• book/src/contributing/coding-conventions.md

• 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

[github-actions]

📖 Book Preview built successfully.

Download the preview from the workflow artifacts.
To view locally: download the artifact, unzip, and open index.html.

Updated at 2026-11-04T05:05:40.695Z

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

✅ Final review complete — Phase 1 only — no blockers (commit 16f8f4e) · triage: trivial

[thepastaclaw]

Final review — Phase 1 only (trivial change)

Verified the documentation finding against head ae304e4: the chain-lock walkthrough omits the explicit verifychainlock pre-check introduced in protocol version 15. No blocking issues were confirmed. The book preview passed CI, code suites were skipped for this documentation-only change, and PR Hygiene remains pending; the base's additional SUMMARY.md entry does not supersede this change.

🟡 1 suggestion(s)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: architecture-layering); final verifier: gpt-6.1-sol (agent: sol-gate-verifier, role: final-verifier)

• Triage: trivial by gpt-6.1-sol (effort low) — The diff changes only book documentation and navigation, with no runtime, consensus, storage, or other behavioural changes.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — architecture-layering (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 96% left, weekly 73% left; passed over gemini-3.8-flash-high (antigravity quota lookup failed)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-gate-verifier
• Phase 2 reviewers: not run (triage rated this change trivial); this review comments and never approves

🤖 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 `book/src/architecture/component-pipeline.md`:
- [SUGGESTION] book/src/architecture/component-pipeline.md:333-337: Distinguish Core signature verification from chain-lock submission
  The walkthrough leaves Core's signature check implicit while naming only submitchainlock. From protocol version 15, verify_chain_lock_through_core_v1 explicitly calls Core's verifychainlock before submitting a chain lock that Platform cannot verify locally; a false result returns (false, None) without submission. This distinction matters because submission alone does not authenticate a chain lock when Core already holds one at or above its height. The version tables and should_check_signature_before_submitting_from_protocol_version_15 explicitly preserve that older behavior through version 14. Name verifychainlock and its version gate here so readers do not mistake submission and the returned height for signature verification.

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.

• Track validation of Core's block hash at the chain-locked height — The new documentation identifies a concrete pre-existing gap: make_sure_core_is_synced_to_chain_lock_v0 treats Core's reported best chain-lock height as sufficient and does not compare Core's active-chain block hash at c with the verified chain lock. A competing block at that height can therefore feed inconsistent subsequent reads. This documentation-only PR exposes the limitation without introducing or worsening it.
  • Follow-up: Track a separate consensus review of checking Core's block hash against the verified chain lock before dependent reads, including protocol-version placement and unknown-block behavior.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.

[github-actions]

Waiting for bot review — thepastaclaw not yet. Wait for the missing reviews, or a writer can post /skip-bots to proceed without them; blocking findings still need addressing.
Full checklist in the description.

[thepastaclaw]

Re-review — Final review — Phase 1 only (trivial change)

Verified the documentation changes at head 16f8f4e and confirmed that the prior signature-verification suggestion is fixed. No actionable findings remain in the supplied reviewer evidence. The book-preview check passes, policy reconciliation passes, and PR Hygiene remains pending; code build and test jobs were skipped for this documentation-only change.

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

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: architecture-layering); final verifier: gpt-6.1-sol (agent: sol-gate-verifier, role: final-verifier)

• Triage: trivial by gpt-6.1-sol (effort low) — The diff changes only book documentation and navigation, without changing runtime behaviour, consensus rules, or persisted state.
• Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — architecture-layering (completed, effort high); agent phase1-reviewer
• Phase 1 model: glm-5.3-flash — zai quota: 5h 92% left, weekly 69% left; passed over gemini-3.8-flash-high (antigravity quota lookup failed)
• Fresh verifier: gpt-6.1-sol — final-verifier; agent sol-gate-verifier
• Phase 2 reviewers: not run (triage rated this change trivial); this review comments and never approves

🤖 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.

[github-actions]

Bots are done — your move: post /self-reviewed.
Full checklist in the description.