From bc3cd63be52a8ca8c5f7bbc5e57673793de3b092 Mon Sep 17 00:00:00 2001
From: Chris O'Neil <chriso83@protonmail.com>
Date: Thu, 2 May 2024 15:18:05 +0100
Subject: [PATCH] fix: do not create wallet on registry refresh

Use the `try_load_from` mechanism, rather than `try_load`, because the former will not create the
wallet directory if it does not exist.

The node manager refreshes its registry when it runs commands like `start` and `stop`, which can be
running as the root user. The refresh now involves getting the balance of the wallet. Therefore,
when `start` ran for the first time, the `try_load` function was creating a directory owned by the
root user. This would cause the node to crash because it couldn't write to that directory.

A new Clippy warning is also fixed and I removed the superfluous comments around the code it was
referring to.
---
 sn_logging/src/lib.rs                   | 9 +++------
 sn_node_manager/src/add_services/mod.rs | 4 +++-
 sn_node_manager/src/lib.rs              | 2 +-
 3 files changed, 7 insertions(+), 8 deletions(-)

diff --git a/sn_logging/src/lib.rs b/sn_logging/src/lib.rs
index 39a4c868e7..7778507b9f 100644
--- a/sn_logging/src/lib.rs
+++ b/sn_logging/src/lib.rs
@@ -37,16 +37,13 @@ fn current_exe_name() -> String {
         .next()
         .and_then(|arg| {
             std::path::Path::new(&arg).file_name().map(|s| {
-                let mut name = s.to_string_lossy().into_owned();
-                // remove sn_ prefix if present
-                name = name.strip_prefix("sn_").unwrap_or(&name).to_owned();
+                let mut name = s.to_string_lossy().to_string();
+                name = name.strip_prefix("sn_").unwrap_or(&name).to_string();
 
-                // remove .exe prefix on windows
                 if cfg!(windows) && name.to_lowercase().ends_with(".exe") {
-                    name = name.strip_suffix(".exe").unwrap_or(&name).to_owned();
+                    name = name.strip_suffix(".exe").unwrap_or(&name).to_string();
                 }
 
-                // if the name is safe, identify it is the client
                 if name == "safe" {
                     name = "client".to_string();
                 }
diff --git a/sn_node_manager/src/add_services/mod.rs b/sn_node_manager/src/add_services/mod.rs
index 39e0b110dd..83e81eaeca 100644
--- a/sn_node_manager/src/add_services/mod.rs
+++ b/sn_node_manager/src/add_services/mod.rs
@@ -96,7 +96,9 @@ pub async fn add_node(
         }
 
         if options.env_variables.is_some() {
-            node_registry.environment_variables = options.env_variables.clone();
+            node_registry
+                .environment_variables
+                .clone_from(&options.env_variables);
             should_save = true;
         }
 
diff --git a/sn_node_manager/src/lib.rs b/sn_node_manager/src/lib.rs
index a3a01320f6..233a9d0cc5 100644
--- a/sn_node_manager/src/lib.rs
+++ b/sn_node_manager/src/lib.rs
@@ -429,7 +429,7 @@ pub async fn refresh_node_registry(
     for node in &mut node_registry.nodes {
         // The `status` command can run before a node is started and therefore before its wallet
         // exists.
-        match HotWallet::load_from(&node.data_dir_path) {
+        match HotWallet::try_load_from(&node.data_dir_path) {
             Ok(wallet) => node.reward_balance = Some(wallet.balance()),
             Err(_) => node.reward_balance = None,
         }