-
Notifications
You must be signed in to change notification settings - Fork 286
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
feat: support no_std for alloy-genesis/alloy-serde #320
Conversation
crates/serde/src/json_u256.rs
Outdated
@@ -1,5 +1,6 @@ | |||
//! Json U256 serde helpers. | |||
|
|||
#[allow(unused_imports)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems the clippy warnings are not correct, for no_std must import String
/ToString
.
If not imports them, cargo test -p alloy-serde --no-default-features
will fail.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can feature gate these imports instead:
see also
https://github.com/alloy-rs/alloy/blob/main/crates/eips/src/eip2930.rs#L7-L8
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
supportive
smol nits, replace unused_imports with #[cfg(not(feature = "std"))]
@@ -226,7 +230,7 @@ pub struct GenesisAccount { | |||
skip_serializing_if = "Option::is_none", | |||
deserialize_with = "deserialize_storage_map" | |||
)] | |||
pub storage: Option<HashMap<B256, B256>>, | |||
pub storage: Option<BTreeMap<B256, B256>>, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually prefer the BtreeMap change here so serialize/deserialize is consistent
crates/serde/src/json_u256.rs
Outdated
@@ -1,5 +1,6 @@ | |||
//! Json U256 serde helpers. | |||
|
|||
#[allow(unused_imports)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can feature gate these imports instead:
see also
https://github.com/alloy-rs/alloy/blob/main/crates/eips/src/eip2930.rs#L7-L8
crates/serde/src/json_u256.rs
Outdated
#[allow(unused_imports)] | ||
use alloc::{vec, vec::Vec}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm,
pending @DaniPopes
Co-authored-by: Oliver Nordbjerg <onbjerg@users.noreply.github.com>
feat: support no_std for alloy-genesis/alloy-serde (alloy-rs#320)
Motivation
Solution
PR Checklist