Skip to content

fix: Added CI back for check_no_std with compilation to wasm32#9347

Closed
puma314 wants to merge 2 commits intoparadigmxyz:mainfrom
sp1-patches:uma/no_std_compilation
Closed

fix: Added CI back for check_no_std with compilation to wasm32#9347
puma314 wants to merge 2 commits intoparadigmxyz:mainfrom
sp1-patches:uma/no_std_compilation

Conversation

@puma314
Copy link

@puma314 puma314 commented Jul 6, 2024

Previously the CI check for check_no_std was commented out, as the riscv32imac-unknown-none-elf target doesn't come with std and there are many dependencies in reth that don't support no_std, making it difficult to compile to this target.

However, there are several embedded environments (namely zkVMs like SP1) that do support std, but do not (easily) support crates with embedded C++ dependencies (like secp256k1, or blst or z-std) and also do not support networking crates like socket. The zkVM embedded environment is therefore similar to a target like wasm32-wasi which comes with std, but also does not support crates like secp256k1, blst and does not support networking crates.

This PR adds back the no_std CI check but with the wasm32-wasi target, which should be significantly easier to support within Reth and also will ensure that all relevant packages that might be used in a zkVM context can compile.

With this PR, reth-codecs and reth-primitives-traits compiles with no-default-features to wasm32-wasi. reth-primitives almost compiles with no-default-features, but I'm running into this weird error that I was hoping @JackG-eth could help with since I saw he did the replacement of thiserror_no_std::Error everywhere.

Compiling reth-primitives v1.0.0 (/Users/umaroy/Documents/reth/crates/primitives)
error[E0433]: failed to resolve: use of undeclared crate or module `std`
 --> crates/primitives/src/transaction/error.rs:6:1
  |
6 | pub enum InvalidTransactionError {
  | ^^^ use of undeclared crate or module `std`
  |
help: consider importing this module
  |
1 + use core::error;

error[E0433]: failed to resolve: use of undeclared crate or module `std`
  --> crates/primitives/src/transaction/error.rs:59:1
   |
59 | pub enum TransactionConversionError {
   | ^^^ use of undeclared crate or module `std`
   |
help: consider importing this module
   |
1  + use core::error;
   |

error[E0433]: failed to resolve: use of undeclared crate or module `std`
  --> crates/primitives/src/transaction/error.rs:70:1
   |
70 | pub enum TryFromRecoveredTransactionError {
   | ^^^ use of undeclared crate or module `std`
   |
help: consider importing this module
   |
1  + use core::error;

@puma314
Copy link
Author

puma314 commented Jul 6, 2024

@mattsse after this PR is merged, I can work on making reth-primitives in the workspace have default-features=false and then importing default features only in packages that need them. Then, it should be straightforward to add back reth-consensus, reth-db, reth-errors and all the other commented out packages in the check_no_std.sh CI test.

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.

this currently does not compile without default features because we need to use this combo:

thiserror-no-std = { workspace = true, default-features = false }

std = ["thiserror-no-std/std"]

"rand",
] }
], optional = true }
k256 = { version = "0.13.3", default-features = false, features = ["ecdsa"] }
Copy link
Collaborator

Choose a reason for hiding this comment

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

why do we need this if we're not using it?

Copy link
Member

@Rjected Rjected left a comment

Choose a reason for hiding this comment

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

This is cool! It would be great to have this in CI


for package in "${no_std_packages[@]}"; do
cmd="cargo +stable build -p $package --target riscv32imac-unknown-none-elf --no-default-features"
cmd="cargo +stable build -p $package --target wasm32-wasi --no-default-features"
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
cmd="cargo +stable build -p $package --target wasm32-wasi --no-default-features"
cmd="cargo +stable build -p $package --target wasm32-wasip1 --no-default-features"

wasm32-wasi is being renamed to wasm32-wasip1 and will be removed
rust-lang/compiler-team#607

@shekhirin
Copy link
Member

Superseded by #9430 and #9982

@shekhirin shekhirin closed this Aug 1, 2024
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.

4 participants