feat(starknet_integration_tests): share code between update revert co…

…nfig and constructor

crates/starknet_integration_tests/src/config_utils.rs

@@ -1,47 +1,8 @@
-use std::path::PathBuf;
-
-use papyrus_config::dumping::{combine_config_map_and_pointers, SerializeConfig};
 use serde_json::Value;
-use starknet_sequencer_node::config::config_utils::{
-    config_to_preset,
-    dump_json_data,
-    RequiredParams,
-};
-use starknet_sequencer_node::config::node_config::{
-    SequencerNodeConfig,
-    CONFIG_NON_POINTERS_WHITELIST,
-    CONFIG_POINTERS,
-};
 
 // TODO(Tsabary): Move here all config-related functions from "integration_test_utils.rs".
 
-const NODE_CONFIG_CHANGES_FILE_PATH: &str = "node_integration_test_config_changes.json";
-
-/// Creates a config file for the sequencer node for an integration test.
-pub(crate) fn dump_config_file_changes(
-    config: &SequencerNodeConfig,
-    required_params: RequiredParams,
-    dir: PathBuf,
-) -> PathBuf {
-    // Create the entire mapping of the config and the pointers, without the required params.
-    let config_as_map = combine_config_map_and_pointers(
-        config.dump(),
-        &CONFIG_POINTERS,
-        &CONFIG_NON_POINTERS_WHITELIST,
-    )
-    .unwrap();
-
-    // Extract only the required fields from the config map.
-    let mut preset = config_to_preset(&config_as_map);
-
-    // Add the required params to the preset.
-    add_required_params_to_preset(&mut preset, required_params.as_json());
-
-    // Dump the preset to a file, return its path.
-    let node_config_path = dump_json_data(preset, NODE_CONFIG_CHANGES_FILE_PATH, dir);
-    assert!(node_config_path.exists(), "File does not exist: {:?}", node_config_path);
-    node_config_path
-}
+pub const NODE_CONFIG_CHANGES_FILE_PATH: &str = "node_integration_test_config_changes.json";
 
 /// Merges required parameters into an existing preset JSON object.
 ///
@@ -62,7 +23,7 @@ pub(crate) fn dump_config_file_changes(
 /// # Panics
 /// This function panics if either `preset` or `required_params` is not a JSON dictionary object, or
 /// if the `preset` already contains a key from the `required_params`.
-fn add_required_params_to_preset(preset: &mut Value, required_params: Value) {
+pub fn add_required_params_to_preset(preset: &mut Value, required_params: Value) {
     if let (Value::Object(preset_map), Value::Object(required_params_map)) =
         (preset, required_params)
     {

crates/starknet_integration_tests/src/integration_test_setup.rs

@@ -1,11 +1,10 @@
-use std::fs;
 use std::net::SocketAddr;
 use std::path::{Path, PathBuf};
 
 use blockifier::context::ChainInfo;
 use mempool_test_utils::starknet_api_test_utils::AccountTransactionGenerator;
+use papyrus_config::dumping::{combine_config_map_and_pointers, SerializeConfig};
 use papyrus_storage::StorageConfig;
-use serde_json::Value;
 use starknet_api::block::BlockNumber;
 use starknet_api::rpc_transaction::RpcTransaction;
 use starknet_api::transaction::TransactionHash;
@@ -18,13 +17,23 @@ use starknet_mempool_p2p::config::MempoolP2pConfig;
 use starknet_monitoring_endpoint::config::MonitoringEndpointConfig;
 use starknet_monitoring_endpoint::test_utils::MonitoringClient;
 use starknet_sequencer_node::config::component_config::ComponentConfig;
+use starknet_sequencer_node::config::config_utils::{
+    config_to_preset,
+    dump_json_data,
+    RequiredParams,
+};
+use starknet_sequencer_node::config::node_config::{
+    SequencerNodeConfig,
+    CONFIG_NON_POINTERS_WHITELIST,
+    CONFIG_POINTERS,
+};
 use starknet_sequencer_node::test_utils::node_runner::NodeRunner;
 use starknet_state_sync::config::StateSyncConfig;
 use tempfile::{tempdir, TempDir};
 use tokio::fs::create_dir_all;
 use tracing::instrument;
 
-use crate::config_utils::dump_config_file_changes;
+use crate::config_utils::{add_required_params_to_preset, NODE_CONFIG_CHANGES_FILE_PATH};
 use crate::state_reader::StorageTestSetup;
 use crate::utils::{create_node_config, spawn_local_success_recorder};
 
@@ -87,6 +96,10 @@ pub struct ExecutableSetup {
     state_sync_storage_handle: Option<TempDir>,
     #[allow(dead_code)]
     class_manager_storage_handles: Option<FileHandles>,
+    // The content of the configuration file.
+    config: SequencerNodeConfig,
+    // The required params of the configuration file.
+    required_params: RequiredParams,
 }
 
 // TODO(Tsabary/ Nadin): reduce number of args.
@@ -142,7 +155,7 @@ impl ExecutableSetup {
             component_config,
         );
 
-        let (node_config_dir, node_config_dir_handle) = match config_path_dir {
+        let (mut node_config_path, node_config_dir_handle) = match config_path_dir {
             Some(config_path_dir) => {
                 create_dir_all(&config_path_dir).await.unwrap();
                 (config_path_dir, None)
@@ -152,7 +165,7 @@ impl ExecutableSetup {
                 (node_config_dir_handle.path().to_path_buf(), Some(node_config_dir_handle))
             }
         };
-        let node_config_path = dump_config_file_changes(&config, required_params, node_config_dir);
+        node_config_path.push(NODE_CONFIG_CHANGES_FILE_PATH);
 
         // Wait for the node to start.
         let MonitoringEndpointConfig { ip, port, .. } = config.monitoring_endpoint_config;
@@ -161,18 +174,22 @@ impl ExecutableSetup {
         let HttpServerConfig { ip, port } = config.http_server_config;
         let add_tx_http_client = HttpTestClient::new(SocketAddr::from((ip, port)));
 
-        Self {
+        let result = Self {
             node_execution_id,
             add_tx_http_client,
             monitoring_client,
             batcher_storage_handle,
-            batcher_storage_config: config.batcher_config.storage,
+            batcher_storage_config: config.batcher_config.storage.clone(),
             node_config_dir_handle,
             node_config_path,
             state_sync_storage_handle,
-            state_sync_storage_config: config.state_sync_config.storage_config,
+            state_sync_storage_config: config.state_sync_config.storage_config.clone(),
             class_manager_storage_handles,
-        }
+            config,
+            required_params,
+        };
+        result.update_config_file();
+        result
     }
 
     pub async fn assert_add_tx_success(&self, tx: RpcTransaction) -> TransactionHash {
@@ -181,24 +198,63 @@ impl ExecutableSetup {
 
     // TODO(noamsp): Change this into change_config once we need to change other values in the
     // config.
-    pub fn update_revert_config(&self, value: Option<BlockNumber>) {
-        let config_path = self.node_config_path.clone();
-        let config_path = config_path.to_str().unwrap();
-        let mut data = fs::read_to_string(config_path).unwrap();
-        let mut json_data: Value = serde_json::from_str(&data).unwrap();
-
+    pub fn update_revert_config(&mut self, value: Option<BlockNumber>) {
         match value {
             Some(value) => {
-                json_data["revert_config.revert_up_to_and_including"] = Value::from(value.0);
-                json_data["revert_config.should_revert"] = Value::from(true);
+                self.config.state_sync_config.revert_config.revert_up_to_and_including = value;
+                self.config.consensus_manager_config.revert_config.revert_up_to_and_including =
+                    value;
+                self.config.state_sync_config.revert_config.should_revert = true;
+                self.config.consensus_manager_config.revert_config.should_revert = true;
             }
             // If should revert is false, the revert_up_to_and_including value is irrelevant.
             None => {
-                json_data["revert_config.should_revert"] = Value::from(false);
+                self.config.state_sync_config.revert_config.should_revert = false;
+                self.config.consensus_manager_config.revert_config.should_revert = false;
             }
         }
+        self.update_config_file();
+    }
 
-        data = serde_json::to_string_pretty(&json_data).unwrap();
-        fs::write(config_path, data).unwrap();
+    fn update_config_file(&self) {
+        // Create the entire mapping of the config and the pointers, without the required params.
+        let config_as_map = combine_config_map_and_pointers(
+            self.config.dump(),
+            &CONFIG_POINTERS,
+            &CONFIG_NON_POINTERS_WHITELIST,
+        )
+        .unwrap();
+
+        // Extract only the required fields from the config map.
+        let mut preset = config_to_preset(&config_as_map);
+
+        // Add the required params to the preset.
+        add_required_params_to_preset(&mut preset, self.required_params.as_json());
+
+        // Dump the preset to a file, return its path.
+        let node_config_path = dump_json_data(
+            preset,
+            // TODO(Itay): Change dump_json_data to receive the path.
+            self.node_config_path
+                .as_path()
+                .file_name()
+                .unwrap_or_else(|| {
+                    panic!("Config path {:?} has no file name", self.node_config_path)
+                })
+                .to_str()
+                .unwrap_or_else(|| {
+                    panic!(
+                        "Failed converting {:?} to a string",
+                        self.node_config_path.as_path().file_name(),
+                    )
+                }),
+            self.node_config_path
+                .parent()
+                .unwrap_or_else(|| panic!("Config path {:?} has no parent.", self.node_config_path))
+                .to_path_buf(),
+        );
+        assert!(node_config_path.exists(), "File does not exist: {:?}", node_config_path);
+        // TODO(Itay): Change dump_json_data to receive the path and remove this assertion.
+        assert_eq!(node_config_path, self.node_config_path);
     }
 }

crates/starknet_integration_tests/src/sequencer_manager.rs

@@ -233,8 +233,8 @@ impl IntegrationTestManager {
 
     pub fn update_revert_config_to_all_idle_nodes(&mut self, value: Option<BlockNumber>) {
         info!("Updating revert config to all idle nodes.");
-        self.idle_nodes.values().for_each(|idle_node| {
-            idle_node.executables.iter().for_each(|executable| {
+        self.idle_nodes.values_mut().for_each(|idle_node| {
+            idle_node.executables.iter_mut().for_each(|executable| {
                 executable.update_revert_config(value);
             });
         });