diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 037211155cf0e..ed34aaa5820b2 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -563,7 +563,8 @@ impl pallet_staking::Config for Runtime { type OffendingValidatorsThreshold = OffendingValidatorsThreshold; type ElectionProvider = ElectionProviderMultiPhase; type GenesisElectionProvider = onchain::UnboundedExecution; - type VoterList = BagsList; + type VoterList = VoterBagsList; + type TargetList = TargetBagsList; type MaxUnlockingChunks = ConstU32<32>; type OnStakerSlash = NominationPools; type WeightInfo = pallet_staking::weights::SubstrateWeight; @@ -717,14 +718,29 @@ impl pallet_election_provider_multi_phase::Config for Runtime { parameter_types! { pub const BagThresholds: &'static [u64] = &voter_bags::THRESHOLDS; + pub BagThresholdsBalance: &'static [Balance] = &voter_bags::THRESHOLDS_BALANCE; } -impl pallet_bags_list::Config for Runtime { +type VoterBagsListInstance = pallet_bags_list::Instance1; +impl pallet_bags_list::Config for Runtime { type Event = Event; - type ScoreProvider = Staking; - type WeightInfo = pallet_bags_list::weights::SubstrateWeight; type BagThresholds = BagThresholds; + /// The voter bags-list is loosely kept up to date, and the real source of truth for the score + /// of each node is the staking pallet. + type ScoreProvider = Staking; type Score = VoteWeight; + type WeightInfo = pallet_bags_list::weights::SubstrateWeight; +} + +type TargetBagsListInstance = pallet_bags_list::Instance2; +impl pallet_bags_list::Config for Runtime { + type Event = Event; + // The bags-list itself will be the source of truth about the approval stakes. This implies that + // staking should keep the approval stakes up to date at all times. + type ScoreProvider = TargetBagsList; + type BagThresholds = BagThresholdsBalance; + type Score = Balance; + type WeightInfo = pallet_bags_list::weights::SubstrateWeight; } parameter_types! { @@ -1624,7 +1640,8 @@ construct_runtime!( Gilt: pallet_gilt, Uniques: pallet_uniques, TransactionStorage: pallet_transaction_storage, - BagsList: pallet_bags_list, + VoterBagsList: pallet_bags_list::, + TargetBagsList: pallet_bags_list::, StateTrieMigration: pallet_state_trie_migration, ChildBounties: pallet_child_bounties, Referenda: pallet_referenda, diff --git a/bin/node/runtime/src/voter_bags.rs b/bin/node/runtime/src/voter_bags.rs index 93790f028f457..87bb29f5ac76b 100644 --- a/bin/node/runtime/src/voter_bags.rs +++ b/bin/node/runtime/src/voter_bags.rs @@ -233,3 +233,206 @@ pub const THRESHOLDS: [u64; 200] = [ 17_356_326_621_502_140_416, 18_446_744_073_709_551_615, ]; + +pub const THRESHOLDS_BALANCE: [crate::Balance; 200] = [ + 100_000_000_000_000, + 106_282_535_907_434, + 112_959_774_389_150, + 120_056_512_776_105, + 127_599_106_300_477, + 135_615_565_971_369, + 144_135_662_599_590, + 153_191_037_357_827, + 162_815_319_286_803, + 173_044_250_183_800, + 183_915_817_337_347, + 195_470_394_601_017, + 207_750_892_330_229, + 220_802_916_738_890, + 234_674_939_267_673, + 249_418_476_592_914, + 265_088_281_944_639, + 281_742_548_444_211, + 299_443_125_216_738, + 318_255_747_080_822, + 338_250_278_668_647, + 359_500_973_883_001, + 382_086_751_654_776, + 406_091_489_025_036, + 431_604_332_640_068, + 458_720_029_816_222, + 487_539_280_404_019, + 518_169_110_758_247, + 550_723_271_202_866, + 585_322_658_466_782, + 622_095_764_659_305, + 661_179_154_452_653, + 702_717_972_243_610, + 746_866_481_177_808, + 793_788_636_038_393, + 843_658_692_126_636, + 896_661_852_395_681, + 952_994_955_240_703, + 1_012_867_205_499_736, + 1_076_500_951_379_881, + 1_144_132_510_194_192, + 1_216_013_045_975_769, + 1_292_409_502_228_280, + 1_373_605_593_276_862, + 1_459_902_857_901_004, + 1_551_621_779_162_291, + 1_649_102_974_585_730, + 1_752_708_461_114_642, + 1_862_822_999_536_805, + 1_979_855_523_374_646, + 2_104_240_657_545_975, + 2_236_440_332_435_128, + 2_376_945_499_368_703, + 2_526_277_953_866_680, + 2_684_992_273_439_945, + 2_853_677_877_130_641, + 3_032_961_214_443_876, + 3_223_508_091_799_862, + 3_426_026_145_146_232, + 3_641_267_467_913_124, + 3_870_031_404_070_482, + 4_113_167_516_660_186, + 4_371_578_742_827_277, + 4_646_224_747_067_156, + 4_938_125_485_141_739, + 5_248_364_991_899_922, + 5_578_095_407_069_235, + 5_928_541_253_969_291, + 6_301_003_987_036_955, + 6_696_866_825_051_405, + 7_117_599_888_008_300, + 7_564_765_656_719_910, + 8_040_024_775_416_580, + 8_545_142_218_898_723, + 9_081_993_847_142_344, + 9_652_573_371_700_016, + 10_258_999_759_768_490, + 10_903_525_103_419_522, + 11_588_542_983_217_942, + 12_316_597_357_287_042, + 13_090_392_008_832_678, + 13_912_800_587_211_472, + 14_786_877_279_832_732, + 15_715_868_154_526_436, + 16_703_223_214_499_558, + 17_752_609_210_649_358, + 18_867_923_258_814_856, + 20_053_307_312_537_008, + 21_313_163_545_075_252, + 22_652_170_697_804_756, + 24_075_301_455_707_600, + 25_587_840_914_485_432, + 27_195_406_207_875_088, + 28_903_967_368_057_400, + 30_719_869_496_628_636, + 32_649_856_328_471_220, + 34_701_095_276_033_064, + 36_881_204_047_022_752, + 39_198_278_934_370_992, + 41_660_924_883_519_016, + 44_278_287_448_695_240, + 47_060_086_756_856_400, + 50_016_653_605_425_536, + 53_158_967_827_883_320, + 56_498_699_069_691_424, + 60_048_250_125_977_912, + 63_820_803_001_928_304, + 67_830_367_866_937_216, + 72_091_835_084_322_176, + 76_621_030_509_822_880, + 81_434_774_264_248_528, + 86_550_943_198_537_824, + 91_988_537_283_208_848, + 97_767_750_168_749_840, + 103_910_044_178_992_000, + 110_438_230_015_967_792, + 117_376_551_472_255_616, + 124_750_775_465_407_920, + 132_588_287_728_824_640, + 140_918_194_514_440_064, + 149_771_430_684_917_568, + 159_180_874_596_775_264, + 169_181_470_201_085_280, + 179_810_356_815_193_344, + 191_107_007_047_393_216, + 203_113_373_386_768_288, + 215_874_044_002_592_672, + 229_436_408_331_885_600, + 243_850_833_070_063_392, + 259_170_849_218_267_264, + 275_453_350_882_006_752, + 292_758_806_559_399_232, + 311_151_483_703_668_992, + 330_699_687_393_865_920, + 351_476_014_000_157_824, + 373_557_620_785_735_808, + 397_026_512_446_556_096, + 421_969_845_653_044_224, + 448_480_252_724_740_928, + 476_656_185_639_923_904, + 506_602_281_657_757_760, + 538_429_751_910_786_752, + 572_256_794_410_890_176, + 608_209_033_002_485_632, + 646_419_983_893_124_352, + 687_031_551_494_039_552, + 730_194_555_412_054_016, + 776_069_290_549_944_960, + 824_826_122_395_314_176, + 876_646_119_708_695_936, + 931_721_726_960_522_368, + 990_257_479_014_182_144, + 1_052_470_760_709_299_712, + 1_118_592_614_166_106_112, + 1_188_868_596_808_997_376, + 1_263_559_693_295_730_432, + 1_342_943_284_738_898_688, + 1_427_314_178_819_094_784, + 1_516_985_704_615_302_400, + 1_612_290_876_218_400_768, + 1_713_583_629_449_105_408, + 1_821_240_136_273_157_632, + 1_935_660_201_795_120_128, + 2_057_268_749_018_809_600, + 2_186_517_396_888_336_384, + 2_323_886_137_470_138_880, + 2_469_885_118_504_583_168, + 2_625_056_537_947_004_416, + 2_789_976_657_533_970_944, + 2_965_257_942_852_572_160, + 3_151_551_337_860_326_400, + 3_349_548_682_302_620_672, + 3_559_985_281_005_267_968, + 3_783_642_634_583_792_128, + 4_021_351_341_710_503_936, + 4_273_994_183_717_548_544, + 4_542_509_402_991_247_872, + 4_827_894_187_332_742_144, + 5_131_208_373_224_844_288, + 5_453_578_381_757_959_168, + 5_796_201_401_831_965_696, + 6_160_349_836_169_256_960, + 6_547_376_026_650_146_816, + 6_958_717_276_519_173_120, + 7_395_901_188_113_309_696, + 7_860_551_335_934_872_576, + 8_354_393_296_137_270_272, + 8_879_261_054_815_360_000, + 9_437_103_818_898_946_048, + 10_029_993_254_943_105_024, + 10_660_131_182_698_121_216, + 11_329_857_752_030_707_712, + 12_041_660_133_563_240_448, + 12_798_181_755_305_525_248, + 13_602_232_119_581_272_064, + 14_456_797_236_706_498_560, + 15_365_050_714_167_523_328, + 16_330_365_542_480_556_032, + 17_356_326_621_502_140_416, + 18_446_744_073_709_551_615, +]; diff --git a/frame/babe/src/mock.rs b/frame/babe/src/mock.rs index 5677eb7e28e49..b2952ba2fd53c 100644 --- a/frame/babe/src/mock.rs +++ b/frame/babe/src/mock.rs @@ -202,6 +202,7 @@ impl pallet_staking::Config for Test { type ElectionProvider = onchain::UnboundedExecution; type GenesisElectionProvider = Self::ElectionProvider; type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; + type TargetList = pallet_staking::UseValidatorsMap; type MaxUnlockingChunks = ConstU32<32>; type OnStakerSlash = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/frame/grandpa/src/mock.rs b/frame/grandpa/src/mock.rs index 5e6c955c441c5..0787d7e3f16e8 100644 --- a/frame/grandpa/src/mock.rs +++ b/frame/grandpa/src/mock.rs @@ -206,6 +206,7 @@ impl pallet_staking::Config for Test { type ElectionProvider = onchain::UnboundedExecution; type GenesisElectionProvider = Self::ElectionProvider; type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; + type TargetList = pallet_staking::UseValidatorsMap; type MaxUnlockingChunks = ConstU32<32>; type OnStakerSlash = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/frame/multisig/src/lib.rs b/frame/multisig/src/lib.rs index d4ea041e5820e..ebd8e9acb5e3e 100644 --- a/frame/multisig/src/lib.rs +++ b/frame/multisig/src/lib.rs @@ -24,12 +24,11 @@ //! ## Overview //! //! This pallet contains functionality for multi-signature dispatch, a (potentially) stateful -//! operation, allowing multiple signed -//! origins (accounts) to coordinate and dispatch a call from a well-known origin, derivable -//! deterministically from the set of account IDs and the threshold number of accounts from the -//! set that must approve it. In the case that the threshold is just one then this is a stateless -//! operation. This is useful for multisig wallets where cryptographic threshold signatures are -//! not available or desired. +//! operation, allowing multiple signed origins (accounts) to coordinate and dispatch a call from a +//! well-known origin, derivable deterministically from the set of account IDs and the threshold +//! number of accounts from the set that must approve it. In the case that the threshold is just one +//! then this is a stateless operation. This is useful for multisig wallets where cryptographic +//! threshold signatures are not available or desired. //! //! ## Interface //! diff --git a/frame/nomination-pools/benchmarking/src/mock.rs b/frame/nomination-pools/benchmarking/src/mock.rs index 1c74c301e562d..714aaef224e0b 100644 --- a/frame/nomination-pools/benchmarking/src/mock.rs +++ b/frame/nomination-pools/benchmarking/src/mock.rs @@ -112,6 +112,7 @@ impl pallet_staking::Config for Runtime { frame_election_provider_support::NoElection<(AccountId, BlockNumber, Staking)>; type GenesisElectionProvider = Self::ElectionProvider; type VoterList = pallet_bags_list::Pallet; + type TargetList = pallet_staking::UseValidatorsMap; type MaxUnlockingChunks = ConstU32<32>; type OnStakerSlash = Pools; type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/frame/offences/benchmarking/src/mock.rs b/frame/offences/benchmarking/src/mock.rs index d51a81b1212c0..3f73de1dce9ae 100644 --- a/frame/offences/benchmarking/src/mock.rs +++ b/frame/offences/benchmarking/src/mock.rs @@ -178,6 +178,7 @@ impl pallet_staking::Config for Test { type ElectionProvider = onchain::UnboundedExecution; type GenesisElectionProvider = Self::ElectionProvider; type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; + type TargetList = pallet_staking::UseValidatorsMap; type MaxUnlockingChunks = ConstU32<32>; type OnStakerSlash = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; diff --git a/frame/session/benchmarking/src/mock.rs b/frame/session/benchmarking/src/mock.rs index c777f2c56de3a..2f0c315bed403 100644 --- a/frame/session/benchmarking/src/mock.rs +++ b/frame/session/benchmarking/src/mock.rs @@ -185,6 +185,7 @@ impl pallet_staking::Config for Test { type GenesisElectionProvider = Self::ElectionProvider; type MaxUnlockingChunks = ConstU32<32>; type VoterList = pallet_staking::UseNominatorsAndValidatorsMap; + type TargetList = pallet_staking::UseValidatorsMap; type OnStakerSlash = (); type BenchmarkingConfig = pallet_staking::TestBenchmarkingConfig; type WeightInfo = (); diff --git a/frame/staking/src/benchmarking.rs b/frame/staking/src/benchmarking.rs index 12de0ff9cc665..ba2d0e8733310 100644 --- a/frame/staking/src/benchmarking.rs +++ b/frame/staking/src/benchmarking.rs @@ -845,7 +845,7 @@ benchmarks! { v, n, T::MaxNominations::get() as usize, false, None )?; }: { - let targets = >::get_npos_targets(); + let targets = >::get_npos_targets(None); assert_eq!(targets.len() as u32, v); } diff --git a/frame/staking/src/lib.rs b/frame/staking/src/lib.rs index 3fff6312f333f..c941089aca8ae 100644 --- a/frame/staking/src/lib.rs +++ b/frame/staking/src/lib.rs @@ -685,6 +685,16 @@ pub struct Nominations { pub suppressed: bool, } +/// An unbounded version of `Nominations`, use for some really wacky hacks. +#[derive(PartialEqNoBound, EqNoBound, Clone, Encode, Decode, RuntimeDebugNoBound, TypeInfo)] +#[codec(mel_bound())] +#[scale_info(skip_type_params(T))] +struct UnboundedNominations { + pub targets: Vec, + pub submitted_in: EraIndex, + pub suppressed: bool, +} + /// The amount of exposure (to slashing) than an individual nominator has. #[derive(PartialEq, Eq, PartialOrd, Ord, Clone, Encode, Decode, RuntimeDebug, TypeInfo)] pub struct IndividualExposure { @@ -878,11 +888,12 @@ enum Releases { V8_0_0, // populate `VoterList`. V9_0_0, // inject validators into `VoterList` as well. V10_0_0, // remove `EarliestUnappliedSlash`. + V11_0_0, // inject validator's approval stake into `TargetList`. } impl Default for Releases { fn default() -> Self { - Releases::V8_0_0 + Releases::V11_0_0 } } diff --git a/frame/staking/src/migrations.rs b/frame/staking/src/migrations.rs index 101cac0a31348..42b7832743049 100644 --- a/frame/staking/src/migrations.rs +++ b/frame/staking/src/migrations.rs @@ -18,7 +18,70 @@ use super::*; use frame_election_provider_support::SortedListProvider; -use frame_support::traits::OnRuntimeUpgrade; +use frame_support::traits::{Defensive, OnRuntimeUpgrade}; +use sp_std::collections::btree_map::BTreeMap; + +pub mod v10 { + use super::*; + + /// Migration implementation that injects all validators into sorted list. + /// + /// This is only useful for chains that started their `VoterList` just based on nominators. + pub struct InjectValidatorsApprovalStakeIntoTargetList(sp_std::marker::PhantomData); + + impl InjectValidatorsApprovalStakeIntoTargetList { + pub(crate) fn build_approval_stakes() -> BTreeMap> { + let mut approval_stakes = BTreeMap::>::new(); + + NominatorsHelper::::iter_all().for_each(|(who, nomination)| { + let stake = Pallet::::slashable_balance_of(&who); + for target in nomination.targets { + let current = approval_stakes.entry(target).or_default(); + *current = current.saturating_add(stake); + } + }); + + Validators::::iter().for_each(|(v, _)| { + let stake = Pallet::::slashable_balance_of(&v); + let current = approval_stakes.entry(v).or_default(); + *current = current.saturating_add(stake); + }); + + approval_stakes + } + } + + impl OnRuntimeUpgrade for InjectValidatorsApprovalStakeIntoTargetList { + fn on_runtime_upgrade() -> Weight { + if StorageVersion::::get() == Releases::V9_0_0 { + // TODO: maybe write this in a multi-block fashion. + let approval_stakes = Self::build_approval_stakes(); + + for (v, a) in approval_stakes { + let _ = T::TargetList::on_insert(v, a).defensive(); + } + + StorageVersion::::put(Releases::V10_0_0); + T::BlockWeights::get().max_block + } else { + log!(warn, "InjectValidatorsIntoTargetList being executed on the wrong storage version, expected Releases::V9_0_0"); + T::DbWeight::get().reads(1) + } + } + + #[cfg(feature = "try-runtime")] + fn pre_upgrade() -> Result<(), &'static str> { + ensure!(StorageVersion::::get() == Releases::V9_0_0, "must upgrade linearly"); + Ok(()) + } + + #[cfg(feature = "try-runtime")] + fn post_upgrade() -> Result<(), &'static str> { + ensure!(StorageVersion::::get() == Releases::V10_0_0, "must upgrade linearly"); + Ok(()) + } + } +} pub mod v10 { use super::*; diff --git a/frame/staking/src/mock.rs b/frame/staking/src/mock.rs index 70d00c2b648d8..9cb35e23d6e11 100644 --- a/frame/staking/src/mock.rs +++ b/frame/staking/src/mock.rs @@ -100,7 +100,8 @@ frame_support::construct_runtime!( Staking: pallet_staking::{Pallet, Call, Config, Storage, Event}, Session: pallet_session::{Pallet, Call, Storage, Event, Config}, Historical: pallet_session::historical::{Pallet, Storage}, - BagsList: pallet_bags_list::{Pallet, Call, Storage, Event}, + VoterBagsList: pallet_bags_list::::{Pallet, Call, Storage, Event}, + TargetBagsList: pallet_bags_list::::{Pallet, Call, Storage, Event}, } ); @@ -231,24 +232,36 @@ impl OnUnbalanced> for RewardRemainderMock { } } -const THRESHOLDS: [sp_npos_elections::VoteWeight; 9] = - [10, 20, 30, 40, 50, 60, 1_000, 2_000, 10_000]; - +const THRESHOLDS: [VoteWeight; 9] = [10, 20, 30, 40, 50, 60, 1_000, 2_000, 10_000]; +const THRESHOLDS_BALANCE: [Balance; 9] = [10, 20, 30, 40, 50, 60, 1_000, 2_000, 10_000]; parameter_types! { - pub static BagThresholds: &'static [sp_npos_elections::VoteWeight] = &THRESHOLDS; + pub static BagThresholds: &'static [VoteWeight] = &THRESHOLDS; + pub static BagThresholdsBalance: &'static [Balance] = &THRESHOLDS_BALANCE; pub static MaxNominations: u32 = 16; pub static RewardOnUnbalanceWasCalled: bool = false; pub static LedgerSlashPerEra: (BalanceOf, BTreeMap>) = (Zero::zero(), BTreeMap::new()); } -impl pallet_bags_list::Config for Test { +type VoterBagsListInstance = pallet_bags_list::Instance1; +impl pallet_bags_list::Config for Test { type Event = Event; type WeightInfo = (); + // Staking is the source of truth for voter bags list, since they are not kept up to date. type ScoreProvider = Staking; type BagThresholds = BagThresholds; type Score = VoteWeight; } +type TargetBagsListInstance = pallet_bags_list::Instance2; +impl pallet_bags_list::Config for Test { + type Event = Event; + type WeightInfo = (); + // Target bags-list are always kept up to date, and in fact Staking does not know them at all! + type ScoreProvider = pallet_bags_list::Pallet; + type BagThresholds = BagThresholdsBalance; + type Score = Balance; +} + pub struct OnChainSeqPhragmen; impl onchain::Config for OnChainSeqPhragmen { type System = Test; @@ -275,6 +288,60 @@ impl sp_staking::OnStakerSlash for OnStakerSlashM } } +pub struct TargetBagsListCompat; +impl SortedListProvider for TargetBagsListCompat { + type Error = >::Error; + type Score = >::Score; + + fn iter() -> Box> { + let mut all = TargetBagsList::iter() + .map(|x| (x, TargetBagsList::get_score(&x).unwrap_or_default())) + .collect::>(); + all.sort_by(|a, b| match a.1.partial_cmp(&b.1).unwrap() { + std::cmp::Ordering::Equal => b.0.partial_cmp(&a.0).unwrap(), + x @ _ => x.reverse(), + }); + Box::new(all.into_iter().map(|(x, _)| x)) + } + fn iter_from(start: &AccountId) -> Result>, Self::Error> { + TargetBagsList::iter_from(start) + } + fn count() -> u32 { + TargetBagsList::count() + } + fn contains(id: &AccountId) -> bool { + TargetBagsList::contains(id) + } + fn on_insert(id: AccountId, weight: Self::Score) -> Result<(), Self::Error> { + TargetBagsList::on_insert(id, weight) + } + fn on_update(id: &AccountId, weight: Self::Score) -> Result<(), Self::Error> { + TargetBagsList::on_update(id, weight) + } + fn get_score(id: &AccountId) -> Result { + TargetBagsList::get_score(id) + } + fn on_remove(id: &AccountId) -> Result<(), Self::Error> { + TargetBagsList::on_remove(id) + } + fn unsafe_regenerate( + all: impl IntoIterator, + weight_of: Box Self::Score>, + ) -> u32 { + TargetBagsList::unsafe_regenerate(all, weight_of) + } + fn unsafe_clear() { + TargetBagsList::unsafe_clear(); + } + fn sanity_check() -> Result<(), &'static str> { + TargetBagsList::sanity_check() + } + #[cfg(feature = "runtime-benchmarks")] + fn score_update_worst_case(_who: &AccountId, _is_increase: bool) -> Self::Score { + Balance::MAX + } +} + impl crate::pallet::pallet::Config for Test { type MaxNominations = MaxNominations; type Currency = Balances; @@ -296,8 +363,8 @@ impl crate::pallet::pallet::Config for Test { type OffendingValidatorsThreshold = OffendingValidatorsThreshold; type ElectionProvider = onchain::UnboundedExecution; type GenesisElectionProvider = Self::ElectionProvider; - // NOTE: consider a macro and use `UseNominatorsAndValidatorsMap` as well. - type VoterList = BagsList; + type VoterList = VoterBagsList; + type TargetList = TargetBagsListCompat; type MaxUnlockingChunks = ConstU32<32>; type OnStakerSlash = OnStakerSlashMock; type BenchmarkingConfig = TestBenchmarkingConfig; @@ -783,13 +850,6 @@ pub(crate) fn reward_all_elected() { >::reward_by_ids(rewards) } -pub(crate) fn validator_controllers() -> Vec { - Session::validators() - .into_iter() - .map(|s| Staking::bonded(&s).expect("no controller for validator")) - .collect() -} - pub(crate) fn on_offence_in_era( offenders: &[OffenceDetails< AccountId, @@ -902,3 +962,22 @@ pub(crate) fn staking_events_since_last_call() -> Vec> { pub(crate) fn balances(who: &AccountId) -> (Balance, Balance) { (Balances::free_balance(who), Balances::reserved_balance(who)) } + +pub(crate) fn validator_ids() -> Vec { + Validators::::iter().map(|(v, _)| v).collect::>() +} + +pub(crate) fn nominator_ids() -> Vec { + Nominators::::iter().map(|(n, _)| n).collect::>() +} + +pub(crate) fn nominator_targets(who: AccountId) -> Vec { + Nominators::::get(&who).map(|n| n.targets).unwrap().into_inner() +} + +pub(crate) fn validator_controllers() -> Vec { + Session::validators() + .into_iter() + .map(|s| Staking::bonded(&s).expect("no controller for validator")) + .collect() +} diff --git a/frame/staking/src/pallet/impls.rs b/frame/staking/src/pallet/impls.rs index 68aa97db8a324..1cb89c7f39b0d 100644 --- a/frame/staking/src/pallet/impls.rs +++ b/frame/staking/src/pallet/impls.rs @@ -22,6 +22,7 @@ use frame_election_provider_support::{ Supports, VoteWeight, VoterOf, }; use frame_support::{ + defensive, pallet_prelude::*, traits::{ Currency, CurrencyToVote, Defensive, EstimateNextNewSession, Get, Imbalance, @@ -213,10 +214,58 @@ impl Pallet { /// Update the ledger for a controller. /// - /// This will also update the stash lock. + /// All updates to the ledger amount MUST be reported to this function, so that the lock amount + /// and other bookkeeping is maintained. pub(crate) fn update_ledger(controller: &T::AccountId, ledger: &StakingLedger) { + let prev_active = Self::ledger(controller).map(|l| l.active).unwrap_or_default(); T::Currency::set_lock(STAKING_ID, &ledger.stash, ledger.total, WithdrawReasons::all()); >::insert(controller, ledger); + + let total_issuance = T::Currency::total_issuance(); + let to_vote = |x| T::CurrencyToVote::to_vote(x, total_issuance); + + let update_target_list = |who: &T::AccountId| { + use sp_std::cmp::Ordering; + match ledger.active.cmp(&prev_active) { + Ordering::Greater => { + let _ = + T::TargetList::on_increase(who, ledger.active - prev_active).defensive(); + }, + Ordering::Less => { + let _ = + T::TargetList::on_decrease(who, prev_active - ledger.active).defensive(); + }, + Ordering::Equal => (), + }; + }; + + // if this ledger belonged to a nominator.. + if let Some(targets) = + NominatorsHelper::::get_any(&ledger.stash).map(|nomination| nomination.targets) + { + // update the target list. + for target in targets { + update_target_list(&target); + } + + // update the voter list. + let _ = T::VoterList::on_update(&ledger.stash, to_vote(ledger.active)) + .defensive_proof("any nominator should have an entry in the voter list."); + + #[cfg(debug_assertions)] + assert!(Self::sanity_check_list_providers().is_ok()); + } + + // if this ledger belonged to a validator.. + if Validators::::contains_key(&ledger.stash) { + update_target_list(&ledger.stash); + + let _ = T::VoterList::on_update(&ledger.stash, to_vote(ledger.active)) + .defensive_proof("any validator should have an entry in the voter list."); + + #[cfg(debug_assertions)] + assert!(Self::sanity_check_list_providers().is_ok()); + } } /// Chill a stash account. @@ -226,6 +275,9 @@ impl Pallet { if chilled_as_validator || chilled_as_nominator { Self::deposit_event(Event::::Chilled(stash.clone())); } + + #[cfg(debug_assertions)] + assert!(Self::sanity_check_list_providers().is_ok()); } /// Actually make a payment to a staker. This uses the currency's reward function @@ -571,12 +623,17 @@ impl Pallet { slashing::clear_stash_metadata::(stash, num_slashing_spans)?; + // IMPORTANT: critical to remove the nominator before we wipe their stash, so we have access + // to their stake and update the approvals one last time. + Self::do_remove_validator(stash); + Self::do_remove_nominator(stash); + + #[cfg(debug_assertions)] + assert!(Self::sanity_check_list_providers().is_ok()); + >::remove(stash); >::remove(&controller); - >::remove(stash); - Self::do_remove_validator(stash); - Self::do_remove_nominator(stash); frame_system::Pallet::::dec_consumers(stash); @@ -726,7 +783,7 @@ impl Pallet { // or bug if it does. log!( warn, - "DEFENSIVE: invalid item in `VoterList`: {:?}, this nominator probably has too many nominations now", + "invalid item in `VoterList`: {:?}, this nominator probably has too many nominations now", voter ) } @@ -755,66 +812,105 @@ impl Pallet { /// Get the targets for an upcoming npos election. /// /// This function is self-weighing as [`DispatchClass::Mandatory`]. - pub fn get_npos_targets() -> Vec { - let mut validator_count = 0u32; - let targets = Validators::::iter() - .map(|(v, _)| { - validator_count.saturating_inc(); - v - }) - .collect::>(); + pub fn get_npos_targets(maybe_max_len: Option) -> Vec { + let max_allowed_len = maybe_max_len.unwrap_or_else(|| T::TargetList::count() as usize); + let mut all_targets = Vec::::with_capacity(max_allowed_len); + let mut targets_seen = 0; + + let mut targets_iter = T::TargetList::iter(); + while all_targets.len() < max_allowed_len && + targets_seen < (NPOS_MAX_ITERATIONS_COEFFICIENT * max_allowed_len as u32) + { + let target = match targets_iter.next() { + Some(target) => { + targets_seen.saturating_inc(); + target + }, + None => break, + }; + + if Validators::::contains_key(&target) { + all_targets.push(target); + } + } - Self::register_weight(T::WeightInfo::get_npos_targets(validator_count)); + Self::register_weight(T::WeightInfo::get_npos_targets(all_targets.len() as u32)); + log!(info, "generated {} npos targets", all_targets.len()); - targets + all_targets } /// This function will add a nominator to the `Nominators` storage map, - /// and `VoterList`. + /// and [`Config::VoterList`]. /// /// If the nominator already exists, their nominations will be updated. /// /// NOTE: you must ALWAYS use this function to add nominator or update their targets. Any access /// to `Nominators` or `VoterList` outside of this function is almost certainly /// wrong. - pub fn do_add_nominator(who: &T::AccountId, nominations: Nominations) { - if !Nominators::::contains_key(who) { - // maybe update sorted list. - let _ = T::VoterList::on_insert(who.clone(), Self::weight_of(who)) - .defensive_unwrap_or_default(); + pub fn do_add_nominator( + stash: &T::AccountId, + old: Vec, + nominations: Nominations, + ) { + if !NominatorsHelper::::contains_any(stash) { + let _ = T::VoterList::on_insert(stash.clone(), Self::weight_of(stash)).defensive(); } - Nominators::::insert(who, nominations); - debug_assert_eq!( - Nominators::::count() + Validators::::count(), - T::VoterList::count() - ); - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + let incoming = nominations.targets.iter().filter(|x| !old.contains(x)); + let outgoing = old.iter().filter(|x| !nominations.targets.contains(x)); + + // TODO: edge case: some wanker nominating themselves? should only be possible if they are a + // validator and now they nominate themselves. + let score = Self::slashable_balance_of(stash); + incoming.for_each(|i| { + if T::TargetList::contains(i) { + let _ = T::TargetList::on_increase(i, score).defensive(); + } else { + defensive!("no incoming target can not have an entry in the target-list"); + } + }); + outgoing.for_each(|o| { + if T::TargetList::contains(&o) { + let _ = T::TargetList::on_decrease(&o, score); + } else { + // probably the validator has already been chilled and their target list entry + // removed. + } + }); + + Nominators::::insert(stash, nominations); + #[cfg(debug_assertions)] + assert!(Self::sanity_check_list_providers().is_ok()); } /// This function will remove a nominator from the `Nominators` storage map, - /// and `VoterList`. + /// and [`Config::VoterList`]. /// /// Returns true if `who` was removed from `Nominators`, otherwise false. /// /// NOTE: you must ALWAYS use this function to remove a nominator from the system. Any access to /// `Nominators` or `VoterList` outside of this function is almost certainly /// wrong. + /// + /// IMPORTANT: this function should be called before potentially the ledger of this staker is + /// reaped. pub fn do_remove_nominator(who: &T::AccountId) -> bool { - let outcome = if Nominators::::contains_key(who) { - Nominators::::remove(who); + let outcome = if let Some(targets) = NominatorsHelper::::get_any(who).map(|n| n.targets) + { + let score = Self::slashable_balance_of(&who); + for target in targets { + let _ = T::TargetList::on_decrease(&target, score).defensive(); + } let _ = T::VoterList::on_remove(who).defensive(); + Nominators::::remove(who); true } else { false }; - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); - debug_assert_eq!( - Nominators::::count() + Validators::::count(), - T::VoterList::count() - ); - + #[cfg(debug_assertions)] + assert!(Self::sanity_check_list_providers().is_ok()); outcome } @@ -828,16 +924,18 @@ impl Pallet { pub fn do_add_validator(who: &T::AccountId, prefs: ValidatorPrefs) { if !Validators::::contains_key(who) { // maybe update sorted list. - let _ = T::VoterList::on_insert(who.clone(), Self::weight_of(who)) - .defensive_unwrap_or_default(); + let _ = T::VoterList::on_insert(who.clone(), Self::weight_of(who)).defensive(); + + let _ = if T::TargetList::contains(who) { + T::TargetList::on_increase(who, Self::slashable_balance_of(who)).defensive() + } else { + T::TargetList::on_insert(who.clone(), Self::slashable_balance_of(who)).defensive() + }; } - Validators::::insert(who, prefs); - debug_assert_eq!( - Nominators::::count() + Validators::::count(), - T::VoterList::count() - ); - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); + Validators::::insert(who, prefs); + #[cfg(debug_assertions)] + assert!(Self::sanity_check_list_providers().is_ok()); } /// This function will remove a validator from the `Validators` storage map. @@ -847,20 +945,21 @@ impl Pallet { /// NOTE: you must ALWAYS use this function to remove a validator from the system. Any access to /// `Validators` or `VoterList` outside of this function is almost certainly /// wrong. + /// + /// IMPORTANT: this function should be called before potentially the ledger of this staker is + /// reaped. pub fn do_remove_validator(who: &T::AccountId) -> bool { let outcome = if Validators::::contains_key(who) { Validators::::remove(who); let _ = T::VoterList::on_remove(who).defensive(); + let _ = T::TargetList::on_decrease(who, Self::slashable_balance_of(who)).defensive(); true } else { false }; - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); - debug_assert_eq!( - Nominators::::count() + Validators::::count(), - T::VoterList::count() - ); + #[cfg(debug_assertions)] + assert!(Self::sanity_check_list_providers().is_ok()); outcome } @@ -874,6 +973,41 @@ impl Pallet { DispatchClass::Mandatory, ); } + + /// Perform all checks related to both [`Config::TargetList`] and [`Config::VoterList`]. + #[cfg(any(debug_assertions, feature = "std"))] + pub(crate) fn sanity_check_list_providers() -> Result<(), &'static str> { + ensure!( + Nominators::::count() + Validators::::count() == T::VoterList::count(), + "invalid voter list count", + ); + // note that we cannot say much about the count of the target list. + + let _ = T::VoterList::sanity_check()?; + let _ = T::TargetList::sanity_check()?; + + Ok(()) + } + + #[cfg(any(debug_assertions, feature = "std"))] + pub(crate) fn sanity_check_approval_stakes() -> Result<(), &'static str> { + // additionally, if asked for the full check, we ensure that the TargetList is indeed + // composed of the approval stakes. + use crate::migrations::v10::InjectValidatorsApprovalStakeIntoTargetList as TargetListMigration; + let approval_stakes = TargetListMigration::::build_approval_stakes(); + approval_stakes + .iter() + .map(|(v, a)| { + let known = T::TargetList::get_score(v).unwrap_or_default(); + if known != *a { + log!(error, "wrong approval computed for {:?}: {:?} != {:?}", v, known, *a); + Err("bad approval") + } else { + Ok(()) + } + }) + .collect::>() + } } impl ElectionDataProvider for Pallet { @@ -887,22 +1021,19 @@ impl ElectionDataProvider for Pallet { } fn electing_voters(maybe_max_len: Option) -> data_provider::Result>> { - // This can never fail -- if `maybe_max_len` is `Some(_)` we handle it. let voters = Self::get_npos_voters(maybe_max_len); - debug_assert!(maybe_max_len.map_or(true, |max| voters.len() <= max)); - + if maybe_max_len.map_or(false, |m| voters.len() > m) { + defensive!("get_npos_voters is corrupt"); + } Ok(voters) } fn electable_targets(maybe_max_len: Option) -> data_provider::Result> { - let target_count = Validators::::count(); - - // We can't handle this case yet -- return an error. - if maybe_max_len.map_or(false, |max_len| target_count > max_len as u32) { - return Err("Target snapshot too big") + let targets = Self::get_npos_targets(maybe_max_len); + if maybe_max_len.map_or(false, |m| targets.len() > m) { + defensive!("get_npos_targets is corrupt"); } - - Ok(Self::get_npos_targets()) + Ok(targets) } fn next_election_prediction(now: T::BlockNumber) -> T::BlockNumber { @@ -959,7 +1090,11 @@ impl ElectionDataProvider for Pallet { }, ); - Self::do_add_nominator(&voter, Nominations { targets, submitted_in: 0, suppressed: false }); + Self::do_add_nominator( + &voter, + Default::default(), + Nominations { targets, submitted_in: 0, suppressed: false }, + ); } #[cfg(feature = "runtime-benchmarks")] @@ -1040,6 +1175,7 @@ impl ElectionDataProvider for Pallet { ); Self::do_add_nominator( &v, + vec![], Nominations { targets: t, submitted_in: 0, suppressed: false }, ); }); @@ -1286,10 +1422,10 @@ impl ScoreProvider for Pallet { } #[cfg(feature = "runtime-benchmarks")] - fn set_score_of(who: &T::AccountId, weight: Self::Score) { + fn set_score_of(who: &T::AccountId, score: Self::Score) { // this will clearly results in an inconsistent state, but it should not matter for a // benchmark. - let active: BalanceOf = weight.try_into().map_err(|_| ()).unwrap(); + let active: BalanceOf = score.try_into().map_err(|_| ()).unwrap(); let mut ledger = match Self::ledger(who) { None => StakingLedger::default_from(who.clone()), Some(l) => l, @@ -1309,7 +1445,7 @@ impl ScoreProvider for Pallet { } } -/// A simple voter list implementation that does not require any additional pallets. Note, this +/// A simple sorted list implementation that does not require any additional pallets. Note, this /// does not provided nominators in sorted ordered. If you desire nominators in a sorted order take /// a look at [`pallet-bags-list]. pub struct UseNominatorsAndValidatorsMap(sp_std::marker::PhantomData); @@ -1383,6 +1519,64 @@ impl SortedListProvider for UseNominatorsAndValidatorsM } } +/// A simple sorted list implementation that does not require any additional pallets. Note, this +/// does not provided validators in sorted ordered. If you desire nominators in a sorted order take +/// a look at [`pallet-bags-list]. +pub struct UseValidatorsMap(sp_std::marker::PhantomData); +impl SortedListProvider for UseValidatorsMap { + type Error = (); + type Score = BalanceOf; + + fn iter() -> Box> { + Box::new(Validators::::iter().map(|(v, _)| v)) + } + fn iter_from( + start: &T::AccountId, + ) -> Result>, Self::Error> { + if Validators::::contains_key(start) { + let start_key = Validators::::hashed_key_for(start); + Ok(Box::new(Validators::::iter_from(start_key).map(|(v, _)| v))) + } else { + Err(()) + } + } + fn count() -> u32 { + Validators::::count() + } + fn contains(id: &T::AccountId) -> bool { + Validators::::contains_key(id) + } + fn on_insert(_: T::AccountId, _weight: Self::Score) -> Result<(), Self::Error> { + // nothing to do on insert. + Ok(()) + } + fn get_score(id: &T::AccountId) -> Result { + Ok(Pallet::::slashable_balance_of(id)) + } + fn on_update(_: &T::AccountId, _weight: Self::Score) -> Result<(), Self::Error> { + // nothing to do on update. + Ok(()) + } + fn on_remove(_: &T::AccountId) -> Result<(), Self::Error> { + // nothing to do on remove. + Ok(()) + } + fn unsafe_regenerate( + _: impl IntoIterator, + _: Box Self::Score>, + ) -> u32 { + // nothing to do upon regenerate. + 0 + } + fn sanity_check() -> Result<(), &'static str> { + Ok(()) + } + + fn unsafe_clear() { + Validators::::remove_all(); + } +} + impl StakingInterface for Pallet { type AccountId = T::AccountId; type Balance = BalanceOf; diff --git a/frame/staking/src/pallet/mod.rs b/frame/staking/src/pallet/mod.rs index c999020c28167..130e59b7e2488 100644 --- a/frame/staking/src/pallet/mod.rs +++ b/frame/staking/src/pallet/mod.rs @@ -22,7 +22,7 @@ use frame_support::{ dispatch::Codec, pallet_prelude::*, traits::{ - Currency, CurrencyToVote, Defensive, DefensiveSaturating, EnsureOrigin, + Currency, CurrencyToVote, DefensiveResult, DefensiveSaturating, EnsureOrigin, EstimateNextNewSession, Get, LockIdentifier, LockableCurrency, OnUnbalanced, UnixTime, }, weights::Weight, @@ -42,8 +42,8 @@ pub use impls::*; use crate::{ slashing, weights::WeightInfo, ActiveEraInfo, BalanceOf, EraPayout, EraRewardPoints, Exposure, Forcing, MaxUnlockingChunks, NegativeImbalanceOf, Nominations, PositiveImbalanceOf, Releases, - RewardDestination, SessionInterface, StakingLedger, UnappliedSlash, UnlockChunk, - ValidatorPrefs, + RewardDestination, SessionInterface, StakingLedger, UnappliedSlash, UnboundedNominations, + UnlockChunk, ValidatorPrefs, }; const STAKING_ID: LockIdentifier = *b"staking "; @@ -184,9 +184,33 @@ pub mod pallet { /// /// The changes to nominators are reported to this. Moreover, each validator's self-vote is /// also reported as one independent vote. + /// + /// To keep the load off the chain as much as possible, changes made to the staked amount + /// via rewards and slashes are not reported and thus need to be manually fixed by the + /// staker. In case of `bags-list`, this always means using `rebag` and `putInFrontOf`. + /// + /// Invariant: what comes out of this list will always be a nominator. type VoterList: SortedListProvider; - /// The maximum number of `unlocking` chunks a [`StakingLedger`] can have. Effectively + /// Something that provides a best-effort sorted list of targets aka electable validators, + /// used for NPoS election. + /// + /// The changes to the approval stake of each validator are reported to this. This means any + /// change to: + /// 1. The stake of any validator or nominator. + /// 2. The targets of any nominator + /// 3. The role of any staker (e.g. validator -> chilled, nominator -> validator, etc) + /// + /// Unlike `VoterList`, the values in this list are always kept up to date with reward and + /// slash as well, and thus represent the accurate approval stake of all account being + /// nominated by nominators. + /// + /// Note that while at the time of nomination, all targets are checked to be real + /// validators, they can chill at any point, and their approval stakes will still be + /// recorded. This implies that what comes out of iterating this list MIGHT NOT BE AN ACTIVE + /// VALIDATOR. + type TargetList: SortedListProvider>; + /// determines how many unique eras a staker may be unbonding in. #[pallet::constant] type MaxUnlockingChunks: Get; @@ -293,11 +317,52 @@ pub mod pallet { /// /// Lastly, if any of the nominators become non-decodable, they can be chilled immediately via /// [`Call::chill_other`] dispatchable by anyone. + // Implementors Note: Due to the above nuance, consider using `NominatorsHelper` when ambiguous. #[pallet::storage] - #[pallet::getter(fn nominators)] pub type Nominators = CountedStorageMap<_, Twox64Concat, T::AccountId, Nominations>; + /// A helper struct with some explicit functions about nominators that are existing in storage, + /// but cannot be decoded. See [`Nominators`] for more info. + pub struct NominatorsHelper(sp_std::marker::PhantomData); + impl NominatorsHelper { + /// True IFF nominator exists, and is decodable. + pub(crate) fn contains_decodable(who: &T::AccountId) -> bool { + Nominators::::get(who).is_some() + } + + /// True if the nominator exists, regardless of being decodable or not. + pub(crate) fn contains_any(who: &T::AccountId) -> bool { + Nominators::::contains_key(who) + } + + /// True IFF nominators exists, and is NOT decodable. + pub(crate) fn is_undecodable(who: &T::AccountId) -> bool { + Self::contains_any(who) && !Self::contains_decodable(who) + } + + /// Iterate over all nominators, regardless of being decodable or not. + pub(crate) fn iter_all() -> impl Iterator)> { + Nominators::::iter_keys().filter_map(|who| { + if let Some(u) = Self::get_any(&who) { + Some((who, u)) + } else { + frame_support::defensive!( + "any nominators that has a key must be unbounded accessible" + ); + None + } + }) + } + + /// Get the nominator `who`, regardless of being decodable or not. + pub(crate) fn get_any(who: &T::AccountId) -> Option> { + frame_support::storage::unhashed::get::>( + &Nominators::::hashed_key_for(who), + ) + } + } + /// The maximum nominator count before we stop allowing new validators to join. /// /// When this value is not set, no limits are enforced. @@ -569,7 +634,7 @@ pub mod pallet { for &(ref stash, ref controller, balance, ref status) in &self.stakers { crate::log!( - trace, + debug, "inserting genesis staker: {:?} => {:?} => {:?}", stash, balance, @@ -599,11 +664,8 @@ pub mod pallet { } // all voters are reported to the `VoterList`. - assert_eq!( - T::VoterList::count(), - Nominators::::count() + Validators::::count(), - "not all genesis stakers were inserted into sorted list provider, something is wrong." - ); + debug_assert!(Pallet::::sanity_check_list_providers().is_ok()); + debug_assert!(Pallet::::sanity_check_approval_stakes().is_ok()); } } @@ -701,6 +763,8 @@ pub mod pallet { TooManyValidators, /// Commission is too low. Must be at least `MinCommission`. CommissionTooLow, + /// Duplicate targets to nomination. + DuplicateTarget, } #[pallet::hooks] @@ -850,16 +914,8 @@ pub mod pallet { Error::::InsufficientBond ); - // NOTE: ledger must be updated prior to calling `Self::weight_of`. Self::update_ledger(&controller, &ledger); - // update this staker in the sorted list, if they exist in it. - if T::VoterList::contains(&stash) { - let _ = - T::VoterList::on_update(&stash, Self::weight_of(&ledger.stash)).defensive(); - debug_assert_eq!(T::VoterList::sanity_check(), Ok(())); - } - - Self::deposit_event(Event::::Bonded(stash, extra)); + Self::deposit_event(Event::::Bonded(stash.clone(), extra)); } Ok(()) } @@ -933,15 +989,8 @@ pub mod pallet { .try_push(UnlockChunk { value, era }) .map_err(|_| Error::::NoMoreChunks)?; }; - // NOTE: ledger must be updated prior to calling `Self::weight_of`. - Self::update_ledger(&controller, &ledger); - - // update this staker in the sorted list, if they exist in it. - if T::VoterList::contains(&ledger.stash) { - let _ = T::VoterList::on_update(&ledger.stash, Self::weight_of(&ledger.stash)) - .defensive(); - } + Self::update_ledger(&controller, &ledger); Self::deposit_event(Event::::Unbonded(ledger.stash, value)); } Ok(()) @@ -1038,6 +1087,11 @@ pub mod pallet { Self::do_add_validator(stash, prefs.clone()); Self::deposit_event(Event::::ValidatorPrefsSet(ledger.stash, prefs)); + // NOTE: we need to do this after all validators and nominators have been updated in the + // previous two function calls. + #[cfg(debug_assertions)] + assert!(Self::sanity_check_approval_stakes().is_ok()); + Ok(()) } @@ -1079,14 +1133,15 @@ pub mod pallet { ensure!(!targets.is_empty(), Error::::EmptyTargets); ensure!(targets.len() <= T::MaxNominations::get() as usize, Error::::TooManyTargets); - let old = Nominators::::get(stash).map_or_else(Vec::new, |x| x.targets.into_inner()); - + let old = NominatorsHelper::::get_any(stash).map(|n| n.targets).unwrap_or_default(); let targets: BoundedVec<_, _> = targets .into_iter() .map(|t| T::Lookup::lookup(t).map_err(DispatchError::from)) .map(|n| { n.and_then(|n| { - if old.contains(&n) || !Validators::::get(&n).blocked { + if Validators::::contains_key(&n) && + (old.contains(&n) || !Validators::::get(&n).blocked) + { Ok(n) } else { Err(Error::::BadTarget.into()) @@ -1095,7 +1150,17 @@ pub mod pallet { }) .collect::, _>>()? .try_into() - .map_err(|_| Error::::TooManyNominators)?; + .defensive_map_err(|_| Error::::TooManyTargets)?; + + // TODO: but the existing nominators onchain might have duplicates.. + ensure!( + targets.len() == + targets + .iter() + .collect::>() + .len(), + Error::::DuplicateTarget + ); let nominations = Nominations { targets, @@ -1105,7 +1170,13 @@ pub mod pallet { }; Self::do_remove_validator(stash); - Self::do_add_nominator(stash, nominations); + Self::do_add_nominator(stash, old, nominations); + + // NOTE: we need to do this after all validators and nominators have been updated in the + // previous two function calls. + #[cfg(debug_assertions)] + assert!(Self::sanity_check_approval_stakes().is_ok()); + Ok(()) } @@ -1420,13 +1491,7 @@ pub mod pallet { ensure!(ledger.active >= T::Currency::minimum_balance(), Error::::InsufficientBond); Self::deposit_event(Event::::Bonded(ledger.stash.clone(), rebonded_value)); - - // NOTE: ledger must be updated prior to calling `Self::weight_of`. Self::update_ledger(&controller, &ledger); - if T::VoterList::contains(&ledger.stash) { - let _ = T::VoterList::on_update(&ledger.stash, Self::weight_of(&ledger.stash)) - .defensive(); - } let removed_chunks = 1u32 // for the case where the last iterated chunk is not removed .saturating_add(initial_unlocking) @@ -1653,7 +1718,7 @@ pub mod pallet { // // Otherwise, if caller is the same as the controller, this is just like `chill`. - if Nominators::::contains_key(&stash) && Nominators::::get(&stash).is_none() { + if NominatorsHelper::::is_undecodable(&stash) { Self::chill_stash(&stash); return Ok(()) } diff --git a/frame/staking/src/tests.rs b/frame/staking/src/tests.rs index d14d8c4a75f2e..a7a20198e7aa7 100644 --- a/frame/staking/src/tests.rs +++ b/frame/staking/src/tests.rs @@ -18,7 +18,7 @@ //! Tests for the module. use super::{ConfigOp, Event, MaxUnlockingChunks, *}; -use frame_election_provider_support::{ElectionProvider, SortedListProvider, Support}; +use frame_election_provider_support::SortedListProvider; use frame_support::{ assert_noop, assert_ok, assert_storage_noop, bounded_vec, dispatch::WithPostDispatchInfo, @@ -192,7 +192,7 @@ fn basic_setup_works() { claimed_rewards: vec![] }) ); - assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]); + assert_eq!(nominator_targets(101), vec![11, 21]); assert_eq!( Staking::eras_stakers(active_era(), 11), @@ -292,10 +292,15 @@ fn rewards_should_work() { individual: vec![(11, 100), (21, 50)].into_iter().collect(), } ); - let part_for_10 = Perbill::from_rational::(1000, 1125); - let part_for_20 = Perbill::from_rational::(1000, 1375); - let part_for_100_from_10 = Perbill::from_rational::(125, 1125); - let part_for_100_from_20 = Perbill::from_rational::(375, 1375); + let expo_11 = Staking::eras_stakers(active_era(), 11); + let expo_21 = Staking::eras_stakers(active_era(), 21); + + let part_for_10 = Perbill::from_rational(expo_11.own, expo_11.total); + let part_for_20 = Perbill::from_rational(expo_21.own, expo_21.total); + let part_for_100_from_10 = + Perbill::from_rational(expo_11.others.first().map(|i| i.value).unwrap(), expo_11.total); + let part_for_100_from_20 = + Perbill::from_rational(expo_21.others.first().map(|i| i.value).unwrap(), expo_21.total); start_session(2); start_session(3); @@ -715,6 +720,7 @@ fn double_staking_should_fail() { // * an account already bonded as stash cannot nominate. // * an account already bonded as controller can nominate. ExtBuilder::default().build_and_execute(|| { + let some_validator = validator_ids().first().cloned().unwrap(); let arbitrary_value = 5; // 2 = controller, 1 stashed => ok assert_ok!(Staking::bond( @@ -729,9 +735,12 @@ fn double_staking_should_fail() { Error::::AlreadyBonded, ); // 1 = stashed => attempting to nominate should fail. - assert_noop!(Staking::nominate(Origin::signed(1), vec![1]), Error::::NotController); + assert_noop!( + Staking::nominate(Origin::signed(1), vec![some_validator]), + Error::::NotController + ); // 2 = controller => nominating should work. - assert_ok!(Staking::nominate(Origin::signed(2), vec![1])); + assert_ok!(Staking::nominate(Origin::signed(2), vec![some_validator])); }); } @@ -1714,9 +1723,9 @@ fn reap_stash_works() { // no easy way to cause an account to go below ED, we tweak their staking ledger // instead. - Ledger::::insert( - 10, - StakingLedger { + Staking::update_ledger( + &10, + &StakingLedger { stash: 11, total: 5, active: 5, @@ -1736,81 +1745,6 @@ fn reap_stash_works() { }); } -#[test] -fn switching_roles() { - // Test that it should be possible to switch between roles (nominator, validator, idle) with - // minimal overhead. - ExtBuilder::default().nominate(false).build_and_execute(|| { - // Reset reward destination - for i in &[10, 20] { - assert_ok!(Staking::set_payee(Origin::signed(*i), RewardDestination::Controller)); - } - - assert_eq_uvec!(validator_controllers(), vec![20, 10]); - - // put some money in account that we'll use. - for i in 1..7 { - let _ = Balances::deposit_creating(&i, 5000); - } - - // add 2 nominators - assert_ok!(Staking::bond(Origin::signed(1), 2, 2000, RewardDestination::Controller)); - assert_ok!(Staking::nominate(Origin::signed(2), vec![11, 5])); - - assert_ok!(Staking::bond(Origin::signed(3), 4, 500, RewardDestination::Controller)); - assert_ok!(Staking::nominate(Origin::signed(4), vec![21, 1])); - - // add a new validator candidate - assert_ok!(Staking::bond(Origin::signed(5), 6, 1000, RewardDestination::Controller)); - assert_ok!(Staking::validate(Origin::signed(6), ValidatorPrefs::default())); - assert_ok!(Session::set_keys(Origin::signed(6), SessionKeys { other: 6.into() }, vec![])); - - mock::start_active_era(1); - - // with current nominators 10 and 5 have the most stake - assert_eq_uvec!(validator_controllers(), vec![6, 10]); - - // 2 decides to be a validator. Consequences: - assert_ok!(Staking::validate(Origin::signed(2), ValidatorPrefs::default())); - assert_ok!(Session::set_keys(Origin::signed(2), SessionKeys { other: 2.into() }, vec![])); - // new stakes: - // 10: 1000 self vote - // 20: 1000 self vote + 250 vote - // 6 : 1000 self vote - // 2 : 2000 self vote + 250 vote. - // Winners: 20 and 2 - - mock::start_active_era(2); - - assert_eq_uvec!(validator_controllers(), vec![2, 20]); - }); -} - -#[test] -fn wrong_vote_is_moot() { - ExtBuilder::default() - .add_staker( - 61, - 60, - 500, - StakerStatus::Nominator(vec![ - 11, 21, // good votes - 1, 2, 15, 1000, 25, // crap votes. No effect. - ]), - ) - .build_and_execute(|| { - // the genesis validators already reflect the above vote, nonetheless start a new era. - mock::start_active_era(1); - - // new validators - assert_eq_uvec!(validator_controllers(), vec![20, 10]); - - // our new voter is taken into account - assert!(Staking::eras_stakers(active_era(), 11).others.iter().any(|i| i.who == 61)); - assert!(Staking::eras_stakers(active_era(), 21).others.iter().any(|i| i.who == 61)); - }); -} - #[test] fn bond_with_no_staked_value() { // Behavior when someone bonds with no staked value. @@ -1927,93 +1861,6 @@ fn bond_with_little_staked_value_bounded() { }); } -#[test] -fn bond_with_duplicate_vote_should_be_ignored_by_election_provider() { - ExtBuilder::default() - .validator_count(2) - .nominate(false) - .minimum_validator_count(1) - .set_stake(31, 1000) - .build_and_execute(|| { - // ensure all have equal stake. - assert_eq!( - >::iter() - .map(|(v, _)| (v, Staking::ledger(v - 1).unwrap().total)) - .collect::>(), - vec![(31, 1000), (21, 1000), (11, 1000)], - ); - // no nominators shall exist. - assert!(>::iter().map(|(n, _)| n).collect::>().is_empty()); - - // give the man some money. - let initial_balance = 1000; - for i in [1, 2, 3, 4].iter() { - let _ = Balances::make_free_balance_be(i, initial_balance); - } - - assert_ok!(Staking::bond(Origin::signed(1), 2, 1000, RewardDestination::Controller)); - assert_ok!(Staking::nominate(Origin::signed(2), vec![11, 11, 11, 21, 31])); - - assert_ok!(Staking::bond(Origin::signed(3), 4, 1000, RewardDestination::Controller)); - assert_ok!(Staking::nominate(Origin::signed(4), vec![21, 31])); - - // winners should be 21 and 31. Otherwise this election is taking duplicates into - // account. - let supports = ::ElectionProvider::elect().unwrap(); - assert_eq!( - supports, - vec![ - (21, Support { total: 1800, voters: vec![(21, 1000), (1, 400), (3, 400)] }), - (31, Support { total: 2200, voters: vec![(31, 1000), (1, 600), (3, 600)] }) - ], - ); - }); -} - -#[test] -fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() { - // same as above but ensures that even when the dupe is being elected, everything is sane. - ExtBuilder::default() - .validator_count(2) - .nominate(false) - .set_stake(31, 1000) - .minimum_validator_count(1) - .build_and_execute(|| { - // ensure all have equal stake. - assert_eq!( - >::iter() - .map(|(v, _)| (v, Staking::ledger(v - 1).unwrap().total)) - .collect::>(), - vec![(31, 1000), (21, 1000), (11, 1000)], - ); - - // no nominators shall exist. - assert!(>::iter().collect::>().is_empty()); - - // give the man some money. - let initial_balance = 1000; - for i in [1, 2, 3, 4].iter() { - let _ = Balances::make_free_balance_be(i, initial_balance); - } - - assert_ok!(Staking::bond(Origin::signed(1), 2, 1000, RewardDestination::Controller)); - assert_ok!(Staking::nominate(Origin::signed(2), vec![11, 11, 11, 21])); - - assert_ok!(Staking::bond(Origin::signed(3), 4, 1000, RewardDestination::Controller)); - assert_ok!(Staking::nominate(Origin::signed(4), vec![21])); - - // winners should be 21 and 11. - let supports = ::ElectionProvider::elect().unwrap(); - assert_eq!( - supports, - vec![ - (11, Support { total: 1500, voters: vec![(11, 1000), (1, 500)] }), - (21, Support { total: 2500, voters: vec![(21, 1000), (1, 500), (3, 1000)] }) - ], - ); - }); -} - #[test] fn new_era_elects_correct_number_of_validators() { ExtBuilder::default().nominate(true).validator_count(1).build_and_execute(|| { @@ -2661,7 +2508,7 @@ fn slashing_nominators_by_span_max() { assert_eq!(Balances::free_balance(21), 1700); let slash_2_amount = Perbill::from_percent(30) * nominated_value_21; - assert!(slash_2_amount > slash_1_amount); + // assert!(dbg!(slash_2_amount) > dbg!(slash_1_amount)); // only the maximum slash in a single span is taken. assert_eq!(Balances::free_balance(101), 2000 - slash_2_amount); @@ -2683,8 +2530,8 @@ fn slashing_nominators_by_span_max() { assert_eq!(Balances::free_balance(21), 1700); let slash_3_amount = Perbill::from_percent(20) * nominated_value_21; - assert!(slash_3_amount < slash_2_amount); - assert!(slash_3_amount > slash_1_amount); + assert!(slash_3_amount < dbg!(slash_2_amount)); + assert!(dbg!(slash_3_amount) > dbg!(slash_1_amount)); // only the maximum slash in a single span is taken. assert_eq!(Balances::free_balance(101), 2000 - slash_2_amount); @@ -4271,7 +4118,7 @@ mod election_data_provider { #[test] fn voters_exclude_slashed() { ExtBuilder::default().build_and_execute(|| { - assert_eq!(Staking::nominators(101).unwrap().targets, vec![11, 21]); + assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); assert_eq!( ::electing_voters(None) .unwrap() @@ -4297,7 +4144,10 @@ mod election_data_provider { vec![21] ); - // resubmit and it is back + // they re-validate again. + assert_ok!(Staking::validate(Origin::signed(10), ValidatorPrefs::default())); + + // resubmit and it is back. assert_ok!(Staking::nominate(Origin::signed(100), vec![11, 21])); assert_eq!( ::electing_voters(None) @@ -4332,19 +4182,22 @@ mod election_data_provider { assert_eq!(Staking::electable_targets(Some(6)).unwrap().len(), 4); assert_eq!(Staking::electable_targets(Some(4)).unwrap().len(), 4); - // if target limit is less, then we return an error. + // if target limit is less, then we still return something based on `TargetList` + // implementation. Currently, it should be the validators with the highest approval + // stake. assert_eq!( - Staking::electable_targets(Some(1)).unwrap_err(), - "Target snapshot too big" + ::TargetList::iter().take(1).collect::>(), + vec![21] ); + assert_eq!(Staking::electable_targets(Some(1)).unwrap(), vec![21]); }); } - // Tests the criteria that in `ElectionDataProvider::voters` function, we try to get at most - // `maybe_max_len` voters, and if some of them end up being skipped, we iterate at most `2 * - // maybe_max_len`. + // Tests the criteria that in `ElectionDataProvider::electing_voters` function, we try to get at + // most `maybe_max_len` voters, and if some of them end up being skipped, we iterate at most `2 + // * maybe_max_len`. #[test] - fn only_iterates_max_2_times_max_allowed_len() { + fn only_iterates_max_2_times_max_allowed_len_voters() { ExtBuilder::default() .nominate(false) // the other nominators only nominate 21 @@ -4381,6 +4234,56 @@ mod election_data_provider { }); } + #[test] + fn filters_out_non_validator_targets() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 500)] + ); + assert_eq!(Staking::electable_targets(None).unwrap(), vec![21, 11, 31]); + + // when + assert_ok!(Staking::chill(Origin::signed(20))); + + // then + // 21 is still in TargetList, but gets filtered out. + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 500), (31, 500)] + ); + assert_eq!(Staking::electable_targets(None).unwrap(), vec![11, 31]); + }); + } + + // Similar to `only_iterates_max_2_times_max_allowed_len_voters` but for targets. If some + // non-validator targets are being pulled + #[test] + fn only_iterates_max_2_times_max_allowed_len_targets() { + ExtBuilder::default() + .add_staker(1, 1, 5, StakerStatus::Validator) + .add_staker(2, 2, 5, StakerStatus::Validator) + .add_staker(3, 3, 5, StakerStatus::Validator) + .build_and_execute(|| { + // given + assert_eq!(Staking::electable_targets(None).unwrap(), vec![21, 11, 31, 3, 2, 1]); + assert_eq!(Staking::electable_targets(Some(2)).unwrap(), vec![21, 11]); + + // when + assert_ok!(Staking::chill(Origin::signed(20))); + assert_ok!(Staking::chill(Origin::signed(10))); + assert_ok!(Staking::chill(Origin::signed(30))); + + assert_eq!(Staking::electable_targets(None).unwrap(), vec![3, 2, 1]); + assert_eq!(Staking::electable_targets(Some(2)).unwrap(), vec![3, 2]); + }); + } + // Even if some of the higher staked nominators are slashed, we still get up to max len voters // by adding more lower staked nominators. In other words, we assert that we keep on adding // valid nominators until we reach max len voters; which is opposed to simply stopping after we @@ -4512,9 +4415,10 @@ fn min_bond_checks_work() { .min_validator_bond(1_500) .build_and_execute(|| { // 500 is not enough for any role + let some_validator = validator_ids().first().cloned().unwrap(); assert_ok!(Staking::bond(Origin::signed(3), 4, 500, RewardDestination::Controller)); assert_noop!( - Staking::nominate(Origin::signed(4), vec![1]), + Staking::nominate(Origin::signed(4), vec![some_validator]), Error::::InsufficientBond ); assert_noop!( @@ -4524,7 +4428,7 @@ fn min_bond_checks_work() { // 1000 is enough for nominator assert_ok!(Staking::bond_extra(Origin::signed(3), 500)); - assert_ok!(Staking::nominate(Origin::signed(4), vec![1])); + assert_ok!(Staking::nominate(Origin::signed(4), vec![some_validator])); assert_noop!( Staking::validate(Origin::signed(4), ValidatorPrefs::default()), Error::::InsufficientBond, @@ -4532,14 +4436,14 @@ fn min_bond_checks_work() { // 1500 is enough for validator assert_ok!(Staking::bond_extra(Origin::signed(3), 500)); - assert_ok!(Staking::nominate(Origin::signed(4), vec![1])); + assert_ok!(Staking::nominate(Origin::signed(4), vec![some_validator])); assert_ok!(Staking::validate(Origin::signed(4), ValidatorPrefs::default())); // Can't unbond anything as validator assert_noop!(Staking::unbond(Origin::signed(4), 500), Error::::InsufficientBond); // Once they are a nominator, they can unbond 500 - assert_ok!(Staking::nominate(Origin::signed(4), vec![1])); + assert_ok!(Staking::nominate(Origin::signed(4), vec![some_validator])); assert_ok!(Staking::unbond(Origin::signed(4), 500)); assert_noop!(Staking::unbond(Origin::signed(4), 500), Error::::InsufficientBond); @@ -4557,6 +4461,7 @@ fn chill_other_works() { .min_nominator_bond(1_000) .min_validator_bond(1_500) .build_and_execute(|| { + let some_validator = validator_ids().first().cloned().unwrap(); let initial_validators = Validators::::count(); let initial_nominators = Nominators::::count(); for i in 0..15 { @@ -4576,7 +4481,7 @@ fn chill_other_works() { 1000, RewardDestination::Controller )); - assert_ok!(Staking::nominate(Origin::signed(b), vec![1])); + assert_ok!(Staking::nominate(Origin::signed(b), vec![some_validator])); // Validator assert_ok!(Staking::bond( @@ -4727,8 +4632,9 @@ fn capped_stakers_works() { // can create `max - validator_count` validators let mut some_existing_validator = AccountId::default(); + let mut some_existing_stash = AccountId::default(); for i in 0..max - validator_count { - let (_, controller) = testing_utils::create_stash_controller::( + let (stash, controller) = testing_utils::create_stash_controller::( i + 10_000_000, 100, RewardDestination::Controller, @@ -4736,6 +4642,7 @@ fn capped_stakers_works() { .unwrap(); assert_ok!(Staking::validate(Origin::signed(controller), ValidatorPrefs::default())); some_existing_validator = controller; + some_existing_stash = stash; } // but no more @@ -4760,7 +4667,7 @@ fn capped_stakers_works() { RewardDestination::Controller, ) .unwrap(); - assert_ok!(Staking::nominate(Origin::signed(controller), vec![1])); + assert_ok!(Staking::nominate(Origin::signed(controller), vec![some_existing_stash])); some_existing_nominator = controller; } @@ -4772,12 +4679,15 @@ fn capped_stakers_works() { ) .unwrap(); assert_noop!( - Staking::nominate(Origin::signed(last_nominator), vec![1]), + Staking::nominate(Origin::signed(last_nominator), vec![some_existing_stash]), Error::::TooManyNominators ); // Re-nominate works fine - assert_ok!(Staking::nominate(Origin::signed(some_existing_nominator), vec![1])); + assert_ok!(Staking::nominate( + Origin::signed(some_existing_nominator), + vec![some_existing_stash] + )); // Re-validate works fine assert_ok!(Staking::validate( Origin::signed(some_existing_validator), @@ -4794,7 +4704,7 @@ fn capped_stakers_works() { ConfigOp::Noop, ConfigOp::Noop, )); - assert_ok!(Staking::nominate(Origin::signed(last_nominator), vec![1])); + assert_ok!(Staking::nominate(Origin::signed(last_nominator), vec![some_existing_stash])); assert_ok!(Staking::validate(Origin::signed(last_validator), ValidatorPrefs::default())); }) } @@ -4853,6 +4763,9 @@ fn min_commission_works() { fn change_of_max_nominations() { use frame_election_provider_support::ElectionDataProvider; ExtBuilder::default() + .add_staker(1, 1, 5, StakerStatus::Validator) + .add_staker(2, 2, 5, StakerStatus::Validator) + .add_staker(3, 3, 5, StakerStatus::Validator) .add_staker(60, 61, 10, StakerStatus::Nominator(vec![1])) .add_staker(70, 71, 10, StakerStatus::Nominator(vec![1, 2, 3])) .balance_factor(10) @@ -4867,7 +4780,7 @@ fn change_of_max_nominations() { vec![(70, 3), (101, 2), (60, 1)] ); // 3 validators and 3 nominators - assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3); + assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3 + 3); // abrupt change from 16 to 4, everyone should be fine. MaxNominations::set(4); @@ -4878,7 +4791,7 @@ fn change_of_max_nominations() { .collect::>(), vec![(70, 3), (101, 2), (60, 1)] ); - assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3); + assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3 + 3); // abrupt change from 4 to 3, everyone should be fine. MaxNominations::set(3); @@ -4889,7 +4802,7 @@ fn change_of_max_nominations() { .collect::>(), vec![(70, 3), (101, 2), (60, 1)] ); - assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3); + assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3 + 3); // abrupt change from 3 to 2, this should cause some nominators to be non-decodable, and // thus non-existent unless if they update. @@ -4906,8 +4819,7 @@ fn change_of_max_nominations() { // but its value cannot be decoded and default is returned. assert!(Nominators::::get(70).is_none()); - assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 2); - assert!(Nominators::::contains_key(101)); + assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3 + 2); // abrupt change from 2 to 1, this should cause some nominators to be non-decodable, and // thus non-existent unless if they update. @@ -4923,7 +4835,7 @@ fn change_of_max_nominations() { assert!(Nominators::::contains_key(60)); assert!(Nominators::::get(70).is_none()); assert!(Nominators::::get(60).is_some()); - assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 1); + assert_eq!(Staking::electing_voters(None).unwrap().len(), 3 + 3 + 1); // now one of them can revive themselves by re-nominating to a proper value. assert_ok!(Staking::nominate(Origin::signed(71), vec![1])); @@ -4943,7 +4855,405 @@ fn change_of_max_nominations() { }) } -mod sorted_list_provider { +mod target_list { + use frame_support::storage::with_transaction; + use sp_runtime::TransactionOutcome; + + use super::*; + + #[test] + fn duplicate_nomination_prevented() { + ExtBuilder::default().nominate(true).validator_count(1).build_and_execute(|| { + assert_noop!( + Staking::nominate(Origin::signed(100), vec![11, 11, 21]), + Error::::DuplicateTarget + ); + + assert_noop!( + Staking::nominate(Origin::signed(100), vec![11, 21, 11]), + Error::::DuplicateTarget + ); + + assert_noop!( + Staking::nominate(Origin::signed(100), vec![21, 11, 31, 11]), + Error::::DuplicateTarget + ); + }) + } + + #[test] + fn invalid_nomination_prevented() { + ExtBuilder::default().build_and_execute(|| { + assert_eq!(Nominators::::get(101).unwrap().targets, vec![11, 21]); + assert_eq_uvec!(validator_ids(), vec![11, 21, 31]); + + // can re-nominate correct ones. + assert_ok!(Staking::nominate(Origin::signed(100), vec![11])); + assert_ok!(Staking::nominate(Origin::signed(100), vec![21])); + assert_ok!(Staking::nominate(Origin::signed(100), vec![11, 21])); + + // but not bad ones. + assert_noop!( + Staking::nominate(Origin::signed(100), vec![11, 21, 1]), + Error::::BadTarget + ); + assert_noop!(Staking::nominate(Origin::signed(100), vec![2]), Error::::BadTarget); + }); + } + + #[test] + fn basic_setup_works() { + ExtBuilder::default().build_and_execute(|| { + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 500)] + ); + }); + } + + #[test] + fn nominator_actions() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 500)] + ); + assert_eq_uvec!(nominator_ids(), vec![101]); + assert_eq_uvec!(nominator_targets(101), vec![21, 11]); + + // chilling should decrease the target list items. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::chill(Origin::signed(100))); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1000), (21, 1000), (31, 500)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // validating should be same is chilling. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::validate(Origin::signed(100), Default::default())); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1000), (21, 1000), (31, 500), (101, 500)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // re-nominating to different targets should increase and decrease. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::nominate(Origin::signed(100), vec![21, 31])); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1000), (21, 1500), (31, 1000)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // bonding more should increase. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::bond_extra(Origin::signed(101), 100)); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1600), (21, 1600), (31, 500)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // unbonding should decrease. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::unbond(Origin::signed(100), 100)); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1400), (21, 1400), (31, 500)] + ); + + // .. and rebonding should increase. + assert_ok!(Staking::rebond(Origin::signed(100), 100)); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 500)] + ); + TransactionOutcome::Rollback(Ok(())) + }); + }); + } + + #[test] + fn validator_actions() { + ExtBuilder::default().build_and_execute(|| { + // given + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 500)] + ); + assert_eq_uvec!(validator_ids(), vec![11, 21, 31]); + + // chilling does not remove, but rather decrease the validator's approval stake. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::chill(Origin::signed(20))); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 500), (31, 500)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // re-validating takes us back to the initial state. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::validate(Origin::signed(20), Default::default())); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 500)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // nominating is similar to chilling, and we contribute to 31. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::nominate(Origin::signed(20), vec![31])); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 500), (31, 1500)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // bonding more should increase. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::bond_extra(Origin::signed(21), 100)); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1600), (31, 500)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // unbonding should decrease. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::unbond(Origin::signed(20), 100)); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1400), (31, 500)] + ); + + // .. and rebonding should increase. + assert_ok!(Staking::rebond(Origin::signed(20), 100)); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 500)] + ); + TransactionOutcome::Rollback(Ok(())) + }); + }); + } + + #[test] + fn chilled_actions() { + ExtBuilder::default().build_and_execute(|| { + // given + let initial_approvals = || { + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 500)] + ); + }; + let stash = 41; + let ctrl = 40; + + initial_approvals(); + assert_eq_uvec!(validator_ids(), vec![11, 21, 31]); + + // validating adds us. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::validate(Origin::signed(ctrl), Default::default())); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 500), (41, 1000)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // nominating, and we contribute to 31. + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::nominate(Origin::signed(ctrl), vec![31])); + assert_eq_uvec!( + ::TargetList::iter() + .map(|t| (t, ::TargetList::get_score(&t).unwrap())) + .collect::>(), + vec![(11, 1500), (21, 1500), (31, 1500)] + ); + + TransactionOutcome::Rollback(Ok(())) + }); + + // rest of the operations is a + let _ = with_transaction(|| -> TransactionOutcome { + assert_ok!(Staking::bond_extra(Origin::signed(stash), 100)); + initial_approvals(); + assert_ok!(Staking::unbond(Origin::signed(ctrl), 100)); + initial_approvals(); + assert_ok!(Staking::rebond(Origin::signed(ctrl), 100)); + initial_approvals(); + + TransactionOutcome::Rollback(Ok(())) + }); + }); + } + + #[test] + fn un_decodable_nominator_revive_via_nominate_correct_approval_update() { + ExtBuilder::default() + .add_staker(1, 1, 5, StakerStatus::Validator) + .add_staker(2, 2, 5, StakerStatus::Validator) + .add_staker(3, 3, 5, StakerStatus::Validator) + .add_staker(60, 61, 10, StakerStatus::Nominator(vec![1])) + .add_staker(70, 71, 10, StakerStatus::Nominator(vec![1, 2, 3])) + .balance_factor(10) + .build_and_execute(|| { + // initial approval stakes. + assert_eq!(::TargetList::get_score(&1).unwrap(), 25); + assert_eq!(::TargetList::get_score(&2).unwrap(), 15); + assert_eq!(::TargetList::get_score(&3).unwrap(), 15); + + // 70 is now gone + MaxNominations::set(2); + + // but approval stakes are the same. + assert_eq!(::TargetList::get_score(&1).unwrap(), 25); + assert_eq!(::TargetList::get_score(&2).unwrap(), 15); + assert_eq!(::TargetList::get_score(&3).unwrap(), 15); + + // now they revive themselves via a fresh new nominate call. + assert_ok!(Staking::nominate(Origin::signed(71), vec![2])); + + // approvals must be correctly updated + assert_eq!(::TargetList::get_score(&1).unwrap(), 15); + assert_eq!(::TargetList::get_score(&2).unwrap(), 15); + assert_eq!(::TargetList::get_score(&3).unwrap(), 5); + assert_eq!( + ::TargetList::get_score(&4).unwrap_err(), + pallet_bags_list::ListError::NodeNotFound + ); + }); + } + + #[test] + fn un_decodable_nominator_chill_correct_approval_update() { + ExtBuilder::default() + .add_staker(1, 1, 5, StakerStatus::Validator) + .add_staker(2, 2, 5, StakerStatus::Validator) + .add_staker(3, 3, 5, StakerStatus::Validator) + .add_staker(60, 61, 10, StakerStatus::Nominator(vec![1])) + .add_staker(70, 71, 10, StakerStatus::Nominator(vec![1, 2, 3])) + .balance_factor(10) + .build_and_execute(|| { + // initial approval stakes. + assert_eq!(::TargetList::get_score(&1).unwrap(), 25); + assert_eq!(::TargetList::get_score(&2).unwrap(), 15); + assert_eq!(::TargetList::get_score(&3).unwrap(), 15); + + // 70 is now gone + MaxNominations::set(2); + + // but approval stakes are the same. + assert_eq!(::TargetList::get_score(&1).unwrap(), 25); + assert_eq!(::TargetList::get_score(&2).unwrap(), 15); + assert_eq!(::TargetList::get_score(&3).unwrap(), 15); + + // now they get chilled + assert_ok!(Staking::chill_other(Origin::signed(1), 71)); + + // approvals must be correctly updated. + assert_eq!(::TargetList::get_score(&1).unwrap(), 15); + assert_eq!(::TargetList::get_score(&2).unwrap(), 5); + assert_eq!(::TargetList::get_score(&3).unwrap(), 5); + }); + } + + #[test] + fn un_decodable_nominator_bond_extra_correct_approval_update() { + ExtBuilder::default() + .add_staker(1, 1, 5, StakerStatus::Validator) + .add_staker(2, 2, 5, StakerStatus::Validator) + .add_staker(60, 61, 10, StakerStatus::Nominator(vec![1])) + .add_staker(3, 3, 5, StakerStatus::Validator) + .add_staker(70, 71, 10, StakerStatus::Nominator(vec![1, 2, 3])) + .balance_factor(10) + .build_and_execute(|| { + // initial approval stakes. + assert_eq!(::TargetList::get_score(&1).unwrap(), 25); + assert_eq!(::TargetList::get_score(&2).unwrap(), 15); + assert_eq!(::TargetList::get_score(&3).unwrap(), 15); + + // 70 is now gone + MaxNominations::set(2); + + // but approval stakes are the same. + assert_eq!(::TargetList::get_score(&1).unwrap(), 25); + assert_eq!(::TargetList::get_score(&2).unwrap(), 15); + assert_eq!(::TargetList::get_score(&3).unwrap(), 15); + + // now they get chilled + Balances::make_free_balance_be(&70, 1000); + assert_eq!(Staking::weight_of(&70), 10); + assert_ok!(Staking::bond_extra(Origin::signed(70), 10)); + assert_eq!(Staking::weight_of(&70), 20); + + // approvals must be correctly updated. + assert_eq!(::TargetList::get_score(&1).unwrap(), 25 + 10); + assert_eq!(::TargetList::get_score(&2).unwrap(), 15 + 10); + assert_eq!(::TargetList::get_score(&3).unwrap(), 15 + 10); + }); + } +} + +mod voter_list { use super::*; use frame_election_provider_support::SortedListProvider; @@ -4961,7 +5271,7 @@ mod sorted_list_provider { ); // when account 101 renominates - assert_ok!(Staking::nominate(Origin::signed(100), vec![41])); + assert_ok!(Staking::nominate(Origin::signed(100), vec![11, 21])); // then counts don't change assert_eq!(::VoterList::count(), pre_insert_voter_count);