Skip to content

starknet_patricia_storage: remove code dup for mget#12175

Open
nimrod-starkware wants to merge 1 commit intonimrod/parallel-reads/spawn-based-on-configfrom
nimrod/parallel-reads/remove-dup
Open

starknet_patricia_storage: remove code dup for mget#12175
nimrod-starkware wants to merge 1 commit intonimrod/parallel-reads/spawn-based-on-configfrom
nimrod/parallel-reads/remove-dup

Conversation

@nimrod-starkware
Copy link
Contributor

@nimrod-starkware nimrod-starkware commented Feb 1, 2026

Note

Low Risk
Pure refactor confined to RocksDB batch-read code; behavior should be unchanged aside from potential edge-case differences in iterator/key types passed to multi_get.

Overview
Refactors RocksDbStorage::mget to remove duplicated multi_get/error-mapping logic by introducing a shared helper mget_from_raw_keys.

Both the spawn_blocking and direct-read branches now call this helper, keeping behavior the same while centralizing the result conversion and error handling.

Written by Cursor Bugbot for commit 4ef3e58. This will update automatically on new commits. Configure here.

@reviewable-StarkWare
Copy link

This change is Reviewable

Copy link
Contributor Author

nimrod-starkware commented Feb 1, 2026

@chatgpt-codex-connector
Copy link

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.

@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch from 1ab4b2e to 96e6b8b Compare February 2, 2026 08:36
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch from 1729e41 to b32ebca Compare February 2, 2026 09:09
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch 3 times, most recently from 9ead71a to 51f7b26 Compare February 3, 2026 14:59
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch from 58b2329 to 198f552 Compare February 3, 2026 14:59
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch from 198f552 to 48301ce Compare February 3, 2026 15:30
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch from 51f7b26 to 1f5af39 Compare February 3, 2026 15:30
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch from 48301ce to 6398d19 Compare February 3, 2026 15:58
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch from 1ab4ce9 to e03a849 Compare February 5, 2026 08:27
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch from 6da69c6 to 94c3443 Compare February 5, 2026 08:27
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch from e03a849 to 8222312 Compare February 9, 2026 14:27
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch from 94c3443 to 246f8b2 Compare February 9, 2026 14:27
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch 3 times, most recently from f3aca6a to 3e72115 Compare February 10, 2026 09:07
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch 2 times, most recently from 1e4407b to e990000 Compare February 10, 2026 09:44
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch from 3e72115 to 36a133f Compare February 10, 2026 09:44
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch from e990000 to e7cd356 Compare February 10, 2026 12:34
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch from 36a133f to ab3f029 Compare February 10, 2026 12:34
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch from ab3f029 to dc6c416 Compare February 17, 2026 07:13
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch from e7cd356 to 4a46808 Compare February 17, 2026 07:13
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/spawn-based-on-config branch from 4a46808 to 695d081 Compare February 19, 2026 09:21
@nimrod-starkware nimrod-starkware force-pushed the nimrod/parallel-reads/remove-dup branch from dc6c416 to 4ef3e58 Compare February 19, 2026 09:21
Copy link
Contributor

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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

    }

    fn mget_from_raw_keys<K: AsRef<[u8]>, RK: IntoIterator<Item = K>>(

consider the proposal from the previous PR:

fn multi_get(
    db: &DB,
    keys: impl IntoIterator<Item = impl AsRef<[u8]>>,
) -> Result<Vec<Option<DbValue>>, PatriciaStorageError> {
    db.multi_get(keys)
        .into_iter()
        .map(|r| r.map(|opt| opt.map(DbValue)).map_err(|e| e.into()))
        .collect()
}

no new generics here


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

        db.multi_get(raw_keys)
            .into_iter()
            .map(|r| r.map(|opt| opt.map(DbValue)).map_err(|e| e.into()))

I think things compile without it

Code quote:

map_err(|e| e.into()

Copy link
Contributor

@ArielElp ArielElp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

Comments