-
Notifications
You must be signed in to change notification settings - Fork 97
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
use RuntimeGenesisConfig in genesis config presets #451
base: main
Are you sure you want to change the base?
Changes from all commits
1ccb486
3f12e7c
0e9243d
dcd050a
fbf97c6
a09b416
83a9076
529a574
d6cca64
098bac9
ac021d9
79a3d5b
1354b29
c3210cf
c4c5f35
4140168
6736951
f725e7a
ef78c8e
b83513a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ | |
|
||
use parachains_common::AuraId; | ||
use polkadot_primitives::{AccountId, AccountPublic}; | ||
use serde_json::Value; | ||
use sp_core::{sr25519, Pair, Public}; | ||
use sp_runtime::traits::IdentifyAccount; | ||
#[cfg(not(feature = "std"))] | ||
|
@@ -66,3 +67,20 @@ where | |
|
||
/// The default XCM version to set in genesis config. | ||
pub const SAFE_XCM_VERSION: u32 = xcm::prelude::XCM_VERSION; | ||
|
||
pub fn remove_phantom_fields(value: &mut Value) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @dharjeezy I think that we don't need to remove anything here, e.g. these phantoms... so I would revert There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @michalkucharczyk wdyt here? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This means i have to open a PR in polkadot-sdk to include the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, PR shall be opened. In that case, IMO we could keep this function until clean-up PR is merged (some annotations would be nice). As alternative (and probably better solution) you could experiment with removing the fields that were not customized from the final JSON (see this discussion). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
@dharjeezy actually, this pallet was fixed already in the master: https://github.com/paritytech/polkadot-sdk/blob/master/bridges/modules/messages/src/lib.rs#L539-L540, but if you have there any others, please feel free to open PR to polkadot-sdk There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i just saw that it is now added, we will have to wait for a new release. |
||
match value { | ||
Value::Object(map) => { | ||
map.remove("phantom"); | ||
|
||
for (_, v) in map.iter_mut() { | ||
remove_phantom_fields(v); | ||
} | ||
}, | ||
Value::Array(arr) => | ||
for v in arr.iter_mut() { | ||
remove_phantom_fields(v); | ||
}, | ||
_ => {}, | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dharjeezy Is it intentional to name all the genesis fields here? Could we just use
..Default::default(),
for them? Every time we add a new pallet to the runtime, we would also need to addxyz = Default::default()
here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@michalkucharczyk wdyt here also?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
..Default::default()
shall be used.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also adding link to related discussion: paritytech/polkadot-sdk#5327 (comment)