fix: support trusted warp sync checkpoints - #3113
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE VERY HIGH scrutiny: <90-day account with zero public repos, mitigated by repository admin access, substantial contributions, matching author/committer, and no known Gittensor association; fix/finney-light-sync-checkpoint -> release-v450. Static analysis only; no PR-controlled code was executed. The previously identified automatic remote checkpoint-refresh path remains absent, and checkpoint changes are explicit chain-spec diffs. FindingsNo findings. ConclusionNo malicious behavior or security vulnerability was found in the current diff. The historical checkpoint is treated as an explicit chain-spec trust anchor while subsequent GRANDPA transitions remain proof-verified. 🔍 AI Review — Auditor (domain review)VERDICT: 👍 Established repository administrator with substantial prior contributions; gittensor association UNKNOWN. Overlapping PR #2989 is unrelated. The trusted checkpoint implementation, transition validation, populated Finney data, generation RPC, and documented rollout evidence are internally consistent. Quick checks passed: FindingsNo findings. ConclusionNo substantive correctness or domain issues remain. The PR is ready from the Auditor perspective. |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
|
🔄 AI review updated — Skeptic: SAFE |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
| curl "${curl_args[@]}" "$rpc_url" >"$rpc_response" | ||
|
|
||
| if jq -e '.error != null' "$rpc_response" >/dev/null; then | ||
| jq -r '"grandpa_genWarpSyncSpec failed: " + (.error | tostring)' "$rpc_response" >&2 |
There was a problem hiding this comment.
[MEDIUM] Private RPC credentials are exposed in process arguments
Both $rpc_url and the optional --user user:password entry are expanded into curl's argv. On hosts where process arguments are visible to other users or monitoring agents, credentials can be recovered while this potentially five-minute request runs. Supply authentication through a permission-restricted curl config/netrc file, and reject secret-bearing URL components.
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
4b19dc3 to
431019a
Compare
|
|
||
| if ! jq -e --arg expected_genesis_hash "$expected_genesis_hash" \ | ||
| '.result.genesisHash == $expected_genesis_hash' "$rpc_response" >/dev/null; then |
There was a problem hiding this comment.
[HIGH] Remote endpoint self-authenticates the consensus checkpoint
The genesis hash is supplied by the same RPC response as the checkpoint, so this comparison proves only that the endpoint echoed the expected constant. A compromised or misconfigured endpoint can return that hash alongside an attacker-selected header and authority set; the script then writes those values into both chain specs as a trusted GRANDPA hard fork, allowing proofs rooted in attacker-controlled authorities to be accepted. Authenticate the checkpoint independently—for example, require an operator-provided expected transition block hash and authority-set digest, or cryptographically verify the transition and justification from an existing trusted anchor before replacing the specs.
|
🔄 AI review updated — Skeptic: VULNERABLE |
90848c5 to
dd61625
Compare
431019a to
a9608b1
Compare
| if ! jq -e --arg expected_genesis_hash "$expected_genesis_hash" \ | ||
| '.result.genesisHash == $expected_genesis_hash' "$rpc_response" >/dev/null; then |
There was a problem hiding this comment.
[HIGH] Remote endpoint self-authenticates the consensus checkpoint
Matching the endpoint-reported genesis hash proves only that the response claims the expected chain; it does not authenticate the returned historical header or authority set. Because this same response is then written as a trusted GRANDPA hard fork, a compromised or misconfigured endpoint can supply attacker-controlled authorities. Verify the checkpoint against an independent immutable trust source (or cryptographically validate its ancestry and GRANDPA justification from an already trusted anchor) before updating either chain spec.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| if ! jq -e --arg expected_genesis_hash "$expected_genesis_hash" \ | ||
| '.result.genesisHash == $expected_genesis_hash' "$rpc_response" >/dev/null; then |
There was a problem hiding this comment.
[HIGH] Remote endpoint self-authenticates the consensus checkpoint
This compares the expected genesis hash with a value supplied by the same endpoint that supplies the checkpoint. The newly added server-side checks establish only internal consistency: a hostile endpoint can report the expected genesis hash while returning a transition header and authority set from an attacker-controlled fork. Because these values become a trusted GRANDPA hard fork, authenticate the checkpoint independently—for example, require a separately obtained expected transition hash and authority-set digest before updating either chain spec.
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Summary
grandpaWarpSyncCheckpointchain-spec extension containing a historical GRANDPA transition header and the authority set that signed itgrandpa_genWarpSyncCheckpointresponse behind the opt-in--enable-warp-sync-checkpoint-rpcflaggrandpaWarpSyncCheckpointwhenever the raw and plain chain specs are rebuiltAddresses #3112.
Trust and safety
Rollout
The initial Finney checkpoint is populated from retained public archive history at block 8,867,448 (set ID 5, the signing set immediately before set ID 6). The public archive retains the transition header, GRANDPA justification, and historical parent state required by the generator. No automatic refresh path is included: a future checkpoint change must be an explicit, independently verified chain-spec diff. Normal scheduled authority rotations do not require such a refresh.
The end-to-end Finney regression is complete:
--sync=warp, then imported the current state and caught up through normal block sync.isSyncing: false, and finalized new blocks normally.The PR remains a draft intentionally after completing this validation.
The checkpoint is a historical proof-verification anchor, not a state-download target. Normal scheduled authority rotations do not invalidate it and do not require release-by-release refreshes or a hardcoded current set ID.
Proof generation is unchanged by this PR: the client still requests the normal proof sequence from genesis and uses the checkpoint when that proof reaches the trusted transition. Serving peers therefore do not need this PR image, but they do need the Finney GRANDPA set-ID fix first released in v446 and retained transition data. Until compatible provider adoption is sufficient, operators should use at least one known-compatible v446-or-newer provider.
Validation
cargo fmt --check --allbash -n scripts/build_all_chainspecs.shgit diff --check origin/release-v450...HEADSKIP_WASM_BUILD=1 cargo check -p node-subtensor --libSKIP_WASM_BUILD=1 cargo test -p node-subtensor service:: --lib--sync=warpregression against a known-compatible provider, including current-state import, GRANDPA set ID 6, and advancing finalityThe ordinary local compile was blocked by this Mac's Clang lacking the
wasm32-unknown-unknownC target; native node compilation and the targeted tests passed with runtime WASM generation skipped. CI builds the full runtime in its configured environment.