feat: swap_basket — validator basket trading (consolidated) - #3155
Conversation
|
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}
| if credits == 0 { | ||
| 0 | ||
| } else { | ||
| credits.saturating_add(4 * 256) |
There was a problem hiding this comment.
[HIGH] Flush weight omits per-credit retry sweeps
credits + 4 * 256 budgets for a single deposit attempt, but deposit_root_alpha_batch (basket_flush.rs:311–323) retries every credit separately when a multi-credit batch fails. Each retry can repeat full holding valuations and curated deployment, making work scale with credits × holdings, even when both counts are below 256. The existing test_flush_failure_path_requeues_and_splits demonstrates a reachable soft-failure case.
Once trading is enabled, an authorized caller can trigger this flush before the trade’s balance and turnover checks; a subsequent rejection does not undo the computation. Returning a larger actual weight cannot repair underdeclared admission weight because FRAME caps post-dispatch charging at the declaration. The new benchmark uses an empty queue and misses this path.
Bound flush execution explicitly or include the batch attempt plus every singleton retry in the declared weight. Add a regression with many holdings and queued credits that forces batch failure and verifies the complete work remains within the admission bound.
🛡️ AI Review — Skeptic (security review)VERDICT: VULNERABLE LOW contributor risk / baseline scrutiny: write access and substantive merged contributions; no supplied Gittensor match; cursor/swap-basket-consolidated-b2ac → main. No evidence of malicious intent or changes to AI-review trust files was identified. Checks: Findings
Prior-comment reconciliation
ConclusionThe CLI still permits submission without the user's requested minimum-output protection after a read failure. Require a successful quote unless the user explicitly disables that protection. 📜 Previous run (superseded)
🔍 AI Review — Auditor (domain review)VERDICT: 👍 Established contributor with write permission; Gittensor association UNKNOWN. Supplied overlap context identifies no competing basket-trading implementation. Both prior findings are addressed: the migration now runs through the runtime upgrade tuple with pre/post validation, and the Ledger fixture contains distinct JavaScript and Rust proofs with matching 512-byte tails. Description discrepancies: update spec 456 to 460, document removal of root-weight curation, and correct the flush allowance to Validation: static review, Ledger fixture comparison, and frontmatter presence checks on 212 documentation files. FindingsNo findings. Prior-comment reconciliation
ConclusionThe previous blockers are resolved, and no new substantive issue was identified in the reviewed paths. Approval remains subject to CI validation. 📜 Previous run (superseded)
|
|
🔄 AI review updated — Skeptic: VULNERABLE |
| if credits == 0 { | ||
| 0 | ||
| } else { | ||
| credits.saturating_add(4 * 256) |
There was a problem hiding this comment.
[HIGH] Flush weight omits per-credit retry sweeps
pallets/subtensor/src/staking/basket_trade.rs:366 budgets only credits + 1024 flush units. However, deposit_root_alpha_batch (pallets/subtensor/src/staking/basket_flush.rs:316-329) retries every credit separately after a multi-credit batch fails. Each retry repeats basket valuation and, for curated funds, destination deployment work, making the cost scale with credits × holdings rather than their sum. Failed singleton deposits are requeued, so subsequent calls can repeat this work. The flush runs before the trade’s balance and turnover checks, and returning a larger post-dispatch weight cannot increase the weight reserved at admission.
Bound and charge the initial batch plus all possible singleton retries before dispatch, or limit the flush to explicitly budgeted work. Add a regression covering a failing multi-credit batch with many holdings and assert that declared weight covers all executed work.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| if credits == 0 { | ||
| 0 | ||
| } else { | ||
| credits.saturating_add(4 * 256) |
There was a problem hiding this comment.
[HIGH] Flush weight omits per-credit retry sweeps
pallets/subtensor/src/staking/basket_trade.rs:366 reserves only credits + 1024 flush units. However, deposit_root_alpha_batch (basket_flush.rs:316–327) retries every origin separately after a multi-credit batch fails. Each curated retry can repeat three holding sweeps and destination deployment, making work grow with credits × holdings, rather than the declared additive bound.
An authorized trader can trigger this work through swap_basket once trading is enabled. Returning a larger actual weight cannot increase the pre-dispatch reservation, and a later trade failure still incurs the flush work. Bound or meter the retries before execution, include their worst-case cost in admission, and add a regression covering a failed multi-credit batch with many holdings.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| if credits == 0 { | ||
| 0 | ||
| } else { | ||
| credits.saturating_add(4 * 256) |
There was a problem hiding this comment.
[HIGH] Flush weight omits per-credit retry sweeps
pallets/subtensor/src/staking/basket_trade.rs:366 budgets only credits + 1024 quote units. However, deposit_root_alpha_batch (pallets/subtensor/src/staking/basket_flush.rs:317–326) retries each credit separately after a soft batch failure. Each retry repeats the holding valuations and, for curated funds, destination deployment work. The resulting work grows with credits × holdings, beyond this single-batch allowance.
An authorized trader can trigger this flush repeatedly while failing credits remain queued, including through trades that subsequently fail. Returning a larger actual weight cannot repair pre-dispatch admission: post-dispatch weight is capped by the declared weight.
Bound and charge the complete batch-plus-singleton retry path before execution, or limit flush work to the admitted budget. Add a failure-path regression asserting declared weight covers the complete flush and trade.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| if credits == 0 { | ||
| 0 | ||
| } else { | ||
| credits.saturating_add(4 * 256) |
There was a problem hiding this comment.
[HIGH] Flush weight omits per-credit retry sweeps
pallets/subtensor/src/staking/basket_trade.rs:395 still bounds flushing as credits + 1024, but deposit_root_alpha_batch (basket_flush.rs:319–328) retries every credit separately after a multi-credit batch fails. Each retry repeats the fund NAV sweep and, for curated funds, deployment work. This makes the reachable flush cost scale with credits × holdings, rather than the additive bound declared here.
An authorized trader can trigger this work with queued deposits even when the subsequent trade fails. Returning a larger post-dispatch weight cannot repair an insufficient block-admission reservation. Bound all batch and singleton retry work before dispatch, or cap flushing to the reserved work and defer the remainder; cover the failing-batch retry path in the weight regression test.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| if credits == 0 { | ||
| 0 | ||
| } else { | ||
| credits.saturating_add(4 * 256) |
There was a problem hiding this comment.
[HIGH] Flush weight omits per-credit retry sweeps
pallets/subtensor/src/staking/basket_trade.rs:395 budgets only credits + 1024 flush units. However, deposit_root_alpha_batch retries every credit separately after a multi-credit batch fails (pallets/subtensor/src/staking/basket_flush.rs:316–327). Each retry repeats basket valuations; curated retries account for 3 × holdings + destinations + 1 units apiece. Work therefore grows with credits × holdings, beyond this fixed sweep allowance.
An authorized trader can trigger this path with pending credits once trading is enabled, including on calls that subsequently fail. Reporting higher post-dispatch weight cannot increase the pre-dispatch admission limit. Bound all batch and singleton retry work before admission, or cap flush execution to the declared budget, and cover a failed multi-credit batch in a weight-bound regression test.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| if credits == 0 { | ||
| 0 | ||
| } else { | ||
| credits.saturating_add(4 * 256) |
There was a problem hiding this comment.
[HIGH] Flush weight omits per-credit retry sweeps
pallets/subtensor/src/staking/basket_trade.rs:395 budgets only one batch's sweeps. However, deposit_root_alpha_batch in pallets/subtensor/src/staking/basket_flush.rs:318-329 retries every credit separately after a multi-credit batch fails. Each retry repeats the holding valuations, so flush work can grow as credits × holdings instead of credits + a fixed sweep allowance.
An authorized trader can invoke this path with pending credits once trading is enabled, including through trades that subsequently fail. The larger weight returned on success cannot retroactively increase the pre-dispatch reservation; post-dispatch accounting is capped by the declared weight. Bound the initial batch plus all singleton retries, or enforce a metered flush limit before executing them. Add a regression covering a failed multi-credit batch with many holdings and verify that declared weight covers all executed work.
|
🔄 AI review updated — Skeptic: VULNERABLE |
| if credits == 0 { | ||
| 0 | ||
| } else { | ||
| credits.saturating_add(4 * 256) |
There was a problem hiding this comment.
[HIGH] Flush weight omits per-credit retry sweeps
This is still not an upper bound on the work performed. deposit_root_alpha_batch (basket_flush.rs:315–327) retries every credit individually after a multi-credit batch fails. Each retry can repeat the full holding valuations and destination deployment, so flush work can scale as credits × holdings rather than credits + 4 * 256. Recoverable deposit failures can leave credits queued for subsequent calls.
Once trading is enabled, an authorized trader can trigger this work before the trade's balance and budget checks, exceeding the weight reserved for block admission. Reporting a larger post-dispatch weight cannot repair an underestimated pre-dispatch bound. Include the initial batch and every possible singleton retry in the declared bound, or cap flush execution to a reserved work budget; cover a late-failing multi-credit batch in the weight regression test.
|
🔄 AI review updated — Skeptic: VULNERABLE |
`codegen.check --names` requires every chain error to carry a semantic code and a description. Main's runtime 456 fixes added Crowdloan.FundsNotSettled (invalid_argument, alongside InvalidFinalizationConfig) and LimitOrders.OrderSignerFrozen (disabled, like ColdkeySwapAnnounced/ColdkeySwapDisputed which it wraps). Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Source-line anchors moved with main's changes, and the two new chain errors gain their generated pages. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Mainnet took v456-v458 as the security and accounting releases while this branch was open, so the basket release page and the guides that cite it now name v459. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
…basket Root validators no longer publish a target vector. Dividends accumulate in place on the subnet they were earned on, direct deposits mirror the fund's current holdings by realizable value, and a fund with no holdings holds its first deposit as the root cash slot. Only swap_basket changes composition. Weights never decided what a validator earned, so no validator's income moves. Chain: - Delete set_root_weights (call 146), do_set_root_weights, its rate-limit arms, get_valid_basket_weights, the curated tao x w/sum(w) deploy branch, and the sell-and-redeploy dividend flow. The accumulate-in-place path is the only dividend path; the deposit engine is user-only and mirrors holdings. - Drop RootWeightSettingEnabled and sudo_set_root_weight_setting_enabled. - Repurpose the 1/16 concentration cap as BasketConcentrationCap (a StorageValue guarding swap_basket buys only); rename the setter to sudo_set_basket_concentration_cap (index 105) and the error to BasketConcentrationCapExceeded. - Dust sweep keys off actual holdings alone: every sub-threshold row is cashed (a swap_basket position is far above the threshold at entry). - Remove get_validator_root_weights, the get_validator_weights runtime API / RPC, and BasketSummary.weights (freeze hash updated). - Migration remove_root_weights_v1 clears Weights[ROOT], kills the gate, carries RootWeightsCap[ROOT] into BasketConcentrationCap, and drops the retired map. The three superseded weight migrations are removed; the NetworkUpdateWeightsOnRoot dissolution phase is a pass-through no-op. - Retired-but-kept for SCALE index stability: errors BasketHasNoWeights and RootWeightSettingDisabled, event RootWeightsSet. - Flush allowance re-derived for the single path (10x rows quotes); stake_into_basket declared weight sizes 256 mirrored slots. - spec_version 460 (459 is taken by the SharePool emergency runtime). SDK/docs: - Remove SetRootWeights, btcli root weights, the validator_root_weights read, and weights from the basket summary. Regenerate _generated (spec 460), golden.json, the shape corpus, the Ledger proof vector, and the metadata digest against a fast-runtime localnet built from this branch; codegen drift and static gates clean. - Rewrite the Root Reborn and basket-trading guides for the no-weights model; regenerate reference pages; move the release page to v460-upgrade; drop setRootWeights from ts-tests. Tests: 1557 pallet-subtensor + 86 admin-utils pass; clippy -D warnings clean; SDK pytest 1775 pass. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
…er proof Address the auditor findings on the previous commit: - Run migrate_remove_root_weights from the runtime `Migrations` tuple as an `OnRuntimeUpgrade` (like stamp_beta_baselines) instead of the pallet hook. `pre_upgrade` snapshots the retired cap, the new cap, and the non-root Weights row count; `post_upgrade` checks the run marker, an empty Weights[ROOT], the killed gate, the emptied retired map, the exact cap carry-over (default kept when governance never set one, untouched on a re-run), and that non-root weights survived. - Record polkadot_js_proof_hex from @polkadot-api/merkleize-metadata 1.3.1 on the same metadata, parts, and chain info instead of mirroring the Rust proof. The JS digest matches the Rust digest and the proof tails agree, so the vector cross-checks implementations again. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Registering the weights-removal migration in runtime/src/lib.rs shifted the runtime API implementation lines the generated query pages link to. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
The swap_basket band referenced only the slow emission EMA (a ~monthly half-life) and spot, and re-anchored to spot on every leg. After any real price move the slow EMA sits stale, so a compromised trader key could lift or dump spot inside a block and have the fund fill leg after leg at the manipulated price: 1.1 / 4.1 / 6.9% of NAV per day on the buy leg and 1.1 / 3.1 / 4.7% on the sell leg at slow-EMA/spot divergences of 1.35 / 2 / 4 (swarm E8: BK-11 F1, BK-03 F1/F2), far above the accepted ~1.3%/day ceiling. The concentration cap and turnover budget were also measured against a realizable NAV a same-block pump of a held thin pool could mark up to the pump size (BK-15 F2, BK-03 F3). - Add SubnetFastMovingPrice: an unclamped U64F64 EMA of spot with a 600-block (~2h) half-life, advanced right after the slow EMA in update_moving_price from the previous block's closing spot, seeded at spot on first update. No extrinsic can move it within a block. - Bound both legs to it: buy ceiling 1.02 x min(slow, fast, spot), sell floor 0.98 x max(slow, fast, spot). A subnet missing either EMA is refused (SlippageTooHigh), as before for the slow one. - Measure the turnover budget and the concentration cap against a guarded NAV that marks each holding at min(realizable, alpha x slow EMA); root cash passes through. One-sided by design: an over-EMA holding is under-marked, which only tightens the guards. basket_trading_status reports the budget from the same mark. Regression tests (tests/swap_basket_stale_ema.rs) replay the PoC sequences with the fast anchor held at the block's opening spot, and a control that re-pins it to live spot before every leg (the old band): buy leg control 113 / 415 / 691 bps of NAV -> fixed 0 bps, one in-band leg; sell leg control 114 / 330 / 507 bps -> fixed 0 bps; a 100k and a 1M TAO same-block pump of a held 10k/10k pool marks realizable NAV 10,825 -> 919,402 TAO while guarded NAV stays 10,900, and the 1,500 TAO (cap) and 9,500 TAO (budget) buys stay refused. Honest trades at spot still fill; a dip the fast EMA has followed is tradeable. Existing trading tests pin the fast EMA alongside the slow one; two "documents current behaviour" tests (one-block walk to the EMA ceiling, profit taking walking to the EMA floor) now assert the anchored behaviour. Docs: the band, cap, budget, and extraction-ceiling sections of the basket-trading guides and the v460 release page restate the anchors and re-derive the ceiling (same-block attacks ~0; the 1.3%/day bound applies only to a pump held ~10h against arbitrage). SDK bindings regenerated (new storage item); golden fixture and Ledger vector re-recorded (metadata digest unchanged). Co-authored-by: Arbos <unarbos@users.noreply.github.com>
…ings and docs Mainnet is on spec 459 (#3162) and hotfix 2 takes 460, so the basket runtime ships as 461. After the rebase: - runtime/src/lib.rs: spec_version 461; the Migrations tuple carries both #3162's root pot reconciliation and the root-weights removal. - Release page and guides: v460 -> v461. - SDK: bindings, golden fixture, shape corpus, Ledger proof vector, and the metadata digest (affbcc25...) re-recorded against a fast-runtime localnet built from this branch (spec 461); duplicate error-map and description entries from the auto-merge removed (main had already classified FundsNotSettled / OrderSignerFrozen); reference docs regenerated. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
Golden fixture and Ledger proof vector recorded against a localnet built from the rebased head (spec 461; digest unchanged). Generated bindings, namespaces stub, and reference pages verified in sync (no drift). Co-authored-by: Arbos <unarbos@users.noreply.github.com>
3c19ee5 to
5ce614d
Compare
| except Exception: | ||
| # Display-only context; a quoting hiccup (or a pre-v4 node) must not block the swap. | ||
| holdings, status, expected_out = [], None, None |
There was a problem hiding this comment.
[MEDIUM] Quote failures disable the requested slippage protection
root_swap.py:196–198 converts any context-read failure into an absent quote; the inner quote handler does the same. Submission then proceeds with the original intent's min_amount_out = 0, even with an explicit --max-slippage 0 or the default 1% limit. A transient failure in the status read alone therefore removes the user's protection. The runtime's 2% per-leg band does not enforce that requested output floor, and a warning on the review card does not protect unattended submission.
Abort when a required quote fails or produces no usable output, unless the user explicitly selected --max-slippage 100. Keep optional status-read failures separate from mandatory quoting.
|
🔄 AI review updated — Skeptic: VULNERABLE |
swap_basket now refuses a subnet without a fast price anchor, so the benchmark pools need it pinned alongside the slow EMA. Co-authored-by: Arbos <unarbos@users.noreply.github.com>
| except Exception: | ||
| # Display-only context; a quoting hiccup (or a pre-v4 node) must not block the swap. | ||
| holdings, status, expected_out = [], None, None |
There was a problem hiding this comment.
[MEDIUM] Quote failures disable the requested slippage protection
sdk/python/bittensor/cli/commands/root_swap.py:196-198: A failure reading holdings, trading status, or either swap quote leaves the original intent's min_amount_out at zero and still reaches app_ctx.submit. This also happens when the user explicitly requests a tight --max-slippage. The protocol's per-leg price bands do not enforce that requested bound relative to the user's quote, so a trade can execute at a worse output than authorized by that option. The review-card warning does not protect unattended submissions. Abort when a positive quote cannot be obtained unless the user explicitly selected --max-slippage 100; apply this to both exception paths.
|
🔄 AI review updated — Skeptic: VULNERABLE |
a7a3f19 to
495b72b
Compare
| except Exception: | ||
| # Display-only context; a quoting hiccup (or a pre-v4 node) must not block the swap. | ||
| holdings, status, expected_out = [], None, None |
There was a problem hiding this comment.
[MEDIUM] Quote failures disable the requested slippage protection
A failure reading holdings, trading status, or either quote leaves expected_out=None, so submission proceeds with the intent's default min_amount_out=0, even when the caller requested --max-slippage 1 (or a stricter limit). The inner exception handler at lines 188–190 does the same. The protocol's per-leg price bands do not enforce the user's quoted-output floor, so the trade can execute worse than the requested tolerance. A review-card warning does not preserve that constraint, particularly for unattended submissions. Abort when a usable quote is unavailable unless the caller explicitly selects --max-slippage 100; keep optional status-read failures separate from required quoting.
|
🔄 AI review updated — Skeptic: VULNERABLE |
495b72b to
a7a3f19
Compare
| except Exception: | ||
| # Display-only context; a quoting hiccup (or a pre-v4 node) must not block the swap. | ||
| holdings, status, expected_out = [], None, None |
There was a problem hiding this comment.
[MEDIUM] Quote failures disable the requested slippage protection
root_swap.py:186–198 catches quote and context-read failures and continues with the original intent, whose min_amount_out defaults to zero. Thus a transient RPC failure allows submission even with an explicit --max-slippage 1, without enforcing that requested floor. The protocol's per-leg price band does not enforce the user's quoted-output limit, and the review-card warning does not restore it. Abort when a usable quote is unavailable unless the user explicitly selected --max-slippage 100; apply this to both exception paths.
|
🔄 AI review updated — Skeptic: VULNERABLE |
Co-authored-by: Arbos <unarbos@users.noreply.github.com>
2 participants
Summary
Single PR that carries all
swap_basketwork ontomain. It replaces the stacked chain #3150 → #3152 → #3153 → #3154 (closed, branches kept; every superseded tip is an ancestor of this branch).Enables root validators to rebalance their beta basket via
swap_basket, with guardrails, a caller-signed minimum output, a bounded pending-deposit flush model, measuredWeightInfo,spec_version456, SDK/docs regen, and release notes.What ships
Feature (
swap_basket)swap_basket(hotkey, origin_netuid, destination_netuid, amount, min_amount_out): sell one escrow holding for TAO and buy another (netuid0= fund TAO cash slot, moved as one unit).min_amount_out: caller-signed floor on the destination leg (alpha ofdestination_netuid, TAO when it is 0).BasketMinOutNotMetwhen the executed output is below it; classifiedinsufficient_liquidityin the SDK.BasketTradingproxy (index 18) to a trader multisig; submit through MEV shield.swap_basketrequires theBasketTradingproxy.BasketTradingEnabled(global, default off),BasketTradingFrozen(per hotkey),BasketDailyTurnoverCap,BasketLiquidityCap.Guardrails (launch defaults)
min_amount_outSubnetAlphaIn); root exempt, selling never cappedAdversarial loss bound (with liquidity cap): ≈ 13% of turnover ≈ 1.3% of NAV per bucket / day sustained, assuming pools near the 0.5 balancer-weight equilibrium.
Flush weight model
BasketFlushWork { quotes, rows }: read-only NAV quotes are priced like NAV-sweep rows, executed rows (a swap plus stake/reserve/queue writes) like redeemedclaim_rootrows.swap_basket,stake_into_basket,claim_root,claim_root_with_hotkey) declares the same flat allowancebasket_flush_weight_bound()=8 * MAX_BASKET_ROWSquotes +2 * MAX_BASKET_ROWSrows, derived inbasket_flush_work_bound, and refunds the real work post-dispatch.RootClaimTooHeavy).Weights (reference-measured)
swap_basket(h)and the four basket admin setters were measured by theValidate-Benchmarksworkflow on the referenceBenchmarkingrunner (run 34758967315,run-benchmarkslabel, steps 50 / repeat 20, head0208953e9).swap_basketis taken verbatim from the workflow'sbench-patchartifact viascripts/benchmark_action.py selective-patch:794_300_412 + 86_269_632*href-time,47 + 10hreads,27writes,11_344 + 2_609hproof,h ∈ [1, 256](was the hand estimate60_000_000 + 20_000_000*h,24 + 8hreads,16writes).sudo_set_root_weights_capalready uses:trading_enabled3_890_000,trading_frozen4_922_000,daily_turnover_cap3_841_000,liquidity_cap3_893_000 (all r0 w1; were 4_916_000).pallet_subtensorentries in that patch pre-date this branch and are left to the nightly drift process. The label was removed after the artifact was retrieved.SDK / CLI
SwapBasketintent gainsmin_amount_out(Money, default 0);btcli root tradeis renamedbtcli root swapand adds--max-slippage(percent, default 1.0), quoting both legs and setting the floor toquote × (1 − pct/100).basket_trading_statusview, reads, error map (BasketMinOutNotMet,BasketDepositPending),_generated/bindings regenerated (codegen.check --drift: no drift).BasketMinOutNotMet); metadata digest and Ledger proof vector re-pinned; shape corpus re-recorded (unchanged).Release prep
spec_version455 → 456.BasketSwappedappended at the event enum tail (no mid-enum renumbering).RUSTSEC-2026-0269(wasmtime pin, same class as existing ignores); docs-preview lock overrides.Supersedes (closed, branches kept)
Breaking change
Runtime upgrade to spec 456.
swap_basketcall shape is five arguments (min_amount_outadded before any mainnet release of the call).Test plan
cargo test -p pallet-subtensor swap_basket(49),basket(135),claim_root(61);cargo test -p pallet-admin-utils basket(4);cargo test -p bittensor-core digest(4)sdk/python: 1793 offline unit tests pass against the re-recorded fixtures;ruff check/ruff format --checkcleancargo fmt --check --all,git diff --checkmain(skeptic advisory)swap_basket, band / floor / liquidity / turnover / concentration rejects