merge #5512 again after reverting due to incorrect rebase (#5520)

* setup workspace global lints to prevent needless panics

* removed sources of panic in nym-crypto, nym-node and nym-api

* adjusted test code
This commit is contained in:
Jędrzej Stuczyński
2025-02-26 10:52:09 +00:00
committed by GitHub
parent 69b2448500
commit 65175fee09
40 changed files with 181 additions and 69 deletions
+10
View File
@@ -437,3 +437,13 @@ opt-level = 'z'
[profile.release.package.mix-fetch-wasm] [profile.release.package.mix-fetch-wasm]
# lto = true # lto = true
opt-level = 'z' opt-level = 'z'
[workspace.lints.clippy]
unwrap_used = "deny"
expect_used = "deny"
todo = "deny"
dbg_macro = "deny"
exit = "deny"
panic = "deny"
unimplemented = "deny"
unreachable = "deny"
+1
View File
@@ -1,2 +1,3 @@
allow-unwrap-in-tests = true allow-unwrap-in-tests = true
allow-expect-in-tests = true allow-expect-in-tests = true
allow-panic-in-tests = true
+4 -1
View File
@@ -43,4 +43,7 @@ serde = ["dep:serde", "serde_bytes", "ed25519-dalek/serde", "x25519-dalek/serde"
asymmetric = ["x25519-dalek", "ed25519-dalek", "zeroize"] asymmetric = ["x25519-dalek", "ed25519-dalek", "zeroize"]
hashing = ["blake3", "digest", "hkdf", "hmac", "generic-array", "sha2"] hashing = ["blake3", "digest", "hkdf", "hmac", "generic-array", "sha2"]
stream_cipher = ["aes", "ctr", "cipher", "generic-array"] stream_cipher = ["aes", "ctr", "cipher", "generic-array"]
sphinx = ["nym-sphinx-types/sphinx"] sphinx = ["nym-sphinx-types/sphinx"]
[lints]
workspace = true
+10 -4
View File
@@ -16,8 +16,11 @@ pub fn compute_keyed_hmac<D>(key: &[u8], data: &[u8]) -> HmacOutput<D>
where where
D: Digest + BlockSizeUser, D: Digest + BlockSizeUser,
{ {
let mut hmac = SimpleHmac::<D>::new_from_slice(key) // SAFETY: hmac is fine with keys of any size; if they're smaller than the block size of the underlying
.expect("HMAC was instantiated with a key of an invalid size!"); // digest, they're padded with 0. if they're larger they're hashed and padded
// the reason for `Result` return type is due to the trait definition
#[allow(clippy::unwrap_used)]
let mut hmac = SimpleHmac::<D>::new_from_slice(key).unwrap();
hmac.update(data); hmac.update(data);
hmac.finalize() hmac.finalize()
} }
@@ -27,8 +30,11 @@ pub fn recompute_keyed_hmac_and_verify_tag<D>(key: &[u8], data: &[u8], tag: &[u8
where where
D: Digest + BlockSizeUser, D: Digest + BlockSizeUser,
{ {
let mut hmac = SimpleHmac::<D>::new_from_slice(key) // SAFETY: hmac is fine with keys of any size; if they're smaller than the block size of the underlying
.expect("HMAC was instantiated with a key of an invalid size!"); // digest, they're padded with 0. if they're larger they're hashed and padded
// the reason for `Result` return type is due to the trait definition
#[allow(clippy::unwrap_used)]
let mut hmac = SimpleHmac::<D>::new_from_slice(key).unwrap();
hmac.update(data); hmac.update(data);
let tag_arr = Output::<D>::from_slice(tag); let tag_arr = Output::<D>::from_slice(tag);
+14 -5
View File
@@ -27,12 +27,16 @@ where
// after performing diffie-hellman we don't care about the private component anymore // after performing diffie-hellman we don't care about the private component anymore
let dh_result = ephemeral_keypair.private_key().diffie_hellman(remote_key); let dh_result = ephemeral_keypair.private_key().diffie_hellman(remote_key);
// there is no reason for this to fail as our okm is expected to be only C::KeySize bytes // SAFETY: while this is a relatively weak assumption, it's unlikely that any stream cipher has `C::key_size()`
// larger than 255 * chunk_size of the digest (so for example keys larger than 8160 bytes if sh256 is used)
#[allow(clippy::expect_used)]
let okm = hkdf::extract_then_expand::<D>(None, &dh_result, None, C::key_size()) let okm = hkdf::extract_then_expand::<D>(None, &dh_result, None, C::key_size())
.expect("somehow too long okm was provided"); .expect("somehow too long okm was provided");
let derived_shared_key = // SAFETY: the generated okm has exactly `C::key_size()` elements,
Key::<C>::from_exact_iter(okm).expect("okm was expanded to incorrect length!"); // so this call is safe
#[allow(clippy::unwrap_used)]
let derived_shared_key = Key::<C>::from_exact_iter(okm).unwrap();
(ephemeral_keypair, derived_shared_key) (ephemeral_keypair, derived_shared_key)
} }
@@ -48,9 +52,14 @@ where
{ {
let dh_result = local_key.diffie_hellman(remote_key); let dh_result = local_key.diffie_hellman(remote_key);
// there is no reason for this to fail as our okm is expected to be only C::KeySize bytes // SAFETY: while this is a relatively weak assumption, it's unlikely that any stream cipher has `C::key_size()`
// larger than 255 * chunk_size of the digest (so for example keys larger than 8160 bytes if sh256 is used)
#[allow(clippy::expect_used)]
let okm = hkdf::extract_then_expand::<D>(None, &dh_result, None, C::key_size()) let okm = hkdf::extract_then_expand::<D>(None, &dh_result, None, C::key_size())
.expect("somehow too long okm was provided"); .expect("somehow too long okm was provided");
Key::<C>::from_exact_iter(okm).expect("okm was expanded to incorrect length!") // SAFETY: the generated okm has exactly `C::key_size()` elements,
// so this call is safe
#[allow(clippy::unwrap_used)]
Key::<C>::from_exact_iter(okm).unwrap()
} }
+4 -9
View File
@@ -60,20 +60,15 @@ where
Iv::<C>::default() Iv::<C>::default()
} }
pub fn iv_from_slice<C>(b: &[u8]) -> &IV<C> pub fn try_iv_from_slice<C>(b: &[u8]) -> Option<&IV<C>>
where where
C: IvSizeUser, C: IvSizeUser,
{ {
if b.len() != C::iv_size() { if b.len() != C::iv_size() {
// `from_slice` would have caused a panic about this issue anyway. None
// Now we at least have slightly more information } else {
panic!( Some(IV::<C>::from_slice(b))
"Tried to convert {} bytes to IV. Expected {}",
b.len(),
C::iv_size()
)
} }
IV::<C>::from_slice(b)
} }
// TODO: there's really no way to use more parts of the keystream if it was required at some point. // TODO: there's really no way to use more parts of the keystream if it was required at some point.
@@ -2,7 +2,9 @@
// SPDX-License-Identifier: Apache-2.0 // SPDX-License-Identifier: Apache-2.0
use crate::AckKey; use crate::AckKey;
use nym_crypto::symmetric::stream_cipher::{self, encrypt, iv_from_slice, random_iv, IvSizeUser}; use nym_crypto::symmetric::stream_cipher::{
self, encrypt, random_iv, try_iv_from_slice, IvSizeUser,
};
use nym_sphinx_params::{AckEncryptionAlgorithm, SerializedFragmentIdentifier, FRAG_ID_LEN}; use nym_sphinx_params::{AckEncryptionAlgorithm, SerializedFragmentIdentifier, FRAG_ID_LEN};
use rand::{CryptoRng, RngCore}; use rand::{CryptoRng, RngCore};
@@ -25,7 +27,11 @@ pub fn recover_identifier(
iv_id_ciphertext: &[u8], iv_id_ciphertext: &[u8],
) -> Option<SerializedFragmentIdentifier> { ) -> Option<SerializedFragmentIdentifier> {
let iv_size = AckEncryptionAlgorithm::iv_size(); let iv_size = AckEncryptionAlgorithm::iv_size();
let iv = iv_from_slice::<AckEncryptionAlgorithm>(&iv_id_ciphertext[..iv_size]); if iv_id_ciphertext.len() < FRAG_ID_LEN + iv_size {
return None;
}
let iv = try_iv_from_slice::<AckEncryptionAlgorithm>(&iv_id_ciphertext[..iv_size])?;
let id = stream_cipher::decrypt::<AckEncryptionAlgorithm>( let id = stream_cipher::decrypt::<AckEncryptionAlgorithm>(
key.inner(), key.inner(),
+3
View File
@@ -141,3 +141,6 @@ cw3 = { workspace = true }
cw-utils = { workspace = true } cw-utils = { workspace = true }
rand_chacha = { workspace = true } rand_chacha = { workspace = true }
sha2 = "0.9" sha2 = "0.9"
[lints]
workspace = true
+3
View File
@@ -1,6 +1,9 @@
use sqlx::{Connection, FromRow, SqliteConnection}; use sqlx::{Connection, FromRow, SqliteConnection};
use std::env; use std::env;
// it's fine if compilation fails
#[allow(clippy::unwrap_used)]
#[allow(clippy::expect_used)]
#[tokio::main] #[tokio::main]
async fn main() { async fn main() {
let out_dir = env::var("OUT_DIR").unwrap(); let out_dir = env::var("OUT_DIR").unwrap();
+3 -1
View File
@@ -47,6 +47,8 @@ impl CachedEpoch {
let now = OffsetDateTime::now_utc(); let now = OffsetDateTime::now_utc();
let validity_duration = if let Some(epoch_finish) = epoch.deadline { let validity_duration = if let Some(epoch_finish) = epoch.deadline {
// SAFETY: values set in our contract are valid unix timestamps
#[allow(clippy::unwrap_used)]
let state_end = let state_end =
OffsetDateTime::from_unix_timestamp(epoch_finish.seconds() as i64).unwrap(); OffsetDateTime::from_unix_timestamp(epoch_finish.seconds() as i64).unwrap();
let until_epoch_state_end = state_end - now; let until_epoch_state_end = state_end - now;
@@ -103,7 +105,7 @@ impl APICommunicationChannel for QueryCommunicationChannel {
drop(guard); drop(guard);
let guard = self.update_epoch_cache().await?; let guard = self.update_epoch_cache().await?;
return Ok(guard.current_epoch.epoch_id); Ok(guard.current_epoch.epoch_id)
} }
// TODO: perhaps this should be returning a ReadGuard instead? // TODO: perhaps this should be returning a ReadGuard instead?
+4 -1
View File
@@ -189,7 +189,10 @@ impl<R: RngCore + CryptoRng + Clone> DkgController<R> {
self.state.clear_previous_epoch(epoch_id); self.state.clear_previous_epoch(epoch_id);
// SAFETY: we just accessed this item in an immutable way, thus it MUST exist so the unwrap is fine // SAFETY: we just accessed this item in an immutable way, thus it MUST exist so the unwrap is fine
self.state.in_progress_state_mut(epoch_id).unwrap().entered = true; #[allow(clippy::unwrap_used)]
{
self.state.in_progress_state_mut(epoch_id).unwrap().entered = true;
}
} }
// so at this point we don't need to be polling the contract so often anymore, but we can't easily // so at this point we don't need to be polling the contract so often anymore, but we can't easily
+2 -1
View File
@@ -204,8 +204,9 @@ impl<R: RngCore + CryptoRng> DkgController<R> {
chunk_dealing(*dealing_index, dealing.to_bytes(), Self::DEALING_CHUNK_SIZE); chunk_dealing(*dealing_index, dealing.to_bytes(), Self::DEALING_CHUNK_SIZE);
for chunk_index in needs_resubmission { for chunk_index in needs_resubmission {
// this is a hard failure, panic level, actually. // this is a hard failure, panic level, actually.
// because we have already committed to dealings of particular size // because we have already committed to dealings of particular size,
// yet we don't have relevant chunks after chunking // yet we don't have relevant chunks after chunking
#[allow(clippy::expect_used)]
let chunk = chunks let chunk = chunks
.remove(chunk_index) .remove(chunk_index)
.expect("chunking specification has changed mid-exchange!"); .expect("chunking specification has changed mid-exchange!");
+4
View File
@@ -177,6 +177,8 @@ impl<R: RngCore + CryptoRng> DkgController<R> {
// SAFETY: // SAFETY:
// since this share appears as 'verified' on the chain, it means the consensus of dealers confirmed its validity // since this share appears as 'verified' on the chain, it means the consensus of dealers confirmed its validity
// and thus they must have been able to parse it, so the unwrap/expect here is fine // and thus they must have been able to parse it, so the unwrap/expect here is fine
// (unless quorum of validators is malicious, but at that point we have bigger problems...)
#[allow(clippy::expect_used)]
Ok(Some( Ok(Some(
VerificationKeyAuth::try_from_bs58(&share.share) VerificationKeyAuth::try_from_bs58(&share.share)
.expect("failed to deserialize VERIFIED key"), .expect("failed to deserialize VERIFIED key"),
@@ -415,6 +417,7 @@ impl<R: RngCore + CryptoRng> DkgController<R> {
// SAFETY: combining shares can only fail if we have different number shares and indices // SAFETY: combining shares can only fail if we have different number shares and indices
// however, we returned an explicit error if decryption of any share failed and thus we know those values must match // however, we returned an explicit error if decryption of any share failed and thus we know those values must match
#[allow(clippy::unwrap_used)]
let secret = combine_shares(shares, &all_dealers).unwrap(); let secret = combine_shares(shares, &all_dealers).unwrap();
if derived_x.is_none() { if derived_x.is_none() {
derived_x = Some(secret) derived_x = Some(secret)
@@ -426,6 +429,7 @@ impl<R: RngCore + CryptoRng> DkgController<R> {
// SAFETY: // SAFETY:
// we know we had a non-empty map of dealings and thus, at the very least, we must have derived a single secret // we know we had a non-empty map of dealings and thus, at the very least, we must have derived a single secret
// (i.e. the x-element) // (i.e. the x-element)
#[allow(clippy::unwrap_used)]
let sk = SecretKeyAuth::create_from_raw(derived_x.unwrap(), derived_secrets); let sk = SecretKeyAuth::create_from_raw(derived_x.unwrap(), derived_secrets);
let derived_vk = sk.verification_key(); let derived_vk = sk.verification_key();
+3 -2
View File
@@ -140,6 +140,9 @@ impl EcashState {
} }
} }
// whilst we normally don't want to panic, this one would only occur at startup,
// if some logical invariants got broken (which have to be fixed in code anyway)
#[allow(clippy::panic)]
pub(crate) fn spawn_background_cleaner(&mut self) { pub(crate) fn spawn_background_cleaner(&mut self) {
match std::mem::take(&mut self.background_cleaner_state) { match std::mem::take(&mut self.background_cleaner_state) {
BackgroundCleanerState::WaitingStartup(cleaner) => { BackgroundCleanerState::WaitingStartup(cleaner) => {
@@ -147,8 +150,6 @@ impl EcashState {
_handle: cleaner.start(), _handle: cleaner.start(),
} }
} }
// whilst we normally don't want to panic, this one would only occur at startup,
// if some logical invariants got broken (which have to be fixed in code anyway)
_ => panic!("attempted to spawn background cleaner more than once"), _ => panic!("attempted to spawn background cleaner more than once"),
} }
} }
+6
View File
@@ -13,6 +13,9 @@ struct StorageBorrowedSerdeWrapper<'a, T>(&'a T);
#[derive(Serialize, Deserialize)] #[derive(Serialize, Deserialize)]
struct StorageSerdeWrapper<T>(T); struct StorageSerdeWrapper<T>(T);
// SAFETY: we're not using custom serialiser for AnnotatedCoinIndexSignature
// and we're within bound limits
#[allow(clippy::unwrap_used)]
pub(crate) fn serialise_coin_index_signatures(sigs: &[AnnotatedCoinIndexSignature]) -> Vec<u8> { pub(crate) fn serialise_coin_index_signatures(sigs: &[AnnotatedCoinIndexSignature]) -> Vec<u8> {
storage_serialiser() storage_serialiser()
.serialize(&StorageBorrowedSerdeWrapper(&sigs)) .serialize(&StorageBorrowedSerdeWrapper(&sigs))
@@ -28,6 +31,9 @@ pub(crate) fn deserialise_coin_index_signatures(
Ok(de.0) Ok(de.0)
} }
// SAFETY: we're not using custom serialiser for AnnotatedExpirationDateSignature
// and we're within bound limits
#[allow(clippy::unwrap_used)]
pub(crate) fn serialise_expiration_date_signatures( pub(crate) fn serialise_expiration_date_signatures(
sigs: &[AnnotatedExpirationDateSignature], sigs: &[AnnotatedExpirationDateSignature],
) -> Vec<u8> { ) -> Vec<u8> {
+10 -10
View File
@@ -428,27 +428,27 @@ impl FakeChainState {
); );
let epoch_id = self.dkg_contract.epoch.epoch_id; let epoch_id = self.dkg_contract.epoch.epoch_id;
let Some(shares) = self.dkg_contract.verification_shares.get_mut(&epoch_id) else { let Some(shares) = self.dkg_contract.verification_shares.get_mut(&epoch_id) else {
unimplemented!("no shares for epoch") panic!("no shares for epoch")
}; };
let Some(share) = shares.get_mut(owner.as_str()) else { let Some(share) = shares.get_mut(owner.as_str()) else {
unimplemented!("no shares for owner") panic!("no shares for owner")
}; };
share.verified = true share.verified = true
} }
other => unimplemented!("unimplemented exec of {other:?}"), other => panic!("unimplemented exec of {other:?}"),
} }
} }
// TODO: make it return a result // TODO: make it return a result
fn execute_contract_msg(&mut self, contract: &String, msg: &Binary, sender: MessageInfo) { fn execute_contract_msg(&mut self, contract: &String, msg: &Binary, sender: MessageInfo) {
if contract == &self.group_contract.address { if contract == &self.group_contract.address {
unimplemented!("group contract exec") panic!("group contract exec")
} }
if contract == &self.multisig_contract.address { if contract == &self.multisig_contract.address {
unimplemented!("multisig contract exec") panic!("multisig contract exec")
} }
if contract == &self.ecash_contract.address { if contract == &self.ecash_contract.address {
unimplemented!("bandwidth contract exec") panic!("bandwidth contract exec")
} }
if contract == self.dkg_contract.address.as_ref() { if contract == self.dkg_contract.address.as_ref() {
return self.execute_dkg_contract(sender, msg); return self.execute_dkg_contract(sender, msg);
@@ -467,7 +467,7 @@ impl FakeChainState {
let sender = mock_info(sender_address.as_ref(), funds); let sender = mock_info(sender_address.as_ref(), funds);
self.execute_contract_msg(contract_addr, msg, sender) self.execute_contract_msg(contract_addr, msg, sender)
} }
other => unimplemented!("unimplemented wasm proposal for {other:?}"), other => panic!("unimplemented wasm proposal for {other:?}"),
} }
} }
@@ -477,7 +477,7 @@ impl FakeChainState {
CosmosMsg::Wasm(wasm_msg) => { CosmosMsg::Wasm(wasm_msg) => {
self.execute_wasm_msg(wasm_msg, Addr::unchecked(sender_address.as_ref())) self.execute_wasm_msg(wasm_msg, Addr::unchecked(sender_address.as_ref()))
} }
other => unimplemented!("unimplemented proposal for {other:?}"), other => panic!("unimplemented proposal for {other:?}"),
}; };
} }
} }
@@ -907,7 +907,7 @@ impl super::client::Client for DummyClient {
}; };
if proposal.status != cw3::Status::Passed { if proposal.status != cw3::Status::Passed {
unimplemented!("proposal hasn't been passed") panic!("proposal hasn't been passed")
} }
proposal.status = cw3::Status::Executed; proposal.status = cw3::Status::Executed;
@@ -954,7 +954,7 @@ impl super::client::Client for DummyClient {
if !epoch_dealers.contains_key(self.validator_address.as_ref()) { if !epoch_dealers.contains_key(self.validator_address.as_ref()) {
epoch_dealers.insert(self.validator_address.to_string(), dealer_details); epoch_dealers.insert(self.validator_address.to_string(), dealer_details);
} else { } else {
unimplemented!("already registered") panic!("already registered")
} }
let transaction_hash = guard._counters.next_tx_hash(); let transaction_hash = guard._counters.next_tx_hash();
+3
View File
@@ -105,8 +105,11 @@ impl EpochAdvancer {
let standby_node_work_factor = global_rewarding_params.standby_node_work(); let standby_node_work_factor = global_rewarding_params.standby_node_work();
// SANITY CHECK: // SANITY CHECK:
// SAFETY: 0 decimal places is within the range of `Decimal`
#[allow(clippy::unwrap_used)]
let standby_share = Decimal::from_atomics(nodes.standby.len() as u128, 0).unwrap() let standby_share = Decimal::from_atomics(nodes.standby.len() as u128, 0).unwrap()
* standby_node_work_factor; * standby_node_work_factor;
#[allow(clippy::unwrap_used)]
let active_share = Decimal::from_atomics(nodes.active_set_size() as u128, 0).unwrap() let active_share = Decimal::from_atomics(nodes.active_set_size() as u128, 0).unwrap()
* active_node_work_factor; * active_node_work_factor;
let total_work = standby_share + active_share; let total_work = standby_share + active_share;
@@ -139,6 +139,7 @@ impl EpochAdvancer {
let mut layer2 = Vec::new(); let mut layer2 = Vec::new();
let mut layer3 = Vec::new(); let mut layer3 = Vec::new();
#[allow(clippy::panic)]
for (i, mix) in mixnodes_vec.iter().enumerate() { for (i, mix) in mixnodes_vec.iter().enumerate() {
match i % 3 { match i % 3 {
0 => layer1.push(*mix), 0 => layer1.push(*mix),
@@ -207,6 +208,7 @@ impl EpochAdvancer {
let mut with_performance = Vec::new(); let mut with_performance = Vec::new();
// SAFETY: the cache MUST HAVE been initialised before now // SAFETY: the cache MUST HAVE been initialised before now
#[allow(clippy::unwrap_used)]
let described_cache = self.described_cache.get().await.unwrap(); let described_cache = self.described_cache.get().await.unwrap();
let Some(status_cache) = self.status_cache.node_annotations().await else { let Some(status_cache) = self.status_cache.node_annotations().await else {
@@ -229,6 +229,8 @@ impl<R: MessageReceiver + Send + Sync> Monitor<R> {
// ideally we would blacklist all nodes regardless of the result so we would not use them anymore // ideally we would blacklist all nodes regardless of the result so we would not use them anymore
// however, currently we have huge imbalance of gateways to mixnodes so we might accidentally // however, currently we have huge imbalance of gateways to mixnodes so we might accidentally
// discard working gateway because it was paired with broken mixnode // discard working gateway because it was paired with broken mixnode
// SAFETY: the results is subset of candidates so the entry must exist
#[allow(clippy::unwrap_used)]
if *results.get(&candidate.id()).unwrap() { if *results.get(&candidate.id()).unwrap() {
// if the path is fully working, blacklist those nodes so we wouldn't construct // if the path is fully working, blacklist those nodes so we wouldn't construct
// any other path through any of those nodes // any other path through any of those nodes
@@ -157,6 +157,7 @@ impl PacketPreparer {
self.contract_cache.wait_for_initial_values().await; self.contract_cache.wait_for_initial_values().await;
self.described_cache.naive_wait_for_initial_values().await; self.described_cache.naive_wait_for_initial_values().await;
#[allow(clippy::expect_used)]
let described_nodes = self let described_nodes = self
.described_cache .described_cache
.get() .get()
@@ -374,6 +375,7 @@ impl PacketPreparer {
.collect::<Vec<_>>(); .collect::<Vec<_>>();
// the unwrap on `min()` is fine as we know the iterator is not empty // the unwrap on `min()` is fine as we know the iterator is not empty
#[allow(clippy::unwrap_used)]
let most_available = *[ let most_available = *[
rand_l1.len(), rand_l1.len(),
rand_l2.len(), rand_l2.len(),
@@ -427,6 +429,7 @@ impl PacketPreparer {
// 1. the topology is definitely valid (otherwise we wouldn't be here) // 1. the topology is definitely valid (otherwise we wouldn't be here)
// 2. the recipient is specified (by calling **mix**_tester) // 2. the recipient is specified (by calling **mix**_tester)
// 3. the test message is not too long, i.e. when serialized it will fit in a single sphinx packet // 3. the test message is not too long, i.e. when serialized it will fit in a single sphinx packet
#[allow(clippy::unwrap_used)]
let mix_packets = plaintexts let mix_packets = plaintexts
.into_iter() .into_iter()
.map(|p| tester.wrap_plaintext_data(p, &topology, None).unwrap()) .map(|p| tester.wrap_plaintext_data(p, &topology, None).unwrap())
@@ -492,6 +495,7 @@ impl PacketPreparer {
) -> PreparedPackets { ) -> PreparedPackets {
let (mixnodes, gateways) = self.all_legacy_mixnodes_and_gateways().await; let (mixnodes, gateways) = self.all_legacy_mixnodes_and_gateways().await;
#[allow(clippy::expect_used)]
let descriptions = self let descriptions = self
.described_cache .described_cache
.get() .get()
@@ -169,6 +169,9 @@ where
where where
R: Sync + Send + 'static, R: Sync + Send + 'static,
{ {
// this panic could only be triggered by incorrect startup sequence and shouldn't affect
// the binary beyond that
#[allow(clippy::expect_used)]
let mut receiver_task = self let mut receiver_task = self
.receiver_task .receiver_task
.take() .take()
@@ -8,7 +8,7 @@ use futures::StreamExt;
use nym_crypto::asymmetric::identity; use nym_crypto::asymmetric::identity;
use nym_gateway_client::{AcknowledgementReceiver, MixnetMessageReceiver}; use nym_gateway_client::{AcknowledgementReceiver, MixnetMessageReceiver};
use nym_task::TaskClient; use nym_task::TaskClient;
use tracing::trace; use tracing::{error, trace};
pub(crate) type GatewayClientUpdateSender = mpsc::UnboundedSender<GatewayClientUpdate>; pub(crate) type GatewayClientUpdateSender = mpsc::UnboundedSender<GatewayClientUpdate>;
pub(crate) type GatewayClientUpdateReceiver = mpsc::UnboundedReceiver<GatewayClientUpdate>; pub(crate) type GatewayClientUpdateReceiver = mpsc::UnboundedReceiver<GatewayClientUpdate>;
@@ -52,9 +52,9 @@ impl PacketReceiver {
} }
fn process_gateway_messages(&self, messages: GatewayMessages) { fn process_gateway_messages(&self, messages: GatewayMessages) {
self.processor_sender if self.processor_sender.unbounded_send(messages).is_err() {
.unbounded_send(messages) error!("packet processor seems to have crashed!")
.expect("packet processor seems to have crashed!"); }
} }
pub(crate) async fn run(&mut self, mut shutdown: TaskClient) { pub(crate) async fn run(&mut self, mut shutdown: TaskClient) {
@@ -66,7 +66,13 @@ impl PacketReceiver {
} }
// unwrap here is fine as it can only return a `None` if the PacketSender has died // unwrap here is fine as it can only return a `None` if the PacketSender has died
// and if that was the case, then the entire monitor is already in an undefined state // and if that was the case, then the entire monitor is already in an undefined state
update = self.clients_updater.next() => self.process_gateway_update(update.unwrap()), update = self.clients_updater.next() => {
if let Some(update) = update {
self.process_gateway_update(update)
} else {
error!("UpdateHandler: Client stream ended!");
}
},
Some((_gateway_id, messages)) = self.gateways_reader.next() => { Some((_gateway_id, messages)) = self.gateways_reader.next() => {
self.process_gateway_messages(messages) self.process_gateway_messages(messages)
} }
@@ -246,6 +246,8 @@ impl PacketSender {
trace!("Sending {} packets...", mix_packets.len()); trace!("Sending {} packets...", mix_packets.len());
if mix_packets.len() == 1 { if mix_packets.len() == 1 {
// SAFETY: we just explicitly checked we have 1 message
#[allow(clippy::unwrap_used)]
client.send_mix_packet(mix_packets.pop().unwrap()).await?; client.send_mix_packet(mix_packets.pop().unwrap()).await?;
} else { } else {
client.batch_send_mix_packets(mix_packets).await?; client.batch_send_mix_packets(mix_packets).await?;
@@ -104,6 +104,7 @@ impl TestRoute {
// the unwrap here is fine as the failure can only occur due to serialization and we're not // the unwrap here is fine as the failure can only occur due to serialization and we're not
// using any custom implementations // using any custom implementations
#[allow(clippy::unwrap_used)]
NymApiTestMessageExt::new(self.id, ROUTE_TESTING_TEST_NONCE) NymApiTestMessageExt::new(self.id, ROUTE_TESTING_TEST_NONCE)
.mix_plaintexts(mix, count as u32) .mix_plaintexts(mix, count as u32)
.unwrap() .unwrap()
+5
View File
@@ -125,6 +125,8 @@ impl TryFrom<i64> for Uptime {
impl From<Uptime> for Performance { impl From<Uptime> for Performance {
fn from(uptime: Uptime) -> Self { fn from(uptime: Uptime) -> Self {
// SAFETY: uptime has a valid range to be transformed into a `Performance`
#[allow(clippy::unwrap_used)]
Performance::from_percentage_value(uptime.0 as u64).unwrap() Performance::from_percentage_value(uptime.0 as u64).unwrap()
} }
} }
@@ -481,6 +483,9 @@ pub enum NymApiStorageError {
// this one would never be returned to users since it's only possible on startup // this one would never be returned to users since it's only possible on startup
#[error("failed to perform startup SQL migration - {0}")] #[error("failed to perform startup SQL migration - {0}")]
StartupMigrationFailure(#[from] sqlx::migrate::MigrateError), StartupMigrationFailure(#[from] sqlx::migrate::MigrateError),
#[error("{value} is not a valid unix timestamp")]
InvalidTimestampProvided { value: i64 },
} }
impl From<sqlx::Error> for NymApiStorageError { impl From<sqlx::Error> for NymApiStorageError {
@@ -8,7 +8,8 @@ use crate::node_status_api::ONE_DAY;
use crate::storage::NymApiStorage; use crate::storage::NymApiStorage;
use nym_task::{TaskClient, TaskManager}; use nym_task::{TaskClient, TaskManager};
use std::time::Duration; use std::time::Duration;
use time::{OffsetDateTime, PrimitiveDateTime, Time}; use time::macros::time;
use time::{OffsetDateTime, PrimitiveDateTime};
use tokio::time::{interval_at, Instant}; use tokio::time::{interval_at, Instant};
use tracing::error; use tracing::error;
use tracing::{info, trace, warn}; use tracing::{info, trace, warn};
@@ -75,18 +76,20 @@ impl HistoricalUptimeUpdater {
// nodes update for different days // nodes update for different days
// the unwrap is fine as 23:00:00 is a valid time // the unwrap is fine as 23:00:00 is a valid time
let update_time = Time::from_hms(23, 0, 0).unwrap(); let update_time = time!(23:00:00);
let now = OffsetDateTime::now_utc(); let now = OffsetDateTime::now_utc();
// is the current time within 0:00 - 22:59:59 or 23:00 - 23:59:59 ? // is the current time within 0:00 - 22:59:59 or 23:00 - 23:59:59 ?
let update_date = if now.hour() < 23 { let update_date = if now.hour() < 23 {
now.date() now.date()
} else { } else {
// the unwrap is fine as (**PRESUMABLY**) we're not running this code in the year 9999 // the unwrap is fine as (**PRESUMABLY**) we're not running this code in the year 9999
#[allow(clippy::unwrap_used)]
now.date().next_day().unwrap() now.date().next_day().unwrap()
}; };
let update_datetime = PrimitiveDateTime::new(update_date, update_time).assume_utc(); let update_datetime = PrimitiveDateTime::new(update_date, update_time).assume_utc();
// the unwrap here is fine as we're certain `update_datetime` is in the future and thus the // the unwrap here is fine as we're certain `update_datetime` is in the future and thus the
// resultant Duration is positive // resultant Duration is positive
#[allow(clippy::unwrap_used)]
let time_left: Duration = (update_datetime - now).try_into().unwrap(); let time_left: Duration = (update_datetime - now).try_into().unwrap();
info!( info!(
+1
View File
@@ -109,6 +109,7 @@ impl NodeUptimes {
// the unwraps in Uptime::from_ratio are fine because it's impossible for us to have more "up" results // the unwraps in Uptime::from_ratio are fine because it's impossible for us to have more "up" results
// than total test runs as we just bounded them // than total test runs as we just bounded them
#[allow(clippy::unwrap_used)]
NodeUptimes { NodeUptimes {
most_recent: most_recent.try_into().unwrap(), most_recent: most_recent.try_into().unwrap(),
last_hour: Uptime::from_uptime_sum(last_hour_sum, last_hour_test_runs).unwrap(), last_hour: Uptime::from_uptime_sum(last_hour_sum, last_hour_test_runs).unwrap(),
+1
View File
@@ -128,6 +128,7 @@ impl NymContractCacheRefresher {
.collect(); .collect();
let mut gateways = Vec::with_capacity(gateway_bonds.len()); let mut gateways = Vec::with_capacity(gateway_bonds.len());
#[allow(clippy::panic)]
for bond in gateway_bonds { for bond in gateway_bonds {
// we explicitly panic here because that value MUST exist. // we explicitly panic here because that value MUST exist.
// if it doesn't, we messed up the migration and we have big problems // if it doesn't, we messed up the migration and we have big problems
@@ -47,5 +47,8 @@ impl LegacyAnnotation for GatewayBondAnnotated {
pub(crate) fn refreshed_at( pub(crate) fn refreshed_at(
iter: impl IntoIterator<Item = OffsetDateTime>, iter: impl IntoIterator<Item = OffsetDateTime>,
) -> OffsetDateTimeJsonSchemaWrapper { ) -> OffsetDateTimeJsonSchemaWrapper {
iter.into_iter().min().unwrap().into() iter.into_iter()
.min()
.unwrap_or(OffsetDateTime::UNIX_EPOCH)
.into()
} }
+1 -1
View File
@@ -108,7 +108,7 @@ pub(crate) struct Args {
async fn start_nym_api_tasks_axum(config: &Config) -> anyhow::Result<ShutdownHandles> { async fn start_nym_api_tasks_axum(config: &Config) -> anyhow::Result<ShutdownHandles> {
let task_manager = TaskManager::new(TASK_MANAGER_TIMEOUT_S); let task_manager = TaskManager::new(TASK_MANAGER_TIMEOUT_S);
let nyxd_client = nyxd::Client::new(config); let nyxd_client = nyxd::Client::new(config)?;
let connected_nyxd = config.get_nyxd_url(); let connected_nyxd = config.get_nyxd_url();
let nym_network_details = NymNetworkDetails::new_from_env(); let nym_network_details = NymNetworkDetails::new_from_env();
let network_details = NetworkDetails::new(connected_nyxd.to_string(), nym_network_details); let network_details = NetworkDetails::new(connected_nyxd.to_string(), nym_network_details);
+2
View File
@@ -268,6 +268,8 @@ pub struct Base {
impl Base { impl Base {
pub fn new_default<S: Into<String>>(id: S) -> Self { pub fn new_default<S: Into<String>>(id: S) -> Self {
// SAFETY: the provided hardcoded value is well-formed
#[allow(clippy::expect_used)]
let default_validator: Url = DEFAULT_LOCAL_VALIDATOR let default_validator: Url = DEFAULT_LOCAL_VALIDATOR
.parse() .parse()
.expect("default local validator is malformed!"); .expect("default local validator is malformed!");
+9 -8
View File
@@ -4,7 +4,7 @@
use crate::ecash::error::EcashError; use crate::ecash::error::EcashError;
use crate::epoch_operations::RewardedNodeWithParams; use crate::epoch_operations::RewardedNodeWithParams;
use crate::support::config::Config; use crate::support::config::Config;
use anyhow::Result; use anyhow::{Context, Result};
use async_trait::async_trait; use async_trait::async_trait;
use cw3::{ProposalResponse, VoteResponse}; use cw3::{ProposalResponse, VoteResponse};
use cw4::MemberResponse; use cw4::MemberResponse;
@@ -99,12 +99,13 @@ pub enum ClientInner {
} }
impl Client { impl Client {
pub(crate) fn new(config: &Config) -> Self { pub(crate) fn new(config: &Config) -> anyhow::Result<Self> {
let details = NymNetworkDetails::new_from_env(); let details = NymNetworkDetails::new_from_env();
let nyxd_url = config.get_nyxd_url(); let nyxd_url = config.get_nyxd_url();
let client_config = nyxd::Config::try_from_nym_network_details(&details) let client_config = nyxd::Config::try_from_nym_network_details(&details).context(
.expect("failed to construct valid validator client config with the provided network"); "failed to construct valid validator client config with the provided network",
)?;
let inner = if let Some(mnemonic) = config.get_mnemonic() { let inner = if let Some(mnemonic) = config.get_mnemonic() {
ClientInner::Signing( ClientInner::Signing(
@@ -113,18 +114,18 @@ impl Client {
nyxd_url.as_str(), nyxd_url.as_str(),
mnemonic.clone(), mnemonic.clone(),
) )
.expect("Failed to connect to nyxd!"), .context("Failed to connect to nyxd!")?,
) )
} else { } else {
ClientInner::Query( ClientInner::Query(
QueryHttpRpcNyxdClient::connect(client_config, nyxd_url.as_str()) QueryHttpRpcNyxdClient::connect(client_config, nyxd_url.as_str())
.expect("Failed to connect to nyxd!"), .context("Failed to connect to nyxd!")?,
) )
}; };
Client { Ok(Client {
inner: Arc::new(RwLock::new(inner)), inner: Arc::new(RwLock::new(inner)),
} })
} }
pub(crate) async fn read(&self) -> RwLockReadGuard<'_, ClientInner> { pub(crate) async fn read(&self) -> RwLockReadGuard<'_, ClientInner> {
+19 -16
View File
@@ -225,9 +225,9 @@ impl NymApiStorage {
.get_monitor_runs_count(day_ago, now.unix_timestamp()) .get_monitor_runs_count(day_ago, now.unix_timestamp())
.await?; .await?;
let mixnode_identity = self.manager.get_mixnode_identity_key(mix_id).await?.expect( let Some(mixnode_identity) = self.manager.get_mixnode_identity_key(mix_id).await? else {
"The node doesn't have an identity even though we have status information on it!", return Err(NymApiStorageError::DatabaseInconsistency { reason: format!("The node {mix_id} doesn't have an identity even though we have status information on it!") });
); };
Ok(MixnodeStatusReport::construct_from_last_day_reports( Ok(MixnodeStatusReport::construct_from_last_day_reports(
now, now,
@@ -262,13 +262,9 @@ impl NymApiStorage {
.get_monitor_runs_count(day_ago, now.unix_timestamp()) .get_monitor_runs_count(day_ago, now.unix_timestamp())
.await?; .await?;
let gateway_identity = self let Some(gateway_identity) = self.manager.get_gateway_identity_key(node_id).await? else {
.manager return Err(NymApiStorageError::DatabaseInconsistency { reason: format!("The node {node_id} doesn't have an identity even though we have status information on it!") });
.get_gateway_identity_key(node_id) };
.await?
.expect(
"The node doesn't have an identity even though we have status information on it!",
);
Ok(GatewayStatusReport::construct_from_last_day_reports( Ok(GatewayStatusReport::construct_from_last_day_reports(
now, now,
@@ -290,10 +286,9 @@ impl NymApiStorage {
return Err(NymApiStorageError::MixnodeUptimeHistoryNotFound { mix_id }); return Err(NymApiStorageError::MixnodeUptimeHistoryNotFound { mix_id });
} }
let mixnode_identity = let Some(mixnode_identity) = self.manager.get_mixnode_identity_key(mix_id).await? else {
self.manager.get_mixnode_identity_key(mix_id).await?.expect( return Err(NymApiStorageError::DatabaseInconsistency { reason: format!("The node {mix_id} doesn't have an identity even though we uptime history for it!") });
"The node doesn't have an identity even though we have uptime history for it!", };
);
Ok(MixnodeUptimeHistory::new(mix_id, mixnode_identity, history)) Ok(MixnodeUptimeHistory::new(mix_id, mixnode_identity, history))
} }
@@ -536,6 +531,10 @@ impl NymApiStorage {
start: i64, start: i64,
end: i64, end: i64,
) -> Result<Vec<MixnodeStatusReport>, NymApiStorageError> { ) -> Result<Vec<MixnodeStatusReport>, NymApiStorageError> {
let Ok(end_timestamp) = OffsetDateTime::from_unix_timestamp(end) else {
return Err(NymApiStorageError::InvalidTimestampProvided { value: end });
};
if (end - start) as u64 != ONE_DAY.as_secs() { if (end - start) as u64 != ONE_DAY.as_secs() {
warn!("Our current interval length breaks the 24h length assumption") warn!("Our current interval length breaks the 24h length assumption")
} }
@@ -553,7 +552,7 @@ impl NymApiStorage {
.into_iter() .into_iter()
.map(|statuses| { .map(|statuses| {
MixnodeStatusReport::construct_from_last_day_reports( MixnodeStatusReport::construct_from_last_day_reports(
OffsetDateTime::from_unix_timestamp(end).unwrap(), end_timestamp,
statuses.mix_id, statuses.mix_id,
statuses.identity, statuses.identity,
statuses.statuses, statuses.statuses,
@@ -579,6 +578,10 @@ impl NymApiStorage {
start: i64, start: i64,
end: i64, end: i64,
) -> Result<Vec<GatewayStatusReport>, NymApiStorageError> { ) -> Result<Vec<GatewayStatusReport>, NymApiStorageError> {
let Ok(end_timestamp) = OffsetDateTime::from_unix_timestamp(end) else {
return Err(NymApiStorageError::InvalidTimestampProvided { value: end });
};
if (end - start) as u64 != ONE_DAY.as_secs() { if (end - start) as u64 != ONE_DAY.as_secs() {
warn!("Our current interval length breaks the 24h length assumption") warn!("Our current interval length breaks the 24h length assumption")
} }
@@ -596,7 +599,7 @@ impl NymApiStorage {
.into_iter() .into_iter()
.map(|statuses| { .map(|statuses| {
GatewayStatusReport::construct_from_last_day_reports( GatewayStatusReport::construct_from_last_day_reports(
OffsetDateTime::from_unix_timestamp(end).unwrap(), end_timestamp,
statuses.node_id, statuses.node_id,
statuses.identity, statuses.identity,
statuses.statuses, statuses.statuses,
+4
View File
@@ -98,3 +98,7 @@ nym-ip-packet-router = { path = "../service-providers/ip-packet-router" }
[build-dependencies] [build-dependencies]
# temporary bonding information v1 (to grab and parse nym-mixnode and nym-gateway package versions) # temporary bonding information v1 (to grab and parse nym-mixnode and nym-gateway package versions)
cargo_metadata = { workspace = true } cargo_metadata = { workspace = true }
[lints]
workspace = true
+2
View File
@@ -68,6 +68,8 @@ fn print_signed_contract_msg(
println!("{}", output.format(&sign_output)); println!("{}", output.format(&sign_output));
} }
// SAFETY: clippy ArgGroup ensures only a single branch is actually called
#[allow(clippy::unreachable)]
pub async fn execute(args: Args) -> Result<(), NymNodeError> { pub async fn execute(args: Args) -> Result<(), NymNodeError> {
let config = try_load_current_config(args.config.config_path()).await?; let config = try_load_current_config(args.config.config_path()).await?;
let identity_keypair = let identity_keypair =
+3
View File
@@ -45,6 +45,9 @@ impl MetricsAggregator {
self.event_sender.clone() self.event_sender.clone()
} }
// we must panic here to terminate as soon as possible, because the underlying code
// has to be resolved as it implies some serious logic bugs
#[allow(clippy::panic)]
pub fn register_handler<H>(&mut self, handler: H, update_interval: impl Into<Option<Duration>>) pub fn register_handler<H>(&mut self, handler: H, update_interval: impl Into<Option<Duration>>)
where where
H: MetricsHandler, H: MetricsHandler,
@@ -226,6 +226,8 @@ impl OnUpdateMetricsHandler for PrometheusGlobalNodeMetricsRegistryUpdater {
impl MetricsHandler for PrometheusGlobalNodeMetricsRegistryUpdater { impl MetricsHandler for PrometheusGlobalNodeMetricsRegistryUpdater {
type Events = GlobalPrometheusData; type Events = GlobalPrometheusData;
// SAFETY: `PrometheusNodeMetricsRegistryUpdater` doesn't have any associated events
#[allow(clippy::panic)]
async fn handle_event(&mut self, _event: Self::Events) { async fn handle_event(&mut self, _event: Self::Events) {
panic!("this should have never been called! MetricsHandler has been incorrectly called on PrometheusNodeMetricsRegistryUpdater") panic!("this should have never been called! MetricsHandler has been incorrectly called on PrometheusNodeMetricsRegistryUpdater")
} }
@@ -62,6 +62,8 @@ impl OnUpdateMetricsHandler for LegacyMixingStatsUpdater {
impl MetricsHandler for LegacyMixingStatsUpdater { impl MetricsHandler for LegacyMixingStatsUpdater {
type Events = LegacyMixingData; type Events = LegacyMixingData;
// SAFETY: `LegacyMixingStatsUpdater` doesn't have any associated events
#[allow(clippy::panic)]
async fn handle_event(&mut self, _event: Self::Events) { async fn handle_event(&mut self, _event: Self::Events) {
panic!("this should have never been called! MetricsHandler has been incorrectly called on LegacyMixingStatsUpdater") panic!("this should have never been called! MetricsHandler has been incorrectly called on LegacyMixingStatsUpdater")
} }
@@ -101,6 +101,8 @@ impl OnUpdateMetricsHandler for MixnetMetricsCleaner {
impl MetricsHandler for MixnetMetricsCleaner { impl MetricsHandler for MixnetMetricsCleaner {
type Events = StaleMixnetMetrics; type Events = StaleMixnetMetrics;
// SAFETY: `MixnetMetricsCleaner` doesn't have any associated events
#[allow(clippy::panic)]
async fn handle_event(&mut self, _event: Self::Events) { async fn handle_event(&mut self, _event: Self::Events) {
panic!("this should have never been called! MetricsHandler has been incorrectly called on MixnetMetricsCleaner") panic!("this should have never been called! MetricsHandler has been incorrectly called on MixnetMetricsCleaner")
} }
@@ -54,6 +54,8 @@ impl OnUpdateMetricsHandler for PendingEgressPacketsUpdater {
impl MetricsHandler for PendingEgressPacketsUpdater { impl MetricsHandler for PendingEgressPacketsUpdater {
type Events = PendingEgressPackets; type Events = PendingEgressPackets;
// SAFETY: `PendingEgressPacketsUpdater` doesn't have any associated events
#[allow(clippy::panic)]
async fn handle_event(&mut self, _event: Self::Events) { async fn handle_event(&mut self, _event: Self::Events) {
panic!("this should have never been called! MetricsHandler has been incorrectly called on PendingEgressPacketsUpdater") panic!("this should have never been called! MetricsHandler has been incorrectly called on PendingEgressPacketsUpdater")
} }