Release 446 - #3077
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE VERY HIGH scrutiny (42-day-old account, one public repo), mitigated by repository write permission; no Gittensor association found; release-v446 → main. Static analysis of the complete release diff found no security vulnerability or malicious behavior. The runtime specification is bumped to 446, and no AI-review trust-boundary files or dependency manifests were changed. FindingsNo findings. ConclusionThe accounting migrations are mainnet- and generation-guarded, privileged calls retain appropriate origin checks, and the timelock, GRANDPA, consensus, and CI changes introduce no credible adverse path. 🔍 AI Review — Auditor (domain review)VERDICT: 👎 UNKNOWN Gittensor association; newer contributor with repository write permission, so maximum scrutiny applied. Duplicate workThis PR is the better candidate. Recommend closing #3073. This release incorporates the conviction/accounting changes with broader migration and behavioral test coverage. The PR description is substantive and Findings
Prior-comment reconciliation
ConclusionThe release remains blocked because a one-shot mainnet migration can silently clamp invalid counters to zero and permanently mark itself complete. Validate every affected counter before making any mutations. 📜 Previous run (superseded)
|
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
This reverts commit 2dadde2.
Decrypt errors were wiping TimelockEncrypted storage without writing RevealedCommitments, which silently dropped SDK UserData envelopes after the pulse arrived. Co-authored-by: Cursor <cursoragent@cursor.com>
Failed decrypts and rounds older than the oldest kept Drand pulse leave TimelockedIndex after one attempt, so on_initialize does not pair or emit forever. Co-authored-by: Cursor <cursoragent@cursor.com>
Fix alpha accounting and conviction terms
|
🔄 AI review updated — Skeptic: VULNERABLE |
…sus-mode Liquid Alpha 2.1 Consensus Modes
| T::AlphaAssets::rebase_alpha_counters( | ||
| netuid, | ||
| AlphaBalance::from(issuance), | ||
| AlphaBalance::from(burned), | ||
| AlphaBalance::from(recycled), | ||
| ); |
There was a problem hiding this comment.
[HIGH] Rebase silently accepts invalid live counters
rebase_alpha_counters uses saturating subtraction, but this migration never verifies that each live counter is at least its embedded offset. If the snapshot is stale or any live counter differs unexpectedly, the migration silently clamps it to zero, continues with the remaining rows, and later records HasMigrationRun, making the corruption permanent. The tests only construct counters as offset + remainder, so they do not cover this failure mode.
Before mutating, read and validate all affected counters (including recycled), and abort or skip without setting the run marker on any inconsistency. Add try-runtime pre/post checks proving that each post-state equals its exact pre-state minus the offset, plus a test where a counter is below its offset.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
Fix GRANDPA set ID handling
| AlphaBalance::from(issuance), | ||
| AlphaBalance::from(burned), | ||
| AlphaBalance::from(recycled), | ||
| ); |
There was a problem hiding this comment.
[HIGH] Rebase silently accepts invalid live counters
This helper uses saturating_sub without proving each live counter is at least its embedded offset. If a constant is wrong or live state differs, the migration clamps the counter to zero, continues applying corrections, and later sets HasMigrationRun, making the partial corruption permanent. Preflight every matching generation and all three counters before any mutation; on failure, change nothing and leave the marker unset. Add a live < offset regression test.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
…on-failure Keep timelock commitments when reveal fails
| T::AlphaAssets::rebase_alpha_counters( | ||
| netuid, | ||
| AlphaBalance::from(issuance), | ||
| AlphaBalance::from(burned), | ||
| AlphaBalance::from(recycled), | ||
| ); |
There was a problem hiding this comment.
[HIGH] Rebase silently accepts invalid live counters
rebase_alpha_counters applies each offset with saturating_sub. If any live counter is below its archived offset, this silently writes zero; the migration then records completion, making the corruption permanent. Preflight every matching generation and verify all three current counters are at least their offsets before mutating any entry. On validation failure, leave all counters and the migration marker untouched.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
| T::AlphaAssets::rebase_alpha_counters( | ||
| netuid, | ||
| AlphaBalance::from(issuance), | ||
| AlphaBalance::from(burned), | ||
| AlphaBalance::from(recycled), | ||
| ); |
There was a problem hiding this comment.
[HIGH] Rebase silently accepts invalid live counters
This still passes offsets directly to rebase_alpha_counters, whose saturating_sub operations silently clamp any live counter below its expected historical offset to zero. The migration then records completion, making corrupted results permanent; processing rows sequentially can also leave a partial rebase if validation is added only inside this loop. Preflight-read and validate all three counters for every generation-matching row before performing any mutation, and do not set HasMigrationRun when validation fails. Add a test where one counter is below its offset and assert that no rows are changed and the migration remains retryable.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
| T::AlphaAssets::rebase_alpha_counters( | ||
| netuid, | ||
| AlphaBalance::from(issuance), | ||
| AlphaBalance::from(burned), | ||
| AlphaBalance::from(recycled), | ||
| ); |
There was a problem hiding this comment.
[HIGH] Rebase silently accepts invalid live counters
rebase_alpha_counters uses saturating subtraction. If any live issuance, burned, or recycled counter is below its hard-coded offset, this silently clamps that counter to zero; the migration then continues and records itself complete. Because this is a one-shot mainnet accounting migration, preflight every affected generation and counter before performing any writes, and abort without setting HasMigrationRun when an offset exceeds the live value. Add a regression test covering an undersized counter.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
Release 446
Runtime spec 446 introduces corrected alpha accounting, a refined conviction ownership quorum, configurable Liquid Alpha consensus modes, auditable timelock reveal failures, and a GRANDPA warp-sync fix.
Alpha accounting and conviction ownership
SubnetAlphaOutundercounts and backfills alpha burned beforeAlphaBurnedtracking began.SubnetAlphaOut - SubnetProtocolAlpha - AlphaBurned, using saturating subtraction.Liquid Alpha consensus modes
LiquidAlphaConsensusModewithCurrent,Previous, and defaultAutomodes.Currentuses consensus calculated in the current epoch.Previoususes persisted consensus from the prior epoch, falling back to current consensus when none is stored.Autouses previous consensus whenbonds_penalty == u16::MAXand current consensus otherwise.Timelock commitment recovery
UserDataenvelope for timelocked commitments.CommitmentRevealFailedonce, retains the failed field inCommitmentOffor audit, and removes it from the retry index.TimelockRevealFailedfields; only the runtime can create this state.Timelocked.encryptedfor retrieving the pallet-native inner ciphertext.GRANDPA warp sync
Runtime
spec_versionto446.