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.

Sign up for GitHub

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

Jump to bottom

chore: fix serde std flags for no-std build #987

Merged
merged 7 commits into from
Jan 18, 2024
Merged

chore: fix serde std flags for no-std build #987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
cbbfe9c
use serde as no-std
leonardoalt Nov 24, 2023
15c51fa
optional serde std
CeciliaZ030 Jan 18, 2024
6f95113
delete comment #"std"
CeciliaZ030 Jan 18, 2024
cf4da2f
"preserve_order"
CeciliaZ030 Jan 18, 2024
9a03c7e
Merge branch 'main' into serde-no-std
CeciliaZ030 Jan 18, 2024
2f0f19f
revme revert
CeciliaZ030 Jan 18, 2024
53d6e8b
Update bins/revme/Cargo.toml
rakita Jan 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions crates/interpreter/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ rustdoc-args = ["--cfg", "docsrs"]
revm-primitives = { path = "../primitives", version = "1.3.0", default-features = false }

# optional
serde = { version = "1.0", features = ["derive", "rc"], optional = true }
serde = { version = "1.0", default-features = false, features = ["derive", "rc"], optional = true }

[features]
default = ["std"]
std = ["revm-primitives/std"]
std = ["serde?/std", "revm-primitives/std"]
serde = ["dep:serde", "revm-primitives/serde"]
arbitrary = ["std", "revm-primitives/arbitrary"]
asm-keccak = ["revm-primitives/asm-keccak"]
Expand Down
5 changes: 3 additions & 2 deletions crates/primitives/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,14 +34,15 @@ enumn = "0.1"
derive_more = { version = "0.99", optional = true }

# optional
serde = { version = "1.0", features = ["derive", "rc"], optional = true }
serde = { version = "1.0", default-features = false, features = ["derive", "rc"], optional = true }

[build-dependencies]
hex = "0.4"
hex = { version = "0.4", default-features = false }

[features]
default = ["std", "c-kzg"]
std = [
"serde?/std",
"alloy-primitives/std",
"hex/std",
"bitvec/std",
Expand Down
6 changes: 3 additions & 3 deletions crates/revm/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,8 @@ revm-precompile = { path = "../precompile", version = "2.2.0", default-features
auto_impl = { version = "1.1", default-features = false }

# Optional
serde = { version = "1.0", features = ["derive", "rc"], optional = true }
serde_json = { version = "1.0", features = ["preserve_order"], optional = true }
serde = { version = "1.0", default-features = false, features = ["derive", "rc"], optional = true }
serde_json = { version = "1.0", default-features = false, features = ["alloc", "preserve_order"], optional = true }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rakita how critical is having preserve_order here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems not at all critical, I thought it is there to format output, but after checking, this is not the case. I am okay with removing it.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice, thanks!

Copy link
Member

@rakita rakita Jan 19, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just curious, why would you need binary to be no_std compatible?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My need (and in this case @CeciliaZ030 's as well) is not the binary per se, but rather revm+serde. We want no-std revm to make ZK proofs for it with powdr , which works fine. However it would be great to also be able to enable the serde feature in revm, in order to have better UX in message passing between the host and the guest when making ZK proofs.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense, I don't focus a lot of my attention on revme (binary) as it is used only for testing, was curious to see if I need to revise that view.

Okay, I just noticed that preserve_order is in both revm and revme. preserved_order is nice in revm as it formats trace json output that is easier to read, still okay with removing it.

Can you make a PR?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure! We can also wait to see if serde-rs/json#1101 gets merged, then we don't need to remove it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Your call, if this blocks you or you can use this right now it would be best to make PR


# ethersdb
tokio = { version = "1.35", features = [
Expand All @@ -42,7 +42,7 @@ indicatif = "0.17"

[features]
default = ["std", "c-kzg", "secp256k1"]
std = ["revm-interpreter/std", "revm-precompile/std"]
std = ["serde?/std", "serde_json?/std", "revm-interpreter/std", "revm-precompile/std"]
serde = ["dep:serde", "dep:serde_json", "revm-interpreter/serde"]
arbitrary = ["revm-interpreter/arbitrary"]
asm-keccak = ["revm-interpreter/asm-keccak", "revm-precompile/asm-keccak"]
Expand Down