Remove empty and obsolete storage entries - #3061
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE VERY HIGH scrutiny: recent account with repository write access and substantive merged contributions; final commit is authored by a different contributor; no Gittensor association found; branch correctly targets main. Findings
Prior-comment reconciliation
ConclusionThe Subtensor sweep is bounded, but the Swap cleanup still performs unbounded storage deletion and iteration synchronously during runtime upgrade, risking a block-budget overrun. 📜 Previous run (superseded)
🔍 AI Review — Auditor (domain review)VERDICT: 👎 Gittensor association UNKNOWN by the trusted allowlists; the author is a recent account with repository write access and substantial prior Subtensor activity. PR body was empty/trivial; the Auditor has auto-filled it. Please review. The Subtensor cleanup is resumable and tested, and the runtime No auto-fixes were applicable. Findings
ConclusionThe unbounded Swap migration can overrun the runtime-upgrade block budget. It must be converted to a cursor-based multi-block cleanup before merge. |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| "AlphaSqrtPrice", | ||
| "CurrentTick", | ||
| "EnabledUserLiquidity", | ||
| "FeeGlobalTao", | ||
| "FeeGlobalAlpha", | ||
| "LastPositionId", | ||
| "ScrapReservoirTao", | ||
| "ScrapReservoirAlpha", | ||
| "Ticks", | ||
| "TickIndexBitmapWords", | ||
| "SwapV3Initialized", | ||
| "CurrentLiquidity", | ||
| "Positions", | ||
| ] { | ||
| remove_prefix::<T>("Swap", storage_name, &mut weight); |
There was a problem hiding this comment.
[HIGH] Bound the Swap cleanup across blocks
Each listed prefix is cleared with an effectively unlimited u32::MAX limit, and the same upgrade also fully iterates both reservoir maps. All of this executes synchronously in on_runtime_upgrade; the stated current row count is not an enforced bound, so excessive state can overrun the upgrade block's execution budget and prevent reliable runtime activation. Convert this cleanup to a cursor-based, weight-limited multi-block migration like the Subtensor sweep.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| for storage_name in [ | ||
| "AlphaSqrtPrice", | ||
| "CurrentTick", | ||
| "EnabledUserLiquidity", | ||
| "FeeGlobalTao", | ||
| "FeeGlobalAlpha", | ||
| "LastPositionId", | ||
| "ScrapReservoirTao", | ||
| "ScrapReservoirAlpha", | ||
| "Ticks", | ||
| "TickIndexBitmapWords", | ||
| "SwapV3Initialized", | ||
| "CurrentLiquidity", | ||
| "Positions", | ||
| ] { | ||
| remove_prefix::<T>("Swap", storage_name, &mut weight); |
There was a problem hiding this comment.
[HIGH] Bound the Swap cleanup across blocks
This invokes clear_prefix(..., Some(u32::MAX)) for 13 prefixes during on_runtime_upgrade, followed by full reservoir-map iterations. The asserted current row count does not enforce a runtime bound, and the migration can exceed the upgrade block budget as state grows or differs from the snapshot. Use a persistent cursor and consume bounded on_idle weight, as the Subtensor cleanup does; mark completion only after every prefix and reservoir map is exhausted.
|
🔄 AI review updated — Skeptic: VULNERABLE |
2 participants
Motivation
Remove obsolete and explicit-default storage entries that unnecessarily increase chain-state size, while preserving all live nonzero state.
Changes
on_idleafter the beta-basket migration completes.spec_versionfrom 443 to 444.Files of interest
pallets/subtensor/src/migrations/migrate_storage_bloat_v2.rspallets/subtensor/src/macros/hooks.rspallets/swap/src/pallet/migrations/migrate_storage_cleanup_v2.rspallets/swap/src/pallet/hooks.rsruntime/src/lib.rsBehavioral and migration impact
The migration removes only abandoned prefixes or entries whose encoded value represents the storage default. Nonzero Subtensor stake/share state and nonzero Swap reservoir balances are preserved. The Subtensor sweep is spread across idle block capacity and records completion in
HasMigrationRun.Testing
Added tests covering bounded/resumable Subtensor cleanup, preservation of nonzero state, migration completion guards, preservation of root stake age, Swap legacy-prefix removal, and preservation of pending nonzero reservoir balances.