[WIP] Staking Currency <> Fungible migration with no provider #5658
Conversation
This file contains 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
balance hold checks both frozen and reserved wip: around 25 tests failing check Holds instead of locks 20 tests failing fmt 11 fails 4 fails 2 failing 1 fail all tests pass but pending a hygiene check of code fix compile minor refactor remove T::Currency calls from asset mod
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
…staking-migrate-currency-to-fungible-2
11 tasks
github-merge-queue bot
pushed a commit
that referenced
this pull request
Jan 16, 2025
Migrate staking currency from `traits::LockableCurrency` to `traits::fungible::holds`. Resolves part of #226. ## Changes ### Nomination Pool TransferStake is now incompatible with fungible migration as old pools were not meant to have additional ED. Since they are anyways deprecated, removed its usage from all test runtimes. ### Staking - Config: `Currency` becomes of type `Fungible` while `OldCurrency` is the `LockableCurrency` used before. - Lazy migration of accounts. Any ledger update will create a new hold with no extra reads/writes. A permissionless extrinsic `migrate_currency()` releases the old `lock` along with some housekeeping. - Staking now requires ED to be left free. It also adds no consumer to staking accounts. - If hold cannot be applied to all stake, the un-holdable part is force withdrawn from the ledger. ### Delegated Staking The pallet does not add provider for agents anymore. ## Migration stats ### Polkadot Total accounts that can be migrated: 59564 Accounts failing to migrate: 0 Accounts with stake force withdrawn greater than ED: 59 Total force withdrawal: 29591.26 DOT ### Kusama Total accounts that can be migrated: 26311 Accounts failing to migrate: 0 Accounts with stake force withdrawn greater than ED: 48 Total force withdrawal: 1036.05 KSM [Full logs here](https://hackmd.io/@ak0n/BklDuFra0). ## Note about locks (freeze) vs holds With locks or freezes, staking could use total balance of an account. But with holds, the account needs to be left with at least Existential Deposit in free balance. This would also affect nomination pools which till now has been able to stake all funds contributed to it. An alternate version of this PR is #5658 where staking pallet does not add any provider, but means pools and delegated-staking pallet has to provide for these accounts and makes the end to end logic (of provider and consumer ref) lot less intuitive and prone to bug. This PR now introduces requirement for stakers to maintain ED in their free balance. This helps with removing the bug prone incrementing and decrementing of consumers and providers. ## TODO - [x] Test: Vesting + governance locked funds can be staked. - [ ] can `Call::restore_ledger` be removed? @gpestana - [x] Ensure unclaimed withdrawals is not affected by no provider for pool accounts. - [x] Investigate kusama accounts with balance between 0 and ED. - [x] Permissionless call to release lock. - [x] Migration of consumer (dec) and provider (inc) for direct stakers. - [x] force unstake if hold cannot be applied to all stake. - [x] Fix try state checks (it thinks nothing is staked for unmigrated ledgers). - [x] Bench `migrate_currency`. - [x] Virtual Staker migration test. - [x] Ensure total issuance is upto date when minting rewards. ## Followup - #5742 --------- Co-authored-by: command-bot <>
Nathy-bajo
pushed a commit
to Nathy-bajo/polkadot-sdk
that referenced
this pull request
Jan 21, 2025
Migrate staking currency from `traits::LockableCurrency` to `traits::fungible::holds`. Resolves part of paritytech#226. ## Changes ### Nomination Pool TransferStake is now incompatible with fungible migration as old pools were not meant to have additional ED. Since they are anyways deprecated, removed its usage from all test runtimes. ### Staking - Config: `Currency` becomes of type `Fungible` while `OldCurrency` is the `LockableCurrency` used before. - Lazy migration of accounts. Any ledger update will create a new hold with no extra reads/writes. A permissionless extrinsic `migrate_currency()` releases the old `lock` along with some housekeeping. - Staking now requires ED to be left free. It also adds no consumer to staking accounts. - If hold cannot be applied to all stake, the un-holdable part is force withdrawn from the ledger. ### Delegated Staking The pallet does not add provider for agents anymore. ## Migration stats ### Polkadot Total accounts that can be migrated: 59564 Accounts failing to migrate: 0 Accounts with stake force withdrawn greater than ED: 59 Total force withdrawal: 29591.26 DOT ### Kusama Total accounts that can be migrated: 26311 Accounts failing to migrate: 0 Accounts with stake force withdrawn greater than ED: 48 Total force withdrawal: 1036.05 KSM [Full logs here](https://hackmd.io/@ak0n/BklDuFra0). ## Note about locks (freeze) vs holds With locks or freezes, staking could use total balance of an account. But with holds, the account needs to be left with at least Existential Deposit in free balance. This would also affect nomination pools which till now has been able to stake all funds contributed to it. An alternate version of this PR is paritytech#5658 where staking pallet does not add any provider, but means pools and delegated-staking pallet has to provide for these accounts and makes the end to end logic (of provider and consumer ref) lot less intuitive and prone to bug. This PR now introduces requirement for stakers to maintain ED in their free balance. This helps with removing the bug prone incrementing and decrementing of consumers and providers. ## TODO - [x] Test: Vesting + governance locked funds can be staked. - [ ] can `Call::restore_ledger` be removed? @gpestana - [x] Ensure unclaimed withdrawals is not affected by no provider for pool accounts. - [x] Investigate kusama accounts with balance between 0 and ED. - [x] Permissionless call to release lock. - [x] Migration of consumer (dec) and provider (inc) for direct stakers. - [x] force unstake if hold cannot be applied to all stake. - [x] Fix try state checks (it thinks nothing is staked for unmigrated ledgers). - [x] Bench `migrate_currency`. - [x] Virtual Staker migration test. - [x] Ensure total issuance is upto date when minting rewards. ## Followup - paritytech#5742 --------- Co-authored-by: command-bot <>
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.
No provider version of #5501