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

gensis-config: patching default RuntimeGenesisConfig fixed #6382

Merged
merged 17 commits into from
Nov 7, 2024
Merged
Show file tree
Hide file tree
Changes from 12 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
12 changes: 12 additions & 0 deletions prdoc/pr_6382.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
title: 'gensis-config: patching default `RuntimeGenesisConfig` fixed'
doc:
- audience: Node Dev
description: |-
This PR fixes issue reported in #6306.
It changes the behavior of `sc_chain_spec::json_patch::merge` function which no longer removes any keys from the base JSON object.

crates:
- name: staging-chain-spec-builder
bump: major
- name: sc-chain-spec
bump: major
18 changes: 11 additions & 7 deletions substrate/bin/utils/chain-spec-builder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,19 +261,23 @@ impl ChainSpecBuilder {
.map_err(|e| format!("Conversion to json failed: {e}"))?;

// We want to extract only raw genesis ("genesis::raw" key), and apply it as a patch
// for the original json file. However, the file also contains original plain
// genesis ("genesis::runtimeGenesis") so set it to null so the patch will erase it.
// for the original json file.
genesis_json.as_object_mut().map(|map| {
map.retain(|key, _| key == "genesis");
});

let mut org_chain_spec_json = extract_chain_spec_json(input_chain_spec.as_path())?;

// The original plain genesis ("genesis::runtimeGenesis") is no longer needed, so
// just remove it:
org_chain_spec_json.as_object_mut().map(|map| {
map.get_mut("genesis").map(|genesis| {
genesis.as_object_mut().map(|genesis_map| {
genesis_map
.insert("runtimeGenesis".to_string(), serde_json::Value::Null);
});
genesis
.as_object_mut()
.map(|genesis_map| genesis_map.remove("runtimeGenesis"));
michalkucharczyk marked this conversation as resolved.
Show resolved Hide resolved
});
});

let mut org_chain_spec_json = extract_chain_spec_json(input_chain_spec.as_path())?;
json_patch::merge(&mut org_chain_spec_json, genesis_json);

let chain_spec_json = serde_json::to_string_pretty(&org_chain_spec_json)
Expand Down
4 changes: 1 addition & 3 deletions substrate/client/chain-spec/src/genesis_config_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,9 @@ where
/// The patching process modifies the default `RuntimeGenesisConfig` according to the following
/// rules:
/// 1. Existing keys in the default configuration will be overridden by the corresponding values
/// in the patch.
/// in the patch (also applies to `null` values).
/// 2. If a key exists in the patch but not in the default configuration, it will be added to
/// the resulting `RuntimeGenesisConfig`.
/// 3. Keys in the default configuration that have null values in the patch will be removed from
/// the resulting `RuntimeGenesisConfig`. This is helpful for changing enum variant value.
///
/// Please note that the patch may contain full `RuntimeGenesisConfig`.
pub fn get_storage_for_patch(&self, patch: Value) -> core::result::Result<Storage, String> {
Expand Down
25 changes: 16 additions & 9 deletions substrate/client/chain-spec/src/json_patch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,9 +22,10 @@ use serde_json::Value;

/// Recursively merges two JSON objects, `a` and `b`, into a single object.
///
/// If a key exists in both objects, the value from `b` will override the value from `a`.
/// If a key exists in `b` with a `null` value, it will be removed from `a`.
/// If a key exists in both objects, the value from `b` will override the value from `a` (also if
/// value in `b` is `null`).
/// If a key exists only in `b` and not in `a`, it will be added to `a`.
/// No keys will be removed from `a`.
///
/// # Arguments
///
Expand All @@ -34,11 +35,7 @@ pub fn merge(a: &mut Value, b: Value) {
match (a, b) {
(Value::Object(a), Value::Object(b)) =>
for (k, v) in b {
if v.is_null() {
a.remove(&k);
} else {
merge(a.entry(k).or_insert(Value::Null), v);
}
merge(a.entry(k).or_insert(Value::Null), v);
},
(a, b) => *a = b,
};
Expand Down Expand Up @@ -166,7 +163,7 @@ mod tests {
}

#[test]
fn test6_patch_removes_keys_if_null() {
fn test6_patch_does_not_remove_keys_if_null() {
let mut j1 = json!({
"a": {
"name": "xxx",
Expand All @@ -186,6 +183,16 @@ mod tests {
});

merge(&mut j1, j2);
assert_eq!(j1, json!({ "a": {"name":"xxx", "value":456, "enum_variant_2": 32 }}));
assert_eq!(
j1,
json!({
"a": {
"name":"xxx",
"value":456,
"enum_variant_1": null,
"enum_variant_2": 32
}
})
);
}
}
Loading