Skip to content

Commit

Permalink
fix: minor fixes for multiple address support (tari-project#5617)
Browse files Browse the repository at this point in the history
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

<!-- Does this include a breaking change? If so, include this line as a
footer -->
<!-- BREAKING CHANGE: Description what the user should do, e.g. delete a
database, resync the chain -->

---------

Co-authored-by: SW van Heerden <swvheerden@gmail.com>
  • Loading branch information
sdbondi and SWvheerden committed Aug 11, 2023
1 parent 8a085cd commit efa36eb
Show file tree
Hide file tree
Showing 8 changed files with 32 additions and 17 deletions.
Expand Up @@ -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;
Expand Down
19 changes: 14 additions & 5 deletions applications/minotari_console_wallet/src/init/mod.rs
Expand Up @@ -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::<Vec<_>>()
.join(", ")
);
if prompt(
"Would you like to use this base node? IF YOU DID NOT START THIS BASE NODE YOU SHOULD SELECT \
Expand Down Expand Up @@ -499,14 +504,18 @@ async fn detect_local_base_node(network: Network) -> Option<SeedPeer> {
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::<Vec<_>>();
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::<Vec<_>>().join(",")
);
Some(SeedPeer::new(public_key, vec![address]))
Some(SeedPeer::new(public_key, addresses))
}

fn setup_identity_from_db<D: WalletBackend + 'static>(
Expand Down
9 changes: 7 additions & 2 deletions applications/minotari_node/src/commands/command/whoami.rs
Expand Up @@ -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::<Vec<_>>()
.join("::")
);
let network = self.config.network();
let qr_link = format!(
Expand Down
1 change: 0 additions & 1 deletion base_layer/p2p/src/initialization.rs
Expand Up @@ -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,
Expand Down
9 changes: 5 additions & 4 deletions comms/core/src/peer_manager/node_identity.rs
Expand Up @@ -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<Multiaddr>) {
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;
}
}
Expand Down Expand Up @@ -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())
Expand Down
3 changes: 1 addition & 2 deletions comms/core/tests/tests/rpc.rs
Expand Up @@ -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;
Expand Down
3 changes: 2 additions & 1 deletion comms/core/tests/tests/rpc_stress.rs
Expand Up @@ -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
}

Expand Down
3 changes: 2 additions & 1 deletion comms/core/tests/tests/substream_stress.rs
Expand Up @@ -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)
}

Expand Down

0 comments on commit efa36eb

Please sign in to comment.