From efa36eb7dc92905cc085359c35255678136a15b1 Mon Sep 17 00:00:00 2001 From: Stan Bondi Date: Fri, 11 Aug 2023 10:44:32 +0400 Subject: [PATCH] fix: minor fixes for multiple address support (#5617) Description --- Fixed a few places in the code where multiple addresses were not completely supported, namely - wallet base node detection via GRPC, and - whoami QR code Fixed incorrect format for QR Code Replaces replace_public_address with set_public_addresses. Motivation and Context --- Minor fixes to use all addresses rather than a single address. With set_public_addresses, I wanted to set the addresses to only the public addresses. However, this is problematic because the tor address is added at a very late point in the comms initialization. This would mean that if public addresses were configured along with a late-configured tor address, the identity sig would change every time. Other than in the comms builder, the tor hidden service code is completely decoupled from comms (that is, we could pass in the transport to comms and not the hidden service abstraction itself). In order to solve this we would setup the tor HS earlier before we initialize the node identity, and remove the code that mutates the node identity in the comms builder. We'd know the final tor address and would be able to add it to the public address list and have a final address set for node identity. Although I anticipate this refactor would be fairly straightforward, is not strictly necessary at this point so is not attempted. How Has This Been Tested? --- Cucumbers What process can a PR reviewer use to test or verify this change? --- Add multiple addresses to the base node and check that the wallet is aware of those addresses. Breaking Changes --- - [x] None - [ ] Requires data directory on base node to be deleted - [ ] Requires hard fork - [ ] Other - Please specify --------- Co-authored-by: SW van Heerden --- .../src/identity_management.rs | 2 +- .../minotari_console_wallet/src/init/mod.rs | 19 ++++++++++++++----- .../src/commands/command/whoami.rs | 9 +++++++-- base_layer/p2p/src/initialization.rs | 1 - comms/core/src/peer_manager/node_identity.rs | 9 +++++---- comms/core/tests/tests/rpc.rs | 3 +-- comms/core/tests/tests/rpc_stress.rs | 3 ++- comms/core/tests/tests/substream_stress.rs | 3 ++- 8 files changed, 32 insertions(+), 17 deletions(-) diff --git a/applications/minotari_app_utilities/src/identity_management.rs b/applications/minotari_app_utilities/src/identity_management.rs index 96b8dbc6ef..c077f78048 100644 --- a/applications/minotari_app_utilities/src/identity_management.rs +++ b/applications/minotari_app_utilities/src/identity_management.rs @@ -20,7 +20,7 @@ // 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::{clone::Clone, fs, io, path::Path, sync::Arc}; +use std::{fs, io, path::Path, sync::Arc}; use log::*; use rand::rngs::OsRng; diff --git a/applications/minotari_console_wallet/src/init/mod.rs b/applications/minotari_console_wallet/src/init/mod.rs index 943e1f415a..f42cc0f183 100644 --- a/applications/minotari_console_wallet/src/init/mod.rs +++ b/applications/minotari_console_wallet/src/init/mod.rs @@ -300,7 +300,12 @@ pub async fn get_base_node_peer_config( println!( "Local Base Node detected with public key {} and address {}", detected_node.public_key, - detected_node.addresses.first().unwrap() + detected_node + .addresses + .iter() + .map(ToString::to_string) + .collect::>() + .join(", ") ); if prompt( "Would you like to use this base node? IF YOU DID NOT START THIS BASE NODE YOU SHOULD SELECT \ @@ -499,14 +504,18 @@ async fn detect_local_base_node(network: Network) -> Option { let resp = node_conn.identify(Empty {}).await.ok()?; let identity = resp.get_ref(); let public_key = CommsPublicKey::from_bytes(&identity.public_key).ok()?; - let address = Multiaddr::from_str(identity.public_addresses.first()?).ok()?; + let addresses = identity + .public_addresses + .iter() + .filter_map(|s| Multiaddr::from_str(s).ok()) + .collect::>(); debug!( target: LOG_TARGET, - "Local base node found with pk={} and addr={}", + "Local base node found with pk={} and addresses={}", public_key.to_hex(), - address + addresses.iter().map(|a| a.to_string()).collect::>().join(",") ); - Some(SeedPeer::new(public_key, vec![address])) + Some(SeedPeer::new(public_key, addresses)) } fn setup_identity_from_db( diff --git a/applications/minotari_node/src/commands/command/whoami.rs b/applications/minotari_node/src/commands/command/whoami.rs index f287cf16ab..58d2772a57 100644 --- a/applications/minotari_node/src/commands/command/whoami.rs +++ b/applications/minotari_node/src/commands/command/whoami.rs @@ -45,9 +45,14 @@ impl CommandContext { pub fn whoami(&self) -> Result<(), Error> { println!("{}", self.base_node_identity); let peer = format!( - "{}::{:?}", + "{}::{}", self.base_node_identity.public_key().to_hex(), - self.base_node_identity.public_addresses() + self.base_node_identity + .public_addresses() + .iter() + .map(|addr| addr.to_string()) + .collect::>() + .join("::") ); let network = self.config.network(); let qr_link = format!( diff --git a/base_layer/p2p/src/initialization.rs b/base_layer/p2p/src/initialization.rs index 7595abafd9..bf144c16be 100644 --- a/base_layer/p2p/src/initialization.rs +++ b/base_layer/p2p/src/initialization.rs @@ -19,7 +19,6 @@ // 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. -#![allow(dead_code)] use std::{ fs, diff --git a/comms/core/src/peer_manager/node_identity.rs b/comms/core/src/peer_manager/node_identity.rs index 6f11ddbfea..69c82818fe 100644 --- a/comms/core/src/peer_manager/node_identity.rs +++ b/comms/core/src/peer_manager/node_identity.rs @@ -145,13 +145,14 @@ impl NodeIdentity { } } - pub fn replace_public_address(&self, address: Multiaddr) { + /// Set the available addresses. If none of the addresses have changed, the identity signature remains unchanged. + pub fn set_public_addresses(&self, addresses: Vec) { let mut must_sign = false; { let mut lock = acquire_write_lock!(self.public_addresses); - if !lock.contains(&address) { + if addresses.len() != lock.len() || addresses.iter().any(|a| !lock.contains(a)) { lock.clear(); - lock.push(address); + lock.extend(addresses); must_sign = true; } } @@ -272,7 +273,7 @@ impl fmt::Display for NodeIdentity { writeln!(f, "Node ID: {}", self.node_id)?; writeln!( f, - "Public Address: {}", + "Public Addresses: {}", acquire_read_lock!(self.public_addresses) .iter() .map(|s| s.to_string()) diff --git a/comms/core/tests/tests/rpc.rs b/comms/core/tests/tests/rpc.rs index 809c875307..d97a0596d4 100644 --- a/comms/core/tests/tests/rpc.rs +++ b/comms/core/tests/tests/rpc.rs @@ -52,14 +52,13 @@ async fn spawn_node(signal: ShutdownSignal) -> (CommsNode, RpcServerHandle) { comms .node_identity() - .replace_public_address(comms.listening_address().clone()); + .set_public_addresses(vec![comms.listening_address().clone()]); (comms, rpc_server_hnd) } #[tokio::test(flavor = "multi_thread", worker_threads = 4)] async fn client_prematurely_ends_session() { - env_logger::init(); let shutdown = Shutdown::new(); let (node1, _rpc_server1) = spawn_node(shutdown.to_signal()).await; let (node2, mut rpc_server2) = spawn_node(shutdown.to_signal()).await; diff --git a/comms/core/tests/tests/rpc_stress.rs b/comms/core/tests/tests/rpc_stress.rs index 211af5fb50..0e27fa38f9 100644 --- a/comms/core/tests/tests/rpc_stress.rs +++ b/comms/core/tests/tests/rpc_stress.rs @@ -54,7 +54,8 @@ async fn spawn_node(signal: ShutdownSignal) -> CommsNode { comms .node_identity() - .replace_public_address(comms.listening_address().clone()); + .set_public_addresses(vec![comms.listening_address().clone()]); + comms } diff --git a/comms/core/tests/tests/substream_stress.rs b/comms/core/tests/tests/substream_stress.rs index 8dcaf78425..d36a26d673 100644 --- a/comms/core/tests/tests/substream_stress.rs +++ b/comms/core/tests/tests/substream_stress.rs @@ -49,7 +49,8 @@ pub async fn spawn_node(signal: ShutdownSignal) -> (CommsNode, ProtocolNotificat comms .node_identity() - .replace_public_address(comms.listening_address().clone()); + .set_public_addresses(vec![comms.listening_address().clone()]); + (comms, notif_rx) }