Skip to content

Handle balancer misbalance on injection#2758

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

Handle balancer misbalance on injection#2758
gztensor wants to merge 7 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.

Comment thread pallets/swap/src/pallet/impls.rs Outdated
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 prior reservoir-cleanup finding has been addressed in the current revision.

Findings

No findings.

Prior-comment reconciliation

  • 5b48401d: addressed — The current diff now 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. The reservoir cleanup path now folds nonzero reservoir balances into reserve accounting before clearing subnet/swap state, so the prior issue no longer applies.


📜 Previous run (superseded)
Sev File Finding Status
HIGH pallets/swap/src/pallet/impls.rs:351 Reservoir cleanup drops materialized liquidity from accounting ✅ Addressed
The current diff now materializes nonzero BalancerTaoReservoir and BalancerAlphaReservoir into reserve accounting before dissolution/direct cleanup removes the reservoir storage.

🔍 AI Review — Auditor (domain review)

VERDICT: 👍

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

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.

Validation: git diff --check 4ff1e30c174acc0d6c766e4139f7f5ccd29ea488...HEAD passed. I attempted the focused reservoir tests, but cargo/rustc could not start because rustup tried to write temp files under read-only /home/runner/.rustup/tmp; no workspace files were modified. The runtime spec_version is bumped from 423 to 424.

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

  • d13dc776: addressedget_alpha_issuance now includes protocol_alpha_reservoir, and subnet dissolve materializes reservoirs into the reserve accounting before destroying alpha state.

Conclusion

The prior alpha reservoir issuance concern has been addressed: reservoir alpha is included in subnet issuance and dissolve now folds reservoirs into the normal cleanup path before teardown. The remaining issue is low-severity PR hygiene around unrelated generated contract-test metadata.


📜 Previous run (superseded)
Sev File Finding Status
HIGH pallets/subtensor/src/coinbase/run_coinbase.rs:168 Reservoir alpha is minted outside the subnet issuance counters ✅ Addressed
get_alpha_issuance now includes protocol_alpha_reservoir, and subnet dissolve materializes reservoirs into the reserve accounting before destroying alpha state.

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

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