Fix conviction aggregate roll-forward- #3060 - #3073
Conversation
… challengers to independently meet the 10% conviction threshold.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
| weight = weight.saturating_add(T::DbWeight::get().reads(3)); | ||
|
|
||
| // Collect before rewriting Lock so mutation cannot disturb the iterator. | ||
| let locks: Vec<_> = Lock::<T>::iter().collect(); |
There was a problem hiding this comment.
[HIGH] Runtime upgrade materializes every lock without a hard bound
on_runtime_upgrade collects the entire permissionlessly growable Lock map into WASM memory, then clears several maps and rewrites every retained row in the same upgrade block. The archive snapshot is not a protocol bound and state can grow before deployment; returning the consumed weight only after execution cannot prevent an overweight or memory-exhausting upgrade. Stage this migration with a cursor and per-block limit, or enforce and validate a hard storage bound before performing the rebuild.
| /// complete member-scaled work instead of adding permanent storage bookkeeping. | ||
| pub fn owner_transition_member_count(netuid: NetUid, new_owner_hotkey: &T::AccountId) -> u32 { | ||
| let old_owner_hotkey = SubnetOwnerHotkey::<T>::get(netuid); | ||
| let old_owner_members = LockingColdkeys::<T>::iter_prefix((netuid, &old_owner_hotkey)) |
There was a problem hiding this comment.
[HIGH] Ownership transitions scan an unbounded member index
This unbounded prefix scan is evaluated while determining dispatch weight, and the transition subsequently collects and rewrites every indexed member. LockingColdkeys has no protocol-level per-hotkey bound, so historical observations do not constrain adversarial state growth. Automatic ownership changes also reach the same work from the block hook. Dynamic weight accounting does not stop execution once the block limit is exceeded; introduce a maintained bound or a staged transition with bounded work per block.
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE VERY HIGH scrutiny by account-age/repository tier; author has write access, no Gittensor association was found, and the PR targets feature branch fix/restore-miner-burn-scaling. The aggregate-accounting changes introduce unbounded runtime work in both the upgrade and recurring ownership-transition paths. Returned weight accounts for completed work but does not impose an execution bound. Findings
ConclusionThe PR is legitimate-looking, but the unbounded storage scans create credible chain-liveness risks and must be bounded or staged before merge. # 🔍 AI Review — Auditor (domain review) has not yet run on this PR. |
|
🔄 AI review updated — Skeptic: VULNERABLE |
Summary
Fix conviction-lock aggregate accounting, repair state corrupted by the v443 roll-forward semantics, make ownership transitions update canonical individual locks, and require takeover challengers to independently satisfy the 10% conviction threshold.
Motivation
Runtime v443 could roll one individual lock forward, apply only that member’s delta to its aggregate, and then advance the aggregate’s
last_update.This broke the aggregate invariant in two ways:
Because corrupted aggregates do not identify which member contributions were represented at their stored timestamp, they cannot be safely repaired incrementally.
Ownership changes exposed a related issue. Promoting only an aggregate to owner conviction could leave an orphaned owner boost after the hotkey was demoted and an individual member was later updated.
The takeover gate also used subnet-wide conviction while selecting the winner by individual hotkey, allowing unrelated hotkeys to contribute toward a challenger’s quorum.
Changes
Conviction model
Aggregate accounting
cleanup_lock_if_zeroso staking and unstaking operations mature the entire aggregate, including sibling contributions.Lock-class changes
Ownership transitions
Ownership-transition weights
Takeover rule
Require the winning hotkey’s own rolled conviction to satisfy:
Unrelated hotkeys and the incumbent can no longer supply the challenger’s required quorum. Locks backing the challenger’s hotkey continue to count normally.
Runtime migration
Add
migrate_rebuild_conviction_aggregates, guarded byHasMigrationRun.The migration:
LockingColdkeys.A mainnet archive scan at block 8,793,919 found:
This makes the one-time upgrade scan small in the currently deployed state.
Runtime integration
Update affected precompile code for the new roll-forward return type.
Regenerate affected pallet weights.
Bump the runtime specification version from 444 to 445.
Behavioral impact
Testing
Added or updated regression coverage for:
Validation included pallet tests, precompile compilation, affected runtime-benchmark tests, strict Clippy, and git diff --check.