Skip to content

fix(reth-evm-ethereum): no_std test compilation errors#10602

Closed
martinezjorge wants to merge 2 commits intoparadigmxyz:mainfrom
martinezjorge:reth-evm-ethereum-no_std-support
Closed

fix(reth-evm-ethereum): no_std test compilation errors#10602
martinezjorge wants to merge 2 commits intoparadigmxyz:mainfrom
martinezjorge:reth-evm-ethereum-no_std-support

Conversation

@martinezjorge
Copy link
Contributor

@martinezjorge martinezjorge commented Aug 29, 2024

Towards #10088

In reth-evm-ethereum

The tests fail when running:

cargo t -p reth-evm-ethereum --no-default-features

These changes fix the compilation issues

Copy link
Member

@shekhirin shekhirin left a comment

Choose a reason for hiding this comment

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

It's a step in the right direction, and we will need this. This doesn't fully fix the compilation for no_std, because the crate still depends on c-kzg.

reth-etl = { path = "crates/etl" }
reth-evm = { path = "crates/evm" }
reth-evm-ethereum = { path = "crates/ethereum/evm" }
reth-evm-ethereum = { path = "crates/ethereum/evm", default-features = false }
Copy link
Member

Choose a reason for hiding this comment

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

given that, we don't enable the std feature anywhere in the project. @mattsse we should propagate this change to all crates that use reth-evm-ethereum as a dependency, so they enable the std feature of it, right?

Copy link
Contributor Author

@martinezjorge martinezjorge Aug 29, 2024

Choose a reason for hiding this comment

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

The following dependencies in reth-evm-ethereum have c-kzg as default features:

  1. reth-primitives
  2. reth-revm

As far as I can tell, it seems that these are the main culprits; although, the rabbit hole may go deeper. Going off of #9430, would the move be to go from c-kzg -> kzg-rs in the reth-primitives and reth-revm?

@shekhirin shekhirin added C-enhancement New feature or request A-execution Related to the Execution and EVM labels Aug 29, 2024
@martinezjorge
Copy link
Contributor Author

martinezjorge commented Aug 29, 2024

@shekhirin

Thank you for the review.

I'm happy to wait to see what @mattsse says in regards to setting default-crates = false on crates that depend on reth-evm-ethereum.

I wasn't exactly aiming to progress #10088 in this PR but I'm happy to make it the focus. I was mostly aiming in this PR to just get the failing tests to pass when running it with the --no-default-features but maybe it affects things upstream of reth-evm-ethereum that I'm not aware of? It just seems to me that propagating default-features = false and wasm-waspi1 support are separate tasks or is that not the case? Would love to hear your thoughts.

In regards to wasm-wasip1 support, is the aim then for the command:

cargo +stable build -p reth-evm-ethereum --target wasm32-wasip1 --no-default-features

to run successfully?

@github-actions github-actions bot added the S-stale This issue/PR is stale and will close with no further activity label Sep 20, 2024
@github-actions github-actions bot closed this Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-execution Related to the Execution and EVM C-enhancement New feature or request S-stale This issue/PR is stale and will close with no further activity

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants