Skip to content
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

Schedule Coldkey Swap #620

Merged
merged 57 commits into from
Jul 9, 2024
Merged

Schedule Coldkey Swap #620

merged 57 commits into from
Jul 9, 2024

Conversation

distributedstatemachine
Copy link
Collaborator

@distributedstatemachine distributedstatemachine commented Jul 7, 2024

Description

This PR introduces a new extrinsic called schedule_coldkey_swap that allows users to swap their potentially compromised coldkeys while implementing a time-based arbitration mechanism for resolving multiple swap requests.

Motivation for change

The primary motivation for this change is to address a recent security incident where coldkeys were potentially compromised. This feature enhances the security and flexibility of the Subtensor network by providing users with a mechanism to replace compromised coldkeys. It's crucial for maintaining the integrity of user accounts and protecting their assets in case of security breaches or key compromises.

Behaviour pre-change

Before this change, users had no built-in mechanism to swap their coldkeys if they were compromised. This lack of functionality could lead to potential security risks and loss of assets if a coldkey was compromised without any means of replacement. In light of the recent security incident, this limitation posed a significant risk to affected users.

Behaviour post-change

After implementing this feature:

  1. Users can initiate a coldkey swap by calling the schedule_coldkey_swap function, providing the new coldkey and a proof of work.

  2. The system implements an arbitration period to handle multiple swap requests:

    • If it's the first swap request for a coldkey, it's scheduled for the next arbitration period.
    • If there are already two destination coldkeys for the old coldkey, the request is rejected.
  3. The difficulty of the proof of work increases exponentially with each subsequent swap attempt, adding an extra layer of security.

  4. After the arbitration period, the system either performs the swap if there's only one destination coldkey or extends the arbitration period if there are multiple requests.

  5. The swap process includes transferring all associated data and balances from the old coldkey to the new one, including stakes, owned hotkeys, and subnet ownership.

This new functionality allows users affected by the recent security incident to secure their accounts and assets by replacing potentially compromised coldkeys.

Alternatives considered

  1. Immediate swaps without arbitration: This was deemed too risky as it could lead to potential abuse or unauthorized swaps, especially in the context of the recent security incident.

  2. Manual review process: While more secure, this would be less efficient and require more administrative overhead, which could delay the resolution for affected users.

TODO:

  • Fix broken tests
  • Ensure ColdkeyDestinationMap isnt emptied

Related Issue(s)

  • Closes #[issue number]

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

If this PR introduces a breaking change, please provide a detailed description of the impact and the migration path for existing applications.

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 cargo fmt and cargo clippy 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

Screenshots (if applicable)

Please include any relevant screenshots or GIFs that demonstrate the changes made.

Additional Notes

Please provide any additional information or context that may be helpful for reviewers.

@distributedstatemachine distributedstatemachine marked this pull request as draft July 7, 2024 22:53
@distributedstatemachine distributedstatemachine changed the title Arbitrage coldkeys Schedule Coldkey Swap Jul 7, 2024
pallets/subtensor/src/lib.rs Outdated Show resolved Hide resolved
pallets/subtensor/src/swap.rs Show resolved Hide resolved
Copy link
Contributor

@gztensor gztensor left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a small nit, but if tempo is 0, it means we have 7.2 emissions per day, not 0, because of how blocks_until_next_epoch function is defined. The code in get_delegate_by_existing_account should be:

if tempo > U64F64::from_num(0) {
    let epochs_per_day: U64F64 = U64F64::from_num(7200).saturating_div(tempo);
    emissions_per_day =
        emissions_per_day.saturating_add(emission.saturating_mul(epochs_per_day));
} else {
    emissions_per_day = emissions_per_day.saturating_add(U64F64.from_num(7.2))
}

pallets/subtensor/src/swap.rs Show resolved Hide resolved
@distributedstatemachine distributedstatemachine marked this pull request as ready for review July 9, 2024 20:41
@opentensor opentensor deleted a comment from orriin Jul 9, 2024
@distributedstatemachine distributedstatemachine merged commit 0efeaf3 into main Jul 9, 2024
12 of 13 checks passed
@distributedstatemachine distributedstatemachine deleted the arbitrage_coldkeys branch July 10, 2024 08:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
devnet-pass PR has been tested on devnet testnet-pass PR was successfully tested on testnet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants