Skip to content

release-root-reborn - #2968

Open
unarbos wants to merge 57 commits into
mainfrom
release-v438
Open

release-root-reborn#2968
unarbos wants to merge 57 commits into
mainfrom
release-v438

Conversation

@unarbos

@unarbos unarbos commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

Summary

Runtime release 438. Brings the Root Reborn feature branch (#2759, root-reborn) up to date with current main and bumps spec_version to 441.

Root Reborn (beta baskets)

  • Root dividends are reinvested into per-validator beta baskets: a single fund per validator, held under a global escrow coldkey, distributed per the validator's root weight vector (set_root_weights, stored in Weights[ROOT]).
  • Deposits mint fund shares at pre-deposit NAV; stakers accrue claimable shares via a per-validator rate accumulator. claim_root redeems pro-rata across every holding into root TAO.
  • Retires set_root_claim_type (call index 122) and sudo_set_num_root_claims (123) plus the auto-claim scheduler. RootClaimable/RootClaimed remain as legacy storage, drained by the new migrate_seed_beta_basket_v2 migration.
  • New BetaBasketRuntimeApi (basket NAV, holdings, owed TAO, validator weights views).

Rank-pinned emission gate bar (#3014)

  • Adds EmissionBarRank (default 64): pins theta to the Nth-largest demand share instead of the q-mass quantile.
  • New root extrinsic sudo_set_emission_bar_rank; N=0 falls back to q-mass.
  • One-shot migration migrate_reset_emission_gate_bar kills the stale quantile bar so rank-64 applies immediately at upgrade.
  • Proxy group + Python SDK bindings updated.

Key merge decisions (root-reborn was based on spec-418 main)

  • set_root_weights moved from call index 139 (taken on main) to 146.
  • Coinbase: main's miner-collateral settlement is kept; the claimable remainder now flows to distribute_root_alpha_to_basket instead of the removed claim-root ledger.
  • Dissolution: basket holdings on a dissolving subnet are converted to the fund's root slot at queue time in do_dissolve_network.
  • Hotkey-swap root-cleanliness gate now checks BasketRate/BasketShares and residual legacy claim state.
  • Proxy filters: root-reborn's removal of set_root_claim_type ported into main's refactored proxy_filters/ call groups; set_root_weights added to SubtensorCommonCalls.

Also included

  • srtool builder image: retry the whole build up to 5 times with linear backoff on transient CI egress failures.

Test plan

  • CI: wasm runtime build, try-runtime, zombienet e2e (02.0x claim-root suites rewritten for baskets)

unconst and others added 11 commits June 15, 2026 15:30
Replace the per-block auto-sell of root dividends with a compounding,
redeemable beta basket. Root validators set a distribution vector over
subnets via `set_root_weights`; each validator's root dividends are sold
to TAO and re-bought as alpha across those subnets, staked under a global
escrow coldkey (so the basket counts toward the validator's stake and
compounds), and redeemed to TAO on demand through the existing claim path
using an E/P growth multiplier. Auto-claim/auto-sell removed. Adds a
dedicated `RootBasketWeights` map, `BasketPrincipal` accounting, hotkey-swap
and subnet-dissolve handling, a legacy-state seed migration, and RPC views
(staker pending TAO, validator NAV + basket, network-wide NAV).

Co-authored-by: Cursor <cursoragent@cursor.com>
Revive the existing root-weights plumbing: store the basket vector under
Weights[ROOT][uid] (uid-keyed, so it follows the validator through hotkey
swaps automatically and reuses existing weight terms/limits) rather than a
separate RootBasketWeights map. Keep the dedicated `set_root_weights`
extrinsic since the generic set_weights rejects netuid 0 and root needs
different checks. Retain the dust-recycle fix (Σ owed == BasketPrincipal).

Co-authored-by: Cursor <cursoragent@cursor.com>
On subnet dissolve, liquidate_basket_to_root_stakers previously credited the
swapped basket value to the validator's current root nominators in proportion
to their *current root stake* (increase_stake_for_hotkey_on_subnet), ignoring
the per-coldkey owed entitlement. That windfalls recent/large-current-stake
nominators and short-changes stakers who actually accrued the basket, then
wipes the ledger — a provable intra-staker fairness bug.

Now: swap the whole basket once, then distribute the realized TAO pro-rata by
each staker's owed (rate*root_stake - claimed == owed*E/P), crediting each
coldkey individually and rebasing its claimed watermark (mirrors a normal
claim). Degenerate zero-owed case falls back to stake-proportional so value is
never orphaned. Adds a regression test proving a zero-owed fresh staker
receives nothing while the accruing staker receives the basket.

Co-authored-by: Cursor <cursoragent@cursor.com>
Mint basket principal *shares* at the live escrow NAV (E/P) instead of at
par. A deposit into an already-compounded basket now mints fewer shares
than the alpha bought, leaving E/P unchanged: existing holders are not
diluted and a late staker cannot skim past compounding. Makes the
staker-facing guarantee strict — a new staker only ever earns their fair
share of distributions from the point they join forward.

Adds tests proving claims 1-4 (principal never lost; accrued beta
unchanged by others staking; beta compounds; no dilution/skim on late
stake, incl. E/P invariance across a deposit).

Also includes the dissolve liquidation distributing pro-rata by owed
entitlement rather than current root share.

Co-authored-by: Cursor <cursoragent@cursor.com>
Add observability for off-chain indexing / future tokenization cost-basis:
- Events: BasketDeposited (alpha bought + shares minted at NAV, per
  validator/subnet), BasketClaimed (TAO realized by a staker), and
  BasketLiquidated (TAO returned to root stakers on subnet dissolve).
- RPC/runtime-API: betaBasket_getValidatorWeights returns a validator's
  basket weight vector (its curation strategy) so dashboards can display it.

Co-authored-by: Cursor <cursoragent@cursor.com>
… claim type

- Record protocol outflow on the origin sell and inflow on each
  redistribution buy in distribute_root_alpha_to_basket, so a deposit->claim
  round-trip nets ~0 on the dest pools (symmetric with the claim/liquidation
  outflow that was already recorded). Records sit inside with_transaction so
  they roll back with the swaps.
- Exclude the beta-escrow coldkey from clear_small_nomination_if_required:
  basket positions are not nominations, and sweeping one stranded TAO in the
  keyless escrow account while leaving BasketPrincipal untouched (breaking
  Sum(owed) == BasketPrincipal and zeroing every staker's owed * E/P payout).
- Deprecate the claim-type surface: set_root_claim_type now rejects the no-op
  Keep/KeepSubnets variants (new RootClaimTypeNotSupported error); fixed the
  false "(Keep was removed)" comment and documented the variants as no-ops.
- Remove dead auto-claim machinery (run_auto_claim_root_divs,
  block_hash_to_indices, block_hash_to_indices_weight) and its false-coverage
  test; update affected tests and benchmarks.

Co-authored-by: Cursor <cursoragent@cursor.com>
A validator can now weight root (uid 0) in its basket vector to opt out of
subnet exposure: that slice is held as root stake (TAO at 1:1) under the
escrow instead of being swapped into subnet alpha, and it compounds and is
claimable through the same E/P machinery as the alpha slots.

Root has no AMM pool, so the swap is elided (valuation is already 1:1) and
the reserve bookkeeping is mirrored directly; the escrow custody account is
excluded from the claimant base and from dissolution payouts since it is not
a claimant. Adds 4 tests covering deposit, claim (reassign, no swap),
compounding, and the escrow-denominator exclusion.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ttlement

Align the set_root_weights producer with the basket consumer so a validator
can actually populate a root (uid 0) slot on-chain (previously the extrinsic
rejected root, leaving the new path unreachable in production).

Code-quality cleanups: extract credit_root_reserves (the SubnetTAO/
SubnetAlphaOut/TotalStake triple, previously hand-mirrored in 3 places) and
hoist the shared post-claim watermark advance so root and subnet claims share
one settlement tail instead of duplicating it.

Co-authored-by: Cursor <cursoragent@cursor.com>
…ble-ready)

Restructure the basket's unit of account so entitlements are shares of a
single fund per validator, never claims on a specific subnet's alpha. This
decouples what stakers are owed (fund shares) from what the fund holds
(escrow positions), which is the prerequisite for validator-directed
rebalancing and share tokenization later: holdings can change without
touching any staker's claim.

How it works now:

* Storage: BasketShares(hot) = outstanding fund shares P (TAO-denominated);
  BasketRate(hot) = single shares-per-root-stake accumulator; and
  BasketClaimed(hot, cold) = signed i128 claimed watermark. The watermark is
  signed on purpose: stake-change rebasing (claimed +/- rate * delta) must be
  exact in both directions or unstake-before-claim forfeits accrued
  entitlement (the old unsigned per-subnet RootClaimed had this bug).
* Deposit: each root dividend is sold for TAO and deployed across subnets
  per the validator's Weights[ROOT] vector into the keyless escrow. Fund NAV
  N is snapshotted after the origin sell (the fund may hold origin-subnet
  alpha) and shares = tao_deployed * P / N mint at the pre-deposit NAV, so
  existing holders are never diluted and late deposits cannot skim past
  compounding. Mint/payout math is u128 (U96F32 saturates at chain scale).
  Dust deposits (rate increment below I96F32 resolution) roll back and
  recycle so sum(owed) == P is never broken.
* Claim (claim_root, now arg-less): fund-level pro-rata redemption. Owed
  shares define fraction f = owed / P; exactly f of every holding is
  redeemed (subnet alpha sold to TAO, the root cash slot reassigned without
  a swap) and staked on root. Composition is preserved by every claim. A
  claim that realizes zero TAO (all alpha takes floor to zero) rolls back
  rather than burning shares. Only the ROOT RootClaimableThreshold entry
  gates dust claims; the sudo setter rejects other netuids.
* Dissolution: a dying subnet's holdings convert into each fund's root
  (TAO) slot. NAV is continuous, entitlements untouched; the old
  liquidate-to-stakers machinery is deleted.
* Key swaps: root hotkey swaps move the whole fund (shares, rate,
  watermarks, holdings) by value; coldkey swaps carry the watermark even at
  zero live root stake (negative watermark = owed with no stake).
* Migration is migrate_seed_beta_basket_v2 under a fresh key: the v1 name
  was already consumed on chains that ran the abandoned per-slot seed, which
  would have silently skipped conversion and stranded every basket. v2
  converts legacy per-subnet state at fixed moving prices (spot fallback),
  preserving each staker's owed TAO value exactly (sum(owed) == P), tolerates
  pre-existing v1 escrow/root-slot state without double-staking or minting
  unbacked shares, and clears orphaned BasketPrincipal entries.
* Retired dead surface: set_root_claim_type (call 122), sudo_set_num_root_
  claims (call 123), RootClaimType/RootClaimTypeEnum/NumRootClaim storage,
  and their events/errors/benchmarks/ts-tests. Redemption is always a full
  swap to root TAO; there is no auto-claim scheduler.

Covered by 50+ basket/migration tests including conservation under
interleaved deposits/claims, chain-scale magnitudes, self-referential
origin deposits, v1-already-ran migration state, coldkey-swap
entitlement carry, and zero-realized-claim no-ops.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

# Conflicts:
#	pallets/subtensor/src/benchmarks/benchmarks.rs
#	pallets/subtensor/src/coinbase/root.rs
#	pallets/subtensor/src/coinbase/run_coinbase.rs
#	pallets/subtensor/src/lib.rs
#	pallets/subtensor/src/macros/dispatches.rs
#	pallets/subtensor/src/macros/hooks.rs
#	pallets/subtensor/src/staking/claim_root.rs
#	pallets/subtensor/src/subnets/weights.rs
#	pallets/subtensor/src/swap/swap_hotkey.rs
#	pallets/subtensor/src/tests/claim_root.rs
#	pallets/subtensor/src/tests/migration.rs
#	runtime/src/lib.rs
#	ts-tests/suites/zombienet_staking/02.04-claim-root-hotkey-swap.test.ts
…ures.

Co-authored-by: Cursor <cursoragent@cursor.com>
@vercel

vercel Bot commented Jul 23, 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 Jul 29, 2026 10:55pm

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.

Comment on lines +93 to +144
let hotkeys: Vec<T::AccountId> = RootClaimable::<T>::iter_keys().collect();
weight.saturating_accrue(T::DbWeight::get().reads(hotkeys.len() as u64));

let mut seeded_slots: u64 = 0;

for hotkey in hotkeys.iter() {
let total_root: I96F32 = I96F32::saturating_from_num(
Pallet::<T>::get_stake_for_hotkey_on_subnet(hotkey, NetUid::ROOT).saturating_sub(
// On a v1 chain the escrow may already hold a root-slot position; it is custody,
// not a claimant, so it is excluded from the claimant base like everywhere else.
Pallet::<T>::get_stake_for_hotkey_and_coldkey_on_subnet(
hotkey,
&escrow,
NetUid::ROOT,
),
),
);
weight.saturating_accrue(T::DbWeight::get().reads(2));

let claimable = RootClaimable::<T>::take(hotkey);
weight.saturating_accrue(T::DbWeight::get().reads_writes(1, 1));

let mut fund_rate: I96F32 = I96F32::saturating_from_num(0);
let mut fund_shares: u64 = 0;
let mut fund_claimed: BTreeMap<T::AccountId, i128> = BTreeMap::new();

for (netuid, rate) in claimable.iter() {
// Fixed conversion price for this subnet: the moving/EMA price (manipulation
// resistant), falling back to spot if the EMA has not warmed up yet so legacy
// claims never convert to zero shares on a young subnet. Root converts 1:1.
let price: U96F32 = if netuid.is_root() {
U96F32::saturating_from_num(1)
} else {
let moving: U96F32 =
U96F32::saturating_from_num(Pallet::<T>::get_moving_alpha_price(*netuid));
if moving > U96F32::saturating_from_num(0) {
moving
} else {
U96F32::saturating_from_num(T::SwapInterface::current_alpha_price(
(*netuid).into(),
))
}
};
weight.saturating_accrue(T::DbWeight::get().reads(1));

// Gross credited principal (alpha) = rate * total_root_stake.
let gross: I96F32 = rate.saturating_mul(total_root);

// Total already claimed by all coldkeys on this (netuid, hotkey), converting each
// coldkey's watermark to TAO-valued fund shares while we scan.
let mut claimed_sum: I96F32 = I96F32::saturating_from_num(0);
for (coldkey, claimed) in RootClaimed::<T>::drain_prefix((*netuid, 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.

[CRITICAL] Unbounded one-block migration can halt the runtime upgrade

This runtime-upgrade migration collects every RootClaimable hotkey, visits every claimable subnet, and drains every matching RootClaimed entry in one block. It later also clears BasketPrincipal with u32::MAX. These state dimensions are unbounded, and the comment at lines 70–72 explicitly leaves fit as an unchecked condition. Returning the measured weight afterward does not constrain execution or make an oversized upgrade block importable. A sufficiently large live state can exhaust the upgrade block and prevent the chain from applying spec 438. Convert this to a cursor-based multi-block migration or establish and enforce a snapshot-derived upper bound before deployment.

@@ -1888,66 +1911,21 @@ mod dispatches {
///
#[pallet::call_index(121)]
#[pallet::weight(<T as crate::pallet::Config>::WeightInfo::claim_root())]

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] Fixed pre-dispatch weight permits an unbounded claim

claim_root is admitted using the old constant benchmark weight, but the new implementation iterates the caller's unbounded StakingHotkeys vector and, for each hotkey, iterates every basket holding and performs swaps and storage mutations. The dynamically returned post-dispatch weight is only known after execution and cannot protect block admission. An account with many staking hotkeys can therefore execute far more work than the declared weight, creating a block-exhaustion/DoS path. Bound both dimensions and benchmark the maximum, or redesign the call to process a caller-supplied bounded page whose pre-dispatch weight scales with its limit.

@github-actions

github-actions Bot commented Jul 23, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

MEDIUM scrutiny: young but highly active contributor with repository write access and no known Gittensor association; release-v438 -> main.

The prior Skeptic verdict was SAFE and contained no finding IDs requiring reconciliation. Static review covered the runtime economic changes, bounded basket claims/deposits and migrations, swap-state handling, runtime version bump, and CI retry change. No AI-review trust-boundary files or dependency manifests are changed.

Findings

No findings.

Conclusion

No malicious behavior or security vulnerability was found in the current diff.


🔍 AI Review — Auditor (domain review)

VERDICT: 👍

Established repository contributor with write access and substantial prior contributions; no trusted Gittensor association found.

The final commit’s relaxed basket assertions match runtime timing: dividends may increase the old validator’s rate and shares between the pre-swap read and finalization, while the zero-value checks on the old hotkey still prove that the fund moved completely.

The migration bounds, cooldown behavior, and spec_version 441 remain intact. Overlapping PRs #3009 and #2989 are unrelated despite sharing release/generated files. Static analysis was sufficient; no runtime tests or auto-fixes were needed.

Findings

No findings.

Conclusion

The latest test adjustment reflects valid runtime behavior without weakening the ownership invariant. No substantive domain issues remain.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

A tiny buy on a thin pool can push spot arbitrarily high, inflating a
validator's marked basket NAV (up to u64 saturation) so that every
subsequent root dividend deposit rounds to zero shares and is recycled,
and network-wide NAV metrics go blind.

- Replace alpha_to_tao_value (spot price * amount) with
  realizable_tao_for_alpha: a sim_swap quote bounded by the pool's TAO
  reserve, 1:1 for root and stable-mechanism holdings. Deposit share
  pricing, redemption sizing, and all NAV views now share this one
  valuation, so they cannot diverge under a manipulated spot price.
- When shares are outstanding but NAV marks to zero, only mint at par if
  the stale shares are rounding dust from a full drain (capture <= ~1%);
  otherwise recycle the dividend instead of mispricing the mint.
- Accumulate the network-wide NAV aggregate in u128 before saturating.

Co-authored-by: Cursor <cursoragent@cursor.com>

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

// Total already claimed by all coldkeys on this (netuid, hotkey), converting each
// coldkey's watermark to TAO-valued fund shares while we scan.
let mut claimed_sum: I96F32 = I96F32::saturating_from_num(0);
for (coldkey, claimed) in RootClaimed::<T>::drain_prefix((*netuid, 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.

[CRITICAL] Unbounded one-block migration can halt the runtime upgrade

This nested drain_prefix is reached after collecting every RootClaimable hotkey, and the migration also clears BasketPrincipal with u32::MAX. None of these state-dependent loops is bounded by the block limit; returning the measured weight afterward cannot prevent the upgrade block from exceeding its execution budget. The source comment explicitly acknowledges this risk. Convert this to a cursor-based multi-block migration with an enforced WeightMeter before deployment.

@@ -1888,66 +1911,21 @@ mod dispatches {
///
#[pallet::call_index(121)]
#[pallet::weight(<T as crate::pallet::Config>::WeightInfo::claim_root())]

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] Fixed pre-dispatch weight permits an unbounded claim

claim_root is admitted using the fixed benchmark weight, but execution traverses the caller's unbounded StakingHotkeys vector and every escrow holding for each hotkey. The dynamically returned post-dispatch weight only adjusts the fee; it does not reserve block capacity before execution. A caller with many staking hotkeys/holdings can therefore make one call consume far more work than its declared weight. Bound the inputs/state traversed per call or expose bounded pagination and charge the worst-case weight pre-dispatch.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Adds get_basket_payout(hotkey, coldkey) to BetaBasketRuntimeApi (+ RPC) so
clients can itemize owed dividends per validator. Regenerates the SDK chain
bindings and golden/shape fixtures against a v438 localnet. SDK: new
SetRootWeights and SetRootClaimThreshold intents, parameterless ClaimRoot,
reads/root.py wrapping the beta basket runtime APIs and claim threshold;
retires SetRootClaimType and the root_claim_type read. CLI: btcli weights
set-root/get-root and stake claim/basket/owed replace set-claim and
process-claim. Docs: new root-dividends guide, staking guide rewrite,
migration mappings, regenerated tx/query/error references, and the
v438-upgrade release page.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions github-actions Bot mentioned this pull request Jul 23, 2026
unarbos and others added 2 commits July 23, 2026 15:27
…igration.

Co-authored-by: Cursor <cursoragent@cursor.com>
…basket.

Dividend deposits now mint shares against the realizable NAV actually added
(net of slippage and fees) instead of the raw TAO deployed, and the slice of
each dividend attributable to the fund's own root-slot position accrues to
existing share holders via NAV growth rather than minting new shares. Adds a
stake_into_basket extrinsic that deploys a staker's TAO across the
validator's weight vector and credits shares through the signed claimed
watermark, plus tests covering round-trip symmetry, dilution protection, and
the root-slot yield attribution fix.

Co-authored-by: Cursor <cursoragent@cursor.com>
@unarbos unarbos changed the title Release v438: Root Reborn (beta baskets) on current main release-root-reborn Jul 23, 2026

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

// Total already claimed by all coldkeys on this (netuid, hotkey), converting each
// coldkey's watermark to TAO-valued fund shares while we scan.
let mut claimed_sum: I96F32 = I96F32::saturating_from_num(0);
for (coldkey, claimed) in RootClaimed::<T>::drain_prefix((*netuid, 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.

[CRITICAL] Unbounded one-block migration can halt the runtime upgrade

This drains every RootClaimed entry for every claimable subnet and hotkey in a single on_runtime_upgrade, after already collecting all RootClaimable keys; the later clear(u32::MAX, None) is likewise unbounded. Accruing the measured weight after doing the work does not enforce a block limit. A sufficiently large live state can exceed the upgrade block budget and prevent the runtime upgrade from completing. Convert this to a cursor-based multi-block migration with a strict per-block item/weight bound.

@@ -1888,66 +1911,59 @@ mod dispatches {
///
#[pallet::call_index(121)]
#[pallet::weight(<T as crate::pallet::Config>::WeightInfo::claim_root())]

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] Fixed pre-dispatch weight permits an unbounded claim

The declared claim_root() weight is fixed, but do_root_claim traverses the caller's entire StakingHotkeys vector and every basket holding for each hotkey. Returning the measured weight afterward does not protect block admission: an attacker can submit a claim whose actual work exceeds the reserved weight, causing block resource exhaustion. Bound the number of hotkeys/holdings processed per call or accept bounded pagination and benchmark the enforced maximum for pre-dispatch weight.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Security pass: the stake_into_basket extrinsic now declares a capacity-sized
weight cap and refunds the actual cost post-dispatch, scaled by both the
weight-vector length and the holdings the NAV valuations sweep; a user deposit
slice whose swap rounds to zero alpha now rolls the whole deposit back instead
of silently donating the TAO, and zero-alpha slices no longer book a protocol
inflow with no matching outflow; the clear-root-basket-weights migration drops
its redundant counting pre-pass and documents the 64-uid bound that makes a
single-block clear safe.

Quality pass: both deposit paths now share one deployment engine
(deploy_tao_into_basket with an explicit funding-source enum) instead of two
copy-pasted buy loops; the read-only valuation views moved to a new
basket_views.rs, putting claim_root.rs back under 1k lines; the negative-
watermark share grant got a named mutator (grant_basket_shares) and honest
storage docs; the direct-deposit tests moved to their own module with named
tolerance constants replacing magic epsilons.

Co-authored-by: Cursor <cursoragent@cursor.com>

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

// Total already claimed by all coldkeys on this (netuid, hotkey), converting each
// coldkey's watermark to TAO-valued fund shares while we scan.
let mut claimed_sum: I96F32 = I96F32::saturating_from_num(0);
for (coldkey, claimed) in RootClaimed::<T>::drain_prefix((*netuid, 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.

[CRITICAL] Unbounded one-block migration can halt the runtime upgrade

This upgrade migration collects every RootClaimable hotkey, then drains every corresponding RootClaimed prefix and finally calls clear(u32::MAX, None). None of these state-dependent scans is bounded or resumable. Returning the accumulated weight after completing the work does not constrain execution; sufficiently large production state can exceed the upgrade block budget and prevent the runtime upgrade from completing. Convert this to a cursor-based multi-block migration with an explicit per-block item/weight limit.

@@ -1888,66 +1911,63 @@ mod dispatches {
///
#[pallet::call_index(121)]
#[pallet::weight(<T as crate::pallet::Config>::WeightInfo::claim_root())]

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] Fixed pre-dispatch weight permits an unbounded claim

claim_root declares a fixed legacy weight, but do_root_claim traverses the caller's unbounded StakingHotkeys and every basket holding for each hotkey, performing swaps and storage updates. Returning the measured weight as post-dispatch weight only adjusts the fee after execution; it does not reserve or cap the block resources consumed. An account with many hotkeys/holdings can therefore submit work far beyond the declared weight. Bound the inputs/state traversal or expose bounded pagination and benchmark the worst-case pre-dispatch weight.

Conversion stays on_idle so try-runtime's double ORU pass stays
idempotent on testnet/mainnet (large RootClaimable prefixes).

Co-authored-by: Cursor <cursoragent@cursor.com>

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

Comment on lines +909 to +910
let claimed_entries: Vec<(T::AccountId, i128)> =
BasketClaimed::<T>::iter_prefix(old_hotkey).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] Root hotkey swap still collects an unbounded claimant map

BasketClaimed has one row per historical claimant, but this code collects the entire prefix and migrates it synchronously during swap_hotkey. The extrinsic still declares a fixed benchmarked pre-dispatch weight; accruing the measured row count afterward only adjusts fees and cannot reserve additional block capacity or bound execution. A popular validator can therefore make this path exceed its declared weight and exhaust block execution. Bound the number of rows before mutation or move the migration into a metered, resumable cursor-based process.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

- Clone hotkey-swap regression: swap on the working subnet only so the
  post-upgrade beta-basket seed does not block the test.
- Root-claim threshold e2e: assert non-ROOT storage is unchanged (Sudo
  finalizes even when the inner call fails).
- Non-root hotkey-swap e2e: allow BasketRate to grow from ongoing
  dividends instead of requiring exact equality.
- Root-touching hotkey swap: reserve pre-dispatch weight for
  BasketClaimed rows so large claimant prefixes are charged up front.

Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>

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

if !touches_root {
return Weight::zero();
}
let claimed_rows = BasketClaimed::<T>::iter_prefix(old_hotkey).count() as u64;

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] Dispatch-weight calculation performs an unbounded storage scan

#[pallet::weight] is evaluated while obtaining dispatch info and validating transactions, before the declared weight can reserve block capacity or charge the sender. Because historical BasketClaimed rows are unbounded, repeatedly submitting a root-touching swap can force nodes to scan the entire prefix during transaction validation. Bound claimant state or redesign the migration as bounded/resumable work; counting an unbounded prefix inside the weight expression is not safe metering.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Drain claimant watermarks in bounded passes (extrinsic + on_idle) so
popular root validators can still swap without an unbounded one-shot
collect under weight. Regenerate stale swap-hotkey docs.

Co-authored-by: Cursor <cursoragent@cursor.com>

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

/// * `NotEnoughBalanceToStake`: The caller cannot cover `amount_staked`.
/// * `BasketHasNoWeights`: The validator's weight vector filters to nothing.
#[pallet::call_index(147)]
// Declared weight is a cap sized for a 128-slot weight vector over 256 holdings

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] Basket deposit weight cap is not enforced

The declared cap assumes at most 128 weight slots and 256 existing holdings, but do_stake_into_basket measures the actual vectors after dispatch and never rejects values above those limits. Holdings persist across weight-vector changes, so a validator can accumulate positions across far more than 256 of the available subnet IDs; this call then scans them twice for NAV and can return a post-dispatch weight larger than the pre-dispatch cap. Substrate admission reserves only the declared weight, enabling undercharged block execution. Enforce both bounds before the scans and prevent state from exceeding them, or declare a cap covering the protocol maximum.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

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

Comment on lines +1961 to +1966
// Declared weight is a cap sized for a 128-slot weight vector over 256 holdings
// (each slot costs a balance transfer + swap + escrow write; each holding two NAV
// sim-swap valuations); the actual weight is computed in `do_stake_into_basket`
// from the real slot and holding counts and refunded post-dispatch, mirroring
// `claim_root`.
#[pallet::weight((Pallet::<T>::stake_into_basket_weight(128, 256), DispatchClass::Normal, Pays::Yes))]

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] Basket deposit weight cap is not enforced

The declared cap assumes at most 128 active weight slots and 256 holdings, but neither limit is enforced. set_root_weights accepts an unbounded destination vector, while holdings can persist as weights change, so both dimensions can exceed these constants. do_stake_into_basket then performs swaps and full NAV scans using the larger actual counts; returning a larger post-dispatch weight cannot charge above this pre-dispatch cap. Enforce both bounds before the transaction or derive a safe cap from protocol-enforced maxima.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Reject deposits whose weight-vector slots or NAV holdings exceed the
declared pre-dispatch cap so Substrate cannot undercharge the block.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

Align single-subnet cooldown with the all-subnets path: only enforce
HotkeySwapOnSubnetInterval when a prior LastHotkeySwapOnNetuid exists.
Fixes clone-upgrade tests on young chains where interval >= block.

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

Co-authored-by: Cursor <cursoragent@cursor.com>
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

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.

3 participants