[MOON-1460] change eligibility ratio to be a number (#38)

* use eligibility number instead of percentage

* use NonZeroU32

* remove Half nomenclature to signify default value

* use default when count is zero during migration

README.md

@@ -92,7 +92,7 @@ consensus engine simply by creating filters that implement the `CanAuthor` trait
 
 This repository comes with a few example filters already, and additional examples are welcome. The examples are:
 * PseudoRandom FixedSized Subset - This filter takes a finite set (eg a staked set) and filters it down to a pseudo-random
-subset at each height. The eligible ratio is configurable in the pallet. This is a good learning example.
+subset at each height. The eligible count is configurable in the pallet. This is a good learning example.
 * Aura - The authority round consensus engine is popular in the Substrate ecosystem because it was one
 of the first (and simplest!) engines implemented in Substrate. Aura can be expressed in the Nimbus
 filter framework and is included as an example filter. If you are considering using aura, that crate

pallets/author-slot-filter/Cargo.toml

@@ -12,7 +12,7 @@ log = { version = "0.4", default-features = false }
 nimbus-primitives = { path = "../../nimbus-primitives", default-features = false }
 parity-scale-codec = { version = "2.0.0", default-features = false, features = [ "derive" ] }
 scale-info = { version = "1.0.0", default-features = false, features = [ "derive" ] }
-serde = { version = "1.0.101", optional = true, features = [ "derive" ] }
+serde = { version = "1.0.101", default-features = false, features = [ "derive" ] }
 sp-core = { git = "https:/paritytech/substrate", branch = "master", default-features = false }
 sp-runtime = { git = "https:/paritytech/substrate", branch = "master", default-features = false }
 sp-std = { git = "https:/paritytech/substrate", branch = "master", default-features = false }
@@ -24,7 +24,6 @@ frame-benchmarking = { git = "https:/paritytech/substrate", branch =
 frame-support-test = { version = "3.0.0", git = "https:/paritytech/substrate", branch = "master" }
 sp-io = { git = "https:/paritytech/substrate", branch = "master"}
 
-
 [features]
 default = [ "std" ]
 std = [
@@ -35,7 +34,7 @@ std = [
  "nimbus-primitives/std",
  "parity-scale-codec/std",
  "scale-info/std",
- "serde",
+ "serde/std",
  "sp-core/std",
  "sp-runtime/std",
  "sp-std/std",

pallets/author-slot-filter/src/lib.rs

@@ -32,11 +32,19 @@ pub use pallet::*;
 #[cfg(any(test, feature = "runtime-benchmarks"))]
 mod benchmarks;
 
+pub mod migration;
+pub mod num;
 pub mod weights;
 
+#[cfg(test)]
+mod mock;
+#[cfg(test)]
+mod tests;
+
 #[pallet]
 pub mod pallet {
 
+ use crate::num::NonZeroU32;
  use crate::weights::WeightInfo;
  use frame_support::{pallet_prelude::*, traits::Randomness};
  use frame_system::pallet_prelude::*;
@@ -74,7 +82,11 @@ pub mod pallet {
  mut active: Vec<T::AccountId>,
  seed: &u32,
  ) -> (Vec<T::AccountId>, Vec<T::AccountId>) {
- let num_eligible = EligibleRatio::<T>::get().mul_ceil(active.len());
+ let mut num_eligible = EligibleCount::<T>::get().get() as usize;
+ if num_eligible > active.len() {
+ num_eligible = active.len();
+ }
+
  let mut eligible = Vec::with_capacity(num_eligible);
 
  for i in 0..num_eligible {
@@ -128,20 +140,26 @@ pub mod pallet {
 
  #[pallet::call]
  impl<T: Config> Pallet<T> {
- /// Update the eligible ratio. Intended to be called by governance.
+ /// Update the eligible count. Intended to be called by governance.
  #[pallet::weight(T::WeightInfo::set_eligible())]
- pub fn set_eligible(origin: OriginFor<T>, new: Percent) -> DispatchResultWithPostInfo {
+ pub fn set_eligible(
+ origin: OriginFor<T>,
+ new: EligibilityValue,
+ ) -> DispatchResultWithPostInfo {
  ensure_root(origin)?;
- EligibleRatio::<T>::put(&new);
+ EligibleCount::<T>::put(&new);
  <Pallet<T>>::deposit_event(Event::EligibleUpdated(new));
 
  Ok(Default::default())
  }
  }
 
- /// The percentage of active authors that will be eligible at each height.
+ /// The type of eligibility to use
+ pub type EligibilityValue = NonZeroU32;
+
  #[pallet::storage]
  #[pallet::getter(fn eligible_ratio)]
+ #[deprecated]
  pub type EligibleRatio<T: Config> = StorageValue<_, Percent, ValueQuery, Half<T>>;
 
  // Default value for the `EligibleRatio` is one half.
@@ -150,119 +168,46 @@ pub mod pallet {
  Percent::from_percent(50)
  }
 
+ /// The number of active authors that will be eligible at each height.
+ #[pallet::storage]
+ #[pallet::getter(fn eligible_count)]
+ pub type EligibleCount<T: Config> =
+ StorageValue<_, EligibilityValue, ValueQuery, DefaultEligibilityValue<T>>;
+
+ /// Default total number of eligible authors, must NOT be 0.
+ pub const DEFAULT_TOTAL_ELIGIBLE_AUTHORS: EligibilityValue = NonZeroU32::new_unchecked(50);
+
+ // Default value for the `EligibleCount`.
+ #[pallet::type_value]
+ pub fn DefaultEligibilityValue<T: Config>() -> EligibilityValue {
+ DEFAULT_TOTAL_ELIGIBLE_AUTHORS
+ }
+
  #[pallet::genesis_config]
  pub struct GenesisConfig {
- pub eligible_ratio: Percent,
+ pub eligible_count: EligibilityValue,
  }
 
  #[cfg(feature = "std")]
  impl Default for GenesisConfig {
  fn default() -> Self {
  Self {
- eligible_ratio: Percent::from_percent(50),
+ eligible_count: DEFAULT_TOTAL_ELIGIBLE_AUTHORS,
  }
  }
  }
 
  #[pallet::genesis_build]
  impl<T: Config> GenesisBuild<T> for GenesisConfig {
  fn build(&self) {
- EligibleRatio::<T>::put(self.eligible_ratio);
+ EligibleCount::<T>::put(self.eligible_count.clone());
  }
  }
 
  #[pallet::event]
  #[pallet::generate_deposit(fn deposit_event)]
  pub enum Event {
  /// The amount of eligible authors for the filter to select has been changed.
- EligibleUpdated(Percent),
- }
-}
-
-#[cfg(test)]
-pub mod tests {
- use super::*;
- use crate as author_slot_filter;
-
- use frame_support::{assert_ok, parameter_types, traits::Everything};
- use frame_support_test::TestRandomness;
- use sp_core::H256;
- use sp_io::TestExternalities;
- use sp_runtime::{
- testing::Header,
- traits::{BlakeTwo256, IdentityLookup},
- Percent,
- };
- use sp_std::vec;
- const AUTHOR_ID: u64 = 1;
-
- pub fn new_test_ext() -> TestExternalities {
- let t = frame_system::GenesisConfig::default()
- .build_storage::<Test>()
- .unwrap();
- TestExternalities::new(t)
- }
-
- type UncheckedExtrinsic = frame_system::mocking::MockUncheckedExtrinsic<Test>;
- type Block = frame_system::mocking::MockBlock<Test>;
-
- // Configure a mock runtime to test the pallet.
- frame_support::construct_runtime!(
- pub enum Test where
- Block = Block,
- NodeBlock = Block,
- UncheckedExtrinsic = UncheckedExtrinsic,
- {
- System: frame_system::{Pallet, Call, Config, Storage, Event<T>},
- AuthorSlotFilter: author_slot_filter::{Pallet, Call, Storage, Event, Config},
- }
- );
-
- parameter_types! {
- pub const BlockHashCount: u64 = 250;
- }
- impl frame_system::Config for Test {
- type BaseCallFilter = Everything;
- type BlockWeights = ();
- type BlockLength = ();
- type DbWeight = ();
- type Origin = Origin;
- type Index = u64;
- type BlockNumber = u64;
- type Call = Call;
- type Hash = H256;
- type Hashing = BlakeTwo256;
- type AccountId = u64;
- type Lookup = IdentityLookup<Self::AccountId>;
- type Header = Header;
- type Event = Event;
- type BlockHashCount = BlockHashCount;
- type Version = ();
- type PalletInfo = PalletInfo;
- type AccountData = ();
- type OnNewAccount = ();
- type OnKilledAccount = ();
- type SystemWeightInfo = ();
- type SS58Prefix = ();
- type OnSetCode = ();
- }
- parameter_types! {
- pub Authors: Vec<u64> = vec![AUTHOR_ID];
- }
- impl Config for Test {
- type Event = Event;
- type RandomnessSource = TestRandomness<Self>;
- type PotentialAuthors = Authors;
- type WeightInfo = ();
- }
-
- #[test]
- fn set_eligibility_works() {
- new_test_ext().execute_with(|| {
- let percent = Percent::from_percent(34);
-
- assert_ok!(AuthorSlotFilter::set_eligible(Origin::root(), percent));
- assert_eq!(AuthorSlotFilter::eligible_ratio(), percent)
- });
+ EligibleUpdated(EligibilityValue),
  }
 }

pallets/author-slot-filter/src/migration.rs

@@ -0,0 +1,86 @@
+// Copyright 2019-2021 PureStake Inc.
+// This file is part of Nimbus.
+
+// Nimbus is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Nimbus is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Nimbus. If not, see <http://www.gnu.org/licenses/>.
+
+use core::marker::PhantomData;
+use frame_support::storage::migration;
+use frame_support::traits::Get;
+use frame_support::traits::OnRuntimeUpgrade;
+use frame_support::weights::Weight;
+use sp_runtime::Percent;
+
+#[cfg(feature = "try-runtime")]
+use frame_support::traits::OnRuntimeUpgradeHelpersExt;
+
+use super::num::NonZeroU32;
+use super::pallet::Config;
+
+pub struct EligibleRatioToEligiblityCount<T>(PhantomData<T>);
+
+pub const PALLET_NAME: &[u8] = b"AuthorSlotFilter";
+pub const ELIGIBLE_RATIO_ITEM_NAME: &[u8] = b"EligibleRatio";
+pub const ELIGIBLE_COUNT_ITEM_NAME: &[u8] = b"EligibleCount";
+
+impl<T> OnRuntimeUpgrade for EligibleRatioToEligiblityCount<T>
+where
+ T: Config,
+{
+ fn on_runtime_upgrade() -> Weight {
+ log::info!(target: "EligibleRatioToEligiblityCount", "starting migration");
+
+ if let Some(old_value) =
+ migration::get_storage_value::<Percent>(PALLET_NAME, ELIGIBLE_RATIO_ITEM_NAME, &[])
+ {
+ let total_authors = <T as Config>::PotentialAuthors::get().len();
+ let new_value: u32 = percent_of_num(old_value, total_authors as u32);
+ migration::put_storage_value(
+ PALLET_NAME,
+ ELIGIBLE_COUNT_ITEM_NAME,
+ &[],
+ NonZeroU32::new(new_value).unwrap_or(crate::pallet::DEFAULT_TOTAL_ELIGIBLE_AUTHORS),
+ );
+
+ let db_weights = T::DbWeight::get();
+ db_weights.write + db_weights.read
+ } else {
+ 0
+ }
+ }
+
+ #[cfg(feature = "try-runtime")]
+ fn pre_upgrade() -> Result<(), &'static str> {
+ if let Some(eligible_ratio) =
+ migration::get_storage_value::<Percent>(PALLET_NAME, ELIGIBLE_RATIO_ITEM_NAME, &[])
+ {
+ let total_authors = <T as Config>::PotentialAuthors::get().len();
+ let eligible_count: u32 = percent_of_num(eligible_ratio, total_authors as u32);
+ let eligible_count = NonZeroU32::new_unchecked(eligible_count);
+ Self::set_temp_storage(new_value, "expected_eligible_count");
+ }
+ }
+
+ #[cfg(feature = "try-runtime")]
+ fn post_upgrade() -> Result<(), &'static str> {
+ let expected = Self::get_temp_storage::<NonZeroU32>("expected_eligible_count");
+ let actual =
+ migration::get_storage_value::<NonZeroU32>(PALLET_NAME, ELIGIBLE_COUNT_ITEM_NAME, &[]);
+
+ assert_eq!(expected, actual);
+ }
+}
+
+fn percent_of_num(percent: Percent, num: u32) -> u32 {
+ percent.mul_ceil(num as u32)
+}