alloy-rs / core

High-performance, well-tested & documented core libraries for Ethereum, in Rust
https://alloy.rs
Apache License 2.0
763 stars 137 forks source link

[Bug] Alloy-sol-type-parser blocking compile of alloy crate #717

Closed CRossel87a closed 2 weeks ago

CRossel87a commented 2 weeks ago

Component

sol-type-parser

What version of Alloy are you on?

0.3.0

Operating System

Windows

Describe the bug

Compiling alloy-sol-type-parser v0.8.0 error[E0277]: the trait bound CustomError: StdError is not satisfied --> C:\Users\chris.cargo\registry\src\index.crates.io-6f17d22bba15001f\alloy-sol-type-parser-0.8.0\src\input.rs:39:13 | 39 | winnow::error::ErrMode::from_externalerror(input, winnow::error::ErrorKind::Eof, err) | ^^^^^^^^^^^^^^^^^^^^^^ the trait StdError is not implemented for CustomError, which is required by `ErrMode<>: FromExternalError<, >`

Running latest Rust version

CRossel87a commented 2 weeks ago

Ah, this only happens if you have ethers in the same dependency list

shuhuiluo commented 2 weeks ago

This issue is also bugging me. I see "the trait core::error::Error is not implemented for CustomError, which is required by ErrMode<_>: FromExternalError<_, _>".

Can confirm removing ethers solves the problem. Or add

#[cfg(not(feature = "std"))]
impl core::error::Error for CustomError {}

to alloy-sol-type-parser input.rs.

DaniPopes commented 2 weeks ago

are you using no_std? you need to enable alloy-sol-type-parser/std feature if winnow/std is also enabled

shuhuiluo commented 2 weeks ago

are you using no_std? you need to enable alloy-sol-type-parser/std feature if winnow/std is also enabled

I'm. But it doesn't compile with or without std feature.

DaniPopes commented 2 weeks ago

I dont believe that, the problem is a cfg(std) impl in winnow, you have to enable std in alloy as well for it to work

shuhuiluo commented 2 weeks ago

I dont believe that, the problem is a cfg(std) impl in winnow, you have to enable std in alloy as well for it to work

@DaniPopes Can you take a look at this branch? I can only make it compile by removing ethers or impl core::error::Error for CustomError.

Wollac commented 2 weeks ago

I can confirm that I see this problem as well. It seems to be an issue with feature unification/deduplication. The build fails when alloy is used with certain other dependencies that use toml_edit or winnow respectively. It does not matter if the std flag is given or not. The following is a minimal example that fails and only builds if the other dependency is removed:

[dependencies]
alloy = { version = "0.3.0" , features = ["full"] }
num_enum = "0.7.3"

or as mentioned by @shuhuiluo ethers:

[dependencies]
alloy = { version = "0.3.0" , features = ["full"] }
ethers = "2.0.14"
CRossel87a commented 2 weeks ago

I dont believe that, the problem is a cfg(std) impl in winnow, you have to enable std in alloy as well for it to work

@DaniPopes Can you take a look at this branch? I can only make it compile by removing ethers or impl core::error::Error for CustomError.

Does not compile with stable: error[E0658]: use of unstable library feature 'error_in_core' --> \crates\sol-type-parser\src\input.rs:28:6 | 28 | impl core::error::Error for CustomError {}

CRossel87a commented 2 weeks ago

I found a short term workaround for devs. This set of dependencies will compile:

[dependencies] alloy = { version = "0.2.1", features = ["full"] } ethers = "2.0.14" winapi = { version = "0.3.9", features = ["winerror"] }

Wollac commented 2 weeks ago

Downgrading alloy works, but this issue should also be fixed for the latest version.

DaniPopes commented 2 weeks ago

I have no idea what is going on here, because just importing alloy-sol-type-parser alone, or with winnow, with any feature combination works fine, but the moment you import alloy with full and num_enum it fails

it should work because std is enabled by default, which enables the required impls

it also doesnt make sense that there is another error type in the same crate with the same configuration, and doesnt error...

Wollac commented 2 weeks ago

It's a very strange issue indeed, and I thought I was going crazy trying to track it down. The only two options I can think of right now are to a) remove the no_std support entirely or b) find an alternative to #718 that does not break the MSRV.

CRossel87a commented 2 weeks ago

cheers

robriks commented 1 week ago

Another workaround is to avoid the use of Alloy-sol-type-parser by using the sol macro's syn-solidity parser instead of the solc standard json abi approach

DaniPopes commented 1 week ago

Those are two separate parsers for two different purposes, don't know what you're trying to achieve; alloy-sol-type-parser is used internally and the issue is fixed in >=0.8.1