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

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

Open
mattsse opened this issue Jul 12, 2024 · 12 comments
Open

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

mattsse opened this issue Jul 12, 2024 · 12 comments
Assignees
Labels
C-debt Refactor of code section that is hard to understand or maintain D-good-first-issue Nice and easy! A great choice to get started

Comments

@mattsse
Copy link
Collaborator

mattsse commented Jul 12, 2024

Describe the feature

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

  • ethereum-forks
  • primitives
  • primitives-traits
  • ...

TODO

  • fix by doing various fix like
#[cfg(not(feature = "std"))]
use alloc::vec::Vec;

extern crate alloc

etc

Additional context

No response

@mattsse mattsse added C-enhancement New feature or request S-needs-triage This issue needs to be labelled D-good-first-issue Nice and easy! A great choice to get started C-debt Refactor of code section that is hard to understand or maintain and removed S-needs-triage This issue needs to be labelled labels Jul 12, 2024
@emhane emhane removed the C-enhancement New feature or request label Jul 12, 2024
@martinezjorge
Copy link
Contributor

May I?

@mattsse
Copy link
Collaborator Author

mattsse commented Jul 13, 2024

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

@martinezjorge
Copy link
Contributor

martinezjorge commented Jul 17, 2024

I went through all the crates running the command below:

cargo c --no-default-features -p <crate_name>

and the following crates had compilation errors:

@nkysg
Copy link
Contributor

nkysg commented Jul 18, 2024

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

@martinezjorge
Copy link
Contributor

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
Copy link
Contributor

nkysg commented Jul 18, 2024

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

@nkysg
Copy link
Contributor

nkysg commented Jul 18, 2024

Maybe we need add crate reth-db

no_std_packages=(
# reth-codecs
# reth-consensus
# reth-db
# reth-errors

@martinezjorge
Copy link
Contributor

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
Copy link
Contributor

nkysg commented Jul 18, 2024

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

@citizen-stig
Copy link
Contributor

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: dtolnay/thiserror#64

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

@martinezjorge
Copy link
Contributor

martinezjorge commented Aug 23, 2024

@mattsse

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

@martinezjorge
Copy link
Contributor

martinezjorge commented Aug 28, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-debt Refactor of code section that is hard to understand or maintain D-good-first-issue Nice and easy! A great choice to get started
Projects
Status: Todo
Development

Successfully merging a pull request may close this issue.

5 participants