chore: remove/allow panics, expects and unwraps

[carneiro-cw]

PR Type

Enhancement, Bug fix

---

Description

• Improved error handling across multiple modules by replacing expect and panic calls with more robust error handling mechanisms using Result, ok_or_else, and anyhow!.
• Added #[allow(clippy::expect_used)], #[allow(clippy::unwrap_used)], and #[allow(clippy::panic)] attributes to specific functions and modules where these operations are deemed necessary or safe.
• Refactored parse_revm_execution and parse_revm_state functions in the EVM executor to return Result types for better error propagation.
• Updated various primitive types and conversions to use anyhow::Error for more informative error messages.
• Replaced a panic! call with todo!() in the Address conversion from NameOrAddress.
• Made minor adjustments to improve code quality and maintainability across multiple files.

---

Changes walkthrough 📝

	Relevant files
Configuration changes	9 files build.rs Allow specific clippy lints in build script                            build.rs Added allow attributes for clippy lints: unwrap_used, expect_used, and panic +4/-0      postgres.rs Allow panic lint in postgres module                                            src/eth/external_rpc/postgres.rs Added allow attribute for clippy::panic lint +2/-0      importer.rs Allow expect usage in block fetching                                          src/eth/follower/importer/importer.rs Added allow attribute for clippy::expect_used lint in fetch_block function +1/-0      miner.rs Allow expect usage in miner ticker                                              src/eth/miner/miner.rs Added allow attribute for clippy::expect_used lint in interval_miner_ticker module +1/-0      external_block.rs Allow expect usage in external block methods                          src/eth/primitives/external_block.rs Added allow attributes for clippy::expect_used lint in hash and number methods +2/-0      external_receipt.rs Allow expect usage in external receipt methods                      src/eth/primitives/external_receipt.rs Added allow attributes for clippy::expect_used lint in block_number and block_hash methods +2/-0      ext.rs Allow expect usage in extension traits                                      src/ext.rs Added allow attributes for clippy::expect_used lint in various implementations +6/-0      globals.rs Allow expect usage in global services initialization          src/globals.rs Added allow attribute for clippy::expect_used lint in GlobalServices implementation +1/-0      kafka.rs Allow expect usage in Kafka connector creation                      src/infra/kafka/kafka.rs Added allow attribute for clippy::expect_used lint in KafkaConnector::new method +1/-0
Error handling	4 files evm.rs Improve error handling in EVM execution parsing                    src/eth/executor/evm.rs Modified parse_revm_execution to return Result instead of EvmExecution Changed parse_revm_state to return Result instead of ExecutionChanges Replaced panic with StratusError in parse_revm_state +11/-8    execution.rs Improve error handling in execution primitives                      src/eth/primitives/execution.rs Replaced expect with ok_or_else and anyhow! for better error handling Modified error handling in EvmExecution implementation +2/-2      log_mined.rs Improve error handling in log conversion                                  src/eth/primitives/log_mined.rs Replaced expect calls with ok_or_else and anyhow! for better error handling in TryFrom implementation +14/-5    inmemory.rs Enhance error handling in temporary storage                            src/eth/storage/temporary/inmemory.rs Replaced expect with ok_or_else and anyhow! for better error handling in TemporaryStorage implementation +5/-1
Enhancement	1 files address.rs Replace panic with todo in address conversion                        src/eth/primitives/address.rs Replaced panic with todo! in From implementation for Address +1/-1

---

💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

[github-actions]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Error Handling The changes in this file introduce new error handling mechanisms. Ensure that the new Result types are properly propagated and handled throughout the codebase. fn parse_revm_execution(revm_result: RevmResultAndState, input: EvmInput, execution_changes: ExecutionChanges) -> Result<EvmExecution, StratusError> { let (result, tx_output, logs, gas) = parse_revm_result(revm_result.result); 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; for changes in changes.values() { if changes.bytecode.is_modified() { deployed_contract_address = Some(changes.address); } } Ok(EvmExecution { block_timestamp: input.block_timestamp, receipt_applied: false, result, output: tx_output, logs, gas, changes, deployed_contract_address, }) } fn parse_revm_result(result: RevmExecutionResult) -> (ExecutionResult, Bytes, Vec<Log>, Gas) { match result { RevmExecutionResult::Success { output, gas_used, logs, .. } => { let result = ExecutionResult::Success; let output = Bytes::from(output); let logs = logs.into_iter().map_into().collect(); let gas = Gas::from(gas_used); (result, output, logs, gas) } RevmExecutionResult::Revert { output, gas_used } => { let result = ExecutionResult::Reverted; let output = Bytes::from(output); let gas = Gas::from(gas_used); (result, output, Vec::new(), gas) } RevmExecutionResult::Halt { reason, gas_used } => { let result = ExecutionResult::new_halted(format!("{:?}", reason)); let output = Bytes::default(); let gas = Gas::from(gas_used); (result, output, Vec::new(), gas) } } } fn parse_revm_state(revm_state: RevmState, mut execution_changes: ExecutionChanges) -> Result<ExecutionChanges, StratusError> { for (revm_address, revm_account) in revm_state { let address: Address = revm_address.into(); if address.is_ignored() { continue; } // apply changes according to account status tracing::debug!( %address, status = ?revm_account.status, balance = %revm_account.info.balance, nonce = %revm_account.info.nonce, slots = %revm_account.storage.len(), "evm account" ); let (account_created, account_touched) = (revm_account.is_created(), revm_account.is_touched()); // parse revm types to stratus primitives let account: Account = (revm_address, revm_account.info).into(); let account_modified_slots: Vec<Slot> = revm_account .storage .into_iter() .filter_map(|(index, value)| match value.is_changed() { true => Some(Slot::new(index.into(), value.present_value.into())), false => None, }) .collect(); // handle account created (contracts) or touched (everything else) if account_created { let account_changes = ExecutionAccountChanges::from_modified_values(account, account_modified_slots); execution_changes.insert(account_changes.address, account_changes); } 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"); 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); } } Ok(execution_changes) } Potential Panic The use of ok_or_else with anyhow! macro might still lead to a panic if the error is not properly handled upstream. Verify that this error is caught and handled appropriately. .ok_or_else(|| anyhow!("original nonce value not found when it should have been populated by from_original_values"))? .next_nonce(); Tracing Improvement Several functions are using expect with tracing. Consider adding more context to these tracing events, such as including relevant variables or more detailed error messages. #[allow(clippy::expect_used)] fn expect_infallible(self) -> T { if let Err(ref e) = self { tracing::error!(reason = ?e, "expected infallible serde serialization/deserialization"); } self.expect("serde serialization/deserialization") } } impl InfallibleExt<Decimal, ()> for Option<Decimal> { #[allow(clippy::expect_used)] fn expect_infallible(self) -> Decimal { if self.is_none() { tracing::error!("expected infallible decimal conversion"); } self.expect("infallible decimal conversion") } } impl InfallibleExt<DateTime<Utc>, ()> for Option<DateTime<Utc>> { #[allow(clippy::expect_used)] fn expect_infallible(self) -> DateTime<Utc> { if self.is_none() { tracing::error!("expected infallible datetime conversion"); } self.expect("infallible datetime conversion") } } pub trait MutexResultExt<T> { fn map_lock_error(self, function_name: &str) -> Result<T, StratusError>; } impl<T> MutexResultExt<T> for Result<T, std::sync::PoisonError<T>> { fn map_lock_error(self, function_name: &str) -> Result<T, StratusError> { self.map_err(|_| StratusError::Unexpected(anyhow::anyhow!("accessed poisoned Mutex at function `{function_name}`"))) .inspect_err(|err| { tracing::error!(reason = ?err, "FATAL: Mutex is poisoned"); }) } } // ----------------------------------------------------------------------------- // Duration // ----------------------------------------------------------------------------- /// Parses a duration specified using human-time notation or fallback to milliseconds. pub fn parse_duration(s: &str) -> anyhow::Result<Duration> { // try millis let millis: Result<u64, _> = s.parse(); if let Ok(millis) = millis { return Ok(Duration::from_millis(millis)); } // try humantime if let Ok(parsed) = humantime::parse_duration(s) { return Ok(parsed); } // error Err(anyhow!("invalid duration format: {}", s)) } // ----------------------------------------------------------------------------- // Tokio // ----------------------------------------------------------------------------- /// Indicates why a sleep is happening. #[derive(Debug, strum::Display)] pub enum SleepReason { /// Task is executed at predefined intervals. #[strum(to_string = "interval")] Interval, /// Task is awaiting a backoff before retrying the operation. #[strum(to_string = "retry-backoff")] RetryBackoff, /// Task is awaiting an external system or component to produde or synchronize data. #[strum(to_string = "sync-data")] SyncData, } /// Sleeps the current task and tracks why it is sleeping. #[cfg(feature = "tracing")] #[inline(always)] pub async fn traced_sleep(duration: Duration, reason: SleepReason) { use tracing::Instrument; let span = tracing::debug_span!("tokio::sleep", duration_ms = %duration.as_millis(), %reason); async { tracing::debug!(duration_ms = %duration.as_millis(), %reason, "sleeping"); tokio::time::sleep(duration).await; } .instrument(span) .await; } #[cfg(not(feature = "tracing"))] #[inline(always)] pub async fn traced_sleep(duration: Duration, _: SleepReason) { tokio::time::sleep(duration).await; } /// Spawns an async Tokio task with a name to be displayed in tokio-console. #[track_caller] #[allow(clippy::expect_used)] pub fn spawn_named<T>(name: &str, task: impl std::future::Future<Output = T> + Send + 'static) -> tokio::task::JoinHandle<T> where T: Send + 'static, { info_task_spawn(name); tokio::task::Builder::new() .name(name) .spawn(task) .expect("spawning named async task should not fail") } /// 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<T>(name: &str, task: impl FnOnce() -> T + Send + 'static) -> tokio::task::JoinHandle<T> where T: Send + 'static, { info_task_spawn(name); tokio::task::Builder::new() .name(name) .spawn_blocking(task) .expect("spawning named blocking task should not fail") } /// Spawns a thread with the given name. Thread has access to Tokio current runtime. #[allow(clippy::expect_used)] #[track_caller] pub fn spawn_thread<T>(name: &str, task: impl FnOnce() -> T + Send + 'static) -> std::thread::JoinHandle<T> where T: Send + 'static,

[github-actions]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Score
Possible issue	Implement proper error handling for unhandled enum variant instead of using todo!() Instead of using todo!(), implement proper error handling for the NameOrAddress::Name variant. This could involve returning a Result with an appropriate error type or implementing a conversion method for the Name variant. src/eth/primitives/address.rs [99-106] -impl From<NameOrAddress> for Address { - fn from(value: NameOrAddress) -> Self { +impl TryFrom<NameOrAddress> for Address { + type Error = anyhow::Error; + fn try_from(value: NameOrAddress) -> Result<Self, Self::Error> { match value { - NameOrAddress::Name(_) => todo!(), - NameOrAddress::Address(value) => Self(value), + NameOrAddress::Name(_) => Err(anyhow::anyhow!("Name to Address conversion not implemented")), + NameOrAddress::Address(value) => Ok(Self(value)), } } } Suggestion importance[1-10]: 8 Why: The suggestion addresses a critical issue by replacing a todo!() placeholder with proper error handling. This improvement enhances the robustness and completeness of the code, making it production-ready.	8
General	Improve error handling by providing more context for EVM execution errors Propagate the error from parse_revm_execution instead of using the ? operator, which may hide important error context. Consider wrapping the error with additional context about the EVM execution. src/eth/executor/evm.rs [150] -Ok(parse_revm_execution(result, session_input, session_storage_changes)?) +parse_revm_execution(result, session_input, session_storage_changes).map_err(|e| StratusError::EvmExecution(Box::new(e))) Suggestion importance[1-10]: 7 Why: This suggestion enhances error handling by providing more context for EVM execution errors. It's an important improvement that can significantly aid in debugging and error tracing in a critical part of the system.	7
	Improve error handling by providing more context when unwrapping optional values Replace the direct unwrapping of Option with proper error handling using ok_or_else() to provide a meaningful error message. src/eth/primitives/execution.rs [200] -let sender_balance = *sender_changes.balance.take_ref().ok_or(anyhow!("sender balance was None"))?; +let sender_balance = *sender_changes.balance.take_ref().ok_or_else(|| anyhow::anyhow!("Sender balance not found when it should have been present"))?; Suggestion importance[1-10]: 5 Why: The suggestion slightly improves the error message by providing more context. While it's a valid improvement, the impact is moderate as it only affects error reporting and not core functionality.	5