Conversation
…rst turnover window at the first trade Two behaviours the new swap_basket tests exposed: - BasketSwapped was inserted mid-enum (before VotingPowerTrackingEnabled), renumbering every later event and failing the append-only regression test (tests::epoch::regression_liquid_alpha_event_indices_are_append_only). Move it after BasketAlphaWrittenOff so historical indices stay stable. - basket_trade_window_at treated the (0, 0) storage default as an open window starting at block 0, so on a young chain every fund shared a first window ending at block 7200 regardless of when it first traded. Treat an uncharged window as absent and open it at the current block. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Pallet tests (tests/swap_basket.rs): happy paths for alpha->alpha, alpha->root and root->alpha (composition-only, reserve lockstep, author fee, protocol flow, event payload, post-dispatch weight); every gate and error; slippage band on both legs (EMA- and spot-anchored refusals, a just-inside fill); turnover budget (accumulates, refuses, rolls at 7200 blocks, root origin at face); concentration cap (refuse over cap, sell out of an over-cap position, young-chain softening); guardrails follow hotkey swap; failed second leg rolls back fully; BasketSwapped index pinned. AdminUtils: auth, storage, and events for the three sudo setters. Runtime: BasketTrading proxy admits exactly swap_basket and no other narrow proxy admits it. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Behaviour-documenting tests that pass today and must flip when a fix lands (each names its finding in docs/pr-3150-swap-basket-exploit-calibration.md): - §2.1 thin-pool drain: sliced root->C buys with counterparty sell-back pass every guardrail until the turnover budget is spent and lose >5% of NAV (~90% of the TAO traded); the concentration cap passes on realizable value while the same holding at spot is many times the cap share. - §2.2 window-boundary burst: 2x the daily budget moved in two adjacent blocks across the window edge. - §2.3 chained legs in one block walk the price >20% up to 1.02 x EMA. - crash lock: selling refused with spot 5% below EMA, buying refused 5% above; cash slot capped at RootWeightsCap (1/16) once the cap binds. - proxies: NonTransfer / NonCritical currently admit swap_basket without an explicit BasketTrading grant. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE LOW contributor risk / baseline scrutiny: write-role maintainer with substantive merged history; disclosed Cursor coauthorship; no listed Gittensor association. Target: feat/basket-trading-swap-basket. Reviewed all six changed files and the turnover-accounting call path. No review-trust, CI, or dependency changes. The added weakness-demonstration tests cover behavior inherited from #3150; this verdict applies only to #3152’s delta. Checks: FindingsNo findings. ConclusionNo malicious behavior or newly introduced security vulnerability found. The event relocation restores historical ordering, and the window change preserves accounting for already-charged turnover. 🔍 AI Review — Auditor (domain review)VERDICT: 👍 Established contributor with write permission; Gittensor UNKNOWN. Supplied overlap metadata shows no competing swap-basket coverage PR. The event-index and initial-window fixes are sound, and the added tests provide substantial coverage. One calibration test needs its timing corrected to demonstrate the scenario claimed in the PR body. Checks: Findings
ConclusionApprove with the test correction below. No blocking regression was identified in the changes against the feature branch. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| let start = System::block_number(); | ||
| assert_ok!(swap(&fund, fund.netuid_a, fund.netuid_b, half)); | ||
| assert_ok!(swap(&fund, fund.netuid_a, fund.netuid_b, half)); |
There was a problem hiding this comment.
[MEDIUM] Spend the first budget at the window boundary
At pallets/subtensor/src/tests/swap_basket.rs:1203–1205, both first-window trades execute at start; the next successful trades execute at start + 7200. Consequently, moved_in_two_blocks counts trades separated by an entire window, rather than adjacent blocks as the test and PR description claim.
Establish an earlier window with a small successful trade, advance to its penultimate block, spend the remaining budget there, then spend the refreshed budget in the next block. Assert the combined tao_mid from those two adjacent blocks, excluding the opening trade. This makes the test demonstrate the boundary burst it is intended to document.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👍 |
Open the window with a minimal trade, spend the remaining budget at start + 7199, then a full budget at start + 7200, and assert on the sum of those two adjacent blocks only (review finding on the earlier shape, which spent the two budgets a whole window apart). Co-authored-by: Arbos <unarbos@users.noreply.github.com>
…t.rs Matches the other test modules (consensus.rs, migration.rs); CI runs clippy with -D warnings over --all-targets. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Drop BasketTradingCalls from the NonTransfer and NonCritical allow lists so an existing broad delegate does not gain basket-trading power at upgrade; the explicit BasketTrading grant is the only proxy type that admits swap_basket (Any aside). NonTransfer is no longer a superset of BasketTrading. Golden subtractive tests updated; the pinned broad-proxy test now asserts the denial. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Describe the swap_basket stack (#3150, #3152, #3153): what validators can do, the BasketTrading proxy model, every guardrail with its launch default (2% per-leg band, token-bucket turnover budget of 10% of NAV refilling over 7200 blocks, 10% per-pool liquidity cap, 1/16 concentration cap, enable and freeze switches), the corrected worst-case loss bound, the new calls, events, errors, storage, admin setters, runtime API, and SDK/btcli surface. Tag the shipped v450 entry with its version so only the new release is 'next'. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Describe the swap_basket stack (#3150, #3152, #3153): what validators can do, the BasketTrading proxy model, every guardrail with its launch default (2% per-leg band, token-bucket turnover budget of 10% of NAV refilling over 7200 blocks, 10% per-pool liquidity cap, 1/16 concentration cap, enable and freeze switches), the corrected worst-case loss bound, the new calls, events, errors, storage, admin setters, runtime API, and SDK/btcli surface. Tag the shipped v450 entry with its version so only the new release is 'next'. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Describe the swap_basket stack (#3150, #3152, #3153): what validators can do, the BasketTrading proxy model, every guardrail with its launch default (2% per-leg band, token-bucket turnover budget of 10% of NAV refilling over 7200 blocks, 10% per-pool liquidity cap, 1/16 concentration cap, enable and freeze switches), the corrected worst-case loss bound, the new calls, events, errors, storage, admin setters, runtime API, and SDK/btcli surface. Tag the shipped v450 entry with its version so only the new release is 'next'. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Describe the swap_basket stack (#3150, #3152, #3153): what validators can do, the BasketTrading proxy model, every guardrail with its launch default (2% per-leg band, token-bucket turnover budget of 10% of NAV refilling over 7200 blocks, 10% per-pool liquidity cap, 1/16 concentration cap, enable and freeze switches), the corrected worst-case loss bound, the new calls, events, errors, storage, admin setters, runtime API, and SDK/btcli surface. Tag the shipped v450 entry with its version so only the new release is 'next'. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
2 participants
Description
Stacked on #3150 (
feat/basket-trading-swap-basket). Adds the pallet, AdminUtils, and proxy-filter tests forswap_basketthat the feature PR shipped without (its tests were temporary and removed), plus behaviour-documenting tests for the weaknesses found in the calibration pass.Commits, in order:
fix(swap_basket)— two non-test changes the tests exposed (kept separate on purpose):pallets/subtensor/src/macros/events.rs:BasketSwappedwas inserted mid-enum (beforeVotingPowerTrackingEnabled), renumbering every later event and failingtests::epoch::regression_liquid_alpha_event_indices_are_append_only(this is thecargo testfailure on feat: swap_basket — validator-directed beta basket rebalancing #3150's CI). Moved to the tail (index 149).pallets/subtensor/src/staking/basket_views.rs::basket_trade_window_at: the(0, 0)storage default was treated as an open window starting at block 0, so on a young chain every fund shared a first window ending at block 7200 regardless of when it first traded. An uncharged window now opens at the current block. (Never bites on mainnet wherenow ≫ 7200; matters for localnet/tests and thebasket_trading_statusview.)test: swap_basket coverage—pallets/subtensor/src/tests/swap_basket.rs(31 tests),pallets/admin-utils/src/tests/mod.rs(+3),runtime/src/proxy_filters/mod.rs(+1).test: pin the swap_basket weaknesses— 5 pallet tests + 1 proxy test that document the findings indocs/pr-3150-swap-basket-exploit-calibration.md; each carries a comment naming the finding so it flips when the corresponding fix lands. No guardrail logic was changed.fix(proxy): swap_basket requires BasketTrading proxy—BasketTradingCallsremoved from theNonTransferandNonCriticalallow lists (runtime/src/proxy_filters/mod.rs);BasketTradingis the only proxy type that admitsswap_basket(Anyaside),Stakingstays without it, andNonTransferis no longer a superset ofBasketTrading. The pinned broad-proxy test now asserts the denial (§5.6 of the calibration report).Covered
BasketRate, watermark, owed, root stake unchanged; NAV within fees); root reserve lockstep (SubnetTAO[ROOT],SubnetAlphaOut[ROOT]credited/debited by exactlytao_mid);TotalStakedrops by exactly the sell-leg author fee; block author paid on both legs; protocol flow (sell outflow =tao_mid+ author fee, buy inflow fee-excluded, user flow untouched);BasketSwappedpayload; claim after rebalance pays the same value; post-dispatch weight =swap_basket_weight(rows)< declared cap.BasketTradingDisabled,BasketTradingFrozen(and unfreeze),BasketSameSubnet,NonAssociatedColdKey(stranger and non-existent hotkey),HotKeyNotRegisteredInSubNet,BetaBasketSeedInProgress,ColdkeySwapAnnouncedvia theCheckColdkeySwapdispatch extension,SubnetNotExists(both sides),SubtokenDisabled,AmountTooLow(zero and sub-DefaultMinStake),NotEnoughStakeToWithdraw(over-held and empty origin).tao_midacross trades, refuses at the cap, refuses atstart + 7199, rolls atstart + 7200with a fresh window, root origin charged at face,basket_trading_statusagrees. (These tests andbasket_trade_window_atwill be reworked in fix(swap_basket): liquidity-relative destination cap, token-bucket turnover, flush weight #3153, which replaces the fixed window with a token bucket and stacks on this branch.)TotalStake, author balance, turnover window, protocol flow, and events untouched.sudo_set_basket_trading_enabled,sudo_set_basket_trading_frozen,sudo_set_basket_daily_turnover_cap—BadOriginfor signed,ValueNotInBoundsfor a zero cap, storage effect, events.BasketTradingadmits exactlyswap_basketand denies adjacent calls; no other narrow proxy admitsswap_basket;NonTransfer,NonCritical, andNonFungibledeny it; superset relation is{Any}only.BasketSwappedpinned at 149 afterBasketAlphaWrittenOff(148); the existing append-only regression test passes again.Weaknesses pinned (pass today, must flip with a fix)
finding_2_1_thin_pool_drain_passes_every_guardrailfinding_2_2_window_boundary_lets_two_budgets_through_in_adjacent_blocksstart + 7199,start + 7200)finding_2_3_chained_legs_in_one_block_walk_price_to_ema_ceilingfinding_crash_lock_blocks_selling_a_falling_holdingfinding_cash_slot_is_capped_at_root_weights_capbroad_proxies_currently_admit_swap_basket_without_opt_inbroad_proxies_do_not_admit_swap_basket_without_explicit_grantasserts the denialBugs the tests exposed
BasketSwappedevent index shift (breaks feat: swap_basket — validator-directed beta basket rebalancing #3150 CI) — fixed in commit 1.NonTransfer/NonCriticaldelegates gainedswap_basketwithout opting in — fixed in commit 4.TotalStakekeeps the buy-leg author fee counted as stake — inherited fromstake_into_subnet; the happy-path test asserts the exact current relation (TotalStakedrops only by the sell-leg fee).Type of Change
Checklist
./scripts/fix_rust.sh(creates a commit; rancargo fmt --check --allinstead)cargo clippy -p pallet-subtensor --all-targets -D warningsclean)cargo test -p pallet-subtensor swap_basket: 36 passed;cargo test -p pallet-admin-utils basket: 3 passed;cargo test -p node-subtensor-runtime proxy_filters: 17 passed;regression_liquid_alpha_event_indices_are_append_only: passes)Additional Notes
Per
AGENTS.md: nospec_versionchange, no labels, no benchmarks.cargo fmt --check --allandgit diff --checkclean. Runtime tests were run withSKIP_WASM_BUILD=1andCXX=g++ CC=gcc LIBRARY_PATH=/usr/lib/gcc/x86_64-linux-gnu/13(this VM's defaultccis clang without libstdc++ on the link path).