From 349ac8957bc513cd4110eaac69550ffa0816862b Mon Sep 17 00:00:00 2001 From: Martin Stefcek <35243812+Cifko@users.noreply.github.com> Date: Thu, 13 Apr 2023 13:48:44 +0200 Subject: [PATCH] fix: allow public addresses from command line (#5303) Description --- Add `from_str` to `public_addresses` so it can now be set via command line or envvar. How Has This Been Tested? --- There are cargo test and also manually. What process can a PR reviewer use to test or verify this change? --- You can set `public_addresses` via commandline. Fixes: #5260 Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify --- applications/tari_base_node/src/main.rs | 2 +- .../tari_console_wallet/src/init/mod.rs | 14 +- base_layer/contacts/tests/contacts_service.rs | 4 +- base_layer/p2p/src/config.rs | 5 +- base_layer/wallet/tests/wallet.rs | 6 +- base_layer/wallet_ffi/src/lib.rs | 12 +- common/src/configuration/mod.rs | 2 + common/src/configuration/multiaddr_list.rs | 236 ++++++++++++++++++ .../tests/utils/base_node_process.rs | 11 +- .../tests/utils/wallet_process.rs | 4 +- 10 files changed, 273 insertions(+), 23 deletions(-) create mode 100644 common/src/configuration/multiaddr_list.rs diff --git a/applications/tari_base_node/src/main.rs b/applications/tari_base_node/src/main.rs index 094ddac3a8..ef282c96a5 100644 --- a/applications/tari_base_node/src/main.rs +++ b/applications/tari_base_node/src/main.rs @@ -136,7 +136,7 @@ fn main_inner() -> Result<(), ExitError> { // Load or create the Node identity let node_identity = setup_node_identity( &config.base_node.identity_file, - config.base_node.p2p.public_addresses.clone(), + config.base_node.p2p.public_addresses.clone().into_vec(), cli.non_interactive_mode || cli.init, PeerFeatures::COMMUNICATION_NODE, )?; diff --git a/applications/tari_console_wallet/src/init/mod.rs b/applications/tari_console_wallet/src/init/mod.rs index e7f18d4893..2a3964a88f 100644 --- a/applications/tari_console_wallet/src/init/mod.rs +++ b/applications/tari_console_wallet/src/init/mod.rs @@ -31,6 +31,7 @@ use tari_app_utilities::identity_management::setup_node_identity; use tari_common::{ configuration::{ bootstrap::{grpc_default_port, prompt, ApplicationType}, + MultiaddrList, Network, }, exit_codes::{ExitCode, ExitError}, @@ -378,8 +379,8 @@ pub async fn init_wallet( let node_addresses = if config.wallet.p2p.public_addresses.is_empty() { match wallet_db.get_node_address()? { - Some(addr) => vec![addr], - None => vec![], + Some(addr) => MultiaddrList::from(vec![addr]), + None => MultiaddrList::default(), } } else { config.wallet.p2p.public_addresses.clone() @@ -394,9 +395,14 @@ pub async fn init_wallet( "Node identity overridden by file {}", identity_file.to_string_lossy() ); - setup_node_identity(identity_file, node_addresses, true, PeerFeatures::COMMUNICATION_CLIENT)? + setup_node_identity( + identity_file, + node_addresses.to_vec(), + true, + PeerFeatures::COMMUNICATION_CLIENT, + )? }, - None => setup_identity_from_db(&wallet_db, &master_seed, node_addresses)?, + None => setup_identity_from_db(&wallet_db, &master_seed, node_addresses.to_vec())?, }; let mut wallet_config = config.wallet.clone(); diff --git a/base_layer/contacts/tests/contacts_service.rs b/base_layer/contacts/tests/contacts_service.rs index ac0a886a37..29922b223f 100644 --- a/base_layer/contacts/tests/contacts_service.rs +++ b/base_layer/contacts/tests/contacts_service.rs @@ -23,7 +23,7 @@ use std::{convert::TryInto, sync::Arc, time::Duration}; use rand::rngs::OsRng; -use tari_common::configuration::{Network, StringList}; +use tari_common::configuration::{MultiaddrList, Network, StringList}; use tari_common_sqlite::connection::{DbConnection, DbConnectionUrl}; use tari_common_types::{tari_address::TariAddress, types::PublicKey}; use tari_comms::{peer_manager::PeerFeatures, NodeIdentity}; @@ -67,7 +67,7 @@ pub fn setup_contacts_service( )); let comms_config = P2pConfig { override_from: None, - public_addresses: vec![], + public_addresses: MultiaddrList::default(), transport: TransportConfig { transport_type: TransportType::Memory, memory: MemoryTransportConfig { diff --git a/base_layer/p2p/src/config.rs b/base_layer/p2p/src/config.rs index 5fa125388b..cfe8521f47 100644 --- a/base_layer/p2p/src/config.rs +++ b/base_layer/p2p/src/config.rs @@ -30,6 +30,7 @@ use tari_common::{ configuration::{ serializers, utils::{deserialize_string_or_struct, serialize_string}, + MultiaddrList, StringList, }, DnsNameServer, @@ -87,7 +88,7 @@ pub struct P2pConfig { /// The public address advertised to other peers by this node. If not set it will be set automatically depending on /// the transport type. The TCP transport is not able to determine the users public IP, so this will need to be /// manually set. - pub public_addresses: Vec, + pub public_addresses: MultiaddrList, /// Transport configuration pub transport: TransportConfig, /// Path to the LMDB data files. @@ -132,7 +133,7 @@ impl Default for P2pConfig { fn default() -> Self { Self { override_from: None, - public_addresses: vec![], + public_addresses: MultiaddrList::default(), transport: Default::default(), datastore_path: PathBuf::from("peer_db"), peer_database_name: "peers".to_string(), diff --git a/base_layer/wallet/tests/wallet.rs b/base_layer/wallet/tests/wallet.rs index c7e91a1aca..4a5e9a33c3 100644 --- a/base_layer/wallet/tests/wallet.rs +++ b/base_layer/wallet/tests/wallet.rs @@ -25,7 +25,7 @@ use std::{mem::size_of, panic, path::Path, sync::Arc, time::Duration}; use chacha20poly1305::{Key, KeyInit, XChaCha20Poly1305}; use rand::{rngs::OsRng, RngCore}; use support::utils::make_input; -use tari_common::configuration::StringList; +use tari_common::configuration::{MultiaddrList, StringList}; use tari_common_types::{ chain_metadata::ChainMetadata, tari_address::TariAddress, @@ -127,7 +127,7 @@ async fn create_wallet( let node_identity = NodeIdentity::random(&mut OsRng, get_next_memory_address(), PeerFeatures::COMMUNICATION_NODE); let comms_config = P2pConfig { override_from: None, - public_addresses: vec![], + public_addresses: MultiaddrList::default(), transport: TransportConfig::new_memory(MemoryTransportConfig { listener_address: node_identity.first_public_address(), }), @@ -669,7 +669,7 @@ async fn test_import_utxo() { let (connection, _temp_dir) = make_wallet_database_connection(None); let comms_config = P2pConfig { override_from: None, - public_addresses: vec![], + public_addresses: MultiaddrList::default(), transport: TransportConfig::new_tcp(TcpTransportConfig { listener_address: "/ip4/127.0.0.1/tcp/0".parse().unwrap(), tor_socks_address: None, diff --git a/base_layer/wallet_ffi/src/lib.rs b/base_layer/wallet_ffi/src/lib.rs index b697bd12eb..0ebbbb04cd 100644 --- a/base_layer/wallet_ffi/src/lib.rs +++ b/base_layer/wallet_ffi/src/lib.rs @@ -86,7 +86,7 @@ use log4rs::{ }; use num_traits::FromPrimitive; use rand::rngs::OsRng; -use tari_common::configuration::StringList; +use tari_common::configuration::{MultiaddrList, StringList}; use tari_common_types::{ emoji::emoji_set, tari_address::{TariAddress, TariAddressError}, @@ -4772,9 +4772,9 @@ pub unsafe extern "C" fn comms_config_create( match public_address { Ok(public_address) => { let addresses = if (*transport).transport_type == TransportType::Tor { - vec![] + MultiaddrList::default() } else { - vec![public_address] + MultiaddrList::from(vec![public_address]) }; let config = TariCommsConfig { @@ -5294,8 +5294,8 @@ pub unsafe extern "C" fn wallet_create( let node_features = wallet_database.get_node_features()?.unwrap_or_default(); let node_addresses = if comms_config.public_addresses.is_empty() { match wallet_database.get_node_address()? { - Some(addr) => vec![addr], - None => vec![], + Some(addr) => MultiaddrList::from(vec![addr]), + None => MultiaddrList::default(), } } else { comms_config.public_addresses.clone() @@ -5316,7 +5316,7 @@ pub unsafe extern "C" fn wallet_create( // SAFETY: we are manually checking the validity of this signature before adding Some(..) let node_identity = Arc::new(NodeIdentity::with_signature_unchecked( comms_secret_key, - node_addresses, + node_addresses.to_vec(), node_features, identity_sig, )); diff --git a/common/src/configuration/mod.rs b/common/src/configuration/mod.rs index 47e4fb16d7..44f30f90c8 100644 --- a/common/src/configuration/mod.rs +++ b/common/src/configuration/mod.rs @@ -43,6 +43,7 @@ pub mod loader; mod network; pub use network::Network; mod common_config; +mod multiaddr_list; pub mod name_server; pub mod serializers; mod string_list; @@ -52,6 +53,7 @@ use std::{iter::FromIterator, net::SocketAddr}; pub use common_config::CommonConfig; use multiaddr::{Error, Multiaddr, Protocol}; +pub use multiaddr_list::MultiaddrList; pub use string_list::StringList; /// Interpret a string as either a socket address (first) or a multiaddr format string. diff --git a/common/src/configuration/multiaddr_list.rs b/common/src/configuration/multiaddr_list.rs new file mode 100644 index 0000000000..db9993cf04 --- /dev/null +++ b/common/src/configuration/multiaddr_list.rs @@ -0,0 +1,236 @@ +// Copyright 2022. The Tari Project +// +// Redistribution and use in source and binary forms, with or without modification, are permitted provided that the +// following conditions are met: +// +// 1. Redistributions of source code must retain the above copyright notice, this list of conditions and the following +// disclaimer. +// +// 2. Redistributions in binary form must reproduce the above copyright notice, this list of conditions and the +// following disclaimer in the documentation and/or other materials provided with the distribution. +// +// 3. Neither the name of the copyright holder nor the names of its contributors may be used to endorse or promote +// products derived from this software without specific prior written permission. +// +// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, +// INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +// DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL, +// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +// SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, +// WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE +// USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + +use std::{fmt, ops::Deref, slice, str::FromStr, vec}; + +use multiaddr::Multiaddr; +use serde::{ + de, + de::{SeqAccess, Visitor}, + Deserialize, + Deserializer, + Serialize, +}; + +/// Supports deserialization from a sequence of strings or comma-delimited strings +#[derive(Debug, Default, Clone, Serialize, PartialEq, Eq)] +pub struct MultiaddrList(Vec); + +impl MultiaddrList { + pub fn new() -> Self { + Self(vec![]) + } + + pub fn with_capacity(size: usize) -> Self { + Self(Vec::with_capacity(size)) + } + + pub fn into_vec(self) -> Vec { + self.0 + } + + pub fn as_slice(&self) -> &[Multiaddr] { + self.0.as_slice() + } +} + +impl Deref for MultiaddrList { + type Target = [Multiaddr]; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} + +impl AsRef<[Multiaddr]> for MultiaddrList { + fn as_ref(&self) -> &[Multiaddr] { + self.0.as_ref() + } +} + +impl From> for MultiaddrList { + fn from(v: Vec) -> Self { + Self(v) + } +} + +impl IntoIterator for MultiaddrList { + type IntoIter = as IntoIterator>::IntoIter; + type Item = as IntoIterator>::Item; + + fn into_iter(self) -> Self::IntoIter { + self.0.into_iter() + } +} + +impl<'a> IntoIterator for &'a MultiaddrList { + type IntoIter = slice::Iter<'a, Multiaddr>; + type Item = ::Item; + + fn into_iter(self) -> Self::IntoIter { + self.0.iter() + } +} + +impl<'de> Deserialize<'de> for MultiaddrList { + fn deserialize(deserializer: D) -> Result + where D: Deserializer<'de> { + struct MultiaddrListVisitor; + + impl<'de> Visitor<'de> for MultiaddrListVisitor { + type Value = MultiaddrList; + + fn expecting(&self, f: &mut fmt::Formatter) -> fmt::Result { + write!(f, "a comma delimited multiaddr or multiple multiaddr elements") + } + + fn visit_str(self, v: &str) -> Result + where E: de::Error { + Ok(MultiaddrList( + v.split(',') + .map(|s| s.trim()) + .filter(|s| !s.is_empty()) + .map(Multiaddr::from_str) + .collect::, _>>() + .map_err(|e| E::invalid_value(de::Unexpected::Str(e.to_string().as_str()), &self))?, + )) + } + + fn visit_newtype_struct(self, deserializer: D) -> Result + where D: Deserializer<'de> { + deserializer.deserialize_seq(MultiaddrListVisitor) + } + + fn visit_seq(self, mut seq: A) -> Result + where A: SeqAccess<'de> { + let mut buf = seq.size_hint().map(Vec::with_capacity).unwrap_or_default(); + while let Some(v) = seq.next_element::()? { + buf.push(v) + } + Ok(MultiaddrList(buf)) + } + } + + if deserializer.is_human_readable() { + deserializer.deserialize_seq(MultiaddrListVisitor) + } else { + deserializer.deserialize_newtype_struct("MultiaddrList", MultiaddrListVisitor) + } + } +} + +#[cfg(test)] +mod tests { + use config::Config; + + use super::*; + + #[derive(Deserialize)] + struct Test { + something: MultiaddrList, + } + + #[test] + fn with_capacity_test() { + let new_str_lst = MultiaddrList::with_capacity(3); + assert_eq!(new_str_lst.into_vec().capacity(), 3); + } + + #[test] + fn from_vec_string_list() { + let vec_multiaddr = vec![Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap()]; + let multiaddr_lst = MultiaddrList::from(vec_multiaddr); + assert_eq!(multiaddr_lst.into_vec(), vec![Multiaddr::from_str( + "/ip4/127.0.0.1/tcp/1234" + ) + .unwrap()]); + } + + #[test] + fn as_ref_multiaddr_list() { + let vec_multiaddr = vec![Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap()]; + let vec_as_ref: &[Multiaddr] = vec_multiaddr.as_ref(); + let multiaddr_lst = MultiaddrList::from(vec![Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap()]); + assert_eq!(multiaddr_lst.as_ref(), vec_as_ref); + } + + #[test] + fn into_iter_multiaddr_list() { + let vec_multiaddr = vec![ + Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap(), + Multiaddr::from_str("/ip4/192.168.0.1/tcp/1234").unwrap(), + Multiaddr::from_str("/ip4/10.0.0.1/tcp/1234").unwrap(), + ]; + let multiaddr_lst = MultiaddrList::from(vec_multiaddr); + let mut res_iter = multiaddr_lst.into_iter(); + + assert_eq!( + Some(Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap()), + res_iter.next() + ); + assert_eq!( + Some(Multiaddr::from_str("/ip4/192.168.0.1/tcp/1234").unwrap()), + res_iter.next() + ); + assert_eq!( + Some(Multiaddr::from_str("/ip4/10.0.0.1/tcp/1234").unwrap()), + res_iter.next() + ); + assert_eq!(None, res_iter.into_iter().next()); + } + + #[test] + fn it_deserializes_from_toml() { + let config_str = + r#"something = ["/ip4/127.0.0.1/tcp/1234","/ip4/192.168.0.1/tcp/1234","/ip4/10.0.0.1/tcp/1234"]"#; + let test = toml::from_str::(config_str).unwrap(); + assert_eq!(test.something.0, vec![ + Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap(), + Multiaddr::from_str("/ip4/192.168.0.1/tcp/1234").unwrap(), + Multiaddr::from_str("/ip4/10.0.0.1/tcp/1234").unwrap() + ]); + } + + #[test] + fn it_returns_error() { + let config_str = r#"something = ["Not multiaddr","/ip4/192.168.0.1/tcp/1234","/ip4/10.0.0.1/tcp/1234"]"#; + assert!(toml::from_str::(config_str).is_err()); + } + + #[test] + fn it_deserializes_from_config_comma_delimited() { + let config = Config::builder() + .set_override( + "something", + "/ip4/127.0.0.1/tcp/1234, /ip4/192.168.0.1/tcp/1234, /ip4/10.0.0.1/tcp/1234,", + ) + .unwrap() + .build() + .unwrap(); + let test = config.try_deserialize::().unwrap(); + assert_eq!(test.something.0, vec![ + Multiaddr::from_str("/ip4/127.0.0.1/tcp/1234").unwrap(), + Multiaddr::from_str("/ip4/192.168.0.1/tcp/1234").unwrap(), + Multiaddr::from_str("/ip4/10.0.0.1/tcp/1234").unwrap() + ]); + } +} diff --git a/integration_tests/tests/utils/base_node_process.rs b/integration_tests/tests/utils/base_node_process.rs index de6ebe81ca..b96b0e56cc 100644 --- a/integration_tests/tests/utils/base_node_process.rs +++ b/integration_tests/tests/utils/base_node_process.rs @@ -33,7 +33,7 @@ use std::{ use rand::rngs::OsRng; use tari_base_node::{run_base_node, BaseNodeConfig, MetricsConfig}; use tari_base_node_grpc_client::BaseNodeGrpcClient; -use tari_common::configuration::CommonConfig; +use tari_common::configuration::{CommonConfig, MultiaddrList}; use tari_comms::{multiaddr::Multiaddr, peer_manager::PeerFeatures, NodeIdentity}; use tari_comms_dht::DhtConfig; use tari_p2p::{auto_update::AutoUpdateConfig, Network, PeerSeedsConfig, TransportType}; @@ -173,8 +173,13 @@ pub async fn spawn_base_node_with_config( base_node_config.base_node.p2p.transport.transport_type = TransportType::Tcp; base_node_config.base_node.p2p.transport.tcp.listener_address = format!("/ip4/127.0.0.1/tcp/{}", port).parse().unwrap(); - base_node_config.base_node.p2p.public_addresses = - vec![base_node_config.base_node.p2p.transport.tcp.listener_address.clone()]; + base_node_config.base_node.p2p.public_addresses = MultiaddrList::from(vec![base_node_config + .base_node + .p2p + .transport + .tcp + .listener_address + .clone()]); // base_node_config.base_node.p2p.datastore_path = temp_dir_path.to_path_buf(); // base_node_config.base_node.p2p.peer_database_name = "peer_db.mdb".to_string(); base_node_config.base_node.p2p.dht = DhtConfig::default_local_test(); diff --git a/integration_tests/tests/utils/wallet_process.rs b/integration_tests/tests/utils/wallet_process.rs index 69fa6972d2..6806580dd8 100644 --- a/integration_tests/tests/utils/wallet_process.rs +++ b/integration_tests/tests/utils/wallet_process.rs @@ -24,7 +24,7 @@ use std::{path::PathBuf, str::FromStr, thread, time::Duration}; use tari_app_grpc::tari_rpc::SetBaseNodeRequest; use tari_app_utilities::common_cli_args::CommonCliArgs; -use tari_common::configuration::CommonConfig; +use tari_common::configuration::{CommonConfig, MultiaddrList}; use tari_comms::multiaddr::Multiaddr; use tari_comms_dht::DhtConfig; use tari_console_wallet::{run_wallet_with_cli, Cli}; @@ -127,7 +127,7 @@ pub async fn spawn_wallet( wallet_config.wallet.p2p.transport.tcp.listener_address = Multiaddr::from_str(&format!("/ip4/127.0.0.1/tcp/{}", port)).unwrap(); wallet_config.wallet.p2p.public_addresses = - vec![wallet_config.wallet.p2p.transport.tcp.listener_address.clone()]; + MultiaddrList::from(vec![wallet_config.wallet.p2p.transport.tcp.listener_address.clone()]); wallet_config.wallet.p2p.datastore_path = temp_dir_path.clone().join("peer_db").join("wallet"); wallet_config.wallet.p2p.dht = DhtConfig::default_local_test(); wallet_config.wallet.p2p.allow_test_addresses = true;