Skip to content

Handle balancer misbalance on injection#2758

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

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: 👍

@IntiTechnologies

Copy link
Copy Markdown

Closing: superseded by #2832, which squashes this change together with the other pending fixes.

@IntiTechnologies IntiTechnologies deleted the fix/handle-misbalance-on-injection branch July 9, 2026 18:57
@UnArbosFour UnArbosFour mentioned this pull request Jul 10, 2026
13 tasks
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.

3 participants