Skip to content

Commit

Permalink
Merge branch 'tomas/more-checked-arith' (#3214)
Browse files Browse the repository at this point in the history
* origin/tomas/more-checked-arith:
  changelog: add #3214
  doc/sdk: fix bare urls
  doc/gas: fix broken link
  apps: fix lint warnings
  apps: add lints
  namada: fix lint warnings
  namada: add lints
  vp_env: fix lint warnings
  vp_env: add lints
  tx_env: add lints
  ethereum_bridge: fix lint warnings
  ethereum_bridge: add lints
  ibc: fix lint warnings
  ibc: add lints
  vm_env: fix lint warnings
  vm_env: add lints
  state: fix lint warnings
  state: add lints
  token: fix lint warnings
  token: add lints
  proof_of_stake: fix lint warnings
  proof_of_stake: add lints
  governance: fix lint warnings
  governance: add lints
  account: fix lint warnings
  account: add lints
  shielded_token: fix lint warnings
  shielded_token: add lints
  trans_token: add lints
  parameters: add lints
  controller: fix lint warnings
  controller: add lints
  storage: fix lint warnings
  storage: add lints
  vote_ext: add lints
  tx: fix lints warnings
  tx: add lints
  gas: fix lints warning
  gas: add lints
  merkle_tree: fix lints warning
  merkle_tree: add lints
  crates: update for checked events gas
  events: use checked arith
  events: add lints
  replay_protection: add lints
  • Loading branch information
brentstone committed May 21, 2024
2 parents 594b1bc + 860132b commit b4c95c3
Show file tree
Hide file tree
Showing 127 changed files with 1,472 additions and 605 deletions.
2 changes: 2 additions & 0 deletions .changelog/unreleased/improvements/3214-more-checked-arith.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
- Sanitized unchecked arithmetics and conversions in the codebase.
([\#3214](https://github.com/anoma/namada/pull/3214))
5 changes: 5 additions & 0 deletions Cargo.lock

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

14 changes: 14 additions & 0 deletions crates/account/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@
//! using public key(s) and signature threshold (minimum number of signatures
//! needed to authorize an action) stored on-chain.
#![doc(html_favicon_url = "https://dev.namada.net/master/favicon.png")]
#![doc(html_logo_url = "https://dev.namada.net/master/rustdoc-logo.png")]
#![deny(rustdoc::broken_intra_doc_links)]
#![deny(rustdoc::private_intra_doc_links)]
#![warn(
missing_docs,
rust_2018_idioms,
clippy::cast_sign_loss,
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,
clippy::cast_lossless,
clippy::arithmetic_side_effects
)]

mod storage;
mod storage_key;
mod types;
Expand Down
5 changes: 3 additions & 2 deletions crates/account/src/storage.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
//! Cryptographic signature keys storage API
use namada_core::storage;
use namada_storage::{Result, StorageRead, StorageWrite};
use namada_storage::{Result, ResultExt, StorageRead, StorageWrite};

use super::*;

Expand Down Expand Up @@ -31,7 +31,7 @@ where
S: StorageWrite + StorageRead,
{
for (index, public_key) in public_keys.iter().enumerate() {
let index = index as u8;
let index = u8::try_from(index).into_storage_result()?;
pks_handle(owner).insert(storage, index, public_key.clone())?;
}
let threshold_key = threshold_key(owner);
Expand Down Expand Up @@ -114,6 +114,7 @@ where
S: StorageWrite + StorageRead,
{
let total_pks = pks_handle(owner).len(storage)?;
let total_pks = u8::try_from(total_pks).into_storage_result()?;
for index in 0..total_pks as u8 {
pks_handle(owner).remove(storage, &index)?;
}
Expand Down
1 change: 1 addition & 0 deletions crates/account/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ pub struct UpdateAccount {
pub threshold: Option<u8>,
}

#[allow(clippy::cast_possible_truncation)]
#[cfg(any(test, feature = "testing"))]
/// Tests and strategies for accounts
pub mod tests {
Expand Down
1 change: 1 addition & 0 deletions crates/apps/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,7 @@ serde_json = {workspace = true, features = ["raw_value"]}
serde.workspace = true
sha2.workspace = true
signal-hook.workspace = true
smooth-operator.workspace = true
sysinfo.workspace = true
tar.workspace = true
tempfile.workspace = true
Expand Down
2 changes: 2 additions & 0 deletions crates/apps/src/lib/bench_utils.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
//! Library code for benchmarks provides a wrapper of the ledger's shell
//! `BenchShell` and helper functions to generate transactions.
#![allow(clippy::arithmetic_side_effects)]

use std::cell::RefCell;
use std::collections::BTreeSet;
use std::fs::{File, OpenOptions};
Expand Down
2 changes: 2 additions & 0 deletions crates/apps/src/lib/client/mod.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::arithmetic_side_effects)]

pub mod masp;
pub mod rpc;
pub mod tx;
Expand Down
6 changes: 5 additions & 1 deletion crates/apps/src/lib/config/genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,10 @@ impl<'de> Deserialize<'de> for GenesisAddress {
impl<'de> serde::de::Visitor<'de> for FieldVisitor {
type Value = GenesisAddress;

fn expecting(&self, formatter: &mut Formatter) -> std::fmt::Result {
fn expecting(
&self,
formatter: &mut Formatter<'_>,
) -> std::fmt::Result {
formatter.write_str(
"a bech32m encoded public key or an established address",
)
Expand Down Expand Up @@ -324,6 +327,7 @@ pub struct Parameters {
///
/// This includes adding the Ethereum bridge parameters and
/// adding a specified number of validators.
#[allow(clippy::arithmetic_side_effects)]
#[cfg(all(any(test, feature = "benches"), not(feature = "integration")))]
pub fn make_dev_genesis(
num_validators: u64,
Expand Down
15 changes: 12 additions & 3 deletions crates/apps/src/lib/config/genesis/chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -254,8 +254,14 @@ impl Finalized {
if !is_localhost {
set_ip(&mut config.ledger.cometbft.rpc.laddr, "0.0.0.0");
}
set_port(&mut config.ledger.cometbft.rpc.laddr, first_port + 1);
set_port(&mut config.ledger.cometbft.proxy_app, first_port + 2);
set_port(
&mut config.ledger.cometbft.rpc.laddr,
first_port.checked_add(1).expect("Port must not overflow"),
);
set_port(
&mut config.ledger.cometbft.proxy_app,
first_port.checked_add(2).expect("Port must not overflow"),
);

// Validator node should turned off peer exchange reactor
config.ledger.cometbft.p2p.pex = false;
Expand Down Expand Up @@ -318,7 +324,10 @@ impl Finalized {
.ok()
.map(Hash::sha256);

let min_duration: i64 = 60 * 60 * 24 * 365 / (epochs_per_year as i64);
let epy_i64 = i64::try_from(epochs_per_year)
.expect("`epochs_per_year` must not exceed `i64::MAX`");
#[allow(clippy::arithmetic_side_effects)]
let min_duration: i64 = 60 * 60 * 24 * 365 / epy_i64;
let epoch_duration = EpochDuration {
min_num_of_blocks,
min_duration: namada::core::time::Duration::seconds(min_duration)
Expand Down
8 changes: 8 additions & 0 deletions crates/apps/src/lib/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,14 @@
#![doc(html_logo_url = "https://dev.namada.net/master/rustdoc-logo.png")]
#![deny(rustdoc::broken_intra_doc_links)]
#![deny(rustdoc::private_intra_doc_links)]
#![warn(
rust_2018_idioms,
clippy::cast_sign_loss,
clippy::cast_possible_truncation,
clippy::cast_possible_wrap,
clippy::cast_lossless,
clippy::arithmetic_side_effects
)]

#[cfg(feature = "benches")]
pub mod bench_utils;
Expand Down
10 changes: 7 additions & 3 deletions crates/apps/src/lib/node/ledger/broadcaster.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,14 +55,18 @@ impl Broadcaster {
} else {
DEFAULT_BROADCAST_TIMEOUT
};
let now = {
#[allow(clippy::disallowed_methods)]
time::Instant::now()
};
let status_result = time::Sleep {
strategy: time::Constant(time::Duration::from_secs(1)),
}
.timeout(
#[allow(clippy::arithmetic_side_effects)]
{
#[allow(clippy::disallowed_methods)]
time::Instant::now()
} + time::Duration::from_secs(timeout),
now + time::Duration::from_secs(timeout)
},
|| async {
match self.client.status().await {
Ok(status) => ControlFlow::Break(status),
Expand Down
5 changes: 4 additions & 1 deletion crates/apps/src/lib/node/ledger/ethereum_oracle/events.rs
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,10 @@ pub mod eth_events {
/// Check if the minimum number of confirmations has been
/// reached at the input block height.
pub fn is_confirmed(&self, height: &Uint256) -> bool {
self.confirmations <= height.clone() - self.block_height.clone()
use num_traits::CheckedSub;

self.confirmations
<= height.checked_sub(&self.block_height).unwrap_or_default()
}
}

Expand Down
2 changes: 1 addition & 1 deletion crates/apps/src/lib/node/ledger/ethereum_oracle/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -470,7 +470,7 @@ async fn process_events_in_block<C: RpcClient>(
let last_processed_block_ref = oracle.last_processed_block.borrow();
let last_processed_block = last_processed_block_ref.as_ref();
let backoff = oracle.backoff;
#[allow(clippy::disallowed_methods)]
#[allow(clippy::arithmetic_side_effects)]
let deadline = Instant::now() + oracle.ceiling;
let latest_block = match oracle
.client
Expand Down
25 changes: 15 additions & 10 deletions crates/apps/src/lib/node/ledger/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -151,10 +151,13 @@ pub fn migrating_state() -> Option<BlockHeight> {
fn emit_warning_on_non_64bit_cpu() {
if std::mem::size_of::<usize>() != 8 {
tracing::warn!("");
tracing::warn!(
"Your machine has a {}-bit CPU...",
8 * std::mem::size_of::<usize>()
);
#[allow(clippy::arithmetic_side_effects)]
{
tracing::warn!(
"Your machine has a {}-bit CPU...",
8 * std::mem::size_of::<usize>()
);
}
tracing::warn!("");
tracing::warn!("A majority of nodes will run on 64-bit hardware!");
tracing::warn!("");
Expand Down Expand Up @@ -416,7 +419,7 @@ async fn run_aux_setup(
let available_memory_bytes = sys.available_memory();
tracing::info!(
"Available memory: {}",
Byte::from_bytes(available_memory_bytes as u128)
Byte::from_bytes(u128::from(available_memory_bytes))
.get_appropriate_unit(true)
);
available_memory_bytes
Expand All @@ -441,7 +444,7 @@ async fn run_aux_setup(
};
tracing::info!(
"VP WASM compilation cache size: {}",
Byte::from_bytes(vp_wasm_compilation_cache as u128)
Byte::from_bytes(u128::from(vp_wasm_compilation_cache))
.get_appropriate_unit(true)
);

Expand All @@ -464,7 +467,7 @@ async fn run_aux_setup(
};
tracing::info!(
"Tx WASM compilation cache size: {}",
Byte::from_bytes(tx_wasm_compilation_cache as u128)
Byte::from_bytes(u128::from(tx_wasm_compilation_cache))
.get_appropriate_unit(true)
);

Expand All @@ -484,7 +487,7 @@ async fn run_aux_setup(
};
tracing::info!(
"RocksDB block cache size: {}",
Byte::from_bytes(db_block_cache_size_bytes as u128)
Byte::from_bytes(u128::from(db_block_cache_size_bytes))
.get_appropriate_unit(true)
);

Expand Down Expand Up @@ -549,8 +552,10 @@ fn start_abci_broadcaster_shell(
};

// Setup DB cache, it must outlive the DB instance that's in the shell
let db_cache =
rocksdb::Cache::new_lru_cache(db_block_cache_size_bytes as usize);
let db_cache = rocksdb::Cache::new_lru_cache(
usize::try_from(db_block_cache_size_bytes)
.expect("`db_block_cache_size_bytes` must not exceed `usize::MAX`"),
);

// Construct our ABCI application.
let tendermint_mode = config.shell.tendermint_mode.clone();
Expand Down
29 changes: 23 additions & 6 deletions crates/apps/src/lib/node/ledger/shell/block_alloc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,9 +197,15 @@ impl<State> BlockAllocator<State> {
/// to each [`TxBin`] instance in a [`BlockAllocator`].
#[inline]
fn unoccupied_space_in_bytes(&self) -> u64 {
let total_bin_space =
self.protocol_txs.occupied + self.normal_txs.space.occupied;
self.block.allotted - total_bin_space
let total_bin_space = self
.protocol_txs
.occupied
.checked_add(self.normal_txs.space.occupied)
.expect("Shouldn't overflow");
self.block
.allotted
.checked_sub(total_bin_space)
.expect("Shouldn't underflow")
}
}

Expand All @@ -220,7 +226,9 @@ impl<R: Resource> TxBin<R> {
/// Return the amount of resource left in this [`TxBin`].
#[inline]
pub fn resource_left(&self) -> u64 {
self.allotted - self.occupied
self.allotted
.checked_sub(self.occupied)
.expect("Shouldn't underflow")
}

/// Construct a new [`TxBin`], with a capacity of `max_capacity`.
Expand Down Expand Up @@ -255,7 +263,10 @@ impl<R: Resource> TxBin<R> {
bin_resource: bin_size,
});
}
let occupied = self.occupied + resource;
let occupied = self
.occupied
.checked_add(resource)
.expect("Shouldn't overflow");
if occupied <= self.allotted {
self.occupied = occupied;
Ok(())
Expand Down Expand Up @@ -322,14 +333,20 @@ pub mod threshold {

/// Return a [`Threshold`] over some free space.
pub fn over(self, free_space_in_bytes: u64) -> u64 {
(self.0 * free_space_in_bytes).to_integer()
use num_traits::ops::checked::CheckedMul;
(self
.0
.checked_mul(&free_space_in_bytes.into())
.expect("Must not overflow"))
.to_integer()
}
}

/// Divide free space in half.
pub const ONE_HALF: Threshold = Threshold::new(1, 2);
}

#[allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)]
#[cfg(test)]
mod tests {

Expand Down
Loading

0 comments on commit b4c95c3

Please sign in to comment.