Skip to content

Commit

Permalink
[Staking] Noop refactor to prep pallet for currency fungible migration (
Browse files Browse the repository at this point in the history
#5399)

This is a no-op refactor of staking pallet to move all `T::Currency` api
calls under one module.

A followup PR (#5501)
will implement the Currency <> Fungible migration for the pallet.

Introduces the new `asset` module that centralizes all interaction with
`T::Currency`. This is an attempt to try minimising staking logic
changes to minimal parts of the codebase.

## Things of note
- `T::Currency::free_balance` in current implementation includes both
staked (locked) and liquid tokens (kinda sounds wrong to call it free
then). This PR renames it to `stakeable_balance` (any better name
suggestions?). With #5501, this will become `free balance that can be
held/staked` + `already held/staked balance`.
  • Loading branch information
Ank4n authored Oct 7, 2024
1 parent 5f55185 commit 9128dca
Show file tree
Hide file tree
Showing 19 changed files with 538 additions and 392 deletions.
4 changes: 1 addition & 3 deletions substrate/frame/delegated-staking/src/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ impl<T: Config> DelegationMigrator for Pallet<T> {

/// Only used for testing.
#[cfg(feature = "runtime-benchmarks")]
fn migrate_to_direct_staker(agent: Agent<Self::AccountId>) {
fn force_kill_agent(agent: Agent<Self::AccountId>) {
<Agents<T>>::remove(agent.clone().get());
<Delegators<T>>::iter()
.filter(|(_, delegation)| delegation.agent == agent.clone().get())
Expand All @@ -136,8 +136,6 @@ impl<T: Config> DelegationMigrator for Pallet<T> {
);
<Delegators<T>>::remove(&delegator);
});

T::CoreStaking::migrate_to_direct_staker(&agent.get());
}
}

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/delegated-staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -676,7 +676,7 @@ mod staking_integration {
// in equal parts. lets try to migrate this nominator into delegate based stake.

// all balance currently is in 200
assert_eq!(Balances::free_balance(agent), agent_amount);
assert_eq!(pallet_staking::asset::stakeable_balance::<T>(&agent), agent_amount);

// to migrate, nominator needs to set an account as a proxy delegator where staked funds
// will be moved and delegated back to this old nominator account. This should be funded
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -322,24 +322,24 @@ fn automatic_unbonding_pools() {
assert_eq!(<Runtime as pallet_nomination_pools::Config>::MaxUnbonding::get(), 1);

// init state of pool members.
let init_free_balance_2 = Balances::free_balance(2);
let init_free_balance_3 = Balances::free_balance(3);
let init_stakeable_balance_2 = pallet_staking::asset::stakeable_balance::<Runtime>(&2);
let init_stakeable_balance_3 = pallet_staking::asset::stakeable_balance::<Runtime>(&3);

let pool_bonded_account = Pools::generate_bonded_account(1);

// creates a pool with 5 bonded, owned by 1.
assert_ok!(Pools::create(RuntimeOrigin::signed(1), 5, 1, 1, 1));
assert_eq!(locked_amount_for(pool_bonded_account), 5);
assert_eq!(staked_amount_for(pool_bonded_account), 5);

let init_tvl = TotalValueLocked::<Runtime>::get();

// 2 joins the pool.
assert_ok!(Pools::join(RuntimeOrigin::signed(2), 10, 1));
assert_eq!(locked_amount_for(pool_bonded_account), 15);
assert_eq!(staked_amount_for(pool_bonded_account), 15);

// 3 joins the pool.
assert_ok!(Pools::join(RuntimeOrigin::signed(3), 10, 1));
assert_eq!(locked_amount_for(pool_bonded_account), 25);
assert_eq!(staked_amount_for(pool_bonded_account), 25);

assert_eq!(TotalValueLocked::<Runtime>::get(), 25);

Expand All @@ -350,7 +350,7 @@ fn automatic_unbonding_pools() {
assert_ok!(Pools::unbond(RuntimeOrigin::signed(2), 2, 10));

// amount is still locked in the pool, needs to wait for unbonding period.
assert_eq!(locked_amount_for(pool_bonded_account), 25);
assert_eq!(staked_amount_for(pool_bonded_account), 25);

// max chunks in the ledger are now filled up (`MaxUnlockingChunks == 1`).
assert_eq!(unlocking_chunks_of(pool_bonded_account), 1);
Expand All @@ -372,8 +372,8 @@ fn automatic_unbonding_pools() {
assert_eq!(current_era(), 3);
System::reset_events();

let locked_before_withdraw_pool = locked_amount_for(pool_bonded_account);
assert_eq!(Balances::free_balance(pool_bonded_account), 26);
let staked_before_withdraw_pool = staked_amount_for(pool_bonded_account);
assert_eq!(pallet_staking::asset::stakeable_balance::<Runtime>(&pool_bonded_account), 26);

// now unbonding 3 will work, although the pool's ledger still has the unlocking chunks
// filled up.
Expand All @@ -391,20 +391,21 @@ fn automatic_unbonding_pools() {
);

// balance of the pool remains the same, it hasn't withdraw explicitly from the pool yet.
assert_eq!(Balances::free_balance(pool_bonded_account), 26);
assert_eq!(pallet_staking::asset::stakeable_balance::<Runtime>(&pool_bonded_account), 26);
// but the locked amount in the pool's account decreases due to the auto-withdraw:
assert_eq!(locked_before_withdraw_pool - 10, locked_amount_for(pool_bonded_account));
assert_eq!(staked_before_withdraw_pool - 10, staked_amount_for(pool_bonded_account));

// TVL correctly updated.
assert_eq!(TotalValueLocked::<Runtime>::get(), 25 - 10);

// however, note that the withdrawing from the pool still works for 2, the funds are taken
// from the pool's free balance.
assert_eq!(Balances::free_balance(pool_bonded_account), 26);
// from the pool's non staked balance.
assert_eq!(pallet_staking::asset::stakeable_balance::<Runtime>(&pool_bonded_account), 26);
assert_eq!(pallet_staking::asset::staked::<Runtime>(&pool_bonded_account), 15);
assert_ok!(Pools::withdraw_unbonded(RuntimeOrigin::signed(2), 2, 10));
assert_eq!(Balances::free_balance(pool_bonded_account), 16);
assert_eq!(pallet_staking::asset::stakeable_balance::<Runtime>(&pool_bonded_account), 16);

assert_eq!(Balances::free_balance(2), 20);
assert_eq!(pallet_staking::asset::stakeable_balance::<Runtime>(&2), 20);
assert_eq!(TotalValueLocked::<Runtime>::get(), 15);

// 3 cannot withdraw yet.
Expand All @@ -423,9 +424,15 @@ fn automatic_unbonding_pools() {
assert_ok!(Pools::withdraw_unbonded(RuntimeOrigin::signed(3), 3, 10));

// final conditions are the expected.
assert_eq!(Balances::free_balance(pool_bonded_account), 6); // 5 init bonded + ED
assert_eq!(Balances::free_balance(2), init_free_balance_2);
assert_eq!(Balances::free_balance(3), init_free_balance_3);
assert_eq!(pallet_staking::asset::stakeable_balance::<Runtime>(&pool_bonded_account), 6); // 5 init bonded + ED
assert_eq!(
pallet_staking::asset::stakeable_balance::<Runtime>(&2),
init_stakeable_balance_2
);
assert_eq!(
pallet_staking::asset::stakeable_balance::<Runtime>(&3),
init_stakeable_balance_3
);

assert_eq!(TotalValueLocked::<Runtime>::get(), init_tvl);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -915,9 +915,8 @@ pub(crate) fn set_minimum_election_score(
.map_err(|_| ())
}

pub(crate) fn locked_amount_for(account_id: AccountId) -> Balance {
let lock = pallet_balances::Locks::<Runtime>::get(account_id);
lock[0].amount
pub(crate) fn staked_amount_for(account_id: AccountId) -> Balance {
pallet_staking::asset::staked::<Runtime>(&account_id)
}

pub(crate) fn staking_events() -> Vec<pallet_staking::Event<Runtime>> {
Expand Down
20 changes: 14 additions & 6 deletions substrate/frame/fast-unstake/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -137,15 +137,16 @@ fn deregister_works() {
ExtBuilder::default().build_and_execute(|| {
ErasToCheckPerBlock::<T>::put(1);

assert_eq!(<T as Config>::Currency::reserved_balance(&1), 0);
// reserved balance prior to registering for fast unstake.
let pre_reserved = <T as Config>::Currency::reserved_balance(&1);

// Controller account registers for fast unstake.
assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)));
assert_eq!(<T as Config>::Currency::reserved_balance(&1), Deposit::get());
assert_eq!(<T as Config>::Currency::reserved_balance(&1) - pre_reserved, Deposit::get());

// Controller then changes mind and deregisters.
assert_ok!(FastUnstake::deregister(RuntimeOrigin::signed(1)));
assert_eq!(<T as Config>::Currency::reserved_balance(&1), 0);
assert_eq!(<T as Config>::Currency::reserved_balance(&1) - pre_reserved, 0);

// Ensure stash no longer exists in the queue.
assert_eq!(Queue::<T>::get(1), None);
Expand Down Expand Up @@ -243,15 +244,19 @@ mod on_idle {
CurrentEra::<T>::put(BondingDuration::get());

// given
assert_eq!(<T as Config>::Currency::reserved_balance(&1), 0);
// reserved balance prior to registering for fast unstake.
let pre_reserved = <T as Config>::Currency::reserved_balance(&1);

assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)));
assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(3)));
assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(5)));
assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(7)));
assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(9)));

assert_eq!(<T as Config>::Currency::reserved_balance(&1), Deposit::get());
assert_eq!(
<T as Config>::Currency::reserved_balance(&1) - pre_reserved,
Deposit::get()
);

assert_eq!(Queue::<T>::count(), 5);
assert_eq!(Head::<T>::get(), None);
Expand Down Expand Up @@ -279,6 +284,9 @@ mod on_idle {
// when
next_block(true);

// pre_reserve may change due to unstaked amount.
let pre_reserved = <T as Config>::Currency::reserved_balance(&1);

// then
assert_eq!(
Head::<T>::get(),
Expand All @@ -289,7 +297,7 @@ mod on_idle {
);
assert_eq!(Queue::<T>::count(), 3);

assert_eq!(<T as Config>::Currency::reserved_balance(&1), 0);
assert_eq!(<T as Config>::Currency::reserved_balance(&1) - pre_reserved, 0);

assert_eq!(
fast_unstake_events_since_last_call(),
Expand Down
4 changes: 3 additions & 1 deletion substrate/frame/nomination-pools/benchmarking/src/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ use sp_runtime::{
traits::{Bounded, StaticLookup, Zero},
Perbill,
};
use sp_staking::EraIndex;
use sp_staking::{EraIndex, StakingUnchecked};
// `frame_benchmarking::benchmarks!` macro needs this
use pallet_nomination_pools::Call;

Expand Down Expand Up @@ -131,6 +131,8 @@ fn migrate_to_transfer_stake<T: Config>(pool_id: PoolId) {
)
.expect("member should have enough balance to transfer");
});

pallet_staking::Pallet::<T>::migrate_to_direct_staker(&pool_acc);
}

fn vote_to_balance<T: pallet_nomination_pools::Config>(
Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/nomination-pools/src/adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -460,6 +460,6 @@ impl<

#[cfg(feature = "runtime-benchmarks")]
fn remove_as_agent(pool: Pool<Self::AccountId>) {
Delegation::migrate_to_direct_staker(pool.into())
Delegation::force_kill_agent(pool.into())
}
}
63 changes: 37 additions & 26 deletions substrate/frame/offences/benchmarking/src/inner.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
use alloc::{vec, vec::Vec};

use frame_benchmarking::v1::{account, benchmarks};
use frame_support::traits::{Currency, Get};
use frame_support::traits::Get;
use frame_system::{Config as SystemConfig, Pallet as System, RawOrigin};

use sp_runtime::{
Expand Down Expand Up @@ -77,8 +77,7 @@ where
}

type LookupSourceOf<T> = <<T as SystemConfig>::Lookup as StaticLookup>::Source;
type BalanceOf<T> =
<<T as StakingConfig>::Currency as Currency<<T as SystemConfig>::AccountId>>::Balance;
type BalanceOf<T> = <T as StakingConfig>::CurrencyBalance;

struct Offender<T: Config> {
pub controller: T::AccountId,
Expand All @@ -89,7 +88,7 @@ struct Offender<T: Config> {
}

fn bond_amount<T: Config>() -> BalanceOf<T> {
T::Currency::minimum_balance().saturating_mul(10_000u32.into())
pallet_staking::asset::existential_deposit::<T>().saturating_mul(10_000u32.into())
}

fn create_offender<T: Config>(n: u32, nominators: u32) -> Result<Offender<T>, &'static str> {
Expand All @@ -99,7 +98,7 @@ fn create_offender<T: Config>(n: u32, nominators: u32) -> Result<Offender<T>, &'
let amount = bond_amount::<T>();
// add twice as much balance to prevent the account from being killed.
let free_amount = amount.saturating_mul(2u32.into());
T::Currency::make_free_balance_be(&stash, free_amount);
pallet_staking::asset::set_stakeable_balance::<T>(&stash, free_amount);
Staking::<T>::bond(
RawOrigin::Signed(stash.clone()).into(),
amount,
Expand All @@ -116,7 +115,7 @@ fn create_offender<T: Config>(n: u32, nominators: u32) -> Result<Offender<T>, &'
for i in 0..nominators {
let nominator_stash: T::AccountId =
account("nominator stash", n * MAX_NOMINATORS + i, SEED);
T::Currency::make_free_balance_be(&nominator_stash, free_amount);
pallet_staking::asset::set_stakeable_balance::<T>(&nominator_stash, free_amount);

Staking::<T>::bond(
RawOrigin::Signed(nominator_stash.clone()).into(),
Expand Down Expand Up @@ -172,6 +171,14 @@ fn make_offenders<T: Config>(
}

benchmarks! {
where_clause {
where
<T as frame_system::Config>::RuntimeEvent: TryInto<pallet_staking::Event<T>>,
<T as frame_system::Config>::RuntimeEvent: TryInto<pallet_balances::Event<T>>,
<T as frame_system::Config>::RuntimeEvent: TryInto<pallet_offences::Event>,
<T as frame_system::Config>::RuntimeEvent: TryInto<frame_system::Event<T>>,
}

report_offence_grandpa {
let n in 0 .. MAX_NOMINATORS.min(MaxNominationsOf::<T>::get());

Expand All @@ -196,17 +203,19 @@ benchmarks! {
let _ = Offences::<T>::report_offence(reporters, offence);
}
verify {
// make sure that all slashes have been applied
#[cfg(test)]
assert_eq!(
System::<T>::event_count(), 0
+ 1 // offence
+ 3 // reporter (reward + endowment)
+ 1 // offenders reported
+ 3 // offenders slashed
+ 1 // offenders chilled
+ 3 * n // nominators slashed
);
{
// make sure that all slashes have been applied
// (n nominators + one validator) * (slashed + unlocked) + deposit to reporter + reporter
// account endowed + some funds rescinded from issuance.
assert_eq!(System::<T>::read_events_for_pallet::<pallet_balances::Event<T>>().len(), 2 * (n + 1) as usize + 3);
// (n nominators + one validator) * slashed + Slash Reported
assert_eq!(System::<T>::read_events_for_pallet::<pallet_staking::Event<T>>().len(), 1 * (n + 1) as usize + 1);
// offence
assert_eq!(System::<T>::read_events_for_pallet::<pallet_offences::Event>().len(), 1);
// reporter new account
assert_eq!(System::<T>::read_events_for_pallet::<frame_system::Event<T>>().len(), 1);
}
}

report_offence_babe {
Expand All @@ -233,17 +242,19 @@ benchmarks! {
let _ = Offences::<T>::report_offence(reporters, offence);
}
verify {
// make sure that all slashes have been applied
#[cfg(test)]
assert_eq!(
System::<T>::event_count(), 0
+ 1 // offence
+ 3 // reporter (reward + endowment)
+ 1 // offenders reported
+ 3 // offenders slashed
+ 1 // offenders chilled
+ 3 * n // nominators slashed
);
{
// make sure that all slashes have been applied
// (n nominators + one validator) * (slashed + unlocked) + deposit to reporter + reporter
// account endowed + some funds rescinded from issuance.
assert_eq!(System::<T>::read_events_for_pallet::<pallet_balances::Event<T>>().len(), 2 * (n + 1) as usize + 3);
// (n nominators + one validator) * slashed + Slash Reported
assert_eq!(System::<T>::read_events_for_pallet::<pallet_staking::Event<T>>().len(), 1 * (n + 1) as usize + 1);
// offence
assert_eq!(System::<T>::read_events_for_pallet::<pallet_offences::Event>().len(), 1);
// reporter new account
assert_eq!(System::<T>::read_events_for_pallet::<frame_system::Event<T>>().len(), 1);
}
}

impl_benchmark_test_suite!(Pallet, crate::mock::new_test_ext(), crate::mock::Test);
Expand Down
Loading

0 comments on commit 9128dca

Please sign in to comment.