diff --git a/.changelog/unreleased/improvements/3214-more-checked-arith.md b/.changelog/unreleased/improvements/3214-more-checked-arith.md new file mode 100644 index 00000000000..4fa50bf6973 --- /dev/null +++ b/.changelog/unreleased/improvements/3214-more-checked-arith.md @@ -0,0 +1,2 @@ +- Sanitized unchecked arithmetics and conversions in the codebase. + ([\#3214](https://github.com/anoma/namada/pull/3214)) \ No newline at end of file diff --git a/Cargo.lock b/Cargo.lock index ae341298548..69b9c4d7222 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4614,6 +4614,7 @@ dependencies = [ "serde_json", "sha2 0.9.9", "signal-hook", + "smooth-operator", "sparse-merkle-tree", "sysinfo", "tar", @@ -5072,6 +5073,7 @@ dependencies = [ "proptest", "rayon", "serde 1.0.193", + "smooth-operator", "test-log", "tracing", ] @@ -5101,6 +5103,7 @@ dependencies = [ "pretty_assertions", "proptest", "sha2 0.9.9", + "smooth-operator", "sparse-merkle-tree", "test-log", "thiserror", @@ -5124,6 +5127,7 @@ dependencies = [ "namada_tx", "regex", "serde 1.0.193", + "smooth-operator", "thiserror", "tracing", ] @@ -5308,6 +5312,7 @@ dependencies = [ "namada_ibc", "namada_storage", "namada_tx", + "smooth-operator", "thiserror", ] diff --git a/crates/account/src/lib.rs b/crates/account/src/lib.rs index 5b25e1d06ea..80e5f700b5f 100644 --- a/crates/account/src/lib.rs +++ b/crates/account/src/lib.rs @@ -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; diff --git a/crates/account/src/storage.rs b/crates/account/src/storage.rs index f9f328919c4..6af91c62cc8 100644 --- a/crates/account/src/storage.rs +++ b/crates/account/src/storage.rs @@ -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::*; @@ -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); @@ -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)?; } diff --git a/crates/account/src/types.rs b/crates/account/src/types.rs index f0b355b387f..f8b5791d7ea 100644 --- a/crates/account/src/types.rs +++ b/crates/account/src/types.rs @@ -55,6 +55,7 @@ pub struct UpdateAccount { pub threshold: Option, } +#[allow(clippy::cast_possible_truncation)] #[cfg(any(test, feature = "testing"))] /// Tests and strategies for accounts pub mod tests { diff --git a/crates/apps/Cargo.toml b/crates/apps/Cargo.toml index f3ce2af7053..4388c53f2f8 100644 --- a/crates/apps/Cargo.toml +++ b/crates/apps/Cargo.toml @@ -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 diff --git a/crates/apps/src/lib/bench_utils.rs b/crates/apps/src/lib/bench_utils.rs index 2dd425e4651..79fcf608f44 100644 --- a/crates/apps/src/lib/bench_utils.rs +++ b/crates/apps/src/lib/bench_utils.rs @@ -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}; diff --git a/crates/apps/src/lib/client/mod.rs b/crates/apps/src/lib/client/mod.rs index 7ad6d57e5f2..69e200dc0d0 100644 --- a/crates/apps/src/lib/client/mod.rs +++ b/crates/apps/src/lib/client/mod.rs @@ -1,3 +1,5 @@ +#![allow(clippy::arithmetic_side_effects)] + pub mod masp; pub mod rpc; pub mod tx; diff --git a/crates/apps/src/lib/config/genesis.rs b/crates/apps/src/lib/config/genesis.rs index 9360bb9115b..30153e0db45 100644 --- a/crates/apps/src/lib/config/genesis.rs +++ b/crates/apps/src/lib/config/genesis.rs @@ -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", ) @@ -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, diff --git a/crates/apps/src/lib/config/genesis/chain.rs b/crates/apps/src/lib/config/genesis/chain.rs index d213ce9e0e5..b6865900258 100644 --- a/crates/apps/src/lib/config/genesis/chain.rs +++ b/crates/apps/src/lib/config/genesis/chain.rs @@ -252,8 +252,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; @@ -310,7 +316,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) diff --git a/crates/apps/src/lib/mod.rs b/crates/apps/src/lib/mod.rs index 22cb4d78d55..451729f496a 100644 --- a/crates/apps/src/lib/mod.rs +++ b/crates/apps/src/lib/mod.rs @@ -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; diff --git a/crates/apps/src/lib/node/ledger/broadcaster.rs b/crates/apps/src/lib/node/ledger/broadcaster.rs index 5bf038c182c..ec616003d2d 100644 --- a/crates/apps/src/lib/node/ledger/broadcaster.rs +++ b/crates/apps/src/lib/node/ledger/broadcaster.rs @@ -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), diff --git a/crates/apps/src/lib/node/ledger/ethereum_oracle/events.rs b/crates/apps/src/lib/node/ledger/ethereum_oracle/events.rs index f3e040abbe9..7ecfe51906f 100644 --- a/crates/apps/src/lib/node/ledger/ethereum_oracle/events.rs +++ b/crates/apps/src/lib/node/ledger/ethereum_oracle/events.rs @@ -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() } } diff --git a/crates/apps/src/lib/node/ledger/ethereum_oracle/mod.rs b/crates/apps/src/lib/node/ledger/ethereum_oracle/mod.rs index aa73ad29a72..97e52d5ef33 100644 --- a/crates/apps/src/lib/node/ledger/ethereum_oracle/mod.rs +++ b/crates/apps/src/lib/node/ledger/ethereum_oracle/mod.rs @@ -470,7 +470,7 @@ async fn process_events_in_block( 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 diff --git a/crates/apps/src/lib/node/ledger/mod.rs b/crates/apps/src/lib/node/ledger/mod.rs index 888ace92749..651672995de 100644 --- a/crates/apps/src/lib/node/ledger/mod.rs +++ b/crates/apps/src/lib/node/ledger/mod.rs @@ -396,7 +396,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 @@ -421,7 +421,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) ); @@ -444,7 +444,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) ); @@ -464,7 +464,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) ); @@ -529,8 +529,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(); diff --git a/crates/apps/src/lib/node/ledger/shell/block_alloc.rs b/crates/apps/src/lib/node/ledger/shell/block_alloc.rs index 625712cfe0e..7f57094c432 100644 --- a/crates/apps/src/lib/node/ledger/shell/block_alloc.rs +++ b/crates/apps/src/lib/node/ledger/shell/block_alloc.rs @@ -197,9 +197,15 @@ impl BlockAllocator { /// 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") } } @@ -220,7 +226,9 @@ impl TxBin { /// 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`. @@ -255,7 +263,10 @@ impl TxBin { 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(()) @@ -322,7 +333,12 @@ 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() } } @@ -330,6 +346,7 @@ pub mod threshold { pub const ONE_HALF: Threshold = Threshold::new(1, 2); } +#[allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)] #[cfg(test)] mod tests { diff --git a/crates/apps/src/lib/node/ledger/shell/finalize_block.rs b/crates/apps/src/lib/node/ledger/shell/finalize_block.rs index cd2f5eee0ea..776f18299bc 100644 --- a/crates/apps/src/lib/node/ledger/shell/finalize_block.rs +++ b/crates/apps/src/lib/node/ledger/shell/finalize_block.rs @@ -82,7 +82,10 @@ where "Will begin a new epoch {} in {} blocks starting at height {}", current_epoch.next(), EPOCH_SWITCH_BLOCKS_DELAY, - height.0 + u64::from(EPOCH_SWITCH_BLOCKS_DELAY) + height + .0 + .checked_add(u64::from(EPOCH_SWITCH_BLOCKS_DELAY)) + .expect("Shouldn't overflow") ); } tracing::debug!( @@ -214,7 +217,23 @@ where TxType::Wrapper(wrapper) => { stats.increment_wrapper_txs(); let tx_event = new_tx_event(&tx, height.0); - let gas_meter = TxGasMeter::new(wrapper.gas_limit); + let gas_limit = match Gas::try_from(wrapper.gas_limit) { + Ok(value) => value, + Err(_) => { + response.events.emit( + new_tx_event(&tx, height.0) + .with(Code(ResultCode::InvalidTx)) + .with(Info( + "The wrapper gas limit overflowed \ + gas representation" + .to_owned(), + )) + .with(GasUsed(0.into())), + ); + continue; + } + }; + let gas_meter = TxGasMeter::new(gas_limit); if let Some(code_sec) = tx .get_section(tx.code_sechash()) .and_then(|x| Section::code_sec(x.as_ref())) @@ -511,8 +530,12 @@ where // Update the MASP commitment tree anchor if the tree was updated let tree_key = token::storage_key::masp_commitment_tree_key(); - if let Some(StorageModification::Write { value }) = - self.state.write_log().read(&tree_key).0 + if let Some(StorageModification::Write { value }) = self + .state + .write_log() + .read(&tree_key) + .expect("Must be able to read masp commitment tree") + .0 { let updated_tree = CommitmentTree::::try_from_slice(value) .into_storage_result()?; @@ -604,10 +627,20 @@ where // Get the number of blocks in the last epoch let first_block_of_last_epoch = self.state.in_mem().block.pred_epochs.first_block_heights - [last_epoch.0 as usize] - .0; - let num_blocks_in_last_epoch = - self.state.in_mem().block.height.0 - first_block_of_last_epoch; + [usize::try_from(last_epoch.0) + .expect("Last epoch shouldn't exceed `usize::MAX`")] + .0; + let num_blocks_in_last_epoch = self + .state + .in_mem() + .block + .height + .0 + .checked_sub(first_block_of_last_epoch) + .expect( + "First block of last epoch must always be lower than or equal \ + to current block height", + ); // PoS inflation namada_proof_of_stake::rewards::apply_inflation( @@ -721,6 +754,7 @@ fn pos_votes_from_abci( /// We test the failure cases of [`finalize_block`]. The happy flows /// are covered by the e2e tests. +#[allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)] #[cfg(test)] mod test_finalize_block { use std::collections::BTreeMap; diff --git a/crates/apps/src/lib/node/ledger/shell/init_chain.rs b/crates/apps/src/lib/node/ledger/shell/init_chain.rs index e691be1abbd..e3bae818057 100644 --- a/crates/apps/src/lib/node/ledger/shell/init_chain.rs +++ b/crates/apps/src/lib/node/ledger/shell/init_chain.rs @@ -222,8 +222,10 @@ where let ts: protobuf::Timestamp = init.time.into(); let initial_height = init.initial_height.into(); // TODO hacky conversion, depends on https://github.com/informalsystems/tendermint-rs/issues/870 - let genesis_time: DateTimeUtc = (Utc - .timestamp_opt(ts.seconds, ts.nanos as u32)) + let genesis_time: DateTimeUtc = (Utc.timestamp_opt( + ts.seconds, + u32::try_from(ts.nanos).expect("Time nanos cannot be negative"), + )) .single() .expect("genesis time should be a valid timestamp") .into(); @@ -757,7 +759,7 @@ where pub fn new( shell: &'shell mut Shell, dry_run: bool, - ) -> InitChainValidation { + ) -> InitChainValidation<'_, D, H> { Self { shell, errors: vec![], diff --git a/crates/apps/src/lib/node/ledger/shell/mod.rs b/crates/apps/src/lib/node/ledger/shell/mod.rs index a4a83c6a0e6..546b55061f8 100644 --- a/crates/apps/src/lib/node/ledger/shell/mod.rs +++ b/crates/apps/src/lib/node/ledger/shell/mod.rs @@ -203,16 +203,22 @@ impl EthereumReceiver { where F: FnMut(&EthereumEvent) -> bool, { - let mut new_events = 0; - let mut filtered_events = 0; + let mut new_events: usize = 0; + let mut filtered_events: usize = 0; while let Ok(eth_event) = self.channel.try_recv() { if keep_event(ð_event) && self.queue.insert(eth_event) { - new_events += 1; + new_events = + new_events.checked_add(1).expect("Cannot overflow"); } else { - filtered_events += 1; + filtered_events = + filtered_events.checked_add(1).expect("Cannot overflow"); } } - if new_events + filtered_events > 0 { + if new_events + .checked_add(filtered_events) + .expect("Cannot overflow") + > 0 + { tracing::info!( new_events, filtered_events, @@ -515,11 +521,15 @@ where mode, vp_wasm_cache: VpCache::new( vp_wasm_cache_dir, - vp_wasm_compilation_cache as usize, + usize::try_from(vp_wasm_compilation_cache).expect( + "`vp_wasm_compilation_cache` must not exceed `usize::MAX`", + ), ), tx_wasm_cache: TxCache::new( tx_wasm_cache_dir, - tx_wasm_compilation_cache as usize, + usize::try_from(tx_wasm_compilation_cache).expect( + "`tx_wasm_compilation_cache` must not exceed `usize::MAX`", + ), ), storage_read_past_height_limit, // TODO: config event log params @@ -1056,7 +1066,17 @@ where } // Tx gas limit - let mut gas_meter = TxGasMeter::new(wrapper.gas_limit); + let gas_limit = match Gas::try_from(wrapper.gas_limit) { + Ok(value) => value, + Err(_) => { + response.code = ResultCode::InvalidTx.into(); + response.log = "The wrapper gas limit overflowed gas \ + representation" + .to_owned(); + return response; + } + }; + let mut gas_meter = TxGasMeter::new(gas_limit); if gas_meter.add_wrapper_gas(tx_bytes).is_err() { response.code = ResultCode::TxGasLimit.into(); response.log = "{INVALID_MSG}: Wrapper transaction \ @@ -1068,7 +1088,8 @@ where // Max block gas let block_gas_limit: Gas = Gas::from_whole_units( namada::parameters::get_max_block_gas(&self.state).unwrap(), - ); + ) + .expect("Gas limit from parameter must not overflow"); if gas_meter.tx_gas_limit > block_gas_limit { response.code = ResultCode::AllocationError.into(); response.log = "{INVALID_MSG}: Wrapper transaction \ @@ -1204,7 +1225,7 @@ where /// block construction and validation pub fn replay_protection_checks( wrapper: &Tx, - temp_state: &mut TempWlState, + temp_state: &mut TempWlState<'_, D, H>, ) -> Result<()> where D: DB + for<'iter> DBIter<'iter> + Sync + 'static, @@ -1244,7 +1265,7 @@ where fn mempool_fee_check( wrapper: &WrapperTx, masp_transaction: Option, - shell_params: &mut ShellParams<'_, TempWlState, D, H, CA>, + shell_params: &mut ShellParams<'_, TempWlState<'_, D, H>, D, H, CA>, ) -> Result<()> where D: DB + for<'iter> DBIter<'iter> + Sync + 'static, @@ -1276,7 +1297,7 @@ pub fn wrapper_fee_check( wrapper: &WrapperTx, masp_transaction: Option, minimum_gas_price: token::Amount, - shell_params: &mut ShellParams<'_, TempWlState, D, H, CA>, + shell_params: &mut ShellParams<'_, TempWlState<'_, D, H>, D, H, CA>, ) -> Result<()> where D: DB + for<'iter> DBIter<'iter> + Sync + 'static, @@ -1319,7 +1340,7 @@ where fn fee_unshielding_validation( wrapper: &WrapperTx, masp_transaction: Transaction, - shell_params: &mut ShellParams<'_, TempWlState, D, H, CA>, + shell_params: &mut ShellParams<'_, TempWlState<'_, D, H>, D, H, CA>, ) -> Result<()> where D: DB + for<'iter> DBIter<'iter> + Sync + 'static, @@ -1360,7 +1381,7 @@ where // Performs validation on the optional fee unshielding data carried by // the wrapper. fn check_fee_unshielding( - temp_state: &TempWlState, + temp_state: &TempWlState<'_, D, H>, unshield: &Transaction, ) -> Result<()> where @@ -1420,6 +1441,7 @@ where } /// for the shell +#[allow(clippy::arithmetic_side_effects, clippy::cast_possible_wrap)] #[cfg(test)] mod test_utils { use std::ops::{Deref, DerefMut}; diff --git a/crates/apps/src/lib/node/ledger/shell/prepare_proposal.rs b/crates/apps/src/lib/node/ledger/shell/prepare_proposal.rs index 82e9d39245d..9a2f399ab77 100644 --- a/crates/apps/src/lib/node/ledger/shell/prepare_proposal.rs +++ b/crates/apps/src/lib/node/ledger/shell/prepare_proposal.rs @@ -5,7 +5,7 @@ use std::cell::RefCell; use masp_primitives::transaction::Transaction; use namada::core::address::Address; use namada::core::key::tm_raw_hash_to_string; -use namada::gas::TxGasMeter; +use namada::gas::{Gas, TxGasMeter}; use namada::hash::Hash; use namada::ledger::protocol::{self, ShellParams}; use namada::proof_of_stake::storage::find_validator_by_raw_hash; @@ -263,7 +263,7 @@ fn validate_wrapper_bytes( block_time: Option, block_proposer: &Address, proposer_local_config: Option<&ValidatorLocalConfig>, - temp_state: &mut TempWlState, + temp_state: &mut TempWlState<'_, D, H>, vp_wasm_cache: &mut VpCache, tx_wasm_cache: &mut TxCache, ) -> Result @@ -288,7 +288,8 @@ where tx.validate_tx().map_err(|_| ())?; if let TxType::Wrapper(wrapper) = tx.header().tx_type { // Check tx gas limit for tx size - let mut tx_gas_meter = TxGasMeter::new(wrapper.gas_limit); + let gas_limit = Gas::try_from(wrapper.gas_limit).map_err(|_| ())?; + let mut tx_gas_meter = TxGasMeter::new(gas_limit); tx_gas_meter.add_wrapper_gas(tx_bytes).map_err(|_| ())?; super::replay_protection_checks(&tx, temp_state).map_err(|_| ())?; @@ -322,7 +323,7 @@ fn prepare_proposal_fee_check( masp_transaction: Option, proposer: &Address, proposer_local_config: Option<&ValidatorLocalConfig>, - shell_params: &mut ShellParams<'_, TempWlState, D, H, CA>, + shell_params: &mut ShellParams<'_, TempWlState<'_, D, H>, D, H, CA>, ) -> Result<(), Error> where D: DB + for<'iter> DBIter<'iter> + Sync + 'static, @@ -370,6 +371,7 @@ where .map_err(Error::TxApply) } +#[allow(clippy::cast_possible_wrap, clippy::cast_possible_truncation)] #[cfg(test)] // TODO: write tests for validator set update vote extensions in // prepare proposals diff --git a/crates/apps/src/lib/node/ledger/shell/process_proposal.rs b/crates/apps/src/lib/node/ledger/shell/process_proposal.rs index 15a1552c42e..fd00cff01f5 100644 --- a/crates/apps/src/lib/node/ledger/shell/process_proposal.rs +++ b/crates/apps/src/lib/node/ledger/shell/process_proposal.rs @@ -192,7 +192,7 @@ where &self, tx_bytes: &[u8], metadata: &mut ValidationMeta, - temp_state: &mut TempWlState, + temp_state: &mut TempWlState<'_, D, H>, block_time: DateTimeUtc, vp_wasm_cache: &mut VpCache, tx_wasm_cache: &mut TxCache, @@ -407,7 +407,19 @@ where // Account for the tx's resources let allocated_gas = metadata.user_gas.try_dump(u64::from(wrapper.gas_limit)); - let mut tx_gas_meter = TxGasMeter::new(wrapper.gas_limit); + + let gas_limit = match Gas::try_from(wrapper.gas_limit) { + Ok(value) => value, + Err(_) => { + return TxResult { + code: ResultCode::InvalidTx.into(), + info: "The wrapper gas limit overflowed gas \ + representation" + .to_owned(), + }; + } + }; + let mut tx_gas_meter = TxGasMeter::new(gas_limit); if tx_gas_meter.add_wrapper_gas(tx_bytes).is_err() || allocated_gas.is_err() { @@ -503,7 +515,7 @@ fn process_proposal_fee_check( wrapper_tx_hash: Hash, masp_transaction: Option, proposer: &Address, - shell_params: &mut ShellParams<'_, TempWlState, D, H, CA>, + shell_params: &mut ShellParams<'_, TempWlState<'_, D, H>, D, H, CA>, ) -> Result<()> where D: DB + for<'iter> DBIter<'iter> + Sync + 'static, diff --git a/crates/apps/src/lib/node/ledger/shell/stats.rs b/crates/apps/src/lib/node/ledger/shell/stats.rs index 9162bb792bb..91dc193d680 100644 --- a/crates/apps/src/lib/node/ledger/shell/stats.rs +++ b/crates/apps/src/lib/node/ledger/shell/stats.rs @@ -1,3 +1,5 @@ +#![allow(clippy::arithmetic_side_effects)] + use std::fmt::Display; use namada::core::collections::HashMap; diff --git a/crates/apps/src/lib/node/ledger/shell/testing/mod.rs b/crates/apps/src/lib/node/ledger/shell/testing/mod.rs index fff1df00ba9..49d30d613e3 100644 --- a/crates/apps/src/lib/node/ledger/shell/testing/mod.rs +++ b/crates/apps/src/lib/node/ledger/shell/testing/mod.rs @@ -1,3 +1,9 @@ +#![allow( + clippy::arithmetic_side_effects, + clippy::cast_possible_wrap, + clippy::cast_possible_truncation +)] + pub mod client; pub mod node; pub mod utils; diff --git a/crates/apps/src/lib/node/ledger/shell/vote_extensions/bridge_pool_vext.rs b/crates/apps/src/lib/node/ledger/shell/vote_extensions/bridge_pool_vext.rs index de990eac745..892bc7ad5f7 100644 --- a/crates/apps/src/lib/node/ledger/shell/vote_extensions/bridge_pool_vext.rs +++ b/crates/apps/src/lib/node/ledger/shell/vote_extensions/bridge_pool_vext.rs @@ -51,6 +51,7 @@ where } } +#[allow(clippy::cast_possible_truncation)] #[cfg(test)] mod test_bp_vote_extensions { use namada::core::ethereum_events::Uint; diff --git a/crates/apps/src/lib/node/ledger/shell/vote_extensions/eth_events.rs b/crates/apps/src/lib/node/ledger/shell/vote_extensions/eth_events.rs index 56457f499b1..f8cd28dec70 100644 --- a/crates/apps/src/lib/node/ledger/shell/vote_extensions/eth_events.rs +++ b/crates/apps/src/lib/node/ledger/shell/vote_extensions/eth_events.rs @@ -134,6 +134,7 @@ where } } +#[allow(clippy::cast_possible_truncation)] #[cfg(test)] mod test_vote_extensions { use namada::core::address::testing::gen_established_address; diff --git a/crates/apps/src/lib/node/ledger/shell/vote_extensions/val_set_update.rs b/crates/apps/src/lib/node/ledger/shell/vote_extensions/val_set_update.rs index e71c3ba2cdf..dcbf4e7c0f7 100644 --- a/crates/apps/src/lib/node/ledger/shell/vote_extensions/val_set_update.rs +++ b/crates/apps/src/lib/node/ledger/shell/vote_extensions/val_set_update.rs @@ -107,6 +107,7 @@ where } } +#[allow(clippy::cast_possible_truncation)] #[cfg(test)] mod test_vote_extensions { use namada::core::key::RefTo; diff --git a/crates/apps/src/lib/node/ledger/shims/abcipp_shim.rs b/crates/apps/src/lib/node/ledger/shims/abcipp_shim.rs index b6fdaffac8d..06329696335 100644 --- a/crates/apps/src/lib/node/ledger/shims/abcipp_shim.rs +++ b/crates/apps/src/lib/node/ledger/shims/abcipp_shim.rs @@ -215,7 +215,9 @@ impl AbciService { ) -> (bool, Option<>::Future>) { let hght = match check { CheckAction::AlreadySuspended => BlockHeight::from(u64::MAX), - CheckAction::Check(hght) => BlockHeight::from(hght as u64), + CheckAction::Check(hght) => BlockHeight::from( + u64::try_from(hght).expect("Height cannot be negative"), + ), CheckAction::NoAction => BlockHeight::default(), }; match action_at_height { diff --git a/crates/apps/src/lib/node/ledger/storage/mod.rs b/crates/apps/src/lib/node/ledger/storage/mod.rs index ba1cdf0c158..dbf7fc09ce0 100644 --- a/crates/apps/src/lib/node/ledger/storage/mod.rs +++ b/crates/apps/src/lib/node/ledger/storage/mod.rs @@ -51,6 +51,7 @@ fn new_blake2b() -> Blake2b { Blake2bBuilder::new(32).personal(b"namada storage").build() } +#[allow(clippy::arithmetic_side_effects, clippy::cast_sign_loss)] #[cfg(test)] mod tests { use borsh::BorshDeserialize; @@ -245,7 +246,7 @@ mod tests { state.commit_block().expect("commit failed"); - let (iter, gas) = state.db_iter_prefix(&prefix); + let (iter, gas) = state.db_iter_prefix(&prefix).unwrap(); assert_eq!(gas, (prefix.len() as u64) * STORAGE_ACCESS_GAS_PER_BYTE); for (k, v, gas) in iter { match expected.pop() { diff --git a/crates/apps/src/lib/node/ledger/storage/rocksdb.rs b/crates/apps/src/lib/node/ledger/storage/rocksdb.rs index 22f4e7d44d2..f178f7149f2 100644 --- a/crates/apps/src/lib/node/ledger/storage/rocksdb.rs +++ b/crates/apps/src/lib/node/ledger/storage/rocksdb.rs @@ -71,6 +71,7 @@ use namada::storage::{ DbColFam, BLOCK_CF, DIFFS_CF, REPLAY_PROTECTION_CF, ROLLBACK_CF, STATE_CF, SUBSPACE_CF, }; +use namada_sdk::arith::checked; use namada_sdk::migrations::DBUpdateVisitor; use rayon::prelude::*; use regex::Regex; @@ -124,11 +125,11 @@ pub fn open( cache: Option<&rocksdb::Cache>, ) -> Result { let logical_cores = num_cpus::get(); - let compaction_threads = num_of_threads( + let compaction_threads = i32::try_from(num_of_threads( ENV_VAR_ROCKSDB_COMPACTION_THREADS, // If not set, default to quarter of logical CPUs count logical_cores / 4, - ) as i32; + ))?; tracing::info!( "Using {} compactions threads for RocksDB.", compaction_threads @@ -822,7 +823,7 @@ impl DB for RocksDB { fn add_block_to_batch( &self, - state: BlockStateWrite, + state: BlockStateWrite<'_>, batch: &mut Self::WriteBatch, is_full_commit: bool, ) -> Result<()> { @@ -842,7 +843,7 @@ impl DB for RocksDB { ethereum_height, eth_events_queue, commit_only_data, - }: BlockStateWrite = state; + }: BlockStateWrite<'_> = state; let state_cf = self.get_column_family(STATE_CF)?; @@ -1074,7 +1075,7 @@ impl DB for RocksDB { // If the value didn't change at the given height, we try to look for it // at successor heights, up to the `last_height` - let mut raw_height = height.0 + 1; + let mut raw_height = checked!(height.0 + 1)?; loop { // Try to find the next diff on this key let (old_val_key, new_val_key) = @@ -1100,7 +1101,7 @@ impl DB for RocksDB { // Read from latest height return self.read_subspace_val(key); } else { - raw_height += 1 + raw_height = checked!(raw_height + 1)? } } } @@ -1166,7 +1167,9 @@ impl DB for RocksDB { let size_diff = match self.read_value_bytes(subspace_cf, key.to_string())? { Some(old_value) => { - let size_diff = value.len() as i64 - old_value.len() as i64; + let len = i64::try_from(value.len())?; + let old_len = i64::try_from(old_value.len())?; + let size_diff = checked!(len - old_len)?; // Persist the previous value self.batch_write_subspace_diff( batch, @@ -1187,7 +1190,7 @@ impl DB for RocksDB { Some(value), persist_diffs, )?; - value.len() as i64 + i64::try_from(value.len())? } }; @@ -1210,7 +1213,7 @@ impl DB for RocksDB { let prev_len = match self.read_value_bytes(subspace_cf, key.to_string())? { Some(prev_value) => { - let prev_len = prev_value.len() as i64; + let prev_len = i64::try_from(prev_value.len())?; // Persist the previous value self.batch_write_subspace_diff( batch, @@ -1664,7 +1667,7 @@ impl<'a> Iterator for PersistentPrefixIterator<'a> { let key = String::from_utf8(key.to_vec()) .expect("Cannot convert from bytes to key string"); if let Some(k) = key.strip_prefix(&self.0.stripped_prefix) { - let gas = k.len() + val.len(); + let gas = k.len().checked_add(val.len())?; return Some((k.to_owned(), val.to_vec(), gas as _)); } else { tracing::warn!( @@ -1708,7 +1711,7 @@ fn make_iter_read_opts(prefix: Option) -> ReadOptions { if let Some(prefix) = prefix { let mut upper_prefix = prefix.into_bytes(); if let Some(last) = upper_prefix.last_mut() { - *last += 1; + *last = last.checked_add(1).expect("cannot overflow"); read_opts.set_iterate_upper_bound(upper_prefix); } } @@ -1791,6 +1794,7 @@ mod imp { } } +#[allow(clippy::arithmetic_side_effects)] #[cfg(test)] mod test { use namada::address::EstablishedAddressGen; diff --git a/crates/benches/host_env.rs b/crates/benches/host_env.rs index 6d663864dd3..fc8675cdbc7 100644 --- a/crates/benches/host_env.rs +++ b/crates/benches/host_env.rs @@ -198,7 +198,7 @@ fn write_log_read(c: &mut Criterion) { format!("key: {key}, bytes: {throughput_len}"), |b| { b.iter_with_large_drop(|| { - shell.state.write_log().read(&key).0.unwrap() + shell.state.write_log().read(&key).unwrap().0.unwrap() }) }, ); diff --git a/crates/controller/src/lib.rs b/crates/controller/src/lib.rs index 5b2686ce7d9..afe986d23cf 100644 --- a/crates/controller/src/lib.rs +++ b/crates/controller/src/lib.rs @@ -1,8 +1,25 @@ +//! Inflation PD-controller + +#![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 +)] + use namada_core::arith::{self, checked}; use namada_core::dec::Dec; use namada_core::uint::Uint; use thiserror::Error; +#[allow(missing_docs)] #[derive(Clone, Debug)] pub struct PDController { total_native_amount: Uint, @@ -15,6 +32,7 @@ pub struct PDController { last_metric: Dec, } +#[allow(missing_docs)] #[derive(Error, Debug)] pub enum Error { #[error("Arithmetic {0}")] @@ -28,6 +46,7 @@ pub enum Error { } impl PDController { + /// Instantiate a new PD-controller #[allow(clippy::too_many_arguments)] pub fn new( total_native_amount: Uint, @@ -51,6 +70,7 @@ impl PDController { } } + /// Compute inflation amount pub fn compute_inflation( &self, control_coeff: Dec, @@ -60,10 +80,12 @@ impl PDController { self.compute_inflation_aux(control) } + /// Get total native amount as decimal pub fn get_total_native_dec(&self) -> Result { Dec::try_from(self.total_native_amount).map_err(Into::into) } + /// Get epochs per year pub fn get_epochs_per_year(&self) -> u64 { self.epochs_per_year } diff --git a/crates/ethereum_bridge/src/event.rs b/crates/ethereum_bridge/src/event.rs index b7e2d87e0bf..dca3bf611c9 100644 --- a/crates/ethereum_bridge/src/event.rs +++ b/crates/ethereum_bridge/src/event.rs @@ -150,6 +150,7 @@ impl EventToEmit for EthBridgeEvent { const DOMAIN: &'static str = "eth-bridge"; } +/// Hash of bridge pool transaction pub struct BridgePoolTxHash<'tx>(pub &'tx KeccakHash); impl<'tx> EventAttributeEntry<'tx> for BridgePoolTxHash<'tx> { diff --git a/crates/ethereum_bridge/src/lib.rs b/crates/ethereum_bridge/src/lib.rs index 7bf152bb775..29bea8a7957 100644 --- a/crates/ethereum_bridge/src/lib.rs +++ b/crates/ethereum_bridge/src/lib.rs @@ -1,4 +1,18 @@ -extern crate core; +//! Ethereum bridge + +#![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 +)] pub mod event; pub mod oracle; diff --git a/crates/ethereum_bridge/src/oracle.rs b/crates/ethereum_bridge/src/oracle.rs index ef68c36943d..3bc1cb1e49c 100644 --- a/crates/ethereum_bridge/src/oracle.rs +++ b/crates/ethereum_bridge/src/oracle.rs @@ -1 +1,3 @@ +//! Ethereum bridge oracle + pub mod config; diff --git a/crates/ethereum_bridge/src/protocol/mod.rs b/crates/ethereum_bridge/src/protocol/mod.rs index 5d311291657..ec47d370e39 100644 --- a/crates/ethereum_bridge/src/protocol/mod.rs +++ b/crates/ethereum_bridge/src/protocol/mod.rs @@ -1,2 +1,4 @@ +//! Ethereum bridge protocol transactions and validation. + pub mod transactions; pub mod validation; diff --git a/crates/ethereum_bridge/src/protocol/transactions/bridge_pool_roots.rs b/crates/ethereum_bridge/src/protocol/transactions/bridge_pool_roots.rs index 8cf9f77120b..83675d3fc8a 100644 --- a/crates/ethereum_bridge/src/protocol/transactions/bridge_pool_roots.rs +++ b/crates/ethereum_bridge/src/protocol/transactions/bridge_pool_roots.rs @@ -1,3 +1,5 @@ +//! Functions dealing with bridge pool root hash. + use eyre::Result; use namada_core::address::Address; use namada_core::collections::{HashMap, HashSet}; diff --git a/crates/ethereum_bridge/src/protocol/transactions/ethereum_events/events.rs b/crates/ethereum_bridge/src/protocol/transactions/ethereum_events/events.rs index c8b34a73bce..f545110106a 100644 --- a/crates/ethereum_bridge/src/protocol/transactions/ethereum_events/events.rs +++ b/crates/ethereum_bridge/src/protocol/transactions/ethereum_events/events.rs @@ -311,8 +311,15 @@ where // Check time out and refund if state.in_mem().block.height.0 > timeout_offset { - let timeout_height = - BlockHeight(state.in_mem().block.height.0 - timeout_offset); + let timeout_height = BlockHeight( + state + .in_mem() + .block + .height + .0 + .checked_sub(timeout_offset) + .expect("Cannot underflow - checked above"), + ); for key in pending_keys { let inserted_height = BlockHeight::try_from_slice( &state.in_mem().block.tree.get(&key)?, @@ -501,6 +508,7 @@ where Ok(changed_keys) } +#[allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)] #[cfg(test)] mod tests { use assert_matches::assert_matches; diff --git a/crates/ethereum_bridge/src/protocol/transactions/ethereum_events/mod.rs b/crates/ethereum_bridge/src/protocol/transactions/ethereum_events/mod.rs index 8bf67c22054..c4b5435fb98 100644 --- a/crates/ethereum_bridge/src/protocol/transactions/ethereum_events/mod.rs +++ b/crates/ethereum_bridge/src/protocol/transactions/ethereum_events/mod.rs @@ -287,7 +287,12 @@ where return Ok(Vec::new()); } - let timeout_epoch = Epoch(current_epoch.0 - unbonding_len); + let timeout_epoch = Epoch( + current_epoch + .0 + .checked_sub(unbonding_len) + .expect("Cannot underflow - checked above"), + ); let prefix = vote_tallies::eth_msgs_prefix(); let mut cur_keys: Option> = None; let mut is_timed_out = false; diff --git a/crates/ethereum_bridge/src/protocol/transactions/mod.rs b/crates/ethereum_bridge/src/protocol/transactions/mod.rs index 5b249deb7d8..a6b646e5e92 100644 --- a/crates/ethereum_bridge/src/protocol/transactions/mod.rs +++ b/crates/ethereum_bridge/src/protocol/transactions/mod.rs @@ -4,6 +4,7 @@ //! to update their blockchain state in a deterministic way. This can be done //! natively rather than via the wasm environment as happens with regular //! transactions. + pub mod bridge_pool_roots; pub mod ethereum_events; mod read; diff --git a/crates/ethereum_bridge/src/protocol/transactions/validator_set_update/mod.rs b/crates/ethereum_bridge/src/protocol/transactions/validator_set_update/mod.rs index 87dda518548..0254c507092 100644 --- a/crates/ethereum_bridge/src/protocol/transactions/validator_set_update/mod.rs +++ b/crates/ethereum_bridge/src/protocol/transactions/validator_set_update/mod.rs @@ -67,6 +67,7 @@ where }) } +/// Aggregate validators' votes pub fn aggregate_votes( state: &mut WlState, ext: validator_set_update::VextDigest, diff --git a/crates/ethereum_bridge/src/protocol/transactions/votes/update.rs b/crates/ethereum_bridge/src/protocol/transactions/votes/update.rs index b4ff7788ecf..fa899fbaa68 100644 --- a/crates/ethereum_bridge/src/protocol/transactions/votes/update.rs +++ b/crates/ethereum_bridge/src/protocol/transactions/votes/update.rs @@ -208,6 +208,7 @@ fn keys_changed( changed_keys } +#[allow(clippy::arithmetic_side_effects)] #[cfg(test)] mod tests { use std::collections::BTreeMap; diff --git a/crates/ethereum_bridge/src/protocol/validation.rs b/crates/ethereum_bridge/src/protocol/validation.rs index 410655a3c8b..3d8242dde07 100644 --- a/crates/ethereum_bridge/src/protocol/validation.rs +++ b/crates/ethereum_bridge/src/protocol/validation.rs @@ -7,6 +7,7 @@ pub mod validator_set_update; use thiserror::Error; /// The error yielded from validating faulty vote extensions. +#[allow(missing_docs)] #[derive(Error, Debug)] pub enum VoteExtensionError { #[error( diff --git a/crates/ethereum_bridge/src/protocol/validation/validator_set_update.rs b/crates/ethereum_bridge/src/protocol/validation/validator_set_update.rs index 59b093eea44..035b1576841 100644 --- a/crates/ethereum_bridge/src/protocol/validation/validator_set_update.rs +++ b/crates/ethereum_bridge/src/protocol/validation/validator_set_update.rs @@ -100,7 +100,11 @@ where ); return Err(VoteExtensionError::DivergesFromStorage); } - no_local_consensus_eth_addresses += 1; + // At most the number of consensus validator addresses - cannot overflow + #[allow(clippy::arithmetic_side_effects)] + { + no_local_consensus_eth_addresses += 1; + } } if no_local_consensus_eth_addresses != ext.data.voting_powers.len() { tracing::debug!( diff --git a/crates/ethereum_bridge/src/storage/eth_bridge_queries.rs b/crates/ethereum_bridge/src/storage/eth_bridge_queries.rs index 038a966f703..734b7032de5 100644 --- a/crates/ethereum_bridge/src/storage/eth_bridge_queries.rs +++ b/crates/ethereum_bridge/src/storage/eth_bridge_queries.rs @@ -1,3 +1,5 @@ +//! Storage queries for ethereum bridge. + use borsh::{BorshDeserialize, BorshSerialize}; use namada_core::address::Address; use namada_core::eth_abi::Encode; @@ -84,7 +86,9 @@ pub enum SendValsetUpd { )] /// An enum indicating if the Ethereum bridge is enabled. pub enum EthBridgeStatus { + /// The bridge is disabled Disabled, + /// The bridge is enabled Enabled(EthBridgeEnabled), } @@ -103,13 +107,12 @@ pub enum EthBridgeStatus { /// Enum indicating if the bridge was initialized at genesis /// or a later epoch. pub enum EthBridgeEnabled { + /// Bridge is enabled from genesis AtGenesis, - AtEpoch( - // bridge is enabled from this epoch - // onwards. a validator set proof must - // exist for this epoch. - Epoch, - ), + /// Bridge is enabled from this epoch + /// onwards. a validator set proof must + /// exist for this epoch. + AtEpoch(Epoch), } /// Methods used to query blockchain Ethereum bridge related state. diff --git a/crates/ethereum_bridge/src/storage/proof.rs b/crates/ethereum_bridge/src/storage/proof.rs index 4fee82a3ae8..32f4ad99775 100644 --- a/crates/ethereum_bridge/src/storage/proof.rs +++ b/crates/ethereum_bridge/src/storage/proof.rs @@ -27,6 +27,7 @@ pub struct EthereumProof { pub data: T, } +/// Ethereum bridge pool root proof. pub type BridgePoolRootProof = EthereumProof<(KeccakHash, Uint)>; impl EthereumProof { diff --git a/crates/ethereum_bridge/src/storage/vp/bridge_pool.rs b/crates/ethereum_bridge/src/storage/vp/bridge_pool.rs index f09fd9ebe36..effc51927ec 100644 --- a/crates/ethereum_bridge/src/storage/vp/bridge_pool.rs +++ b/crates/ethereum_bridge/src/storage/vp/bridge_pool.rs @@ -1,3 +1,5 @@ +//! Ethereum bridge pool VP storage + use namada_core::ethereum_events::Uint; use namada_storage::{StorageRead, StorageWrite}; use namada_trans_token::storage_key::balance_key; diff --git a/crates/ethereum_bridge/src/storage/vp/ethereum_bridge.rs b/crates/ethereum_bridge/src/storage/vp/ethereum_bridge.rs index c1a8dd8e163..20d3375cbd0 100644 --- a/crates/ethereum_bridge/src/storage/vp/ethereum_bridge.rs +++ b/crates/ethereum_bridge/src/storage/vp/ethereum_bridge.rs @@ -1,3 +1,5 @@ +//! Ethereum bridge VP storage + use namada_storage::{StorageRead, StorageWrite}; use namada_trans_token::storage_key::balance_key; use namada_trans_token::Amount; diff --git a/crates/ethereum_bridge/src/test_utils.rs b/crates/ethereum_bridge/src/test_utils.rs index cc678a79b2a..f162788a8a9 100644 --- a/crates/ethereum_bridge/src/test_utils.rs +++ b/crates/ethereum_bridge/src/test_utils.rs @@ -1,5 +1,7 @@ //! Test utilities for the Ethereum bridge crate. +#![allow(clippy::arithmetic_side_effects)] + use std::num::NonZeroU64; use namada_account::protocol_pk_key; diff --git a/crates/events/src/lib.rs b/crates/events/src/lib.rs index f0f5ce819e9..072a5340e09 100644 --- a/crates/events/src/lib.rs +++ b/crates/events/src/lib.rs @@ -1,5 +1,19 @@ //! Events emitted by the Namada ledger. +#![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 +)] + pub mod extend; #[cfg(any(test, feature = "testing"))] pub mod testing; @@ -437,14 +451,15 @@ impl Event { self } - /// Compute the gas cost of emitting this event. + /// Compute the gas cost of emitting this event. Returns `None` on u64 + /// overflow. #[inline] - pub fn emission_gas_cost(&self, cost_per_byte: u64) -> u64 { - let len = self - .attributes - .iter() - .fold(0, |acc, (k, v)| acc + k.len() + v.len()); - len as u64 * cost_per_byte + pub fn emission_gas_cost(&self, cost_per_byte: u64) -> Option { + let len = self.attributes.iter().try_fold(0_usize, |acc, (k, v)| { + acc.checked_add(k.len()) + .and_then(|val| val.checked_add(v.len())) + })?; + (len as u64).checked_mul(cost_per_byte) } } diff --git a/crates/gas/src/event.rs b/crates/gas/src/event.rs index a586298c8f7..d76889fece0 100644 --- a/crates/gas/src/event.rs +++ b/crates/gas/src/event.rs @@ -4,7 +4,7 @@ use namada_events::extend::EventAttributeEntry; use super::Gas; -/// Extend an [`Event`] with gas used data. +/// Extend an [`namada_events::Event`] with gas used data. pub struct GasUsed(pub Gas); impl EventAttributeEntry<'static> for GasUsed { diff --git a/crates/gas/src/lib.rs b/crates/gas/src/lib.rs index 124d4c14cd7..4816a49c25d 100644 --- a/crates/gas/src/lib.rs +++ b/crates/gas/src/lib.rs @@ -1,12 +1,25 @@ //! Gas accounting module to track the gas usage in a block for transactions and //! validity predicates triggered by transactions. +#![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 +)] + pub mod event; pub mod storage; use std::fmt::Display; use std::num::ParseIntError; -use std::ops::Div; use std::str::FromStr; use namada_core::borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; @@ -30,8 +43,12 @@ pub enum Error { #[allow(missing_docs)] #[derive(Error, Debug, Clone, PartialEq, Eq)] -#[error("Failed to parse gas: {0}")] -pub struct GasParseError(pub ParseIntError); +pub enum GasParseError { + #[error("Failed to parse gas: {0}")] + Parse(ParseIntError), + #[error("Gas overflowed")] + Overflow, +} const COMPILE_GAS_PER_BYTE: u64 = 1_955; const PARALLEL_GAS_DIVIDER: u64 = 10; @@ -60,6 +77,7 @@ pub const STORAGE_WRITE_GAS_PER_BYTE: u64 = /// The cost of verifying a single signature of a transaction pub const VERIFY_TX_SIG_GAS: u64 = 594_290; /// The cost for requesting one more page in wasm (64KiB) +#[allow(clippy::cast_possible_truncation)] // const in u32 range pub const WASM_MEMORY_PAGE_GAS: u32 = MEMORY_ACCESS_GAS_PER_BYTE as u32 * 64 * 1_024; /// The cost to validate an Ibc action @@ -116,6 +134,11 @@ impl Gas { self.sub.checked_sub(rhs.sub).map(|sub| Self { sub }) } + /// Checked div of `Gas`. Returns `None` if `rhs` is zero. + pub fn checked_div(&self, rhs: u64) -> Option { + self.sub.checked_div(rhs).map(|sub| Self { sub }) + } + /// Converts the sub gas units to whole ones. If the sub units are not a /// multiple of the `SCALE` than ceil the quotient fn get_whole_gas_units(&self) -> u64 { @@ -123,23 +146,16 @@ impl Gas { if self.sub % SCALE == 0 { quotient } else { - quotient + 1 + quotient + .checked_add(1) + .expect("Cannot overflow as the quotient is scaled down u64") } } /// Generates a `Gas` instance from a whole amount - pub fn from_whole_units(whole: u64) -> Self { - Self { sub: whole * SCALE } - } -} - -impl Div for Gas { - type Output = Gas; - - fn div(self, rhs: u64) -> Self::Output { - Self { - sub: self.sub / rhs, - } + pub fn from_whole_units(whole: u64) -> Option { + let sub = whole.checked_mul(SCALE)?; + Some(Self { sub }) } } @@ -166,8 +182,9 @@ impl FromStr for Gas { type Err = GasParseError; fn from_str(s: &str) -> std::result::Result { - let gas: u64 = s.parse().map_err(GasParseError)?; - Ok(Gas::from_whole_units(gas)) + let raw: u64 = s.parse().map_err(GasParseError::Parse)?; + let gas = Gas::from_whole_units(raw).ok_or(GasParseError::Overflow)?; + Ok(gas) } } @@ -457,10 +474,14 @@ impl VpsGas { /// Get the gas consumed by the parallelized VPs fn get_current_gas(&self) -> Result { - let parallel_gas = - self.rest.iter().try_fold(Gas::default(), |acc, gas| { + let parallel_gas = self + .rest + .iter() + .try_fold(Gas::default(), |acc, gas| { acc.checked_add(*gas).ok_or(Error::GasOverflow) - })? / PARALLEL_GAS_DIVIDER; + })? + .checked_div(PARALLEL_GAS_DIVIDER) + .expect("Div by non-zero int cannot fail"); self.max.checked_add(parallel_gas).ok_or(Error::GasOverflow) } } diff --git a/crates/governance/src/cli/validation.rs b/crates/governance/src/cli/validation.rs index 11898f50271..e2dea7f907c 100644 --- a/crates/governance/src/cli/validation.rs +++ b/crates/governance/src/cli/validation.rs @@ -65,7 +65,7 @@ pub enum ProposalValidation { #[error("invalid proposal extra data: cannot be empty.")] InvalidPgfFundingExtraData, #[error("Arithmetic {0}.")] - Arith(arith::Error), + Arith(#[from] arith::Error), } pub fn is_valid_author_balance( @@ -89,7 +89,7 @@ pub fn is_valid_start_epoch( ) -> Result<(), ProposalValidation> { let start_epoch_greater_than_current = proposal_start_epoch > current_epoch; let start_epoch_is_multipler = - proposal_start_epoch.0 % proposal_epoch_multiplier == 0; + checked!(proposal_start_epoch.0 % proposal_epoch_multiplier)? == 0; if start_epoch_greater_than_current && start_epoch_is_multipler { Ok(()) @@ -110,7 +110,8 @@ pub fn is_valid_end_epoch( min_proposal_voting_period: u64, max_proposal_period: u64, ) -> Result<(), ProposalValidation> { - let voting_period = proposal_end_epoch.0 - proposal_start_epoch.0; + let voting_period = + checked!(proposal_end_epoch.0 - proposal_start_epoch.0)?; let end_epoch_is_multipler = checked!(proposal_end_epoch % proposal_epoch_multiplier) .map_err(ProposalValidation::Arith)? @@ -134,7 +135,8 @@ pub fn is_valid_activation_epoch( proposal_end_epoch: Epoch, min_proposal_grace_epochs: u64, ) -> Result<(), ProposalValidation> { - let grace_period = proposal_activation_epoch.0 - proposal_end_epoch.0; + let grace_period = + checked!(proposal_activation_epoch.0 - proposal_end_epoch.0)?; if grace_period > 0 && grace_period >= min_proposal_grace_epochs { Ok(()) @@ -151,7 +153,8 @@ pub fn is_valid_proposal_period( proposal_activation_epoch: Epoch, max_proposal_period: u64, ) -> Result<(), ProposalValidation> { - let proposal_period = proposal_activation_epoch.0 - proposal_start_epoch.0; + let proposal_period = + checked!(proposal_activation_epoch.0 - proposal_start_epoch.0)?; if proposal_period > 0 && proposal_period <= max_proposal_period { Ok(()) @@ -173,8 +176,9 @@ pub fn is_valid_content( .values() .map(|value| value.len() as u64) .sum(); - let proposal_content_length = - proposal_content_values_length + proposal_content_keys_length; + let proposal_content_length = checked!( + proposal_content_values_length + proposal_content_keys_length + )?; if proposal_content_length <= max_content_length { Ok(()) diff --git a/crates/governance/src/event.rs b/crates/governance/src/event.rs index f0089c4f013..f8421fcb37a 100644 --- a/crates/governance/src/event.rs +++ b/crates/governance/src/event.rs @@ -71,7 +71,7 @@ impl GovernanceEvent { } } - /// Create a new proposal event for defaultwithwasm proposal + /// Create a new proposal event for default with wasm proposal pub fn passed_proposal( proposal_id: u64, has_proposal_code: bool, @@ -86,6 +86,7 @@ impl GovernanceEvent { } } + /// Event for a reject proposal pub fn rejected_proposal( proposal_id: u64, has_proposal_code: bool, @@ -101,14 +102,22 @@ impl GovernanceEvent { #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Hash)] pub enum ProposalEventKind { /// New proposal event - NewProposal { proposal_type: GovProposalType }, + NewProposal { + /// Type of proposal + proposal_type: GovProposalType, + }, /// Passed proposal Passed { + /// Does the proposal contain code? has_proposal_code: bool, + /// Did the proposal code run successfully? is_proposal_code_successful: bool, }, /// Rejected proposal - Rejected { has_proposal_code: bool }, + Rejected { + /// Does the proposal contain code? + has_proposal_code: bool, + }, } impl From for Event { diff --git a/crates/governance/src/lib.rs b/crates/governance/src/lib.rs index 925195bb415..18d29f2d05e 100644 --- a/crates/governance/src/lib.rs +++ b/crates/governance/src/lib.rs @@ -1,5 +1,19 @@ //! Governance library code +#![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 +)] + use namada_core::address::{self, Address}; /// governance CLI structures diff --git a/crates/governance/src/storage/mod.rs b/crates/governance/src/storage/mod.rs index 5333a3e7930..2a2182f5979 100644 --- a/crates/governance/src/storage/mod.rs +++ b/crates/governance/src/storage/mod.rs @@ -74,7 +74,12 @@ where governance_keys::get_activation_epoch_key(proposal_id); storage.write(&activation_epoch_key, data.activation_epoch)?; - storage.write(&counter_key, proposal_id + 1)?; + storage.write( + &counter_key, + proposal_id + .checked_add(1) + .expect("Number of proposals should never exceed `u64::MAX`"), + )?; let min_proposal_funds_key = governance_keys::get_min_proposal_fund_key(); let min_proposal_funds: token::Amount = diff --git a/crates/ibc/src/actions.rs b/crates/ibc/src/actions.rs index d2529b76260..9460e2ae6b2 100644 --- a/crates/ibc/src/actions.rs +++ b/crates/ibc/src/actions.rs @@ -17,8 +17,9 @@ use namada_events::{EmitEvents, EventTypeBuilder}; use namada_governance::storage::proposal::PGFIbcTarget; use namada_parameters::read_epoch_duration_parameter; use namada_state::{ - DBIter, Epochs, ResultExt, State, StateRead, StorageError, StorageHasher, - StorageRead, StorageResult, StorageWrite, TxHostEnvState, WlState, DB, + DBIter, Epochs, OptionExt, ResultExt, State, StateRead, StorageError, + StorageHasher, StorageRead, StorageResult, StorageWrite, TxHostEnvState, + WlState, DB, }; use namada_token as token; use token::DenominatedAmount; @@ -120,7 +121,10 @@ where H: 'static + StorageHasher, { fn emit_ibc_event(&mut self, event: IbcEvent) -> Result<(), StorageError> { - let gas = self.write_log_mut().emit_event(event); + let gas = self + .write_log_mut() + .emit_event(event) + .ok_or_err_msg("Gas overflow")?; self.charge_gas(gas).into_storage_result()?; Ok(()) } @@ -193,6 +197,7 @@ where S: State + EmitEvents, { fn emit_ibc_event(&mut self, event: IbcEvent) -> Result<(), StorageError> { + // There's no gas cost for protocol, we can ignore result self.state.write_log_mut().emit_event(event); Ok(()) } @@ -293,6 +298,7 @@ where receiver: target.target.clone().into(), memo: String::default().into(), }; + #[allow(clippy::arithmetic_side_effects)] let timeout_timestamp = state .in_mem() .header diff --git a/crates/ibc/src/lib.rs b/crates/ibc/src/lib.rs index b7efdbc268a..197e9dbd4dd 100644 --- a/crates/ibc/src/lib.rs +++ b/crates/ibc/src/lib.rs @@ -1,5 +1,19 @@ //! IBC library code +#![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 actions; pub mod context; pub mod event; diff --git a/crates/merkle_tree/src/eth_bridge_pool.rs b/crates/merkle_tree/src/eth_bridge_pool.rs index 83cfdc98314..27bdd7087c8 100644 --- a/crates/merkle_tree/src/eth_bridge_pool.rs +++ b/crates/merkle_tree/src/eth_bridge_pool.rs @@ -301,7 +301,13 @@ pub struct BridgePoolProof { impl BridgePoolProof { /// Verify a membership proof matches the provided root pub fn verify(&self, root: KeccakHash) -> bool { - if self.proof.len() + self.leaves.len() != self.flags.len() + 1 { + // Cannot overflow + #[allow(clippy::arithmetic_side_effects)] + let expected_len = self.flags.len() + 1; + #[allow(clippy::arithmetic_side_effects)] + let actual_len = self.proof.len() + self.leaves.len(); + + if actual_len != expected_len { return false; } if self.flags.is_empty() { @@ -322,6 +328,8 @@ impl BridgePoolProof { let mut leaf_pos = 0usize; let mut proof_pos = 0usize; + // At most 2 additions per iter, cannot overflow usize + #[allow(clippy::arithmetic_side_effects)] for i in 0..total_hashes { let (left, prefix) = if leaf_pos < leaf_len { let next = self.leaves[leaf_pos].keccak256(); @@ -383,6 +391,7 @@ impl Encode<3> for BridgePoolProof { } } +#[allow(clippy::cast_lossless)] #[cfg(test)] mod test_bridge_pool_tree { diff --git a/crates/merkle_tree/src/lib.rs b/crates/merkle_tree/src/lib.rs index 9fb26efda1b..a1217053a31 100644 --- a/crates/merkle_tree/src/lib.rs +++ b/crates/merkle_tree/src/lib.rs @@ -1,5 +1,19 @@ //! The merkle tree in the storage +#![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 +)] + pub mod eth_bridge_pool; pub mod ics23_specs; @@ -51,7 +65,7 @@ pub trait SubTreeRead { fn subtree_membership_proof( &self, keys: &[Key], - values: Vec, + values: Vec>, ) -> Result; } @@ -62,7 +76,7 @@ pub trait SubTreeWrite { fn subtree_update( &mut self, key: &Key, - value: StorageBytes, + value: StorageBytes<'_>, ) -> Result; /// Delete a key from the sub-tree fn subtree_delete(&mut self, key: &Key) -> Result; @@ -182,7 +196,7 @@ pub enum Store { impl Store { /// Convert to a `StoreRef` with borrowed store - pub fn as_ref(&self) -> StoreRef { + pub fn as_ref(&self) -> StoreRef<'_> { match self { Self::Base(store) => StoreRef::Base(store), Self::Account(store) => StoreRef::Account(store), @@ -396,6 +410,7 @@ impl From for CommitDataRoot { } impl CommitDataRoot { + /// Storage key for commit data pub fn get_commit_data_key() -> Key { Key::parse("commit_data").expect("Should be able to parse the key.") } @@ -621,7 +636,7 @@ impl MerkleTree { } /// Get the stores of the base and sub trees - pub fn stores(&self) -> MerkleTreeStoresWrite { + pub fn stores(&self) -> MerkleTreeStoresWrite<'_> { MerkleTreeStoresWrite { base: (self.base.root().into(), self.base.store()), account: (self.account.root().into(), self.account.store()), @@ -639,7 +654,7 @@ impl MerkleTree { pub fn get_sub_tree_existence_proof( &self, keys: &[Key], - values: Vec, + values: Vec>, ) -> Result { let first_key = keys.iter().next().ok_or_else(|| { Error::InvalidMerkleKey( @@ -814,7 +829,7 @@ impl MerkleTreeStoresRead { } /// Read the backing store of the requested type - pub fn get_store(&self, store_type: StoreType) -> StoreRef { + pub fn get_store(&self, store_type: StoreType) -> StoreRef<'_> { match store_type { StoreType::Base => StoreRef::Base(&self.base.1), StoreType::Account => StoreRef::Account(&self.account.1), @@ -862,7 +877,7 @@ impl<'a> MerkleTreeStoresWrite<'a> { } /// Get the store of the given store type - pub fn store(&self, store_type: &StoreType) -> StoreRef { + pub fn store(&self, store_type: &StoreType) -> StoreRef<'_> { match store_type { StoreType::Base => StoreRef::Base(self.base.1), StoreType::Account => StoreRef::Account(self.account.1), @@ -961,7 +976,7 @@ impl<'a, H: StorageHasher + Default> SubTreeRead for &'a Smt { fn subtree_membership_proof( &self, keys: &[Key], - mut values: Vec, + mut values: Vec>, ) -> Result { if keys.len() != 1 || values.len() != 1 { return Err(Error::Ics23MultiLeaf); @@ -990,7 +1005,7 @@ impl<'a, H: StorageHasher + Default> SubTreeWrite for &'a mut Smt { fn subtree_update( &mut self, key: &Key, - value: StorageBytes, + value: StorageBytes<'_>, ) -> Result { let value = H::hash(value); self.update(H::hash(key.to_string()).into(), value.into()) @@ -1034,7 +1049,7 @@ impl<'a, H: StorageHasher + Default> SubTreeRead for &'a Amt { fn subtree_membership_proof( &self, keys: &[Key], - _: Vec, + _: Vec>, ) -> Result { if keys.len() != 1 { return Err(Error::Ics23MultiLeaf); @@ -1061,7 +1076,7 @@ impl<'a, H: StorageHasher + Default> SubTreeWrite for &'a mut Amt { fn subtree_update( &mut self, key: &Key, - value: StorageBytes, + value: StorageBytes<'_>, ) -> Result { let key = StringKey::try_from_bytes(key.to_string().as_bytes())?; let value = TreeBytes::from(value.to_vec()); @@ -1103,7 +1118,7 @@ impl<'a> SubTreeRead for &'a BridgePoolTree { fn subtree_membership_proof( &self, _: &[Key], - values: Vec, + values: Vec>, ) -> Result { let values = values .iter() @@ -1119,7 +1134,7 @@ impl<'a> SubTreeWrite for &'a mut BridgePoolTree { fn subtree_update( &mut self, key: &Key, - value: StorageBytes, + value: StorageBytes<'_>, ) -> Result { let height = BlockHeight::try_from_slice(value) .map_err(|err| Error::MerkleTree(err.to_string()))?; @@ -1156,7 +1171,7 @@ impl<'a> SubTreeRead for &'a CommitDataRoot { fn subtree_membership_proof( &self, _keys: &[Key], - _values: Vec, + _values: Vec>, ) -> Result { unimplemented!("Commit data subspace hold only a single hash value.") } @@ -1171,7 +1186,7 @@ impl<'a> SubTreeWrite for &'a mut CommitDataRoot { fn subtree_update( &mut self, _key: &Key, - value: StorageBytes, + value: StorageBytes<'_>, ) -> Result { self.0 = Hash::sha256(value); Ok(self.0) diff --git a/crates/namada/src/ledger/governance/mod.rs b/crates/namada/src/ledger/governance/mod.rs index 4add3773990..295ec33da02 100644 --- a/crates/namada/src/ledger/governance/mod.rs +++ b/crates/namada/src/ledger/governance/mod.rs @@ -5,7 +5,7 @@ pub mod utils; use std::collections::BTreeSet; use borsh::BorshDeserialize; -use namada_core::arith::checked; +use namada_core::arith::{self, checked}; use namada_core::booleans::{BoolResultUnitExt, ResultBoolExt}; use namada_governance::storage::proposal::{ AddRemove, PGFAction, ProposalType, @@ -46,6 +46,8 @@ pub enum Error { "Action {0} not authorized by {1} which is not part of verifier set" )] Unauthorized(&'static str, Address), + #[error("Arithmetic {0}")] + Arith(#[from] arith::Error), } /// Governance VP @@ -242,7 +244,7 @@ where } } - Ok((true, post_counter - pre_counter)) + Ok((true, checked!(post_counter - pre_counter)?)) } fn is_valid_vote_key( @@ -586,11 +588,11 @@ where // check that they are unique by checking that the set of add // plus the set of remove plus the set of retro is equal to the // total fundings - let are_continuous_fundings_unique = + let are_continuous_fundings_unique = checked!( are_continuous_add_targets_unique.len() + are_continuous_remove_targets_unique.len() + total_retro_targets - == fundings.len(); + )? == fundings.len(); if !are_continuous_fundings_unique { return Err(native_vp::Error::new_const( @@ -730,7 +732,8 @@ where return Err(error); } let is_valid_max_proposal_period = start_epoch < activation_epoch - && activation_epoch.0 - start_epoch.0 <= max_proposal_period; + && checked!(activation_epoch.0 - start_epoch.0)? + <= max_proposal_period; if !is_valid_max_proposal_period { let error = native_vp::Error::new_alloc(format!( "Expected max duration between the start and grace epoch \ @@ -808,7 +811,7 @@ where let latency: u64 = self.force_read(&max_latency_paramater_key, ReadType::Pre)?; - if start_epoch.0 - current_epoch.0 > latency { + if checked!(start_epoch.0 - current_epoch.0)? > latency { return Err(native_vp::Error::new_alloc(format!( "Starting epoch {start_epoch} of the proposal with id \ {proposal_id} is too far in the future (more than {latency} \ @@ -1082,7 +1085,7 @@ where let post_counter: u64 = self.force_read(&counter_key, ReadType::Post)?; - let expected_counter = pre_counter + set_count; + let expected_counter = checked!(pre_counter + set_count)?; let valid_counter = expected_counter == post_counter; valid_counter.ok_or_else(|| { @@ -1289,6 +1292,7 @@ impl KeyType { } } +#[allow(clippy::arithmetic_side_effects)] #[cfg(test)] mod test { use std::cell::RefCell; diff --git a/crates/namada/src/ledger/mod.rs b/crates/namada/src/ledger/mod.rs index ba16e3ad657..606d2b469bd 100644 --- a/crates/namada/src/ledger/mod.rs +++ b/crates/namada/src/ledger/mod.rs @@ -58,8 +58,9 @@ mod dry_run_tx { // Wrapper dry run to allow estimating the gas cost of a transaction let tx_gas_meter = match tx.header().tx_type { TxType::Wrapper(wrapper) => { - let tx_gas_meter = - RefCell::new(TxGasMeter::new(wrapper.gas_limit.to_owned())); + let gas_limit = + Gas::try_from(wrapper.gas_limit).into_storage_result()?; + let tx_gas_meter = RefCell::new(TxGasMeter::new(gas_limit)); protocol::apply_wrapper_tx( tx.clone(), &wrapper, @@ -83,18 +84,22 @@ mod dry_run_tx { TxType::Protocol(_) => { // If dry run only the inner tx, use the max block gas as // the gas limit - TxGasMeter::new(GasLimit::from( - namada_parameters::get_max_block_gas(ctx.state).unwrap(), - )) + let max_block_gas = + namada_parameters::get_max_block_gas(ctx.state)?; + let gas_limit = Gas::try_from(GasLimit::from(max_block_gas)) + .into_storage_result()?; + TxGasMeter::new(gas_limit) } TxType::Raw => { // Cast tx to a decrypted for execution // If dry run only the inner tx, use the max block gas as // the gas limit - TxGasMeter::new(GasLimit::from( - namada_parameters::get_max_block_gas(ctx.state).unwrap(), - )) + let max_block_gas = + namada_parameters::get_max_block_gas(ctx.state)?; + let gas_limit = Gas::try_from(GasLimit::from(max_block_gas)) + .into_storage_result()?; + TxGasMeter::new(gas_limit) } }; diff --git a/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs b/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs index 14f728b2e9c..34b51901457 100644 --- a/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs +++ b/crates/namada/src/ledger/native_vp/ethereum_bridge/bridge_pool_vp.rs @@ -133,7 +133,7 @@ where #[inline] fn check_escrowed_toks( &self, - delta: EscrowDelta, + delta: EscrowDelta<'_, K>, ) -> Result { self.check_escrowed_toks_balance(delta) .map(|balance| balance.is_some()) @@ -144,7 +144,7 @@ where /// the updated escrow balance. fn check_escrowed_toks_balance( &self, - delta: EscrowDelta, + delta: EscrowDelta<'_, K>, ) -> Result, Error> { let EscrowDelta { token, diff --git a/crates/namada/src/ledger/native_vp/ibc/context.rs b/crates/namada/src/ledger/native_vp/ibc/context.rs index eb8d74a0582..b7c4072c264 100644 --- a/crates/namada/src/ledger/native_vp/ibc/context.rs +++ b/crates/namada/src/ledger/native_vp/ibc/context.rs @@ -3,6 +3,7 @@ use std::collections::BTreeSet; use borsh_ext::BorshSerializeExt; +use namada_core::arith::checked; use namada_core::collections::{HashMap, HashSet}; use namada_core::storage::Epochs; use namada_gas::MEMORY_ACCESS_GAS_PER_BYTE; @@ -80,25 +81,27 @@ where fn read_bytes(&self, key: &Key) -> Result>> { match self.store.get(key) { Some(StorageModification::Write { ref value }) => { - let gas = key.len() + value.len(); + let gas = checked!(key.len() + value.len())? as u64; self.ctx .ctx - .charge_gas(gas as u64 * MEMORY_ACCESS_GAS_PER_BYTE)?; + .charge_gas(checked!(gas * MEMORY_ACCESS_GAS_PER_BYTE)?)?; Ok(Some(value.clone())) } Some(StorageModification::Delete) => { - self.ctx.ctx.charge_gas( - key.len() as u64 * MEMORY_ACCESS_GAS_PER_BYTE, - )?; + let len = key.len() as u64; + self.ctx + .ctx + .charge_gas(checked!(len * MEMORY_ACCESS_GAS_PER_BYTE)?)?; Ok(None) } Some(StorageModification::InitAccount { .. }) => Err( StorageError::new_const("InitAccount shouldn't be inserted"), ), None => { - self.ctx.ctx.charge_gas( - key.len() as u64 * MEMORY_ACCESS_GAS_PER_BYTE, - )?; + let len = key.len() as u64; + self.ctx + .ctx + .charge_gas(checked!(len * MEMORY_ACCESS_GAS_PER_BYTE)?)?; self.ctx.read_bytes(key) } } @@ -164,19 +167,20 @@ where value: impl AsRef<[u8]>, ) -> Result<()> { let value = value.as_ref().to_vec(); - let gas = key.len() + value.len(); + let gas = checked!(key.len() + value.len())? as u64; self.store .insert(key.clone(), StorageModification::Write { value }); self.ctx .ctx - .charge_gas(gas as u64 * MEMORY_ACCESS_GAS_PER_BYTE) + .charge_gas(checked!(gas * MEMORY_ACCESS_GAS_PER_BYTE)?) } fn delete(&mut self, key: &Key) -> Result<()> { self.store.insert(key.clone(), StorageModification::Delete); + let len = key.len() as u64; self.ctx .ctx - .charge_gas(key.len() as u64 * MEMORY_ACCESS_GAS_PER_BYTE) + .charge_gas(checked!(len * MEMORY_ACCESS_GAS_PER_BYTE)?) } } diff --git a/crates/namada/src/ledger/native_vp/ibc/mod.rs b/crates/namada/src/ledger/native_vp/ibc/mod.rs index a69478c558f..66f6b6f0061 100644 --- a/crates/namada/src/ledger/native_vp/ibc/mod.rs +++ b/crates/namada/src/ledger/native_vp/ibc/mod.rs @@ -9,6 +9,7 @@ use std::time::Duration; use context::{PseudoExecutionContext, VpValidationContext}; use namada_core::address::Address; +use namada_core::arith::{self, checked}; use namada_core::collections::HashSet; use namada_core::storage::Key; use namada_gas::{IBC_ACTION_EXECUTE_GAS, IBC_ACTION_VALIDATE_GAS}; @@ -52,6 +53,8 @@ pub enum Error { IbcEvent(String), #[error("IBC rate limit: {0}")] RateLimit(String), + #[error("Arithmetic {0}")] + Arith(#[from] arith::Error), } /// IBC functions result @@ -196,7 +199,7 @@ where let epoch_duration = read_epoch_duration_parameter(&self.ctx.post()) .map_err(Error::NativeVpError)?; let unbonding_period_secs = - pipeline_len * epoch_duration.min_duration.0; + checked!(pipeline_len * epoch_duration.min_duration.0)?; Ok(ValidationParams { chain_id: IbcChainId::from_str(&chain_id) .map_err(ActionError::ChainId)?, @@ -388,6 +391,7 @@ pub fn get_dummy_genesis_validator() } } +#[allow(clippy::arithmetic_side_effects)] #[cfg(test)] mod tests { use std::str::FromStr; diff --git a/crates/namada/src/ledger/native_vp/masp.rs b/crates/namada/src/ledger/native_vp/masp.rs index 4e2af0d32b7..b15c32f4ab4 100644 --- a/crates/namada/src/ledger/native_vp/masp.rs +++ b/crates/namada/src/ledger/native_vp/masp.rs @@ -405,9 +405,8 @@ where return Err(error); } - let mut transparent_tx_pool = I128Sum::zero(); // The Sapling value balance adds to the transparent tx pool - transparent_tx_pool += shielded_tx.sapling_value_balance(); + let mut transparent_tx_pool = shielded_tx.sapling_value_balance(); // Check the validity of the keys and get the transfer data let transfer = @@ -459,7 +458,7 @@ where .checked_add( &I128Sum::from_nonnegative( vin.asset_type, - vin.value as i128, + i128::from(vin.value), ) .ok() .ok_or_err_msg( @@ -643,7 +642,7 @@ where .checked_sub( &I128Sum::from_nonnegative( out.asset_type, - out.value as i128, + i128::from(out.value), ) .ok() .ok_or_err_msg( diff --git a/crates/namada/src/ledger/native_vp/mod.rs b/crates/namada/src/ledger/native_vp/mod.rs index 4db6f4ad03b..4089d39a169 100644 --- a/crates/namada/src/ledger/native_vp/mod.rs +++ b/crates/namada/src/ledger/native_vp/mod.rs @@ -89,7 +89,7 @@ where /// Read access to the prior storage (state before tx execution) via /// [`trait@StorageRead`]. #[derive(Debug)] -pub struct CtxPreStorageRead<'view, 'a: 'view, S, CA> +pub struct CtxPreStorageRead<'view, 'a, S, CA> where S: StateRead, CA: WasmCacheAccess, @@ -100,7 +100,7 @@ where /// Read access to the posterior storage (state after tx execution) via /// [`trait@StorageRead`]. #[derive(Debug)] -pub struct CtxPostStorageRead<'view, 'a: 'view, S, CA> +pub struct CtxPostStorageRead<'view, 'a, S, CA> where S: StateRead, CA: WasmCacheAccess, diff --git a/crates/namada/src/ledger/protocol/mod.rs b/crates/namada/src/ledger/protocol/mod.rs index 7a2d8fccb39..df61dc9a19c 100644 --- a/crates/namada/src/ledger/protocol/mod.rs +++ b/crates/namada/src/ledger/protocol/mod.rs @@ -13,7 +13,7 @@ use namada_events::extend::{ ComposeEvent, Height as HeightAttr, TxHash as TxHashAttr, }; use namada_events::EventLevel; -use namada_gas::TxGasMeter; +use namada_gas::{Gas, TxGasMeter}; use namada_sdk::tx::TX_TRANSFER_WASM; use namada_state::StorageWrite; use namada_token::event::{TokenEvent, TokenOperation, UserAccount}; @@ -183,7 +183,7 @@ pub fn dispatch_tx<'a, D, H, CA>( state: &'a mut WlState, vp_wasm_cache: &'a mut VpCache, tx_wasm_cache: &'a mut TxCache, - wrapper_args: Option<&mut WrapperArgs>, + wrapper_args: Option<&mut WrapperArgs<'_>>, ) -> Result where D: 'static + DB + for<'iter> DBIter<'iter> + Sync, @@ -267,7 +267,7 @@ pub(crate) fn apply_wrapper_tx( fee_unshield_transaction: Option, tx_bytes: &[u8], mut shell_params: ShellParams<'_, S, D, H, CA>, - wrapper_args: Option<&mut WrapperArgs>, + wrapper_args: Option<&mut WrapperArgs<'_>>, ) -> Result> where S: State + Sync, @@ -337,7 +337,7 @@ fn charge_fee( masp_transaction: Option, shell_params: &mut ShellParams<'_, S, D, H, CA>, changed_keys: &mut BTreeSet, - wrapper_args: Option<&mut WrapperArgs>, + wrapper_args: Option<&mut WrapperArgs<'_>>, ) -> Result<()> where S: State + Sync, @@ -416,7 +416,10 @@ where .expect("Error reading the storage") .expect("Missing fee unshielding gas limit in storage") .min(tx_gas_meter.borrow().tx_gas_limit.into()); - let mut unshield_gas_meter = TxGasMeter::new(GasLimit::from(min_gas_limit)); + let mut unshield_gas_meter = TxGasMeter::new( + Gas::try_from(GasLimit::from(min_gas_limit)) + .expect("Min gas limit must not overflow"), + ); unshield_gas_meter .copy_consumed_gas_from(&tx_gas_meter.borrow()) .map_err(|e| Error::GasError(e.to_string()))?; diff --git a/crates/namada/src/ledger/vp_host_fns.rs b/crates/namada/src/ledger/vp_host_fns.rs index 2e5d1f57805..652991cf3f8 100644 --- a/crates/namada/src/ledger/vp_host_fns.rs +++ b/crates/namada/src/ledger/vp_host_fns.rs @@ -5,6 +5,7 @@ use std::fmt::Debug; use std::num::TryFromIntError; use namada_core::address::{Address, ESTABLISHED_ADDRESS_BYTES_LEN}; +use namada_core::arith::{self, checked}; use namada_core::hash::{Hash, HASH_LENGTH}; use namada_core::storage::{ BlockHeight, Epoch, Epochs, Header, Key, TxIndex, TX_INDEX_LENGTH, @@ -12,7 +13,7 @@ use namada_core::storage::{ use namada_events::{Event, EventTypeBuilder}; use namada_gas::MEMORY_ACCESS_GAS_PER_BYTE; use namada_state::write_log::WriteLog; -use namada_state::{write_log, DBIter, StateRead, DB}; +use namada_state::{write_log, DBIter, ResultExt, StateRead, DB}; use namada_tx::{Section, Tx}; use thiserror::Error; @@ -25,14 +26,16 @@ use crate::ledger::gas::{GasMetering, VpGasMeter}; pub enum RuntimeError { #[error("Out of gas: {0}")] OutOfGas(gas::Error), + #[error("State error: {0}")] + StateError(namada_state::Error), #[error("Storage error: {0}")] - StorageError(namada_state::Error), + StorageError(#[from] namada_state::StorageError), #[error("Storage data error: {0}")] StorageDataError(crate::storage::Error), #[error("Encoding error: {0}")] EncodingError(std::io::Error), #[error("Numeric conversion error: {0}")] - NumConversionError(TryFromIntError), + NumConversionError(#[from] TryFromIntError), #[error("Memory error: {0}")] MemoryError(Box), #[error("Invalid transaction code hash")] @@ -43,6 +46,8 @@ pub enum RuntimeError { InvalidSectionSignature(String), #[error("{0}")] Erased(String), // type erased error + #[error("Arithmetic {0}")] + Arith(#[from] arith::Error), } /// VP environment function result @@ -69,7 +74,8 @@ pub fn read_pre( where S: StateRead + Debug, { - let (log_val, gas) = state.write_log().read_pre(key); + let (log_val, gas) = + state.write_log().read_pre(key).into_storage_result()?; add_gas(gas_meter, gas)?; match log_val { Some(write_log::StorageModification::Write { ref value }) => { @@ -88,7 +94,7 @@ where None => { // When not found in write log, try to read from the storage let (value, gas) = - state.db_read(key).map_err(RuntimeError::StorageError)?; + state.db_read(key).map_err(RuntimeError::StateError)?; add_gas(gas_meter, gas)?; Ok(value) } @@ -106,7 +112,7 @@ where S: StateRead + Debug, { // Try to read from the write log first - let (log_val, gas) = state.write_log().read(key); + let (log_val, gas) = state.write_log().read(key).into_storage_result()?; add_gas(gas_meter, gas)?; match log_val { Some(write_log::StorageModification::Write { value }) => { @@ -124,7 +130,7 @@ where // When not found in write log, try // to read from the storage let (value, gas) = - state.db_read(key).map_err(RuntimeError::StorageError)?; + state.db_read(key).map_err(RuntimeError::StateError)?; add_gas(gas_meter, gas)?; Ok(value) } @@ -141,7 +147,8 @@ pub fn read_temp( where S: StateRead + Debug, { - let (log_val, gas) = state.write_log().read_temp(key); + let (log_val, gas) = + state.write_log().read_temp(key).into_storage_result()?; add_gas(gas_meter, gas)?; Ok(log_val.cloned()) } @@ -157,7 +164,8 @@ where S: StateRead + Debug, { // Try to read from the write log first - let (log_val, gas) = state.write_log().read_pre(key); + let (log_val, gas) = + state.write_log().read_pre(key).into_storage_result()?; add_gas(gas_meter, gas)?; match log_val { Some(&write_log::StorageModification::Write { .. }) => Ok(true), @@ -169,7 +177,7 @@ where None => { // When not found in write log, try to check the storage let (present, gas) = - state.db_has_key(key).map_err(RuntimeError::StorageError)?; + state.db_has_key(key).map_err(RuntimeError::StateError)?; add_gas(gas_meter, gas)?; Ok(present) } @@ -187,7 +195,7 @@ where S: StateRead + Debug, { // Try to read from the write log first - let (log_val, gas) = state.write_log().read(key); + let (log_val, gas) = state.write_log().read(key).into_storage_result()?; add_gas(gas_meter, gas)?; match log_val { Some(write_log::StorageModification::Write { .. }) => Ok(true), @@ -200,7 +208,7 @@ where // When not found in write log, try // to check the storage let (present, gas) = - state.db_has_key(key).map_err(RuntimeError::StorageError)?; + state.db_has_key(key).map_err(RuntimeError::StateError)?; add_gas(gas_meter, gas)?; Ok(present) } @@ -244,7 +252,7 @@ where S: StateRead + Debug, { let (header, gas) = StateRead::get_block_header(state, Some(height)) - .map_err(RuntimeError::StorageError)?; + .map_err(RuntimeError::StateError)?; add_gas(gas_meter, gas)?; Ok(header) } @@ -255,7 +263,12 @@ pub fn get_tx_code_hash( gas_meter: &RefCell, tx: &Tx, ) -> EnvResult> { - add_gas(gas_meter, HASH_LENGTH as u64 * MEMORY_ACCESS_GAS_PER_BYTE)?; + add_gas( + gas_meter, + (HASH_LENGTH as u64) + .checked_mul(MEMORY_ACCESS_GAS_PER_BYTE) + .expect("Consts mul that cannot overflow"), + )?; let hash = tx .get_section(tx.code_sechash()) .and_then(|x| Section::code_sec(x.as_ref())) @@ -285,7 +298,9 @@ pub fn get_tx_index( ) -> EnvResult { add_gas( gas_meter, - TX_INDEX_LENGTH as u64 * MEMORY_ACCESS_GAS_PER_BYTE, + (TX_INDEX_LENGTH as u64) + .checked_mul(MEMORY_ACCESS_GAS_PER_BYTE) + .expect("Consts mul that cannot overflow"), )?; Ok(*tx_index) } @@ -300,7 +315,9 @@ where { add_gas( gas_meter, - ESTABLISHED_ADDRESS_BYTES_LEN as u64 * MEMORY_ACCESS_GAS_PER_BYTE, + (ESTABLISHED_ADDRESS_BYTES_LEN as u64) + .checked_mul(MEMORY_ACCESS_GAS_PER_BYTE) + .expect("Consts mul that cannot overflow"), )?; Ok(state.in_mem().native_token.clone()) } @@ -313,12 +330,8 @@ pub fn get_pred_epochs( where S: StateRead + Debug, { - add_gas( - gas_meter, - state.in_mem().block.pred_epochs.first_block_heights.len() as u64 - * 8 - * MEMORY_ACCESS_GAS_PER_BYTE, - )?; + let len = state.in_mem().block.pred_epochs.first_block_heights.len() as u64; + add_gas(gas_meter, checked!(len * 8 * MEMORY_ACCESS_GAS_PER_BYTE)?)?; Ok(state.in_mem().block.pred_epochs.clone()) } @@ -354,7 +367,7 @@ pub fn iter_prefix_pre<'a, D>( where D: DB + for<'iter> DBIter<'iter>, { - let (iter, gas) = namada_state::iter_prefix_pre(write_log, db, prefix); + let (iter, gas) = namada_state::iter_prefix_pre(write_log, db, prefix)?; add_gas(gas_meter, gas)?; Ok(iter) } @@ -373,7 +386,7 @@ pub fn iter_prefix_post<'a, D>( where D: DB + for<'iter> DBIter<'iter>, { - let (iter, gas) = namada_state::iter_prefix_post(write_log, db, prefix); + let (iter, gas) = namada_state::iter_prefix_post(write_log, db, prefix)?; add_gas(gas_meter, gas)?; Ok(iter) } @@ -381,7 +394,7 @@ where /// Get the next item in a storage prefix iterator (pre or post). pub fn iter_next( gas_meter: &RefCell, - iter: &mut namada_state::PrefixIter, + iter: &mut namada_state::PrefixIter<'_, DB>, ) -> EnvResult)>> where DB: namada_state::DB + for<'iter> namada_state::DBIter<'iter>, diff --git a/crates/namada/src/lib.rs b/crates/namada/src/lib.rs index 15290a714fc..e4882618dda 100644 --- a/crates/namada/src/lib.rs +++ b/crates/namada/src/lib.rs @@ -2,9 +2,17 @@ #![doc(html_favicon_url = "https://dev.namada.net/master/favicon.png")] #![doc(html_logo_url = "https://dev.namada.net/master/rustdoc-logo.png")] -#![warn(missing_docs)] #![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 +)] pub use namada_core::{ address, chain, dec, decode, encode, eth_abi, eth_bridge_pool, diff --git a/crates/namada/src/vm/host_env.rs b/crates/namada/src/vm/host_env.rs index 2b52406f494..259a0443b79 100644 --- a/crates/namada/src/vm/host_env.rs +++ b/crates/namada/src/vm/host_env.rs @@ -10,6 +10,7 @@ use borsh_ext::BorshSerializeExt; use gas::IBC_TX_GAS; use masp_primitives::transaction::Transaction; use namada_core::address::ESTABLISHED_ADDRESS_BYTES_LEN; +use namada_core::arith::{self, checked}; use namada_core::internal::KeyVal; use namada_core::storage::TX_INDEX_LENGTH; use namada_events::{Event, EventTypeBuilder}; @@ -19,8 +20,9 @@ use namada_gas::{ }; use namada_state::write_log::{self, WriteLog}; use namada_state::{ - DBIter, InMemory, State, StateRead, StorageError, StorageHasher, - StorageRead, StorageWrite, TxHostEnvState, VpHostEnvState, DB, + DBIter, InMemory, OptionExt, ResultExt, State, StateRead, StorageError, + StorageHasher, StorageRead, StorageWrite, TxHostEnvState, VpHostEnvState, + DB, }; use namada_token::storage_key::is_any_token_parameter_key; use namada_tx::data::TxSentinel; @@ -51,6 +53,8 @@ use crate::vm::{HostRef, MutHostRef}; pub enum TxRuntimeError { #[error("Out of gas: {0}")] OutOfGas(gas::Error), + #[error("Gas overflow")] + GasOverflow, #[error("Trying to modify storage for an address that doesn't exit {0}")] UnknownAddressStorageModification(Address), #[error( @@ -72,7 +76,7 @@ pub enum TxRuntimeError { #[error("Address error: {0}")] AddressError(address::DecodeError), #[error("Numeric conversion error: {0}")] - NumConversionError(TryFromIntError), + NumConversionError(#[from] TryFromIntError), #[error("Memory error: {0}")] MemoryError(Box), #[error("Missing tx data")] @@ -83,6 +87,8 @@ pub enum TxRuntimeError { NoValueInResultBuffer, #[error("VP code is not allowed in allowlist parameter.")] DisallowedVp, + #[error("Arithmetic {0}")] + Arith(#[from] arith::Error), } /// Result of a tx host env fn call @@ -215,7 +221,7 @@ where } /// Access state from within a tx - pub fn state(&self) -> TxHostEnvState { + pub fn state(&self) -> TxHostEnvState<'_, D, H> { self.ctx.state() } } @@ -242,7 +248,7 @@ where CA: WasmCacheAccess, { /// Access state from within a tx - pub fn state(&self) -> TxHostEnvState { + pub fn state(&self) -> TxHostEnvState<'_, D, H> { let write_log = unsafe { self.write_log.get() }; let db = unsafe { self.db.get() }; let in_mem = unsafe { self.in_mem.get() }; @@ -434,7 +440,7 @@ where } /// Access state from within a VP - pub fn state(&self) -> VpHostEnvState { + pub fn state(&self) -> VpHostEnvState<'_, D, H> { self.ctx.state() } } @@ -523,7 +529,7 @@ where } /// Access state from within a VP - pub fn state(&self) -> VpHostEnvState { + pub fn state(&self) -> VpHostEnvState<'_, D, H> { let write_log = unsafe { self.write_log.get() }; let db = unsafe { self.db.get() }; let in_mem = unsafe { self.in_mem.get() }; @@ -575,7 +581,7 @@ where /// Add a gas cost incured in a transaction pub fn tx_charge_gas( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, used_gas: u64, ) -> TxResult<()> where @@ -599,7 +605,7 @@ where /// Called from VP wasm to request to use the given gas amount pub fn vp_charge_gas( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, used_gas: u64, ) -> vp_host_fns::EnvResult<()> where @@ -616,7 +622,7 @@ where /// Storage `has_key` function exposed to the wasm VM Tx environment. It will /// try to check the write log first and if no entry found then the storage. pub fn tx_has_key( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, key_ptr: u64, key_len: u64, ) -> TxResult @@ -628,7 +634,7 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -648,7 +654,7 @@ where /// Returns `-1` when the key is not present, or the length of the data when /// the key is present (the length may be `0`). pub fn tx_read( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, key_ptr: u64, key_len: u64, ) -> TxResult @@ -660,7 +666,7 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -691,7 +697,7 @@ where /// Returns `-1` when the key is not present, or the length of the data when /// the key is present (the length may be `0`). pub fn tx_read_temp( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, key_ptr: u64, key_len: u64, ) -> TxResult @@ -703,7 +709,7 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -712,7 +718,7 @@ where let key = Key::parse(key).map_err(TxRuntimeError::StorageDataError)?; let write_log = unsafe { env.ctx.write_log.get() }; - let (log_val, gas) = write_log.read_temp(&key); + let (log_val, gas) = write_log.read_temp(&key).into_storage_result()?; tx_charge_gas::(env, gas)?; match log_val { Some(value) => { @@ -737,7 +743,7 @@ where /// any) back to the guest, the second step reads the value from cache into a /// pre-allocated buffer with the obtained size. pub fn tx_result_buffer( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, result_ptr: u64, ) -> TxResult<()> where @@ -761,7 +767,7 @@ where /// It will try to get an iterator from the storage and return the corresponding /// ID of the iterator, ordered by storage keys. pub fn tx_iter_prefix( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, prefix_ptr: u64, prefix_len: u64, ) -> TxResult @@ -773,7 +779,7 @@ where { let (prefix, gas) = env .memory - .read_string(prefix_ptr, prefix_len as _) + .read_string(prefix_ptr, prefix_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -784,11 +790,14 @@ where let write_log = unsafe { env.ctx.write_log.get() }; let db = unsafe { env.ctx.db.get() }; - let (iter, gas) = namada_state::iter_prefix_post(write_log, db, &prefix); + let (iter, gas) = namada_state::iter_prefix_post(write_log, db, &prefix)?; tx_charge_gas::(env, gas)?; let iterators = unsafe { env.ctx.iterators.get() }; - Ok(iterators.insert(iter).id()) + Ok(iterators + .insert(iter) + .ok_or_err_msg("Iterator ID overflow")? + .id()) } /// Storage prefix iterator next function exposed to the wasm VM Tx environment. @@ -798,7 +807,7 @@ where /// Returns `-1` when the key is not present, or the length of the data when /// the key is present (the length may be `0`). pub fn tx_iter_next( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, iter_id: u64, ) -> TxResult where @@ -813,11 +822,14 @@ where let iterators = unsafe { env.ctx.iterators.get() }; let iter_id = PrefixIteratorId::new(iter_id); while let Some((key, val, iter_gas)) = iterators.next(iter_id) { - let (log_val, log_gas) = state.write_log().read( - &Key::parse(key.clone()) - .map_err(TxRuntimeError::StorageDataError)?, - ); - tx_charge_gas::(env, iter_gas + log_gas)?; + let (log_val, log_gas) = state + .write_log() + .read( + &Key::parse(key.clone()) + .map_err(TxRuntimeError::StorageDataError)?, + ) + .into_storage_result()?; + tx_charge_gas::(env, checked!(iter_gas + log_gas)?)?; match log_val { Some(write_log::StorageModification::Write { ref value }) => { let key_val = borsh::to_vec(&KeyVal { @@ -860,7 +872,7 @@ where /// Storage write function exposed to the wasm VM Tx environment. The given /// key/value will be written to the write log. pub fn tx_write( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, key_ptr: u64, key_len: u64, val_ptr: u64, @@ -874,12 +886,12 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let (value, gas) = env .memory - .read_bytes(val_ptr, val_len as _) + .read_bytes(val_ptr, val_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -902,7 +914,7 @@ where /// given key/value will be written only to the write log. It will be never /// written to the storage. pub fn tx_write_temp( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, key_ptr: u64, key_len: u64, val_ptr: u64, @@ -916,12 +928,12 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let (value, gas) = env .memory - .read_bytes(val_ptr, val_len as _) + .read_bytes(val_ptr, val_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -940,7 +952,7 @@ where } fn check_address_existence( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, key: &Key, ) -> TxResult<()> where @@ -987,7 +999,7 @@ where /// Storage delete function exposed to the wasm VM Tx environment. The given /// key/value will be written as deleted to the write log. pub fn tx_delete( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, key_ptr: u64, key_len: u64, ) -> TxResult<()> @@ -999,7 +1011,7 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -1017,7 +1029,7 @@ where /// Expose the functionality to emit events to the wasm VM's Tx environment. /// An emitted event will land in the write log. pub fn tx_emit_event( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, event_ptr: u64, event_len: u64, ) -> TxResult<()> @@ -1029,19 +1041,22 @@ where { let (event, gas) = env .memory - .read_bytes(event_ptr, event_len as _) + .read_bytes(event_ptr, event_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let event: Event = BorshDeserialize::try_from_slice(&event) .map_err(TxRuntimeError::EncodingError)?; let mut state = env.state(); - let gas = state.write_log_mut().emit_event(event); + let gas = state + .write_log_mut() + .emit_event(event) + .ok_or(TxRuntimeError::GasOverflow)?; tx_charge_gas::(env, gas) } /// Expose the functionality to query events from the wasm VM's Tx environment. pub fn tx_get_events( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, event_type_ptr: u64, event_type_len: u64, ) -> TxResult @@ -1053,7 +1068,7 @@ where { let (event_type, gas) = env .memory - .read_string(event_type_ptr, event_type_len as _) + .read_string(event_type_ptr, event_type_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let state = env.state(); @@ -1082,7 +1097,7 @@ where /// Returns `-1` when the key is not present, or the length of the data when /// the key is present (the length may be `0`). pub fn vp_read_pre( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, key_ptr: u64, key_len: u64, ) -> vp_host_fns::EnvResult @@ -1095,7 +1110,7 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = env.ctx.gas_meter(); vp_host_fns::add_gas(gas_meter, gas)?; @@ -1132,7 +1147,7 @@ where /// Returns `-1` when the key is not present, or the length of the data when /// the key is present (the length may be `0`). pub fn vp_read_post( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, key_ptr: u64, key_len: u64, ) -> vp_host_fns::EnvResult @@ -1145,7 +1160,7 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = env.ctx.gas_meter(); vp_host_fns::add_gas(gas_meter, gas)?; @@ -1177,7 +1192,7 @@ where /// Returns `-1` when the key is not present, or the length of the data when /// the key is present (the length may be `0`). pub fn vp_read_temp( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, key_ptr: u64, key_len: u64, ) -> vp_host_fns::EnvResult @@ -1190,7 +1205,7 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = env.ctx.gas_meter(); vp_host_fns::add_gas(gas_meter, gas)?; @@ -1225,7 +1240,7 @@ where /// any) back to the guest, the second step reads the value from cache into a /// pre-allocated buffer with the obtained size. pub fn vp_result_buffer( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, result_ptr: u64, ) -> vp_host_fns::EnvResult<()> where @@ -1250,7 +1265,7 @@ where /// Storage `has_key` in prior state (before tx execution) function exposed to /// the wasm VM VP environment. It will try to read from the storage. pub fn vp_has_key_pre( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, key_ptr: u64, key_len: u64, ) -> vp_host_fns::EnvResult @@ -1263,7 +1278,7 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = env.ctx.gas_meter(); vp_host_fns::add_gas(gas_meter, gas)?; @@ -1281,7 +1296,7 @@ where /// to the wasm VM VP environment. It will try to check the write log first and /// if no entry found then the storage. pub fn vp_has_key_post( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, key_ptr: u64, key_len: u64, ) -> vp_host_fns::EnvResult @@ -1294,7 +1309,7 @@ where { let (key, gas) = env .memory - .read_string(key_ptr, key_len as _) + .read_string(key_ptr, key_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = env.ctx.gas_meter(); vp_host_fns::add_gas(gas_meter, gas)?; @@ -1313,7 +1328,7 @@ where /// the storage and return the corresponding ID of the iterator, ordered by /// storage keys. pub fn vp_iter_prefix_pre( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, prefix_ptr: u64, prefix_len: u64, ) -> vp_host_fns::EnvResult @@ -1326,7 +1341,7 @@ where { let (prefix, gas) = env .memory - .read_string(prefix_ptr, prefix_len as _) + .read_string(prefix_ptr, prefix_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = env.ctx.gas_meter(); vp_host_fns::add_gas(gas_meter, gas)?; @@ -1341,7 +1356,10 @@ where let iter = vp_host_fns::iter_prefix_pre(gas_meter, write_log, db, &prefix)?; let iterators = unsafe { env.ctx.iterators.get() }; - Ok(iterators.insert(iter).id()) + Ok(iterators + .insert(iter) + .ok_or_err_msg("Iterator ID overflow")? + .id()) } /// Storage prefix iterator function for posterior state (after tx execution) @@ -1349,7 +1367,7 @@ where /// the storage and return the corresponding ID of the iterator, ordered by /// storage keys. pub fn vp_iter_prefix_post( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, prefix_ptr: u64, prefix_len: u64, ) -> vp_host_fns::EnvResult @@ -1362,7 +1380,7 @@ where { let (prefix, gas) = env .memory - .read_string(prefix_ptr, prefix_len as _) + .read_string(prefix_ptr, prefix_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = env.ctx.gas_meter(); vp_host_fns::add_gas(gas_meter, gas)?; @@ -1378,7 +1396,10 @@ where vp_host_fns::iter_prefix_post(gas_meter, write_log, db, &prefix)?; let iterators = unsafe { env.ctx.iterators.get() }; - Ok(iterators.insert(iter).id()) + Ok(iterators + .insert(iter) + .ok_or_err_msg("Iterator ID overflow")? + .id()) } /// Storage prefix iterator for prior or posterior state function @@ -1387,7 +1408,7 @@ where /// Returns `-1` when the key is not present, or the length of the data when /// the key is present (the length may be `0`). pub fn vp_iter_next( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, iter_id: u64, ) -> vp_host_fns::EnvResult where @@ -1420,7 +1441,7 @@ where /// Verifier insertion function exposed to the wasm VM Tx environment. pub fn tx_insert_verifier( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, addr_ptr: u64, addr_len: u64, ) -> TxResult<()> @@ -1432,7 +1453,7 @@ where { let (addr, gas) = env .memory - .read_string(addr_ptr, addr_len as _) + .read_string(addr_ptr, addr_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -1443,7 +1464,10 @@ where let verifiers = unsafe { env.ctx.verifiers.get() }; // This is not a storage write, use the same multiplier used for a storage // read - tx_charge_gas::(env, addr_len * MEMORY_ACCESS_GAS_PER_BYTE)?; + tx_charge_gas::( + env, + checked!(addr_len * MEMORY_ACCESS_GAS_PER_BYTE)?, + )?; verifiers.insert(addr); Ok(()) @@ -1451,7 +1475,7 @@ where /// Update a validity predicate function exposed to the wasm VM Tx environment pub fn tx_update_validity_predicate( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, addr_ptr: u64, addr_len: u64, code_hash_ptr: u64, @@ -1467,7 +1491,7 @@ where { let (addr, gas) = env .memory - .read_string(addr_ptr, addr_len as _) + .read_string(addr_ptr, addr_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -1476,7 +1500,7 @@ where let (code_tag, gas) = env .memory - .read_bytes(code_tag_ptr, code_tag_len as _) + .read_bytes(code_tag_ptr, code_tag_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let code_tag = Option::::try_from_slice(&code_tag) @@ -1485,7 +1509,7 @@ where let key = Key::validity_predicate(&addr); let (code_hash, gas) = env .memory - .read_bytes(code_hash_ptr, code_hash_len as _) + .read_bytes(code_hash_ptr, code_hash_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -1502,7 +1526,7 @@ where /// Initialize a new account established address. #[allow(clippy::too_many_arguments)] pub fn tx_init_account( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, code_hash_ptr: u64, code_hash_len: u64, code_tag_ptr: u64, @@ -1519,13 +1543,13 @@ where { let (code_hash, gas) = env .memory - .read_bytes(code_hash_ptr, code_hash_len as _) + .read_bytes(code_hash_ptr, code_hash_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let (code_tag, gas) = env .memory - .read_bytes(code_tag_ptr, code_tag_len as _) + .read_bytes(code_tag_ptr, code_tag_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let code_tag = Option::::try_from_slice(&code_tag) @@ -1535,7 +1559,7 @@ where let (entropy_source, gas) = env .memory - .read_bytes(entropy_source_ptr, entropy_source_len as _) + .read_bytes(entropy_source_ptr, entropy_source_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -1558,7 +1582,7 @@ where /// Getting the chain ID function exposed to the wasm VM Tx environment. pub fn tx_get_chain_id( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, result_ptr: u64, ) -> TxResult<()> where @@ -1581,7 +1605,7 @@ where /// environment. The height is that of the block to which the current /// transaction is being applied. pub fn tx_get_block_height( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, ) -> TxResult where MEM: VmMemory, @@ -1599,7 +1623,7 @@ where /// environment. The index is that of the transaction being applied /// in the current block. pub fn tx_get_tx_index( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, ) -> TxResult where MEM: VmMemory, @@ -1609,7 +1633,9 @@ where { tx_charge_gas::( env, - TX_INDEX_LENGTH as u64 * MEMORY_ACCESS_GAS_PER_BYTE, + (TX_INDEX_LENGTH as u64) + .checked_mul(MEMORY_ACCESS_GAS_PER_BYTE) + .expect("Consts mul that cannot overflow"), )?; let tx_index = unsafe { env.ctx.tx_index.get() }; Ok(tx_index.0) @@ -1619,7 +1645,7 @@ where /// environment. The height is that of the block to which the current /// transaction is being applied. pub fn vp_get_tx_index( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, ) -> vp_host_fns::EnvResult where MEM: VmMemory, @@ -1638,7 +1664,7 @@ where /// environment. The epoch is that of the block to which the current /// transaction is being applied. pub fn tx_get_block_epoch( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, ) -> TxResult where MEM: VmMemory, @@ -1654,7 +1680,7 @@ where /// Get predecessor epochs function exposed to the wasm VM Tx environment. pub fn tx_get_pred_epochs( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, ) -> TxResult where MEM: VmMemory, @@ -1669,9 +1695,10 @@ where .len() .try_into() .map_err(TxRuntimeError::NumConversionError)?; + let len_u64 = u64::try_from(len)?; tx_charge_gas::( env, - MEMORY_ACCESS_GAS_PER_BYTE * len as u64, + checked!(MEMORY_ACCESS_GAS_PER_BYTE * len_u64)?, )?; let result_buffer = unsafe { env.ctx.result_buffer.get() }; result_buffer.replace(bytes); @@ -1680,7 +1707,7 @@ where /// Get the native token's address pub fn tx_get_native_token( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, result_ptr: u64, ) -> TxResult<()> where @@ -1692,7 +1719,9 @@ where // Gas for getting the native token address from storage tx_charge_gas::( env, - ESTABLISHED_ADDRESS_BYTES_LEN as u64 * MEMORY_ACCESS_GAS_PER_BYTE, + (ESTABLISHED_ADDRESS_BYTES_LEN as u64) + .checked_mul(MEMORY_ACCESS_GAS_PER_BYTE) + .expect("Consts mul that cannot overflow"), )?; let state = env.state(); let native_token = state.in_mem().native_token.clone(); @@ -1706,7 +1735,7 @@ where /// Getting the block header function exposed to the wasm VM Tx environment. pub fn tx_get_block_header( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, height: u64, ) -> TxResult where @@ -1738,7 +1767,7 @@ where /// Getting the chain ID function exposed to the wasm VM VP environment. pub fn vp_get_chain_id( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, result_ptr: u64, ) -> vp_host_fns::EnvResult<()> where @@ -1762,7 +1791,7 @@ where /// environment. The height is that of the block to which the current /// transaction is being applied. pub fn vp_get_block_height( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, ) -> vp_host_fns::EnvResult where MEM: VmMemory, @@ -1779,7 +1808,7 @@ where /// Getting the block header function exposed to the wasm VM VP environment. pub fn vp_get_block_header( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, height: u64, ) -> vp_host_fns::EnvResult where @@ -1793,7 +1822,7 @@ where let state = env.state(); let (header, gas) = StateRead::get_block_header(&state, Some(BlockHeight(height))) - .map_err(vp_host_fns::RuntimeError::StorageError)?; + .map_err(vp_host_fns::RuntimeError::StateError)?; vp_host_fns::add_gas(gas_meter, gas)?; Ok(match header { Some(h) => { @@ -1812,7 +1841,7 @@ where /// Getting the transaction hash function exposed to the wasm VM VP environment. pub fn vp_get_tx_code_hash( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, result_ptr: u64, ) -> vp_host_fns::EnvResult<()> where @@ -1843,7 +1872,7 @@ where /// environment. The epoch is that of the block to which the current /// transaction is being applied. pub fn vp_get_block_epoch( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, ) -> vp_host_fns::EnvResult where MEM: VmMemory, @@ -1860,7 +1889,7 @@ where /// Get predecessor epochs function exposed to the wasm VM VP environment. pub fn vp_get_pred_epochs( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, ) -> vp_host_fns::EnvResult where MEM: VmMemory, @@ -1884,7 +1913,7 @@ where /// Expose the functionality to query events from the wasm VM's VP environment. pub fn vp_get_events( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, event_type_ptr: u64, event_type_len: u64, ) -> vp_host_fns::EnvResult @@ -1897,7 +1926,7 @@ where { let (event_type, gas) = env .memory - .read_string(event_type_ptr, event_type_len as _) + .read_string(event_type_ptr, event_type_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = env.ctx.gas_meter(); vp_host_fns::add_gas(gas_meter, gas)?; @@ -1918,7 +1947,7 @@ where /// performance #[allow(clippy::too_many_arguments)] pub fn vp_verify_tx_section_signature( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, hash_list_ptr: u64, hash_list_len: u64, public_keys_map_ptr: u64, @@ -1938,7 +1967,7 @@ where { let (hash_list, gas) = env .memory - .read_bytes(hash_list_ptr, hash_list_len as _) + .read_bytes(hash_list_ptr, hash_list_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; let gas_meter = env.ctx.gas_meter(); @@ -1948,7 +1977,7 @@ where let (public_keys_map, gas) = env .memory - .read_bytes(public_keys_map_ptr, public_keys_map_len as _) + .read_bytes(public_keys_map_ptr, public_keys_map_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; vp_host_fns::add_gas(gas_meter, gas)?; let public_keys_map = @@ -1959,7 +1988,7 @@ where let (signer, gas) = env .memory - .read_bytes(signer_ptr, signer_len as _) + .read_bytes(signer_ptr, signer_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; vp_host_fns::add_gas(gas_meter, gas)?; let signer = Address::try_from_slice(&signer) @@ -1967,7 +1996,7 @@ where let (max_signatures, gas) = env .memory - .read_bytes(max_signatures_ptr, max_signatures_len as _) + .read_bytes(max_signatures_ptr, max_signatures_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; vp_host_fns::add_gas(gas_meter, gas)?; let max_signatures = Option::::try_from_slice(&max_signatures) @@ -2000,7 +2029,7 @@ where /// printed at the [`tracing::Level::INFO`]. This function is for development /// only. pub fn tx_log_string( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, str_ptr: u64, str_len: u64, ) -> TxResult<()> @@ -2012,7 +2041,7 @@ where { let (str, _gas) = env .memory - .read_string(str_ptr, str_len as _) + .read_string(str_ptr, str_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tracing::info!("WASM Transaction log: {}", str); Ok(()) @@ -2022,7 +2051,7 @@ where // Temporarily the IBC tx execution is implemented via a host function to // workaround wasm issue. pub fn tx_ibc_execute( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, ) -> TxResult where MEM: VmMemory, @@ -2064,7 +2093,9 @@ where for addr in verifiers.into_iter() { tx_charge_gas::( env, - ESTABLISHED_ADDRESS_BYTES_LEN as u64 * MEMORY_ACCESS_GAS_PER_BYTE, + (ESTABLISHED_ADDRESS_BYTES_LEN as u64) + .checked_mul(MEMORY_ACCESS_GAS_PER_BYTE) + .expect("Consts mul that cannot overflow"), )?; verifiers_in_env.insert(addr); } @@ -2081,7 +2112,7 @@ where /// Validate a VP WASM code hash in a tx environment. fn tx_validate_vp_code_hash( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, code_hash: &[u8], code_tag: &Option, ) -> TxResult<()> @@ -2139,8 +2170,9 @@ where } /// Set the sentinel for an invalid tx section commitment -pub fn tx_set_commitment_sentinel(env: &TxVmEnv) -where +pub fn tx_set_commitment_sentinel( + env: &TxVmEnv<'_, MEM, D, H, CA>, +) where D: 'static + DB + for<'iter> DBIter<'iter>, H: 'static + StorageHasher, MEM: VmMemory, @@ -2153,7 +2185,7 @@ where /// Verify a transaction signature #[allow(clippy::too_many_arguments)] pub fn tx_verify_tx_section_signature( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, hash_list_ptr: u64, hash_list_len: u64, public_keys_map_ptr: u64, @@ -2170,7 +2202,7 @@ where { let (hash_list, gas) = env .memory - .read_bytes(hash_list_ptr, hash_list_len as _) + .read_bytes(hash_list_ptr, hash_list_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -2179,7 +2211,7 @@ where let (public_keys_map, gas) = env .memory - .read_bytes(public_keys_map_ptr, public_keys_map_len as _) + .read_bytes(public_keys_map_ptr, public_keys_map_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let public_keys_map = @@ -2192,7 +2224,7 @@ where let (max_signatures, gas) = env .memory - .read_bytes(max_signatures_ptr, max_signatures_len as _) + .read_bytes(max_signatures_ptr, max_signatures_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let max_signatures = Option::::try_from_slice(&max_signatures) @@ -2225,7 +2257,7 @@ where /// Appends the new note commitments to the tree in storage pub fn tx_update_masp_note_commitment_tree( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, transaction_ptr: u64, transaction_len: u64, ) -> TxResult @@ -2239,7 +2271,7 @@ where let _gas_meter = unsafe { env.ctx.gas_meter.get() }; let (serialized_transaction, gas) = env .memory - .read_bytes(transaction_ptr, transaction_len as _) + .read_bytes(transaction_ptr, transaction_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; @@ -2262,7 +2294,7 @@ where /// Yield a byte array value from the guest. pub fn tx_yield_value( - env: &TxVmEnv, + env: &TxVmEnv<'_, MEM, D, H, CA>, buf_ptr: u64, buf_len: u64, ) -> TxResult<()> @@ -2274,7 +2306,7 @@ where { let (value_to_yield, gas) = env .memory - .read_bytes(buf_ptr, buf_len as _) + .read_bytes(buf_ptr, buf_len.try_into()?) .map_err(|e| TxRuntimeError::MemoryError(Box::new(e)))?; tx_charge_gas::(env, gas)?; let host_buf = unsafe { env.ctx.yielded_value.get() }; @@ -2299,7 +2331,7 @@ where { let (vp_code_hash, gas) = env .memory - .read_bytes(vp_code_hash_ptr, vp_code_hash_len as _) + .read_bytes(vp_code_hash_ptr, vp_code_hash_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; // The borrowed `gas_meter` must be dropped before eval, @@ -2310,7 +2342,7 @@ where let (input_data, gas) = env .memory - .read_bytes(input_data_ptr, input_data_len as _) + .read_bytes(input_data_ptr, input_data_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; vp_host_fns::add_gas(gas_meter, gas)?; let tx: Tx = BorshDeserialize::try_from_slice(&input_data) @@ -2330,7 +2362,7 @@ where /// Get the native token's address pub fn vp_get_native_token( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, result_ptr: u64, ) -> vp_host_fns::EnvResult<()> where @@ -2355,7 +2387,7 @@ where /// printed at the [`tracing::Level::INFO`]. This function is for development /// only. pub fn vp_log_string( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, str_ptr: u64, str_len: u64, ) -> vp_host_fns::EnvResult<()> @@ -2368,7 +2400,7 @@ where { let (str, _gas) = env .memory - .read_string(str_ptr, str_len as _) + .read_string(str_ptr, str_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; tracing::info!("WASM Validity predicate log: {}", str); Ok(()) @@ -2376,7 +2408,7 @@ where /// Yield a byte array value from the guest. pub fn vp_yield_value( - env: &VpVmEnv, + env: &VpVmEnv<'_, MEM, D, H, EVAL, CA>, buf_ptr: u64, buf_len: u64, ) -> vp_host_fns::EnvResult<()> @@ -2389,7 +2421,7 @@ where { let (value_to_yield, gas) = env .memory - .read_bytes(buf_ptr, buf_len as _) + .read_bytes(buf_ptr, buf_len.try_into()?) .map_err(|e| vp_host_fns::RuntimeError::MemoryError(Box::new(e)))?; vp_host_fns::add_gas(env.ctx.gas_meter(), gas)?; let host_buf = unsafe { env.ctx.yielded_value.get() }; diff --git a/crates/namada/src/vm/mod.rs b/crates/namada/src/vm/mod.rs index e7aeeaa6cb9..4e7117c5f55 100644 --- a/crates/namada/src/vm/mod.rs +++ b/crates/namada/src/vm/mod.rs @@ -78,7 +78,7 @@ impl WasmCacheAccess for WasmCacheRoAccess { /// reference conversion, care must be taken that while this reference is /// borrowed, no other process can modify it. #[derive(Clone, Debug)] -pub struct HostRef<'a, T: 'a> { +pub struct HostRef<'a, T> { data: *const c_void, phantom: PhantomData<&'a T>, } @@ -116,7 +116,7 @@ impl<'a, T: 'a> HostRef<'a, &T> { /// conversion, care must be taken that while this slice is borrowed, no other /// process can modify it. #[derive(Clone)] -pub struct HostSlice<'a, T: 'a> { +pub struct HostSlice<'a, T> { data: *const c_void, len: usize, phantom: PhantomData<&'a T>, @@ -155,7 +155,7 @@ impl<'a, T: 'a> HostSlice<'a, &[T]> { /// not thread-safe. Also, care must be taken that while this reference is /// borrowed, no other process can read or modify it. #[derive(Clone, Debug)] -pub struct MutHostRef<'a, T: 'a> { +pub struct MutHostRef<'a, T> { data: *mut c_void, phantom: PhantomData<&'a T>, } @@ -195,7 +195,7 @@ impl<'a, T: 'a> MutHostRef<'a, &T> { /// conversion, care must be taken that while this slice is borrowed, no other /// process can modify it. #[derive(Clone)] -pub struct MutHostSlice<'a, T: 'a> { +pub struct MutHostSlice<'a, T> { data: *mut c_void, len: usize, phantom: PhantomData<&'a T>, diff --git a/crates/namada/src/vm/prefix_iter.rs b/crates/namada/src/vm/prefix_iter.rs index 2d9ceac4b7c..a36e52272bb 100644 --- a/crates/namada/src/vm/prefix_iter.rs +++ b/crates/namada/src/vm/prefix_iter.rs @@ -19,12 +19,16 @@ impl<'iter, DB> PrefixIterators<'iter, DB> where DB: namada_state::DB + namada_state::DBIter<'iter>, { - /// Insert a new prefix iterator to the temporary storage. - pub fn insert(&mut self, iter: PrefixIter<'iter, DB>) -> PrefixIteratorId { + /// Insert a new prefix iterator to the temporary storage. Returns `None` on + /// prefix iterator ID overflow + pub fn insert( + &mut self, + iter: PrefixIter<'iter, DB>, + ) -> Option { let id = self.index; self.iterators.insert(id, iter); - self.index = id.next_id(); - id + self.index = id.next_id()?; + Some(id) } /// Get the next item in the given prefix iterator. @@ -71,8 +75,8 @@ impl PrefixIteratorId { self.0 } - /// Get the ID for the next prefix iterator. - fn next_id(&self) -> PrefixIteratorId { - PrefixIteratorId(self.0 + 1) + /// Get the ID for the next prefix iterator. Returns `None` on overflow + fn next_id(&self) -> Option { + Some(PrefixIteratorId(self.0.checked_add(1)?)) } } diff --git a/crates/namada/src/vm/wasm/compilation_cache/common.rs b/crates/namada/src/vm/wasm/compilation_cache/common.rs index e22da51b6c9..b23d79e50ce 100644 --- a/crates/namada/src/vm/wasm/compilation_cache/common.rs +++ b/crates/namada/src/vm/wasm/compilation_cache/common.rs @@ -193,7 +193,11 @@ impl Cache { hash.to_string() ); sleep(exponential_backoff.backoff(&iter)); - iter += 1; + // Cannot overflow + #[allow(clippy::arithmetic_side_effects)] + { + iter += 1; + } continue; } None => { @@ -289,7 +293,11 @@ impl Cache { hash.to_string() ); sleep(exponential_backoff.backoff(&iter)); - iter += 1; + // Cannot overflow + #[allow(clippy::arithmetic_side_effects)] + { + iter += 1; + } continue; } None => { @@ -625,6 +633,7 @@ pub mod testing { } } +#[allow(clippy::arithmetic_side_effects)] #[cfg(test)] mod test { use std::cmp::max; diff --git a/crates/namada/src/vm/wasm/memory.rs b/crates/namada/src/vm/wasm/memory.rs index 671af064d30..593a0f7456b 100644 --- a/crates/namada/src/vm/wasm/memory.rs +++ b/crates/namada/src/vm/wasm/memory.rs @@ -7,11 +7,12 @@ use std::sync::Arc; use borsh_ext::BorshSerializeExt; use namada_gas::MEMORY_ACCESS_GAS_PER_BYTE; +use namada_sdk::arith::{self, checked}; use namada_tx::Tx; use thiserror::Error; use wasmer::{ vm, BaseTunables, HostEnvInitError, LazyInit, Memory, MemoryError, - MemoryType, Pages, TableType, Target, Tunables, + MemoryType, Pages, TableType, Target, Tunables, WASM_PAGE_SIZE, }; use wasmer_vm::{ MemoryStyle, TableStyle, VMMemoryDefinition, VMTableDefinition, @@ -35,6 +36,10 @@ pub enum Error { UninitializedMemory, #[error("Invalid utf8 string read from memory")] InvalidUtf8String(Utf8Error), + #[error("Arithmetic {0}")] + Arith(#[from] arith::Error), + #[error("{0}")] + TryFromInt(#[from] std::num::TryFromIntError), } /// Result of a function that may fail @@ -128,22 +133,22 @@ pub fn write_vp_inputs( data, keys_changed, verifiers, - }: VpInput, + }: VpInput<'_>, ) -> Result { - let addr_ptr = 0; + let addr_ptr = 0_u64; let addr_bytes = addr.serialize_to_vec(); let addr_len = addr_bytes.len() as _; let data_bytes = data.serialize_to_vec(); - let data_ptr = addr_ptr + addr_len; + let data_ptr = checked!(addr_ptr + addr_len)?; let data_len = data_bytes.len() as _; let keys_changed_bytes = keys_changed.serialize_to_vec(); - let keys_changed_ptr = data_ptr + data_len; + let keys_changed_ptr = checked!(data_ptr + data_len)?; let keys_changed_len = keys_changed_bytes.len() as _; let verifiers_bytes = verifiers.serialize_to_vec(); - let verifiers_ptr = keys_changed_ptr + keys_changed_len; + let verifiers_ptr = checked!(keys_changed_ptr + keys_changed_len)?; let verifiers_len = verifiers_bytes.len() as _; let bytes = [ @@ -179,7 +184,7 @@ fn check_bounds(memory: &Memory, base_addr: u64, offset: usize) -> Result<()> { let desired_offset = base_addr .checked_add(offset as u64) .and_then(|off| { - if off < u32::MAX as u64 { + if off < u64::from(u32::MAX) { // wasm pointers are 32 bits wide, therefore we can't // read from/write to offsets past `u32::MAX` Some(off) @@ -189,18 +194,20 @@ fn check_bounds(memory: &Memory, base_addr: u64, offset: usize) -> Result<()> { }) .ok_or(Error::OverflowingOffset(base_addr, offset))?; if memory.data_size() < desired_offset { - let cur_pages = memory.size().0; - let capacity = cur_pages as usize * wasmer::WASM_PAGE_SIZE; + let cur_pages = memory.size().0 as usize; + let capacity = checked!(cur_pages * WASM_PAGE_SIZE)?; // usizes should be at least 32 bits wide on most architectures, // so this cast shouldn't cause panics, given the invariant that // `desired_offset` is at most a 32 bit wide value. moreover, // `capacity` should not be larger than `memory.data_size()`, // so this subtraction should never fail - let missing = desired_offset as usize - capacity; + let desired_offset = usize::try_from(desired_offset)?; + let missing = checked!(desired_offset - capacity)?; // extrapolate the number of pages missing to allow addressing // the desired memory offset - let req_pages = ((missing + wasmer::WASM_PAGE_SIZE - 1) - / wasmer::WASM_PAGE_SIZE) as u32; + let req_pages = + checked!((missing + WASM_PAGE_SIZE - 1) / WASM_PAGE_SIZE)?; + let req_pages: u32 = u32::try_from(req_pages)?; tracing::debug!(req_pages, "Attempting to grow wasm memory"); memory.grow(req_pages).map_err(Error::MemoryOutOfBounds)?; tracing::debug!( @@ -218,8 +225,8 @@ fn read_memory_bytes( len: usize, ) -> Result> { check_bounds(memory, offset, len)?; - let offset = offset as usize; - let vec: Vec<_> = memory.view()[offset..(offset + len)] + let offset = usize::try_from(offset)?; + let vec: Vec<_> = memory.view()[offset..checked!(offset + len)?] .iter() .map(|cell| cell.get()) .collect(); @@ -235,8 +242,8 @@ fn write_memory_bytes( let slice = bytes.as_ref(); let len = slice.len(); check_bounds(memory, offset, len as _)?; - let offset = offset as usize; - memory.view()[offset..(offset + len)] + let offset = usize::try_from(offset)?; + memory.view()[offset..checked!(offset + len)?] .iter() .zip(slice.iter()) .for_each(|(cell, v)| cell.set(*v)); @@ -280,7 +287,8 @@ impl VmMemory for WasmMemory { fn read_bytes(&self, offset: u64, len: usize) -> Result<(Vec, u64)> { let memory = self.inner.get_ref().ok_or(Error::UninitializedMemory)?; let bytes = read_memory_bytes(memory, offset, len)?; - let gas = bytes.len() as u64 * MEMORY_ACCESS_GAS_PER_BYTE; + let len = bytes.len() as u64; + let gas = checked!(len * MEMORY_ACCESS_GAS_PER_BYTE)?; Ok((bytes, gas)) } @@ -289,7 +297,8 @@ impl VmMemory for WasmMemory { // No need for a separate gas multiplier for writes since we are only // writing to memory and we already charge gas for every memory page // allocated - let gas = bytes.as_ref().len() as u64 * MEMORY_ACCESS_GAS_PER_BYTE; + let len = bytes.as_ref().len() as u64; + let gas = checked!(len * MEMORY_ACCESS_GAS_PER_BYTE)?; let memory = self.inner.get_ref().ok_or(Error::UninitializedMemory)?; write_memory_bytes(memory, offset, bytes)?; Ok(gas) diff --git a/crates/namada/src/vm/wasm/run.rs b/crates/namada/src/vm/wasm/run.rs index ea73c78a965..a244be0b0d9 100644 --- a/crates/namada/src/vm/wasm/run.rs +++ b/crates/namada/src/vm/wasm/run.rs @@ -356,7 +356,7 @@ fn run_vp( verifiers: &BTreeSet
, yielded_value: MutHostRef<'_, &'_ Option>>, ) -> Result<()> { - let input: VpInput = VpInput { + let input: VpInput<'_> = VpInput { addr: address, data: input_data, keys_changed, diff --git a/crates/parameters/src/lib.rs b/crates/parameters/src/lib.rs index 3c3e1528671..158dc2410dd 100644 --- a/crates/parameters/src/lib.rs +++ b/crates/parameters/src/lib.rs @@ -1,4 +1,19 @@ //! Protocol parameters + +#![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 +)] + pub mod storage; mod wasm_allowlist; use std::collections::BTreeMap; diff --git a/crates/proof_of_stake/src/epoched.rs b/crates/proof_of_stake/src/epoched.rs index f4713717279..76ffb3d9c8d 100644 --- a/crates/proof_of_stake/src/epoched.rs +++ b/crates/proof_of_stake/src/epoched.rs @@ -121,7 +121,7 @@ where && epoch > Self::sub_past_epochs(params, last_update) { - epoch = Epoch(epoch.0 - 1); + epoch = epoch.prev().expect("Cannot underflow"); } else { return Ok(None); } @@ -867,7 +867,10 @@ impl EpochOffset for OffsetUnbondingLen { pub struct OffsetPipelinePlusUnbondingLen; impl EpochOffset for OffsetPipelinePlusUnbondingLen { fn value(params: &PosParams) -> u64 { - params.pipeline_len + params.unbonding_len + params + .pipeline_len + .checked_add(params.unbonding_len) + .expect("Params addition must not overflow") } fn dyn_offset() -> DynEpochOffset { @@ -915,7 +918,10 @@ impl EpochOffset for OffsetSlashProcessingLen { pub struct OffsetSlashProcessingLenPlus; impl EpochOffset for OffsetSlashProcessingLenPlus { fn value(params: &PosParams) -> u64 { - params.slash_processing_epoch_offset() + DEFAULT_NUM_PAST_EPOCHS + params + .slash_processing_epoch_offset() + .checked_add(DEFAULT_NUM_PAST_EPOCHS) + .expect("Params addition must not overflow") } fn dyn_offset() -> DynEpochOffset { @@ -987,7 +993,10 @@ impl EpochOffset for OffsetMaxProposalPeriod { pub struct OffsetMaxProposalPeriodPlus; impl EpochOffset for OffsetMaxProposalPeriodPlus { fn value(params: &PosParams) -> u64 { - params.max_proposal_period + DEFAULT_NUM_PAST_EPOCHS + params + .max_proposal_period + .checked_add(DEFAULT_NUM_PAST_EPOCHS) + .expect("Params addition must not overflow") } fn dyn_offset() -> DynEpochOffset { @@ -1043,7 +1052,9 @@ impl EpochOffset for OffsetMaxProposalPeriodOrSlashProcessingLenPlus { cmp::max( params.slash_processing_epoch_offset(), params.max_proposal_period, - ) + DEFAULT_NUM_PAST_EPOCHS + ) + .checked_add(DEFAULT_NUM_PAST_EPOCHS) + .expect("Params addition must not overflow") } fn dyn_offset() -> DynEpochOffset { diff --git a/crates/proof_of_stake/src/lib.rs b/crates/proof_of_stake/src/lib.rs index f919c2dadb9..42da174ba51 100644 --- a/crates/proof_of_stake/src/lib.rs +++ b/crates/proof_of_stake/src/lib.rs @@ -2,9 +2,17 @@ #![doc(html_favicon_url = "https://dev.namada.net/master/favicon.png")] #![doc(html_logo_url = "https://dev.namada.net/master/rustdoc-logo.png")] -#![warn(missing_docs)] #![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 +)] pub mod epoched; pub mod event; @@ -1978,7 +1986,9 @@ where let params = read_pos_params(storage)?; // Check that the validator is jailed up to the pipeline epoch - for epoch in current_epoch.iter_range(params.pipeline_len + 1) { + for epoch in current_epoch.iter_range( + params.pipeline_len.checked_add(1).expect("Cannot overflow"), + ) { let state = validator_state_handle(validator).get(storage, epoch, ¶ms)?; if let Some(state) = state { @@ -2510,10 +2520,13 @@ where if pruned_missing_vote { // Update liveness data - liveness_sum_missed_votes.update( + liveness_sum_missed_votes.try_update( storage, cons_validator.clone(), - |missed_votes| missed_votes.unwrap() - 1, + |missed_votes| { + checked!(missed_votes.unwrap_or_default() - 1) + .map_err(Into::into) + }, )?; } } @@ -2526,17 +2539,19 @@ where .insert(storage, votes_height.0)?; // Update liveness data - liveness_sum_missed_votes.update( + liveness_sum_missed_votes.try_update( storage, cons_validator, |missed_votes| { match missed_votes { - Some(missed_votes) => missed_votes + 1, + Some(missed_votes) => { + checked!(missed_votes + 1).map_err(Into::into) + } None => { // Missing liveness data for the validator (newly // added to the consensus // set), initialize it - 1 + Ok(1) } } }, @@ -2917,8 +2932,10 @@ where } } - // Safe sub cause `validator_set_update_epoch > current_epoch` - let start_offset = validator_set_update_epoch.0 - current_epoch.0; + let start_offset = validator_set_update_epoch + .0 + .checked_sub(current_epoch.0) + .expect("Safe sub cause `validator_set_update_epoch > current_epoch`"); // Set the validator state as `Jailed` thru the pipeline epoch for offset in start_offset..=params.pipeline_len { validator_state_handle(validator).set( diff --git a/crates/proof_of_stake/src/parameters.rs b/crates/proof_of_stake/src/parameters.rs index 345f97ef50c..2bee07d8f85 100644 --- a/crates/proof_of_stake/src/parameters.rs +++ b/crates/proof_of_stake/src/parameters.rs @@ -210,14 +210,18 @@ impl OwnedPosParams { /// Get the epoch offset from which an unbonded bond can withdrawn pub fn withdrawable_epoch_offset(&self) -> u64 { - self.pipeline_len - + self.unbonding_len - + self.cubic_slashing_window_length + checked!( + self.pipeline_len + + self.unbonding_len + + self.cubic_slashing_window_length + ) + .expect("Params addition must not overflow") } /// Get the epoch offset for processing slashes pub fn slash_processing_epoch_offset(&self) -> u64 { - self.unbonding_len + self.cubic_slashing_window_length + 1 + checked!(self.unbonding_len + self.cubic_slashing_window_length + 1) + .expect("Params addition must not overflow") } /// Get the first and the last epoch of a cubic slash window. @@ -315,6 +319,7 @@ mod tests { } /// Testing helpers +#[allow(clippy::arithmetic_side_effects)] #[cfg(any(test, feature = "testing"))] pub mod testing { use proptest::prelude::*; diff --git a/crates/proof_of_stake/src/queries.rs b/crates/proof_of_stake/src/queries.rs index d71c475e2ed..0926ed589d4 100644 --- a/crates/proof_of_stake/src/queries.rs +++ b/crates/proof_of_stake/src/queries.rs @@ -489,8 +489,10 @@ fn make_unbond_details( && slash.epoch < withdraw .checked_sub( - params.unbonding_len - + params.cubic_slashing_window_length, + params + .unbonding_len + .checked_add(params.cubic_slashing_window_length) + .expect("Cannot overflow"), ) .unwrap_or_default() { diff --git a/crates/proof_of_stake/src/tests/helpers.rs b/crates/proof_of_stake/src/tests/helpers.rs index a1b81c595aa..2589d388692 100644 --- a/crates/proof_of_stake/src/tests/helpers.rs +++ b/crates/proof_of_stake/src/tests/helpers.rs @@ -1,3 +1,5 @@ +#![allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)] + use std::cmp::max; use std::ops::Range; diff --git a/crates/proof_of_stake/src/tests/state_machine.rs b/crates/proof_of_stake/src/tests/state_machine.rs index 50871d62045..459da961cb8 100644 --- a/crates/proof_of_stake/src/tests/state_machine.rs +++ b/crates/proof_of_stake/src/tests/state_machine.rs @@ -1,5 +1,7 @@ //! Test PoS transitions with a state machine +#![allow(clippy::arithmetic_side_effects)] + use std::cmp; use std::collections::{BTreeMap, BTreeSet, VecDeque}; use std::ops::Deref; diff --git a/crates/proof_of_stake/src/tests/state_machine_v2.rs b/crates/proof_of_stake/src/tests/state_machine_v2.rs index 96ed54b48c6..4b8fb024e2a 100644 --- a/crates/proof_of_stake/src/tests/state_machine_v2.rs +++ b/crates/proof_of_stake/src/tests/state_machine_v2.rs @@ -1,5 +1,7 @@ //! Test PoS transitions with a state machine +#![allow(clippy::arithmetic_side_effects)] + use std::collections::{BTreeMap, BTreeSet, VecDeque}; use std::ops::{AddAssign, Deref}; use std::{cmp, mem}; diff --git a/crates/proof_of_stake/src/tests/test_helper_fns.rs b/crates/proof_of_stake/src/tests/test_helper_fns.rs index ef685b0a534..9d0d443f4a8 100644 --- a/crates/proof_of_stake/src/tests/test_helper_fns.rs +++ b/crates/proof_of_stake/src/tests/test_helper_fns.rs @@ -1,3 +1,5 @@ +#![allow(clippy::arithmetic_side_effects, clippy::cast_possible_truncation)] + use std::collections::{BTreeMap, BTreeSet}; use namada_core::address::testing::{ diff --git a/crates/proof_of_stake/src/tests/test_pos.rs b/crates/proof_of_stake/src/tests/test_pos.rs index 8aeffcee80b..8d853e60866 100644 --- a/crates/proof_of_stake/src/tests/test_pos.rs +++ b/crates/proof_of_stake/src/tests/test_pos.rs @@ -1,5 +1,7 @@ //! PoS system tests +#![allow(clippy::arithmetic_side_effects, clippy::cast_sign_loss)] + use std::collections::BTreeMap; use assert_matches::assert_matches; diff --git a/crates/proof_of_stake/src/tests/test_slash_and_redel.rs b/crates/proof_of_stake/src/tests/test_slash_and_redel.rs index 1d895c62b10..4f04e90945d 100644 --- a/crates/proof_of_stake/src/tests/test_slash_and_redel.rs +++ b/crates/proof_of_stake/src/tests/test_slash_and_redel.rs @@ -1,3 +1,5 @@ +#![allow(clippy::arithmetic_side_effects)] + use std::collections::BTreeMap; use std::ops::Deref; use std::str::FromStr; diff --git a/crates/proof_of_stake/src/tests/test_validator.rs b/crates/proof_of_stake/src/tests/test_validator.rs index 3b8de8bb316..b17750cd4bc 100644 --- a/crates/proof_of_stake/src/tests/test_validator.rs +++ b/crates/proof_of_stake/src/tests/test_validator.rs @@ -1,3 +1,5 @@ +#![allow(clippy::arithmetic_side_effects)] + use std::cmp::min; use namada_core::address::testing::arb_established_address; diff --git a/crates/proof_of_stake/src/tests/utils.rs b/crates/proof_of_stake/src/tests/utils.rs index 1e5f5acf629..1e7deac4f4c 100644 --- a/crates/proof_of_stake/src/tests/utils.rs +++ b/crates/proof_of_stake/src/tests/utils.rs @@ -1,3 +1,5 @@ +#![allow(clippy::cast_lossless, clippy::arithmetic_side_effects)] + use std::marker::PhantomData; use std::str::FromStr; use std::sync::atomic::AtomicUsize; diff --git a/crates/proof_of_stake/src/types/mod.rs b/crates/proof_of_stake/src/types/mod.rs index f216cc07eec..936c17507ab 100644 --- a/crates/proof_of_stake/src/types/mod.rs +++ b/crates/proof_of_stake/src/types/mod.rs @@ -6,7 +6,6 @@ use core::fmt::Debug; use std::collections::BTreeMap; use std::fmt::Display; use std::hash::Hash; -use std::ops::Sub; use borsh::{BorshDeserialize, BorshSchema, BorshSerialize}; use namada_core::address::Address; @@ -520,14 +519,6 @@ impl KeySeg for Position { } } -impl Sub for Position { - type Output = Self; - - fn sub(self, rhs: Position) -> Self::Output { - Position(self.0 - rhs.0) - } -} - impl Position { /// Position value of 1 pub const ONE: Position = Position(1_u64); @@ -536,6 +527,11 @@ impl Position { pub fn next(&self) -> Self { Self(self.0.wrapping_add(1)) } + + /// Checked subtraction + pub fn checked_sub(self, rhs: Self) -> Option { + Some(Self(self.0.checked_sub(rhs.0)?)) + } } /// Validator's state. diff --git a/crates/replay_protection/src/lib.rs b/crates/replay_protection/src/lib.rs index b72d069fd7a..a9011b35317 100644 --- a/crates/replay_protection/src/lib.rs +++ b/crates/replay_protection/src/lib.rs @@ -1,5 +1,19 @@ //! Replay protection storage keys +#![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 +)] + use namada_core::hash::Hash; use namada_core::storage::Key; diff --git a/crates/sdk/src/wallet/derivation_path.rs b/crates/sdk/src/wallet/derivation_path.rs index c6a365cf409..99207e53673 100644 --- a/crates/sdk/src/wallet/derivation_path.rs +++ b/crates/sdk/src/wallet/derivation_path.rs @@ -60,7 +60,7 @@ impl DerivationPath { } /// Check if the path is BIP-0044 conform - /// https://github.com/bitcoin/bips/blob/master/bip-0044.mediawiki#path-levels + /// pub fn is_bip44_conform(&self, strict: bool) -> bool { // check the path conforms the structure: // m / purpose' / coin_type' / account' / change / address_index @@ -90,7 +90,7 @@ impl DerivationPath { } /// Check if the path is SLIP-0010 conform - /// https://github.com/satoshilabs/slips/blob/master/slip-0010.md#child-key-derivation-ckd-functions + /// pub fn is_slip10_conform(&self, scheme: SchemeType) -> bool { match scheme { SchemeType::Ed25519 => { @@ -103,7 +103,7 @@ impl DerivationPath { } /// Check if the path is ZIP-0032 conform - /// https://zips.z.cash/zip-0032#sapling-key-path + /// pub fn is_zip32_conform(&self) -> bool { // check the path conforms one of the structure: // m / purpose' / coin_type' / account' diff --git a/crates/shielded_token/Cargo.toml b/crates/shielded_token/Cargo.toml index 8f4ef5a1e2e..c9d7618ab96 100644 --- a/crates/shielded_token/Cargo.toml +++ b/crates/shielded_token/Cargo.toml @@ -28,6 +28,7 @@ borsh.workspace = true masp_primitives.workspace = true rayon = { workspace = true, optional = true } serde.workspace = true +smooth-operator.workspace = true tracing.workspace = true [dev-dependencies] diff --git a/crates/shielded_token/src/conversion.rs b/crates/shielded_token/src/conversion.rs index 7c842ffc26f..b068aed0051 100644 --- a/crates/shielded_token/src/conversion.rs +++ b/crates/shielded_token/src/conversion.rs @@ -2,6 +2,7 @@ use namada_controller::PDController; use namada_core::address::{Address, MASP}; +use namada_core::arith::checked; #[cfg(any(feature = "multicore", test))] use namada_core::borsh::BorshSerializeExt; use namada_core::dec::Dec; @@ -81,10 +82,10 @@ where // the threshold of holdings required in order to receive non-zero rewards. // This value should be fixed constant for each asset type. Here we choose // a thousandth of the given asset. - Ok(( - 10u128.pow(std::cmp::max(u32::from(denomination.0), 3) - 3), - denomination, - )) + let precision_denom = std::cmp::max(u32::from(denomination.0), 3) + .checked_sub(3) + .expect("Cannot underflow"); + Ok((checked!(10u128 ^ precision_denom)?, denomination)) } /// Compute the MASP rewards by applying the PD-controller to the genesis @@ -181,8 +182,10 @@ where }) }; let inflation_amount = Amount::from_uint( - (total_tokens_in_masp.raw_amount() / precision) - * Uint::from(noterized_inflation), + checked!( + total_tokens_in_masp.raw_amount() / precision.into() + * Uint::from(noterized_inflation) + )?, 0, ) .unwrap(); @@ -233,6 +236,7 @@ where Ok(()) } +#[allow(clippy::arithmetic_side_effects)] #[cfg(any(feature = "multicore", test))] /// Update the MASP's allowed conversions pub fn update_allowed_conversions( @@ -385,12 +389,14 @@ where (token.clone(), denom, digit), (MaspAmount::from_pair( old_asset, - -(normed_inflation as i128), + -i128::try_from(normed_inflation) + .into_storage_result()?, ) .unwrap() + MaspAmount::from_pair( new_asset, - new_normed_inflation as i128, + i128::try_from(new_normed_inflation) + .into_storage_result()?, ) .unwrap()) .into(), @@ -448,15 +454,17 @@ where // The conversion is computed such that if consecutive // conversions are added together, the // intermediate tokens cancel/ telescope out + let reward_i128 = + i128::try_from(reward.1).into_storage_result()?; current_convs.insert( (token.clone(), denom, digit), - (MaspAmount::from_pair(old_asset, -(reward.1 as i128)) - .unwrap() - + MaspAmount::from_pair(new_asset, reward.1 as i128) + (MaspAmount::from_pair(old_asset, -reward_i128).unwrap() + + MaspAmount::from_pair(new_asset, reward_i128) .unwrap() + MaspAmount::from_pair( reward_assets[digit as usize], - real_reward as i128, + i128::try_from(real_reward) + .into_storage_result()?, ) .unwrap()) .into(), @@ -538,6 +546,7 @@ where // across multiple cores // Merkle trees must have exactly 2^n leaves to be mergeable let mut notes_per_thread_rounded = 1; + // Cannot overflow while notes_per_thread_max > notes_per_thread_rounded * 4 { notes_per_thread_rounded *= 2; } @@ -594,6 +603,7 @@ where Ok(()) } +#[allow(clippy::arithmetic_side_effects)] #[cfg(test)] mod tests { use std::str::FromStr; diff --git a/crates/shielded_token/src/lib.rs b/crates/shielded_token/src/lib.rs index 08c70719b8d..cc5b2f3548f 100644 --- a/crates/shielded_token/src/lib.rs +++ b/crates/shielded_token/src/lib.rs @@ -1,5 +1,19 @@ //! Namada shielded token. +#![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 +)] + pub mod conversion; mod storage; pub mod storage_key; diff --git a/crates/shielded_token/src/storage.rs b/crates/shielded_token/src/storage.rs index 0c975296ab7..0b91da75a9a 100644 --- a/crates/shielded_token/src/storage.rs +++ b/crates/shielded_token/src/storage.rs @@ -1,4 +1,5 @@ use namada_core::address::Address; +use namada_core::arith::checked; use namada_core::token; use namada_core::token::Amount; use namada_core::uint::Uint; @@ -31,8 +32,10 @@ where storage.write(&masp_kp_gain_key(address), kp_gain_nom)?; storage.write(&masp_kd_gain_key(address), kd_gain_nom)?; - let raw_target = Uint::from(*locked_amount_target) - * Uint::from(10).checked_pow(Uint::from(denom.0)).unwrap(); + let locked_amount_target = Uint::from(*locked_amount_target); + let raw_target = checked!( + locked_amount_target * (Uint::from(10) ^ Uint::from(denom.0)) + )?; let raw_target = Amount::from_uint(raw_target, 0).into_storage_result()?; storage.write(&masp_locked_amount_target_key(address), raw_target)?; Ok(()) diff --git a/crates/state/Cargo.toml b/crates/state/Cargo.toml index 7b587ca938f..5af49dd6675 100644 --- a/crates/state/Cargo.toml +++ b/crates/state/Cargo.toml @@ -42,6 +42,7 @@ ics23.workspace = true itertools.workspace = true linkme = {workspace = true, optional = true} sha2.workspace = true +smooth-operator.workspace = true thiserror.workspace = true tiny-keccak.workspace = true tracing.workspace = true diff --git a/crates/state/src/host_env.rs b/crates/state/src/host_env.rs index d270c11d190..6f97c66f4b2 100644 --- a/crates/state/src/host_env.rs +++ b/crates/state/src/host_env.rs @@ -8,7 +8,7 @@ use crate::in_memory::InMemory; use crate::write_log::WriteLog; use crate::{DBIter, Error, Result, State, StateRead, StorageHasher, DB}; -// State with mutable write log and gas metering for tx host env. +/// State with mutable write log and gas metering for tx host env. #[derive(Debug)] pub struct TxHostEnvState<'a, D, H> where @@ -17,7 +17,7 @@ where { /// Write log pub write_log: &'a mut WriteLog, - // DB + /// DB handle pub db: &'a D, /// State pub in_mem: &'a InMemory, @@ -27,7 +27,7 @@ where pub sentinel: &'a RefCell, } -// Read-only state with gas metering for VP host env. +/// Read-only state with gas metering for VP host env. #[derive(Debug)] pub struct VpHostEnvState<'a, D, H> where @@ -36,7 +36,7 @@ where { /// Write log pub write_log: &'a WriteLog, - // DB + /// DB handle pub db: &'a D, /// State pub in_mem: &'a InMemory, diff --git a/crates/state/src/in_memory.rs b/crates/state/src/in_memory.rs index 1487d879a1a..785f4d4ccae 100644 --- a/crates/state/src/in_memory.rs +++ b/crates/state/src/in_memory.rs @@ -176,12 +176,15 @@ where Ok(()) } + /// Store in memory a total gas of a transaction with the given hash. pub fn add_tx_gas(&mut self, tx_hash: Hash, gas: u64) { self.commit_only_data.tx_gas.insert(tx_hash, gas); } /// Get the chain ID as a raw string pub fn get_chain_id(&self) -> (String, u64) { + // Adding consts that cannot overflow + #[allow(clippy::arithmetic_side_effects)] ( self.chain_id.to_string(), CHAIN_ID_LENGTH as u64 * MEMORY_ACCESS_GAS_PER_BYTE, @@ -190,6 +193,8 @@ where /// Get the block height pub fn get_block_height(&self) -> (BlockHeight, u64) { + // Adding consts that cannot overflow + #[allow(clippy::arithmetic_side_effects)] ( self.block.height, BLOCK_HEIGHT_LENGTH as u64 * MEMORY_ACCESS_GAS_PER_BYTE, @@ -198,6 +203,8 @@ where /// Get the current (yet to be committed) block epoch pub fn get_current_epoch(&self) -> (Epoch, u64) { + // Adding consts that cannot overflow + #[allow(clippy::arithmetic_side_effects)] ( self.block.epoch, EPOCH_TYPE_LENGTH as u64 * MEMORY_ACCESS_GAS_PER_BYTE, @@ -206,6 +213,8 @@ where /// Get the epoch of the last committed block pub fn get_last_epoch(&self) -> (Epoch, u64) { + // Adding consts that cannot overflow + #[allow(clippy::arithmetic_side_effects)] ( self.last_epoch, EPOCH_TYPE_LENGTH as u64 * MEMORY_ACCESS_GAS_PER_BYTE, @@ -226,7 +235,11 @@ where self.next_epoch_min_start_height = initial_height .checked_add(min_num_of_blocks) .expect("Next epoch min block height shouldn't overflow"); - self.next_epoch_min_start_time = genesis_time + min_duration; + // Time must not overflow + #[allow(clippy::arithmetic_side_effects)] + { + self.next_epoch_min_start_time = genesis_time + min_duration; + } self.block.pred_epochs = Epochs { first_block_heights: vec![initial_height], }; @@ -277,9 +290,12 @@ where /// Get the oldest epoch where we can read a value pub fn get_oldest_epoch(&self) -> Epoch { let oldest_height = match self.storage_read_past_height_limit { - Some(limit) if limit < self.get_last_block_height().0 => { - (self.get_last_block_height().0 - limit).into() - } + Some(limit) if limit < self.get_last_block_height().0 => (self + .get_last_block_height() + .0 + .checked_sub(limit) + .expect("Cannot underflow")) + .into(), _ => BlockHeight(1), }; self.block diff --git a/crates/state/src/lib.rs b/crates/state/src/lib.rs index f3ae471c7c9..5e9e9ece1a2 100644 --- a/crates/state/src/lib.rs +++ b/crates/state/src/lib.rs @@ -1,5 +1,19 @@ //! Ledger's state storage with key-value backed store and a merkle tree +#![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 host_env; mod in_memory; mod wl_state; @@ -11,6 +25,7 @@ use std::iter::Peekable; pub use host_env::{TxHostEnvState, VpHostEnvState}; pub use in_memory::{BlockStorage, InMemory, LastBlock}; use namada_core::address::Address; +use namada_core::arith::{self, checked}; use namada_core::eth_bridge_pool::is_pending_transfer_key; pub use namada_core::hash::Sha256Hasher; use namada_core::hash::{Error as HashError, Hash}; @@ -65,14 +80,16 @@ pub trait StateRead: StorageRead + Debug { /// Borrow `InMemory` state fn in_mem(&self) -> &InMemory; + /// Try to charge a given gas amount. Returns an error on out-of-gas. fn charge_gas(&self, gas: u64) -> Result<()>; /// Check if the given key is present in storage. Returns the result and the /// gas cost. fn db_has_key(&self, key: &storage::Key) -> Result<(bool, u64)> { + let len = key.len() as u64; Ok(( self.db().read_subspace_val(key)?.is_some(), - key.len() as u64 * STORAGE_ACCESS_GAS_PER_BYTE, + checked!(len * STORAGE_ACCESS_GAS_PER_BYTE)?, )) } @@ -82,11 +99,15 @@ pub trait StateRead: StorageRead + Debug { match self.db().read_subspace_val(key)? { Some(v) => { - let gas = - (key.len() + v.len()) as u64 * STORAGE_ACCESS_GAS_PER_BYTE; + let len = checked!(key.len() + v.len())? as u64; + let gas = checked!(len * STORAGE_ACCESS_GAS_PER_BYTE)?; Ok((Some(v), gas)) } - None => Ok((None, key.len() as u64 * STORAGE_ACCESS_GAS_PER_BYTE)), + None => { + let len = key.len() as u64; + let gas = checked!(len * STORAGE_ACCESS_GAS_PER_BYTE)?; + Ok((None, gas)) + } } } @@ -98,11 +119,12 @@ pub trait StateRead: StorageRead + Debug { fn db_iter_prefix( &self, prefix: &Key, - ) -> (>::PrefixIter, u64) { - ( + ) -> Result<(>::PrefixIter, u64)> { + let len = prefix.len() as u64; + Ok(( self.db().iter_prefix(Some(prefix)), - prefix.len() as u64 * STORAGE_ACCESS_GAS_PER_BYTE, - ) + checked!(len * STORAGE_ACCESS_GAS_PER_BYTE)?, + )) } /// Returns an iterator over the block results @@ -141,7 +163,8 @@ pub trait StateRead: StorageRead + Debug { let header = self.in_mem().header.clone(); let gas = match header { Some(ref header) => { - header.encoded_len() as u64 * MEMORY_ACCESS_GAS_PER_BYTE + let len = header.encoded_len() as u64; + checked!(len * MEMORY_ACCESS_GAS_PER_BYTE)? } None => MEMORY_ACCESS_GAS_PER_BYTE, }; @@ -149,8 +172,8 @@ pub trait StateRead: StorageRead + Debug { } Some(h) => match self.db().read_block_header(h)? { Some(header) => { - let gas = header.encoded_len() as u64 - * STORAGE_ACCESS_GAS_PER_BYTE; + let len = header.encoded_len() as u64; + let gas = checked!(len * STORAGE_ACCESS_GAS_PER_BYTE)?; Ok((Some(header), gas)) } None => Ok((None, STORAGE_ACCESS_GAS_PER_BYTE)), @@ -179,6 +202,8 @@ pub trait State: StateRead + StorageWrite { } } +/// Implement [`trait StorageRead`] using its [`trait StateRead`] +/// implementation. #[macro_export] macro_rules! impl_storage_read { ($($type:ty)*) => { @@ -194,7 +219,7 @@ macro_rules! impl_storage_read { key: &storage::Key, ) -> namada_storage::Result>> { // try to read from the write log first - let (log_val, gas) = self.write_log().read(key); + let (log_val, gas) = self.write_log().read(key)?; self.charge_gas(gas).into_storage_result()?; match log_val { Some(write_log::StorageModification::Write { value }) => { @@ -215,7 +240,7 @@ macro_rules! impl_storage_read { fn has_key(&self, key: &storage::Key) -> namada_storage::Result { // try to read from the write log first - let (log_val, gas) = self.write_log().read(key); + let (log_val, gas) = self.write_log().read(key)?; self.charge_gas(gas).into_storage_result()?; match log_val { Some(&write_log::StorageModification::Write { .. }) @@ -238,7 +263,7 @@ macro_rules! impl_storage_read { prefix: &storage::Key, ) -> namada_storage::Result> { let (iter, gas) = - iter_prefix_post(self.write_log(), self.db(), prefix); + iter_prefix_post(self.write_log(), self.db(), prefix)?; self.charge_gas(gas).into_storage_result()?; Ok(iter) } @@ -314,6 +339,7 @@ macro_rules! impl_storage_read { } } +/// Implement [`trait StorageWrite`] using its [`trait State`] implementation. #[macro_export] macro_rules! impl_storage_write { ($($type:ty)*) => { @@ -390,10 +416,6 @@ impl_storage_read!(TxHostEnvState<'_, D, H>); impl_storage_read!(VpHostEnvState<'_, D, H>); impl_storage_write!(TxHostEnvState<'_, D, H>); -pub fn merklize_all_keys(_key: &storage::Key) -> bool { - true -} - #[allow(missing_docs)] #[derive(Error, Debug)] pub enum Error { @@ -419,6 +441,8 @@ pub enum Error { Gas(namada_gas::Error), #[error("{0}")] StorageError(#[from] namada_storage::Error), + #[error("Arithmetic {0}")] + Arith(#[from] arith::Error), } impl From for Error { @@ -448,19 +472,20 @@ pub fn iter_prefix_pre<'a, D>( write_log: &'a WriteLog, db: &'a D, prefix: &storage::Key, -) -> (PrefixIter<'a, D>, u64) +) -> namada_storage::Result<(PrefixIter<'a, D>, u64)> where D: DB + for<'iter> DBIter<'iter>, { let storage_iter = db.iter_prefix(Some(prefix)).peekable(); let write_log_iter = write_log.iter_prefix_pre(prefix).peekable(); - ( + let len = prefix.len() as u64; + Ok(( PrefixIter:: { storage_iter, write_log_iter, }, - prefix.len() as u64 * namada_gas::STORAGE_ACCESS_GAS_PER_BYTE, - ) + checked!(len * STORAGE_ACCESS_GAS_PER_BYTE)?, + )) } /// Iterate write-log storage items posterior to a tx execution, matching the @@ -472,19 +497,20 @@ pub fn iter_prefix_post<'a, D>( write_log: &'a WriteLog, db: &'a D, prefix: &storage::Key, -) -> (PrefixIter<'a, D>, u64) +) -> namada_storage::Result<(PrefixIter<'a, D>, u64)> where D: DB + for<'iter> DBIter<'iter>, { let storage_iter = db.iter_prefix(Some(prefix)).peekable(); let write_log_iter = write_log.iter_prefix_post(prefix).peekable(); - ( + let len = prefix.len() as u64; + Ok(( PrefixIter:: { storage_iter, write_log_iter, }, - prefix.len() as u64 * namada_gas::STORAGE_ACCESS_GAS_PER_BYTE, - ) + checked!(len * STORAGE_ACCESS_GAS_PER_BYTE)?, + )) } impl<'iter, D> Iterator for PrefixIter<'iter, D> @@ -574,6 +600,7 @@ pub mod testing { use super::mockdb::MockDB; use super::*; + /// A full-access state with a `MockDB` nd sha256 hasher. pub type TestState = FullAccessState; impl Default for TestState { @@ -587,6 +614,10 @@ pub mod testing { } } + fn merklize_all_keys(_key: &storage::Key) -> bool { + true + } + /// In memory State for testing. pub type InMemoryState = InMemory; @@ -627,6 +658,11 @@ pub mod testing { } } +#[allow( + clippy::arithmetic_side_effects, + clippy::cast_sign_loss, + clippy::cast_possible_wrap +)] #[cfg(test)] mod tests { use std::collections::BTreeMap; @@ -1087,7 +1123,8 @@ mod tests { // Collect the values from prior state prefix iterator let (iter_pre, _gas) = - iter_prefix_pre(s.write_log(), s.db(), &storage::Key::default()); + iter_prefix_pre(s.write_log(), s.db(), &storage::Key::default()) + .unwrap(); let mut read_pre = BTreeMap::new(); for (key, val, _gas) in iter_pre { let key = storage::Key::parse(key).unwrap(); @@ -1127,7 +1164,8 @@ mod tests { // Collect the values from posterior state prefix iterator let (iter_post, _gas) = - iter_prefix_post(s.write_log(), s.db(), &storage::Key::default()); + iter_prefix_post(s.write_log(), s.db(), &storage::Key::default()) + .unwrap(); let mut read_post = BTreeMap::new(); for (key, val, _gas) in iter_post { let key = storage::Key::parse(key).unwrap(); diff --git a/crates/state/src/wl_state.rs b/crates/state/src/wl_state.rs index 85607ef0b58..ad76bd020ea 100644 --- a/crates/state/src/wl_state.rs +++ b/crates/state/src/wl_state.rs @@ -2,6 +2,7 @@ use std::cmp::Ordering; use std::ops::{Deref, DerefMut}; use namada_core::address::Address; +use namada_core::arith::checked; use namada_core::borsh::BorshSerializeExt; use namada_core::chain::ChainId; use namada_core::storage; @@ -68,26 +69,32 @@ where D: 'static + DB + for<'iter> DBIter<'iter>, H: 'static + StorageHasher, { + /// Mutably borrow write-log pub fn write_log_mut(&mut self) -> &mut WriteLog { &mut self.0.write_log } + /// Mutably borrow in-memory state pub fn in_mem_mut(&mut self) -> &mut InMemory { &mut self.0.in_mem } + /// Mutably borrow DB handle pub fn db_mut(&mut self) -> &mut D { &mut self.0.db } + /// Borrow state with mutable write-log. pub fn restrict_writes_to_write_log(&mut self) -> &mut WlState { &mut self.0 } + /// Borrow read-only write-log and state pub fn read_only(&self) -> &WlState { &self.0 } + /// Instantiate a full-access state. Loads the last state from a DB, if any. pub fn open( db_path: impl AsRef, cache: Option<&D::Cache>, @@ -147,7 +154,7 @@ where } } Some(blocks_until_switch) => { - *blocks_until_switch -= 1; + *blocks_until_switch = checked!(blocks_until_switch - 1)?; } }; let new_epoch = @@ -166,7 +173,11 @@ where self.in_mem.next_epoch_min_start_height = height .checked_add(min_num_of_blocks) .expect("Next epoch min block height shouldn't overflow"); - self.in_mem.next_epoch_min_start_time = time + min_duration; + // Time must not overflow + #[allow(clippy::arithmetic_side_effects)] + { + self.in_mem.next_epoch_min_start_time = time + min_duration; + } self.in_mem.block.pred_epochs.new_epoch(height); tracing::info!("Began a new epoch {}", self.in_mem.block.epoch); @@ -470,6 +481,7 @@ where } } + /// Commit the data from in-memory state into the block's merkle tree. pub fn commit_only_data(&mut self) -> Result<()> { let data = self.in_mem().commit_only_data.serialize(); self.in_mem_mut() @@ -560,27 +572,33 @@ where D: 'static + DB + for<'iter> DBIter<'iter>, H: 'static + StorageHasher, { + /// Borrow write-log pub fn write_log(&self) -> &WriteLog { &self.write_log } + /// Borrow in-memory state pub fn in_mem(&self) -> &InMemory { &self.in_mem } + /// Mutably borrow in-memory state pub fn in_mem_mut(&mut self) -> &mut InMemory { &mut self.in_mem } + /// Borrow DB handle pub fn db(&self) -> &D { // NOTE: `WlState` must not be allowed mutable access to DB &self.db } + /// Mutably borrow write-log pub fn write_log_mut(&mut self) -> &mut WriteLog { &mut self.write_log } + /// Borrow in-memory state and DB handle with a mutable temporary write-log. pub fn with_temp_write_log(&self) -> TempWlState<'_, D, H> { TempWlState { write_log: WriteLog::default(), @@ -611,6 +629,7 @@ where self.write_log.redundant_tx_hash(hash) } + /// Get the height of the next block #[inline] pub fn get_current_decision_height(&self) -> BlockHeight { self.in_mem @@ -660,12 +679,12 @@ where self.in_mem().get_last_block_height(), )? { Some(v) => { - let gas = (key.len() + v.len()) as u64 - * STORAGE_ACCESS_GAS_PER_BYTE; - Ok((Some(v), gas)) + let gas = checked!(key.len() + v.len())? as u64; + Ok((Some(v), checked!(gas * STORAGE_ACCESS_GAS_PER_BYTE)?)) } None => { - Ok((None, key.len() as u64 * STORAGE_ACCESS_GAS_PER_BYTE)) + let gas = key.len() as u64; + Ok((None, checked!(gas * STORAGE_ACCESS_GAS_PER_BYTE)?)) } } } @@ -673,6 +692,7 @@ where /// Write a value to the specified subspace and returns the gas cost and the /// size difference + #[allow(clippy::arithmetic_side_effects)] #[cfg(any(test, feature = "testing", feature = "benches"))] pub fn db_write( &mut self, @@ -711,6 +731,11 @@ where /// Delete the specified subspace and returns the gas cost and the size /// difference + #[allow( + clippy::cast_sign_loss, + clippy::arithmetic_side_effects, + clippy::cast_possible_truncation + )] #[cfg(any(test, feature = "testing", feature = "benches"))] pub fn db_delete(&mut self, key: &Key) -> Result<(u64, i64)> { // Note that this method is the same as `StorageWrite::delete`, @@ -741,7 +766,7 @@ where pub fn get_existence_proof( &self, key: &Key, - value: namada_merkle_tree::StorageBytes, + value: namada_merkle_tree::StorageBytes<'_>, height: BlockHeight, ) -> Result { use std::array; @@ -1009,18 +1034,22 @@ where D: 'static + DB + for<'iter> DBIter<'iter>, H: 'static + StorageHasher, { + /// Borrow write-log pub fn write_log(&self) -> &WriteLog { &self.write_log } + /// Borrow in-memory state pub fn in_mem(&self) -> &InMemory { self.in_mem } + /// Borrow DB handle pub fn db(&self) -> &D { self.db } + /// Mutably borrow write-log pub fn write_log_mut(&mut self) -> &mut WriteLog { &mut self.write_log } @@ -1266,7 +1295,7 @@ where &self, key: &storage::Key, ) -> Result> { - let (log_val, _) = self.write_log().read_temp(key); + let (log_val, _) = self.write_log().read_temp(key).unwrap(); match log_val { Some(value) => { let value = diff --git a/crates/state/src/write_log.rs b/crates/state/src/write_log.rs index 8c904af99a4..3ba12e24286 100644 --- a/crates/state/src/write_log.rs +++ b/crates/state/src/write_log.rs @@ -5,9 +5,10 @@ use std::collections::{BTreeMap, BTreeSet}; use itertools::Itertools; use namada_core::address::{Address, EstablishedAddressGen}; +use namada_core::arith::checked; use namada_core::collections::{HashMap, HashSet}; use namada_core::hash::Hash; -use namada_core::storage; +use namada_core::{arith, storage}; use namada_events::{Event, EventToEmit, EventType}; use namada_gas::{MEMORY_ACCESS_GAS_PER_BYTE, STORAGE_WRITE_GAS_PER_BYTE}; use patricia_tree::map::StringPatriciaMap; @@ -33,6 +34,12 @@ pub enum Error { WriteTempAfterWrite, #[error("Replay protection key: {0}")] ReplayProtection(String), + #[error("Arithmetic {0}")] + Arith(#[from] arith::Error), + #[error("Sized-diff overflowed")] + SizeDiffOverflow, + #[error("Value length overflowed")] + ValueLenOverflow, } /// Result for functions that may fail @@ -149,7 +156,8 @@ impl WriteLog { pub fn read( &self, key: &storage::Key, - ) -> (Option<&StorageModification>, u64) { + ) -> std::result::Result<(Option<&StorageModification>, u64), arith::Error> + { // try to read from tx write log first match self .tx_write_log @@ -165,16 +173,19 @@ impl WriteLog { Some(v) => { let gas = match v { StorageModification::Write { ref value } => { - key.len() + value.len() + checked!(key.len() + value.len())? } StorageModification::Delete => key.len(), StorageModification::InitAccount { ref vp_code_hash } => { - key.len() + vp_code_hash.len() + checked!(key.len() + vp_code_hash.len())? } - }; - (Some(v), gas as u64 * MEMORY_ACCESS_GAS_PER_BYTE) + } as u64; + Ok((Some(v), checked!(gas * MEMORY_ACCESS_GAS_PER_BYTE)?)) + } + None => { + let gas = key.len() as u64; + Ok((None, checked!(gas * MEMORY_ACCESS_GAS_PER_BYTE)?)) } - None => (None, key.len() as u64 * MEMORY_ACCESS_GAS_PER_BYTE), } } @@ -184,36 +195,46 @@ impl WriteLog { pub fn read_pre( &self, key: &storage::Key, - ) -> (Option<&StorageModification>, u64) { + ) -> std::result::Result<(Option<&StorageModification>, u64), arith::Error> + { match self.block_write_log.get(key) { Some(v) => { let gas = match v { StorageModification::Write { ref value } => { - key.len() + value.len() + checked!(key.len() + value.len())? } StorageModification::Delete => key.len(), StorageModification::InitAccount { ref vp_code_hash } => { - key.len() + vp_code_hash.len() + checked!(key.len() + vp_code_hash.len())? } - }; - (Some(v), gas as u64 * MEMORY_ACCESS_GAS_PER_BYTE) + } as u64; + Ok((Some(v), checked!(gas * MEMORY_ACCESS_GAS_PER_BYTE)?)) + } + None => { + let gas = key.len() as u64; + Ok((None, checked!(gas * MEMORY_ACCESS_GAS_PER_BYTE)?)) } - None => (None, key.len() as u64 * MEMORY_ACCESS_GAS_PER_BYTE), } } /// Read a temp value at the given key and return the value and the gas /// cost, returns [`None`] if the key is not present in the temp write /// log - pub fn read_temp(&self, key: &storage::Key) -> (Option<&Vec>, u64) { + pub fn read_temp( + &self, + key: &storage::Key, + ) -> Result<(Option<&Vec>, u64)> { // try to read from tx write log first match self.tx_temp_log.get(key) { Some(value) => { - let gas = key.len() + value.len(); + let gas = checked!(key.len() + value.len())? as u64; - (Some(value), gas as u64 * MEMORY_ACCESS_GAS_PER_BYTE) + Ok((Some(value), checked!(gas * MEMORY_ACCESS_GAS_PER_BYTE)?)) + } + None => { + let gas = key.len() as u64; + Ok((None, checked!(gas * MEMORY_ACCESS_GAS_PER_BYTE)?)) } - None => (None, key.len() as u64 * MEMORY_ACCESS_GAS_PER_BYTE), } } @@ -228,10 +249,11 @@ impl WriteLog { value: Vec, ) -> Result<(u64, i64)> { let len = value.len(); - let gas = key.len() + len; if self.tx_temp_log.contains_key(key) { return Err(Error::UpdateTemporaryValue); } + let len_signed = + i64::try_from(len).map_err(|_| Error::ValueLenOverflow)?; let size_diff = match self .tx_write_log .get(key) @@ -239,9 +261,11 @@ impl WriteLog { { Some(prev) => match prev { StorageModification::Write { ref value } => { - len as i64 - value.len() as i64 + let val_len = i64::try_from(value.len()) + .map_err(|_| Error::ValueLenOverflow)?; + checked!(len_signed - val_len)? } - StorageModification::Delete => len as i64, + StorageModification::Delete => len_signed, StorageModification::InitAccount { .. } => { // NOTE: errors from host functions force a shudown of the // wasm environment without the need for cooperation from @@ -253,13 +277,14 @@ impl WriteLog { }, // set just the length of the value because we don't know if // the previous value exists on the storage - None => len as i64, + None => len_signed, }; self.tx_write_log .insert(key.clone(), StorageModification::Write { value }); - Ok((gas as u64 * STORAGE_WRITE_GAS_PER_BYTE, size_diff)) + let gas = checked!(key.len() + len)? as u64; + Ok((checked!(gas * STORAGE_WRITE_GAS_PER_BYTE)?, size_diff)) } /// Write a key and a value. @@ -322,19 +347,25 @@ impl WriteLog { } let len = value.len(); - let gas = key.len() + len; + let len_signed = + i64::try_from(len).map_err(|_| Error::ValueLenOverflow)?; let size_diff = match self.tx_temp_log.get(key) { - Some(prev) => len as i64 - prev.len() as i64, + Some(prev) => { + let prev_len = i64::try_from(prev.len()) + .map_err(|_| Error::ValueLenOverflow)?; + checked!(len_signed - prev_len)? + } // set just the length of the value because we don't know if // the previous value exists on the storage - None => len as i64, + None => len_signed, }; self.tx_temp_log.insert(key.clone(), value); // Temp writes are not propagated to db so just charge the cost of // accessing storage - Ok((gas as u64 * MEMORY_ACCESS_GAS_PER_BYTE, size_diff)) + let gas = checked!(key.len() + len)? as u64; + Ok((checked!(gas * MEMORY_ACCESS_GAS_PER_BYTE)?, size_diff)) } /// Delete a key and its value, and return the gas cost and the size @@ -351,7 +382,7 @@ impl WriteLog { .or_else(|| self.tx_precommit_write_log.get(key)) { Some(prev) => match prev { - StorageModification::Write { ref value } => value.len() as i64, + StorageModification::Write { ref value } => value.len(), StorageModification::Delete => 0, StorageModification::InitAccount { .. } => { return Err(Error::DeleteVp); @@ -364,8 +395,12 @@ impl WriteLog { self.tx_write_log .insert(key.clone(), StorageModification::Delete); - let gas = key.len() + size_diff as usize; - Ok((gas as u64 * STORAGE_WRITE_GAS_PER_BYTE, -size_diff)) + let gas = checked!(key.len() + size_diff)? as u64; + let size_diff = i64::try_from(size_diff) + .ok() + .and_then(i64::checked_neg) + .ok_or(Error::SizeDiffOverflow)?; + Ok((checked!(gas * STORAGE_WRITE_GAS_PER_BYTE)?, size_diff)) } /// Delete a key and its value. @@ -404,22 +439,29 @@ impl WriteLog { .get_or_insert_with(|| storage_address_gen.clone()); let addr = address_gen.generate_address(entropy_source); let key = storage::Key::validity_predicate(&addr); - let gas = (key.len() + vp_code_hash.len()) as u64 - * STORAGE_WRITE_GAS_PER_BYTE; + let gas = ((key + .len() + .checked_add(vp_code_hash.len()) + .expect("Cannot overflow")) as u64) + .checked_mul(STORAGE_WRITE_GAS_PER_BYTE) + .expect("Canno overflow"); self.tx_write_log .insert(key, StorageModification::InitAccount { vp_code_hash }); (addr, gas) } - /// Set an event and return the gas cost. - pub fn emit_event(&mut self, event: E) -> u64 { + /// Set an event and return the gas cost. Returns `None` on gas u64 + /// overflow. + pub fn emit_event(&mut self, event: E) -> Option { let event = event.into(); let gas_cost = event.emission_gas_cost(MEMORY_ACCESS_GAS_PER_BYTE); - let event_type = event.kind().to_string(); - if !self.events.tree.contains_key(&event_type) { - self.events.tree.insert(&event_type, HashSet::new()); + if gas_cost.as_ref().is_some() { + let event_type = event.kind().to_string(); + if !self.events.tree.contains_key(&event_type) { + self.events.tree.insert(&event_type, HashSet::new()); + } + self.events.tree.get_mut(&event_type).unwrap().insert(event); } - self.events.tree.get_mut(&event_type).unwrap().insert(event); gas_cost } @@ -665,6 +707,7 @@ impl WriteLog { } } +#[allow(clippy::cast_possible_wrap)] #[cfg(test)] mod tests { use assert_matches::assert_matches; @@ -682,7 +725,7 @@ mod tests { storage::Key::parse("key").expect("cannot parse the key string"); // read a non-existing key - let (value, gas) = write_log.read(&key); + let (value, gas) = write_log.read(&key).unwrap(); assert!(value.is_none()); assert_eq!(gas, (key.len() as u64) * MEMORY_ACCESS_GAS_PER_BYTE); @@ -701,7 +744,7 @@ mod tests { assert_eq!(diff, inserted.len() as i64); // read the value - let (value, gas) = write_log.read(&key); + let (value, gas) = write_log.read(&key).unwrap(); match value.expect("no read value") { StorageModification::Write { value } => { assert_eq!(*value, inserted) @@ -736,7 +779,7 @@ mod tests { assert_eq!(diff, 0); // read the deleted key - let (value, gas) = write_log.read(&key); + let (value, gas) = write_log.read(&key).unwrap(); match &value.expect("no read value") { StorageModification::Delete => {} _ => panic!("unexpected result"), @@ -769,7 +812,7 @@ mod tests { ); // read - let (value, gas) = write_log.read(&vp_key); + let (value, gas) = write_log.read(&vp_key).unwrap(); match value.expect("no read value") { StorageModification::InitAccount { vp_code_hash } => { assert_eq!(*vp_code_hash, vp_hash) diff --git a/crates/storage/Cargo.toml b/crates/storage/Cargo.toml index cfb60ac1cec..ca0f5e4c508 100644 --- a/crates/storage/Cargo.toml +++ b/crates/storage/Cargo.toml @@ -35,6 +35,7 @@ borsh.workspace = true itertools.workspace = true linkme = {workspace = true, optional = true} serde.workspace = true +smooth-operator.workspace = true thiserror.workspace = true tracing.workspace = true regex = "1.10.2" diff --git a/crates/storage/src/collections/lazy_vec.rs b/crates/storage/src/collections/lazy_vec.rs index 2826620fd7a..92c2534463c 100644 --- a/crates/storage/src/collections/lazy_vec.rs +++ b/crates/storage/src/collections/lazy_vec.rs @@ -3,6 +3,7 @@ use std::fmt::Debug; use std::marker::PhantomData; +use namada_core::arith::checked; use namada_core::borsh::{BorshDeserialize, BorshSerialize}; use namada_core::storage::{self, DbKeySeg, KeySeg}; use thiserror::Error; @@ -193,7 +194,7 @@ where let len = self.len(storage)?; let data_key = self.get_data_key(len); storage.write(&data_key, val)?; - storage.write(&self.get_len_key(), len + 1) + storage.write(&self.get_len_key(), checked!(len + 1)?) } /// Removes the last element from a vector and returns it, or `Ok(None)` if @@ -208,7 +209,7 @@ where if len == 0 { Ok(None) } else { - let index = len - 1; + let index = checked!(len - 1)?; let data_key = self.get_data_key(index); if len == 1 { storage.delete(&self.get_len_key())?; @@ -260,7 +261,7 @@ where S: StorageRead, { let len = self.len(storage)?; - self.get(storage, len - 1) + self.get(storage, checked!(len - 1)?) } /// An iterator visiting all elements. The iterator element type is diff --git a/crates/storage/src/db.rs b/crates/storage/src/db.rs index 0491cdb58cf..afaec5d2309 100644 --- a/crates/storage/src/db.rs +++ b/crates/storage/src/db.rs @@ -1,4 +1,5 @@ use std::fmt::Debug; +use std::num::TryFromIntError; use namada_core::address::EstablishedAddressGen; use namada_core::hash::{Error as HashError, Hash}; @@ -7,7 +8,7 @@ use namada_core::storage::{ Key, }; use namada_core::time::DateTimeUtc; -use namada_core::{ethereum_events, ethereum_structs}; +use namada_core::{arith, ethereum_events, ethereum_structs}; use namada_merkle_tree::{ Error as MerkleTreeError, MerkleTreeStoresRead, MerkleTreeStoresWrite, StoreType, @@ -39,6 +40,10 @@ pub enum Error { NoMerkleTree { height: BlockHeight }, #[error("Code hash error: {0}")] InvalidCodeHash(HashError), + #[error("Numeric conversion error: {0}")] + NumConversionError(#[from] TryFromIntError), + #[error("Arithmetic {0}")] + Arith(#[from] arith::Error), } /// A result of a function that may fail @@ -133,7 +138,7 @@ pub trait DB: Debug { /// `is_full_commit` is `true` (typically on a beginning of a new epoch). fn add_block_to_batch( &self, - state: BlockStateWrite, + state: BlockStateWrite<'_>, batch: &mut Self::WriteBatch, is_full_commit: bool, ) -> Result<()>; @@ -276,8 +281,9 @@ pub trait DB: Debug { /// A database prefix iterator. pub trait DBIter<'iter> { - /// The concrete type of the iterator + /// Prefix iterator type PrefixIter: Debug + Iterator, u64)>; + /// Pattern iterator type PatternIter: Debug + Iterator, u64)>; /// WARNING: This only works for values that have been committed to DB. diff --git a/crates/storage/src/lib.rs b/crates/storage/src/lib.rs index 674e7062432..3427f4cc893 100644 --- a/crates/storage/src/lib.rs +++ b/crates/storage/src/lib.rs @@ -1,6 +1,20 @@ //! The common storage read trait is implemented in the storage, client RPC, tx //! and VPs (both native and WASM). +#![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 +)] + pub mod collections; pub mod conversion_state; mod db; diff --git a/crates/storage/src/mockdb.rs b/crates/storage/src/mockdb.rs index a5c484f4ff1..101829f434b 100644 --- a/crates/storage/src/mockdb.rs +++ b/crates/storage/src/mockdb.rs @@ -1,5 +1,7 @@ //! DB mock for testing +#![allow(clippy::cast_possible_wrap, clippy::arithmetic_side_effects)] + use std::cell::RefCell; use std::collections::{btree_map, BTreeMap}; use std::path::Path; @@ -193,7 +195,7 @@ impl DB for MockDB { fn add_block_to_batch( &self, - state: BlockStateWrite, + state: BlockStateWrite<'_>, _batch: &mut Self::WriteBatch, is_full_commit: bool, ) -> Result<()> { @@ -213,7 +215,7 @@ impl DB for MockDB { ethereum_height, eth_events_queue, commit_only_data, - }: BlockStateWrite = state; + }: BlockStateWrite<'_> = state; self.write_value( NEXT_EPOCH_MIN_START_HEIGHT_KEY, @@ -777,6 +779,7 @@ impl Iterator for PrefixIterator { } } +/// MockDB pattern iterator #[derive(Debug)] pub struct MockPatternIterator { inner: PatternIterator, diff --git a/crates/storage/src/tx_queue.rs b/crates/storage/src/tx_queue.rs index 2b11cf4edbe..1ee71134f6a 100644 --- a/crates/storage/src/tx_queue.rs +++ b/crates/storage/src/tx_queue.rs @@ -1,3 +1,5 @@ +//! Transaction queue + use namada_core::borsh::{BorshDeserialize, BorshSerialize}; use namada_core::ethereum_events::EthereumEvent; use namada_macros::BorshDeserializer; diff --git a/crates/storage/src/types.rs b/crates/storage/src/types.rs index eaf7fac6de1..e5f323bf09d 100644 --- a/crates/storage/src/types.rs +++ b/crates/storage/src/types.rs @@ -67,10 +67,12 @@ impl std::fmt::Debug for PatternIterator { /// Structure holding data that will be committed to the merkle tree #[derive(Debug, BorshSerialize, BorshDeserialize, Default)] pub struct CommitOnlyData { + /// Map from tx hashes to their gas cost pub tx_gas: BTreeMap, } impl CommitOnlyData { + /// Serialize to bytes pub fn serialize(&self) -> Vec { self.serialize_to_vec() } diff --git a/crates/token/src/lib.rs b/crates/token/src/lib.rs index d39a9a18a2f..213fdf24339 100644 --- a/crates/token/src/lib.rs +++ b/crates/token/src/lib.rs @@ -1,9 +1,24 @@ //! Namada transparent and shielded token types, storage keys and storage //! fns. +#![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 +)] + pub use namada_shielded_token::*; pub use namada_trans_token::*; +/// Token storage keys pub mod storage_key { pub use namada_shielded_token::storage_key::*; pub use namada_trans_token::storage_key::*; @@ -32,6 +47,7 @@ where Ok(()) } +/// Apply token logic for finalizing block (i.e. shielded token rewards) pub fn finalize_block( storage: &mut S, _events: &mut impl EmitEvents, diff --git a/crates/trans_token/src/lib.rs b/crates/trans_token/src/lib.rs index 04b80b152ce..dd3ead6e309 100644 --- a/crates/trans_token/src/lib.rs +++ b/crates/trans_token/src/lib.rs @@ -1,5 +1,19 @@ //! Transparent token types, storage functions, and validation. +#![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 +)] + pub mod event; mod storage; pub mod storage_key; diff --git a/crates/tx/src/action.rs b/crates/tx/src/action.rs index 1d7377e87ca..c61575305f9 100644 --- a/crates/tx/src/action.rs +++ b/crates/tx/src/action.rs @@ -19,6 +19,7 @@ pub use crate::data::pos::{ pub type Actions = Vec; /// An action applied from a tx. +#[allow(missing_docs)] #[derive(Clone, Debug, BorshDeserialize, BorshSerialize)] pub enum Action { Pos(PosAction), @@ -27,6 +28,7 @@ pub enum Action { } /// PoS tx actions. +#[allow(missing_docs)] #[derive(Clone, Debug, BorshDeserialize, BorshSerialize)] pub enum PosAction { BecomeValidator(Address), @@ -44,6 +46,7 @@ pub enum PosAction { } /// Gov tx actions. +#[allow(missing_docs)] #[derive(Clone, Debug, BorshDeserialize, BorshSerialize)] pub enum GovAction { InitProposal { author: Address }, @@ -51,6 +54,7 @@ pub enum GovAction { } /// PGF tx actions. +#[allow(missing_docs)] #[derive(Clone, Debug, BorshDeserialize, BorshSerialize)] pub enum PgfAction { ResignSteward(Address), @@ -62,6 +66,7 @@ pub trait Read { /// Storage access errors type Err: fmt::Debug; + /// Read a temporary key-val fn read_temp( &self, key: &storage::Key, @@ -78,6 +83,7 @@ pub trait Read { /// Write actions to temporary storage pub trait Write: Read { + /// Write a temporary key-val fn write_temp( &mut self, key: &storage::Key, diff --git a/crates/tx/src/data/eval_vp.rs b/crates/tx/src/data/eval_vp.rs index 7011a4ab1f4..68a77e21524 100644 --- a/crates/tx/src/data/eval_vp.rs +++ b/crates/tx/src/data/eval_vp.rs @@ -1,3 +1,5 @@ +//! Tx data to evaluate a validity-predicate + use namada_core::borsh::{BorshDeserialize, BorshSerialize}; use namada_core::hash::Hash; use namada_macros::BorshDeserializer; diff --git a/crates/tx/src/data/mod.rs b/crates/tx/src/data/mod.rs index de7c6c75bdd..fc782f30606 100644 --- a/crates/tx/src/data/mod.rs +++ b/crates/tx/src/data/mod.rs @@ -265,7 +265,7 @@ pub struct VpsResult { } impl fmt::Display for TxResult { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { if f.alternate() { write!( f, @@ -294,7 +294,7 @@ impl FromStr for TxResult { } impl fmt::Display for VpsResult { - fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { write!( f, "{}{}{}", diff --git a/crates/tx/src/data/wrapper.rs b/crates/tx/src/data/wrapper.rs index eb36b0a3255..3ce7182a9d3 100644 --- a/crates/tx/src/data/wrapper.rs +++ b/crates/tx/src/data/wrapper.rs @@ -126,11 +126,18 @@ pub mod wrapper_tx { } } - impl From for Gas { - // Derive a Gas instance with a sub amount which is exactly a whole - // amount since the limit represents gas in whole units - fn from(value: GasLimit) -> Self { - Self::from_whole_units(u64::from(value)) + impl TryFrom for Gas { + type Error = std::io::Error; + + /// Derive a Gas instance with a sub amount which is exactly a whole + /// amount since the limit represents gas in whole units + fn try_from(value: GasLimit) -> Result { + Self::from_whole_units(u64::from(value)).ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "gas overflow", + ) + }) } } diff --git a/crates/tx/src/lib.rs b/crates/tx/src/lib.rs index bd81aecf11e..041065acad9 100644 --- a/crates/tx/src/lib.rs +++ b/crates/tx/src/lib.rs @@ -1,4 +1,18 @@ -#![allow(missing_docs)] +//! Namada transaction. + +#![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 +)] pub mod action; pub mod data; diff --git a/crates/tx/src/proto/generated.rs b/crates/tx/src/proto/generated.rs index cd408564ea0..28ae8613e72 100644 --- a/crates/tx/src/proto/generated.rs +++ b/crates/tx/src/proto/generated.rs @@ -1 +1,3 @@ +#![allow(missing_docs)] + pub mod types; diff --git a/crates/tx/src/proto/mod.rs b/crates/tx/src/proto/mod.rs index 6aa71c7eec6..790bf4429c7 100644 --- a/crates/tx/src/proto/mod.rs +++ b/crates/tx/src/proto/mod.rs @@ -1,3 +1,5 @@ +//! Generated protobuf tx type. + mod generated; pub use generated::types::Tx; diff --git a/crates/tx/src/types.rs b/crates/tx/src/types.rs index aab92f293e3..d01041b52ec 100644 --- a/crates/tx/src/types.rs +++ b/crates/tx/src/types.rs @@ -45,6 +45,8 @@ pub enum VerifySigError { InvalidWrapperSignature, #[error("The section signature is invalid: {0}")] InvalidSectionSignature(String), + #[error("The number of PKs overflows u8::MAX")] + PksOverflow, } #[allow(missing_docs)] @@ -224,7 +226,9 @@ pub fn verify_standalone_sig>( Deserialize, )] pub struct Data { + /// Salt with additional random data (usually a timestamp) pub salt: [u8; 8], + /// Data bytes pub data: Vec, } @@ -376,6 +380,7 @@ impl Code { } } +/// A memo field (bytes). pub type Memo = Vec; /// Indicates the list of public keys against which signatures will be verified @@ -431,7 +436,11 @@ impl Authorization { // Make sure the corresponding public keys can be represented by a // vector instead of a map assert!( - secret_keys.keys().cloned().eq(0..(secret_keys.len() as u8)), + secret_keys + .keys() + .cloned() + .eq(0..(u8::try_from(secret_keys.len()) + .expect("Number of SKs must not exceed `u8::MAX`"))), "secret keys must be enumerated when signer address is absent" ); Signer::PubKeys(secret_keys.values().map(RefTo::ref_to).collect()) @@ -458,8 +467,9 @@ impl Authorization { } } - pub fn total_signatures(&self) -> u8 { - self.signatures.len() as u8 + /// Get the number of signatures if it fits in `u8` + pub fn total_signatures(&self) -> Option { + u8::try_from(self.signatures.len()).ok() } /// Hash this signature section @@ -475,6 +485,7 @@ impl Authorization { ) } + /// Get a hash of this section with its signer and signatures removed pub fn get_raw_hash(&self) -> namada_core::hash::Hash { Self { signer: Signer::PubKeys(vec![]), @@ -512,7 +523,11 @@ impl Authorization { sig, )?; verified_pks.insert(*idx); - verifications += 1; + // Cannot overflow + #[allow(clippy::arithmetic_side_effects)] + { + verifications += 1; + } } } } @@ -526,14 +541,20 @@ impl Authorization { if let Some(map_idx) = public_keys_index_map.get_index_from_public_key(pk) { + let sig_idx = u8::try_from(idx) + .map_err(|_| VerifySigError::PksOverflow)?; consume_verify_sig_gas()?; common::SigScheme::verify_signature( pk, &self.get_raw_hash(), - &self.signatures[&(idx as u8)], + &self.signatures[&sig_idx], )?; verified_pks.insert(map_idx); - verifications += 1; + // Cannot overflow + #[allow(clippy::arithmetic_side_effects)] + { + verifications += 1; + } } } } @@ -576,7 +597,12 @@ impl CompressedAuthorization { // The 255th section is the raw header targets.push(tx.raw_header_hash()); } else { - targets.push(tx.sections[idx as usize - 1].get_hash()); + targets.push( + tx.sections[(idx as usize) + .checked_sub(1) + .expect("cannot underflow")] + .get_hash(), + ); } } Authorization { @@ -1026,7 +1052,7 @@ impl Tx { HEXUPPER.encode(&tx_bytes) } - // Deserialize from hex encoding + /// Deserialize from hex encoding pub fn deserialize(data: &[u8]) -> Result { if let Ok(hex) = serde_json::from_slice::(data) { match HEXUPPER.decode(hex.as_bytes()) { @@ -1076,7 +1102,7 @@ impl Tx { pub fn get_section( &self, hash: &namada_core::hash::Hash, - ) -> Option> { + ) -> Option> { if self.header_hash() == *hash { return Some(Cow::Owned(Section::Header(self.header.clone()))); } else if self.raw_header_hash() == *hash { @@ -1228,7 +1254,10 @@ impl Tx { .iter() .all(|x| self.get_section(x).is_some()) { - if signatures.total_signatures() > max_signatures { + if signatures + .total_signatures() + .map_or(false, |len| len > max_signatures) + { return Err(VerifySigError::InvalidSectionSignature( "too many signatures.".to_string(), )); @@ -1287,6 +1316,7 @@ impl Tx { .map_err(|_| VerifySigError::InvalidWrapperSignature) } + /// Compute signatures for the given keys pub fn compute_section_signature( &self, secret_keys: &[common::SecretKey], @@ -1570,9 +1600,11 @@ impl Tx { section.signatures.insert(*idx, signature.signature); } else if let Signer::PubKeys(pks) = &mut pk_section.signer { // Add the signature under its corresponding public key - pk_section - .signatures - .insert(pks.len() as u8, signature.signature); + pk_section.signatures.insert( + u8::try_from(pks.len()) + .expect("Number of PKs must not exceed u8 capacity"), + signature.signature, + ); pks.push(signature.pubkey); } } diff --git a/crates/tx_env/src/lib.rs b/crates/tx_env/src/lib.rs index c4281ec8186..ebf76cb5d12 100644 --- a/crates/tx_env/src/lib.rs +++ b/crates/tx_env/src/lib.rs @@ -1,6 +1,20 @@ //! Transaction environment contains functions that can be called from //! inside a tx. +#![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 +)] + use namada_core::address::Address; use namada_core::borsh::{BorshDeserialize, BorshSerialize, BorshSerializeExt}; use namada_core::storage; diff --git a/crates/vm_env/src/lib.rs b/crates/vm_env/src/lib.rs index 26f85aceef0..51b8922c19d 100644 --- a/crates/vm_env/src/lib.rs +++ b/crates/vm_env/src/lib.rs @@ -4,6 +4,15 @@ #![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 +)] use borsh::BorshDeserialize; use namada_core::internal::{HostEnvResult, KeyVal}; @@ -13,27 +22,27 @@ pub mod tx { // These host functions are implemented in the Namada's [`host_env`] // module. The environment provides calls to them via this C interface. extern "C" { - // Read variable-length data when we don't know the size up-front, - // returns the size of the value (can be 0), or -1 if the key is - // not present. If a value is found, it will be placed in the read - // cache, because we cannot allocate a buffer for it before we know - // its size. + /// Read variable-length data when we don't know the size up-front, + /// returns the size of the value (can be 0), or -1 if the key is + /// not present. If a value is found, it will be placed in the read + /// cache, because we cannot allocate a buffer for it before we know + /// its size. pub fn namada_tx_read(key_ptr: u64, key_len: u64) -> i64; - // Read variable-length temporary state when we don't know the size - // up-front, returns the size of the value (can be 0), or -1 if - // the key is not present. If a value is found, it will be placed in the - // result buffer, because we cannot allocate a buffer for it before - // we know its size. + /// Read variable-length temporary state when we don't know the size + /// up-front, returns the size of the value (can be 0), or -1 if + /// the key is not present. If a value is found, it will be placed in + /// the result buffer, because we cannot allocate a buffer for + /// it before we know its size. pub fn namada_tx_read_temp(key_ptr: u64, key_len: u64) -> i64; - // Read a value from result buffer. + /// Read a value from result buffer. pub fn namada_tx_result_buffer(result_ptr: u64); - // Returns 1 if the key is present, -1 otherwise. + /// Returns 1 if the key is present, -1 otherwise. pub fn namada_tx_has_key(key_ptr: u64, key_len: u64) -> i64; - // Write key/value + /// Write key/value pub fn namada_tx_write( key_ptr: u64, key_len: u64, @@ -41,7 +50,7 @@ pub mod tx { val_len: u64, ); - // Write a temporary key/value + /// Write a temporary key/value pub fn namada_tx_write_temp( key_ptr: u64, key_len: u64, @@ -49,23 +58,23 @@ pub mod tx { val_len: u64, ); - // Delete the given key and its value + /// Delete the given key and its value pub fn namada_tx_delete(key_ptr: u64, key_len: u64); - // Get an ID of a data iterator with key prefix, ordered by storage - // keys. + /// Get an ID of a data iterator with key prefix, ordered by storage + /// keys. pub fn namada_tx_iter_prefix(prefix_ptr: u64, prefix_len: u64) -> u64; - // Returns the size of the value (can be 0), or -1 if there's no next - // value. If a value is found, it will be placed in the read - // cache, because we cannot allocate a buffer for it before we know - // its size. + /// Returns the size of the value (can be 0), or -1 if there's no next + /// value. If a value is found, it will be placed in the read + /// cache, because we cannot allocate a buffer for it before we know + /// its size. pub fn namada_tx_iter_next(iter_id: u64) -> i64; - // Insert a verifier + /// Insert a verifier pub fn namada_tx_insert_verifier(addr_ptr: u64, addr_len: u64); - // Update a validity predicate + /// Update a validity predicate pub fn namada_tx_update_validity_predicate( addr_ptr: u64, addr_len: u64, @@ -75,7 +84,7 @@ pub mod tx { code_tag_len: u64, ); - // Initialize a new account + /// Initialize a new account pub fn namada_tx_init_account( code_hash_ptr: u64, code_hash_len: u64, @@ -86,37 +95,37 @@ pub mod tx { result_ptr: u64, ); - // Emit an event + /// Emit an event pub fn namada_tx_emit_event(event_ptr: u64, event_len: u64); - // Get events + /// Get events pub fn namada_tx_get_events( event_type_ptr: u64, event_type_len: u64, ) -> i64; - // Get the chain ID + /// Get the chain ID pub fn namada_tx_get_chain_id(result_ptr: u64); - // Get the current block height + /// Get the current block height pub fn namada_tx_get_block_height() -> u64; - // Get the current block header + /// Get the current block header pub fn namada_tx_get_block_header(height: u64) -> i64; - // Get the current block epoch + /// Get the current block epoch pub fn namada_tx_get_block_epoch() -> u64; - // Get the predecessor epochs + /// Get the predecessor epochs pub fn namada_tx_get_pred_epochs() -> i64; - // Get the current tx index + /// Get the current tx index pub fn namada_tx_get_tx_index() -> u32; - // Get the native token address + /// Get the native token address pub fn namada_tx_get_native_token(result_ptr: u64); - // Requires a node running with "Info" log level + /// Requires a node running with "Info" log level pub fn namada_tx_log_string(str_ptr: u64, str_len: u64); /// Charge the provided amount of gas for the current tx @@ -129,7 +138,7 @@ pub mod tx { /// Set the sentinel for a wrong tx section commitment pub fn namada_tx_set_commitment_sentinel(); - // Verify the signatures of a tx + /// Verify the signatures of a tx pub fn namada_tx_verify_tx_section_signature( hash_list_ptr: u64, hash_list_len: u64, @@ -146,7 +155,7 @@ pub mod tx { transaction_len: u64, ) -> i64; - // Yield a byte array value back to the host. + /// Yield a byte array value back to the host. pub fn namada_tx_yield_value(buf_ptr: u64, buf_len: u64); } } @@ -156,94 +165,94 @@ pub mod vp { // These host functions are implemented in the Namada's [`host_env`] // module. The environment provides calls to them via this C interface. extern "C" { - // Read variable-length prior state when we don't know the size - // up-front, returns the size of the value (can be 0), or -1 if - // the key is not present. If a value is found, it will be placed in the - // result buffer, because we cannot allocate a buffer for it before - // we know its size. + /// Read variable-length prior state when we don't know the size + /// up-front, returns the size of the value (can be 0), or -1 if + /// the key is not present. If a value is found, it will be placed in + /// the result buffer, because we cannot allocate a buffer for + /// it before we know its size. pub fn namada_vp_read_pre(key_ptr: u64, key_len: u64) -> i64; - // Read variable-length posterior state when we don't know the size - // up-front, returns the size of the value (can be 0), or -1 if - // the key is not present. If a value is found, it will be placed in the - // result buffer, because we cannot allocate a buffer for it before - // we know its size. + /// Read variable-length posterior state when we don't know the size + /// up-front, returns the size of the value (can be 0), or -1 if + /// the key is not present. If a value is found, it will be placed in + /// the result buffer, because we cannot allocate a buffer for + /// it before we know its size. pub fn namada_vp_read_post(key_ptr: u64, key_len: u64) -> i64; - // Read variable-length temporary state when we don't know the size - // up-front, returns the size of the value (can be 0), or -1 if - // the key is not present. If a value is found, it will be placed in the - // result buffer, because we cannot allocate a buffer for it before - // we know its size. + /// Read variable-length temporary state when we don't know the size + /// up-front, returns the size of the value (can be 0), or -1 if + /// the key is not present. If a value is found, it will be placed in + /// the result buffer, because we cannot allocate a buffer for + /// it before we know its size. pub fn namada_vp_read_temp(key_ptr: u64, key_len: u64) -> i64; - // Read a value from result buffer. + /// Read a value from result buffer. pub fn namada_vp_result_buffer(result_ptr: u64); - // Returns 1 if the key is present in prior state, -1 otherwise. + /// Returns 1 if the key is present in prior state, -1 otherwise. pub fn namada_vp_has_key_pre(key_ptr: u64, key_len: u64) -> i64; - // Returns 1 if the key is present in posterior state, -1 otherwise. + /// Returns 1 if the key is present in posterior state, -1 otherwise. pub fn namada_vp_has_key_post(key_ptr: u64, key_len: u64) -> i64; - // Get an ID of a data iterator with key prefix in prior state, ordered - // by storage keys. + /// Get an ID of a data iterator with key prefix in prior state, ordered + /// by storage keys. pub fn namada_vp_iter_prefix_pre( prefix_ptr: u64, prefix_len: u64, ) -> u64; - // Get an ID of a data iterator with key prefix in posterior state, - // ordered by storage keys. + /// Get an ID of a data iterator with key prefix in posterior state, + /// ordered by storage keys. pub fn namada_vp_iter_prefix_post( prefix_ptr: u64, prefix_len: u64, ) -> u64; - // Read variable-length iterator's next value when we don't know the - // size up-front, returns the size of the value (can be 0), or - // -1 if the key is not present. If a value is found, it will be - // placed in the result buffer, because we cannot allocate a - // buffer for it before we know its size. + /// Read variable-length iterator's next value when we don't know the + /// size up-front, returns the size of the value (can be 0), or + /// -1 if the key is not present. If a value is found, it will be + /// placed in the result buffer, because we cannot allocate a + /// buffer for it before we know its size. pub fn namada_vp_iter_next(iter_id: u64) -> i64; - // Get the chain ID + /// Get the chain ID pub fn namada_vp_get_chain_id(result_ptr: u64); - // Get the current block height + /// Get the current block height pub fn namada_vp_get_block_height() -> u64; - // Get the current block header + /// Get the current block header pub fn namada_vp_get_block_header(height: u64) -> i64; - // Get the current tx hash + /// Get the current tx hash pub fn namada_vp_get_tx_code_hash(result_ptr: u64); - // Get the current block epoch + /// Get the current block epoch pub fn namada_vp_get_block_epoch() -> u64; - // Get the predecessor epochs + /// Get the predecessor epochs pub fn namada_vp_get_pred_epochs() -> i64; - // Get the current tx index + /// Get the current tx index pub fn namada_vp_get_tx_index() -> u32; - // Get the native token address + /// Get the native token address pub fn namada_vp_get_native_token(result_ptr: u64); - // Get events emitted by the current tx + /// Get events emitted by the current tx pub fn namada_vp_get_events( event_type_ptr: u64, event_type_len: u64, ) -> i64; - // Yield a byte array value back to the host. + /// Yield a byte array value back to the host. pub fn namada_vp_yield_value(buf_ptr: u64, buf_len: u64); - // Requires a node running with "Info" log level + /// Requires a node running with "Info" log level pub fn namada_vp_log_string(str_ptr: u64, str_len: u64); - // Verify the signatures of a tx + /// Verify the signatures of a tx pub fn namada_vp_verify_tx_section_signature( hash_list_ptr: u64, hash_list_len: u64, @@ -256,6 +265,7 @@ pub mod vp { max_signatures_len: u64, ); + /// Evaluate a validity-predicate pub fn namada_vp_eval( vp_code_hash_ptr: u64, vp_code_hash_len: u64, @@ -283,7 +293,8 @@ pub fn read_from_buffer( if HostEnvResult::is_fail(read_result) { None } else { - let result = vec![0u8; read_result as _]; + let len = usize::try_from(read_result).ok()?; + let result = vec![0u8; len]; let offset = result.as_slice().as_ptr() as u64; unsafe { result_buffer(offset) }; Some(result) diff --git a/crates/vote_ext/src/lib.rs b/crates/vote_ext/src/lib.rs index 60f7208da9e..2a990c1c58d 100644 --- a/crates/vote_ext/src/lib.rs +++ b/crates/vote_ext/src/lib.rs @@ -1,5 +1,19 @@ //! This module contains types necessary for processing vote extensions. +#![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 +)] + pub mod bridge_pool_roots; pub mod ethereum_events; pub mod validator_set_update; diff --git a/crates/vp_env/Cargo.toml b/crates/vp_env/Cargo.toml index 7de5369916b..29045213aa0 100644 --- a/crates/vp_env/Cargo.toml +++ b/crates/vp_env/Cargo.toml @@ -21,4 +21,5 @@ namada_ibc = { path = "../ibc" } derivative.workspace = true masp_primitives.workspace = true +smooth-operator.workspace = true thiserror.workspace = true diff --git a/crates/vp_env/src/collection_validation/lazy_vec.rs b/crates/vp_env/src/collection_validation/lazy_vec.rs index f6af86c805e..af32d49db57 100644 --- a/crates/vp_env/src/collection_validation/lazy_vec.rs +++ b/crates/vp_env/src/collection_validation/lazy_vec.rs @@ -3,6 +3,7 @@ use std::collections::BTreeSet; use std::fmt::Debug; +use namada_core::arith::checked; use namada_core::borsh::{BorshDeserialize, BorshSerialize}; use namada_core::storage; use namada_storage::collections::lazy_vec::{ @@ -113,9 +114,9 @@ where } if post > pre { post_gt_pre = true; - len_diff = post - pre; + len_diff = checked!(post - pre)?; } else { - len_diff = pre - post; + len_diff = checked!(pre - post)?; } len_pre = pre; } @@ -153,21 +154,21 @@ where if len_diff != 0 && !(if post_gt_pre { - deleted_len + len_diff == added_len + checked!(deleted_len + len_diff)? == added_len } else { - added_len + len_diff == deleted_len + checked!(added_len + len_diff)? == deleted_len }) { return Err(ValidationError::InvalidLenDiff).into_storage_result(); } - let mut last_added = Option::None; + let mut last_added: Option = Option::None; // Iterate additions in increasing order of indices for index in added { if let Some(last_added) = last_added { // Following additions should be at monotonically increasing // indices - let expected = last_added + 1; + let expected = checked!(last_added + 1)?; if expected != index { return Err(ValidationError::UnexpectedPushIndex { got: index, @@ -189,13 +190,13 @@ where last_added = Some(index); } - let mut last_deleted = Option::None; + let mut last_deleted: Option = Option::None; // Also iterate deletions in increasing order of indices for index in deleted { if let Some(last_added) = last_deleted { // Following deletions should be at monotonically increasing // indices - let expected = last_added + 1; + let expected = checked!(last_added + 1)?; if expected != index { return Err(ValidationError::UnexpectedPopIndex { got: index, @@ -208,7 +209,7 @@ where } if let Some(index) = last_deleted { if len_pre > 0 { - let expected = len_pre - 1; + let expected = checked!(len_pre - 1)?; if index != expected { // The last deletion must be at the pre length value minus 1 return Err(ValidationError::UnexpectedPopIndex { @@ -223,7 +224,7 @@ where // And finally iterate updates for index in updated { // Update index has to be within the length bounds - let max = len_pre + len_diff; + let max = checked!(len_pre + len_diff)?; if index >= max { return Err(ValidationError::UnexpectedUpdateIndex { got: index, diff --git a/crates/vp_env/src/collection_validation/mod.rs b/crates/vp_env/src/collection_validation/mod.rs index a1ba83a6831..0a2cab5db8e 100644 --- a/crates/vp_env/src/collection_validation/mod.rs +++ b/crates/vp_env/src/collection_validation/mod.rs @@ -69,6 +69,7 @@ where }) } +/// Extensions for [`LazyCollection`]s for validation. pub trait LazyCollectionExt: LazyCollection { /// Actions on the collection determined from changed storage keys by /// `Self::validate` diff --git a/crates/vp_env/src/lib.rs b/crates/vp_env/src/lib.rs index d785d7b5707..539f530ecd1 100644 --- a/crates/vp_env/src/lib.rs +++ b/crates/vp_env/src/lib.rs @@ -1,6 +1,20 @@ //! Validity predicate environment contains functions that can be called from //! inside validity predicates. +#![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 +)] + pub mod collection_validation; // TODO: this should be re-exported from namada_shielded_token diff --git a/wasm/Cargo.lock b/wasm/Cargo.lock index 0cf96f6666a..8064311c644 100644 --- a/wasm/Cargo.lock +++ b/wasm/Cargo.lock @@ -4091,6 +4091,7 @@ dependencies = [ "namada_storage", "namada_trans_token", "serde", + "smooth-operator", "tracing", ] @@ -4116,6 +4117,7 @@ dependencies = [ "patricia_tree", "proptest", "sha2 0.9.9", + "smooth-operator", "sparse-merkle-tree", "thiserror", "tiny-keccak", @@ -4138,6 +4140,7 @@ dependencies = [ "namada_tx", "regex", "serde", + "smooth-operator", "thiserror", "tracing", ] @@ -4299,6 +4302,7 @@ dependencies = [ "namada_ibc", "namada_storage", "namada_tx", + "smooth-operator", "thiserror", ] diff --git a/wasm_for_tests/Cargo.lock b/wasm_for_tests/Cargo.lock index 52a27b3eb95..48c516a1f67 100644 --- a/wasm_for_tests/Cargo.lock +++ b/wasm_for_tests/Cargo.lock @@ -4041,6 +4041,7 @@ dependencies = [ "namada_storage", "namada_trans_token", "serde", + "smooth-operator", "tracing", ] @@ -4064,6 +4065,7 @@ dependencies = [ "patricia_tree", "proptest", "sha2 0.9.9", + "smooth-operator", "sparse-merkle-tree", "thiserror", "tiny-keccak", @@ -4084,6 +4086,7 @@ dependencies = [ "namada_tx", "regex", "serde", + "smooth-operator", "thiserror", "tracing", ] @@ -4241,6 +4244,7 @@ dependencies = [ "namada_ibc", "namada_storage", "namada_tx", + "smooth-operator", "thiserror", ]