Skip to content

Commit

Permalink
[Enhancement] Convert fast-unstake to use StakingInterface, decouplin… (
Browse files Browse the repository at this point in the history
paritytech#12424)

* [Enhancement] Convert fast-unstake to use StakingInterface, decoupling it from Staking

* Update primitives/staking/src/lib.rs

Co-authored-by: Kian Paimani <[email protected]>

* Update primitives/staking/src/lib.rs

Co-authored-by: Kian Paimani <[email protected]>

* fix validator comment

* remove todo

* ideas from Kian (paritytech#12474)

Co-authored-by: kianenigma <[email protected]>

* Rename StakingInterface -> Staking for nomination-pools

* Staking fixes

* StakingInterface changes

* fix fast-unstake

* fix nomination-pools

* Fix fast-unstake tests

* Fix benches for fast-unstake

* fix is_unbonding

* fix nomination pools

* fix node code

* add mock comments

* fix imports

* remove todo

* more fixes

* more fixes

* bench fixes

* more fixes

* more fixes

* import fix

* more fixes

* more bench fix

* refix

* refix

* Update primitives/staking/src/lib.rs

Co-authored-by: Kian Paimani <[email protected]>

* is_unbonding returns a result

* fix

* review fixes

* more review fixes

* more fixes

* more fixes

* Update frame/fast-unstake/src/benchmarking.rs

Co-authored-by: Squirrel <[email protected]>

* remove redundant CurrencyBalance from nom-pools

* remove CB

* rephrase

* Apply suggestions from code review

* Update frame/nomination-pools/src/tests.rs

* finish damn renamed

* clippy fix

* fix

Co-authored-by: Kian Paimani <[email protected]>
Co-authored-by: kianenigma <[email protected]>
Co-authored-by: parity-processbot <>
Co-authored-by: Squirrel <[email protected]>
  • Loading branch information
4 people authored Oct 29, 2022
1 parent be25923 commit e8cbc1e
Show file tree
Hide file tree
Showing 16 changed files with 373 additions and 306 deletions.
6 changes: 3 additions & 3 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -585,7 +585,8 @@ impl pallet_fast_unstake::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type ControlOrigin = frame_system::EnsureRoot<AccountId>;
type Deposit = ConstU128<{ DOLLARS }>;
type DepositCurrency = Balances;
type Currency = Balances;
type Staking = Staking;
type WeightInfo = ();
}

Expand Down Expand Up @@ -773,11 +774,10 @@ impl pallet_nomination_pools::Config for Runtime {
type WeightInfo = ();
type RuntimeEvent = RuntimeEvent;
type Currency = Balances;
type CurrencyBalance = Balance;
type RewardCounter = FixedU128;
type BalanceToU256 = BalanceToU256;
type U256ToBalance = U256ToBalance;
type StakingInterface = pallet_staking::Pallet<Self>;
type Staking = Staking;
type PostUnbondingPoolsWindow = PostUnbondPoolsWindow;
type MaxMetadataLen = ConstU32<256>;
type MaxUnbonding = ConstU32<8>;
Expand Down
13 changes: 5 additions & 8 deletions frame/fast-unstake/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@ sp-io = { version = "6.0.0", default-features = false, path = "../../primitives/
sp-runtime = { version = "6.0.0", default-features = false, path = "../../primitives/runtime" }
sp-std = { version = "4.0.0", default-features = false, path = "../../primitives/std" }
sp-staking = { default-features = false, path = "../../primitives/staking" }

pallet-balances = { default-features = false, path = "../balances" }
pallet-timestamp = { default-features = false, path = "../timestamp" }
pallet-staking = { default-features = false, path = "../staking" }
frame-election-provider-support = { default-features = false, path = "../election-provider-support" }

frame-benchmarking = { version = "4.0.0-dev", default-features = false, optional = true, path = "../benchmarking" }
Expand All @@ -37,6 +33,10 @@ pallet-staking-reward-curve = { version = "4.0.0-dev", path = "../staking/reward
sp-core = { version = "6.0.0", default-features = false, path = "../../primitives/core" }
substrate-test-utils = { version = "4.0.0-dev", path = "../../test-utils" }
sp-tracing = { version = "5.0.0", path = "../../primitives/tracing" }
pallet-staking = { path = "../staking" }
pallet-balances = { path = "../balances" }
pallet-timestamp = { path = "../timestamp" }


[features]
default = ["std"]
Expand All @@ -53,16 +53,13 @@ std = [
"sp-runtime/std",
"sp-std/std",

"pallet-staking/std",
"pallet-balances/std",
"pallet-timestamp/std",
"frame-election-provider-support/std",

"frame-benchmarking/std",
]
runtime-benchmarks = [
"frame-benchmarking/runtime-benchmarks",
"frame-system/runtime-benchmarks",
"pallet-staking/runtime-benchmarks",
"sp-staking/runtime-benchmarks"
]
try-runtime = ["frame-support/try-runtime"]
40 changes: 11 additions & 29 deletions frame/fast-unstake/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,22 +26,15 @@ use frame_support::{
traits::{Currency, EnsureOrigin, Get, Hooks},
};
use frame_system::RawOrigin;
use pallet_staking::Pallet as Staking;
use sp_runtime::traits::{StaticLookup, Zero};
use sp_staking::EraIndex;
use sp_runtime::traits::Zero;
use sp_staking::{EraIndex, StakingInterface};
use sp_std::prelude::*;

const USER_SEED: u32 = 0;
const DEFAULT_BACKER_PER_VALIDATOR: u32 = 128;
const MAX_VALIDATORS: u32 = 128;

type CurrencyOf<T> = <T as pallet_staking::Config>::Currency;

fn l<T: Config>(
who: T::AccountId,
) -> <<T as frame_system::Config>::Lookup as StaticLookup>::Source {
T::Lookup::unlookup(who)
}
type CurrencyOf<T> = <T as Config>::Currency;

fn create_unexposed_nominator<T: Config>() -> T::AccountId {
let account = frame_benchmarking::account::<T::AccountId>("nominator_42", 0, USER_SEED);
Expand All @@ -53,18 +46,9 @@ fn fund_and_bond_account<T: Config>(account: &T::AccountId) {
let stake = CurrencyOf::<T>::minimum_balance() * 100u32.into();
CurrencyOf::<T>::make_free_balance_be(&account, stake * 10u32.into());

let account_lookup = l::<T>(account.clone());
// bond and nominate ourselves, this will guarantee that we are not backing anyone.
assert_ok!(Staking::<T>::bond(
RawOrigin::Signed(account.clone()).into(),
account_lookup.clone(),
stake,
pallet_staking::RewardDestination::Controller,
));
assert_ok!(Staking::<T>::nominate(
RawOrigin::Signed(account.clone()).into(),
vec![account_lookup]
));
assert_ok!(T::Staking::bond(account, stake, account));
assert_ok!(T::Staking::nominate(account, vec![account.clone()]));
}

pub(crate) fn fast_unstake_events<T: Config>() -> Vec<crate::Event<T>> {
Expand All @@ -91,13 +75,11 @@ fn setup_staking<T: Config>(v: u32, until: EraIndex) {
.map(|s| {
let who = frame_benchmarking::account::<T::AccountId>("nominator", era, s);
let value = ed;
pallet_staking::IndividualExposure { who, value }
(who, value)
})
.collect::<Vec<_>>();
let exposure =
pallet_staking::Exposure { total: Default::default(), own: Default::default(), others };
validators.iter().for_each(|v| {
Staking::<T>::add_era_stakers(era, v.clone(), exposure.clone());
T::Staking::add_era_stakers(&era, &v, others.clone());
});
}
}
Expand Down Expand Up @@ -137,13 +119,13 @@ benchmarks! {
// on_idle, when we check some number of eras,
on_idle_check {
// number of eras multiplied by validators in that era.
let x in (<T as pallet_staking::Config>::BondingDuration::get() * 1) .. (<T as pallet_staking::Config>::BondingDuration::get() * MAX_VALIDATORS);
let x in (T::Staking::bonding_duration() * 1) .. (T::Staking::bonding_duration() * MAX_VALIDATORS);

let v = x / <T as pallet_staking::Config>::BondingDuration::get();
let u = <T as pallet_staking::Config>::BondingDuration::get();
let u = T::Staking::bonding_duration();
let v = x / u;

ErasToCheckPerBlock::<T>::put(u);
pallet_staking::CurrentEra::<T>::put(u);
T::Staking::set_current_era(u);

// setup staking with v validators and u eras of data (0..=u)
setup_staking::<T>(v, u);
Expand Down
85 changes: 34 additions & 51 deletions frame/fast-unstake/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,18 +80,16 @@ macro_rules! log {
pub mod pallet {
use super::*;
use crate::types::*;
use frame_election_provider_support::ElectionProviderBase;
use frame_support::{
pallet_prelude::*,
traits::{Defensive, ReservableCurrency},
};
use frame_system::{pallet_prelude::*, RawOrigin};
use pallet_staking::Pallet as Staking;
use frame_system::pallet_prelude::*;
use sp_runtime::{
traits::{Saturating, Zero},
DispatchResult,
};
use sp_staking::EraIndex;
use sp_staking::{EraIndex, StakingInterface};
use sp_std::{prelude::*, vec::Vec};
pub use weights::WeightInfo;

Expand All @@ -101,22 +99,22 @@ pub mod pallet {
pub struct MaxChecking<T: Config>(sp_std::marker::PhantomData<T>);
impl<T: Config> frame_support::traits::Get<u32> for MaxChecking<T> {
fn get() -> u32 {
<T as pallet_staking::Config>::BondingDuration::get() + 1
T::Staking::bonding_duration() + 1
}
}

#[pallet::pallet]
pub struct Pallet<T>(_);

#[pallet::config]
pub trait Config: frame_system::Config + pallet_staking::Config {
pub trait Config: frame_system::Config {
/// The overarching event type.
type RuntimeEvent: From<Event<Self>>
+ IsType<<Self as frame_system::Config>::RuntimeEvent>
+ TryInto<Event<Self>>;

/// The currency used for deposits.
type DepositCurrency: ReservableCurrency<Self::AccountId, Balance = BalanceOf<Self>>;
type Currency: ReservableCurrency<Self::AccountId>;

/// Deposit to take for unstaking, to make sure we're able to slash the it in order to cover
/// the costs of resources on unsuccessful unstake.
Expand All @@ -125,6 +123,9 @@ pub mod pallet {
/// The origin that can control this pallet.
type ControlOrigin: frame_support::traits::EnsureOrigin<Self::RuntimeOrigin>;

/// The access to staking functionality.
type Staking: StakingInterface<Balance = BalanceOf<Self>, AccountId = Self::AccountId>;

/// The weight information of this pallet.
type WeightInfo: WeightInfo;
}
Expand Down Expand Up @@ -221,28 +222,25 @@ pub mod pallet {
let ctrl = ensure_signed(origin)?;

ensure!(ErasToCheckPerBlock::<T>::get() != 0, <Error<T>>::CallNotAllowed);

let ledger =
pallet_staking::Ledger::<T>::get(&ctrl).ok_or(Error::<T>::NotController)?;
ensure!(!Queue::<T>::contains_key(&ledger.stash), Error::<T>::AlreadyQueued);
let stash_account =
T::Staking::stash_by_ctrl(&ctrl).map_err(|_| Error::<T>::NotController)?;
ensure!(!Queue::<T>::contains_key(&stash_account), Error::<T>::AlreadyQueued);
ensure!(
Head::<T>::get().map_or(true, |UnstakeRequest { stash, .. }| stash != ledger.stash),
Head::<T>::get()
.map_or(true, |UnstakeRequest { stash, .. }| stash_account != stash),
Error::<T>::AlreadyHead
);
// second part of the && is defensive.
ensure!(
ledger.active == ledger.total && ledger.unlocking.is_empty(),
Error::<T>::NotFullyBonded
);

ensure!(!T::Staking::is_unbonding(&stash_account)?, Error::<T>::NotFullyBonded);

// chill and fully unstake.
Staking::<T>::chill(RawOrigin::Signed(ctrl.clone()).into())?;
Staking::<T>::unbond(RawOrigin::Signed(ctrl).into(), ledger.total)?;
T::Staking::chill(&stash_account)?;
T::Staking::fully_unbond(&stash_account)?;

T::DepositCurrency::reserve(&ledger.stash, T::Deposit::get())?;
T::Currency::reserve(&stash_account, T::Deposit::get())?;

// enqueue them.
Queue::<T>::insert(ledger.stash, T::Deposit::get());
Queue::<T>::insert(stash_account, T::Deposit::get());
Ok(())
}

Expand All @@ -259,18 +257,18 @@ pub mod pallet {

ensure!(ErasToCheckPerBlock::<T>::get() != 0, <Error<T>>::CallNotAllowed);

let stash = pallet_staking::Ledger::<T>::get(&ctrl)
.map(|l| l.stash)
.ok_or(Error::<T>::NotController)?;
ensure!(Queue::<T>::contains_key(&stash), Error::<T>::NotQueued);
let stash_account =
T::Staking::stash_by_ctrl(&ctrl).map_err(|_| Error::<T>::NotController)?;
ensure!(Queue::<T>::contains_key(&stash_account), Error::<T>::NotQueued);
ensure!(
Head::<T>::get().map_or(true, |UnstakeRequest { stash, .. }| stash != stash),
Head::<T>::get()
.map_or(true, |UnstakeRequest { stash, .. }| stash_account != stash),
Error::<T>::AlreadyHead
);
let deposit = Queue::<T>::take(stash.clone());
let deposit = Queue::<T>::take(stash_account.clone());

if let Some(deposit) = deposit.defensive() {
let remaining = T::DepositCurrency::unreserve(&stash, deposit);
let remaining = T::Currency::unreserve(&stash_account, deposit);
if !remaining.is_zero() {
frame_support::defensive!("`not enough balance to unreserve`");
ErasToCheckPerBlock::<T>::put(0);
Expand Down Expand Up @@ -314,7 +312,7 @@ pub mod pallet {

// NOTE: here we're assuming that the number of validators has only ever increased,
// meaning that the number of exposures to check is either this per era, or less.
let validator_count = pallet_staking::ValidatorCount::<T>::get();
let validator_count = T::Staking::desired_validator_count();

// determine the number of eras to check. This is based on both `ErasToCheckPerBlock`
// and `remaining_weight` passed on to us from the runtime executive.
Expand All @@ -330,8 +328,7 @@ pub mod pallet {
}
}

if <<T as pallet_staking::Config>::ElectionProvider as ElectionProviderBase>::ongoing()
{
if T::Staking::election_ongoing() {
// NOTE: we assume `ongoing` does not consume any weight.
// there is an ongoing election -- we better not do anything. Imagine someone is not
// exposed anywhere in the last era, and the snapshot for the election is already
Expand Down Expand Up @@ -366,8 +363,8 @@ pub mod pallet {
);

// the range that we're allowed to check in this round.
let current_era = pallet_staking::CurrentEra::<T>::get().unwrap_or_default();
let bonding_duration = <T as pallet_staking::Config>::BondingDuration::get();
let current_era = T::Staking::current_era();
let bonding_duration = T::Staking::bonding_duration();
// prune all the old eras that we don't care about. This will help us keep the bound
// of `checked`.
checked.retain(|e| *e >= current_era.saturating_sub(bonding_duration));
Expand Down Expand Up @@ -401,16 +398,9 @@ pub mod pallet {
);

if unchecked_eras_to_check.is_empty() {
// `stash` is not exposed in any era now -- we can let go of them now.
let num_slashing_spans = Staking::<T>::slashing_spans(&stash).iter().count() as u32;
let result = T::Staking::force_unstake(stash.clone());

let result = pallet_staking::Pallet::<T>::force_unstake(
RawOrigin::Root.into(),
stash.clone(),
num_slashing_spans,
);

let remaining = T::DepositCurrency::unreserve(&stash, deposit);
let remaining = T::Currency::unreserve(&stash, deposit);
if !remaining.is_zero() {
frame_support::defensive!("`not enough balance to unreserve`");
ErasToCheckPerBlock::<T>::put(0);
Expand All @@ -426,7 +416,7 @@ pub mod pallet {
let mut eras_checked = 0u32;
let is_exposed = unchecked_eras_to_check.iter().any(|e| {
eras_checked.saturating_inc();
Self::is_exposed_in_era(&stash, e)
T::Staking::is_exposed_in_era(&stash, e)
});

log!(
Expand All @@ -442,7 +432,7 @@ pub mod pallet {
// the last 28 eras, have registered yourself to be unstaked, midway being checked,
// you are exposed.
if is_exposed {
T::DepositCurrency::slash_reserved(&stash, deposit);
T::Currency::slash_reserved(&stash, deposit);
log!(info, "slashed {:?} by {:?}", stash, deposit);
Self::deposit_event(Event::<T>::Slashed { stash, amount: deposit });
} else {
Expand Down Expand Up @@ -472,12 +462,5 @@ pub mod pallet {
<T as Config>::WeightInfo::on_idle_check(validator_count * eras_checked)
}
}

/// Checks whether an account `staker` has been exposed in an era.
fn is_exposed_in_era(staker: &T::AccountId, era: &EraIndex) -> bool {
pallet_staking::ErasStakers::<T>::iter_prefix(era).any(|(validator, exposures)| {
validator == *staker || exposures.others.iter().any(|i| i.who == *staker)
})
}
}
}
3 changes: 2 additions & 1 deletion frame/fast-unstake/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ parameter_types! {
impl fast_unstake::Config for Runtime {
type RuntimeEvent = RuntimeEvent;
type Deposit = DepositAmount;
type DepositCurrency = Balances;
type Currency = Balances;
type Staking = Staking;
type ControlOrigin = frame_system::EnsureRoot<Self::AccountId>;
type WeightInfo = ();
}
Expand Down
Loading

0 comments on commit e8cbc1e

Please sign in to comment.