[FRAME] Test for sane genesis default (#3412)

Closes #2713 --------- Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io> Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com>
paritytech · Feb 22, 2024 · e76b244 · e76b244
1 parent cd91c6b
commit e76b244
Show file tree

Hide file tree

Showing 16 changed files with 55 additions and 79 deletions.
diff --git a/cumulus/pallets/aura-ext/src/lib.rs b/cumulus/pallets/aura-ext/src/lib.rs
@@ -117,12 +117,6 @@ pub mod pallet {
 	impl<T: Config> BuildGenesisConfig for GenesisConfig<T> {
 		fn build(&self) {
 			let authorities = Aura::<T>::authorities();
-
-			assert!(
-				!authorities.is_empty(),
-				"AuRa authorities empty, maybe wrong order in `construct_runtime!`?",
-			);
-
 			Authorities::<T>::put(authorities);
 		}
 	}

diff --git a/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs b/cumulus/parachains/integration-tests/emulated/chains/relays/rococo/src/genesis.rs
@@ -78,7 +78,7 @@ pub fn genesis() -> Storage {
 		},
 		babe: rococo_runtime::BabeConfig {
 			authorities: Default::default(),
-			epoch_config: Some(rococo_runtime::BABE_GENESIS_EPOCH_CONFIG),
+			epoch_config: rococo_runtime::BABE_GENESIS_EPOCH_CONFIG,
 			..Default::default()
 		},
 		sudo: rococo_runtime::SudoConfig {

diff --git a/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs b/cumulus/parachains/integration-tests/emulated/chains/relays/westend/src/genesis.rs
@@ -94,7 +94,7 @@ pub fn genesis() -> Storage {
 		},
 		babe: westend_runtime::BabeConfig {
 			authorities: Default::default(),
-			epoch_config: Some(westend_runtime::BABE_GENESIS_EPOCH_CONFIG),
+			epoch_config: westend_runtime::BABE_GENESIS_EPOCH_CONFIG,
 			..Default::default()
 		},
 		configuration: westend_runtime::ConfigurationConfig { config: get_host_config() },

diff --git a/prdoc/pr_3412.prdoc b/prdoc/pr_3412.prdoc
@@ -0,0 +1,17 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: "[FRAME] Add genesis test and remove some checks"
+
+doc:
+  - audience: Runtime Dev
+    description: |
+      The construct_runtime macro now generates a test to assert that all `GenesisConfig`s of all
+      pallets can be build within the runtime. This ensures that the `BuildGenesisConfig` runtime
+      API works.
+      Further, some checks from a few pallets were removed to make this pass.
+
+crates:
+  - name: pallet-babe
+  - name: pallet-aura-ext
+  - name: pallet-session
diff --git a/substrate/bin/node/cli/tests/res/default_genesis_config.json b/substrate/bin/node/cli/tests/res/default_genesis_config.json
@@ -2,7 +2,13 @@
   "system": {},
   "babe": {
     "authorities": [],
-    "epochConfig": null
+    "epochConfig": {
+        "allowed_slots": "PrimaryAndSecondaryVRFSlots",
+        "c": [
+            1,
+            4
+        ]
+    }
   },
   "indices": {
     "indices": []

diff --git a/substrate/bin/node/testing/src/genesis.rs b/substrate/bin/node/testing/src/genesis.rs
@@ -20,9 +20,8 @@
 
 use crate::keyring::*;
 use kitchensink_runtime::{
-	constants::currency::*, AccountId, AssetsConfig, BabeConfig, BalancesConfig, GluttonConfig,
-	GrandpaConfig, IndicesConfig, RuntimeGenesisConfig, SessionConfig, SocietyConfig, StakerStatus,
-	StakingConfig, BABE_GENESIS_EPOCH_CONFIG,
+	constants::currency::*, AccountId, AssetsConfig, BalancesConfig, IndicesConfig,
+	RuntimeGenesisConfig, SessionConfig, SocietyConfig, StakerStatus, StakingConfig,
 };
 use sp_keyring::Ed25519Keyring;
 use sp_runtime::Perbill;
@@ -47,7 +46,6 @@ pub fn config_endowed(extra_endowed: Vec<AccountId>) -> RuntimeGenesisConfig {
 	endowed.extend(extra_endowed.into_iter().map(|endowed| (endowed, 100 * DOLLARS)));
 
 	RuntimeGenesisConfig {
-		system: Default::default(),
 		indices: IndicesConfig { indices: vec![] },
 		balances: BalancesConfig { balances: endowed },
 		session: SessionConfig {
@@ -69,39 +67,8 @@ pub fn config_endowed(extra_endowed: Vec<AccountId>) -> RuntimeGenesisConfig {
 			invulnerables: vec![alice(), bob(), charlie()],
 			..Default::default()
 		},
-		babe: BabeConfig {
-			authorities: vec![],
-			epoch_config: Some(BABE_GENESIS_EPOCH_CONFIG),
-			..Default::default()
-		},
-		grandpa: GrandpaConfig { authorities: vec![], _config: Default::default() },
-		beefy: Default::default(),
-		im_online: Default::default(),
-		authority_discovery: Default::default(),
-		democracy: Default::default(),
-		council: Default::default(),
-		technical_committee: Default::default(),
-		technical_membership: Default::default(),
-		elections: Default::default(),
-		sudo: Default::default(),
-		treasury: Default::default(),
 		society: SocietyConfig { pot: 0 },
-		vesting: Default::default(),
 		assets: AssetsConfig { assets: vec![(9, alice(), true, 1)], ..Default::default() },
-		pool_assets: Default::default(),
-		transaction_storage: Default::default(),
-		transaction_payment: Default::default(),
-		alliance: Default::default(),
-		alliance_motion: Default::default(),
-		nomination_pools: Default::default(),
-		safe_mode: Default::default(),
-		tx_pause: Default::default(),
-		glutton: GluttonConfig {
-			compute: Default::default(),
-			storage: Default::default(),
-			trash_data_count: Default::default(),
-			..Default::default()
-		},
-		mixnet: Default::default(),
+		..Default::default()
 	}
 }
diff --git a/substrate/client/chain-spec/src/chain_spec.rs b/substrate/client/chain-spec/src/chain_spec.rs
@@ -1237,15 +1237,7 @@ mod tests {
 			"TestName",
 			"test",
 			ChainType::Local,
-			move || substrate_test_runtime::RuntimeGenesisConfig {
-				babe: substrate_test_runtime::BabeConfig {
-					epoch_config: Some(
-						substrate_test_runtime::TEST_RUNTIME_BABE_EPOCH_CONFIGURATION,
-					),
-					..Default::default()
-				},
-				..Default::default()
-			},
+			|| Default::default(),
 			Vec::new(),
 			None,
 			None,

diff --git a/substrate/client/chain-spec/src/genesis_config_builder.rs b/substrate/client/chain-spec/src/genesis_config_builder.rs
@@ -149,7 +149,7 @@ mod tests {
 			<GenesisConfigBuilderRuntimeCaller>::new(substrate_test_runtime::wasm_binary_unwrap())
 				.get_default_config()
 				.unwrap();
-		let expected = r#"{"system":{},"babe":{"authorities":[],"epochConfig":null},"substrateTest":{"authorities":[]},"balances":{"balances":[]}}"#;
+		let expected = r#"{"babe": {"authorities": [], "epochConfig": {"allowed_slots": "PrimaryAndSecondaryVRFSlots", "c": [1, 4]}}, "balances": {"balances": []}, "substrateTest": {"authorities": []}, "system": {}}"#;
 		assert_eq!(from_str::<Value>(expected).unwrap(), config);
 	}
 

diff --git a/substrate/client/consensus/babe/rpc/src/lib.rs b/substrate/client/consensus/babe/rpc/src/lib.rs
@@ -258,7 +258,7 @@ mod tests {
 
 		let request = r#"{"jsonrpc":"2.0","method":"babe_epochAuthorship","params": [],"id":1}"#;
 		let (response, _) = api.raw_json_request(request, 1).await.unwrap();
-		let expected = r#"{"jsonrpc":"2.0","result":{"5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY":{"primary":[0],"secondary":[1,2,4],"secondary_vrf":[]}},"id":1}"#;
+		let expected = r#"{"jsonrpc":"2.0","result":{"5GrwvaEF5zXb26Fz9rcQpDWS57CtERHpNehXCPcNoHGKutQY":{"primary":[0],"secondary":[],"secondary_vrf":[1,2,4]}},"id":1}"#;
 
 		assert_eq!(response, expected);
 	}

diff --git a/substrate/frame/babe/src/lib.rs b/substrate/frame/babe/src/lib.rs
@@ -323,7 +323,7 @@ pub mod pallet {
 	#[pallet::genesis_config]
 	pub struct GenesisConfig<T: Config> {
 		pub authorities: Vec<(AuthorityId, BabeAuthorityWeight)>,
-		pub epoch_config: Option<BabeEpochConfiguration>,
+		pub epoch_config: BabeEpochConfiguration,
 		#[serde(skip)]
 		pub _config: sp_std::marker::PhantomData<T>,
 	}
@@ -333,9 +333,7 @@ pub mod pallet {
 		fn build(&self) {
 			SegmentIndex::<T>::put(0);
 			Pallet::<T>::initialize_genesis_authorities(&self.authorities);
-			EpochConfig::<T>::put(
-				self.epoch_config.clone().expect("epoch_config must not be None"),
-			);
+			EpochConfig::<T>::put(&self.epoch_config);
 		}
 	}
 

diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs
@@ -460,27 +460,13 @@ pub mod pallet {
 					);
 					self.keys.iter().map(|x| x.1.clone()).collect()
 				});
-			assert!(
-				!initial_validators_0.is_empty(),
-				"Empty validator set for session 0 in genesis block!"
-			);
 
 			let initial_validators_1 = T::SessionManager::new_session_genesis(1)
 				.unwrap_or_else(|| initial_validators_0.clone());
-			assert!(
-				!initial_validators_1.is_empty(),
-				"Empty validator set for session 1 in genesis block!"
-			);
 
 			let queued_keys: Vec<_> = initial_validators_1
-				.iter()
-				.cloned()
-				.map(|v| {
-					(
-						v.clone(),
-						Pallet::<T>::load_keys(&v).expect("Validator in session 1 missing keys!"),
-					)
-				})
+				.into_iter()
+				.filter_map(|v| Pallet::<T>::load_keys(&v).map(|k| (v, k)))
 				.collect();
 
 			// Tell everyone about the genesis session keys

diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/config.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/config.rs
@@ -99,6 +99,17 @@ pub fn expand_outer_config(
 				<AllPalletsWithSystem as #scrate::traits::OnGenesis>::on_genesis();
 			}
 		}
+
+		/// Test the `Default` derive impl of the `RuntimeGenesisConfig`.
+		#[cfg(test)]
+		#[test]
+		fn test_genesis_config_builds() {
+			#scrate::__private::sp_io::TestExternalities::default().execute_with(|| {
+				<RuntimeGenesisConfig as #scrate::traits::BuildGenesisConfig>::build(
+					&RuntimeGenesisConfig::default()
+				);
+			});
+		}
 	}
 }
 

diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs
@@ -780,7 +780,7 @@ macro_rules! assert_err_with_weight {
 			$crate::assert_err!($call.map(|_| ()).map_err(|e| e.error), $err);
 			assert_eq!(dispatch_err_with_post.post_info.actual_weight, $weight);
 		} else {
-			panic!("expected Err(_), got Ok(_).")
+			::core::panic!("expected Err(_), got Ok(_).")
 		}
 	};
 }

diff --git a/substrate/primitives/consensus/babe/src/lib.rs b/substrate/primitives/consensus/babe/src/lib.rs
@@ -256,6 +256,12 @@ pub struct BabeEpochConfiguration {
 	pub allowed_slots: AllowedSlots,
 }
 
+impl Default for BabeEpochConfiguration {
+	fn default() -> Self {
+		Self { c: (1, 4), allowed_slots: AllowedSlots::PrimaryAndSecondaryVRFSlots }
+	}
+}
+
 /// Verifies the equivocation proof by making sure that: both headers have
 /// different hashes, are targetting the same slot, and have valid signatures by
 /// the same authority.

diff --git a/substrate/test-utils/runtime/src/genesismap.rs b/substrate/test-utils/runtime/src/genesismap.rs
@@ -124,7 +124,6 @@ impl GenesisStorageBuilder {
 					.into_iter()
 					.map(|x| (x.into(), 1))
 					.collect(),
-				epoch_config: Some(crate::TEST_RUNTIME_BABE_EPOCH_CONFIGURATION),
 				..Default::default()
 			},
 			substrate_test: substrate_test_pallet::GenesisConfig {

diff --git a/substrate/test-utils/runtime/src/lib.rs b/substrate/test-utils/runtime/src/lib.rs
@@ -1291,7 +1291,7 @@ mod tests {
 			let r = Vec::<u8>::decode(&mut &r[..]).unwrap();
 			let json = String::from_utf8(r.into()).expect("returned value is json. qed.");
 
-			let expected = r#"{"system":{},"babe":{"authorities":[],"epochConfig":null},"substrateTest":{"authorities":[]},"balances":{"balances":[]}}"#;
+			let expected = r#"{"system":{},"babe":{"authorities":[],"epochConfig":{"c":[1,4],"allowed_slots":"PrimaryAndSecondaryVRFSlots"}},"substrateTest":{"authorities":[]},"balances":{"balances":[]}}"#;
 			assert_eq!(expected.to_string(), json);
 		}