perf: remove split_transactions

[dinhani-cw]

PR Type

Enhancement

---

Description

• Removed the split_transactions method and related logic for performance optimization
• Simplified the mining process in both mine_external and mine_local methods
• Updated PendingBlock structure to use transactions instead of tx_executions
• Refactored block_from_local to handle cases with empty transaction lists
• Added a TODO comment regarding the source of block_timestamp in EvmInput
• Removed unnecessary logging and updated tracing in the miner module
• Updated references in InMemoryTemporaryStorage to match PendingBlock changes

---

Changes walkthrough 📝

Relevant files

Enhancement

evm_input.rs Add TODO for block timestamp source                                            src/eth/executor/evm_input.rs Added a TODO comment regarding the block_timestamp in EvmInput::from_transaction +1/-1      miner.rs Refactor mining process for performance                                    src/eth/miner/miner.rs Removed split_transactions usage Simplified mine_external and mine_local methods Updated block_from_local to handle empty transaction list Removed unnecessary logging and tracing +33/-39  pending_block.rs Simplify PendingBlock structure                                                    src/eth/primitives/pending_block.rs Renamed tx_executions to transactions Removed split_transactions method +2/-22    inmemory_temporary.rs Update references to match PendingBlock changes                    src/eth/storage/inmemory/inmemory_temporary.rs Updated references from tx_executions to transactions +2/-2

---

💡 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 Potential Bug The block_timestamp is now set to UnixTime::now() with a TODO comment. This might lead to inconsistent timestamps if not addressed properly. Performance Concern The mine_local and mine_external methods now iterate through all transactions twice, once to separate them and once to mine them. This could be optimized to a single pass. Code Smell The split_transactions method has been removed, which might affect other parts of the codebase that relied on this functionality.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Use a more appropriate source for the block timestamp instead of generating it on the spot Instead of using UnixTime::now() for the block_timestamp, consider passing this value as a parameter to the function or retrieving it from a more appropriate source. This will make the code more deterministic and easier to test. src/eth/executor/evm_input.rs [94] -block_timestamp: UnixTime::now(), // TODO: this should come from the pending block +block_timestamp: pending_block.timestamp, // Assuming pending_block has a timestamp field Suggestion importance[1-10]: 8 Why: The suggestion correctly identifies an important issue with using UnixTime::now() for block timestamps, which can lead to non-deterministic behavior and testing difficulties.	8
Enhancement	Improve error message clarity for non-external transactions in external blocks Consider using a more descriptive error message when a non-external transaction is found in an external block. This will make debugging easier. src/eth/miner/miner.rs [156] -return log_and_err!("failed to mine external block because one of the transactions is not an external transaction"); +return log_and_err!("Failed to mine external block: encountered a non-external transaction. All transactions in an external block must be external."); Suggestion importance[1-10]: 5 Why: The suggestion offers a minor improvement in error message clarity, which can aid in debugging but doesn't address a critical issue.	5
Performance	Add a check for empty transactions before processing them Consider adding a check for empty external_txs before calling mine_external_transactions. This can prevent unnecessary function calls and potential errors. src/eth/miner/miner.rs [159] -let mined_external_txs = mine_external_transactions(block.number, external_txs)?; +let mined_external_txs = if !external_txs.is_empty() { + mine_external_transactions(block.number, external_txs)? +} else { + Vec::new() +}; Suggestion importance[1-10]: 3 Why: While the suggestion aims to improve performance, it adds unnecessary complexity. The mine_external_transactions function likely handles empty input efficiently already.	3
	Use a more efficient data structure for storing transactions if order is not important Consider using a more efficient data structure for transactions if you frequently need to access transactions by their hash. A HashMap might be more suitable than an IndexMap if order is not important. src/eth/primitives/pending_block.rs [13] -pub transactions: IndexMap<Hash, TransactionExecution>, +pub transactions: HashMap<Hash, TransactionExecution>, Suggestion importance[1-10]: 2 Why: The suggestion is not well-founded. IndexMap is likely chosen for its ordered nature, which may be important for transaction processing order.	2