Skip to content

Fix conviction aggregate roll-forward- #3060 - #3073

Open
UnArbosFour wants to merge 8 commits into
fix/restore-miner-burn-scalingfrom
fix/zero-lock-aggregates
Open

Fix conviction aggregate roll-forward- #3060#3073
UnArbosFour wants to merge 8 commits into
fix/restore-miner-burn-scalingfrom
fix/zero-lock-aggregates

Conversation

@UnArbosFour

Copy link
Copy Markdown
Contributor

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:

  • Untouched sibling locks did not mature before the aggregate timestamp advanced.
  • A later update to an older individual lock could count its maturation again.

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

  • Introduce concrete lock-state types for the four lock classes:
    • Perpetual general
    • Decaying general
    • Perpetual owner
    • Decaying owner
  • Scope each ConvictionModel to one individual lock and its corresponding aggregate bucket.
  • Move class-specific roll-forward and merge behavior onto the concrete lock-state types.
  • Remove the obsolete RollDelta implementation.
  • Keep read-only lock queries pure.
  • Centralize individual dust collection in ConvictionModel.
  • Log and leave state unchanged when incompatible lock classes are merged.

Aggregate accounting

  • Roll both the individual lock and the complete selected aggregate to the current block before mutation.
  • Stop advancing an aggregate timestamp after applying only one member’s delta.
  • Prevent a subsequently touched member from contributing already-counted maturation again.
  • Update cleanup_lock_if_zero so staking and unstaking operations mature the entire aggregate, including sibling contributions.
  • Preserve aggregate consistency when adding, reducing, moving, or transferring locked mass.
  • Re-read destination models after saving source models so shared aggregate buckets cannot be overwritten with stale state.
  • Merge with pre-existing destination locks instead of replacing them.
  • Remove complete evolved contributions when move or transfer operations collect individual dust.

Lock-class changes

  • Rework perpetual/decaying changes as explicit source-to-destination class transitions.
  • Roll the source model to the current block, remove its individual contribution from the old aggregate, and merge it into the destination aggregate.
  • Apply the same model when changing owner/non-owner classification.

Ownership transitions

  • Reclassify every indexed lock belonging to both the outgoing and incoming owner hotkeys.
  • Roll outgoing members to the transition block under their previous owner role before demoting them.
  • Promote incoming members to full owner conviction at the transition block.
  • Keep owner conviction canonical on both individual rows and aggregate buckets, preventing ghost conviction after later demotion or member updates.
  • Apply canonical ownership transitions to:
    • Automatic conviction-based takeover
    • Lease termination
    • Administrative owner-hotkey changes

Ownership-transition weights

  • Add member-scaled weights for ownership-transition work.
  • Charge ownership-transition count lookups as member_count + 3 database reads.
  • Charge lease ownership-transition lookups as member_count + 4 reads.
  • Calculate each dispatch’s member count once and reuse it for both lookup and transition weights.
  • Keep the transition scan intentionally unbounded. Ownership changes are expected only a few times per year, and the mainnet scan found at most 44 lock rows on any subnet.

Takeover rule

Require the winning hotkey’s own rolled conviction to satisfy:

10 × challenger conviction >= subnet alpha out

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 by HasMigrationRun.

The migration:

  • Runs after the subnet-hotkey-swap repair migration.
  • Reads canonical individual Lock rows.
  • Rolls each individual to the upgrade block using its current lock mode and owner role.
  • Removes locks that have decayed to dust.
  • Reconstructs LockingColdkeys.
  • Clears all four existing aggregate maps.
  • Rebuilds aggregate state exclusively from retained individual rows.
  • Preserves earned individual conviction while discarding corrupted aggregate accounting.

A mainnet archive scan at block 8,793,919 found:

  • 352 individual lock rows
  • 352 matching LockingColdkeys rows
  • 193 aggregate rows
  • 125 subnets containing locks
  • At most 44 lock rows on one subnet

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

  • Touching one member evolves sibling contributions before advancing the aggregate timestamp.
  • Lock top-ups no longer count previously matured conviction twice.
  • Existing v444 aggregate corruption is rebuilt from canonical individual state.
  • Move and transfer operations no longer leave orphaned dust contributions.
  • Former owners retain conviction earned during their ownership period without retaining an orphaned owner boost.
  • Challengers must independently hold 10% conviction before taking over a subnet.
  • Ownership-transition lookup and execution costs scale with the number of affected lock members.

Testing

Added or updated regression coverage for:

  • Sibling maturity during cleanup_lock_if_zero
  • Double-count prevention after aggregate-only roll-forward
  • Decaying sibling dust cleanup
  • Move and transfer dust removal
  • Pre-existing destination-lock merging
  • Pure read APIs
  • Lock-class changes
  • Corrupted aggregate reconstruction
  • Migration idempotence
  • Migration dust and orphan-index cleanup
  • Real runtime-upgrade ordering
  • SN72 former-owner state
  • Challenger-specific 10% takeover gating
  • Unrelated conviction exclusion
  • Owner → non-owner → member-update ghost conviction
  • Decaying-owner demotion under the previous role
  • Automatic, lease, and administrative owner transitions
  • Member-scaled ownership-transition lookup weights
  • Ownership-transition runtime benchmarks

Validation included pallet tests, precompile compilation, affected runtime-benchmark tests, strict Clippy, and git diff --check.

@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
subtensor Ready Ready Preview Aug 11, 2026 7:31pm

Request Review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

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();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions

Copy link
Copy Markdown
Contributor

🛡️ 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

Sev File Finding
HIGH pallets/subtensor/src/migrations/migrate_rebuild_conviction_aggregates.rs:75 Runtime upgrade materializes every lock without a hard bound inline
HIGH pallets/subtensor/src/staking/lock.rs:623 Ownership transitions scan an unbounded member index inline

Conclusion

The 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.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

This was referenced Aug 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants