Skip to content
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.

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

Exploratory PR for checking cargo features for the most important crates #10130

Conversation

citizen-stig
Copy link
Contributor

Hello!

Sovereign team uses reth for building ZK rollups and for this we need crates reth-primitives and reth-rpc-types to be compiled without default features or with some minimal set of features.

This PR adds cargo hack to CI and makefile to validate that crates reth-primitives-traits, reth-primitives, reth-codecs and reth-rpc-typescan compile with all feature combinations.

These checks fail on recent main branch, so this PR also introduces fixes for it, described below

Main fixes

  • remove thiserror-no-std in favor of original thiserror and feature gate it behind std. Some details I've described in Fix various compilation errors for no-std in crates #9478 (comment) . But this PR does not remove it from whole repo, I leave it up to the reth team.
  • zstd is only supported together with std, as it is very hard to get zstd work in no_std
  • c-kzg is only supported together with std, for similar reason as zstd.
  • put all jsonrpsee related types in reth-rpc-types behind default feature flag, so it is possible to compile it for riscv without default features. This was mentioned in auto-closed issue Add CI check that reth-primitives compiles to RISC-V #8176

As I put it in the title, I treat this PR as exploratory, but I tried my best to make it mergable and the best quality.

I don't insist on any attribution for these changes, I am interested in having core idea of it in the main branch.
This means that reth team can copy paste any parts of this PR under their name in their own pace.

Copy link
Collaborator

@mattsse mattsse left a comment

Choose a reason for hiding this comment

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

thanks for looking into this,
very much supportive, but I'd like to do this incrementally.

I think we can start with the std feature changes in reth-primitives and then looking at the rpc stuff

@citizen-stig
Copy link
Contributor Author

citizen-stig commented Aug 6, 2024

thanks for looking into this, very much supportive, but I'd like to do this incrementally.

I think we can start with the std feature changes in reth-primitives and then looking at the rpc stuff

Sure @mattsse , I can split this into 4 PRs actually.

  1. Fixes in reth-codecs and make commend only for checking it Check features powerset in reth-codecs #10134
  2. Fixes in primitive types Check features powerset for reth-primitives #10138
  3. Fixes in rpc types Feature gate jsonrpsee related crates and check features powerset for reth-rpc-types #10141
  4. Adding CI check Add CI lint check using cargo hack for checking features powerset #10142

Does this plan work?

@citizen-stig
Copy link
Contributor Author

Closing as splitted PRs got merged

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