refactor: minor stuff

Author: Adrian Nagy

cli/src/commands/node/mod.rs

@@ -138,20 +138,26 @@ pub struct Node {
     #[arg(short = 'c', long, env)]
     pub config: Option<PathBuf>,
 
-    // /// Enable archive mode (seding blocks to the archive process).
-    // #[arg(long, env)]
-    // pub archive_address: Option<Url>,
     /// Enable local precomputed storage.
+    ///
+    /// This option requires the following environment variables to be set:
+    /// - OPENMINA_ARCHIVE_LOCAL_STORAGE_PATH (otherwise the path to the working directory will be used)
     #[arg(long, env)]
     pub archive_local_storage: bool,
 
-    // TODO(adonagy): Sort out this... Do we want to support the ocaml options 1:1?
-    // we could move the addrss to an env var, just like gcp and aws
     /// Enable archiver process.
+    ///
+    /// This requires the following environment variables to be set:
+    /// - OPENMINA_ARCHIVE_ADDRESS
     #[arg(long, env)]
     pub archive_archiver_process: bool,
 
     /// Enable GCP precomputed storage.
+    ///
+    /// This requires the following environment variables to be set:
+    /// - GCP_CREDENTIALS_JSON
+    /// - GCP_BUCKET_NAME
+    ///
     #[arg(long, env)]
     pub archive_gcp_storage: bool,
 
@@ -333,7 +339,6 @@ impl Node {
                 .validate_env_vars()
                 .map_err(|e| anyhow::anyhow!(e))?;
 
-            // TODO(adonagy): add options
             node_builder.archive(archive_storage_options, work_dir.clone());
         }
 

mina-p2p-messages/src/v2/manual.rs

@@ -836,9 +836,10 @@ pub struct PrecomputedBlock {
     pub staged_ledger_diff: StagedLedgerDiffDiffStableV2,
     // FIXME: for some reason in OCaml the base58check conversion for the JSON value
     // uses version byte = 0x05 (ledger hash) instead of 0x10 (StateHash) and 0x11 (StateBodyHash)
+    // Note: keeping the proper types here, we should raise an issue in the ocaml repo
     pub delta_transition_chain_proof: (
-        LedgerHash,       // StateHash
-        List<LedgerHash>, // StateBodyHash
+        StateHash,           // LedgerHash,       // StateHash
+        List<StateBodyHash>, // List<LedgerHash>, // StateBodyHash
     ),
     pub protocol_version: ProtocolVersionStableV2,
     #[serde(default)]

node/common/src/service/archive/gcp.rs

@@ -12,24 +12,31 @@ pub(crate) struct ArchiveGCPClient {
 
 impl ArchiveGCPClient {
     pub async fn new() -> Result<Self, Error> {
-        let cred_file = env::var("GCP_CREDENTIALS_JSON")
-            .map_err(|_| Error::EnvironmentVariableNotSet("GCP_CREDENTIALS_JSON".to_string()))?;
-        let bucket_name = env::var("GCP_BUCKET_NAME")
-            .map_err(|_| Error::EnvironmentVariableNotSet("GCP_BUCKET_NAME".to_string()))?;
-        let credentials = GcpCredentialsFile::new_from_file(cred_file).await.unwrap();
+        let get_env_var = |var: &str| -> Result<String, Error> {
+            env::var(var).map_err(|_| Error::EnvironmentVariableNotSet(var.to_string()))
+        };
+
+        let cred_file = get_env_var("GCP_CREDENTIALS_JSON")?;
+        let bucket_name = get_env_var("GCP_BUCKET_NAME")?;
+
+        let credentials = GcpCredentialsFile::new_from_file(cred_file)
+            .await
+            .map_err(|e| Error::UploadError(format!("GCP credentials error: {}", e)))?;
+
         let config = gcs::client::ClientConfig::default()
             .with_credentials(credentials)
             .await
-            .unwrap();
-        let client = gcs::client::Client::new(config);
+            .map_err(|e| Error::UploadError(format!("GCP config error: {}", e)))?;
+
         Ok(ArchiveGCPClient {
-            client,
+            client: gcs::client::Client::new(config),
             bucket_name,
         })
     }
 
     pub async fn upload_block(&self, key: &str, data: &[u8]) -> Result<(), Error> {
         let upload_type = gcs_upload::UploadType::Simple(gcs_upload::Media::new(key.to_string()));
+
         self.client
             .upload_object(
                 &gcs_upload::UploadObjectRequest {
@@ -40,7 +47,8 @@ impl ArchiveGCPClient {
                 &upload_type,
             )
             .await
-            .map_err(|e| Error::UploadError(e.to_string()))?;
+            .map_err(|e| Error::UploadError(format!("GCP upload failed: {}", e)))?;
+
         Ok(())
     }
 }

node/common/src/service/archive/mod.rs

@@ -114,7 +114,9 @@ impl ArchiveServiceClients {
             let precomputed_block: PrecomputedBlock = match breadcrumb.try_into() {
                 Ok(block) => block,
                 Err(_) => {
-                    node::core::warn!(summary = "Failed to convert breadcrumb to precomputed block");
+                    node::core::warn!(
+                        summary = "Failed to convert breadcrumb to precomputed block"
+                    );
                     return;
                 }
             };
@@ -131,11 +133,19 @@ impl ArchiveServiceClients {
             };
 
             if options.uses_local_precomputed_storage() {
-                // TODO(adonagy): Cleanup the unwraps (fn that returns a Result + log the error)
                 if let Some(path) = &self.local_path {
-                    let file_path = Path::new(path).join(key.clone());
-                    let mut file = File::create(file_path).unwrap();
-                    file.write_all(&data).unwrap();
+                    let key_clone = key.clone();
+                    match write_to_local_storage(path, &key, &data) {
+                        Ok(_) => node::core::info!(
+                            summary = "Successfully wrote precomputed block to local storage",
+                            key = key_clone
+                        ),
+                        Err(e) => node::core::warn!(
+                            summary = "Failed to write precomputed block to local storage",
+                            key = key_clone,
+                            error = e.to_string()
+                        ),
+                    }
                 } else {
                     node::core::warn!(summary = "Local precomputed storage path not set");
                 }
@@ -273,3 +283,22 @@ impl node::transition_frontier::archive::archive_service::ArchiveService for Nod
 // Note: Placeholder for the wasm implementation, if we decide to include an archive mode in the future
 #[cfg(target_arch = "wasm32")]
 mod rpc {}
+
+fn write_to_local_storage(base_path: &str, key: &str, data: &[u8]) -> Result<(), Error> {
+    use std::fs::{create_dir_all, File};
+    use std::path::Path;
+
+    let path = Path::new(base_path).join(key);
+    if let Some(parent) = path.parent() {
+        create_dir_all(parent)
+            .map_err(|e| Error::UploadError(format!("Directory creation failed: {}", e)))?;
+    }
+
+    let mut file = File::create(&path)
+        .map_err(|e| Error::UploadError(format!("File creation failed: {}", e)))?;
+
+    file.write_all(data)
+        .map_err(|e| Error::UploadError(format!("File write failed: {}", e)))?;
+
+    Ok(())
+}

node/src/ledger/write/mod.rs

@@ -194,11 +194,7 @@ impl TryFrom<&BlockApplyResult> for v2::PrecomputedBlock {
                 .clone()
                 .into(),
             staged_ledger_diff: value.block.body().staged_ledger_diff.clone(),
-            // TODO(adonagy): add the actual delta transition chain proof
-            delta_transition_chain_proof: (
-                mina_p2p_messages::v2::LedgerHash::zero(),
-                mina_p2p_messages::list::List::new(),
-            ),
+            delta_transition_chain_proof: value.block.header().delta_block_chain_proof.clone(),
             protocol_version: value.block.header().current_protocol_version.clone(),
             proposed_protocol_version: None,
             accounts_accessed: archive_transition_frontier_diff.accounts_accessed(),