feat: implement in-memory storage backend #183

meyer9 · 2025-09-26T17:27:22Z

This PR adds an in-memory storage backend and tests that work with any storage backend. We can easily add more test cases to the file to test against a future SQLite backend.

wiz-b4c72f16a4 · 2025-10-08T15:50:10Z

Wiz Scan Summary

Scanner	Findings
Vulnerabilities	-
Sensitive Data	-

Total	-

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

dhyaniarun1993 · 2025-10-09T10:00:46Z

crates/exex/external-proofs/src/in_memory.rs

+        let mut collected_entries: Vec<(Nibbles, BranchNodeCompact)> =
+            if let Some(addr) = hashed_address {
+                // Storage trie cursor
+                for ((block, address, path), branch) in &storage.storage_branches {


Shouldn't we merge the changes from trie_updates for all blocks up to the specified max_block_number?

I think this should merge them all already. The BTreeMap is sorted by block number ASC, so when we collect all the entries, it will choose the latest block <= max_block_number.

If I understand correctly, we are storing live updates only in the trie_updates BTreeMap. We aren't saving data to storage_branches BTreeMap in the live syncing.

Hence live updates changes won't be visible to the cursor since cursor is only reading storage_branches BTreeMap

ah I see, that should be saved to *_branches and hashed_*. The trie_updates and post_states are just indexes from block number to the slots/trie nodes changed for pruning/reorgs.

fixed: fa65493

Thanks. Will review it.

dhyaniarun1993 · 2025-10-09T10:01:31Z

crates/exex/external-proofs/src/in_memory.rs

+                    .collect()
+            } else {
+                // Account trie cursor
+                for ((block, path), branch) in &storage.account_branches {


emhane · 2025-10-09T10:48:35Z

does this pr reference any issue?

Copilot

Pull Request Overview

This PR implements an in-memory storage backend for external trie node storage and creates a comprehensive test suite that can work with any storage backend implementation. The changes enable easier testing and development of the external proofs feature while providing a foundation for adding future storage backends like SQLite.

Key changes:

Implements a complete in-memory storage backend with thread-safe operations using tokio RwLock
Creates an extensive test suite covering all storage operations, cursor functionality, and edge cases
Adds proper documentation and comments to the storage trait interfaces

Reviewed Changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
`crates/exex/external-proofs/src/in_memory.rs`	Complete in-memory implementation of ExternalStorage trait with cursors for accounts, storage, and trie branches
`crates/exex/external-proofs/src/storage_tests.rs`	Comprehensive test suite covering all storage operations, cursor navigation, block versioning, and edge cases
`crates/exex/external-proofs/src/storage.rs`	Added documentation comments to the storage trait and BlockStateDiff struct
`crates/exex/external-proofs/src/lib.rs`	Added module declarations for in_memory and storage_tests
`crates/exex/external-proofs/Cargo.toml`	Added tokio dependency and test dependencies with feature flags

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

Copilot · 2025-10-10T06:38:08Z

crates/exex/external-proofs/src/storage_tests.rs

+#[cfg(test)]
+mod tests {


The entire file is wrapped in a #[cfg(test)] module, but the file itself is already conditionally included only for tests via #[cfg(test)] in lib.rs. This creates redundant test-only compilation guards.

Copilot · 2025-10-10T06:38:09Z

crates/exex/external-proofs/src/in_memory.rs

+        let inner = self
+            .inner
+            .try_read()
+            .map_err(|_| ExternalStorageError::Other(eyre::eyre!("Failed to acquire read lock")))?;


Using try_read() in synchronous cursor methods could fail under contention. Consider using blocking read() or redesigning the API to be async, as lock contention in this context likely indicates a design issue rather than a recoverable error.

Copilot · 2025-10-10T06:38:09Z

crates/exex/external-proofs/src/in_memory.rs

+        let inner = self
+            .inner
+            .try_read()
+            .map_err(|_| ExternalStorageError::Other(eyre::eyre!("Failed to acquire read lock")))?;


Same lock contention issue as in trie_cursor. Using try_read() in synchronous cursor methods could fail under contention. Consider using blocking read() or redesigning the API to be async.

Copilot · 2025-10-10T06:38:09Z

crates/exex/external-proofs/src/in_memory.rs

+        let inner = self
+            .inner
+            .try_read()
+            .map_err(|_| ExternalStorageError::Other(eyre::eyre!("Failed to acquire read lock")))?;


Same lock contention issue as in previous cursor methods. Using try_read() in synchronous cursor methods could fail under contention. Consider using blocking read() or redesigning the API to be async.

dhyaniarun1993 · 2025-10-10T07:10:20Z

crates/exex/external-proofs/src/in_memory.rs

+        blocks_to_add: HashMap<u64, BlockStateDiff>,
+    ) -> ExternalStorageResult<()> {
+        let mut inner = self.inner.write().await;
+


Here as well, we need to update account_branches, storage_branches, hashed_accounts and hashed_storage.

dhyaniarun1993 · 2025-10-10T07:12:30Z

crates/exex/external-proofs/src/in_memory.rs

+        }
+
+        for (hashed_address, storage) in &block_state_diff.post_state.storages {
+            for (slot, value) in &storage.storage {


I think it's better to move the wiped logic here. Let me know what you think.

good idea, moved this to a common fn: 65e441e

meyer9 · 2025-10-10T16:07:10Z

Clippy does not recognize the lint as being used even though it definitely is (removing it causes warnings). I had to use #[allow(dead_code)] instead to fix this, but it will be removed very soon anyway. Seems like a bug in Clippy

Based on #183 This PR adds a backfill job that accepts a DB transaction and copies the current state to the database. The transaction ensures we see a consistent view of the database at the current block, even if the node is syncing. This requires `--db.read-transaction-timeout 0`. This currently doesn't handle interrupting the job because the state may update while syncing and may read a different version of the database upon restart. --------- Co-authored-by: Arun Dhyani <[email protected]> Co-authored-by: Emilia Hane <[email protected]>

Ref #176 This PR adds an in-memory storage backend and tests that work with any storage backend. We can easily add more test cases to the file to test against a future SQLite backend. --------- Co-authored-by: Arun Dhyani <[email protected]> Co-authored-by: Emilia Hane <[email protected]>