Skip to content

Commit

Permalink
gensis-config: patching default RuntimeGenesisConfig fixed (#6382)
Browse files Browse the repository at this point in the history
This PR changes the behavior of `json_patch::merge` function which no
longer removes any keys from the base JSON object.

fixes: #6306

---------

Co-authored-by: GitHub Action <action@github.com>
  • Loading branch information
michalkucharczyk and actions-user authored Nov 7, 2024
1 parent 1100c18 commit 566706d
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 20 deletions.
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
16 changes: 8 additions & 8 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,19 @@ 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");
map.get_mut("genesis").map(|genesis| {
genesis.as_object_mut().map(|genesis_map| {
genesis_map
.insert("runtimeGenesis".to_string(), serde_json::Value::Null);
});
});
});

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
.get_mut("genesis")
.and_then(|genesis| genesis.as_object_mut())
.and_then(|genesis| genesis.remove("runtimeGenesis"));
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
}
})
);
}
}

0 comments on commit 566706d

Please sign in to comment.