Fix coldkey-wide root claims and stale staking relationships - #3141
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE VERY HIGH scrutiny (one public repo), mitigated by repository write access and substantive merged history; no Gittensor association or author/committer mismatch. fix/claim-budget-cap -> release-454. The pre-dispatch declaration is now additive, but post-dispatch accounting still collapses two sequential workloads with Findings
Prior-comment reconciliation
ConclusionColdkey-wide claims can still underreport consumed weight because relationship classification and claim processing are accounted non-additively. This remains merge-blocking. 📜 Previous run (superseded)
🔍 AI Review — Auditor (domain review)VERDICT: 👍 LIKELY Gittensor-associated contributor with repository write access and substantial recent Subtensor activity; no substantive duplicate PR identified. The chain-specific admission envelope, declared/actual weight accounting, rounding behavior, SDK preview, documentation, and focused regressions are consistent with the PR description. The Checks: FindingsNo findings. ConclusionThe change is narrowly scoped, preserves the mainnet admission cap, correctly accounts for both redemption and scan work, and adds appropriate boundary and value-preservation coverage. Ready to merge subject to CI. |
|
🔄 AI review updated — Skeptic: VULNERABLE |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
UnArbosFour
changed the title
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| let active = hotkey_count | ||
| .max(outcome.realized.saturating_add(outcome.swept)) | ||
| // Classifying a StakingHotkeys relationship reads the position's share-pool state | ||
| // and basket watermark. Price it conservatively as a full hotkey unit. | ||
| .max(selection_scanned) |
There was a problem hiding this comment.
[HIGH] Post-dispatch weight drops an additive selection workload
selection_scanned is work performed before redemption, but folding it into active with max charges for either relationship classification or active basket processing—not both. For example, a coldkey can have 256 staking relationships while its selected root hotkey also processes many basket rows; the call then reports only claim_root(256) (plus non-realized row scans), despite performing both workloads. Because this value is returned as actual_weight, the declared claim_root(256) + claim_root_scan(256) reservation is refunded and repeated calls can make a block execute materially more work than its recorded weight. Account for selection scans additively using a benchmarked component while retaining the declared envelope.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| let active = hotkey_count | ||
| .max(outcome.realized.saturating_add(outcome.swept)) | ||
| // Classifying a StakingHotkeys relationship reads the position's share-pool state | ||
| // and basket watermark. Price it conservatively as a full hotkey unit. | ||
| .max(selection_scanned) |
There was a problem hiding this comment.
[HIGH] Post-dispatch weight still drops additive selection work
Every StakingHotkeys relationship is classified before the selected hotkeys and basket rows are processed. Taking the maximum of selection_scanned and claim work charges only the larger workload, although both execute. An attacker can therefore pack blocks using understated post-dispatch weight. Account for classification additively, consistently with the declared claim_root + claim_root_scan envelope.
| let active = hotkey_count | |
| .max(outcome.realized.saturating_add(outcome.swept)) | |
| // Classifying a StakingHotkeys relationship reads the position's share-pool state | |
| // and basket watermark. Price it conservatively as a full hotkey unit. | |
| .max(selection_scanned) | |
| let active = hotkey_count | |
| .max(outcome.realized.saturating_add(outcome.swept)) | |
| .max(1); | |
| let scanned = outcome | |
| .rows | |
| .saturating_sub(outcome.realized) | |
| .saturating_add(selection_scanned); | |
| <T as crate::pallet::Config>::WeightInfo::claim_root(active).saturating_add( | |
| <T as crate::pallet::Config>::WeightInfo::claim_root_scan(scanned), | |
| ) |
|
🔄 AI review updated — Skeptic: VULNERABLE |
| .max(outcome.realized.saturating_add(outcome.swept)) | ||
| // Classifying a StakingHotkeys relationship reads the position's share-pool state | ||
| // and basket watermark. Price it conservatively as a full hotkey unit. | ||
| .max(selection_scanned) |
There was a problem hiding this comment.
[HIGH] Post-dispatch weight still drops additive selection work
selection_scanned is combined with claim work using max, although classification happens before and in addition to basket processing. A call can classify up to 256 relationships and then perform up to 256 admitted claim-work units, yet its post-dispatch weight reports only the larger workload. Charge classification additively (using its benchmarked cost) before adding basket scan/redeem weight.
|
🔄 AI review updated — Skeptic: VULNERABLE |
UnArbosFour
added
the
skip-cargo-audit
| .max(outcome.realized.saturating_add(outcome.swept)) | ||
| // Classifying a StakingHotkeys relationship reads the position's share-pool state | ||
| // and basket watermark. Price it conservatively as a full hotkey unit. | ||
| .max(selection_scanned) |
There was a problem hiding this comment.
[HIGH] Post-dispatch weight still drops additive selection work
selection_scanned represents candidate-classification work performed before the selected hotkeys are claimed. Taking the maximum charges for either classification or claim processing, not both, so a call with many candidates and substantial claim work receives an excessive refund. Account for classification additively—consistent with the declared claim_root(limit) + claim_root_scan(limit) envelope—rather than folding it into active.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| let active = hotkey_count | ||
| .max(outcome.realized.saturating_add(outcome.swept)) | ||
| // Classifying a StakingHotkeys relationship reads the position's share-pool state | ||
| // and basket watermark. Price it conservatively as a full hotkey unit. | ||
| .max(selection_scanned) |
There was a problem hiding this comment.
[HIGH] Post-dispatch weight still drops additive selection work
The classification scan and subsequent claim processing execute sequentially, but max(selection_scanned) charges only the larger workload. A claim with many raw StakingHotkeys entries and substantial selected-hotkey/basket work therefore refunds weight that was actually consumed, permitting blocks to execute beyond their accounted limit. Charge selection work additively and ensure the declared envelope covers the resulting worst case.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| let active = hotkey_count | ||
| .max(outcome.realized.saturating_add(outcome.swept)) | ||
| // Classifying a StakingHotkeys relationship reads the position's share-pool state | ||
| // and basket watermark. Price it conservatively as a full hotkey unit. | ||
| .max(selection_scanned) |
There was a problem hiding this comment.
[HIGH] Post-dispatch weight still drops additive selection work
selection_scanned measures the classification pass performed before do_root_claim, so its cost is additive to selected-hotkey and redemption work. Combining it with max refunds one of two sequential workloads, allowing an extrinsic to report less weight than it consumed. Mirror the additive declaration by adding the measured classification weight separately.
|
🔄 AI review updated — Skeptic: VULNERABLE |
2 participants
Summary
Fixes root claims that could be blocked by unrelated subnet stakes, stale
StakingHotkeysrelationships, subnet count, or zero-rounded basket holdings.The runtime spec version is bumped to 454.
Behavior changes
StakingHotkeysentry increased claim workhotkeys × networksBasketClaimed < 0Admission now uses:
The raw
StakingHotkeysclassification scan is independently capped at 256 and charged through actual post-dispatch weight.Mainnet and testnet
The previous genesis-hash-specific testnet envelope has been removed. Both networks now use the same fixed budget and refund unused work through post-dispatch weight.
Stale relationship cleanup
The bounded storage cleanup and
StakingHotkeyscleanup are rerun under fresh migration names:migrate_storage_bloat_v3migrate_cleanup_staking_hotkeys_v2The storage cleanup now also removes exact-zero
AlphaV2rows. Once zero share rows are gone, the relationship cleanup removes entries that have neither share rows nor a nonzero basket watermark.Existing positive sub-RAO shares are retained because deleting only their index would leave live shares undiscoverable if their value later increased. They no longer block root claims when they are not root-relevant, and the share-pool change prevents new zero-valued residue from being created during full withdrawals.
Validation
cargo fmt --check --allgit diff --checkAlphaV2cleanup, migration reruns, and share-residue canonicalizationThe generated documentation check detected broad unrelated pre-existing drift, so generated reference files were not rewritten.