feat: eth_getTransactionByHash returns executed, but not mined transa…

[dinhani-cw]

…ctions

[github-actions]

PR Reviewer Guide 🔍

⏱️ Estimated effort to review [1-5]	4
🧪 Relevant tests	No
🔒 Security concerns	No
⚡ Key issues to review	Logging and Tracing: The PR introduces changes to how transactions are handled and logged. Ensure that all new transaction stages are properly logged and that the tracing spans include all necessary identifiers. The use of tracing::instrument on eth_get_transaction_by_hash and eth_get_transaction_receipt should include more detailed fields if necessary to improve traceability.
	Error Handling: The use of expect_infallible in serialization functions could be risky. Although Rust's type system ensures that certain errors are impossible, it's generally safer to handle unexpected cases gracefully. Consider replacing expect_infallible with proper error handling that doesn't panic.
	Code Duplication: The serialization logic in TransactionStage is somewhat repetitive and could be refactored to reduce duplication and improve maintainability.
	Cloning vs References: The PR does not show signs of unnecessary cloning, but it's important to ensure that data is passed efficiently, using references where possible to avoid unnecessary data duplication.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Possible bug	Add error handling for transaction reading to prevent server crashes Add error handling for the case where read_transaction might fail. Currently, the code unwraps the result directly, which could lead to a panic if there's an error reading the transaction. src/eth/rpc/rpc_server.rs [401] -let tx = ctx.storage.read_transaction(&hash)?; +let tx = match ctx.storage.read_transaction(&hash) { + Ok(tx) => tx, + Err(e) => { + log::error!("Failed to read transaction: {}", e); + return Err(RpcError::from(e)); + } +}; Suggestion importance[1-10]: 10 Why: This suggestion addresses a critical issue where a failed transaction read could cause the server to crash, thus improving the stability and reliability of the system.	10
Possible issue	Improve error handling during serialization to prevent potential panics Consider handling potential serialization errors gracefully instead of using expect_infallible(), which will panic if serialization fails. This can improve the robustness of the code by allowing it to handle errors more gracefully. src/eth/primitives/transaction_stage.rs [26] -serde_json::to_value(json_rpc_payload).expect_infallible() +serde_json::to_value(json_rpc_payload).unwrap_or_else(|e| { + log::error!("Failed to serialize transaction: {}", e); + JsonValue::Null +}) Suggestion importance[1-10]: 9 Why: This suggestion addresses a potential panic point in the code, improving robustness and error handling, which is crucial for production systems.	9
Maintainability	Refactor transaction reading code to reduce duplication and improve maintainability Refactor the repeated code for reading transactions from temporary and permanent storage into a separate method to improve code maintainability and reduce duplication. src/eth/storage/stratus_storage.rs [355-366] -let temp_tx = timed(|| self.temp.read_transaction(hash)).with(|m| { - metrics::inc_storage_read_transaction(m.elapsed, label::TEMP, m.result.is_ok()); -})?; -if let Some(tx_temp) = temp_tx { - return Ok(Some(TransactionStage::new_executed(tx_temp))); +let transaction = self.read_transaction_from_storage(hash)?; +if let Some(tx) = transaction { + return Ok(Some(tx)); } -let perm_tx = timed(|| self.perm.read_transaction(hash)).with(|m| { - metrics::inc_storage_read_transaction(m.elapsed, label::PERM, m.result.is_ok()); -})?; Suggestion importance[1-10]: 7 Why: This suggestion improves code maintainability by reducing duplication, making the codebase easier to manage and understand, although it does not address a critical bug.	7
Enhancement	Improve enum variant names for better code clarity and readability Use a more descriptive name for the TransactionStage variants to improve code readability and maintainability. src/eth/primitives/transaction_stage.rs [14-17] -Executed(TransactionExecution), -Mined(TransactionMined), +AwaitingMining(TransactionExecution), +AddedToBlock(TransactionMined), Suggestion importance[1-10]: 6 Why: This suggestion enhances code readability and maintainability by using more descriptive names, which is beneficial but not critical.	6