Skip to content

Add pallet-derivatives: longs and shorts on subnet alpha (spec 455) - #3135

Open
unarbos wants to merge 19 commits into
mainfrom
feat/derivatives
Open

Add pallet-derivatives: longs and shorts on subnet alpha (spec 455)#3135
unarbos wants to merge 19 commits into
mainfrom
feat/derivatives

Conversation

@unarbos

@unarbos unarbos commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Summary

  • New pallet-derivatives (index 33): longs and shorts on a subnet's alpha, borrowed from the subnet's own pool. A position lifts a slice phi of both reserves without moving price, trades one half through the ordinary swap, and reverses the trade at settlement. Nothing is minted or burned; the pool only ever gets its own liquidity back.
  • One position per coldkey and subnet, one call to move it. add(netuid, side, deposit, leverage_percent) opens, grows, reduces, flips, and rolls. Adding on the held side folds a new tranche in; adding on the other side settles that share at the current price; asking for more than is held closes and opens the rest on the new side. close settles in full. Every quantity in a position is a plain sum over its tranches; do_add runs in one storage layer.
  • Leverage is the owner's choice under a per-side ceiling: max_short_leverage_percent 100 (1x), max_long_leverage_percent 200 (2x). At 1x a long can never cost the pool anything; at 2x its worst case (a halving) is as rare as the doubling that wipes a 1x short.
  • Cushions are TAO or alpha. Deposit::Tao / Deposit::Alpha { hotkey, amount }; the position's Cushion holds both and the hotkey the alpha returns to. Alpha cushions are gated per side by alpha_cushion_shorts / alpha_cushion_longs, off at launch, so a subnet team cannot post alpha it minted to itself as collateral. Alpha comes back in kind, or is sold to TAO if the hotkey is gone.
  • One fee law, both sides. fee_per_day = rate_per_day (0.05 %) × TAO exposure, fixed per tranche when added. One day is booked at each add, the rest accrues per block; every settlement pays fee_accrued to the pool. A 2x long pays twice a 1x short of the same cushion. Per-subnet overrides can set rate_per_day for a pool the flat rate underprices.
  • A 90-day term. expires_at = first add + lifetime_blocks (648,000). Adding does not move it. After expiry anyone may close and is paid one day of fee (Closer::Expired), the owner getting the rest; the owner's own same-side add is a roll: settle at today's price, reopen from the deposit, one transaction. Expiry never blocks a reduce or flip.
  • Health inside the term. A position is healthy while its equity (cushion value + proceeds − debt at the pool's quote − fee owed) covers one more day of fee. Below that anyone may close it (Closer::Liquidator) and is paid the fee owed plus what is left, topped up by the pool to one day of fee; the owner gets nothing. No chain sweep; permissionless work. OwnerOnly otherwise.
  • Underwater rule. If the closing trade cannot repay the debt, the owner is paid nothing for that share and the pool takes everything the pallet held for it. Enforced at settlement, not inferred from quotes.
  • Emission ignores longs. The price the emission EMA tracks is computed with every open long's alpha counted back into the pool (get_emission_alpha_price), so a team cannot long its own subnet for emission. Shorts are left in.
  • Dissolution settles every position first, at one price: shorts' alpha debt netted against longs' alpha, the difference quoted once against the pool (DissolutionPriced), every position settled at that ratio with no further swap.
  • Root: sudo_set_params (rejects zero leverage ceiling, cap, rate, or lifetime) and sudo_set_subnet_override(netuid, Option<SubnetOverride>) (pause a side, replace the cap or rate on one subnet; adds read it, settlements ignore it).
  • Plumbing: DerivativesPoolInterface in swap-interface with exact-output swaps (buy_alpha_for / sell_alpha_for), transfer_stake_internal, tao_out_for_alpha; exp_scaled in the balancer saturates instead of returning 0; one dissolution hook trait; pallet hotkey claimed in on_runtime_upgrade from the parent block hash; derivatives calls NonCriticalAllowed only.
  • SDK: AddPosition (deposit_in, hotkey_ss58, leverage) and ClosePosition intents; derivative_position(s), derivative_positions_on_subnet, derivatives_params, derivatives_subnet_override reads with expires_at, expired, equity_tao, healthy; btcli deriv short|long [--in alpha --hotkey] |list|closable|close [--owner]|params [--netuid]. Bindings regenerated from the built node.
  • Docs: docs/guides/derivatives.mdx with the animated lifecycle deck and payoff figure, generated query/tx/error pages, and the v455 release page.
  • spec_version 454 → 455.
  • CI hygiene folded in: cargo audit accepts RUSTSEC-2026-0269 (wasmtime 8.0.1, same polkadot-sdk pin as the existing wasmtime ignores; the runtime WASM has no filesystem), and the docs-preview lock pins fast-uri 3.1.7.

Weights

pallets/derivatives/src/weights.rs holds hand-written placeholders. Benchmarks cover the worst-case paths: add as a flip of a short left underwater by a pump, close as a liquidation of the same (the exact-output buyback runs every pass, then forfeits, and the pool tops the liquidator up). Needs the run-benchmarks CI run before release.

Test plan

  • cargo test -p pallet-derivatives (54) and the benchmark test suite with runtime-benchmarks; runtime and pallet-subtensor compile
  • cargo fmt --check --all
  • SDK: ruff, pytest (1,828), codegen.check --coverage|--names|--drift against a spec-455 dev node
  • Website: tsc, generate.py --check
  • CI: try-runtime against the mainnet snapshot, WASM build, benchmarks

Made with Cursor

@vercel

vercel Bot commented Sep 2, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
subtensor Ready Ready Preview Sep 10, 2026 6:53pm UTC

Request Review

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

eco-tests changed — indexer review required

This PR modifies files under eco-tests/. and may affect downstream indexing.
cc @evgeny-s — please review manually

Changed files
  • eco-tests/src/mock.rs

@github-actions
github-actions Bot requested a review from evgeny-s September 2, 2026 19:47

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/derivatives/src/lib.rs Outdated
Comment on lines +203 to +205
fn on_runtime_upgrade() -> Weight {
let _ =
T::Pool::register_pallet_hotkey(&Self::pallet_account(), &T::PalletHotkey::get());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +138 to +186
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(

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: VULNERABLE

MEDIUM contributor risk: six-month account, substantive merged history, write access, disclosed Cursor co-authorship, no listed Gittensor association; feat/derivatives → main.

Two prior concerns remain; two affected mechanisms have been removed. The replacement dissolution pricing introduces a separate extraction path. No AI-review trust-boundary files changed.

Static checks passed: git diff --check 67dcf7f791dc495064c293f080a0702cb433e51e HEAD, git diff --check, and git status --short (clean). Builds, tests, and formatters were excluded by Skeptic constraints; actionlint was unavailable.

Findings

Sev File Finding
HIGH pallets/derivatives/src/lib.rs:380 Dissolution pays traders for their own price impact inline
HIGH pallets/derivatives/src/settle.rs:213 Pool-cap estimate still assumes equal Balancer weights inline
HIGH pallets/derivatives/src/settle.rs:318 Partial swaps leave refunded assets without position accounting inline

Prior-comment reconciliation

  • e2ae0bbd: not addressed — The equal-weight projection remains the only cap check; actual footprint is recorded afterward.
  • f5709349: not addressed — Swap helpers still refund unused input without exposing it to position settlement accounting.
  • c70aab79: no longer applies — Alpha collateral has been removed; cushions are now TAO-only.
  • 7aabd214: no longer applies — The unweighted balanced-netting calculation was removed. Its replacement uses Balancer spot pricing, which introduces the separate price-impact finding above.

Conclusion

Pool-cap enforcement, refund accounting, and manipulable dissolution pricing expose pool funds and position assets to loss. No evidence of malicious intent was found.


📜 Previous run (superseded)
Sev File Finding Status
HIGH pallets/derivatives/src/settle.rs:305 Pool-cap estimate assumes equal Balancer weights ➡️ Carried forward to current findings
The equal-weight projection remains the only cap check; actual footprint is recorded afterward.
HIGH pallets/derivatives/src/settle.rs:444 Partial closing sales leave untracked alpha in custody ➡️ Carried forward to current findings
Swap helpers still refund unused input without exposing it to position settlement accounting.
HIGH pallets/derivatives/src/settle.rs:651 Dissolution returns alpha collateral before covering liabilities ⏭️ No longer applies
Alpha collateral has been removed; cushions are now TAO-only.
HIGH pallets/subtensor/src/staking/derivatives_pool.rs:360 Balanced dissolution still ignores Balancer weights ⏭️ No longer applies
The unweighted balanced-netting calculation was removed. Its replacement uses Balancer spot pricing, which introduces the separate price-impact finding above.

🔍 AI Review — Auditor (domain review)

VERDICT: 👎

Gittensor association UNKNOWN; established high-volume contributor with repository write access. No substantive duplicate PR identified.

The implementation and substantive PR description agree, and spec_version is bumped to 455. The PR title still references spec 453 and should be updated.

git diff --check passed and the working tree is clean. No build was run because the blocking weight issue is statically confirmed.

Findings

Sev File Finding
HIGH pallets/derivatives/src/weights.rs:4 Replace placeholder weights before merge inline

Prior-comment reconciliation

  • f5046818: not addressed — The file still explicitly identifies its weights as hand-written placeholders.

Conclusion

Block merge until benchmark-generated weights replace the placeholders. Accurate accounting is required for dispatchables and automatic settlement processing.


📜 Previous run (superseded)
Sev File Finding Status
HIGH pallets/derivatives/src/weights.rs:4 Replace placeholder weights before merge ➡️ Carried forward to current findings
The file still explicitly identifies its weights as hand-written placeholders.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/derivatives/src/settle.rs Outdated
.cushion
.alpha_hotkey()
.cloned();
let (tao_back, alpha_back) = Self::do_settle(&owner, netuid, side, Closer::Roll)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/derivatives/src/settle.rs Outdated
.cushion
.alpha_hotkey()
.cloned();
let (tao_back, alpha_back) = Self::do_settle(&owner, netuid, side, Closer::Roll)?;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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 ExpiryQueueFullroll 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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +3 to +4
//! 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +3 to +4
//! 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +3 to +4
//! 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[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.

@github-actions

github-actions Bot commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +3 to +4
//! 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Replace placeholder weights before merge

This remains explicitly placeholder resource accounting for new economic dispatchables and automatic on_idle/dissolution work. Storage read/write estimates do not establish execution time or proof size, and underestimated weights can admit excessive block work. Run the reference benchmarks and commit their generated WeightInfo output before merging; merely scheduling a future CI benchmark is insufficient for a merge-ready runtime.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +3 to +4
//! 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.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Replace placeholder weights before merge

These weights are explicitly handwritten estimates. They meter the new dispatchables as well as on_idle expiry sweeping and resumable subnet dissolution, so an underestimate can permit substantially more pool and storage work than the block budget accounts for. Run the existing benchmarks on reference hardware and commit their generated WeightInfo output before enabling this pallet.

@github-actions

github-actions Bot commented Sep 4, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👎

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

A long leaves the pool's TAO and takes alpha out, which lifts the spot
price while it is open. Emission is weighted by the price EMA, so a team
could long its own subnet and be paid for it. The EMA now tracks
get_emission_alpha_price: the spot price with the long-side Footprint
added back to the alpha reserve, which is the price the pool would show
if every long returned its slice in kind. Shorts are not adjusted for.

DerivativesHook (common) carries long_alpha_outstanding into
pallet-subtensor; SwapHandler gains alpha_price_for_reserves.

Co-authored-by: Cursor <cursoragent@cursor.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +240 to +245
let cap = max_pool_share.mul_floor(lent_reserve);
let projected = projected_footprint(phi, lent_reserve);
ensure!(
Footprint::<T>::get(netuid, side).saturating_add(projected) <= cap,
Error::<T>::PoolCapExceeded
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Pool-cap estimate assumes equal Balancer weights

pallets/derivatives/src/settle.rs:240-245 enforces the cap using phi * (2 - phi) * reserve, which assumes equal weights. The actual swap uses the subnet's Balancer weights, and the resulting legs.footprint() is recorded without another cap check. For example, with a short, base/quote weights of 80/20 and phi = 5%, the estimate is 9.75% of the original TAO reserve, but the actual footprint is 1 - 0.95^5 ≈ 22.62%. This passes the default 10% cap while removing over twice the intended reserve share. Check the actual aggregate footprint against the captured cap inside the existing storage transaction before committing, or use a conservative weight-aware bound.

Comment on lines +344 to +349
pot = pot.saturating_add(T::Pool::sell_alpha_internal(
&pallet_account,
&pallet_hotkey,
netuid,
proceeds,
)?);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Partial closing sales leave untracked alpha in custody

pallets/derivatives/src/settle.rs:344-351 treats sell_alpha_internal(proceeds) as consuming all proceeds. The swap engine can successfully fill only part of the sale at its minimum price; sell_alpha_internal refunds unused alpha to the pallet's stake but returns only the TAO output. Settlement returns only escrow alpha to the pool, then removes the entire requested proceeds amount from the position and footprint. Both full closes and partial reductions therefore leave refunded alpha without position accounting, losing the owner's claim and potentially reporting a pool shortfall despite retained assets. Return the actual alpha consumed from the helper and explicitly retain or settle every unsold unit before reducing the position.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Positions on a dissolving subnet were cancelled at par, which cancelled
a short's gain on the one event it exists to capture. They are now
cash-settled, before any staker is paid, at the dissolution price: the
pool's TAO over every alpha the payout counts, read once with all open
positions netted out (short footprint back in the TAO, long footprint
back in the alpha), so no position moves the price it settles at.

A short's alpha debt is charged in TAO at that price; a long's alpha is
handed to the pool and credited at it. Fees are paid. Underwater
positions forfeit as at any settlement. DissolutionTotals holds the
fixed pair across blocks and DissolutionPriced reports it.

Co-authored-by: Cursor <cursoragent@cursor.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +274 to +277
let cap = max_pool_share.mul_floor(lent_reserve);
let projected = projected_footprint(phi, lent_reserve);
ensure!(
Footprint::<T>::get(netuid, side).saturating_add(projected) <= cap,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Pool-cap estimate assumes equal Balancer weights

pallets/derivatives/src/settle.rs:274–277 checks phi * (2 - phi) * reserve, which assumes equal pool weights. Dynamic pools support unequal Balancer weights, so the opening swap can withdraw substantially more than projected. For example, a short lifting 5% with an alpha/TAO weight ratio of 4 passes the 10% cap using a 9.75% projection, but its escrow plus proceeds consumes approximately 22.62% of the original TAO reserve. The actual legs.footprint() is subsequently booked without another cap check, exposing more pool liquidity than authorized. Check the actual aggregate footprint against the captured cap inside the existing storage transaction, and cover unequal weights in regression tests.

Comment on lines +378 to +383
pot = pot.saturating_add(T::Pool::sell_alpha_internal(
&pallet_account,
&pallet_hotkey,
netuid,
proceeds,
)?);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Partial closing sales leave untracked alpha in custody

pallets/derivatives/src/settle.rs:378–383 assumes all proceeds were sold. However, sell_alpha_internal refunds unsold alpha to the custody stake when the swap hits its minimum price and returns only the TAO output. Settlement returns only escrow alpha to the pool, then deletes the position or subtracts the entire settled part and its footprint. A long closed or reduced near the price floor therefore loses accounting for its unsold alpha, stranding assets and understating outstanding pool liquidity. Return the actual alpha consumed from the swap helper and explicitly retain or settle the remainder before releasing its accounting.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Shorts' alpha debt is netted against longs' alpha holdings and only the
difference is quoted against the pool, exactly as its swap would price it.
The quote is fixed before the first position settles and every position
settles at it, so a lone position gets what close would have paid in that
block and offsetting positions pay no slippage. AlphaToSettle tracks the
per-side alpha book alongside Footprint; SwapHandler gains the exact sell
quote tao_out_for_alpha.

Co-authored-by: Cursor <cursoragent@cursor.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +274 to +277
let cap = max_pool_share.mul_floor(lent_reserve);
let projected = projected_footprint(phi, lent_reserve);
ensure!(
Footprint::<T>::get(netuid, side).saturating_add(projected) <= cap,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Pool-cap estimate assumes equal Balancer weights

pallets/derivatives/src/settle.rs:274–277 checks phi * (2 - phi) * reserve, which assumes equal pool weights. For a short with alpha/TAO weight ratio 9 and phi = 5%, the projected footprint is 9.75% of the TAO reserve, but escrow plus sale proceeds is approximately 1 - 0.95^10 = 40.1%. This passes the default 10% cap and then books the much larger actual footprint without another check, defeating the pool's exposure bound.

Check the existing footprint plus legs.footprint() against the captured cap after the opening swap, inside the existing storage transaction, or use a conservative weight-aware bound.

Comment on lines +381 to +386
pot = pot.saturating_add(T::Pool::sell_alpha_internal(
&pallet_account,
&pallet_hotkey,
netuid,
proceeds,
)?);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Partial closing sales leave untracked alpha in custody

pallets/derivatives/src/settle.rs:381–386 treats the entire proceeds amount as sold. However, sell_alpha_internal refunds unused alpha to the pallet's stake when the swap reaches its minimum price and returns only the TAO output. Settlement returns only escrow alpha to the pool, then removes the entire settled proceeds from position and aggregate accounting.

A long closed or reduced near the minimum price can therefore strand unsold alpha in custody while recording an inflated shortfall and reducing the owner's payout. Return the actual alpha consumed from the swap interface and explicitly retain or return the remainder before retiring the corresponding position accounting.

Comment on lines +296 to +297
} else {
<Self as DerivativesPoolInterface<T::AccountId>>::reserves(netuid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Netted dissolution ignores Balancer weights

pallets/subtensor/src/staking/derivatives_pool.rs:296–297 uses TAO reserve / alpha reserve as the settlement price when short debt exactly matches long proceeds. Balancer spot price also includes weight_alpha / weight_tao. With valid weights of 0.1/0.9, this branch values alpha at nine times its actual spot price, while either neighboring nonzero-net branch uses weighted quotes.

The resulting cash settlement overcredits longs and overcharges shorts; because short losses stop at their available pot, artificial shortfalls can become pool losses without a corresponding market move. Use the actual weighted spot price for the zero-net case and cover offsetting positions in an unequal-weight pool.

@github-actions

github-actions Bot commented Sep 7, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

The fee is one rate for both sides, rate_per_day (0.05%) of a tranche's
TAO exposure, fixed when the tranche is added. This replaces the short
C*phi premium, the long rate, and the (1 - phi)^-4 size factor.

Positions expire lifetime_blocks (90 days) after their first add; adding
does not move the expiry. After it anyone may close the position and is
paid one day of fee (Closer::Expired), the owner getting the rest. An
owner's same-side add on an expired position is a roll: it settles at
today's price and reopens from the deposit in one storage layer.

Cushions can be TAO or alpha (Deposit enum, Cushion struct holding both
and the hotkey the alpha returns to); alpha cushions are gated per side
by alpha_cushion_shorts / alpha_cushion_longs, off by default. Subnet
overrides can set rate_per_day. Healthy -> OwnerOnly.

SDK: AddPosition takes deposit_in and hotkey_ss58; reads expose the
cushion legs, expires_at, expired, blocks_left; btcli deriv --in alpha,
closable replaces unhealthy. Bindings regenerated from the built node.
Guide, release page, and lifecycle deck updated; reference docs
regenerated.

Co-authored-by: Cursor <cursoragent@cursor.com>
@unarbos unarbos changed the title Add pallet-derivatives: 30-day longs and shorts on subnet alpha (spec 455) Add pallet-derivatives: longs and shorts on subnet alpha (spec 455) Sep 8, 2026
@unarbos unarbos closed this Sep 8, 2026
@unarbos unarbos reopened this Sep 8, 2026
…rated bindings.

Mainnet shipped 455, so this release moves to 456; with an equal spec the
clone upgrade is a no-op and the bindings read as drifted. Proxy filter tests
take the union of both sides' denylists (indirect-value pallets from main,
DerivativesCalls from this branch). lifetime_blocks default uses
saturating_mul for clippy::arithmetic_side_effects.

Co-authored-by: Cursor <cursoragent@cursor.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +312 to +315
let cap = max_pool_share.mul_floor(lent_reserve);
let projected = projected_footprint(phi, lent_reserve);
ensure!(
Footprint::<T>::get(netuid, side).saturating_add(projected) <= cap,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Pool-cap estimate assumes equal Balancer weights

pallets/derivatives/src/settle.rs:312-315 checks phi * (2 - phi) * reserve, which assumes equal pool weights. The actual borrowed footprint includes the weighted swap output and is recorded afterward without checking the cap again. For example, a short with alpha/TAO weights 80/20 and phi = 0.05 passes the 10% cap with a 9.75% estimate, but removes approximately 1 - 0.95^5 = 22.62% of the original TAO reserve. This exceeds the configured pool-loss exposure limit. Enforce the cap against the actual resulting footprint within the existing transaction, or use a conservative weighted projection.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +450 to +455
pot.tao = pot.tao.saturating_add(T::Pool::sell_alpha_internal(
&pallet_account,
&pallet_hotkey,
netuid,
proceeds,
)?);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Partial closing sales leave untracked alpha in custody

pallets/derivatives/src/settle.rs:450-455 treats all proceeds as sold. However, the swap can stop at its price limit, and sell_alpha_internal refunds unused alpha to the pallet's stake while returning only the TAO output. Settlement then removes the entire settled leg and footprint without returning or tracking that refund, stranding position assets. Pot::cash_out similarly clears all alpha, and swap_until reports requested input as spent even when refunded. Propagate actual consumed input through these helpers and retain or distribute every unswapped remainder before deleting its accounting.

let sell = alpha_held.saturating_sub(alpha_owed);
(T::SwapInterface::tao_out_for_alpha(netuid, sell), sell)
} else {
<Self as DerivativesPoolInterface<T::AccountId>>::reserves(netuid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Balanced dissolution still ignores Balancer weights

pallets/subtensor/src/staking/derivatives_pool.rs:360 returns TAO reserve / alpha reserve when aggregate short debt exactly matches long proceeds. Balancer spot is instead (w_alpha / w_tao) * TAO reserve / alpha reserve. Nonzero net exposure now uses weighted quotes, but exact cancellation still settles every position at a different price—for 80/20 weights, one quarter of spot. Netting cancels the aggregate alpha trade, not the individual owners' claims; this misallocates payouts and can create pool-funded shortfalls. Use the weighted spot price for the zero-net case.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +670 to +677
let alpha_to_owner = Self::return_alpha(
&pallet_account,
&pallet_hotkey,
owner,
position.cushion.alpha_hotkey.as_ref(),
netuid,
position.cushion.alpha,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Dissolution returns alpha collateral before covering liabilities

pallets/derivatives/src/settle.rs:670-677 returns the entire alpha cushion before collecting debt and fees. Successfully returned alpha is excluded from the remaining settlement pot, so an underwater position keeps its collateral while the pool absorbs the shortfall; even a solvent position can avoid fees that its alpha cushion should cover. This path becomes reachable when root enables alpha cushions, which are off by default. Retain collateral until debt and fees are settled at the dissolution price, and return only the surplus, preserving the ordinary settlement's underwater-forfeiture rule.

@github-actions

github-actions Bot commented Sep 8, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

The pool rents out at most max_pool_share of itself at rate_per_year of
exposure, the same for shorts and longs. Those two numbers are the design.

- Drop lifetime_blocks, expires_at, Closer::Expired and the roll path. A
  position lives until its owner closes it or it can no longer pay one day
  of rent; only then may anyone else close it (OwnerOnly otherwise).
- rate_per_day -> rate_per_year (Perbill, default 20%). The position still
  carries fee_per_day = rate_per_year * exposure / 365, so accrual, health,
  and the liquidator bounty are unchanged.
- SubnetOverride.rate_per_day -> rate_per_year.
- SDK reads/intents/CLI drop expires_at, blocks_left, expired and the
  expires column; OwnerOnly classifies as not_authorized.
- Guide, release page, lifecycle and payoff figures, regenerated docs and
  bindings.

Co-authored-by: Cursor <cursoragent@cursor.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +302 to +305
let cap = max_pool_share.mul_floor(lent_reserve);
let projected = projected_footprint(phi, lent_reserve);
ensure!(
Footprint::<T>::get(netuid, side).saturating_add(projected) <= cap,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Pool-cap estimate assumes equal Balancer weights

pallets/derivatives/src/settle.rs:302-305 checks the constant-product projection phi * (2 - phi) * reserve, but the opening swap uses variable Balancer weights. For a long with phi = 0.05 and quote/base weight ratio 9, this predicts 9.75% of the alpha reserve while the actual footprint is approximately 0.05 + 0.95 * (1 - 0.95^9) = 40.1%. The default 10% cap therefore accepts a position that removes roughly 40% of that reserve. Recording the actual footprint afterward does not reject the opening transaction. Enforce the cap against the actual resulting legs inside the existing storage transaction, or use a conservative weighted quote.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +439 to +444
pot.tao = pot.tao.saturating_add(T::Pool::sell_alpha_internal(
&pallet_account,
&pallet_hotkey,
netuid,
proceeds,
)?);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Partial closing sales leave untracked alpha in custody

pallets/derivatives/src/settle.rs:439-444 treats the long's entire proceeds as sold. However, sell_alpha_internal restores unused alpha when the swap reaches its price limit and returns only the TAO output. Settlement then removes the position's accounting without returning or recording that residual alpha, stranding assets in pallet custody. Pot::cash_out likewise clears all alpha, and swap_until counts requested input as spent despite refunds. Return actual input consumption from the swap helpers and preserve, return, or forfeit every unswapped remainder explicitly.

Comment thread pallets/derivatives/src/settle.rs Outdated
Comment on lines +644 to +651
let alpha_to_owner = Self::return_alpha(
&pallet_account,
&pallet_hotkey,
owner,
position.cushion.alpha_hotkey.as_ref(),
netuid,
position.cushion.alpha,
);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Dissolution returns alpha collateral before covering liabilities

pallets/derivatives/src/settle.rs:644-651 returns the entire alpha cushion before collecting debt and fees. When alpha cushions are enabled and the destination hotkey exists, an underwater position therefore recovers its collateral while the pool absorbs the unpaid liability. Ordinary settlement consumes this collateral before paying the owner, so dissolution bypasses that protection. Value and apply the cushion toward debt and fees first, then return only the remaining alpha.

let sell = alpha_held.saturating_sub(alpha_owed);
(T::SwapInterface::tao_out_for_alpha(netuid, sell), sell)
} else {
<Self as DerivativesPoolInterface<T::AccountId>>::reserves(netuid)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Balanced dissolution still ignores Balancer weights

pallets/subtensor/src/staking/derivatives_pool.rs:360 returns TAO reserve / alpha reserve when aggregate short debt exactly matches long proceeds. Balancer spot is (base_weight / quote_weight) * TAO reserve / alpha reserve. Consequently, exact cancellation settles every position at a different price from the weighted quotes used for nonzero net exposure, misallocating payouts and potentially shifting an undercollateralized side's losses onto the pool. Use the weighted spot price for the zero-net case and verify continuity with small net buys and sells.

@github-actions

github-actions Bot commented Sep 9, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

…nly close

The pool lends out at most pool_share of itself per side at interest_rate on
TAO exposure; both 25%, both root-set. Everything else is a constant.

- Params shrink to pool_share and interest_rate. Leverage ceilings (1x short,
  2x long) and the 0.1 TAO minimum deposit become runtime constants.
  pool_share = 0 is the pause. SubnetOverride, the enable switches, and
  InvalidParams are gone.
- Cushions are TAO only. Deposit, Cushion, the alpha-selling settlement paths,
  sell_alpha_for, transfer_stake_internal, and --in/--hotkey are removed.
- No liquidation. Only the owner can close; close(netuid) loses its owner
  argument. equity, is_healthy, the pool quotes, and OwnerOnly are gone.
- Interest accrues per block with nothing booked up front. Once a week, on
  its own block, each position's interest is taken from its cushion, spent
  buying alpha, and the alpha recycled: interest is buy pressure on either
  side. The same path pays interest at an owner's close or reduce. A
  block-indexed queue (Due, NextDue) does a bounded number of collections
  per block and catches up after a stall. A cushion that cannot pay its week
  is forfeited to the pool in kind, with no swap (Closer::Starved).
- Sizing bounds both tokens against live and smoothed reserves (the moving
  price projected on today's depth), and the cap uses the tighter reserve,
  so a same-block spot swap buys neither a bigger slice nor more room.
- Dissolution cash-settles every position at the pool's spot price with no
  swap; AlphaToSettle and the netted quote are removed.
- rent -> interest throughout. SDK reads expose due and runway_days; the CLI
  is short, long, close, list, params. Guide, release page, figures, bindings,
  and generated docs updated.

Co-authored-by: Cursor <cursoragent@cursor.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AI review — see the sticky summary comment for the verdict and the inline comments below for specific findings.

Comment on lines +210 to +213
let cap = params.pool_share.mul_floor(live_lent.min(smoothed_lent));
let projected = projected_footprint(phi, live_lent);
ensure!(
!phi.is_one() && Footprint::<T>::get(netuid, side).saturating_add(projected) <= cap,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Pool-cap estimate still assumes equal Balancer weights

projected_footprint uses phi * (2 - phi) * reserve, but the actual opening swap uses unequal Balancer weights. For a short with base/quote weight ratio 2 and phi = 0.1, the estimate is 0.19T, while escrow plus actual proceeds is 0.271T: the default 0.25T cap accepts it. The smoothed-reserve bound does not eliminate this case. Actual footprint is booked afterward without another check, allowing borrowing beyond the pool's configured loss bound. Enforce the cap against the actual resulting footprint inside the existing storage transaction, or use a conservative weight-aware projection.

Comment on lines +313 to +318
pot = pot.saturating_add(T::Pool::sell_alpha_internal(
&pallet_account,
&pallet_hotkey,
netuid,
proceeds,
)?);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Partial swaps leave refunded assets without position accounting

When a closing sale reaches the swap's minimum price, sell_alpha_internal restores unused alpha to pallet custody but returns only TAO output. Settlement nevertheless removes the entire proceeds leg and returns only escrow alpha, leaving the refunded alpha untracked and unavailable to the owner or pool. The same interface problem affects TAO refunds: swap_until counts requested input as spent, and burn_interest treats a successful partial buy as consuming all interest. Return actual input/output amounts and explicitly retain, return, or settle every unused balance before removing its accounting.

Comment on lines +377 to +380
let price = DissolutionPrice::<T>::get(netuid).unwrap_or_else(|| {
meter.consume(per_position);
let price = T::Pool::spot_price(netuid);
DissolutionPrice::<T>::insert(netuid, price);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[HIGH] Dissolution pays traders for their own price impact

The settlement price includes the opening trade's price impact, but cash settlement never reverses that trade. A trader can open a short immediately before a pending dissolution or registration-driven pruning and collect that self-created decline from the pool. In an equal-weight 1,000 TAO pool, a 10% lift sells for 90 TAO while the resulting spot values its alpha debt at 81 TAO, yielding approximately 9 TAO without an external price move. The added a_lone_short_that_moved_the_price_keeps_that_move_at_dissolution test explicitly asserts this gain. Use settlement pricing that accounts for unwinding the outstanding inventory rather than paying at the manipulable terminal spot; otherwise derivative payouts drain funds before ordinary stake settlement.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant