-
Notifications
You must be signed in to change notification settings - Fork 267
Rework coldkey swap #2251
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: devnet-ready
Are you sure you want to change the base?
Rework coldkey swap #2251
Conversation
59d349c to
2f1ad59
Compare
# Conflicts: # pallets/admin-utils/src/lib.rs
shamil-gadelshin
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good! I have a general question about enums and public API preservation.
| TooManyChildren, | ||
| /// Default transaction rate limit exceeded. | ||
| TxRateLimitExceeded, | ||
| /// Swap already scheduled. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we deprecate enum items instead of deleting them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo this is fine because we should expect clients to use the metadata from the chain to handle decoding, etc.
| >>::Balance, | ||
| }, | ||
| /// A coldkey swap has been scheduled | ||
| ColdkeySwapScheduled { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we deprecate enum items instead of deleting them?
Closes #2230
Summary
This PR refactors the coldkey swap mechanism from a scheduler-based system to an announcement-based system with a configurable delay period. This change improves security, simplifies the codebase, and provides better transparency for coldkey swap operations.
Key Changes
1. New Announcement-Based Flow
announce_coldkey_swap: Users must first announce their intention to swap coldkeys by providing a hash of the new coldkeyswap_coldkey_announced: After the announcement delay period, users can execute the swap by providing the actual new coldkey2. Configuration Changes
InitialColdkeySwapScheduleDuration→InitialColdkeySwapAnnouncementDelayInitialColdkeySwapRescheduleDuration(no longer needed)ColdkeySwapScheduledmap →ColdkeySwapAnnouncementsmap3. Refactored Core Swap Logic
do_swap_coldkey: Consolidated swap logic with clearer separation of concernstransfer_subnet_ownershiptransfer_auto_stake_destinationtransfer_coldkey_staketransfer_staking_hotkeystransfer_hotkeys_ownership4. Updated
swap_coldkeyExtrinsicDispatchResultinstead ofDispatchResultWithPostInfo5. Admin Utilities
sudo_set_coldkey_swap_schedule_duration(call index 54)sudo_set_coldkey_swap_announcement_delay(call index 84)remove_coldkey_swap_announcement(call index 127) - root-only removal of announcements6. Events
ColdkeySwapScheduledColdkeySwapAnnounced- emitted when an announcement is madeColdkeySwapAnnouncementRemoved- emitted when an announcement is removedColdkeySwapAnnouncementDelaySet(renamed fromColdkeySwapScheduleDurationSet)7. Error Handling
ColdkeyIsInArbitration,SwapAlreadyScheduled,FailedToScheduleColdkeySwapAnnouncementNotFoundColdkeySwapTooEarlyColdkeySwapReannouncedTooEarlyAnnouncedColdkeyHashDoesNotMatchDeprecated(for the oldschedule_swap_coldkeycall)8. Transaction Extension
CustomTransactionError::ColdkeyInSwapSchedule→ColdkeySwapAnnounced9. Migration
migrate_coldkey_swap_scheduled_to_announcementsto migrate existing scheduled swaps to the new announcement systemscheduled_time - delayso swaps can execute at the originally scheduled timeBenefits
swap_coldkeycallBreaking Changes
schedule_swap_coldkeyextrinsic is deprecated and will return an errorMigration Path
For users with existing scheduled coldkey swaps:
swap_coldkey_announced