feat: support no_std for alloy-eips
#181
Conversation
yjhmelody
requested review from
DaniPopes,
Evalir,
gakonst and
prestwich
as code owners
There was a problem hiding this comment.
lg on the no-std changes
though about the error type. I'm skeptical that this even belongs in there if it's only used in the consensus crate
https://github.com/search?q=repo%3Aalloy-rs%2Falloy%20Eip2718Error&type=code
we should move it there imo
wdyt @DaniPopes
Cargo.toml
Outdated
| @@ -83,10 +81,11 @@ rand = "0.8.5" | |||
| reqwest = { version = "0.11.18", default-features = false } | |||
| semver = "1.0" | |||
| thiserror = "1.0" | |||
| thiserror-no-std = { version = "2.0", default-features = false } | |||
| /// Some other error occurred. | ||
| #[error(transparent)] | ||
| Custom(#[from] Box<dyn std::error::Error>), |
There was a problem hiding this comment.
I think eventually we'd need this, so perhaps the better question for this PR is, does this type even belong in here?
perhaps we should rather move this to the consensus crate instead, I believe it's better suited in there
There was a problem hiding this comment.
What do you mean?
Move all eip error type to consensus crate? and add Error type to trait such as Decodable2718?
There was a problem hiding this comment.
But that might be a break change, which the current one is not? Maybe it should be improved in subsequent versions?
There was a problem hiding this comment.
removing a variant is a breaking change as well, I'd really prefer moving the error. imo it does not belong in the eip crate
| #[derive(Debug, Copy, Clone)] | ||
| pub enum Eip2718Error { |
There was a problem hiding this comment.
I still think we should move this error type and keep the custom variant
There was a problem hiding this comment.
I still think we should move this error type and keep the custom variant
i want to how tohandle the related trait
not opposed to removing it, the purpose of it is to allow for non-RLP encoding schemes in EIP-2718 in non-Ethereum networks. E.g. if Optimism decides to have a fallible SSZ encoding for some transaction type, the existing API can still capture that behavior via the |
|
'I have rebased this and it should go through re-review @mattsse |
|
cleaning up CI before merge 👍 |
|
nightly clippy strikes again |
|
ty for you contribution @yjhmelody, sorry for leaving it open so long 🙇♀️ |
And remove unused error
Custom