From 17e8bdde609472fe10ec3846096f41cf8ce8163d Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 20 Feb 2024 17:36:51 +0100 Subject: [PATCH 1/9] Fix namespace Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/support/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index 8935acf4e2bb..cd12da6de54e 100644 --- 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(_).") } }; } From ed32a79dafff33c1eb765b7bec9add557c42a3ee Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 20 Feb 2024 17:37:24 +0100 Subject: [PATCH 2/9] Test that genesis config can be build Signed-off-by: Oliver Tale-Yazdi --- .../procedural/src/construct_runtime/expand/config.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/substrate/frame/support/procedural/src/construct_runtime/expand/config.rs b/substrate/frame/support/procedural/src/construct_runtime/expand/config.rs index ffe55bceb80e..5613047359a7 100644 --- 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( ::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(|| { + ::build( + &RuntimeGenesisConfig::default() + ); + }); + } } } From 3da0d0d5fd930dc7f9091cb1e230cec689b1edce Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Tue, 20 Feb 2024 17:40:23 +0100 Subject: [PATCH 3/9] Default for BabeEpochConfiguration Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/babe/src/lib.rs | 6 ++---- substrate/primitives/consensus/babe/src/lib.rs | 6 ++++++ 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/substrate/frame/babe/src/lib.rs b/substrate/frame/babe/src/lib.rs index a6e44390dbc5..5fb107dde3ba 100644 --- 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 { pub authorities: Vec<(AuthorityId, BabeAuthorityWeight)>, - pub epoch_config: Option, + pub epoch_config: BabeEpochConfiguration, #[serde(skip)] pub _config: sp_std::marker::PhantomData, } @@ -333,9 +333,7 @@ pub mod pallet { fn build(&self) { SegmentIndex::::put(0); Pallet::::initialize_genesis_authorities(&self.authorities); - EpochConfig::::put( - self.epoch_config.clone().expect("epoch_config must not be None"), - ); + EpochConfig::::put(&self.epoch_config); } } diff --git a/substrate/primitives/consensus/babe/src/lib.rs b/substrate/primitives/consensus/babe/src/lib.rs index d6b2cdd55e0d..b5a23fc3f443 100644 --- 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::PrimaryAndSecondaryPlainSlots } + } +} + /// 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. From f9cdc48c4fb44929a588222368b0ed8fe68a882a Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 21 Feb 2024 13:48:44 +0100 Subject: [PATCH 4/9] Update substrate/primitives/consensus/babe/src/lib.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: André Silva <123550+andresilva@users.noreply.github.com> --- substrate/primitives/consensus/babe/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/primitives/consensus/babe/src/lib.rs b/substrate/primitives/consensus/babe/src/lib.rs index b5a23fc3f443..ff0b4568226e 100644 --- a/substrate/primitives/consensus/babe/src/lib.rs +++ b/substrate/primitives/consensus/babe/src/lib.rs @@ -258,7 +258,7 @@ pub struct BabeEpochConfiguration { impl Default for BabeEpochConfiguration { fn default() -> Self { - Self { c: (1, 4), allowed_slots: AllowedSlots::PrimaryAndSecondaryPlainSlots } + Self { c: (1, 4), allowed_slots: AllowedSlots::PrimaryAndSecondaryVRFSlots } } } From 1f5d09ed35365b5d5834027cad4ff733a7fd8f08 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 21 Feb 2024 16:50:10 +0100 Subject: [PATCH 5/9] Dont use Option to construct Babe config Signed-off-by: Oliver Tale-Yazdi --- .../chains/relays/rococo/src/genesis.rs | 2 +- .../chains/relays/westend/src/genesis.rs | 2 +- substrate/bin/node/testing/src/genesis.rs | 39 ++----------------- substrate/client/chain-spec/src/chain_spec.rs | 10 +---- .../test-utils/runtime/src/genesismap.rs | 1 - 5 files changed, 6 insertions(+), 48 deletions(-) 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 index 7db9679f1c3e..55437645b052 100644 --- 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 index 578b307dd933..700b80e63f6c 100644 --- 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/substrate/bin/node/testing/src/genesis.rs b/substrate/bin/node/testing/src/genesis.rs index 6ec21fbe0934..c79612d68444 100644 --- 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) -> 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) -> 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 index fe8fcfda216e..78e81e10d2b6 100644 --- 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/test-utils/runtime/src/genesismap.rs b/substrate/test-utils/runtime/src/genesismap.rs index 5ed9c8a64588..9e972886b377 100644 --- 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 { From 0a7d3fedae7edbcb4a6e7ddbe08ba2e688c7f878 Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 21 Feb 2024 16:50:32 +0100 Subject: [PATCH 6/9] Update test closures Signed-off-by: Oliver Tale-Yazdi --- .../bin/node/cli/tests/res/default_genesis_config.json | 8 +++++++- substrate/client/chain-spec/src/genesis_config_builder.rs | 2 +- substrate/client/consensus/babe/rpc/src/lib.rs | 2 +- substrate/test-utils/runtime/src/lib.rs | 2 +- 4 files changed, 10 insertions(+), 4 deletions(-) diff --git a/substrate/bin/node/cli/tests/res/default_genesis_config.json b/substrate/bin/node/cli/tests/res/default_genesis_config.json index 1465a6497cad..e21fbb47da8c 100644 --- 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/client/chain-spec/src/genesis_config_builder.rs b/substrate/client/chain-spec/src/genesis_config_builder.rs index 8766dd5c5ad2..6b956316203c 100644 --- 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 { ::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::(expected).unwrap(), config); } diff --git a/substrate/client/consensus/babe/rpc/src/lib.rs b/substrate/client/consensus/babe/rpc/src/lib.rs index 8b183e7dfdcd..a3e811baecff 100644 --- 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/test-utils/runtime/src/lib.rs b/substrate/test-utils/runtime/src/lib.rs index 8bc6f72a82ec..db9ff187b707 100644 --- 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::::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); } From decfd0bbaa393dcec710f4050edd79f47d89e75c Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 21 Feb 2024 16:50:44 +0100 Subject: [PATCH 7/9] Remove pallet-session genesis checks Signed-off-by: Oliver Tale-Yazdi --- substrate/frame/session/src/lib.rs | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/substrate/frame/session/src/lib.rs b/substrate/frame/session/src/lib.rs index 178d43f596b2..fc35cd6ddd82 100644 --- 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::::load_keys(&v).expect("Validator in session 1 missing keys!"), - ) - }) + .into_iter() + .filter_map(|v| Pallet::::load_keys(&v).map(|k| (v, k))) .collect(); // Tell everyone about the genesis session keys From 374825d82aa31cba8420bfe63561d5b2b5f4c5fb Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 21 Feb 2024 16:50:52 +0100 Subject: [PATCH 8/9] Remove pallet-aura-ext genesis checks Signed-off-by: Oliver Tale-Yazdi --- cumulus/pallets/aura-ext/src/lib.rs | 6 ------ 1 file changed, 6 deletions(-) diff --git a/cumulus/pallets/aura-ext/src/lib.rs b/cumulus/pallets/aura-ext/src/lib.rs index 34a41557152d..31b571816a0c 100644 --- a/cumulus/pallets/aura-ext/src/lib.rs +++ b/cumulus/pallets/aura-ext/src/lib.rs @@ -117,12 +117,6 @@ pub mod pallet { impl BuildGenesisConfig for GenesisConfig { fn build(&self) { let authorities = Aura::::authorities(); - - assert!( - !authorities.is_empty(), - "AuRa authorities empty, maybe wrong order in `construct_runtime!`?", - ); - Authorities::::put(authorities); } } From e0819223563f6f8f67ea79503a8672540bfa64da Mon Sep 17 00:00:00 2001 From: Oliver Tale-Yazdi Date: Wed, 21 Feb 2024 18:07:49 +0100 Subject: [PATCH 9/9] Add prdoc Signed-off-by: Oliver Tale-Yazdi --- prdoc/pr_3412.prdoc | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 prdoc/pr_3412.prdoc diff --git a/prdoc/pr_3412.prdoc b/prdoc/pr_3412.prdoc new file mode 100644 index 000000000000..d68f05e45dcf --- /dev/null +++ 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