chore(sdk): Simplify generics syntax by using adapters #549
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
There was a problem hiding this comment.
Pull request overview
This PR simplifies generic syntax throughout the codebase by introducing and using the PrimitivesTy<N> type adapter instead of the fully qualified syntax <N as NodeTypes>::Primitives. The change improves code readability by reducing boilerplate in type declarations.
Key changes:
- Introduces
PrimitivesTytype adapter for cleaner access toNodeTypes::Primitives - Updates type bounds and associated types across node components, RPC handlers, and engine services
- Replaces verbose
<Node::Types as NodeTypes>::Primitivessyntax with concisePrimitivesTy<Node::Types>
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
crates/optimism/node/tests/it/builder.rs |
Updates test EVM configuration to use PrimitivesTy adapter |
crates/optimism/node/src/node.rs |
Updates EVM config and validator builder to use PrimitivesTy adapter |
crates/node/builder/src/rpc.rs |
Updates RPC handle types and engine validator traits to use PrimitivesTy adapter |
crates/node/builder/src/node.rs |
Adds import and updates NodeTypes implementation with PrimitivesTy adapter |
crates/node/builder/src/launch/invalid_block_hook.rs |
Updates invalid block hook extension trait to use PrimitivesTy adapter |
crates/node/builder/src/launch/common.rs |
Updates static file provider return type to use PrimitivesTy adapter |
crates/node/builder/src/components/mod.rs |
Updates component trait bounds to use PrimitivesTy adapter |
crates/node/api/src/node.rs |
Adds import and updates component traits and context to use PrimitivesTy adapter |
crates/engine/service/src/service.rs |
Updates engine service type aliases to use PrimitivesTy adapter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
emhane
changed the title
PrimitivesTy adapter
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
emhane
force-pushed
the
emhane/use-primitive-types-adapters
branch
from
c30e6f7 to
a105bfe
Compare
…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.
Fixes label name on bug issues
Closes #179 --------- Co-authored-by: Julian Meyer <[email protected]>
Fixes #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 #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 #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.
Closes #566 --------- Co-authored-by: Himess <[email protected]> Co-authored-by: Himess <[email protected]>
Ref #588 Adds tests for conversions to `OpProofsStorageError`
Closes #600 --------- Co-authored-by: Arun Dhyani <[email protected]> Co-authored-by: Sadiqur Rahman <[email protected]>
Ref #611 Reverts redundant import statement diff with stable to prepare for release
Closes #567 This PR improves the performance of the historical state pruning logic. By refining how state updates are handled and written during pruning, we observe a significant reduction in write latency. Performance Benchmarks: Comparison run using [load_test](https://github.com/op-rs/op-reth/blob/541e889e886f3ffbee275a15f25f738b2242266a/crates/optimism/trie/src/db/store.rs#L3462) Metric | Current (Baseline) | Optimised (This PR) | Improvement -- | -- | -- | -- Write Time | ~107ms | ~50ms | ~2.1x Faster Total Duration | ~129ms | ~81ms | ~1.6x Faster
Ref #611 Checks out deny.toml from stable branch
emhane
force-pushed
the
emhane/use-primitive-types-adapters
branch
from
58d4418 to
7c5ff98
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Ref #520
Simplifies generics in various places by instead of using fully qualified syntax to access associated type, using adapters
PrimitivesTyto access associated typeNodeTypes::PrimitivesTxTyto accessNodePrimitives::SignedTxReceiptTyto accessNodePrimitives::ReceiptBlockTyto accessNodePrimitives::BlockHeaderTyto accessNodePrimitives::Header