Skip to content

Removed the last dependencies breaking no-std build. #669

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

Merged
rakita merged 6 commits into from
Sep 1, 2023
Merged

Removed the last dependencies breaking no-std build. #669

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
f8ad992
Removed the last dependencies breaking no-std build.
lvella Aug 29, 2023
45b3cb1
Reverting back to OnceCell.
lvella Aug 31, 2023
102fa52
Hex encoding debug print.
lvella Aug 31, 2023
e5b6091
Merge branch 'main' into no-std
lvella Aug 31, 2023
ebb85b5
Using race::OnceBox instead of critical-section.
lvella Sep 1, 2023
3f17224
Lint error.
lvella Sep 1, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 1 addition & 11 deletions Cargo.lock
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 6 additions & 4 deletions crates/precompile/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,18 @@ repository = "https://github.com/bluealloy/revm"
version = "2.0.3"

[dependencies]
revm-primitives = { path = "../primitives", version="1.1.2", default-features = false }
revm-primitives = { path = "../primitives", version = "1.1.2", default-features = false }
bn = { package = "substrate-bn", version = "0.6", default-features = false }
k256 = { version = "0.13", default-features = false, features = ["ecdsa"] }
num = { version = "0.4.0", default-features = false, features = ["alloc"] }
once_cell = "1.17"
ripemd = { version = "0.1", default-features = false }
secp256k1 = { version = "0.27.0", default-features = false, features = ["alloc", "recovery"], optional = true }
secp256k1 = { version = "0.27.0", default-features = false, features = [
"alloc",
"recovery",
], optional = true }
sha2 = { version = "0.10.5", default-features = false }
sha3 = { version = "0.10.7", default-features = false }
lazy_static = "1.4"
Copy link
Collaborator

@DaniPopes DaniPopes Aug 30, 2023

Choose a reason for hiding this comment

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

lazy_static is unmaintained. Please just use once_cell with default-features = false.

Copy link
Contributor Author

@lvella lvella Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

That was my first try, doesn't work with once_cell::unsync::OnceCell, which is not Sync, thus it can't be used to initialize a static variable. It seems lazy_static manages to do it by using a spin-lock, what once_cell doesn't have.

Anyway, the other crate I removed, to-binary, is also unmaintained and is older, so I don't see the issue of using lazy_static.

Copy link
Collaborator

@DaniPopes DaniPopes Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

See the once_cell FAQ:

  • Does this crate support no_std
  • Should I use std::cell::OnceCell, once_cell, or lazy_static?

I say we enable the critical-section feature. cc @rakita

Copy link
Contributor Author

@lvella lvella Aug 31, 2023

Choose a reason for hiding this comment

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

Just found once_cell::race. I'll use it instead of lazy_static.

Copy link
Member

@rakita rakita Aug 31, 2023

Choose a reason for hiding this comment

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

OnceCell was stabilised inside rust, I would check if it works
https://doc.rust-lang.org/nightly/core/cell/struct.OnceCell.html#:~:text=Struct%20core%3A%3Acell%3A%3AOnceCell&text=A%20cell%20which%20can%20be,borrow%20checks%20(unlike%20RefCell%20).

to-binary is not relevant at all as it had very simpler usecase,

Copy link
Contributor Author

@lvella lvella Aug 31, 2023

Choose a reason for hiding this comment

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

Sorry, it seems once_cell::race doesn't provide a generic OnceCell, instead it is specialized for some types that can be handled by atomic instructions. I'll change this PR to use critical-section.

Copy link
Member

@rakita rakita Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Sorry, it seems once_cell::race doesn't provide a generic OnceCell, instead it is specialized for some types that can be handled by atomic instructions. I'll change this PR to use critical-section.

Just came here to write this. We can use once_cell::race::OnlyBox, it does not require std and it fits nicely with the code

Copy link
Contributor Author

@lvella lvella Aug 31, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Well, I just pushed a version using critical-section, had to add a new feature and all. Should I change?

Copy link
Member

@rakita rakita Aug 31, 2023

Choose a reason for hiding this comment

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

Well, I just pushed a version using critical-section, had to add a new feature and all. Should I change?

It would be better to change it, fewer features to maintain and it would work out of the box ( pun intended :))

Copy link
Contributor Author

@lvella lvella Sep 1, 2023

Choose a reason for hiding this comment

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

done


[dev-dependencies]
hex = "0.4"
Expand All @@ -28,4 +31,3 @@ default = ["secp256k1"]
# Only problem that it has, it fails to build for wasm target on windows and mac as it is c lib.
# If you dont require wasm on win/mac, i would recommend its usage.
secp256k1 = ["dep:secp256k1"]

136 changes: 74 additions & 62 deletions crates/precompile/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ mod identity;
mod modexp;
mod secp256k1;

use once_cell::sync::OnceCell;
pub use primitives::{
precompile::{PrecompileError as Error, *},
Bytes, HashMap,
Expand All @@ -23,6 +22,7 @@ pub type B256 = [u8; 32];

use alloc::vec::Vec;
use core::fmt;
use lazy_static::lazy_static;

pub fn calc_linear_cost_u32(len: usize, base: u64, word: u64) -> u64 {
(len as u64 + 32 - 1) / 32 * word + base
Expand Down Expand Up @@ -119,76 +119,88 @@ impl SpecId {

impl Precompiles {
pub fn homestead() -> &'static Self {
static INSTANCE: OnceCell<Precompiles> = OnceCell::new();
INSTANCE.get_or_init(|| {
let fun = [
secp256k1::ECRECOVER,
hash::SHA256,
hash::RIPEMD160,
identity::FUN,
]
.into_iter()
.map(From::from)
.collect();
Self { fun }
})
lazy_static! {
static ref INSTANCE: Precompiles = {
let fun = [
secp256k1::ECRECOVER,
hash::SHA256,
hash::RIPEMD160,
identity::FUN,
]
.into_iter()
.map(From::from)
.collect();
Precompiles { fun }
};
}

&INSTANCE
}

pub fn byzantium() -> &'static Self {
static INSTANCE: OnceCell<Precompiles> = OnceCell::new();
INSTANCE.get_or_init(|| {
let mut precompiles = Self::homestead().clone();
precompiles.fun.extend(
[
// EIP-196: Precompiled contracts for addition and scalar multiplication on the elliptic curve alt_bn128.
// EIP-197: Precompiled contracts for optimal ate pairing check on the elliptic curve alt_bn128.
bn128::add::BYZANTIUM,
bn128::mul::BYZANTIUM,
bn128::pair::BYZANTIUM,
// EIP-198: Big integer modular exponentiation.
modexp::BYZANTIUM,
]
.into_iter()
.map(From::from),
);
precompiles
})
lazy_static! {
static ref INSTANCE: Precompiles = {
let mut precompiles = Precompiles::homestead().clone();
precompiles.fun.extend(
[
// EIP-196: Precompiled contracts for addition and scalar multiplication on the elliptic curve alt_bn128.
// EIP-197: Precompiled contracts for optimal ate pairing check on the elliptic curve alt_bn128.
bn128::add::BYZANTIUM,
bn128::mul::BYZANTIUM,
bn128::pair::BYZANTIUM,
// EIP-198: Big integer modular exponentiation.
modexp::BYZANTIUM,
]
.into_iter()
.map(From::from),
);
precompiles
};
}

&INSTANCE
}

pub fn istanbul() -> &'static Self {
static INSTANCE: OnceCell<Precompiles> = OnceCell::new();
INSTANCE.get_or_init(|| {
let mut precompiles = Self::byzantium().clone();
precompiles.fun.extend(
[
// EIP-152: Add BLAKE2 compression function `F` precompile.
blake2::FUN,
// EIP-1108: Reduce alt_bn128 precompile gas costs.
bn128::add::ISTANBUL,
bn128::mul::ISTANBUL,
bn128::pair::ISTANBUL,
]
.into_iter()
.map(From::from),
);
precompiles
})
lazy_static! {
static ref INSTANCE: Precompiles = {
let mut precompiles = Precompiles::byzantium().clone();
precompiles.fun.extend(
[
// EIP-152: Add BLAKE2 compression function `F` precompile.
blake2::FUN,
// EIP-1108: Reduce alt_bn128 precompile gas costs.
bn128::add::ISTANBUL,
bn128::mul::ISTANBUL,
bn128::pair::ISTANBUL,
]
.into_iter()
.map(From::from),
);
precompiles
};
}

&INSTANCE
}

pub fn berlin() -> &'static Self {
static INSTANCE: OnceCell<Precompiles> = OnceCell::new();
INSTANCE.get_or_init(|| {
let mut precompiles = Self::istanbul().clone();
precompiles.fun.extend(
[
// EIP-2565: ModExp Gas Cost.
modexp::BERLIN,
]
.into_iter()
.map(From::from),
);
precompiles
})
lazy_static! {
static ref INSTANCE: Precompiles = {
let mut precompiles = Precompiles::istanbul().clone();
precompiles.fun.extend(
[
// EIP-2565: ModExp Gas Cost.
modexp::BERLIN,
]
.into_iter()
.map(From::from),
);
precompiles
};
}

&INSTANCE
}

pub fn latest() -> &'static Self {
Expand Down
17 changes: 13 additions & 4 deletions crates/primitives/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,18 +14,22 @@ bytes = { version = "1.4", default-features = false }
hashbrown = "0.14"
primitive-types = { version = "0.12", default-features = false }
rlp = { version = "0.5", default-features = false } # used for create2 address calculation
ruint = { version = "1.8.0", features = ["primitive-types", "rlp"] }
ruint = { version = "1.8.0", default-features = false, features = [
"primitive-types",
"rlp",
] }
auto_impl = "1.1"
bitvec = { version = "1", default-features = false, features = ["alloc"] }
bitflags = { version = "2.4.0", default-features = false }

# bits B256 B160 crate
fixed-hash = { version = "0.8", default-features = false, features = ["rustc-hex"] }
fixed-hash = { version = "0.8", default-features = false, features = [
"rustc-hex",
] }

#utility
hex-literal = "0.4"
hex = { version = "0.4", default-features = false, features = ["alloc"] }
to-binary = "0.4"
derive_more = "0.99"
enumn = "0.1"

Expand All @@ -42,7 +46,12 @@ proptest-derive = { version = "0.3", optional = true }
arbitrary = { version = "1.3", features = ["derive"] }
proptest = "1.1"
proptest-derive = "0.3"
ruint = { version = "1.10.1", features = ["primitive-types", "rlp", "proptest", "arbitrary"] }
ruint = { version = "1.10.1", features = [
"primitive-types",
"rlp",
"proptest",
"arbitrary",
] }

[features]
default = ["std"]
Expand Down
3 changes: 1 addition & 2 deletions crates/primitives/src/bytecode.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ use bitvec::prelude::{bitvec, Lsb0};
use bitvec::vec::BitVec;
use bytes::Bytes;
use core::fmt::Debug;
use to_binary::BinaryString;

/// A map of valid `jump` destinations.
#[derive(Clone, Eq, PartialEq, Default)]
Expand All @@ -14,7 +13,7 @@ pub struct JumpMap(pub Arc<BitVec<u8>>);
impl Debug for JumpMap {
fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
f.debug_struct("JumpMap")
.field("map", &BinaryString::from(self.0.as_raw_slice()))
.field("map", &self.0.as_raw_slice())
.finish()
}
}
Expand Down
1 change: 0 additions & 1 deletion crates/primitives/src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ pub use bytes;
pub use bytes::Bytes;
pub use hex;
pub use hex_literal;
pub use to_binary;
/// Address type is last 20 bytes of hash of ethereum account
pub type Address = B160;
/// Hash, in Ethereum usually keccak256.
Expand Down