fix(coinbase): clear AutoStakeDestination on subnet dissolve#2652
Open
RUNECTZ33 wants to merge 2 commits into
Open
fix(coinbase): clear AutoStakeDestination on subnet dissolve#2652RUNECTZ33 wants to merge 2 commits into
RUNECTZ33 wants to merge 2 commits into
Conversation
`AutoStakeDestination` and its reverse index `AutoStakeDestinationColdkeys` both embed `netuid` as the second key of a `StorageDoubleMap`, but neither is cleaned up by `remove_network` during subnet dissolution. Because netuids are recycled by subsequent `do_register_network` calls, a stale `(coldkey, dissolved_netuid) → hotkey` mapping silently survives and applies to the *next* subnet registered on that netuid. The misdirection surfaces in `coinbase::run_coinbase` (auto-stake path), where the lookup `AutoStakeDestination::<T>::get(&owner, netuid)` returns the stale destination from the previous occupant of that netuid. Mining incentive on the newly-registered subnet is then auto-staked to a hotkey that has no relationship to the new subnet. Add two `iter().filter_map().remove()` blocks in the `--- 21.` section of `remove_network`, matching the existing pattern used for the other DMaps that embed netuid as a non-leading key (ChildkeyTake, ChildKeys, ParentKeys, LastHotkeyEmissionOnNetuid, TotalHotkeyAlphaLastEpoch, StakingOperationRateLimiter). Extends `dissolve_clears_all_per_subnet_storages` to cover both maps and adds a focused regression `dissolve_clears_auto_stake_destination_preventing_stale_routing` that exercises the actual user-facing routing hazard. No new dependencies, no API changes, no migration required.
Contributor
|
format the code. |
- `cargo fmt` collapses a wrapped `assert!` macro into a single line. - Bump runtime `spec_version` 406 → 407 per the `Check spec_version bump` CI requirement (this PR touches `remove_network`, which is reachable from a runtime extrinsic).
Author
|
Thanks @open-junius — pushed a fix-up:
Notes on the remaining failed checks (happy to act on any of these if you'd prefer):
If any of the above are mistaken, let me know and I'll iterate. |
11 tasks
Author
|
Quick note in case it's helpful: both this PR's fix-up commit and #2654 are sitting in |
11 tasks
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.
Description
AutoStakeDestinationand its reverse indexAutoStakeDestinationColdkeysareStorageDoubleMaps withnetuidas the second key. Neither is cleaned up byremove_networkduring subnet dissolution. Because netuids are recycled by subsequentdo_register_networkcalls, a stale(coldkey, dissolved_netuid) → hotkeymapping silently survives and applies to the next subnet registered on that netuid.The misdirection surfaces in
coinbase::run_coinbase(auto-stake path) atrun_coinbase.rs:557:The lookup returns the stale destination from the previous occupant of that netuid. Mining incentive on the newly-registered subnet is then auto-staked to a hotkey that has no relationship to the new subnet.
Fix
Add two
iter().filter_map().remove()blocks in the--- 21.section ofremove_network, matching the existing pattern used for the other DMaps that embed netuid as a non-leading key (ChildkeyTake,ChildKeys,ParentKeys,LastHotkeyEmissionOnNetuid,TotalHotkeyAlphaLastEpoch,StakingOperationRateLimiter).Tests
dissolve_clears_all_per_subnet_storagesto populate both maps before dissolve and assert both are cleared after.dissolve_clears_auto_stake_destination_preventing_stale_routingthat exercises the user-facing routing hazard directly.Type of Change
Breaking Change
N/A — adds cleanup that was previously missing. No behavioral change for live subnets, no migration, no new dependencies, no API changes.
Checklist
./scripts/fix_rust.shto ensure my code is formatted and linted correctly (no local Rust toolchain — relying on CI; happy to follow up on any fmt/clippy nits)