chore(config): re-enable config pointers with a new whitelist

commit-id:e4597e85

crates/papyrus_config/src/config_test.rs

@@ -1,4 +1,4 @@
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashSet};
 use std::env;
 use std::fs::File;
 use std::path::PathBuf;
@@ -257,24 +257,41 @@ fn test_pointers_flow() {
     const POINTING_PARAM_DESCRIPTION: &str = "This is a.";
     const PUBLIC_POINTING_PARAM_NAME: &str = "public_a.a";
     const PRIVATE_POINTING_PARAM_NAME: &str = "private_a.a";
+    const WHITELISTED_POINTING_PARAM_NAME: &str = "non_pointing.a";
+    const VALUE: usize = 5;
 
     let config_map = BTreeMap::from([
         ser_param(
             PUBLIC_POINTING_PARAM_NAME,
-            &json!(5),
+            &json!(VALUE),
             POINTING_PARAM_DESCRIPTION,
             ParamPrivacyInput::Public,
         ),
         ser_param(
             PRIVATE_POINTING_PARAM_NAME,
-            &json!(5),
+            &json!(VALUE),
+            POINTING_PARAM_DESCRIPTION,
+            ParamPrivacyInput::Private,
+        ),
+        ser_param(
+            WHITELISTED_POINTING_PARAM_NAME,
+            &json!(VALUE),
             POINTING_PARAM_DESCRIPTION,
             ParamPrivacyInput::Private,
         ),
     ]);
-    let pointers =
-        vec![ser_pointer_target_param(TARGET_PARAM_NAME, &json!(10), TARGET_PARAM_DESCRIPTION)];
-    let stored_map = combine_config_map_and_pointers(config_map, &pointers).unwrap();
+    let pointers = vec![(
+        ser_pointer_target_param(TARGET_PARAM_NAME, &json!(10), TARGET_PARAM_DESCRIPTION),
+        HashSet::from([
+            PUBLIC_POINTING_PARAM_NAME.to_string(),
+            PRIVATE_POINTING_PARAM_NAME.to_string(),
+        ]),
+    )];
+    let non_pointer_params = HashSet::from([WHITELISTED_POINTING_PARAM_NAME.to_string()]);
+    let stored_map =
+        combine_config_map_and_pointers(config_map, &pointers, &non_pointer_params).unwrap();
+
+    // Assert the pointing parameters are correctly set.
     assert_eq!(
         stored_map[PUBLIC_POINTING_PARAM_NAME],
         json!(SerializedParam {
@@ -291,6 +308,18 @@ fn test_pointers_flow() {
             privacy: ParamPrivacy::Private,
         })
     );
+
+    // Assert the whitelisted parameter is correctly set.
+    assert_eq!(
+        stored_map[WHITELISTED_POINTING_PARAM_NAME],
+        json!(SerializedParam {
+            description: POINTING_PARAM_DESCRIPTION.to_owned(),
+            content: SerializedContent::DefaultValue(json!(VALUE)),
+            privacy: ParamPrivacy::Private,
+        })
+    );
+
+    // Assert the pointed parameter is correctly set as a required parameter.
     assert_eq!(
         stored_map[TARGET_PARAM_NAME],
         json!(SerializedParam {
@@ -316,27 +345,43 @@ fn test_required_pointers_flow() {
     const POINTING_PARAM_DESCRIPTION: &str = "This is b.";
     const PUBLIC_POINTING_PARAM_NAME: &str = "public_b.b";
     const PRIVATE_POINTING_PARAM_NAME: &str = "private_b.b";
+    const WHITELISTED_POINTING_PARAM_NAME: &str = "non_pointing.b";
+    const VALUE: usize = 6;
 
     let config_map = BTreeMap::from([
         ser_param(
             PUBLIC_POINTING_PARAM_NAME,
-            &json!(6),
+            &json!(VALUE),
             POINTING_PARAM_DESCRIPTION,
             ParamPrivacyInput::Public,
         ),
         ser_param(
             PRIVATE_POINTING_PARAM_NAME,
-            &json!(6),
+            &json!(VALUE),
+            POINTING_PARAM_DESCRIPTION,
+            ParamPrivacyInput::Private,
+        ),
+        ser_param(
+            WHITELISTED_POINTING_PARAM_NAME,
+            &json!(VALUE),
             POINTING_PARAM_DESCRIPTION,
             ParamPrivacyInput::Private,
         ),
     ]);
-    let pointers = vec![ser_pointer_target_required_param(
-        REQUIRED_PARAM_NAME,
-        SerializationType::PositiveInteger,
-        REQUIRED_PARAM_DESCRIPTION,
+    let pointers = vec![(
+        ser_pointer_target_required_param(
+            REQUIRED_PARAM_NAME,
+            SerializationType::PositiveInteger,
+            REQUIRED_PARAM_DESCRIPTION,
+        ),
+        HashSet::from([
+            PUBLIC_POINTING_PARAM_NAME.to_string(),
+            PRIVATE_POINTING_PARAM_NAME.to_string(),
+        ]),
     )];
-    let stored_map = combine_config_map_and_pointers(config_map, &pointers).unwrap();
+    let non_pointer_params = HashSet::from([WHITELISTED_POINTING_PARAM_NAME.to_string()]);
+    let stored_map =
+        combine_config_map_and_pointers(config_map, &pointers, &non_pointer_params).unwrap();
 
     // Assert the pointing parameters are correctly set.
     assert_eq!(
@@ -356,6 +401,16 @@ fn test_required_pointers_flow() {
         })
     );
 
+    // Assert the whitelisted parameter is correctly set.
+    assert_eq!(
+        stored_map[WHITELISTED_POINTING_PARAM_NAME],
+        json!(SerializedParam {
+            description: POINTING_PARAM_DESCRIPTION.to_owned(),
+            content: SerializedContent::DefaultValue(json!(VALUE)),
+            privacy: ParamPrivacy::Private,
+        })
+    );
+
     // Assert the pointed parameter is correctly set as a required parameter.
     assert_eq!(
         stored_map[REQUIRED_PARAM_NAME],
@@ -367,6 +422,35 @@ fn test_required_pointers_flow() {
     );
 }
 
+#[test]
+#[should_panic(
+    expected = "The target param should_be_pointing.c should point to c, or to be whitelisted."
+)]
+fn test_missing_pointer_flow() {
+    const TARGET_PARAM_NAME: &str = "c";
+    const TARGET_PARAM_DESCRIPTION: &str = "This is common c.";
+    const PARAM_DESCRIPTION: &str = "This is c.";
+    const NON_POINTING_PARAM_NAME: &str = "should_be_pointing.c";
+
+    // Define a non-pointing parameter and a target pointer such that the parameter name matches the
+    // target.
+    let config_map = BTreeMap::from([ser_param(
+        NON_POINTING_PARAM_NAME,
+        &json!(7),
+        PARAM_DESCRIPTION,
+        ParamPrivacyInput::Private,
+    )]);
+    let pointers = vec![(
+        ser_pointer_target_param(TARGET_PARAM_NAME, &json!(10), TARGET_PARAM_DESCRIPTION),
+        HashSet::new(),
+    )];
+    // Do not whitelist the non-pointing parameter.
+    let non_pointer_params = HashSet::new();
+
+    // Attempt to combine the config map and pointers. This should panic.
+    combine_config_map_and_pointers(config_map, &pointers, &non_pointer_params).unwrap();
+}
+
 #[test]
 fn test_replace_pointers() {
     let (mut config_map, _) = split_values_and_types(BTreeMap::from([ser_param(
@@ -416,7 +500,7 @@ fn load_custom_config(args: Vec<&str>) -> CustomConfig {
     let dir = TempDir::new().unwrap();
     let file_path = dir.path().join("config.json");
     CustomConfig { param_path: "default value".to_owned(), seed: 5 }
-        .dump_to_file(&vec![], file_path.to_str().unwrap())
+        .dump_to_file(&vec![], &HashSet::new(), file_path.to_str().unwrap())
         .unwrap();
 
     load_and_process_config::<CustomConfig>(
@@ -506,7 +590,7 @@ fn load_required_param_path(args: Vec<&str>) -> String {
     let dir = TempDir::new().unwrap();
     let file_path = dir.path().join("config.json");
     RequiredConfig { param_path: "default value".to_owned(), num: 3 }
-        .dump_to_file(&vec![], file_path.to_str().unwrap())
+        .dump_to_file(&vec![], &HashSet::new(), file_path.to_str().unwrap())
         .unwrap();
 
     let loaded_config = load_and_process_config::<CustomConfig>(
@@ -600,7 +684,7 @@ fn deeply_nested_optionals() {
     let dir = TempDir::new().unwrap();
     let file_path = dir.path().join("config2.json");
     Level0 { level0_value: 1, level1: None }
-        .dump_to_file(&vec![], file_path.to_str().unwrap())
+        .dump_to_file(&vec![], &HashSet::new(), file_path.to_str().unwrap())
         .unwrap();
 
     let l0 = load_and_process_config::<Level0>(

crates/papyrus_config/src/dumping.rs

@@ -34,7 +34,7 @@
 //! }
 //! ```
 
-use std::collections::BTreeMap;
+use std::collections::{BTreeMap, HashSet};
 use std::fs::File;
 use std::io::{BufWriter, Write};
 
@@ -54,6 +54,9 @@ use crate::{
     IS_NONE_MARK,
 };
 
+/// Detailing pointers in the config map.
+pub type ConfigPointers = Vec<((ParamPath, SerializedParam), HashSet<ParamPath>)>;
+
 /// Serialization for configs.
 pub trait SerializeConfig {
     /// Conversion of a configuration to a mapping of flattened parameters to their descriptions and
@@ -69,7 +72,7 @@ pub trait SerializeConfig {
     /// # Example
     ///
     /// ```
-    /// # use std::collections::BTreeMap;
+    /// # use std::collections::{BTreeMap, HashSet};
     ///
     /// # use papyrus_config::dumping::{ser_param, SerializeConfig};
     /// # use papyrus_config::{ParamPath, ParamPrivacyInput, SerializedParam};
@@ -94,15 +97,17 @@ pub trait SerializeConfig {
     ///
     /// let dir = TempDir::new().unwrap();
     /// let file_path = dir.path().join("config.json");
-    /// ConfigExample { key: 42 }.dump_to_file(&vec![], file_path.to_str().unwrap());
+    /// ConfigExample { key: 42 }.dump_to_file(&vec![], &HashSet::new(), file_path.to_str().unwrap());
     /// ```
     /// Note, in the case of a None sub configs, its elements will not be included in the file.
     fn dump_to_file(
         &self,
-        config_pointers: &Vec<(ParamPath, SerializedParam)>,
+        config_pointers: &ConfigPointers,
+        non_pointer_params: &HashSet<ParamPath>,
         file_path: &str,
     ) -> Result<(), ConfigError> {
-        let combined_map = combine_config_map_and_pointers(self.dump(), config_pointers)?;
+        let combined_map =
+            combine_config_map_and_pointers(self.dump(), config_pointers, non_pointer_params)?;
 
         // Create file writer.
         let file = File::create(file_path)?;
@@ -293,27 +298,52 @@ pub fn ser_pointer_target_required_param(
 /// the target parameter paths to contain only the name of the target they point to.
 pub(crate) fn combine_config_map_and_pointers(
     mut config_map: BTreeMap<ParamPath, SerializedParam>,
-    pointers: &Vec<(ParamPath, SerializedParam)>,
+    pointers: &ConfigPointers,
+    non_pointer_params: &HashSet<ParamPath>,
 ) -> Result<Value, ConfigError> {
     // Update config with target params.
-    for (target_param, serialized_pointer) in pointers {
+    for ((target_param, serialized_pointer), pointing_params_vec) in pointers {
         // Insert target param.
         config_map.insert(target_param.clone(), serialized_pointer.clone());
 
-        // Update config entries that match the target param as pointers.
-        config_map.iter_mut().for_each(|(param_path, serialized_param)| {
-            // Check if the param is the target param.
-            if param_path.ends_with(format!("{FIELD_SEPARATOR}{target_param}").as_str()) {
-                // Point to the target param.
-                *serialized_param = SerializedParam {
-                    description: serialized_param.description.clone(),
+        // Update pointing params to point at the target param.
+        for pointing_param in pointing_params_vec {
+            let pointing_serialized_param =
+                config_map.get(pointing_param).ok_or(ConfigError::PointerSourceNotFound {
+                    pointing_param: pointing_param.to_owned(),
+                })?;
+            config_map.insert(
+                pointing_param.to_owned(),
+                SerializedParam {
+                    description: pointing_serialized_param.description.clone(),
                     content: SerializedContent::PointerTarget(target_param.to_owned()),
-                    privacy: serialized_param.privacy.clone(),
-                };
-            }
-        });
+                    privacy: pointing_serialized_param.privacy.clone(),
+                },
+            );
+        }
     }
 
+    // Iterate over the config, check that all parameters whose name matches a pointer target either
+    // point at it or are in the whitelist.
+    config_map.iter().for_each(|(param_path, serialized_param)| {
+        for ((target_param, _), _) in pointers {
+            // Check if the param name matches a pointer target, and that it is not in the
+            // whitelist.
+            if param_path.ends_with(format!("{FIELD_SEPARATOR}{target_param}").as_str())
+                && !non_pointer_params.contains(param_path)
+            {
+                // Check that the param points to the target param.
+                assert!(
+                    serialized_param.content
+                        == SerializedContent::PointerTarget(target_param.to_owned()),
+                    "The target param {} should point to {}, or to be whitelisted.",
+                    param_path,
+                    target_param
+                );
+            };
+        }
+    });
+
     Ok(json!(config_map))
 }
 

crates/papyrus_config/src/lib.rs

@@ -7,7 +7,7 @@
 //! # Example
 //!
 //! ```
-//! use std::collections::BTreeMap;
+//! use std::collections::{BTreeMap, HashSet};
 //! use std::fs::File;
 //! use std::path::Path;
 //!
@@ -36,7 +36,7 @@
 //!
 //! let dir = TempDir::new().unwrap();
 //! let file_path = dir.path().join("config.json");
-//! ConfigExample { key: 42 }.dump_to_file(&vec![], file_path.to_str().unwrap());
+//! ConfigExample { key: 42 }.dump_to_file(&vec![], &HashSet::new(), file_path.to_str().unwrap());
 //! let file = File::open(file_path).unwrap();
 //! let loaded_config = load_and_process_config::<ConfigExample>(
 //!     file,

crates/papyrus_node/src/bin/papyrus_dump_config.rs

@@ -5,7 +5,7 @@
 #[cfg(feature = "rpc")]
 use papyrus_config::dumping::SerializeConfig;
 #[cfg(feature = "rpc")]
-use papyrus_node::config::pointers::CONFIG_POINTERS;
+use papyrus_node::config::pointers::{CONFIG_NON_POINTERS_WHITELIST, CONFIG_POINTERS};
 #[cfg(feature = "rpc")]
 use papyrus_node::config::{NodeConfig, DEFAULT_CONFIG_PATH};
 
@@ -15,7 +15,7 @@ use papyrus_node::config::{NodeConfig, DEFAULT_CONFIG_PATH};
 fn main() {
     #[cfg(feature = "rpc")]
     NodeConfig::default()
-        .dump_to_file(&CONFIG_POINTERS, DEFAULT_CONFIG_PATH)
+        .dump_to_file(&CONFIG_POINTERS, &CONFIG_NON_POINTERS_WHITELIST, DEFAULT_CONFIG_PATH)
         .expect("dump to file error");
     // TODO(shahak): Try to find a way to remove this binary altogether when the feature rpc is
     // turned off.

crates/papyrus_node/src/config/config_test.rs

@@ -23,7 +23,7 @@ use tempfile::NamedTempFile;
 use validator::Validate;
 
 #[cfg(feature = "rpc")]
-use crate::config::pointers::CONFIG_POINTERS;
+use crate::config::pointers::{CONFIG_NON_POINTERS_WHITELIST, CONFIG_POINTERS};
 use crate::config::{node_command, NodeConfig, DEFAULT_CONFIG_PATH};
 
 // Returns the required and generated params in config/papyrus/default_config.json with the default
@@ -137,7 +137,13 @@ fn default_config_file_is_up_to_date() {
     // Create a temporary file and dump the default config to it.
     let mut tmp_file_path = env::temp_dir();
     tmp_file_path.push("cfg.json");
-    NodeConfig::default().dump_to_file(&CONFIG_POINTERS, tmp_file_path.to_str().unwrap()).unwrap();
+    NodeConfig::default()
+        .dump_to_file(
+            &CONFIG_POINTERS,
+            &CONFIG_NON_POINTERS_WHITELIST,
+            tmp_file_path.to_str().unwrap(),
+        )
+        .unwrap();
 
     // Read the dumped config from the file.
     let from_code: serde_json::Value =