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 6 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 {
ensure!(Self::era_election_status().is_closed(), Error::<T>::CallNotAllowed);
let controller = ensure_signed(origin)?;
gavofyork marked this conversation as resolved.
Show resolved Hide resolved
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