Skip to content

Commit

Permalink
fix: fix panic when no public addresses (#5367)
Browse files Browse the repository at this point in the history
Description
---
If there are no public addresses specified, there was a panic. This is
solved by not starting a liveness service if there is no public address.

NOTE: I have changed the reference on tari-crypto to be the lowest
version that it works with (i.e. 0.16, instead of 0.16.*). This is the
best practice for libraries, according to Rust, and also helps with
mismatches on the tari-dan repo.

Motivation and Context
---
When using TCP, a node might not have a public address, this may cause a
panic with the current code. AFAIK a comms node (base node, wallet, dan
indexer) can still work without a public address, but it can only
initiate outbound connections.

How Has This Been Tested?
---
existing tests

What process can a PR reviewer use to test or verify this change?
---
TODO: I'm busy checking this on the DAN indexer, which has no public
address during testing

<!-- Checklist -->
<!-- 1. Is the title of your PR in the form that would make nice release
notes? The title, excluding the conventional commit
tag, will be included exactly as is in the CHANGELOG, so please think
about it carefully. -->


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 -->
  • Loading branch information
stringhandler committed May 10, 2023
1 parent 45685b9 commit 49be2a2
Show file tree
Hide file tree
Showing 35 changed files with 65 additions and 69 deletions.
15 changes: 2 additions & 13 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion applications/tari_app_grpc/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ edition = "2018"
tari_common_types = { path = "../../base_layer/common_types" }
tari_comms = { path = "../../comms/core" }
tari_core = { path = "../../base_layer/core" }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_script = { path = "../../infrastructure/tari_script" }
tari_utilities = { version = "0.4.10"}

Expand Down
2 changes: 1 addition & 1 deletion applications/tari_app_utilities/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ tari_utilities = { version = "0.4.10"}

clap = { version = "3.2.0", features = ["derive", "env"] }
futures = { version = "^0.3.16", default-features = false, features = ["alloc"] }
json5 = "0.2.2"
json5 = "0.4"
log = { version = "0.4.8", features = ["std"] }
rand = "0.7.3"
tokio = { version = "1.23", features = ["signal"] }
Expand Down
2 changes: 1 addition & 1 deletion applications/tari_base_node/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ tari_comms = { path = "../../comms/core", features = ["rpc"] }
tari_common_types = { path = "../../base_layer/common_types" }
tari_comms_dht = { path = "../../comms/dht" }
tari_core = { path = "../../base_layer/core", default-features = false, features = ["transactions"] }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_libtor = { path = "../../infrastructure/libtor", optional = true }
tari_p2p = { path = "../../base_layer/p2p", features = ["auto-update"] }
tari_storage = {path="../../infrastructure/storage"}
Expand Down
2 changes: 1 addition & 1 deletion applications/tari_console_wallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ tari_common_types = { path = "../../base_layer/common_types" }
tari_comms = { path = "../../comms/core" }
tari_comms_dht = { path = "../../comms/dht" }
tari_contacts = { path = "../../base_layer/contacts" }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_key_manager = { path = "../../base_layer/key_manager" }
tari_libtor = { path = "../../infrastructure/libtor", optional = true }
tari_p2p = { path = "../../base_layer/p2p", features = ["auto-update"] }
Expand Down
2 changes: 1 addition & 1 deletion applications/tari_miner/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ tari_common_types = { path = "../../base_layer/common_types" }
tari_comms = { path = "../../comms/core" }
tari_app_utilities = { path = "../tari_app_utilities" }
tari_app_grpc = { path = "../tari_app_grpc" }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_utilities = "0.4.10"

borsh = "0.9.3"
Expand Down
2 changes: 1 addition & 1 deletion base_layer/common_types/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ version = "0.50.0-pre.1"
edition = "2018"

[dependencies]
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_utilities = "0.4.10"
# TODO: remove this dependency and move Network into tari_common_types
tari_common = { path = "../../common" }
Expand Down
4 changes: 2 additions & 2 deletions base_layer/contacts/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ tari_common_sqlite = { path = "../../common_sqlite" }
tari_common_types = { path = "../../base_layer/common_types" }
tari_comms = { path = "../../comms/core" }
tari_comms_dht = { path = "../../comms/dht" }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_p2p = { path = "../p2p", features = ["auto-update"] }
tari_service_framework = { path = "../service_framework" }
tari_shutdown = { path = "../../infrastructure/shutdown" }
Expand All @@ -30,7 +30,7 @@ rand = "0.7.3"
thiserror = "1.0.26"
tokio = { version = "1.23", features = ["sync", "macros"] }
tower = "0.4"
uuid = { version = "1.3.1", features = ["v4"] }
uuid = { version = "1.3", features = ["v4"] }

[dev-dependencies]
tari_comms_dht = { path = "../../comms/dht", features = ["test-mocks"] }
Expand Down
2 changes: 1 addition & 1 deletion base_layer/contacts/tests/contacts_service.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub fn setup_contacts_service<T: ContactsBackend + 'static>(
transport: TransportConfig {
transport_type: TransportType::Memory,
memory: MemoryTransportConfig {
listener_address: node_identity.first_public_address(),
listener_address: node_identity.first_public_address().unwrap(),
},
..Default::default()
},
Expand Down
2 changes: 1 addition & 1 deletion base_layer/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ tari_common_types = { path = "../../base_layer/common_types" }
tari_comms = { path = "../../comms/core" }
tari_comms_dht = { path = "../../comms/dht" }
tari_comms_rpc_macros = { path = "../../comms/rpc_macros" }
tari_crypto = { version = "0.16.12", features = ["borsh"] }
tari_crypto = { version = "0.16", features = ["borsh"] }
tari_metrics = { path = "../../infrastructure/metrics" }
tari_mmr = { path = "../../base_layer/mmr", optional = true, features = ["native_bitmap"] }
tari_p2p = { path = "../../base_layer/p2p" }
Expand Down
2 changes: 1 addition & 1 deletion base_layer/key_manager/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ crate-type = ["lib", "cdylib"]

# NB: All dependencies must support or be gated for the WASM target.
[dependencies]
tari_crypto = {version = "0.16.12"}
tari_crypto = {version = "0.16"}
tari_utilities = "0.4.10"
tari_common_sqlite = { path = "../../common_sqlite" }
tari_common_types = { path = "../../base_layer/common_types"}
Expand Down
2 changes: 1 addition & 1 deletion base_layer/mmr/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ benches = ["criterion"]

[dependencies]
tari_utilities = "0.4.10"
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_common = {path = "../../common"}
thiserror = "1.0.26"
borsh = "0.9.3"
Expand Down
2 changes: 1 addition & 1 deletion base_layer/p2p/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ edition = "2018"
tari_comms = { path = "../../comms/core" }
tari_comms_dht = { path = "../../comms/dht" }
tari_common = { path = "../../common" }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_service_framework = { path = "../service_framework" }
tari_shutdown = { path = "../../infrastructure/shutdown" }
tari_storage = { path = "../../infrastructure/storage" }
Expand Down
2 changes: 1 addition & 1 deletion base_layer/p2p/src/initialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ pub async fn initialize_local_test_comms<P: AsRef<Path>>(

let comms = CommsBuilder::new()
.allow_test_addresses()
.with_listener_address(node_identity.first_public_address())
.with_listener_address(node_identity.first_public_address().unwrap())
.with_listener_liveness_max_sessions(1)
.with_node_identity(node_identity)
.with_user_agent(&"/test/1.0")
Expand Down
2 changes: 1 addition & 1 deletion base_layer/tari_mining_helper_ffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ edition = "2018"

[dependencies]
tari_comms = { path = "../../comms/core" }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_common = { path = "../../common" }
tari_core = { path = "../core", default-features = false, features = ["transactions"]}
tari_utilities = "0.4.10"
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ tari_common = { path = "../../common" }
tari_common_types = { path = "../../base_layer/common_types" }
tari_comms = { path = "../../comms/core" }
tari_comms_dht = { path = "../../comms/dht" }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_key_manager = { path = "../key_manager", features = ["key_manager_service"] }
tari_p2p = { path = "../p2p", features = ["auto-update"] }
tari_script = { path = "../../infrastructure/tari_script" }
Expand Down
2 changes: 2 additions & 0 deletions base_layer/wallet/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,8 @@ pub enum WalletError {
TransportChannelError(#[from] TransportChannelError),
#[error("Unexpected API Response while calling method `{method}` on `{api}`")]
UnexpectedApiResponse { method: String, api: String },
#[error("Public address not set for this wallet")]
PublicAddressNotSet,
}

pub const LOG_TARGET: &str = "tari::application";
Expand Down
7 changes: 6 additions & 1 deletion base_layer/wallet/src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,12 @@ where

// Persist the comms node address and features after it has been spawned to capture any modifications made
// during comms startup. In the case of a Tor Transport the public address could have been generated
wallet_database.set_node_address(comms.node_identity().first_public_address())?;
wallet_database.set_node_address(
comms
.node_identity()
.first_public_address()
.ok_or(WalletError::PublicAddressNotSet)?,
)?;
wallet_database.set_node_features(comms.node_identity().features())?;
let identity_sig = comms.node_identity().identity_signature_read().as_ref().cloned();
if let Some(identity_sig) = identity_sig {
Expand Down
13 changes: 8 additions & 5 deletions base_layer/wallet/tests/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ async fn create_wallet(
override_from: None,
public_addresses: MultiaddrList::default(),
transport: TransportConfig::new_memory(MemoryTransportConfig {
listener_address: node_identity.first_public_address(),
listener_address: node_identity.first_public_address().unwrap(),
}),
datastore_path: data_path.to_path_buf(),
peer_database_name: random::string(8),
Expand Down Expand Up @@ -255,7 +255,7 @@ async fn test_wallet() {
.peer_manager()
.add_peer(create_peer(
bob_identity.public_key().clone(),
bob_identity.first_public_address(),
bob_identity.first_public_address().unwrap(),
))
.await
.unwrap();
Expand All @@ -265,15 +265,15 @@ async fn test_wallet() {
.peer_manager()
.add_peer(create_peer(
alice_identity.public_key().clone(),
alice_identity.first_public_address(),
alice_identity.first_public_address().unwrap(),
))
.await
.unwrap();

alice_wallet
.set_base_node_peer(
(*base_node_identity.public_key()).clone(),
base_node_identity.first_public_address().clone(),
base_node_identity.first_public_address().unwrap(),
)
.await
.unwrap();
Expand Down Expand Up @@ -735,7 +735,10 @@ async fn test_import_utxo() {
let expected_output_hash = output.hash();
let node_address = TariAddress::new(node_identity.public_key().clone(), network);
alice_wallet
.set_base_node_peer(node_identity.public_key().clone(), node_identity.first_public_address())
.set_base_node_peer(
node_identity.public_key().clone(),
node_identity.first_public_address().unwrap(),
)
.await
.unwrap();
let tx_id = alice_wallet
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet_ffi/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ tari_common = { path="../../common" }
tari_common_types = { path="../common_types" }
tari_comms = { path = "../../comms/core", features = ["c_integration"]}
tari_comms_dht = { path = "../../comms/dht", default-features = false }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_key_manager = { path = "../key_manager" }
tari_p2p = { path = "../p2p" }
tari_script = { path = "../../infrastructure/tari_script" }
Expand Down
2 changes: 1 addition & 1 deletion base_layer/wallet_ffi/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10754,7 +10754,7 @@ mod test {
NodeIdentity::random(&mut OsRng, get_next_memory_address(), PeerFeatures::COMMUNICATION_NODE);
let base_node_peer_public_key_ptr = Box::into_raw(Box::new(node_identity.public_key().clone()));
let base_node_peer_address_ptr =
CString::into_raw(CString::new(node_identity.first_public_address().to_string()).unwrap())
CString::into_raw(CString::new(node_identity.first_public_address().unwrap().to_string()).unwrap())
as *const c_char;
wallet_add_base_node_peer(
wallet_ptr,
Expand Down
2 changes: 1 addition & 1 deletion common/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ build = ["toml", "prost-build"]
static-application-info = ["git2"]

[dependencies]
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}

anyhow = "1.0.53"
config = { version = "0.13.0", default_features = false, features = ["toml"] }
Expand Down
2 changes: 1 addition & 1 deletion comms/core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ version = "0.50.0-pre.1"
edition = "2018"

[dependencies]
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_metrics = { path = "../../infrastructure/metrics" }
tari_storage = { path = "../../infrastructure/storage" }
tari_shutdown = { path = "../../infrastructure/shutdown" }
Expand Down
19 changes: 8 additions & 11 deletions comms/core/src/builder/comms_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -244,17 +244,14 @@ impl UnspawnedCommsNode {
);

// Spawn liveness check now that we have the final address
let liveness_watch = connection_manager_config
.liveness_self_check_interval
.map(|interval| {
LivenessCheck::spawn(
transport,
node_identity.first_public_address(),
interval,
shutdown_signal.clone(),
)
})
.unwrap_or_else(|| watch::channel(LivenessStatus::Disabled).1);
let liveness_watch = if let Some(public_address) = node_identity.first_public_address() {
connection_manager_config
.liveness_self_check_interval
.map(|interval| LivenessCheck::spawn(transport, public_address, interval, shutdown_signal.clone()))
.unwrap_or_else(|| watch::channel(LivenessStatus::Disabled).1)
} else {
watch::channel(LivenessStatus::Disabled).1
};

Ok(CommsNode {
shutdown_signal,
Expand Down
4 changes: 2 additions & 2 deletions comms/core/src/peer_manager/node_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,8 +126,8 @@ impl NodeIdentity {
acquire_read_lock!(self.public_addresses).clone()
}

pub fn first_public_address(&self) -> Multiaddr {
acquire_read_lock!(self.public_addresses)[0].clone()
pub fn first_public_address(&self) -> Option<Multiaddr> {
acquire_read_lock!(self.public_addresses).get(0).cloned()
}

/// Modify the public address.
Expand Down
4 changes: 2 additions & 2 deletions comms/core/src/protocol/rpc/test/comms_integration.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ async fn run_service() {
let mock_state = rpc_service.shared_state();
let shutdown = Shutdown::new();
let comms1 = CommsBuilder::new()
.with_listener_address(node_identity1.first_public_address())
.with_listener_address(node_identity1.first_public_address().unwrap())
.with_node_identity(node_identity1)
.with_shutdown_signal(shutdown.to_signal())
.with_peer_storage(CommsDatabase::new(), None)
Expand All @@ -57,7 +57,7 @@ async fn run_service() {

let node_identity2 = build_node_identity(Default::default());
let comms2 = CommsBuilder::new()
.with_listener_address(node_identity2.first_public_address())
.with_listener_address(node_identity2.first_public_address().unwrap())
.with_shutdown_signal(shutdown.to_signal())
.with_node_identity(node_identity2.clone())
.with_peer_storage(CommsDatabase::new(), None)
Expand Down
2 changes: 1 addition & 1 deletion comms/dht/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ edition = "2018"
tari_comms = { path = "../core", features = ["rpc"] }
tari_common = { path = "../../common" }
tari_comms_rpc_macros = { path = "../rpc_macros" }
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16.2"}
tari_utilities = "0.4.10"
tari_shutdown = { path = "../../infrastructure/shutdown" }
tari_storage = { path = "../../infrastructure/storage" }
Expand Down
2 changes: 1 addition & 1 deletion comms/dht/examples/memory_net/utilities.rs
Original file line number Diff line number Diff line change
Expand Up @@ -903,7 +903,7 @@ async fn setup_comms_dht(
let comms = CommsBuilder::new()
.allow_test_addresses()
// In this case the listener address and the public address are the same (/memory/...)
.with_listener_address(node_identity.first_public_address())
.with_listener_address(node_identity.first_public_address().unwrap())
.with_shutdown_signal(shutdown_signal)
.with_node_identity(node_identity)
.with_min_connectivity(1)
Expand Down
2 changes: 1 addition & 1 deletion comms/dht/tests/dht.rs
Original file line number Diff line number Diff line change
Expand Up @@ -172,7 +172,7 @@ async fn setup_comms_dht(
let comms = CommsBuilder::new()
.allow_test_addresses()
// In this case the listener address and the public address are the same (/memory/...)
.with_listener_address(node_identity.first_public_address())
.with_listener_address(node_identity.first_public_address().unwrap())
.with_shutdown_signal(shutdown_signal)
.with_node_identity(node_identity)
.with_peer_storage(storage,None)
Expand Down
2 changes: 1 addition & 1 deletion infrastructure/tari_script/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ readme = "README.md"
license = "BSD-3-Clause"

[dependencies]
tari_crypto = { version = "0.16.12"}
tari_crypto = { version = "0.16"}
tari_utilities = "0.4.10"

blake2 = "0.9"
Expand Down
2 changes: 1 addition & 1 deletion integration_tests/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ tari_base_node = { path = "../applications/tari_base_node" }
tari_base_node_grpc_client = { path = "../clients/rust/base_node_grpc_client" }
tari_chat_client = { path = "../base_layer/contacts/examples/chat_client" }
tari_chat_ffi = { path = "../base_layer/chat_ffi" }
tari_crypto = "0.16.12"
tari_crypto = "0.16"
tari_common = { path = "../common" }
tari_common_types = { path = "../base_layer/common_types" }
tari_comms = { path = "../comms/core" }
Expand Down
Loading

0 comments on commit 49be2a2

Please sign in to comment.