paradigmxyz / reth

Modular, contributor-friendly and blazing-fast implementation of the Ethereum protocol, in Rust
https://reth.rs/
Apache License 2.0
3.8k stars 1.03k forks source link

Fix various compilation errors for no-std in crates #9478

Open mattsse opened 2 months ago

mattsse commented 2 months ago

Describe the feature

crates that are offending cargo c --no-default-features

TODO

#[cfg(not(feature = "std"))]
use alloc::vec::Vec;

extern crate alloc

etc

Additional context

No response

martinezjorge commented 2 months ago

May I?

mattsse commented 2 months ago

assigned, usually the todos are using core instead of std and alloc over std

martinezjorge commented 1 month ago

I went through all the crates running the command below:

cargo c --no-default-features -p

and the following crates had compilation errors:

nkysg commented 1 month ago

It seems that jsonrpsee-types doesn't support no_std. https://github.com/paritytech/jsonrpsee/issues/1. so reth-rpc-types is tough

martinezjorge commented 1 month ago

It seems that jsonrpsee-types doesn't support no_std. paritytech/jsonrpsee#1. so reth-rpc-types is tough

I ran into that issue and was wondering how this would be approached. Would the approach be to make the ToRpcError trait even more generic?

use jsonrpsee_types::ErrorObject;

/// A trait to convert an error to an RPC error.
pub trait ToRpcError: std::error::Error + Send + Sync + 'static {
    /// Converts the error to a JSON-RPC error object.
    fn to_rpc_error(&self) -> ErrorObject<'static>;
}

I guess it depends on why we are returning an ErrorObject<'static>? Or could we implement our own version of ErrorObject that satisfies no_std and jsonrcpsee?

nkysg commented 1 month ago

I think we can fix other crates. In my opinion, maybe need to let jsonrpsee-types support no_std.

nkysg commented 1 month ago

Maybe we need add crate reth-db https://github.com/paradigmxyz/reth/blob/c3347f323c0a6d7784772624d91edebf4a76d36d/.github/assets/check_no_std.sh#L5-L9

martinezjorge commented 1 month ago

Yeah that one is erroring for me, I must have missed it or something was updated that caused it to offend. In any case, I've added reth-db to the list.

As more code is added to main there will likely be others until #9479 is closed. Let me know if you notice any others.

I'm currently working on reth-primitives, let me know what you're working on or when you start a new one so we don't duplicate efforts.

nkysg commented 1 month ago

Thanks @martinezjorge . Right now I am working on reth-db. After finishing reth-db, I want to try other issues.

citizen-stig commented 1 month ago

Hello!

I just want to point out, that it looks like that thiserror-no-std might be not the best candidate for usage.

Documentation page does not point to the actual repo, which is https://github.com/Stupremee/thiserror and it hasn't been updated in 2 years, which is a little bit concerning.

But most importantly, it does not work:

cargo check --no-default-features` on reth-primitives

info: running `cargo check --no-default-features` on reth-primitives (7/234)
    Checking reth-primitives v1.0.3 (/Users/nikolai/workspace/sovereign/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;

https://github.com/Stupremee/thiserror/blob/2fd08ccb500231c600083f88c4d2b68ad7024255/src/aserror.rs#L1

Considering that original author of thiserror is not keen on support of the no_std: https://github.com/dtolnay/thiserror/pull/64

It might be a good time to reconsider how this is handled in reth.

martinezjorge commented 3 weeks ago

@mattsse

All of the crates pass cargo c -p <package_name> --no-default-features so I'm thinking we can close this issue

martinezjorge commented 2 weeks ago

@mattsse is there something I'm missing here? I'd be happy to keep working on it, just let me know