fix(statesync): full-node privval error, backfill failure visibility, docs refresh - #1425
fix(statesync): full-node privval error, backfill failure visibility, docs refresh#1425PastaPastaPasta wants to merge 9 commits into
Conversation
…tation The guide still documented the upstream trust anchor settings (trust_height, trust_hash, trust_period) which were removed in 33cda2d, and used underscore key names that do not match the actual dashed config keys. Rewrite it to document the real [statesync] options (enable, use-p2p, rpc-servers, discovery-time, retries, temp-dir, chunk-request-timeout, fetchers), including the block-sync fallback when retries are exhausted, the two-server minimum for rpc-servers when use-p2p=false, the 5s minimum for chunk-request-timeout, and the fact that Dash needs no trust anchors because light blocks are verified via Dash Core quorum threshold signature verification. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The [statesync] section still listed trust-height, trust-hash and trust-period, which were removed from the config in 33cda2d (Tenderdash verifies light blocks via Dash Core quorum verification and needs no trust anchors). Replace them with the retries option so the section matches the template generated by config/toml.go. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The spec still described the upstream format/index/chunks model. Rewrite it to match the shipped proto (proto/tendermint/statesync/types.proto and proto/tendermint/abci/types.proto): snapshots carry version + hash with no format or chunk count, chunks are content-addressed by chunk_id with the snapshot hash as the initial chunk, fetching is driven by next_chunks in ResponseApplySnapshotChunk, and restoration terminates on the COMPLETE_SNAPSHOT result. Also correct message names (SnapshotsRequest/SnapshotsResponse), field numbers, drop the stale Message oneof section that no longer exists in the proto, and note that light block verification uses Dash Core quorum signature verification instead of upstream trust anchors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
makeNode's ModeFull branch eagerly constructed core.NewMockClient(..., privValidator, false) when [priv-validator] core-rpc-host was empty, but privValidator is only assigned in the ModeValidator branch, so it was always nil there and NewMockClient panicked with the unhelpful message 'localPV must be set'. Every full node started without core-rpc-host hit this panic at construction time. A hard configuration error was chosen over a lazy failure because no supported configuration can break: the panic fired unconditionally and eagerly for this mode/config combination, so any setup that would now receive the error was already unable to start. All e2e manifests exercised in CI (dashcore.toml, rotate.toml, statesync-backfill.toml) set privval_protocol = 'dashcore' on their full nodes, which populates core-rpc-host; ci.toml's full01 omits it but ci.toml is not in the CI matrix and would have panicked. No node package tests construct ModeFull nodes, and all other NewMockClient callers pass a non-nil private validator. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When backfill fails after a successful snapshot restore, the reactor proceeds optimistically by design, but the log line gave the operator no hint of the consequences or of what to do. Expand the error-level message to explain that the node lacks historical light blocks within the evidence age and may be unable to validate evidence of misbehavior committed before the snapshot height, and to suggest checking connectivity to peers retaining older blocks or re-syncing from a more recent snapshot. The proceed-optimistically behavior is unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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. Comment |
|
Note on the red 🤖 Posted autonomously by Claude on behalf of pasta. |
|
✅ Final review complete — no blockers (commit 7511fff) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — GLM Flash + Sol
The panic-to-error change is directionally correct and the wire-spec rewrite matches the implementation, but the new error path leaks resources and the operator documentation contains two startup-affecting inaccuracies. These three blocking issues should be fixed before merge; the remaining findings cover regression testing and smaller documentation corrections.
Source: reviewer backends claude-opus-4-6 and glm-5.3-flash; Codex checkpoint verifier gpt-5.6-sol; final verifier backend claude-opus-4-6.
Review provenance
- Phase 1 reviewers (GLM Flash):
glm-5.3-flash— general (completed),glm-5.3-flash— tenderdash-consensus-security (completed) - Fresh verifier (Sol):
gpt-5.6-sol— final-verifier - Phase 2 reviewers (Sol):
gpt-5.6-sol— general (completed),gpt-5.6-sol— tenderdash-consensus-security (completed)
🔴 3 blocking | 🟡 2 suggestion(s) | 💬 2 nitpick(s)
🤖 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 `node/node.go`:
- [BLOCKING] node/node.go:214-217: Validate the full-node configuration before allocating resources
This direct return occurs after `makeNode` has derived a context, opened the block-store and state-store databases, and potentially opened a configured transaction-index event sink. The database closers are registered in `closers`, but this path bypasses them, and event sinks are normally closed only after a `nodeImpl` has been constructed. This is observable through the public in-process constructors: a caller that fixes the configuration and retries can retain database handles and LevelDB directory locks from the failed attempt. Move this configuration validation to the beginning of `makeNode`, before `initDBs` and `EventSinksFromConfig`, rather than returning after initialization.
- [SUGGESTION] node/node.go:214-217: Add a regression test for the full-node configuration error
This error path is the PR's central behavioral change, but no test constructs `ModeFull`; existing node-construction fixtures use `config.TestBaseConfig()`, which sets `ModeValidator`, and the PR changes no test files. Add a test that sets full mode with an empty `CoreRPCHost`, verifies construction does not panic, and asserts the returned error names `core-rpc-host`. The test should also verify that resources opened during a failed construction are closed, or validation should be moved ahead of those allocations so the test can prove none are opened.
- [NITPICK] node/node.go:209-213: Do not describe evidence handling as a Core RPC client consumer
The Dash Core client created here is passed only to the state-sync reactor's light-client state providers. Evidence verification receives the block store and state store, verifies duplicate votes using stored validator public keys, and never receives or calls this client. State-sync backfill supplies history that evidence handling later needs, but backfill verifies commits locally through `ValidatorSet.VerifyCommit`. Narrow the comment to the actual direct use so future readers do not infer an evidence-verification dependency that does not exist.
In `docs/nodes/state-sync.md`:
- [BLOCKING] docs/nodes/state-sync.md:19-25: Document the required Dash Core RPC configuration
The guide says verification uses Dash Core but never tells a full-node operator how to configure that connection. The new `ModeFull` branch rejects startup whenever `[priv-validator] core-rpc-host` is empty, independently of the Tenderdash `rpc-servers` configured for state sync. Because full mode is the default and `core-rpc-host` defaults to empty, an operator following either example still receives the new configuration error. Distinguish the Dash Core endpoint from the two Tenderdash RPC state providers and document the required setting.
- [BLOCKING] docs/nodes/state-sync.md:45-49: Document that retries = 0 retries indefinitely
The text presents zero as disabling retries after promising a block-sync fallback, but `syncer.SyncAny` only returns `errNoSnapshots` through the retry limit when `retries > 0`. With `retries = 0` and a nonzero `discovery-time`, it continues requesting snapshots every interval until cancellation, so the reactor does not fall back to block sync merely because no snapshot is available. An operator expecting immediate fallback can instead leave startup waiting indefinitely. Correct this explanation here and in `docs/nodes/configuration.md` lines 355-358.
- [NITPICK] docs/nodes/state-sync.md:32-34: Exclude seed nodes from the snapshot-serving claim
The statement that every node serves snapshots and light blocks is false for `mode = "seed"`. `makeSeedNode` starts only the P2P router and PEX reactor and never constructs a state-sync reactor. The claim applies to full and validator nodes regardless of whether their own state-sync consumption is enabled.
In `internal/statesync/reactor.go`:
- [SUGGESTION] internal/statesync/reactor.go:378-379: Make the state-sync restart remediation actionable
State sync cannot be retried by an ordinary node restart after this message. `stateStore.Bootstrap(state)` has already persisted a nonzero `LastBlockHeight`, and `makeNode` explicitly skips state sync at nonzero height; the application has also restored the snapshot. Tell operators that retrying from another snapshot requires stopping the node and resetting both Tenderdash and application state, rather than leaving “restart state sync” undefined.
| if cfg.PrivValidator.CoreRPCHost == "" { | ||
| return nil, errors.New("a full node requires a Dash Core RPC connection for light client " + | ||
| "verification: set core-rpc-host in the [priv-validator] section of config.toml " + | ||
| "(and core-rpc-username/core-rpc-password as needed)") |
There was a problem hiding this comment.
🔴 Blocking: Validate the full-node configuration before allocating resources
This direct return occurs after makeNode has derived a context, opened the block-store and state-store databases, and potentially opened a configured transaction-index event sink. The database closers are registered in closers, but this path bypasses them, and event sinks are normally closed only after a nodeImpl has been constructed. This is observable through the public in-process constructors: a caller that fixes the configuration and retries can retain database handles and LevelDB directory locks from the failed attempt. Move this configuration validation to the beginning of makeNode, before initDBs and EventSinksFromConfig, rather than returning after initialization.
source: ['claude']
There was a problem hiding this comment.
Resolved in ded421c — Validate the full-node configuration before allocating resources no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| Unlike upstream Tendermint, Tenderdash does **not** require trust anchors | ||
| (`trust-height`, `trust-hash`, `trust-period`). Light blocks are verified by | ||
| checking the quorum threshold signature against the active validator quorum via | ||
| Dash Core (`quorum verify`), so there is no need to obtain a trusted block hash | ||
| out of band. If you are migrating a configuration that still contains | ||
| `trust-height`, `trust-hash`, or `trust-period` under `[statesync]`, remove | ||
| them — they are no longer valid options. |
There was a problem hiding this comment.
🔴 Blocking: Document the required Dash Core RPC configuration
The guide says verification uses Dash Core but never tells a full-node operator how to configure that connection. The new ModeFull branch rejects startup whenever [priv-validator] core-rpc-host is empty, independently of the Tenderdash rpc-servers configured for state sync. Because full mode is the default and core-rpc-host defaults to empty, an operator following either example still receives the new configuration error. Distinguish the Dash Core endpoint from the two Tenderdash RPC state providers and document the required setting.
| Unlike upstream Tendermint, Tenderdash does **not** require trust anchors | |
| (`trust-height`, `trust-hash`, `trust-period`). Light blocks are verified by | |
| checking the quorum threshold signature against the active validator quorum via | |
| Dash Core (`quorum verify`), so there is no need to obtain a trusted block hash | |
| out of band. If you are migrating a configuration that still contains | |
| `trust-height`, `trust-hash`, or `trust-period` under `[statesync]`, remove | |
| them — they are no longer valid options. | |
| Unlike upstream Tendermint, Tenderdash does **not** require trust anchors | |
| (`trust-height`, `trust-hash`, `trust-period`). Light blocks are verified by | |
| checking the quorum threshold signature against the active validator quorum via | |
| Dash Core (`quorum verify`), so there is no need to obtain a trusted block hash | |
| out of band. Full nodes must set `core-rpc-host` under `[priv-validator]` (and | |
| configure `core-rpc-username` and `core-rpc-password` as required), in addition | |
| to any Tenderdash `rpc-servers` configured below. If you are migrating a | |
| configuration that still contains `trust-height`, `trust-hash`, or | |
| `trust-period` under `[statesync]`, remove them — they are no longer valid | |
| options. |
source: ['claude']
There was a problem hiding this comment.
Resolved in 56d5391 — Document the required Dash Core RPC configuration no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| - `retries`: Number of times to retry state sync before giving up. When | ||
| retries are exhausted, the node **falls back to regular block sync**. Set to | ||
| `0` to disable retries (default: `3`). Note that in the pessimistic case it | ||
| will take at least `discovery-time * retries` before falling back to block | ||
| sync. |
There was a problem hiding this comment.
🔴 Blocking: Document that retries = 0 retries indefinitely
The text presents zero as disabling retries after promising a block-sync fallback, but syncer.SyncAny only returns errNoSnapshots through the retry limit when retries > 0. With retries = 0 and a nonzero discovery-time, it continues requesting snapshots every interval until cancellation, so the reactor does not fall back to block sync merely because no snapshot is available. An operator expecting immediate fallback can instead leave startup waiting indefinitely. Correct this explanation here and in docs/nodes/configuration.md lines 355-358.
| - `retries`: Number of times to retry state sync before giving up. When | |
| retries are exhausted, the node **falls back to regular block sync**. Set to | |
| `0` to disable retries (default: `3`). Note that in the pessimistic case it | |
| will take at least `discovery-time * retries` before falling back to block | |
| sync. | |
| - `retries`: Controls how long snapshot discovery retries before giving up | |
| (default: `3`). With a positive value, exhausting discovery retries makes the | |
| node **fall back to regular block sync**. A value of `0` means retry | |
| indefinitely: the node keeps requesting snapshots every `discovery-time` and | |
| does not fall back to block sync solely because none are available. |
source: ['claude', 'codex']
There was a problem hiding this comment.
Resolved in 56d5391 — Document that retries = 0 retries indefinitely no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| "Check connectivity to peers that retain older blocks, or restart state sync from "+ | ||
| "a more recent snapshot.", |
There was a problem hiding this comment.
🟡 Suggestion: Make the state-sync restart remediation actionable
State sync cannot be retried by an ordinary node restart after this message. stateStore.Bootstrap(state) has already persisted a nonzero LastBlockHeight, and makeNode explicitly skips state sync at nonzero height; the application has also restored the snapshot. Tell operators that retrying from another snapshot requires stopping the node and resetting both Tenderdash and application state, rather than leaving “restart state sync” undefined.
| "Check connectivity to peers that retain older blocks, or restart state sync from "+ | |
| "a more recent snapshot.", | |
| "Check connectivity to peers that retain older blocks. To retry state sync from "+ | |
| "a more recent snapshot, stop the node, reset both Tenderdash and application state, "+ | |
| "and start the node again.", |
source: ['claude']
There was a problem hiding this comment.
Resolved in 7511fff — Make the state-sync restart remediation actionable no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| if cfg.PrivValidator.CoreRPCHost == "" { | ||
| return nil, errors.New("a full node requires a Dash Core RPC connection for light client " + | ||
| "verification: set core-rpc-host in the [priv-validator] section of config.toml " + | ||
| "(and core-rpc-username/core-rpc-password as needed)") |
There was a problem hiding this comment.
🟡 Suggestion: Add a regression test for the full-node configuration error
This error path is the PR's central behavioral change, but no test constructs ModeFull; existing node-construction fixtures use config.TestBaseConfig(), which sets ModeValidator, and the PR changes no test files. Add a test that sets full mode with an empty CoreRPCHost, verifies construction does not panic, and asserts the returned error names core-rpc-host. The test should also verify that resources opened during a failed construction are closed, or validation should be moved ahead of those allocations so the test can prove none are opened.
source: ['claude', 'codex']
There was a problem hiding this comment.
Resolved in ded421c — Add a regression test for the full-node configuration error no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| // Full nodes still need a Dash Core RPC connection: light client | ||
| // verification (used by state sync and evidence handling) checks quorum | ||
| // threshold signatures via Dash Core. Without core-rpc-host the only | ||
| // fallback would be a mock client backed by a local private validator, | ||
| // which a full node does not have. |
There was a problem hiding this comment.
💬 Nitpick: Do not describe evidence handling as a Core RPC client consumer
The Dash Core client created here is passed only to the state-sync reactor's light-client state providers. Evidence verification receives the block store and state store, verifies duplicate votes using stored validator public keys, and never receives or calls this client. State-sync backfill supplies history that evidence handling later needs, but backfill verifies commits locally through ValidatorSet.VerifyCommit. Narrow the comment to the actual direct use so future readers do not infer an evidence-verification dependency that does not exist.
| // Full nodes still need a Dash Core RPC connection: light client | |
| // verification (used by state sync and evidence handling) checks quorum | |
| // threshold signatures via Dash Core. Without core-rpc-host the only | |
| // fallback would be a mock client backed by a local private validator, | |
| // which a full node does not have. | |
| // Full nodes still need a Dash Core RPC connection: state sync light | |
| // client verification checks quorum threshold signatures via Dash Core. | |
| // Without core-rpc-host, the only fallback would be a mock client backed | |
| // by a local private validator, which a full node does not have. |
source: ['codex']
There was a problem hiding this comment.
Resolved in ded421c — Do not describe evidence handling as a Core RPC client consumer no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| - `enable`: Inform the node that you will be using state sync to bootstrap. | ||
| This only controls *consuming* snapshots at first start; every node serves | ||
| snapshots and light blocks to peers regardless of this setting. |
There was a problem hiding this comment.
💬 Nitpick: Exclude seed nodes from the snapshot-serving claim
The statement that every node serves snapshots and light blocks is false for mode = "seed". makeSeedNode starts only the P2P router and PEX reactor and never constructs a state-sync reactor. The claim applies to full and validator nodes regardless of whether their own state-sync consumption is enabled.
| - `enable`: Inform the node that you will be using state sync to bootstrap. | |
| This only controls *consuming* snapshots at first start; every node serves | |
| snapshots and light blocks to peers regardless of this setting. | |
| - `enable`: Inform the node that you will be using state sync to bootstrap. | |
| This only controls *consuming* snapshots at first start; full and validator | |
| nodes serve snapshots and light blocks to peers regardless of this setting. | |
| Seed nodes run only the PEX reactor and do not serve state-sync data. |
source: ['codex']
There was a problem hiding this comment.
Resolved in 56d5391 — Exclude seed nodes from the snapshot-serving claim no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
The check only needs cfg.Mode and cfg.PrivValidator.CoreRPCHost, so run it at the top of makeNode: previously it fired after the block/state stores and event sinks were opened and returned without the registered closers, leaking DB handles and LevelDB locks to in-process callers who retry. Also narrow the comment: the Dash Core RPC client feeds only the state sync light client providers; evidence verification checks validator-set signatures locally. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Pins that the constructor returns an error naming core-rpc-host (no panic), leaves the data directory untouched, and that an immediate retry gets the same config error rather than a DB-lock error from leaked handles. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…e core-rpc-host retries=0 with a nonzero discovery-time retries indefinitely and never falls back to block sync (syncer.SyncAny only exits via errNoSnapshots when retries are exhausted or discovery is disabled), so stop claiming it disables retries; align the config template and struct comment too. Document that full nodes must set [priv-validator] core-rpc-host to start, include it in both examples, and scope the snapshot-serving claim to full/validator nodes since seed nodes only run PEX. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Retrying state sync after a backfill failure requires stopping the node and resetting both the tenderdash data directory and the application state, because the state store was already bootstrapped at the snapshot height and state sync is skipped on any nonzero height. Say so instead of suggesting a bare restart. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
All seven findings verified against the code and addressed (pushed through
🤖 Posted autonomously by Claude on behalf of pasta. |
lklimek
left a comment
lklimek
left a comment
There was a problem hiding this comment.
Please just get thepastaclaw approve it
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Final validation — Sol-only technical fallback
All seven previously verified findings are fixed at the exact head: configuration validation now precedes resource allocation, the regression test covers the failure path, retry semantics and full-node requirements are documented, and the backfill remediation is actionable. Focused normal and race-enabled tests pass, but the state-sync guide still incorrectly implies that every validator backend supplies the required Dash Core verifier.
Source: reviewer 1: gpt-5.6-sol (agent: sol-fallback-reviewer, role: general); reviewer 2: gpt-5.6-sol (agent: sol-fallback-reviewer, role: tenderdash-consensus-security); final verifier: gpt-5.6-sol (agent: sol-verifier, role: final-verifier)
One or more required Phase-1 GLM Flash lanes remained technically unusable after the bounded exact-model retry. Their evidence was discarded as authoritative, and the complete selected role cohort was rerun fresh on exact gpt-5.6-sol before this fresh Sol verifier produced the final decision. No additional Phase-2 reviewer pass ran.
Review provenance
- Phase 1 GLM evidence: technically unusable after bounded retry; discarded from the decision
- GLM failure attempts:
codex-general-ebd8b71d97b5483eac945c375c0e1e65(completed),codex-tenderdash-consensus-security-979f7f764c97475bbdfc1b76d35977d1(failed),codex-tenderdash-consensus-security-818ee98220604c67b10b684b48571516(failed) - Sol-only fallback reasons:
launch_transport_or_nonzero_exit - Sol-only fallback reviewers:
gpt-5.6-sol— general (completed); agentsol-fallback-reviewer,gpt-5.6-sol— tenderdash-consensus-security (completed); agentsol-fallback-reviewer - Fresh verifier (Sol):
gpt-5.6-sol— final-verifier; agentsol-verifier - Additional Phase 2 pass: not run; the Sol-only fallback is final
💬 1 nitpick(s)
🤖 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 `docs/nodes/state-sync.md`:
- [NITPICK] docs/nodes/state-sync.md:27-31: Do not imply every validator already has a Dash Core verifier
The final sentence is true only for validators using the Dash Core private-validator backend. In `makeNode`, `dashCoreRPCClient` is populated for a validator only when the private validator implements `privval.DashPrivValidator`; the supported local `FilePV` and TCP/gRPC signer clients do not. If one of those validators enables state sync in a multi-validator network, the state provider passes a nil verifier to `light.NewClient`, which returns `ErrNoDashCoreClient` and causes state-sync startup to fail. Narrow the statement so operators using another supported private-validator backend are not told that the required verifier is already available.
| Because light block verification goes through Dash Core, a full node **must** | ||
| be configured with a Dash Core RPC connection or it will refuse to start: set | ||
| `core-rpc-host` (and `core-rpc-username`/`core-rpc-password` as needed) in the | ||
| `[priv-validator]` section of `config.toml`. Validator nodes already have this | ||
| connection configured. |
There was a problem hiding this comment.
💬 Nitpick: Do not imply every validator already has a Dash Core verifier
The final sentence is true only for validators using the Dash Core private-validator backend. In makeNode, dashCoreRPCClient is populated for a validator only when the private validator implements privval.DashPrivValidator; the supported local FilePV and TCP/gRPC signer clients do not. If one of those validators enables state sync in a multi-validator network, the state provider passes a nil verifier to light.NewClient, which returns ErrNoDashCoreClient and causes state-sync startup to fail. Narrow the statement so operators using another supported private-validator backend are not told that the required verifier is already available.
| Because light block verification goes through Dash Core, a full node **must** | |
| be configured with a Dash Core RPC connection or it will refuse to start: set | |
| `core-rpc-host` (and `core-rpc-username`/`core-rpc-password` as needed) in the | |
| `[priv-validator]` section of `config.toml`. Validator nodes already have this | |
| connection configured. | |
| Because light block verification goes through Dash Core, a full node **must** | |
| be configured with a Dash Core RPC connection or it will refuse to start: set | |
| `core-rpc-host` (and `core-rpc-username`/`core-rpc-password` as needed) in the | |
| `[priv-validator]` section of `config.toml`. Validator nodes using the Dash Core | |
| private-validator backend reuse that connection. Other private-validator | |
| backends do not currently supply the Dash Core verifier required by state sync. |
source: ['claude']
3 participants
Issue being fixed or feature implemented
Polish pass on state sync ahead of enabling it for Dash Platform nodes: the operator docs no longer match the shipped implementation, non-validator full nodes crash with an unhelpful panic when
core-rpc-hostis unset, and backfill failures are swallowed with no operator guidance.What was done?
docs/nodes/state-sync.mdand fixeddocs/nodes/configuration.mdto match the actual config surface: removed thetrust-height/trust-hash/trust-periodkeys (deleted from the code in 33cda2d), fixed key names to their real dashed forms, documenteduse-p2p,retries(with block-sync fallback), the ≥2rpc-serversrequirement in RPC mode, and that Dash needs no trust anchors (light blocks are verified via Dash Core quorum signatures).spec/p2p/messages/state-sync.mdfrom the shipped protos: content-addressedchunk_id,next_chunks-driven fetching,COMPLETE_SNAPSHOTterminator,version+hashsnapshot identity (noformat/chunkscount).makeNode: a full node without[priv-validator] core-rpc-hostpreviously hit an eagerpanic("localPV must be set")viacore.NewMockClientwith a nil privval. It now returns a configuration error telling the operator exactly what to set. No working configuration changes behavior: the panic fired unconditionally for this mode/config combination, and all CI e2e manifests setprivval_protocol = "dashcore"on their full nodes.How Has This Been Tested?
gofmtclean;go build ./...clean;go test -short ./internal/statesync/... ./light/... ./node/...all pass.Breaking Changes
None. (
test/e2e/networks/ci.toml'sfull01still omitsprivval_protocol— it now gets the clear error instead of a panic; that manifest is not in the CI matrix.)Checklist:
🤖 Generated with Claude Code