chore: separate storage and account cursors #229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merged

emhane merged 4 commits into unstable from meyer9/split-storage-account-cursor

Oct 15, 2025

Collaborator

meyer9 commented Oct 14, 2025 •

edited

Loading

This PR separates storage and account cursors which makes sense because these two cursors may have very different seek/next implementations. In the case of in-memory, a single cursor impl can still handle both, but for MDBX, it makes sense to implement separate cursors.

Closes #236


          chore: separate storage and account cursors

2bb5a36

Copilot AI review requested due to automatic review settings

October 14, 2025 20:40

meyer9 requested review from dhyaniarun1993, emhane, itschaindev and sadiq1971 as code owners

October 14, 2025 20:40

Copilot AI reviewed

View reviewed changes

Copilot AI left a comment

Pull Request Overview

This PR separates the storage and account cursors into distinct APIs to allow for specialized implementations in different storage backends like MDBX while maintaining compatibility with in-memory implementations.

Replaces single trie_cursor method with separate account_trie_cursor and storage_trie_cursor methods
Updates trait definitions to include separate cursor types for storage and account tries
Migrates all test code to use the new cursor-specific methods

Reviewed Changes

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

Show a summary per file

File	Description
crates/optimism/trie/src/api.rs	Defines separate cursor types and methods for storage/account tries
crates/optimism/trie/src/in_memory.rs	Implements new cursor methods for in-memory storage
crates/optimism/trie/src/db/store.rs	Updates MDBX implementation with separate cursor methods
crates/optimism/trie/tests/in_memory.rs	Updates all test calls to use specific cursor methods
crates/optimism/trie/src/backfill.rs	Updates backfill operations to use new cursor methods

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

meyer9 mentioned this pull request

chore: add deletion tests for op proofs storage #230

Merged

dhyaniarun1993 approved these changes

View reviewed changes

emhane approved these changes

View reviewed changes

crates/optimism/trie/src/in_memory.rs Show resolved Hide resolved

crates/optimism/trie/src/db/store.rs Show resolved Hide resolved

Member

emhane commented Oct 15, 2025

also pr description is missing which issue it references/closes

sadiq1971 approved these changes

View reviewed changes


          Merge branch 'unstable' into meyer9/split-storage-account-cursor

53f693b

Collaborator Author

meyer9 commented Oct 15, 2025 •

edited

Loading

~~fix for this upstream: paradigmxyz#19043~~

Actually decided to just import optimism primitives here with serde-bincode-compat.


          fix: optimism primitives should include serde-bincode-compat

b918de4

meyer9 requested review from BioMark3r and theochap as code owners

October 15, 2025 16:44

wiz-b4c72f16a4 bot commented Oct 15, 2025 •

edited

Loading

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.


          chore: add explanation to weird import

6f3d862

emhane approved these changes

View reviewed changes

crates/optimism/exex/Cargo.toml

Comment on lines +22 to +24

    
              # for ethereum types, `serde-bincode-compat` is added by `reth-storage-api`, however this does not work with `op` until

              # `reth-storage-api` is updated to support `op`, so we add it here.

              reth-optimism-primitives = { workspace = true, features = ["reth-codec", "serde-bincode-compat", "serde"] }

Member

emhane Oct 15, 2025

hopefully this is solved in #242 so that we don't have to mess with the manifests outside of the optimism directory for this op feature

emhane added this pull request to the merge queue

Merged via the queue into unstable with commit 040a33b

40 checks passed

emhane deleted the meyer9/split-storage-account-cursor branch

October 15, 2025 17:47

github-merge-queue bot pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

f098118

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane mentioned this pull request

feat(deps): Add serde-bincode-compat feature to reth-trie #250

Merged

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

d552c79

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

3313a87

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <[email protected]>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

b6f2830

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

meyer9 added a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

ff8298c

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <[email protected]>

meyer9 added a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

6308e7b

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

meyer9 pushed a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

f95574c

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

3500f40

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <[email protected]>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

b364603

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

53212f4

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

meyer9 added a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

b10a9c1

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <[email protected]>

meyer9 added a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

13fcb26

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

meyer9 pushed a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

23b9c3e

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

22c0f5f

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <[email protected]>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

b04564f

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

746ad4e

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

emhane pushed a commit that referenced this pull request


          chore: separate storage and account cursors (#229)

cf79096

This PR separates storage and account cursors which makes sense because
these two cursors may have very different seek/next implementations. In
the case of in-memory, a single cursor impl can still handle both, but
for MDBX, it makes sense to implement separate cursors.

Closes #236

---------

Co-authored-by: Arun Dhyani <[email protected]>

emhane pushed a commit that referenced this pull request


          chore: add deletion tests for op proofs storage (#230)

081705b

Based on #229 

Adds tests to ensure that storing TrieUpdates that include deletions
actually deletes the nodes at that block height, and that updates take
precedence over deletions (same as `write_trie_updates` in Reth).

Closes #238

emhane added a commit that referenced this pull request


          feat(deps): Add serde-bincode-compat feature to reth-trie (#250)

3bb0901

Ref #241

- Fixes comment from deps fix in
#229 (fix correct but comment
misleading)
- `serde-bincode-compat` is a feature which must be enabled when the
`bincode` dependency is used. Asides for dev-deps, this only happens in
`reth-stages`. `reth-exex` runs in `reth-stages`. In this wokrstream we
use the `TrieUpdate` in exex. `serde-bincode-compat` is already
implemented for `TrieUpdates` in `reth-trie-common`, this PR simply
makes that feature accessible via `reth-trie` and in turn
`reth-optimism-trie`, in order to enable it in `reth-optimism-exex`.

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

Reviewers

Copilot code review Copilot Copilot left review comments

sadiq1971 sadiq1971 approved these changes

dhyaniarun1993 dhyaniarun1993 approved these changes

emhane emhane approved these changes

itschaindev Awaiting requested review from itschaindev itschaindev is a code owner

theochap Awaiting requested review from theochap theochap is a code owner

BioMark3r Awaiting requested review from BioMark3r BioMark3r is a code owner

Labels

None yet