Add pallet-derivatives: 30-day longs and shorts on subnet alpha (spec 453) - #3135
Add pallet-derivatives: 30-day longs and shorts on subnet alpha (spec 453)#3135unarbos wants to merge 6 commits into
Conversation
… 453). Positions lift a slice of the subnet pool without moving price, trade one half through the ordinary swap, and reverse it at close; nothing is minted. Fees are fixed at open: shorts pay short_fee_per_day x pool share (pump risk scales with 1/T), longs pay long_rate_per_day x exposure. Includes roll, bounded expiry retries, dissolution unwind, SDK reads/intents, btcli deriv, the guide, and the v453 release page. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
eco-tests changed — indexer review requiredThis PR modifies files under Changed files
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| fn on_runtime_upgrade() -> Weight { | ||
| let _ = | ||
| T::Pool::register_pallet_hotkey(&Self::pallet_account(), &T::PalletHotkey::get()); |
There was a problem hiding this comment.
[CRITICAL] Custody hotkey can be preclaimed before the upgrade
This migration ignores the registration result and never verifies that PalletHotkey belongs to the pallet account. The hotkey address is public and deterministic, while create_account_if_non_existent is a no-op when it already exists. An attacker can therefore claim it before this runtime upgrade; subsequent derivative alpha is staked under an attacker-owned hotkey, which the owner can migrate through swap_hotkey. Abort the upgrade on an ownership collision or use a custody identity that cannot be externally claimed, and verify ownership before accepting positions.
There was a problem hiding this comment.
Fixed in 98f0797. PalletHotkey is no longer a compile-time constant. on_runtime_upgrade now calls claim_hotkey, which hashes (PalletId, "hotkey", parent_hash, nonce) into an address, skips any address that already exists, registers it to the pallet account, and only stores it after pallet_hotkey_registered confirms ownership. The address depends on the parent hash of the upgrade block, so it cannot be known before that block, and hooks run before any extrinsic in it. Until the storage is set every open fails with PalletHotkeyUnset. Tests: upgrade_claims_a_fresh_hotkey_for_the_pallet_account, claim_skips_a_hotkey_someone_registered_first, nothing_opens_until_the_hotkey_is_claimed.
| match &deposit { | ||
| Deposit::Tao(amount) => T::Pool::transfer_tao(&owner, &pallet_account, *amount)?, | ||
| Deposit::Alpha { hotkey, amount } => T::Pool::transfer_staked_alpha( | ||
| &owner, | ||
| hotkey, | ||
| &pallet_account, | ||
| &pallet_hotkey, | ||
| netuid, | ||
| *amount, | ||
| true, | ||
| false, | ||
| )?, | ||
| } | ||
|
|
||
| let (lifted_tao, lifted_alpha) = | ||
| T::Pool::lift_liquidity(netuid, phi, &pallet_account, &pallet_hotkey)?; | ||
| let legs = match side { | ||
| Side::Short => { | ||
| let proceeds = T::Pool::sell_alpha_internal( | ||
| &pallet_account, | ||
| &pallet_hotkey, | ||
| netuid, | ||
| lifted_alpha, | ||
| )?; | ||
| ensure!(!proceeds.is_zero(), Error::<T>::SwapReturnedZero); | ||
| Legs::Short { | ||
| proceeds, | ||
| debt: lifted_alpha, | ||
| escrow: lifted_tao, | ||
| } | ||
| } | ||
| Side::Long => { | ||
| let proceeds = T::Pool::buy_alpha_internal( | ||
| &pallet_account, | ||
| &pallet_hotkey, | ||
| netuid, | ||
| lifted_tao, | ||
| )?; | ||
| ensure!(!proceeds.is_zero(), Error::<T>::SwapReturnedZero); | ||
| Legs::Long { | ||
| proceeds, | ||
| debt: lifted_tao, | ||
| escrow: lifted_alpha, | ||
| } | ||
| } | ||
| }; | ||
|
|
||
| let now = frame_system::Pallet::<T>::block_number(); | ||
| let expires_at = Self::schedule_expiry( |
There was a problem hiding this comment.
[HIGH] Failed opens commit partial transfers and pool mutations
do_open transfers the cushion, lifts liquidity, and executes a swap before this fallible expiry-queue insertion, but neither open nor do_open establishes an outer storage transaction. If this or another later check fails, the extrinsic returns an error while those earlier mutations remain and no Position is recorded. Queue saturation makes this failure adversarially reachable. The same atomicity gap affects roll: settlement can commit before reopening fails. Wrap each complete open and roll operation in one transaction and add regression tests asserting all balances, reserves, stake, queues, and position state are unchanged on every late failure.
There was a problem hiding this comment.
Fixed in 98f0797. do_open now wraps its body in with_storage_layer, the same way do_settle already did, so the cushion transfer, the lift, and the opening swap roll back if the expiry-queue insert (or anything else) fails, regardless of caller. The open and roll extrinsics were already transactional as #[pallet::call] dispatchables, but the guarantee is now local to the function.
🛡️ AI Review — Skeptic (security review)VERDICT: SAFE MEDIUM scrutiny: five-month-old contributor with repository write access, substantial contribution history, no listed Gittensor association, and disclosed Cursor co-authorship; feat/derivatives → main. Since the prior SAFE verdict, the branch added a substantial TAO-only-cushion revision. Static review of that revision found the simplified open, settlement, roll, unwind, and pool-interface accounting remains atomic and does not introduce a security vulnerability. No AI-review trust-boundary files are changed. FindingsNo findings. ConclusionStatic review found no security vulnerability or evidence of hostile intent in the current head. 🔍 AI Review — Auditor (domain review)VERDICT: 👎 Gittensor association UNKNOWN; established high-volume contributor with repository write access. No substantive duplicate PR identified. The implementation matches the substantive PR description and bumps Findings
Prior-comment reconciliation
ConclusionBlock merge until benchmark-generated weights replace the placeholders. Dispatchables and automatic expiry processing require measured resource accounting. 📜 Previous run (superseded)
|
|
🔄 AI review updated — Skeptic: VULNERABLE |
…wn storage layer. A compile-time hotkey address is public, and hotkey registration is first come first served, so it could be claimed before the upgrade and later swapped away together with the pallet's stake. The hotkey is now hashed from the parent block hash in on_runtime_upgrade and registered in the same block, before any extrinsic can run. do_open runs in its own storage layer so a late failure rolls back the cushion transfer and opening swap for every caller. Also: try-runtime feature propagation, fmt, and the regenerated tx docs for the roll sample. Co-authored-by: Cursor <cursoragent@cursor.com>
| .cushion | ||
| .alpha_hotkey() | ||
| .cloned(); | ||
| let (tao_back, alpha_back) = Self::do_settle(&owner, netuid, side, Closer::Roll)?; |
There was a problem hiding this comment.
[HIGH] Failed rolls permanently settle the old position
do_settle commits its own storage layer before the top-up is validated or do_open runs. Dispatch errors do not automatically roll back earlier writes, so TopUpMismatch, a disabled side, a full expiry queue, a changed pool cap, or another reopen failure returns an error after closing the user's position and executing its settlement swaps. Wrap the entire settle-and-reopen sequence in one outer storage layer so any later error restores the original position and financial state.
|
🔄 AI review updated — Skeptic: VULNERABLE |
…tives errors. Bindings dumped from a node built from this branch (spec 453); reads and intents now use the generated Derivatives storage items and call builders. Every new chain error name is mapped to a semantic code and described, and the generated error pages and catalogs follow. Co-authored-by: Cursor <cursoragent@cursor.com>
| .cushion | ||
| .alpha_hotkey() | ||
| .cloned(); | ||
| let (tao_back, alpha_back) = Self::do_settle(&owner, netuid, side, Closer::Roll)?; |
There was a problem hiding this comment.
[HIGH] Failed rolls permanently settle the old position
do_settle commits its own storage layer before do_open runs. If deposit construction or reopening subsequently fails—for example with TopUpMismatch, DepositTooLow, PoolCapExceeded, or ExpiryQueueFull—roll returns an error but the old position has already been removed and paid out. Wrap the entire settle-and-reopen sequence in one outer storage layer so every later error rolls back settlement.
There was a problem hiding this comment.
Fixed in 0111d05. do_roll now runs settle and reopen inside one with_storage_layer, so any error from deposit construction or do_open rolls the settlement back too. failed_roll_leaves_the_old_position_untouched asserts the position, footprint, owner balance, and pool reserves are unchanged after a TopUpMismatch roll; roll_settles_and_reopens_with_the_payout covers the happy path.
|
🔄 AI review updated — Skeptic: VULNERABLE |
…ition. do_settle commits its own layer; without an outer one around settle and reopen, a reopen error would leave the position settled at the dispatch level only. Tests cover the happy path and the rollback. Co-authored-by: Cursor <cursoragent@cursor.com>
| //! These are hand-written placeholders sized from storage reads and writes so the pallet can be | ||
| //! wired up. CI's reference benchmark run replaces them with measured values. |
There was a problem hiding this comment.
[HIGH] Replace placeholder weights before merge
These unmeasured constants govern three swap-heavy dispatchables and are also used to meter on_idle expiry settlement and dissolution cleanup. If close() underestimates execution, automatic processing can consume more block resources than charged or metered. Run the reference benchmarks and commit their generated WeightInfo output before enabling this pallet; a future CI benchmark run is not sufficient protection for the code merged here.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
Co-authored-by: Cursor <cursoragent@cursor.com>
| //! These are hand-written placeholders sized from storage reads and writes so the pallet can be | ||
| //! wired up. CI's reference benchmark run replaces them with measured values. |
There was a problem hiding this comment.
[HIGH] Replace placeholder weights before merge
These estimates are used by open, close, roll, and the on_idle weight meter despite covering multi-swap and extensive storage paths. Unmeasured undercharging can admit more execution than a block's weight limit and let expiry processing overrun its budget. Run the pallet benchmarks on reference hardware and commit the generated WeightInfo implementation before enabling this pallet.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
Positions are backed by TAO from the coldkey only. Drops the Deposit enum, the alpha-cushion paths in open/settle/unwind (stake transfer to the pallet hotkey, sell-if-hotkey-gone fallback), transfer_stake_internal and the forget-hotkey benchmark hook, and TopUpMismatch. roll takes a plain TAO top-up. At 1x a long can no longer be underwater, and a subnet team cannot post self-minted alpha as collateral. Benchmarks now close a short after a pump (the exact-output buyback is the heaviest settle path). SDK intents, reads, CLI, generated bindings, and docs follow. Co-authored-by: Cursor <cursoragent@cursor.com>
| //! These are hand-written placeholders sized from storage reads and writes so the pallet can be | ||
| //! wired up. CI's reference benchmark run replaces them with measured values. |
There was a problem hiding this comment.
[HIGH] Replace placeholder weights before merge
These weights govern three stateful dispatchables and automatic expiry settlement, but are explicitly hand-written estimates. Underestimated execution or proof-size costs can let blocks exceed their resource limits. Run the pallet benchmarks on the reference hardware and commit the generated WeightInfo before merging; a future CI benchmark patch is not sufficient for release-ready runtime code.
|
🔄 AI review updated — Skeptic: SAFE Auditor: 👎 |
1 participant
Summary
pallet-derivatives(index 33): expiry-bounded longs and shorts on a subnet's alpha, borrowed from the subnet's own pool. A position lifts a slicephiof both reserves without moving price, trades one half through the ordinary swap, and reverses the trade at close. Nothing is minted or burned; the pool only ever gets its own liquidity back.open(side, deposit),close(owner any time, anyone after expiry),roll(atomic close-and-reopen), root-onlysudo_set_paramswith validation.on_idlesweeps expired positions with bounded retries; dissolution unwinds positions at par.short_fee_per_day (5 τ) × phi(pump cost in a CPMM scales with1/T), longs paylong_rate_per_day (0.02 %) × exposure. One-day minimum, paid to the pool at close. Underwater positions pay the owner nothing; the pool absorbs the shortfall, bounded bymax_pool_share(10 %).DerivativesPoolInterfaceinswap-interfacewith exact-output swaps (buy_alpha_for/sell_alpha_for) implemented inpallet-subtensor;exp_scaledin the balancer now saturates instead of returning 0.SubnetDissolveHookandCommitmentsInterfacemerged into one dissolution hook.NonCriticalAllowedonly (they move funds).derivative_position(s)andderivatives_paramsreads;OpenShort,OpenLong,RollPosition,ClosePositionintents;btcli deriv short|long|list|roll|close|params.docs/guides/derivatives.mdxwith animated lifecycle and payoff figures, generated query/tx pages, and the v453 release page.spec_version452 → 453 (Finney is already on 452).Weights
pallets/derivatives/src/weights.rsholds hand-written placeholders. Benchmarks cover the worst-case paths (openwith full expiry queues,closewith an alpha cushion and a vanished hotkey,rollas both). Needs the CI benchmark run before release.Test plan
cargo test -p pallet-derivatives(26) and the subtensor, swap, runtime, admin-utils, transaction-fee, precompiles, commitments suites locallycargo clippy --workspace --all-targetswith and without--all-features,-D warningscargo fmt --check --allgenerate.py --checkMade with Cursor