chore: public_address for TCP node has higher priority from env/config (

#3600)

Description
---
The tcp `public_address` read from env/config (if set) instead of `base_node_id.json`.
I've added a comment to the newly generated `base_node_id.json` that this value is overwritten.
I've chosen json5 so it's backwards compatible.

How Has This Been Tested?
---
Manually.

applications/tari_app_utilities/Cargo.toml

@@ -16,6 +16,7 @@ futures = { version = "^0.3.16", default-features = false, features = ["alloc"]
 qrcode = { version = "0.12" }
 dirs-next = "1.0.2"
 serde_json = "1.0"
+json5 = "0.2.2"
 log = { version = "0.4.8", features = ["std"] }
 rand = "0.8"
 tokio = { version = "1.11", features = ["signal"] }

applications/tari_app_utilities/src/identity_management.rs

@@ -22,8 +22,11 @@
 
 use log::*;
 use rand::rngs::OsRng;
-use std::{clone::Clone, fs, path::Path, string::ToString, sync::Arc};
-use tari_common::{configuration::bootstrap::prompt, exit_codes::ExitCodes};
+use std::{clone::Clone, fs, path::Path, str::FromStr, string::ToString, sync::Arc};
+use tari_common::{
+    configuration::{bootstrap::prompt, utils::get_local_ip},
+    exit_codes::ExitCodes,
+};
 use tari_common_types::types::PrivateKey;
 use tari_comms::{multiaddr::Multiaddr, peer_manager::PeerFeatures, NodeIdentity};
 use tari_crypto::{
@@ -44,12 +47,18 @@ pub const LOG_TARGET: &str = "tari_application";
 /// A NodeIdentity wrapped in an atomic reference counter on success, the exit code indicating the reason on failure
 pub fn setup_node_identity<P: AsRef<Path>>(
     identity_file: P,
-    public_address: &Multiaddr,
+    public_address: &Option<Multiaddr>,
     create_id: bool,
     peer_features: PeerFeatures,
 ) -> Result<Arc<NodeIdentity>, ExitCodes> {
     match load_identity(&identity_file) {
-        Ok(id) => Ok(Arc::new(id)),
+        Ok(id) => match public_address {
+            Some(public_address) => {
+                id.set_public_address(public_address.clone());
+                Ok(Arc::new(id))
+            },
+            None => Ok(Arc::new(id)),
+        },
         Err(e) => {
             if !create_id {
                 let prompt = prompt("Node identity does not exist.\nWould you like to to create one (Y/n)?");
@@ -117,7 +126,7 @@ pub fn load_identity<P: AsRef<Path>>(path: P) -> Result<NodeIdentity, String> {
             e.to_string()
         )
     })?;
-    let id = NodeIdentity::from_json(&id_str).map_err(|e| {
+    let id = json5::from_str::<NodeIdentity>(&id_str).map_err(|e| {
         format!(
             "The node identity file, {}, has an error. {}",
             path.as_ref().to_str().unwrap_or("?"),
@@ -141,11 +150,20 @@ pub fn load_identity<P: AsRef<Path>>(path: P) -> Result<NodeIdentity, String> {
 /// Result containing the node identity, string will indicate reason on error
 pub fn create_new_identity<P: AsRef<Path>>(
     path: P,
-    public_addr: Multiaddr,
+    public_addr: Option<Multiaddr>,
     features: PeerFeatures,
 ) -> Result<NodeIdentity, String> {
     let private_key = PrivateKey::random(&mut OsRng);
-    let node_identity = NodeIdentity::new(private_key, public_addr, features);
+    let node_identity = NodeIdentity::new(
+        private_key,
+        match public_addr {
+            Some(public_addr) => public_addr,
+            None => format!("{}/tcp/18141", get_local_ip().ok_or("Can't get local ip address")?)
+                .parse()
+                .map_err(|e: <Multiaddr as FromStr>::Err| e.to_string())?,
+        },
+        features,
+    );
     save_as_json(path, &node_identity)?;
     Ok(node_identity)
 }
@@ -203,7 +221,12 @@ pub fn save_as_json<P: AsRef<Path>, T: MessageFormat>(path: P, object: &T) -> Re
             fs::create_dir_all(p).map_err(|e| format!("Could not save json to data folder. {}", e.to_string()))?;
         }
     }
-    fs::write(path.as_ref(), json.as_bytes()).map_err(|e| {
+    let json_with_comment = format!(
+        "// The public address is overwritten by the config/environment variable \
+         (TARI_BASE_NODE__<network>__PUBLIC_ADDRESS)\n{}",
+        json
+    );
+    fs::write(path.as_ref(), json_with_comment.as_bytes()).map_err(|e| {
         format!(
             "Error writing json file, {}. {}",
             path.as_ref().to_str().unwrap_or("<invalid UTF-8>"),

applications/tari_base_node/src/bootstrap.rs

@@ -58,6 +58,7 @@ use tari_p2p::{
     initialization::{P2pConfig, P2pInitializer},
     peer_seeds::SeedPeer,
     services::liveness::{LivenessConfig, LivenessInitializer},
+    transport::TransportType,
 };
 use tari_service_framework::{ServiceHandles, StackBuilder};
 use tari_shutdown::ShutdownSignal;
@@ -184,11 +185,16 @@ where B: BlockchainBackend + 'static
 
         let comms = comms.add_protocol_extension(mempool_protocol);
         let comms = Self::setup_rpc_services(comms, &handles, self.db.into(), config);
-        let comms = initialization::spawn_comms_using_transport(comms, transport_type).await?;
+        let comms = initialization::spawn_comms_using_transport(comms, transport_type.clone()).await?;
         // Save final node identity after comms has initialized. This is required because the public_address can be
         // changed by comms during initialization when using tor.
-        identity_management::save_as_json(&config.base_node_identity_file, &*comms.node_identity())
-            .map_err(|e| anyhow!("Failed to save node identity: {:?}", e))?;
+        match transport_type {
+            TransportType::Tcp { .. } => {}, // Do not overwrite TCP public_address in the base_node_id!
+            _ => {
+                identity_management::save_as_json(&config.base_node_identity_file, &*comms.node_identity())
+                    .map_err(|e| anyhow!("Failed to save node identity: {:?}", e))?;
+            },
+        };
         if let Some(hs) = comms.hidden_service() {
             identity_management::save_as_json(&config.base_node_tor_identity_file, hs.tor_identity())
                 .map_err(|e| anyhow!("Failed to save tor identity: {:?}", e))?;

applications/tari_console_wallet/src/init/mod.rs

@@ -299,7 +299,10 @@ pub async fn init_wallet(
     );
 
     let node_address = match wallet_db.get_node_address().await? {
-        None => config.public_address.clone(),
+        None => config
+            .public_address
+            .clone()
+            .ok_or_else(|| ExitCodes::ConfigError("node public address error".to_string()))?,
         Some(a) => a,
     };
 

common/src/configuration/global.rs

@@ -75,7 +75,7 @@ pub struct GlobalConfig {
     pub pruned_mode_cleanup_interval: u64,
     pub core_threads: Option<usize>,
     pub base_node_identity_file: PathBuf,
-    pub public_address: Multiaddr,
+    pub public_address: Option<Multiaddr>,
     pub grpc_enabled: bool,
     pub grpc_base_node_address: Multiaddr,
     pub grpc_console_wallet_address: Multiaddr,
@@ -328,13 +328,12 @@ fn convert_node_config(
 
     // Public address
     let key = config_string("base_node", net_str, "public_address");
-    let public_address = cfg
-        .get_str(&key)
-        .map_err(|e| ConfigurationError::new(&key, &e.to_string()))
-        .and_then(|addr| {
+    let public_address = optional(cfg.get_str(&key))?
+        .map(|addr| {
             addr.parse::<Multiaddr>()
                 .map_err(|e| ConfigurationError::new(&key, &e.to_string()))
-        })?;
+        })
+        .transpose()?;
 
     // GPRC enabled
     let key = config_string("base_node", net_str, "grpc_enabled");

common/src/configuration/utils.rs

@@ -226,11 +226,6 @@ pub fn default_config(bootstrap: &ConfigBootstrap) -> Config {
         default_subdir("config/base_node_id.json", Some(&bootstrap.base_path)),
     )
     .unwrap();
-    cfg.set_default(
-        "base_node.weatherwax.public_address",
-        format!("{}/tcp/18141", local_ip_addr),
-    )
-    .unwrap();
 
     cfg.set_default("base_node.weatherwax.allow_test_addresses", false)
         .unwrap();
@@ -435,7 +430,7 @@ fn set_transport_defaults(cfg: &mut Config) -> Result<(), config::ConfigError> {
     Ok(())
 }
 
-fn get_local_ip() -> Option<Multiaddr> {
+pub fn get_local_ip() -> Option<Multiaddr> {
     use std::net::IpAddr;
 
     get_if_addrs::get_if_addrs().ok().and_then(|if_addrs| {

integration_tests/helpers/baseNodeProcess.js

@@ -6,6 +6,7 @@ const BaseNodeClient = require("./baseNodeClient");
 const { getFreePort } = require("./util");
 const dateFormat = require("dateformat");
 const { createEnv } = require("./config");
+const JSON5 = require("json5");
 
 let outputProcess;
 class BaseNodeProcess {
@@ -78,7 +79,7 @@ class BaseNodeProcess {
       }
     }
 
-    this.nodeInfo = JSON.parse(
+    this.nodeInfo = JSON5.parse(
       fs.readFileSync(this.baseDir + "/" + this.nodeFile, "utf8")
     );
   }

integration_tests/package.json

@@ -46,6 +46,7 @@
     "dateformat": "^3.0.3",
     "glob": "^7.2.0",
     "hex64": "^0.4.0",
+    "json5": "^2.2.0",
     "jszip": "^3.7.1",
     "nvm": "^0.0.4",
     "sha3": "^2.1.3",