Re-enable balancer by gztensor · Pull Request #2505 · opentensor/subtensor

gztensor · 2026-03-17T19:48:07Z

Description

Reintroduce balancer swap. The main PR:

#2290

Type of Change

Bug fix (non-breaking change which fixes an issue)
New feature (non-breaking change which adds functionality)
Breaking change (fix or feature that would cause existing functionality to not work as expected)
Documentation update
Other (please describe):

Breaking Change

See #2290

Checklist

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have run ./scripts/fix_rust.sh to ensure my code is formatted and linted correctly
I have made corresponding changes to the documentation
My changes generate no new warnings
I have added tests that prove my fix is effective or that my feature works
New and existing unit tests pass locally with my changes
Any dependent changes have been merged and published in downstream modules

github-actions · 2026-05-25T19:22:01Z

🛡️ AI Review — Skeptic (security review)

VERDICT: VULNERABLE

BASELINE scrutiny: author has repo write permission and substantial prior subtensor activity; no Gittensor allowlist hit found; branch feat/balancer_swap -> devnet-ready.

I found no .github/ai-review/* or .github/copilot-instructions.md modifications in this PR.

Findings

Sev	File	Finding
HIGH	pallets/subtensor/src/macros/hooks.rs::on_runtime_upgrade	Cleanup migration for provided liquidity is added but never wired into runtime upgrade	(off-diff)
HIGH	`Cargo.toml:324`	Do not globally patch zstd to author-controlled Git forks	inline
HIGH	`pallets/swap/src/pallet/migrations/migrate_swapv3_to_balancer.rs:55`	Do not replace failed price initialization with default weights before deleting old price state	inline
MEDIUM	`Cargo.toml:68`	New runtime math dependency is pulled from Git instead of a vetted release	inline

Other findings

[HIGH] Cleanup migration for provided liquidity is added but never wired into runtime upgrade (pallets/subtensor/src/macros/hooks.rs::on_runtime_upgrade) — The PR adds migrate_cleanup_swap_v3, which moves SubnetTaoProvided and SubnetAlphaInProvided into the active reserves and removes the old maps, but it is not called from pallets/subtensor/src/macros/hooks.rs::on_runtime_upgrade. The rest of the PR removes normal reads/writes of those provided-liquidity maps, so any live values remain orphaned and excluded from reserve/pricing calculations after upgrade. Wire this migration into the Subtensor runtime-upgrade chain before removing use of the old maps.

Prior-comment reconciliation

b4b1fd62: not addressed — migrate_cleanup_swap_v3 is still added/exported, but no call was added to the Subtensor on_runtime_upgrade chain.
afc4fe7b: not addressed — The [patch.crates-io] entries for zstd-sys and zstd-safe still point to github.com/gztensor/*.
f2ba865f: not addressed — Partially changed, but the migration still swallows initialization failure by inserting a default balancer and then clearing the old Swap V3 price state.
52b90a8c: not addressed — safe-bigmath remains a Git dependency used by the runtime swap math.

Conclusion

I found no evidence of intentional backdoor, but this PR is not safe to merge while runtime upgrade state can still be lost/orphaned and build/runtime dependencies are sourced from unvetted Git repositories.

📜 Previous run (superseded)

~~Sev~~	~~File~~	~~Finding~~	Status
~~HIGH~~	~~pallets/subtensor/src/macros/hooks.rs on_runtime_upgrade~~	~~Cleanup migration for provided liquidity is added but never wired into runtime upgrade~~	➡️ Carried forward to current findings `migrate_cleanup_swap_v3` is still added/exported, but no call was added to the Subtensor `on_runtime_upgrade` chain.
~~HIGH~~	~~`Cargo.toml:324`~~	~~Do not globally patch zstd to author-controlled git forks~~	❔ Status unknown in current run
~~HIGH~~	~~`pallets/swap/src/pallet/hooks.rs:20`~~	~~Migration drops old swap price state even when balancer initialization fails~~	❔ Status unknown in current run
~~MEDIUM~~	~~`Cargo.toml:68`~~	~~New runtime math dependency is pulled from Git instead of a vetted release~~	➡️ Carried forward to current findings `safe-bigmath` remains a Git dependency used by the runtime swap math.

# 🔍 AI Review — Auditor (domain review) has not yet run on this PR.

github-actions · 2026-05-25T19:22:07Z

🔄 AI review updated — Skeptic: VULNERABLE

github-actions

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

github-actions · 2026-05-27T22:20:21Z

+zstd-sys = { git = "https://github.com/gztensor/zstd-sys" }
+zstd-safe = { git = "https://github.com/gztensor/zstd-safe" }


[HIGH] Do not globally patch zstd to author-controlled git forks

This globally replaces crates.io zstd-sys and zstd-safe with Git forks under the PR author's account. zstd-sys includes build-time native code, so once CI/builds are approved this is a supply-chain execution path that bypasses normal crates.io provenance. Remove the patch or replace it with a vetted upstream release or an organization-controlled, pinned fork with a documented reason.

github-actions · 2026-05-27T22:20:21Z

+                // Cleanup uniswap v3 and migrate to balancer
+                .saturating_add(
+                    migrations::migrate_swapv3_to_balancer::migrate_swapv3_to_balancer::<T>(),


[HIGH] Migration drops old swap price state even when balancer initialization fails

This newly wires migrate_swapv3_to_balancer, but that migration calls maybe_initialize_palswap(...).unwrap_or_default() and then clears AlphaSqrtPrice and the rest of the V3 maps. If balancer initialization rejects an imbalanced subnet, the error is swallowed and the old price/liquidity state is still deleted, leaving that subnet migrated as if it succeeded. Make the migration fail-safe: only clear old state after successful initialization for every subnet, or preserve/record failures and do not mark the migration complete.

github-actions · 2026-05-27T22:20:21Z

 pallet-subtensor-swap-runtime-api = { path = "pallets/swap/runtime-api", default-features = false }
 pallet-subtensor-swap-rpc = { path = "pallets/swap/rpc", default-features = false }
 procedural-fork = { path = "support/procedural-fork", default-features = false }
+safe-bigmath = { package = "safe-bigmath", default-features = false, git = "https://github.com/sam0x17/safe-bigmath" }


[MEDIUM] New runtime math dependency is pulled from Git instead of a vetted release

safe-bigmath is introduced as a Git dependency and is used in runtime swap math. Cargo.lock currently pins a commit, but the dependency lacks normal crates.io provenance and review signals in the manifest. For runtime economic math, use a vetted release or vendor/audit the exact source under organization control before merging.

github-actions · 2026-05-27T22:20:31Z

🔄 AI review updated — Skeptic: VULNERABLE

…in migration

github-actions

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

github-actions · 2026-05-29T18:42:57Z

+zstd-sys = { git = "https://github.com/gztensor/zstd-sys" }
+zstd-safe = { git = "https://github.com/gztensor/zstd-safe" }


[HIGH] Do not globally patch zstd to author-controlled Git forks

This workspace-wide [patch.crates-io] overrides every zstd-sys and zstd-safe consumer to build from github.com/gztensor/*. Those crates run native build logic during CI and developer builds, so this is a supply-chain execution path controlled by the PR author rather than by crates.io checksums or an opentensor-owned fork. Remove the patch or move any required zstd fix to a vetted release/opentensor-controlled source before merging.

github-actions · 2026-05-29T18:42:57Z

+        if let Err(error) = crate::Pallet::<T>::maybe_initialize_palswap(netuid, Some(price)) {
+            log::warn!(
+                "Migration '{}' failed to initialize balancer with V3 price for netuid {}: {:?}. Falling back to default balancer.",
+                String::from_utf8_lossy(&migration_name),
+                netuid,
+                error,
+            );
+            SwapBalancer::<T>::insert(netuid, Balancer::default());
+            PalSwapInitialized::<T>::insert(netuid, true);


[HIGH] Do not replace failed price initialization with default weights before deleting old price state

If maybe_initialize_palswap(netuid, Some(price)) rejects the V3 price/reserve combination, this fallback marks the subnet initialized with Balancer::default() and the migration then removes AlphaSqrtPrice below. That converts an initialization failure into a permanent price reset to default weights while deleting the only old price state that could be retried or diagnosed. Runtime migration should preserve the old state or fail/skip cleanup for that netuid instead of silently initializing a materially different pool.

This is expected behavior: If the state of v3 pool is not representable with balancer, it will be set to default balancer. Additionally, this case does not appear neither on mainnet nor testnet currently and it is nearly impossible that liquidity balance shifts so much between now and next release.

github-actions · 2026-05-29T18:42:57Z

 pallet-subtensor-swap-runtime-api = { path = "pallets/swap/runtime-api", default-features = false }
 pallet-subtensor-swap-rpc = { path = "pallets/swap/rpc", default-features = false }
 procedural-fork = { path = "support/procedural-fork", default-features = false }
+safe-bigmath = { package = "safe-bigmath", default-features = false, git = "https://github.com/sam0x17/safe-bigmath" }


[MEDIUM] New runtime math dependency is pulled from Git instead of a vetted release

safe-bigmath is used in runtime swap math, but this adds it directly from Git. Even though Cargo.lock pins the current revision, this bypasses normal crates.io provenance and makes future lock refreshes depend on a Git source for consensus-critical arithmetic. Vendor it, use a reviewed crates.io release, or move the needed code into an audited internal primitive before enabling it in runtime code.

github-actions · 2026-05-29T18:43:07Z

🔄 AI review updated — Skeptic: VULNERABLE

Re-enable balancer

211007c

gztensor added the skip-cargo-audit This PR fails cargo audit but needs to be merged anyway label Mar 17, 2026

gztensor added 11 commits March 17, 2026 16:17

Merge branch 'devnet-ready' into feat/balancer_swap

5fe6d60

Merge devnet-ready

1d48cac

Patch weights

689cffe

nudge CI

51587e9

Merge branch 'devnet-ready' into feat/balancer_swap

c0998f0

Merge in progress

e28d2c9

Cleanup merge

47e6de7

Merge devnet-ready

6a0c9ad

merge devnet-ready

189cf38

fmt

513c08a

Merge branch 'devnet-ready' into feat/balancer_swap

15844b9

basfroman mentioned this pull request May 27, 2026

New balancer (attempt 3) latent-to/bittensor#3362

Open

gztensor added 2 commits May 27, 2026 18:12

Merge branch 'devnet-ready' into feat/balancer_swap

4dc60d4

spec bump

522cb66

github-actions Bot reviewed May 27, 2026

View reviewed changes

gztensor added 2 commits May 29, 2026 13:47

Merge branch 'devnet-ready' into feat/balancer_swap

9684496

Address ai reviewer comment about incomplete balancer initialization …

b1e9356

…in migration

github-actions Bot reviewed May 29, 2026

View reviewed changes

		zstd-sys = { git = "https://github.com/gztensor/zstd-sys" }
		zstd-safe = { git = "https://github.com/gztensor/zstd-safe" } No newline at end of file

Conversation

gztensor commented Mar 17, 2026

Description

Type of Change

Breaking Change

Checklist

Uh oh!

github-actions Bot commented May 25, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🛡️ AI Review — Skeptic (security review)

Findings

Other findings

Prior-comment reconciliation

Conclusion

Uh oh!

github-actions Bot commented May 25, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 27, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 27, 2026

Uh oh!

github-actions Bot left a comment

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

gztensor May 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot May 29, 2026

Choose a reason for hiding this comment

Uh oh!

github-actions Bot commented May 29, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

github-actions Bot commented May 25, 2026 •

edited

Loading