Skip to content

Handle balancer misbalance on injection#2758

Open
gztensor wants to merge 10 commits into
devnet-readyfrom
fix/handle-misbalance-on-injection
Open

Handle balancer misbalance on injection#2758
gztensor wants to merge 10 commits into
devnet-readyfrom
fix/handle-misbalance-on-injection

Conversation

@gztensor

@gztensor gztensor commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

Description

Fixes balancer emission injection when a block's TAO/alpha liquidity would push swap balancer weights outside the allowed range. Instead of dropping the whole injection attempt, the swap pallet now stores non-price-active TAO or alpha in per-subnet reservoirs and retries those reservoirs on later injection updates.

What Changed

  • Updated SwapHandler::adjust_protocol_liquidity to return separate price-active and materialization amounts for TAO and alpha.
  • Added BalancerTaoReservoir and BalancerAlphaReservoir storage in pallets/swap.
  • Changed coinbase injection to materialize TAO before offering it to swap reservoir accounting, so failed TAO spending cannot activate unescrowed current-block TAO.
  • Updated subnet dissolve / swap cleanup tests to include the new reservoir storage.
  • Added focused swap and coinbase tests for TAO/alpha reservoiring, retrying reservoirs, failed TAO materialization, and cleanup.

Behavioral Impact

Out-of-range protocol liquidity is no longer silently skipped. TAO/alpha that cannot be made price-active immediately is retained for later balancer updates while price-active reserve changes remain bounded by balancer weight constraints.

Runtime / Migration Notes

This changes pallet storage and runtime behavior. A spec_version bump may be required by the devnet-ready spec-version check depending on the live devnet runtime version.

Testing

The PR adds unit coverage in pallets/swap/src/pallet/tests.rs, pallets/subtensor/src/tests/coinbase.rs, and pallets/subtensor/src/tests/networks.rs for the new reservoir behavior and cleanup paths.

@gztensor gztensor self-assigned this Jun 15, 2026
@gztensor gztensor added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Jun 15, 2026

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

BalancerTaoReservoir::<T>::insert(netuid, TaoBalance::ZERO);
BalancerAlphaReservoir::<T>::insert(netuid, AlphaBalance::ZERO);
SwapBalancer::<T>::insert(netuid, new_balancer);
return (pending_tao, pending_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] TAO reservoir can inject unescrowed prior-block credit

pending_tao includes BalancerTaoReservoir from earlier blocks, but TAO credit is not escrowed when it is stored there. In inject_and_maybe_swap, the unspent TAO credit from a failed/withheld injection is recycled at the end of that block; a later call that returns pending_tao asks the caller to fund old TAO from the current block's remaining_credit. If that spend fails, this function has already cleared the reservoir and inserted the updated SwapBalancer; if it succeeds, this subnet can consume emission credit that belonged to the current block's other allocations. The TAO-only branch below has the same issue. Fix by either escrowing/reserving TAO alongside the reservoir and committing the balancer update only after funding succeeds, or by not carrying TAO reservoir amounts across blocks under this interface.

@github-actions

github-actions Bot commented Jun 15, 2026

Copy link
Copy Markdown
Contributor

🛡️ AI Review — Skeptic (security review)

VERDICT: SAFE

BASELINE scrutiny: established contributor with repo write permission; no Gittensor allowlist hit; branch fix/handle-misbalance-on-injection -> devnet-ready.

Static review used only the trusted instructions and pre-fetched context. The diff does not modify .github/, build scripts, Cargo manifests, lockfiles, or Rust dependencies. The remaining generated .papi artifacts do not add executable install hooks or dependency changes.

Findings

No findings.

Prior-comment reconciliation

  • 5b48401d: addressed — The current diff still materializes nonzero BalancerTaoReservoir and BalancerAlphaReservoir into reserve accounting before dissolution/direct cleanup removes the reservoir storage.

Conclusion

No malicious intent or security vulnerability was found in the current diff. Reservoir balances are materialized into reserve accounting before subnet dissolution/direct swap cleanup clears them, and the runtime storage change includes a spec_version bump.


🔍 AI Review — Auditor (domain review)

VERDICT: 👍

LIKELY Gittensor/ecosystem contributor: not in trusted allowlists, but has repo write permission and substantial recent subtensor contribution history; no better duplicate candidate found.

PR body is substantive and matches the implementation. The runtime spec_version is bumped from 424 to 426.

Validation: git diff --check 6a4206fac7b98e8c2d91e18bbd027fec30e3ca41...HEAD passed. I did not run cargo tests because the current pass did not require runtime confirmation.

Duplicate-work check: overlapping open PRs touch shared runtime/pallet files, but their titles/scopes address different work, so I do not see a better duplicate candidate.

Findings

Sev File Finding
LOW contract-tests/.papi/ Unrelated generated contract-test metadata is included in the economic fix (off-diff)

Other findings

  • [LOW] Unrelated generated contract-test metadata is included in the economic fix (contract-tests/.papi/) — The PR is scoped to swap/subtensor reservoir accounting, but it also adds a new .papi contract metadata bundle and devnet SCALE blob under contract-tests/ with no corresponding test harness or PR-body explanation. If these artifacts are required for this fix, document why; otherwise remove them so this runtime accounting change does not carry unrelated generated files.

Prior-comment reconciliation

  • 5d36cf5e: not addressed — The unrelated .papi contract metadata bundle is still present in the diff.

Conclusion

The reservoir accounting changes are covered by focused tests and I do not see a blocking domain issue. The only carried concern is low-severity PR hygiene around unrelated generated contract-test metadata.


📜 Previous run (superseded)
Sev File Finding Status
LOW contract-tests/.papi/ Unrelated generated contract-test metadata is included in the economic fix ➡️ Carried forward to current findings
The unrelated .papi contract metadata bundle is still present in the diff.

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@gztensor gztensor marked this pull request as draft June 15, 2026 14:59
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

@gztensor gztensor marked this pull request as ready for review June 17, 2026 14:11

@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/subtensor/src/coinbase/run_coinbase.rs Outdated
@github-actions

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 thread pallets/swap/src/pallet/impls.rs Outdated
@github-actions

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/swap/src/pallet/impls.rs Outdated
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: VULNERABLE

@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

@gztensor gztensor marked this pull request as draft June 25, 2026 14:51
@gztensor gztensor marked this pull request as ready for review June 25, 2026 18:46
@github-actions

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🔄 AI review updated — Skeptic: SAFE Auditor: 👍

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

Labels

skip-cargo-audit This PR fails cargo audit but needs to be merged anyway

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants