From 0811cb9beb2773648cbdbf509dd19f5e0ed866fb Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Fri, 28 Apr 2023 15:59:30 +0700 Subject: [PATCH 01/60] update set_controller --- frame/staking/README.md | 5 +++-- frame/staking/src/benchmarking.rs | 8 +++----- frame/staking/src/lib.rs | 2 +- frame/staking/src/pallet/mod.rs | 17 +++++++---------- frame/staking/src/testing_utils.rs | 20 ++++++++++++++++++++ frame/staking/src/tests.rs | 8 ++++---- 6 files changed, 38 insertions(+), 22 deletions(-) diff --git a/frame/staking/README.md b/frame/staking/README.md index bbd5bd18f6e81..8ea95ab6ea1fa 100644 --- a/frame/staking/README.md +++ b/frame/staking/README.md @@ -50,8 +50,9 @@ used. An account pair can become bonded using the [`bond`](https://docs.rs/pallet-staking/latest/pallet_staking/enum.Call.html#variant.bond) call. -Stash accounts can change their associated controller using the -[`set_controller`](https://docs.rs/pallet-staking/latest/pallet_staking/enum.Call.html#variant.set_controller) call. +Stash accounts can update their associated controller back to their stash account using the +[`set_controller`](https://docs.rs/pallet-staking/latest/pallet_staking/enum.Call.html#variant.set_controller) +call. There are three possible roles that any staked account pair can be in: `Validator`, `Nominator` and `Idle` (defined in [`StakerStatus`](https://docs.rs/pallet-staking/latest/pallet_staking/enum.StakerStatus.html)). There are three diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index ad7aab984bdca..a144bd0425dd8 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -470,13 +470,11 @@ benchmarks! { } set_controller { - let (stash, _) = create_stash_controller::(USER_SEED, 100, Default::default())?; - let new_controller = create_funded_user::("new_controller", USER_SEED, 100); - let new_controller_lookup = T::Lookup::unlookup(new_controller.clone()); + let (stash, _) = create_stash_controller_inc::(USER_SEED, 100, Default::default())?; whitelist_account!(stash); - }: _(RawOrigin::Signed(stash), new_controller_lookup) + }: _(RawOrigin::Signed(stash)) verify { - assert!(Ledger::::contains_key(&new_controller)); + assert!(Ledger::::contains_key(&stash)); } set_validator_count { diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 28d970b9121e7..c87aeb681a226 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -67,7 +67,7 @@ //! //! An account pair can become bonded using the [`bond`](Call::bond) call. //! -//! Stash accounts can change their associated controller using the +//! Stash accounts can update their associated controller back to the stash account using the //! [`set_controller`](Call::set_controller) call. //! //! There are three possible roles that any staked account pair can be in: `Validator`, `Nominator` diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index d8f1855da4bc0..7e827ba1a5648 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -1237,7 +1237,7 @@ pub mod pallet { Ok(()) } - /// (Re-)set the controller of a stash. + /// (Re-)sets the controller of a stash to the stash itself. /// /// Effects will be felt instantly (as soon as this function is completed successfully). /// @@ -1250,20 +1250,17 @@ pub mod pallet { /// - Writes are limited to the `origin` account key. #[pallet::call_index(8)] #[pallet::weight(T::WeightInfo::set_controller())] - pub fn set_controller( - origin: OriginFor, - controller: AccountIdLookupOf, - ) -> DispatchResult { + pub fn set_controller(origin: OriginFor) -> DispatchResult { let stash = ensure_signed(origin)?; let old_controller = Self::bonded(&stash).ok_or(Error::::NotStash)?; - let controller = T::Lookup::lookup(controller)?; - if >::contains_key(&controller) { + + if >::contains_key(&stash) { return Err(Error::::AlreadyPaired.into()) } - if controller != old_controller { - >::insert(&stash, &controller); + if old_controller != stash { + >::insert(&stash, &stash); if let Some(l) = >::take(&old_controller) { - >::insert(&controller, l); + >::insert(&stash, l); } } Ok(()) diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index 9bd231cce6c73..8410fa686713f 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -89,6 +89,26 @@ pub fn create_stash_controller( Ok((stash, controller)) } +/// Create a unique stash and controller pair. +pub fn create_stash_controller_inc( + n: u32, + balance_factor: u32, + destination: RewardDestination, +) -> Result<(T::AccountId, T::AccountId), &'static str> { + let stash = create_funded_user::("stash", n, balance_factor); + let controller = create_funded_user::("controller", n + 1, balance_factor); + + let controller_lookup = T::Lookup::unlookup(controller.clone()); + let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); + Staking::::bond( + RawOrigin::Signed(stash.clone()).into(), + controller_lookup, + amount, + destination, + )?; + Ok((stash, controller)) +} + /// Create a stash and controller pair with fixed balance. pub fn create_stash_controller_with_balance( n: u32, diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index affee60026500..a02e5441d2943 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -231,15 +231,15 @@ fn basic_setup_works() { #[test] fn change_controller_works() { ExtBuilder::default().build_and_execute(|| { - // 10 and 11 are bonded as stash controller. + // 10 and 11 are bonded as controller and stash respectively. assert_eq!(Staking::bonded(&11), Some(10)); // 10 can control 11 who is initially a validator. assert_ok!(Staking::chill(RuntimeOrigin::signed(10))); // change controller - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(11), 5)); - assert_eq!(Staking::bonded(&11), Some(5)); + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(11))); + assert_eq!(Staking::bonded(&11), Some(11)); mock::start_active_era(1); // 10 is no longer in control. @@ -247,7 +247,7 @@ fn change_controller_works() { Staking::validate(RuntimeOrigin::signed(10), ValidatorPrefs::default()), Error::::NotController, ); - assert_ok!(Staking::validate(RuntimeOrigin::signed(5), ValidatorPrefs::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(11), ValidatorPrefs::default())); }) } From 6f0be00892845351bb4e993703cf3da3e883e4b4 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Fri, 28 Apr 2023 16:11:45 +0700 Subject: [PATCH 02/60] clone --- frame/staking/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index a144bd0425dd8..d4bcca1d713c0 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -472,7 +472,7 @@ benchmarks! { set_controller { let (stash, _) = create_stash_controller_inc::(USER_SEED, 100, Default::default())?; whitelist_account!(stash); - }: _(RawOrigin::Signed(stash)) + }: _(RawOrigin::Signed(stash.clone())) verify { assert!(Ledger::::contains_key(&stash)); } From 75beff41d33d873972ac5bab95a7c85b414e937f Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 12:00:21 +0700 Subject: [PATCH 03/60] bond uses `stash` --- frame/staking/src/pallet/mod.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 7e827ba1a5648..4654216de546b 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -860,9 +860,7 @@ pub mod pallet { return Err(Error::::AlreadyBonded.into()) } - let controller = T::Lookup::lookup(controller)?; - - if >::contains_key(&controller) { + if >::contains_key(&stash) { return Err(Error::::AlreadyPaired.into()) } @@ -875,7 +873,7 @@ pub mod pallet { // You're auto-bonded forever, here. We might improve this by only bonding when // you actually validate/nominate and remove once you unbond __everything__. - >::insert(&stash, &controller); + >::insert(&stash, &stash); >::insert(&stash, payee); let current_era = CurrentEra::::get().unwrap_or(0); @@ -886,7 +884,7 @@ pub mod pallet { let value = value.min(stash_balance); Self::deposit_event(Event::::Bonded { stash: stash.clone(), amount: value }); let item = StakingLedger { - stash, + stash: stash.clone(), total: value, active: value, unlocking: Default::default(), @@ -897,7 +895,7 @@ pub mod pallet { // satisfied. .defensive_map_err(|_| Error::::BoundNotMet)?, }; - Self::update_ledger(&controller, &item); + Self::update_ledger(&stash, &item); Ok(()) } From 74d34eac5ace759c9bdc830d41d31612f731b392 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 16:50:49 +0700 Subject: [PATCH 04/60] remove controller from bond(), chill_other test works --- frame/fast-unstake/src/mock.rs | 1 - frame/fast-unstake/src/tests.rs | 2 +- frame/offences/benchmarking/src/lib.rs | 2 - frame/staking/src/mock.rs | 17 ++-- frame/staking/src/pallet/impls.rs | 1 - frame/staking/src/pallet/mod.rs | 2 - frame/staking/src/tests.rs | 103 ++++++++----------------- 7 files changed, 39 insertions(+), 89 deletions(-) diff --git a/frame/fast-unstake/src/mock.rs b/frame/fast-unstake/src/mock.rs index fbe6c4592bf67..efa089cb79c09 100644 --- a/frame/fast-unstake/src/mock.rs +++ b/frame/fast-unstake/src/mock.rs @@ -377,7 +377,6 @@ pub fn create_exposed_nominator(exposed: AccountId, era: u32) { Balances::make_free_balance_be(&exposed, 100); assert_ok!(Staking::bond( RuntimeOrigin::signed(exposed), - exposed, 10, pallet_staking::RewardDestination::Staked )); diff --git a/frame/fast-unstake/src/tests.rs b/frame/fast-unstake/src/tests.rs index b4bf1f1cb994a..3fa7b032ce12b 100644 --- a/frame/fast-unstake/src/tests.rs +++ b/frame/fast-unstake/src/tests.rs @@ -812,7 +812,7 @@ mod on_idle { // create a new validator that 100% not exposed. Balances::make_free_balance_be(&42, 100 + Deposit::get()); - assert_ok!(Staking::bond(RuntimeOrigin::signed(42), 42, 10, RewardDestination::Staked)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(42), 10, RewardDestination::Staked)); assert_ok!(Staking::validate(RuntimeOrigin::signed(42), Default::default())); // let them register: diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 894a725b5ce2f..97196f2d41460 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -116,7 +116,6 @@ fn create_offender(n: u32, nominators: u32) -> Result, &' T::Currency::make_free_balance_be(&stash, free_amount); Staking::::bond( RawOrigin::Signed(stash.clone()).into(), - controller_lookup.clone(), amount, reward_destination.clone(), )?; @@ -139,7 +138,6 @@ fn create_offender(n: u32, nominators: u32) -> Result, &' Staking::::bond( RawOrigin::Signed(nominator_stash.clone()).into(), - nominator_controller_lookup.clone(), amount, reward_destination.clone(), )?; diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index c2f559a9780eb..28dccf2605975 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -465,18 +465,18 @@ impl ExtBuilder { stakers = vec![ // (stash, ctrl, stake, status) // these two will be elected in the default test where we elect 2. - (11, 10, self.balance_factor * 1000, StakerStatus::::Validator), - (21, 20, self.balance_factor * 1000, StakerStatus::::Validator), + (11, 11, self.balance_factor * 1000, StakerStatus::::Validator), + (21, 21, self.balance_factor * 1000, StakerStatus::::Validator), // a loser validator - (31, 30, self.balance_factor * 500, StakerStatus::::Validator), + (31, 31, self.balance_factor * 500, StakerStatus::::Validator), // an idle validator - (41, 40, self.balance_factor * 1000, StakerStatus::::Idle), + (41, 41, self.balance_factor * 1000, StakerStatus::::Idle), ]; // optionally add a nominator if self.nominate { stakers.push(( 101, - 100, + 101, self.balance_factor * 500, StakerStatus::::Nominator(vec![11, 21]), )) @@ -566,12 +566,7 @@ pub(crate) fn current_era() -> EraIndex { pub(crate) fn bond(stash: AccountId, ctrl: AccountId, val: Balance) { let _ = Balances::make_free_balance_be(&stash, val); let _ = Balances::make_free_balance_be(&ctrl, val); - assert_ok!(Staking::bond( - RuntimeOrigin::signed(stash), - ctrl, - val, - RewardDestination::Controller - )); + assert_ok!(Staking::bond(RuntimeOrigin::signed(stash), val, RewardDestination::Controller)); } pub(crate) fn bond_validator(stash: AccountId, ctrl: AccountId, val: Balance) { diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 984871f7f9813..1d9838572857d 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -1651,7 +1651,6 @@ impl StakingInterface for Pallet { ) -> DispatchResult { Self::bond( RawOrigin::Signed(who.clone()).into(), - T::Lookup::unlookup(who.clone()), value, RewardDestination::Account(payee.clone()), ) diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 4654216de546b..6f4fba63d03a4 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -645,7 +645,6 @@ pub mod pallet { ); frame_support::assert_ok!(>::bond( T::RuntimeOrigin::from(Some(stash.clone()).into()), - T::Lookup::unlookup(controller.clone()), balance, RewardDestination::Staked, )); @@ -850,7 +849,6 @@ pub mod pallet { #[pallet::weight(T::WeightInfo::bond())] pub fn bond( origin: OriginFor, - controller: AccountIdLookupOf, #[pallet::compact] value: BalanceOf, payee: RewardDestination, ) -> DispatchResult { diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index a02e5441d2943..5f4b916e8878b 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -390,7 +390,7 @@ fn staking_should_work() { // --- Block 2: start_session(2); // add a new candidate for being a validator. account 3 controlled by 4. - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 4, 1500, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, RewardDestination::Controller)); assert_ok!(Staking::validate(RuntimeOrigin::signed(4), ValidatorPrefs::default())); assert_ok!(Session::set_keys( RuntimeOrigin::signed(4), @@ -584,7 +584,6 @@ fn nominating_and_rewards_should_work() { // bond two account pairs and state interest in nomination. assert_ok!(Staking::bond( RuntimeOrigin::signed(1), - 2, 1000, RewardDestination::Controller )); @@ -592,7 +591,6 @@ fn nominating_and_rewards_should_work() { assert_ok!(Staking::bond( RuntimeOrigin::signed(3), - 4, 1000, RewardDestination::Controller )); @@ -749,18 +747,12 @@ fn double_staking_should_fail() { // 2 = controller, 1 stashed => ok assert_ok!(Staking::bond( RuntimeOrigin::signed(1), - 2, arbitrary_value, RewardDestination::default() )); // 4 = not used so far, 1 stashed => not allowed. assert_noop!( - Staking::bond( - RuntimeOrigin::signed(1), - 4, - arbitrary_value, - RewardDestination::default() - ), + Staking::bond(RuntimeOrigin::signed(1), arbitrary_value, RewardDestination::default()), Error::::AlreadyBonded, ); // 1 = stashed => attempting to nominate should fail. @@ -783,18 +775,12 @@ fn double_controlling_should_fail() { // 2 = controller, 1 stashed => ok assert_ok!(Staking::bond( RuntimeOrigin::signed(1), - 2, arbitrary_value, RewardDestination::default(), )); // 2 = controller, 3 stashed (Note that 2 is reused.) => no-op assert_noop!( - Staking::bond( - RuntimeOrigin::signed(3), - 2, - arbitrary_value, - RewardDestination::default() - ), + Staking::bond(RuntimeOrigin::signed(3), arbitrary_value, RewardDestination::default()), Error::::AlreadyPaired, ); }); @@ -1833,14 +1819,14 @@ fn switching_roles() { } // add 2 nominators - assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 2, 2000, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 2000, RewardDestination::Controller)); assert_ok!(Staking::nominate(RuntimeOrigin::signed(2), vec![11, 5])); - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 4, 500, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 500, RewardDestination::Controller)); assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![21, 1])); // add a new validator candidate - assert_ok!(Staking::bond(RuntimeOrigin::signed(5), 6, 1000, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(5), 1000, RewardDestination::Controller)); assert_ok!(Staking::validate(RuntimeOrigin::signed(6), ValidatorPrefs::default())); assert_ok!(Session::set_keys( RuntimeOrigin::signed(6), @@ -1911,16 +1897,11 @@ fn bond_with_no_staked_value() { .build_and_execute(|| { // Can't bond with 1 assert_noop!( - Staking::bond(RuntimeOrigin::signed(1), 2, 1, RewardDestination::Controller), + Staking::bond(RuntimeOrigin::signed(1), 1, RewardDestination::Controller), Error::::InsufficientBond, ); // bonded with absolute minimum value possible. - assert_ok!(Staking::bond( - RuntimeOrigin::signed(1), - 2, - 5, - RewardDestination::Controller - )); + assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 5, RewardDestination::Controller)); assert_eq!(Balances::locks(&1)[0].amount, 5); // unbonding even 1 will cause all to be unbonded. @@ -1970,12 +1951,7 @@ fn bond_with_little_staked_value_bounded() { let init_balance_10 = Balances::free_balance(&10); // Stingy validator. - assert_ok!(Staking::bond( - RuntimeOrigin::signed(1), - 2, - 1, - RewardDestination::Controller - )); + assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 1, RewardDestination::Controller)); assert_ok!(Staking::validate(RuntimeOrigin::signed(2), ValidatorPrefs::default())); assert_ok!(Session::set_keys( RuntimeOrigin::signed(2), @@ -2053,7 +2029,6 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider() { assert_ok!(Staking::bond( RuntimeOrigin::signed(1), - 2, 1000, RewardDestination::Controller )); @@ -2061,7 +2036,6 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider() { assert_ok!(Staking::bond( RuntimeOrigin::signed(3), - 4, 1000, RewardDestination::Controller )); @@ -2108,7 +2082,6 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() { assert_ok!(Staking::bond( RuntimeOrigin::signed(1), - 2, 1000, RewardDestination::Controller )); @@ -2116,7 +2089,6 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() { assert_ok!(Staking::bond( RuntimeOrigin::signed(3), - 4, 1000, RewardDestination::Controller )); @@ -2202,8 +2174,7 @@ fn reward_validator_slashing_validator_does_not_overflow() { let _ = Balances::make_free_balance_be(&2, stake); // only slashes out of bonded stake are applied. without this line, it is 0. - Staking::bond(RuntimeOrigin::signed(2), 20000, stake - 1, RewardDestination::default()) - .unwrap(); + Staking::bond(RuntimeOrigin::signed(2), stake - 1, RewardDestination::default()).unwrap(); // Override exposure of 11 ErasStakers::::insert( 0, @@ -3688,7 +3659,6 @@ fn test_max_nominator_rewarded_per_validator_and_cant_steal_someone_else_reward( Balances::make_free_balance_be(&stash, balance); assert_ok!(Staking::bond( RuntimeOrigin::signed(stash), - controller, balance, RewardDestination::Stash )); @@ -4707,12 +4677,7 @@ fn min_bond_checks_work() { .min_validator_bond(1_500) .build_and_execute(|| { // 500 is not enough for any role - assert_ok!(Staking::bond( - RuntimeOrigin::signed(3), - 4, - 500, - RewardDestination::Controller - )); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 500, RewardDestination::Controller)); assert_noop!( Staking::nominate(RuntimeOrigin::signed(4), vec![1]), Error::::InsufficientBond @@ -4767,31 +4732,27 @@ fn chill_other_works() { let initial_nominators = Nominators::::count(); for i in 0..15 { let a = 4 * i; - let b = 4 * i + 1; - let c = 4 * i + 2; - let d = 4 * i + 3; + let b = 4 * i + 2; + let c = 4 * i + 3; Balances::make_free_balance_be(&a, 100_000); Balances::make_free_balance_be(&b, 100_000); Balances::make_free_balance_be(&c, 100_000); - Balances::make_free_balance_be(&d, 100_000); // Nominator assert_ok!(Staking::bond( RuntimeOrigin::signed(a), - b, 1000, RewardDestination::Controller )); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(b), vec![1])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(a), vec![1])); // Validator assert_ok!(Staking::bond( - RuntimeOrigin::signed(c), - d, + RuntimeOrigin::signed(b), 1500, RewardDestination::Controller )); - assert_ok!(Staking::validate(RuntimeOrigin::signed(d), ValidatorPrefs::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(b), ValidatorPrefs::default())); } // To chill other users, we need to: @@ -4804,11 +4765,11 @@ fn chill_other_works() { // Can't chill these users assert_noop!( - Staking::chill_other(RuntimeOrigin::signed(1337), 1), + Staking::chill_other(RuntimeOrigin::signed(1337), 0), Error::::CannotChillOther ); assert_noop!( - Staking::chill_other(RuntimeOrigin::signed(1337), 3), + Staking::chill_other(RuntimeOrigin::signed(1337), 2), Error::::CannotChillOther ); @@ -4825,11 +4786,11 @@ fn chill_other_works() { // Still can't chill these users assert_noop!( - Staking::chill_other(RuntimeOrigin::signed(1337), 1), + Staking::chill_other(RuntimeOrigin::signed(1337), 0), Error::::CannotChillOther ); assert_noop!( - Staking::chill_other(RuntimeOrigin::signed(1337), 3), + Staking::chill_other(RuntimeOrigin::signed(1337), 2), Error::::CannotChillOther ); @@ -4846,11 +4807,11 @@ fn chill_other_works() { // Still can't chill these users assert_noop!( - Staking::chill_other(RuntimeOrigin::signed(1337), 1), + Staking::chill_other(RuntimeOrigin::signed(1337), 0), Error::::CannotChillOther ); assert_noop!( - Staking::chill_other(RuntimeOrigin::signed(1337), 3), + Staking::chill_other(RuntimeOrigin::signed(1337), 2), Error::::CannotChillOther ); @@ -4867,11 +4828,11 @@ fn chill_other_works() { // Still can't chill these users assert_noop!( - Staking::chill_other(RuntimeOrigin::signed(1337), 1), + Staking::chill_other(RuntimeOrigin::signed(1337), 0), Error::::CannotChillOther ); assert_noop!( - Staking::chill_other(RuntimeOrigin::signed(1337), 3), + Staking::chill_other(RuntimeOrigin::signed(1337), 2), Error::::CannotChillOther ); @@ -4893,8 +4854,8 @@ fn chill_other_works() { // Users can now be chilled down to 7 people, so we try to remove 9 of them (starting // with 16) for i in 6..15 { - let b = 4 * i + 1; - let d = 4 * i + 3; + let b = 4 * i; + let d = 4 * i + 2; assert_ok!(Staking::chill_other(RuntimeOrigin::signed(1337), b)); assert_ok!(Staking::chill_other(RuntimeOrigin::signed(1337), d)); } @@ -4902,12 +4863,12 @@ fn chill_other_works() { // chill a nominator. Limit is not reached, not chill-able assert_eq!(Nominators::::count(), 7); assert_noop!( - Staking::chill_other(RuntimeOrigin::signed(1337), 1), + Staking::chill_other(RuntimeOrigin::signed(1337), 0), Error::::CannotChillOther ); // chill a validator. Limit is reached, chill-able. assert_eq!(Validators::::count(), 9); - assert_ok!(Staking::chill_other(RuntimeOrigin::signed(1337), 3)); + assert_ok!(Staking::chill_other(RuntimeOrigin::signed(1337), 2)); }) } @@ -5504,7 +5465,7 @@ fn pre_bonding_era_cannot_be_claimed() { mock::start_active_era(current_era); // add a new candidate for being a validator. account 3 controlled by 4. - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 4, 1500, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, RewardDestination::Controller)); let claimed_rewards: BoundedVec<_, _> = (start_reward_era..=last_reward_era).collect::>().try_into().unwrap(); @@ -5568,7 +5529,7 @@ fn reducing_history_depth_abrupt() { mock::start_active_era(current_era); // add a new candidate for being a staker. account 3 controlled by 4. - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 4, 1500, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, RewardDestination::Controller)); // all previous era before the bonding action should be marked as // claimed. @@ -5606,7 +5567,7 @@ fn reducing_history_depth_abrupt() { ); // new stakers can still bond - assert_ok!(Staking::bond(RuntimeOrigin::signed(5), 6, 1200, RewardDestination::Controller)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(5), 1200, RewardDestination::Controller)); // new staking ledgers created will be bounded by the current history depth let last_reward_era = current_era - 1; @@ -5637,7 +5598,7 @@ fn reducing_max_unlocking_chunks_abrupt() { // given a staker at era=10 and MaxUnlockChunks set to 2 MaxUnlockingChunks::set(2); start_active_era(10); - assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 4, 300, RewardDestination::Staked)); + assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 300, RewardDestination::Staked)); assert!(matches!(Staking::ledger(4), Some(_))); // when staker unbonds From fc11538fc20dc6c8e8393300586faffef42e55a2 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 16:53:46 +0700 Subject: [PATCH 05/60] remove ctlr from testing_utils & dead ctlr -> dead payee --- frame/session/benchmarking/src/lib.rs | 6 +-- frame/staking/src/benchmarking.rs | 15 +++---- frame/staking/src/testing_utils.rs | 62 ++++++++------------------- 3 files changed, 25 insertions(+), 58 deletions(-) diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs index 7f64dc70f35d5..f06b0e2e4e908 100644 --- a/frame/session/benchmarking/src/lib.rs +++ b/frame/session/benchmarking/src/lib.rs @@ -58,8 +58,7 @@ benchmarks! { let (v_stash, _) = create_validator_with_nominators::( n, ::MaxNominations::get(), - false, - RewardDestination::Staked, + false )?; let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; @@ -75,8 +74,7 @@ benchmarks! { let (v_stash, _) = create_validator_with_nominators::( n, ::MaxNominations::get(), - false, - RewardDestination::Staked + false )?; let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; let keys = T::Keys::decode(&mut TrailingZeroInput::zeroes()).unwrap(); diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index d4bcca1d713c0..c78c9bd37a821 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -72,14 +72,13 @@ pub fn create_validator_with_nominators( n: u32, upper_bound: u32, dead: bool, - destination: RewardDestination, ) -> Result<(T::AccountId, Vec<(T::AccountId, T::AccountId)>), &'static str> { // Clean up any existing state. clear_validators_and_nominators::(); let mut points_total = 0; let mut points_individual = Vec::new(); - let (v_stash, v_controller) = create_stash_controller::(0, 100, destination.clone())?; + let (v_stash, v_controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; Staking::::validate(RawOrigin::Signed(v_controller).into(), validator_prefs)?; @@ -94,9 +93,9 @@ pub fn create_validator_with_nominators( // Give the validator n nominators, but keep total users in the system the same. for i in 0..upper_bound { let (n_stash, n_controller) = if !dead { - create_stash_controller::(u32::MAX - i, 100, destination.clone())? + create_stash_controller::(u32::MAX - i, 100, RewardDestination::Staked)? } else { - create_stash_and_dead_controller::(u32::MAX - i, 100, destination.clone())? + create_stash_and_dead_payee::(u32::MAX - i, 100)? }; if i < n { Staking::::nominate( @@ -548,8 +547,7 @@ benchmarks! { let (validator, nominators) = create_validator_with_nominators::( n, T::MaxNominatorRewardedPerValidator::get() as u32, - true, - RewardDestination::Controller, + true )?; let current_era = CurrentEra::::get().unwrap(); @@ -581,8 +579,7 @@ benchmarks! { let (validator, nominators) = create_validator_with_nominators::( n, T::MaxNominatorRewardedPerValidator::get() as u32, - false, - RewardDestination::Staked, + false )?; let current_era = CurrentEra::::get().unwrap(); @@ -976,7 +973,6 @@ mod tests { n, <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, - RewardDestination::Staked, ) .unwrap(); @@ -1005,7 +1001,6 @@ mod tests { n, <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, - RewardDestination::Staked, ) .unwrap(); diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index 8410fa686713f..df73934c1e4f7 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -76,17 +76,10 @@ pub fn create_stash_controller( balance_factor: u32, destination: RewardDestination, ) -> Result<(T::AccountId, T::AccountId), &'static str> { - let stash = create_funded_user::("stash", n, balance_factor); - let controller = create_funded_user::("controller", n, balance_factor); - let controller_lookup = T::Lookup::unlookup(controller.clone()); + let staker = create_funded_user::("stash", n, balance_factor); let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); - Staking::::bond( - RawOrigin::Signed(stash.clone()).into(), - controller_lookup, - amount, - destination, - )?; - Ok((stash, controller)) + Staking::::bond(RawOrigin::Signed(staker.clone()).into(), amount, destination)?; + Ok((staker.clone(), staker)) } /// Create a unique stash and controller pair. @@ -95,18 +88,10 @@ pub fn create_stash_controller_inc( balance_factor: u32, destination: RewardDestination, ) -> Result<(T::AccountId, T::AccountId), &'static str> { - let stash = create_funded_user::("stash", n, balance_factor); - let controller = create_funded_user::("controller", n + 1, balance_factor); - - let controller_lookup = T::Lookup::unlookup(controller.clone()); + let staker = create_funded_user::("stash", n, balance_factor); let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); - Staking::::bond( - RawOrigin::Signed(stash.clone()).into(), - controller_lookup, - amount, - destination, - )?; - Ok((stash, controller)) + Staking::::bond(RawOrigin::Signed(staker.clone()).into(), amount, destination)?; + Ok((staker.clone(), staker)) } /// Create a stash and controller pair with fixed balance. @@ -115,38 +100,27 @@ pub fn create_stash_controller_with_balance( balance: crate::BalanceOf, destination: RewardDestination, ) -> Result<(T::AccountId, T::AccountId), &'static str> { - let stash = create_funded_user_with_balance::("stash", n, balance); - let controller = create_funded_user_with_balance::("controller", n, balance); - let controller_lookup = T::Lookup::unlookup(controller.clone()); - - Staking::::bond( - RawOrigin::Signed(stash.clone()).into(), - controller_lookup, - balance, - destination, - )?; - Ok((stash, controller)) + let staker = create_funded_user_with_balance::("stash", n, balance); + Staking::::bond(RawOrigin::Signed(staker.clone()).into(), balance, destination)?; + Ok((staker.clone(), staker)) } -/// Create a stash and controller pair, where the controller is dead, and payouts go to controller. -/// This is used to test worst case payout scenarios. -pub fn create_stash_and_dead_controller( +/// Create a stash and controller pair, where payouts go to a dead payee account. This is used to +/// test worst case payout scenarios. +pub fn create_stash_and_dead_payee( n: u32, balance_factor: u32, - destination: RewardDestination, ) -> Result<(T::AccountId, T::AccountId), &'static str> { - let stash = create_funded_user::("stash", n, balance_factor); - // controller has no funds - let controller = create_funded_user::("controller", n, 0); - let controller_lookup = T::Lookup::unlookup(controller.clone()); + let staker = create_funded_user::("stash", n, 0); + // payee has no funds + let payee = create_funded_user::("payee", n, 0); let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); Staking::::bond( - RawOrigin::Signed(stash.clone()).into(), - controller_lookup, + RawOrigin::Signed(staker.clone()).into(), amount, - destination, + RewardDestination::Account(payee), )?; - Ok((stash, controller)) + Ok((staker.clone(), staker)) } /// create `max` validators. From 35d873c13e9bf60f8ac2accfdb2ad5e76f833100 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 17:02:07 +0700 Subject: [PATCH 06/60] mvs controllers to stashes for 3 tests --- frame/staking/src/tests.rs | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 5f4b916e8878b..f17aac203d094 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -139,16 +139,16 @@ fn kill_stash_works() { fn basic_setup_works() { // Verifies initial conditions of mock ExtBuilder::default().build_and_execute(|| { - // Account 11 is stashed and locked, and account 10 is the controller - assert_eq!(Staking::bonded(&11), Some(10)); - // Account 21 is stashed and locked, and account 20 is the controller - assert_eq!(Staking::bonded(&21), Some(20)); + // Account 11 is stashed and locked, and is the controller + assert_eq!(Staking::bonded(&11), Some(11)); + // Account 21 is stashed and locked and is the controller + assert_eq!(Staking::bonded(&21), Some(21)); // Account 1 is not a stashed assert_eq!(Staking::bonded(&1), None); - // Account 10 controls the stash from account 11, which is 100 * balance_factor units + // Account 11 controls its own stash, which is 100 * balance_factor units assert_eq!( - Staking::ledger(&10).unwrap(), + Staking::ledger(&11).unwrap(), StakingLedger { stash: 11, total: 1000, @@ -157,9 +157,9 @@ fn basic_setup_works() { claimed_rewards: bounded_vec![], } ); - // Account 20 controls the stash from account 21, which is 200 * balance_factor units + // Account 21 controls its own stash, which is 200 * balance_factor units assert_eq!( - Staking::ledger(&20), + Staking::ledger(&21), Some(StakingLedger { stash: 21, total: 1000, @@ -182,7 +182,7 @@ fn basic_setup_works() { ); assert_eq!( - Staking::ledger(100), + Staking::ledger(101), Some(StakingLedger { stash: 101, total: 500, @@ -459,20 +459,20 @@ fn blocking_and_kicking_works() { .build_and_execute(|| { // block validator 10/11 assert_ok!(Staking::validate( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(11), ValidatorPrefs { blocked: true, ..Default::default() } )); // attempt to nominate from 100/101... - assert_ok!(Staking::nominate(RuntimeOrigin::signed(100), vec![11])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(101), vec![11])); // should have worked since we're already nominated them assert_eq!(Nominators::::get(&101).unwrap().targets, vec![11]); // kick the nominator - assert_ok!(Staking::kick(RuntimeOrigin::signed(10), vec![101])); + assert_ok!(Staking::kick(RuntimeOrigin::signed(11), vec![101])); // should have been kicked now assert!(Nominators::::get(&101).unwrap().targets.is_empty()); // attempt to nominate from 100/101... assert_noop!( - Staking::nominate(RuntimeOrigin::signed(100), vec![11]), + Staking::nominate(RuntimeOrigin::signed(101), vec![11]), Error::::BadTarget ); }); @@ -1390,7 +1390,7 @@ fn auto_withdraw_may_not_unlock_all_chunks() { // fills the chunking slots for account mock::start_active_era(current_era); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 1)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1)); current_era += 1; mock::start_active_era(current_era); @@ -1398,12 +1398,12 @@ fn auto_withdraw_may_not_unlock_all_chunks() { // unbonding will fail because i) there are no remaining chunks and ii) no filled chunks // can be released because current chunk hasn't stay in the queue for at least // `BondingDuration` - assert_noop!(Staking::unbond(RuntimeOrigin::signed(10), 1), Error::::NoMoreChunks); + assert_noop!(Staking::unbond(RuntimeOrigin::signed(11), 1), Error::::NoMoreChunks); // fast-forward a few eras for unbond to be successful with implicit withdraw current_era += 10; mock::start_active_era(current_era); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 1)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1)); }) } From 6e73503bd3b27de41f8eccb4a1c19cdb7ddaa244 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 17:56:10 +0700 Subject: [PATCH 07/60] migrate mock bond fns & fix 1 test --- frame/staking/src/mock.rs | 24 ++++++++++------------ frame/staking/src/tests.rs | 42 +++++++++++++++++++------------------- 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 28dccf2605975..ec3a3b79d6cdc 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -563,30 +563,28 @@ pub(crate) fn current_era() -> EraIndex { Staking::current_era().unwrap() } -pub(crate) fn bond(stash: AccountId, ctrl: AccountId, val: Balance) { - let _ = Balances::make_free_balance_be(&stash, val); - let _ = Balances::make_free_balance_be(&ctrl, val); - assert_ok!(Staking::bond(RuntimeOrigin::signed(stash), val, RewardDestination::Controller)); +pub(crate) fn bond(who: AccountId, val: Balance) { + let _ = Balances::make_free_balance_be(&who, val); + assert_ok!(Staking::bond(RuntimeOrigin::signed(who), val, RewardDestination::Controller)); } -pub(crate) fn bond_validator(stash: AccountId, ctrl: AccountId, val: Balance) { - bond(stash, ctrl, val); - assert_ok!(Staking::validate(RuntimeOrigin::signed(ctrl), ValidatorPrefs::default())); +pub(crate) fn bond_validator(who: AccountId, val: Balance) { + bond(who, val); + assert_ok!(Staking::validate(RuntimeOrigin::signed(who), ValidatorPrefs::default())); assert_ok!(Session::set_keys( - RuntimeOrigin::signed(ctrl), - SessionKeys { other: ctrl.into() }, + RuntimeOrigin::signed(who), + SessionKeys { other: who.into() }, vec![] )); } pub(crate) fn bond_nominator( - stash: AccountId, - ctrl: AccountId, + who: AccountId, val: Balance, target: Vec, ) { - bond(stash, ctrl, val); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(ctrl), target)); + bond(who, val); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(who), target)); } /// Progress to the given block, triggering session and era changes as we progress. diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index f17aac203d094..6510a765b49ef 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -2127,11 +2127,11 @@ fn phragmen_should_not_overflow() { let _ = Staking::chill(RuntimeOrigin::signed(10)); let _ = Staking::chill(RuntimeOrigin::signed(20)); - bond_validator(3, 2, Votes::max_value() as Balance); - bond_validator(5, 4, Votes::max_value() as Balance); + bond_validator(3, Votes::max_value() as Balance); + bond_validator(5, Votes::max_value() as Balance); - bond_nominator(7, 6, Votes::max_value() as Balance, vec![3, 5]); - bond_nominator(9, 8, Votes::max_value() as Balance, vec![3, 5]); + bond_nominator(7, Votes::max_value() as Balance, vec![3, 5]); + bond_nominator(9, Votes::max_value() as Balance, vec![3, 5]); mock::start_active_era(1); @@ -3697,13 +3697,13 @@ fn test_payout_stakers() { // Track the exposure of the validator and the nominators that will get paid out. let mut payout_exposure = balance; // Create a validator: - bond_validator(11, 10, balance); // Default(64) + bond_validator(11, balance); // Default(64) assert_eq!(Validators::::count(), 1); // Create nominators, targeting stash of validators for i in 0..100 { let bond_amount = balance + i as Balance; - bond_nominator(1000 + i, 100 + i, bond_amount, vec![11]); + bond_nominator(1000 + i, bond_amount, vec![11]); total_exposure += bond_amount; if i >= 36 { payout_exposure += bond_amount; @@ -3849,11 +3849,11 @@ fn payout_stakers_handles_basic_errors() { // Same setup as the test above let balance = 1000; - bond_validator(11, 10, balance); // Default(64) + bond_validator(11, balance); // Default(64) // Create nominators, targeting stash for i in 0..100 { - bond_nominator(1000 + i, 100 + i, balance + i as Balance, vec![11]); + bond_nominator(1000 + i, balance + i as Balance, vec![11]); } mock::start_active_era(1); @@ -3945,7 +3945,7 @@ fn payout_stakers_handles_weight_refund() { assert!(max_nom_rewarded_weight.any_gt(half_max_nom_rewarded_weight)); let balance = 1000; - bond_validator(11, 10, balance); + bond_validator(11, balance); // Era 1 start_active_era(1); @@ -3956,7 +3956,7 @@ fn payout_stakers_handles_weight_refund() { // Add some `half_max_nom_rewarded` nominators who will start backing the validator in the // next era. for i in 0..half_max_nom_rewarded { - bond_nominator((1000 + i).into(), (100 + i).into(), balance + i as Balance, vec![11]); + bond_nominator((1000 + i).into(), balance + i as Balance, vec![11]); } // Era 2 @@ -3998,7 +3998,7 @@ fn payout_stakers_handles_weight_refund() { // Add enough nominators so that we are at the limit. They will be active nominators // in the next era. for i in half_max_nom_rewarded..max_nom_rewarded { - bond_nominator((1000 + i).into(), (100 + i).into(), balance + i as Balance, vec![11]); + bond_nominator((1000 + i).into(), balance + i as Balance, vec![11]); } // Era 5 @@ -4032,9 +4032,9 @@ fn payout_stakers_handles_weight_refund() { fn bond_during_era_correctly_populates_claimed_rewards() { ExtBuilder::default().has_stakers(false).build_and_execute(|| { // Era = None - bond_validator(9, 8, 1000); + bond_validator(9, 1000); assert_eq!( - Staking::ledger(&8), + Staking::ledger(&9), Some(StakingLedger { stash: 9, total: 1000, @@ -4044,9 +4044,9 @@ fn bond_during_era_correctly_populates_claimed_rewards() { }) ); mock::start_active_era(5); - bond_validator(11, 10, 1000); + bond_validator(11, 1000); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -4060,9 +4060,9 @@ fn bond_during_era_correctly_populates_claimed_rewards() { let current_era = 99; let last_reward_era = 99 - HistoryDepth::get(); mock::start_active_era(current_era); - bond_validator(13, 12, 1000); + bond_validator(13, 1000); assert_eq!( - Staking::ledger(&12), + Staking::ledger(&13), Some(StakingLedger { stash: 13, total: 1000, @@ -4151,10 +4151,10 @@ fn payout_creates_controller() { ExtBuilder::default().has_stakers(false).build_and_execute(|| { let balance = 1000; // Create a validator: - bond_validator(11, 10, balance); + bond_validator(11, balance); // Create a stash/controller pair - bond_nominator(1234, 1337, 100, vec![11]); + bond_nominator(1234, 100, vec![11]); // kill controller assert_ok!(Balances::transfer_allow_death(RuntimeOrigin::signed(1337), 1234, 100)); @@ -4177,10 +4177,10 @@ fn payout_to_any_account_works() { ExtBuilder::default().has_stakers(false).build_and_execute(|| { let balance = 1000; // Create a validator: - bond_validator(11, 10, balance); // Default(64) + bond_validator(11, balance); // Default(64) // Create a stash/controller pair - bond_nominator(1234, 1337, 100, vec![11]); + bond_nominator(1234, 100, vec![11]); // Update payout location assert_ok!(Staking::set_payee(RuntimeOrigin::signed(1337), RewardDestination::Account(42))); From 0379c652a2f83e9490d7f943172bf6a11419d446 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 18:11:42 +0700 Subject: [PATCH 08/60] mvs controllers to stashes for 7 tests --- frame/staking/src/tests.rs | 112 ++++++++++++++++++------------------- 1 file changed, 56 insertions(+), 56 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 6510a765b49ef..c279b1245f274 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -1149,10 +1149,10 @@ fn bond_extra_works() { // Check that account 10 is a validator assert!(>::contains_key(11)); // Check that account 10 is bonded to account 11 - assert_eq!(Staking::bonded(&11), Some(10)); + assert_eq!(Staking::bonded(&11), Some(11)); // Check how much is at stake assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1169,7 +1169,7 @@ fn bond_extra_works() { assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(11), 100)); // There should be 100 more `total` and `active` in the ledger assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000 + 100, @@ -1183,7 +1183,7 @@ fn bond_extra_works() { assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(11), Balance::max_value())); // The full amount of the funds should now be in the total and active assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000000, @@ -1205,7 +1205,7 @@ fn bond_extra_and_withdraw_unbonded_works() { // * Once the unbonding period is done, it can actually take the funds out of the stash. ExtBuilder::default().nominate(false).build_and_execute(|| { // Set payee to controller. avoids confusion - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(10), RewardDestination::Controller)); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller)); // Give account 11 some large free balance greater than total let _ = Balances::make_free_balance_be(&11, 1000000); @@ -1214,14 +1214,14 @@ fn bond_extra_and_withdraw_unbonded_works() { assert_eq!(active_era(), 0); // check the balance of a validator accounts. - assert_eq!(Balances::total_balance(&10), 1); + assert_eq!(Balances::total_balance(&11), 1000000); // confirm that 10 is a normal validator and gets paid at the end of the era. mock::start_active_era(1); - // Initial state of 10 + // Initial state of 11 assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1239,7 +1239,7 @@ fn bond_extra_and_withdraw_unbonded_works() { Staking::bond_extra(RuntimeOrigin::signed(11), 100).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000 + 100, @@ -1260,7 +1260,7 @@ fn bond_extra_and_withdraw_unbonded_works() { // ledger should be the same. assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000 + 100, @@ -1276,9 +1276,9 @@ fn bond_extra_and_withdraw_unbonded_works() { ); // Unbond almost all of the funds in stash. - Staking::unbond(RuntimeOrigin::signed(10), 1000).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 1000).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000 + 100, @@ -1289,9 +1289,9 @@ fn bond_extra_and_withdraw_unbonded_works() { ); // Attempting to free the balances now will fail. 2 eras need to pass. - assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(10), 0)); + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0)); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000 + 100, @@ -1305,9 +1305,9 @@ fn bond_extra_and_withdraw_unbonded_works() { mock::start_active_era(3); // nothing yet - assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(10), 0)); + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0)); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000 + 100, @@ -1320,10 +1320,10 @@ fn bond_extra_and_withdraw_unbonded_works() { // trigger next era. mock::start_active_era(5); - assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(10), 0)); + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(11), 0)); // Now the value is free and the staking ledger is updated. assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 100, @@ -1905,9 +1905,9 @@ fn bond_with_no_staked_value() { assert_eq!(Balances::locks(&1)[0].amount, 5); // unbonding even 1 will cause all to be unbonded. - assert_ok!(Staking::unbond(RuntimeOrigin::signed(2), 1)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(1), 1)); assert_eq!( - Staking::ledger(2), + Staking::ledger(1), Some(StakingLedger { stash: 1, active: 0, @@ -1921,15 +1921,15 @@ fn bond_with_no_staked_value() { mock::start_active_era(2); // not yet removed. - assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(2), 0)); - assert!(Staking::ledger(2).is_some()); + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(1), 0)); + assert!(Staking::ledger(1).is_some()); assert_eq!(Balances::locks(&1)[0].amount, 5); mock::start_active_era(3); // poof. Account 1 is removed from the staking system. - assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(2), 0)); - assert!(Staking::ledger(2).is_none()); + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(1), 0)); + assert!(Staking::ledger(1).is_none()); assert_eq!(Balances::locks(&1).len(), 0); }); } @@ -1942,20 +1942,20 @@ fn bond_with_little_staked_value_bounded() { .minimum_validator_count(1) .build_and_execute(|| { // setup - assert_ok!(Staking::chill(RuntimeOrigin::signed(30))); + assert_ok!(Staking::chill(RuntimeOrigin::signed(31))); assert_ok!(Staking::set_payee( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(11), RewardDestination::Controller )); - let init_balance_2 = Balances::free_balance(&2); - let init_balance_10 = Balances::free_balance(&10); + let init_balance_1 = Balances::free_balance(&1); + let init_balance_11 = Balances::free_balance(&11); // Stingy validator. assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 1, RewardDestination::Controller)); - assert_ok!(Staking::validate(RuntimeOrigin::signed(2), ValidatorPrefs::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(1), ValidatorPrefs::default())); assert_ok!(Session::set_keys( - RuntimeOrigin::signed(2), - SessionKeys { other: 2.into() }, + RuntimeOrigin::signed(1), + SessionKeys { other: 1.into() }, vec![] )); @@ -1968,17 +1968,17 @@ fn bond_with_little_staked_value_bounded() { mock::make_all_reward_payment(0); // 2 is elected. - assert_eq_uvec!(validator_controllers(), vec![20, 10, 2]); + assert_eq_uvec!(validator_controllers(), vec![21, 11, 1]); assert_eq!(Staking::eras_stakers(active_era(), 2).total, 0); // Old ones are rewarded. assert_eq_error_rate!( - Balances::free_balance(10), - init_balance_10 + total_payout_0 / 3, + Balances::free_balance(11), + init_balance_11 + total_payout_0 / 3, 1 ); // no rewards paid to 2. This was initial election. - assert_eq!(Balances::free_balance(2), init_balance_2); + assert_eq!(Balances::free_balance(1), init_balance_1); // reward era 2 let total_payout_1 = current_total_payout_for_duration(reward_time_per_era()); @@ -1986,18 +1986,18 @@ fn bond_with_little_staked_value_bounded() { mock::start_active_era(2); mock::make_all_reward_payment(1); - assert_eq_uvec!(validator_controllers(), vec![20, 10, 2]); + assert_eq_uvec!(validator_controllers(), vec![21, 11, 1]); assert_eq!(Staking::eras_stakers(active_era(), 2).total, 0); // 2 is now rewarded. assert_eq_error_rate!( - Balances::free_balance(2), - init_balance_2 + total_payout_1 / 3, + Balances::free_balance(1), + init_balance_1 + total_payout_1 / 3, 1 ); assert_eq_error_rate!( - Balances::free_balance(&10), - init_balance_10 + total_payout_0 / 3 + total_payout_1 / 3, + Balances::free_balance(&11), + init_balance_11 + total_payout_0 / 3 + total_payout_1 / 3, 2, ); }); @@ -2014,7 +2014,7 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider() { // ensure all have equal stake. assert_eq!( >::iter() - .map(|(v, _)| (v, Staking::ledger(v - 1).unwrap().total)) + .map(|(v, _)| (v, Staking::ledger(v).unwrap().total)) .collect::>(), vec![(31, 1000), (21, 1000), (11, 1000)], ); @@ -2032,14 +2032,14 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider() { 1000, RewardDestination::Controller )); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(2), vec![11, 11, 11, 21, 31])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(1), vec![11, 11, 11, 21, 31])); assert_ok!(Staking::bond( RuntimeOrigin::signed(3), 1000, RewardDestination::Controller )); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![21, 31])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![21, 31])); // winners should be 21 and 31. Otherwise this election is taking duplicates into // account. @@ -2066,7 +2066,7 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() { // ensure all have equal stake. assert_eq!( >::iter() - .map(|(v, _)| (v, Staking::ledger(v - 1).unwrap().total)) + .map(|(v, _)| (v, Staking::ledger(v).unwrap().total)) .collect::>(), vec![(31, 1000), (21, 1000), (11, 1000)], ); @@ -2085,14 +2085,14 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() { 1000, RewardDestination::Controller )); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(2), vec![11, 11, 11, 21])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(1), vec![11, 11, 11, 21])); assert_ok!(Staking::bond( RuntimeOrigin::signed(3), 1000, RewardDestination::Controller )); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![21])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![21])); // winners should be 21 and 11. let supports = ::ElectionProvider::elect().unwrap(); @@ -4345,31 +4345,31 @@ fn cannot_rebond_to_lower_than_ed() { #[test] fn cannot_bond_extra_to_lower_than_ed() { ExtBuilder::default() - .existential_deposit(10) - .balance_factor(10) + .existential_deposit(11) + .balance_factor(11) .build_and_execute(|| { // initial stuff. assert_eq!( - Staking::ledger(&20).unwrap(), + Staking::ledger(&21).unwrap(), StakingLedger { stash: 21, - total: 10 * 1000, - active: 10 * 1000, + total: 11 * 1000, + active: 11 * 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], } ); // unbond all of it. must be chilled first. - assert_ok!(Staking::chill(RuntimeOrigin::signed(20))); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(20), 10 * 1000)); + assert_ok!(Staking::chill(RuntimeOrigin::signed(21))); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(21), 11 * 1000)); assert_eq!( - Staking::ledger(&20).unwrap(), + Staking::ledger(&21).unwrap(), StakingLedger { stash: 21, - total: 10 * 1000, + total: 11 * 1000, active: 0, - unlocking: bounded_vec![UnlockChunk { value: 10 * 1000, era: 3 }], + unlocking: bounded_vec![UnlockChunk { value: 11 * 1000, era: 3 }], claimed_rewards: bounded_vec![], } ); From d29a6a5c5740f9b43aa578c941cfec2b63fc131d Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 18:36:42 +0700 Subject: [PATCH 09/60] mvs controllers to stashes for 9 tests --- frame/staking/src/mock.rs | 6 +- frame/staking/src/tests.rs | 143 +++++++++++++++++++------------------ 2 files changed, 74 insertions(+), 75 deletions(-) diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index ec3a3b79d6cdc..9904282375b56 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -578,11 +578,7 @@ pub(crate) fn bond_validator(who: AccountId, val: Balance) { )); } -pub(crate) fn bond_nominator( - who: AccountId, - val: Balance, - target: Vec, -) { +pub(crate) fn bond_nominator(who: AccountId, val: Balance, target: Vec) { bond(who, val); assert_ok!(Staking::nominate(RuntimeOrigin::signed(who), target)); } diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index c279b1245f274..4f324ae58bb3b 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -229,16 +229,19 @@ fn basic_setup_works() { } #[test] -fn change_controller_works() { +fn change_controller_already_paired_once_stash() { ExtBuilder::default().build_and_execute(|| { // 10 and 11 are bonded as controller and stash respectively. - assert_eq!(Staking::bonded(&11), Some(10)); + assert_eq!(Staking::bonded(&11), Some(11)); - // 10 can control 11 who is initially a validator. - assert_ok!(Staking::chill(RuntimeOrigin::signed(10))); + // 11 is initially a validator. + assert_ok!(Staking::chill(RuntimeOrigin::signed(11))); - // change controller - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(11))); + // Controller cannot change once matching with stash. + assert_noop!( + Staking::set_controller(RuntimeOrigin::signed(11)), + Error::::AlreadyPaired + ); assert_eq!(Staking::bonded(&11), Some(11)); mock::start_active_era(1); @@ -939,21 +942,21 @@ fn cannot_transfer_staked_balance() { // Tests that a stash account cannot transfer funds ExtBuilder::default().nominate(false).build_and_execute(|| { // Confirm account 11 is stashed - assert_eq!(Staking::bonded(&11), Some(10)); + assert_eq!(Staking::bonded(&11), Some(11)); // Confirm account 11 has some free balance assert_eq!(Balances::free_balance(11), 1000); - // Confirm account 11 (via controller 10) is totally staked + // Confirm account 11 (via controller) is totally staked assert_eq!(Staking::eras_stakers(active_era(), 11).total, 1000); // Confirm account 11 cannot transfer as a result assert_noop!( - Balances::transfer_allow_death(RuntimeOrigin::signed(11), 20, 1), + Balances::transfer_allow_death(RuntimeOrigin::signed(11), 21, 1), TokenError::Frozen, ); // Give account 11 extra free balance let _ = Balances::make_free_balance_be(&11, 10000); // Confirm that account 11 can now transfer some balance - assert_ok!(Balances::transfer_allow_death(RuntimeOrigin::signed(11), 20, 1)); + assert_ok!(Balances::transfer_allow_death(RuntimeOrigin::signed(11), 21, 1)); }); } @@ -964,17 +967,17 @@ fn cannot_transfer_staked_balance_2() { // 21 has 2000 free balance but 1000 at stake ExtBuilder::default().nominate(false).build_and_execute(|| { // Confirm account 21 is stashed - assert_eq!(Staking::bonded(&21), Some(20)); + assert_eq!(Staking::bonded(&21), Some(21)); // Confirm account 21 has some free balance assert_eq!(Balances::free_balance(21), 2000); - // Confirm account 21 (via controller 20) is totally staked + // Confirm account 21 (via controller) is totally staked assert_eq!(Staking::eras_stakers(active_era(), 21).total, 1000); // Confirm account 21 can transfer at most 1000 assert_noop!( - Balances::transfer_allow_death(RuntimeOrigin::signed(21), 20, 1001), + Balances::transfer_allow_death(RuntimeOrigin::signed(21), 21, 1001), TokenError::Frozen, ); - assert_ok!(Balances::transfer_allow_death(RuntimeOrigin::signed(21), 20, 1000)); + assert_ok!(Balances::transfer_allow_death(RuntimeOrigin::signed(21), 21, 1000)); }); } @@ -983,7 +986,7 @@ fn cannot_reserve_staked_balance() { // Checks that a bonded account cannot reserve balance from free balance ExtBuilder::default().build_and_execute(|| { // Confirm account 11 is stashed - assert_eq!(Staking::bonded(&11), Some(10)); + assert_eq!(Staking::bonded(&11), Some(11)); // Confirm account 11 has some free balance assert_eq!(Balances::free_balance(11), 1000); // Confirm account 11 (via controller 10) is totally staked @@ -3446,18 +3449,18 @@ fn disabled_validators_are_kept_disabled_for_whole_era() { // nominations are not updated. assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]); - // validator 10 should not be disabled since the offence wasn't slashable - assert!(!is_disabled(10)); - // validator 20 gets disabled since it got slashed - assert!(is_disabled(20)); + // validator 11 should not be disabled since the offence wasn't slashable + assert!(!is_disabled(11)); + // validator 21 gets disabled since it got slashed + assert!(is_disabled(21)); advance_session(); // disabled validators should carry-on through all sessions in the era - assert!(!is_disabled(10)); - assert!(is_disabled(20)); + assert!(!is_disabled(11)); + assert!(is_disabled(21)); - // validator 10 should now get disabled + // validator 11 should now get disabled on_offence_now( &[OffenceDetails { offender: (11, exposure_11.clone()), reporters: vec![] }], &[Perbill::from_percent(25)], @@ -3469,14 +3472,14 @@ fn disabled_validators_are_kept_disabled_for_whole_era() { advance_session(); // and both are disabled in the last session of the era - assert!(is_disabled(10)); - assert!(is_disabled(20)); + assert!(is_disabled(11)); + assert!(is_disabled(21)); mock::start_active_era(2); // when a new era starts disabled validators get cleared - assert!(!is_disabled(10)); - assert!(!is_disabled(20)); + assert!(!is_disabled(11)); + assert!(!is_disabled(21)); }); } @@ -3490,11 +3493,11 @@ fn claim_reward_at_the_last_era_and_no_double_claim_and_invalid_claim() { // Consumed weight for all payout_stakers dispatches that fail let err_weight = ::WeightInfo::payout_stakers_alive_staked(0); - let init_balance_10 = Balances::total_balance(&10); - let init_balance_100 = Balances::total_balance(&100); + let init_balance_11 = Balances::total_balance(&11); + let init_balance_101 = Balances::total_balance(&101); - let part_for_10 = Perbill::from_rational::(1000, 1125); - let part_for_100 = Perbill::from_rational::(125, 1125); + let part_for_11 = Perbill::from_rational::(1000, 1125); + let part_for_101 = Perbill::from_rational::(125, 1125); // Check state Payee::::insert(11, RewardDestination::Controller); @@ -3554,12 +3557,12 @@ fn claim_reward_at_the_last_era_and_no_double_claim_and_invalid_claim() { // only era 1 and 2 can be rewarded. assert_eq!( - Balances::total_balance(&10), - init_balance_10 + part_for_10 * (total_payout_1 + total_payout_2), + Balances::total_balance(&11), + init_balance_11 + part_for_11 * (total_payout_1 + total_payout_2), ); assert_eq!( - Balances::total_balance(&100), - init_balance_100 + part_for_100 * (total_payout_1 + total_payout_2), + Balances::total_balance(&101), + init_balance_101 + part_for_101 * (total_payout_1 + total_payout_2), ); }); } @@ -4305,38 +4308,38 @@ fn session_buffering_no_offset() { #[test] fn cannot_rebond_to_lower_than_ed() { ExtBuilder::default() - .existential_deposit(10) - .balance_factor(10) + .existential_deposit(11) + .balance_factor(11) .build_and_execute(|| { // initial stuff. assert_eq!( - Staking::ledger(&20).unwrap(), + Staking::ledger(&21).unwrap(), StakingLedger { stash: 21, - total: 10 * 1000, - active: 10 * 1000, + total: 11 * 1000, + active: 11 * 1000, unlocking: Default::default(), claimed_rewards: bounded_vec![], } ); // unbond all of it. must be chilled first. - assert_ok!(Staking::chill(RuntimeOrigin::signed(20))); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(20), 10 * 1000)); + assert_ok!(Staking::chill(RuntimeOrigin::signed(21))); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(21), 11 * 1000)); assert_eq!( - Staking::ledger(&20).unwrap(), + Staking::ledger(&21).unwrap(), StakingLedger { stash: 21, - total: 10 * 1000, + total: 11 * 1000, active: 0, - unlocking: bounded_vec![UnlockChunk { value: 10 * 1000, era: 3 }], + unlocking: bounded_vec![UnlockChunk { value: 11 * 1000, era: 3 }], claimed_rewards: bounded_vec![], } ); // now bond a wee bit more assert_noop!( - Staking::rebond(RuntimeOrigin::signed(20), 5), + Staking::rebond(RuntimeOrigin::signed(21), 5), Error::::InsufficientBond ); }) @@ -4391,7 +4394,7 @@ fn do_not_die_when_active_is_ed() { .build_and_execute(|| { // given assert_eq!( - Staking::ledger(&20).unwrap(), + Staking::ledger(&21).unwrap(), StakingLedger { stash: 21, total: 1000 * ed, @@ -4402,13 +4405,13 @@ fn do_not_die_when_active_is_ed() { ); // when unbond all of it except ed. - assert_ok!(Staking::unbond(RuntimeOrigin::signed(20), 999 * ed)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(21), 999 * ed)); start_active_era(3); - assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(20), 100)); + assert_ok!(Staking::withdraw_unbonded(RuntimeOrigin::signed(21), 100)); // then assert_eq!( - Staking::ledger(&20).unwrap(), + Staking::ledger(&21).unwrap(), StakingLedger { stash: 21, total: ed, @@ -4471,9 +4474,9 @@ mod election_data_provider { fn set_minimum_active_stake_is_correct() { ExtBuilder::default() .nominate(false) - .add_staker(61, 60, 2_000, StakerStatus::::Nominator(vec![21])) - .add_staker(71, 70, 10, StakerStatus::::Nominator(vec![21])) - .add_staker(81, 80, 50, StakerStatus::::Nominator(vec![21])) + .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(|| { assert_ok!(::electing_voters(None)); assert_eq!(MinimumActiveStake::::get(), 10); @@ -4545,19 +4548,19 @@ mod election_data_provider { // ppl, but then lower the MaxNomination limit. .add_staker( 61, - 60, + 61, 2_000, StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), ) .add_staker( 71, - 70, + 71, 2_000, StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), ) .add_staker( 81, - 80, + 81, 2_000, StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), ) @@ -5026,8 +5029,8 @@ fn min_commission_works() { fn change_of_max_nominations() { use frame_election_provider_support::ElectionDataProvider; ExtBuilder::default() - .add_staker(60, 61, 10, StakerStatus::Nominator(vec![1])) - .add_staker(70, 71, 10, StakerStatus::Nominator(vec![1, 2, 3])) + .add_staker(61, 61, 10, StakerStatus::Nominator(vec![1])) + .add_staker(71, 71, 10, StakerStatus::Nominator(vec![1, 2, 3])) .balance_factor(10) .build_and_execute(|| { // pre-condition @@ -5037,7 +5040,7 @@ fn change_of_max_nominations() { Nominators::::iter() .map(|(k, n)| (k, n.targets.len())) .collect::>(), - vec![(70, 3), (101, 2), (60, 1)] + vec![(101, 2), (71, 3), (61, 1)] ); // 3 validators and 3 nominators assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3); @@ -5049,7 +5052,7 @@ fn change_of_max_nominations() { Nominators::::iter() .map(|(k, n)| (k, n.targets.len())) .collect::>(), - vec![(70, 3), (101, 2), (60, 1)] + vec![(101, 2), (71, 3), (61, 1)] ); assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3); @@ -5060,7 +5063,7 @@ fn change_of_max_nominations() { Nominators::::iter() .map(|(k, n)| (k, n.targets.len())) .collect::>(), - vec![(70, 3), (101, 2), (60, 1)] + vec![(101, 2), (71, 3), (61, 1)] ); assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3); @@ -5072,12 +5075,12 @@ fn change_of_max_nominations() { Nominators::::iter() .map(|(k, n)| (k, n.targets.len())) .collect::>(), - vec![(101, 2), (60, 1)] + vec![(101, 2), (61, 1)] ); // 70 is still in storage.. - assert!(Nominators::::contains_key(70)); + assert!(Nominators::::contains_key(71)); // but its value cannot be decoded and default is returned. - assert!(Nominators::::get(70).is_none()); + assert!(Nominators::::get(71).is_none()); assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 2); assert!(Nominators::::contains_key(101)); @@ -5090,12 +5093,12 @@ fn change_of_max_nominations() { Nominators::::iter() .map(|(k, n)| (k, n.targets.len())) .collect::>(), - vec![(60, 1)] + vec![(61, 1)] ); - assert!(Nominators::::contains_key(70)); - assert!(Nominators::::contains_key(60)); - assert!(Nominators::::get(70).is_none()); - assert!(Nominators::::get(60).is_some()); + assert!(Nominators::::contains_key(71)); + assert!(Nominators::::contains_key(61)); + assert!(Nominators::::get(71).is_none()); + assert!(Nominators::::get(61).is_some()); assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 1); // now one of them can revive themselves by re-nominating to a proper value. @@ -5104,13 +5107,13 @@ fn change_of_max_nominations() { Nominators::::iter() .map(|(k, n)| (k, n.targets.len())) .collect::>(), - vec![(70, 1), (60, 1)] + vec![(71, 1), (61, 1)] ); // or they can be chilled by any account. assert!(Nominators::::contains_key(101)); assert!(Nominators::::get(101).is_none()); - assert_ok!(Staking::chill_other(RuntimeOrigin::signed(70), 100)); + assert_ok!(Staking::chill_other(RuntimeOrigin::signed(71), 101)); assert!(!Nominators::::contains_key(101)); assert!(Nominators::::get(101).is_none()); }) From 71289da4acec6712b25648ea76e48a6ac5840729 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 18:37:18 +0700 Subject: [PATCH 10/60] remove double_controlling_should_fail --- frame/staking/src/tests.rs | 21 --------------------- 1 file changed, 21 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 4f324ae58bb3b..f0f27a242bb88 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -768,27 +768,6 @@ fn double_staking_should_fail() { }); } -#[test] -fn double_controlling_should_fail() { - // should test (in the same order): - // * an account already bonded as controller CANNOT be reused as the controller of another - // account. - ExtBuilder::default().build_and_execute(|| { - let arbitrary_value = 5; - // 2 = controller, 1 stashed => ok - assert_ok!(Staking::bond( - RuntimeOrigin::signed(1), - arbitrary_value, - RewardDestination::default(), - )); - // 2 = controller, 3 stashed (Note that 2 is reused.) => no-op - assert_noop!( - Staking::bond(RuntimeOrigin::signed(3), arbitrary_value, RewardDestination::default()), - Error::::AlreadyPaired, - ); - }); -} - #[test] fn session_and_eras_work_simple() { ExtBuilder::default().period(1).build_and_execute(|| { From 5198f5330d2309e0d9b4e455a8fa6557356577c7 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 18:38:18 +0700 Subject: [PATCH 11/60] remove double_staking_should_fail --- frame/staking/src/tests.rs | 29 ----------------------------- 1 file changed, 29 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index f0f27a242bb88..caae17e36a4f9 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -739,35 +739,6 @@ fn nominators_also_get_slashed_pro_rata() { }); } -#[test] -fn double_staking_should_fail() { - // should test (in the same order): - // * an account already bonded as stash cannot be be stashed again. - // * an account already bonded as stash cannot nominate. - // * an account already bonded as controller can nominate. - ExtBuilder::default().build_and_execute(|| { - let arbitrary_value = 5; - // 2 = controller, 1 stashed => ok - assert_ok!(Staking::bond( - RuntimeOrigin::signed(1), - arbitrary_value, - RewardDestination::default() - )); - // 4 = not used so far, 1 stashed => not allowed. - assert_noop!( - Staking::bond(RuntimeOrigin::signed(1), arbitrary_value, RewardDestination::default()), - Error::::AlreadyBonded, - ); - // 1 = stashed => attempting to nominate should fail. - assert_noop!( - Staking::nominate(RuntimeOrigin::signed(1), vec![1]), - Error::::NotController - ); - // 2 = controller => nominating should work. - assert_ok!(Staking::nominate(RuntimeOrigin::signed(2), vec![1])); - }); -} - #[test] fn session_and_eras_work_simple() { ExtBuilder::default().period(1).build_and_execute(|| { From db899113ebed197bd3032b90609ce391b16d2293 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 18:53:12 +0700 Subject: [PATCH 12/60] mvs controllers to stashes for 10 tests --- frame/staking/src/tests.rs | 134 ++++++++++++++++++------------------- 1 file changed, 67 insertions(+), 67 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index caae17e36a4f9..2231db677b0c8 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -92,8 +92,8 @@ fn set_staking_configs_works() { #[test] fn force_unstake_works() { ExtBuilder::default().build_and_execute(|| { - // Account 11 is stashed and locked, and account 10 is the controller - assert_eq!(Staking::bonded(&11), Some(10)); + // Account 11 (also controller) is stashed and locked + assert_eq!(Staking::bonded(&11), Some(11)); // Adds 2 slashing spans add_slash(&11); // Cant transfer @@ -120,8 +120,8 @@ fn force_unstake_works() { #[test] fn kill_stash_works() { ExtBuilder::default().build_and_execute(|| { - // Account 11 is stashed and locked, and account 10 is the controller - assert_eq!(Staking::bonded(&11), Some(10)); + // Account 11 (also controller) is stashed and locked + assert_eq!(Staking::bonded(&11), Some(11)); // Adds 2 slashing spans add_slash(&11); // Only can kill a stash account @@ -490,12 +490,12 @@ fn less_than_needed_candidates_works() { .build_and_execute(|| { assert_eq!(Staking::validator_count(), 4); assert_eq!(Staking::minimum_validator_count(), 1); - assert_eq_uvec!(validator_controllers(), vec![30, 20, 10]); + assert_eq_uvec!(validator_controllers(), vec![31, 21, 11]); mock::start_active_era(1); // Previous set is selected. NO election algorithm is even executed. - assert_eq_uvec!(validator_controllers(), vec![30, 20, 10]); + assert_eq_uvec!(validator_controllers(), vec![31, 21, 11]); // But the exposure is updated in a simple way. No external votes exists. // This is purely self-vote. @@ -513,21 +513,21 @@ fn no_candidate_emergency_condition() { .nominate(false) .build_and_execute(|| { // initial validators - assert_eq_uvec!(validator_controllers(), vec![10, 20, 30, 40]); + assert_eq_uvec!(validator_controllers(), vec![11, 21, 31, 41]); let prefs = ValidatorPrefs { commission: Perbill::one(), ..Default::default() }; Validators::::insert(11, prefs.clone()); // set the minimum validator count. - MinimumValidatorCount::::put(10); + MinimumValidatorCount::::put(11); // try to chill - let res = Staking::chill(RuntimeOrigin::signed(10)); + let res = Staking::chill(RuntimeOrigin::signed(11)); assert_ok!(res); let current_era = CurrentEra::::get(); // try trigger new era - mock::run_to_block(20); + mock::run_to_block(21); assert_eq!(*staking_events().last().unwrap(), Event::StakingElectionFailed); // No new era is created assert_eq!(current_era, CurrentEra::::get()); @@ -537,7 +537,7 @@ fn no_candidate_emergency_condition() { // Previous ones are elected. chill is not effective in active era (as era hasn't // changed) - assert_eq_uvec!(validator_controllers(), vec![10, 20, 30, 40]); + assert_eq_uvec!(validator_controllers(), vec![11, 21, 31, 41]); // The chill is still pending. assert!(!Validators::::contains_key(11)); // No new era is created. @@ -554,33 +554,33 @@ fn nominating_and_rewards_should_work() { .set_status(31, StakerStatus::Idle) .build_and_execute(|| { // initial validators. - assert_eq_uvec!(validator_controllers(), vec![40, 20]); + assert_eq_uvec!(validator_controllers(), vec![41, 21]); // re-validate with 11 and 31. - assert_ok!(Staking::validate(RuntimeOrigin::signed(10), Default::default())); - assert_ok!(Staking::validate(RuntimeOrigin::signed(30), Default::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(11), Default::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(31), Default::default())); // Set payee to controller. assert_ok!(Staking::set_payee( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(11), RewardDestination::Controller )); assert_ok!(Staking::set_payee( - RuntimeOrigin::signed(20), + RuntimeOrigin::signed(21), RewardDestination::Controller )); assert_ok!(Staking::set_payee( - RuntimeOrigin::signed(30), + RuntimeOrigin::signed(31), RewardDestination::Controller )); assert_ok!(Staking::set_payee( - RuntimeOrigin::signed(40), + RuntimeOrigin::signed(41), RewardDestination::Controller )); // give the man some money let initial_balance = 1000; - for i in [1, 2, 3, 4, 5, 10, 11, 20, 21].iter() { + for i in [1, 3, 5, 11, 21].iter() { let _ = Balances::make_free_balance_be(i, initial_balance); } @@ -590,14 +590,14 @@ fn nominating_and_rewards_should_work() { 1000, RewardDestination::Controller )); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(2), vec![11, 21, 31])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(1), vec![11, 21, 31])); assert_ok!(Staking::bond( RuntimeOrigin::signed(3), 1000, RewardDestination::Controller )); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![11, 21, 41])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![11, 21, 41])); // the total reward for era 0 let total_payout_0 = current_total_payout_for_duration(reward_time_per_era()); @@ -607,15 +607,15 @@ fn nominating_and_rewards_should_work() { mock::start_active_era(1); // 10 and 20 have more votes, they will be chosen. - assert_eq_uvec!(validator_controllers(), vec![20, 10]); + assert_eq_uvec!(validator_controllers(), vec![21, 11]); // old validators must have already received some rewards. - let initial_balance_40 = Balances::total_balance(&40); - let mut initial_balance_20 = Balances::total_balance(&20); + let initial_balance_41 = Balances::total_balance(&41); + let mut initial_balance_21 = Balances::total_balance(&21); mock::make_all_reward_payment(0); - assert_eq!(Balances::total_balance(&40), initial_balance_40 + total_payout_0 / 2); - assert_eq!(Balances::total_balance(&20), initial_balance_20 + total_payout_0 / 2); - initial_balance_20 = Balances::total_balance(&20); + assert_eq!(Balances::total_balance(&41), initial_balance_41 + total_payout_0 / 2); + assert_eq!(Balances::total_balance(&21), initial_balance_21 + total_payout_0 / 2); + initial_balance_21 = Balances::total_balance(&21); assert_eq!(ErasStakers::::iter_prefix_values(active_era()).count(), 2); assert_eq!( @@ -652,34 +652,34 @@ fn nominating_and_rewards_should_work() { // nominators will also be paid. See below mock::make_all_reward_payment(1); - let payout_for_10 = total_payout_1 / 3; - let payout_for_20 = 2 * total_payout_1 / 3; - // Nominator 2: has [400/1800 ~ 2/9 from 10] + [600/2200 ~ 3/11 from 20]'s reward. ==> + let payout_for_11 = total_payout_1 / 3; + let payout_for_21 = 2 * total_payout_1 / 3; + // Nominator 2: has [400/1800 ~ 2/9 from 10] + [600/2200 ~ 3/11 from 21]'s reward. ==> // 2/9 + 3/11 assert_eq_error_rate!( - Balances::total_balance(&2), - initial_balance + (2 * payout_for_10 / 9 + 3 * payout_for_20 / 11), + Balances::total_balance(&1), + initial_balance + (2 * payout_for_11 / 9 + 3 * payout_for_21 / 11), 2, ); - // Nominator 4: has [400/1800 ~ 2/9 from 10] + [600/2200 ~ 3/11 from 20]'s reward. ==> + // Nominator 3: has [400/1800 ~ 2/9 from 10] + [600/2200 ~ 3/11 from 21]'s reward. ==> // 2/9 + 3/11 assert_eq_error_rate!( - Balances::total_balance(&4), - initial_balance + (2 * payout_for_10 / 9 + 3 * payout_for_20 / 11), + Balances::total_balance(&3), + initial_balance + (2 * payout_for_11 / 9 + 3 * payout_for_21 / 11), 2, ); - // Validator 10: got 800 / 1800 external stake => 8/18 =? 4/9 => Validator's share = 5/9 + // Validator 11: got 800 / 1800 external stake => 8/18 =? 4/9 => Validator's share = 5/9 assert_eq_error_rate!( - Balances::total_balance(&10), - initial_balance + 5 * payout_for_10 / 9, + Balances::total_balance(&11), + initial_balance + 5 * payout_for_11 / 9, 2, ); - // Validator 20: got 1200 / 2200 external stake => 12/22 =? 6/11 => Validator's share = + // Validator 21: got 1200 / 2200 external stake => 12/22 =? 6/11 => Validator's share = // 5/11 assert_eq_error_rate!( - Balances::total_balance(&20), - initial_balance_20 + 5 * payout_for_20 / 11, + Balances::total_balance(&21), + initial_balance_21 + 5 * payout_for_21 / 11, 2, ); }); @@ -1300,7 +1300,7 @@ fn many_unbond_calls_should_work() { // There is only 1 chunk per era, so we need to be in a new era to create a chunk. current_era = i as u32; mock::start_active_era(current_era); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 1)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1)); } current_era += 1; @@ -1308,9 +1308,9 @@ fn many_unbond_calls_should_work() { // This chunk is locked at `current_era` through `current_era + 2` (because // `BondingDuration` == 3). - assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 1)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1)); assert_eq!( - Staking::ledger(&10).map(|l| l.unlocking.len()).unwrap(), + Staking::ledger(&11).map(|l| l.unlocking.len()).unwrap(), <::MaxUnlockingChunks as Get>::get() as usize ); @@ -1320,12 +1320,12 @@ fn many_unbond_calls_should_work() { // There is only 1 chunk per era, so we need to be in a new era to create a chunk. current_era = i as u32; mock::start_active_era(current_era); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 1)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1)); } // only slots within last `BondingDuration` are filled. assert_eq!( - Staking::ledger(&10).map(|l| l.unlocking.len()).unwrap(), + Staking::ledger(&11).map(|l| l.unlocking.len()).unwrap(), <::BondingDuration>::get() as usize ); }) @@ -4632,44 +4632,44 @@ fn min_bond_checks_work() { // 500 is not enough for any role assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 500, RewardDestination::Controller)); assert_noop!( - Staking::nominate(RuntimeOrigin::signed(4), vec![1]), + Staking::nominate(RuntimeOrigin::signed(3), vec![1]), Error::::InsufficientBond ); assert_noop!( - Staking::validate(RuntimeOrigin::signed(4), ValidatorPrefs::default()), + Staking::validate(RuntimeOrigin::signed(3), ValidatorPrefs::default()), Error::::InsufficientBond, ); // 1000 is enough for nominator assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(3), 500)); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![1])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![1])); assert_noop!( - Staking::validate(RuntimeOrigin::signed(4), ValidatorPrefs::default()), + Staking::validate(RuntimeOrigin::signed(3), ValidatorPrefs::default()), Error::::InsufficientBond, ); // 1500 is enough for validator assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(3), 500)); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![1])); - assert_ok!(Staking::validate(RuntimeOrigin::signed(4), ValidatorPrefs::default())); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![1])); + assert_ok!(Staking::validate(RuntimeOrigin::signed(3), ValidatorPrefs::default())); // Can't unbond anything as validator assert_noop!( - Staking::unbond(RuntimeOrigin::signed(4), 500), + Staking::unbond(RuntimeOrigin::signed(3), 500), Error::::InsufficientBond ); // Once they are a nominator, they can unbond 500 - assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![1])); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(4), 500)); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![1])); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(3), 500)); assert_noop!( - Staking::unbond(RuntimeOrigin::signed(4), 500), + Staking::unbond(RuntimeOrigin::signed(3), 500), Error::::InsufficientBond ); // Once they are chilled they can unbond everything - assert_ok!(Staking::chill(RuntimeOrigin::signed(4))); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(4), 1000)); + assert_ok!(Staking::chill(RuntimeOrigin::signed(3))); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(3), 1000)); }) } @@ -4928,9 +4928,9 @@ fn capped_stakers_works() { #[test] fn min_commission_works() { ExtBuilder::default().build_and_execute(|| { - // account 10 controls the stash from account 11 + // account 11 controls the stash of itself. assert_ok!(Staking::validate( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(11), ValidatorPrefs { commission: Perbill::from_percent(5), blocked: false } )); @@ -4956,7 +4956,7 @@ fn min_commission_works() { // can't make it less than 10 now assert_noop!( Staking::validate( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(11), ValidatorPrefs { commission: Perbill::from_percent(5), blocked: false } ), Error::::CommissionTooLow @@ -4964,12 +4964,12 @@ fn min_commission_works() { // can only change to higher. assert_ok!(Staking::validate( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(11), ValidatorPrefs { commission: Perbill::from_percent(10), blocked: false } )); assert_ok!(Staking::validate( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(11), ValidatorPrefs { commission: Perbill::from_percent(15), blocked: false } )); }) @@ -5125,8 +5125,8 @@ fn force_apply_min_commission_works() { let prefs = |c| ValidatorPrefs { commission: Perbill::from_percent(c), blocked: false }; let validators = || Validators::::iter().collect::>(); ExtBuilder::default().build_and_execute(|| { - assert_ok!(Staking::validate(RuntimeOrigin::signed(30), prefs(10))); - assert_ok!(Staking::validate(RuntimeOrigin::signed(20), prefs(5))); + assert_ok!(Staking::validate(RuntimeOrigin::signed(31), prefs(10))); + assert_ok!(Staking::validate(RuntimeOrigin::signed(21), prefs(5))); // Given assert_eq!(validators(), vec![(31, prefs(10)), (21, prefs(5)), (11, prefs(0))]); @@ -5675,7 +5675,7 @@ fn set_min_commission_works_with_admin_origin() { // setting commission below min_commission fails assert_noop!( Staking::validate( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(11), ValidatorPrefs { commission: Perbill::from_percent(14), blocked: false } ), Error::::CommissionTooLow @@ -5683,7 +5683,7 @@ fn set_min_commission_works_with_admin_origin() { // setting commission >= min_commission works assert_ok!(Staking::validate( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(11), ValidatorPrefs { commission: Perbill::from_percent(15), blocked: false } )); }) From d13643ec608c9d3792cb6d183750577971df30c8 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 22:31:17 +0700 Subject: [PATCH 13/60] mvs controllers to stashes for 2 tests --- frame/staking/src/tests.rs | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 2231db677b0c8..d6b25b8186041 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -695,9 +695,9 @@ fn nominators_also_get_slashed_pro_rata() { assert_eq!(initial_exposure.others.first().unwrap().who, 101); // staked values; - let nominator_stake = Staking::ledger(100).unwrap().active; + let nominator_stake = Staking::ledger(101).unwrap().active; let nominator_balance = balances(&101).0; - let validator_stake = Staking::ledger(10).unwrap().active; + let validator_stake = Staking::ledger(11).unwrap().active; let validator_balance = balances(&11).0; let exposed_stake = initial_exposure.total; let exposed_validator = initial_exposure.own; @@ -710,8 +710,8 @@ fn nominators_also_get_slashed_pro_rata() { ); // both stakes must have been decreased. - assert!(Staking::ledger(100).unwrap().active < nominator_stake); - assert!(Staking::ledger(10).unwrap().active < validator_stake); + assert!(Staking::ledger(101).unwrap().active < nominator_stake); + assert!(Staking::ledger(11).unwrap().active < validator_stake); let slash_amount = slash_percent * exposed_stake; let validator_share = @@ -724,8 +724,8 @@ fn nominators_also_get_slashed_pro_rata() { assert!(nominator_share > 0); // both stakes must have been decreased pro-rata. - assert_eq!(Staking::ledger(100).unwrap().active, nominator_stake - nominator_share); - assert_eq!(Staking::ledger(10).unwrap().active, validator_stake - validator_share); + assert_eq!(Staking::ledger(101).unwrap().active, nominator_stake - nominator_share); + assert_eq!(Staking::ledger(11).unwrap().active, validator_stake - validator_share); assert_eq!( balances(&101).0, // free balance nominator_balance - nominator_share, @@ -735,7 +735,7 @@ fn nominators_also_get_slashed_pro_rata() { validator_balance - validator_share, ); // Because slashing happened. - assert!(is_disabled(10)); + assert!(is_disabled(11)); }); } @@ -3260,9 +3260,9 @@ fn non_slashable_offence_doesnt_disable_validator() { ); // the offence for validator 10 wasn't slashable so it wasn't disabled - assert!(!is_disabled(10)); + assert!(!is_disabled(11)); // whereas validator 20 gets disabled - assert!(is_disabled(20)); + assert!(is_disabled(21)); }); } From 79f97562763de49a0865bf6d84487d5f19991709 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 22:31:46 +0700 Subject: [PATCH 14/60] remove payout_creates_controller --- frame/staking/src/tests.rs | 26 -------------------------- 1 file changed, 26 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index d6b25b8186041..c93a8e8451de2 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4099,32 +4099,6 @@ fn offences_weight_calculated_correctly() { }); } -#[test] -fn payout_creates_controller() { - ExtBuilder::default().has_stakers(false).build_and_execute(|| { - let balance = 1000; - // Create a validator: - bond_validator(11, balance); - - // Create a stash/controller pair - bond_nominator(1234, 100, vec![11]); - - // kill controller - assert_ok!(Balances::transfer_allow_death(RuntimeOrigin::signed(1337), 1234, 100)); - assert_eq!(Balances::free_balance(1337), 0); - - mock::start_active_era(1); - Staking::reward_by_ids(vec![(11, 1)]); - // compute and ensure the reward amount is greater than zero. - let _ = current_total_payout_for_duration(reward_time_per_era()); - mock::start_active_era(2); - assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(1337), 11, 1)); - - // Controller is created - assert!(Balances::free_balance(1337) > 0); - }) -} - #[test] fn payout_to_any_account_works() { ExtBuilder::default().has_stakers(false).build_and_execute(|| { From b5d5426361386ad88b25a2b79b49dc1eac9615b7 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 23:06:05 +0700 Subject: [PATCH 15/60] mvs controllers to stashes for 27 tests --- frame/staking/src/tests.rs | 276 ++++++++++++++++++------------------- 1 file changed, 135 insertions(+), 141 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index c93a8e8451de2..f6f8dc5dbe786 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -383,7 +383,7 @@ fn rewards_should_work() { fn staking_should_work() { ExtBuilder::default().nominate(false).build_and_execute(|| { // remember + compare this along with the test. - assert_eq_uvec!(validator_controllers(), vec![20, 10]); + assert_eq_uvec!(validator_controllers(), vec![21, 11]); // put some money in account that we'll use. for i in 1..5 { @@ -394,21 +394,21 @@ fn staking_should_work() { start_session(2); // add a new candidate for being a validator. account 3 controlled by 4. assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, RewardDestination::Controller)); - assert_ok!(Staking::validate(RuntimeOrigin::signed(4), ValidatorPrefs::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(3), ValidatorPrefs::default())); assert_ok!(Session::set_keys( - RuntimeOrigin::signed(4), + RuntimeOrigin::signed(3), SessionKeys { other: 4.into() }, vec![] )); // No effects will be seen so far. - assert_eq_uvec!(validator_controllers(), vec![20, 10]); + assert_eq_uvec!(validator_controllers(), vec![21, 11]); // --- Block 3: start_session(3); // No effects will be seen so far. Era has not been yet triggered. - assert_eq_uvec!(validator_controllers(), vec![20, 10]); + assert_eq_uvec!(validator_controllers(), vec![21, 11]); // --- Block 4: the validators will now be queued. start_session(4); @@ -420,25 +420,25 @@ fn staking_should_work() { // --- Block 6: the validators will now be changed. start_session(6); - assert_eq_uvec!(validator_controllers(), vec![20, 4]); + assert_eq_uvec!(validator_controllers(), vec![21, 3]); // --- Block 6: Unstake 4 as a validator, freeing up the balance stashed in 3 // 4 will chill - Staking::chill(RuntimeOrigin::signed(4)).unwrap(); + Staking::chill(RuntimeOrigin::signed(3)).unwrap(); - // --- Block 7: nothing. 4 is still there. + // --- Block 7: nothing. 3 is still there. start_session(7); - assert_eq_uvec!(validator_controllers(), vec![20, 4]); + assert_eq_uvec!(validator_controllers(), vec![21, 3]); // --- Block 8: start_session(8); // --- Block 9: 4 will not be a validator. start_session(9); - assert_eq_uvec!(validator_controllers(), vec![20, 10]); + assert_eq_uvec!(validator_controllers(), vec![21, 11]); // Note: the stashed value of 4 is still lock assert_eq!( - Staking::ledger(&4), + Staking::ledger(&3), Some(StakingLedger { stash: 3, total: 1500, @@ -963,7 +963,7 @@ fn reward_destination_works() { assert_eq!(Balances::free_balance(11), 1000); // Check how much is at stake assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -986,7 +986,7 @@ fn reward_destination_works() { assert_eq!(Balances::free_balance(11), 1000 + total_payout_0); // Check that amount at stake increased accordingly assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000 + total_payout_0, @@ -1010,11 +1010,9 @@ fn reward_destination_works() { assert_eq!(Staking::payee(&11), RewardDestination::Stash); // Check that reward went to the stash account assert_eq!(Balances::free_balance(11), 1000 + total_payout_0 + total_payout_1); - // Record this value - let recorded_stash_balance = 1000 + total_payout_0 + total_payout_1; // Check that amount at stake is NOT increased assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000 + total_payout_0, @@ -1028,7 +1026,7 @@ fn reward_destination_works() { >::insert(&11, RewardDestination::Controller); // Check controller balance - assert_eq!(Balances::free_balance(10), 1); + assert_eq!(Balances::free_balance(11), 23150); // Compute total payout now for whole duration as other parameter won't change let total_payout_2 = current_total_payout_for_duration(reward_time_per_era()); @@ -1040,10 +1038,10 @@ fn reward_destination_works() { // Check that RewardDestination is Controller assert_eq!(Staking::payee(&11), RewardDestination::Controller); // Check that reward went to the controller account - assert_eq!(Balances::free_balance(10), 1 + total_payout_2); + assert_eq!(Balances::free_balance(11), 23150 + total_payout_2); // Check that amount at stake is NOT increased assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000 + total_payout_0, @@ -1052,8 +1050,6 @@ fn reward_destination_works() { claimed_rewards: bounded_vec![0, 1, 2], }) ); - // Check that amount in staked account is NOT increased. - assert_eq!(Balances::free_balance(11), recorded_stash_balance); }); } @@ -1073,8 +1069,8 @@ fn validator_payment_prefs_work() { mock::start_active_era(1); mock::make_all_reward_payment(0); - let balance_era_1_10 = Balances::total_balance(&10); - let balance_era_1_100 = Balances::total_balance(&100); + let balance_era_1_11 = Balances::total_balance(&11); + let balance_era_1_101 = Balances::total_balance(&101); // Compute total payout now for whole duration as other parameter won't change let total_payout_1 = current_total_payout_for_duration(reward_time_per_era()); @@ -1088,8 +1084,8 @@ fn validator_payment_prefs_work() { let shared_cut = total_payout_1 - taken_cut; let reward_of_10 = shared_cut * exposure_1.own / exposure_1.total + taken_cut; let reward_of_100 = shared_cut * exposure_1.others[0].value / exposure_1.total; - assert_eq_error_rate!(Balances::total_balance(&10), balance_era_1_10 + reward_of_10, 2); - assert_eq_error_rate!(Balances::total_balance(&100), balance_era_1_100 + reward_of_100, 2); + assert_eq_error_rate!(Balances::total_balance(&11), balance_era_1_11 + reward_of_10, 2); + assert_eq_error_rate!(Balances::total_balance(&101), balance_era_1_101 + reward_of_100, 2); }); } @@ -1369,7 +1365,7 @@ fn rebond_works() { // * it can re-bond a portion of the funds scheduled to unlock. ExtBuilder::default().nominate(false).build_and_execute(|| { // Set payee to controller. avoids confusion - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(10), RewardDestination::Controller)); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller)); // Give account 11 some large free balance greater than total let _ = Balances::make_free_balance_be(&11, 1000000); @@ -1377,9 +1373,9 @@ fn rebond_works() { // confirm that 10 is a normal validator and gets paid at the end of the era. mock::start_active_era(1); - // Initial state of 10 + // Initial state of 11 assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1393,12 +1389,12 @@ fn rebond_works() { assert_eq!(active_era(), 2); // Try to rebond some funds. We get an error since no fund is unbonded. - assert_noop!(Staking::rebond(RuntimeOrigin::signed(10), 500), Error::::NoUnlockChunk); + assert_noop!(Staking::rebond(RuntimeOrigin::signed(11), 500), Error::::NoUnlockChunk); // Unbond almost all of the funds in stash. - Staking::unbond(RuntimeOrigin::signed(10), 900).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 900).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1409,9 +1405,9 @@ fn rebond_works() { ); // Re-bond all the funds unbonded. - Staking::rebond(RuntimeOrigin::signed(10), 900).unwrap(); + Staking::rebond(RuntimeOrigin::signed(11), 900).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1422,9 +1418,9 @@ fn rebond_works() { ); // Unbond almost all of the funds in stash. - Staking::unbond(RuntimeOrigin::signed(10), 900).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 900).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1435,9 +1431,9 @@ fn rebond_works() { ); // Re-bond part of the funds unbonded. - Staking::rebond(RuntimeOrigin::signed(10), 500).unwrap(); + Staking::rebond(RuntimeOrigin::signed(11), 500).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1448,9 +1444,9 @@ fn rebond_works() { ); // Re-bond the remainder of the funds unbonded. - Staking::rebond(RuntimeOrigin::signed(10), 500).unwrap(); + Staking::rebond(RuntimeOrigin::signed(11), 500).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1461,11 +1457,11 @@ fn rebond_works() { ); // Unbond parts of the funds in stash. - Staking::unbond(RuntimeOrigin::signed(10), 300).unwrap(); - Staking::unbond(RuntimeOrigin::signed(10), 300).unwrap(); - Staking::unbond(RuntimeOrigin::signed(10), 300).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 300).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 300).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 300).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1476,9 +1472,9 @@ fn rebond_works() { ); // Re-bond part of the funds unbonded. - Staking::rebond(RuntimeOrigin::signed(10), 500).unwrap(); + Staking::rebond(RuntimeOrigin::signed(11), 500).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1495,7 +1491,7 @@ fn rebond_is_fifo() { // Rebond should proceed by reversing the most recent bond operations. ExtBuilder::default().nominate(false).build_and_execute(|| { // Set payee to controller. avoids confusion - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(10), RewardDestination::Controller)); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller)); // Give account 11 some large free balance greater than total let _ = Balances::make_free_balance_be(&11, 1000000); @@ -1505,7 +1501,7 @@ fn rebond_is_fifo() { // Initial state of 10 assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1518,9 +1514,9 @@ fn rebond_is_fifo() { mock::start_active_era(2); // Unbond some of the funds in stash. - Staking::unbond(RuntimeOrigin::signed(10), 400).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 400).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1533,9 +1529,9 @@ fn rebond_is_fifo() { mock::start_active_era(3); // Unbond more of the funds in stash. - Staking::unbond(RuntimeOrigin::signed(10), 300).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 300).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1551,9 +1547,9 @@ fn rebond_is_fifo() { mock::start_active_era(4); // Unbond yet more of the funds in stash. - Staking::unbond(RuntimeOrigin::signed(10), 200).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 200).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1568,9 +1564,9 @@ fn rebond_is_fifo() { ); // Re-bond half of the unbonding funds. - Staking::rebond(RuntimeOrigin::signed(10), 400).unwrap(); + Staking::rebond(RuntimeOrigin::signed(11), 400).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1591,7 +1587,7 @@ fn rebond_emits_right_value_in_event() { // and the rebond event emits the actual value rebonded. ExtBuilder::default().nominate(false).build_and_execute(|| { // Set payee to controller. avoids confusion - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(10), RewardDestination::Controller)); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Controller)); // Give account 11 some large free balance greater than total let _ = Balances::make_free_balance_be(&11, 1000000); @@ -1600,9 +1596,9 @@ fn rebond_emits_right_value_in_event() { mock::start_active_era(1); // Unbond almost all of the funds in stash. - Staking::unbond(RuntimeOrigin::signed(10), 900).unwrap(); + Staking::unbond(RuntimeOrigin::signed(11), 900).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1613,9 +1609,9 @@ fn rebond_emits_right_value_in_event() { ); // Re-bond less than the total - Staking::rebond(RuntimeOrigin::signed(10), 100).unwrap(); + Staking::rebond(RuntimeOrigin::signed(11), 100).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1628,9 +1624,9 @@ fn rebond_emits_right_value_in_event() { assert_eq!(*staking_events().last().unwrap(), Event::Bonded { stash: 11, amount: 100 }); // Re-bond way more than available - Staking::rebond(RuntimeOrigin::signed(10), 100_000).unwrap(); + Staking::rebond(RuntimeOrigin::signed(11), 100_000).unwrap(); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -1686,17 +1682,22 @@ fn reward_to_stake_works() { mock::make_all_reward_payment(0); assert_eq!(Staking::eras_stakers(active_era(), 11).total, 1000); - assert_eq!(Staking::eras_stakers(active_era(), 21).total, 69); + assert_eq!(Staking::eras_stakers(active_era(), 21).total, 2000); let _11_balance = Balances::free_balance(&11); + let _21_balance = Balances::free_balance(&21); + assert_eq!(_11_balance, 1000 + total_payout_0 / 2); + assert_eq!(_21_balance, 2000 + total_payout_0 / 2); // Trigger another new era as the info are frozen before the era start. mock::start_active_era(2); + println!("{:?} / 2 = {:?}", total_payout_0, total_payout_0 / 2); + // -- new infos - assert_eq!(Staking::eras_stakers(active_era(), 11).total, 1000 + total_payout_0 / 2); - assert_eq!(Staking::eras_stakers(active_era(), 21).total, 69 + total_payout_0 / 2); + assert_eq!(Staking::eras_stakers(active_era(), 11).total, _11_balance); + assert_eq!(Staking::eras_stakers(active_era(), 21).total, _21_balance); }); } @@ -1707,11 +1708,10 @@ fn reap_stash_works() { .balance_factor(10) .build_and_execute(|| { // given - assert_eq!(Balances::free_balance(10), 10); assert_eq!(Balances::free_balance(11), 10 * 1000); - assert_eq!(Staking::bonded(&11), Some(10)); + assert_eq!(Staking::bonded(&11), Some(11)); - assert!(>::contains_key(&10)); + assert!(>::contains_key(&11)); assert!(>::contains_key(&11)); assert!(>::contains_key(&11)); assert!(>::contains_key(&11)); @@ -1721,16 +1721,11 @@ fn reap_stash_works() { Staking::reap_stash(RuntimeOrigin::signed(20), 11, 0), Error::::FundedTarget ); - // controller or any other account is not reapable - assert_noop!( - Staking::reap_stash(RuntimeOrigin::signed(20), 10, 0), - Error::::NotStash - ); // no easy way to cause an account to go below ED, we tweak their staking ledger // instead. Ledger::::insert( - 10, + 11, StakingLedger { stash: 11, total: 5, @@ -1744,7 +1739,7 @@ fn reap_stash_works() { assert_ok!(Staking::reap_stash(RuntimeOrigin::signed(20), 11, 0)); // then - assert!(!>::contains_key(&10)); + assert!(!>::contains_key(&11)); assert!(!>::contains_key(&11)); assert!(!>::contains_key(&11)); assert!(!>::contains_key(&11)); @@ -1757,14 +1752,14 @@ fn switching_roles() { // minimal overhead. ExtBuilder::default().nominate(false).build_and_execute(|| { // Reset reward destination - for i in &[10, 20] { + for i in &[11, 21] { assert_ok!(Staking::set_payee( RuntimeOrigin::signed(*i), RewardDestination::Controller )); } - assert_eq_uvec!(validator_controllers(), vec![20, 10]); + assert_eq_uvec!(validator_controllers(), vec![21, 11]); // put some money in account that we'll use. for i in 1..7 { @@ -1773,42 +1768,42 @@ fn switching_roles() { // add 2 nominators assert_ok!(Staking::bond(RuntimeOrigin::signed(1), 2000, RewardDestination::Controller)); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(2), vec![11, 5])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(1), vec![11, 5])); assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 500, RewardDestination::Controller)); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(4), vec![21, 1])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(3), vec![21, 1])); // add a new validator candidate assert_ok!(Staking::bond(RuntimeOrigin::signed(5), 1000, RewardDestination::Controller)); - assert_ok!(Staking::validate(RuntimeOrigin::signed(6), ValidatorPrefs::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(5), ValidatorPrefs::default())); assert_ok!(Session::set_keys( - RuntimeOrigin::signed(6), + RuntimeOrigin::signed(5), SessionKeys { other: 6.into() }, vec![] )); mock::start_active_era(1); - // with current nominators 10 and 5 have the most stake - assert_eq_uvec!(validator_controllers(), vec![6, 10]); + // with current nominators 11 and 5 have the most stake + assert_eq_uvec!(validator_controllers(), vec![5, 11]); // 2 decides to be a validator. Consequences: - assert_ok!(Staking::validate(RuntimeOrigin::signed(2), ValidatorPrefs::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(1), ValidatorPrefs::default())); assert_ok!(Session::set_keys( - RuntimeOrigin::signed(2), + RuntimeOrigin::signed(1), SessionKeys { other: 2.into() }, vec![] )); // new stakes: - // 10: 1000 self vote - // 20: 1000 self vote + 250 vote - // 6 : 1000 self vote - // 2 : 2000 self vote + 250 vote. - // Winners: 20 and 2 + // 11: 1000 self vote + // 21: 1000 self vote + 250 vote + // 5 : 1000 self vote + // 1 : 2000 self vote + 250 vote. + // Winners: 21 and 1 mock::start_active_era(2); - assert_eq_uvec!(validator_controllers(), vec![2, 20]); + assert_eq_uvec!(validator_controllers(), vec![1, 21]); }); } @@ -1817,7 +1812,7 @@ fn wrong_vote_is_moot() { ExtBuilder::default() .add_staker( 61, - 60, + 61, 500, StakerStatus::Nominator(vec![ 11, 21, // good votes @@ -1829,7 +1824,7 @@ fn wrong_vote_is_moot() { mock::start_active_era(1); // new validators - assert_eq_uvec!(validator_controllers(), vec![20, 10]); + assert_eq_uvec!(validator_controllers(), vec![21, 11]); // our new voter is taken into account assert!(Staking::eras_stakers(active_era(), 11).others.iter().any(|i| i.who == 61)); @@ -2088,7 +2083,7 @@ fn phragmen_should_not_overflow() { mock::start_active_era(1); - assert_eq_uvec!(validator_controllers(), vec![4, 2]); + assert_eq_uvec!(validator_controllers(), vec![3, 5]); // We can safely convert back to values within [u64, u128]. assert!(Staking::eras_stakers(active_era(), 3).total > Votes::max_value() as Balance); @@ -2198,7 +2193,7 @@ fn unbonded_balance_is_not_slashable() { // total amount staked is slashable. assert_eq!(Staking::slashable_balance_of(&11), 1000); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 800)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 800)); // only the active portion. assert_eq!(Staking::slashable_balance_of(&11), 200); @@ -2335,7 +2330,7 @@ fn slash_in_old_span_does_not_deselect() { mock::start_active_era(2); - Staking::validate(RuntimeOrigin::signed(10), Default::default()).unwrap(); + Staking::validate(RuntimeOrigin::signed(11), Default::default()).unwrap(); assert_eq!(Staking::force_era(), Forcing::NotForcing); assert!(>::contains_key(11)); assert!(!Session::validators().contains(&11)); @@ -2376,7 +2371,7 @@ fn slash_in_old_span_does_not_deselect() { assert!(Validators::::iter().any(|(stash, _)| stash == 11)); // but it's disabled - assert!(is_disabled(10)); + assert!(is_disabled(11)); // and we are still forcing a new era assert_eq!(Staking::force_era(), Forcing::ForceNew); }); @@ -2760,7 +2755,7 @@ fn slashes_are_summed_across_spans() { assert_eq!(Balances::free_balance(21), 1900); // 21 has been force-chilled. re-signal intent to validate. - Staking::validate(RuntimeOrigin::signed(20), Default::default()).unwrap(); + Staking::validate(RuntimeOrigin::signed(21), Default::default()).unwrap(); mock::start_active_era(4); @@ -2891,8 +2886,8 @@ fn retroactive_deferred_slashes_one_before() { // unbond at slash era. mock::start_active_era(2); - assert_ok!(Staking::chill(RuntimeOrigin::signed(10))); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 100)); + assert_ok!(Staking::chill(RuntimeOrigin::signed(11))); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 100)); mock::start_active_era(3); System::reset_events(); @@ -2905,7 +2900,7 @@ fn retroactive_deferred_slashes_one_before() { mock::start_active_era(4); - assert_eq!(Staking::ledger(10).unwrap().total, 1000); + assert_eq!(Staking::ledger(11).unwrap().total, 1000); // slash happens after the next line. mock::start_active_era(5); @@ -2920,9 +2915,9 @@ fn retroactive_deferred_slashes_one_before() { )); // their ledger has already been slashed. - assert_eq!(Staking::ledger(10).unwrap().total, 900); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(10), 1000)); - assert_eq!(Staking::ledger(10).unwrap().total, 900); + assert_eq!(Staking::ledger(11).unwrap().total, 900); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(11), 1000)); + assert_eq!(Staking::ledger(11).unwrap().total, 900); }) } @@ -2947,14 +2942,14 @@ fn staker_cannot_bail_deferred_slash() { ); // now we chill - assert_ok!(Staking::chill(RuntimeOrigin::signed(100))); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(100), 500)); + assert_ok!(Staking::chill(RuntimeOrigin::signed(101))); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(101), 500)); assert_eq!(Staking::current_era().unwrap(), 1); assert_eq!(active_era(), 1); assert_eq!( - Ledger::::get(100).unwrap(), + Ledger::::get(101).unwrap(), StakingLedger { active: 0, total: 500, @@ -2984,10 +2979,10 @@ fn staker_cannot_bail_deferred_slash() { // and cannot yet unbond: assert_storage_noop!(assert!( - Staking::withdraw_unbonded(RuntimeOrigin::signed(100), 0).is_ok() + Staking::withdraw_unbonded(RuntimeOrigin::signed(101), 0).is_ok() )); assert_eq!( - Ledger::::get(100).unwrap().unlocking.into_inner(), + Ledger::::get(101).unwrap().unlocking.into_inner(), vec![UnlockChunk { era: 4u32, value: 500 as Balance }], ); @@ -3194,7 +3189,7 @@ fn slash_kicks_validators_not_nominators_and_disables_nominator_for_kicked_valid assert!(Validators::::iter().all(|(stash, _)| stash != 11)); // actually re-bond the slashed validator - assert_ok!(Staking::validate(RuntimeOrigin::signed(10), Default::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(11), Default::default())); mock::start_active_era(2); let exposure_11 = Staking::eras_stakers(active_era(), &11); @@ -3323,9 +3318,9 @@ fn slashing_independent_of_disabling_validator() { ); // the offence for validator 10 was explicitly disabled - assert!(is_disabled(10)); - // whereas validator 20 is explicitly not disabled - assert!(!is_disabled(20)); + assert!(is_disabled(11)); + // whereas validator 21 is explicitly not disabled + assert!(!is_disabled(21)); }); } @@ -3607,7 +3602,6 @@ fn test_max_nominator_rewarded_per_validator_and_cant_steal_someone_else_reward( ExtBuilder::default().build_and_execute(|| { for i in 0..=<::MaxNominatorRewardedPerValidator as Get<_>>::get() { let stash = 10_000 + i as AccountId; - let controller = 20_000 + i as AccountId; let balance = 10_000 + i as Balance; Balances::make_free_balance_be(&stash, balance); assert_ok!(Staking::bond( @@ -3615,7 +3609,7 @@ fn test_max_nominator_rewarded_per_validator_and_cant_steal_someone_else_reward( balance, RewardDestination::Stash )); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller), vec![11])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(stash), vec![11])); } mock::start_active_era(1); @@ -4110,7 +4104,7 @@ fn payout_to_any_account_works() { bond_nominator(1234, 100, vec![11]); // Update payout location - assert_ok!(Staking::set_payee(RuntimeOrigin::signed(1337), RewardDestination::Account(42))); + assert_ok!(Staking::set_payee(RuntimeOrigin::signed(1234), RewardDestination::Account(42))); // Reward Destination account doesn't exist assert_eq!(Balances::free_balance(42), 0); @@ -5061,7 +5055,7 @@ mod sorted_list_provider { ); // when account 101 renominates - assert_ok!(Staking::nominate(RuntimeOrigin::signed(100), vec![41])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(101), vec![41])); // then counts don't change assert_eq!(::VoterList::count(), pre_insert_voter_count); @@ -5084,7 +5078,7 @@ mod sorted_list_provider { assert_eq!(::VoterList::iter().collect::>(), vec![11, 21, 31]); // when account 11 re-validates - assert_ok!(Staking::validate(RuntimeOrigin::signed(10), Default::default())); + assert_ok!(Staking::validate(RuntimeOrigin::signed(11), Default::default())); // then counts don't change assert_eq!(::VoterList::count(), pre_insert_voter_count); @@ -5397,7 +5391,7 @@ fn pre_bonding_era_cannot_be_claimed() { let claimed_rewards: BoundedVec<_, _> = (start_reward_era..=last_reward_era).collect::>().try_into().unwrap(); assert_eq!( - Staking::ledger(&4).unwrap(), + Staking::ledger(&3).unwrap(), StakingLedger { stash: 3, total: 1500, @@ -5412,14 +5406,14 @@ fn pre_bonding_era_cannot_be_claimed() { mock::start_active_era(current_era); // claiming reward for last era in which validator was active works - assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(4), 3, current_era - 1)); + assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(3), 3, current_era - 1)); // consumed weight for all payout_stakers dispatches that fail let err_weight = ::WeightInfo::payout_stakers_alive_staked(0); // cannot claim rewards for an era before bonding occured as it is // already marked as claimed. assert_noop!( - Staking::payout_stakers(RuntimeOrigin::signed(4), 3, current_era - 2), + Staking::payout_stakers(RuntimeOrigin::signed(3), 3, current_era - 2), Error::::AlreadyClaimed.with_weight(err_weight) ); @@ -5429,7 +5423,7 @@ fn pre_bonding_era_cannot_be_claimed() { // make sure stakers still cannot claim rewards that they are not meant to assert_noop!( - Staking::payout_stakers(RuntimeOrigin::signed(4), 3, current_era - 2), + Staking::payout_stakers(RuntimeOrigin::signed(3), 3, current_era - 2), Error::::NotController ); @@ -5447,7 +5441,7 @@ fn reducing_history_depth_abrupt() { let last_reward_era = current_era - 1; let start_reward_era = current_era - original_history_depth; - // put some money in (stash, controller)=(3,4),(5,6). + // put some money in (stash, controller)=(3,3),(5,5). for i in 3..7 { let _ = Balances::make_free_balance_be(&i, 2000); } @@ -5455,7 +5449,7 @@ fn reducing_history_depth_abrupt() { // start current era mock::start_active_era(current_era); - // add a new candidate for being a staker. account 3 controlled by 4. + // add a new candidate for being a staker. account 3 controlled by 3. assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, RewardDestination::Controller)); // all previous era before the bonding action should be marked as @@ -5463,7 +5457,7 @@ fn reducing_history_depth_abrupt() { let claimed_rewards: BoundedVec<_, _> = (start_reward_era..=last_reward_era).collect::>().try_into().unwrap(); assert_eq!( - Staking::ledger(&4).unwrap(), + Staking::ledger(&3).unwrap(), StakingLedger { stash: 3, total: 1500, @@ -5478,7 +5472,7 @@ fn reducing_history_depth_abrupt() { mock::start_active_era(current_era); // claiming reward for last era in which validator was active works - assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(4), 3, current_era - 1)); + assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(3), 3, current_era - 1)); // next era current_era = current_era + 1; @@ -5489,7 +5483,7 @@ fn reducing_history_depth_abrupt() { HistoryDepth::set(history_depth); // claiming reward does not work anymore assert_noop!( - Staking::payout_stakers(RuntimeOrigin::signed(4), 3, current_era - 1), + Staking::payout_stakers(RuntimeOrigin::signed(3), 3, current_era - 1), Error::::NotController ); @@ -5502,7 +5496,7 @@ fn reducing_history_depth_abrupt() { let claimed_rewards: BoundedVec<_, _> = (start_reward_era..=last_reward_era).collect::>().try_into().unwrap(); assert_eq!( - Staking::ledger(&6).unwrap(), + Staking::ledger(&5).unwrap(), StakingLedger { stash: 5, total: 1200, @@ -5526,16 +5520,16 @@ fn reducing_max_unlocking_chunks_abrupt() { MaxUnlockingChunks::set(2); start_active_era(10); assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 300, RewardDestination::Staked)); - assert!(matches!(Staking::ledger(4), Some(_))); + assert!(matches!(Staking::ledger(3), Some(_))); // when staker unbonds - assert_ok!(Staking::unbond(RuntimeOrigin::signed(4), 20)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(3), 20)); // then an unlocking chunk is added at `current_era + bonding_duration` // => 10 + 3 = 13 let expected_unlocking: BoundedVec, MaxUnlockingChunks> = bounded_vec![UnlockChunk { value: 20 as Balance, era: 13 as EraIndex }]; - assert!(matches!(Staking::ledger(4), + assert!(matches!(Staking::ledger(3), Some(StakingLedger { unlocking, .. @@ -5543,11 +5537,11 @@ fn reducing_max_unlocking_chunks_abrupt() { // when staker unbonds at next era start_active_era(11); - assert_ok!(Staking::unbond(RuntimeOrigin::signed(4), 50)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(3), 50)); // then another unlock chunk is added let expected_unlocking: BoundedVec, MaxUnlockingChunks> = bounded_vec![UnlockChunk { value: 20, era: 13 }, UnlockChunk { value: 50, era: 14 }]; - assert!(matches!(Staking::ledger(4), + assert!(matches!(Staking::ledger(3), Some(StakingLedger { unlocking, .. @@ -5556,13 +5550,13 @@ fn reducing_max_unlocking_chunks_abrupt() { // when staker unbonds further start_active_era(12); // then further unbonding not possible - assert_noop!(Staking::unbond(RuntimeOrigin::signed(4), 20), Error::::NoMoreChunks); + assert_noop!(Staking::unbond(RuntimeOrigin::signed(3), 20), Error::::NoMoreChunks); // when max unlocking chunks is reduced abruptly to a low value MaxUnlockingChunks::set(1); // then unbond, rebond ops are blocked with ledger in corrupt state - assert_noop!(Staking::unbond(RuntimeOrigin::signed(4), 20), Error::::NotController); - assert_noop!(Staking::rebond(RuntimeOrigin::signed(4), 100), Error::::NotController); + assert_noop!(Staking::unbond(RuntimeOrigin::signed(3), 20), Error::::NotController); + assert_noop!(Staking::rebond(RuntimeOrigin::signed(3), 100), Error::::NotController); // reset the ledger corruption MaxUnlockingChunks::set(2); @@ -5675,13 +5669,13 @@ mod staking_interface { // without slash let _ = with_storage_layer::<(), _, _>(|| { // bond an account, can unstake - assert_eq!(Staking::bonded(&11), Some(10)); + 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(10)); + assert_eq!(Staking::bonded(&11), Some(11)); add_slash(&11); assert_ok!(::force_unstake(11)); }); @@ -5698,16 +5692,16 @@ mod staking_interface { &[Perbill::from_percent(100)], ); - assert_eq!(Staking::bonded(&11), Some(10)); + assert_eq!(Staking::bonded(&11), Some(11)); assert_noop!( - Staking::withdraw_unbonded(RuntimeOrigin::signed(10), 0), + 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(10), + RuntimeOrigin::signed(11), num_slashing_spans as u32 )); }); From b2f0ae1f64ef7800f57dd99942feef4f59a67395 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 23:07:54 +0700 Subject: [PATCH 16/60] remove println! --- frame/staking/src/tests.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index f6f8dc5dbe786..5966d32097318 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -1693,8 +1693,6 @@ fn reward_to_stake_works() { // Trigger another new era as the info are frozen before the era start. mock::start_active_era(2); - println!("{:?} / 2 = {:?}", total_payout_0, total_payout_0 / 2); - // -- new infos assert_eq!(Staking::eras_stakers(active_era(), 11).total, _11_balance); assert_eq!(Staking::eras_stakers(active_era(), 21).total, _21_balance); From 9eeb64743970b8f62078ecc0402f02769dc50195 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 23:13:34 +0700 Subject: [PATCH 17/60] fix rewards_should_work --- frame/staking/src/tests.rs | 52 +++++++++++++++----------------------- 1 file changed, 20 insertions(+), 32 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 5966d32097318..828f3a3c19977 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -257,11 +257,8 @@ fn change_controller_already_paired_once_stash() { #[test] fn rewards_should_work() { ExtBuilder::default().nominate(true).session_per_era(3).build_and_execute(|| { - let init_balance_10 = Balances::total_balance(&10); let init_balance_11 = Balances::total_balance(&11); - let init_balance_20 = Balances::total_balance(&20); let init_balance_21 = Balances::total_balance(&21); - let init_balance_100 = Balances::total_balance(&100); let init_balance_101 = Balances::total_balance(&101); // Set payees @@ -281,11 +278,8 @@ fn rewards_should_work() { start_session(1); assert_eq_uvec!(Session::validators(), vec![11, 21]); - assert_eq!(Balances::total_balance(&10), init_balance_10); assert_eq!(Balances::total_balance(&11), init_balance_11); - assert_eq!(Balances::total_balance(&20), init_balance_20); assert_eq!(Balances::total_balance(&21), init_balance_21); - assert_eq!(Balances::total_balance(&100), init_balance_100); assert_eq!(Balances::total_balance(&101), init_balance_101); assert_eq!( Staking::eras_reward_points(active_era()), @@ -294,10 +288,10 @@ fn rewards_should_work() { individual: vec![(11, 100), (21, 50)].into_iter().collect(), } ); - let part_for_10 = Perbill::from_rational::(1000, 1125); - let part_for_20 = Perbill::from_rational::(1000, 1375); - let part_for_100_from_10 = Perbill::from_rational::(125, 1125); - let part_for_100_from_20 = Perbill::from_rational::(375, 1375); + let part_for_11 = Perbill::from_rational::(1000, 1125); + let part_for_21 = Perbill::from_rational::(1000, 1375); + let part_for_101_from_11 = Perbill::from_rational::(125, 1125); + let part_for_101_from_21 = Perbill::from_rational::(375, 1375); start_session(2); start_session(3); @@ -315,25 +309,22 @@ fn rewards_should_work() { mock::make_all_reward_payment(0); assert_eq_error_rate!( - Balances::total_balance(&10), - init_balance_10 + part_for_10 * total_payout_0 * 2 / 3, + Balances::total_balance(&11), + init_balance_11 + part_for_11 * total_payout_0 * 2 / 3, 2, ); - assert_eq_error_rate!(Balances::total_balance(&11), init_balance_11, 2); assert_eq_error_rate!( - Balances::total_balance(&20), - init_balance_20 + part_for_20 * total_payout_0 * 1 / 3, + Balances::total_balance(&21), + init_balance_21 + part_for_21 * total_payout_0 * 1 / 3, 2, ); - assert_eq_error_rate!(Balances::total_balance(&21), init_balance_21, 2); assert_eq_error_rate!( - Balances::total_balance(&100), - init_balance_100 + - part_for_100_from_10 * total_payout_0 * 2 / 3 + - part_for_100_from_20 * total_payout_0 * 1 / 3, + 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, 2 ); - assert_eq_error_rate!(Balances::total_balance(&101), init_balance_101, 2); assert_eq_uvec!(Session::validators(), vec![11, 21]); Pallet::::reward_by_ids(vec![(11, 1)]); @@ -357,25 +348,22 @@ fn rewards_should_work() { mock::make_all_reward_payment(1); assert_eq_error_rate!( - Balances::total_balance(&10), - init_balance_10 + part_for_10 * (total_payout_0 * 2 / 3 + total_payout_1), + Balances::total_balance(&11), + init_balance_11 + part_for_11 * (total_payout_0 * 2 / 3 + total_payout_1), 2, ); - assert_eq_error_rate!(Balances::total_balance(&11), init_balance_11, 2); assert_eq_error_rate!( - Balances::total_balance(&20), - init_balance_20 + part_for_20 * total_payout_0 * 1 / 3, + Balances::total_balance(&21), + init_balance_21 + part_for_21 * total_payout_0 * 1 / 3, 2, ); - assert_eq_error_rate!(Balances::total_balance(&21), init_balance_21, 2); assert_eq_error_rate!( - Balances::total_balance(&100), - init_balance_100 + - part_for_100_from_10 * (total_payout_0 * 2 / 3 + total_payout_1) + - part_for_100_from_20 * total_payout_0 * 1 / 3, + 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, 2 ); - assert_eq_error_rate!(Balances::total_balance(&101), init_balance_101, 2); }); } From 93c012f1f3a1b086e23a5ad26c74da430fba4fcd Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 23:18:58 +0700 Subject: [PATCH 18/60] fix test_payout_stakers --- frame/staking/src/tests.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 828f3a3c19977..df892decf6f81 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -3665,18 +3665,18 @@ fn test_payout_stakers() { // Top 64 nominators of validator 11 automatically paid out, including the validator // Validator payout goes to controller. - assert!(Balances::free_balance(&10) > balance); + assert!(Balances::free_balance(&11) > balance); for i in 36..100 { - assert!(Balances::free_balance(&(100 + i)) > balance + i as Balance); + assert!(Balances::free_balance(&(1000 + i)) > balance + i as Balance); } // The bottom 36 do not for i in 0..36 { - assert_eq!(Balances::free_balance(&(100 + i)), balance + i as Balance); + assert_eq!(Balances::free_balance(&(1000 + i)), balance + i as Balance); } // We track rewards in `claimed_rewards` vec assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -3707,7 +3707,7 @@ fn test_payout_stakers() { // We track rewards in `claimed_rewards` vec assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -3740,7 +3740,7 @@ fn test_payout_stakers() { expected_last_reward_era )); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, @@ -3755,7 +3755,7 @@ fn test_payout_stakers() { assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(1337), 11, 23)); assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(1337), 11, 42)); assert_eq!( - Staking::ledger(&10), + Staking::ledger(&11), Some(StakingLedger { stash: 11, total: 1000, From efec776487bf1f853b207405120de94c9df8816a Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 23:27:45 +0700 Subject: [PATCH 19/60] fix bond benchmark --- frame/staking/src/benchmarking.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index c78c9bd37a821..68fc035405828 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -216,15 +216,13 @@ const USER_SEED: u32 = 999666; benchmarks! { bond { let stash = create_funded_user::("stash", USER_SEED, 100); - let controller = create_funded_user::("controller", USER_SEED, 100); - let controller_lookup = T::Lookup::unlookup(controller.clone()); let reward_destination = RewardDestination::Staked; let amount = T::Currency::minimum_balance() * 10u32.into(); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash.clone()), controller_lookup, amount, reward_destination) + }: _(RawOrigin::Signed(stash.clone()), amount, reward_destination) verify { assert!(Bonded::::contains_key(stash)); - assert!(Ledger::::contains_key(controller)); + assert!(Ledger::::contains_key(stash)); } bond_extra { From b3a1e3f480abbd816903ff172a5a7c50a32c96ef Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 3 May 2023 23:36:12 +0700 Subject: [PATCH 20/60] clone --- frame/staking/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 68fc035405828..84a7f653c6d67 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -221,7 +221,7 @@ benchmarks! { whitelist_account!(stash); }: _(RawOrigin::Signed(stash.clone()), amount, reward_destination) verify { - assert!(Bonded::::contains_key(stash)); + assert!(Bonded::::contains_key(stash.clone())); assert!(Ledger::::contains_key(stash)); } From ceb863707ad6816c921bdb9950c5a0f32992c069 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 00:24:37 +0700 Subject: [PATCH 21/60] rm unused import --- frame/session/benchmarking/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs index f06b0e2e4e908..2f2629680aa2f 100644 --- a/frame/session/benchmarking/src/lib.rs +++ b/frame/session/benchmarking/src/lib.rs @@ -34,8 +34,7 @@ use frame_support::{ use frame_system::RawOrigin; use pallet_session::{historical::Pallet as Historical, Pallet as Session, *}; use pallet_staking::{ - benchmarking::create_validator_with_nominators, testing_utils::create_validators, - RewardDestination, + benchmarking::create_validator_with_nominators, testing_utils::create_validators }; const MAX_VALIDATORS: u32 = 1000; From 24f8dd0da4da50ff9788047ba436c11a9ed827c9 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 00:30:55 +0700 Subject: [PATCH 22/60] rm unused var --- frame/offences/benchmarking/src/lib.rs | 2 -- frame/session/benchmarking/src/lib.rs | 2 +- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 97196f2d41460..9b0df650892cc 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -132,8 +132,6 @@ fn create_offender(n: u32, nominators: u32) -> Result, &' account("nominator stash", n * MAX_NOMINATORS + i, SEED); let nominator_controller: T::AccountId = account("nominator controller", n * MAX_NOMINATORS + i, SEED); - let nominator_controller_lookup: LookupSourceOf = - T::Lookup::unlookup(nominator_controller.clone()); T::Currency::make_free_balance_be(&nominator_stash, free_amount); Staking::::bond( diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs index 2f2629680aa2f..97fedc652e0eb 100644 --- a/frame/session/benchmarking/src/lib.rs +++ b/frame/session/benchmarking/src/lib.rs @@ -34,7 +34,7 @@ use frame_support::{ use frame_system::RawOrigin; use pallet_session::{historical::Pallet as Historical, Pallet as Session, *}; use pallet_staking::{ - benchmarking::create_validator_with_nominators, testing_utils::create_validators + benchmarking::create_validator_with_nominators, testing_utils::create_validators, }; const MAX_VALIDATORS: u32 = 1000; From c3bf9844f38a38ffec7da5b6540e458dffb56a28 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 00:35:31 +0700 Subject: [PATCH 23/60] rm controller from create_offender --- frame/offences/benchmarking/src/lib.rs | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index 9b0df650892cc..ac544225dee43 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -108,7 +108,7 @@ fn bond_amount() -> BalanceOf { fn create_offender(n: u32, nominators: u32) -> Result, &'static str> { let stash: T::AccountId = account("stash", n, SEED); let controller: T::AccountId = account("controller", n, SEED); - let controller_lookup: LookupSourceOf = T::Lookup::unlookup(controller.clone()); + let stash_lookup: LookupSourceOf = T::Lookup::unlookup(stash.clone()); let reward_destination = RewardDestination::Staked; let amount = bond_amount::(); // add twice as much balance to prevent the account from being killed. @@ -122,7 +122,7 @@ fn create_offender(n: u32, nominators: u32) -> Result, &' let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; - Staking::::validate(RawOrigin::Signed(controller.clone()).into(), validator_prefs)?; + Staking::::validate(RawOrigin::Signed(stash.clone()).into(), validator_prefs)?; let mut individual_exposures = vec![]; let mut nominator_stashes = vec![]; @@ -130,8 +130,6 @@ fn create_offender(n: u32, nominators: u32) -> Result, &' for i in 0..nominators { let nominator_stash: T::AccountId = account("nominator stash", n * MAX_NOMINATORS + i, SEED); - let nominator_controller: T::AccountId = - account("nominator controller", n * MAX_NOMINATORS + i, SEED); T::Currency::make_free_balance_be(&nominator_stash, free_amount); Staking::::bond( @@ -140,9 +138,9 @@ fn create_offender(n: u32, nominators: u32) -> Result, &' reward_destination.clone(), )?; - let selected_validators: Vec> = vec![controller_lookup.clone()]; + let selected_validators: Vec> = vec![stash_lookup.clone()]; Staking::::nominate( - RawOrigin::Signed(nominator_controller.clone()).into(), + RawOrigin::Signed(nominator_stash.clone()).into(), selected_validators, )?; From dd857e69d359fd625708c0dc488bb71d2ddc57ac Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 12:26:07 +0700 Subject: [PATCH 24/60] fix GenesisConfig stakers --- bin/node/testing/src/genesis.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/bin/node/testing/src/genesis.rs b/bin/node/testing/src/genesis.rs index b8c80aeb116a9..d542bb29c2539 100644 --- a/bin/node/testing/src/genesis.rs +++ b/bin/node/testing/src/genesis.rs @@ -65,9 +65,9 @@ pub fn config_endowed(code: Option<&[u8]>, extra_endowed: Vec) -> Gen }, staking: StakingConfig { stakers: vec![ - (dave(), alice(), 111 * DOLLARS, StakerStatus::Validator), - (eve(), bob(), 100 * DOLLARS, StakerStatus::Validator), - (ferdie(), charlie(), 100 * DOLLARS, StakerStatus::Validator), + (dave(), dave(), 111 * DOLLARS, StakerStatus::Validator), + (eve(), eve(), 100 * DOLLARS, StakerStatus::Validator), + (ferdie(), ferdie(), 100 * DOLLARS, StakerStatus::Validator), ], validator_count: 3, minimum_validator_count: 0, From 6fb61f428b58ac1b24790cf93a690e92b556a87a Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 12:36:09 +0700 Subject: [PATCH 25/60] fix controllers in consensus pallets --- frame/babe/src/mock.rs | 4 ++-- frame/beefy/src/mock.rs | 4 ++-- frame/grandpa/src/mock.rs | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index bccdb42c71ac3..a6e82c1149619 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -369,10 +369,10 @@ pub fn new_test_ext_raw_authorities(authorities: Vec) -> sp_io::Tes .assimilate_storage(&mut t) .unwrap(); - // controllers are the index + 1000 + // controllers are same as stash let stakers: Vec<_> = (0..authorities.len()) .map(|i| { - (i as u64, i as u64 + 1000, 10_000, pallet_staking::StakerStatus::::Validator) + (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator) }) .collect(); diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index ceb95263e2436..df4b983ffdcde 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -284,10 +284,10 @@ pub fn new_test_ext_raw_authorities(authorities: Vec) -> TestExternalit .assimilate_storage(&mut t) .unwrap(); - // controllers are the index + 1000 + // controllers are same as stash let stakers: Vec<_> = (0..authorities.len()) .map(|i| { - (i as u64, i as u64 + 1000, 10_000, pallet_staking::StakerStatus::::Validator) + (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator) }) .collect(); diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index a7359f6896db7..ae0fe2e997ff7 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -284,10 +284,10 @@ pub fn new_test_ext_raw_authorities(authorities: AuthorityList) -> sp_io::TestEx .assimilate_storage(&mut t) .unwrap(); - // controllers are the index + 1000 + // controllers are the same as stash let stakers: Vec<_> = (0..authorities.len()) .map(|i| { - (i as u64, i as u64 + 1000, 10_000, pallet_staking::StakerStatus::::Validator) + (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator) }) .collect(); From 17bba2cd959a0e29e1bd09817ba3c8366f1140d1 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 12:53:22 +0700 Subject: [PATCH 26/60] fix unqiue controller in chain_spec --- bin/node/cli/src/chain_spec.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bin/node/cli/src/chain_spec.rs b/bin/node/cli/src/chain_spec.rs index 4732e12f9c76e..85a08e71cc5a9 100644 --- a/bin/node/cli/src/chain_spec.rs +++ b/bin/node/cli/src/chain_spec.rs @@ -274,7 +274,7 @@ pub fn testnet_genesis( let mut rng = rand::thread_rng(); let stakers = initial_authorities .iter() - .map(|x| (x.0.clone(), x.1.clone(), STASH, StakerStatus::Validator)) + .map(|x| (x.0.clone(), x.0.clone(), STASH, StakerStatus::Validator)) .chain(initial_nominators.iter().map(|x| { use rand::{seq::SliceRandom, Rng}; let limit = (MaxNominations::get() as usize).min(initial_authorities.len()); From 465bc131123147b60bf66b8b53bca355533bb97c Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 12:55:35 +0700 Subject: [PATCH 27/60] fmt --- frame/babe/src/mock.rs | 4 +--- frame/beefy/src/mock.rs | 4 +--- frame/grandpa/src/mock.rs | 4 +--- 3 files changed, 3 insertions(+), 9 deletions(-) diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index a6e82c1149619..478323a8364ff 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -371,9 +371,7 @@ pub fn new_test_ext_raw_authorities(authorities: Vec) -> sp_io::Tes // controllers are same as stash let stakers: Vec<_> = (0..authorities.len()) - .map(|i| { - (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator) - }) + .map(|i| (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator)) .collect(); let staking_config = pallet_staking::GenesisConfig:: { diff --git a/frame/beefy/src/mock.rs b/frame/beefy/src/mock.rs index df4b983ffdcde..6b6ffd6751fbe 100644 --- a/frame/beefy/src/mock.rs +++ b/frame/beefy/src/mock.rs @@ -286,9 +286,7 @@ pub fn new_test_ext_raw_authorities(authorities: Vec) -> TestExternalit // controllers are same as stash let stakers: Vec<_> = (0..authorities.len()) - .map(|i| { - (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator) - }) + .map(|i| (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator)) .collect(); let staking_config = pallet_staking::GenesisConfig:: { diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index ae0fe2e997ff7..ffc566ffe74de 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -286,9 +286,7 @@ pub fn new_test_ext_raw_authorities(authorities: AuthorityList) -> sp_io::TestEx // controllers are the same as stash let stakers: Vec<_> = (0..authorities.len()) - .map(|i| { - (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator) - }) + .map(|i| (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator)) .collect(); let staking_config = pallet_staking::GenesisConfig:: { From 0d5adaa48bcf1d7f46e4e7ed58e7e53161387fcb Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 13:07:39 +0700 Subject: [PATCH 28/60] fix create_offender --- frame/offences/benchmarking/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/offences/benchmarking/src/lib.rs b/frame/offences/benchmarking/src/lib.rs index ac544225dee43..e7fc39657a190 100644 --- a/frame/offences/benchmarking/src/lib.rs +++ b/frame/offences/benchmarking/src/lib.rs @@ -107,7 +107,6 @@ fn bond_amount() -> BalanceOf { fn create_offender(n: u32, nominators: u32) -> Result, &'static str> { let stash: T::AccountId = account("stash", n, SEED); - let controller: T::AccountId = account("controller", n, SEED); let stash_lookup: LookupSourceOf = T::Lookup::unlookup(stash.clone()); let reward_destination = RewardDestination::Staked; let amount = bond_amount::(); @@ -153,7 +152,7 @@ fn create_offender(n: u32, nominators: u32) -> Result, &' let current_era = 0u32; Staking::::add_era_stakers(current_era, stash.clone(), exposure); - Ok(Offender { controller, stash, nominator_stashes }) + Ok(Offender { controller: stash.clone(), stash, nominator_stashes }) } fn make_offenders( From 7b978ea070334a83d7556248e789e9038f7f8c4a Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 13:42:19 +0700 Subject: [PATCH 29/60] fix set_controller benchmark --- frame/staking/src/benchmarking.rs | 15 ++++++++++++++- frame/staking/src/testing_utils.rs | 7 ++++--- 2 files changed, 18 insertions(+), 4 deletions(-) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 84a7f653c6d67..b2a21ff5bbeb6 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -467,7 +467,20 @@ benchmarks! { } set_controller { - let (stash, _) = create_stash_controller_inc::(USER_SEED, 100, Default::default())?; + let (stash, ctlr) = create_stash_controller_inc::(9000, 100, Default::default())?; + + // update ledger to be a *different* controller to stash + if let Some(l) = Ledger::::take(&stash) { + >::insert(&ctlr, l); + } + // update bonded account to be unique controller + >::insert(&stash, &ctlr); + + // ensure `ctlr` is the currently stored controller + assert!(!Ledger::::contains_key(&stash)); + assert!(Ledger::::contains_key(&ctlr)); + assert_eq!(Bonded::::get(&stash), Some(ctlr.clone())); + whitelist_account!(stash); }: _(RawOrigin::Signed(stash.clone())) verify { diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index df73934c1e4f7..d06b13267e44a 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -88,10 +88,11 @@ pub fn create_stash_controller_inc( balance_factor: u32, destination: RewardDestination, ) -> Result<(T::AccountId, T::AccountId), &'static str> { - let staker = create_funded_user::("stash", n, balance_factor); + let stash = create_funded_user::("stash", n, balance_factor); + let unused_controller = create_funded_user::("controller", n + 1, balance_factor); let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); - Staking::::bond(RawOrigin::Signed(staker.clone()).into(), amount, destination)?; - Ok((staker.clone(), staker)) + Staking::::bond(RawOrigin::Signed(stash.clone()).into(), amount, destination)?; + Ok((stash, unused_controller)) } /// Create a stash and controller pair with fixed balance. From bd91aae623d49cc80b290514ead309525ad85c7f Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 4 May 2023 13:49:17 +0700 Subject: [PATCH 30/60] add TODO --- frame/staking/src/benchmarking.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index b2a21ff5bbeb6..e81f175f25ae4 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -68,6 +68,9 @@ pub fn add_slashing_spans(who: &T::AccountId, spans: u32) { // This function clears all existing validators and nominators from the set, and generates one new // validator being nominated by n nominators, and returns the validator stash account and the // nominators' stash and controller. It also starts an era and creates pending payouts. +// +// TODO: duplicate the creation functions used here and replace them to have the unique controller +// scenario. pub fn create_validator_with_nominators( n: u32, upper_bound: u32, From ada3aeb52c0e65c8e49b4129f88c628de655630b Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 11:57:57 +0800 Subject: [PATCH 31/60] create_unique_stash_controller --- frame/session/benchmarking/src/lib.rs | 6 ++++-- frame/staking/src/benchmarking.rs | 28 ++++++++++++++------------- frame/staking/src/testing_utils.rs | 13 ++++++++++--- 3 files changed, 29 insertions(+), 18 deletions(-) diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs index 97fedc652e0eb..eb8eef8e5d5c3 100644 --- a/frame/session/benchmarking/src/lib.rs +++ b/frame/session/benchmarking/src/lib.rs @@ -57,7 +57,8 @@ benchmarks! { let (v_stash, _) = create_validator_with_nominators::( n, ::MaxNominations::get(), - false + false, + true )?; let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; @@ -73,7 +74,8 @@ benchmarks! { let (v_stash, _) = create_validator_with_nominators::( n, ::MaxNominations::get(), - false + false, + true, )?; let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; let keys = T::Keys::decode(&mut TrailingZeroInput::zeroes()).unwrap(); diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index e81f175f25ae4..5bc96c20c2e08 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -75,13 +75,19 @@ pub fn create_validator_with_nominators( n: u32, upper_bound: u32, dead: bool, + unique_controller: bool, ) -> Result<(T::AccountId, Vec<(T::AccountId, T::AccountId)>), &'static str> { // Clean up any existing state. clear_validators_and_nominators::(); let mut points_total = 0; let mut points_individual = Vec::new(); - let (v_stash, v_controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; + let (v_stash, v_controller) = if unique_controller { + create_unique_stash_controller::(0, 100, RewardDestination::Staked)? + } else { + create_stash_controller::(0, 100, RewardDestination::Staked)? + }; + let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; Staking::::validate(RawOrigin::Signed(v_controller).into(), validator_prefs)?; @@ -470,16 +476,8 @@ benchmarks! { } set_controller { - let (stash, ctlr) = create_stash_controller_inc::(9000, 100, Default::default())?; - - // update ledger to be a *different* controller to stash - if let Some(l) = Ledger::::take(&stash) { - >::insert(&ctlr, l); - } - // update bonded account to be unique controller - >::insert(&stash, &ctlr); - - // ensure `ctlr` is the currently stored controller + let (stash, ctlr) = create_unique_stash_controller::(9000, 100, Default::default())?; + // ensure `ctlr` is the currently stored controller. assert!(!Ledger::::contains_key(&stash)); assert!(Ledger::::contains_key(&ctlr)); assert_eq!(Bonded::::get(&stash), Some(ctlr.clone())); @@ -561,7 +559,8 @@ benchmarks! { let (validator, nominators) = create_validator_with_nominators::( n, T::MaxNominatorRewardedPerValidator::get() as u32, - true + true, + true, )?; let current_era = CurrentEra::::get().unwrap(); @@ -593,7 +592,8 @@ benchmarks! { let (validator, nominators) = create_validator_with_nominators::( n, T::MaxNominatorRewardedPerValidator::get() as u32, - false + false, + true, )?; let current_era = CurrentEra::::get().unwrap(); @@ -987,6 +987,7 @@ mod tests { n, <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, + false, ) .unwrap(); @@ -1015,6 +1016,7 @@ mod tests { n, <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, + false, ) .unwrap(); diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index d06b13267e44a..de48326d42dfe 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -83,16 +83,23 @@ pub fn create_stash_controller( } /// Create a unique stash and controller pair. -pub fn create_stash_controller_inc( +pub fn create_unique_stash_controller( n: u32, balance_factor: u32, destination: RewardDestination, ) -> Result<(T::AccountId, T::AccountId), &'static str> { let stash = create_funded_user::("stash", n, balance_factor); - let unused_controller = create_funded_user::("controller", n + 1, balance_factor); + let controller = create_funded_user::("controller", n + 1, balance_factor); let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); Staking::::bond(RawOrigin::Signed(stash.clone()).into(), amount, destination)?; - Ok((stash, unused_controller)) + // update ledger to be a *different* controller to stash + if let Some(l) = Ledger::::take(&stash) { + >::insert(&controller, l); + } + // update bonded account to be unique controller + >::insert(&stash, &controller); + + Ok((stash, controller)) } /// Create a stash and controller pair with fixed balance. From d63f28f10efee86fa691a6a1888048068ca6a76a Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 12:41:35 +0800 Subject: [PATCH 32/60] staking benchmarks working --- frame/staking/src/benchmarking.rs | 30 ++++++++++++++++++++---------- frame/staking/src/testing_utils.rs | 9 ++++++++- 2 files changed, 28 insertions(+), 11 deletions(-) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 5bc96c20c2e08..ef7a1c8e528a0 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -68,14 +68,12 @@ pub fn add_slashing_spans(who: &T::AccountId, spans: u32) { // This function clears all existing validators and nominators from the set, and generates one new // validator being nominated by n nominators, and returns the validator stash account and the // nominators' stash and controller. It also starts an era and creates pending payouts. -// -// TODO: duplicate the creation functions used here and replace them to have the unique controller -// scenario. pub fn create_validator_with_nominators( n: u32, upper_bound: u32, - dead: bool, + dead_controller: bool, unique_controller: bool, + destination: RewardDestination, ) -> Result<(T::AccountId, Vec<(T::AccountId, T::AccountId)>), &'static str> { // Clean up any existing state. clear_validators_and_nominators::(); @@ -83,9 +81,9 @@ pub fn create_validator_with_nominators( let mut points_individual = Vec::new(); let (v_stash, v_controller) = if unique_controller { - create_unique_stash_controller::(0, 100, RewardDestination::Staked)? + create_unique_stash_controller::(0, 100, destination.clone(), false)? } else { - create_stash_controller::(0, 100, RewardDestination::Staked)? + create_stash_controller::(0, 100, destination.clone())? }; let validator_prefs = @@ -101,10 +99,10 @@ pub fn create_validator_with_nominators( // Give the validator n nominators, but keep total users in the system the same. for i in 0..upper_bound { - let (n_stash, n_controller) = if !dead { - create_stash_controller::(u32::MAX - i, 100, RewardDestination::Staked)? + let (n_stash, n_controller) = if !dead_controller { + create_stash_controller::(u32::MAX - i, 100, destination.clone())? } else { - create_stash_and_dead_payee::(u32::MAX - i, 100)? + create_unique_stash_controller::(u32::MAX - i, 100, destination.clone(), true)? }; if i < n { Staking::::nominate( @@ -476,7 +474,7 @@ benchmarks! { } set_controller { - let (stash, ctlr) = create_unique_stash_controller::(9000, 100, Default::default())?; + let (stash, ctlr) = create_unique_stash_controller::(9000, 100, Default::default(), false)?; // ensure `ctlr` is the currently stored controller. assert!(!Ledger::::contains_key(&stash)); assert!(Ledger::::contains_key(&ctlr)); @@ -561,6 +559,7 @@ benchmarks! { T::MaxNominatorRewardedPerValidator::get() as u32, true, true, + RewardDestination::Controller, )?; let current_era = CurrentEra::::get().unwrap(); @@ -577,6 +576,14 @@ benchmarks! { }: payout_stakers(RawOrigin::Signed(caller), validator, current_era) verify { let balance_after = T::Currency::free_balance(&validator_controller); + println!("{:?}", balance_before); + println!("{:?}", balance_after); + + + ensure!( + balance_before < balance_after, + "Balance of validator controller should have increased after payout.", + ); ensure!( balance_before < balance_after, "Balance of validator controller should have increased after payout.", @@ -594,6 +601,7 @@ benchmarks! { T::MaxNominatorRewardedPerValidator::get() as u32, false, true, + RewardDestination::Staked, )?; let current_era = CurrentEra::::get().unwrap(); @@ -988,6 +996,7 @@ mod tests { <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, false, + RewardDestination::Staked, ) .unwrap(); @@ -1017,6 +1026,7 @@ mod tests { <::MaxNominatorRewardedPerValidator as Get<_>>::get(), false, false, + RewardDestination::Staked, ) .unwrap(); diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index de48326d42dfe..24644c3c2cdcb 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -87,11 +87,18 @@ pub fn create_unique_stash_controller( n: u32, balance_factor: u32, destination: RewardDestination, + dead_controller: bool, ) -> Result<(T::AccountId, T::AccountId), &'static str> { let stash = create_funded_user::("stash", n, balance_factor); - let controller = create_funded_user::("controller", n + 1, balance_factor); + + let controller = if dead_controller { + create_funded_user::("controller", n, 0) + } else { + create_funded_user::("controller", n, balance_factor) + }; let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); Staking::::bond(RawOrigin::Signed(stash.clone()).into(), amount, destination)?; + // update ledger to be a *different* controller to stash if let Some(l) = Ledger::::take(&stash) { >::insert(&controller, l); From 36fe69e341116b7c2a82243f2b07cf9d488859fd Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 12:46:14 +0800 Subject: [PATCH 33/60] fmt --- frame/staking/src/testing_utils.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/testing_utils.rs b/frame/staking/src/testing_utils.rs index 24644c3c2cdcb..28e08230d701d 100644 --- a/frame/staking/src/testing_utils.rs +++ b/frame/staking/src/testing_utils.rs @@ -98,7 +98,7 @@ pub fn create_unique_stash_controller( }; let amount = T::Currency::minimum_balance() * (balance_factor / 10).max(1).into(); Staking::::bond(RawOrigin::Signed(stash.clone()).into(), amount, destination)?; - + // update ledger to be a *different* controller to stash if let Some(l) = Ledger::::take(&stash) { >::insert(&controller, l); From e74e8df6e6debd4fb99dfb41ae3f797ee699ce29 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 12:53:31 +0800 Subject: [PATCH 34/60] fix args --- frame/session/benchmarking/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs index eb8eef8e5d5c3..a94affb32029c 100644 --- a/frame/session/benchmarking/src/lib.rs +++ b/frame/session/benchmarking/src/lib.rs @@ -58,7 +58,8 @@ benchmarks! { n, ::MaxNominations::get(), false, - true + true, + RewardDestination::Staked, )?; let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; @@ -76,6 +77,7 @@ benchmarks! { ::MaxNominations::get(), false, true, + RewardDestination::Staked, )?; let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; let keys = T::Keys::decode(&mut TrailingZeroInput::zeroes()).unwrap(); From e5ce360968e1ec9a0702332d6d1b5a50221cf044 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 12:54:07 +0800 Subject: [PATCH 35/60] rm println --- frame/staking/src/benchmarking.rs | 4 ---- 1 file changed, 4 deletions(-) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index ef7a1c8e528a0..785357e0870a6 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -576,10 +576,6 @@ benchmarks! { }: payout_stakers(RawOrigin::Signed(caller), validator, current_era) verify { let balance_after = T::Currency::free_balance(&validator_controller); - println!("{:?}", balance_before); - println!("{:?}", balance_after); - - ensure!( balance_before < balance_after, "Balance of validator controller should have increased after payout.", From d8b0012f1291bec9c49f4f8d4e01efc24644e476 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 12:54:53 +0800 Subject: [PATCH 36/60] import --- frame/session/benchmarking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs index a94affb32029c..dcda36e4f4d99 100644 --- a/frame/session/benchmarking/src/lib.rs +++ b/frame/session/benchmarking/src/lib.rs @@ -34,7 +34,7 @@ use frame_support::{ use frame_system::RawOrigin; use pallet_session::{historical::Pallet as Historical, Pallet as Session, *}; use pallet_staking::{ - benchmarking::create_validator_with_nominators, testing_utils::create_validators, + benchmarking::create_validator_with_nominators, testing_utils::create_validators, RewardDestination }; const MAX_VALIDATORS: u32 = 1000; From 0009a84bcb134c755dd9fc3d3038174ac0e241cb Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 12:54:59 +0800 Subject: [PATCH 37/60] import --- frame/session/benchmarking/src/lib.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/session/benchmarking/src/lib.rs b/frame/session/benchmarking/src/lib.rs index dcda36e4f4d99..a7e326fb27ac3 100644 --- a/frame/session/benchmarking/src/lib.rs +++ b/frame/session/benchmarking/src/lib.rs @@ -34,7 +34,8 @@ use frame_support::{ use frame_system::RawOrigin; use pallet_session::{historical::Pallet as Historical, Pallet as Session, *}; use pallet_staking::{ - benchmarking::create_validator_with_nominators, testing_utils::create_validators, RewardDestination + benchmarking::create_validator_with_nominators, testing_utils::create_validators, + RewardDestination, }; const MAX_VALIDATORS: u32 = 1000; From 177896ea8e7f3eab8d9b8f16451b2fa8c53a43c6 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 13:22:28 +0800 Subject: [PATCH 38/60] fix fast unstake tests --- frame/fast-unstake/src/mock.rs | 16 ++---- frame/fast-unstake/src/tests.rs | 90 ++++++++++++++++----------------- 2 files changed, 50 insertions(+), 56 deletions(-) diff --git a/frame/fast-unstake/src/mock.rs b/frame/fast-unstake/src/mock.rs index efa089cb79c09..d75c893807990 100644 --- a/frame/fast-unstake/src/mock.rs +++ b/frame/fast-unstake/src/mock.rs @@ -234,11 +234,11 @@ impl Default for ExtBuilder { fn default() -> Self { Self { unexposed: vec![ - (1, 2, 7 + 100), - (3, 4, 7 + 100), - (5, 6, 7 + 100), - (7, 8, 7 + 100), - (9, 10, 7 + 100), + (1, 1, 7 + 100), + (3, 3, 7 + 100), + (5, 5, 7 + 100), + (7, 7, 7 + 100), + (9, 9, 7 + 100), ], } } @@ -290,12 +290,6 @@ impl ExtBuilder { .clone() .into_iter() .map(|(stash, _, balance)| (stash, balance * 2)) - .chain( - self.unexposed - .clone() - .into_iter() - .map(|(_, ctrl, balance)| (ctrl, balance * 2)), - ) .chain(validators_range.clone().map(|x| (x, 7 + 100))) .chain(nominators_range.clone().map(|x| (x, 7 + 100))) .collect::>(), diff --git a/frame/fast-unstake/src/tests.rs b/frame/fast-unstake/src/tests.rs index 3fa7b032ce12b..c51c817ec6a74 100644 --- a/frame/fast-unstake/src/tests.rs +++ b/frame/fast-unstake/src/tests.rs @@ -37,7 +37,7 @@ fn register_works() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); // Controller account registers for fast unstake. - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); // Ensure stash is in the queue. assert_ne!(Queue::::get(1), None); }); @@ -52,7 +52,7 @@ fn register_insufficient_funds_fails() { // Controller account registers for fast unstake. assert_noop!( - FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2)), + FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)), BalancesError::::InsufficientBalance, ); @@ -65,7 +65,7 @@ fn register_insufficient_funds_fails() { fn register_disabled_fails() { ExtBuilder::default().build_and_execute(|| { assert_noop!( - FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2)), + FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)), Error::::CallNotAllowed ); }); @@ -81,7 +81,7 @@ fn cannot_register_if_not_bonded() { } // Attempt to fast unstake. assert_noop!( - FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)), + FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2)), Error::::NotController ); }); @@ -95,7 +95,7 @@ fn cannot_register_if_in_queue() { Queue::::insert(1, 10); // Cannot re-register, already in queue assert_noop!( - FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2)), + FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)), Error::::AlreadyQueued ); }); @@ -112,7 +112,7 @@ fn cannot_register_if_head() { }); // Controller attempts to regsiter assert_noop!( - FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2)), + FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)), Error::::AlreadyHead ); }); @@ -123,10 +123,10 @@ fn cannot_register_if_has_unlocking_chunks() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); // Start unbonding half of staked tokens - assert_ok!(Staking::unbond(RuntimeOrigin::signed(2), 50_u128)); + assert_ok!(Staking::unbond(RuntimeOrigin::signed(1), 50_u128)); // Cannot register for fast unstake with unlock chunks active assert_noop!( - FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2)), + FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)), Error::::NotFullyBonded ); }); @@ -140,11 +140,11 @@ fn deregister_works() { assert_eq!(::Currency::reserved_balance(&1), 0); // Controller account registers for fast unstake. - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); assert_eq!(::Currency::reserved_balance(&1), Deposit::get()); // Controller then changes mind and deregisters. - assert_ok!(FastUnstake::deregister(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::deregister(RuntimeOrigin::signed(1))); assert_eq!(::Currency::reserved_balance(&1), 0); // Ensure stash no longer exists in the queue. @@ -156,9 +156,9 @@ fn deregister_works() { fn deregister_disabled_fails() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); ErasToCheckPerBlock::::put(0); - assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(2)), Error::::CallNotAllowed); + assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(1)), Error::::CallNotAllowed); }); } @@ -166,10 +166,10 @@ fn deregister_disabled_fails() { fn cannot_deregister_if_not_controller() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); - // Controller account registers for fast unstake. - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); - // Stash tries to deregister. - assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(1)), Error::::NotController); + // Controller (same as stash) account registers for fast unstake. + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); + // Another account tries to deregister. + assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(2)), Error::::NotController); }); } @@ -178,7 +178,7 @@ fn cannot_deregister_if_not_queued() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); // Controller tries to deregister without first registering - assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(2)), Error::::NotQueued); + assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(1)), Error::::NotQueued); }); } @@ -187,14 +187,14 @@ fn cannot_deregister_already_head() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); // Controller attempts to register, should fail - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); // Insert some Head item for stash. Head::::put(UnstakeRequest { stashes: bounded_vec![(1, Deposit::get())], checked: bounded_vec![], }); // Controller attempts to deregister - assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(2)), Error::::AlreadyHead); + assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(1)), Error::::AlreadyHead); }); } @@ -210,7 +210,7 @@ fn control_works() { fn control_must_be_control_origin() { ExtBuilder::default().build_and_execute(|| { // account without control (root) origin wants to only check 1 era per block. - assert_noop!(FastUnstake::control(RuntimeOrigin::signed(1), 1_u32), BadOrigin); + assert_noop!(FastUnstake::control(RuntimeOrigin::signed(2), 1_u32), BadOrigin); }); } @@ -224,7 +224,7 @@ mod on_idle { CurrentEra::::put(BondingDuration::get()); // set up Queue item - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); assert_eq!(Queue::::get(1), Some(Deposit::get())); // call on_idle with no remaining weight @@ -245,11 +245,11 @@ mod on_idle { // given assert_eq!(::Currency::reserved_balance(&1), 0); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(4))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(6))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(8))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(10))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(3))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(5))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(7))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(9))); assert_eq!(::Currency::reserved_balance(&1), Deposit::get()); @@ -310,9 +310,9 @@ mod on_idle { CurrentEra::::put(BondingDuration::get()); // register multi accounts for fast unstake - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); assert_eq!(Queue::::get(1), Some(Deposit::get())); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(4))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(3))); assert_eq!(Queue::::get(3), Some(Deposit::get())); // assert 2 queue items are in Queue & None in Head to start with @@ -363,7 +363,7 @@ mod on_idle { CurrentEra::::put(BondingDuration::get()); // register for fast unstake - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); assert_eq!(Queue::::get(1), Some(Deposit::get())); // process on idle @@ -405,7 +405,7 @@ mod on_idle { Balances::make_free_balance_be(&2, 100); // register for fast unstake - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); assert_eq!(Queue::::get(1), Some(Deposit::get())); // process on idle @@ -446,7 +446,7 @@ mod on_idle { CurrentEra::::put(BondingDuration::get()); // register for fast unstake - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); assert_eq!(Queue::::get(1), Some(Deposit::get())); // process on idle @@ -524,7 +524,7 @@ mod on_idle { CurrentEra::::put(BondingDuration::get()); // register for fast unstake - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); assert_eq!(Queue::::get(1), Some(Deposit::get())); next_block(true); @@ -605,7 +605,7 @@ mod on_idle { CurrentEra::::put(BondingDuration::get()); // register for fast unstake - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); // process 2 blocks next_block(true); @@ -851,10 +851,10 @@ mod batched { ErasToCheckPerBlock::::put(BondingDuration::get() + 1); CurrentEra::::put(BondingDuration::get()); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(4))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(6))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(8))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(3))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(5))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(7))); assert_eq!(Queue::::count(), 4); assert_eq!(Head::::get(), None); @@ -902,10 +902,10 @@ mod batched { ErasToCheckPerBlock::::put(2); CurrentEra::::put(BondingDuration::get()); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(4))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(6))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(8))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(3))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(5))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(7))); assert_eq!(Queue::::count(), 4); assert_eq!(Head::::get(), None); @@ -969,8 +969,8 @@ mod batched { CurrentEra::::put(BondingDuration::get()); // register two good ones. - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(4))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(3))); create_exposed_nominator(666, 1); create_exposed_nominator(667, 3); @@ -1045,8 +1045,8 @@ mod batched { next_block(true); // ..and register two good ones. - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2))); - assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(4))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); + assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(3))); // then one of the bad ones is reaped. assert_eq!( From 76547e043fc2b36f25452634933cbfffc54a22b2 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 16:44:19 +0800 Subject: [PATCH 39/60] fix staking-tests-e2e --- .../test-staking-e2e/src/mock.rs | 24 +++++++++---------- 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index c0ad6936f07c7..cb74ce8c34768 100644 --- a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -372,19 +372,19 @@ impl Default for StakingExtBuilder { let stakers = vec![ // (stash, ctrl, stake, status) // these two will be elected in the default test where we elect 2. - (11, 10, 1000, StakerStatus::::Validator), - (21, 20, 1000, StakerStatus::::Validator), - // loser validatos if validator_count() is default. - (31, 30, 500, StakerStatus::::Validator), - (41, 40, 500, StakerStatus::::Validator), - (51, 50, 500, StakerStatus::::Validator), - (61, 60, 500, StakerStatus::::Validator), - (71, 70, 500, StakerStatus::::Validator), - (81, 80, 500, StakerStatus::::Validator), - (91, 90, 500, StakerStatus::::Validator), - (101, 100, 500, StakerStatus::::Validator), + (11, 11, 1000, StakerStatus::::Validator), + (21, 21, 1000, StakerStatus::::Validator), + // loser validators if validator_count() is default. + (31, 31, 500, StakerStatus::::Validator), + (41, 41,1500, StakerStatus::::Validator), + (51, 51,1500, StakerStatus::::Validator), + (61, 61,1500, StakerStatus::::Validator), + (71, 71,1500, StakerStatus::::Validator), + (81, 81,1500, StakerStatus::::Validator), + (91, 91,1500, StakerStatus::::Validator), + (101, 101, 500, StakerStatus::::Validator), // an idle validator - (201, 200, 1000, StakerStatus::::Idle), + (201, 201, 1000, StakerStatus::::Idle), ]; Self { From 55cf31a197908263c50246e2adf003560b1e03b5 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 19:19:36 +0800 Subject: [PATCH 40/60] fix root-offenses --- frame/root-offences/src/mock.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/root-offences/src/mock.rs b/frame/root-offences/src/mock.rs index 828551e4d9c19..a4529fd599504 100644 --- a/frame/root-offences/src/mock.rs +++ b/frame/root-offences/src/mock.rs @@ -277,12 +277,12 @@ impl ExtBuilder { let stakers = vec![ // (stash, ctrl, stake, status) // these two will be elected in the default test where we elect 2. - (11, 10, 1000, StakerStatus::::Validator), - (21, 20, 1000, StakerStatus::::Validator), + (11, 11, 1000, StakerStatus::::Validator), + (21, 21, 1000, StakerStatus::::Validator), // a loser validator - (31, 30, 500, StakerStatus::::Validator), + (31, 31, 500, StakerStatus::::Validator), // an idle validator - (41, 40, 1000, StakerStatus::::Idle), + (41, 41, 1000, StakerStatus::::Idle), ]; let _ = pallet_staking::GenesisConfig:: { From bed829865975c602356afc371531c4735180b8ab Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 19:23:49 +0800 Subject: [PATCH 41/60] fmt --- .../test-staking-e2e/src/mock.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index cb74ce8c34768..88a62fd910d8e 100644 --- a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -376,12 +376,12 @@ impl Default for StakingExtBuilder { (21, 21, 1000, StakerStatus::::Validator), // loser validators if validator_count() is default. (31, 31, 500, StakerStatus::::Validator), - (41, 41,1500, StakerStatus::::Validator), - (51, 51,1500, StakerStatus::::Validator), - (61, 61,1500, StakerStatus::::Validator), - (71, 71,1500, StakerStatus::::Validator), - (81, 81,1500, StakerStatus::::Validator), - (91, 91,1500, StakerStatus::::Validator), + (41, 41, 1500, StakerStatus::::Validator), + (51, 51, 1500, StakerStatus::::Validator), + (61, 61, 1500, StakerStatus::::Validator), + (71, 71, 1500, StakerStatus::::Validator), + (81, 81, 1500, StakerStatus::::Validator), + (91, 91, 1500, StakerStatus::::Validator), (101, 101, 500, StakerStatus::::Validator), // an idle validator (201, 201, 1000, StakerStatus::::Idle), From 8b7e46efa2b5c7065652f7f31c42732facd900e6 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 21:40:38 +0800 Subject: [PATCH 42/60] differentiate controller to stash --- frame/staking/src/pallet/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 6f4fba63d03a4..0ea2c3e7ae28d 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -853,12 +853,13 @@ pub mod pallet { payee: RewardDestination, ) -> DispatchResult { let stash = ensure_signed(origin)?; + let controller = stash.clone(); if >::contains_key(&stash) { return Err(Error::::AlreadyBonded.into()) } - if >::contains_key(&stash) { + if >::contains_key(&controller) { return Err(Error::::AlreadyPaired.into()) } @@ -893,7 +894,7 @@ pub mod pallet { // satisfied. .defensive_map_err(|_| Error::::BoundNotMet)?, }; - Self::update_ledger(&stash, &item); + Self::update_ledger(&controller, &item); Ok(()) } From 4772a72447d3f00adaadc2ededd1454e17f9ffbf Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 21:55:35 +0800 Subject: [PATCH 43/60] bring back change_controller_works w. unique ctrl --- frame/staking/src/tests.rs | 36 +++++++++++++++++++++++++++++++++++- 1 file changed, 35 insertions(+), 1 deletion(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index df892decf6f81..59be9d9ee6d0e 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -92,7 +92,7 @@ fn set_staking_configs_works() { #[test] fn force_unstake_works() { ExtBuilder::default().build_and_execute(|| { - // Account 11 (also controller) is stashed and locked + // Account 11 (also controller) is stashed and locked assert_eq!(Staking::bonded(&11), Some(11)); // Adds 2 slashing spans add_slash(&11); @@ -228,6 +228,40 @@ fn basic_setup_works() { }); } +#[test] +fn change_controller_works() { + ExtBuilder::default().build_and_execute(|| { + let (stash, controller) = testing_utils::create_unique_stash_controller::( + 0, + 100, + RewardDestination::Staked, + false, + ) + .unwrap(); + + // ensure `stash` and `controller` are bonded as stash controller pair. + assert_eq!(Staking::bonded(&stash), Some(controller.clone())); + + // `controller` can control `stash` who is initially a validator. + assert_ok!(Staking::chill(RuntimeOrigin::signed(controller))); + + // sets controller back to `stash`. + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(stash.clone()))); + assert_eq!(Staking::bonded(&stash), Some(stash.clone())); + mock::start_active_era(1); + + // `controller` is no longer in control. `stash` is now controller. + assert_noop!( + Staking::validate(RuntimeOrigin::signed(controller), ValidatorPrefs::default()), + Error::::NotController, + ); + assert_ok!(Staking::validate( + RuntimeOrigin::signed(stash.clone()), + ValidatorPrefs::default() + )); + }) +} + #[test] fn change_controller_already_paired_once_stash() { ExtBuilder::default().build_and_execute(|| { From 813f460f24c824dc43a77afc3f5c0fd16965c22b Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 22:02:04 +0800 Subject: [PATCH 44/60] bring back double_staking_should_fail --- frame/staking/src/tests.rs | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 59be9d9ee6d0e..1f64eff4a63b7 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -761,6 +761,42 @@ fn nominators_also_get_slashed_pro_rata() { }); } +#[test] +fn double_staking_should_fail() { + // should test (in the same order): + // * an account already bonded as stash cannot be be stashed again. + // * an account already bonded as stash cannot nominate. + // * an account already bonded as controller can nominate. + ExtBuilder::default().build_and_execute(|| { + let arbitrary_value = 5; + // 1 = controller, 1 stashed => ok + let (stash, controller) = testing_utils::create_unique_stash_controller::( + 0, + 100, + RewardDestination::Staked, + false, + ) + .unwrap(); + + // 4 = not used so far, stash => not allowed. + assert_noop!( + Staking::bond( + RuntimeOrigin::signed(stash.clone()), + arbitrary_value, + RewardDestination::default() + ), + Error::::AlreadyBonded, + ); + // stash => attempting to nominate should fail. + assert_noop!( + Staking::nominate(RuntimeOrigin::signed(stash.clone()), vec![1]), + Error::::NotController + ); + // controller => nominating should work. + assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller.clone()), vec![1])); + }); +} + #[test] fn session_and_eras_work_simple() { ExtBuilder::default().period(1).build_and_execute(|| { From dbdb1a6fe7d6ca8603e24009c2c0321417747606 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 22:06:19 +0800 Subject: [PATCH 45/60] double_controlling_attempt_should_fail --- frame/staking/src/tests.rs | 34 ++++++++++++++++++++++++++++++---- 1 file changed, 30 insertions(+), 4 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 1f64eff4a63b7..8a79a94cd149b 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -769,11 +769,10 @@ fn double_staking_should_fail() { // * an account already bonded as controller can nominate. ExtBuilder::default().build_and_execute(|| { let arbitrary_value = 5; - // 1 = controller, 1 stashed => ok let (stash, controller) = testing_utils::create_unique_stash_controller::( 0, - 100, - RewardDestination::Staked, + arbitrary_value, + RewardDestination::default(), false, ) .unwrap(); @@ -782,7 +781,7 @@ fn double_staking_should_fail() { assert_noop!( Staking::bond( RuntimeOrigin::signed(stash.clone()), - arbitrary_value, + arbitrary_value.into(), RewardDestination::default() ), Error::::AlreadyBonded, @@ -797,6 +796,33 @@ fn double_staking_should_fail() { }); } +#[test] +fn double_controlling_attempt_should_fail() { + // should test (in the same order): + // * an account already bonded as controller CANNOT be reused as the controller of another + // account. + ExtBuilder::default().build_and_execute(|| { + let arbitrary_value = 5; + let (stash, _) = testing_utils::create_unique_stash_controller::( + 0, + arbitrary_value, + RewardDestination::default(), + false, + ) + .unwrap(); + + // Note that 2 controller (same as stash) is reused => no-op. + assert_noop!( + Staking::bond( + RuntimeOrigin::signed(stash.clone()), + arbitrary_value.into(), + RewardDestination::default() + ), + Error::::AlreadyBonded, + ); + }); +} + #[test] fn session_and_eras_work_simple() { ExtBuilder::default().period(1).build_and_execute(|| { From 74314f80cdced6cb81114c69e24e60fe180fa093 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 22:15:41 +0800 Subject: [PATCH 46/60] bring back payout_creates_controller --- frame/staking/src/tests.rs | 37 +++++++++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 8a79a94cd149b..96deddc0c17b8 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4175,6 +4175,43 @@ fn offences_weight_calculated_correctly() { }); } +#[test] +fn payout_creates_controller() { + ExtBuilder::default().has_stakers(false).build_and_execute(|| { + let balance = 1000; + // Create a validator: + bond_validator(11, balance); + + // create a stash/controller pair and nominate + let (stash, controller) = testing_utils::create_unique_stash_controller::( + 0, + 100, + RewardDestination::Controller, + false, + ) + .unwrap(); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller.clone()), vec![11])); + + // kill controller + assert_ok!(Balances::transfer_allow_death( + RuntimeOrigin::signed(controller.clone()), + stash, + 100 + )); + assert_eq!(Balances::free_balance(controller.clone()), 0); + + mock::start_active_era(1); + Staking::reward_by_ids(vec![(11, 1)]); + // compute and ensure the reward amount is greater than zero. + let _ = current_total_payout_for_duration(reward_time_per_era()); + mock::start_active_era(2); + assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(controller.clone()), 11, 1)); + + // Controller is created + assert!(Balances::free_balance(controller.clone()) > 0); + }) +} + #[test] fn payout_to_any_account_works() { ExtBuilder::default().has_stakers(false).build_and_execute(|| { From 5243561694757d271dafb93f1b49075b3ed56d4a Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 22:24:16 +0800 Subject: [PATCH 47/60] add commnet to controller balances --- .../election-provider-multi-phase/test-staking-e2e/src/mock.rs | 2 +- frame/root-offences/src/mock.rs | 2 +- frame/staking/src/mock.rs | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index 88a62fd910d8e..e1abbb909cbb1 100644 --- a/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -439,7 +439,7 @@ impl Default for BalancesExtBuilder { (2, 20), (3, 300), (4, 400), - // controllers + // controllers (still used in some tests. Soon to be deprecated). (10, 100), (20, 100), (30, 100), diff --git a/frame/root-offences/src/mock.rs b/frame/root-offences/src/mock.rs index a4529fd599504..e48360ed34e24 100644 --- a/frame/root-offences/src/mock.rs +++ b/frame/root-offences/src/mock.rs @@ -259,7 +259,7 @@ impl ExtBuilder { pallet_balances::GenesisConfig:: { balances: vec![ - //controllers + // controllers (still used in some tests. Soon to be deprecated). (10, self.balance_factor * 50), (20, self.balance_factor * 50), (30, self.balance_factor * 50), diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 9904282375b56..98b58010a2434 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -432,7 +432,7 @@ impl ExtBuilder { (2, 20 * self.balance_factor), (3, 300 * self.balance_factor), (4, 400 * self.balance_factor), - // controllers + // controllers (still used in some tests. Soon to be deprecated). (10, self.balance_factor), (20, self.balance_factor), (30, self.balance_factor), From cd751a27e1a944e5af014039985c63ae2bec5193 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 22:29:52 +0800 Subject: [PATCH 48/60] + set_controller call description --- frame/staking/src/pallet/mod.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index 0ea2c3e7ae28d..a871352f6be00 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -1234,7 +1234,10 @@ pub mod pallet { Ok(()) } - /// (Re-)sets the controller of a stash to the stash itself. + /// (Re-)sets the controller of a stash to the stash itself. This function previously accepted a + /// `controller` argument to set the controller to an account other than the stash itself. This + /// functionality has now been removed, now only setting the controller to the stash, if it is + /// not already. /// /// Effects will be felt instantly (as soon as this function is completed successfully). /// From 7580e0d9b8d72e4424f6b219134dd272d767a289 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 22:33:55 +0800 Subject: [PATCH 49/60] fmt --- frame/staking/src/pallet/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index a871352f6be00..a9a00ff8db32c 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -1234,10 +1234,10 @@ pub mod pallet { Ok(()) } - /// (Re-)sets the controller of a stash to the stash itself. This function previously accepted a - /// `controller` argument to set the controller to an account other than the stash itself. This - /// functionality has now been removed, now only setting the controller to the stash, if it is - /// not already. + /// (Re-)sets the controller of a stash to the stash itself. This function previously + /// accepted a `controller` argument to set the controller to an account other than the + /// stash itself. This functionality has now been removed, now only setting the controller + /// to the stash, if it is not already. /// /// Effects will be felt instantly (as soon as this function is completed successfully). /// From ead11a96ed3cb341da9e49c5c4a46664b55f03a9 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 22:45:18 +0800 Subject: [PATCH 50/60] rm clones --- frame/staking/src/tests.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 96deddc0c17b8..639f33fabdc16 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -240,7 +240,7 @@ fn change_controller_works() { .unwrap(); // ensure `stash` and `controller` are bonded as stash controller pair. - assert_eq!(Staking::bonded(&stash), Some(controller.clone())); + assert_eq!(Staking::bonded(&stash), Some(controller)); // `controller` can control `stash` who is initially a validator. assert_ok!(Staking::chill(RuntimeOrigin::signed(controller))); @@ -792,7 +792,7 @@ fn double_staking_should_fail() { Error::::NotController ); // controller => nominating should work. - assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller.clone()), vec![1])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller), vec![1])); }); } @@ -4190,25 +4190,25 @@ fn payout_creates_controller() { false, ) .unwrap(); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller.clone()), vec![11])); + assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller), vec![11])); // kill controller assert_ok!(Balances::transfer_allow_death( - RuntimeOrigin::signed(controller.clone()), + RuntimeOrigin::signed(controller), stash, 100 )); - assert_eq!(Balances::free_balance(controller.clone()), 0); + assert_eq!(Balances::free_balance(controller), 0); mock::start_active_era(1); Staking::reward_by_ids(vec![(11, 1)]); // compute and ensure the reward amount is greater than zero. let _ = current_total_payout_for_duration(reward_time_per_era()); mock::start_active_era(2); - assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(controller.clone()), 11, 1)); + assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(controller), 11, 1)); // Controller is created - assert!(Balances::free_balance(controller.clone()) > 0); + assert!(Balances::free_balance(controller) > 0); }) } From e43a67629a1f211a0436a051c783c10ecc020b34 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 22:45:31 +0800 Subject: [PATCH 51/60] fmt --- frame/staking/src/tests.rs | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 639f33fabdc16..db56bb6550d1a 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4193,11 +4193,7 @@ fn payout_creates_controller() { assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller), vec![11])); // kill controller - assert_ok!(Balances::transfer_allow_death( - RuntimeOrigin::signed(controller), - stash, - 100 - )); + assert_ok!(Balances::transfer_allow_death(RuntimeOrigin::signed(controller), stash, 100)); assert_eq!(Balances::free_balance(controller), 0); mock::start_active_era(1); From d94fa54e9c9f0c560298b99f05e958185ac8b275 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 23:10:38 +0800 Subject: [PATCH 52/60] clippy fixes --- frame/staking/src/tests.rs | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index db56bb6550d1a..c9d0f1b93eb95 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -246,8 +246,8 @@ fn change_controller_works() { assert_ok!(Staking::chill(RuntimeOrigin::signed(controller))); // sets controller back to `stash`. - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(stash.clone()))); - assert_eq!(Staking::bonded(&stash), Some(stash.clone())); + assert_ok!(Staking::set_controller(RuntimeOrigin::signed(stash))); + assert_eq!(Staking::bonded(&stash), Some(stash)); mock::start_active_era(1); // `controller` is no longer in control. `stash` is now controller. @@ -256,7 +256,7 @@ fn change_controller_works() { Error::::NotController, ); assert_ok!(Staking::validate( - RuntimeOrigin::signed(stash.clone()), + RuntimeOrigin::signed(stash), ValidatorPrefs::default() )); }) @@ -780,7 +780,7 @@ fn double_staking_should_fail() { // 4 = not used so far, stash => not allowed. assert_noop!( Staking::bond( - RuntimeOrigin::signed(stash.clone()), + RuntimeOrigin::signed(stash), arbitrary_value.into(), RewardDestination::default() ), @@ -788,7 +788,7 @@ fn double_staking_should_fail() { ); // stash => attempting to nominate should fail. assert_noop!( - Staking::nominate(RuntimeOrigin::signed(stash.clone()), vec![1]), + Staking::nominate(RuntimeOrigin::signed(stash), vec![1]), Error::::NotController ); // controller => nominating should work. @@ -814,7 +814,7 @@ fn double_controlling_attempt_should_fail() { // Note that 2 controller (same as stash) is reused => no-op. assert_noop!( Staking::bond( - RuntimeOrigin::signed(stash.clone()), + RuntimeOrigin::signed(stash), arbitrary_value.into(), RewardDestination::default() ), From ba21e5819b91cdf3237359898c60cf2c34fbf71d Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 23:12:16 +0800 Subject: [PATCH 53/60] fmt --- frame/staking/src/tests.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index c9d0f1b93eb95..8c196bb949d65 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -255,10 +255,7 @@ fn change_controller_works() { Staking::validate(RuntimeOrigin::signed(controller), ValidatorPrefs::default()), Error::::NotController, ); - assert_ok!(Staking::validate( - RuntimeOrigin::signed(stash), - ValidatorPrefs::default() - )); + assert_ok!(Staking::validate(RuntimeOrigin::signed(stash), ValidatorPrefs::default())); }) } From 7b528795812a215e08e72ad4272c0e86cd9df7ba Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 23:24:52 +0800 Subject: [PATCH 54/60] update README --- frame/staking/README.md | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/staking/README.md b/frame/staking/README.md index 8ea95ab6ea1fa..ccb9901a6796e 100644 --- a/frame/staking/README.md +++ b/frame/staking/README.md @@ -52,7 +52,10 @@ An account pair can become bonded using the [`bond`](https://docs.rs/pallet-stak Stash accounts can update their associated controller back to their stash account using the [`set_controller`](https://docs.rs/pallet-staking/latest/pallet_staking/enum.Call.html#variant.set_controller) -call. +call. + +Note: Controller accounts are being deprecated in favor of proxy accounts, so it is no longer +possible to set a unique address for a stash's controller. There are three possible roles that any staked account pair can be in: `Validator`, `Nominator` and `Idle` (defined in [`StakerStatus`](https://docs.rs/pallet-staking/latest/pallet_staking/enum.StakerStatus.html)). There are three From 74b29aab497374f002cd3ca828f1844bc7d05791 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 9 May 2023 23:51:53 +0800 Subject: [PATCH 55/60] small fixes --- frame/staking/src/benchmarking.rs | 4 ---- frame/staking/src/tests.rs | 2 +- 2 files changed, 1 insertion(+), 5 deletions(-) diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 785357e0870a6..53589ecfe4dbc 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -580,10 +580,6 @@ benchmarks! { balance_before < balance_after, "Balance of validator controller should have increased after payout.", ); - ensure!( - balance_before < balance_after, - "Balance of validator controller should have increased after payout.", - ); for (_, controller) in &nominators { let balance = T::Currency::free_balance(controller); ensure!(!balance.is_zero(), "Payout not given to controller."); diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index 8c196bb949d65..eb45c2e6713a8 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -808,7 +808,7 @@ fn double_controlling_attempt_should_fail() { ) .unwrap(); - // Note that 2 controller (same as stash) is reused => no-op. + // Note that controller (same as stash) is reused => no-op. assert_noop!( Staking::bond( RuntimeOrigin::signed(stash), From 65727126a432c0c20442a4dc3f37526149f8eb9c Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 10 May 2023 22:28:40 +0800 Subject: [PATCH 56/60] use controller_to_be_deprecated --- frame/staking/src/pallet/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index a9a00ff8db32c..a725d43515b4c 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -853,13 +853,13 @@ pub mod pallet { payee: RewardDestination, ) -> DispatchResult { let stash = ensure_signed(origin)?; - let controller = stash.clone(); + let controller_to_be_deprecated = stash.clone(); if >::contains_key(&stash) { return Err(Error::::AlreadyBonded.into()) } - if >::contains_key(&controller) { + if >::contains_key(&controller_to_be_deprecated) { return Err(Error::::AlreadyPaired.into()) } @@ -894,7 +894,7 @@ pub mod pallet { // satisfied. .defensive_map_err(|_| Error::::BoundNotMet)?, }; - Self::update_ledger(&controller, &item); + Self::update_ledger(&controller_to_be_deprecated, &item); Ok(()) } From d949382634be21bda47bc775b908a40e579846d3 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 11 May 2023 00:59:17 +0800 Subject: [PATCH 57/60] .comment --- frame/staking/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index eb45c2e6713a8..b534fa72363ee 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4967,7 +4967,7 @@ fn capped_stakers_works() { some_existing_nominator = controller; } - // one more is too many + // one more is too many. let (_, last_nominator) = testing_utils::create_stash_controller::( 30_000_000, 100, @@ -4979,7 +4979,7 @@ fn capped_stakers_works() { Error::::TooManyNominators ); - // Re-nominate works fine + // Re-nominate works fine. assert_ok!(Staking::nominate(RuntimeOrigin::signed(some_existing_nominator), vec![1])); // Re-validate works fine assert_ok!(Staking::validate( From 86505f00a878ea8aa48288cd5a927ad56450ccd8 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 11 May 2023 18:27:28 +0800 Subject: [PATCH 58/60] comment --- frame/staking/src/tests.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index b534fa72363ee..eb45c2e6713a8 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4967,7 +4967,7 @@ fn capped_stakers_works() { some_existing_nominator = controller; } - // one more is too many. + // one more is too many let (_, last_nominator) = testing_utils::create_stash_controller::( 30_000_000, 100, @@ -4979,7 +4979,7 @@ fn capped_stakers_works() { Error::::TooManyNominators ); - // Re-nominate works fine. + // Re-nominate works fine assert_ok!(Staking::nominate(RuntimeOrigin::signed(some_existing_nominator), vec![1])); // Re-validate works fine assert_ok!(Staking::validate( From 8a3e87607bb534343b87d8894b25d527ad6261f2 Mon Sep 17 00:00:00 2001 From: Javier Viola Date: Thu, 11 May 2023 09:44:46 -0300 Subject: [PATCH 59/60] bump zombienet version --- .gitlab-ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index cc950865ce07e..fe70d3532a9b1 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -61,7 +61,7 @@ variables: NEXTEST_FAILURE_OUTPUT: immediate-final NEXTEST_SUCCESS_OUTPUT: final - ZOMBIENET_IMAGE: "docker.io/paritytech/zombienet:v1.3.48" + ZOMBIENET_IMAGE: "docker.io/paritytech/zombienet:v1.3.52" default: retry: From fa530a34b01e4e4b0405cda963590b723fed8131 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Thu, 11 May 2023 21:11:04 +0800 Subject: [PATCH 60/60] ci --- frame/staking/src/tests.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index eb45c2e6713a8..e3ee4cd1a8e9f 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -4967,7 +4967,7 @@ fn capped_stakers_works() { some_existing_nominator = controller; } - // one more is too many + // one more is too many. let (_, last_nominator) = testing_utils::create_stash_controller::( 30_000_000, 100,