Skip to content

docs: align consensus references and guides with current behavior - #1586

Merged
lklimek merged 29 commits into
devfrom
docs/consensus-source-corrections
Nov 6, 2026
Merged

lklimek merged 29 commits into
devfrom
docs/consensus-source-corrections

Conversation

@lklimek

@lklimek lklimek commented Nov 4, 2026 •

Copy link
Copy Markdown
Collaborator

TL;DR: Correct inherited documentation so operators and application developers follow the consensus behavior Tenderdash actually implements.

User story

As a Tenderdash developer or operator, I want the guides to agree with the protocol specification, so that outdated examples and duplicated explanations do not lead me to incorrect assumptions.

Scenario

Base flow

Read the node, validator, application, signing, and recovery documentation while configuring a node or implementing an application.

Actual behavior

Several pages still describe an older application lifecycle, a following block needed to expose application state, unsupported restart protection, and simplified voting or signing rules that conflict with the current implementation.

Expected behavior

Guides give current, limited instructions and link to the canonical protocol chapters for detailed rules. Historical interfaces are labeled, and documented limitations stay explicit.

Detailed discussion

Issue being fixed or feature implemented

Guides and current references contain contradictory consensus rules and ABCI examples that do not match the shipped implementation.

What was done

This PR targets dev after merging the documentation migration from #1585. It contains the substantive content corrections separated from that migration.

  • Replace obsolete guide/tutorial/architecture/roadmap descriptions and incompatible examples with current guidance or references to shipped examples and canonical specifications.
  • Correct same-block AppHash, empty-block scheduling, absent commit timeout, validator membership/power, h+1 updates, aggregate commits, and distinct vote/recovery thresholds.
  • Consolidate the current application interface into spec/abci/, remove superseded ABCI reference pages and spec/abci++, and update the generated API output path and all inbound links.
  • Correct ABCI CLI defaults, socket/gRPC address forms and standalone command syntax; document missing protocol methods/fields, abbreviated responses and reused-connection parsing limits, with canonical interface links. Correct transaction/block event documentation; remove legacy Commit/DeliverTx lifecycle claims from current references and remove superseded legacy ABCI pages.
  • Consolidate duplicated state, signing, application-call-sequence, rejection/reprocessing, and gossip explanations into links to the source-derived consensus chapters.
  • Replace fully superseded PBTS documents with redirects, label unreplaced research historical/outdated, and replace the bundled original consensus paper with an attributed external publication link. Remove the unsupported Rust-client comparison from RPC support tables; repair the remaining hosted RPC-reference link.
  • Correct signing-domain and app-version assertions. Record the observed FilePV repeated-proposal signature assignment in known issues, with verification failure explicitly labeled inferred and without a reproducing test.
  • Mark light clients unsupported, link validator onboarding to the Dash evonode setup guide, remove the roadmap, and repair RPC subscription examples and rendered navigation.
  • Align InitChain/Query wire tables and byte-based snapshot restoration with the shipped schema; distinguish startup, advertised and proposal execution versions; document transport-dependent concurrency.
  • Replace stale wire tables with canonical generated schemas; correct proposal transaction handling, callback sequences, event attributes and signing timeliness exceptions. Regenerate API/Go comments from the protobuf source.
  • Update duplicated configuration-template comments about the unused double-sign-check setting. No settings, wire formats, consensus actions, or runtime behavior change.

Testing

  • Complete mdBook build on the final tree: 160 canonical chapters; local pages/assets checked across 164 HTML files. All nine documentation pipeline tests pass.
  • Corrected public Go RPC subscription example compiles against the current client. Final reviewer verification reports no remaining findings after fixing eleven documentation defects.
  • Protobuf API and Go regeneration completed; generated Go and configuration changes verified to contain comments only.
  • gofmt, whitespace and ephemeral-reference checks pass. Targeted Markdown lint comparison introduces no new rule-count regressions and reduces inherited findings from 1,028 to 668.
  • Follow-up validation: 46 wire fields across 10 tables and seven snapshot results checked against protobuf; all nine staging tests and the 160-chapter/164-page documentation build pass; targeted Markdown lint improves from 39 to 23 inherited findings.
  • No full runtime suite: runtime logic, settings and wire schemas are unchanged. Live Pages deployment was not run.

Breaking changes

No runtime or public API changes. Some obsolete duplicated recipes are replaced by links to their canonical documentation; legacy headings/useful anchors are retained where practical.

Prior work

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/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

Attribution

🤖 Co-authored by Claudius the Magnificent AI Agent

PR Hygiene · f8cecf6

  • Bots — thepastaclaw ✓
  • Self-review — posted; again after any push
  • 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.

lklimek and others added 9 commits November 4, 2026 08:25
Update current product references and implementation links, connect companion
specifications locally, and retain upstream research and wire identifiers.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Resolve legacy light-client references to existing companion sections and
correct the accepted genesis key types and BLS12-381 default.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Rename core guides, update commands and module links, and replace stale
upstream product overviews with current Tenderdash and ABCI guidance.
Preserve historical design references and protocol identifiers.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Build guides, protocol specifications, and RPC reference with the same
mdBook version and GitHub Pages artifact flow as Dash Platform. Include
all consensus companion chapters, local search, and Mermaid diagrams.
Validate chapter coverage, rendered links, and migration edge cases;
remove obsolete VuePress assets and inherited product branding.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: OpenAI Codex <noreply@openai.com>
Keep generator, navigation, branding, paths and link changes here.
Move guide rewrites and protocol-content corrections to a separate
stacked documentation pull request.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: OpenAI Codex <noreply@openai.com>
Replace inherited consensus/application descriptions and obsolete
examples with current Tenderdash guidance and canonical-spec links.
This content correction is separate from the mdBook toolchain migration.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: OpenAI Codex <noreply@openai.com>
Replace obsolete execution, quorum and ABCI examples with current guidance
and canonical specification links. Correct duplicated configuration template
comments without changing settings or runtime behavior.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Replace legacy state, signing and ABCI lifecycle assertions with current rules
and links to the canonical consensus chapters. Clarify threshold recovery,
commit-round handling, event results and observed FilePV proposal reuse behavior.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Regenerate the Go documentation after the RequestCheckTx branding update.
The output changes one comment and leaves generated code unchanged.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Nov 4, 2026 •

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • Trigger review
  • 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

@thepastaclaw

thepastaclaw commented Nov 4, 2026 •

Copy link
Copy Markdown

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

Comment thread docs/app-dev/indexing-transactions.md Resolved
Comment thread docs/nodes/configuration.md Resolved
Comment thread docs/nodes/light-client.md Resolved
Comment thread docs/nodes/validators.md Resolved
Comment thread docs/roadmap/roadmap.md Resolved
Comment thread spec/abci/README.md Resolved
lklimek and others added 6 commits November 4, 2026 10:34
Co-Authored-By: Claude gpt-6.1-sol <noreply@anthropic.com>
…PC client

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Follow the repository default branch for automatic publication and allow
manual publication from a selected branch. Document HTTP previews, repair
RPC documentation links, and check local links embedded in OpenAPI text.

Restore upstream provenance in historical tutorials and project examples
without importing the separate current-API guide rewrites.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Integrate the current development branch before propagating the updated
documentation base through the stacked content and cleanup branches.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Preserve the current Tenderdash guides while incorporating the publication,
preview and RPC hyperlink fixes from the updated technical base.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Claude GPT-6 <noreply@anthropic.com>
Base automatically changed from docs/mdbook-v1.8 to v1.8-dev November 4, 2026 15:26
lklimek and others added 2 commits November 4, 2026 15:34
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Codex <noreply@openai.com>
Replace legacy ABCI pages with one current reference, repair rendered links and public-client examples, clarify unsupported light clients and validator registration, and remove the roadmap. Correct protocol descriptions against source and regenerate API/Go comments from protobuf documentation.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Codex <noreply@openai.com>

lklimek commented Nov 4, 2026

Copy link
Copy Markdown
Collaborator Author

/self-reviewed

Reviewed dbe317f after merging v1.8-dev. Fixed eleven documentation findings across independent protocol/security, correctness and final consolidation passes; the final scoped verification found no remaining findings. Addressed the three open bot comments from #1585 and replied to the six comments on this PR. Removal of the unused double-sign-check-height setting remains a separately tracked follow-up at the reviewer's request.

Validation: full book build (160 chapters; local pages/assets across 164 HTML files), nine documentation tests, compilation of the public Go subscription example, protobuf/API regeneration, gofmt, comments-only Go invariant and whitespace checks. Targeted Markdown lint adds no new rule-count regressions. No runtime logic or wire schemas change; no full runtime suite or live Pages deployment was run.

🤖 Co-authored by Claudius the Magnificent AI Agent

@lklimek
lklimek marked this pull request as ready for review November 4, 2026 18:03
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added the waiting-bots Waiting for the review bots to report on this head label Nov 4, 2026

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Preliminary review — Phase 1 blocker gate

Both supplied findings are confirmed at e64c1bd: the consolidated ABCI reference documents an incorrect CheckTx response schema and unsupported CheckTx events. Both are in scope because this PR moves the reference and explicitly rewrites event documentation to match the implementation. Documentation CI passes; tests (04) fails with workerpool races, light-client failures, node timeouts/leaks, and signer failures in unchanged runtime code, which appear unrelated to these documentation and comment changes.

Validated blockers were found by the Phase-1 review and confirmed by a fresh verifier. Phase 2 is deferred until a fresh same-head revalidation clears the blocker gate.

🔴 2 blocking

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); final verifier: gpt-6.1-sol (agent: sol-gate-verifier, role: verifier)

  • Triage: normal by gpt-6.1-sol (effort low) — This is a large, cross-cutting correction of protocol and application-interface documentation that warrants substantive verification, but the diff does not change runtime consensus rules, signing behavior, or other critical surfaces.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (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)
  • Fresh verifier: gpt-6.1-sol — verifier; agent sol-gate-verifier
  • Phase 2 reviewers: not run (deferred by blocker gate)
🤖 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 `spec/abci/methods.md`:
- [BLOCKING] spec/abci/methods.md:135-146: CheckTx response table invents fields and field numbers
  The canonical CheckTx response table contradicts ResponseCheckTx in proto/tendermint/abci/types.proto:455-463 and the generated Go struct. The actual fields are code=1, data=2, info=3, gas_wanted=4, codespace=5, sender=6, and priority=7; log, gas_used, and events do not exist. Implementing the documented schema would produce incompatible wire messages. Although inherited from the former abci++ page, this table is moved into the consolidated current reference by this PR and must match the shipped interface. Replace it with the seven actual fields or link to the generated schema.

In `proto/tendermint/abci/types.proto`:
- [BLOCKING] proto/tendermint/abci/types.proto:614-615: Event comment falsely attributes events to ResponseCheckTx
  ResponseCheckTx has no events field, but this rewritten comment says it carries Event values. The same false claim appears in spec/abci/basic-concepts.md:225, whose example at lines 268-270 constructs ResponseCheckTx{Events: ...} and cannot compile against the generated Go type. The actual event-bearing results are ResponseProcessProposal.events and ExecTxResult.events. Remove CheckTx from the protobuf comment, regenerate the Go/API documentation, and update the basic-concepts prose and example to use a supported response type. These changes are directly within the PR's stated correction of transaction/block event documentation.

Comment thread spec/abci/methods.md Resolved
Comment thread proto/tendermint/abci/types.proto Resolved
@github-actions

github-actions Bot commented Nov 4, 2026

Copy link
Copy Markdown

Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them.
Full checklist in the description.

Align InitChain and Query fields with the shipped protobuf schema, document
byte snapshot identifiers and explicit completion, and distinguish advertised
application versions from proposal execution versions. Describe transport
concurrency and configured voting/signature recovery thresholds accurately.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-Authored-By: Codex GPT-6 <noreply@openai.com>
@github-actions github-actions Bot removed the waiting-self-review Waiting for the author to post /self-reviewed label Nov 5, 2026

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Re-review — Preliminary review — Phase 1 blocker gate

Verified the exact head e17287a: all ten prior findings are fixed, but two documentation contradictions remain in the consolidated protocol reference. The canonical vote-extension table gives an incorrect signing-field number, and the protobuf snapshot comments retain the obsolete numeric chunk contract. Current CI reports no failed or pending checks; validation here was static only.

Validated blockers were found by the Phase-1 review and confirmed by a fresh verifier. Phase 2 is deferred until a fresh same-head revalidation clears the blocker gate.

🔴 2 blocking

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

🔴 Blocking: Remove the obsolete numeric snapshot-chunk contract from the canonical schema
proto/tendermint/abci/types.proto:178-179

The chunk_id comment still specifies an index starting at zero, contradicting both the corrected methods.md reference and the implementation: internal/statesync/syncer.go initializes the queue with snapshot.Hash and enqueues application-provided NextChunks after ACCEPT. The preceding request documentation also describes completion as all chunks being accepted, whereas applyChunks returns successfully only on COMPLETE_SNAPSHOT; ACCEPT with no further identifiers does not complete restoration. These are missed canonical-source counterparts of the byte-based restoration correction explicitly included in this PR's scope. Update the request comments to describe byte identifiers and explicit completion, review the related refetch wording so it does not imply numeric ordering, and regenerate the Go/API documentation.

source: muse-spark-1.3-contributor (phase1-reviewer: general)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (agent: phase1-reviewer, role: general); final verifier: gpt-6.1-sol (agent: sol-gate-verifier, role: verifier)

  • Triage: normal by gpt-6.1-sol (effort low) — This is a large, cross-cutting correction of protocol and application-interface documentation requiring substantive correctness checks, but the diff does not change runtime behavior or a critical execution surface.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (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)
  • Fresh verifier: gpt-6.1-sol — verifier; agent sol-gate-verifier
  • Phase 2 reviewers: not run (deferred by blocker gate)
🤖 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 `spec/abci/methods.md`:
- [BLOCKING] spec/abci/methods.md:782: Correct the canonical vote-extension type field number
  The table assigns field number 100 to CanonicalVoteExtension.type, but proto/tendermint/types/canonical.proto defines it as field 5. This is the canonical structure used to produce signing bytes, so implementing this table would produce incompatible serialization and signatures. The PR moves this table into the consolidated current reference and explicitly aims to align wire documentation with the shipped schema. Change the field number to 5.

In `proto/tendermint/abci/types.proto`:
- [BLOCKING] proto/tendermint/abci/types.proto:178-179: Remove the obsolete numeric snapshot-chunk contract from the canonical schema
  The chunk_id comment still specifies an index starting at zero, contradicting both the corrected methods.md reference and the implementation: internal/statesync/syncer.go initializes the queue with snapshot.Hash and enqueues application-provided NextChunks after ACCEPT. The preceding request documentation also describes completion as all chunks being accepted, whereas applyChunks returns successfully only on COMPLETE_SNAPSHOT; ACCEPT with no further identifiers does not complete restoration. These are missed canonical-source counterparts of the byte-based restoration correction explicitly included in this PR's scope. Update the request comments to describe byte identifiers and explicit completion, review the related refetch wording so it does not imply numeric ordering, and regenerate the Go/API documentation.

Comment thread spec/abci/methods.md Resolved
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix; thepastaclaw left review threads unresolved; resolve them.
Full checklist in the description.

@github-actions github-actions Bot added the waiting-self-review Waiting for the author to post /self-reviewed label Nov 6, 2026
Remove the 12 approved design, research and campaign documents and their
book navigation entries. Point surviving consensus references to operational
monitoring documentation; retain its dashboard and alert assets unchanged.

<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>

Co-authored-by: Claude GPT-6 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

Your move: thepastaclaw left review threads unresolved; resolve them.
Full checklist in the description.

lklimek and others added 2 commits November 6, 2026 10:42
The vote-extension `type` field is number 5, and height/round are
sfixed64 in the signed canonical encoding.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added waiting-bots Waiting for the review bots to report on this head and removed waiting-self-review Waiting for the author to post /self-reviewed labels Nov 6, 2026

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Verified head 470f1e8: nine prior findings are fixed, two were unsupported and are withdrawn, and the canonical snapshot-contract finding remains valid. Two additional in-scope documentation issues remain: unconditional voting promises and a broken enum link. Static verification only; current CI reports no failures, with build, lint and protobuf checks passing while documentation and several test jobs remain pending; the supplied base drift has no conflicting files.

🔴 2 blocking | 🟡 1 suggestion(s)

3 finding(s) not shown inline (GitHub refused the PR diff as too large)

🔴 Blocking: Remove the obsolete numeric snapshot-chunk contract from the canonical schema
proto/tendermint/abci/types.proto:178-179

The protobuf still describes chunk_id as a zero-based index applied sequentially, and the relocated spec/abci/api.md publishes the same contract. The implementation instead seeds the queue with Snapshot.hash (internal/statesync/syncer.go:261) and discovers further byte identifiers through ResponseApplySnapshotChunk.next_chunks after ACCEPT. The nearby lifecycle comment also says accepting all chunks triggers Info, but applyChunks returns successfully only on COMPLETE_SNAPSHOT; ACCEPT without more chunks does not finish restoration. An application following this reference can reject the initial identifier or never signal completion, preventing successful state sync. These comments are in scope because this PR consolidates the generated reference and explicitly corrects snapshot restoration. Update the owning protobuf comments to match the corrected methods.md contract, including explicit completion, then regenerate the affected Go/API documentation.

source: muse-spark-1.3-contributor (phase1-reviewer: general); gpt-6.1-sol (phase2-reviewer: general)

🔴 Blocking: Do not equate application acceptance with a guaranteed block prevote
spec/abci/methods.md:749-750

These relocated descriptions promise voting transitions that application verdicts do not guarantee. In internal/consensus/state_prevoter.go, prevoter.Do processes the proposal before signVote checks the locking/POL rules: a validator locked on block A can receive ACCEPT for a fresh block B and still prevote nil. Execution-commitment validation can also reject an ACCEPT response. ProcessProposal additionally runs during commit processing and replay, where REJECT is not simply a nil-prevote transition. This contradicts the corrected ProcessProposal discussion earlier on this page. Describe ACCEPT and REJECT as application validity verdicts, and qualify their voting effects by the consensus checks and call path.

source: gpt-6.1-sol (phase2-reviewer: general)

🟡 Suggestion: Fix the VoteExtensionType link anchor in the CanonicalVoteExtension table
spec/abci/methods.md:782

The updated row links to ../core/data_structures.md#voteExtensiontype, but that page has no matching heading or explicit anchor; its relevant section is VoteExtension. The enum is documented on this page under VoteExtensionType. Link to that local section so readers of the consolidated signing table can reach the enum definition.

    | type      | VoteExtensionType | [VoteExtensionType](#voteextensiontype)                                  | 5            |

source: muse-spark-1.3-contributor (phase1-reviewer: general)

Review provenance

Source: reviewer 1: muse-spark-1.3-contributor (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: tenderdash-consensus-security); final verifier: gpt-6.1-sol (agent: sol-verifier, role: final-verifier)

  • Triage: normal by gpt-6.1-sol (effort low) — This is a large, cross-cutting correction of technical documentation and generated references requiring substantive consistency checks, but the diff does not change runtime behavior or a critical executable surface.
  • Phase 1 reviewers: muse-spark-1.3-contributor — general (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)
  • 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 — tenderdash-consensus-security (completed, effort high); 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 `proto/tendermint/abci/types.proto`:
- [BLOCKING] proto/tendermint/abci/types.proto:178-179: Remove the obsolete numeric snapshot-chunk contract from the canonical schema
  The protobuf still describes chunk_id as a zero-based index applied sequentially, and the relocated spec/abci/api.md publishes the same contract. The implementation instead seeds the queue with Snapshot.hash (internal/statesync/syncer.go:261) and discovers further byte identifiers through ResponseApplySnapshotChunk.next_chunks after ACCEPT. The nearby lifecycle comment also says accepting all chunks triggers Info, but applyChunks returns successfully only on COMPLETE_SNAPSHOT; ACCEPT without more chunks does not finish restoration. An application following this reference can reject the initial identifier or never signal completion, preventing successful state sync. These comments are in scope because this PR consolidates the generated reference and explicitly corrects snapshot restoration. Update the owning protobuf comments to match the corrected methods.md contract, including explicit completion, then regenerate the affected Go/API documentation.

In `spec/abci/methods.md`:
- [BLOCKING] spec/abci/methods.md:749-750: Do not equate application acceptance with a guaranteed block prevote
  These relocated descriptions promise voting transitions that application verdicts do not guarantee. In internal/consensus/state_prevoter.go, prevoter.Do processes the proposal before signVote checks the locking/POL rules: a validator locked on block A can receive ACCEPT for a fresh block B and still prevote nil. Execution-commitment validation can also reject an ACCEPT response. ProcessProposal additionally runs during commit processing and replay, where REJECT is not simply a nil-prevote transition. This contradicts the corrected ProcessProposal discussion earlier on this page. Describe ACCEPT and REJECT as application validity verdicts, and qualify their voting effects by the consensus checks and call path.
- [SUGGESTION] spec/abci/methods.md:782: Fix the VoteExtensionType link anchor in the CanonicalVoteExtension table
  The updated row links to ../core/data_structures.md#voteExtensiontype, but that page has no matching heading or explicit anchor; its relevant section is VoteExtension. The enum is documented on this page under VoteExtensionType. Link to that local section so readers of the consolidated signing table can reach the enum definition.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

Your move: thepastaclaw requested changes on this head; dismiss the review or push a fix.
Full checklist in the description.

@github-actions github-actions Bot added waiting-self-review Waiting for the author to post /self-reviewed and removed waiting-bots Waiting for the review bots to report on this head labels Nov 6, 2026
@lklimek lklimek added this to the v2.0 milestone Nov 6, 2026
lklimek and others added 2 commits November 6, 2026 12:41
- Describe ApplySnapshotChunk chunk_id as an application-defined identifier
  seeded from Snapshot.hash and extended via next_chunks, not a numeric index.
- State that restoration finishes only on COMPLETE_SNAPSHOT, followed by the
  Info verification of app hash, height and app version.
- Describe ProposalStatus as the application's validity verdict; voting
  effects depend on execution-result validation, lock/POL rules and call path.
- Fix the VoteExtensionType link to the local methods.md section.
- Regenerate abci/types/types.pb.go and spec/abci/api.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

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.

@github-actions github-actions Bot added waiting-bots Waiting for the review bots to report on this head and removed waiting-self-review Waiting for the author to post /self-reviewed labels Nov 6, 2026
- Describe LoadSnapshotChunk chunk_id as an application-defined identifier,
  matching ApplySnapshotChunk and methods.md.
- Remove a doubled backtick in the ApplySnapshotChunk comment.
- Add the empty Description cell to the PrepareProposal method table row,
  matching the ProcessProposal and FinalizeBlock rows.
- Regenerate abci/types/types.pb.go and spec/abci/api.md.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

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.

@lklimek

lklimek commented Nov 6, 2026

Copy link
Copy Markdown
Collaborator Author

/self-reviewed

@thepastaclaw thepastaclaw left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Verified the documentation changes at f8cecf6: twelve prior findings are fixed, and the two previously withdrawn findings remain withdrawn. Six non-blocking documentation findings remain, including an incorrect StateID wire type and inconsistent Query-height descriptions. Current CI reports successful code and documentation checks, with PR Hygiene pending; no local builds or tests were run for this static review.

🟡 4 suggestion(s) | 💬 2 nitpick(s)

6 finding(s) not shown inline (GitHub refused the PR diff as too large)

🟡 Suggestion: Correct the StateID time field's wire type
spec/consensus/signing.md:142-143

The StateID schema quoted here declares field 5 as google.protobuf.Timestamp, but proto/tendermint/types/types.proto defines it as fixed64 containing milliseconds since epoch. This section explicitly describes state_id as the checksum of the protobuf-encoded StateID, so following this snippet would produce a different encoding and signing digest. The PR consolidates the signing reference while retaining this schema; replace these lines with the shipped definition.

  // Time of the block in milliseconds since epoch, truncated (rounded down) to milliseconds
  fixed64 time = 5;

source: glm-5.3-flash (phase1-reviewer: general, tenderdash-consensus-security)

🟡 Suggestion: Reconcile Query-height semantics across the current references
spec/abci/methods.md:127

Both Query tables describe state after committing block Height, while RequestQuery.height and ResponseQuery.height in types.proto describe state after committing Height-1. The generated api.md repeats Height-1, so the consolidated reference gives conflicting guidance about the state and proof associated with a query height. Same-block execution supports revisiting the inherited schema comments, but does not justify leaving both contracts published. Establish the intended query contract, then align the request and response descriptions in the protobuf source, generated documentation, and this page.

source: glm-5.3-flash (phase1-reviewer: general, tenderdash-consensus-security)

🟡 Suggestion: Align the ConsensusParams table with all seven protobuf fields
spec/abci/methods.md:637-641

The consolidated table stops at version, but proto/tendermint/types/params.proto also defines synchrony=5, timeout=6, and abci=7. The version type is VersionParams, not VersionsParams, and includes both application and consensus versions; the schema also marks this field deprecated. Readers using this as the current parameter reference miss supported consensus controls. Complete the table using the shipped definitions, or replace it with a link to the canonical schema to avoid another incomplete copy.

source: glm-5.3-flash (phase1-reviewer: general, tenderdash-consensus-security)

🟡 Suggestion: Remove unsupported application and peer-dial examples from the guide
docs/tenderdash-core/using-tenderdash.md:80-83

The current guide still advertises an in-process counter application, but internal/proxy/client.go only recognizes kvstore, persistent_kvstore, e2e, and noop; counter falls through to remote-client creation. The statement that these applications ship with abci-cli is also inaccurate: its application command is kvstore. Later examples at lines 315–334 advertise /dial_seeds and /dial_peers, neither of which is registered in internal/rpc/core/routes.go, and the persistent-peer command uses --p2p.persistent_peers instead of the registered --p2p.persistent-peers. These unsupported recipes remain in a guide this PR rewrites to match the implementation. Document the supported applications and startup peer configuration, and remove the nonexistent RPC alternatives.

source: glm-5.3-flash (phase1-reviewer: general, tenderdash-consensus-security)

💬 Nitpick: Name the explicit snapshot-restoration completion signal
spec/abci/basic-concepts.md:96-98

The overview says verification follows when all chunks have been applied, but does not state how completion is signaled. internal/statesync/syncer.go continues after ACCEPT and returns successfully only on COMPLETE_SNAPSHOT; exhausting queued identifiers is not completion. Sequential application itself is accurate, but this relocated overview should name the explicit signal, consistently with the corrected methods reference.

As the new node receives "chunks" the application applies them one at a time
with `ApplySnapshotChunk`. Restoration finishes only when the Application
returns `COMPLETE_SNAPSHOT`; the node then verifies the restored state via an
`Info` query.

source: glm-5.3-flash (phase1-reviewer: general, tenderdash-consensus-security)

💬 Nitpick: Correct typos in the consolidated ABCI reference
spec/abci/methods.md:483

The relocated reference retains "propsed" here, "arbitray" in methods.md:203, and "avaiable" in basic-concepts.md:87. Correct these to "proposed", "arbitrary", and "available" while publishing the consolidated replacement.

source: glm-5.3-flash (phase1-reviewer: general, tenderdash-consensus-security)

Review provenance

Source: reviewer 1: glm-5.3-flash (agent: phase1-reviewer, role: general); reviewer 2: glm-5.3-flash (agent: phase1-reviewer, role: tenderdash-consensus-security); 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 documentation, protobuf and generated comments, reference links, and configuration comments without changing runtime behavior, wire formats, or consensus rules.
  • Phase 1 reviewers: glm-5.3-flash — general (completed, effort high); agent phase1-reviewer, glm-5.3-flash — tenderdash-consensus-security (completed, effort high); agent phase1-reviewer
  • Phase 1 model: glm-5.3-flash — zai quota: 5h 98% left, weekly 43% 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 `spec/consensus/signing.md`:
- [SUGGESTION] spec/consensus/signing.md:142-143: Correct the StateID time field's wire type
  The StateID schema quoted here declares field 5 as google.protobuf.Timestamp, but proto/tendermint/types/types.proto defines it as fixed64 containing milliseconds since epoch. This section explicitly describes state_id as the checksum of the protobuf-encoded StateID, so following this snippet would produce a different encoding and signing digest. The PR consolidates the signing reference while retaining this schema; replace these lines with the shipped definition.

In `spec/abci/methods.md`:
- [SUGGESTION] spec/abci/methods.md:127: Reconcile Query-height semantics across the current references
  Both Query tables describe state after committing block Height, while RequestQuery.height and ResponseQuery.height in types.proto describe state after committing Height-1. The generated api.md repeats Height-1, so the consolidated reference gives conflicting guidance about the state and proof associated with a query height. Same-block execution supports revisiting the inherited schema comments, but does not justify leaving both contracts published. Establish the intended query contract, then align the request and response descriptions in the protobuf source, generated documentation, and this page.
- [SUGGESTION] spec/abci/methods.md:637-641: Align the ConsensusParams table with all seven protobuf fields
  The consolidated table stops at version, but proto/tendermint/types/params.proto also defines synchrony=5, timeout=6, and abci=7. The version type is VersionParams, not VersionsParams, and includes both application and consensus versions; the schema also marks this field deprecated. Readers using this as the current parameter reference miss supported consensus controls. Complete the table using the shipped definitions, or replace it with a link to the canonical schema to avoid another incomplete copy.
- [NITPICK] spec/abci/methods.md:483: Correct typos in the consolidated ABCI reference
  The relocated reference retains "propsed" here, "arbitray" in methods.md:203, and "avaiable" in basic-concepts.md:87. Correct these to "proposed", "arbitrary", and "available" while publishing the consolidated replacement.

In `docs/tenderdash-core/using-tenderdash.md`:
- [SUGGESTION] docs/tenderdash-core/using-tenderdash.md:80-83: Remove unsupported application and peer-dial examples from the guide
  The current guide still advertises an in-process counter application, but internal/proxy/client.go only recognizes kvstore, persistent_kvstore, e2e, and noop; counter falls through to remote-client creation. The statement that these applications ship with abci-cli is also inaccurate: its application command is kvstore. Later examples at lines 315–334 advertise /dial_seeds and /dial_peers, neither of which is registered in internal/rpc/core/routes.go, and the persistent-peer command uses --p2p.persistent_peers instead of the registered --p2p.persistent-peers. These unsupported recipes remain in a guide this PR rewrites to match the implementation. Document the supported applications and startup peer configuration, and remove the nonexistent RPC alternatives.

In `spec/abci/basic-concepts.md`:
- [NITPICK] spec/abci/basic-concepts.md:96-98: Name the explicit snapshot-restoration completion signal
  The overview says verification follows when all chunks have been applied, but does not state how completion is signaled. internal/statesync/syncer.go continues after ACCEPT and returns successfully only on COMPLETE_SNAPSHOT; exhausting queued identifiers is not completion. Sequential application itself is accurate, but this relocated overview should name the explicit signal, consistently with the corrected methods reference.
Out-of-scope follow-up suggestions (2)

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.

  • Remove nonexistent peer-dial endpoints from the OpenAPI reference — rpc/openapi/openapi.yaml still documents /dial_seeds and /dial_peers, although the server route table implements neither. Those endpoint definitions predate this PR and are not changed by its OpenAPI edits.
    • Follow-up: A separate RPC documentation PR should reconcile these endpoint definitions with the registered routes.
  • Remove the stale commit entry from abci-cli command metadata — The CLI metadata predates this PR, and the revised guide accurately documents the missing command. Removing stale runtime help metadata is adjacent cleanup outside this documentation change.
    • Follow-up: Consider creating a separate issue or author/maintainer-requested PR for this.

@github-actions

github-actions Bot commented Nov 6, 2026

Copy link
Copy Markdown

Policy satisfied — this can merge.
Full checklist in the description.

@github-actions github-actions Bot removed the waiting-bots Waiting for the review bots to report on this head label Nov 6, 2026
@lklimek
lklimek merged commit 739d8b8 into dev Nov 6, 2026
31 checks passed
@lklimek
lklimek deleted the docs/consensus-source-corrections branch November 6, 2026 16:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

priority: medium Medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants