starknet_patricia_storage: define storage task trait#13629
starknet_patricia_storage: define storage task trait#13629nimrod-starkware wants to merge 1 commit intonimrod/parallel-reads/single-trie-refactorfrom
Conversation
|
This change is |
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, have a team admin enable autofix in the Cursor dashboard.
| #[async_trait] | ||
| pub trait StorageTask<'a, S: ImmutableReadOnlyStorage + 'a>: StorageTaskOutput<S> + Send { | ||
| async fn run_with_storage(self, storage: &mut ReadsCollectorStorage<'a, S>) -> Self::Output; | ||
| } |
There was a problem hiding this comment.
Inconsistent async pattern adds unnecessary boxing overhead
Low Severity
The StorageTask trait uses #[async_trait] (which boxes the returned future), while every other async trait in this same file deliberately uses RPITIT (-> impl Future<...> + Send) — with seven explicit comments explaining that design choice. This introduces unnecessary heap allocation per task execution, adds a new async-trait proc-macro dependency to the crate, and breaks the file's established convention. The same impl Future desugaring pattern used by ImmutableReadOnlyStorage, ReadOnlyStorage, and Storage works here too.
Additional Locations (1)
2 participants
Note
Medium Risk
Adds new public async traits and a new dependency (
async-trait), expanding the library API surface and affecting downstream implementors/compile behavior.Overview
Introduces a new abstraction for batching/concurrently executing storage read work by adding
StorageTaskOutputand asyncStorageTasktraits, which run against aReadsCollectorStoragewrapper.Updates
starknet_patricia_storagedependencies (async-trait) and lockfile entries to support async trait methods.Written by Cursor Bugbot for commit 2d05567. This will update automatically on new commits. Configure here.