fix: only update account databases when actually needed

[carneiro-cw]

Simple fix to only update the account databases in case an ExecutionAccountChange actually changes the account. Previously we were inserting a new entry in accounts_histry.rocksdb for every ExecutionAccountChange (even if for instance it only updated a single slot). In the future it might make sense to refactor ExecutionAccountChange to contain two structures, one for every change on the values of the account (bytecode, nonce, balance) and one only for the slots (and have them be Option<>)

[github-actions]

PR Review 🔍

⏱️ Estimated effort to review [1-5]	2, because the changes are localized to specific functions and the logic is straightforward. The PR modifies how account changes are handled to prevent unnecessary database updates, which is a relatively simple logic change.
🧪 Relevant tests	No
⚡ Possible issues	Possible Bug: The method is_account_change() checks for modifications in nonce, balance, or bytecode. However, if other properties of the account are modified (not covered by these three), the database will not update these changes. This could lead to inconsistencies if additional account properties are introduced in the future.
🔒 Security concerns	No

Code feedback:

---

relevant file	src/eth/storage/rocks/rocks_state.rs
suggestion	Consider adding a check for the is_account_update() condition before processing account changes. This would ensure that only existing accounts are updated, which aligns with the intended logic of checking account modifications. [important]
relevant line	if change.is_account_change() {

---

relevant file	src/eth/primitives/execution_account_changes.rs
suggestion	It might be beneficial to add logging inside the is_account_change() method to trace which properties are being modified. This would help in debugging and understanding the flow of account changes. [medium]
relevant line	pub fn is_account_change(&self) -> bool {

---

relevant file	src/eth/storage/rocks/rocks_state.rs
suggestion	To avoid cloning the nonce, balance, and bytecode multiple times (which occurs in the current implementation), consider restructuring the code to clone these properties only once per iteration. This could improve performance, especially with large numbers of account changes. [medium]
relevant line	if let Some(nonce) = change.nonce.clone().take_modified() {

---

relevant file	src/eth/storage/rocks/rocks_state.rs
suggestion	Implement unit tests to verify that the is_account_change() function correctly identifies when an account should be updated. This is crucial for ensuring the reliability of the new functionality. [important]
relevant line	if change.is_account_change() {

---

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Best practice	Use the Rust idiomatic ! operator for boolean negation Replace the use of not function with the ! operator for boolean negation to align with Rust's idiomatic practices. src/eth/primitives/execution_account_changes.rs [114] -not(self.new_account) +!self.new_account Suggestion importance[1-10]: 9 Why: This suggestion aligns with Rust's idiomatic practices and improves code readability. It is a minor change but enhances maintainability and consistency in the codebase.	9
Performance	Reduce cloning operations by cloning data once per iteration Avoid cloning the nonce, balance, and bytecode multiple times within the loop. Instead, clone them once per iteration before the if condition to reduce redundant operations. src/eth/storage/rocks/rocks_state.rs [404-411] -if let Some(nonce) = change.nonce.clone().take_modified() { +let modified_nonce = change.nonce.clone().take_modified(); +let modified_balance = change.balance.clone().take_modified(); +let modified_bytecode = change.bytecode.clone().take_modified(); +if let Some(nonce) = modified_nonce { account_info_entry.nonce = nonce.into(); } -if let Some(balance) = change.balance.clone().take_modified() { +if let Some(balance) = modified_balance { account_info_entry.balance = balance.into(); } -if let Some(bytecode) = change.bytecode.clone().take_modified() { +if let Some(bytecode) = modified_bytecode { account_info_entry.bytecode = bytecode.map_into(); } Suggestion importance[1-10]: 8 Why: This suggestion reduces redundant cloning operations, which can improve performance, especially in a loop. It is a practical optimization that enhances efficiency.	8
Enhancement	Add logging for better traceability of account updates Use structured logging to provide more context about the operations, especially useful for debugging and tracing the flow of account updates. src/eth/storage/rocks/rocks_state.rs [414-415] +log::info!("Account updated: {:?}", (address, account_info_entry.clone())); account_changes.push((address, account_info_entry.clone())); +log::info!("Account history updated: {:?}", ((address, block_number.into()), account_info_entry)); account_history_changes.push(((address, block_number.into()), account_info_entry)); Suggestion importance[1-10]: 7 Why: Adding structured logging can significantly aid in debugging and tracing the flow of account updates. This enhancement improves observability but is not critical for functionality.	7
Maintainability	Ensure atomic updates by using transactions for account changes Consider using a transaction or batch update mechanism to ensure atomicity and consistency when updating account_changes and account_history_changes. src/eth/storage/rocks/rocks_state.rs [414-415] -account_changes.push((address, account_info_entry.clone())); -account_history_changes.push(((address, block_number.into()), account_info_entry)); +let transaction = accounts.transaction(); +transaction.push((address, account_info_entry.clone())); +transaction.push_history(((address, block_number.into()), account_info_entry)); +transaction.commit(); Suggestion importance[1-10]: 6 Why: While using transactions can ensure atomicity and consistency, the suggested code does not directly align with the existing code structure and may require additional context to implement correctly. It is a good suggestion for maintainability but needs careful integration.	6