fix: computer external_transaction metrics only after transaction is persisted

[dinhani-cw]

PR Type

Bug fix, Enhancement

---

Description

• Refactored the EVM executor to remove unused code and simplify transaction execution logic.
• Fixed the issue of computing external transaction metrics only after the transaction is persisted.
• Renamed extract_function to solidity_signature across multiple files for consistency.
• Added and documented new methods in ExternalTransaction for better code organization.

---

Changes walkthrough 📝

	Relevant files
Enhancement	evm.rs Refactor EVM executor and remove unused code                          src/eth/executor/evm.rs Removed unused imports and functions. Simplified the execute_external_transaction function. Moved solidity_signature function to ExternalTransaction. +0/-17    call_input.rs Rename and document function signature extraction                src/eth/primitives/call_input.rs Renamed extract_function to solidity_signature. Added documentation for solidity_signature. +4/-1      external_transaction.rs Add and refactor methods in ExternalTransaction                    src/eth/primitives/external_transaction.rs Added solidity_signature and is_contract_deployment methods. Moved solidity_signature logic from EvmInput. +21/-0    transaction_input.rs Rename and document function signature extraction                src/eth/primitives/transaction_input.rs Renamed extract_function to solidity_signature. Added documentation for solidity_signature. +5/-2      rpc_middleware.rs Use unified function signature extraction in RPC middleware src/eth/rpc/rpc_middleware.rs Replaced extract_function with solidity_signature. +2/-2
Bug fix	executor.rs Fix and enhance external transaction execution and metrics src/eth/executor/executor.rs Modified execute_external_transaction to persist state before computing metrics. Updated metric tracking to use solidity_signature. Simplified transaction execution logic. +74/-60

---

💡 PR-Agent usage:
Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

[github-actions]

PR Reviewer Guide 🔍

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

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Tracing Fields The execute_external_transaction function is instrumented with tracing::instrument, but it does not record any identifiers as span fields. Consider adding relevant fields such as tx_hash or block_number to the span. Error Logging When logging errors, ensure the original error is logged in a field called reason. This is done correctly in some places but missed in others, such as when comparing execution with receipt. Dynamic Fields in Tracing Avoid formatting dynamic fields in tracing events. Instead, add these dynamic fields as tracing fields. For example, in the error logging within execute_external_transaction, the json_tx, json_receipt, and json_execution_logs should be added as fields. Tokio Task Naming If there are any tokio::spawn or tokio::spawn_blocking calls, consider using spawn_named or spawn_blocking_named for better traceability.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Move the metrics tracking code outside of the match block to ensure metrics are always tracked To ensure that the metrics are always tracked, even if an error occurs, consider moving the metrics tracking code outside of the match block. src/eth/executor/executor.rs [337-347] +#[cfg(feature = "metrics")] +{ + let evm_execution = tx_execution.execution(); + let evm_metrics = tx_execution.metrics(); + *block_metrics += *evm_metrics; + let function = tx.solidity_signature(); + metrics::inc_executor_external_transaction(start.elapsed(), function.clone()); + metrics::inc_executor_external_transaction_account_reads(evm_metrics.account_reads, function.clone()); + metrics::inc_executor_external_transaction_slot_reads(evm_metrics.slot_reads, function.clone()); + metrics::inc_executor_external_transaction_gas(evm_execution.gas.as_u64() as usize, function); +} Suggestion importance[1-10]: 9 Why: This suggestion ensures that metrics are always tracked, which is crucial for monitoring and debugging.	9
Maintainability	Extract the logic for handling failed external transactions into a separate function for better readability and maintainability To improve readability and maintainability, consider extracting the logic for handling failed external transactions into a separate function. src/eth/executor/executor.rs [321-329] -false => { - let sender = self.storage.read_account(&receipt.from.into(), &StoragePointInTime::Pending)?; - let execution = EvmExecution::from_failed_external_transaction(sender, receipt, block)?; - let evm_result = EvmExecutionResult { - execution, - metrics: EvmExecutionMetrics::default(), - }; - ExternalTransactionExecution::new(tx.clone(), receipt.clone(), evm_result) -} +false => self.handle_failed_external_transaction(tx, receipt, block)? Suggestion importance[1-10]: 8 Why: This suggestion significantly improves code readability and maintainability by modularizing the code.	8
Possible issue	Use if let instead of match to handle the result of receipt.is_success() to avoid potential panics To avoid potential panics, consider using if let instead of match when handling the result of receipt.is_success(). This will ensure that the code gracefully handles the Err case. src/eth/executor/executor.rs [286-297] -let tx_execution = match receipt.is_success() { - true => { - // re-execute transaction - let evm_input = EvmInput::from_external(tx, receipt, block)?; - let evm_execution = self.evms.execute(evm_input.clone(), EvmRoute::External); - // handle re-execution result - let mut evm_execution = match evm_execution { - Ok(inner) => inner, - Err(e) => { - let json_tx = to_json_string(&tx); - let json_receipt = to_json_string(&receipt); - tracing::error!(reason = ?e, block_number = %block.number(), tx_hash = %tx.hash(), %json_tx, %json_receipt, "failed to reexecute external transaction"); - return Err(e.into()); - } - }; +let tx_execution = if receipt.is_success() { + // re-execute transaction + let evm_input = EvmInput::from_external(tx, receipt, block)?; + let evm_execution = self.evms.execute(evm_input.clone(), EvmRoute::External); + // handle re-execution result + let mut evm_execution = match evm_execution { + Ok(inner) => inner, + Err(e) => { + let json_tx = to_json_string(&tx); + let json_receipt = to_json_string(&receipt); + tracing::error!(reason = ?e, block_number = %block.number(), tx_hash = %tx.hash(), %json_tx, %json_receipt, "failed to reexecute external transaction"); + return Err(e.into()); + } + }; Suggestion importance[1-10]: 7 Why: This suggestion improves code safety by avoiding potential panics, but it does not address a critical issue.	7
	Use get(..4).map_or to avoid potential panics when parsing the Solidity function signature To avoid potential panics, consider using the get(..4).map_or method instead of get(..4)?.try_into().ok()? when parsing the Solidity function signature. src/eth/primitives/external_transaction.rs [38-39] -let sig = Signature::Function(self.input.get(..4)?.try_into().ok()?); +let sig = Signature::Function(self.input.get(..4).map_or(None, |slice| slice.try_into().ok())?); Some(sig.extract()) Suggestion importance[1-10]: 7 Why: This suggestion improves code safety by avoiding potential panics, but it does not address a critical issue.	7