From 1764b6e30c84856faed4ef912915dac8313d5f5a Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:07:43 +0700 Subject: [PATCH 01/63] remove deprecate_controller_batch --- polkadot/runtime/test-runtime/src/lib.rs | 1 - polkadot/runtime/westend/src/lib.rs | 2 - .../westend/src/weights/pallet_staking.rs | 20 -- substrate/bin/node/runtime/src/lib.rs | 2 - substrate/frame/babe/src/mock.rs | 1 - substrate/frame/beefy/src/mock.rs | 1 - substrate/frame/delegated-staking/src/mock.rs | 1 - .../test-staking-e2e/src/mock.rs | 1 - substrate/frame/fast-unstake/src/mock.rs | 1 - substrate/frame/grandpa/src/mock.rs | 1 - .../nomination-pools/benchmarking/src/mock.rs | 1 - .../test-delegate-stake/src/mock.rs | 1 - .../test-transfer-stake/src/mock.rs | 1 - .../frame/offences/benchmarking/src/mock.rs | 1 - substrate/frame/root-offences/src/mock.rs | 1 - .../frame/session/benchmarking/src/mock.rs | 1 - substrate/frame/staking/src/benchmarking.rs | 33 --- substrate/frame/staking/src/lib.rs | 2 +- substrate/frame/staking/src/mock.rs | 2 - substrate/frame/staking/src/pallet/mod.rs | 50 ---- substrate/frame/staking/src/tests.rs | 223 ------------------ substrate/frame/staking/src/weights.rs | 39 --- 22 files changed, 1 insertion(+), 385 deletions(-) diff --git a/polkadot/runtime/test-runtime/src/lib.rs b/polkadot/runtime/test-runtime/src/lib.rs index 9eb0fcca6678b..b00434546d54f 100644 --- a/polkadot/runtime/test-runtime/src/lib.rs +++ b/polkadot/runtime/test-runtime/src/lib.rs @@ -359,7 +359,6 @@ impl pallet_staking::Config for Runtime { type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = pallet_staking::FixedNominationsQuota; type MaxUnlockingChunks = frame_support::traits::ConstU32<32>; - type MaxControllersInDeprecationBatch = ConstU32<5900>; type HistoryDepth = frame_support::traits::ConstU32<84>; type BenchmarkingConfig = runtime_common::StakingBenchmarkingConfig; type EventListeners = (); diff --git a/polkadot/runtime/westend/src/lib.rs b/polkadot/runtime/westend/src/lib.rs index 4bf132d82c963..bcfa3f3206e5b 100644 --- a/polkadot/runtime/westend/src/lib.rs +++ b/polkadot/runtime/westend/src/lib.rs @@ -618,7 +618,6 @@ parameter_types! { // of nominators. pub const MaxNominators: u32 = 64; pub const MaxNominations: u32 = ::LIMIT as u32; - pub const MaxControllersInDeprecationBatch: u32 = 751; } impl pallet_staking::Config for Runtime { @@ -645,7 +644,6 @@ impl pallet_staking::Config for Runtime { type NominationsQuota = pallet_staking::FixedNominationsQuota<{ MaxNominations::get() }>; type MaxUnlockingChunks = frame_support::traits::ConstU32<32>; type HistoryDepth = frame_support::traits::ConstU32<84>; - type MaxControllersInDeprecationBatch = MaxControllersInDeprecationBatch; type BenchmarkingConfig = runtime_common::StakingBenchmarkingConfig; type EventListeners = (NominationPools, DelegatedStaking); type WeightInfo = weights::pallet_staking::WeightInfo; diff --git a/polkadot/runtime/westend/src/weights/pallet_staking.rs b/polkadot/runtime/westend/src/weights/pallet_staking.rs index 393fa0b37176a..3d79a648d8911 100644 --- a/polkadot/runtime/westend/src/weights/pallet_staking.rs +++ b/polkadot/runtime/westend/src/weights/pallet_staking.rs @@ -406,26 +406,6 @@ impl pallet_staking::WeightInfo for WeightInfo { .saturating_add(Weight::from_parts(11_785, 0).saturating_mul(v.into())) .saturating_add(T::DbWeight::get().writes(1)) } - /// Storage: `Staking::Ledger` (r:1502 w:1502) - /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) - /// Storage: `Staking::Bonded` (r:751 w:751) - /// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`) - /// Storage: `Staking::Payee` (r:751 w:0) - /// Proof: `Staking::Payee` (`max_values`: None, `max_size`: Some(73), added: 2548, mode: `MaxEncodedLen`) - /// The range of component `i` is `[0, 751]`. - fn deprecate_controller_batch(i: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `680 + i * (227 ±0)` - // Estimated: `990 + i * (7132 ±0)` - // Minimum execution time: 4_321_000 picoseconds. - Weight::from_parts(4_407_000, 0) - .saturating_add(Weight::from_parts(0, 990)) - // Standard Error: 37_239 - .saturating_add(Weight::from_parts(21_300_598, 0).saturating_mul(i.into())) - .saturating_add(T::DbWeight::get().reads((4_u64).saturating_mul(i.into()))) - .saturating_add(T::DbWeight::get().writes((3_u64).saturating_mul(i.into()))) - .saturating_add(Weight::from_parts(0, 7132).saturating_mul(i.into())) - } /// Storage: `Staking::SlashingSpans` (r:1 w:1) /// Proof: `Staking::SlashingSpans` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `Staking::Bonded` (r:1 w:1) diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs index 7d9128bb940a5..c89a0ebc89e9c 100644 --- a/substrate/bin/node/runtime/src/lib.rs +++ b/substrate/bin/node/runtime/src/lib.rs @@ -656,7 +656,6 @@ parameter_types! { pub const SlashDeferDuration: sp_staking::EraIndex = 24 * 7; // 1/4 the bonding duration. pub const RewardCurve: &'static PiecewiseLinear<'static> = &REWARD_CURVE; pub const MaxNominators: u32 = 64; - pub const MaxControllersInDeprecationBatch: u32 = 5900; pub OffchainRepeat: BlockNumber = 5; pub HistoryDepth: u32 = 84; } @@ -698,7 +697,6 @@ impl pallet_staking::Config for Runtime { // This a placeholder, to be introduced in the next PR as an instance of bags-list type TargetList = pallet_staking::UseValidatorsMap; type MaxUnlockingChunks = ConstU32<32>; - type MaxControllersInDeprecationBatch = MaxControllersInDeprecationBatch; type HistoryDepth = HistoryDepth; type EventListeners = NominationPools; type WeightInfo = pallet_staking::weights::SubstrateWeight; diff --git a/substrate/frame/babe/src/mock.rs b/substrate/frame/babe/src/mock.rs index 395a86e652880..296bad458c509 100644 --- a/substrate/frame/babe/src/mock.rs +++ b/substrate/frame/babe/src/mock.rs @@ -180,7 +180,6 @@ impl pallet_staking::Config for Test { type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = FixedNominationsQuota<16>; type MaxUnlockingChunks = ConstU32<32>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type HistoryDepth = ConstU32<84>; type EventListeners = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/substrate/frame/beefy/src/mock.rs b/substrate/frame/beefy/src/mock.rs index 0b87de6bf5d79..b0b1d8d9c7b6c 100644 --- a/substrate/frame/beefy/src/mock.rs +++ b/substrate/frame/beefy/src/mock.rs @@ -194,7 +194,6 @@ impl pallet_staking::Config for Test { type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = pallet_staking::FixedNominationsQuota<16>; type MaxUnlockingChunks = ConstU32<32>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type HistoryDepth = ConstU32<84>; type EventListeners = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/substrate/frame/delegated-staking/src/mock.rs b/substrate/frame/delegated-staking/src/mock.rs index b9eaffb970e13..31195459bd84e 100644 --- a/substrate/frame/delegated-staking/src/mock.rs +++ b/substrate/frame/delegated-staking/src/mock.rs @@ -130,7 +130,6 @@ impl pallet_staking::Config for Runtime { type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = pallet_staking::FixedNominationsQuota<16>; type MaxUnlockingChunks = ConstU32<10>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type EventListeners = (Pools, DelegatedStaking); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index e5987ec33f06c..caabd576c5a1b 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -314,7 +314,6 @@ impl pallet_staking::Config for Runtime { type NominationsQuota = pallet_staking::FixedNominationsQuota; type TargetList = pallet_staking::UseValidatorsMap; type MaxUnlockingChunks = MaxUnlockingChunks; - type MaxControllersInDeprecationBatch = ConstU32<100>; type HistoryDepth = HistoryDepth; type EventListeners = Pools; type WeightInfo = pallet_staking::weights::SubstrateWeight; diff --git a/substrate/frame/fast-unstake/src/mock.rs b/substrate/frame/fast-unstake/src/mock.rs index d876f9f6171e5..b5bc9af4e2e12 100644 --- a/substrate/frame/fast-unstake/src/mock.rs +++ b/substrate/frame/fast-unstake/src/mock.rs @@ -140,7 +140,6 @@ impl pallet_staking::Config for Runtime { type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = pallet_staking::FixedNominationsQuota<16>; type MaxUnlockingChunks = ConstU32<32>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type EventListeners = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); diff --git a/substrate/frame/grandpa/src/mock.rs b/substrate/frame/grandpa/src/mock.rs index 2d54f525b1f0c..6777cb6e74e26 100644 --- a/substrate/frame/grandpa/src/mock.rs +++ b/substrate/frame/grandpa/src/mock.rs @@ -182,7 +182,6 @@ impl pallet_staking::Config for Test { type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = pallet_staking::FixedNominationsQuota<16>; type MaxUnlockingChunks = ConstU32<32>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type HistoryDepth = ConstU32<84>; type EventListeners = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/substrate/frame/nomination-pools/benchmarking/src/mock.rs b/substrate/frame/nomination-pools/benchmarking/src/mock.rs index def98b4d2945e..dec075afa8d4e 100644 --- a/substrate/frame/nomination-pools/benchmarking/src/mock.rs +++ b/substrate/frame/nomination-pools/benchmarking/src/mock.rs @@ -117,7 +117,6 @@ impl pallet_staking::Config for Runtime { type VoterList = VoterList; type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = pallet_staking::FixedNominationsQuota<16>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type MaxUnlockingChunks = ConstU32<32>; type HistoryDepth = ConstU32<84>; type EventListeners = (Pools, DelegatedStaking); diff --git a/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs b/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs index 1c0a0166fd9a9..4af4a3d35a623 100644 --- a/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs +++ b/substrate/frame/nomination-pools/test-delegate-stake/src/mock.rs @@ -134,7 +134,6 @@ impl pallet_staking::Config for Runtime { type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = pallet_staking::FixedNominationsQuota<16>; type MaxUnlockingChunks = ConstU32<32>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type HistoryDepth = ConstU32<84>; type EventListeners = (Pools, DelegatedStaking); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/substrate/frame/nomination-pools/test-transfer-stake/src/mock.rs b/substrate/frame/nomination-pools/test-transfer-stake/src/mock.rs index 0970570453b46..eae460fb3bf50 100644 --- a/substrate/frame/nomination-pools/test-transfer-stake/src/mock.rs +++ b/substrate/frame/nomination-pools/test-transfer-stake/src/mock.rs @@ -132,7 +132,6 @@ impl pallet_staking::Config for Runtime { type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = pallet_staking::FixedNominationsQuota<16>; type MaxUnlockingChunks = ConstU32<32>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type HistoryDepth = ConstU32<84>; type EventListeners = Pools; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/substrate/frame/offences/benchmarking/src/mock.rs b/substrate/frame/offences/benchmarking/src/mock.rs index eeaa1364504ab..35d2b52075d34 100644 --- a/substrate/frame/offences/benchmarking/src/mock.rs +++ b/substrate/frame/offences/benchmarking/src/mock.rs @@ -180,7 +180,6 @@ impl pallet_staking::Config for Test { type TargetList = pallet_staking::UseValidatorsMap; type NominationsQuota = pallet_staking::FixedNominationsQuota<16>; type MaxUnlockingChunks = ConstU32<32>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type HistoryDepth = ConstU32<84>; type EventListeners = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/substrate/frame/root-offences/src/mock.rs b/substrate/frame/root-offences/src/mock.rs index 7e7332c3f7e3b..2bef514c0d1a9 100644 --- a/substrate/frame/root-offences/src/mock.rs +++ b/substrate/frame/root-offences/src/mock.rs @@ -158,7 +158,6 @@ impl pallet_staking::Config for Test { type NominationsQuota = pallet_staking::FixedNominationsQuota<16>; type MaxUnlockingChunks = ConstU32<32>; type HistoryDepth = ConstU32<84>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; type EventListeners = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/substrate/frame/session/benchmarking/src/mock.rs b/substrate/frame/session/benchmarking/src/mock.rs index 6cefa8f39a8c6..dfe42aa0acdb4 100644 --- a/substrate/frame/session/benchmarking/src/mock.rs +++ b/substrate/frame/session/benchmarking/src/mock.rs @@ -177,7 +177,6 @@ impl pallet_staking::Config for Test { type ElectionProvider = onchain::OnChainExecution; type GenesisElectionProvider = Self::ElectionProvider; type MaxUnlockingChunks = ConstU32<32>; - type MaxControllersInDeprecationBatch = ConstU32<100>; type HistoryDepth = ConstU32<84>; type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; type TargetList = pallet_staking::UseValidatorsMap; diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 3ed33ffea4223..b299df7569769 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -526,39 +526,6 @@ benchmarks! { assert_eq!(Invulnerables::::get().len(), v as usize); } - deprecate_controller_batch { - // We pass a dynamic number of controllers to the benchmark, up to - // `MaxControllersInDeprecationBatch`. - let i in 0 .. T::MaxControllersInDeprecationBatch::get(); - - let mut controllers: Vec<_> = vec![]; - let mut stashes: Vec<_> = vec![]; - for n in 0..i as u32 { - let (stash, controller) = create_unique_stash_controller::( - n, - 100, - RewardDestination::Staked, - false - )?; - controllers.push(controller); - stashes.push(stash); - } - let bounded_controllers: BoundedVec<_, T::MaxControllersInDeprecationBatch> = - BoundedVec::try_from(controllers.clone()).unwrap(); - }: _(RawOrigin::Root, bounded_controllers) - verify { - for n in 0..i as u32 { - let stash = &stashes[n as usize]; - let controller = &controllers[n as usize]; - // Ledger no longer keyed by controller. - assert_eq!(Ledger::::get(controller), None); - // Bonded now maps to the stash. - assert_eq!(Bonded::::get(stash), Some(stash.clone())); - // Ledger is now keyed by stash. - assert_eq!(Ledger::::get(stash).unwrap().stash, *stash); - } - } - force_unstake { // Slashing Spans let s in 0 .. MAX_SPANS; diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index 053ecdef2b00b..bfa091efe2945 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -400,7 +400,7 @@ impl Default for EraRewardPoints { #[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub enum RewardDestination { /// Pay into the stash account, increasing the amount at stake accordingly. - Staked, +Staked, /// Pay into the stash account, not increasing the amount at stake. Stash, #[deprecated( diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 8c60dec65a81a..9bc6287910901 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -115,7 +115,6 @@ parameter_types! { pub static SlashDeferDuration: EraIndex = 0; pub static Period: BlockNumber = 5; pub static Offset: BlockNumber = 0; - pub static MaxControllersInDeprecationBatch: u32 = 5900; } #[derive_impl(frame_system::config_preludes::TestDefaultConfig)] @@ -294,7 +293,6 @@ impl crate::pallet::pallet::Config for Test { type NominationsQuota = WeightedNominationsQuota<16>; type MaxUnlockingChunks = MaxUnlockingChunks; type HistoryDepth = HistoryDepth; - type MaxControllersInDeprecationBatch = MaxControllersInDeprecationBatch; type EventListeners = EventListenerMock; type BenchmarkingConfig = TestBenchmarkingConfig; type WeightInfo = (); diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 284a801a0f050..6c9b835bf8fb4 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -267,9 +267,6 @@ pub mod pallet { #[pallet::constant] type MaxUnlockingChunks: Get; - /// The maximum amount of controller accounts that can be deprecated in one call. - type MaxControllersInDeprecationBatch: Get; - /// Something that listens to staking updates and performs actions based on the data it /// receives. /// @@ -1938,53 +1935,6 @@ pub mod pallet { Ok(Pays::No.into()) } - /// Updates a batch of controller accounts to their corresponding stash account if they are - /// not the same. Ignores any controller accounts that do not exist, and does not operate if - /// the stash and controller are already the same. - /// - /// Effects will be felt instantly (as soon as this function is completed successfully). - /// - /// The dispatch origin must be `T::AdminOrigin`. - #[pallet::call_index(28)] - #[pallet::weight(T::WeightInfo::deprecate_controller_batch(controllers.len() as u32))] - pub fn deprecate_controller_batch( - origin: OriginFor, - controllers: BoundedVec, - ) -> DispatchResultWithPostInfo { - T::AdminOrigin::ensure_origin(origin)?; - - // Ignore controllers that do not exist or are already the same as stash. - let filtered_batch_with_ledger: Vec<_> = controllers - .iter() - .filter_map(|controller| { - let ledger = Self::ledger(StakingAccount::Controller(controller.clone())); - ledger.ok().map_or(None, |ledger| { - // If the controller `RewardDestination` is still the deprecated - // `Controller` variant, skip deprecating this account. - let payee_deprecated = Payee::::get(&ledger.stash) == { - #[allow(deprecated)] - Some(RewardDestination::Controller) - }; - - if ledger.stash != *controller && !payee_deprecated { - Some(ledger) - } else { - None - } - }) - }) - .collect(); - - // Update unique pairs. - let mut failures = 0; - for ledger in filtered_batch_with_ledger { - let _ = ledger.clone().set_controller_to_stash().map_err(|_| failures += 1); - } - Self::deposit_event(Event::::ControllerBatchDeprecated { failures }); - - Ok(Some(T::WeightInfo::deprecate_controller_batch(controllers.len() as u32)).into()) - } - /// Restores the state of a ledger which is in an inconsistent state. /// /// The requirements to restore a ledger are the following: diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 76afa3333cb46..f6668f83b2f1d 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -7611,229 +7611,6 @@ mod ledger { }) } - #[test] - fn deprecate_controller_batch_works_full_weight() { - ExtBuilder::default().try_state(false).build_and_execute(|| { - // Given: - - let start = 1001; - let mut controllers: Vec<_> = vec![]; - for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { - let ctlr: u64 = n.into(); - let stash: u64 = (n + 10000).into(); - - Ledger::::insert( - ctlr, - StakingLedger { - controller: None, - total: (10 + ctlr).into(), - active: (10 + ctlr).into(), - ..StakingLedger::default_from(stash) - }, - ); - Bonded::::insert(stash, ctlr); - Payee::::insert(stash, RewardDestination::Staked); - - controllers.push(ctlr); - } - - // When: - - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(controllers).unwrap(); - - // Only `AdminOrigin` can sign. - assert_noop!( - Staking::deprecate_controller_batch( - RuntimeOrigin::signed(2), - bounded_controllers.clone() - ), - BadOrigin - ); - - let result = - Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); - assert_ok!(result); - assert_eq!( - result.unwrap().actual_weight.unwrap(), - ::WeightInfo::deprecate_controller_batch( - ::MaxControllersInDeprecationBatch::get() - ) - ); - - // Then: - - for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { - let ctlr: u64 = n.into(); - let stash: u64 = (n + 10000).into(); - - // Ledger no longer keyed by controller. - assert_eq!(Ledger::::get(ctlr), None); - // Bonded now maps to the stash. - assert_eq!(Bonded::::get(stash), Some(stash)); - - // Ledger is now keyed by stash. - let ledger_updated = Ledger::::get(stash).unwrap(); - assert_eq!(ledger_updated.stash, stash); - - // Check `active` and `total` values match the original ledger set by controller. - assert_eq!(ledger_updated.active, (10 + ctlr).into()); - assert_eq!(ledger_updated.total, (10 + ctlr).into()); - } - }) - } - - #[test] - fn deprecate_controller_batch_works_half_weight() { - ExtBuilder::default().build_and_execute(|| { - // Given: - - let start = 1001; - let mut controllers: Vec<_> = vec![]; - for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { - let ctlr: u64 = n.into(); - - // Only half of entries are unique pairs. - let stash: u64 = if n % 2 == 0 { (n + 10000).into() } else { ctlr }; - - Ledger::::insert( - ctlr, - StakingLedger { controller: None, ..StakingLedger::default_from(stash) }, - ); - Bonded::::insert(stash, ctlr); - Payee::::insert(stash, RewardDestination::Staked); - - controllers.push(ctlr); - } - - // When: - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(controllers.clone()).unwrap(); - - let result = - Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); - assert_ok!(result); - assert_eq!( - result.unwrap().actual_weight.unwrap(), - ::WeightInfo::deprecate_controller_batch(controllers.len() as u32) - ); - - // Then: - - for n in start..(start + MaxControllersInDeprecationBatch::get()).into() { - let unique_pair = n % 2 == 0; - let ctlr: u64 = n.into(); - let stash: u64 = if unique_pair { (n + 10000).into() } else { ctlr }; - - // Side effect of migration for unique pair. - if unique_pair { - assert_eq!(Ledger::::get(ctlr), None); - } - // Bonded maps to the stash. - assert_eq!(Bonded::::get(stash), Some(stash)); - - // Ledger is keyed by stash. - let ledger_updated = Ledger::::get(stash).unwrap(); - assert_eq!(ledger_updated.stash, stash); - } - }) - } - - #[test] - fn deprecate_controller_batch_skips_unmigrated_controller_payees() { - ExtBuilder::default().try_state(false).build_and_execute(|| { - // Given: - - let stash: u64 = 1000; - let ctlr: u64 = 1001; - - Ledger::::insert( - ctlr, - StakingLedger { controller: None, ..StakingLedger::default_from(stash) }, - ); - Bonded::::insert(stash, ctlr); - #[allow(deprecated)] - Payee::::insert(stash, RewardDestination::Controller); - - // When: - - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![ctlr]).unwrap(); - - let result = - Staking::deprecate_controller_batch(RuntimeOrigin::root(), bounded_controllers); - assert_ok!(result); - assert_eq!( - result.unwrap().actual_weight.unwrap(), - ::WeightInfo::deprecate_controller_batch(1 as u32) - ); - - // Then: - - // Esure deprecation did not happen. - assert_eq!(Ledger::::get(ctlr).is_some(), true); - - // Bonded still keyed by controller. - assert_eq!(Bonded::::get(stash), Some(ctlr)); - - // Ledger is still keyed by controller. - let ledger_updated = Ledger::::get(ctlr).unwrap(); - assert_eq!(ledger_updated.stash, stash); - }) - } - - #[test] - fn deprecate_controller_batch_with_bad_state_ok() { - ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // now let's deprecate all the controllers for all the existing ledgers. - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![333, 444, 555, 777]).unwrap(); - - assert_ok!(Staking::deprecate_controller_batch( - RuntimeOrigin::root(), - bounded_controllers - )); - - assert_eq!( - *staking_events().last().unwrap(), - Event::ControllerBatchDeprecated { failures: 0 } - ); - }) - } - - #[test] - fn deprecate_controller_batch_with_bad_state_failures() { - ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // now let's deprecate all the controllers for all the existing ledgers. - let bounded_controllers: BoundedVec< - _, - ::MaxControllersInDeprecationBatch, - > = BoundedVec::try_from(vec![777, 555, 444, 333]).unwrap(); - - assert_ok!(Staking::deprecate_controller_batch( - RuntimeOrigin::root(), - bounded_controllers - )); - - assert_eq!( - *staking_events().last().unwrap(), - Event::ControllerBatchDeprecated { failures: 2 } - ); - }) - } - #[test] fn set_controller_with_bad_state_ok() { ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { diff --git a/substrate/frame/staking/src/weights.rs b/substrate/frame/staking/src/weights.rs index cd4e7f973ce33..22747c548ba9a 100644 --- a/substrate/frame/staking/src/weights.rs +++ b/substrate/frame/staking/src/weights.rs @@ -68,7 +68,6 @@ pub trait WeightInfo { fn force_new_era() -> Weight; fn force_new_era_always() -> Weight; fn set_invulnerables(v: u32, ) -> Weight; - fn deprecate_controller_batch(i: u32, ) -> Weight; fn force_unstake(s: u32, ) -> Weight; fn cancel_deferred_slash(s: u32, ) -> Weight; fn payout_stakers_alive_staked(n: u32, ) -> Weight; @@ -429,25 +428,6 @@ impl WeightInfo for SubstrateWeight { .saturating_add(Weight::from_parts(10_058, 0).saturating_mul(v.into())) .saturating_add(T::DbWeight::get().writes(1_u64)) } - /// Storage: `Staking::Ledger` (r:11800 w:11800) - /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) - /// Storage: `Staking::Bonded` (r:5900 w:5900) - /// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`) - /// Storage: `Staking::Payee` (r:5900 w:0) - /// Proof: `Staking::Payee` (`max_values`: None, `max_size`: Some(73), added: 2548, mode: `MaxEncodedLen`) - /// The range of component `i` is `[0, 5900]`. - fn deprecate_controller_batch(i: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `1746 + i * (229 ±0)` - // Estimated: `990 + i * (7132 ±0)` - // Minimum execution time: 5_300_000 picoseconds. - Weight::from_parts(5_437_000, 990) - // Standard Error: 66_261 - .saturating_add(Weight::from_parts(30_172_457, 0).saturating_mul(i.into())) - .saturating_add(T::DbWeight::get().reads((4_u64).saturating_mul(i.into()))) - .saturating_add(T::DbWeight::get().writes((3_u64).saturating_mul(i.into()))) - .saturating_add(Weight::from_parts(0, 7132).saturating_mul(i.into())) - } /// Storage: `Staking::SlashingSpans` (r:1 w:1) /// Proof: `Staking::SlashingSpans` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `Staking::Bonded` (r:1 w:1) @@ -1179,25 +1159,6 @@ impl WeightInfo for () { .saturating_add(Weight::from_parts(10_058, 0).saturating_mul(v.into())) .saturating_add(RocksDbWeight::get().writes(1_u64)) } - /// Storage: `Staking::Ledger` (r:11800 w:11800) - /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) - /// Storage: `Staking::Bonded` (r:5900 w:5900) - /// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`) - /// Storage: `Staking::Payee` (r:5900 w:0) - /// Proof: `Staking::Payee` (`max_values`: None, `max_size`: Some(73), added: 2548, mode: `MaxEncodedLen`) - /// The range of component `i` is `[0, 5900]`. - fn deprecate_controller_batch(i: u32, ) -> Weight { - // Proof Size summary in bytes: - // Measured: `1746 + i * (229 ±0)` - // Estimated: `990 + i * (7132 ±0)` - // Minimum execution time: 5_300_000 picoseconds. - Weight::from_parts(5_437_000, 990) - // Standard Error: 66_261 - .saturating_add(Weight::from_parts(30_172_457, 0).saturating_mul(i.into())) - .saturating_add(RocksDbWeight::get().reads((4_u64).saturating_mul(i.into()))) - .saturating_add(RocksDbWeight::get().writes((3_u64).saturating_mul(i.into()))) - .saturating_add(Weight::from_parts(0, 7132).saturating_mul(i.into())) - } /// Storage: `Staking::SlashingSpans` (r:1 w:1) /// Proof: `Staking::SlashingSpans` (`max_values`: None, `max_size`: None, mode: `Measured`) /// Storage: `Staking::Bonded` (r:1 w:1) From bc986b8e25879103041db45e304845c86451b4dd Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:09:35 +0700 Subject: [PATCH 02/63] rm event --- substrate/frame/staking/src/pallet/mod.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 6c9b835bf8fb4..4ab716d70b3b9 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -301,7 +301,7 @@ pub mod pallet { #[pallet::unbounded] pub type Invulnerables = StorageValue<_, Vec, ValueQuery>; - /// Map from all locked "stash" accounts to the controller account. + /// Map from all locked "stash" accounts to the now-deprecated controller account. /// /// TWOX-NOTE: SAFE since `AccountId` is a secure hash. #[pallet::storage] @@ -326,6 +326,7 @@ pub mod pallet { pub type MinCommission = StorageValue<_, Perbill, ValueQuery>; /// Map from all (unlocked) "controller" accounts to the info regarding the staking. + /// TODO: Map ledgers from "stash" accounts instead of controllers. /// /// Note: All the reads and mutations to this storage *MUST* be done through the methods exposed /// by [`StakingLedger`] to ensure data and lock consistency. From 3cd2f23ec681b2f3de6170c6f31a5324fc7dc410 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:09:45 +0700 Subject: [PATCH 03/63] rm event --- substrate/frame/staking/src/pallet/mod.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 4ab716d70b3b9..2b73903fff4b0 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -796,8 +796,6 @@ pub mod pallet { SnapshotTargetsSizeExceeded { size: u32 }, /// A new force era mode was set. ForceEra { mode: Forcing }, - /// Report of a controller batch deprecation. - ControllerBatchDeprecated { failures: u32 }, } #[pallet::error] From c681a448d3d3f57552442c7b7e8644e0ea60b522 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:14:31 +0700 Subject: [PATCH 04/63] bond: rm controller cannot stash --- substrate/frame/staking/src/lib.rs | 2 +- substrate/frame/staking/src/pallet/mod.rs | 8 +------- substrate/frame/staking/src/tests.rs | 22 ---------------------- 3 files changed, 2 insertions(+), 30 deletions(-) diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index bfa091efe2945..053ecdef2b00b 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -400,7 +400,7 @@ impl Default for EraRewardPoints { #[derive(PartialEq, Eq, Copy, Clone, Encode, Decode, RuntimeDebug, TypeInfo, MaxEncodedLen)] pub enum RewardDestination { /// Pay into the stash account, increasing the amount at stake accordingly. -Staked, + Staked, /// Pay into the stash account, not increasing the amount at stake. Stash, #[deprecated( diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 2b73903fff4b0..1711a4d8d74f1 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -921,8 +921,7 @@ pub mod pallet { #[pallet::call] impl Pallet { - /// Take the origin account as a stash and lock up `value` of its balance. `controller` will - /// be the account that controls it. + /// Take the origin account as a stash and lock up `value` of its balance. /// /// `value` must be more than the `minimum_balance` specified by `T::Currency`. /// @@ -950,11 +949,6 @@ pub mod pallet { return Err(Error::::AlreadyBonded.into()) } - // An existing controller cannot become a stash. - if StakingLedger::::is_bonded(StakingAccount::Controller(stash.clone())) { - return Err(Error::::AlreadyPaired.into()) - } - // Reject a bond which is considered to be _dust_. if value < T::Currency::minimum_balance() { return Err(Error::::InsufficientBond.into()) diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index f6668f83b2f1d..2c3b95dcdd7f6 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -7537,28 +7537,6 @@ mod ledger { }) } - #[test] - fn bond_controller_cannot_be_stash_works() { - ExtBuilder::default().build_and_execute(|| { - let (stash, controller) = testing_utils::create_unique_stash_controller::( - 0, - 10, - RewardDestination::Staked, - false, - ) - .unwrap(); - - assert_eq!(Bonded::::get(stash), Some(controller)); - assert_eq!(Ledger::::get(controller).map(|l| l.stash), Some(stash)); - - // existing controller should not be able become a stash. - assert_noop!( - Staking::bond(RuntimeOrigin::signed(controller), 10, RewardDestination::Staked), - Error::::AlreadyPaired, - ); - }) - } - #[test] fn is_bonded_works() { ExtBuilder::default().build_and_execute(|| { From 1c6f60fa38aca88c1c4a73bd1611a3a87319e9ec Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:23:49 +0700 Subject: [PATCH 05/63] do_withdraw_unbonded: use stash --- substrate/frame/staking/src/pallet/impls.rs | 10 +++++----- substrate/frame/staking/src/pallet/mod.rs | 15 +++++++-------- 2 files changed, 12 insertions(+), 13 deletions(-) diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 90374451a3a52..96ca6ca8a4232 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -188,11 +188,11 @@ impl Pallet { } pub(super) fn do_withdraw_unbonded( - controller: &T::AccountId, + stash: &T::AccountId, num_slashing_spans: u32, ) -> Result { - let mut ledger = Self::ledger(Controller(controller.clone()))?; - let (stash, old_total) = (ledger.stash.clone(), ledger.total); + let mut ledger = Self::ledger(Stash(stash.clone()))?; + let old_total = ledger.total; if let Some(current_era) = Self::current_era() { ledger = ledger.consolidate_unlocked(current_era) } @@ -220,10 +220,10 @@ impl Pallet { if new_total < old_total { // Already checked that this won't overflow by entry condition. let value = old_total.defensive_saturating_sub(new_total); - Self::deposit_event(Event::::Withdrawn { stash, amount: value }); + Self::deposit_event(Event::::Withdrawn { stash: stash.clone(), amount: value }); // notify listeners. - T::EventListeners::on_withdraw(controller, value); + T::EventListeners::on_withdraw(stash, value); } Ok(used_weight) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 1711a4d8d74f1..0006c4cfe4807 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -971,7 +971,7 @@ pub mod pallet { /// Add some extra amount that have appeared in the stash `free_balance` into the balance up /// for staking. /// - /// The dispatch origin for this call must be _Signed_ by the stash, not the controller. + /// The dispatch origin for this call must be _Signed_ by the stash. /// /// Use this if there are additional funds in your stash account that you wish to bond. /// Unlike [`bond`](Self::bond) or [`unbond`](Self::unbond) this function does not impose @@ -996,7 +996,7 @@ pub mod pallet { /// period ends. If this leaves an amount actively bonded less than /// T::Currency::minimum_balance(), then it is increased to the full amount. /// - /// The dispatch origin for this call must be _Signed_ by the controller, not the stash. + /// The dispatch origin for this call must be _Signed_ by the stash. /// /// Once the unlock period is done, you can call `withdraw_unbonded` to actually move /// the funds out of management ready for transfer. @@ -1019,17 +1019,16 @@ pub mod pallet { origin: OriginFor, #[pallet::compact] value: BalanceOf, ) -> DispatchResultWithPostInfo { - let controller = ensure_signed(origin)?; - let unlocking = - Self::ledger(Controller(controller.clone())).map(|l| l.unlocking.len())?; + let stash = ensure_signed(origin)?; + let unlocking = Self::ledger(Stash(stash.clone())).map(|l| l.unlocking.len())?; // if there are no unlocking chunks available, try to withdraw chunks older than // `BondingDuration` to proceed with the unbonding. let maybe_withdraw_weight = { if unlocking == T::MaxUnlockingChunks::get() as usize { let real_num_slashing_spans = - Self::slashing_spans(&controller).map_or(0, |s| s.iter().count()); - Some(Self::do_withdraw_unbonded(&controller, real_num_slashing_spans as u32)?) + Self::slashing_spans(&stash).map_or(0, |s| s.iter().count()); + Some(Self::do_withdraw_unbonded(&stash, real_num_slashing_spans as u32)?) } else { None } @@ -1037,7 +1036,7 @@ pub mod pallet { // we need to fetch the ledger again because it may have been mutated in the call // to `Self::do_withdraw_unbonded` above. - let mut ledger = Self::ledger(Controller(controller))?; + let mut ledger = Self::ledger(Stash(stash))?; let mut value = value.min(ledger.active); let stash = ledger.stash.clone(); From b9a920291ddd9911dc64e5c4d26ad8f1d7aa9737 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:26:10 +0700 Subject: [PATCH 06/63] withdraw_unbonded: use stash --- substrate/frame/staking/src/pallet/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 0006c4cfe4807..1fbd150ba263b 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1106,7 +1106,7 @@ pub mod pallet { /// This essentially frees up that balance to be used by the stash account to do whatever /// it wants. /// - /// The dispatch origin for this call must be _Signed_ by the controller. + /// The dispatch origin for this call must be _Signed_ by the stash. /// /// Emits `Withdrawn`. /// @@ -1130,9 +1130,9 @@ pub mod pallet { origin: OriginFor, num_slashing_spans: u32, ) -> DispatchResultWithPostInfo { - let controller = ensure_signed(origin)?; + let stash = ensure_signed(origin)?; - let actual_weight = Self::do_withdraw_unbonded(&controller, num_slashing_spans)?; + let actual_weight = Self::do_withdraw_unbonded(&stash, num_slashing_spans)?; Ok(Some(actual_weight).into()) } From fdb0458964da2f209f617098592e2ba9f543ae72 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:32:33 +0700 Subject: [PATCH 07/63] validate: uses stash --- substrate/frame/staking/src/pallet/mod.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 1fbd150ba263b..c32eddf0c7403 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1136,17 +1136,17 @@ pub mod pallet { Ok(Some(actual_weight).into()) } - /// Declare the desire to validate for the origin controller. + /// Declare the desire to validate for the given stash. /// /// Effects will be felt at the beginning of the next era. /// - /// The dispatch origin for this call must be _Signed_ by the controller, not the stash. + /// The dispatch origin for this call must be _Signed_ by the stash. #[pallet::call_index(4)] #[pallet::weight(T::WeightInfo::validate())] pub fn validate(origin: OriginFor, prefs: ValidatorPrefs) -> DispatchResult { - let controller = ensure_signed(origin)?; + let stash = ensure_signed(origin)?; - let ledger = Self::ledger(Controller(controller))?; + let ledger = Self::ledger(Stash(stash))?; ensure!(ledger.active >= MinValidatorBond::::get(), Error::::InsufficientBond); let stash = &ledger.stash; From fc80bdf41e206887c9ba0073cde1957589000b6d Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:32:50 +0700 Subject: [PATCH 08/63] rm obsolete test, temp-fix for test --- substrate/frame/staking/src/tests.rs | 31 +++------------------------- 1 file changed, 3 insertions(+), 28 deletions(-) diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 2c3b95dcdd7f6..610052cf68bf2 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -275,40 +275,15 @@ fn change_controller_works() { assert_eq!(raw_ledger.controller, None); // `controller` is no longer in control. `stash` is now controller. + // TODO: Remove this test once controller logic is removed. assert_noop!( Staking::validate(RuntimeOrigin::signed(controller), ValidatorPrefs::default()), - Error::::NotController, + Error::::NotStash, // NOTE: was `SetController`. ); assert_ok!(Staking::validate(RuntimeOrigin::signed(stash), ValidatorPrefs::default())); }) } -#[test] -fn change_controller_already_paired_once_stash() { - ExtBuilder::default().build_and_execute(|| { - // 11 and 11 are bonded as controller and stash respectively. - assert_eq!(Staking::bonded(&11), Some(11)); - - // 11 is initially a validator. - assert_ok!(Staking::chill(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); - - // 10 is no longer in control. - assert_noop!( - Staking::validate(RuntimeOrigin::signed(10), ValidatorPrefs::default()), - Error::::NotController, - ); - assert_ok!(Staking::validate(RuntimeOrigin::signed(11), ValidatorPrefs::default())); - }) -} - #[test] fn rewards_should_work() { ExtBuilder::default().nominate(true).session_per_era(3).build_and_execute(|| { @@ -435,7 +410,7 @@ fn staking_should_work() { // --- Block 2: start_session(2); - // add a new candidate for being a validator. account 3 controlled by 4. + // add a new candidate for being a validator. assert_ok!(Staking::bond(RuntimeOrigin::signed(3), 1500, RewardDestination::Account(3))); assert_ok!(Staking::validate(RuntimeOrigin::signed(3), ValidatorPrefs::default())); assert_ok!(Session::set_keys( From 3b2df257fc6342f1c0eccbd8e7ed96b3898901fc Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:33:03 +0700 Subject: [PATCH 09/63] benchmarks: use stash for fns so far --- substrate/frame/staking/src/benchmarking.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index b299df7569769..3bc9b575fd1c7 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -279,8 +279,8 @@ benchmarks! { let ledger = Ledger::::get(&controller).ok_or("ledger not created before")?; let original_bonded: BalanceOf = ledger.active; - whitelist_account!(controller); - }: _(RawOrigin::Signed(controller.clone()), amount) + whitelist_account!(stash); + }: _(RawOrigin::Signed(stash.clone()), amount) verify { let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; let new_bonded: BalanceOf = ledger.active; @@ -299,7 +299,7 @@ benchmarks! { let ledger = Ledger::::get(&controller).ok_or("ledger not created before")?; let original_total: BalanceOf = ledger.total; whitelist_account!(controller); - }: withdraw_unbonded(RawOrigin::Signed(controller.clone()), s) + }: withdraw_unbonded(RawOrigin::Signed(stash.clone()), s) verify { let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; let new_total: BalanceOf = ledger.total; @@ -330,14 +330,14 @@ benchmarks! { CurrentEra::::put(EraIndex::max_value()); whitelist_account!(controller); - }: withdraw_unbonded(RawOrigin::Signed(controller.clone()), s) + }: withdraw_unbonded(RawOrigin::Signed(stash.clone()), s) verify { assert!(!Ledger::::contains_key(controller)); assert!(!T::VoterList::contains(&stash)); } validate { - let (stash, controller) = create_stash_controller::( + let (stash,) = create_stash_controller::( MaxNominationsOf::::get() - 1, 100, RewardDestination::Staked, @@ -346,8 +346,8 @@ benchmarks! { assert!(!T::VoterList::contains(&stash)); let prefs = ValidatorPrefs::default(); - whitelist_account!(controller); - }: _(RawOrigin::Signed(controller), prefs) + whitelist_account!(stash); + }: _(RawOrigin::Signed(stash), prefs) verify { assert!(Validators::::contains_key(&stash)); assert!(T::VoterList::contains(&stash)); From a3b805126e388dd0a6ce78f61d86dd6cc8d6f7f9 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:44:59 +0700 Subject: [PATCH 10/63] benchmarks: fix tuple, nominate takes stash --- substrate/frame/staking/src/benchmarking.rs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 3bc9b575fd1c7..f290dee4a0226 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -337,7 +337,7 @@ benchmarks! { } validate { - let (stash,) = create_stash_controller::( + let (stash,_) = create_stash_controller::( MaxNominationsOf::::get() - 1, 100, RewardDestination::Staked, @@ -428,7 +428,7 @@ benchmarks! { // setup a worst case list scenario. Note we don't care about the destination position, because // we are just doing an insert into the origin position. let scenario = ListScenario::::new(origin_weight, true)?; - let (stash, controller) = create_stash_controller_with_balance::( + let (stash,_) = create_stash_controller_with_balance::( SEED + MaxNominationsOf::::get() + 1, // make sure the account does not conflict with others origin_weight, RewardDestination::Staked, @@ -438,8 +438,8 @@ benchmarks! { assert!(!T::VoterList::contains(&stash)); let validators = create_validators::(n, 100).unwrap(); - whitelist_account!(controller); - }: _(RawOrigin::Signed(controller), validators) + whitelist_account!(stash); + }: _(RawOrigin::Signed(stash), validators) verify { assert!(Nominators::::contains_key(&stash)); assert!(T::VoterList::contains(&stash)) From 9e3a4ce779da54765b4cf81d1b09ce91c2296e0f Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:49:51 +0700 Subject: [PATCH 11/63] nominate takes stash, add TODOs --- substrate/frame/staking/src/mock.rs | 1 + substrate/frame/staking/src/pallet/mod.rs | 8 ++++---- substrate/frame/staking/src/tests.rs | 21 ++++++++++----------- 3 files changed, 15 insertions(+), 15 deletions(-) diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 9bc6287910901..d451f557cb94a 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -483,6 +483,7 @@ impl ExtBuilder { if self.has_stakers { stakers = vec![ // (stash, ctrl, stake, status) + // TODO: Remove `ctrl` from stakers. // these two will be elected in the default test where we elect 2. (11, 11, self.balance_factor * 1000, StakerStatus::::Validator), (21, 21, self.balance_factor * 1000, StakerStatus::::Validator), diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index c32eddf0c7403..abb241971607d 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1174,11 +1174,11 @@ pub mod pallet { Ok(()) } - /// Declare the desire to nominate `targets` for the origin controller. + /// Declare the desire to nominate `targets` for a stash. /// /// Effects will be felt at the beginning of the next era. /// - /// The dispatch origin for this call must be _Signed_ by the controller, not the stash. + /// The dispatch origin for this call must be _Signed_ by the stash. /// /// ## Complexity /// - The transaction's complexity is proportional to the size of `targets` (N) @@ -1190,9 +1190,9 @@ pub mod pallet { origin: OriginFor, targets: Vec>, ) -> DispatchResult { - let controller = ensure_signed(origin)?; + let stash = ensure_signed(origin)?; - let ledger = Self::ledger(StakingAccount::Controller(controller.clone()))?; + let ledger = Self::ledger(Stash(stash))?; ensure!(ledger.active >= MinNominatorBond::::get(), Error::::InsufficientBond); let stash = &ledger.stash; diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 610052cf68bf2..00f217a7b07b2 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -757,7 +757,6 @@ 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().try_state(false).build_and_execute(|| { let arbitrary_value = 5; let (stash, controller) = testing_utils::create_unique_stash_controller::( @@ -777,13 +776,13 @@ fn double_staking_should_fail() { ), Error::::AlreadyBonded, ); - // stash => attempting to nominate should fail. + // controller => attempting to nominate should fail. assert_noop!( - Staking::nominate(RuntimeOrigin::signed(stash), vec![1]), - Error::::NotController + Staking::nominate(RuntimeOrigin::signed(controller), vec![1]), + Error::::NotStash // TODO: Remove this assert once controller is gone. ); - // controller => nominating should work. - assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller), vec![1])); + // stash => nominating should work. + assert_ok!(Staking::nominate(RuntimeOrigin::signed(stash), vec![1])); }); } @@ -5812,14 +5811,14 @@ fn capped_stakers_works() { // same with nominators let mut some_existing_nominator = AccountId::default(); for i in 0..max - nominator_count { - let (_, controller) = testing_utils::create_stash_controller::( + let (stash, _) = testing_utils::create_stash_controller::( i + 20_000_000, 100, RewardDestination::Stash, ) .unwrap(); - assert_ok!(Staking::nominate(RuntimeOrigin::signed(controller), vec![1])); - some_existing_nominator = controller; + assert_ok!(Staking::nominate(RuntimeOrigin::signed(stash), vec![1])); + some_existing_nominator = stash; } // one more is too many. @@ -6415,8 +6414,8 @@ fn reducing_max_unlocking_chunks_abrupt() { // 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(3), 20), Error::::NotController); - assert_noop!(Staking::rebond(RuntimeOrigin::signed(3), 100), Error::::NotController); + assert_noop!(Staking::unbond(RuntimeOrigin::signed(3), 20), Error::::NotController); // TODO: use `NotStash` once variant is removed from ledeger. + assert_noop!(Staking::rebond(RuntimeOrigin::signed(3), 100), Error::::NotController); // TODO: use `NotStash` once variant is removed from ledeger. // reset the ledger corruption MaxUnlockingChunks::set(2); From 2f5e6e2f0798c0336a826a13216ce8e1b1ad7fb7 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:55:20 +0700 Subject: [PATCH 12/63] chill: takes stash --- substrate/frame/staking/src/benchmarking.rs | 5 ++--- substrate/frame/staking/src/pallet/mod.rs | 6 +++--- substrate/frame/staking/src/tests.rs | 2 +- 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index f290dee4a0226..0e847d5d1a004 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -454,12 +454,11 @@ benchmarks! { // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. let scenario = ListScenario::::new(origin_weight, true)?; - let controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1; assert!(T::VoterList::contains(&stash)); - whitelist_account!(controller); - }: _(RawOrigin::Signed(controller)) + whitelist_account!(stash); + }: _(RawOrigin::Signed(stash)) verify { assert!(!T::VoterList::contains(&stash)); } diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index abb241971607d..ca2691a9e5b43 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1250,7 +1250,7 @@ pub mod pallet { /// /// Effects will be felt at the beginning of the next era. /// - /// The dispatch origin for this call must be _Signed_ by the controller, not the stash. + /// The dispatch origin for this call must be _Signed_ by the stash. /// /// ## Complexity /// - Independent of the arguments. Insignificant complexity. @@ -1259,9 +1259,9 @@ pub mod pallet { #[pallet::call_index(6)] #[pallet::weight(T::WeightInfo::chill())] pub fn chill(origin: OriginFor) -> DispatchResult { - let controller = ensure_signed(origin)?; + let stash = ensure_signed(origin)?; - let ledger = Self::ledger(StakingAccount::Controller(controller))?; + let ledger = Self::ledger(StakingAccount::Stash(stash))?; Self::chill_stash(&ledger.stash); Ok(()) diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 00f217a7b07b2..1c829b8956833 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -253,7 +253,7 @@ fn change_controller_works() { assert_eq!(Staking::bonded(&stash), Some(controller)); // `controller` can control `stash` who is initially a validator. - assert_ok!(Staking::chill(RuntimeOrigin::signed(controller))); + assert_ok!(Staking::chill(RuntimeOrigin::signed(stash))); // sets controller back to `stash`. assert_ok!(Staking::set_controller(RuntimeOrigin::signed(stash))); From a82d5114df050ecc50f2600208c0ea06e910b86c Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 09:57:06 +0700 Subject: [PATCH 13/63] set_payee: takes stash --- substrate/frame/staking/src/benchmarking.rs | 4 ++-- substrate/frame/staking/src/pallet/mod.rs | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 0e847d5d1a004..9bd4785f1bcc9 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -466,8 +466,8 @@ benchmarks! { set_payee { let (stash, controller) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; assert_eq!(Payee::::get(&stash), Some(RewardDestination::Staked)); - whitelist_account!(controller); - }: _(RawOrigin::Signed(controller.clone()), RewardDestination::Account(controller.clone())) + whitelist_account!(stash); + }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(controller.clone())) verify { assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(controller))); } diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index ca2691a9e5b43..73b5ae673dca5 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1267,11 +1267,11 @@ pub mod pallet { Ok(()) } - /// (Re-)set the payment target for a controller. + /// (Re-)set the payment target for a stash. /// /// Effects will be felt instantly (as soon as this function is completed successfully). /// - /// The dispatch origin for this call must be _Signed_ by the controller, not the stash. + /// The dispatch origin for this call must be _Signed_ by the stash. /// /// ## Complexity /// - O(1) @@ -1285,8 +1285,8 @@ pub mod pallet { origin: OriginFor, payee: RewardDestination, ) -> DispatchResult { - let controller = ensure_signed(origin)?; - let ledger = Self::ledger(Controller(controller.clone()))?; + let stash = ensure_signed(origin)?; + let ledger = Self::ledger(Stash(stash.clone()))?; ensure!( (payee != { From 477e3e9bc17697236ffb512892fd92a65a9fc914 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 10:02:21 +0700 Subject: [PATCH 14/63] rm set_controller call --- .../westend/src/weights/pallet_staking.rs | 14 ---- substrate/frame/staking/README.md | 3 - substrate/frame/staking/src/benchmarking.rs | 13 ---- substrate/frame/staking/src/lib.rs | 4 - substrate/frame/staking/src/pallet/mod.rs | 34 -------- substrate/frame/staking/src/tests.rs | 77 ------------------- substrate/frame/staking/src/weights.rs | 27 ------- 7 files changed, 172 deletions(-) diff --git a/polkadot/runtime/westend/src/weights/pallet_staking.rs b/polkadot/runtime/westend/src/weights/pallet_staking.rs index 3d79a648d8911..20a316d0d2dfd 100644 --- a/polkadot/runtime/westend/src/weights/pallet_staking.rs +++ b/polkadot/runtime/westend/src/weights/pallet_staking.rs @@ -334,20 +334,6 @@ impl pallet_staking::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(3)) .saturating_add(T::DbWeight::get().writes(1)) } - /// Storage: `Staking::Bonded` (r:1 w:1) - /// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`) - /// Storage: `Staking::Ledger` (r:2 w:2) - /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) - fn set_controller() -> Weight { - // Proof Size summary in bytes: - // Measured: `865` - // Estimated: `8122` - // Minimum execution time: 21_159_000 picoseconds. - Weight::from_parts(21_706_000, 0) - .saturating_add(Weight::from_parts(0, 8122)) - .saturating_add(T::DbWeight::get().reads(3)) - .saturating_add(T::DbWeight::get().writes(3)) - } /// Storage: `Staking::ValidatorCount` (r:0 w:1) /// Proof: `Staking::ValidatorCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) fn set_validator_count() -> Weight { diff --git a/substrate/frame/staking/README.md b/substrate/frame/staking/README.md index 2938e2fe77066..0801473591537 100644 --- a/substrate/frame/staking/README.md +++ b/substrate/frame/staking/README.md @@ -51,9 +51,6 @@ issues instructions on how funds shall be used. An account can become a bonded stash account using the [`bond`](https://docs.rs/pallet-staking/latest/pallet_staking/enum.Call.html#variant.bond) 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. - 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. diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 9bd4785f1bcc9..d32a0a99ea43b 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -484,19 +484,6 @@ benchmarks! { assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(controller))); } - set_controller { - let (stash, ctlr) = create_unique_stash_controller::(9000, 100, RewardDestination::Staked, false)?; - // 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 { - assert!(Ledger::::contains_key(&stash)); - } - set_validator_count { let validator_count = MaxValidators::::get(); }: _(RawOrigin::Root, validator_count) diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index 053ecdef2b00b..e77749a2881b0 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -67,10 +67,6 @@ //! //! An account can become a bonded stash account using the [`bond`](Call::bond) call. //! -//! In the event stash accounts registered a unique controller account before the controller account -//! deprecation, they 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` //! and `Idle` (defined in [`StakerStatus`]). There are three corresponding instructions to change //! between roles, namely: [`validate`](Call::validate), [`nominate`](Call::nominate), and diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 73b5ae673dca5..d8b738a77435a 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1303,40 +1303,6 @@ 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. - /// - /// Effects will be felt instantly (as soon as this function is completed successfully). - /// - /// The dispatch origin for this call must be _Signed_ by the stash, not the controller. - /// - /// ## Complexity - /// O(1) - /// - Independent of the arguments. Insignificant complexity. - /// - Contains a limited number of reads. - /// - Writes are limited to the `origin` account key. - #[pallet::call_index(8)] - #[pallet::weight(T::WeightInfo::set_controller())] - pub fn set_controller(origin: OriginFor) -> DispatchResult { - let stash = ensure_signed(origin)?; - - Self::ledger(StakingAccount::Stash(stash.clone())).map(|ledger| { - let controller = ledger.controller() - .defensive_proof("Ledger's controller field didn't exist. The controller should have been fetched using StakingLedger.") - .ok_or(Error::::NotController)?; - - if controller == stash { - // Stash is already its own controller. - return Err(Error::::AlreadyPaired.into()) - } - - let _ = ledger.set_controller_to_stash()?; - Ok(()) - })? - } - /// Sets the ideal number of validators. /// /// The dispatch origin must be Root. diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 1c829b8956833..5d751848857f0 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -238,52 +238,6 @@ 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)); - - // `controller` can control `stash` who is initially a validator. - assert_ok!(Staking::chill(RuntimeOrigin::signed(stash))); - - // sets controller back to `stash`. - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(stash))); - assert_eq!(Staking::bonded(&stash), Some(stash)); - mock::start_active_era(1); - - // fetch the ledger from storage and check if the controller is correct. - let ledger = Staking::ledger(StakingAccount::Stash(stash)).unwrap(); - assert_eq!(ledger.controller(), Some(stash)); - - // same if we fetch the ledger by controller. - let ledger = Staking::ledger(StakingAccount::Controller(stash)).unwrap(); - assert_eq!(ledger.controller, Some(stash)); - assert_eq!(ledger.controller(), Some(stash)); - - // the raw storage ledger's controller is always `None`. however, we can still fetch the - // correct controller with `ledger.controller()`. - let raw_ledger = >::get(&stash).unwrap(); - assert_eq!(raw_ledger.controller, None); - - // `controller` is no longer in control. `stash` is now controller. - // TODO: Remove this test once controller logic is removed. - assert_noop!( - Staking::validate(RuntimeOrigin::signed(controller), ValidatorPrefs::default()), - Error::::NotStash, // NOTE: was `SetController`. - ); - assert_ok!(Staking::validate(RuntimeOrigin::signed(stash), ValidatorPrefs::default())); - }) -} - #[test] fn rewards_should_work() { ExtBuilder::default().nominate(true).session_per_era(3).build_and_execute(|| { @@ -7562,37 +7516,6 @@ mod ledger { assert_eq!(Payee::::get(&21), Some(RewardDestination::Stash)); }) } - - #[test] - fn set_controller_with_bad_state_ok() { - ExtBuilder::default().has_stakers(false).nominate(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // in this case, setting controller works due to the ordering of the calls. - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(444))); - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(555))); - }) - } - - #[test] - fn set_controller_with_bad_state_fails() { - ExtBuilder::default().has_stakers(false).try_state(false).build_and_execute(|| { - setup_double_bonded_ledgers(); - - // setting the controller of ledger associated with stash 555 fails since its stash is a - // controller of another ledger. - assert_noop!( - Staking::set_controller(RuntimeOrigin::signed(555)), - Error::::BadState - ); - assert_noop!( - Staking::set_controller(RuntimeOrigin::signed(444)), - Error::::BadState - ); - assert_ok!(Staking::set_controller(RuntimeOrigin::signed(333))); - }) - } } mod ledger_recovery { diff --git a/substrate/frame/staking/src/weights.rs b/substrate/frame/staking/src/weights.rs index 22747c548ba9a..fc1451a7519c5 100644 --- a/substrate/frame/staking/src/weights.rs +++ b/substrate/frame/staking/src/weights.rs @@ -62,7 +62,6 @@ pub trait WeightInfo { fn chill() -> Weight; fn set_payee() -> Weight; fn update_payee() -> Weight; - fn set_controller() -> Weight; fn set_validator_count() -> Weight; fn force_no_eras() -> Weight; fn force_new_era() -> Weight; @@ -362,19 +361,6 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(3_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } - /// Storage: `Staking::Bonded` (r:1 w:1) - /// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`) - /// Storage: `Staking::Ledger` (r:2 w:2) - /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) - fn set_controller() -> Weight { - // Proof Size summary in bytes: - // Measured: `902` - // Estimated: `8122` - // Minimum execution time: 23_479_000 picoseconds. - Weight::from_parts(24_502_000, 8122) - .saturating_add(T::DbWeight::get().reads(3_u64)) - .saturating_add(T::DbWeight::get().writes(3_u64)) - } /// Storage: `Staking::ValidatorCount` (r:0 w:1) /// Proof: `Staking::ValidatorCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) fn set_validator_count() -> Weight { @@ -1093,19 +1079,6 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(3_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } - /// Storage: `Staking::Bonded` (r:1 w:1) - /// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`) - /// Storage: `Staking::Ledger` (r:2 w:2) - /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) - fn set_controller() -> Weight { - // Proof Size summary in bytes: - // Measured: `902` - // Estimated: `8122` - // Minimum execution time: 23_479_000 picoseconds. - Weight::from_parts(24_502_000, 8122) - .saturating_add(RocksDbWeight::get().reads(3_u64)) - .saturating_add(RocksDbWeight::get().writes(3_u64)) - } /// Storage: `Staking::ValidatorCount` (r:0 w:1) /// Proof: `Staking::ValidatorCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) fn set_validator_count() -> Weight { From 93eddce751277ac863020b08e55847c5a75af02f Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 10:03:09 +0700 Subject: [PATCH 15/63] rm obsolete fn: set_controller_to_stash --- substrate/frame/staking/src/ledger.rs | 24 ------------------------ 1 file changed, 24 deletions(-) diff --git a/substrate/frame/staking/src/ledger.rs b/substrate/frame/staking/src/ledger.rs index 294918376d82c..a9300b4a335ab 100644 --- a/substrate/frame/staking/src/ledger.rs +++ b/substrate/frame/staking/src/ledger.rs @@ -233,30 +233,6 @@ impl StakingLedger { Ok(()) } - /// Sets the ledger controller to its stash. - pub(crate) fn set_controller_to_stash(self) -> Result<(), Error> { - let controller = self.controller.as_ref() - .defensive_proof("Ledger's controller field didn't exist. The controller should have been fetched using StakingLedger.") - .ok_or(Error::::NotController)?; - - ensure!(self.stash != *controller, Error::::AlreadyPaired); - - // check if the ledger's stash is a controller of another ledger. - if let Some(bonded_ledger) = Ledger::::get(&self.stash) { - // there is a ledger bonded by the stash. In this case, the stash of the bonded ledger - // should be the same as the ledger's stash. Otherwise fail to prevent data - // inconsistencies. See for more - // details. - ensure!(bonded_ledger.stash == self.stash, Error::::BadState); - } - - >::remove(&controller); - >::insert(&self.stash, &self); - >::insert(&self.stash, &self.stash); - - Ok(()) - } - /// Clears all data related to a staking ledger and its bond in both [`Ledger`] and [`Bonded`] /// storage items and updates the stash staking lock. pub(crate) fn kill(stash: &T::AccountId) -> Result<(), Error> { From df11287d403940b2d8f4f01f86d88374bd4473b6 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 10:07:12 +0700 Subject: [PATCH 16/63] rebond: takes stash --- substrate/frame/staking/src/benchmarking.rs | 4 ++-- substrate/frame/staking/src/ledger.rs | 5 +---- substrate/frame/staking/src/pallet/mod.rs | 6 +++--- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index d32a0a99ea43b..1ff81a7781c82 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -627,8 +627,8 @@ benchmarks! { Ledger::::insert(controller.clone(), staking_ledger.clone()); let original_bonded: BalanceOf = staking_ledger.active; - whitelist_account!(controller); - }: _(RawOrigin::Signed(controller.clone()), rebond_amount) + whitelist_account!(stash); + }: _(RawOrigin::Signed(stash.clone()), rebond_amount) verify { let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; let new_bonded: BalanceOf = ledger.active; diff --git a/substrate/frame/staking/src/ledger.rs b/substrate/frame/staking/src/ledger.rs index a9300b4a335ab..28894191554b7 100644 --- a/substrate/frame/staking/src/ledger.rs +++ b/substrate/frame/staking/src/ledger.rs @@ -31,10 +31,7 @@ //! performed through the methods exposed by the [`StakingLedger`] implementation in order to ensure //! state consistency. -use frame_support::{ - defensive, ensure, - traits::{Defensive, LockableCurrency}, -}; +use frame_support::{defensive, ensure, traits::LockableCurrency}; use sp_staking::{StakingAccount, StakingInterface}; use sp_std::prelude::*; diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index d8b738a77435a..158a39546c6e0 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1527,7 +1527,7 @@ pub mod pallet { /// Rebond a portion of the stash scheduled to be unlocked. /// - /// The dispatch origin must be signed by the controller. + /// The dispatch origin must be signed by the stash. /// /// ## Complexity /// - Time complexity: O(L), where L is unlocking chunks @@ -1538,8 +1538,8 @@ pub mod pallet { origin: OriginFor, #[pallet::compact] value: BalanceOf, ) -> DispatchResultWithPostInfo { - let controller = ensure_signed(origin)?; - let ledger = Self::ledger(Controller(controller))?; + let stash = ensure_signed(origin)?; + let ledger = Self::ledger(Stash(stash))?; ensure!(!ledger.unlocking.is_empty(), Error::::NoUnlockChunk); let initial_unlocking = ledger.unlocking.len() as u32; From 92995ce75ea02b1fbf9e04ccbb415f4c8b1dc9d1 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 10:12:21 +0700 Subject: [PATCH 17/63] kick: takes stash --- substrate/frame/staking/src/benchmarking.rs | 10 +++++----- substrate/frame/staking/src/pallet/mod.rs | 8 ++++---- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 1ff81a7781c82..56e7ccc00b6df 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -373,14 +373,14 @@ benchmarks! { let stash_lookup = T::Lookup::unlookup(stash.clone()); // they start validating. - Staking::::validate(RawOrigin::Signed(controller.clone()).into(), Default::default())?; + Staking::::validate(RawOrigin::Signed(stash.clone()).into(), Default::default())?; // we now create the nominators. there will be `k` of them; each will nominate all // validators. we will then kick each of the `k` nominators from the main validator. let mut nominator_stashes = Vec::with_capacity(k as usize); for i in 0 .. k { // create a nominator stash. - let (n_stash, n_controller) = create_stash_controller::( + let (n_stash, _) = create_stash_controller::( MaxNominationsOf::::get() + i, 100, RewardDestination::Staked, @@ -392,7 +392,7 @@ benchmarks! { // optimisations/pessimisations. nominations.insert(i as usize % (nominations.len() + 1), stash_lookup.clone()); // then we nominate. - Staking::::nominate(RawOrigin::Signed(n_controller.clone()).into(), nominations)?; + Staking::::nominate(RawOrigin::Signed(n_stash.clone()).into(), nominations)?; nominator_stashes.push(n_stash); } @@ -407,8 +407,8 @@ benchmarks! { .map(|n| T::Lookup::unlookup(n.clone())) .collect::>(); - whitelist_account!(controller); - }: _(RawOrigin::Signed(controller), kicks) + whitelist_account!(stash); + }: _(RawOrigin::Signed(stash), kicks) verify { // all nominators now should *not* be nominating our validator... for n in nominator_stashes.iter() { diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 158a39546c6e0..eeb9089c3011f 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -38,7 +38,7 @@ use sp_runtime::{ use sp_staking::{ EraIndex, Page, SessionIndex, - StakingAccount::{self, Controller, Stash}, + StakingAccount::{self, Stash}, StakingInterface, }; use sp_std::prelude::*; @@ -1617,7 +1617,7 @@ pub mod pallet { /// /// Effects will be felt at the beginning of the next era. /// - /// The dispatch origin for this call must be _Signed_ by the controller, not the stash. + /// The dispatch origin for this call must be _Signed_ by the stash. /// /// - `who`: A list of nominator stash accounts who are nominating this validator which /// should no longer be nominating this validator. @@ -1627,8 +1627,8 @@ pub mod pallet { #[pallet::call_index(21)] #[pallet::weight(T::WeightInfo::kick(who.len() as u32))] pub fn kick(origin: OriginFor, who: Vec>) -> DispatchResult { - let controller = ensure_signed(origin)?; - let ledger = Self::ledger(Controller(controller))?; + let signed = ensure_signed(origin)?; + let ledger = &Self::ledger(Stash(signed))?; let stash = &ledger.stash; for nom_stash in who From 3717d0e25ce257bcabd7252266077a9f1d90dbd3 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 10:15:47 +0700 Subject: [PATCH 18/63] chill_other: check caller is not stash instead --- substrate/frame/staking/src/pallet/mod.rs | 20 +++++++------------- 1 file changed, 7 insertions(+), 13 deletions(-) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index eeb9089c3011f..d97ac37d66e06 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1708,19 +1708,19 @@ pub mod pallet { config_op_exp!(MaxStakedRewards, max_staked_rewards); Ok(()) } - /// Declare a `controller` to stop participating as either a validator or nominator. + /// Declare a `stash` to stop participating as either a validator or nominator. /// /// Effects will be felt at the beginning of the next era. /// /// The dispatch origin for this call must be _Signed_, but can be called by anyone. /// - /// If the caller is the same as the controller being targeted, then no further checks are + /// If the caller is the same as the stash being targeted, then no further checks are /// enforced, and this function behaves just like `chill`. /// - /// If the caller is different than the controller being targeted, the following conditions + /// If the caller is different than the stash being targeted, the following conditions /// must be met: /// - /// * `controller` must belong to a nominator who has become non-decodable, + /// * `stash` must belong to a nominator who has become non-decodable, /// /// Or: /// @@ -1740,16 +1740,10 @@ pub mod pallet { // Anyone can call this function. let caller = ensure_signed(origin)?; let ledger = Self::ledger(Stash(stash.clone()))?; - let controller = ledger - .controller() - .defensive_proof( - "Ledger's controller field didn't exist. The controller should have been fetched using StakingLedger.", - ) - .ok_or(Error::::NotController)?; // In order for one user to chill another user, the following conditions must be met: // - // * `controller` belongs to a nominator who has become non-decodable, + // * `stash` belongs to a nominator who has become non-decodable, // // Or // @@ -1761,14 +1755,14 @@ pub mod pallet { // determine this is a person that should be chilled because they have not met the // threshold bond required. // - // Otherwise, if caller is the same as the controller, this is just like `chill`. + // Otherwise, if caller is the same as the stash, this is just like `chill`. if Nominators::::contains_key(&stash) && Nominators::::get(&stash).is_none() { Self::chill_stash(&stash); return Ok(()) } - if caller != controller { + if caller != stash { let threshold = ChillThreshold::::get().ok_or(Error::::CannotChillOther)?; let min_active_bond = if Nominators::::contains_key(&stash) { let max_nominator_count = From c75692192be03aafecaa14838ebf1df78e7eea63 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 10:18:12 +0700 Subject: [PATCH 19/63] add note --- substrate/frame/delegated-staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index 8581a4a981fe4..1e2e2df813dbe 100644 --- a/substrate/frame/delegated-staking/src/lib.rs +++ b/substrate/frame/delegated-staking/src/lib.rs @@ -493,7 +493,7 @@ impl Pallet { // This should never fail but if it does, it indicates bad state and we abort. T::Currency::transfer(who, &proxy_delegator, amount_to_transfer, Preservation::Expendable)?; - T::CoreStaking::update_payee(who, reward_account)?; + T::CoreStaking::update_payee(who, reward_account)?; // NOTE: Not sure why this is being used. // delegate all transferred funds back to agent. Self::do_delegate(&proxy_delegator, who, amount_to_transfer)?; From f25847a5084eace7931742105f2792ba2b66fae1 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 14:33:55 +0700 Subject: [PATCH 20/63] rm note --- substrate/frame/delegated-staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index 1e2e2df813dbe..8581a4a981fe4 100644 --- a/substrate/frame/delegated-staking/src/lib.rs +++ b/substrate/frame/delegated-staking/src/lib.rs @@ -493,7 +493,7 @@ impl Pallet { // This should never fail but if it does, it indicates bad state and we abort. T::Currency::transfer(who, &proxy_delegator, amount_to_transfer, Preservation::Expendable)?; - T::CoreStaking::update_payee(who, reward_account)?; // NOTE: Not sure why this is being used. + T::CoreStaking::update_payee(who, reward_account)?; // delegate all transferred funds back to agent. Self::do_delegate(&proxy_delegator, who, amount_to_transfer)?; From b1d54b9ab72dcc4850906a710946141114711f4d Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 14:35:15 +0700 Subject: [PATCH 21/63] note --- substrate/frame/delegated-staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index 8581a4a981fe4..1e2e2df813dbe 100644 --- a/substrate/frame/delegated-staking/src/lib.rs +++ b/substrate/frame/delegated-staking/src/lib.rs @@ -493,7 +493,7 @@ impl Pallet { // This should never fail but if it does, it indicates bad state and we abort. T::Currency::transfer(who, &proxy_delegator, amount_to_transfer, Preservation::Expendable)?; - T::CoreStaking::update_payee(who, reward_account)?; + T::CoreStaking::update_payee(who, reward_account)?; // NOTE: Not sure why this is being used. // delegate all transferred funds back to agent. Self::do_delegate(&proxy_delegator, who, amount_to_transfer)?; From 9a5b78818c108a596f1f792f882b314051ef6714 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 14:42:43 +0700 Subject: [PATCH 22/63] init prdoc, bump staking pallet --- Cargo.lock | 2 +- prdoc/pr_4574.prdoc | 10 ++++++++++ substrate/frame/staking/Cargo.toml | 2 +- 3 files changed, 12 insertions(+), 2 deletions(-) create mode 100644 prdoc/pr_4574.prdoc diff --git a/Cargo.lock b/Cargo.lock index 1767245c6abfc..05b87120c9256 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11236,7 +11236,7 @@ dependencies = [ [[package]] name = "pallet-staking" -version = "28.0.0" +version = "29.0.0" dependencies = [ "frame-benchmarking", "frame-election-provider-support", diff --git a/prdoc/pr_4574.prdoc b/prdoc/pr_4574.prdoc new file mode 100644 index 0000000000000..a458b722cf479 --- /dev/null +++ b/prdoc/pr_4574.prdoc @@ -0,0 +1,10 @@ +title: Remove controller logic from staking pallet + +doc: + - audience: Runtime Dev + description: | + Removes controller account logic from the staking pallet. + +crates: + - name: pallet-staking + bump: major \ No newline at end of file diff --git a/substrate/frame/staking/Cargo.toml b/substrate/frame/staking/Cargo.toml index 22df746d667ab..b822fbf424ed9 100644 --- a/substrate/frame/staking/Cargo.toml +++ b/substrate/frame/staking/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-staking" -version = "28.0.0" +version = "29.0.0" authors.workspace = true edition.workspace = true license = "Apache-2.0" From 245b9377ec09a44d8f97f500e39a44cf92001aa5 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 14:45:26 +0700 Subject: [PATCH 23/63] amend prdoc --- prdoc/pr_4574.prdoc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/prdoc/pr_4574.prdoc b/prdoc/pr_4574.prdoc index a458b722cf479..55e7be8f0e358 100644 --- a/prdoc/pr_4574.prdoc +++ b/prdoc/pr_4574.prdoc @@ -1,8 +1,8 @@ -title: Remove controller logic from staking pallet +title: Remove controller logic from staking pallet. doc: - audience: Runtime Dev - description: | + description: Removes controller account logic from the staking pallet. crates: From c12ec061388c3b691b71672f6f12f5c47d9e38b9 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 14:48:05 +0700 Subject: [PATCH 24/63] benchmark fixes --- substrate/frame/staking/src/benchmarking.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 56e7ccc00b6df..2194879a66de6 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -25,7 +25,6 @@ use codec::Decode; use frame_election_provider_support::{bounds::DataProviderBounds, SortedListProvider}; use frame_support::{ pallet_prelude::*, - storage::bounded_vec::BoundedVec, traits::{Currency, Get, Imbalance, UnfilteredDispatchable}, }; use sp_runtime::{ @@ -347,7 +346,7 @@ benchmarks! { let prefs = ValidatorPrefs::default(); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash), prefs) + }: _(RawOrigin::Signed(stash.clone()), prefs) verify { assert!(Validators::::contains_key(&stash)); assert!(T::VoterList::contains(&stash)); @@ -408,7 +407,7 @@ benchmarks! { .collect::>(); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash), kicks) + }: _(RawOrigin::Signed(stash.clone()), kicks) verify { // all nominators now should *not* be nominating our validator... for n in nominator_stashes.iter() { @@ -439,7 +438,7 @@ benchmarks! { let validators = create_validators::(n, 100).unwrap(); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash), validators) + }: _(RawOrigin::Signed(stash.clone()), validators) verify { assert!(Nominators::::contains_key(&stash)); assert!(T::VoterList::contains(&stash)) @@ -458,7 +457,7 @@ benchmarks! { assert!(T::VoterList::contains(&stash)); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash)) + }: _(RawOrigin::Signed(stash.clone())) verify { assert!(!T::VoterList::contains(&stash)); } From e5f8d4318a291ca98b98bb912a5a81e3710f8384 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 14:51:03 +0700 Subject: [PATCH 25/63] fmt --- substrate/frame/delegated-staking/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index 1e2e2df813dbe..896c41838e36c 100644 --- a/substrate/frame/delegated-staking/src/lib.rs +++ b/substrate/frame/delegated-staking/src/lib.rs @@ -494,7 +494,7 @@ impl Pallet { T::Currency::transfer(who, &proxy_delegator, amount_to_transfer, Preservation::Expendable)?; T::CoreStaking::update_payee(who, reward_account)?; // NOTE: Not sure why this is being used. - // delegate all transferred funds back to agent. + // delegate all transferred funds back to agent. Self::do_delegate(&proxy_delegator, who, amount_to_transfer)?; // if the transferred/delegated amount was greater than the stake, mark the extra as From 1a217ca432608dd29bff8eabb908bb35c2c1a6c6 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:05:36 +0700 Subject: [PATCH 26/63] testing_utils: create_stash_controller_with_balance -> create_stash_with_balance --- substrate/frame/staking/src/benchmarking.rs | 60 ++++++++------------ substrate/frame/staking/src/testing_utils.rs | 8 +-- 2 files changed, 28 insertions(+), 40 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 2194879a66de6..0a7719729ce2c 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -143,8 +143,6 @@ pub fn create_validator_with_nominators( struct ListScenario { /// Stash that is expected to be moved. origin_stash1: T::AccountId, - /// Controller of the Stash that is expected to be moved. - origin_controller1: T::AccountId, dest_weight: BalanceOf, } @@ -172,24 +170,24 @@ impl ListScenario { // create accounts with the origin weight - let (origin_stash1, origin_controller1) = create_stash_controller_with_balance::( + let origin_stash1 = create_stash_with_balance::( USER_SEED + 2, origin_weight, RewardDestination::Staked, )?; Staking::::nominate( - RawOrigin::Signed(origin_controller1.clone()).into(), + RawOrigin::Signed(origin_stash1.clone()).into(), // NOTE: these don't really need to be validators. vec![T::Lookup::unlookup(account("random_validator", 0, SEED))], )?; - let (_origin_stash2, origin_controller2) = create_stash_controller_with_balance::( + let origin_stash2 = create_stash_with_balance::( USER_SEED + 3, origin_weight, RewardDestination::Staked, )?; Staking::::nominate( - RawOrigin::Signed(origin_controller2).into(), + RawOrigin::Signed(origin_stash2).into(), vec![T::Lookup::unlookup(account("random_validator", 0, SEED))], )?; @@ -203,17 +201,14 @@ impl ListScenario { T::CurrencyToVote::to_currency(dest_weight_as_vote as u128, total_issuance); // create an account with the worst case destination weight - let (_dest_stash1, dest_controller1) = create_stash_controller_with_balance::( - USER_SEED + 1, - dest_weight, - RewardDestination::Staked, - )?; + let dest_stash1 = + create_stash_with_balance::(USER_SEED + 1, dest_weight, RewardDestination::Staked)?; Staking::::nominate( - RawOrigin::Signed(dest_controller1).into(), + RawOrigin::Signed(dest_stash1).into(), vec![T::Lookup::unlookup(account("random_validator", 0, SEED))], )?; - Ok(ListScenario { origin_stash1, origin_controller1, dest_weight }) + Ok(ListScenario { origin_stash1, dest_weight }) } } @@ -245,16 +240,15 @@ benchmarks! { let max_additional = scenario.dest_weight - origin_weight; let stash = scenario.origin_stash1.clone(); - let controller = scenario.origin_controller1; let original_bonded: BalanceOf - = Ledger::::get(&controller).map(|l| l.active).ok_or("ledger not created after")?; + = Ledger::::get(&stash).map(|l| l.active).ok_or("ledger not created after")?; let _ = T::Currency::deposit_into_existing(&stash, max_additional).unwrap(); whitelist_account!(stash); }: _(RawOrigin::Signed(stash), max_additional) verify { - let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; + let ledger = Ledger::::get(&stash).ok_or("ledger not created after")?; let new_bonded: BalanceOf = ledger.active; assert!(original_bonded < new_bonded); } @@ -273,15 +267,14 @@ benchmarks! { let scenario = ListScenario::::new(origin_weight, false)?; let stash = scenario.origin_stash1.clone(); - let controller = scenario.origin_controller1.clone(); let amount = origin_weight - scenario.dest_weight; - let ledger = Ledger::::get(&controller).ok_or("ledger not created before")?; + let ledger = Ledger::::get(&stash).ok_or("ledger not created before")?; let original_bonded: BalanceOf = ledger.active; whitelist_account!(stash); }: _(RawOrigin::Signed(stash.clone()), amount) verify { - let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; + let ledger = Ledger::::get(&stash).ok_or("ledger not created after")?; let new_bonded: BalanceOf = ledger.active; assert!(original_bonded > new_bonded); } @@ -317,21 +310,20 @@ benchmarks! { // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. let scenario = ListScenario::::new(origin_weight, true)?; - let controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1; add_slashing_spans::(&stash, s); assert!(T::VoterList::contains(&stash)); let ed = T::Currency::minimum_balance(); - let mut ledger = Ledger::::get(&controller).unwrap(); + let mut ledger = Ledger::::get(&stash).unwrap(); ledger.active = ed - One::one(); - Ledger::::insert(&controller, ledger); + Ledger::::insert(&stash, ledger); CurrentEra::::put(EraIndex::max_value()); - whitelist_account!(controller); + whitelist_account!(stash); }: withdraw_unbonded(RawOrigin::Signed(stash.clone()), s) verify { - assert!(!Ledger::::contains_key(controller)); + assert!(!Ledger::::contains_key(&stash)); assert!(!T::VoterList::contains(&stash)); } @@ -427,7 +419,7 @@ benchmarks! { // setup a worst case list scenario. Note we don't care about the destination position, because // we are just doing an insert into the origin position. let scenario = ListScenario::::new(origin_weight, true)?; - let (stash,_) = create_stash_controller_with_balance::( + let stash = create_stash_with_balance::( SEED + MaxNominationsOf::::get() + 1, // make sure the account does not conflict with others origin_weight, RewardDestination::Staked, @@ -522,14 +514,13 @@ benchmarks! { // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. let scenario = ListScenario::::new(origin_weight, true)?; - let controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1; assert!(T::VoterList::contains(&stash)); add_slashing_spans::(&stash, s); }: _(RawOrigin::Root, stash.clone(), s) verify { - assert!(!Ledger::::contains_key(&controller)); + assert!(!Ledger::::contains_key(&stash)); assert!(!T::VoterList::contains(&stash)); } @@ -617,19 +608,18 @@ benchmarks! { }; let stash = scenario.origin_stash1.clone(); - let controller = scenario.origin_controller1; - let mut staking_ledger = Ledger::::get(controller.clone()).unwrap(); + let mut staking_ledger = Ledger::::get(stash.clone()).unwrap(); for _ in 0 .. l { staking_ledger.unlocking.try_push(unlock_chunk.clone()).unwrap() } - Ledger::::insert(controller.clone(), staking_ledger.clone()); + Ledger::::insert(stash.clone(), staking_ledger.clone()); let original_bonded: BalanceOf = staking_ledger.active; whitelist_account!(stash); }: _(RawOrigin::Signed(stash.clone()), rebond_amount) verify { - let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; + let ledger = Ledger::::get(&stash).ok_or("ledger not created after")?; let new_bonded: BalanceOf = ledger.active; assert!(original_bonded < new_bonded); } @@ -644,7 +634,6 @@ benchmarks! { // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. let scenario = ListScenario::::new(origin_weight, true)?; - let controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1; add_slashing_spans::(&stash, s); @@ -652,13 +641,13 @@ benchmarks! { stash.clone(), T::Currency::minimum_balance() - One::one(), ); - Ledger::::insert(&controller, l); + Ledger::::insert(&stash, l); assert!(Bonded::::contains_key(&stash)); assert!(T::VoterList::contains(&stash)); - whitelist_account!(controller); - }: _(RawOrigin::Signed(controller), stash.clone(), s) + whitelist_account!(stash); + }: _(RawOrigin::Signed(stash.clone()), stash.clone(), s) verify { assert!(!Bonded::::contains_key(&stash)); assert!(!T::VoterList::contains(&stash)); @@ -848,7 +837,6 @@ benchmarks! { // setup a worst case list scenario. Note that we don't care about the setup of the // destination position because we are doing a removal from the list but no insert. let scenario = ListScenario::::new(origin_weight, true)?; - let controller = scenario.origin_controller1.clone(); let stash = scenario.origin_stash1; assert!(T::VoterList::contains(&stash)); diff --git a/substrate/frame/staking/src/testing_utils.rs b/substrate/frame/staking/src/testing_utils.rs index d4938ea43ebe2..5043fc502aa75 100644 --- a/substrate/frame/staking/src/testing_utils.rs +++ b/substrate/frame/staking/src/testing_utils.rs @@ -110,15 +110,15 @@ pub fn create_unique_stash_controller( Ok((stash, controller)) } -/// Create a stash and controller pair with fixed balance. -pub fn create_stash_controller_with_balance( +/// Create a stash account with a fixed balance. +pub fn create_stash_with_balance( n: u32, balance: crate::BalanceOf, destination: RewardDestination, -) -> Result<(T::AccountId, T::AccountId), &'static str> { +) -> Result { let staker = create_funded_user_with_balance::("stash", n, balance); Staking::::bond(RawOrigin::Signed(staker.clone()).into(), balance, destination)?; - Ok((staker.clone(), staker)) + Ok(staker) } /// Create a stash and controller pair, where payouts go to a dead payee account. This is used to From fc275a869971af56ce64419ed4fc4b759a6c9954 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:16:26 +0700 Subject: [PATCH 27/63] testing_utils: rm `dead_controller` param from create_validator_with_nominators --- .../frame/session/benchmarking/src/inner.rs | 28 +++++++++---------- substrate/frame/staking/src/benchmarking.rs | 14 +++------- 2 files changed, 17 insertions(+), 25 deletions(-) diff --git a/substrate/frame/session/benchmarking/src/inner.rs b/substrate/frame/session/benchmarking/src/inner.rs index d86c5d9ad278e..65502ac90e39e 100644 --- a/substrate/frame/session/benchmarking/src/inner.rs +++ b/substrate/frame/session/benchmarking/src/inner.rs @@ -51,36 +51,34 @@ benchmarks! { let (v_stash, _) = create_validator_with_nominators::( n, MaxNominationsOf::::get(), - false, true, RewardDestination::Staked, )?; - let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; + let v_stash = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; let keys = T::Keys::decode(&mut TrailingZeroInput::zeroes()).unwrap(); let proof: Vec = vec![0,1,2,3]; - // Whitelist controller account from further DB operations. - let v_controller_key = frame_system::Account::::hashed_key_for(&v_controller); - frame_benchmarking::benchmarking::add_to_whitelist(v_controller_key.into()); - }: _(RawOrigin::Signed(v_controller), keys, proof) + // Whitelist stash account from further DB operations. + let v_stash_key = frame_system::Account::::hashed_key_for(&v_stash); + frame_benchmarking::benchmarking::add_to_whitelist(v_stash_key.into()); + }: _(RawOrigin::Signed(v_stash), keys, proof) purge_keys { let n = MaxNominationsOf::::get(); let (v_stash, _) = create_validator_with_nominators::( n, MaxNominationsOf::::get(), - false, true, RewardDestination::Staked, )?; - let v_controller = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; + let v_stash = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; let keys = T::Keys::decode(&mut TrailingZeroInput::zeroes()).unwrap(); let proof: Vec = vec![0,1,2,3]; - Session::::set_keys(RawOrigin::Signed(v_controller.clone()).into(), keys, proof)?; - // Whitelist controller account from further DB operations. - let v_controller_key = frame_system::Account::::hashed_key_for(&v_controller); - frame_benchmarking::benchmarking::add_to_whitelist(v_controller_key.into()); - }: _(RawOrigin::Signed(v_controller)) + Session::::set_keys(RawOrigin::Signed(v_stash.clone()).into(), keys, proof)?; + // Whitelist stash account from further DB operations. + let v_stash_key = frame_system::Account::::hashed_key_for(&v_stash); + frame_benchmarking::benchmarking::add_to_whitelist(v_stash_key.into()); + }: _(RawOrigin::Signed(v_stash)) #[extra] check_membership_proof_current_session { @@ -129,7 +127,7 @@ fn check_membership_proof_setup( use rand::{RngCore, SeedableRng}; let validator = T::Lookup::lookup(who).unwrap(); - let controller = pallet_staking::Pallet::::bonded(&validator).unwrap(); + let stash = pallet_staking::Pallet::::bonded(&validator).unwrap(); let keys = { let mut keys = [0u8; 128]; @@ -146,7 +144,7 @@ fn check_membership_proof_setup( let keys: T::Keys = Decode::decode(&mut &keys[..]).unwrap(); let proof: Vec = vec![]; - Session::::set_keys(RawOrigin::Signed(controller).into(), keys, proof).unwrap(); + Session::::set_keys(RawOrigin::Signed(stash).into(), keys, proof).unwrap(); } Pallet::::on_initialize(frame_system::pallet_prelude::BlockNumberFor::::one()); diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 0a7719729ce2c..4d2032af778c5 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -66,11 +66,10 @@ 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. +// nominators' stash. It also starts an era and creates pending payouts. pub fn create_validator_with_nominators( n: u32, upper_bound: u32, - dead_controller: bool, unique_controller: bool, destination: RewardDestination, ) -> Result<(T::AccountId, Vec<(T::AccountId, T::AccountId)>), &'static str> { @@ -98,11 +97,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_controller { - create_stash_controller::(u32::MAX - i, 100, destination.clone())? - } else { - create_unique_stash_controller::(u32::MAX - i, 100, destination.clone(), true)? - }; + let (n_stash, n_controller) = + create_stash_controller::(u32::MAX - i, 100, destination.clone())?; + if i < n { Staking::::nominate( RawOrigin::Signed(n_controller.clone()).into(), @@ -545,7 +542,6 @@ benchmarks! { let (validator, nominators) = create_validator_with_nominators::( n, T::MaxExposurePageSize::get() as u32, - false, true, RewardDestination::Staked, )?; @@ -951,7 +947,6 @@ mod tests { n, <::MaxExposurePageSize as Get<_>>::get(), false, - false, RewardDestination::Staked, ) .unwrap(); @@ -982,7 +977,6 @@ mod tests { n, <::MaxExposurePageSize as Get<_>>::get(), false, - false, RewardDestination::Staked, ) .unwrap(); From 25953bb0db49d852d42b3182c21565047d001300 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:21:44 +0700 Subject: [PATCH 28/63] testing_utils: rm v_controller from create_validator_with_nominators --- substrate/frame/staking/src/benchmarking.rs | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 4d2032af778c5..162e62779cf9e 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -78,7 +78,7 @@ pub fn create_validator_with_nominators( let mut points_total = 0; let mut points_individual = Vec::new(); - let (v_stash, v_controller) = if unique_controller { + let (v_stash, _) = if unique_controller { create_unique_stash_controller::(0, 100, destination.clone(), false)? } else { create_stash_controller::(0, 100, destination.clone())? @@ -86,7 +86,7 @@ pub fn create_validator_with_nominators( let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; - Staking::::validate(RawOrigin::Signed(v_controller).into(), validator_prefs)?; + Staking::::validate(RawOrigin::Signed(v_stash.clone()).into(), validator_prefs)?; let stash_lookup = T::Lookup::unlookup(v_stash.clone()); points_total += 10; @@ -97,15 +97,14 @@ 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) = - create_stash_controller::(u32::MAX - i, 100, destination.clone())?; + let (n_stash, _) = create_stash_controller::(u32::MAX - i, 100, destination.clone())?; if i < n { Staking::::nominate( - RawOrigin::Signed(n_controller.clone()).into(), + RawOrigin::Signed(n_stash.clone()).into(), vec![stash_lookup.clone()], )?; - nominators.push((n_stash, n_controller)); + nominators.push((n_stash.clone(), n_stash)); } } @@ -973,7 +972,7 @@ mod tests { ExtBuilder::default().build_and_execute(|| { let n = 10; - let (validator_stash, _nominators) = create_validator_with_nominators::( + let (validator_stash, _) = create_validator_with_nominators::( n, <::MaxExposurePageSize as Get<_>>::get(), false, From 8273cdc75054b23f7d81e5d5729e3adb5fc0c4c8 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:24:32 +0700 Subject: [PATCH 29/63] testing_utils: rm `unique_controller` param from create_validator_with_nominators --- substrate/frame/session/benchmarking/src/inner.rs | 2 -- substrate/frame/staking/src/benchmarking.rs | 10 +--------- 2 files changed, 1 insertion(+), 11 deletions(-) diff --git a/substrate/frame/session/benchmarking/src/inner.rs b/substrate/frame/session/benchmarking/src/inner.rs index 65502ac90e39e..231b2107112f5 100644 --- a/substrate/frame/session/benchmarking/src/inner.rs +++ b/substrate/frame/session/benchmarking/src/inner.rs @@ -51,7 +51,6 @@ benchmarks! { let (v_stash, _) = create_validator_with_nominators::( n, MaxNominationsOf::::get(), - true, RewardDestination::Staked, )?; let v_stash = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; @@ -68,7 +67,6 @@ benchmarks! { let (v_stash, _) = create_validator_with_nominators::( n, MaxNominationsOf::::get(), - true, RewardDestination::Staked, )?; let v_stash = pallet_staking::Pallet::::bonded(&v_stash).ok_or("not stash")?; diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 162e62779cf9e..173c812918521 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -70,7 +70,6 @@ pub fn add_slashing_spans(who: &T::AccountId, spans: u32) { pub fn create_validator_with_nominators( n: u32, upper_bound: u32, - unique_controller: bool, destination: RewardDestination, ) -> Result<(T::AccountId, Vec<(T::AccountId, T::AccountId)>), &'static str> { // Clean up any existing state. @@ -78,11 +77,7 @@ pub fn create_validator_with_nominators( let mut points_total = 0; let mut points_individual = Vec::new(); - let (v_stash, _) = if unique_controller { - create_unique_stash_controller::(0, 100, destination.clone(), false)? - } else { - create_stash_controller::(0, 100, destination.clone())? - }; + let (v_stash, _) = create_stash_controller::(0, 100, destination.clone())?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; @@ -541,7 +536,6 @@ benchmarks! { let (validator, nominators) = create_validator_with_nominators::( n, T::MaxExposurePageSize::get() as u32, - true, RewardDestination::Staked, )?; @@ -945,7 +939,6 @@ mod tests { let (validator_stash, nominators) = create_validator_with_nominators::( n, <::MaxExposurePageSize as Get<_>>::get(), - false, RewardDestination::Staked, ) .unwrap(); @@ -975,7 +968,6 @@ mod tests { let (validator_stash, _) = create_validator_with_nominators::( n, <::MaxExposurePageSize as Get<_>>::get(), - false, RewardDestination::Staked, ) .unwrap(); From f47e3d2f5145846faef7f304c884d910bc2ba13b Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:25:43 +0700 Subject: [PATCH 30/63] fix benchmark --- substrate/frame/staking/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 173c812918521..1e3d1a2337b62 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -237,7 +237,7 @@ benchmarks! { let _ = T::Currency::deposit_into_existing(&stash, max_additional).unwrap(); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash), max_additional) + }: _(RawOrigin::Signed(stash.clone()), max_additional) verify { let ledger = Ledger::::get(&stash).ok_or("ledger not created after")?; let new_bonded: BalanceOf = ledger.active; From 16becbbbdf13c0b2bc29bbfc81cdc384f7322896 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:36:47 +0700 Subject: [PATCH 31/63] testing_utils: use stash only from create_stash_controller --- substrate/frame/staking/src/benchmarking.rs | 44 +++++++++++--------- substrate/frame/staking/src/testing_utils.rs | 15 +++---- substrate/frame/staking/src/tests.rs | 13 +++--- 3 files changed, 35 insertions(+), 37 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 1e3d1a2337b62..0d2b562c604d1 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -274,17 +274,17 @@ benchmarks! { withdraw_unbonded_update { // Slashing Spans let s in 0 .. MAX_SPANS; - let (stash, controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; + let (stash, _) = create_stash_controller::(0, 100, RewardDestination::Staked)?; add_slashing_spans::(&stash, s); let amount = T::Currency::minimum_balance() * 5u32.into(); // Half of total - Staking::::unbond(RawOrigin::Signed(controller.clone()).into(), amount)?; + Staking::::unbond(RawOrigin::Signed(stash.clone()).into(), amount)?; CurrentEra::::put(EraIndex::max_value()); - let ledger = Ledger::::get(&controller).ok_or("ledger not created before")?; + let ledger = Ledger::::get(&stash).ok_or("ledger not created before")?; let original_total: BalanceOf = ledger.total; - whitelist_account!(controller); + whitelist_account!(stash); }: withdraw_unbonded(RawOrigin::Signed(stash.clone()), s) verify { - let ledger = Ledger::::get(&controller).ok_or("ledger not created after")?; + let ledger = Ledger::::get(&stash).ok_or("ledger not created after")?; let new_total: BalanceOf = ledger.total; assert!(original_total > new_total); } @@ -347,7 +347,7 @@ benchmarks! { let rest_of_validators = create_validators_with_seed::(MaxNominationsOf::::get() - 1, 100, 415)?; // this is the validator that will be kicking. - let (stash, controller) = create_stash_controller::( + let (stash, _) = create_stash_controller::( MaxNominationsOf::::get() - 1, 100, RewardDestination::Staked, @@ -446,24 +446,26 @@ benchmarks! { } set_payee { - let (stash, controller) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; + let (stash, _) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; assert_eq!(Payee::::get(&stash), Some(RewardDestination::Staked)); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(controller.clone())) + }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(101u32.into())) verify { - assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(controller))); + assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(stash))); } + // NOTE: This benchmark will not be worst case as stash is now the same as controller. Remove this + // once is resolved. update_payee { - let (stash, controller) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; + let (stash, _) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; Payee::::insert(&stash, { #[allow(deprecated)] RewardDestination::Controller }); - whitelist_account!(controller); - }: _(RawOrigin::Signed(controller.clone()), controller.clone()) + whitelist_account!(stash); + }: _(RawOrigin::Signed(stash.clone()), stash.clone()) verify { - assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(controller))); + assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(stash))); } set_validator_count { @@ -714,8 +716,8 @@ benchmarks! { #[extra] do_slash { let l in 1 .. T::MaxUnlockingChunks::get() as u32; - let (stash, controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; - let mut staking_ledger = Ledger::::get(controller.clone()).unwrap(); + let (stash, _) = create_stash_controller::(0, 100, RewardDestination::Staked)?; + let mut staking_ledger = Ledger::::get(stash.clone()).unwrap(); let unlock_chunk = UnlockChunk::> { value: 1u32.into(), era: EraIndex::zero(), @@ -723,7 +725,7 @@ benchmarks! { for _ in 0 .. l { staking_ledger.unlocking.try_push(unlock_chunk.clone()).unwrap(); } - Ledger::::insert(controller, staking_ledger); + Ledger::::insert(stash.clone(), staking_ledger); let slash_amount = T::Currency::minimum_balance() * 10u32.into(); let balance_before = T::Currency::free_balance(&stash); }: { @@ -851,11 +853,11 @@ benchmarks! { clear_validators_and_nominators::(); // Create a validator with a commission of 50% - let (stash, controller) = + let (stash, _) = create_stash_controller::(1, 1, RewardDestination::Staked)?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; - Staking::::validate(RawOrigin::Signed(controller).into(), validator_prefs)?; + Staking::::validate(RawOrigin::Signed(stash.clone()).into(), validator_prefs)?; // Sanity check assert_eq!( @@ -882,10 +884,12 @@ benchmarks! { assert_eq!(MinCommission::::get(), Perbill::from_percent(100)); } + // NOTE: This benchmark will not work as intended as controller is now stash. Remove this once + // restore_ledger is removed. restore_ledger { - let (stash, controller) = create_stash_controller::(0, 100, RewardDestination::Staked)?; + let (stash, _) = create_stash_controller::(0, 100, RewardDestination::Staked)?; // corrupt ledger. - Ledger::::remove(controller); + Ledger::::remove(stash.clone()); }: _(RawOrigin::Root, stash.clone(), None, None, None) verify { assert_eq!(Staking::::inspect_bond_state(&stash), Ok(LedgerIntegrityState::Ok)); diff --git a/substrate/frame/staking/src/testing_utils.rs b/substrate/frame/staking/src/testing_utils.rs index 5043fc502aa75..fb3654debec07 100644 --- a/substrate/frame/staking/src/testing_utils.rs +++ b/substrate/frame/staking/src/testing_utils.rs @@ -155,11 +155,11 @@ pub fn create_validators_with_seed( ) -> Result>, &'static str> { let mut validators: Vec> = Vec::with_capacity(max as usize); for i in 0..max { - let (stash, controller) = + let (stash, _) = create_stash_controller::(i + seed, balance_factor, RewardDestination::Staked)?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; - Staking::::validate(RawOrigin::Signed(controller).into(), validator_prefs)?; + Staking::::validate(RawOrigin::Signed(stash.clone()).into(), validator_prefs)?; let stash_lookup = T::Lookup::unlookup(stash); validators.push(stash_lookup); } @@ -196,11 +196,11 @@ pub fn create_validators_with_nominators_for_era( // Create validators for i in 0..validators { let balance_factor = if randomize_stake { rng.next_u32() % 255 + 10 } else { 100u32 }; - let (v_stash, v_controller) = + let (v_stash, _) = create_stash_controller::(i, balance_factor, RewardDestination::Staked)?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; - Staking::::validate(RawOrigin::Signed(v_controller.clone()).into(), validator_prefs)?; + Staking::::validate(RawOrigin::Signed(v_stash.clone()).into(), validator_prefs)?; let stash_lookup = T::Lookup::unlookup(v_stash.clone()); validators_stash.push(stash_lookup.clone()); } @@ -211,7 +211,7 @@ pub fn create_validators_with_nominators_for_era( // Create nominators for j in 0..nominators { let balance_factor = if randomize_stake { rng.next_u32() % 255 + 10 } else { 100u32 }; - let (_n_stash, n_controller) = + let (n_stash, _) = create_stash_controller::(u32::MAX - j, balance_factor, RewardDestination::Staked)?; // Have them randomly validate @@ -224,10 +224,7 @@ pub fn create_validators_with_nominators_for_era( let validator = available_validators.remove(selected); selected_validators.push(validator); } - Staking::::nominate( - RawOrigin::Signed(n_controller.clone()).into(), - selected_validators, - )?; + Staking::::nominate(RawOrigin::Signed(n_stash.clone()).into(), selected_validators)?; } ValidatorCount::::put(validators); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 5d751848857f0..31becd36efc08 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -5739,21 +5739,18 @@ fn capped_stakers_works() { // can create `max - validator_count` validators let mut some_existing_validator = AccountId::default(); for i in 0..max - validator_count { - let (_, controller) = testing_utils::create_stash_controller::( + let (stash, _) = testing_utils::create_stash_controller::( i + 10_000_000, 100, RewardDestination::Stash, ) .unwrap(); - assert_ok!(Staking::validate( - RuntimeOrigin::signed(controller), - ValidatorPrefs::default() - )); - some_existing_validator = controller; + assert_ok!(Staking::validate(RuntimeOrigin::signed(stash), ValidatorPrefs::default())); + some_existing_validator = stash; } // but no more - let (_, last_validator) = + let (last_validator, _) = testing_utils::create_stash_controller::(1337, 100, RewardDestination::Stash) .unwrap(); @@ -5776,7 +5773,7 @@ fn capped_stakers_works() { } // one more is too many. - let (_, last_nominator) = testing_utils::create_stash_controller::( + let (last_nominator, _) = testing_utils::create_stash_controller::( 30_000_000, 100, RewardDestination::Stash, From 3a26ca9416922314ac9270724e51d97905d020f9 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:40:24 +0700 Subject: [PATCH 32/63] testing_utils: create_stash_controller -> create_stash, return `stash` only. --- substrate/frame/staking/src/benchmarking.rs | 24 +++++++-------- substrate/frame/staking/src/testing_utils.rs | 17 +++++------ substrate/frame/staking/src/tests.rs | 31 +++++++------------- 3 files changed, 29 insertions(+), 43 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 0d2b562c604d1..d3402401889ee 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -77,7 +77,7 @@ pub fn create_validator_with_nominators( let mut points_total = 0; let mut points_individual = Vec::new(); - let (v_stash, _) = create_stash_controller::(0, 100, destination.clone())?; + let v_stash = create_stash::(0, 100, destination.clone())?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; @@ -92,7 +92,7 @@ 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, _) = create_stash_controller::(u32::MAX - i, 100, destination.clone())?; + let n_stash = create_stash::(u32::MAX - i, 100, destination.clone())?; if i < n { Staking::::nominate( @@ -274,7 +274,7 @@ benchmarks! { withdraw_unbonded_update { // Slashing Spans let s in 0 .. MAX_SPANS; - let (stash, _) = create_stash_controller::(0, 100, RewardDestination::Staked)?; + let stash = create_stash::(0, 100, RewardDestination::Staked)?; add_slashing_spans::(&stash, s); let amount = T::Currency::minimum_balance() * 5u32.into(); // Half of total Staking::::unbond(RawOrigin::Signed(stash.clone()).into(), amount)?; @@ -319,7 +319,7 @@ benchmarks! { } validate { - let (stash,_) = create_stash_controller::( + let stash = create_stash::( MaxNominationsOf::::get() - 1, 100, RewardDestination::Staked, @@ -347,7 +347,7 @@ benchmarks! { let rest_of_validators = create_validators_with_seed::(MaxNominationsOf::::get() - 1, 100, 415)?; // this is the validator that will be kicking. - let (stash, _) = create_stash_controller::( + let stash = create_stash::( MaxNominationsOf::::get() - 1, 100, RewardDestination::Staked, @@ -362,7 +362,7 @@ benchmarks! { let mut nominator_stashes = Vec::with_capacity(k as usize); for i in 0 .. k { // create a nominator stash. - let (n_stash, _) = create_stash_controller::( + let n_stash = create_stash::( MaxNominationsOf::::get() + i, 100, RewardDestination::Staked, @@ -446,7 +446,7 @@ benchmarks! { } set_payee { - let (stash, _) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; + let stash = create_stash::(USER_SEED, 100, RewardDestination::Staked)?; assert_eq!(Payee::::get(&stash), Some(RewardDestination::Staked)); whitelist_account!(stash); }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(101u32.into())) @@ -457,7 +457,7 @@ benchmarks! { // NOTE: This benchmark will not be worst case as stash is now the same as controller. Remove this // once is resolved. update_payee { - let (stash, _) = create_stash_controller::(USER_SEED, 100, RewardDestination::Staked)?; + let (stash, _) = create_stash::(USER_SEED, 100, RewardDestination::Staked)?; Payee::::insert(&stash, { #[allow(deprecated)] RewardDestination::Controller @@ -716,7 +716,7 @@ benchmarks! { #[extra] do_slash { let l in 1 .. T::MaxUnlockingChunks::get() as u32; - let (stash, _) = create_stash_controller::(0, 100, RewardDestination::Staked)?; + let (stash, _) = create_stash::(0, 100, RewardDestination::Staked)?; let mut staking_ledger = Ledger::::get(stash.clone()).unwrap(); let unlock_chunk = UnlockChunk::> { value: 1u32.into(), @@ -853,8 +853,8 @@ benchmarks! { clear_validators_and_nominators::(); // Create a validator with a commission of 50% - let (stash, _) = - create_stash_controller::(1, 1, RewardDestination::Staked)?; + let stash = + create_stash::(1, 1, RewardDestination::Staked)?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; Staking::::validate(RawOrigin::Signed(stash.clone()).into(), validator_prefs)?; @@ -887,7 +887,7 @@ benchmarks! { // NOTE: This benchmark will not work as intended as controller is now stash. Remove this once // restore_ledger is removed. restore_ledger { - let (stash, _) = create_stash_controller::(0, 100, RewardDestination::Staked)?; + let stash = create_stash::(0, 100, RewardDestination::Staked)?; // corrupt ledger. Ledger::::remove(stash.clone()); }: _(RawOrigin::Root, stash.clone(), None, None, None) diff --git a/substrate/frame/staking/src/testing_utils.rs b/substrate/frame/staking/src/testing_utils.rs index fb3654debec07..e87409265994c 100644 --- a/substrate/frame/staking/src/testing_utils.rs +++ b/substrate/frame/staking/src/testing_utils.rs @@ -70,17 +70,17 @@ pub fn create_funded_user_with_balance( user } -/// Create a stash and controller pair. -pub fn create_stash_controller( +/// Create a stash. +pub fn create_stash( n: u32, balance_factor: u32, destination: RewardDestination, -) -> Result<(T::AccountId, T::AccountId), &'static str> { +) -> Result { let staker = create_funded_user::("stash", n, balance_factor); let amount = T::Currency::minimum_balance().max(1u64.into()) * (balance_factor / 10).max(1).into(); Staking::::bond(RawOrigin::Signed(staker.clone()).into(), amount, destination)?; - Ok((staker.clone(), staker)) + Ok(staker) } /// Create a unique stash and controller pair. @@ -155,8 +155,7 @@ pub fn create_validators_with_seed( ) -> Result>, &'static str> { let mut validators: Vec> = Vec::with_capacity(max as usize); for i in 0..max { - let (stash, _) = - create_stash_controller::(i + seed, balance_factor, RewardDestination::Staked)?; + let stash = create_stash::(i + seed, balance_factor, RewardDestination::Staked)?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; Staking::::validate(RawOrigin::Signed(stash.clone()).into(), validator_prefs)?; @@ -196,8 +195,7 @@ pub fn create_validators_with_nominators_for_era( // Create validators for i in 0..validators { let balance_factor = if randomize_stake { rng.next_u32() % 255 + 10 } else { 100u32 }; - let (v_stash, _) = - create_stash_controller::(i, balance_factor, RewardDestination::Staked)?; + let v_stash = create_stash::(i, balance_factor, RewardDestination::Staked)?; let validator_prefs = ValidatorPrefs { commission: Perbill::from_percent(50), ..Default::default() }; Staking::::validate(RawOrigin::Signed(v_stash.clone()).into(), validator_prefs)?; @@ -211,8 +209,7 @@ pub fn create_validators_with_nominators_for_era( // Create nominators for j in 0..nominators { let balance_factor = if randomize_stake { rng.next_u32() % 255 + 10 } else { 100u32 }; - let (n_stash, _) = - create_stash_controller::(u32::MAX - j, balance_factor, RewardDestination::Staked)?; + let n_stash = create_stash::(u32::MAX - j, balance_factor, RewardDestination::Staked)?; // Have them randomly validate let mut available_validators = validator_chosen.clone(); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 31becd36efc08..47f952503aa7a 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -5739,20 +5739,16 @@ fn capped_stakers_works() { // can create `max - validator_count` validators let mut some_existing_validator = AccountId::default(); for i in 0..max - validator_count { - let (stash, _) = testing_utils::create_stash_controller::( - i + 10_000_000, - 100, - RewardDestination::Stash, - ) - .unwrap(); + let stash = + testing_utils::create_stash::(i + 10_000_000, 100, RewardDestination::Stash) + .unwrap(); assert_ok!(Staking::validate(RuntimeOrigin::signed(stash), ValidatorPrefs::default())); some_existing_validator = stash; } // but no more - let (last_validator, _) = - testing_utils::create_stash_controller::(1337, 100, RewardDestination::Stash) - .unwrap(); + let last_validator = + testing_utils::create_stash::(1337, 100, RewardDestination::Stash).unwrap(); assert_noop!( Staking::validate(RuntimeOrigin::signed(last_validator), ValidatorPrefs::default()), @@ -5762,23 +5758,16 @@ fn capped_stakers_works() { // same with nominators let mut some_existing_nominator = AccountId::default(); for i in 0..max - nominator_count { - let (stash, _) = testing_utils::create_stash_controller::( - i + 20_000_000, - 100, - RewardDestination::Stash, - ) - .unwrap(); + let stash = + testing_utils::create_stash::(i + 20_000_000, 100, RewardDestination::Stash) + .unwrap(); assert_ok!(Staking::nominate(RuntimeOrigin::signed(stash), vec![1])); some_existing_nominator = stash; } // one more is too many. - let (last_nominator, _) = testing_utils::create_stash_controller::( - 30_000_000, - 100, - RewardDestination::Stash, - ) - .unwrap(); + let last_nominator = + testing_utils::create_stash::(30_000_000, 100, RewardDestination::Stash).unwrap(); assert_noop!( Staking::nominate(RuntimeOrigin::signed(last_nominator), vec![1]), Error::::TooManyNominators From 3de2984a9200de031a8942702c52a7e9464ff84e Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:46:50 +0700 Subject: [PATCH 33/63] testing_utils: rm create_unique_stash_controller --- substrate/frame/staking/src/testing_utils.rs | 27 ---------------- substrate/frame/staking/src/tests.rs | 33 ++++++-------------- 2 files changed, 10 insertions(+), 50 deletions(-) diff --git a/substrate/frame/staking/src/testing_utils.rs b/substrate/frame/staking/src/testing_utils.rs index e87409265994c..ff2000c2c3b2c 100644 --- a/substrate/frame/staking/src/testing_utils.rs +++ b/substrate/frame/staking/src/testing_utils.rs @@ -83,33 +83,6 @@ pub fn create_stash( Ok(staker) } -/// Create a unique stash and controller pair. -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 = 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); - } - // update bonded account to be unique controller - >::insert(&stash, &controller); - - Ok((stash, controller)) -} - /// Create a stash account with a fixed balance. pub fn create_stash_with_balance( n: u32, diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 47f952503aa7a..757eb1def62cc 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -713,13 +713,9 @@ fn double_staking_should_fail() { // * an account already bonded as stash cannot nominate. ExtBuilder::default().try_state(false).build_and_execute(|| { let arbitrary_value = 5; - let (stash, controller) = testing_utils::create_unique_stash_controller::( - 0, - arbitrary_value, - RewardDestination::Staked, - false, - ) - .unwrap(); + let stash = + testing_utils::create_stash::(0, arbitrary_value, RewardDestination::Staked) + .unwrap(); // 4 = not used so far, stash => not allowed. assert_noop!( @@ -730,32 +726,23 @@ fn double_staking_should_fail() { ), Error::::AlreadyBonded, ); - // controller => attempting to nominate should fail. - assert_noop!( - Staking::nominate(RuntimeOrigin::signed(controller), vec![1]), - Error::::NotStash // TODO: Remove this assert once controller is gone. - ); + // stash => nominating should work. assert_ok!(Staking::nominate(RuntimeOrigin::signed(stash), vec![1])); }); } #[test] -fn double_controlling_attempt_should_fail() { +fn double_stash_attempt_should_fail() { // should test (in the same order): - // * an account already bonded as controller CANNOT be reused as the controller of another - // account. + // * a stash already bonded CANNOT be reused as the stash of another account. ExtBuilder::default().try_state(false).build_and_execute(|| { let arbitrary_value = 5; - let (stash, _) = testing_utils::create_unique_stash_controller::( - 0, - arbitrary_value, - RewardDestination::Staked, - false, - ) - .unwrap(); + let stash = + testing_utils::create_stash::(0, arbitrary_value, RewardDestination::Staked) + .unwrap(); - // Note that controller (same as stash) is reused => no-op. + // Note that stash is reused => no-op. assert_noop!( Staking::bond( RuntimeOrigin::signed(stash), From a9c9235e926d9033208135c17c6483d9a9ab047d Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:48:06 +0700 Subject: [PATCH 34/63] testing_utils: rm create_stash_and_dead_payee --- substrate/frame/staking/src/testing_utils.rs | 18 ------------------ 1 file changed, 18 deletions(-) diff --git a/substrate/frame/staking/src/testing_utils.rs b/substrate/frame/staking/src/testing_utils.rs index ff2000c2c3b2c..a424f3a4cea6c 100644 --- a/substrate/frame/staking/src/testing_utils.rs +++ b/substrate/frame/staking/src/testing_utils.rs @@ -94,24 +94,6 @@ pub fn create_stash_with_balance( Ok(staker) } -/// 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, -) -> Result<(T::AccountId, T::AccountId), &'static str> { - 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(staker.clone()).into(), - amount, - RewardDestination::Account(payee), - )?; - Ok((staker.clone(), staker)) -} - /// create `max` validators. pub fn create_validators( max: u32, From 3082a914c0a2fb1e989da8cf92ae74f8b1bc5ab3 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:53:31 +0700 Subject: [PATCH 35/63] rm `maybe_controller` from `restore_ledger` --- substrate/frame/staking/src/pallet/mod.rs | 3 +-- substrate/frame/staking/src/tests.rs | 17 +++++++---------- 2 files changed, 8 insertions(+), 12 deletions(-) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index d97ac37d66e06..122c8138f837d 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1903,7 +1903,6 @@ pub mod pallet { pub fn restore_ledger( origin: OriginFor, stash: T::AccountId, - maybe_controller: Option, maybe_total: Option>, maybe_unlocking: Option>, T::MaxUnlockingChunks>>, ) -> DispatchResult { @@ -1917,7 +1916,7 @@ pub mod pallet { let (new_controller, new_total) = match Self::inspect_bond_state(&stash) { Ok(LedgerIntegrityState::Corrupted) => { - let new_controller = maybe_controller.unwrap_or(stash.clone()); + let new_controller = stash.clone(); let new_total = if let Some(total) = maybe_total { let new_total = total.min(stash_balance); diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 757eb1def62cc..a911d7c104b73 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -7279,7 +7279,7 @@ mod staking_unchecked { // recover the ledger won't work for virtual staker assert_noop!( - Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None), + Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None), Error::::VirtualStakerNotAllowed ); @@ -7287,7 +7287,7 @@ mod staking_unchecked { >::remove(333); // try restore again - assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); + assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None)); }) } } @@ -7679,7 +7679,7 @@ mod ledger_recovery { assert!(Staking::do_try_state(System::block_number()).is_err()); // recover the ledger bonded by 333 stash. - assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); + assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None)); // try-state checks are ok now. assert_ok!(Staking::do_try_state(System::block_number())); @@ -7723,20 +7723,19 @@ mod ledger_recovery { assert!(Staking::do_try_state(System::block_number()).is_err()); // recover the ledger bonded by 333 stash. - assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); + assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None)); // for the try-state checks to pass, we also need to recover the stash 444 which is // corrupted too by proxy of kill(333). Currently, both the lock and the ledger of 444 // have been cleared so we need to provide the new amount to restore the ledger. assert_noop!( - Staking::restore_ledger(RuntimeOrigin::root(), 444, None, None, None), + Staking::restore_ledger(RuntimeOrigin::root(), 444, None, None), Error::::CannotRestoreLedger ); assert_ok!(Staking::restore_ledger( RuntimeOrigin::root(), 444, - None, Some(total_444_before_corruption), None, )); @@ -7779,7 +7778,7 @@ mod ledger_recovery { assert_ok!(StakingLedger::::kill(&444)); // recover the ledger bonded by 333 stash. - assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None, None)); + assert_ok!(Staking::restore_ledger(RuntimeOrigin::root(), 333, None, None)); // 444 does not need recover in this case since it's been killed successfully. assert_eq!(Staking::inspect_bond_state(&444), Err(Error::::NotStash)); @@ -7835,7 +7834,6 @@ mod ledger_recovery { assert_ok!(Staking::restore_ledger( RuntimeOrigin::root(), 333, - None, Some(lock_333_before + 30), None )); @@ -7844,7 +7842,7 @@ mod ledger_recovery { // of sync. in which case, we need to explicitly set the ledger's lock and amount, // otherwise the ledger recover will fail. assert_noop!( - Staking::restore_ledger(RuntimeOrigin::root(), 444, None, None, None), + Staking::restore_ledger(RuntimeOrigin::root(), 444, None, None), Error::::CannotRestoreLedger ); @@ -7852,7 +7850,6 @@ mod ledger_recovery { assert_ok!(Staking::restore_ledger( RuntimeOrigin::root(), 444, - None, Some(lock_444_before + 40), None )); From c37a8c18e51f0f6da4d1901511e3a6d425dafed4 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 15:54:36 +0700 Subject: [PATCH 36/63] fix benchmark --- substrate/frame/staking/src/benchmarking.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index d3402401889ee..e37455bd8b2d5 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -884,13 +884,11 @@ benchmarks! { assert_eq!(MinCommission::::get(), Perbill::from_percent(100)); } - // NOTE: This benchmark will not work as intended as controller is now stash. Remove this once - // restore_ledger is removed. restore_ledger { let stash = create_stash::(0, 100, RewardDestination::Staked)?; // corrupt ledger. Ledger::::remove(stash.clone()); - }: _(RawOrigin::Root, stash.clone(), None, None, None) + }: _(RawOrigin::Root, stash.clone(), None, None) verify { assert_eq!(Staking::::inspect_bond_state(&stash), Ok(LedgerIntegrityState::Ok)); } From 674cc7f77640328e7c4d607ee4ba447216239f2b Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sat, 25 May 2024 16:04:52 +0700 Subject: [PATCH 37/63] fix benchmarks --- substrate/frame/staking/src/benchmarking.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index e37455bd8b2d5..a0a9fce30c6f6 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -457,7 +457,7 @@ benchmarks! { // NOTE: This benchmark will not be worst case as stash is now the same as controller. Remove this // once is resolved. update_payee { - let (stash, _) = create_stash::(USER_SEED, 100, RewardDestination::Staked)?; + let stash = create_stash::(USER_SEED, 100, RewardDestination::Staked)?; Payee::::insert(&stash, { #[allow(deprecated)] RewardDestination::Controller @@ -716,7 +716,7 @@ benchmarks! { #[extra] do_slash { let l in 1 .. T::MaxUnlockingChunks::get() as u32; - let (stash, _) = create_stash::(0, 100, RewardDestination::Staked)?; + let stash = create_stash::(0, 100, RewardDestination::Staked)?; let mut staking_ledger = Ledger::::get(stash.clone()).unwrap(); let unlock_chunk = UnlockChunk::> { value: 1u32.into(), From ddb8256a03fc3d389edf5f993d289dd66257c6c7 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 10:36:40 +0700 Subject: [PATCH 38/63] tests: remove controller accounts in tests - use stash instead --- substrate/frame/staking/src/mock.rs | 2 +- substrate/frame/staking/src/tests.rs | 116 +++++++++++---------------- 2 files changed, 47 insertions(+), 71 deletions(-) diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index d451f557cb94a..ee451a1f1857b 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -719,7 +719,7 @@ pub(crate) fn reward_all_elected() { >::reward_by_ids(rewards) } -pub(crate) fn validator_controllers() -> Vec { +pub(crate) fn validator_stashes() -> Vec { Session::validators() .into_iter() .map(|s| Staking::bonded(&s).expect("no controller for validator")) diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index a911d7c104b73..7e3bc9cda40b4 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -102,7 +102,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 is stashed and locked assert_eq!(Staking::bonded(&11), Some(11)); // Adds 2 slashing spans add_slash(&11); @@ -130,7 +130,7 @@ fn force_unstake_works() { #[test] fn kill_stash_works() { ExtBuilder::default().build_and_execute(|| { - // Account 11 (also controller) is stashed and locked + // Account 11 is stashed and locked assert_eq!(Staking::bonded(&11), Some(11)); // Adds 2 slashing spans add_slash(&11); @@ -149,9 +149,9 @@ 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 is the controller + // Account 11 is stashed and locked. assert_eq!(Staking::bonded(&11), Some(11)); - // Account 21 is stashed and locked and is the controller + // Account 21 is stashed and locked. assert_eq!(Staking::bonded(&21), Some(21)); // Account 1 is not a stashed assert_eq!(Staking::bonded(&1), None); @@ -355,7 +355,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![21, 11]); + assert_eq_uvec!(validator_stashes(), vec![21, 11]); // put some money in account that we'll use. for i in 1..5 { @@ -374,13 +374,13 @@ fn staking_should_work() { )); // No effects will be seen so far. - assert_eq_uvec!(validator_controllers(), vec![21, 11]); + assert_eq_uvec!(validator_stashes(), 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![21, 11]); + assert_eq_uvec!(validator_stashes(), vec![21, 11]); // --- Block 4: the validators will now be queued. start_session(4); @@ -392,21 +392,21 @@ fn staking_should_work() { // --- Block 6: the validators will now be changed. start_session(6); - assert_eq_uvec!(validator_controllers(), vec![21, 3]); + assert_eq_uvec!(validator_stashes(), vec![21, 3]); // --- Block 6: Unstake 4 as a validator, freeing up the balance stashed in 3 // 4 will chill Staking::chill(RuntimeOrigin::signed(3)).unwrap(); // --- Block 7: nothing. 3 is still there. start_session(7); - assert_eq_uvec!(validator_controllers(), vec![21, 3]); + assert_eq_uvec!(validator_stashes(), 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![21, 11]); + assert_eq_uvec!(validator_stashes(), vec![21, 11]); // Note: the stashed value of 4 is still lock assert_eq!( @@ -462,12 +462,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![31, 21, 11]); + assert_eq_uvec!(validator_stashes(), 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![31, 21, 11]); + assert_eq_uvec!(validator_stashes(), vec![31, 21, 11]); // But the exposure is updated in a simple way. No external votes exists. // This is purely self-vote. @@ -485,7 +485,7 @@ fn no_candidate_emergency_condition() { .nominate(false) .build_and_execute(|| { // initial validators - assert_eq_uvec!(validator_controllers(), vec![11, 21, 31, 41]); + assert_eq_uvec!(validator_stashes(), vec![11, 21, 31, 41]); let prefs = ValidatorPrefs { commission: Perbill::one(), ..Default::default() }; Validators::::insert(11, prefs.clone()); @@ -509,7 +509,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![11, 21, 31, 41]); + assert_eq_uvec!(validator_stashes(), vec![11, 21, 31, 41]); // The chill is still pending. assert!(!Validators::::contains_key(11)); // No new era is created. @@ -526,13 +526,13 @@ fn nominating_and_rewards_should_work() { .set_status(31, StakerStatus::Idle) .build_and_execute(|| { // initial validators. - assert_eq_uvec!(validator_controllers(), vec![41, 21]); + assert_eq_uvec!(validator_stashes(), vec![41, 21]); // re-validate with 11 and 31. assert_ok!(Staking::validate(RuntimeOrigin::signed(11), Default::default())); assert_ok!(Staking::validate(RuntimeOrigin::signed(31), Default::default())); - // Set payee to controller. + // Set payee to the stash. assert_ok!(Staking::set_payee(RuntimeOrigin::signed(11), RewardDestination::Stash)); assert_ok!(Staking::set_payee(RuntimeOrigin::signed(21), RewardDestination::Stash)); assert_ok!(Staking::set_payee(RuntimeOrigin::signed(31), RewardDestination::Stash)); @@ -563,7 +563,7 @@ 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![21, 11]); + assert_eq_uvec!(validator_stashes(), vec![21, 11]); // old validators must have already received some rewards. let initial_balance_41 = Balances::total_balance(&41); @@ -910,7 +910,7 @@ fn cannot_transfer_staked_balance() { 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) is totally staked + // Confirm account 11 is totally staked assert_eq!(Staking::eras_stakers(active_era(), &11).total, 1000); // Confirm account 11 cannot transfer as a result assert_noop!( @@ -935,7 +935,7 @@ fn cannot_transfer_staked_balance_2() { 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) is totally staked + // Confirm account 21 is totally staked assert_eq!(Staking::eras_stakers(active_era(), &21).total, 1000); // Confirm account 21 can transfer at most 1000 assert_noop!( @@ -954,7 +954,7 @@ fn cannot_reserve_staked_balance() { 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 is totally staked assert_eq!(Staking::eras_stakers(active_era(), &11).own, 1000); // Confirm account 11 cannot reserve as a result assert_noop!(Balances::reserve(&11, 1), BalancesError::::LiquidityRestrictions); @@ -1045,10 +1045,10 @@ fn reward_destination_works() { // (era 1, page 0) is claimed assert_eq!(Staking::claimed_rewards(1, &11), vec![0]); - // Change RewardDestination to Account + // Change RewardDestination to Account 11 >::insert(&11, RewardDestination::Account(11)); - // Check controller balance + // Check reward destination balance assert_eq!(Balances::free_balance(11), 23150); // Compute total payout now for whole duration as other parameter won't change @@ -1060,7 +1060,7 @@ fn reward_destination_works() { // Check that RewardDestination is Account(11) assert_eq!(Staking::payee(11.into()), Some(RewardDestination::Account(11))); - // Check that reward went to the controller account + // Check that reward went to the correct account assert_eq!(Balances::free_balance(11), recorded_stash_balance + total_payout_2); // Check that amount at stake is NOT increased assert_eq!( @@ -1140,7 +1140,7 @@ fn bond_extra_works() { // Give account 11 some large free balance greater than total let _ = Balances::make_free_balance_be(&11, 1000000); - // Call the bond_extra function from controller, add only 100 + // Call the bond_extra function, add only 100 assert_ok!(Staking::bond_extra(RuntimeOrigin::signed(11), 100)); // There should be 100 more `total` and `active` in the ledger assert_eq!( @@ -1894,7 +1894,7 @@ fn switching_roles() { assert_ok!(Staking::set_payee(RuntimeOrigin::signed(*i), RewardDestination::Stash)); } - assert_eq_uvec!(validator_controllers(), vec![21, 11]); + assert_eq_uvec!(validator_stashes(), vec![21, 11]); // put some money in account that we'll use. for i in 1..7 { @@ -1920,7 +1920,7 @@ fn switching_roles() { mock::start_active_era(1); // with current nominators 11 and 5 have the most stake - assert_eq_uvec!(validator_controllers(), vec![5, 11]); + assert_eq_uvec!(validator_stashes(), vec![5, 11]); // 2 decides to be a validator. Consequences: assert_ok!(Staking::validate(RuntimeOrigin::signed(1), ValidatorPrefs::default())); @@ -1938,7 +1938,7 @@ fn switching_roles() { mock::start_active_era(2); - assert_eq_uvec!(validator_controllers(), vec![1, 21]); + assert_eq_uvec!(validator_stashes(), vec![1, 21]); }); } @@ -1959,7 +1959,7 @@ fn wrong_vote_is_moot() { mock::start_active_era(1); // new validators - assert_eq_uvec!(validator_controllers(), vec![21, 11]); + assert_eq_uvec!(validator_stashes(), vec![21, 11]); // our new voter is taken into account assert!(Staking::eras_stakers(active_era(), &11).others.iter().any(|i| i.who == 61)); @@ -2048,7 +2048,7 @@ fn bond_with_little_staked_value_bounded() { mock::make_all_reward_payment(0); // 1 is elected. - assert_eq_uvec!(validator_controllers(), vec![21, 11, 1]); + assert_eq_uvec!(validator_stashes(), vec![21, 11, 1]); assert_eq!(Staking::eras_stakers(active_era(), &2).total, 0); // Old ones are rewarded. @@ -2066,7 +2066,7 @@ fn bond_with_little_staked_value_bounded() { mock::start_active_era(2); mock::make_all_reward_payment(1); - assert_eq_uvec!(validator_controllers(), vec![21, 11, 1]); + assert_eq_uvec!(validator_stashes(), vec![21, 11, 1]); assert_eq!(Staking::eras_stakers(active_era(), &2).total, 0); // 2 is now rewarded. @@ -2190,11 +2190,11 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() { fn new_era_elects_correct_number_of_validators() { ExtBuilder::default().nominate(true).validator_count(1).build_and_execute(|| { assert_eq!(Staking::validator_count(), 1); - assert_eq!(validator_controllers().len(), 1); + assert_eq!(validator_stashes().len(), 1); Session::on_initialize(System::block_number()); - assert_eq!(validator_controllers().len(), 1); + assert_eq!(validator_stashes().len(), 1); }) } @@ -2215,7 +2215,7 @@ fn phragmen_should_not_overflow() { mock::start_active_era(1); - assert_eq_uvec!(validator_controllers(), vec![3, 5]); + assert_eq_uvec!(validator_stashes(), 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); @@ -3874,7 +3874,7 @@ fn test_multi_page_payout_stakers_by_page() { RewardOnUnbalanceWasCalled::set(false); System::reset_events(); - let controller_balance_before_p0_payout = Balances::free_balance(&11); + let balance_before_p0_payout = Balances::free_balance(&11); // Payout rewards for first exposure page assert_ok!(Staking::payout_stakers_by_page(RuntimeOrigin::signed(1337), 11, 1, 0)); @@ -3888,14 +3888,14 @@ fn test_multi_page_payout_stakers_by_page() { ] )); - let controller_balance_after_p0_payout = Balances::free_balance(&11); + let balance_after_p0_payout = Balances::free_balance(&11); // verify rewards have been paid out but still some left assert!(Balances::total_issuance() > pre_payout_total_issuance); assert!(Balances::total_issuance() < pre_payout_total_issuance + payout); // verify the validator has been rewarded - assert!(controller_balance_after_p0_payout > controller_balance_before_p0_payout); + assert!(balance_after_p0_payout > balance_before_p0_payout); // Payout the second and last page of nominators assert_ok!(Staking::payout_stakers_by_page(RuntimeOrigin::signed(1337), 11, 1, 1)); @@ -3912,7 +3912,7 @@ fn test_multi_page_payout_stakers_by_page() { ] )); // verify the validator was not rewarded the second time - assert_eq!(Balances::free_balance(&11), controller_balance_after_p0_payout); + assert_eq!(Balances::free_balance(&11), balance_after_p0_payout); // verify all rewards have been paid out assert_eq_error_rate!(Balances::total_issuance(), pre_payout_total_issuance + payout, 2); @@ -4090,7 +4090,7 @@ fn test_multi_page_payout_stakers_backward_compatible() { let pre_payout_total_issuance = Balances::total_issuance(); RewardOnUnbalanceWasCalled::set(false); - let controller_balance_before_p0_payout = Balances::free_balance(&11); + let balance_before_p0_payout = Balances::free_balance(&11); // Payout rewards for first exposure page assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(1337), 11, 1)); // page 0 is claimed @@ -4099,14 +4099,14 @@ fn test_multi_page_payout_stakers_backward_compatible() { Error::::AlreadyClaimed.with_weight(err_weight) ); - let controller_balance_after_p0_payout = Balances::free_balance(&11); + let balance_after_p0_payout = Balances::free_balance(&11); // verify rewards have been paid out but still some left assert!(Balances::total_issuance() > pre_payout_total_issuance); assert!(Balances::total_issuance() < pre_payout_total_issuance + payout); // verify the validator has been rewarded - assert!(controller_balance_after_p0_payout > controller_balance_before_p0_payout); + assert!(balance_after_p0_payout > balance_before_p0_payout); // This should payout the second and last page of nominators assert_ok!(Staking::payout_stakers(RuntimeOrigin::signed(1337), 11, 1)); @@ -4118,14 +4118,13 @@ fn test_multi_page_payout_stakers_backward_compatible() { ); // verify the validator was not rewarded the second time - assert_eq!(Balances::free_balance(&11), controller_balance_after_p0_payout); + assert_eq!(Balances::free_balance(&11), balance_after_p0_payout); // verify all rewards have been paid out assert_eq_error_rate!(Balances::total_issuance(), pre_payout_total_issuance + payout, 2); assert!(RewardOnUnbalanceWasCalled::get()); - // verify all nominators of validator 11 are paid out, including the validator - // Validator payout goes to controller. + // verify all nominators of validator 11 are paid out, including the validator. assert!(Balances::free_balance(&11) > balance); for i in 0..100 { assert!(Balances::free_balance(&(1000 + i)) > balance + i as Balance); @@ -4490,10 +4489,10 @@ fn test_commission_paid_across_pages() { // Payout rewards for first exposure page assert_ok!(Staking::payout_stakers_by_page(RuntimeOrigin::signed(1337), 11, 1, 0)); - let controller_balance_after_p0_payout = Balances::free_balance(&11); + let balance_after_p0_payout = Balances::free_balance(&11); // some commission is paid - assert!(initial_balance < controller_balance_after_p0_payout); + assert!(initial_balance < balance_after_p0_payout); // payout all pages for i in 1..4 { @@ -4753,7 +4752,7 @@ fn payout_to_any_account_works() { // Create a validator: bond_validator(11, balance); // Default(64) - // Create a stash/controller pair + // Create a stash bond_nominator(1234, 100, vec![11]); // Update payout location @@ -5370,7 +5369,7 @@ mod election_data_provider { StakerStatus::::Nominator(vec![16, 15, 14, 13, 12, 11, 10]), ) .build_and_execute(|| { - // nominations of controller 70 won't be added due to voter size limit exceeded. + // nominations of stash 70 won't be added due to voter size limit exceeded. let bounds = ElectionBoundsBuilder::default().voters_size(100.into()).build(); assert_eq!( Staking::electing_voters(bounds.voters) @@ -6944,29 +6943,6 @@ mod staking_interface { assert_eq!(Balances::balance_locked(STAKING_ID, &11), 0); }); } - - #[test] - fn status() { - ExtBuilder::default().build_and_execute(|| { - // stash of a validator is identified as a validator - assert_eq!(Staking::status(&11).unwrap(), StakerStatus::Validator); - // .. but not the controller. - assert!(Staking::status(&10).is_err()); - - // stash of nominator is identified as a nominator - assert_eq!(Staking::status(&101).unwrap(), StakerStatus::Nominator(vec![11, 21])); - // .. but not the controller. - assert!(Staking::status(&100).is_err()); - - // stash of chilled is identified as a chilled - assert_eq!(Staking::status(&41).unwrap(), StakerStatus::Idle); - // .. but not the controller. - assert!(Staking::status(&40).is_err()); - - // random other account. - assert!(Staking::status(&42).is_err()); - }) - } } mod staking_unchecked { From 714f2f9ba3bf92f1196988cd4bc761c1e5467854 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 11:13:25 +0700 Subject: [PATCH 39/63] mock & chain_spec: remove ctrl from stakers --- polkadot/runtime/common/src/try_runtime.rs | 4 +- substrate/bin/node/cli/src/chain_spec.rs | 18 ++------ substrate/frame/delegated-staking/src/mock.rs | 21 ++------- .../test-staking-e2e/src/mock.rs | 31 ++++++------- substrate/frame/root-offences/src/mock.rs | 10 ++--- substrate/frame/staking/src/mock.rs | 27 +++++------- substrate/frame/staking/src/pallet/mod.rs | 5 +-- substrate/frame/staking/src/tests.rs | 44 ++++++------------- 8 files changed, 56 insertions(+), 104 deletions(-) diff --git a/polkadot/runtime/common/src/try_runtime.rs b/polkadot/runtime/common/src/try_runtime.rs index 81aa34317bfd7..000b41e138ce2 100644 --- a/polkadot/runtime/common/src/try_runtime.rs +++ b/polkadot/runtime/common/src/try_runtime.rs @@ -34,7 +34,7 @@ where let mut unstaked_err = 0; let mut unstaked_slashed = 0; - let all_stakers = Ledger::::iter().map(|(ctrl, l)| (ctrl, l.stash)).collect::>(); + let all_stakers = Ledger::::iter().map(|(stash, l)| l.stash).collect::>(); let mut all_exposed = BTreeSet::new(); ErasStakers::::iter().for_each(|(_, val, expo)| { all_exposed.insert(val); @@ -43,7 +43,7 @@ where let eligible = all_stakers .iter() - .filter_map(|(ctrl, stash)| all_exposed.contains(stash).then_some(ctrl)) + .filter_map(|stash| all_exposed.contains(stash).then_some(stash)) .collect::>(); log::info!( diff --git a/substrate/bin/node/cli/src/chain_spec.rs b/substrate/bin/node/cli/src/chain_spec.rs index a3b536e54342d..34f0a916c5fe4 100644 --- a/substrate/bin/node/cli/src/chain_spec.rs +++ b/substrate/bin/node/cli/src/chain_spec.rs @@ -280,7 +280,6 @@ pub fn authority_keys_from_seed( fn configure_accounts( initial_authorities: Vec<( - AccountId, AccountId, GrandpaId, BabeId, @@ -293,19 +292,10 @@ fn configure_accounts( endowed_accounts: Option>, stash: Balance, ) -> ( - Vec<( - AccountId, - AccountId, - GrandpaId, - BabeId, - ImOnlineId, - AuthorityDiscoveryId, - MixnetId, - BeefyId, - )>, + Vec<(AccountId, GrandpaId, BabeId, ImOnlineId, AuthorityDiscoveryId, MixnetId, BeefyId)>, Vec, usize, - Vec<(AccountId, AccountId, Balance, StakerStatus)>, + Vec<(AccountId, Balance, StakerStatus)>, ) { let mut endowed_accounts: Vec = endowed_accounts.unwrap_or_else(|| { vec![ @@ -338,7 +328,7 @@ fn configure_accounts( let mut rng = rand::thread_rng(); let stakers = initial_authorities .iter() - .map(|x| (x.0.clone(), x.0.clone(), stash, StakerStatus::Validator)) + .map(|x| (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()); @@ -361,7 +351,6 @@ fn configure_accounts( /// Helper function to create RuntimeGenesisConfig json patch for testing. pub fn testnet_genesis( initial_authorities: Vec<( - AccountId, AccountId, GrandpaId, BabeId, @@ -386,7 +375,6 @@ pub fn testnet_genesis( .iter() .map(|x| { ( - x.0.clone(), x.0.clone(), session_keys( x.2.clone(), diff --git a/substrate/frame/delegated-staking/src/mock.rs b/substrate/frame/delegated-staking/src/mock.rs index 31195459bd84e..375233c8d2791 100644 --- a/substrate/frame/delegated-staking/src/mock.rs +++ b/substrate/frame/delegated-staking/src/mock.rs @@ -215,24 +215,9 @@ impl ExtBuilder { .assimilate_storage(&mut storage); let stakers = vec![ - ( - GENESIS_VALIDATOR, - GENESIS_VALIDATOR, - 1000, - sp_staking::StakerStatus::::Validator, - ), - ( - GENESIS_NOMINATOR_ONE, - GENESIS_NOMINATOR_ONE, - 100, - sp_staking::StakerStatus::::Nominator(vec![1]), - ), - ( - GENESIS_NOMINATOR_TWO, - GENESIS_NOMINATOR_TWO, - 200, - sp_staking::StakerStatus::::Nominator(vec![1]), - ), + (GENESIS_VALIDATOR, 1000, sp_staking::StakerStatus::::Validator), + (GENESIS_NOMINATOR_ONE, 100, sp_staking::StakerStatus::::Nominator(vec![1])), + (GENESIS_NOMINATOR_TWO, 200, sp_staking::StakerStatus::::Nominator(vec![1])), ]; let _ = pallet_staking::GenesisConfig:: { diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index caabd576c5a1b..73e5a84c29e58 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -394,27 +394,27 @@ pub struct StakingExtBuilder { min_validator_bond: Balance, status: BTreeMap>, stakes: BTreeMap, - stakers: Vec<(AccountId, AccountId, Balance, StakerStatus)>, + stakers: Vec<(AccountId, Balance, StakerStatus)>, } impl Default for StakingExtBuilder { fn default() -> Self { let stakers = vec![ - // (stash, ctrl, stake, status) + // (stash, stake, status) // these two will be elected in the default test where we elect 2. - (11, 11, 1000, StakerStatus::::Validator), - (21, 21, 1000, StakerStatus::::Validator), + (11, 1000, StakerStatus::::Validator), + (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), + (31, 500, StakerStatus::::Validator), + (41, 1500, StakerStatus::::Validator), + (51, 1500, StakerStatus::::Validator), + (61, 1500, StakerStatus::::Validator), + (71, 1500, StakerStatus::::Validator), + (81, 1500, StakerStatus::::Validator), + (91, 1500, StakerStatus::::Validator), + (101, 500, StakerStatus::::Validator), // an idle validator - (201, 201, 1000, StakerStatus::::Idle), + (201, 1000, StakerStatus::::Idle), ]; Self { @@ -493,6 +493,7 @@ impl Default for BalancesExtBuilder { (3, 300), (4, 400), // controllers (still used in some tests. Soon to be deprecated). + // TODO: Ensure these are not being used in tests & remove. (10, 100), (20, 100), (30, 100), @@ -554,7 +555,7 @@ impl ExtBuilder { let mut stakers = self.staking_builder.stakers.clone(); self.staking_builder.status.clone().into_iter().for_each(|(stash, status)| { - let (_, _, _, ref mut prev_status) = stakers + let (_, _, ref mut prev_status) = stakers .iter_mut() .find(|s| s.0 == stash) .expect("set_status staker should exist; qed"); @@ -562,7 +563,7 @@ impl ExtBuilder { }); // replaced any of the stakes if needed. self.staking_builder.stakes.clone().into_iter().for_each(|(stash, stake)| { - let (_, _, ref mut prev_stake, _) = stakers + let (_, ref mut prev_stake, _) = stakers .iter_mut() .find(|s| s.0 == stash) .expect("set_stake staker should exits; qed."); diff --git a/substrate/frame/root-offences/src/mock.rs b/substrate/frame/root-offences/src/mock.rs index 2bef514c0d1a9..c5a42c5b15a7b 100644 --- a/substrate/frame/root-offences/src/mock.rs +++ b/substrate/frame/root-offences/src/mock.rs @@ -239,14 +239,14 @@ impl ExtBuilder { .unwrap(); let stakers = vec![ - // (stash, ctrl, stake, status) + // (stash, stake, status) // these two will be elected in the default test where we elect 2. - (11, 11, 1000, StakerStatus::::Validator), - (21, 21, 1000, StakerStatus::::Validator), + (11, 1000, StakerStatus::::Validator), + (21, 1000, StakerStatus::::Validator), // a loser validator - (31, 31, 500, StakerStatus::::Validator), + (31, 500, StakerStatus::::Validator), // an idle validator - (41, 41, 1000, StakerStatus::::Idle), + (41, 1000, StakerStatus::::Idle), ]; let _ = pallet_staking::GenesisConfig:: { diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index ee451a1f1857b..1018dbea3b9dd 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -338,7 +338,7 @@ pub struct ExtBuilder { balance_factor: Balance, status: BTreeMap>, stakes: BTreeMap, - stakers: Vec<(AccountId, AccountId, Balance, StakerStatus)>, + stakers: Vec<(AccountId, Balance, StakerStatus)>, } impl Default for ExtBuilder { @@ -424,11 +424,10 @@ impl ExtBuilder { pub fn add_staker( mut self, stash: AccountId, - ctrl: AccountId, stake: Balance, status: StakerStatus, ) -> Self { - self.stakers.push((stash, ctrl, stake, status)); + self.stakers.push((stash, stake, status)); self } pub fn balance_factor(mut self, factor: Balance) -> Self { @@ -482,22 +481,20 @@ impl ExtBuilder { let mut stakers = vec![]; if self.has_stakers { stakers = vec![ - // (stash, ctrl, stake, status) - // TODO: Remove `ctrl` from stakers. + // (stash, stake, status) // these two will be elected in the default test where we elect 2. - (11, 11, self.balance_factor * 1000, StakerStatus::::Validator), - (21, 21, self.balance_factor * 1000, StakerStatus::::Validator), + (11, self.balance_factor * 1000, StakerStatus::::Validator), + (21, self.balance_factor * 1000, StakerStatus::::Validator), // a loser validator - (31, 31, self.balance_factor * 500, StakerStatus::::Validator), + (31, self.balance_factor * 500, StakerStatus::::Validator), // an idle validator - (41, 41, self.balance_factor * 1000, StakerStatus::::Idle), - (51, 51, self.balance_factor * 1000, StakerStatus::::Idle), - (201, 201, self.balance_factor * 1000, StakerStatus::::Idle), - (202, 202, self.balance_factor * 1000, StakerStatus::::Idle), + (41, self.balance_factor * 1000, StakerStatus::::Idle), + (51, self.balance_factor * 1000, StakerStatus::::Idle), + (201, self.balance_factor * 1000, StakerStatus::::Idle), + (202, self.balance_factor * 1000, StakerStatus::::Idle), ]; // optionally add a nominator if self.nominate { stakers.push(( - 101, 101, self.balance_factor * 500, StakerStatus::::Nominator(vec![11, 21]), @@ -505,7 +502,7 @@ impl ExtBuilder { } // replace any of the status if needed. self.status.into_iter().for_each(|(stash, status)| { - let (_, _, _, ref mut prev_status) = stakers + let (_, _, ref mut prev_status) = stakers .iter_mut() .find(|s| s.0 == stash) .expect("set_status staker should exist; qed"); @@ -513,7 +510,7 @@ impl ExtBuilder { }); // replaced any of the stakes if needed. self.stakes.into_iter().for_each(|(stash, stake)| { - let (_, _, ref mut prev_stake, _) = stakers + let (_, ref mut prev_stake, _) = stakers .iter_mut() .find(|s| s.0 == stash) .expect("set_stake staker should exits; qed."); diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 122c8138f837d..4cac9adbed7f2 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -679,8 +679,7 @@ pub mod pallet { pub force_era: Forcing, pub slash_reward_fraction: Perbill, pub canceled_payout: BalanceOf, - pub stakers: - Vec<(T::AccountId, T::AccountId, BalanceOf, crate::StakerStatus)>, + pub stakers: Vec<(T::AccountId, BalanceOf, crate::StakerStatus)>, pub min_nominator_bond: BalanceOf, pub min_validator_bond: BalanceOf, pub max_validator_count: Option, @@ -705,7 +704,7 @@ pub mod pallet { MaxNominatorsCount::::put(x); } - for &(ref stash, _, balance, ref status) in &self.stakers { + for &(ref stash, balance, ref status) in &self.stakers { crate::log!( trace, "inserting genesis staker: {:?} => {:?} => {:?}", diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 7e3bc9cda40b4..5eb32b9be23a1 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -1946,7 +1946,6 @@ fn switching_roles() { fn wrong_vote_is_moot() { ExtBuilder::default() .add_staker( - 61, 61, 500, StakerStatus::Nominator(vec![ @@ -5044,9 +5043,9 @@ mod election_data_provider { fn set_minimum_active_stake_is_correct() { ExtBuilder::default() .nominate(false) - .add_staker(61, 61, 2_000, StakerStatus::::Nominator(vec![21])) - .add_staker(71, 71, 10, StakerStatus::::Nominator(vec![21])) - .add_staker(81, 81, 50, StakerStatus::::Nominator(vec![21])) + .add_staker(61, 2_000, StakerStatus::::Nominator(vec![21])) + .add_staker(71, 10, StakerStatus::::Nominator(vec![21])) + .add_staker(81, 50, StakerStatus::::Nominator(vec![21])) .build_and_execute(|| { // default bounds are unbounded. assert_ok!(::electing_voters( @@ -5111,7 +5110,7 @@ mod election_data_provider { ExtBuilder::default() .has_stakers(true) .nominate(true) - .add_staker(61, 61, 2_000, StakerStatus::::Nominator(vec![21])) + .add_staker(61, 2_000, StakerStatus::::Nominator(vec![21])) .build_and_execute(|| { assert_eq!(Staking::weight_of(&101), 500); let voters = ::electing_voters( @@ -5170,24 +5169,9 @@ mod election_data_provider { .nominate(false) // the best way to invalidate a bunch of nominators is to have them nominate a lot of // ppl, but then lower the MaxNomination limit. - .add_staker( - 61, - 61, - 2_000, - StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), - ) - .add_staker( - 71, - 71, - 2_000, - StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), - ) - .add_staker( - 81, - 81, - 2_000, - StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), - ) + .add_staker(61, 2_000, StakerStatus::::Nominator(vec![21, 22, 23, 24, 25])) + .add_staker(71, 2_000, StakerStatus::::Nominator(vec![21, 22, 23, 24, 25])) + .add_staker(81, 2_000, StakerStatus::::Nominator(vec![21, 22, 23, 24, 25])) .build_and_execute(|| { let bounds_builder = ElectionBoundsBuilder::default(); // all voters ordered by stake, @@ -5335,7 +5319,6 @@ mod election_data_provider { .nominate(false) .add_staker( 61, - 60, 300, // 300 bond has 16 nomination quota. StakerStatus::::Nominator(vec![21, 22, 23, 24, 25]), ) @@ -5364,7 +5347,6 @@ mod election_data_provider { .nominate(false) .add_staker( 71, - 70, 333, StakerStatus::::Nominator(vec![16, 15, 14, 13, 12, 11, 10]), ) @@ -5842,8 +5824,8 @@ fn min_commission_works() { fn change_of_absolute_max_nominations() { use frame_election_provider_support::ElectionDataProvider; ExtBuilder::default() - .add_staker(61, 61, 10, StakerStatus::Nominator(vec![1])) - .add_staker(71, 71, 10, StakerStatus::Nominator(vec![1, 2, 3])) + .add_staker(61, 10, StakerStatus::Nominator(vec![1])) + .add_staker(71, 10, StakerStatus::Nominator(vec![1, 2, 3])) .balance_factor(10) .build_and_execute(|| { // pre-condition @@ -5940,10 +5922,10 @@ fn change_of_absolute_max_nominations() { fn nomination_quota_max_changes_decoding() { 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(30, 330, 10, StakerStatus::Nominator(vec![1, 2, 3, 4])) - .add_staker(50, 550, 10, StakerStatus::Nominator(vec![1, 2, 3, 4])) + .add_staker(60, 10, StakerStatus::Nominator(vec![1])) + .add_staker(70, 10, StakerStatus::Nominator(vec![1, 2, 3])) + .add_staker(30, 10, StakerStatus::Nominator(vec![1, 2, 3, 4])) + .add_staker(50, 10, StakerStatus::Nominator(vec![1, 2, 3, 4])) .balance_factor(10) .build_and_execute(|| { // pre-condition. From ce82d9859f794bd395e98b676ca5e6f6ed75e1ee Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 11:42:59 +0700 Subject: [PATCH 40/63] mocks + runtimes: fix `stakers`, remove controller accounts --- substrate/frame/babe/src/mock.rs | 3 +-- substrate/frame/beefy/src/mock.rs | 3 +-- .../test-staking-e2e/src/mock.rs | 13 ------------ substrate/frame/fast-unstake/src/mock.rs | 20 ++++++++----------- substrate/frame/grandpa/src/mock.rs | 3 +-- substrate/frame/root-offences/src/mock.rs | 5 ----- substrate/frame/staking/src/mock.rs | 9 +++------ substrate/frame/staking/src/tests.rs | 18 ++++++----------- 8 files changed, 20 insertions(+), 54 deletions(-) diff --git a/substrate/frame/babe/src/mock.rs b/substrate/frame/babe/src/mock.rs index 296bad458c509..e7a2d3612db78 100644 --- a/substrate/frame/babe/src/mock.rs +++ b/substrate/frame/babe/src/mock.rs @@ -347,9 +347,8 @@ pub fn new_test_ext_raw_authorities(authorities: Vec) -> sp_io::Tes .assimilate_storage(&mut t) .unwrap(); - // 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, 10_000, pallet_staking::StakerStatus::::Validator)) .collect(); let staking_config = pallet_staking::GenesisConfig:: { diff --git a/substrate/frame/beefy/src/mock.rs b/substrate/frame/beefy/src/mock.rs index b0b1d8d9c7b6c..2433e57c5c30b 100644 --- a/substrate/frame/beefy/src/mock.rs +++ b/substrate/frame/beefy/src/mock.rs @@ -247,9 +247,8 @@ impl ExtBuilder { .assimilate_storage(&mut t) .unwrap(); - // controllers are same as stash let stakers: Vec<_> = (0..self.authorities.len()) - .map(|i| (i as u64, i as u64, 10_000, pallet_staking::StakerStatus::::Validator)) + .map(|i| (i as u64, 10_000, pallet_staking::StakerStatus::::Validator)) .collect(); let staking_config = pallet_staking::GenesisConfig:: { diff --git a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs index 73e5a84c29e58..67cafa564c65a 100644 --- a/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs +++ b/substrate/frame/election-provider-multi-phase/test-staking-e2e/src/mock.rs @@ -492,19 +492,6 @@ impl Default for BalancesExtBuilder { (2, 20), (3, 300), (4, 400), - // controllers (still used in some tests. Soon to be deprecated). - // TODO: Ensure these are not being used in tests & remove. - (10, 100), - (20, 100), - (30, 100), - (40, 100), - (50, 100), - (60, 100), - (70, 100), - (80, 100), - (90, 100), - (100, 100), - (200, 100), // stashes (11, 1000), (21, 2000), diff --git a/substrate/frame/fast-unstake/src/mock.rs b/substrate/frame/fast-unstake/src/mock.rs index b5bc9af4e2e12..2ed2ab9272ccb 100644 --- a/substrate/frame/fast-unstake/src/mock.rs +++ b/substrate/frame/fast-unstake/src/mock.rs @@ -204,19 +204,13 @@ pub(crate) fn fast_unstake_events_since_last_call() -> Vec } pub struct ExtBuilder { - unexposed: Vec<(AccountId, AccountId, Balance)>, + unexposed: Vec<(AccountId, Balance)>, } impl Default for ExtBuilder { fn default() -> Self { Self { - unexposed: vec![ - (1, 1, 7 + 100), - (3, 3, 7 + 100), - (5, 5, 7 + 100), - (7, 7, 7 + 100), - (9, 9, 7 + 100), - ], + unexposed: vec![(1, 7 + 100), (3, 7 + 100), (5, 7 + 100), (7, 7 + 100), (9, 7 + 100)], } } } @@ -266,7 +260,7 @@ impl ExtBuilder { .unexposed .clone() .into_iter() - .map(|(stash, _, balance)| (stash, balance * 2)) + .map(|(stash, balance)| (stash, balance * 2)) .chain(validators_range.clone().map(|x| (x, 7 + 100))) .chain(nominators_range.clone().map(|x| (x, 7 + 100))) .collect::>(), @@ -277,9 +271,11 @@ impl ExtBuilder { stakers: self .unexposed .into_iter() - .map(|(x, y, z)| (x, y, z, pallet_staking::StakerStatus::Nominator(vec![42]))) - .chain(validators_range.map(|x| (x, x, 100, StakerStatus::Validator))) - .chain(nominators_range.map(|x| (x, x, 100, StakerStatus::Nominator(vec![x])))) + .map(|(stash, balance)| { + (stash, balance, pallet_staking::StakerStatus::Nominator(vec![42])) + }) + .chain(validators_range.map(|x| (x, 100, StakerStatus::Validator))) + .chain(nominators_range.map(|x| (x, 100, StakerStatus::Nominator(vec![x])))) .collect::>(), ..Default::default() } diff --git a/substrate/frame/grandpa/src/mock.rs b/substrate/frame/grandpa/src/mock.rs index 6777cb6e74e26..17bbf14065793 100644 --- a/substrate/frame/grandpa/src/mock.rs +++ b/substrate/frame/grandpa/src/mock.rs @@ -260,9 +260,8 @@ pub fn new_test_ext_raw_authorities(authorities: AuthorityList) -> sp_io::TestEx .assimilate_storage(&mut t) .unwrap(); - // 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, 10_000, pallet_staking::StakerStatus::::Validator)) .collect(); let staking_config = pallet_staking::GenesisConfig:: { diff --git a/substrate/frame/root-offences/src/mock.rs b/substrate/frame/root-offences/src/mock.rs index c5a42c5b15a7b..a86bf0ccf596e 100644 --- a/substrate/frame/root-offences/src/mock.rs +++ b/substrate/frame/root-offences/src/mock.rs @@ -223,11 +223,6 @@ impl ExtBuilder { pallet_balances::GenesisConfig:: { balances: vec![ - // 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), - (40, self.balance_factor * 50), // stashes (11, self.balance_factor * 1000), (21, self.balance_factor * 1000), diff --git a/substrate/frame/staking/src/mock.rs b/substrate/frame/staking/src/mock.rs index 1018dbea3b9dd..773a7a9b15483 100644 --- a/substrate/frame/staking/src/mock.rs +++ b/substrate/frame/staking/src/mock.rs @@ -448,12 +448,6 @@ impl ExtBuilder { (2, 20 * self.balance_factor), (3, 300 * self.balance_factor), (4, 400 * self.balance_factor), - // controllers (still used in some tests. Soon to be deprecated). - (10, self.balance_factor), - (20, self.balance_factor), - (30, self.balance_factor), - (40, self.balance_factor), - (50, self.balance_factor), // stashes (11, self.balance_factor * 1000), (21, self.balance_factor * 2000), @@ -466,6 +460,9 @@ impl ExtBuilder { (100, self.balance_factor * 2000), (101, self.balance_factor * 2000), // aux accounts + (10, self.balance_factor), + (30, self.balance_factor), + (50, self.balance_factor), (60, self.balance_factor), (61, self.balance_factor * 2000), (70, self.balance_factor), diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 5eb32b9be23a1..70e1b71d1706b 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -229,10 +229,6 @@ fn basic_setup_works() { // Initial Era and session assert_eq!(active_era(), 0); - // Account 10 has `balance_factor` free balance - assert_eq!(Balances::free_balance(10), 1); - assert_eq!(Balances::free_balance(10), 1); - // New era is not being forced assert_eq!(Staking::force_era(), Forcing::NotForcing); }); @@ -972,8 +968,6 @@ fn reward_destination_works() { ExtBuilder::default().nominate(false).build_and_execute(|| { // Check that account 11 is a validator assert!(Session::validators().contains(&11)); - // Check the balance of the validator account - assert_eq!(Balances::free_balance(10), 1); // Check the balance of the stash account assert_eq!(Balances::free_balance(11), 1000); // Check how much is at stake @@ -7256,17 +7250,17 @@ mod ledger { fn paired_account_works() { ExtBuilder::default().try_state(false).build_and_execute(|| { assert_ok!(Staking::bond( - RuntimeOrigin::signed(10), + RuntimeOrigin::signed(61), 100, - RewardDestination::Account(10) + RewardDestination::Account(61) )); - assert_eq!(>::get(&10), Some(10)); + assert_eq!(>::get(&61), Some(61)); assert_eq!( - StakingLedger::::paired_account(StakingAccount::Controller(10)), - Some(10) + StakingLedger::::paired_account(StakingAccount::Controller(61)), + Some(61) ); - assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(10)), Some(10)); + assert_eq!(StakingLedger::::paired_account(StakingAccount::Stash(61)), Some(61)); assert_eq!(>::get(&42), None); assert_eq!(StakingLedger::::paired_account(StakingAccount::Controller(42)), None); From 075e1c7c5bce1f4f562d6bdb13e3f6113ae5e31b Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 11:50:59 +0700 Subject: [PATCH 41/63] fix benchmark --- substrate/frame/staking/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index a0a9fce30c6f6..5869a69c6bb82 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -449,7 +449,7 @@ benchmarks! { let stash = create_stash::(USER_SEED, 100, RewardDestination::Staked)?; assert_eq!(Payee::::get(&stash), Some(RewardDestination::Staked)); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(101u32.into())) + }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(101u64)) verify { assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(stash))); } From c55c5c6e7faa6bc0ca78adaa7c7a0bed032a8481 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 11:55:48 +0700 Subject: [PATCH 42/63] fix westend + rococo genesis configs --- .../emulated/chains/relays/rococo/src/genesis.rs | 1 - .../emulated/chains/relays/westend/src/genesis.rs | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs b/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs index 55437645b0523..cd52b39c6b4fb 100644 --- a/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs +++ b/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs @@ -62,7 +62,6 @@ pub fn genesis() -> Storage { .iter() .map(|x| { ( - x.0.clone(), x.0.clone(), session_keys( x.2.clone(), diff --git a/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs b/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs index 700b80e63f6cf..beb8b666deeae 100644 --- a/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs +++ b/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs @@ -64,7 +64,6 @@ pub fn genesis() -> Storage { .iter() .map(|x| { ( - x.0.clone(), x.0.clone(), session_keys( x.2.clone(), @@ -84,7 +83,7 @@ pub fn genesis() -> Storage { stakers: validators::initial_authorities() .iter() .map(|x| { - (x.0.clone(), x.1.clone(), STASH, westend_runtime::StakerStatus::Validator) + (x.0.clone(), STASH, westend_runtime::StakerStatus::Validator) }) .collect(), invulnerables: validators::initial_authorities().iter().map(|x| x.0.clone()).collect(), From ff558539dc148d34e1c51ead9197181e5f923daa Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 11:58:27 +0700 Subject: [PATCH 43/63] fmt --- .../emulated/chains/relays/westend/src/genesis.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs b/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs index beb8b666deeae..e3f37a8051fe6 100644 --- a/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs +++ b/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs @@ -82,9 +82,7 @@ pub fn genesis() -> Storage { minimum_validator_count: 1, stakers: validators::initial_authorities() .iter() - .map(|x| { - (x.0.clone(), STASH, westend_runtime::StakerStatus::Validator) - }) + .map(|x| (x.0.clone(), STASH, westend_runtime::StakerStatus::Validator)) .collect(), invulnerables: validators::initial_authorities().iter().map(|x| x.0.clone()).collect(), force_era: pallet_staking::Forcing::ForceNone, From 0dd1653e67d8f6f78fed3e01ef5989cdd3403686 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:01:45 +0700 Subject: [PATCH 44/63] revert staking version --- Cargo.lock | 2 +- substrate/frame/staking/Cargo.toml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 05b87120c9256..1767245c6abfc 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -11236,7 +11236,7 @@ dependencies = [ [[package]] name = "pallet-staking" -version = "29.0.0" +version = "28.0.0" dependencies = [ "frame-benchmarking", "frame-election-provider-support", diff --git a/substrate/frame/staking/Cargo.toml b/substrate/frame/staking/Cargo.toml index b822fbf424ed9..22df746d667ab 100644 --- a/substrate/frame/staking/Cargo.toml +++ b/substrate/frame/staking/Cargo.toml @@ -1,6 +1,6 @@ [package] name = "pallet-staking" -version = "29.0.0" +version = "28.0.0" authors.workspace = true edition.workspace = true license = "Apache-2.0" From c7543ec512c14cb06f44f2a178a1d6899ae7119f Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:03:33 +0700 Subject: [PATCH 45/63] fix return type --- cumulus/parachains/integration-tests/emulated/common/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs index cbde0642f1a29..9be9e626a6df9 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs @@ -166,7 +166,6 @@ pub mod validators { use super::*; pub fn initial_authorities() -> Vec<( - AccountId, AccountId, BabeId, GrandpaId, From 83325b88b59f7ab5ea10a35c7815912bbce646d6 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:03:57 +0700 Subject: [PATCH 46/63] fmt --- .../integration-tests/emulated/common/src/lib.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs index 9be9e626a6df9..06a5ac53854dd 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs @@ -165,15 +165,9 @@ pub mod collators { pub mod validators { use super::*; - pub fn initial_authorities() -> Vec<( - AccountId, - BabeId, - GrandpaId, - ValidatorId, - AssignmentId, - AuthorityDiscoveryId, - BeefyId, - )> { + pub fn initial_authorities( + ) -> Vec<(AccountId, BabeId, GrandpaId, ValidatorId, AssignmentId, AuthorityDiscoveryId, BeefyId)> + { let seed = "Alice"; vec![( get_account_id_from_seed::(&format!("{}//stash", seed)), From 119096eb52f09a3e498e201337dcf319272355a0 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:07:23 +0700 Subject: [PATCH 47/63] fix indices --- .../emulated/chains/relays/rococo/src/genesis.rs | 2 +- .../emulated/chains/relays/westend/src/genesis.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs b/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs index cd52b39c6b4fb..efa9c9e99abba 100644 --- a/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs +++ b/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs @@ -64,12 +64,12 @@ pub fn genesis() -> Storage { ( x.0.clone(), session_keys( + x.1.clone(), x.2.clone(), x.3.clone(), x.4.clone(), x.5.clone(), x.6.clone(), - x.7.clone(), ), ) }) diff --git a/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs b/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs index e3f37a8051fe6..1949c8362b6bd 100644 --- a/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs +++ b/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs @@ -66,12 +66,12 @@ pub fn genesis() -> Storage { ( x.0.clone(), session_keys( + x.1.clone(), x.2.clone(), x.3.clone(), x.4.clone(), x.5.clone(), x.6.clone(), - x.7.clone(), ), ) }) From 9ec8aaa9a253bdbf4ff5e203565a635f723b5c10 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:09:02 +0700 Subject: [PATCH 48/63] fix unused key --- polkadot/runtime/common/src/try_runtime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/runtime/common/src/try_runtime.rs b/polkadot/runtime/common/src/try_runtime.rs index 000b41e138ce2..ba1d83dd93e5f 100644 --- a/polkadot/runtime/common/src/try_runtime.rs +++ b/polkadot/runtime/common/src/try_runtime.rs @@ -34,7 +34,7 @@ where let mut unstaked_err = 0; let mut unstaked_slashed = 0; - let all_stakers = Ledger::::iter().map(|(stash, l)| l.stash).collect::>(); + let all_stakers = Ledger::::iter().map(|(_, l)| l.stash).collect::>(); let mut all_exposed = BTreeSet::new(); ErasStakers::::iter().for_each(|(_, val, expo)| { all_exposed.insert(val); From 4fdabd06eafeb3a5c7e6570c2a36e06851de912e Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:10:20 +0700 Subject: [PATCH 49/63] fix authorities return --- cumulus/parachains/integration-tests/emulated/common/src/lib.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs index 06a5ac53854dd..ab8a62c5fd349 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs @@ -171,7 +171,6 @@ pub mod validators { let seed = "Alice"; vec![( get_account_id_from_seed::(&format!("{}//stash", seed)), - get_account_id_from_seed::(seed), get_from_seed::(seed), get_from_seed::(seed), get_from_seed::(seed), From 350b4704477fe52b9a1b1f8f5b39f03515401fe0 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:16:01 +0700 Subject: [PATCH 50/63] fix benchmark --- substrate/frame/staking/src/benchmarking.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 5869a69c6bb82..a24538fd5f2d2 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -447,9 +447,11 @@ benchmarks! { set_payee { let stash = create_stash::(USER_SEED, 100, RewardDestination::Staked)?; + let new_payee = create_funded_user::("new_payee", USER_SEED + 4, 100); + assert_eq!(Payee::::get(&stash), Some(RewardDestination::Staked)); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(101u64)) + }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(new_payee)) verify { assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(stash))); } From d8669ab1d69148ed07d39688a49dd9ce88fd57bc Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:28:39 +0700 Subject: [PATCH 51/63] fix: bring back validator ids --- .../emulated/chains/relays/rococo/src/genesis.rs | 3 ++- .../emulated/chains/relays/westend/src/genesis.rs | 3 ++- .../integration-tests/emulated/common/src/lib.rs | 14 +++++++++++--- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs b/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs index efa9c9e99abba..55437645b0523 100644 --- a/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs +++ b/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs @@ -62,14 +62,15 @@ pub fn genesis() -> Storage { .iter() .map(|x| { ( + x.0.clone(), x.0.clone(), session_keys( - x.1.clone(), x.2.clone(), x.3.clone(), x.4.clone(), x.5.clone(), x.6.clone(), + x.7.clone(), ), ) }) diff --git a/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs b/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs index 1949c8362b6bd..c1ceab6eb3a50 100644 --- a/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs +++ b/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs @@ -64,14 +64,15 @@ pub fn genesis() -> Storage { .iter() .map(|x| { ( + x.0.clone(), x.0.clone(), session_keys( - x.1.clone(), x.2.clone(), x.3.clone(), x.4.clone(), x.5.clone(), x.6.clone(), + x.7.clone(), ), ) }) diff --git a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs index ab8a62c5fd349..cbde0642f1a29 100644 --- a/cumulus/parachains/integration-tests/emulated/common/src/lib.rs +++ b/cumulus/parachains/integration-tests/emulated/common/src/lib.rs @@ -165,12 +165,20 @@ pub mod collators { pub mod validators { use super::*; - pub fn initial_authorities( - ) -> Vec<(AccountId, BabeId, GrandpaId, ValidatorId, AssignmentId, AuthorityDiscoveryId, BeefyId)> - { + pub fn initial_authorities() -> Vec<( + AccountId, + AccountId, + BabeId, + GrandpaId, + ValidatorId, + AssignmentId, + AuthorityDiscoveryId, + BeefyId, + )> { let seed = "Alice"; vec![( get_account_id_from_seed::(&format!("{}//stash", seed)), + get_account_id_from_seed::(seed), get_from_seed::(seed), get_from_seed::(seed), get_from_seed::(seed), From a37c61922c1313913414ae4d6c2e2dbb17474983 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:38:35 +0700 Subject: [PATCH 52/63] fix chain spec --- substrate/bin/node/cli/src/chain_spec.rs | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/substrate/bin/node/cli/src/chain_spec.rs b/substrate/bin/node/cli/src/chain_spec.rs index 34f0a916c5fe4..95fa30ecb52db 100644 --- a/substrate/bin/node/cli/src/chain_spec.rs +++ b/substrate/bin/node/cli/src/chain_spec.rs @@ -280,6 +280,7 @@ pub fn authority_keys_from_seed( fn configure_accounts( initial_authorities: Vec<( + AccountId, AccountId, GrandpaId, BabeId, @@ -292,7 +293,16 @@ fn configure_accounts( endowed_accounts: Option>, stash: Balance, ) -> ( - Vec<(AccountId, GrandpaId, BabeId, ImOnlineId, AuthorityDiscoveryId, MixnetId, BeefyId)>, + Vec<( + AccountId, + AccountId, + GrandpaId, + BabeId, + ImOnlineId, + AuthorityDiscoveryId, + MixnetId, + BeefyId, + )>, Vec, usize, Vec<(AccountId, Balance, StakerStatus)>, @@ -339,7 +349,7 @@ fn configure_accounts( .into_iter() .map(|choice| choice.0.clone()) .collect::>(); - (x.clone(), x.clone(), stash, StakerStatus::Nominator(nominations)) + (x.clone(), stash, StakerStatus::Nominator(nominations)) })) .collect::>(); @@ -351,6 +361,7 @@ fn configure_accounts( /// Helper function to create RuntimeGenesisConfig json patch for testing. pub fn testnet_genesis( initial_authorities: Vec<( + AccountId, AccountId, GrandpaId, BabeId, @@ -375,6 +386,7 @@ pub fn testnet_genesis( .iter() .map(|x| { ( + x.0.clone(), x.0.clone(), session_keys( x.2.clone(), From 85e4614248f9c40e430b01213cba2a4a74158fcf Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 12:46:51 +0700 Subject: [PATCH 53/63] fix genesis --- substrate/bin/node/testing/src/genesis.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/substrate/bin/node/testing/src/genesis.rs b/substrate/bin/node/testing/src/genesis.rs index c79612d68444c..e321ea90df2d7 100644 --- a/substrate/bin/node/testing/src/genesis.rs +++ b/substrate/bin/node/testing/src/genesis.rs @@ -57,9 +57,9 @@ pub fn config_endowed(extra_endowed: Vec) -> RuntimeGenesisConfig { }, staking: StakingConfig { stakers: vec![ - (dave(), dave(), 111 * DOLLARS, StakerStatus::Validator), - (eve(), eve(), 100 * DOLLARS, StakerStatus::Validator), - (ferdie(), ferdie(), 100 * DOLLARS, StakerStatus::Validator), + (dave(), 111 * DOLLARS, StakerStatus::Validator), + (eve(), 100 * DOLLARS, StakerStatus::Validator), + (ferdie(), 100 * DOLLARS, StakerStatus::Validator), ], validator_count: 3, minimum_validator_count: 0, From 4947a8ea9669c7ba36906b0d3fc7471a6bce9238 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 14:25:09 +0700 Subject: [PATCH 54/63] address clippy --- polkadot/runtime/common/src/try_runtime.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/polkadot/runtime/common/src/try_runtime.rs b/polkadot/runtime/common/src/try_runtime.rs index ba1d83dd93e5f..1c7da2775839f 100644 --- a/polkadot/runtime/common/src/try_runtime.rs +++ b/polkadot/runtime/common/src/try_runtime.rs @@ -43,7 +43,7 @@ where let eligible = all_stakers .iter() - .filter_map(|stash| all_exposed.contains(stash).then_some(stash)) + .filter(|stash| all_exposed.contains(stash)) .collect::>(); log::info!( From a5b23e1b0f8adf40dabc372b871f5b7711db03f5 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Sun, 26 May 2024 17:01:32 +0700 Subject: [PATCH 55/63] update prdoc --- prdoc/pr_4574.prdoc | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/prdoc/pr_4574.prdoc b/prdoc/pr_4574.prdoc index 55e7be8f0e358..5fbf1bc75ce0d 100644 --- a/prdoc/pr_4574.prdoc +++ b/prdoc/pr_4574.prdoc @@ -1,10 +1,32 @@ -title: Remove controller logic from staking pallet. +title: Remove staking controller account logic from codebase. doc: - audience: Runtime Dev description: - Removes controller account logic from the staking pallet. + Removes controller account logic from the pallets and runtimes. crates: - name: pallet-staking - bump: major \ No newline at end of file + bump: minor + - name: pallet-babe + bump: minor + - name: polkadot-runtime-common + bump: minor + - name: pallet-fast-unstake + bump: minor + - name: pallet-grandpa + bump: minor + - name: pallet-beefy + bump: minor + - name: westend-runtime + bump: minor + - name: pallet-delegated-staking + bump: minor + - name: pallet-nomination-pools-benchmarking + bump: minor + - name: pallet-offences-benchmarking + bump: minor + - name: pallet-session-benchmarking + bump: minor + - name: pallet-root-offences + bump: minor \ No newline at end of file From 7cfb1010bdf215bb3cc5ce9ef99105180a6cfbf0 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 27 May 2024 08:57:25 +0700 Subject: [PATCH 56/63] remove controller from pallet docs --- substrate/frame/staking/src/lib.rs | 16 +++++++--------- 1 file changed, 7 insertions(+), 9 deletions(-) diff --git a/substrate/frame/staking/src/lib.rs b/substrate/frame/staking/src/lib.rs index e77749a2881b0..81eb20f0a0d00 100644 --- a/substrate/frame/staking/src/lib.rs +++ b/substrate/frame/staking/src/lib.rs @@ -40,8 +40,7 @@ //! producing blocks or guaranteeing finality of the chain. //! - Nominating: The process of placing staked funds behind one or more validators in order to //! share in any reward, and punishment, they take. -//! - Stash account: The account holding an owner's funds used for staking. -//! - Controller account (being deprecated): The account that controls an owner's funds for staking. +//! - Stash account: The account holding and controlling an owner's funds used for staking. //! - Era: A (whole) number of sessions, which is the period that the validator set (and each //! validator's active nominator set) is recalculated and where rewards are paid out. //! - Slash: The punishment of a staker by reducing its funds. @@ -62,8 +61,7 @@ //! Almost any interaction with the Staking pallet requires a process of _**bonding**_ (also known //! as being a _staker_). To become *bonded*, a fund-holding register known as the _stash account_, //! which holds some or all of the funds that become frozen in place as part of the staking process. -//! The controller account, which this pallet now assigns the stash account to, issues instructions -//! on how funds shall be used. +//! The stash account issues instructions on how funds shall be used. //! //! An account can become a bonded stash account using the [`bond`](Call::bond) call. //! @@ -245,11 +243,11 @@ //! //! Any funds already placed into stash can be the target of the following operations: //! -//! The controller account can free a portion (or all) of the funds using the -//! [`unbond`](Call::unbond) call. Note that the funds are not immediately accessible. Instead, a -//! duration denoted by [`Config::BondingDuration`] (in number of eras) must pass until the funds -//! can actually be removed. Once the `BondingDuration` is over, the -//! [`withdraw_unbonded`](Call::withdraw_unbonded) call can be used to actually withdraw the funds. +//! The stash account can free a portion (or all) of the funds using the [`unbond`](Call::unbond) +//! call. Note that the funds are not immediately accessible. Instead, a duration denoted by +//! [`Config::BondingDuration`] (in number of eras) must pass until the funds can actually be +//! removed. Once the `BondingDuration` is over, the [`withdraw_unbonded`](Call::withdraw_unbonded) +//! call can be used to actually withdraw the funds. //! //! Note that there is a limitation to the number of fund-chunks that can be scheduled to be //! unlocked in the future via [`unbond`](Call::unbond). In case this maximum From 1d612160dac9ab5b168c07a252cef7975a5926f2 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 27 May 2024 09:23:14 +0700 Subject: [PATCH 57/63] `fast-unstake`: use stash, not controller --- substrate/frame/fast-unstake/src/lib.rs | 16 ++++++------- substrate/frame/fast-unstake/src/tests.rs | 24 ++++++++++---------- substrate/frame/nomination-pools/src/mock.rs | 2 +- substrate/frame/staking/src/pallet/impls.rs | 8 +++---- substrate/primitives/staking/src/lib.rs | 10 +++----- 5 files changed, 27 insertions(+), 33 deletions(-) diff --git a/substrate/frame/fast-unstake/src/lib.rs b/substrate/frame/fast-unstake/src/lib.rs index f31c9c640260b..f34a21d1f7d7d 100644 --- a/substrate/frame/fast-unstake/src/lib.rs +++ b/substrate/frame/fast-unstake/src/lib.rs @@ -250,10 +250,10 @@ pub mod pallet { #[pallet::error] #[cfg_attr(test, derive(PartialEq))] pub enum Error { - /// The provided Controller account was not found. + /// The provided stash account was not found. /// /// This means that the given account is not bonded. - NotController, + NotStash, /// The bonded account has already been queued. AlreadyQueued, /// The bonded account has active unlocking chunks. @@ -329,11 +329,11 @@ pub mod pallet { #[pallet::call_index(0)] #[pallet::weight(::WeightInfo::register_fast_unstake())] pub fn register_fast_unstake(origin: OriginFor) -> DispatchResult { - let ctrl = ensure_signed(origin)?; + let signed = ensure_signed(origin)?; ensure!(ErasToCheckPerBlock::::get() != 0, Error::::CallNotAllowed); - let stash_account = - T::Staking::stash_by_ctrl(&ctrl).map_err(|_| Error::::NotController)?; + + let stash_account = T::Staking::stash(&signed).map_err(|_| Error::::NotStash)?; ensure!(!Queue::::contains_key(&stash_account), Error::::AlreadyQueued); ensure!(!Self::is_head(&stash_account), Error::::AlreadyHead); ensure!(!T::Staking::is_unbonding(&stash_account)?, Error::::NotFullyBonded); @@ -370,12 +370,12 @@ pub mod pallet { #[pallet::call_index(1)] #[pallet::weight(::WeightInfo::deregister())] pub fn deregister(origin: OriginFor) -> DispatchResult { - let ctrl = ensure_signed(origin)?; + let signed = ensure_signed(origin)?; ensure!(ErasToCheckPerBlock::::get() != 0, Error::::CallNotAllowed); - let stash_account = - T::Staking::stash_by_ctrl(&ctrl).map_err(|_| Error::::NotController)?; + let stash_account = T::Staking::stash(&signed).map_err(|_| Error::::NotStash)?; + ensure!(Queue::::contains_key(&stash_account), Error::::NotQueued); ensure!(!Self::is_head(&stash_account), Error::::AlreadyHead); let deposit = Queue::::take(stash_account.clone()); diff --git a/substrate/frame/fast-unstake/src/tests.rs b/substrate/frame/fast-unstake/src/tests.rs index 77128872f285f..a2d1788dd6697 100644 --- a/substrate/frame/fast-unstake/src/tests.rs +++ b/substrate/frame/fast-unstake/src/tests.rs @@ -36,7 +36,7 @@ fn test_setup_works() { fn register_works() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); - // Controller account registers for fast unstake. + // Stash account registers for fast unstake. assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); // Ensure stash is in the queue. assert_ne!(Queue::::get(1), None); @@ -50,7 +50,7 @@ fn register_insufficient_funds_fails() { ErasToCheckPerBlock::::put(1); ::Currency::make_free_balance_be(&1, 3); - // Controller account registers for fast unstake. + // Stash account registers for fast unstake. assert_noop!( FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)), BalancesError::::InsufficientBalance, @@ -82,7 +82,7 @@ fn cannot_register_if_not_bonded() { // Attempt to fast unstake. assert_noop!( FastUnstake::register_fast_unstake(RuntimeOrigin::signed(2)), - Error::::NotController + Error::::NotStash ); }); } @@ -110,7 +110,7 @@ fn cannot_register_if_head() { stashes: bounded_vec![(1, Deposit::get())], checked: bounded_vec![], }); - // Controller attempts to register + // Stash attempts to register assert_noop!( FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1)), Error::::AlreadyHead @@ -139,11 +139,11 @@ fn deregister_works() { assert_eq!(::Currency::reserved_balance(&1), 0); - // Controller account registers for fast unstake. + // Stash account registers for fast unstake. assert_ok!(FastUnstake::register_fast_unstake(RuntimeOrigin::signed(1))); assert_eq!(::Currency::reserved_balance(&1), Deposit::get()); - // Controller then changes mind and deregisters. + // Stash then changes mind and deregisters. assert_ok!(FastUnstake::deregister(RuntimeOrigin::signed(1))); assert_eq!(::Currency::reserved_balance(&1), 0); @@ -163,13 +163,13 @@ fn deregister_disabled_fails() { } #[test] -fn cannot_deregister_if_not_controller() { +fn cannot_deregister_if_not_stash() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); - // Controller (same as stash) account registers for fast unstake. + // Stash 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); + assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(2)), Error::::NotStash); }); } @@ -177,7 +177,7 @@ fn cannot_deregister_if_not_controller() { fn cannot_deregister_if_not_queued() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); - // Controller tries to deregister without first registering + // Stash tries to deregister without first registering assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(1)), Error::::NotQueued); }); } @@ -186,14 +186,14 @@ fn cannot_deregister_if_not_queued() { fn cannot_deregister_already_head() { ExtBuilder::default().build_and_execute(|| { ErasToCheckPerBlock::::put(1); - // Controller attempts to register, should fail + // Stash attempts to register, should fail 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 + // Stash attempts to deregister assert_noop!(FastUnstake::deregister(RuntimeOrigin::signed(1)), Error::::AlreadyHead); }); } diff --git a/substrate/frame/nomination-pools/src/mock.rs b/substrate/frame/nomination-pools/src/mock.rs index b659c975a8395..2d6d5c7bcc7c7 100644 --- a/substrate/frame/nomination-pools/src/mock.rs +++ b/substrate/frame/nomination-pools/src/mock.rs @@ -183,7 +183,7 @@ impl sp_staking::StakingInterface for StakingMock { Nominations::get() } - fn stash_by_ctrl(_controller: &Self::AccountId) -> Result { + fn stash(_who: &Self::AccountId) -> Result { unimplemented!("method currently not used in testing") } diff --git a/substrate/frame/staking/src/pallet/impls.rs b/substrate/frame/staking/src/pallet/impls.rs index 96ca6ca8a4232..709c04eb25289 100644 --- a/substrate/frame/staking/src/pallet/impls.rs +++ b/substrate/frame/staking/src/pallet/impls.rs @@ -45,7 +45,7 @@ use sp_staking::{ currency_to_vote::CurrencyToVote, offence::{OffenceDetails, OnOffenceHandler}, EraIndex, OnStakingUpdate, Page, SessionIndex, Stake, - StakingAccount::{self, Controller, Stash}, + StakingAccount::{self, Stash}, StakingInterface, }; use sp_std::prelude::*; @@ -1748,10 +1748,8 @@ impl StakingInterface for Pallet { MinValidatorBond::::get() } - fn stash_by_ctrl(controller: &Self::AccountId) -> Result { - Self::ledger(Controller(controller.clone())) - .map(|l| l.stash) - .map_err(|e| e.into()) + fn stash(who: &Self::AccountId) -> Result { + Self::ledger(Stash(who.clone())).map(|l| l.stash).map_err(|e| e.into()) } fn bonding_duration() -> EraIndex { diff --git a/substrate/primitives/staking/src/lib.rs b/substrate/primitives/staking/src/lib.rs index 28a61cd433139..307d30168e87e 100644 --- a/substrate/primitives/staking/src/lib.rs +++ b/substrate/primitives/staking/src/lib.rs @@ -189,13 +189,9 @@ pub trait StakingInterface { /// The minimum amount required to bond in order to set validation intentions. fn minimum_validator_bond() -> Self::Balance; - /// Return a stash account that is controlled by a `controller`. - /// - /// ## Note - /// - /// The controller abstraction is not permanent and might go away. Avoid using this as much as - /// possible. - fn stash_by_ctrl(controller: &Self::AccountId) -> Result; + /// Return a stash account of a ledger associated with the provided account, `Err` if not a + /// staker. + fn stash(who: &Self::AccountId) -> Result; /// Number of eras that staked funds must remain bonded for. fn bonding_duration() -> EraIndex; From fecdcb72319c9e570da18cc5d37c2a1b262a3824 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 27 May 2024 10:13:14 +0700 Subject: [PATCH 58/63] update prdoc --- prdoc/pr_4574.prdoc | 28 ++++++++++++++++------------ 1 file changed, 16 insertions(+), 12 deletions(-) diff --git a/prdoc/pr_4574.prdoc b/prdoc/pr_4574.prdoc index 5fbf1bc75ce0d..fc2063aaec52f 100644 --- a/prdoc/pr_4574.prdoc +++ b/prdoc/pr_4574.prdoc @@ -9,24 +9,28 @@ crates: - name: pallet-staking bump: minor - name: pallet-babe - bump: minor - - name: polkadot-runtime-common - bump: minor + bump: patch - name: pallet-fast-unstake bump: minor - name: pallet-grandpa - bump: minor + bump: patch - name: pallet-beefy - bump: minor - - name: westend-runtime - bump: minor + bump: patch - name: pallet-delegated-staking - bump: minor + bump: patch - name: pallet-nomination-pools-benchmarking - bump: minor + bump: patch - name: pallet-offences-benchmarking - bump: minor + bump: patch - name: pallet-session-benchmarking - bump: minor + bump: patch - name: pallet-root-offences - bump: minor \ No newline at end of file + bump: patch + - name: pallet-nomination-pools + bump: patch + - name: sp-staking + bump: patch + - name: polkadot-runtime-common + bump: patch + - name: westend-runtime + bump: patch \ No newline at end of file From b3468ea84623a36dbb9509ac5aa284d9fa07ff42 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Mon, 27 May 2024 15:48:43 +0700 Subject: [PATCH 59/63] remove note --- substrate/frame/delegated-staking/src/lib.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/substrate/frame/delegated-staking/src/lib.rs b/substrate/frame/delegated-staking/src/lib.rs index 896c41838e36c..fd38b593fa9a6 100644 --- a/substrate/frame/delegated-staking/src/lib.rs +++ b/substrate/frame/delegated-staking/src/lib.rs @@ -493,8 +493,9 @@ impl Pallet { // This should never fail but if it does, it indicates bad state and we abort. T::Currency::transfer(who, &proxy_delegator, amount_to_transfer, Preservation::Expendable)?; - T::CoreStaking::update_payee(who, reward_account)?; // NOTE: Not sure why this is being used. - // delegate all transferred funds back to agent. + T::CoreStaking::update_payee(who, reward_account)?; + + // delegate all transferred funds back to agent. Self::do_delegate(&proxy_delegator, who, amount_to_transfer)?; // if the transferred/delegated amount was greater than the stake, mark the extra as From 635e070d4bf98a99d9cc38881fa710aa82198dc7 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 28 May 2024 09:58:39 +0700 Subject: [PATCH 60/63] `staking`: remove update_payee (deprecation) call --- .../westend/src/weights/pallet_staking.rs | 16 ---------- substrate/frame/staking/src/benchmarking.rs | 14 --------- substrate/frame/staking/src/pallet/mod.rs | 30 ------------------ substrate/frame/staking/src/tests.rs | 21 ------------- substrate/frame/staking/src/weights.rs | 31 ------------------- 5 files changed, 112 deletions(-) diff --git a/polkadot/runtime/westend/src/weights/pallet_staking.rs b/polkadot/runtime/westend/src/weights/pallet_staking.rs index 20a316d0d2dfd..c8da647165e75 100644 --- a/polkadot/runtime/westend/src/weights/pallet_staking.rs +++ b/polkadot/runtime/westend/src/weights/pallet_staking.rs @@ -318,22 +318,6 @@ impl pallet_staking::WeightInfo for WeightInfo { .saturating_add(T::DbWeight::get().reads(2)) .saturating_add(T::DbWeight::get().writes(1)) } - /// Storage: `Staking::Ledger` (r:1 w:0) - /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) - /// Storage: `Staking::Bonded` (r:1 w:0) - /// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`) - /// Storage: `Staking::Payee` (r:1 w:1) - /// Proof: `Staking::Payee` (`max_values`: None, `max_size`: Some(73), added: 2548, mode: `MaxEncodedLen`) - fn update_payee() -> Weight { - // Proof Size summary in bytes: - // Measured: `932` - // Estimated: `4556` - // Minimum execution time: 23_428_000 picoseconds. - Weight::from_parts(24_080_000, 0) - .saturating_add(Weight::from_parts(0, 4556)) - .saturating_add(T::DbWeight::get().reads(3)) - .saturating_add(T::DbWeight::get().writes(1)) - } /// Storage: `Staking::ValidatorCount` (r:0 w:1) /// Proof: `Staking::ValidatorCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) fn set_validator_count() -> Weight { diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index a24538fd5f2d2..c246f5b157c9f 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -456,20 +456,6 @@ benchmarks! { assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(stash))); } - // NOTE: This benchmark will not be worst case as stash is now the same as controller. Remove this - // once is resolved. - update_payee { - let stash = create_stash::(USER_SEED, 100, RewardDestination::Staked)?; - Payee::::insert(&stash, { - #[allow(deprecated)] - RewardDestination::Controller - }); - whitelist_account!(stash); - }: _(RawOrigin::Signed(stash.clone()), stash.clone()) - verify { - assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(stash))); - } - set_validator_count { let validator_count = MaxValidators::::get(); }: _(RawOrigin::Root, validator_count) diff --git a/substrate/frame/staking/src/pallet/mod.rs b/substrate/frame/staking/src/pallet/mod.rs index 4cac9adbed7f2..09e337101a63e 100644 --- a/substrate/frame/staking/src/pallet/mod.rs +++ b/substrate/frame/staking/src/pallet/mod.rs @@ -1856,36 +1856,6 @@ pub mod pallet { Self::do_payout_stakers_by_page(validator_stash, era, page) } - /// Migrates an account's `RewardDestination::Controller` to - /// `RewardDestination::Account(controller)`. - /// - /// Effects will be felt instantly (as soon as this function is completed successfully). - /// - /// This will waive the transaction fee if the `payee` is successfully migrated. - #[pallet::call_index(27)] - #[pallet::weight(T::WeightInfo::update_payee())] - pub fn update_payee( - origin: OriginFor, - controller: T::AccountId, - ) -> DispatchResultWithPostInfo { - let _ = ensure_signed(origin)?; - let ledger = Self::ledger(StakingAccount::Controller(controller.clone()))?; - - ensure!( - (Payee::::get(&ledger.stash) == { - #[allow(deprecated)] - Some(RewardDestination::Controller) - }), - Error::::NotController - ); - - let _ = ledger - .set_payee(RewardDestination::Account(controller)) - .defensive_proof("ledger should have been previously retrieved from storage.")?; - - Ok(Pays::No.into()) - } - /// Restores the state of a ledger which is in an inconsistent state. /// /// The requirements to restore a ledger are the following: diff --git a/substrate/frame/staking/src/tests.rs b/substrate/frame/staking/src/tests.rs index 70e1b71d1706b..b4027c3500b78 100644 --- a/substrate/frame/staking/src/tests.rs +++ b/substrate/frame/staking/src/tests.rs @@ -7420,27 +7420,6 @@ mod ledger { assert_eq!(Payee::::get(&11), Some(RewardDestination::Staked)); }) } - - #[test] - #[allow(deprecated)] - fn update_payee_migration_works() { - ExtBuilder::default().build_and_execute(|| { - // migrate a `Controller` variant to `Account` variant. - Payee::::insert(11, RewardDestination::Controller); - assert_eq!(Payee::::get(&11), Some(RewardDestination::Controller)); - assert_ok!(Staking::update_payee(RuntimeOrigin::signed(11), 11)); - assert_eq!(Payee::::get(&11), Some(RewardDestination::Account(11))); - - // Do not migrate a variant if not `Controller`. - Payee::::insert(21, RewardDestination::Stash); - assert_eq!(Payee::::get(&21), Some(RewardDestination::Stash)); - assert_noop!( - Staking::update_payee(RuntimeOrigin::signed(11), 21), - Error::::NotController - ); - assert_eq!(Payee::::get(&21), Some(RewardDestination::Stash)); - }) - } } mod ledger_recovery { diff --git a/substrate/frame/staking/src/weights.rs b/substrate/frame/staking/src/weights.rs index fc1451a7519c5..2d805ee8832d7 100644 --- a/substrate/frame/staking/src/weights.rs +++ b/substrate/frame/staking/src/weights.rs @@ -61,7 +61,6 @@ pub trait WeightInfo { fn nominate(n: u32, ) -> Weight; fn chill() -> Weight; fn set_payee() -> Weight; - fn update_payee() -> Weight; fn set_validator_count() -> Weight; fn force_no_eras() -> Weight; fn force_new_era() -> Weight; @@ -346,21 +345,6 @@ impl WeightInfo for SubstrateWeight { .saturating_add(T::DbWeight::get().reads(2_u64)) .saturating_add(T::DbWeight::get().writes(1_u64)) } - /// Storage: `Staking::Ledger` (r:1 w:0) - /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) - /// Storage: `Staking::Bonded` (r:1 w:0) - /// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`) - /// Storage: `Staking::Payee` (r:1 w:1) - /// Proof: `Staking::Payee` (`max_values`: None, `max_size`: Some(73), added: 2548, mode: `MaxEncodedLen`) - fn update_payee() -> Weight { - // Proof Size summary in bytes: - // Measured: `969` - // Estimated: `4556` - // Minimum execution time: 23_705_000 picoseconds. - Weight::from_parts(24_409_000, 4556) - .saturating_add(T::DbWeight::get().reads(3_u64)) - .saturating_add(T::DbWeight::get().writes(1_u64)) - } /// Storage: `Staking::ValidatorCount` (r:0 w:1) /// Proof: `Staking::ValidatorCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) fn set_validator_count() -> Weight { @@ -1064,21 +1048,6 @@ impl WeightInfo for () { .saturating_add(RocksDbWeight::get().reads(2_u64)) .saturating_add(RocksDbWeight::get().writes(1_u64)) } - /// Storage: `Staking::Ledger` (r:1 w:0) - /// Proof: `Staking::Ledger` (`max_values`: None, `max_size`: Some(1091), added: 3566, mode: `MaxEncodedLen`) - /// Storage: `Staking::Bonded` (r:1 w:0) - /// Proof: `Staking::Bonded` (`max_values`: None, `max_size`: Some(72), added: 2547, mode: `MaxEncodedLen`) - /// Storage: `Staking::Payee` (r:1 w:1) - /// Proof: `Staking::Payee` (`max_values`: None, `max_size`: Some(73), added: 2548, mode: `MaxEncodedLen`) - fn update_payee() -> Weight { - // Proof Size summary in bytes: - // Measured: `969` - // Estimated: `4556` - // Minimum execution time: 23_705_000 picoseconds. - Weight::from_parts(24_409_000, 4556) - .saturating_add(RocksDbWeight::get().reads(3_u64)) - .saturating_add(RocksDbWeight::get().writes(1_u64)) - } /// Storage: `Staking::ValidatorCount` (r:0 w:1) /// Proof: `Staking::ValidatorCount` (`max_values`: Some(1), `max_size`: Some(4), added: 499, mode: `MaxEncodedLen`) fn set_validator_count() -> Weight { From c6be70d955247994aea51206e5b0902f8a5be849 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Tue, 28 May 2024 10:24:48 +0700 Subject: [PATCH 61/63] `Bonded`: don't assert in benchmarks --- substrate/frame/staking/src/benchmarking.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index c246f5b157c9f..c5dc67f165e4b 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -213,7 +213,6 @@ benchmarks! { whitelist_account!(stash); }: _(RawOrigin::Signed(stash.clone()), amount, reward_destination) verify { - assert!(Bonded::::contains_key(stash.clone())); assert!(Ledger::::contains_key(stash)); } @@ -622,13 +621,11 @@ benchmarks! { ); Ledger::::insert(&stash, l); - assert!(Bonded::::contains_key(&stash)); assert!(T::VoterList::contains(&stash)); whitelist_account!(stash); }: _(RawOrigin::Signed(stash.clone()), stash.clone(), s) verify { - assert!(!Bonded::::contains_key(&stash)); assert!(!T::VoterList::contains(&stash)); } From 5c725d82f5522f7c91b3d38cc8cdc1a3ab835564 Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 29 May 2024 10:34:28 +0700 Subject: [PATCH 62/63] fix benchmark --- substrate/frame/staking/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index c5dc67f165e4b..2536703364c33 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -452,7 +452,7 @@ benchmarks! { whitelist_account!(stash); }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(new_payee)) verify { - assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(stash))); + assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(new_payee))); } set_validator_count { From 8ac11940c1a913d2f7ccbbb9a40a2ca14cc7891b Mon Sep 17 00:00:00 2001 From: Ross Bulat Date: Wed, 29 May 2024 10:34:48 +0700 Subject: [PATCH 63/63] clone --- substrate/frame/staking/src/benchmarking.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/staking/src/benchmarking.rs b/substrate/frame/staking/src/benchmarking.rs index 2536703364c33..9333ec30ff134 100644 --- a/substrate/frame/staking/src/benchmarking.rs +++ b/substrate/frame/staking/src/benchmarking.rs @@ -450,7 +450,7 @@ benchmarks! { assert_eq!(Payee::::get(&stash), Some(RewardDestination::Staked)); whitelist_account!(stash); - }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(new_payee)) + }: _(RawOrigin::Signed(stash.clone()), RewardDestination::Account(new_payee.clone())) verify { assert_eq!(Payee::::get(&stash), Some(RewardDestination::Account(new_payee))); }