fix(testnet): correct GRANDPA warp checkpoint transition - #3082
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE VERY HIGH scrutiny by account-age/public-repository signals, mitigated by repository-admin status and substantial subtensor history; no Gittensor association; author and committer match; release-448 integration branch targets main via PR #3089. Static analysis found the change confined to genesis-gated testnet GRANDPA checkpoint data and regression tests. Repository history and the stored justification corroborate the historical six-authority set and signing set ID 3. No runtime, dependency, CI, or AI-review trust-boundary files changed. FindingsNo findings. ConclusionNo malicious behavior or security vulnerability was found in the supplied diff. 🔍 AI Review — Auditor (domain review)VERDICT: 👍 UNKNOWN Gittensor association; newer account but repository admin with substantial merged subtensor history. Full consensus-path scrutiny applied. The historical six-authority checkpoint configuration remains scoped to the exact testnet genesis. The regression fixture verifies the second checkpoint with signing set ID 3 and rejects ID 2, while the checkpoint header supplies the subsequent authority transition through the existing SDK behavior. PR #2989 merely overlaps this file and is not a competing implementation. FindingsNo findings. ConclusionThe corrected checkpoint metadata, focused regression coverage, and documented live warp-sync validation adequately cover the consensus transition. The PR is ready to merge. |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
| ), | ||
| ( | ||
| 2, | ||
| 3, |
There was a problem hiding this comment.
[HIGH] Restore the checkpoint’s distinct resulting set ID
This value correctly identifies set 3 as the historical signing set, but AuthoritySetHardFork provides no separate resulting-set override here. The final commit removed resulting_set_id: Some(2) and both assertions that protected it, despite the PR description requiring the second checkpoint to leave warp sync at set ID 2. Consequently, successful verification can resume from the wrong GRANDPA set and break testnet finality synchronization. Restore an SDK/API capable of representing signing set 3 with resulting set 2, and retain an integration-level assertion for the returned VerificationResult.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
18833a3 to
fa661f6
Compare
eco-tests changed — indexer review requiredThis PR modifies files under Changed files
|
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Summary
Scope and safety
The checkpoint configuration is gated by the exact testnet genesis hash. Finney continues through the existing one-time initial-set-ID path inherited from
release-448. This PR changes no runtime code, chain state, consensus rules, or network protocol format.Verification
SKIP_WASM_BUILD=1 cargo test -p node-subtensor service::grandpa_warp_sync::tests --bin node-subtensor(4 passed after rebasing ontorelease-448).git range-diffconfirms the four PR patches are unchanged by the rebase.release-448already contains that fix.Earlier end-to-end validation of the same four patches also completed state import, persisted the warp-verified GRANDPA authority set, resumed finality after restart, and advanced best/finalized heads.