Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FRAME] Test for sane genesis default #3412

Merged
merged 10 commits into from
Feb 22, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions cumulus/pallets/aura-ext/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() },
Expand Down
17 changes: 17 additions & 0 deletions prdoc/pr_3412.prdoc
Original file line number Diff line number Diff line change
@@ -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
8 changes: 7 additions & 1 deletion substrate/bin/node/cli/tests/res/default_genesis_config.json
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,13 @@
"system": {},
"babe": {
"authorities": [],
"epochConfig": null
"epochConfig": {
"allowed_slots": "PrimaryAndSecondaryVRFSlots",
"c": [
1,
4
]
}
},
"indices": {
"indices": []
Expand Down
39 changes: 3 additions & 36 deletions substrate/bin/node/testing/src/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 {
Expand All @@ -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()
}
}
10 changes: 1 addition & 9 deletions substrate/client/chain-spec/src/chain_spec.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion substrate/client/chain-spec/src/genesis_config_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
2 changes: 1 addition & 1 deletion substrate/client/consensus/babe/rpc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Expand Down
6 changes: 2 additions & 4 deletions substrate/frame/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>,
}
Expand All @@ -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);
}
}

Expand Down
18 changes: 2 additions & 16 deletions substrate/frame/session/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
);
});
}
}
}

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(_).")
}
};
}
Expand Down
6 changes: 6 additions & 0 deletions substrate/primitives/consensus/babe/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
1 change: 0 additions & 1 deletion substrate/test-utils/runtime/src/genesismap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
2 changes: 1 addition & 1 deletion substrate/test-utils/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
Loading