Round of self-review.

bin/node/runtime/src/lib.rs

@@ -1453,6 +1453,7 @@ impl_runtime_apis! {
 			add_benchmark!(params, batches, pallet_democracy, Democracy);
 			add_benchmark!(params, batches, pallet_election_provider_multi_phase, ElectionProviderMultiPhase);
 			add_benchmark!(params, batches, pallet_elections_phragmen, Elections);
+			add_benchmark!(params, batches, pallet_gilt, Gilt);
 			add_benchmark!(params, batches, pallet_grandpa, Grandpa);
 			add_benchmark!(params, batches, pallet_identity, Identity);
 			add_benchmark!(params, batches, pallet_im_online, ImOnline);

frame/staking/Cargo.toml

@@ -17,7 +17,6 @@ static_assertions = "1.1.0"
 serde = { version = "1.0.101", optional = true }
 codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] }
 sp-std = { version = "3.0.0", default-features = false, path = "../../primitives/std" }
-# TWO_PHASE_NOTE:: ideally we should be able to get rid of this.
 sp-npos-elections = { version = "3.0.0", default-features = false, path = "../../primitives/npos-elections" }
 sp-io ={ version = "3.0.0", default-features = false, path = "../../primitives/io" }
 sp-runtime = { version = "3.0.0", default-features = false, path = "../../primitives/runtime" }

frame/staking/src/lib.rs

@@ -273,9 +273,9 @@ mod mock;
 #[cfg(test)]
 mod tests;
 #[cfg(any(feature = "runtime-benchmarks", test))]
-pub mod benchmarking;
-#[cfg(any(feature = "runtime-benchmarks", test))]
 pub mod testing_utils;
+#[cfg(any(feature = "runtime-benchmarks", test))]
+pub mod benchmarking;
 
 pub mod slashing;
 pub mod inflation;
@@ -1028,7 +1028,7 @@ pub mod migrations {
 			type PalletPrefix: Get<&'static str>;
 		}
 
-		// TODO: complete this
+		// TODO: maybe move this to frame-support in a follow up?
 		macro_rules! generate_storage_types {
 			($name:ident<T: $trait:tt> => Map<$key:ty, $value:ty>) => {
 				unreachable!()
@@ -2124,11 +2124,15 @@ impl<T: Config> Module<T> {
 				.unwrap_or(0); // Must never happen.
 
 			match ForceEra::get() {
+				// Will set to default again, which is `NorForcing`.
 				Forcing::ForceNew => ForceEra::kill(),
+				// Short circuit to `new_era`.
 				Forcing::ForceAlways => (),
+				// Only go to `new_era` if deadline reached.
 				Forcing::NotForcing if era_length >= T::SessionsPerEra::get() => (),
 				_ => {
-					// TODO: triple check this piece.
+					// either `Forcing::ForceNone`,
+					// or `Forcing::NotForcing if era_length >= T::SessionsPerEra::get()`.
 					return None
 				},
 			}
@@ -2319,7 +2323,7 @@ impl<T: Config> Module<T> {
 			return Err(());
 		}
 
-		// Populate Stakers and write slot stake.
+		// Populate stakers, exposures, and the snapshot of validator prefs.
 		let mut total_stake: BalanceOf<T> = Zero::zero();
 		exposures.into_iter().for_each(|(stash, exposure)| {
 			total_stake = total_stake.saturating_add(exposure.total);
@@ -2800,6 +2804,7 @@ where
 	}
 
 	fn can_report() -> bool {
+		// TODO: https://github.com/paritytech/substrate/issues/8343
 		true
 	}
 }

frame/staking/src/tests.rs

@@ -1888,8 +1888,7 @@ fn bond_with_duplicate_vote_should_be_ignored_by_election_provider_elected() {
 			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.
+			// winners should be 21 and 11.
 			let supports = <Test as Config>::ElectionProvider::elect().unwrap();
 			assert_eq!(
 				supports,