From d4d731511f48ab0f028f8345859b0ad746f6bdb9 Mon Sep 17 00:00:00 2001 From: carneiro-cw <156914855+carneiro-cw@users.noreply.github.com> Date: Fri, 27 Dec 2024 13:45:50 -0300 Subject: [PATCH] chore: remove/allow panics, expects and unwraps (#1933) --- build.rs | 4 ++++ src/eth/executor/evm.rs | 19 +++++++++++-------- src/eth/external_rpc/postgres.rs | 2 ++ src/eth/follower/importer/importer.rs | 1 + src/eth/miner/miner.rs | 1 + src/eth/primitives/address.rs | 2 +- src/eth/primitives/execution.rs | 4 ++-- src/eth/primitives/external_block.rs | 2 ++ src/eth/primitives/external_receipt.rs | 2 ++ src/eth/primitives/log_mined.rs | 19 ++++++++++++++----- src/eth/storage/temporary/inmemory.rs | 6 +++++- src/ext.rs | 6 ++++++ src/globals.rs | 1 + src/infra/kafka/kafka.rs | 1 + 14 files changed, 53 insertions(+), 17 deletions(-) diff --git a/build.rs b/build.rs index 63f8212f4..5b123b981 100644 --- a/build.rs +++ b/build.rs @@ -1,3 +1,7 @@ +#![allow(clippy::unwrap_used)] +#![allow(clippy::expect_used)] +#![allow(clippy::panic)] + use std::collections::HashSet; use std::env; use std::fs; diff --git a/src/eth/executor/evm.rs b/src/eth/executor/evm.rs index e15d518f3..071ed0d60 100644 --- a/src/eth/executor/evm.rs +++ b/src/eth/executor/evm.rs @@ -147,7 +147,7 @@ impl Evm { // parse result let execution = match evm_result { // executed - Ok(result) => Ok(parse_revm_execution(result, session_input, session_storage_changes)), + Ok(result) => Ok(parse_revm_execution(result, session_input, session_storage_changes)?), // nonce errors Err(EVMError::Transaction(InvalidTransaction::NonceTooHigh { tx, state })) => Err(StratusError::TransactionNonce { @@ -303,9 +303,9 @@ impl Database for RevmSession { // Conversion // ----------------------------------------------------------------------------- -fn parse_revm_execution(revm_result: RevmResultAndState, input: EvmInput, execution_changes: ExecutionChanges) -> EvmExecution { +fn parse_revm_execution(revm_result: RevmResultAndState, input: EvmInput, execution_changes: ExecutionChanges) -> Result { let (result, tx_output, logs, gas) = parse_revm_result(revm_result.result); - let changes = parse_revm_state(revm_result.state, execution_changes); + let changes = parse_revm_state(revm_result.state, execution_changes)?; tracing::info!(?result, %gas, tx_output_len = %tx_output.len(), %tx_output, "evm executed"); let mut deployed_contract_address = None; @@ -315,7 +315,7 @@ fn parse_revm_execution(revm_result: RevmResultAndState, input: EvmInput, execut } } - EvmExecution { + Ok(EvmExecution { block_timestamp: input.block_timestamp, receipt_applied: false, result, @@ -324,7 +324,7 @@ fn parse_revm_execution(revm_result: RevmResultAndState, input: EvmInput, execut gas, changes, deployed_contract_address, - } + }) } fn parse_revm_result(result: RevmExecutionResult) -> (ExecutionResult, Bytes, Vec, Gas) { @@ -351,7 +351,7 @@ fn parse_revm_result(result: RevmExecutionResult) -> (ExecutionResult, Bytes, Ve } } -fn parse_revm_state(revm_state: RevmState, mut execution_changes: ExecutionChanges) -> ExecutionChanges { +fn parse_revm_state(revm_state: RevmState, mut execution_changes: ExecutionChanges) -> Result { for (revm_address, revm_account) in revm_state { let address: Address = revm_address.into(); if address.is_ignored() { @@ -387,10 +387,13 @@ fn parse_revm_state(revm_state: RevmState, mut execution_changes: ExecutionChang } else if account_touched { let Some(account_changes) = execution_changes.get_mut(&address) else { tracing::error!(keys = ?execution_changes.keys(), %address, "account touched, but not loaded by evm"); - panic!("Account '{}' was expected to be loaded by EVM, but it was not", address); + return Err(StratusError::Unexpected(anyhow!( + "Account '{}' was expected to be loaded by EVM, but it was not", + address + ))); }; account_changes.apply_modifications(account, account_modified_slots); } } - execution_changes + Ok(execution_changes) } diff --git a/src/eth/external_rpc/postgres.rs b/src/eth/external_rpc/postgres.rs index f78a9df77..c5bc389b0 100644 --- a/src/eth/external_rpc/postgres.rs +++ b/src/eth/external_rpc/postgres.rs @@ -1,3 +1,5 @@ +#![allow(clippy::panic)] + use std::time::Duration; use async_trait::async_trait; diff --git a/src/eth/follower/importer/importer.rs b/src/eth/follower/importer/importer.rs index 660643d2c..5998feec7 100644 --- a/src/eth/follower/importer/importer.rs +++ b/src/eth/follower/importer/importer.rs @@ -522,6 +522,7 @@ async fn fetch_block_and_receipts(chain: Arc, block_number: Bl (block, receipts) } +#[allow(clippy::expect_used)] #[tracing::instrument(name = "importer::fetch_block", skip_all, fields(block_number))] async fn fetch_block(chain: Arc, block_number: BlockNumber) -> ExternalBlock { const RETRY_DELAY: Duration = Duration::from_millis(10); diff --git a/src/eth/miner/miner.rs b/src/eth/miner/miner.rs index fbe6bc680..6a1556942 100644 --- a/src/eth/miner/miner.rs +++ b/src/eth/miner/miner.rs @@ -508,6 +508,7 @@ mod interval_miner_ticker { const TASK_NAME: &str = "interval-miner-ticker"; // sync to next second + #[allow(clippy::expect_used)] let next_second = (Utc::now() + Duration::from_secs(1)) .with_nanosecond(0) .expect("nanosecond above is set to `0`, which is always less than 2 billion"); diff --git a/src/eth/primitives/address.rs b/src/eth/primitives/address.rs index d5f1409f7..290f2fd9a 100644 --- a/src/eth/primitives/address.rs +++ b/src/eth/primitives/address.rs @@ -99,7 +99,7 @@ impl From for Address { impl From for Address { fn from(value: NameOrAddress) -> Self { match value { - NameOrAddress::Name(_) => panic!("TODO"), + NameOrAddress::Name(_) => todo!(), NameOrAddress::Address(value) => Self(value), } } diff --git a/src/eth/primitives/execution.rs b/src/eth/primitives/execution.rs index d70a17955..3365adc43 100644 --- a/src/eth/primitives/execution.rs +++ b/src/eth/primitives/execution.rs @@ -66,7 +66,7 @@ impl EvmExecution { let sender_next_nonce = sender_changes .nonce .take_original_ref() - .expect("from_original_values populates original values, so taking original ref here will succeed") + .ok_or_else(|| anyhow!("original nonce value not found when it should have been populated by from_original_values"))? .next_nonce(); sender_changes.nonce.set_modified(sender_next_nonce); @@ -197,7 +197,7 @@ impl EvmExecution { }; // subtract execution cost from sender balance - let sender_balance = *sender_changes.balance.take_ref().expect("balance is never None"); + let sender_balance = *sender_changes.balance.take_ref().ok_or(anyhow!("sender balance was None"))?; let sender_new_balance = if sender_balance > execution_cost { sender_balance - execution_cost } else { diff --git a/src/eth/primitives/external_block.rs b/src/eth/primitives/external_block.rs index fbc26a961..15a62c811 100644 --- a/src/eth/primitives/external_block.rs +++ b/src/eth/primitives/external_block.rs @@ -18,11 +18,13 @@ pub struct ExternalBlock(#[deref] pub EthersBlockExternalTransaction); impl ExternalBlock { /// Returns the block hash. + #[allow(clippy::expect_used)] pub fn hash(&self) -> Hash { self.0.hash.expect("external block must have hash").into() } /// Returns the block number. + #[allow(clippy::expect_used)] pub fn number(&self) -> BlockNumber { self.0.number.expect("external block must have number").into() } diff --git a/src/eth/primitives/external_receipt.rs b/src/eth/primitives/external_receipt.rs index f79e5be75..8e1d514c5 100644 --- a/src/eth/primitives/external_receipt.rs +++ b/src/eth/primitives/external_receipt.rs @@ -21,11 +21,13 @@ impl ExternalReceipt { } /// Returns the block number. + #[allow(clippy::expect_used)] pub fn block_number(&self) -> BlockNumber { self.0.block_number.expect("external receipt must have block number").into() } /// Returns the block hash. + #[allow(clippy::expect_used)] pub fn block_hash(&self) -> Hash { self.0.block_hash.expect("external receipt must have block hash").into() } diff --git a/src/eth/primitives/log_mined.rs b/src/eth/primitives/log_mined.rs index 1c136b3a3..5fd0f1b6c 100644 --- a/src/eth/primitives/log_mined.rs +++ b/src/eth/primitives/log_mined.rs @@ -58,12 +58,21 @@ impl LogMined { impl TryFrom for LogMined { type Error = anyhow::Error; fn try_from(value: EthersLog) -> Result { + let transaction_hash = value.transaction_hash.ok_or_else(|| anyhow::anyhow!("log must have transaction_hash"))?.into(); + let transaction_index = value + .transaction_index + .ok_or_else(|| anyhow::anyhow!("log must have transaction_index"))? + .into(); + let log_index = value.log_index.ok_or_else(|| anyhow::anyhow!("log must have log_index"))?.try_into()?; + let block_number = value.block_number.ok_or_else(|| anyhow::anyhow!("log must have block_number"))?.into(); + let block_hash = value.block_hash.ok_or_else(|| anyhow::anyhow!("log must have block_hash"))?.into(); + Ok(Self { - transaction_hash: value.transaction_hash.expect("log must have transaction_hash").into(), - transaction_index: value.transaction_index.expect("log must have transaction_index").into(), - log_index: value.log_index.expect("log must have log_index").try_into()?, - block_number: value.block_number.expect("log must have block_number").into(), - block_hash: value.block_hash.expect("log must have block_hash").into(), + transaction_hash, + transaction_index, + log_index, + block_number, + block_hash, log: value.into(), }) } diff --git a/src/eth/storage/temporary/inmemory.rs b/src/eth/storage/temporary/inmemory.rs index 7492fdaac..2f35f203f 100644 --- a/src/eth/storage/temporary/inmemory.rs +++ b/src/eth/storage/temporary/inmemory.rs @@ -209,7 +209,11 @@ impl TemporaryStorage for InMemoryTemporaryStorage { #[cfg(not(feature = "dev"))] let finished_block = { let latest = RwLockWriteGuard::>::downgrade(latest); - latest.as_ref().expect("latest should be Some after finishing the pending block").block.clone() + latest + .as_ref() + .ok_or_else(|| anyhow::anyhow!("latest should be Some after finishing the pending block"))? + .block + .clone() }; Ok(finished_block) diff --git a/src/ext.rs b/src/ext.rs index 74547c5c5..ab228d865 100644 --- a/src/ext.rs +++ b/src/ext.rs @@ -135,6 +135,7 @@ impl InfallibleExt for Result where T: Sized, { + #[allow(clippy::expect_used)] fn expect_infallible(self) -> T { if let Err(ref e) = self { tracing::error!(reason = ?e, "expected infallible serde serialization/deserialization"); @@ -144,6 +145,7 @@ where } impl InfallibleExt for Option { + #[allow(clippy::expect_used)] fn expect_infallible(self) -> Decimal { if self.is_none() { tracing::error!("expected infallible decimal conversion"); @@ -153,6 +155,7 @@ impl InfallibleExt for Option { } impl InfallibleExt, ()> for Option> { + #[allow(clippy::expect_used)] fn expect_infallible(self) -> DateTime { if self.is_none() { tracing::error!("expected infallible datetime conversion"); @@ -238,6 +241,7 @@ pub async fn traced_sleep(duration: Duration, _: SleepReason) { /// Spawns an async Tokio task with a name to be displayed in tokio-console. #[track_caller] +#[allow(clippy::expect_used)] pub fn spawn_named(name: &str, task: impl std::future::Future + Send + 'static) -> tokio::task::JoinHandle where T: Send + 'static, @@ -252,6 +256,7 @@ where /// Spawns a blocking Tokio task with a name to be displayed in tokio-console. #[track_caller] +#[allow(clippy::expect_used)] pub fn spawn_blocking_named(name: &str, task: impl FnOnce() -> T + Send + 'static) -> tokio::task::JoinHandle where T: Send + 'static, @@ -265,6 +270,7 @@ where } /// Spawns a thread with the given name. Thread has access to Tokio current runtime. +#[allow(clippy::expect_used)] #[track_caller] pub fn spawn_thread(name: &str, task: impl FnOnce() -> T + Send + 'static) -> std::thread::JoinHandle where diff --git a/src/globals.rs b/src/globals.rs index ce00603d9..e08ce0de2 100644 --- a/src/globals.rs +++ b/src/globals.rs @@ -41,6 +41,7 @@ impl GlobalServices where T: clap::Parser + WithCommonConfig + Debug, { + #[allow(clippy::expect_used)] /// Executes global services initialization. pub fn init() -> Self where diff --git a/src/infra/kafka/kafka.rs b/src/infra/kafka/kafka.rs index c2b17d1f5..50729da47 100644 --- a/src/infra/kafka/kafka.rs +++ b/src/infra/kafka/kafka.rs @@ -92,6 +92,7 @@ impl std::fmt::Display for KafkaSecurityProtocol { } impl KafkaConnector { + #[allow(clippy::expect_used)] pub fn new(config: &KafkaConfig) -> Result { tracing::info!( topic = %config.topic,