enha: redis historical values

[dinhani-cw]

PR Type

Enhancement

---

Description

• Added support for saving and retrieving historical values for accounts and slots in Redis storage.
• Introduced new functions key_account_history and key_slot_history for generating keys to access historical data.
• Modified the save_block function to use zadd for storing historical data in Redis.
• Enhanced the read_account and read_slot functions to handle historical data retrieval based on the StoragePointInTime parameter.

---

Changes walkthrough 📝

Relevant files

Enhancement

redis_permanent.rs Add support for historical values in Redis storage              src/eth/storage/redis/redis_permanent.rs Added support for saving and retrieving historical values for accounts and slots. Introduced new functions key_account_history and key_slot_history for generating history keys. Modified save_block to use zadd for storing historical data. Enhanced read_account and read_slot to handle historical data retrieval. +106/-27

---

💡 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: 4 🔵🔵🔵🔵⚪

🧪 No relevant tests

🔒 No security concerns identified

⚡ Key issues to review Missing Logging The save_block function spawns multiple Redis commands but lacks logging for these operations. Ensure that each Redis command (e.g., mset, zadd) has appropriate logging for better traceability. Use spawn_blocking_named The save_block function performs potentially blocking Redis operations within an async context. Consider using tokio::task::spawn_blocking_named to avoid blocking the Tokio runtime. Use expect instead of unwrap The code should use expect instead of unwrap to provide context in case of a panic. This is particularly important in the save_block function when calling self.conn(). Instrumentation Missing Fields The read_account and read_slot functions are instrumented with tracing::instrument, but they do not record any identifiers as span fields. Consider adding relevant fields for better traceability.

[github-actions]

PR Code Suggestions ✨

Category	Suggestion	Score
Performance	Reduce memory usage by cloning block_json once and using references Instead of cloning block_json multiple times, consider cloning it once and using references to the cloned value. This will reduce memory usage and improve performance. src/eth/storage/redis/redis_permanent.rs [83-87] +let block_json_clone = block_json.clone(); let mut mset_values = vec![ - (key_block_number, block_json.clone()), - (key_block_hash, block_json.clone()), + (key_block_number, block_json_clone.clone()), + (key_block_hash, block_json_clone.clone()), ("block::latest".to_owned(), block_json), ]; Suggestion importance[1-10]: 8 Why: This suggestion reduces memory usage and improves performance by avoiding multiple clones of the same data. It is a meaningful optimization.	8
	Combine the two for loops iterating over block.compact_account_changes() into one Combine the two for loops iterating over block.compact_account_changes() into one to improve readability and performance. src/eth/storage/redis/redis_permanent.rs [98-126] for changes in block.compact_account_changes() { // account let mut account = Account { address: changes.address, ..Account::default() }; if let Some(nonce) = changes.nonce.take() { account.nonce = nonce; } if let Some(bytecode) = changes.bytecode.take() { account.bytecode = bytecode; } let account_value = to_json_string(&account); mset_values.push((key_account(&account.address), account_value.clone())); zadd_values.push((key_account_history(&account.address), account_value, block.number().as_u64())); // slot - for slot in changes.slots.into_values() { - if let Some(slot) = slot.take() { - let slot_value = to_json_string(&slot); - mset_values.push((key_slot(&account.address, &slot.index), slot_value.clone())); - zadd_values.push((key_slot_history(&account.address, &slot.index), slot_value, block.number().as_u64())); - } + for slot in changes.slots.into_values().flatten() { + let slot_value = to_json_string(&slot); + mset_values.push((key_slot(&account.address, &slot.index), slot_value.clone())); + zadd_values.push((key_slot_history(&account.address, &slot.index), slot_value, block.number().as_u64())); } } Suggestion importance[1-10]: 7 Why: Combining the loops improves readability and performance by reducing the number of iterations. However, the improvement is minor as the loops are already nested.	7
Best practice	Add a check to ensure block.transactions is not empty before iterating Add a check to ensure that block.transactions is not empty before iterating over it. This will avoid unnecessary iterations and potential errors. src/eth/storage/redis/redis_permanent.rs [90-95] -for tx in &block.transactions { - let tx_key = key_tx(&tx.input.hash); - let tx_value = to_json_string(&tx); - mset_values.push((tx_key, tx_value)); +if !block.transactions.is_empty() { + for tx in &block.transactions { + let tx_key = key_tx(&tx.input.hash); + let tx_value = to_json_string(&tx); + mset_values.push((tx_key, tx_value)); + } } Suggestion importance[1-10]: 6 Why: Adding a check to ensure block.transactions is not empty before iterating is a good practice to avoid unnecessary iterations, though it is not critical since iterating over an empty collection is not harmful.	6
Maintainability	Use a constant for the string "ZRANGE" to avoid potential typos Use a constant for the string "ZRANGE" to avoid potential typos and make future modifications easier. src/eth/storage/redis/redis_permanent.rs [270] -let mut cmd = redis::cmd("ZRANGE"); +const ZRANGE_CMD: &str = "ZRANGE"; +let mut cmd = redis::cmd(ZRANGE_CMD); Suggestion importance[1-10]: 5 Why: Using a constant for the string "ZRANGE" improves maintainability by reducing the risk of typos and making future modifications easier. However, the impact is relatively small.	5