From 566706dd64bd3816e05c7db8bf1917f23cc96b5d Mon Sep 17 00:00:00 2001
From: Michal Kucharczyk <1728078+michalkucharczyk@users.noreply.github.com>
Date: Thu, 7 Nov 2024 15:01:21 +0100
Subject: [PATCH] gensis-config: patching default `RuntimeGenesisConfig` fixed
(#6382)
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>
---
prdoc/pr_6382.prdoc | 12 +++++++++
.../bin/utils/chain-spec-builder/src/lib.rs | 16 ++++++------
.../chain-spec/src/genesis_config_builder.rs | 4 +--
substrate/client/chain-spec/src/json_patch.rs | 25 ++++++++++++-------
4 files changed, 37 insertions(+), 20 deletions(-)
create mode 100644 prdoc/pr_6382.prdoc
diff --git a/prdoc/pr_6382.prdoc b/prdoc/pr_6382.prdoc
new file mode 100644
index 00000000000..ac6821c1100
--- /dev/null
+++ b/prdoc/pr_6382.prdoc
@@ -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
diff --git a/substrate/bin/utils/chain-spec-builder/src/lib.rs b/substrate/bin/utils/chain-spec-builder/src/lib.rs
index 6f3128ed7eb..73c2868b331 100644
--- a/substrate/bin/utils/chain-spec-builder/src/lib.rs
+++ b/substrate/bin/utils/chain-spec-builder/src/lib.rs
@@ -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)
diff --git a/substrate/client/chain-spec/src/genesis_config_builder.rs b/substrate/client/chain-spec/src/genesis_config_builder.rs
index 66989495d42..5fe8f9dc053 100644
--- a/substrate/client/chain-spec/src/genesis_config_builder.rs
+++ b/substrate/client/chain-spec/src/genesis_config_builder.rs
@@ -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> {
diff --git a/substrate/client/chain-spec/src/json_patch.rs b/substrate/client/chain-spec/src/json_patch.rs
index c3930069a60..a223792374e 100644
--- a/substrate/client/chain-spec/src/json_patch.rs
+++ b/substrate/client/chain-spec/src/json_patch.rs
@@ -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
///
@@ -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,
};
@@ -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",
@@ -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
+ }
+ })
+ );
}
}
--
GitLab