Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

[Enhancement] Convert fast-unstake to use StakingInterface, decouplin… #12424

Merged
merged 48 commits into from
Oct 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
48 commits
Select commit Hold shift + click to select a range
3a11e89
[Enhancement] Convert fast-unstake to use StakingInterface, decouplin…
ruseinov Oct 5, 2022
4f39589
Update primitives/staking/src/lib.rs
ruseinov Oct 6, 2022
9b8e123
Update primitives/staking/src/lib.rs
ruseinov Oct 6, 2022
25520d6
fix validator comment
ruseinov Oct 6, 2022
ad90903
remove todo
ruseinov Oct 10, 2022
89b32b5
ideas from Kian (#12474)
ruseinov Oct 11, 2022
f51c073
Merge remote-tracking branch 'origin/master' into ru/feature/fast-uns…
Oct 11, 2022
41d04ad
Rename StakingInterface -> Staking for nomination-pools
ruseinov Oct 12, 2022
7fc52f8
Staking fixes
ruseinov Oct 12, 2022
d3958ba
StakingInterface changes
ruseinov Oct 12, 2022
2ff689c
fix fast-unstake
ruseinov Oct 12, 2022
1fc6510
fix nomination-pools
ruseinov Oct 12, 2022
28d15d7
Merge remote-tracking branch 'origin/master' into ru/feature/fast-uns…
Oct 18, 2022
0705029
Fix fast-unstake tests
ruseinov Oct 18, 2022
39cf580
Fix benches for fast-unstake
ruseinov Oct 18, 2022
8093567
fix is_unbonding
ruseinov Oct 18, 2022
30a5882
fix nomination pools
ruseinov Oct 18, 2022
37107ef
fix node code
ruseinov Oct 19, 2022
097a9d1
add mock comments
ruseinov Oct 19, 2022
613a701
fix imports
ruseinov Oct 19, 2022
e785dd5
remove todo
ruseinov Oct 19, 2022
03586d7
more fixes
ruseinov Oct 19, 2022
20a4f29
more fixes
ruseinov Oct 19, 2022
9807cf2
bench fixes
ruseinov Oct 19, 2022
3639af8
more fixes
ruseinov Oct 19, 2022
b7827d5
more fixes
ruseinov Oct 20, 2022
5232b99
import fix
ruseinov Oct 20, 2022
f881456
more fixes
ruseinov Oct 20, 2022
c3cee71
more bench fix
ruseinov Oct 20, 2022
068c565
refix
ruseinov Oct 20, 2022
ff36f90
refix
ruseinov Oct 20, 2022
6f862e4
Update primitives/staking/src/lib.rs
ruseinov Oct 25, 2022
bff58a4
is_unbonding returns a result
ruseinov Oct 20, 2022
bdb525e
fix
ruseinov Oct 25, 2022
4bd57b1
review fixes
ruseinov Oct 25, 2022
f6040ec
more review fixes
ruseinov Oct 25, 2022
8c5604d
more fixes
ruseinov Oct 25, 2022
731fab2
more fixes
ruseinov Oct 25, 2022
b90082a
Update frame/fast-unstake/src/benchmarking.rs
ruseinov Oct 26, 2022
1eee8a5
remove redundant CurrencyBalance from nom-pools
ruseinov Oct 26, 2022
1f4fad2
remove CB
ruseinov Oct 26, 2022
27530dc
Merge remote-tracking branch 'origin/master' into ru/feature/fast-uns…
Oct 26, 2022
90cec9f
rephrase
ruseinov Oct 26, 2022
b1560aa
Apply suggestions from code review
kianenigma Oct 27, 2022
75a36bb
Update frame/nomination-pools/src/tests.rs
kianenigma Oct 27, 2022
b3e843e
finish damn renamed
kianenigma Oct 27, 2022
963837b
clippy fix
ruseinov Oct 27, 2022
cbed2f9
fix
ruseinov Oct 27, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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" }
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
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>;
kianenigma marked this conversation as resolved.
Show resolved Hide resolved

/// 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(),
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still need to check for ledger.unlocking.is_empty() ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Nope, we are not exposing the ledger anymore, so the defensive condition has to go.

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);
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
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