Repository navigation
spec 465: safer-only hotfix pack - #3179
Conversation
Close the live proxy wrapper escapes (Multisig + Crowdloan; Sudo already off restricted grants), keep escrow-only basket dividends instead of recycling them, and stop swap_basket wash cycles from resetting the liquidity cap. Cherry-pick LimitOrdersLinkedOutputTtl 7→180 days. Charge unstake_all* for every live subnet it can scan. Do not stamp share-pool reconcile (or remove_root_weights) after a skip; add a v2 retry. Pin SCALE event 149/150 and error 160–165. Require SubtokenEnabled on the origin leg and clear SubnetFastMovingPrice on dissolve. BasketTradingEnabled stays default OFF. No derivatives. No CH-5 paged swap_hotkey. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| let used_rows: sp_std::vec::Vec<_> = | ||
| BasketLiquidityUsed::<T>::iter_prefix(old_hotkey).collect(); | ||
| for (netuid, (old_used, old_block)) in used_rows { |
There was a problem hiding this comment.
[MEDIUM] Charge and bound liquidity-counter migration work
This new loop moves every historical destination counter but never adds those rows to moved_rows. The caller in swap/swap_hotkey.rs:917 uses that return value to charge variable database work, so these reads and writes are omitted. Counters survive selling a holding and expiry, so their count can grow independently of the current holdings charged by the caller. Once basket trading is enabled, a validator can accumulate destination rows and repeatedly move them through hotkey swaps with unaccounted storage work, weakening block resource limits. Include these rows in weight accounting and provide a bound covered by the declared weight (or a metered migration); counting them only after execution does not establish a pre-dispatch bound.
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE LOW contributor risk / baseline scrutiny: repository write access, substantive merged history, no supplied Gittensor match; cursor/spec-465-hotfix-cee2 → main. Reviewed the diff through Passed: FindingsNo findings. ConclusionNo security vulnerabilities or evidence of malicious intent were found. The prior review contained no findings requiring reconciliation. 🔍 AI Review — Auditor (domain review)VERDICT: 👍 Established contributor with repository write access; Gittensor association UNKNOWN. Reviewed migration retries, basket accounting and flow limits, proxy restrictions, claim admission, alpha-fee selection, and SDK alignment. No substantive correctness blocker identified. Description discrepancies: document the 7-to-180-day linked-order lifetime extension ( Overlap check: #3163, #3137, and #3097 cover included changes. Their implementations are absent from the supplied context, so a comparative closing recommendation is unsupported. Checks: FindingsNo findings. ConclusionThe reviewed changes have relevant regression coverage and no identified merge blocker. Formatting and environment-dependent validation remain for CI. |
|
🔄 AI review updated — Skeptic: VULNERABLE |
Skeptic MEDIUM: wash-counter carry was unmetered. Count BasketLiquidityUsed (and pending deposits) in pre-dispatch weight and in moved_rows so the walk is reserved and paid. Carry every dest, including zero-holding, so swap cannot reset wash headroom. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
| Self::share_within_cap(new_used, reserve, cap), | ||
| Error::<T>::BasketLiquidityCapExceeded | ||
| ); | ||
| BasketLiquidityUsed::<T>::insert(hotkey, netuid, (new_used, now)); |
There was a problem hiding this comment.
[HIGH] Account for the new flow-counter storage in swap weights
pallets/subtensor/src/staking/basket_trade.rs:451–459 adds a BasketLiquidityUsed read and write to every successful non-root destination trade. swap_basket_weight still returns the unchanged generated weight, whose storage inventory contains no such map. Consequently neither declared nor refunded weight accounts for this additional storage access and proof. Update the benchmark coverage and weight wiring, with reference measurements produced through CI.
| impl<T: Config> OnRuntimeUpgrade for Migration<T> { | ||
| fn on_runtime_upgrade() -> Weight { | ||
| migrate_reconcile_share_pools_v2::<T>() | ||
| } |
There was a problem hiding this comment.
[HIGH] Add validation and execution tests for the v2 migration
pallets/subtensor/src/migrations/migrate_reconcile_share_pools.rs:406–409 registers the retry with only on_runtime_upgrade. The v1 wrapper has pre/post checks for pool value, row count, denominator consistency and its marker; those hooks do not validate the v2 marker or the v2 execution itself. Share those checks through a migration-name parameter and attach them to v2. Also test execution with v1 already stamped, including successful repair, skipped targets leaving v2 unstamped, and an idempotent second run; the new Boolean-helper test does not exercise these storage transitions.
| fn basket_liquidity_used_at(hotkey: &T::AccountId, netuid: NetUid, now: u64) -> u64 { | ||
| match BasketLiquidityUsed::<T>::get(hotkey, netuid) { | ||
| None => 0, | ||
| Some((used, last_block)) => { | ||
| let elapsed = now.saturating_sub(last_block); | ||
| if elapsed >= crate::BASKET_TRADE_REFILL_BLOCKS { | ||
| 0 | ||
| } else { | ||
| let remaining = crate::BASKET_TRADE_REFILL_BLOCKS.saturating_sub(elapsed); | ||
| Self::mul_div_u64(used, remaining, crate::BASKET_TRADE_REFILL_BLOCKS) |
There was a problem hiding this comment.
[MEDIUM] Test the new liquidity-flow guardrail
pallets/subtensor/src/staking/basket_trade.rs:423–459 introduces a persistent economic counter and decay formula, but no test references BasketLiquidityUsed. Existing standing-position and turnover-bucket tests do not establish its intended behavior. Add regression coverage for buy/unwind/rebuy rejection, partial and full refill boundaries, failed-trade rollback, and hotkey-swap carry after the destination holding reaches zero. Include zero and maximum-value arithmetic cases.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
Reserve BasketLiquidityUsed on swap_basket via DbWeight only (no invented CPU). v2 shares v1 try-runtime with the production skip=no-stamp rule. v1 defers the prefix walk so 459→465 pays once. Flush and mint share can_keep_basket_dividend so a shareless escrow-only credit recycles instead of re-queuing. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Allow expect in the dest-flow test (CI -D clippy::expect-used). Align remove_root_weights try-runtime with production: a cursor skip does not stamp, so offline must not require HasMigrationRun. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Codegen against node-subtensor-release-df9b02df721fd0b4b537a64361e0f57102d2aa3c (specVersion 465). Adds BasketLiquidityUsed. No faucet. Drift clean. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
try-runtime-cli re-runs on_runtime_upgrade and diffs storage. The old v1 catch-up (walk+stamp once v2 had run) changed HasMigrationRun on that second pass. Live 459→465 already walks once in v2. Safer no-op. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Docs and website build failed generate.py --check. The pack shifted rust line anchors. Regenerated tx/query/index and reads.json. No hand edits. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
| let deferred_to_v2 = migration_name == MIGRATION_NAME | ||
| && !already_run | ||
| && !HasMigrationRun::<T>::get(MIGRATION_NAME_V2.to_vec()); |
There was a problem hiding this comment.
[MEDIUM] Keep v1 validation idle after v2 has completed
At pallets/subtensor/src/migrations/migrate_reconcile_share_pools.rs:329–331, deferral ends once v2 is stamped, but the production v1 hook is now unconditionally a no-op. After a 459→465 upgrade, v1 remains unstamped and v2 is stamped. A subsequent try-runtime upgrade check therefore sets both already_run and deferred_to_v2 to false and fails with in-bound reconcile must stamp at line 386, despite correct production behavior. Treat unstamped v1 as deferred regardless of the v2 marker. Regression coverage should run the v1 pre/on/post hooks with v1=false and v2=true.
| let deferred_to_v2 = migration_name == MIGRATION_NAME | |
| && !already_run | |
| && !HasMigrationRun::<T>::get(MIGRATION_NAME_V2.to_vec()); | |
| let deferred_to_v2 = migration_name == MIGRATION_NAME && !already_run; |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Bumps the cargo group with 1 update in the / directory: [tracing-subscriber](https://github.com/tokio-rs/tracing). Updates `tracing-subscriber` from 0.3.19 to 0.3.20 - [Release notes](https://github.com/tokio-rs/tracing/releases) - [Commits](tokio-rs/tracing@tracing-subscriber-0.3.19...tracing-subscriber-0.3.20) --- updated-dependencies: - dependency-name: tracing-subscriber dependency-version: 0.3.20 dependency-type: direct:production dependency-group: cargo ... Signed-off-by: dependabot[bot] <support@github.com>
Single-hotkey claim_root uses a 129-unit envelope (1 + 128 slots). store_encrypted and owner-key rotation stay inventory-only on restricted proxies. Fee netuid walk uses the alpha prefix. Unstamped v1 try-runtime is deferred even after the v2 marker, so live no-op v1 does not require a stamp. Re-anchor generated docs after the rust line shifts. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
| .into_iter() | ||
| pallet_subtensor::Pallet::<T>::alpha_iter_prefix((hotkey, coldkey)) | ||
| .map(|(netuid, _)| netuid) | ||
| .filter(|netuid| pallet_subtensor::SubtokenEnabled::<T>::get(netuid)) |
There was a problem hiding this comment.
[MEDIUM] Exclude retired shares from alpha-fee subnet selection
pallets/transaction-fee/src/lib.rs:231–234: alpha_iter_prefix filters raw nonzero shares but does not check their pool epoch. Retired rows therefore survive this filter even though get_stake_for_hotkey_and_coldkey_on_subnet correctly values them at zero. With one live position and one retired row on enabled subnets, can_withdraw_in_alpha sees two entries and rejects payment because it requires exactly one. This regresses alpha-funded unstake_all* and whole-hotkey swaps for payers without sufficient free TAO. Keep the prefix optimization, but restore the current-stake filter.
| .filter(|netuid| pallet_subtensor::SubtokenEnabled::<T>::get(netuid)) | |
| .filter(|netuid| pallet_subtensor::SubtokenEnabled::<T>::get(netuid)) | |
| .filter(|netuid| { | |
| pallet_subtensor::Pallet::<T>::get_stake_for_hotkey_and_coldkey_on_subnet( | |
| hotkey, coldkey, *netuid, | |
| ) != 0.into() | |
| }) |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Prefix walk is cheaper than scanning every subnet. Retired share rows still appear there with zero current stake. Main filtered those out. Restore that filter so one live position plus one retired row can still pay unstake_all* and swap_hotkey in alpha. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
alpha_iter_prefix keeps raw leftover shares from a closed pool epoch. The current-stake filter already drops them. Pin that unstake_all with one live position and one retired row still selects one subnet and pays when the signer has only ED TAO. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
The leftover 129-unit declared weight and the composed-runtime pin must agree. Embedded unstake-claim SDK policy uses the same single-hotkey limit. WeightInfo CPU is unchanged. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
|
@IntiTechnologies spec 465 Propose is waiting on the Train: https://github.com/RaoFoundation/subtensor/actions/runs/35267938998 Jacob authorized this 465 train through mainnet. After you approve the env, CI publishes
Note: a leftover 464 Watch Mainnet Release run is also waiting on the same |
This branch was successfully deployed
1 participant
Description
Safer-only runtime 465 off current
main(464 already merged at5cd66b8597). Effect is stricter, not broader.Head:
041e17187552282bc025c5e146adf5b93f9a1cb4(unarbos).041e17187: test that one live position plus one retired leftover row still paysunstake_allin alpha when the signer has only ED TAO.c6953216d: restore main's current-stake filter on the leftover alpha-prefix fee walk. Retired rows must not block alpha-fundedunstake_all*/swap_hotkey.41804f9c7: leftover pack (129-unit claim envelope, inventory-only proxy, prefix fee walk, v1 try-runtime deferred).235cf3219:generate.pyre-anchors after rust line shifts.f2516d9c8: SDK_generated/from the CI Linux release node.b8c0d6d80: share-pool v1 one-read no-op after v2.02000d7f4: Clippyexpect-used;remove_root_weightstry-runtime skip=no-stamp.BasketTradingEnabledstays default OFF. Do not sudo-enable. Do not merge #3135.Type of Change
Breaking Change
None intended. Restricted proxies lose wrappers they should not have had.
swap_basketdest flow is capped per refill window (trading remains off on finney).unstake_all*declared weight covers every live subnet it can scan.Checklist
maincargo fmt --check --allgit diff --checkclean041e17187Additional Notes
main.