starknet_patricia_storage: spawn blocking tasks for rocksDB reads

[nimrod-starkware]

Note

Medium Risk
Changes the execution model and error surface of RocksDB reads; while localized, it can impact performance and failure modes (e.g., task join/cancellation errors) under load.

Overview
RocksDB read operations in RocksDbStorage (get/mget) are now executed via tokio::task::spawn_blocking to avoid blocking async executors, with results/errors propagated back through the async Storage API.

To support this, the crate adds a tokio dependency and extends PatriciaStorageError with a JoinError variant; DeserializationError::ValueError is tightened to require Send so it can cross task boundaries.

^{Written by Cursor Bugbot for commit 9fd588f. This will update automatically on new commits. Configure here.}

[reviewable-StarkWare]

This change is 

[nimrod-starkware]

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

• starknet_patricia_storage: spawn blocking tasks for rocksDB reads #12168 : 2 dependent PRs (#12169 , #12172 ) 👈 (View in Graphite)
• starknet_committer: parallel reads using unordered futures #12167
• starknet_committer: create sotrage tries sequentially #12166
• starknet_patricia,starknet_committer: dont pass storage mut when not needed #12165
• starknet_patricia_storage: non mut get in storage trait #12123 : 1 other dependent PR (#12252 )
• starknet_patricia_storage: dont cache reads in cached storage #12251
• starknet_patricia_storage: atomic counters for read stats #12122
• main-v0.14.2

This stack of pull requests is managed by Graphite. Learn more about stacking.

[chatgpt-codex-connector]

Codex usage limits have been reached for code reviews. Please check with the admins of this repo to increase the limits by adding credits.
Credits must be used to enable repository wide code reviews.

[cursor]

Cursor Bugbot has reviewed your changes and found 1 potential issue.

[cursor]

Redundant tokio dev-dependency after promoting to dependency

Low Severity

tokio.workspace = true now appears in both [dependencies] (line 34, newly added) and [dev-dependencies] (line 41, pre-existing). Since both use workspace = true with no additional features, the dev-dependency entry is now redundant — regular dependencies are automatically available in tests and benchmarks.

Additional Locations (1)

• crates/starknet_patricia_storage/Cargo.toml#L33-L34

 

[ArielElp]

@ArielElp reviewed 4 files and all commit messages, and made 2 comments.
Reviewable status: all files reviewed, 3 unresolved discussions (waiting on @nimrod-starkware).

---

crates/starknet_patricia_storage/src/rocksdb_storage.rs line 297 at r1 (raw file):

        let keys: Vec<Vec<u8>> = keys.iter().map(|k| k.0.clone()).collect();
        spawn_blocking(move || {
            let raw_keys = keys.iter().map(|k| k.as_slice());

why is this needed? can't multi_get just get keys?

---

crates/starknet_patricia_storage/src/rocksdb_storage.rs line 303 at r1 (raw file):

                .collect::<Result<Vec<_>, PatriciaStorageError>>()
        })
        .await?

why not use the old structure? hopefuly this avoids map_err and type annotations for the compiler in collect

        spawn_blocking(move || {
            let res = db
                .multi_get(keys)
                .into_iter()
                .map(|r| r.map(|opt| opt.map(DbValue)))
                .collect::<Result<_, _>>()?;

            Ok(res)
        })
        .await?

[ArielElp]

@ArielElp made 1 comment.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @nimrod-starkware).

---

crates/starknet_patricia_storage/src/storage_trait.rs line 41 at r1 (raw file):

    #[error(transparent)]
    Deserialization(#[from] DeserializationError),
    #[error(transparent)]

Why the new error, possible from awaiting spawn_blocking?

[nimrod-starkware]

@nimrod-starkware made 2 comments.
Reviewable status: 2 of 4 files reviewed, 4 unresolved discussions (waiting on ArielElp).

---

crates/starknet_patricia_storage/src/rocksdb_storage.rs line 297 at r1 (raw file):

Previously, ArielElp wrote…

why is this needed? can't multi_get just get keys?

When you call spawn_blocking, you can't pass references unless they have 'static lifetime. (this is why i need to clone)

---

crates/starknet_patricia_storage/src/storage_trait.rs line 41 at r1 (raw file):

Previously, ArielElp wrote…

Why the new error, possible from awaiting spawn_blocking?

exactly

[nimrod-starkware]

@nimrod-starkware made 1 comment.
Reviewable status: 2 of 4 files reviewed, 4 unresolved discussions (waiting on ArielElp).

---

crates/starknet_patricia_storage/src/rocksdb_storage.rs line 303 at r1 (raw file):

Previously, ArielElp wrote…

why not use the old structure? hopefuly this avoids map_err and type annotations for the compiler in collect

        spawn_blocking(move || {
            let res = db
                .multi_get(keys)
                .into_iter()
                .map(|r| r.map(|opt| opt.map(DbValue)))
                .collect::<Result<_, _>>()?;

            Ok(res)
        })
        .await?

This is because the new error variant that can happen as result of joining

[ArielElp]

@ArielElp reviewed 2 files and all commit messages, made 1 comment, and resolved 2 discussions.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on nimrod-starkware).

---

crates/starknet_patricia_storage/src/rocksdb_storage.rs line 303 at r1 (raw file):

Previously, nimrod-starkware wrote…

This is because the new error variant that can happen as result of joining

But PatriciaStorageError has two transparent variants, one for Rocks errors and one for join errors, so shouldn't ? work?