Postpone public key creation in the test GenesisStorageBuilder (#14142)

* Postpone public key creation from account id in the test genesis storage builder

* Store raw sr25519 in substrate test pallet

* Nitpick

substrate/test-utils/runtime/src/genesismap.rs

@@ -18,8 +18,7 @@
 //! Tool for creating the genesis block.
 
 use super::{
-	currency, substrate_test_pallet, wasm_binary_unwrap, AccountId, AuthorityId, Balance,
-	GenesisConfig,
+	currency, substrate_test_pallet, wasm_binary_unwrap, AccountId, Balance, GenesisConfig,
 };
 use codec::Encode;
 use sc_service::construct_genesis_block;
@@ -34,14 +33,19 @@ use sp_runtime::{
 	BuildStorage,
 };
 
-/// Builder for generating storage from substrate-test-runtime genesis config. Default storage can
-/// be extended with additional key-value pairs.
+/// Builder for generating storage from substrate-test-runtime genesis config.
+///
+/// Default storage can be extended with additional key-value pairs.
 pub struct GenesisStorageBuilder {
-	authorities: Vec<AuthorityId>,
+	/// Authorities accounts used by any component requiring an authority set (e.g. babe).
+	authorities: Vec<AccountId>,
+	/// Accounts to be endowed with some funds.
 	balances: Vec<(AccountId, u64)>,
+	/// Override default number of heap pages.
 	heap_pages_override: Option<u64>,
 	/// Additional storage key pairs that will be added to the genesis map.
 	extra_storage: Storage,
+	/// Optional wasm code override.
 	wasm_code: Option<Vec<u8>>,
 }
 
@@ -50,9 +54,9 @@ impl Default for GenesisStorageBuilder {
 	fn default() -> Self {
 		Self::new(
 			vec![
-				sr25519::Public::from(Sr25519Keyring::Alice).into(),
-				sr25519::Public::from(Sr25519Keyring::Bob).into(),
-				sr25519::Public::from(Sr25519Keyring::Charlie).into(),
+				Sr25519Keyring::Alice.into(),
+				Sr25519Keyring::Bob.into(),
+				Sr25519Keyring::Charlie.into(),
 			],
 			(0..16_usize)
 				.into_iter()
@@ -74,7 +78,7 @@ impl GenesisStorageBuilder {
 	/// from `extra_storage` will be injected into built storage. `HEAP_PAGES` key and value will
 	/// also be placed into storage.
 	pub fn new(
-		authorities: Vec<AuthorityId>,
+		authorities: Vec<AccountId>,
 		endowed_accounts: Vec<AccountId>,
 		balance: Balance,
 	) -> Self {
@@ -104,17 +108,28 @@ impl GenesisStorageBuilder {
 	}
 
 	/// Builds the `GenesisConfig` and returns its storage.
-	pub fn build_storage(&mut self) -> Storage {
+	pub fn build(self) -> Storage {
+		let authorities_sr25519: Vec<_> = self
+			.authorities
+			.clone()
+			.into_iter()
+			.map(|id| sr25519::Public::from(id))
+			.collect();
+
 		let genesis_config = GenesisConfig {
 			system: frame_system::GenesisConfig {
 				code: self.wasm_code.clone().unwrap_or(wasm_binary_unwrap().to_vec()),
 			},
 			babe: pallet_babe::GenesisConfig {
-				authorities: self.authorities.clone().into_iter().map(|x| (x, 1)).collect(),
+				authorities: authorities_sr25519
+					.clone()
+					.into_iter()
+					.map(|x| (x.into(), 1))
+					.collect(),
 				epoch_config: Some(crate::TEST_RUNTIME_BABE_EPOCH_CONFIGURATION),
 			},
 			substrate_test: substrate_test_pallet::GenesisConfig {
-				authorities: self.authorities.clone(),
+				authorities: authorities_sr25519.clone(),
 			},
 			balances: pallet_balances::GenesisConfig { balances: self.balances.clone() },
 		};

substrate/test-utils/runtime/src/lib.rs

@@ -64,13 +64,11 @@ use sp_runtime::{
 use sp_version::NativeVersion;
 use sp_version::RuntimeVersion;
 
-// Ensure Babe and Aura use the same crypto to simplify things a bit.
-pub use sp_consensus_babe::{AllowedSlots, AuthorityId, BabeEpochConfiguration, Slot};
+pub use sp_consensus_babe::{AllowedSlots, BabeEpochConfiguration, Slot};
 
 pub use pallet_balances::Call as BalancesCall;
 
 pub type AuraId = sp_consensus_aura::sr25519::AuthorityId;
-
 #[cfg(feature = "std")]
 pub use extrinsic::{ExtrinsicBuilder, Transfer};
 
@@ -616,10 +614,7 @@ impl_runtime_apis! {
 		}
 
 		fn authorities() -> Vec<AuraId> {
-			SubstrateTest::authorities().into_iter().map(|a| {
-				let authority: sr25519::Public = a.into();
-				AuraId::from(authority)
-			}).collect()
+			SubstrateTest::authorities().into_iter().map(|auth| AuraId::from(auth)).collect()
 		}
 	}
 
@@ -630,10 +625,9 @@ impl_runtime_apis! {
 				slot_duration: Babe::slot_duration(),
 				epoch_length: EpochDuration::get(),
 				c: epoch_config.c,
-				authorities: SubstrateTest::authorities()
-					.into_iter().map(|x|(x, 1)).collect(),
-					randomness: Babe::randomness(),
-					allowed_slots: epoch_config.allowed_slots,
+				authorities: Babe::authorities().to_vec(),
+				randomness: Babe::randomness(),
+				allowed_slots: epoch_config.allowed_slots,
 			}
 		}
 
@@ -1087,7 +1081,7 @@ mod tests {
 			vec![AccountKeyring::One.into(), AccountKeyring::Two.into()],
 			1000 * currency::DOLLARS,
 		)
-		.build_storage()
+		.build()
 		.into()
 	}
 
@@ -1095,7 +1089,7 @@ mod tests {
 	fn validate_storage_keys() {
 		assert_eq!(
 			genesismap::GenesisStorageBuilder::default()
-				.build_storage()
+				.build()
 				.top
 				.keys()
 				.cloned()

substrate/test-utils/runtime/src/substrate_test_pallet.rs

@@ -21,8 +21,8 @@
 //! functioning runtime. Some calls are allowed to be submitted as unsigned extrinsics, however most
 //! of them requires signing. Refer to `pallet::Call` for further details.
 
-use crate::AuthorityId;
 use frame_support::{pallet_prelude::*, storage};
+use sp_core::sr25519::Public;
 use sp_runtime::transaction_validity::{
 	InvalidTransaction, TransactionSource, TransactionValidity, ValidTransaction,
 };
@@ -49,12 +49,12 @@ pub mod pallet {
 
 	#[pallet::storage]
 	#[pallet::getter(fn authorities)]
-	pub type Authorities<T> = StorageValue<_, Vec<AuthorityId>, ValueQuery>;
+	pub type Authorities<T> = StorageValue<_, Vec<Public>, ValueQuery>;
 
 	#[pallet::genesis_config]
 	#[cfg_attr(feature = "std", derive(Default))]
 	pub struct GenesisConfig {
-		pub authorities: Vec<AuthorityId>,
+		pub authorities: Vec<Public>,
 	}
 
 	#[pallet::genesis_build]