Skip to content

Conversation

@mikemillaa
Copy link

closes #227

not sure if 100% solve it but handle the case of missing CODSPEED_TOKEN more elegantly.

emhane and others added 30 commits October 20, 2025 13:23
…p-rs#163)

@dhyaniarun1993 @itschaindev @sadiq1971 @meyer9 think these are the only
crates we will need to touch. can update as we go. ideally the new logic
will only live inside of _crates/optimism_, but we may need to
generalise some components in the other crates I listed and make them
customisable up at SDK (node builder _crates/node_) level to achieve
this.
Closes op-rs#179

---------

Co-authored-by: Julian Meyer <[email protected]>
Fixes op-rs#177 

I added a few methods that should allow atomically pruning and reorging
blocks. Each function should represent a single transaction. We can
store trie nodes, reorg, prune in a single transaction now. I kept the
bulk insert methods available for storing the current state.

---------

Co-authored-by: Arun Dhyani <[email protected]>
Ref op-rs#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]>
Based on op-rs#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]>
No token, so this check always fails annoyingly. Disabling the upload
step if missing token.
Description:

- This PR adds scaffold MDBX proofs storage with impl to unblock
parallel work.
- Unblock multiple developers and avoid merge conflicts while the MDBX
backend is being built.
- Provide a compilable stub that wires types and env init, without
committing to final storage logic.
…e` (op-rs#204)

- Adds new crates `reth-optimism-exex` and `reth_optimism_trie`
- Moves `reth_exex::external_proofs` -> `reth_optimism_exex`
- Moves `reth_exex::external_proofs::storage` -> `reth_optimism_trie`
Seems like when `reth-optimism-trie` depends on `reth-db-api`
indirectly, the doctest crate couldn’t resolve `reth_db_api::…` and
failing.

Closes op-rs#233

Co-authored-by: Emilia Hane <[email protected]>
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 op-rs#236

---------

Co-authored-by: Arun Dhyani <[email protected]>
Based on op-rs#203 ,
op-rs#204

This PR implements `StateProvider` given a `OpProofsStorage` instance.
It reads most data from the external database, but falls back on the
latest provider for block hashes and code by hash similar to the
existing historical provider in Reth.

This is an important part to implementing live syncing since we're
running the sync process on the DB being created.

In the Reth implementation, `proof.rs` is contained in `trie/db`, so I
think it makes sense to go in our DB crate. The `provider.rs` is in the
`reth-provider` crate, but I don't think a separate provider crate helps
us here, so I think we should also include that in the trie crate as
well.

---------

Co-authored-by: Emilia Hane <[email protected]>
Based on op-rs#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 op-rs#238
Based on op-rs#197

This PR implements the live state collector on top of the
`OpProofsStateProvider` created in the previous PR. This sync process
tries to re-execute all blocks from the current block of the external
database to tip.

---------

Co-authored-by: Emilia Hane <[email protected]>
Fixes op-rs#225 (does not track errors as these are fatal currently)
Fixes op-rs#237 

Adds more concrete error variants for `OpProofsStorageError`

Co-authored-by: Arun Dhyani <[email protected]>
Fixes op-rs#247 

Removes the `block_number` parameter from:
- store_account_branches
-  store_storage_branches
-  store_hashed_accounts
-  store_hashed_storages

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

Ref op-rs#241

- Fixes comment from deps fix in
op-rs#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`.
Signed-off-by: Ignacio Hagopian <[email protected]>
Co-authored-by: Galoretka <[email protected]>
Co-authored-by: YK <[email protected]>
Co-authored-by: Federico Gimenez <[email protected]>
Co-authored-by: Matthias Seitz <[email protected]>
Co-authored-by: Arsenii Kulikov <[email protected]>
Co-authored-by: sashass1315 <[email protected]>
Co-authored-by: radik878 <[email protected]>
Co-authored-by: drhgencer <[email protected]>
Co-authored-by: James Prestwich <[email protected]>
Co-authored-by: Copilot <[email protected]>
Co-authored-by: yongkangc <[email protected]>
Co-authored-by: Alexey Shekhirin <[email protected]>
Co-authored-by: Claude <[email protected]>
Co-authored-by: GarmashAlex <[email protected]>
Co-authored-by: Brian Picciano <[email protected]>
Co-authored-by: Roman Hodulák <[email protected]>
Co-authored-by: Dan Cline <[email protected]>
Co-authored-by: Mablr <[email protected]>
Co-authored-by: Skylar Ray <[email protected]>
Co-authored-by: kevaundray <[email protected]>
Co-authored-by: Wolfgang Welz <[email protected]>
Co-authored-by: Karl Yu <[email protected]>
Co-authored-by: MozirDmitriy <[email protected]>
Co-authored-by: AJStonewee <[email protected]>
Co-authored-by: Avory <[email protected]>
Co-authored-by: stevencartavia <[email protected]>
Co-authored-by: Léa Narzis <[email protected]>
Co-authored-by: futreall <[email protected]>
Co-authored-by: maradini77 <[email protected]>
Co-authored-by: crazykissshout <[email protected]>
Co-authored-by: Dharm Singh <[email protected]>
Co-authored-by: leopardracer <[email protected]>
Co-authored-by: Ignacio Hagopian <[email protected]>
Co-authored-by: Dmitry <[email protected]>
Co-authored-by: Micke <[email protected]>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-merge-queue <[email protected]>
Ref op-rs#205

Removes redundant dep `reth-db-api` from `reth-optimism-trie`, since
used types are re-exported via `reth-db`

---------

Co-authored-by: Arun Dhyani <[email protected]>
Resolves op-rs#239

---------

Co-authored-by: Einar Rasmussen <[email protected]>
Comment on lines +19 to +21
# Set the CODSPEED_TOKEN as an environment variable at the job level
env:
CODSPEED_TOKEN: ${{ secrets.CODSPEED_TOKEN }}
Copy link
Member

Choose a reason for hiding this comment

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

this change only makes sense if we move the token check to the top of this job, i.e. exit as early as possible

Copy link
Author

Choose a reason for hiding this comment

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

Moved token check to the first step and added conditional checks to all subsequent steps to exit early when CODSPEED_TOKEN is not available, avoiding unnecessary CI resource usage.

@mikemillaa mikemillaa requested a review from emhane October 21, 2025 09:41
@mikemillaa mikemillaa requested a review from emhane October 21, 2025 10:14
Copy link
Member

@emhane emhane left a comment

Choose a reason for hiding this comment

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

nice thanks!

if [ -z "$CODSPEED_TOKEN" ]; then
echo "::notice::CODSPEED_TOKEN is not set, skipping benchmarks"
echo "This is expected for forks. Benchmarks only run in the upstream repository."
exit 0
Copy link
Member

Choose a reason for hiding this comment

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

the check fails in latest CI run,so maybe exit 0 doesn't work here like i expected. wdyt @dhyaniarun1993 ?

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

Labels

A-ci Area: CI K-bug Kind: 🐛

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fix benches workflow

9 participants