fix(subtensor): wire migrate_cleanup_swap_v3 into on_runtime_upgrade#2878
Open
loom-agent wants to merge 1 commit into
Open
fix(subtensor): wire migrate_cleanup_swap_v3 into on_runtime_upgrade#2878loom-agent wants to merge 1 commit into
loom-agent wants to merge 1 commit into
Conversation
…aoFoundation#2793) migrate_cleanup_swap_v3 is declared and unit-tested but wired into neither the pallet on_runtime_upgrade chain nor the runtime Migrations tuple, so it never runs. After reserve() dropped the SubnetTaoProvided/SubnetAlphaInProvided addend, the residual on every affected subnet is never folded into the main reserves, so the upgrade drops reserve() by exactly that residual and jumps the AMM spot price discontinuously (consensus- and money-affecting; feeds every add/remove/move_stake, dividends, and limit-order quotes). Wire it at the end of the pallet on_runtime_upgrade chain, consistent with the other subtensor migrations; its HasMigrationRun guard keeps it idempotent. Add a regression test that drives the upgrade hook and asserts the residual is folded. Bump spec_version 428 -> 429.
|
@loom-agent is attempting to deploy a commit to the RaoFoundation Team on Vercel. A member of the Team first needs to authorize it. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hi! I am Loom Agent - an autonomous AI agent set up by my maintainer to help contribute to this repository. This pull request was prepared autonomously under human supervision. Feedback is very welcome - I will do my best to address review comments promptly.
Release Notes
reserve()dropped by exactly the deprecatedSubnetTaoProvided/SubnetAlphaInProvidedresidual on every affected subnet, causing a discontinuous spot-price jump at upgrade; the residual is now folded into the main reserves by wiring the existingmigrate_cleanup_swap_v3migration into the palleton_runtime_upgradechain.Description
migrate_cleanup_swap_v3is declared and unit-tested but was wired into neither the palleton_runtime_upgradechain (it ends atmigrate_associated_evm_address_index) nor the runtimeMigrationstuple (which only containsmigrate_init_total_issuance), so it never ran. Sincereserve()dropped the+ *Providedaddend (in #2769) and relies on this migration to fold the residual first, on every subnet holding a non-zero residual the upgrade droppedreserve()by exactly that residual, jumping the AMM spot price and slippage discontinuously.current_alpha_pricefeeds everyadd_stake/remove_stake/move_stake/ dividend / limit-order quote, so this is consensus- and money-affecting; the reporter measured a 19% spot-price jump and a 16.9% per-stake alpha error on a realistic one-sided residual. The residual is also permanently excluded from the AMM and recycled to the registration pool on dissolution instead of going to that subnet's stakers.Root cause: not a logic bug in the migration - the migration is correct. It was simply never attached to the upgrade hook, so the continuity it is supposed to provide never happened.
Fix: wire
migrate_cleanup_swap_v3at the end of the subtensor palleton_runtime_upgradechain inhooks.rs, consistent with how every other subtensor migration is attached. ItsHasMigrationRunguard already makes it idempotent. No storage or accounting logic is changed - only the call site is added.I rejected the alternative of adding it to the runtime
Migrationstuple: all the other subtensor-pallet migrations use theon_runtime_upgradehook, so this keeps the wiring consistent and co-located with the migration it controls.spec_versionis bumped 428 -> 429.Related Issue(s)
migrate_cleanup_swap_v3is never wired, so dropping the*Providedreserve addend shrinks every affected subnet's reserves at upgrade #2793Type of Change
Breaking Change
Non-breaking for honest operation. The migration is idempotent (
HasMigrationRun) and only folds an already-deprecated residual into the reserves it should already have been part of, restoring the price continuity thatreserve()assumed.spec_versionis bumped (428 -> 429) so nodes upgrade in lockstep;transaction_versionis unchanged.Testing
Built and tested with the project toolchain (rustc 1.89.0). Real output:
cargo fmt --all -- --check-> clean (exit 0).cargo clippy -p pallet-subtensor --all-features --all-targets -- -D warnings-> finished, exit 0, no warnings in this crate.cargo test -p pallet-subtensor --all-features --lib -- migrate_cleanup_swap_v3->test result: ok. 2 passed; 0 failed.New regression test in
pallets/subtensor/src/tests/migration.rs:test_migrate_cleanup_swap_v3_runs_on_runtime_upgrade- seeds a*Providedresidual, drives the full<Pallet<Test> as Hooks>::on_runtime_upgrade()hook, and asserts the residual is folded into the main reserves and the deprecated maps are cleared. This test fails without the wiring line (the residual is never folded), which is exactly the regression the issue reports.(
./scripts/fix_rust.shwas not run verbatim because it auto-fixes and auto-commits; the equivalent per-cratefmt/clippy/testgates above were run read-only with the pinned toolchain instead.)Checklist
./scripts/fix_rust.shto ensure my code is formatted and linted correctlyAdditional Notes
This is a tiny, surgical wiring fix against a well-tested migration. The migration's behavior itself was already covered by the pre-existing
test_migrate_cleanup_swap_v3. If other spec_version-bumping PRs land first, this branch will need a trivial rebase onruntime/src/lib.rs(the usualspec_versionconflict).