From e11f4c0025e559e1d69e5d914852d278d284eac3 Mon Sep 17 00:00:00 2001 From: Vicente Rocha Date: Tue, 6 Feb 2024 22:51:28 +0000 Subject: [PATCH 1/7] Move test mods to their own files --- .../src/tests/election_data_provider.rs | 433 ++++++++ substrate/frame/staking/src/tests/ledger.rs | 312 ++++++ .../staking/src/{tests.rs => tests/mod.rs} | 936 +----------------- .../staking/src/tests/sorted_list_provider.rs | 42 + .../staking/src/tests/staking_interface.rs | 71 ++ 5 files changed, 871 insertions(+), 923 deletions(-) create mode 100644 substrate/frame/staking/src/tests/election_data_provider.rs create mode 100644 substrate/frame/staking/src/tests/ledger.rs rename substrate/frame/staking/src/{tests.rs => tests/mod.rs} (86%) create mode 100644 substrate/frame/staking/src/tests/sorted_list_provider.rs create mode 100644 substrate/frame/staking/src/tests/staking_interface.rs diff --git a/substrate/frame/staking/src/tests/election_data_provider.rs b/substrate/frame/staking/src/tests/election_data_provider.rs new file mode 100644 index 000000000000..5857d4448305 --- /dev/null +++ b/substrate/frame/staking/src/tests/election_data_provider.rs @@ -0,0 +1,433 @@ +use super::*; +use frame_election_provider_support::ElectionDataProvider; + +#[test] +fn targets_2sec_block() { + let mut validators = 1000; + while ::WeightInfo::get_npos_targets(validators).all_lt(Weight::from_parts( + 2u64 * frame_support::weights::constants::WEIGHT_REF_TIME_PER_SECOND, + u64::MAX, + )) { + validators += 1; + } + + println!("Can create a snapshot of {} validators in 2sec block", validators); +} + +#[test] +fn voters_2sec_block() { + // 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; + let mut nominators = 1000; + + while ::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; + } + + println!( + "Can create a snapshot of {} nominators [{} validators, each 1 slashing] in 2sec block", + nominators, validators + ); +} + +#[test] +fn set_minimum_active_stake_is_correct() { + ExtBuilder::default() + .nominate(false) + .add_staker(61, 61, 2_000, StakerStatus::::Nominator(vec![21])) + .add_staker(71, 71, 10, StakerStatus::::Nominator(vec![21])) + .add_staker(81, 81, 50, StakerStatus::::Nominator(vec![21])) + .build_and_execute(|| { + // default bounds are unbounded. + assert_ok!(::electing_voters( + DataProviderBounds::default() + )); + assert_eq!(MinimumActiveStake::::get(), 10); + + // remove staker with lower bond by limiting the number of voters and check + // `MinimumActiveStake` again after electing voters. + let bounds = ElectionBoundsBuilder::default().voters_count(5.into()).build(); + assert_ok!(::electing_voters(bounds.voters)); + assert_eq!(MinimumActiveStake::::get(), 50); + }); +} + +#[test] +fn set_minimum_active_stake_lower_bond_works() { + // if there are no voters, minimum active stake is zero (should not happen). + ExtBuilder::default().has_stakers(false).build_and_execute(|| { + // default bounds are unbounded. + assert_ok!(::electing_voters( + DataProviderBounds::default() + )); + assert_eq!(::VoterList::count(), 0); + assert_eq!(MinimumActiveStake::::get(), 0); + }); + + // lower non-zero active stake below `MinNominatorBond` is the minimum active stake if + // it is selected as part of the npos voters. + ExtBuilder::default().has_stakers(true).nominate(true).build_and_execute(|| { + assert_eq!(MinNominatorBond::::get(), 1); + assert_eq!(::VoterList::count(), 4); + + assert_ok!(Staking::bond(RuntimeOrigin::signed(4), 5, RewardDestination::Staked,)); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![1])); + assert_eq!(::VoterList::count(), 5); + + let voters_before = + ::electing_voters(DataProviderBounds::default()) + .unwrap(); + assert_eq!(MinimumActiveStake::::get(), 5); + + // update minimum nominator bond. + MinNominatorBond::::set(10); + assert_eq!(MinNominatorBond::::get(), 10); + // voter list still considers nominator 4 for voting, even though its active stake is + // lower than `MinNominatorBond`. + assert_eq!(::VoterList::count(), 5); + + let voters = + ::electing_voters(DataProviderBounds::default()) + .unwrap(); + assert_eq!(voters_before, voters); + + // minimum active stake is lower than `MinNominatorBond`. + assert_eq!(MinimumActiveStake::::get(), 5); + }); +} + +#[test] +fn set_minimum_active_bond_corrupt_state() { + ExtBuilder::default() + .has_stakers(true) + .nominate(true) + .add_staker(61, 61, 2_000, StakerStatus::::Nominator(vec![21])) + .build_and_execute(|| { + assert_eq!(Staking::weight_of(&101), 500); + let voters = + ::electing_voters(DataProviderBounds::default()) + .unwrap(); + assert_eq!(voters.len(), 5); + assert_eq!(MinimumActiveStake::::get(), 500); + + assert_ok!(Staking::unbond(RuntimeOrigin::signed(101), 200)); + start_active_era(10); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(101), 100)); + start_active_era(20); + + // corrupt ledger state by lowering max unlocking chunks bounds. + MaxUnlockingChunks::set(1); + + let voters = + ::electing_voters(DataProviderBounds::default()) + .unwrap(); + // number of returned voters decreases since ledger entry of stash 101 is now + // corrupt. + assert_eq!(voters.len(), 4); + // minimum active stake does not take into consideration the corrupt entry. + assert_eq!(MinimumActiveStake::::get(), 2_000); + + // voter weight of corrupted ledger entry is 0. + assert_eq!(Staking::weight_of(&101), 0); + + // reset max unlocking chunks for try_state to pass. + MaxUnlockingChunks::set(32); + }) +} + +#[test] +fn voters_include_self_vote() { + ExtBuilder::default().nominate(false).build_and_execute(|| { + // default bounds are unbounded. + assert!(>::iter().map(|(x, _)| x).all(|v| Staking::electing_voters( + DataProviderBounds::default() + ) + .unwrap() + .into_iter() + .any(|(w, _, t)| { v == w && t[0] == w }))) + }) +} + +// Tests the criteria that in `ElectionDataProvider::voters` function, we try to get at most +// `maybe_max_len` voters, and if some of them end up being skipped, we iterate at most `2 * +// maybe_max_len`. +#[test] +#[should_panic] +fn only_iterates_max_2_times_max_allowed_len() { + ExtBuilder::default() + .nominate(false) + // 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, 61, 2_000, StakerStatus::::Nominator(vec![21, 22, 23, 24, 25])) + .add_staker(71, 71, 2_000, StakerStatus::::Nominator(vec![21, 22, 23, 24, 25])) + .add_staker(81, 81, 2_000, StakerStatus::::Nominator(vec![21, 22, 23, 24, 25])) + .build_and_execute(|| { + let bounds_builder = ElectionBoundsBuilder::default(); + // all voters ordered by stake, + assert_eq!( + ::VoterList::iter().collect::>(), + vec![61, 71, 81, 11, 21, 31] + ); + + AbsoluteMaxNominations::set(2); + + // we want 2 voters now, and in maximum we allow 4 iterations. This is what happens: + // 61 is pruned; + // 71 is pruned; + // 81 is pruned; + // 11 is taken; + // we finish since the 2x limit is reached. + assert_eq!( + Staking::electing_voters(bounds_builder.voters_count(2.into()).build().voters) + .unwrap() + .iter() + .map(|(stash, _, _)| stash) + .copied() + .collect::>(), + vec![11], + ); + }); +} + +#[test] +fn respects_snapshot_count_limits() { + ExtBuilder::default() + .set_status(41, StakerStatus::Validator) + .build_and_execute(|| { + // sum of all nominators who'd be voters (1), plus the self-votes (4). + assert_eq!(::VoterList::count(), 5); + + let bounds_builder = ElectionBoundsBuilder::default(); + + // if voter count limit is less.. + assert_eq!( + Staking::electing_voters(bounds_builder.voters_count(1.into()).build().voters) + .unwrap() + .len(), + 1 + ); + + // if voter count limit is equal.. + assert_eq!( + Staking::electing_voters(bounds_builder.voters_count(5.into()).build().voters) + .unwrap() + .len(), + 5 + ); + + // if voter count limit is more. + assert_eq!( + Staking::electing_voters(bounds_builder.voters_count(55.into()).build().voters) + .unwrap() + .len(), + 5 + ); + + // if target count limit is more.. + assert_eq!( + Staking::electable_targets(bounds_builder.targets_count(6.into()).build().targets) + .unwrap() + .len(), + 4 + ); + + // if target count limit is equal.. + assert_eq!( + Staking::electable_targets(bounds_builder.targets_count(4.into()).build().targets) + .unwrap() + .len(), + 4 + ); + + // if target limit count is less, then we return an error. + assert_eq!( + Staking::electable_targets(bounds_builder.targets_count(1.into()).build().targets) + .unwrap_err(), + "Target snapshot too big" + ); + }); +} + +#[test] +fn respects_snapshot_size_limits() { + ExtBuilder::default().build_and_execute(|| { + // voters: set size bounds that allows only for 1 voter. + let bounds = ElectionBoundsBuilder::default().voters_size(26.into()).build(); + let elected = Staking::electing_voters(bounds.voters).unwrap(); + assert!(elected.encoded_size() == 26 as usize); + let prev_len = elected.len(); + + // larger size bounds means more quota for voters. + let bounds = ElectionBoundsBuilder::default().voters_size(100.into()).build(); + let elected = Staking::electing_voters(bounds.voters).unwrap(); + assert!(elected.encoded_size() <= 100 as usize); + assert!(elected.len() > 1 && elected.len() > prev_len); + + // targets: set size bounds that allows for only one target to fit in the snapshot. + let bounds = ElectionBoundsBuilder::default().targets_size(10.into()).build(); + let elected = Staking::electable_targets(bounds.targets).unwrap(); + assert!(elected.encoded_size() == 9 as usize); + let prev_len = elected.len(); + + // larger size bounds means more space for targets. + let bounds = ElectionBoundsBuilder::default().targets_size(100.into()).build(); + let elected = Staking::electable_targets(bounds.targets).unwrap(); + assert!(elected.encoded_size() <= 100 as usize); + assert!(elected.len() > 1 && elected.len() > prev_len); + }); +} + +#[test] +fn nomination_quota_checks_at_nominate_works() { + ExtBuilder::default().nominate(false).build_and_execute(|| { + // stash bond of 222 has a nomination quota of 2 targets. + bond(61, 222); + assert_eq!(Staking::api_nominations_quota(222), 2); + + // nominating with targets below the nomination quota works. + assert_ok!(Staking::nominate(RuntimeOrigin::signed(61), vec![11])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(61), vec![11, 12])); + + // nominating with targets above the nomination quota returns error. + assert_noop!( + Staking::nominate(RuntimeOrigin::signed(61), vec![11, 12, 13]), + Error::::TooManyTargets + ); + }); +} + +#[test] +fn lazy_quota_npos_voters_works_above_quota() { + ExtBuilder::default() + .nominate(false) + .add_staker( + 61, + 60, + 300, // 300 bond has 16 nomination quota. + StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), + ) + .build_and_execute(|| { + // unbond 78 from stash 60 so that it's bonded balance is 222, which has a lower + // nomination quota than at nomination time (max 2 targets). + assert_ok!(Staking::unbond(RuntimeOrigin::signed(61), 78)); + assert_eq!(Staking::api_nominations_quota(300 - 78), 2); + + // even through 61 has nomination quota of 2 at the time of the election, all the + // nominations (5) will be used. + assert_eq!( + Staking::electing_voters(DataProviderBounds::default()) + .unwrap() + .iter() + .map(|(stash, _, targets)| (*stash, targets.len())) + .collect::>(), + vec![(11, 1), (21, 1), (31, 1), (61, 5)], + ); + }); +} + +#[test] +fn nominations_quota_limits_size_work() { + ExtBuilder::default() + .nominate(false) + .add_staker( + 71, + 70, + 333, + StakerStatus::::Nominator(vec![16, 15, 14, 13, 12, 11, 10]), + ) + .build_and_execute(|| { + // nominations of controller 70 won't be added due to voter size limit exceeded. + let bounds = ElectionBoundsBuilder::default().voters_size(100.into()).build(); + assert_eq!( + Staking::electing_voters(bounds.voters) + .unwrap() + .iter() + .map(|(stash, _, targets)| (*stash, targets.len())) + .collect::>(), + vec![(11, 1), (21, 1), (31, 1)], + ); + + assert_eq!( + *staking_events().last().unwrap(), + Event::SnapshotVotersSizeExceeded { size: 75 } + ); + + // however, if the election voter size bounds were largers, the snapshot would + // include the electing voters of 70. + let bounds = ElectionBoundsBuilder::default().voters_size(1_000.into()).build(); + assert_eq!( + Staking::electing_voters(bounds.voters) + .unwrap() + .iter() + .map(|(stash, _, targets)| (*stash, targets.len())) + .collect::>(), + vec![(11, 1), (21, 1), (31, 1), (71, 7)], + ); + }); +} + +#[test] +fn estimate_next_election_works() { + ExtBuilder::default().session_per_era(5).period(5).build_and_execute(|| { + // first session is always length 0. + for b in 1..20 { + run_to_block(b); + assert_eq!(Staking::next_election_prediction(System::block_number()), 20); + } + + // election + run_to_block(20); + assert_eq!(Staking::next_election_prediction(System::block_number()), 45); + assert_eq!(staking_events().len(), 1); + assert_eq!(*staking_events().last().unwrap(), Event::StakersElected); + + for b in 21..45 { + run_to_block(b); + assert_eq!(Staking::next_election_prediction(System::block_number()), 45); + } + + // election + run_to_block(45); + assert_eq!(Staking::next_election_prediction(System::block_number()), 70); + assert_eq!(staking_events().len(), 3); + assert_eq!(*staking_events().last().unwrap(), Event::StakersElected); + + Staking::force_no_eras(RuntimeOrigin::root()).unwrap(); + assert_eq!(Staking::next_election_prediction(System::block_number()), u64::MAX); + + Staking::force_new_era_always(RuntimeOrigin::root()).unwrap(); + assert_eq!(Staking::next_election_prediction(System::block_number()), 45 + 5); + + Staking::force_new_era(RuntimeOrigin::root()).unwrap(); + assert_eq!(Staking::next_election_prediction(System::block_number()), 45 + 5); + + // Do a fail election + MinimumValidatorCount::::put(1000); + run_to_block(50); + // Election: failed, next session is a new election + assert_eq!(Staking::next_election_prediction(System::block_number()), 50 + 5); + // The new era is still forced until a new era is planned. + assert_eq!(ForceEra::::get(), Forcing::ForceNew); + + MinimumValidatorCount::::put(2); + run_to_block(55); + assert_eq!(Staking::next_election_prediction(System::block_number()), 55 + 25); + assert_eq!(staking_events().len(), 10); + assert_eq!( + *staking_events().last().unwrap(), + Event::ForceEra { mode: Forcing::NotForcing } + ); + assert_eq!( + *staking_events().get(staking_events().len() - 2).unwrap(), + Event::StakersElected + ); + // The new era has been planned, forcing is changed from `ForceNew` to `NotForcing`. + assert_eq!(ForceEra::::get(), Forcing::NotForcing); + }) +} diff --git a/substrate/frame/staking/src/tests/ledger.rs b/substrate/frame/staking/src/tests/ledger.rs new file mode 100644 index 000000000000..1ba281cc68d2 --- /dev/null +++ b/substrate/frame/staking/src/tests/ledger.rs @@ -0,0 +1,312 @@ +use super::*; + +#[test] +fn paired_account_works() { + ExtBuilder::default().try_state(false).build_and_execute(|| { + assert_ok!(Staking::bond(RuntimeOrigin::signed(10), 100, RewardDestination::Account(10))); + + assert_eq!(>::get(&10), Some(10)); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Controller(10)), Some(10)); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(10)), Some(10)); + + assert_eq!(>::get(&42), None); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Controller(42)), None); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(42)), None); + + // bond manually stash with different controller. This is deprecated but the migration + // has not been complete yet (controller: 100, stash: 200) + assert_ok!(bond_controller_stash(100, 200)); + assert_eq!(>::get(&200), Some(100)); + assert_eq!( + StakingLedger::::paired_account(StakingAccount::Controller(100)), + Some(200) + ); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(200)), Some(100)); + }) +} + +#[test] +fn get_ledger_works() { + ExtBuilder::default().try_state(false).build_and_execute(|| { + // stash does not exist + assert!(StakingLedger::::get(StakingAccount::Stash(42)).is_err()); + + // bonded and paired + assert_eq!(>::get(&11), Some(11)); + + match StakingLedger::::get(StakingAccount::Stash(11)) { + Ok(ledger) => { + assert_eq!(ledger.controller(), Some(11)); + assert_eq!(ledger.stash, 11); + }, + Err(_) => panic!("staking ledger must exist"), + }; + + // bond manually stash with different controller. This is deprecated but the migration + // has not been complete yet (controller: 100, stash: 200) + assert_ok!(bond_controller_stash(100, 200)); + assert_eq!(>::get(&200), Some(100)); + + match StakingLedger::::get(StakingAccount::Stash(200)) { + Ok(ledger) => { + assert_eq!(ledger.controller(), Some(100)); + assert_eq!(ledger.stash, 200); + }, + Err(_) => panic!("staking ledger must exist"), + }; + + match StakingLedger::::get(StakingAccount::Controller(100)) { + Ok(ledger) => { + assert_eq!(ledger.controller(), Some(100)); + assert_eq!(ledger.stash, 200); + }, + Err(_) => panic!("staking ledger must exist"), + }; + }) +} + +#[test] +fn bond_works() { + ExtBuilder::default().build_and_execute(|| { + assert!(!StakingLedger::::is_bonded(StakingAccount::Stash(42))); + assert!(>::get(&42).is_none()); + + let mut ledger: StakingLedger = StakingLedger::default_from(42); + let reward_dest = RewardDestination::Account(10); + + assert_ok!(ledger.clone().bond(reward_dest)); + assert!(StakingLedger::::is_bonded(StakingAccount::Stash(42))); + assert!(>::get(&42).is_some()); + assert_eq!(>::get(&42), Some(reward_dest)); + + // cannot bond again. + assert!(ledger.clone().bond(reward_dest).is_err()); + + // once bonded, update works as expected. + ledger.legacy_claimed_rewards = bounded_vec![1]; + assert_ok!(ledger.update()); + }) +} + +#[test] +fn is_bonded_works() { + ExtBuilder::default().build_and_execute(|| { + assert!(!StakingLedger::::is_bonded(StakingAccount::Stash(42))); + assert!(!StakingLedger::::is_bonded(StakingAccount::Controller(42))); + + // adds entry to Bonded without Ledger pair (should not happen). + >::insert(42, 42); + assert!(!StakingLedger::::is_bonded(StakingAccount::Controller(42))); + + assert_eq!(>::get(&11), Some(11)); + assert!(StakingLedger::::is_bonded(StakingAccount::Stash(11))); + assert!(StakingLedger::::is_bonded(StakingAccount::Controller(11))); + + >::remove(42); // ensures try-state checks pass. + }) +} + +#[test] +#[allow(deprecated)] +fn set_payee_errors_on_controller_destination() { + ExtBuilder::default().build_and_execute(|| { + Payee::::insert(11, RewardDestination::Staked); + assert_noop!( + Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller), + Error::::ControllerDeprecated + ); + assert_eq!(Payee::::get(&11), Some(RewardDestination::Staked)); + }) +} + +#[test] +#[allow(deprecated)] +fn update_payee_migration_works() { + ExtBuilder::default().build_and_execute(|| { + // migrate a `Controller` variant to `Account` variant. + Payee::::insert(11, RewardDestination::Controller); + assert_eq!(Payee::::get(&11), Some(RewardDestination::Controller)); + assert_ok!(Staking::update_payee(RuntimeOrigin::signed(11), 11)); + assert_eq!(Payee::::get(&11), Some(RewardDestination::Account(11))); + + // Do not migrate a variant if not `Controller`. + Payee::::insert(21, RewardDestination::Stash); + assert_eq!(Payee::::get(&21), Some(RewardDestination::Stash)); + assert_noop!( + Staking::update_payee(RuntimeOrigin::signed(11), 21), + Error::::NotController + ); + assert_eq!(Payee::::get(&21), Some(RewardDestination::Stash)); + }) +} + +#[test] +fn deprecate_controller_batch_works_full_weight() { + ExtBuilder::default().build_and_execute(|| { + // Given: + + let start = 1001; + let mut controllers: Vec<_> = vec![]; + for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { + let ctlr: u64 = n.into(); + let stash: u64 = (n + 10000).into(); + + Ledger::::insert( + ctlr, + StakingLedger { + controller: None, + total: (10 + ctlr).into(), + active: (10 + ctlr).into(), + ..StakingLedger::default_from(stash) + }, + ); + Bonded::::insert(stash, ctlr); + Payee::::insert(stash, RewardDestination::Staked); + + controllers.push(ctlr); + } + + // When: + + let bounded_controllers: BoundedVec<_, ::MaxControllersInDeprecationBatch> = + BoundedVec::try_from(controllers).unwrap(); + + // Only `AdminOrigin` can sign. + assert_noop!( + Staking::deprecate_controller_batch( + RuntimeOrigin::signed(2), + bounded_controllers.clone() + ), + BadOrigin + ); + + let result = + Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); + assert_ok!(result); + assert_eq!( + result.unwrap().actual_weight.unwrap(), + ::WeightInfo::deprecate_controller_batch( + ::MaxControllersInDeprecationBatch::get() + ) + ); + + // Then: + + for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { + let ctlr: u64 = n.into(); + let stash: u64 = (n + 10000).into(); + + // Ledger no longer keyed by controller. + assert_eq!(Ledger::::get(ctlr), None); + // Bonded now maps to the stash. + assert_eq!(Bonded::::get(stash), Some(stash)); + + // Ledger is now keyed by stash. + let ledger_updated = Ledger::::get(stash).unwrap(); + assert_eq!(ledger_updated.stash, stash); + + // Check `active` and `total` values match the original ledger set by controller. + assert_eq!(ledger_updated.active, (10 + ctlr).into()); + assert_eq!(ledger_updated.total, (10 + ctlr).into()); + } + }) +} + +#[test] +fn deprecate_controller_batch_works_half_weight() { + ExtBuilder::default().build_and_execute(|| { + // Given: + + let start = 1001; + let mut controllers: Vec<_> = vec![]; + for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { + let ctlr: u64 = n.into(); + + // Only half of entries are unique pairs. + let stash: u64 = if n % 2 == 0 { (n + 10000).into() } else { ctlr }; + + Ledger::::insert( + ctlr, + StakingLedger { controller: None, ..StakingLedger::default_from(stash) }, + ); + Bonded::::insert(stash, ctlr); + Payee::::insert(stash, RewardDestination::Staked); + + controllers.push(ctlr); + } + + // When: + let bounded_controllers: BoundedVec<_, ::MaxControllersInDeprecationBatch> = + BoundedVec::try_from(controllers.clone()).unwrap(); + + let result = + Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); + assert_ok!(result); + assert_eq!( + result.unwrap().actual_weight.unwrap(), + ::WeightInfo::deprecate_controller_batch(controllers.len() as u32) + ); + + // Then: + + for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { + let unique_pair = n % 2 == 0; + let ctlr: u64 = n.into(); + let stash: u64 = if unique_pair { (n + 10000).into() } else { ctlr }; + + // Side effect of migration for unique pair. + if unique_pair { + assert_eq!(Ledger::::get(ctlr), None); + } + // Bonded maps to the stash. + assert_eq!(Bonded::::get(stash), Some(stash)); + + // Ledger is keyed by stash. + let ledger_updated = Ledger::::get(stash).unwrap(); + assert_eq!(ledger_updated.stash, stash); + } + }) +} + +#[test] +fn deprecate_controller_batch_skips_unmigrated_controller_payees() { + ExtBuilder::default().try_state(false).build_and_execute(|| { + // Given: + + let stash: u64 = 1000; + let ctlr: u64 = 1001; + + Ledger::::insert( + ctlr, + StakingLedger { controller: None, ..StakingLedger::default_from(stash) }, + ); + Bonded::::insert(stash, ctlr); + #[allow(deprecated)] + Payee::::insert(stash, RewardDestination::Controller); + + // When: + + let bounded_controllers: BoundedVec<_, ::MaxControllersInDeprecationBatch> = + BoundedVec::try_from(vec![ctlr]).unwrap(); + + let result = + Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); + assert_ok!(result); + assert_eq!( + result.unwrap().actual_weight.unwrap(), + ::WeightInfo::deprecate_controller_batch(1 as u32) + ); + + // Then: + + // Esure deprecation did not happen. + assert_eq!(Ledger::::get(ctlr).is_some(), true); + + // Bonded still keyed by controller. + assert_eq!(Bonded::::get(stash), Some(ctlr)); + + // Ledger is still keyed by controller. + let ledger_updated = Ledger::::get(ctlr).unwrap(); + assert_eq!(ledger_updated.stash, stash); + }) +} diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests/mod.rs similarity index 86% rename from substrate/frame/staking/src/tests.rs rename to substrate/frame/staking/src/tests/mod.rs index 85ee7dd3bf59..cc4993eeb39c 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests/mod.rs @@ -17,6 +17,11 @@ //! Tests for the module. +mod election_data_provider; +mod ledger; +mod sorted_list_provider; +mod staking_interface; + use super::{ConfigOp, Event, *}; use crate::ledger::StakingLedgerInspect; use frame_election_provider_support::{ @@ -370,9 +375,9 @@ fn rewards_should_work() { ); assert_eq_error_rate!( Balances::total_balance(&101), - init_balance_101 + - part_for_101_from_11 * total_payout_0 * 2 / 3 + - part_for_101_from_21 * total_payout_0 * 1 / 3, + init_balance_101 + + part_for_101_from_11 * total_payout_0 * 2 / 3 + + part_for_101_from_21 * total_payout_0 * 1 / 3, 2 ); @@ -409,9 +414,9 @@ fn rewards_should_work() { ); assert_eq_error_rate!( Balances::total_balance(&101), - init_balance_101 + - part_for_101_from_11 * (total_payout_0 * 2 / 3 + total_payout_1) + - part_for_101_from_21 * total_payout_0 * 1 / 3, + init_balance_101 + + part_for_101_from_11 * (total_payout_0 * 2 / 3 + total_payout_1) + + part_for_101_from_21 * total_payout_0 * 1 / 3, 2 ); }); @@ -4938,465 +4943,6 @@ fn on_finalize_weight_is_nonzero() { }) } -mod election_data_provider { - use super::*; - use frame_election_provider_support::ElectionDataProvider; - - #[test] - fn targets_2sec_block() { - let mut validators = 1000; - while ::WeightInfo::get_npos_targets(validators).all_lt(Weight::from_parts( - 2u64 * frame_support::weights::constants::WEIGHT_REF_TIME_PER_SECOND, - u64::MAX, - )) { - validators += 1; - } - - println!("Can create a snapshot of {} validators in 2sec block", validators); - } - - #[test] - fn voters_2sec_block() { - // 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; - let mut nominators = 1000; - - while ::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; - } - - println!( - "Can create a snapshot of {} nominators [{} validators, each 1 slashing] in 2sec block", - nominators, validators - ); - } - - #[test] - fn set_minimum_active_stake_is_correct() { - ExtBuilder::default() - .nominate(false) - .add_staker(61, 61, 2_000, StakerStatus::::Nominator(vec![21])) - .add_staker(71, 71, 10, StakerStatus::::Nominator(vec![21])) - .add_staker(81, 81, 50, StakerStatus::::Nominator(vec![21])) - .build_and_execute(|| { - // default bounds are unbounded. - assert_ok!(::electing_voters( - DataProviderBounds::default() - )); - assert_eq!(MinimumActiveStake::::get(), 10); - - // remove staker with lower bond by limiting the number of voters and check - // `MinimumActiveStake` again after electing voters. - let bounds = ElectionBoundsBuilder::default().voters_count(5.into()).build(); - assert_ok!(::electing_voters(bounds.voters)); - assert_eq!(MinimumActiveStake::::get(), 50); - }); - } - - #[test] - fn set_minimum_active_stake_lower_bond_works() { - // if there are no voters, minimum active stake is zero (should not happen). - ExtBuilder::default().has_stakers(false).build_and_execute(|| { - // default bounds are unbounded. - assert_ok!(::electing_voters( - DataProviderBounds::default() - )); - assert_eq!(::VoterList::count(), 0); - assert_eq!(MinimumActiveStake::::get(), 0); - }); - - // lower non-zero active stake below `MinNominatorBond` is the minimum active stake if - // it is selected as part of the npos voters. - ExtBuilder::default().has_stakers(true).nominate(true).build_and_execute(|| { - assert_eq!(MinNominatorBond::::get(), 1); - assert_eq!(::VoterList::count(), 4); - - assert_ok!(Staking::bond(RuntimeOrigin::signed(4), 5, RewardDestination::Staked,)); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![1])); - assert_eq!(::VoterList::count(), 5); - - let voters_before = - ::electing_voters(DataProviderBounds::default()) - .unwrap(); - assert_eq!(MinimumActiveStake::::get(), 5); - - // update minimum nominator bond. - MinNominatorBond::::set(10); - assert_eq!(MinNominatorBond::::get(), 10); - // voter list still considers nominator 4 for voting, even though its active stake is - // lower than `MinNominatorBond`. - assert_eq!(::VoterList::count(), 5); - - let voters = - ::electing_voters(DataProviderBounds::default()) - .unwrap(); - assert_eq!(voters_before, voters); - - // minimum active stake is lower than `MinNominatorBond`. - assert_eq!(MinimumActiveStake::::get(), 5); - }); - } - - #[test] - fn set_minimum_active_bond_corrupt_state() { - ExtBuilder::default() - .has_stakers(true) - .nominate(true) - .add_staker(61, 61, 2_000, StakerStatus::::Nominator(vec![21])) - .build_and_execute(|| { - assert_eq!(Staking::weight_of(&101), 500); - let voters = ::electing_voters( - DataProviderBounds::default(), - ) - .unwrap(); - assert_eq!(voters.len(), 5); - assert_eq!(MinimumActiveStake::::get(), 500); - - assert_ok!(Staking::unbond(RuntimeOrigin::signed(101), 200)); - start_active_era(10); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(101), 100)); - start_active_era(20); - - // corrupt ledger state by lowering max unlocking chunks bounds. - MaxUnlockingChunks::set(1); - - let voters = ::electing_voters( - DataProviderBounds::default(), - ) - .unwrap(); - // number of returned voters decreases since ledger entry of stash 101 is now - // corrupt. - assert_eq!(voters.len(), 4); - // minimum active stake does not take into consideration the corrupt entry. - assert_eq!(MinimumActiveStake::::get(), 2_000); - - // voter weight of corrupted ledger entry is 0. - assert_eq!(Staking::weight_of(&101), 0); - - // reset max unlocking chunks for try_state to pass. - MaxUnlockingChunks::set(32); - }) - } - - #[test] - fn voters_include_self_vote() { - ExtBuilder::default().nominate(false).build_and_execute(|| { - // default bounds are unbounded. - assert!(>::iter().map(|(x, _)| x).all(|v| Staking::electing_voters( - DataProviderBounds::default() - ) - .unwrap() - .into_iter() - .any(|(w, _, t)| { v == w && t[0] == w }))) - }) - } - - // Tests the criteria that in `ElectionDataProvider::voters` function, we try to get at most - // `maybe_max_len` voters, and if some of them end up being skipped, we iterate at most `2 * - // maybe_max_len`. - #[test] - #[should_panic] - fn only_iterates_max_2_times_max_allowed_len() { - ExtBuilder::default() - .nominate(false) - // 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, - 61, - 2_000, - StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), - ) - .add_staker( - 71, - 71, - 2_000, - StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), - ) - .add_staker( - 81, - 81, - 2_000, - StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), - ) - .build_and_execute(|| { - let bounds_builder = ElectionBoundsBuilder::default(); - // all voters ordered by stake, - assert_eq!( - ::VoterList::iter().collect::>(), - vec![61, 71, 81, 11, 21, 31] - ); - - AbsoluteMaxNominations::set(2); - - // we want 2 voters now, and in maximum we allow 4 iterations. This is what happens: - // 61 is pruned; - // 71 is pruned; - // 81 is pruned; - // 11 is taken; - // we finish since the 2x limit is reached. - assert_eq!( - Staking::electing_voters(bounds_builder.voters_count(2.into()).build().voters) - .unwrap() - .iter() - .map(|(stash, _, _)| stash) - .copied() - .collect::>(), - vec![11], - ); - }); - } - - #[test] - fn respects_snapshot_count_limits() { - ExtBuilder::default() - .set_status(41, StakerStatus::Validator) - .build_and_execute(|| { - // sum of all nominators who'd be voters (1), plus the self-votes (4). - assert_eq!(::VoterList::count(), 5); - - let bounds_builder = ElectionBoundsBuilder::default(); - - // if voter count limit is less.. - assert_eq!( - Staking::electing_voters(bounds_builder.voters_count(1.into()).build().voters) - .unwrap() - .len(), - 1 - ); - - // if voter count limit is equal.. - assert_eq!( - Staking::electing_voters(bounds_builder.voters_count(5.into()).build().voters) - .unwrap() - .len(), - 5 - ); - - // if voter count limit is more. - assert_eq!( - Staking::electing_voters(bounds_builder.voters_count(55.into()).build().voters) - .unwrap() - .len(), - 5 - ); - - // if target count limit is more.. - assert_eq!( - Staking::electable_targets( - bounds_builder.targets_count(6.into()).build().targets - ) - .unwrap() - .len(), - 4 - ); - - // if target count limit is equal.. - assert_eq!( - Staking::electable_targets( - bounds_builder.targets_count(4.into()).build().targets - ) - .unwrap() - .len(), - 4 - ); - - // if target limit count is less, then we return an error. - assert_eq!( - Staking::electable_targets( - bounds_builder.targets_count(1.into()).build().targets - ) - .unwrap_err(), - "Target snapshot too big" - ); - }); - } - - #[test] - fn respects_snapshot_size_limits() { - ExtBuilder::default().build_and_execute(|| { - // voters: set size bounds that allows only for 1 voter. - let bounds = ElectionBoundsBuilder::default().voters_size(26.into()).build(); - let elected = Staking::electing_voters(bounds.voters).unwrap(); - assert!(elected.encoded_size() == 26 as usize); - let prev_len = elected.len(); - - // larger size bounds means more quota for voters. - let bounds = ElectionBoundsBuilder::default().voters_size(100.into()).build(); - let elected = Staking::electing_voters(bounds.voters).unwrap(); - assert!(elected.encoded_size() <= 100 as usize); - assert!(elected.len() > 1 && elected.len() > prev_len); - - // targets: set size bounds that allows for only one target to fit in the snapshot. - let bounds = ElectionBoundsBuilder::default().targets_size(10.into()).build(); - let elected = Staking::electable_targets(bounds.targets).unwrap(); - assert!(elected.encoded_size() == 9 as usize); - let prev_len = elected.len(); - - // larger size bounds means more space for targets. - let bounds = ElectionBoundsBuilder::default().targets_size(100.into()).build(); - let elected = Staking::electable_targets(bounds.targets).unwrap(); - assert!(elected.encoded_size() <= 100 as usize); - assert!(elected.len() > 1 && elected.len() > prev_len); - }); - } - - #[test] - fn nomination_quota_checks_at_nominate_works() { - ExtBuilder::default().nominate(false).build_and_execute(|| { - // stash bond of 222 has a nomination quota of 2 targets. - bond(61, 222); - assert_eq!(Staking::api_nominations_quota(222), 2); - - // nominating with targets below the nomination quota works. - assert_ok!(Staking::nominate(RuntimeOrigin::signed(61), vec![11])); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(61), vec![11, 12])); - - // nominating with targets above the nomination quota returns error. - assert_noop!( - Staking::nominate(RuntimeOrigin::signed(61), vec![11, 12, 13]), - Error::::TooManyTargets - ); - }); - } - - #[test] - fn lazy_quota_npos_voters_works_above_quota() { - ExtBuilder::default() - .nominate(false) - .add_staker( - 61, - 60, - 300, // 300 bond has 16 nomination quota. - StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), - ) - .build_and_execute(|| { - // unbond 78 from stash 60 so that it's bonded balance is 222, which has a lower - // nomination quota than at nomination time (max 2 targets). - assert_ok!(Staking::unbond(RuntimeOrigin::signed(61), 78)); - assert_eq!(Staking::api_nominations_quota(300 - 78), 2); - - // even through 61 has nomination quota of 2 at the time of the election, all the - // nominations (5) will be used. - assert_eq!( - Staking::electing_voters(DataProviderBounds::default()) - .unwrap() - .iter() - .map(|(stash, _, targets)| (*stash, targets.len())) - .collect::>(), - vec![(11, 1), (21, 1), (31, 1), (61, 5)], - ); - }); - } - - #[test] - fn nominations_quota_limits_size_work() { - ExtBuilder::default() - .nominate(false) - .add_staker( - 71, - 70, - 333, - StakerStatus::::Nominator(vec![16, 15, 14, 13, 12, 11, 10]), - ) - .build_and_execute(|| { - // nominations of controller 70 won't be added due to voter size limit exceeded. - let bounds = ElectionBoundsBuilder::default().voters_size(100.into()).build(); - assert_eq!( - Staking::electing_voters(bounds.voters) - .unwrap() - .iter() - .map(|(stash, _, targets)| (*stash, targets.len())) - .collect::>(), - vec![(11, 1), (21, 1), (31, 1)], - ); - - assert_eq!( - *staking_events().last().unwrap(), - Event::SnapshotVotersSizeExceeded { size: 75 } - ); - - // however, if the election voter size bounds were largers, the snapshot would - // include the electing voters of 70. - let bounds = ElectionBoundsBuilder::default().voters_size(1_000.into()).build(); - assert_eq!( - Staking::electing_voters(bounds.voters) - .unwrap() - .iter() - .map(|(stash, _, targets)| (*stash, targets.len())) - .collect::>(), - vec![(11, 1), (21, 1), (31, 1), (71, 7)], - ); - }); - } - - #[test] - fn estimate_next_election_works() { - ExtBuilder::default().session_per_era(5).period(5).build_and_execute(|| { - // first session is always length 0. - for b in 1..20 { - run_to_block(b); - assert_eq!(Staking::next_election_prediction(System::block_number()), 20); - } - - // election - run_to_block(20); - assert_eq!(Staking::next_election_prediction(System::block_number()), 45); - assert_eq!(staking_events().len(), 1); - assert_eq!(*staking_events().last().unwrap(), Event::StakersElected); - - for b in 21..45 { - run_to_block(b); - assert_eq!(Staking::next_election_prediction(System::block_number()), 45); - } - - // election - run_to_block(45); - assert_eq!(Staking::next_election_prediction(System::block_number()), 70); - assert_eq!(staking_events().len(), 3); - assert_eq!(*staking_events().last().unwrap(), Event::StakersElected); - - Staking::force_no_eras(RuntimeOrigin::root()).unwrap(); - assert_eq!(Staking::next_election_prediction(System::block_number()), u64::MAX); - - Staking::force_new_era_always(RuntimeOrigin::root()).unwrap(); - assert_eq!(Staking::next_election_prediction(System::block_number()), 45 + 5); - - Staking::force_new_era(RuntimeOrigin::root()).unwrap(); - assert_eq!(Staking::next_election_prediction(System::block_number()), 45 + 5); - - // Do a fail election - MinimumValidatorCount::::put(1000); - run_to_block(50); - // Election: failed, next session is a new election - assert_eq!(Staking::next_election_prediction(System::block_number()), 50 + 5); - // The new era is still forced until a new era is planned. - assert_eq!(ForceEra::::get(), Forcing::ForceNew); - - MinimumValidatorCount::::put(2); - run_to_block(55); - assert_eq!(Staking::next_election_prediction(System::block_number()), 55 + 25); - assert_eq!(staking_events().len(), 10); - assert_eq!( - *staking_events().last().unwrap(), - Event::ForceEra { mode: Forcing::NotForcing } - ); - assert_eq!( - *staking_events().get(staking_events().len() - 2).unwrap(), - Event::StakersElected - ); - // The new era has been planned, forcing is changed from `ForceNew` to `NotForcing`. - assert_eq!(ForceEra::::get(), Forcing::NotForcing); - }) - } -} - #[test] #[should_panic] fn count_check_works() { @@ -5916,57 +5462,6 @@ fn api_nominations_quota_works() { }) } -mod sorted_list_provider { - use super::*; - use frame_election_provider_support::SortedListProvider; - - #[test] - fn re_nominate_does_not_change_counters_or_list() { - ExtBuilder::default().nominate(true).build_and_execute(|| { - // given - let pre_insert_voter_count = - (Nominators::::count() + Validators::::count()) as u32; - assert_eq!(::VoterList::count(), pre_insert_voter_count); - - assert_eq!( - ::VoterList::iter().collect::>(), - vec![11, 21, 31, 101] - ); - - // when account 101 renominates - assert_ok!(Staking::nominate(RuntimeOrigin::signed(101), vec![41])); - - // then counts don't change - assert_eq!(::VoterList::count(), pre_insert_voter_count); - // and the list is the same - assert_eq!( - ::VoterList::iter().collect::>(), - vec![11, 21, 31, 101] - ); - }); - } - - #[test] - fn re_validate_does_not_change_counters_or_list() { - ExtBuilder::default().nominate(false).build_and_execute(|| { - // given - let pre_insert_voter_count = - (Nominators::::count() + Validators::::count()) as u32; - assert_eq!(::VoterList::count(), pre_insert_voter_count); - - assert_eq!(::VoterList::iter().collect::>(), vec![11, 21, 31]); - - // when account 11 re-validates - assert_ok!(Staking::validate(RuntimeOrigin::signed(11), Default::default())); - - // then counts don't change - assert_eq!(::VoterList::count(), pre_insert_voter_count); - // and the list is the same - assert_eq!(::VoterList::iter().collect::>(), vec![11, 21, 31]); - }); - } -} - #[test] fn force_apply_min_commission_works() { let prefs = |c| ValidatorPrefs { commission: Perbill::from_percent(c), blocked: false }; @@ -6592,8 +6087,8 @@ fn test_validator_exposure_is_backward_compatible_with_non_paged_rewards_payout( let actual_exposure_page_1 = ErasStakersPaged::::get((1, 11, 1)).unwrap(); expected_individual_exposures.iter().for_each(|exposure| { assert!( - actual_exposure_page_0.others.contains(exposure) || - actual_exposure_page_1.others.contains(exposure) + actual_exposure_page_0.others.contains(exposure) + || actual_exposure_page_1.others.contains(exposure) ); }); assert_eq!( @@ -6677,408 +6172,3 @@ fn test_validator_exposure_is_backward_compatible_with_non_paged_rewards_payout( ); }); } - -mod staking_interface { - use frame_support::storage::with_storage_layer; - use sp_staking::StakingInterface; - - use super::*; - - #[test] - fn force_unstake_with_slash_works() { - ExtBuilder::default().build_and_execute(|| { - // without slash - let _ = with_storage_layer::<(), _, _>(|| { - // bond an account, can unstake - assert_eq!(Staking::bonded(&11), Some(11)); - assert_ok!(::force_unstake(11)); - Err(DispatchError::from("revert")) - }); - - // bond again and add a slash, still can unstake. - assert_eq!(Staking::bonded(&11), Some(11)); - add_slash(&11); - assert_ok!(::force_unstake(11)); - }); - } - - #[test] - fn do_withdraw_unbonded_with_wrong_slash_spans_works_as_expected() { - ExtBuilder::default().build_and_execute(|| { - on_offence_now( - &[OffenceDetails { - offender: (11, Staking::eras_stakers(active_era(), &11)), - reporters: vec![], - }], - &[Perbill::from_percent(100)], - ); - - assert_eq!(Staking::bonded(&11), Some(11)); - - assert_noop!( - Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0), - Error::::IncorrectSlashingSpans - ); - - let num_slashing_spans = Staking::slashing_spans(&11).map_or(0, |s| s.iter().count()); - assert_ok!(Staking::withdraw_unbonded( - RuntimeOrigin::signed(11), - num_slashing_spans as u32 - )); - }); - } - - #[test] - fn status() { - ExtBuilder::default().build_and_execute(|| { - // stash of a validator is identified as a validator - assert_eq!(Staking::status(&11).unwrap(), StakerStatus::Validator); - // .. but not the controller. - assert!(Staking::status(&10).is_err()); - - // stash of nominator is identified as a nominator - assert_eq!(Staking::status(&101).unwrap(), StakerStatus::Nominator(vec![11, 21])); - // .. but not the controller. - assert!(Staking::status(&100).is_err()); - - // stash of chilled is identified as a chilled - assert_eq!(Staking::status(&41).unwrap(), StakerStatus::Idle); - // .. but not the controller. - assert!(Staking::status(&40).is_err()); - - // random other account. - assert!(Staking::status(&42).is_err()); - }) - } -} - -mod ledger { - use super::*; - - #[test] - fn paired_account_works() { - ExtBuilder::default().try_state(false).build_and_execute(|| { - assert_ok!(Staking::bond( - RuntimeOrigin::signed(10), - 100, - RewardDestination::Account(10) - )); - - assert_eq!(>::get(&10), Some(10)); - assert_eq!( - StakingLedger::::paired_account(StakingAccount::Controller(10)), - Some(10) - ); - assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(10)), Some(10)); - - assert_eq!(>::get(&42), None); - assert_eq!(StakingLedger::::paired_account(StakingAccount::Controller(42)), None); - assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(42)), None); - - // bond manually stash with different controller. This is deprecated but the migration - // has not been complete yet (controller: 100, stash: 200) - assert_ok!(bond_controller_stash(100, 200)); - assert_eq!(>::get(&200), Some(100)); - assert_eq!( - StakingLedger::::paired_account(StakingAccount::Controller(100)), - Some(200) - ); - assert_eq!( - StakingLedger::::paired_account(StakingAccount::Stash(200)), - Some(100) - ); - }) - } - - #[test] - fn get_ledger_works() { - ExtBuilder::default().try_state(false).build_and_execute(|| { - // stash does not exist - assert!(StakingLedger::::get(StakingAccount::Stash(42)).is_err()); - - // bonded and paired - assert_eq!(>::get(&11), Some(11)); - - match StakingLedger::::get(StakingAccount::Stash(11)) { - Ok(ledger) => { - assert_eq!(ledger.controller(), Some(11)); - assert_eq!(ledger.stash, 11); - }, - Err(_) => panic!("staking ledger must exist"), - }; - - // bond manually stash with different controller. This is deprecated but the migration - // has not been complete yet (controller: 100, stash: 200) - assert_ok!(bond_controller_stash(100, 200)); - assert_eq!(>::get(&200), Some(100)); - - match StakingLedger::::get(StakingAccount::Stash(200)) { - Ok(ledger) => { - assert_eq!(ledger.controller(), Some(100)); - assert_eq!(ledger.stash, 200); - }, - Err(_) => panic!("staking ledger must exist"), - }; - - match StakingLedger::::get(StakingAccount::Controller(100)) { - Ok(ledger) => { - assert_eq!(ledger.controller(), Some(100)); - assert_eq!(ledger.stash, 200); - }, - Err(_) => panic!("staking ledger must exist"), - }; - }) - } - - #[test] - fn bond_works() { - ExtBuilder::default().build_and_execute(|| { - assert!(!StakingLedger::::is_bonded(StakingAccount::Stash(42))); - assert!(>::get(&42).is_none()); - - let mut ledger: StakingLedger = StakingLedger::default_from(42); - let reward_dest = RewardDestination::Account(10); - - assert_ok!(ledger.clone().bond(reward_dest)); - assert!(StakingLedger::::is_bonded(StakingAccount::Stash(42))); - assert!(>::get(&42).is_some()); - assert_eq!(>::get(&42), Some(reward_dest)); - - // cannot bond again. - assert!(ledger.clone().bond(reward_dest).is_err()); - - // once bonded, update works as expected. - ledger.legacy_claimed_rewards = bounded_vec![1]; - assert_ok!(ledger.update()); - }) - } - - #[test] - fn is_bonded_works() { - ExtBuilder::default().build_and_execute(|| { - assert!(!StakingLedger::::is_bonded(StakingAccount::Stash(42))); - assert!(!StakingLedger::::is_bonded(StakingAccount::Controller(42))); - - // adds entry to Bonded without Ledger pair (should not happen). - >::insert(42, 42); - assert!(!StakingLedger::::is_bonded(StakingAccount::Controller(42))); - - assert_eq!(>::get(&11), Some(11)); - assert!(StakingLedger::::is_bonded(StakingAccount::Stash(11))); - assert!(StakingLedger::::is_bonded(StakingAccount::Controller(11))); - - >::remove(42); // ensures try-state checks pass. - }) - } - - #[test] - #[allow(deprecated)] - fn set_payee_errors_on_controller_destination() { - ExtBuilder::default().build_and_execute(|| { - Payee::::insert(11, RewardDestination::Staked); - assert_noop!( - Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller), - Error::::ControllerDeprecated - ); - assert_eq!(Payee::::get(&11), Some(RewardDestination::Staked)); - }) - } - - #[test] - #[allow(deprecated)] - fn update_payee_migration_works() { - ExtBuilder::default().build_and_execute(|| { - // migrate a `Controller` variant to `Account` variant. - Payee::::insert(11, RewardDestination::Controller); - assert_eq!(Payee::::get(&11), Some(RewardDestination::Controller)); - assert_ok!(Staking::update_payee(RuntimeOrigin::signed(11), 11)); - assert_eq!(Payee::::get(&11), Some(RewardDestination::Account(11))); - - // Do not migrate a variant if not `Controller`. - Payee::::insert(21, RewardDestination::Stash); - assert_eq!(Payee::::get(&21), Some(RewardDestination::Stash)); - assert_noop!( - Staking::update_payee(RuntimeOrigin::signed(11), 21), - Error::::NotController - ); - assert_eq!(Payee::::get(&21), Some(RewardDestination::Stash)); - }) - } - - #[test] - fn deprecate_controller_batch_works_full_weight() { - ExtBuilder::default().build_and_execute(|| { - // Given: - - let start = 1001; - let mut controllers: Vec<_> = vec![]; - for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { - let ctlr: u64 = n.into(); - let stash: u64 = (n + 10000).into(); - - Ledger::::insert( - ctlr, - StakingLedger { - controller: None, - total: (10 + ctlr).into(), - active: (10 + ctlr).into(), - ..StakingLedger::default_from(stash) - }, - ); - Bonded::::insert(stash, ctlr); - Payee::::insert(stash, RewardDestination::Staked); - - controllers.push(ctlr); - } - - // When: - - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(controllers).unwrap(); - - // Only `AdminOrigin` can sign. - assert_noop!( - Staking::deprecate_controller_batch( - RuntimeOrigin::signed(2), - bounded_controllers.clone() - ), - BadOrigin - ); - - let result = - Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); - assert_ok!(result); - assert_eq!( - result.unwrap().actual_weight.unwrap(), - ::WeightInfo::deprecate_controller_batch( - ::MaxControllersInDeprecationBatch::get() - ) - ); - - // Then: - - for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { - let ctlr: u64 = n.into(); - let stash: u64 = (n + 10000).into(); - - // Ledger no longer keyed by controller. - assert_eq!(Ledger::::get(ctlr), None); - // Bonded now maps to the stash. - assert_eq!(Bonded::::get(stash), Some(stash)); - - // Ledger is now keyed by stash. - let ledger_updated = Ledger::::get(stash).unwrap(); - assert_eq!(ledger_updated.stash, stash); - - // Check `active` and `total` values match the original ledger set by controller. - assert_eq!(ledger_updated.active, (10 + ctlr).into()); - assert_eq!(ledger_updated.total, (10 + ctlr).into()); - } - }) - } - - #[test] - fn deprecate_controller_batch_works_half_weight() { - ExtBuilder::default().build_and_execute(|| { - // Given: - - let start = 1001; - let mut controllers: Vec<_> = vec![]; - for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { - let ctlr: u64 = n.into(); - - // Only half of entries are unique pairs. - let stash: u64 = if n % 2 == 0 { (n + 10000).into() } else { ctlr }; - - Ledger::::insert( - ctlr, - StakingLedger { controller: None, ..StakingLedger::default_from(stash) }, - ); - Bonded::::insert(stash, ctlr); - Payee::::insert(stash, RewardDestination::Staked); - - controllers.push(ctlr); - } - - // When: - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(controllers.clone()).unwrap(); - - let result = - Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); - assert_ok!(result); - assert_eq!( - result.unwrap().actual_weight.unwrap(), - ::WeightInfo::deprecate_controller_batch(controllers.len() as u32) - ); - - // Then: - - for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { - let unique_pair = n % 2 == 0; - let ctlr: u64 = n.into(); - let stash: u64 = if unique_pair { (n + 10000).into() } else { ctlr }; - - // Side effect of migration for unique pair. - if unique_pair { - assert_eq!(Ledger::::get(ctlr), None); - } - // Bonded maps to the stash. - assert_eq!(Bonded::::get(stash), Some(stash)); - - // Ledger is keyed by stash. - let ledger_updated = Ledger::::get(stash).unwrap(); - assert_eq!(ledger_updated.stash, stash); - } - }) - } - - #[test] - fn deprecate_controller_batch_skips_unmigrated_controller_payees() { - ExtBuilder::default().try_state(false).build_and_execute(|| { - // Given: - - let stash: u64 = 1000; - let ctlr: u64 = 1001; - - Ledger::::insert( - ctlr, - StakingLedger { controller: None, ..StakingLedger::default_from(stash) }, - ); - Bonded::::insert(stash, ctlr); - #[allow(deprecated)] - Payee::::insert(stash, RewardDestination::Controller); - - // When: - - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![ctlr]).unwrap(); - - let result = - Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); - assert_ok!(result); - assert_eq!( - result.unwrap().actual_weight.unwrap(), - ::WeightInfo::deprecate_controller_batch(1 as u32) - ); - - // Then: - - // Esure deprecation did not happen. - assert_eq!(Ledger::::get(ctlr).is_some(), true); - - // Bonded still keyed by controller. - assert_eq!(Bonded::::get(stash), Some(ctlr)); - - // Ledger is still keyed by controller. - let ledger_updated = Ledger::::get(ctlr).unwrap(); - assert_eq!(ledger_updated.stash, stash); - }) - } -} diff --git a/substrate/frame/staking/src/tests/sorted_list_provider.rs b/substrate/frame/staking/src/tests/sorted_list_provider.rs new file mode 100644 index 000000000000..31446c5546ec --- /dev/null +++ b/substrate/frame/staking/src/tests/sorted_list_provider.rs @@ -0,0 +1,42 @@ +use super::*; +use frame_election_provider_support::SortedListProvider; + +#[test] +fn re_nominate_does_not_change_counters_or_list() { + ExtBuilder::default().nominate(true).build_and_execute(|| { + // given + let pre_insert_voter_count = + (Nominators::::count() + Validators::::count()) as u32; + assert_eq!(::VoterList::count(), pre_insert_voter_count); + + assert_eq!(::VoterList::iter().collect::>(), vec![11, 21, 31, 101]); + + // when account 101 renominates + assert_ok!(Staking::nominate(RuntimeOrigin::signed(101), vec![41])); + + // then counts don't change + assert_eq!(::VoterList::count(), pre_insert_voter_count); + // and the list is the same + assert_eq!(::VoterList::iter().collect::>(), vec![11, 21, 31, 101]); + }); +} + +#[test] +fn re_validate_does_not_change_counters_or_list() { + ExtBuilder::default().nominate(false).build_and_execute(|| { + // given + let pre_insert_voter_count = + (Nominators::::count() + Validators::::count()) as u32; + assert_eq!(::VoterList::count(), pre_insert_voter_count); + + assert_eq!(::VoterList::iter().collect::>(), vec![11, 21, 31]); + + // when account 11 re-validates + assert_ok!(Staking::validate(RuntimeOrigin::signed(11), Default::default())); + + // then counts don't change + assert_eq!(::VoterList::count(), pre_insert_voter_count); + // and the list is the same + assert_eq!(::VoterList::iter().collect::>(), vec![11, 21, 31]); + }); +} diff --git a/substrate/frame/staking/src/tests/staking_interface.rs b/substrate/frame/staking/src/tests/staking_interface.rs new file mode 100644 index 000000000000..c3c1d9cc2d15 --- /dev/null +++ b/substrate/frame/staking/src/tests/staking_interface.rs @@ -0,0 +1,71 @@ +use frame_support::storage::with_storage_layer; +use sp_staking::StakingInterface; + +use super::*; + +#[test] +fn force_unstake_with_slash_works() { + ExtBuilder::default().build_and_execute(|| { + // without slash + let _ = with_storage_layer::<(), _, _>(|| { + // bond an account, can unstake + assert_eq!(Staking::bonded(&11), Some(11)); + assert_ok!(::force_unstake(11)); + Err(DispatchError::from("revert")) + }); + + // bond again and add a slash, still can unstake. + assert_eq!(Staking::bonded(&11), Some(11)); + add_slash(&11); + assert_ok!(::force_unstake(11)); + }); +} + +#[test] +fn do_withdraw_unbonded_with_wrong_slash_spans_works_as_expected() { + ExtBuilder::default().build_and_execute(|| { + on_offence_now( + &[OffenceDetails { + offender: (11, Staking::eras_stakers(active_era(), &11)), + reporters: vec![], + }], + &[Perbill::from_percent(100)], + ); + + assert_eq!(Staking::bonded(&11), Some(11)); + + assert_noop!( + Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0), + Error::::IncorrectSlashingSpans + ); + + let num_slashing_spans = Staking::slashing_spans(&11).map_or(0, |s| s.iter().count()); + assert_ok!(Staking::withdraw_unbonded( + RuntimeOrigin::signed(11), + num_slashing_spans as u32 + )); + }); +} + +#[test] +fn status() { + ExtBuilder::default().build_and_execute(|| { + // stash of a validator is identified as a validator + assert_eq!(Staking::status(&11).unwrap(), StakerStatus::Validator); + // .. but not the controller. + assert!(Staking::status(&10).is_err()); + + // stash of nominator is identified as a nominator + assert_eq!(Staking::status(&101).unwrap(), StakerStatus::Nominator(vec![11, 21])); + // .. but not the controller. + assert!(Staking::status(&100).is_err()); + + // stash of chilled is identified as a chilled + assert_eq!(Staking::status(&41).unwrap(), StakerStatus::Idle); + // .. but not the controller. + assert!(Staking::status(&40).is_err()); + + // random other account. + assert!(Staking::status(&42).is_err()); + }) +} From 137f983fec5483b5350390a8556d03f92d88c190 Mon Sep 17 00:00:00 2001 From: Vicente Rocha Date: Thu, 8 Feb 2024 21:56:34 +0000 Subject: [PATCH 2/7] Add header and format --- .../src/tests/election_data_provider.rs | 19 +++++++++++++++++++ substrate/frame/staking/src/tests/ledger.rs | 19 +++++++++++++++++++ substrate/frame/staking/src/tests/mod.rs | 16 ++++++++-------- .../staking/src/tests/sorted_list_provider.rs | 19 +++++++++++++++++++ .../staking/src/tests/staking_interface.rs | 19 +++++++++++++++++++ 5 files changed, 84 insertions(+), 8 deletions(-) diff --git a/substrate/frame/staking/src/tests/election_data_provider.rs b/substrate/frame/staking/src/tests/election_data_provider.rs index 5857d4448305..aeb43cf0a93f 100644 --- a/substrate/frame/staking/src/tests/election_data_provider.rs +++ b/substrate/frame/staking/src/tests/election_data_provider.rs @@ -1,3 +1,22 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for the election data provider. + use super::*; use frame_election_provider_support::ElectionDataProvider; diff --git a/substrate/frame/staking/src/tests/ledger.rs b/substrate/frame/staking/src/tests/ledger.rs index 1ba281cc68d2..00cdd5ec39bd 100644 --- a/substrate/frame/staking/src/tests/ledger.rs +++ b/substrate/frame/staking/src/tests/ledger.rs @@ -1,3 +1,22 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for the ledger. + use super::*; #[test] diff --git a/substrate/frame/staking/src/tests/mod.rs b/substrate/frame/staking/src/tests/mod.rs index cc4993eeb39c..d2c2d5132d36 100644 --- a/substrate/frame/staking/src/tests/mod.rs +++ b/substrate/frame/staking/src/tests/mod.rs @@ -375,9 +375,9 @@ fn rewards_should_work() { ); assert_eq_error_rate!( Balances::total_balance(&101), - init_balance_101 - + part_for_101_from_11 * total_payout_0 * 2 / 3 - + part_for_101_from_21 * total_payout_0 * 1 / 3, + init_balance_101 + + part_for_101_from_11 * total_payout_0 * 2 / 3 + + part_for_101_from_21 * total_payout_0 * 1 / 3, 2 ); @@ -414,9 +414,9 @@ fn rewards_should_work() { ); assert_eq_error_rate!( Balances::total_balance(&101), - init_balance_101 - + part_for_101_from_11 * (total_payout_0 * 2 / 3 + total_payout_1) - + part_for_101_from_21 * total_payout_0 * 1 / 3, + init_balance_101 + + part_for_101_from_11 * (total_payout_0 * 2 / 3 + total_payout_1) + + part_for_101_from_21 * total_payout_0 * 1 / 3, 2 ); }); @@ -6087,8 +6087,8 @@ fn test_validator_exposure_is_backward_compatible_with_non_paged_rewards_payout( let actual_exposure_page_1 = ErasStakersPaged::::get((1, 11, 1)).unwrap(); expected_individual_exposures.iter().for_each(|exposure| { assert!( - actual_exposure_page_0.others.contains(exposure) - || actual_exposure_page_1.others.contains(exposure) + actual_exposure_page_0.others.contains(exposure) || + actual_exposure_page_1.others.contains(exposure) ); }); assert_eq!( diff --git a/substrate/frame/staking/src/tests/sorted_list_provider.rs b/substrate/frame/staking/src/tests/sorted_list_provider.rs index 31446c5546ec..3dc364931b02 100644 --- a/substrate/frame/staking/src/tests/sorted_list_provider.rs +++ b/substrate/frame/staking/src/tests/sorted_list_provider.rs @@ -1,3 +1,22 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for the sorted list provider. + use super::*; use frame_election_provider_support::SortedListProvider; diff --git a/substrate/frame/staking/src/tests/staking_interface.rs b/substrate/frame/staking/src/tests/staking_interface.rs index c3c1d9cc2d15..93ba2d905b45 100644 --- a/substrate/frame/staking/src/tests/staking_interface.rs +++ b/substrate/frame/staking/src/tests/staking_interface.rs @@ -1,3 +1,22 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for the staking interface. + use frame_support::storage::with_storage_layer; use sp_staking::StakingInterface; From b663dd7be92ef3acf694971deaf72544a128e563 Mon Sep 17 00:00:00 2001 From: Vicente Rocha Date: Sun, 14 Apr 2024 17:07:36 +0200 Subject: [PATCH 3/7] Add ledger recovery and move new ledger tests to ledger file --- substrate/frame/staking/src/tests/ledger.rs | 122 ++++++ .../staking/src/tests/ledger_recovery.rs | 390 ++++++++++++++++++ 2 files changed, 512 insertions(+) create mode 100644 substrate/frame/staking/src/tests/ledger_recovery.rs diff --git a/substrate/frame/staking/src/tests/ledger.rs b/substrate/frame/staking/src/tests/ledger.rs index 00cdd5ec39bd..cb61ea7424ba 100644 --- a/substrate/frame/staking/src/tests/ledger.rs +++ b/substrate/frame/staking/src/tests/ledger.rs @@ -329,3 +329,125 @@ fn deprecate_controller_batch_skips_unmigrated_controller_payees() { assert_eq!(ledger_updated.stash, stash); }) } + + #[test] + fn deprecate_controller_batch_skips_unmigrated_controller_payees() { + ExtBuilder::default().try_state(false).build_and_execute(|| { + // Given: + + let stash: u64 = 1000; + let ctlr: u64 = 1001; + + Ledger::::insert( + ctlr, + StakingLedger { controller: None, ..StakingLedger::default_from(stash) }, + ); + Bonded::::insert(stash, ctlr); + #[allow(deprecated)] + Payee::::insert(stash, RewardDestination::Controller); + + // When: + + let bounded_controllers: BoundedVec< + _, + ::MaxControllersInDeprecationBatch, + > = BoundedVec::try_from(vec![ctlr]).unwrap(); + + let result = + Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); + assert_ok!(result); + assert_eq!( + result.unwrap().actual_weight.unwrap(), + ::WeightInfo::deprecate_controller_batch(1 as u32) + ); + + // Then: + + // Esure deprecation did not happen. + assert_eq!(Ledger::::get(ctlr).is_some(), true); + + // Bonded still keyed by controller. + assert_eq!(Bonded::::get(stash), Some(ctlr)); + + // Ledger is still keyed by controller. + let ledger_updated = Ledger::::get(ctlr).unwrap(); + assert_eq!(ledger_updated.stash, stash); + }) + } + + #[test] + fn deprecate_controller_batch_with_bad_state_ok() { + ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // now let's deprecate all the controllers for all the existing ledgers. + let bounded_controllers: BoundedVec< + _, + ::MaxControllersInDeprecationBatch, + > = BoundedVec::try_from(vec![333, 444, 555, 777]).unwrap(); + + assert_ok!(Staking::deprecate_controller_batch( + RuntimeOrigin::root(), + bounded_controllers + )); + + assert_eq!( + *staking_events().last().unwrap(), + Event::ControllerBatchDeprecated { failures: 0 } + ); + }) + } + + #[test] + fn deprecate_controller_batch_with_bad_state_failures() { + ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // now let's deprecate all the controllers for all the existing ledgers. + let bounded_controllers: BoundedVec< + _, + ::MaxControllersInDeprecationBatch, + > = BoundedVec::try_from(vec![777, 555, 444, 333]).unwrap(); + + assert_ok!(Staking::deprecate_controller_batch( + RuntimeOrigin::root(), + bounded_controllers + )); + + assert_eq!( + *staking_events().last().unwrap(), + Event::ControllerBatchDeprecated { failures: 2 } + ); + }) + } + + #[test] + fn set_controller_with_bad_state_ok() { + ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // in this case, setting controller works due to the ordering of the calls. + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(444))); + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(555))); + }) + } + + #[test] + fn set_controller_with_bad_state_fails() { + ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // setting the controller of ledger associated with stash 555 fails since its stash is a + // controller of another ledger. + assert_noop!( + Staking::set_controller(RuntimeOrigin::signed(555)), + Error::::BadState + ); + assert_noop!( + Staking::set_controller(RuntimeOrigin::signed(444)), + Error::::BadState + ); + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); + }) + } diff --git a/substrate/frame/staking/src/tests/ledger_recovery.rs b/substrate/frame/staking/src/tests/ledger_recovery.rs new file mode 100644 index 000000000000..4a92ff4ac6b1 --- /dev/null +++ b/substrate/frame/staking/src/tests/ledger_recovery.rs @@ -0,0 +1,390 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +//! Tests for the ledger recovery. + +use super::*; +use frame_support::traits::InspectLockableCurrency; + +#[test] +fn inspect_recovery_ledger_simple_works() { + ExtBuilder::default().has_stakers(true).try_state(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // non corrupted ledger. + assert_eq!(Staking::inspect_bond_state(&11).unwrap(), LedgerIntegrityState::Ok); + + // non bonded stash. + assert!(Bonded::::get(&1111).is_none()); + assert!(Staking::inspect_bond_state(&1111).is_err()); + + // double bonded but not corrupted. + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); + }) +} + +#[test] +fn inspect_recovery_ledger_corupted_killed_works() { + ExtBuilder::default().has_stakers(true).try_state(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + let lock_333_before = Balances::balance_locked(crate::STAKING_ID, &333); + + // get into corrupted and killed ledger state by killing a corrupted ledger: + // init state: + // (333, 444) + // (444, 555) + // set_controller(444) to 444 + // (333, 444) -> corrupted + // (444, 444) + // kill(333) + // (444, 444) -> corrupted and None. + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); + set_controller_no_checks(&444); + + // now try-state fails. + assert!(Staking::do_try_state(System::block_number()).is_err()); + + // 333 is corrupted since it's controller is linking 444 ledger. + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); + // 444 however is OK. + assert_eq!(Staking::inspect_bond_state(&444).unwrap(), LedgerIntegrityState::Ok); + + // kill the corrupted ledger that is associated with stash 333. + assert_ok!(StakingLedger::::kill(&333)); + + // 333 bond is no more but it returns `BadState` because the lock on this stash is + // still set (see checks below). + assert_eq!(Staking::inspect_bond_state(&333), Err(Error::::BadState)); + // now the *other* ledger associated with 444 has been corrupted and killed (None). + assert_eq!(Staking::inspect_bond_state(&444), Ok(LedgerIntegrityState::CorruptedKilled)); + + // side effects on 333 - ledger, bonded, payee, lock should be completely empty. + // however, 333 lock remains. + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_333_before); // NOK + assert!(Bonded::::get(&333).is_none()); // OK + assert!(Payee::::get(&333).is_none()); // OK + assert!(Ledger::::get(&444).is_none()); // OK + + // side effects on 444 - ledger, bonded, payee, lock should remain be intact. + // however, 444 lock was removed. + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), 0); // NOK + assert!(Bonded::::get(&444).is_some()); // OK + assert!(Payee::::get(&444).is_some()); // OK + assert!(Ledger::::get(&555).is_none()); // NOK + + assert!(Staking::do_try_state(System::block_number()).is_err()); + }) +} + +#[test] +fn inspect_recovery_ledger_corupted_killed_other_works() { + ExtBuilder::default().has_stakers(true).try_state(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + let lock_333_before = Balances::balance_locked(crate::STAKING_ID, &333); + + // get into corrupted and killed ledger state by killing a corrupted ledger: + // init state: + // (333, 444) + // (444, 555) + // set_controller(444) to 444 + // (333, 444) -> corrupted + // (444, 444) + // kill(444) + // (333, 444) -> corrupted and None + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); + set_controller_no_checks(&444); + + // now try-state fails. + assert!(Staking::do_try_state(System::block_number()).is_err()); + + // 333 is corrupted since it's controller is linking 444 ledger. + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); + // 444 however is OK. + assert_eq!(Staking::inspect_bond_state(&444).unwrap(), LedgerIntegrityState::Ok); + + // kill the *other* ledger that is double bonded but not corrupted. + assert_ok!(StakingLedger::::kill(&444)); + + // now 333 is corrupted and None through the *other* ledger being killed. + assert_eq!( + Staking::inspect_bond_state(&333).unwrap(), + LedgerIntegrityState::CorruptedKilled, + ); + // 444 is cleaned and not a stash anymore; no lock left behind. + assert_eq!(Ledger::::get(&444), None); + assert_eq!(Staking::inspect_bond_state(&444), Err(Error::::NotStash)); + + // side effects on 333 - ledger, bonded, payee, lock should be intact. + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_333_before); // OK + assert_eq!(Bonded::::get(&333), Some(444)); // OK + assert!(Payee::::get(&333).is_some()); // OK + // however, ledger associated with its controller was killed. + assert!(Ledger::::get(&444).is_none()); // NOK + + // side effects on 444 - ledger, bonded, payee, lock should be completely removed. + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), 0); // OK + assert!(Bonded::::get(&444).is_none()); // OK + assert!(Payee::::get(&444).is_none()); // OK + assert!(Ledger::::get(&555).is_none()); // OK + + assert!(Staking::do_try_state(System::block_number()).is_err()); + }) +} + +#[test] +fn inspect_recovery_ledger_lock_corrupted_works() { + ExtBuilder::default().has_stakers(true).try_state(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // get into lock corrupted ledger state by bond_extra on a ledger that is double bonded + // with a corrupted ledger. + // init state: + // (333, 444) + // (444, 555) + // set_controller(444) to 444 + // (333, 444) -> corrupted + // (444, 444) + // bond_extra(333, 10) -> lock corrupted on 444 + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); + set_controller_no_checks(&444); + bond_extra_no_checks(&333, 10); + + // now try-state fails. + assert!(Staking::do_try_state(System::block_number()).is_err()); + + // 333 is corrupted since it's controller is linking 444 ledger. + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); + // 444 ledger is not corrupted but locks got out of sync. + assert_eq!(Staking::inspect_bond_state(&444).unwrap(), LedgerIntegrityState::LockCorrupted); + }) +} + +// Corrupted ledger restore. +// +// * Double bonded and corrupted ledger. +#[test] +fn restore_ledger_corrupted_works() { + ExtBuilder::default().has_stakers(true).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // get into corrupted and killed ledger state. + // init state: + // (333, 444) + // (444, 555) + // set_controller(444) to 444 + // (333, 444) -> corrupted + // (444, 444) + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); + set_controller_no_checks(&444); + + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); + + // now try-state fails. + assert!(Staking::do_try_state(System::block_number()).is_err()); + + // recover the ledger bonded by 333 stash. + assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); + + // try-state checks are ok now. + assert_ok!(Staking::do_try_state(System::block_number())); + }) +} + +// Corrupted and killed ledger restore. +// +// * Double bonded and corrupted ledger. +// * Ledger killed by own controller. +#[test] +fn restore_ledger_corrupted_killed_works() { + ExtBuilder::default().has_stakers(true).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // ledger.total == lock + let total_444_before_corruption = Balances::balance_locked(crate::STAKING_ID, &444); + + // get into corrupted and killed ledger state by killing a corrupted ledger: + // init state: + // (333, 444) + // (444, 555) + // set_controller(444) to 444 + // (333, 444) -> corrupted + // (444, 444) + // kill(333) + // (444, 444) -> corrupted and None. + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); + set_controller_no_checks(&444); + + // kill the corrupted ledger that is associated with stash 333. + assert_ok!(StakingLedger::::kill(&333)); + + // 333 bond is no more but it returns `BadState` because the lock on this stash is + // still set (see checks below). + assert_eq!(Staking::inspect_bond_state(&333), Err(Error::::BadState)); + // now the *other* ledger associated with 444 has been corrupted and killed (None). + assert!(Staking::ledger(StakingAccount::Stash(444)).is_err()); + + // try-state should fail. + assert!(Staking::do_try_state(System::block_number()).is_err()); + + // recover the ledger bonded by 333 stash. + assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); + + // for the try-state checks to pass, we also need to recover the stash 444 which is + // corrupted too by proxy of kill(333). Currently, both the lock and the ledger of 444 + // have been cleared so we need to provide the new amount to restore the ledger. + assert_noop!( + Staking::restore_ledger(RuntimeOrigin::root(), 444, None, None, None), + Error::::CannotRestoreLedger + ); + + assert_ok!(Staking::restore_ledger( + RuntimeOrigin::root(), + 444, + None, + Some(total_444_before_corruption), + None, + )); + + // try-state checks are ok now. + assert_ok!(Staking::do_try_state(System::block_number())); + }) +} + +// Corrupted and killed by *other* ledger restore. +// +// * Double bonded and corrupted ledger. +// * Ledger killed by own controller. +#[test] +fn restore_ledger_corrupted_killed_other_works() { + ExtBuilder::default().has_stakers(true).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // get into corrupted and killed ledger state by killing a corrupted ledger: + // init state: + // (333, 444) + // (444, 555) + // set_controller(444) to 444 + // (333, 444) -> corrupted + // (444, 444) + // kill(444) + // (333, 444) -> corrupted and None + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); + set_controller_no_checks(&444); + + // now try-state fails. + assert!(Staking::do_try_state(System::block_number()).is_err()); + + // 333 is corrupted since it's controller is linking 444 ledger. + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); + // 444 however is OK. + assert_eq!(Staking::inspect_bond_state(&444).unwrap(), LedgerIntegrityState::Ok); + + // kill the *other* ledger that is double bonded but not corrupted. + assert_ok!(StakingLedger::::kill(&444)); + + // recover the ledger bonded by 333 stash. + assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); + + // 444 does not need recover in this case since it's been killed successfully. + assert_eq!(Staking::inspect_bond_state(&444), Err(Error::::NotStash)); + + // try-state checks are ok now. + assert_ok!(Staking::do_try_state(System::block_number())); + }) +} + +// Corrupted with bond_extra. +// +// * Double bonded and corrupted ledger. +// * Corrupted ledger calls `bond_extra` +#[test] +fn restore_ledger_corrupted_bond_extra_works() { + ExtBuilder::default().has_stakers(true).build_and_execute(|| { + setup_double_bonded_ledgers(); + + let lock_333_before = Balances::balance_locked(crate::STAKING_ID, &333); + let lock_444_before = Balances::balance_locked(crate::STAKING_ID, &444); + + // get into corrupted and killed ledger state by killing a corrupted ledger: + // init state: + // (333, 444) + // (444, 555) + // set_controller(444) to 444 + // (333, 444) -> corrupted + // (444, 444) + // bond_extra(444, 40) -> OK + // bond_extra(333, 30) -> locks out of sync + + assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); + set_controller_no_checks(&444); + + // now try-state fails. + assert!(Staking::do_try_state(System::block_number()).is_err()); + + // if 444 bonds extra, the locks remain in sync. + bond_extra_no_checks(&444, 40); + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_333_before); + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), lock_444_before + 40); + + // however if 333 bonds extra, the wrong lock is updated. + bond_extra_no_checks(&333, 30); + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_444_before + 40 + 30); //not OK + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), lock_444_before + 40); // OK + + // recover the ledger bonded by 333 stash. Note that the total/lock needs to be + // re-written since on-chain data lock has become out of sync. + assert_ok!(Staking::restore_ledger( + RuntimeOrigin::root(), + 333, + None, + Some(lock_333_before + 30), + None + )); + + // now recover 444 that although it's not corrupted, its lock and ledger.total are out + // of sync. in which case, we need to explicitly set the ledger's lock and amount, + // otherwise the ledger recover will fail. + assert_noop!( + Staking::restore_ledger(RuntimeOrigin::root(), 444, None, None, None), + Error::::CannotRestoreLedger + ); + + //and enforcing a new ledger lock/total on this non-corrupted ledger will work. + assert_ok!(Staking::restore_ledger( + RuntimeOrigin::root(), + 444, + None, + Some(lock_444_before + 40), + None + )); + + // double-check that ledgers got to expected state and bond_extra done during the + // corrupted state is part of the recovered ledgers. + let ledger_333 = Bonded::::get(&333).and_then(Ledger::::get).unwrap(); + let ledger_444 = Bonded::::get(&444).and_then(Ledger::::get).unwrap(); + + assert_eq!(ledger_333.total, lock_333_before + 30); + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), ledger_333.total); + assert_eq!(ledger_444.total, lock_444_before + 40); + assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), ledger_444.total); + + // try-state checks are ok now. + assert_ok!(Staking::do_try_state(System::block_number())); + }) +} From b6d728354743c79ae99731d6d66caf437853907b Mon Sep 17 00:00:00 2001 From: Vicente Rocha Date: Sun, 14 Apr 2024 17:11:05 +0200 Subject: [PATCH 4/7] Add missing test --- substrate/frame/staking/src/tests/mod.rs | 527 +---------------------- 1 file changed, 18 insertions(+), 509 deletions(-) diff --git a/substrate/frame/staking/src/tests/mod.rs b/substrate/frame/staking/src/tests/mod.rs index 809b44c74445..bde5fffa8d4c 100644 --- a/substrate/frame/staking/src/tests/mod.rs +++ b/substrate/frame/staking/src/tests/mod.rs @@ -19,6 +19,7 @@ mod election_data_provider; mod ledger; +mod ledger_recovery; mod sorted_list_provider; mod staking_interface; @@ -1253,6 +1254,23 @@ fn bond_extra_works() { }); } +#[test] +fn bond_extra_controller_bad_state_works() { + ExtBuilder::default().try_state(false).build_and_execute(|| { + assert_eq!(StakingLedger::::get(StakingAccount::Stash(31)).unwrap().stash, 31); + + // simulate ledger in bad state: the controller 41 is associated to the stash 31 and 41. + Bonded::::insert(31, 41); + + // we confirm that the ledger is in bad state: 31 has 41 as controller and when fetching + // the ledger associated with the controller 41, its stash is 41 (and not 31). + assert_eq!(Ledger::::get(41).unwrap().stash, 41); + + // if the ledger is in this bad state, the `bond_extra` should fail. + assert_noop!(Staking::bond_extra(RuntimeOrigin::signed(31), 10), Error::::BadState); + }) +} + #[test] fn bond_extra_and_withdraw_unbonded_works() { // @@ -6253,512 +6271,3 @@ fn test_validator_exposure_is_backward_compatible_with_non_paged_rewards_payout( ); }); } - -mod ledger { - use super::*; - - #[test] - fn deprecate_controller_batch_skips_unmigrated_controller_payees() { - ExtBuilder::default().try_state(false).build_and_execute(|| { - // Given: - - let stash: u64 = 1000; - let ctlr: u64 = 1001; - - Ledger::::insert( - ctlr, - StakingLedger { controller: None, ..StakingLedger::default_from(stash) }, - ); - Bonded::::insert(stash, ctlr); - #[allow(deprecated)] - Payee::::insert(stash, RewardDestination::Controller); - - // When: - - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![ctlr]).unwrap(); - - let result = - Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); - assert_ok!(result); - assert_eq!( - result.unwrap().actual_weight.unwrap(), - ::WeightInfo::deprecate_controller_batch(1 as u32) - ); - - // Then: - - // Esure deprecation did not happen. - assert_eq!(Ledger::::get(ctlr).is_some(), true); - - // Bonded still keyed by controller. - assert_eq!(Bonded::::get(stash), Some(ctlr)); - - // Ledger is still keyed by controller. - let ledger_updated = Ledger::::get(ctlr).unwrap(); - assert_eq!(ledger_updated.stash, stash); - }) - } - - #[test] - fn deprecate_controller_batch_with_bad_state_ok() { - ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // now let's deprecate all the controllers for all the existing ledgers. - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![333, 444, 555, 777]).unwrap(); - - assert_ok!(Staking::deprecate_controller_batch( - RuntimeOrigin::root(), - bounded_controllers - )); - - assert_eq!( - *staking_events().last().unwrap(), - Event::ControllerBatchDeprecated { failures: 0 } - ); - }) - } - - #[test] - fn deprecate_controller_batch_with_bad_state_failures() { - ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // now let's deprecate all the controllers for all the existing ledgers. - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![777, 555, 444, 333]).unwrap(); - - assert_ok!(Staking::deprecate_controller_batch( - RuntimeOrigin::root(), - bounded_controllers - )); - - assert_eq!( - *staking_events().last().unwrap(), - Event::ControllerBatchDeprecated { failures: 2 } - ); - }) - } - - #[test] - fn set_controller_with_bad_state_ok() { - ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // in this case, setting controller works due to the ordering of the calls. - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(444))); - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(555))); - }) - } - - #[test] - fn set_controller_with_bad_state_fails() { - ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // setting the controller of ledger associated with stash 555 fails since its stash is a - // controller of another ledger. - assert_noop!( - Staking::set_controller(RuntimeOrigin::signed(555)), - Error::::BadState - ); - assert_noop!( - Staking::set_controller(RuntimeOrigin::signed(444)), - Error::::BadState - ); - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); - }) - } -} - -mod ledger_recovery { - use super::*; - use frame_support::traits::InspectLockableCurrency; - - #[test] - fn inspect_recovery_ledger_simple_works() { - ExtBuilder::default().has_stakers(true).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // non corrupted ledger. - assert_eq!(Staking::inspect_bond_state(&11).unwrap(), LedgerIntegrityState::Ok); - - // non bonded stash. - assert!(Bonded::::get(&1111).is_none()); - assert!(Staking::inspect_bond_state(&1111).is_err()); - - // double bonded but not corrupted. - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); - }) - } - - #[test] - fn inspect_recovery_ledger_corupted_killed_works() { - ExtBuilder::default().has_stakers(true).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - let lock_333_before = Balances::balance_locked(crate::STAKING_ID, &333); - - // get into corrupted and killed ledger state by killing a corrupted ledger: - // init state: - // (333, 444) - // (444, 555) - // set_controller(444) to 444 - // (333, 444) -> corrupted - // (444, 444) - // kill(333) - // (444, 444) -> corrupted and None. - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); - set_controller_no_checks(&444); - - // now try-state fails. - assert!(Staking::do_try_state(System::block_number()).is_err()); - - // 333 is corrupted since it's controller is linking 444 ledger. - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); - // 444 however is OK. - assert_eq!(Staking::inspect_bond_state(&444).unwrap(), LedgerIntegrityState::Ok); - - // kill the corrupted ledger that is associated with stash 333. - assert_ok!(StakingLedger::::kill(&333)); - - // 333 bond is no more but it returns `BadState` because the lock on this stash is - // still set (see checks below). - assert_eq!(Staking::inspect_bond_state(&333), Err(Error::::BadState)); - // now the *other* ledger associated with 444 has been corrupted and killed (None). - assert_eq!( - Staking::inspect_bond_state(&444), - Ok(LedgerIntegrityState::CorruptedKilled) - ); - - // side effects on 333 - ledger, bonded, payee, lock should be completely empty. - // however, 333 lock remains. - assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_333_before); // NOK - assert!(Bonded::::get(&333).is_none()); // OK - assert!(Payee::::get(&333).is_none()); // OK - assert!(Ledger::::get(&444).is_none()); // OK - - // side effects on 444 - ledger, bonded, payee, lock should remain be intact. - // however, 444 lock was removed. - assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), 0); // NOK - assert!(Bonded::::get(&444).is_some()); // OK - assert!(Payee::::get(&444).is_some()); // OK - assert!(Ledger::::get(&555).is_none()); // NOK - - assert!(Staking::do_try_state(System::block_number()).is_err()); - }) - } - - #[test] - fn inspect_recovery_ledger_corupted_killed_other_works() { - ExtBuilder::default().has_stakers(true).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - let lock_333_before = Balances::balance_locked(crate::STAKING_ID, &333); - - // get into corrupted and killed ledger state by killing a corrupted ledger: - // init state: - // (333, 444) - // (444, 555) - // set_controller(444) to 444 - // (333, 444) -> corrupted - // (444, 444) - // kill(444) - // (333, 444) -> corrupted and None - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); - set_controller_no_checks(&444); - - // now try-state fails. - assert!(Staking::do_try_state(System::block_number()).is_err()); - - // 333 is corrupted since it's controller is linking 444 ledger. - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); - // 444 however is OK. - assert_eq!(Staking::inspect_bond_state(&444).unwrap(), LedgerIntegrityState::Ok); - - // kill the *other* ledger that is double bonded but not corrupted. - assert_ok!(StakingLedger::::kill(&444)); - - // now 333 is corrupted and None through the *other* ledger being killed. - assert_eq!( - Staking::inspect_bond_state(&333).unwrap(), - LedgerIntegrityState::CorruptedKilled, - ); - // 444 is cleaned and not a stash anymore; no lock left behind. - assert_eq!(Ledger::::get(&444), None); - assert_eq!(Staking::inspect_bond_state(&444), Err(Error::::NotStash)); - - // side effects on 333 - ledger, bonded, payee, lock should be intact. - assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_333_before); // OK - assert_eq!(Bonded::::get(&333), Some(444)); // OK - assert!(Payee::::get(&333).is_some()); // OK - // however, ledger associated with its controller was killed. - assert!(Ledger::::get(&444).is_none()); // NOK - - // side effects on 444 - ledger, bonded, payee, lock should be completely removed. - assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), 0); // OK - assert!(Bonded::::get(&444).is_none()); // OK - assert!(Payee::::get(&444).is_none()); // OK - assert!(Ledger::::get(&555).is_none()); // OK - - assert!(Staking::do_try_state(System::block_number()).is_err()); - }) - } - - #[test] - fn inspect_recovery_ledger_lock_corrupted_works() { - ExtBuilder::default().has_stakers(true).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // get into lock corrupted ledger state by bond_extra on a ledger that is double bonded - // with a corrupted ledger. - // init state: - // (333, 444) - // (444, 555) - // set_controller(444) to 444 - // (333, 444) -> corrupted - // (444, 444) - // bond_extra(333, 10) -> lock corrupted on 444 - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); - set_controller_no_checks(&444); - bond_extra_no_checks(&333, 10); - - // now try-state fails. - assert!(Staking::do_try_state(System::block_number()).is_err()); - - // 333 is corrupted since it's controller is linking 444 ledger. - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); - // 444 ledger is not corrupted but locks got out of sync. - assert_eq!( - Staking::inspect_bond_state(&444).unwrap(), - LedgerIntegrityState::LockCorrupted - ); - }) - } - - // Corrupted ledger restore. - // - // * Double bonded and corrupted ledger. - #[test] - fn restore_ledger_corrupted_works() { - ExtBuilder::default().has_stakers(true).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // get into corrupted and killed ledger state. - // init state: - // (333, 444) - // (444, 555) - // set_controller(444) to 444 - // (333, 444) -> corrupted - // (444, 444) - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); - set_controller_no_checks(&444); - - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); - - // now try-state fails. - assert!(Staking::do_try_state(System::block_number()).is_err()); - - // recover the ledger bonded by 333 stash. - assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); - - // try-state checks are ok now. - assert_ok!(Staking::do_try_state(System::block_number())); - }) - } - - // Corrupted and killed ledger restore. - // - // * Double bonded and corrupted ledger. - // * Ledger killed by own controller. - #[test] - fn restore_ledger_corrupted_killed_works() { - ExtBuilder::default().has_stakers(true).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // ledger.total == lock - let total_444_before_corruption = Balances::balance_locked(crate::STAKING_ID, &444); - - // get into corrupted and killed ledger state by killing a corrupted ledger: - // init state: - // (333, 444) - // (444, 555) - // set_controller(444) to 444 - // (333, 444) -> corrupted - // (444, 444) - // kill(333) - // (444, 444) -> corrupted and None. - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); - set_controller_no_checks(&444); - - // kill the corrupted ledger that is associated with stash 333. - assert_ok!(StakingLedger::::kill(&333)); - - // 333 bond is no more but it returns `BadState` because the lock on this stash is - // still set (see checks below). - assert_eq!(Staking::inspect_bond_state(&333), Err(Error::::BadState)); - // now the *other* ledger associated with 444 has been corrupted and killed (None). - assert!(Staking::ledger(StakingAccount::Stash(444)).is_err()); - - // try-state should fail. - assert!(Staking::do_try_state(System::block_number()).is_err()); - - // recover the ledger bonded by 333 stash. - assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); - - // for the try-state checks to pass, we also need to recover the stash 444 which is - // corrupted too by proxy of kill(333). Currently, both the lock and the ledger of 444 - // have been cleared so we need to provide the new amount to restore the ledger. - assert_noop!( - Staking::restore_ledger(RuntimeOrigin::root(), 444, None, None, None), - Error::::CannotRestoreLedger - ); - - assert_ok!(Staking::restore_ledger( - RuntimeOrigin::root(), - 444, - None, - Some(total_444_before_corruption), - None, - )); - - // try-state checks are ok now. - assert_ok!(Staking::do_try_state(System::block_number())); - }) - } - - // Corrupted and killed by *other* ledger restore. - // - // * Double bonded and corrupted ledger. - // * Ledger killed by own controller. - #[test] - fn restore_ledger_corrupted_killed_other_works() { - ExtBuilder::default().has_stakers(true).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // get into corrupted and killed ledger state by killing a corrupted ledger: - // init state: - // (333, 444) - // (444, 555) - // set_controller(444) to 444 - // (333, 444) -> corrupted - // (444, 444) - // kill(444) - // (333, 444) -> corrupted and None - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); - set_controller_no_checks(&444); - - // now try-state fails. - assert!(Staking::do_try_state(System::block_number()).is_err()); - - // 333 is corrupted since it's controller is linking 444 ledger. - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Corrupted); - // 444 however is OK. - assert_eq!(Staking::inspect_bond_state(&444).unwrap(), LedgerIntegrityState::Ok); - - // kill the *other* ledger that is double bonded but not corrupted. - assert_ok!(StakingLedger::::kill(&444)); - - // recover the ledger bonded by 333 stash. - assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); - - // 444 does not need recover in this case since it's been killed successfully. - assert_eq!(Staking::inspect_bond_state(&444), Err(Error::::NotStash)); - - // try-state checks are ok now. - assert_ok!(Staking::do_try_state(System::block_number())); - }) - } - - // Corrupted with bond_extra. - // - // * Double bonded and corrupted ledger. - // * Corrupted ledger calls `bond_extra` - #[test] - fn restore_ledger_corrupted_bond_extra_works() { - ExtBuilder::default().has_stakers(true).build_and_execute(|| { - setup_double_bonded_ledgers(); - - let lock_333_before = Balances::balance_locked(crate::STAKING_ID, &333); - let lock_444_before = Balances::balance_locked(crate::STAKING_ID, &444); - - // get into corrupted and killed ledger state by killing a corrupted ledger: - // init state: - // (333, 444) - // (444, 555) - // set_controller(444) to 444 - // (333, 444) -> corrupted - // (444, 444) - // bond_extra(444, 40) -> OK - // bond_extra(333, 30) -> locks out of sync - - assert_eq!(Staking::inspect_bond_state(&333).unwrap(), LedgerIntegrityState::Ok); - set_controller_no_checks(&444); - - // now try-state fails. - assert!(Staking::do_try_state(System::block_number()).is_err()); - - // if 444 bonds extra, the locks remain in sync. - bond_extra_no_checks(&444, 40); - assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), lock_333_before); - assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), lock_444_before + 40); - - // however if 333 bonds extra, the wrong lock is updated. - bond_extra_no_checks(&333, 30); - assert_eq!( - Balances::balance_locked(crate::STAKING_ID, &333), - lock_444_before + 40 + 30 - ); //not OK - assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), lock_444_before + 40); // OK - - // recover the ledger bonded by 333 stash. Note that the total/lock needs to be - // re-written since on-chain data lock has become out of sync. - assert_ok!(Staking::restore_ledger( - RuntimeOrigin::root(), - 333, - None, - Some(lock_333_before + 30), - None - )); - - // now recover 444 that although it's not corrupted, its lock and ledger.total are out - // of sync. in which case, we need to explicitly set the ledger's lock and amount, - // otherwise the ledger recover will fail. - assert_noop!( - Staking::restore_ledger(RuntimeOrigin::root(), 444, None, None, None), - Error::::CannotRestoreLedger - ); - - //and enforcing a new ledger lock/total on this non-corrupted ledger will work. - assert_ok!(Staking::restore_ledger( - RuntimeOrigin::root(), - 444, - None, - Some(lock_444_before + 40), - None - )); - - // double-check that ledgers got to expected state and bond_extra done during the - // corrupted state is part of the recovered ledgers. - let ledger_333 = Bonded::::get(&333).and_then(Ledger::::get).unwrap(); - let ledger_444 = Bonded::::get(&444).and_then(Ledger::::get).unwrap(); - - assert_eq!(ledger_333.total, lock_333_before + 30); - assert_eq!(Balances::balance_locked(crate::STAKING_ID, &333), ledger_333.total); - assert_eq!(ledger_444.total, lock_444_before + 40); - assert_eq!(Balances::balance_locked(crate::STAKING_ID, &444), ledger_444.total); - - // try-state checks are ok now. - assert_ok!(Staking::do_try_state(System::block_number())); - }) - } -} From c13962448f00d943cc66ba9922cedada034a98d3 Mon Sep 17 00:00:00 2001 From: Vicente Rocha Date: Sun, 14 Apr 2024 17:15:36 +0200 Subject: [PATCH 5/7] Remove duplicated test --- substrate/frame/staking/src/tests/ledger.rs | 168 ++++++-------------- 1 file changed, 53 insertions(+), 115 deletions(-) diff --git a/substrate/frame/staking/src/tests/ledger.rs b/substrate/frame/staking/src/tests/ledger.rs index cb61ea7424ba..3bed02299bf0 100644 --- a/substrate/frame/staking/src/tests/ledger.rs +++ b/substrate/frame/staking/src/tests/ledger.rs @@ -329,125 +329,63 @@ fn deprecate_controller_batch_skips_unmigrated_controller_payees() { assert_eq!(ledger_updated.stash, stash); }) } +#[test] +fn deprecate_controller_batch_with_bad_state_ok() { + ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { + setup_double_bonded_ledgers(); - #[test] - fn deprecate_controller_batch_skips_unmigrated_controller_payees() { - ExtBuilder::default().try_state(false).build_and_execute(|| { - // Given: + // now let's deprecate all the controllers for all the existing ledgers. + let bounded_controllers: BoundedVec<_, ::MaxControllersInDeprecationBatch> = + BoundedVec::try_from(vec![333, 444, 555, 777]).unwrap(); - let stash: u64 = 1000; - let ctlr: u64 = 1001; + assert_ok!(Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers)); - Ledger::::insert( - ctlr, - StakingLedger { controller: None, ..StakingLedger::default_from(stash) }, - ); - Bonded::::insert(stash, ctlr); - #[allow(deprecated)] - Payee::::insert(stash, RewardDestination::Controller); - - // When: - - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![ctlr]).unwrap(); - - let result = - Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); - assert_ok!(result); - assert_eq!( - result.unwrap().actual_weight.unwrap(), - ::WeightInfo::deprecate_controller_batch(1 as u32) - ); + assert_eq!( + *staking_events().last().unwrap(), + Event::ControllerBatchDeprecated { failures: 0 } + ); + }) +} - // Then: +#[test] +fn deprecate_controller_batch_with_bad_state_failures() { + ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { + setup_double_bonded_ledgers(); - // Esure deprecation did not happen. - assert_eq!(Ledger::::get(ctlr).is_some(), true); + // now let's deprecate all the controllers for all the existing ledgers. + let bounded_controllers: BoundedVec<_, ::MaxControllersInDeprecationBatch> = + BoundedVec::try_from(vec![777, 555, 444, 333]).unwrap(); - // Bonded still keyed by controller. - assert_eq!(Bonded::::get(stash), Some(ctlr)); + assert_ok!(Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers)); - // Ledger is still keyed by controller. - let ledger_updated = Ledger::::get(ctlr).unwrap(); - assert_eq!(ledger_updated.stash, stash); - }) - } - - #[test] - fn deprecate_controller_batch_with_bad_state_ok() { - ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // now let's deprecate all the controllers for all the existing ledgers. - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![333, 444, 555, 777]).unwrap(); - - assert_ok!(Staking::deprecate_controller_batch( - RuntimeOrigin::root(), - bounded_controllers - )); - - assert_eq!( - *staking_events().last().unwrap(), - Event::ControllerBatchDeprecated { failures: 0 } - ); - }) - } - - #[test] - fn deprecate_controller_batch_with_bad_state_failures() { - ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // now let's deprecate all the controllers for all the existing ledgers. - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![777, 555, 444, 333]).unwrap(); - - assert_ok!(Staking::deprecate_controller_batch( - RuntimeOrigin::root(), - bounded_controllers - )); - - assert_eq!( - *staking_events().last().unwrap(), - Event::ControllerBatchDeprecated { failures: 2 } - ); - }) - } - - #[test] - fn set_controller_with_bad_state_ok() { - ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // in this case, setting controller works due to the ordering of the calls. - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(444))); - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(555))); - }) - } - - #[test] - fn set_controller_with_bad_state_fails() { - ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // setting the controller of ledger associated with stash 555 fails since its stash is a - // controller of another ledger. - assert_noop!( - Staking::set_controller(RuntimeOrigin::signed(555)), - Error::::BadState - ); - assert_noop!( - Staking::set_controller(RuntimeOrigin::signed(444)), - Error::::BadState - ); - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); - }) - } + assert_eq!( + *staking_events().last().unwrap(), + Event::ControllerBatchDeprecated { failures: 2 } + ); + }) +} + +#[test] +fn set_controller_with_bad_state_ok() { + ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // in this case, setting controller works due to the ordering of the calls. + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(444))); + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(555))); + }) +} + +#[test] +fn set_controller_with_bad_state_fails() { + ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // setting the controller of ledger associated with stash 555 fails since its stash is a + // controller of another ledger. + assert_noop!(Staking::set_controller(RuntimeOrigin::signed(555)), Error::::BadState); + assert_noop!(Staking::set_controller(RuntimeOrigin::signed(444)), Error::::BadState); + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); + }) +} From 6d59ce85a887ed5d7ab9d32ef8c80ccb5719526e Mon Sep 17 00:00:00 2001 From: Vicente Rocha Date: Sun, 14 Apr 2024 17:31:19 +0200 Subject: [PATCH 6/7] Add more missing tests --- substrate/frame/staking/src/tests/ledger.rs | 65 +++++++++++++++++++++ 1 file changed, 65 insertions(+) diff --git a/substrate/frame/staking/src/tests/ledger.rs b/substrate/frame/staking/src/tests/ledger.rs index 3bed02299bf0..e3ead27db08c 100644 --- a/substrate/frame/staking/src/tests/ledger.rs +++ b/substrate/frame/staking/src/tests/ledger.rs @@ -84,6 +84,49 @@ fn get_ledger_works() { }) } +#[test] +fn get_ledger_bad_state_fails() { + ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { + setup_double_bonded_ledgers(); + + // Case 1: double bonded but not corrupted: + // stash 444 has controller 555: + assert_eq!(Bonded::::get(444), Some(555)); + assert_eq!(Ledger::::get(555).unwrap().stash, 444); + + // stash 444 is also a controller of 333: + assert_eq!(Bonded::::get(333), Some(444)); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(333)), Some(444)); + assert_eq!(Ledger::::get(444).unwrap().stash, 333); + + // although 444 is double bonded (it is a controller and a stash of different ledgers), + // we can safely retrieve the ledger and mutate it since the correct ledger is + // returned. + let ledger_result = StakingLedger::::get(StakingAccount::Stash(444)); + assert_eq!(ledger_result.unwrap().stash, 444); // correct ledger. + + let ledger_result = StakingLedger::::get(StakingAccount::Controller(444)); + assert_eq!(ledger_result.unwrap().stash, 333); // correct ledger. + + // fetching ledger 333 by its stash works. + let ledger_result = StakingLedger::::get(StakingAccount::Stash(333)); + assert_eq!(ledger_result.unwrap().stash, 333); + + // Case 2: corrupted ledger bonding. + // in this case, we simulate what happens when fetching a ledger by stash returns a + // ledger with a different stash. when this happens, we return an error instead of the + // ledger to prevent ledger mutations. + let mut ledger = Ledger::::get(444).unwrap(); + assert_eq!(ledger.stash, 333); + ledger.stash = 444; + Ledger::::insert(444, ledger); + + // now, we are prevented from fetching the ledger by stash from 1. It's associated + // controller (2) is now bonding a ledger with a different stash (2, not 1). + assert!(StakingLedger::::get(StakingAccount::Stash(333)).is_err()); + }) +} + #[test] fn bond_works() { ExtBuilder::default().build_and_execute(|| { @@ -107,6 +150,28 @@ fn bond_works() { }) } +#[test] +fn bond_controller_cannot_be_stash_works() { + ExtBuilder::default().build_and_execute(|| { + let (stash, controller) = testing_utils::create_unique_stash_controller::( + 0, + 10, + RewardDestination::Staked, + false, + ) + .unwrap(); + + assert_eq!(Bonded::::get(stash), Some(controller)); + assert_eq!(Ledger::::get(controller).map(|l| l.stash), Some(stash)); + + // existing controller should not be able become a stash. + assert_noop!( + Staking::bond(RuntimeOrigin::signed(controller), 10, RewardDestination::Staked), + Error::::AlreadyPaired, + ); + }) +} + #[test] fn is_bonded_works() { ExtBuilder::default().build_and_execute(|| { From c8fd6328a12d0f0aac8afea5782953674d2fd9b3 Mon Sep 17 00:00:00 2001 From: Vicente Rocha Date: Sun, 14 Apr 2024 17:34:01 +0200 Subject: [PATCH 7/7] Fix test --- substrate/frame/staking/src/tests/ledger.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/tests/ledger.rs b/substrate/frame/staking/src/tests/ledger.rs index e3ead27db08c..05c4c3bcc00e 100644 --- a/substrate/frame/staking/src/tests/ledger.rs +++ b/substrate/frame/staking/src/tests/ledger.rs @@ -226,7 +226,7 @@ fn update_payee_migration_works() { #[test] fn deprecate_controller_batch_works_full_weight() { - ExtBuilder::default().build_and_execute(|| { + ExtBuilder::default().try_state(false).build_and_execute(|| { // Given: let start = 1001;