Do not use Option to wrap GenesisConfig fields (#8275)

Currently we wrap every `GenesisConfig` field in an `Option`, while
we require `Default` being implemented for all pallet genesisconfigs.
Passing `None` also results in the genesis not being initialized, which
is a bug as seen from the perspective of a pallet developer?

This pr changes the fields of the `GenesisConfig` to non `Option` types.
This commit is contained in:
Bastian Köcher
2021-03-06 14:42:21 +01:00
committed by GitHub
parent 1680422f1f
commit b0ebf6498f
10 changed files with 111 additions and 113 deletions
@@ -134,24 +134,24 @@ fn testnet_genesis(
_enable_println: bool, _enable_println: bool,
) -> GenesisConfig { ) -> GenesisConfig {
GenesisConfig { GenesisConfig {
frame_system: Some(SystemConfig { frame_system: SystemConfig {
// Add Wasm runtime to storage. // Add Wasm runtime to storage.
code: wasm_binary.to_vec(), code: wasm_binary.to_vec(),
changes_trie_config: Default::default(), changes_trie_config: Default::default(),
}), },
pallet_balances: Some(BalancesConfig { pallet_balances: BalancesConfig {
// Configure endowed accounts with initial balance of 1 << 60. // Configure endowed accounts with initial balance of 1 << 60.
balances: endowed_accounts.iter().cloned().map(|k|(k, 1 << 60)).collect(), balances: endowed_accounts.iter().cloned().map(|k|(k, 1 << 60)).collect(),
}), },
pallet_aura: Some(AuraConfig { pallet_aura: AuraConfig {
authorities: initial_authorities.iter().map(|x| (x.0.clone())).collect(), authorities: initial_authorities.iter().map(|x| (x.0.clone())).collect(),
}), },
pallet_grandpa: Some(GrandpaConfig { pallet_grandpa: GrandpaConfig {
authorities: initial_authorities.iter().map(|x| (x.1.clone(), 1)).collect(), authorities: initial_authorities.iter().map(|x| (x.1.clone(), 1)).collect(),
}), },
pallet_sudo: Some(SudoConfig { pallet_sudo: SudoConfig {
// Assign network admin rights. // Assign network admin rights.
key: root_key, key: root_key,
}), },
} }
} }
+34 -34
View File
@@ -246,19 +246,19 @@ pub fn testnet_genesis(
const STASH: Balance = ENDOWMENT / 1000; const STASH: Balance = ENDOWMENT / 1000;
GenesisConfig { GenesisConfig {
frame_system: Some(SystemConfig { frame_system: SystemConfig {
code: wasm_binary_unwrap().to_vec(), code: wasm_binary_unwrap().to_vec(),
changes_trie_config: Default::default(), changes_trie_config: Default::default(),
}), },
pallet_balances: Some(BalancesConfig { pallet_balances: BalancesConfig {
balances: endowed_accounts.iter().cloned() balances: endowed_accounts.iter().cloned()
.map(|x| (x, ENDOWMENT)) .map(|x| (x, ENDOWMENT))
.collect() .collect()
}), },
pallet_indices: Some(IndicesConfig { pallet_indices: IndicesConfig {
indices: vec![], indices: vec![],
}), },
pallet_session: Some(SessionConfig { pallet_session: SessionConfig {
keys: initial_authorities.iter().map(|x| { keys: initial_authorities.iter().map(|x| {
(x.0.clone(), x.0.clone(), session_keys( (x.0.clone(), x.0.clone(), session_keys(
x.2.clone(), x.2.clone(),
@@ -267,8 +267,8 @@ pub fn testnet_genesis(
x.5.clone(), x.5.clone(),
)) ))
}).collect::<Vec<_>>(), }).collect::<Vec<_>>(),
}), },
pallet_staking: Some(StakingConfig { pallet_staking: StakingConfig {
validator_count: initial_authorities.len() as u32 * 2, validator_count: initial_authorities.len() as u32 * 2,
minimum_validator_count: initial_authorities.len() as u32, minimum_validator_count: initial_authorities.len() as u32,
stakers: initial_authorities.iter().map(|x| { stakers: initial_authorities.iter().map(|x| {
@@ -277,56 +277,56 @@ pub fn testnet_genesis(
invulnerables: initial_authorities.iter().map(|x| x.0.clone()).collect(), invulnerables: initial_authorities.iter().map(|x| x.0.clone()).collect(),
slash_reward_fraction: Perbill::from_percent(10), slash_reward_fraction: Perbill::from_percent(10),
.. Default::default() .. Default::default()
}), },
pallet_democracy: Some(DemocracyConfig::default()), pallet_democracy: DemocracyConfig::default(),
pallet_elections_phragmen: Some(ElectionsConfig { pallet_elections_phragmen: ElectionsConfig {
members: endowed_accounts.iter() members: endowed_accounts.iter()
.take((num_endowed_accounts + 1) / 2) .take((num_endowed_accounts + 1) / 2)
.cloned() .cloned()
.map(|member| (member, STASH)) .map(|member| (member, STASH))
.collect(), .collect(),
}), },
pallet_collective_Instance1: Some(CouncilConfig::default()), pallet_collective_Instance1: CouncilConfig::default(),
pallet_collective_Instance2: Some(TechnicalCommitteeConfig { pallet_collective_Instance2: TechnicalCommitteeConfig {
members: endowed_accounts.iter() members: endowed_accounts.iter()
.take((num_endowed_accounts + 1) / 2) .take((num_endowed_accounts + 1) / 2)
.cloned() .cloned()
.collect(), .collect(),
phantom: Default::default(), phantom: Default::default(),
}), },
pallet_contracts: Some(ContractsConfig { pallet_contracts: ContractsConfig {
current_schedule: pallet_contracts::Schedule { current_schedule: pallet_contracts::Schedule {
enable_println, // this should only be enabled on development chains enable_println, // this should only be enabled on development chains
..Default::default() ..Default::default()
}, },
}), },
pallet_sudo: Some(SudoConfig { pallet_sudo: SudoConfig {
key: root_key, key: root_key,
}), },
pallet_babe: Some(BabeConfig { pallet_babe: BabeConfig {
authorities: vec![], authorities: vec![],
}), },
pallet_im_online: Some(ImOnlineConfig { pallet_im_online: ImOnlineConfig {
keys: vec![], keys: vec![],
}), },
pallet_authority_discovery: Some(AuthorityDiscoveryConfig { pallet_authority_discovery: AuthorityDiscoveryConfig {
keys: vec![], keys: vec![],
}), },
pallet_grandpa: Some(GrandpaConfig { pallet_grandpa: GrandpaConfig {
authorities: vec![], authorities: vec![],
}), },
pallet_membership_Instance1: Some(Default::default()), pallet_membership_Instance1: Default::default(),
pallet_treasury: Some(Default::default()), pallet_treasury: Default::default(),
pallet_society: Some(SocietyConfig { pallet_society: SocietyConfig {
members: endowed_accounts.iter() members: endowed_accounts.iter()
.take((num_endowed_accounts + 1) / 2) .take((num_endowed_accounts + 1) / 2)
.cloned() .cloned()
.collect(), .collect(),
pot: 0, pot: 0,
max_members: 999, max_members: 999,
}), },
pallet_vesting: Some(Default::default()), pallet_vesting: Default::default(),
pallet_gilt: Some(Default::default()), pallet_gilt: Default::default(),
} }
} }
+28 -28
View File
@@ -56,20 +56,20 @@ pub fn config_endowed(
); );
GenesisConfig { GenesisConfig {
frame_system: Some(SystemConfig { frame_system: SystemConfig {
changes_trie_config: if support_changes_trie { Some(ChangesTrieConfiguration { changes_trie_config: if support_changes_trie { Some(ChangesTrieConfiguration {
digest_interval: 2, digest_interval: 2,
digest_levels: 2, digest_levels: 2,
}) } else { None }, }) } else { None },
code: code.map(|x| x.to_vec()).unwrap_or_else(|| wasm_binary_unwrap().to_vec()), code: code.map(|x| x.to_vec()).unwrap_or_else(|| wasm_binary_unwrap().to_vec()),
}), },
pallet_indices: Some(IndicesConfig { pallet_indices: IndicesConfig {
indices: vec![], indices: vec![],
}), },
pallet_balances: Some(BalancesConfig { pallet_balances: BalancesConfig {
balances: endowed, balances: endowed,
}), },
pallet_session: Some(SessionConfig { pallet_session: SessionConfig {
keys: vec![ keys: vec![
(dave(), alice(), to_session_keys( (dave(), alice(), to_session_keys(
&Ed25519Keyring::Alice, &Ed25519Keyring::Alice,
@@ -84,8 +84,8 @@ pub fn config_endowed(
&Sr25519Keyring::Charlie, &Sr25519Keyring::Charlie,
)), )),
] ]
}), },
pallet_staking: Some(StakingConfig { pallet_staking: StakingConfig {
stakers: vec![ stakers: vec![
(dave(), alice(), 111 * DOLLARS, StakerStatus::Validator), (dave(), alice(), 111 * DOLLARS, StakerStatus::Validator),
(eve(), bob(), 100 * DOLLARS, StakerStatus::Validator), (eve(), bob(), 100 * DOLLARS, StakerStatus::Validator),
@@ -96,29 +96,29 @@ pub fn config_endowed(
slash_reward_fraction: Perbill::from_percent(10), slash_reward_fraction: Perbill::from_percent(10),
invulnerables: vec![alice(), bob(), charlie()], invulnerables: vec![alice(), bob(), charlie()],
.. Default::default() .. Default::default()
}), },
pallet_contracts: Some(ContractsConfig { pallet_contracts: ContractsConfig {
current_schedule: Default::default(), current_schedule: Default::default(),
}), },
pallet_babe: Some(Default::default()), pallet_babe: Default::default(),
pallet_grandpa: Some(GrandpaConfig { pallet_grandpa: GrandpaConfig {
authorities: vec![], authorities: vec![],
}), },
pallet_im_online: Some(Default::default()), pallet_im_online: Default::default(),
pallet_authority_discovery: Some(Default::default()), pallet_authority_discovery: Default::default(),
pallet_democracy: Some(Default::default()), pallet_democracy: Default::default(),
pallet_collective_Instance1: Some(Default::default()), pallet_collective_Instance1: Default::default(),
pallet_collective_Instance2: Some(Default::default()), pallet_collective_Instance2: Default::default(),
pallet_membership_Instance1: Some(Default::default()), pallet_membership_Instance1: Default::default(),
pallet_elections_phragmen: Some(Default::default()), pallet_elections_phragmen: Default::default(),
pallet_sudo: Some(Default::default()), pallet_sudo: Default::default(),
pallet_treasury: Some(Default::default()), pallet_treasury: Default::default(),
pallet_society: Some(SocietyConfig { pallet_society: SocietyConfig {
members: vec![alice(), bob()], members: vec![alice(), bob()],
pot: 0, pot: 0,
max_members: 999, max_members: 999,
}), },
pallet_vesting: Some(Default::default()), pallet_vesting: Default::default(),
pallet_gilt: Some(Default::default()), pallet_gilt: Default::default(),
} }
} }
+5 -5
View File
@@ -1054,15 +1054,15 @@ mod tests {
pub fn new_test_ext() -> sp_io::TestExternalities { pub fn new_test_ext() -> sp_io::TestExternalities {
let mut ext: sp_io::TestExternalities = GenesisConfig { let mut ext: sp_io::TestExternalities = GenesisConfig {
collective_Instance1: Some(collective::GenesisConfig { collective_Instance1: collective::GenesisConfig {
members: vec![1, 2, 3], members: vec![1, 2, 3],
phantom: Default::default(), phantom: Default::default(),
}), },
collective_Instance2: Some(collective::GenesisConfig { collective_Instance2: collective::GenesisConfig {
members: vec![1, 2, 3, 4, 5], members: vec![1, 2, 3, 4, 5],
phantom: Default::default(), phantom: Default::default(),
}), },
collective: None, collective: Default::default(),
}.build_storage().unwrap().into(); }.build_storage().unwrap().into();
ext.execute_with(|| System::set_block_number(1)); ext.execute_with(|| System::set_block_number(1));
ext ext
@@ -1245,7 +1245,7 @@ mod tests {
pub fn build_and_execute(self, test: impl FnOnce() -> ()) { pub fn build_and_execute(self, test: impl FnOnce() -> ()) {
MEMBERS.with(|m| *m.borrow_mut() = self.genesis_members.iter().map(|(m, _)| m.clone()).collect::<Vec<_>>()); MEMBERS.with(|m| *m.borrow_mut() = self.genesis_members.iter().map(|(m, _)| m.clone()).collect::<Vec<_>>());
let mut ext: sp_io::TestExternalities = GenesisConfig { let mut ext: sp_io::TestExternalities = GenesisConfig {
pallet_balances: Some(pallet_balances::GenesisConfig::<Test>{ pallet_balances: pallet_balances::GenesisConfig::<Test>{
balances: vec![ balances: vec![
(1, 10 * self.balance_factor), (1, 10 * self.balance_factor),
(2, 20 * self.balance_factor), (2, 20 * self.balance_factor),
@@ -1254,10 +1254,10 @@ mod tests {
(5, 50 * self.balance_factor), (5, 50 * self.balance_factor),
(6, 60 * self.balance_factor) (6, 60 * self.balance_factor)
], ],
}), },
elections_phragmen: Some(elections_phragmen::GenesisConfig::<Test> { elections_phragmen: elections_phragmen::GenesisConfig::<Test> {
members: self.genesis_members members: self.genesis_members
}), },
}.build_storage().unwrap().into(); }.build_storage().unwrap().into();
ext.execute_with(pre_conditions); ext.execute_with(pre_conditions);
ext.execute_with(test); ext.execute_with(test);
+4 -4
View File
@@ -194,7 +194,7 @@ impl ExtBuilder {
PRESENT_SLASH_PER_VOTER.with(|v| *v.borrow_mut() = self.bad_presentation_punishment); PRESENT_SLASH_PER_VOTER.with(|v| *v.borrow_mut() = self.bad_presentation_punishment);
DECAY_RATIO.with(|v| *v.borrow_mut() = self.decay_ratio); DECAY_RATIO.with(|v| *v.borrow_mut() = self.decay_ratio);
let mut ext: sp_io::TestExternalities = GenesisConfig { let mut ext: sp_io::TestExternalities = GenesisConfig {
pallet_balances: Some(pallet_balances::GenesisConfig::<Test>{ pallet_balances: pallet_balances::GenesisConfig::<Test>{
balances: vec![ balances: vec![
(1, 10 * self.balance_factor), (1, 10 * self.balance_factor),
(2, 20 * self.balance_factor), (2, 20 * self.balance_factor),
@@ -203,13 +203,13 @@ impl ExtBuilder {
(5, 50 * self.balance_factor), (5, 50 * self.balance_factor),
(6, 60 * self.balance_factor) (6, 60 * self.balance_factor)
], ],
}), },
elections: Some(elections::GenesisConfig::<Test>{ elections: elections::GenesisConfig::<Test>{
members: vec![], members: vec![],
desired_seats: self.desired_seats, desired_seats: self.desired_seats,
presentation_duration: 2, presentation_duration: 2,
term_duration: 5, term_duration: 5,
}), },
}.build_storage().unwrap().into(); }.build_storage().unwrap().into();
ext.execute_with(|| System::set_block_number(1)); ext.execute_with(|| System::set_block_number(1));
ext ext
+4 -4
View File
@@ -814,14 +814,14 @@ mod tests {
pub fn new_test_ext() -> sp_io::TestExternalities { pub fn new_test_ext() -> sp_io::TestExternalities {
let t = GenesisConfig { let t = GenesisConfig {
// We use default for brevity, but you can configure as desired if needed. // We use default for brevity, but you can configure as desired if needed.
frame_system: Some(Default::default()), frame_system: Default::default(),
pallet_balances: Some(Default::default()), pallet_balances: Default::default(),
pallet_example: Some(pallet_example::GenesisConfig { pallet_example: pallet_example::GenesisConfig {
dummy: 42, dummy: 42,
// we configure the map with (key, value) pairs. // we configure the map with (key, value) pairs.
bar: vec![(1, 2), (2, 3)], bar: vec![(1, 2), (2, 3)],
foo: 24, foo: 24,
}), },
}.build_storage().unwrap(); }.build_storage().unwrap();
t.into() t.into()
} }
+10 -12
View File
@@ -82,7 +82,7 @@ macro_rules! impl_outer_config {
#[serde(deny_unknown_fields)] #[serde(deny_unknown_fields)]
pub struct $main { pub struct $main {
$( $(
pub [< $snake $(_ $instance )? >]: Option<$config>, pub [< $snake $(_ $instance )? >]: $config,
)* )*
} }
#[cfg(any(feature = "std", test))] #[cfg(any(feature = "std", test))]
@@ -92,15 +92,13 @@ macro_rules! impl_outer_config {
storage: &mut $crate::sp_runtime::Storage, storage: &mut $crate::sp_runtime::Storage,
) -> std::result::Result<(), String> { ) -> std::result::Result<(), String> {
$( $(
if let Some(ref extra) = self.[< $snake $(_ $instance )? >] { $crate::impl_outer_config! {
$crate::impl_outer_config! { @CALL_FN
@CALL_FN $concrete;
$concrete; $snake;
$snake; $( $instance )?;
$( $instance )?; &self.[< $snake $(_ $instance )? >];
extra; storage;
storage;
}
} }
)* )*
@@ -117,7 +115,7 @@ macro_rules! impl_outer_config {
$runtime:ident; $runtime:ident;
$module:ident; $module:ident;
$instance:ident; $instance:ident;
$extra:ident; $extra:expr;
$storage:ident; $storage:ident;
) => { ) => {
$crate::sp_runtime::BuildModuleGenesisStorage::<$runtime, $module::$instance>::build_module_genesis_storage( $crate::sp_runtime::BuildModuleGenesisStorage::<$runtime, $module::$instance>::build_module_genesis_storage(
@@ -129,7 +127,7 @@ macro_rules! impl_outer_config {
$runtime:ident; $runtime:ident;
$module:ident; $module:ident;
; ;
$extra:ident; $extra:expr;
$storage:ident; $storage:ident;
) => { ) => {
$crate::sp_runtime::BuildModuleGenesisStorage:: $crate::sp_runtime::BuildModuleGenesisStorage::
+10 -10
View File
@@ -291,26 +291,26 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic<u32, Call, Signature,
fn new_test_ext() -> sp_io::TestExternalities { fn new_test_ext() -> sp_io::TestExternalities {
GenesisConfig{ GenesisConfig{
module1_Instance1: Some(module1::GenesisConfig { module1_Instance1: module1::GenesisConfig {
value: 3, value: 3,
test: 2, test: 2,
}), },
module1_Instance2: Some(module1::GenesisConfig { module1_Instance2: module1::GenesisConfig {
value: 4, value: 4,
test: 5, test: 5,
}), },
module2: Some(module2::GenesisConfig { module2: module2::GenesisConfig {
value: 4, value: 4,
map: vec![(0, 0)], map: vec![(0, 0)],
double_map: vec![(0, 0, 0)], double_map: vec![(0, 0, 0)],
}), },
module2_Instance1: Some(module2::GenesisConfig { module2_Instance1: module2::GenesisConfig {
value: 4, value: 4,
map: vec![(0, 0)], map: vec![(0, 0)],
double_map: vec![(0, 0, 0)], double_map: vec![(0, 0, 0)],
}), },
module2_Instance2: None, module2_Instance2: Default::default(),
module2_Instance3: None, module2_Instance3: Default::default(),
}.build_storage().unwrap().into() }.build_storage().unwrap().into()
} }
@@ -185,9 +185,9 @@ frame_support::construct_runtime!(
#[test] #[test]
fn create_genesis_config() { fn create_genesis_config() {
GenesisConfig { GenesisConfig {
module: Some(module::GenesisConfig { module: module::GenesisConfig {
request_life_time: 0, request_life_time: 0,
enable_storage_role: true, enable_storage_role: true,
}) }
}; };
} }