Skip to content

Comments

chore: clean up package features#228

Closed
meyer9 wants to merge 9 commits intounstablefrom
meyer9/remove-op-features
Closed

chore: clean up package features#228
meyer9 wants to merge 9 commits intounstablefrom
meyer9/remove-op-features

Conversation

@meyer9
Copy link
Collaborator

@meyer9 meyer9 commented Oct 14, 2025

Make op a toggleable feature instead and ensure that serde and reth-codec flags are enabled where needed.

Depends on paradigmxyz#19005 upstream

Copilot AI review requested due to automatic review settings October 14, 2025 20:33
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors the package feature configuration to make Optimism support toggleable rather than always enabled, improving modularity and reducing dependencies when OP features aren't needed.

  • Made op a feature flag that conditionally enables Optimism-related dependencies and features
  • Ensured serde and reth-codec features are explicitly enabled where required for proper serialization support
  • Added conditional dependency on reth-optimism-primitives only when the op feature is enabled

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

"dep:reth-optimism-primitives",
"reth-primitives-traits/op",
"reth-db/op",
"reth-codecs/op",
Copy link

Copilot AI Oct 14, 2025

Choose a reason for hiding this comment

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

The feature depends on reth-codecs/op but reth-codecs is only listed in [dev-dependencies], not in [dependencies]. This will cause a build error when the op feature is enabled in non-test builds.

Copilot uses AI. Check for mistakes.
@wiz-b4c72f16a4
Copy link

wiz-b4c72f16a4 bot commented Oct 14, 2025

Wiz Scan Summary

Scanner Findings
Vulnerability Finding Vulnerabilities -
Data Finding Sensitive Data -
Total -

View scan details in Wiz

To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension.

Copy link

@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.

the 'op' feature should actually always be enabled for all optimism crates in order for the InMemorySize implementation to return the correct result. i also opened and issue to move this trait to alloy so that it can be implemented in op-alloy instead of in reth and thereby remove the need for the feature in reth cc @mattsse

@emhane
Copy link

emhane commented Oct 15, 2025

we could have a toggle, but then it should be a default feature

Copy link

@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.

likely we have to add the op feature to all the trie crates (or move the InMemorySize trait to alloy cc @mattsse)

@emhane emhane mentioned this pull request Oct 15, 2025
@meyer9
Copy link
Collaborator Author

meyer9 commented Oct 15, 2025

I think we can close this for now

@meyer9 meyer9 closed this Oct 15, 2025
@emhane emhane deleted the meyer9/remove-op-features branch October 16, 2025 11:37
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.

2 participants