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

Allow validators to block and kick their nominator set. #7930

Merged
merged 8 commits into from
Jan 20, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions frame/offences/benchmarking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,7 @@ fn create_offender<T: Config>(n: u32, nominators: u32) -> Result<Offender<T>, &'

let validator_prefs = ValidatorPrefs {
commission: Perbill::from_percent(50),
.. Default::default()
};
Staking::<T>::validate(RawOrigin::Signed(controller.clone()).into(), validator_prefs)?;

Expand Down
57 changes: 57 additions & 0 deletions frame/staking/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ pub fn create_validator_with_nominators<T: Config>(
let (v_stash, v_controller) = create_stash_controller::<T>(0, 100, destination.clone())?;
let validator_prefs = ValidatorPrefs {
commission: Perbill::from_percent(50),
.. Default::default()
};
Staking::<T>::validate(RawOrigin::Signed(v_controller).into(), validator_prefs)?;
let stash_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(v_stash.clone());
Expand Down Expand Up @@ -198,6 +199,61 @@ benchmarks! {
assert!(Validators::<T>::contains_key(stash));
}

kick {
// scenario: we want to kick `k` nominators from nominating us (we are a validator).
// we'll assume that `k` is under 128 for the purposes of determining the slope.
// each nominator should have `MAX_NOMINATIONS` validators nominated, and our validator
// should be somewhere in there.
let k in 1 .. 128;

// these are the other validators; there are `MAX_NOMINATIONS - 1` of them, so there are a
// total of `MAX_NOMINATIONS` validators in the system.
let rest_of_validators = create_validators::<T>(MAX_NOMINATIONS as u32 - 1, 100)?;

// this is the validator that will be kicking.
let (stash, controller) = create_stash_controller::<T>(MAX_NOMINATIONS as u32 - 1, 100, Default::default())?;
let stash_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(stash.clone());

// they start validating.
Staking::<T>::validate(RawOrigin::Signed(controller.clone()).into(), Default::default())?;

// we now create the nominators. there will be `k` of them; each will nominate all
// validators. we will then kick each of the `k` nominators from the main validator.
let mut nominator_stashes = Vec::with_capacity(k as usize);
for i in 0 .. k {
// create a nominator stash.
let (n_stash, n_controller) = create_stash_controller::<T>(MAX_NOMINATIONS as u32 + i, 100, Default::default())?;

// bake the nominations; we first clone them from the rest of the validators.
let mut nominations = rest_of_validators.clone();
// then insert "our" validator somewhere in there (we vary it) to avoid accidental
// optimisations/pessimisations.
nominations.insert(i as usize % (nominations.len() + 1), stash_lookup.clone());
// then we nominate.
Staking::<T>::nominate(RawOrigin::Signed(n_controller.clone()).into(), nominations)?;

nominator_stashes.push(n_stash);
}

// all nominators now should be nominating our validator...
for n in nominator_stashes.iter() {
assert!(Nominators::<T>::get(n).unwrap().targets.contains(&stash));
}

// we need the unlookuped version of the nominator stash for the kick.
let kicks = nominator_stashes.iter()
.map(|n| T::Lookup::unlookup(n.clone()))
.collect::<Vec<_>>();

whitelist_account!(controller);
}: _(RawOrigin::Signed(controller), kicks)
verify {
// all nominators now should *not* be nominating our validator...
for n in nominator_stashes.iter() {
assert!(!Nominators::<T>::get(n).unwrap().targets.contains(&stash));
}
}

// Worst case scenario, MAX_NOMINATIONS
nominate {
let n in 1 .. MAX_NOMINATIONS as u32;
Expand Down Expand Up @@ -814,6 +870,7 @@ mod tests {
assert_ok!(test_benchmark_withdraw_unbonded_update::<Test>());
assert_ok!(test_benchmark_withdraw_unbonded_kill::<Test>());
assert_ok!(test_benchmark_validate::<Test>());
assert_ok!(test_benchmark_kick::<Test>());
assert_ok!(test_benchmark_nominate::<Test>());
assert_ok!(test_benchmark_chill::<Test>());
assert_ok!(test_benchmark_set_payee::<Test>());
Expand Down
98 changes: 93 additions & 5 deletions frame/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -454,12 +454,17 @@ pub struct ValidatorPrefs {
/// nominators.
#[codec(compact)]
pub commission: Perbill,
/// Whether or not this validator is accepting more nominations. If `true`, then no nominator
/// who is not already nominating this validator may nominate them. By default, validators
/// are accepting nominations.
pub blocked: bool,
}

impl Default for ValidatorPrefs {
fn default() -> Self {
ValidatorPrefs {
commission: Default::default(),
blocked: false,
}
}
}
Expand Down Expand Up @@ -896,11 +901,12 @@ enum Releases {
V2_0_0,
V3_0_0,
V4_0_0,
V5_0_0,
}

impl Default for Releases {
fn default() -> Self {
Releases::V4_0_0
Releases::V5_0_0
}
}

Expand Down Expand Up @@ -1087,8 +1093,8 @@ decl_storage! {
/// True if network has been upgraded to this version.
/// Storage version of the pallet.
///
/// This is set to v3.0.0 for new networks.
StorageVersion build(|_: &GenesisConfig<T>| Releases::V4_0_0): Releases;
/// This is set to v5.0.0 for new networks.
StorageVersion build(|_: &GenesisConfig<T>| Releases::V5_0_0): Releases;
}
add_extra_genesis {
config(stakers):
Expand Down Expand Up @@ -1124,6 +1130,29 @@ decl_storage! {
}
}

pub mod migrations {
use super::*;

#[derive(Decode)]
struct OldValidatorPrefs {
#[codec(compact)]
pub commission: Perbill
}
impl OldValidatorPrefs {
fn upgraded(self) -> ValidatorPrefs {
ValidatorPrefs {
commission: self.commission,
.. Default::default()
}
}
}
pub fn migrate_to_blockable<T: Config>() -> frame_support::weights::Weight {
Validators::<T>::translate::<OldValidatorPrefs, _>(|_, p| Some(p.upgraded()));
ErasValidatorPrefs::<T>::translate::<OldValidatorPrefs, _>(|_, _, p| Some(p.upgraded()));
T::BlockWeights::get().max_block
}
}

decl_event!(
pub enum Event<T> where Balance = BalanceOf<T>, <T as frame_system::Config>::AccountId {
/// The era payout has been set; the first balance is the validator-payout; the second is
Expand Down Expand Up @@ -1152,6 +1181,8 @@ decl_event!(
/// An account has called `withdraw_unbonded` and removed unbonding chunks worth `Balance`
/// from the unlocking queue. \[stash, amount\]
Withdrawn(AccountId, Balance),
/// A nominator has been kicked from a validator. \[nominator, stash\]
Kicked(AccountId, AccountId),
}
);

Expand Down Expand Up @@ -1225,6 +1256,10 @@ decl_error! {
IncorrectSlashingSpans,
/// Internal state has become somehow corrupted and the operation cannot continue.
BadState,
/// Too many nomination targets supplied.
TooManyTargets,
/// A nomination target was supplied that was blocked or otherwise not a validator.
BadTarget,
}
}

Expand Down Expand Up @@ -1270,6 +1305,15 @@ decl_module! {

fn deposit_event() = default;

fn on_runtime_upgrade() -> frame_support::weights::Weight {
if StorageVersion::get() == Releases::V4_0_0 {
StorageVersion::put(Releases::V5_0_0);
migrations::migrate_to_blockable::<T>()
} else {
0
}
}

/// sets `ElectionStatus` to `Open(now)` where `now` is the block number at which the
/// election window has opened, if we are at the last session and less blocks than
/// `T::ElectionLookahead` is remaining until the next new session schedule. The offchain
Expand Down Expand Up @@ -1675,9 +1719,17 @@ decl_module! {
let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
let stash = &ledger.stash;
ensure!(!targets.is_empty(), Error::<T>::EmptyTargets);
ensure!(targets.len() <= MAX_NOMINATIONS, Error::<T>::TooManyTargets);

let old = Nominators::<T>::get(stash).map_or_else(Vec::new, |x| x.targets);

let targets = targets.into_iter()
.take(MAX_NOMINATIONS)
.map(|t| T::Lookup::lookup(t))
.map(|t| T::Lookup::lookup(t).map_err(DispatchError::from))
.map(|n| n.and_then(|n| if old.contains(&n) || !Validators::<T>::get(&n).blocked {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this reads nicer if the if-else body is indented.

Ok(n)
} else {
Err(Error::<T>::BadTarget.into())
}))
.collect::<result::Result<Vec<T::AccountId>, _>>()?;

let nominations = Nominations {
Expand Down Expand Up @@ -2168,6 +2220,42 @@ decl_module! {

Ok(adjustments)
}

/// Remove the given nominations from the calling validator.
///
/// Effects will be felt at the beginning of the next era.
///
/// The dispatch origin for this call must be _Signed_ by the controller, not the stash.
/// And, it can be only called when [`EraElectionStatus`] is `Closed`. The controller
/// account should represent a validator.
///
/// - `who`: A list of nominator stash accounts who are nominating this validator which
/// should no longer be nominating this validator.
///
/// Note: Making this call only makes sense if you first set the validator preferences to
/// block any further nominations.
#[weight = T::WeightInfo::kick(who.len() as u32)]
pub fn kick(origin, who: Vec<<T::Lookup as StaticLookup>::Source>) -> DispatchResult {
let controller = ensure_signed(origin)?;
ensure!(Self::era_election_status().is_closed(), Error::<T>::CallNotAllowed);
let ledger = Self::ledger(&controller).ok_or(Error::<T>::NotController)?;
let stash = &ledger.stash;

for nom_stash in who.into_iter()
.map(T::Lookup::lookup)
.collect::<Result<Vec<T::AccountId>, _>>()?
.into_iter()
{
Nominators::<T>::mutate(&nom_stash, |maybe_nom| if let Some(ref mut nom) = maybe_nom {
if let Some(pos) = nom.targets.iter().position(|v| v == stash) {
nom.targets.swap_remove(pos);
Self::deposit_event(RawEvent::Kicked(nom_stash.clone(), stash.clone()));
}
});
}

Ok(())
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions frame/staking/src/offchain_election.rs
Original file line number Diff line number Diff line change
Expand Up @@ -520,6 +520,9 @@ mod test {
fn submit_solution_better(v: u32, n: u32, a: u32, w: u32) -> Weight {
(0 * v + 0 * n + 1000 * a + 0 * w) as Weight
}
fn kick(w: u32) -> Weight {
unimplemented!()
}
}

#[test]
Expand Down
2 changes: 2 additions & 0 deletions frame/staking/src/testing_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ pub fn create_validators<T: Config>(
let (stash, controller) = create_stash_controller::<T>(i, balance_factor, RewardDestination::Staked)?;
let validator_prefs = ValidatorPrefs {
commission: Perbill::from_percent(50),
.. Default::default()
};
Staking::<T>::validate(RawOrigin::Signed(controller).into(), validator_prefs)?;
let stash_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(stash);
Expand Down Expand Up @@ -134,6 +135,7 @@ pub fn create_validators_with_nominators_for_era<T: Config>(
let (v_stash, v_controller) = create_stash_controller::<T>(i, balance_factor, RewardDestination::Staked)?;
let validator_prefs = ValidatorPrefs {
commission: Perbill::from_percent(50),
.. Default::default()
};
Staking::<T>::validate(RawOrigin::Signed(v_controller.clone()).into(), validator_prefs)?;
let stash_lookup: <T::Lookup as StaticLookup>::Source = T::Lookup::unlookup(v_stash.clone());
Expand Down
27 changes: 26 additions & 1 deletion frame/staking/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,30 @@ fn staking_should_work() {
});
}

#[test]
fn blocking_and_kicking_works() {
ExtBuilder::default()
.minimum_validator_count(1)
.validator_count(4)
.nominate(true)
.num_validators(3)
.build()
.execute_with(|| {
// block validator 10/11
assert_ok!(Staking::validate(Origin::signed(10), ValidatorPrefs { blocked: true, .. Default::default() }));
// attempt to nominate from 100/101...
assert_ok!(Staking::nominate(Origin::signed(100), vec![11]));
// should have worked since we're already nominated them
assert_eq!(Nominators::<Test>::get(&101).unwrap().targets, vec![11]);
// kick the nominator
assert_ok!(Staking::kick(Origin::signed(10), vec![101]));
// should have been kicked now
assert!(Nominators::<Test>::get(&101).unwrap().targets.is_empty());
// attempt to nominate from 100/101...
assert_noop!(Staking::nominate(Origin::signed(100), vec![11]), Error::<Test>::BadTarget);
});
}

#[test]
fn less_than_needed_candidates_works() {
ExtBuilder::default()
Expand Down Expand Up @@ -403,7 +427,7 @@ fn no_candidate_emergency_condition() {
.execute_with(|| {
// initial validators
assert_eq_uvec!(validator_controllers(), vec![10, 20, 30, 40]);
let prefs = ValidatorPrefs { commission: Perbill::one() };
let prefs = ValidatorPrefs { commission: Perbill::one(), .. Default::default() };
<Staking as crate::Store>::Validators::insert(11, prefs.clone());

// set the minimum validator count.
Expand Down Expand Up @@ -971,6 +995,7 @@ fn validator_payment_prefs_work() {
let commission = Perbill::from_percent(40);
<Validators<Test>>::insert(&11, ValidatorPrefs {
commission: commission.clone(),
.. Default::default()
});

// Reward controller so staked ratio doesn't change.
Expand Down
Loading