diff --git a/Cargo.toml b/Cargo.toml index c1fee9aca5..bf65a433fa 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -437,3 +437,13 @@ opt-level = 'z' [profile.release.package.mix-fetch-wasm] # lto = true 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" \ No newline at end of file diff --git a/clippy.toml b/clippy.toml index 0358cdb508..9b4ca613ab 100644 --- a/clippy.toml +++ b/clippy.toml @@ -1,2 +1,3 @@ allow-unwrap-in-tests = true allow-expect-in-tests = true +allow-panic-in-tests = true \ No newline at end of file diff --git a/common/crypto/Cargo.toml b/common/crypto/Cargo.toml index ee8819585e..6e1f8cfb12 100644 --- a/common/crypto/Cargo.toml +++ b/common/crypto/Cargo.toml @@ -43,4 +43,7 @@ serde = ["dep:serde", "serde_bytes", "ed25519-dalek/serde", "x25519-dalek/serde" asymmetric = ["x25519-dalek", "ed25519-dalek", "zeroize"] hashing = ["blake3", "digest", "hkdf", "hmac", "generic-array", "sha2"] stream_cipher = ["aes", "ctr", "cipher", "generic-array"] -sphinx = ["nym-sphinx-types/sphinx"] \ No newline at end of file +sphinx = ["nym-sphinx-types/sphinx"] + +[lints] +workspace = true \ No newline at end of file diff --git a/common/crypto/src/hmac.rs b/common/crypto/src/hmac.rs index c2678eb0bd..f878df238c 100644 --- a/common/crypto/src/hmac.rs +++ b/common/crypto/src/hmac.rs @@ -16,8 +16,11 @@ pub fn compute_keyed_hmac(key: &[u8], data: &[u8]) -> HmacOutput where D: Digest + BlockSizeUser, { - let mut hmac = SimpleHmac::::new_from_slice(key) - .expect("HMAC was instantiated with a key of an invalid size!"); + // SAFETY: hmac is fine with keys of any size; if they're smaller than the block size of the underlying + // 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::::new_from_slice(key).unwrap(); hmac.update(data); hmac.finalize() } @@ -27,8 +30,11 @@ pub fn recompute_keyed_hmac_and_verify_tag(key: &[u8], data: &[u8], tag: &[u8 where D: Digest + BlockSizeUser, { - let mut hmac = SimpleHmac::::new_from_slice(key) - .expect("HMAC was instantiated with a key of an invalid size!"); + // SAFETY: hmac is fine with keys of any size; if they're smaller than the block size of the underlying + // 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::::new_from_slice(key).unwrap(); hmac.update(data); let tag_arr = Output::::from_slice(tag); diff --git a/common/crypto/src/shared_key.rs b/common/crypto/src/shared_key.rs index f90e99feb2..def5709339 100644 --- a/common/crypto/src/shared_key.rs +++ b/common/crypto/src/shared_key.rs @@ -27,12 +27,16 @@ where // after performing diffie-hellman we don't care about the private component anymore 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::(None, &dh_result, None, C::key_size()) .expect("somehow too long okm was provided"); - let derived_shared_key = - Key::::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)] + let derived_shared_key = Key::::from_exact_iter(okm).unwrap(); (ephemeral_keypair, derived_shared_key) } @@ -48,9 +52,14 @@ where { 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::(None, &dh_result, None, C::key_size()) .expect("somehow too long okm was provided"); - Key::::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::::from_exact_iter(okm).unwrap() } diff --git a/common/crypto/src/symmetric/stream_cipher.rs b/common/crypto/src/symmetric/stream_cipher.rs index 4bd21eb031..758f32617c 100644 --- a/common/crypto/src/symmetric/stream_cipher.rs +++ b/common/crypto/src/symmetric/stream_cipher.rs @@ -60,20 +60,15 @@ where Iv::::default() } -pub fn iv_from_slice(b: &[u8]) -> &IV +pub fn try_iv_from_slice(b: &[u8]) -> Option<&IV> where C: IvSizeUser, { if b.len() != C::iv_size() { - // `from_slice` would have caused a panic about this issue anyway. - // Now we at least have slightly more information - panic!( - "Tried to convert {} bytes to IV. Expected {}", - b.len(), - C::iv_size() - ) + None + } else { + Some(IV::::from_slice(b)) } - IV::::from_slice(b) } // TODO: there's really no way to use more parts of the keystream if it was required at some point. diff --git a/common/nymsphinx/acknowledgements/src/identifier.rs b/common/nymsphinx/acknowledgements/src/identifier.rs index abe1d58368..544e9be959 100644 --- a/common/nymsphinx/acknowledgements/src/identifier.rs +++ b/common/nymsphinx/acknowledgements/src/identifier.rs @@ -2,7 +2,9 @@ // SPDX-License-Identifier: Apache-2.0 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 rand::{CryptoRng, RngCore}; @@ -25,7 +27,11 @@ pub fn recover_identifier( iv_id_ciphertext: &[u8], ) -> Option { let iv_size = AckEncryptionAlgorithm::iv_size(); - let iv = iv_from_slice::(&iv_id_ciphertext[..iv_size]); + if iv_id_ciphertext.len() < FRAG_ID_LEN + iv_size { + return None; + } + + let iv = try_iv_from_slice::(&iv_id_ciphertext[..iv_size])?; let id = stream_cipher::decrypt::( key.inner(), diff --git a/nym-api/Cargo.toml b/nym-api/Cargo.toml index a6830e21c1..4db77f3ccd 100644 --- a/nym-api/Cargo.toml +++ b/nym-api/Cargo.toml @@ -141,3 +141,6 @@ cw3 = { workspace = true } cw-utils = { workspace = true } rand_chacha = { workspace = true } sha2 = "0.9" + +[lints] +workspace = true \ No newline at end of file diff --git a/nym-api/build.rs b/nym-api/build.rs index d6e0c45770..a2c0377698 100644 --- a/nym-api/build.rs +++ b/nym-api/build.rs @@ -1,6 +1,9 @@ use sqlx::{Connection, FromRow, SqliteConnection}; use std::env; +// it's fine if compilation fails +#[allow(clippy::unwrap_used)] +#[allow(clippy::expect_used)] #[tokio::main] async fn main() { let out_dir = env::var("OUT_DIR").unwrap(); diff --git a/nym-api/src/ecash/comm.rs b/nym-api/src/ecash/comm.rs index fd70c592eb..ef5deceb83 100644 --- a/nym-api/src/ecash/comm.rs +++ b/nym-api/src/ecash/comm.rs @@ -47,6 +47,8 @@ impl CachedEpoch { let now = OffsetDateTime::now_utc(); 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 = OffsetDateTime::from_unix_timestamp(epoch_finish.seconds() as i64).unwrap(); let until_epoch_state_end = state_end - now; @@ -103,7 +105,7 @@ impl APICommunicationChannel for QueryCommunicationChannel { drop(guard); 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? diff --git a/nym-api/src/ecash/dkg/controller/mod.rs b/nym-api/src/ecash/dkg/controller/mod.rs index 25cba6b48d..56d380bb1d 100644 --- a/nym-api/src/ecash/dkg/controller/mod.rs +++ b/nym-api/src/ecash/dkg/controller/mod.rs @@ -189,7 +189,10 @@ impl DkgController { 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 - 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 diff --git a/nym-api/src/ecash/dkg/dealing.rs b/nym-api/src/ecash/dkg/dealing.rs index 31da8a0ea5..ee7ad5b5a3 100644 --- a/nym-api/src/ecash/dkg/dealing.rs +++ b/nym-api/src/ecash/dkg/dealing.rs @@ -204,8 +204,9 @@ impl DkgController { chunk_dealing(*dealing_index, dealing.to_bytes(), Self::DEALING_CHUNK_SIZE); for chunk_index in needs_resubmission { // 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 + #[allow(clippy::expect_used)] let chunk = chunks .remove(chunk_index) .expect("chunking specification has changed mid-exchange!"); diff --git a/nym-api/src/ecash/dkg/key_derivation.rs b/nym-api/src/ecash/dkg/key_derivation.rs index 46818547d4..41cfd11d36 100644 --- a/nym-api/src/ecash/dkg/key_derivation.rs +++ b/nym-api/src/ecash/dkg/key_derivation.rs @@ -177,6 +177,8 @@ impl DkgController { // SAFETY: // 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 + // (unless quorum of validators is malicious, but at that point we have bigger problems...) + #[allow(clippy::expect_used)] Ok(Some( VerificationKeyAuth::try_from_bs58(&share.share) .expect("failed to deserialize VERIFIED key"), @@ -415,6 +417,7 @@ impl DkgController { // 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 + #[allow(clippy::unwrap_used)] let secret = combine_shares(shares, &all_dealers).unwrap(); if derived_x.is_none() { derived_x = Some(secret) @@ -426,6 +429,7 @@ impl DkgController { // SAFETY: // 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) + #[allow(clippy::unwrap_used)] let sk = SecretKeyAuth::create_from_raw(derived_x.unwrap(), derived_secrets); let derived_vk = sk.verification_key(); diff --git a/nym-api/src/ecash/state/mod.rs b/nym-api/src/ecash/state/mod.rs index 5d3c6412af..4ade072fb6 100644 --- a/nym-api/src/ecash/state/mod.rs +++ b/nym-api/src/ecash/state/mod.rs @@ -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) { match std::mem::take(&mut self.background_cleaner_state) { BackgroundCleanerState::WaitingStartup(cleaner) => { @@ -147,8 +150,6 @@ impl EcashState { _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"), } } diff --git a/nym-api/src/ecash/storage/helpers.rs b/nym-api/src/ecash/storage/helpers.rs index c0be8a9183..c7b8a0d507 100644 --- a/nym-api/src/ecash/storage/helpers.rs +++ b/nym-api/src/ecash/storage/helpers.rs @@ -13,6 +13,9 @@ struct StorageBorrowedSerdeWrapper<'a, T>(&'a T); #[derive(Serialize, Deserialize)] struct StorageSerdeWrapper(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 { storage_serialiser() .serialize(&StorageBorrowedSerdeWrapper(&sigs)) @@ -28,6 +31,9 @@ pub(crate) fn deserialise_coin_index_signatures( 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( sigs: &[AnnotatedExpirationDateSignature], ) -> Vec { diff --git a/nym-api/src/ecash/tests/mod.rs b/nym-api/src/ecash/tests/mod.rs index 9a71802f5c..c31ecc2ede 100644 --- a/nym-api/src/ecash/tests/mod.rs +++ b/nym-api/src/ecash/tests/mod.rs @@ -428,27 +428,27 @@ impl FakeChainState { ); let epoch_id = self.dkg_contract.epoch.epoch_id; 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 { - unimplemented!("no shares for owner") + panic!("no shares for owner") }; share.verified = true } - other => unimplemented!("unimplemented exec of {other:?}"), + other => panic!("unimplemented exec of {other:?}"), } } // TODO: make it return a result fn execute_contract_msg(&mut self, contract: &String, msg: &Binary, sender: MessageInfo) { if contract == &self.group_contract.address { - unimplemented!("group contract exec") + panic!("group contract exec") } if contract == &self.multisig_contract.address { - unimplemented!("multisig contract exec") + panic!("multisig contract exec") } if contract == &self.ecash_contract.address { - unimplemented!("bandwidth contract exec") + panic!("bandwidth contract exec") } if contract == self.dkg_contract.address.as_ref() { return self.execute_dkg_contract(sender, msg); @@ -467,7 +467,7 @@ impl FakeChainState { let sender = mock_info(sender_address.as_ref(), funds); 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) => { 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 { - unimplemented!("proposal hasn't been passed") + panic!("proposal hasn't been passed") } 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()) { epoch_dealers.insert(self.validator_address.to_string(), dealer_details); } else { - unimplemented!("already registered") + panic!("already registered") } let transaction_hash = guard._counters.next_tx_hash(); diff --git a/nym-api/src/epoch_operations/helpers.rs b/nym-api/src/epoch_operations/helpers.rs index ed9abf30ce..d21fd1aadb 100644 --- a/nym-api/src/epoch_operations/helpers.rs +++ b/nym-api/src/epoch_operations/helpers.rs @@ -105,8 +105,11 @@ impl EpochAdvancer { let standby_node_work_factor = global_rewarding_params.standby_node_work(); // 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() * standby_node_work_factor; + #[allow(clippy::unwrap_used)] let active_share = Decimal::from_atomics(nodes.active_set_size() as u128, 0).unwrap() * active_node_work_factor; let total_work = standby_share + active_share; diff --git a/nym-api/src/epoch_operations/rewarded_set_assignment.rs b/nym-api/src/epoch_operations/rewarded_set_assignment.rs index c2fda62dd6..40c1124b05 100644 --- a/nym-api/src/epoch_operations/rewarded_set_assignment.rs +++ b/nym-api/src/epoch_operations/rewarded_set_assignment.rs @@ -139,6 +139,7 @@ impl EpochAdvancer { let mut layer2 = Vec::new(); let mut layer3 = Vec::new(); + #[allow(clippy::panic)] for (i, mix) in mixnodes_vec.iter().enumerate() { match i % 3 { 0 => layer1.push(*mix), @@ -207,6 +208,7 @@ impl EpochAdvancer { let mut with_performance = Vec::new(); // SAFETY: the cache MUST HAVE been initialised before now + #[allow(clippy::unwrap_used)] let described_cache = self.described_cache.get().await.unwrap(); let Some(status_cache) = self.status_cache.node_annotations().await else { diff --git a/nym-api/src/network_monitor/monitor/mod.rs b/nym-api/src/network_monitor/monitor/mod.rs index 0145245197..360a946638 100644 --- a/nym-api/src/network_monitor/monitor/mod.rs +++ b/nym-api/src/network_monitor/monitor/mod.rs @@ -229,6 +229,8 @@ impl Monitor { // 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 // 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 the path is fully working, blacklist those nodes so we wouldn't construct // any other path through any of those nodes diff --git a/nym-api/src/network_monitor/monitor/preparer.rs b/nym-api/src/network_monitor/monitor/preparer.rs index 7ae2547844..7ee9888478 100644 --- a/nym-api/src/network_monitor/monitor/preparer.rs +++ b/nym-api/src/network_monitor/monitor/preparer.rs @@ -157,6 +157,7 @@ impl PacketPreparer { self.contract_cache.wait_for_initial_values().await; self.described_cache.naive_wait_for_initial_values().await; + #[allow(clippy::expect_used)] let described_nodes = self .described_cache .get() @@ -374,6 +375,7 @@ impl PacketPreparer { .collect::>(); // the unwrap on `min()` is fine as we know the iterator is not empty + #[allow(clippy::unwrap_used)] let most_available = *[ rand_l1.len(), rand_l2.len(), @@ -427,6 +429,7 @@ impl PacketPreparer { // 1. the topology is definitely valid (otherwise we wouldn't be here) // 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 + #[allow(clippy::unwrap_used)] let mix_packets = plaintexts .into_iter() .map(|p| tester.wrap_plaintext_data(p, &topology, None).unwrap()) @@ -492,6 +495,7 @@ impl PacketPreparer { ) -> PreparedPackets { let (mixnodes, gateways) = self.all_legacy_mixnodes_and_gateways().await; + #[allow(clippy::expect_used)] let descriptions = self .described_cache .get() diff --git a/nym-api/src/network_monitor/monitor/processor.rs b/nym-api/src/network_monitor/monitor/processor.rs index 7a65115d78..b69f919f74 100644 --- a/nym-api/src/network_monitor/monitor/processor.rs +++ b/nym-api/src/network_monitor/monitor/processor.rs @@ -169,6 +169,9 @@ where where 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 .receiver_task .take() diff --git a/nym-api/src/network_monitor/monitor/receiver.rs b/nym-api/src/network_monitor/monitor/receiver.rs index 91a8c67811..a317b9985b 100644 --- a/nym-api/src/network_monitor/monitor/receiver.rs +++ b/nym-api/src/network_monitor/monitor/receiver.rs @@ -8,7 +8,7 @@ use futures::StreamExt; use nym_crypto::asymmetric::identity; use nym_gateway_client::{AcknowledgementReceiver, MixnetMessageReceiver}; use nym_task::TaskClient; -use tracing::trace; +use tracing::{error, trace}; pub(crate) type GatewayClientUpdateSender = mpsc::UnboundedSender; pub(crate) type GatewayClientUpdateReceiver = mpsc::UnboundedReceiver; @@ -52,9 +52,9 @@ impl PacketReceiver { } fn process_gateway_messages(&self, messages: GatewayMessages) { - self.processor_sender - .unbounded_send(messages) - .expect("packet processor seems to have crashed!"); + if self.processor_sender.unbounded_send(messages).is_err() { + error!("packet processor seems to have crashed!") + } } 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 // 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() => { self.process_gateway_messages(messages) } diff --git a/nym-api/src/network_monitor/monitor/sender.rs b/nym-api/src/network_monitor/monitor/sender.rs index ca2743b97f..fa6ba454c9 100644 --- a/nym-api/src/network_monitor/monitor/sender.rs +++ b/nym-api/src/network_monitor/monitor/sender.rs @@ -246,6 +246,8 @@ impl PacketSender { trace!("Sending {} packets...", mix_packets.len()); 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?; } else { client.batch_send_mix_packets(mix_packets).await?; diff --git a/nym-api/src/network_monitor/test_route/mod.rs b/nym-api/src/network_monitor/test_route/mod.rs index 1032baf5db..d1384e851b 100644 --- a/nym-api/src/network_monitor/test_route/mod.rs +++ b/nym-api/src/network_monitor/test_route/mod.rs @@ -104,6 +104,7 @@ impl TestRoute { // the unwrap here is fine as the failure can only occur due to serialization and we're not // using any custom implementations + #[allow(clippy::unwrap_used)] NymApiTestMessageExt::new(self.id, ROUTE_TESTING_TEST_NONCE) .mix_plaintexts(mix, count as u32) .unwrap() diff --git a/nym-api/src/node_status_api/models.rs b/nym-api/src/node_status_api/models.rs index 19120e88b8..53f5d3b7d5 100644 --- a/nym-api/src/node_status_api/models.rs +++ b/nym-api/src/node_status_api/models.rs @@ -125,6 +125,8 @@ impl TryFrom for Uptime { impl From for Performance { 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() } } @@ -481,6 +483,9 @@ pub enum NymApiStorageError { // this one would never be returned to users since it's only possible on startup #[error("failed to perform startup SQL migration - {0}")] StartupMigrationFailure(#[from] sqlx::migrate::MigrateError), + + #[error("{value} is not a valid unix timestamp")] + InvalidTimestampProvided { value: i64 }, } impl From for NymApiStorageError { diff --git a/nym-api/src/node_status_api/uptime_updater.rs b/nym-api/src/node_status_api/uptime_updater.rs index affca9f7f2..a68979ced1 100644 --- a/nym-api/src/node_status_api/uptime_updater.rs +++ b/nym-api/src/node_status_api/uptime_updater.rs @@ -8,7 +8,8 @@ use crate::node_status_api::ONE_DAY; use crate::storage::NymApiStorage; use nym_task::{TaskClient, TaskManager}; use std::time::Duration; -use time::{OffsetDateTime, PrimitiveDateTime, Time}; +use time::macros::time; +use time::{OffsetDateTime, PrimitiveDateTime}; use tokio::time::{interval_at, Instant}; use tracing::error; use tracing::{info, trace, warn}; @@ -75,18 +76,20 @@ impl HistoricalUptimeUpdater { // nodes update for different days // 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(); // is the current time within 0:00 - 22:59:59 or 23:00 - 23:59:59 ? let update_date = if now.hour() < 23 { now.date() } else { // 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() }; 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 // resultant Duration is positive + #[allow(clippy::unwrap_used)] let time_left: Duration = (update_datetime - now).try_into().unwrap(); info!( diff --git a/nym-api/src/node_status_api/utils.rs b/nym-api/src/node_status_api/utils.rs index 4d86be5faa..2604254e92 100644 --- a/nym-api/src/node_status_api/utils.rs +++ b/nym-api/src/node_status_api/utils.rs @@ -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 // than total test runs as we just bounded them + #[allow(clippy::unwrap_used)] NodeUptimes { most_recent: most_recent.try_into().unwrap(), last_hour: Uptime::from_uptime_sum(last_hour_sum, last_hour_test_runs).unwrap(), diff --git a/nym-api/src/nym_contract_cache/cache/refresher.rs b/nym-api/src/nym_contract_cache/cache/refresher.rs index e4f57dae63..787d299de5 100644 --- a/nym-api/src/nym_contract_cache/cache/refresher.rs +++ b/nym-api/src/nym_contract_cache/cache/refresher.rs @@ -128,6 +128,7 @@ impl NymContractCacheRefresher { .collect(); let mut gateways = Vec::with_capacity(gateway_bonds.len()); + #[allow(clippy::panic)] for bond in gateway_bonds { // we explicitly panic here because that value MUST exist. // if it doesn't, we messed up the migration and we have big problems diff --git a/nym-api/src/nym_nodes/handlers/unstable/helpers.rs b/nym-api/src/nym_nodes/handlers/unstable/helpers.rs index 38c981a97a..3940d99237 100644 --- a/nym-api/src/nym_nodes/handlers/unstable/helpers.rs +++ b/nym-api/src/nym_nodes/handlers/unstable/helpers.rs @@ -47,5 +47,8 @@ impl LegacyAnnotation for GatewayBondAnnotated { pub(crate) fn refreshed_at( iter: impl IntoIterator, ) -> OffsetDateTimeJsonSchemaWrapper { - iter.into_iter().min().unwrap().into() + iter.into_iter() + .min() + .unwrap_or(OffsetDateTime::UNIX_EPOCH) + .into() } diff --git a/nym-api/src/support/cli/run.rs b/nym-api/src/support/cli/run.rs index c506d32508..836f048930 100644 --- a/nym-api/src/support/cli/run.rs +++ b/nym-api/src/support/cli/run.rs @@ -108,7 +108,7 @@ pub(crate) struct Args { async fn start_nym_api_tasks_axum(config: &Config) -> anyhow::Result { 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 nym_network_details = NymNetworkDetails::new_from_env(); let network_details = NetworkDetails::new(connected_nyxd.to_string(), nym_network_details); diff --git a/nym-api/src/support/config/mod.rs b/nym-api/src/support/config/mod.rs index 75793726f4..0868f908c5 100644 --- a/nym-api/src/support/config/mod.rs +++ b/nym-api/src/support/config/mod.rs @@ -268,6 +268,8 @@ pub struct Base { impl Base { pub fn new_default>(id: S) -> Self { + // SAFETY: the provided hardcoded value is well-formed + #[allow(clippy::expect_used)] let default_validator: Url = DEFAULT_LOCAL_VALIDATOR .parse() .expect("default local validator is malformed!"); diff --git a/nym-api/src/support/nyxd/mod.rs b/nym-api/src/support/nyxd/mod.rs index 180d1810e4..01a32decec 100644 --- a/nym-api/src/support/nyxd/mod.rs +++ b/nym-api/src/support/nyxd/mod.rs @@ -4,7 +4,7 @@ use crate::ecash::error::EcashError; use crate::epoch_operations::RewardedNodeWithParams; use crate::support::config::Config; -use anyhow::Result; +use anyhow::{Context, Result}; use async_trait::async_trait; use cw3::{ProposalResponse, VoteResponse}; use cw4::MemberResponse; @@ -99,12 +99,13 @@ pub enum ClientInner { } impl Client { - pub(crate) fn new(config: &Config) -> Self { + pub(crate) fn new(config: &Config) -> anyhow::Result { let details = NymNetworkDetails::new_from_env(); let nyxd_url = config.get_nyxd_url(); - let client_config = nyxd::Config::try_from_nym_network_details(&details) - .expect("failed to construct valid validator client config with the provided network"); + let client_config = nyxd::Config::try_from_nym_network_details(&details).context( + "failed to construct valid validator client config with the provided network", + )?; let inner = if let Some(mnemonic) = config.get_mnemonic() { ClientInner::Signing( @@ -113,18 +114,18 @@ impl Client { nyxd_url.as_str(), mnemonic.clone(), ) - .expect("Failed to connect to nyxd!"), + .context("Failed to connect to nyxd!")?, ) } else { ClientInner::Query( 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)), - } + }) } pub(crate) async fn read(&self) -> RwLockReadGuard<'_, ClientInner> { diff --git a/nym-api/src/support/storage/mod.rs b/nym-api/src/support/storage/mod.rs index e1d6b729c7..cc10d55e76 100644 --- a/nym-api/src/support/storage/mod.rs +++ b/nym-api/src/support/storage/mod.rs @@ -225,9 +225,9 @@ impl NymApiStorage { .get_monitor_runs_count(day_ago, now.unix_timestamp()) .await?; - let mixnode_identity = self.manager.get_mixnode_identity_key(mix_id).await?.expect( - "The node doesn't have an identity even though we have status information on it!", - ); + let Some(mixnode_identity) = self.manager.get_mixnode_identity_key(mix_id).await? else { + 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( now, @@ -262,13 +262,9 @@ impl NymApiStorage { .get_monitor_runs_count(day_ago, now.unix_timestamp()) .await?; - let gateway_identity = self - .manager - .get_gateway_identity_key(node_id) - .await? - .expect( - "The node doesn't have an identity even though we have status information on it!", - ); + let Some(gateway_identity) = self.manager.get_gateway_identity_key(node_id).await? else { + return Err(NymApiStorageError::DatabaseInconsistency { reason: format!("The node {node_id} doesn't have an identity even though we have status information on it!") }); + }; Ok(GatewayStatusReport::construct_from_last_day_reports( now, @@ -290,10 +286,9 @@ impl NymApiStorage { return Err(NymApiStorageError::MixnodeUptimeHistoryNotFound { mix_id }); } - let mixnode_identity = - self.manager.get_mixnode_identity_key(mix_id).await?.expect( - "The node doesn't have an identity even though we have uptime history for it!", - ); + let Some(mixnode_identity) = self.manager.get_mixnode_identity_key(mix_id).await? else { + return Err(NymApiStorageError::DatabaseInconsistency { reason: format!("The node {mix_id} doesn't have an identity even though we uptime history for it!") }); + }; Ok(MixnodeUptimeHistory::new(mix_id, mixnode_identity, history)) } @@ -536,6 +531,10 @@ impl NymApiStorage { start: i64, end: i64, ) -> Result, 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() { warn!("Our current interval length breaks the 24h length assumption") } @@ -553,7 +552,7 @@ impl NymApiStorage { .into_iter() .map(|statuses| { MixnodeStatusReport::construct_from_last_day_reports( - OffsetDateTime::from_unix_timestamp(end).unwrap(), + end_timestamp, statuses.mix_id, statuses.identity, statuses.statuses, @@ -579,6 +578,10 @@ impl NymApiStorage { start: i64, end: i64, ) -> Result, 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() { warn!("Our current interval length breaks the 24h length assumption") } @@ -596,7 +599,7 @@ impl NymApiStorage { .into_iter() .map(|statuses| { GatewayStatusReport::construct_from_last_day_reports( - OffsetDateTime::from_unix_timestamp(end).unwrap(), + end_timestamp, statuses.node_id, statuses.identity, statuses.statuses, diff --git a/nym-node/Cargo.toml b/nym-node/Cargo.toml index 6c65b81261..a794ca4e98 100644 --- a/nym-node/Cargo.toml +++ b/nym-node/Cargo.toml @@ -98,3 +98,7 @@ nym-ip-packet-router = { path = "../service-providers/ip-packet-router" } [build-dependencies] # temporary bonding information v1 (to grab and parse nym-mixnode and nym-gateway package versions) cargo_metadata = { workspace = true } + + +[lints] +workspace = true \ No newline at end of file diff --git a/nym-node/src/cli/commands/sign.rs b/nym-node/src/cli/commands/sign.rs index ab22077396..0b9fc56d5b 100644 --- a/nym-node/src/cli/commands/sign.rs +++ b/nym-node/src/cli/commands/sign.rs @@ -68,6 +68,8 @@ fn print_signed_contract_msg( 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> { let config = try_load_current_config(args.config.config_path()).await?; let identity_keypair = diff --git a/nym-node/src/node/metrics/aggregator.rs b/nym-node/src/node/metrics/aggregator.rs index 3b3cc17eb3..7d67e925c2 100644 --- a/nym-node/src/node/metrics/aggregator.rs +++ b/nym-node/src/node/metrics/aggregator.rs @@ -45,6 +45,9 @@ impl MetricsAggregator { 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(&mut self, handler: H, update_interval: impl Into>) where H: MetricsHandler, diff --git a/nym-node/src/node/metrics/handler/global_prometheus_updater/mod.rs b/nym-node/src/node/metrics/handler/global_prometheus_updater/mod.rs index 16587ead63..af7e907d48 100644 --- a/nym-node/src/node/metrics/handler/global_prometheus_updater/mod.rs +++ b/nym-node/src/node/metrics/handler/global_prometheus_updater/mod.rs @@ -226,6 +226,8 @@ impl OnUpdateMetricsHandler for PrometheusGlobalNodeMetricsRegistryUpdater { impl MetricsHandler for PrometheusGlobalNodeMetricsRegistryUpdater { type Events = GlobalPrometheusData; + // SAFETY: `PrometheusNodeMetricsRegistryUpdater` doesn't have any associated events + #[allow(clippy::panic)] async fn handle_event(&mut self, _event: Self::Events) { panic!("this should have never been called! MetricsHandler has been incorrectly called on PrometheusNodeMetricsRegistryUpdater") } diff --git a/nym-node/src/node/metrics/handler/legacy_packet_data.rs b/nym-node/src/node/metrics/handler/legacy_packet_data.rs index 6240993797..d45536e587 100644 --- a/nym-node/src/node/metrics/handler/legacy_packet_data.rs +++ b/nym-node/src/node/metrics/handler/legacy_packet_data.rs @@ -62,6 +62,8 @@ impl OnUpdateMetricsHandler for LegacyMixingStatsUpdater { impl MetricsHandler for LegacyMixingStatsUpdater { type Events = LegacyMixingData; + // SAFETY: `LegacyMixingStatsUpdater` doesn't have any associated events + #[allow(clippy::panic)] async fn handle_event(&mut self, _event: Self::Events) { panic!("this should have never been called! MetricsHandler has been incorrectly called on LegacyMixingStatsUpdater") } diff --git a/nym-node/src/node/metrics/handler/mixnet_data_cleaner.rs b/nym-node/src/node/metrics/handler/mixnet_data_cleaner.rs index 83c2041b5b..0c4db73333 100644 --- a/nym-node/src/node/metrics/handler/mixnet_data_cleaner.rs +++ b/nym-node/src/node/metrics/handler/mixnet_data_cleaner.rs @@ -101,6 +101,8 @@ impl OnUpdateMetricsHandler for MixnetMetricsCleaner { impl MetricsHandler for MixnetMetricsCleaner { type Events = StaleMixnetMetrics; + // SAFETY: `MixnetMetricsCleaner` doesn't have any associated events + #[allow(clippy::panic)] async fn handle_event(&mut self, _event: Self::Events) { panic!("this should have never been called! MetricsHandler has been incorrectly called on MixnetMetricsCleaner") } diff --git a/nym-node/src/node/metrics/handler/pending_egress_packets_updater.rs b/nym-node/src/node/metrics/handler/pending_egress_packets_updater.rs index d1fde40f70..f3a134cc37 100644 --- a/nym-node/src/node/metrics/handler/pending_egress_packets_updater.rs +++ b/nym-node/src/node/metrics/handler/pending_egress_packets_updater.rs @@ -54,6 +54,8 @@ impl OnUpdateMetricsHandler for PendingEgressPacketsUpdater { impl MetricsHandler for PendingEgressPacketsUpdater { type Events = PendingEgressPackets; + // SAFETY: `PendingEgressPacketsUpdater` doesn't have any associated events + #[allow(clippy::panic)] async fn handle_event(&mut self, _event: Self::Events) { panic!("this should have never been called! MetricsHandler has been incorrectly called on PendingEgressPacketsUpdater") }