Skip to content

Commit

Permalink
[Staking] Currency <> Fungible migration (#5501)
Browse files Browse the repository at this point in the history
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 <>
  • Loading branch information
Ank4n authored Jan 16, 2025
1 parent e056586 commit f5673cf
Show file tree
Hide file tree
Showing 47 changed files with 2,100 additions and 2,466 deletions.
24 changes: 1 addition & 23 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -389,7 +389,6 @@ members = [
"substrate/frame/nomination-pools/fuzzer",
"substrate/frame/nomination-pools/runtime-api",
"substrate/frame/nomination-pools/test-delegate-stake",
"substrate/frame/nomination-pools/test-transfer-stake",
"substrate/frame/offences",
"substrate/frame/offences/benchmarking",
"substrate/frame/paged-list",
Expand Down
2 changes: 2 additions & 0 deletions polkadot/runtime/test-runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,11 +366,13 @@ impl onchain::Config for OnChainSeqPhragmen {
const MAX_QUOTA_NOMINATIONS: u32 = 16;

impl pallet_staking::Config for Runtime {
type OldCurrency = Balances;
type Currency = Balances;
type CurrencyBalance = Balance;
type UnixTime = Timestamp;
type CurrencyToVote = polkadot_runtime_common::CurrencyToVote;
type RewardRemainder = ();
type RuntimeHoldReason = RuntimeHoldReason;
type RuntimeEvent = RuntimeEvent;
type Slash = ();
type Reward = ();
Expand Down
2 changes: 2 additions & 0 deletions polkadot/runtime/westend/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -728,8 +728,10 @@ parameter_types! {
}

impl pallet_staking::Config for Runtime {
type OldCurrency = Balances;
type Currency = Balances;
type CurrencyBalance = Balance;
type RuntimeHoldReason = RuntimeHoldReason;
type UnixTime = Timestamp;
type CurrencyToVote = CurrencyToVote;
type RewardRemainder = ();
Expand Down
99 changes: 86 additions & 13 deletions polkadot/runtime/westend/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,25 +155,27 @@ mod remote_tests {

let transport: Transport = var("WS").unwrap_or("ws://127.0.0.1:9900".to_string()).into();
let maybe_state_snapshot: Option<SnapshotConfig> = var("SNAP").map(|s| s.into()).ok();
let online_config = OnlineConfig {
transport,
state_snapshot: maybe_state_snapshot.clone(),
child_trie: false,
pallets: vec![
"Staking".into(),
"System".into(),
"Balances".into(),
"NominationPools".into(),
"DelegatedStaking".into(),
],
..Default::default()
};
let mut ext = Builder::<Block>::default()
.mode(if let Some(state_snapshot) = maybe_state_snapshot {
Mode::OfflineOrElseOnline(
OfflineConfig { state_snapshot: state_snapshot.clone() },
OnlineConfig {
transport,
state_snapshot: Some(state_snapshot),
pallets: vec![
"staking".into(),
"system".into(),
"balances".into(),
"nomination-pools".into(),
"delegated-staking".into(),
],
..Default::default()
},
online_config,
)
} else {
Mode::Online(OnlineConfig { transport, ..Default::default() })
Mode::Online(online_config)
})
.build()
.await
Expand Down Expand Up @@ -241,6 +243,77 @@ mod remote_tests {
);
});
}

#[tokio::test]
async fn staking_curr_fun_migrate() {
// Intended to be run only manually.
if var("RUN_MIGRATION_TESTS").is_err() {
return;
}
sp_tracing::try_init_simple();

let transport: Transport = var("WS").unwrap_or("ws://127.0.0.1:9944".to_string()).into();
let maybe_state_snapshot: Option<SnapshotConfig> = var("SNAP").map(|s| s.into()).ok();
let online_config = OnlineConfig {
transport,
state_snapshot: maybe_state_snapshot.clone(),
child_trie: false,
pallets: vec!["Staking".into(), "System".into(), "Balances".into()],
..Default::default()
};
let mut ext = Builder::<Block>::default()
.mode(if let Some(state_snapshot) = maybe_state_snapshot {
Mode::OfflineOrElseOnline(
OfflineConfig { state_snapshot: state_snapshot.clone() },
online_config,
)
} else {
Mode::Online(online_config)
})
.build()
.await
.unwrap();
ext.execute_with(|| {
// create an account with some balance
let alice = AccountId::from([1u8; 32]);
use frame_support::traits::Currency;
let _ = Balances::deposit_creating(&alice, 100_000 * UNITS);

let mut success = 0;
let mut err = 0;
let mut force_withdraw_acc = 0;
// iterate over all pools
pallet_staking::Ledger::<Runtime>::iter().for_each(|(ctrl, ledger)| {
match pallet_staking::Pallet::<Runtime>::migrate_currency(
RuntimeOrigin::signed(alice.clone()).into(),
ledger.stash.clone(),
) {
Ok(_) => {
let updated_ledger =
pallet_staking::Ledger::<Runtime>::get(&ctrl).expect("ledger exists");
let force_withdraw = ledger.total - updated_ledger.total;
if force_withdraw > 0 {
force_withdraw_acc += force_withdraw;
log::info!(target: "remote_test", "Force withdraw from stash {:?}: value {:?}", ledger.stash, force_withdraw);
}
success += 1;
},
Err(e) => {
log::error!(target: "remote_test", "Error migrating {:?}: {:?}", ledger.stash, e);
err += 1;
},
}
});

log::info!(
target: "remote_test",
"Migration stats: success: {}, err: {}, total force withdrawn stake: {}",
success,
err,
force_withdraw_acc
);
});
}
}

#[test]
Expand Down
Loading

0 comments on commit f5673cf

Please sign in to comment.