fix(coinbase): clear AutoStakeDestination on subnet dissolve#2652
fix(coinbase): clear AutoStakeDestination on subnet dissolve#2652RUNECTZ33 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.
|
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).
|
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. |
|
Quick note in case it's helpful: both this PR's fix-up commit and #2654 are sitting in |
| // mining incentive when the same netuid is later re-registered (see | ||
| // `run_coinbase` auto-stake path). | ||
| { | ||
| let to_rm: sp_std::vec::Vec<T::AccountId> = AutoStakeDestination::<T>::iter() |
There was a problem hiding this comment.
There are 14861 keys on mainnet and 12066 for AutoStakeDestinationColdkeys, so we should be good.
|
BTW, it would be great to update the weights, because we are adding more operations. |
|
Thanks for @evgeny-s' comment about weight. Currently, the weight of dissolve network is totally wrong. I am working on a PR to optimize the extrinsic. I will take care the new clean up ops in the PR. |
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)