Remove implicit approval chilling upon slash. (#12420)

* don't read slashing spans when taking election snapshot

* update cargo.toml

* bring back remote test

* fix merge stuff

* fix npos-voters function sig

* remove as much redundant diff as you can

* Update frame/staking/src/pallet/mod.rs

Co-authored-by: Andronik <write@reusable.software>

* fix

* Update frame/staking/src/pallet/impls.rs

* update lock

* fix all tests

* review comments

* fmt

* fix offence bench

* clippy

* ".git/.scripts/bench-bot.sh" pallet dev pallet_staking

Co-authored-by: Andronik <write@reusable.software>
Co-authored-by: Ankan <ankan.anurag@gmail.com>
Co-authored-by: command-bot <>
This commit is contained in:
Kian Paimani
2022-12-12 17:05:13 +00:00
committed by GitHub
parent af46f85e0c
commit 0b29691688
9 changed files with 526 additions and 546 deletions
+158 -172
View File
@@ -2845,6 +2845,8 @@ fn deferred_slashes_are_deferred() {
assert_eq!(Balances::free_balance(101), 2000);
let nominated_value = exposure.others.iter().find(|o| o.who == 101).unwrap().value;
System::reset_events();
on_offence_now(
&[OffenceDetails {
offender: (11, Staking::eras_stakers(active_era(), 11)),
@@ -2853,6 +2855,9 @@ fn deferred_slashes_are_deferred() {
&[Perbill::from_percent(10)],
);
// nominations are not removed regardless of the deferring.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
assert_eq!(Balances::free_balance(11), 1000);
assert_eq!(Balances::free_balance(101), 2000);
@@ -2866,8 +2871,6 @@ fn deferred_slashes_are_deferred() {
assert_eq!(Balances::free_balance(11), 1000);
assert_eq!(Balances::free_balance(101), 2000);
System::reset_events();
// at the start of era 4, slashes from era 1 are processed,
// after being deferred for at least 2 full eras.
mock::start_active_era(4);
@@ -2875,15 +2878,16 @@ fn deferred_slashes_are_deferred() {
assert_eq!(Balances::free_balance(11), 900);
assert_eq!(Balances::free_balance(101), 2000 - (nominated_value / 10));
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid { era_index: 3, validator_payout: 11075, remainder: 33225 },
assert!(matches!(
staking_events_since_last_call().as_slice(),
&[
Event::Chilled { stash: 11 },
Event::SlashReported { validator: 11, slash_era: 1, .. },
..,
Event::Slashed { staker: 11, amount: 100 },
Event::Slashed { staker: 101, amount: 12 }
]
);
));
})
}
@@ -2896,25 +2900,29 @@ fn retroactive_deferred_slashes_two_eras_before() {
let exposure_11_at_era1 = Staking::eras_stakers(active_era(), 11);
mock::start_active_era(3);
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
System::reset_events();
on_offence_in_era(
&[OffenceDetails { offender: (11, exposure_11_at_era1), reporters: vec![] }],
&[Perbill::from_percent(10)],
1, // should be deferred for two full eras, and applied at the beginning of era 4.
DisableStrategy::Never,
);
System::reset_events();
mock::start_active_era(4);
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid { era_index: 3, validator_payout: 7100, remainder: 21300 },
assert!(matches!(
staking_events_since_last_call().as_slice(),
&[
Event::Chilled { stash: 11 },
Event::SlashReported { validator: 11, slash_era: 1, .. },
..,
Event::Slashed { staker: 11, amount: 100 },
Event::Slashed { staker: 101, amount: 12 },
Event::Slashed { staker: 101, amount: 12 }
]
);
));
})
}
@@ -2932,35 +2940,29 @@ fn retroactive_deferred_slashes_one_before() {
assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 100));
mock::start_active_era(3);
System::reset_events();
on_offence_in_era(
&[OffenceDetails { offender: (11, exposure_11_at_era1), reporters: vec![] }],
&[Perbill::from_percent(10)],
2, // should be deferred for two full eras, and applied at the beginning of era 5.
DisableStrategy::Never,
);
System::reset_events();
mock::start_active_era(4);
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid { era_index: 3, validator_payout: 11075, remainder: 33225 }
]
);
assert_eq!(Staking::ledger(10).unwrap().total, 1000);
// slash happens after the next line.
mock::start_active_era(5);
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid { era_index: 4, validator_payout: 11075, remainder: 33225 },
assert!(matches!(
staking_events_since_last_call().as_slice(),
&[
Event::SlashReported { validator: 11, slash_era: 2, .. },
..,
Event::Slashed { staker: 11, amount: 100 },
Event::Slashed { staker: 101, amount: 12 }
]
);
));
// their ledger has already been slashed.
assert_eq!(Staking::ledger(10).unwrap().total, 900);
@@ -3068,6 +3070,7 @@ fn remove_deferred() {
mock::start_active_era(2);
// reported later, but deferred to start of era 4 as well.
System::reset_events();
on_offence_in_era(
&[OffenceDetails { offender: (11, exposure.clone()), reporters: vec![] }],
&[Perbill::from_percent(15)],
@@ -3094,19 +3097,18 @@ fn remove_deferred() {
// at the start of era 4, slashes from era 1 are processed,
// after being deferred for at least 2 full eras.
System::reset_events();
mock::start_active_era(4);
// the first slash for 10% was cancelled, but the 15% one
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid { era_index: 3, validator_payout: 11075, remainder: 33225 },
// the first slash for 10% was cancelled, but the 15% one not.
assert!(matches!(
staking_events_since_last_call().as_slice(),
&[
Event::SlashReported { validator: 11, slash_era: 1, .. },
..,
Event::Slashed { staker: 11, amount: 50 },
Event::Slashed { staker: 101, amount: 7 }
]
);
));
let slash_10 = Perbill::from_percent(10);
let slash_15 = Perbill::from_percent(15);
@@ -3196,6 +3198,9 @@ fn slash_kicks_validators_not_nominators_and_disables_nominator_for_kicked_valid
assert_eq!(Balances::free_balance(11), 1000);
assert_eq!(Balances::free_balance(101), 2000);
// 100 has approval for 11 as of now
assert!(Staking::nominators(101).unwrap().targets.contains(&11));
// 11 and 21 both have the support of 100
let exposure_11 = Staking::eras_stakers(active_era(), &11);
let exposure_21 = Staking::eras_stakers(active_era(), &21);
@@ -3208,23 +3213,29 @@ fn slash_kicks_validators_not_nominators_and_disables_nominator_for_kicked_valid
&[Perbill::from_percent(10)],
);
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 },
Event::Chilled { stash: 11 },
Event::SlashReported {
validator: 11,
fraction: Perbill::from_percent(10),
slash_era: 1
},
Event::Slashed { staker: 11, amount: 100 },
Event::Slashed { staker: 101, amount: 12 },
]
);
// post-slash balance
let nominator_slash_amount_11 = 125 / 10;
assert_eq!(Balances::free_balance(11), 900);
assert_eq!(Balances::free_balance(101), 2000 - nominator_slash_amount_11);
// This is the best way to check that the validator was chilled; `get` will
// return default value.
for (stash, _) in <Staking as Store>::Validators::iter() {
assert!(stash != 11);
}
let nominations = <Staking as Store>::Nominators::get(&101).unwrap();
// and make sure that the vote will be ignored even if the validator
// re-registers.
let last_slash = <Staking as Store>::SlashingSpans::get(&11).unwrap().last_nonzero_slash();
assert!(nominations.submitted_in < last_slash);
// check that validator was chilled.
assert!(<Staking as Store>::Validators::iter().all(|(stash, _)| stash != 11));
// actually re-bond the slashed validator
assert_ok!(Staking::validate(RuntimeOrigin::signed(10), Default::default()));
@@ -3233,11 +3244,12 @@ fn slash_kicks_validators_not_nominators_and_disables_nominator_for_kicked_valid
let exposure_11 = Staking::eras_stakers(active_era(), &11);
let exposure_21 = Staking::eras_stakers(active_era(), &21);
// 10 is re-elected, but without the support of 100
assert_eq!(exposure_11.total, 900);
// 20 is re-elected, with the (almost) entire support of 100
assert_eq!(exposure_21.total, 1000 + 500 - nominator_slash_amount_11);
// 11's own expo is reduced. sum of support from 11 is less (448), which is 500
// 900 + 146
assert!(matches!(exposure_11, Exposure { own: 900, total: 1046, .. }));
// 1000 + 342
assert!(matches!(exposure_21, Exposure { own: 1000, total: 1342, .. }));
assert_eq!(500 - 146 - 342, nominator_slash_amount_11);
});
}
@@ -3256,12 +3268,40 @@ fn non_slashable_offence_doesnt_disable_validator() {
&[Perbill::zero()],
);
// it does NOT affect the nominator.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
// offence that slashes 25% of the bond
on_offence_now(
&[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }],
&[Perbill::from_percent(25)],
);
// it DOES NOT affect the nominator.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 },
Event::Chilled { stash: 11 },
Event::SlashReported {
validator: 11,
fraction: Perbill::from_percent(0),
slash_era: 1
},
Event::Chilled { stash: 21 },
Event::SlashReported {
validator: 21,
fraction: Perbill::from_percent(25),
slash_era: 1
},
Event::Slashed { staker: 21, amount: 250 },
Event::Slashed { staker: 101, amount: 94 }
]
);
// the offence for validator 10 wasn't slashable so it wasn't disabled
assert!(!is_disabled(10));
// whereas validator 20 gets disabled
@@ -3288,6 +3328,9 @@ fn slashing_independent_of_disabling_validator() {
DisableStrategy::Always,
);
// nomination remains untouched.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
// offence that slashes 25% of the bond, BUT not disabling
on_offence_in_era(
&[OffenceDetails { offender: (21, exposure_21.clone()), reporters: vec![] }],
@@ -3296,6 +3339,31 @@ fn slashing_independent_of_disabling_validator() {
DisableStrategy::Never,
);
// nomination remains untouched.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
assert_eq!(
staking_events_since_last_call(),
vec![
Event::StakersElected,
Event::EraPaid { era_index: 0, validator_payout: 11075, remainder: 33225 },
Event::Chilled { stash: 11 },
Event::SlashReported {
validator: 11,
fraction: Perbill::from_percent(0),
slash_era: 1
},
Event::Chilled { stash: 21 },
Event::SlashReported {
validator: 21,
fraction: Perbill::from_percent(25),
slash_era: 1
},
Event::Slashed { staker: 21, amount: 250 },
Event::Slashed { staker: 101, amount: 94 }
]
);
// the offence for validator 10 was explicitly disabled
assert!(is_disabled(10));
// whereas validator 20 is explicitly not disabled
@@ -3370,6 +3438,9 @@ fn disabled_validators_are_kept_disabled_for_whole_era() {
&[Perbill::from_percent(25)],
);
// nominations are not updated.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
// validator 10 should not be disabled since the offence wasn't slashable
assert!(!is_disabled(10));
// validator 20 gets disabled since it got slashed
@@ -3387,6 +3458,9 @@ fn disabled_validators_are_kept_disabled_for_whole_era() {
&[Perbill::from_percent(25)],
);
// nominations are not updated.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
advance_session();
// and both are disabled in the last session of the era
@@ -3503,18 +3577,10 @@ fn zero_slash_keeps_nominators() {
assert_eq!(Balances::free_balance(11), 1000);
assert_eq!(Balances::free_balance(101), 2000);
// This is the best way to check that the validator was chilled; `get` will
// return default value.
for (stash, _) in <Staking as Store>::Validators::iter() {
assert!(stash != 11);
}
let nominations = <Staking as Store>::Nominators::get(&101).unwrap();
// and make sure that the vote will not be ignored, because the slash was
// zero.
let last_slash = <Staking as Store>::SlashingSpans::get(&11).unwrap().last_nonzero_slash();
assert!(nominations.submitted_in >= last_slash);
// 11 is still removed..
assert!(<Staking as Store>::Validators::iter().all(|(stash, _)| stash != 11));
// but their nominations are kept.
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
});
}
@@ -4380,15 +4446,14 @@ mod election_data_provider {
// we assume a network only wants up to 1000 validators in most cases, thus having 2000
// candidates is as high as it gets.
let validators = 2000;
// we assume the worse case: each validator also has a slashing span.
let slashing_spans = validators;
let mut nominators = 1000;
while <Test as Config>::WeightInfo::get_npos_voters(validators, nominators, slashing_spans)
.all_lt(Weight::from_parts(
while <Test as Config>::WeightInfo::get_npos_voters(validators, nominators).all_lt(
Weight::from_parts(
2u64 * frame_support::weights::constants::WEIGHT_REF_TIME_PER_SECOND,
u64::MAX,
)) {
),
) {
nominators += 1;
}
@@ -4410,49 +4475,6 @@ mod election_data_provider {
})
}
#[test]
fn voters_exclude_slashed() {
ExtBuilder::default().build_and_execute(|| {
assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]);
assert_eq!(
<Staking as ElectionDataProvider>::electing_voters(None)
.unwrap()
.iter()
.find(|x| x.0 == 101)
.unwrap()
.2,
vec![11, 21]
);
start_active_era(1);
add_slash(&11);
// 11 is gone.
start_active_era(2);
assert_eq!(
<Staking as ElectionDataProvider>::electing_voters(None)
.unwrap()
.iter()
.find(|x| x.0 == 101)
.unwrap()
.2,
vec![21]
);
// resubmit and it is back
assert_ok!(Staking::nominate(RuntimeOrigin::signed(100), vec![11, 21]));
assert_eq!(
<Staking as ElectionDataProvider>::electing_voters(None)
.unwrap()
.iter()
.find(|x| x.0 == 101)
.unwrap()
.2,
vec![11, 21]
);
})
}
#[test]
fn respects_snapshot_len_limits() {
ExtBuilder::default()
@@ -4489,10 +4511,26 @@ mod election_data_provider {
fn only_iterates_max_2_times_max_allowed_len() {
ExtBuilder::default()
.nominate(false)
// the other nominators only nominate 21
.add_staker(61, 60, 2_000, StakerStatus::<AccountId>::Nominator(vec![21]))
.add_staker(71, 70, 2_000, StakerStatus::<AccountId>::Nominator(vec![21]))
.add_staker(81, 80, 2_000, StakerStatus::<AccountId>::Nominator(vec![21]))
// the best way to invalidate a bunch of nominators is to have them nominate a lot of
// ppl, but then lower the MaxNomination limit.
.add_staker(
61,
60,
2_000,
StakerStatus::<AccountId>::Nominator(vec![21, 22, 23, 24, 25]),
)
.add_staker(
71,
70,
2_000,
StakerStatus::<AccountId>::Nominator(vec![21, 22, 23, 24, 25]),
)
.add_staker(
81,
80,
2_000,
StakerStatus::<AccountId>::Nominator(vec![21, 22, 23, 24, 25]),
)
.build_and_execute(|| {
// all voters ordered by stake,
assert_eq!(
@@ -4500,10 +4538,7 @@ mod election_data_provider {
vec![61, 71, 81, 11, 21, 31]
);
run_to_block(25);
// slash 21, the only validator nominated by our first 3 nominators
add_slash(&21);
MaxNominations::set(2);
// we want 2 voters now, and in maximum we allow 4 iterations. This is what happens:
// 61 is pruned;
@@ -4523,55 +4558,6 @@ mod election_data_provider {
});
}
// Even if some of the higher staked nominators are slashed, we still get up to max len voters
// by adding more lower staked nominators. In other words, we assert that we keep on adding
// valid nominators until we reach max len voters; which is opposed to simply stopping after we
// have iterated max len voters, but not adding all of them to voters due to some nominators not
// having valid targets.
#[test]
fn get_max_len_voters_even_if_some_nominators_are_slashed() {
ExtBuilder::default()
.nominate(false)
.add_staker(61, 60, 20, StakerStatus::<AccountId>::Nominator(vec![21]))
.add_staker(71, 70, 10, StakerStatus::<AccountId>::Nominator(vec![11, 21]))
.add_staker(81, 80, 10, StakerStatus::<AccountId>::Nominator(vec![11, 21]))
.build_and_execute(|| {
// given our voters ordered by stake,
assert_eq!(
<Test as Config>::VoterList::iter().collect::<Vec<_>>(),
vec![11, 21, 31, 61, 71, 81]
);
// we take 4 voters
assert_eq!(
Staking::electing_voters(Some(4))
.unwrap()
.iter()
.map(|(stash, _, _)| stash)
.copied()
.collect::<Vec<_>>(),
vec![11, 21, 31, 61],
);
// roll to session 5
run_to_block(25);
// slash 21, the only validator nominated by 61.
add_slash(&21);
// we take 4 voters; 71 and 81 are replacing the ejected ones.
assert_eq!(
Staking::electing_voters(Some(4))
.unwrap()
.iter()
.map(|(stash, _, _)| stash)
.copied()
.collect::<Vec<_>>(),
vec![11, 31, 71, 81],
);
});
}
#[test]
fn estimate_next_election_works() {
ExtBuilder::default().session_per_era(5).period(5).build_and_execute(|| {