From 775021d222c9097b431d7f3603f851b80db8fdb6 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 20 Sep 2024 17:25:45 +0200 Subject: [PATCH 01/11] feat(primitives): implement `map` module --- .github/workflows/ci.yml | 2 +- Cargo.toml | 6 + crates/core/Cargo.toml | 17 +- crates/core/src/lib.rs | 4 +- crates/dyn-abi/src/arbitrary.rs | 4 +- crates/primitives/Cargo.toml | 46 ++++-- crates/primitives/src/lib.rs | 8 +- crates/primitives/src/{log.rs => log/mod.rs} | 5 +- .../src/{log_serde.rs => log/serde.rs} | 0 crates/primitives/src/map/fixed.rs | 152 ++++++++++++++++++ crates/primitives/src/map/mod.rs | 89 ++++++++++ 11 files changed, 310 insertions(+), 23 deletions(-) rename crates/primitives/src/{log.rs => log/mod.rs} (98%) rename crates/primitives/src/{log_serde.rs => log/serde.rs} (100%) create mode 100644 crates/primitives/src/map/fixed.rs create mode 100644 crates/primitives/src/map/mod.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index ee18251e0..bf1821f7e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -108,7 +108,7 @@ jobs: with: cache-on-failure: true - name: cargo hack - run: cargo hack check --feature-powerset --depth 2 + run: cargo hack check --feature-powerset --depth 1 --skip nightly check-no-std: name: check no_std ${{ matrix.features }} diff --git a/Cargo.toml b/Cargo.toml index ec488194a..45563d763 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -67,12 +67,18 @@ paste = "1.0" num_enum = "0.7" thiserror = "1.0" +# crypto digest = "0.10" k256 = { version = "0.13", default-features = false } keccak-asm = { version = "0.1.0", default-features = false } tiny-keccak = { version = "2.0", default-features = false } sha3 = { version = "0.10.8", default-features = false } +# maps +hashbrown = { version = "0.14", default-features = false } +indexmap = { version = "2.5", default-features = false } +rustc-hash = { version = "2.0", default-features = false } + # misc allocative = { version = "0.3.2", default-features = false } alloy-rlp = { version = "0.3", default-features = false } diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 88535dc01..887c6ca1d 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -36,27 +36,34 @@ std = [ "alloy-dyn-abi?/std", "alloy-sol-types?/std", ] +nightly = ["alloy-primitives/nightly"] dyn-abi = ["sol-types", "dep:alloy-dyn-abi"] json-abi = ["json", "serde", "dep:alloy-json-abi"] json = ["alloy-sol-types?/json"] sol-types = ["dep:alloy-sol-types"] -tiny-keccak = ["alloy-primitives/tiny-keccak"] -native-keccak = ["alloy-primitives/native-keccak"] asm-keccak = ["alloy-primitives/asm-keccak"] +native-keccak = ["alloy-primitives/native-keccak"] sha3-keccak = ["alloy-primitives/sha3-keccak"] +tiny-keccak = ["alloy-primitives/tiny-keccak"] + +map = ["alloy-primitives/map"] +map-hashbrown = ["alloy-primitives/map-hashbrown"] +map-indexmap = ["alloy-primitives/map-indexmap"] +map-fxhash = ["alloy-primitives/map-fxhash"] -postgres = ["std", "alloy-primitives/postgres"] getrandom = ["alloy-primitives/getrandom"] rand = ["alloy-primitives/rand"] rlp = ["alloy-primitives/rlp", "dep:alloy-rlp"] serde = ["alloy-primitives/serde"] +k256 = ["alloy-primitives/k256"] +eip712 = ["alloy-sol-types?/eip712-serde", "alloy-dyn-abi?/eip712"] + +postgres = ["std", "alloy-primitives/postgres"] arbitrary = [ "std", "alloy-primitives/arbitrary", "alloy-sol-types?/arbitrary", "alloy-dyn-abi?/arbitrary", ] -k256 = ["alloy-primitives/k256"] -eip712 = ["alloy-sol-types?/eip712-serde", "alloy-dyn-abi?/eip712"] diff --git a/crates/core/src/lib.rs b/crates/core/src/lib.rs index 255555b2b..1863c7534 100644 --- a/crates/core/src/lib.rs +++ b/crates/core/src/lib.rs @@ -23,7 +23,7 @@ pub use alloy_json_abi as json_abi; #[cfg(feature = "sol-types")] #[doc(inline)] pub use alloy_sol_types as sol_types; -#[cfg(all(doc, feature = "sol-types"))] // Show this re-export in docs instead of the wrapper below. +#[cfg(all(feature = "sol-types", doc))] // Show this re-export in docs instead of the wrapper below. #[doc(no_inline)] pub use sol_types::sol; @@ -34,7 +34,7 @@ pub use alloy_rlp as rlp; /// [`sol!`](sol_types::sol!) `macro_rules!` wrapper to set import attributes. /// /// See [`sol!`](sol_types::sol!) for the actual macro documentation. -#[cfg(all(not(doc), feature = "sol-types"))] // Show the actual macro in docs. +#[cfg(all(feature = "sol-types", not(doc)))] // Show the actual macro in docs. #[macro_export] macro_rules! sol { ($($t:tt)*) => { diff --git a/crates/dyn-abi/src/arbitrary.rs b/crates/dyn-abi/src/arbitrary.rs index 336fabb8f..2288fab8d 100644 --- a/crates/dyn-abi/src/arbitrary.rs +++ b/crates/dyn-abi/src/arbitrary.rs @@ -14,7 +14,7 @@ use alloy_primitives::{Address, Function, B256, I256, U256}; use arbitrary::{size_hint, Unstructured}; use core::ops::RangeInclusive; use proptest::{ - collection::{hash_set as hash_set_strategy, vec as vec_strategy, VecStrategy}, + collection::{vec as vec_strategy, VecStrategy}, prelude::*, strategy::{Flatten, Map, Recursive, TupleUnion, WA}, }; @@ -240,7 +240,7 @@ macro_rules! custom_struct_strategy { .prop_flat_map(move |sz| { ( IDENT_STRATEGY, - hash_set_strategy(IDENT_STRATEGY, sz..=sz) + proptest::collection::hash_set(IDENT_STRATEGY, sz..=sz) .prop_map(|prop_names| prop_names.into_iter().collect()), vec_strategy(elem.clone(), sz..=sz), ) diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index 6cdd31f5d..3eadf1522 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -28,6 +28,7 @@ itoa.workspace = true ruint.workspace = true # macros +cfg-if.workspace = true derive_more = { workspace = true, features = [ "as_ref", "add", @@ -43,7 +44,7 @@ derive_more = { workspace = true, features = [ "into_iterator", "display", ] } -cfg-if.workspace = true +paste.workspace = true # keccak256 keccak-asm = { workspace = true, optional = true } @@ -65,6 +66,11 @@ rand = { workspace = true, optional = true, features = ["getrandom"] } # k256 k256 = { workspace = true, optional = true, features = ["ecdsa"] } +# map +hashbrown = { workspace = true, optional = true, features = ["inline-more"] } +indexmap = { workspace = true, optional = true } +rustc-hash = { workspace = true, optional = true } + # arbitrary arbitrary = { workspace = true, optional = true } derive_arbitrary = { workspace = true, optional = true } @@ -90,24 +96,46 @@ std = [ "hex/std", "ruint/std", "alloy-rlp?/std", + "indexmap?/std", + "k256?/std", "keccak-asm?/std", "proptest?/std", "rand?/std", + "rustc-hash?/std", "serde?/std", - "k256?/std", "sha3?/std", ] +nightly = [ + "hex/nightly", + "ruint/nightly", + "hashbrown?/nightly", + "rustc-hash?/nightly", +] -tiny-keccak = [] -native-keccak = [] asm-keccak = ["dep:keccak-asm"] +native-keccak = [] sha3-keccak = ["dep:sha3"] +tiny-keccak = [] + +map = ["dep:hashbrown"] +map-hashbrown = ["map"] +map-indexmap = ["map", "dep:indexmap"] +map-fxhash = ["map", "dep:rustc-hash"] -postgres = ["std", "dep:postgres-types", "ruint/postgres"] getrandom = ["dep:getrandom"] -rand = ["dep:rand", "getrandom", "ruint/rand"] +k256 = ["dep:k256"] +rand = ["dep:rand", "getrandom", "ruint/rand", "rustc-hash?/rand"] rlp = ["dep:alloy-rlp", "ruint/alloy-rlp"] -serde = ["dep:serde", "bytes/serde", "hex/serde", "ruint/serde"] +serde = [ + "dep:serde", + "bytes/serde", + "hex/serde", + "ruint/serde", + "hashbrown?/serde", + "indexmap?/serde", +] + +allocative = ["dep:allocative"] arbitrary = [ "std", "dep:arbitrary", @@ -116,9 +144,9 @@ arbitrary = [ "dep:proptest-derive", "ruint/arbitrary", "ruint/proptest", + "indexmap?/arbitrary", ] -k256 = ["dep:k256"] -allocative = ["dep:allocative"] +postgres = ["std", "dep:postgres-types", "ruint/postgres"] # `const-hex` compatibility feature for `hex`. # Should not be needed most of the time. diff --git a/crates/primitives/src/lib.rs b/crates/primitives/src/lib.rs index 2d05d2072..b46e1eaee 100644 --- a/crates/primitives/src/lib.rs +++ b/crates/primitives/src/lib.rs @@ -5,14 +5,15 @@ )] #![cfg_attr(not(test), warn(unused_crate_dependencies))] #![cfg_attr(not(feature = "std"), no_std)] +#![cfg_attr(feature = "nightly", feature(hasher_prefixfree_extras))] #![cfg_attr(docsrs, feature(doc_cfg, doc_auto_cfg))] #[macro_use] extern crate alloc; +use paste as _; #[cfg(feature = "sha3-keccak")] use sha3 as _; - use tiny_keccak as _; #[cfg(feature = "postgres")] @@ -42,8 +43,9 @@ pub use common::TxKind; mod log; pub use log::{IntoLogData, Log, LogData}; -#[cfg(feature = "serde")] -mod log_serde; + +#[cfg(feature = "map")] +pub mod map; mod sealed; pub use sealed::{Sealable, Sealed}; diff --git a/crates/primitives/src/log.rs b/crates/primitives/src/log/mod.rs similarity index 98% rename from crates/primitives/src/log.rs rename to crates/primitives/src/log/mod.rs index f2a900a4a..a9eacaeeb 100644 --- a/crates/primitives/src/log.rs +++ b/crates/primitives/src/log/mod.rs @@ -1,9 +1,12 @@ use crate::{Address, Bytes, B256}; use alloc::vec::Vec; +#[cfg(feature = "serde")] +mod serde; + /// An Ethereum event log object. #[derive(Clone, Debug, Default, PartialEq, Eq, Hash)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "serde", derive(::serde::Serialize, ::serde::Deserialize))] #[cfg_attr(feature = "arbitrary", derive(derive_arbitrary::Arbitrary, proptest_derive::Arbitrary))] pub struct LogData { /// The indexed topic list. diff --git a/crates/primitives/src/log_serde.rs b/crates/primitives/src/log/serde.rs similarity index 100% rename from crates/primitives/src/log_serde.rs rename to crates/primitives/src/log/serde.rs diff --git a/crates/primitives/src/map/fixed.rs b/crates/primitives/src/map/fixed.rs new file mode 100644 index 000000000..eebab0c4a --- /dev/null +++ b/crates/primitives/src/map/fixed.rs @@ -0,0 +1,152 @@ +use super::*; +use crate::{Address, FixedBytes, B256}; +use cfg_if::cfg_if; +use core::hash::{BuildHasher, Hasher}; + +/// [`HashMap`] optimized for hashing [fixed-size byte arrays](FixedBytes). +pub type FbHashMap = + HashMap, V, BuildFixedBytesHasher>; +/// [`HashSet`] optimized for hashing [fixed-size byte arrays](FixedBytes). +pub type FbHashSet = + HashSet, BuildFixedBytesHasher>; + +cfg_if! { + if #[cfg(feature = "map-indexmap")] { + /// [`IndexMap`] optimized for hashing [fixed-size byte arrays](FixedBytes). + pub type FbIndexMap = + indexmap::IndexMap, V, BuildFixedBytesHasher>; + /// [`IndexSet`] optimized for hashing [fixed-size byte arrays](FixedBytes). + pub type FbIndexSet = + indexmap::IndexSet, BuildFixedBytesHasher>; + } +} + +macro_rules! fb_alias_maps { + ($($ty:ident < $n:literal >),* $(,)?) => { paste::paste! { + $( + #[doc = concat!("[`HashMap`] optimized for hashing [`", stringify!($ty), "`].")] + pub type [<$ty HashMap>] = HashMap<$ty, V, S>; + #[doc = concat!("[`HashSet`] optimized for hashing [`", stringify!($ty), "`].")] + pub type [<$ty HashSet>] = HashSet<$ty, S>; + + cfg_if! { + if #[cfg(feature = "map-indexmap")] { + #[doc = concat!("[`IndexMap`] optimized for hashing [`", stringify!($ty), "`].")] + pub type [<$ty IndexMap>] = IndexMap<$ty, V, S>; + #[doc = concat!("[`IndexSet`] optimized for hashing [`", stringify!($ty), "`].")] + pub type [<$ty IndexSet>] = IndexSet<$ty, S>; + } + } + )* + } }; +} + +fb_alias_maps!(Address<20>, B256<32>); + +macro_rules! assert_unchecked { + ($e:expr) => { + if cfg!(debug_assertions) { + assert!($e); + } else if !$e { + unsafe { core::hint::unreachable_unchecked() } + } + }; +} + +/// [`BuildHasher`] optimized for hashing [fixed-size byte arrays](FixedBytes). +/// +/// **NOTE:** this hasher accepts only `N`-length byte arrays! It is UB to hash anything else. +#[derive(Clone, Debug, Default)] +pub struct BuildFixedBytesHasher { + inner: S, + _marker: core::marker::PhantomData<[(); N]>, +} + +impl BuildHasher for BuildFixedBytesHasher { + type Hasher = FixedBytesHasher; + + #[inline] + fn build_hasher(&self) -> Self::Hasher { + FixedBytesHasher { inner: self.inner.build_hasher(), _marker: core::marker::PhantomData } + } +} + +/// [`Hasher`] optimized for hashing [fixed-size byte arrays](FixedBytes). +/// +/// **NOTE:** this hasher accepts only `N`-length byte arrays! It is UB to hash anything else. +#[derive(Clone, Debug, Default)] +pub struct FixedBytesHasher { + inner: H, + _marker: core::marker::PhantomData<[(); N]>, +} + +impl Hasher for FixedBytesHasher { + #[inline] + fn finish(&self) -> u64 { + self.inner.finish() + } + + #[inline] + fn write(&mut self, mut bytes: &[u8]) { + assert_unchecked!(bytes.len() == N); + + while let Some((chunk, rest)) = bytes.split_first_chunk() { + self.inner.write_usize(usize::from_ne_bytes(*chunk)); + bytes = rest; + } + if usize::BITS > 64 { + if let Some((chunk, rest)) = bytes.split_first_chunk() { + self.inner.write_u64(u64::from_ne_bytes(*chunk)); + bytes = rest; + } + } + if usize::BITS > 32 { + if let Some((chunk, rest)) = bytes.split_first_chunk() { + self.inner.write_u32(u32::from_ne_bytes(*chunk)); + bytes = rest; + } + } + if usize::BITS > 16 { + if let Some((chunk, rest)) = bytes.split_first_chunk() { + self.inner.write_u16(u16::from_ne_bytes(*chunk)); + bytes = rest; + } + } + if usize::BITS > 8 { + if let Some((chunk, rest)) = bytes.split_first_chunk() { + self.inner.write_u8(u8::from_ne_bytes(*chunk)); + bytes = rest; + } + } + + debug_assert!(bytes.is_empty()); + } + + #[cfg(feature = "nightly")] + #[inline] + fn write_length_prefix(&mut self, len: usize) { + assert_unchecked!(len == N); + // We can just omit the length prefix entirely since we know it's always `N`. + } +} + +#[cfg(test)] +mod tests { + use super::*; + + macro_rules! hash_zero { + ($n:expr) => { + BuildFixedBytesHasher::<$n>::default().hash_one(&FixedBytes::<$n>::ZERO) + }; + } + + #[test] + fn fb_hasher() { + // Just by running it once we test that it compiles and that debug assertions are correct. + ruint::const_for!(N in [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, + 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, + 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47] { + assert_ne!(hash_zero!(N), 0); + }); + } +} diff --git a/crates/primitives/src/map/mod.rs b/crates/primitives/src/map/mod.rs new file mode 100644 index 000000000..acded3056 --- /dev/null +++ b/crates/primitives/src/map/mod.rs @@ -0,0 +1,89 @@ +//! Re-exports of map types and utilities. + +use cfg_if::cfg_if; + +mod fixed; +pub use fixed::*; + +// Use `hashbrown` if requested with "map-hashbrown" or required by `no_std`. +cfg_if! { + if #[cfg(any(feature = "map-hashbrown", not(feature = "std")))] { + use hashbrown as imp; + } else { + use std::collections as imp; + } +} + +#[doc(no_inline)] +pub use imp::{hash_map, hash_map::Entry, hash_set}; + +/// A [`HashMap`](imp::HashMap) using the [default hasher](DefaultHasher). +/// +/// See [`HashMap`](imp::HashMap) for more information. +pub type HashMap = imp::HashMap; +/// A [`HashSet`](imp::HashSet) using the [default hasher](DefaultHasher). +/// +/// See [`HashSet`](imp::HashSet) for more information. +pub type HashSet = imp::HashSet; + +// Faster hasher. +cfg_if! { + if #[cfg(feature = "map-fxhash")] { + #[doc(no_inline)] + pub use rustc_hash::{self, FxHasher}; + + cfg_if! { + if #[cfg(all(feature = "std", feature = "rand"))] { + #[doc(no_inline)] + pub use rustc_hash::FxRandomState as FxBuildHasher; + } else { + #[doc(no_inline)] + pub use rustc_hash::FxBuildHasher; + } + } + + /// A [`HashMap`] using [`FxHasher`] as its hasher. + pub type FxHashMap = HashMap; + /// A [`HashSet`] using [`FxHasher`] as its hasher. + pub type FxHashSet = HashSet; + } +} + +// Default hasher. +cfg_if! { + if #[cfg(feature = "map-fxhash")] { + type DefaultHashBuilderInner = FxBuildHasher; + } else { + type DefaultHashBuilderInner = hashbrown::hash_map::DefaultHashBuilder; + } +} +/// The default [`BuildHasher`](core::hash::BuildHasher) used by [`HashMap`] and [`HashSet`]. +pub type DefaultHashBuilder = DefaultHashBuilderInner; +/// The default [`Hasher`](core::hash::Hasher) used by [`HashMap`] and [`HashSet`]. +pub type DefaultHasher = ::Hasher; + +// `indexmap` re-exports. +cfg_if! { + if #[cfg(feature = "map-indexmap")] { + #[doc(no_inline)] + pub use indexmap::{self, map::Entry as IndexEntry}; + + /// [`IndexMap`](indexmap::IndexMap) using the [default hasher](DefaultHasher). + /// + /// See [`IndexMap`](indexmap::IndexMap) for more information. + pub type IndexMap = indexmap::IndexMap; + /// [`IndexSet`](indexmap::IndexSet) using the [default hasher](DefaultHasher). + /// + /// See [`IndexSet`](indexmap::IndexSet) for more information. + pub type IndexSet = indexmap::IndexSet; + + cfg_if! { + if #[cfg(feature = "map-fxhash")] { + /// An [`IndexMap`] using [`FxHasher`] as its hasher. + pub type FxIndexMap = IndexMap; + /// An [`IndexSet`] using [`FxHasher`] as its hasher. + pub type FxIndexSet = IndexSet; + } + } + } +} From be1772d737597e27a8d953178f44122cd42c5e23 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 20 Sep 2024 18:39:35 +0200 Subject: [PATCH 02/11] chore: nits --- crates/primitives/src/map/fixed.rs | 40 ++++++++++++++++-------------- 1 file changed, 22 insertions(+), 18 deletions(-) diff --git a/crates/primitives/src/map/fixed.rs b/crates/primitives/src/map/fixed.rs index eebab0c4a..16816d4c3 100644 --- a/crates/primitives/src/map/fixed.rs +++ b/crates/primitives/src/map/fixed.rs @@ -1,23 +1,23 @@ use super::*; -use crate::{Address, FixedBytes, B256}; +use crate::{Address, FixedBytes, Selector, B256}; use cfg_if::cfg_if; use core::hash::{BuildHasher, Hasher}; /// [`HashMap`] optimized for hashing [fixed-size byte arrays](FixedBytes). pub type FbHashMap = - HashMap, V, BuildFixedBytesHasher>; + HashMap, V, BuildFbHasher>; /// [`HashSet`] optimized for hashing [fixed-size byte arrays](FixedBytes). pub type FbHashSet = - HashSet, BuildFixedBytesHasher>; + HashSet, BuildFbHasher>; cfg_if! { if #[cfg(feature = "map-indexmap")] { /// [`IndexMap`] optimized for hashing [fixed-size byte arrays](FixedBytes). pub type FbIndexMap = - indexmap::IndexMap, V, BuildFixedBytesHasher>; + indexmap::IndexMap, V, BuildFbHasher>; /// [`IndexSet`] optimized for hashing [fixed-size byte arrays](FixedBytes). pub type FbIndexSet = - indexmap::IndexSet, BuildFixedBytesHasher>; + indexmap::IndexSet, BuildFbHasher>; } } @@ -25,23 +25,27 @@ macro_rules! fb_alias_maps { ($($ty:ident < $n:literal >),* $(,)?) => { paste::paste! { $( #[doc = concat!("[`HashMap`] optimized for hashing [`", stringify!($ty), "`].")] - pub type [<$ty HashMap>] = HashMap<$ty, V, S>; + pub type [<$ty HashMap>] = + HashMap<$ty, V, BuildFbHasher<$n, S>>; #[doc = concat!("[`HashSet`] optimized for hashing [`", stringify!($ty), "`].")] - pub type [<$ty HashSet>] = HashSet<$ty, S>; + pub type [<$ty HashSet>] = + HashSet<$ty, BuildFbHasher<$n, S>>; cfg_if! { if #[cfg(feature = "map-indexmap")] { #[doc = concat!("[`IndexMap`] optimized for hashing [`", stringify!($ty), "`].")] - pub type [<$ty IndexMap>] = IndexMap<$ty, V, S>; + pub type [<$ty IndexMap>] = + IndexMap<$ty, V, BuildFbHasher<$n, S>>; #[doc = concat!("[`IndexSet`] optimized for hashing [`", stringify!($ty), "`].")] - pub type [<$ty IndexSet>] = IndexSet<$ty, S>; + pub type [<$ty IndexSet>] = + IndexSet<$ty, BuildFbHasher<$n, S>>; } } )* } }; } -fb_alias_maps!(Address<20>, B256<32>); +fb_alias_maps!(Selector<4>, Address<20>, B256<32>); macro_rules! assert_unchecked { ($e:expr) => { @@ -57,17 +61,17 @@ macro_rules! assert_unchecked { /// /// **NOTE:** this hasher accepts only `N`-length byte arrays! It is UB to hash anything else. #[derive(Clone, Debug, Default)] -pub struct BuildFixedBytesHasher { +pub struct BuildFbHasher { inner: S, _marker: core::marker::PhantomData<[(); N]>, } -impl BuildHasher for BuildFixedBytesHasher { - type Hasher = FixedBytesHasher; +impl BuildHasher for BuildFbHasher { + type Hasher = FbHasher; #[inline] fn build_hasher(&self) -> Self::Hasher { - FixedBytesHasher { inner: self.inner.build_hasher(), _marker: core::marker::PhantomData } + FbHasher { inner: self.inner.build_hasher(), _marker: core::marker::PhantomData } } } @@ -75,12 +79,12 @@ impl BuildHasher for BuildFixedBytesHasher /// /// **NOTE:** this hasher accepts only `N`-length byte arrays! It is UB to hash anything else. #[derive(Clone, Debug, Default)] -pub struct FixedBytesHasher { +pub struct FbHasher { inner: H, _marker: core::marker::PhantomData<[(); N]>, } -impl Hasher for FixedBytesHasher { +impl Hasher for FbHasher { #[inline] fn finish(&self) -> u64 { self.inner.finish() @@ -126,7 +130,7 @@ impl Hasher for FixedBytesHasher { #[inline] fn write_length_prefix(&mut self, len: usize) { assert_unchecked!(len == N); - // We can just omit the length prefix entirely since we know it's always `N`. + // We can just skip hashing the length prefix entirely since we know it's always `N`. } } @@ -136,7 +140,7 @@ mod tests { macro_rules! hash_zero { ($n:expr) => { - BuildFixedBytesHasher::<$n>::default().hash_one(&FixedBytes::<$n>::ZERO) + BuildFbHasher::<$n>::default().hash_one(&FixedBytes::<$n>::ZERO) }; } From 876cb92f4d9c1c05e8c2a75836767176c4b2bc2e Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 20 Sep 2024 18:53:29 +0200 Subject: [PATCH 03/11] ci --- .github/workflows/ci.yml | 25 +++++++++++++++++-------- 1 file changed, 17 insertions(+), 8 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bf1821f7e..6f00bae7a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,6 @@ jobs: os: ["ubuntu-latest", "windows-latest"] rust: [ "stable", - "beta", "nightly", "1.79", # MSRV ] @@ -43,12 +42,9 @@ jobs: flags: "--features json" # All features - os: "ubuntu-latest" - rust: "stable" - flags: "--all-features" - - os: "ubuntu-latest" - rust: "beta" + rust: "nightly" flags: "--all-features" - - os: "ubuntu-latest" + - os: "windows-latest" rust: "nightly" flags: "--all-features" steps: @@ -108,7 +104,20 @@ jobs: with: cache-on-failure: true - name: cargo hack - run: cargo hack check --feature-powerset --depth 1 --skip nightly + run: cargo hack check --each-feature --skip nightly + + feature-checks-all-targets: + runs-on: ubuntu-latest + timeout-minutes: 30 + steps: + - uses: actions/checkout@v4 + - uses: dtolnay/rust-toolchain@stable + - uses: taiki-e/install-action@cargo-hack + - uses: Swatinem/rust-cache@v2 + with: + cache-on-failure: true + - name: cargo hack + run: cargo hack check --each-feature --skip nightly --all-targets check-no-std: name: check no_std ${{ matrix.features }} @@ -130,7 +139,7 @@ jobs: timeout-minutes: 30 steps: - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable + - uses: dtolnay/rust-toolchain@nightly with: components: clippy - uses: Swatinem/rust-cache@v2 From 9e5a01cd28f6f9e6334bc65630ce2155f48ea6f3 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 20 Sep 2024 19:39:47 +0200 Subject: [PATCH 04/11] chore: fixes, clippy --- crates/core/Cargo.toml | 11 +++++----- crates/core/tests/sol.rs | 2 +- crates/json-abi/src/param.rs | 7 +++++-- crates/primitives/Cargo.toml | 3 ++- crates/primitives/src/log/serde.rs | 1 + crates/primitives/src/map/fixed.rs | 6 +++++- crates/primitives/src/map/mod.rs | 10 ++++++++++ crates/primitives/src/signature/sig.rs | 20 ++++++++----------- crates/primitives/src/utils/mod.rs | 1 + .../sol-type-parser/src/state_mutability.rs | 4 +--- crates/syn-solidity/tests/contracts.rs | 2 +- 11 files changed, 41 insertions(+), 26 deletions(-) diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 887c6ca1d..f018253b8 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -21,15 +21,16 @@ rustdoc-args = ["--cfg", "docsrs"] workspace = true [dependencies] -alloy-primitives = { workspace = true, default-features = false } +alloy-primitives.workspace = true -alloy-dyn-abi = { workspace = true, default-features = false, optional = true } -alloy-json-abi = { workspace = true, default-features = false, optional = true } -alloy-sol-types = { workspace = true, default-features = false, optional = true } +alloy-dyn-abi = { workspace = true, optional = true } +alloy-json-abi = { workspace = true, optional = true } +alloy-sol-types = { workspace = true, optional = true } -alloy-rlp = { workspace = true, default-features = false, optional = true } +alloy-rlp = { workspace = true, optional = true } [features] +default = ["std", "map-fxhash", "rand"] std = [ "alloy-primitives/std", "alloy-json-abi?/std", diff --git a/crates/core/tests/sol.rs b/crates/core/tests/sol.rs index 991502fd4..2da1bfe29 100644 --- a/crates/core/tests/sol.rs +++ b/crates/core/tests/sol.rs @@ -1,5 +1,5 @@ -#![cfg(feature = "sol-types")] #![allow(missing_docs)] +#![cfg(feature = "sol-types")] use alloy_core::sol; diff --git a/crates/json-abi/src/param.rs b/crates/json-abi/src/param.rs index 2868c5bcb..6c0440580 100644 --- a/crates/json-abi/src/param.rs +++ b/crates/json-abi/src/param.rs @@ -675,8 +675,11 @@ mod tests { let param_value = serde_json::from_str::(param).unwrap(); assert_eq!(serde_json::from_value::(param_value).unwrap(), expected); - let reader = std::io::Cursor::new(param); - assert_eq!(serde_json::from_reader::<_, Param>(reader).unwrap(), expected); + #[cfg(feature = "std")] + { + let reader = std::io::Cursor::new(param); + assert_eq!(serde_json::from_reader::<_, Param>(reader).unwrap(), expected); + } } #[test] diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index 3eadf1522..f8e49000d 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -90,7 +90,7 @@ criterion.workspace = true serde_json.workspace = true [features] -default = ["std"] +default = ["std", "map-fxhash", "rand"] std = [ "bytes/std", "hex/std", @@ -133,6 +133,7 @@ serde = [ "ruint/serde", "hashbrown?/serde", "indexmap?/serde", + "rand?/serde", ] allocative = ["dep:allocative"] diff --git a/crates/primitives/src/log/serde.rs b/crates/primitives/src/log/serde.rs index e11413f81..70e7c6973 100644 --- a/crates/primitives/src/log/serde.rs +++ b/crates/primitives/src/log/serde.rs @@ -80,6 +80,7 @@ mod tests { log::{Log, LogData}, Bytes, }; + use alloc::vec::Vec; #[derive(Debug, PartialEq, serde::Serialize, serde::Deserialize)] struct TestStruct { diff --git a/crates/primitives/src/map/fixed.rs b/crates/primitives/src/map/fixed.rs index 16816d4c3..ed3fc0abc 100644 --- a/crates/primitives/src/map/fixed.rs +++ b/crates/primitives/src/map/fixed.rs @@ -59,6 +59,8 @@ macro_rules! assert_unchecked { /// [`BuildHasher`] optimized for hashing [fixed-size byte arrays](FixedBytes). /// +/// Works best with `fxhash`, enabled by default with the "map-fxhash" feature. +/// /// **NOTE:** this hasher accepts only `N`-length byte arrays! It is UB to hash anything else. #[derive(Clone, Debug, Default)] pub struct BuildFbHasher { @@ -77,6 +79,8 @@ impl BuildHasher for BuildFbHasher { /// [`Hasher`] optimized for hashing [fixed-size byte arrays](FixedBytes). /// +/// Works best with `fxhash`, enabled by default with the "map-fxhash" feature. +/// /// **NOTE:** this hasher accepts only `N`-length byte arrays! It is UB to hash anything else. #[derive(Clone, Debug, Default)] pub struct FbHasher { @@ -134,7 +138,7 @@ impl Hasher for FbHasher { } } -#[cfg(test)] +#[cfg(all(test, any(feature = "std", feature = "map-fxhash")))] mod tests { use super::*; diff --git a/crates/primitives/src/map/mod.rs b/crates/primitives/src/map/mod.rs index acded3056..016d4ec77 100644 --- a/crates/primitives/src/map/mod.rs +++ b/crates/primitives/src/map/mod.rs @@ -5,6 +5,8 @@ use cfg_if::cfg_if; mod fixed; pub use fixed::*; +use hashbrown as _; + // Use `hashbrown` if requested with "map-hashbrown" or required by `no_std`. cfg_if! { if #[cfg(any(feature = "map-hashbrown", not(feature = "std")))] { @@ -53,13 +55,21 @@ cfg_if! { cfg_if! { if #[cfg(feature = "map-fxhash")] { type DefaultHashBuilderInner = FxBuildHasher; + } else if #[cfg(feature = "std")] { + type DefaultHashBuilderInner = std::collections::hash_map::RandomState; } else { type DefaultHashBuilderInner = hashbrown::hash_map::DefaultHashBuilder; } } /// The default [`BuildHasher`](core::hash::BuildHasher) used by [`HashMap`] and [`HashSet`]. +/// +/// This hasher prioritizes speed over security, even if it is still secure enough for most +/// applications thanks to the use of a random seed. pub type DefaultHashBuilder = DefaultHashBuilderInner; /// The default [`Hasher`](core::hash::Hasher) used by [`HashMap`] and [`HashSet`]. +/// +/// This hasher prioritizes speed over security, even if it is still secure enough for most +/// applications thanks to the use of a random seed. pub type DefaultHasher = ::Hasher; // `indexmap` re-exports. diff --git a/crates/primitives/src/signature/sig.rs b/crates/primitives/src/signature/sig.rs index 3835287e3..658a7fc86 100644 --- a/crates/primitives/src/signature/sig.rs +++ b/crates/primitives/src/signature/sig.rs @@ -583,14 +583,13 @@ impl proptest::arbitrary::Arbitrary for Signature { #[cfg(test)] #[allow(unused_imports)] mod tests { - use crate::Bytes; - use super::*; - use std::str::FromStr; + use crate::Bytes; + use core::str::FromStr; + use hex::FromHex; #[cfg(feature = "rlp")] use alloy_rlp::{Decodable, Encodable}; - use hex::FromHex; #[test] #[cfg(feature = "k256")] @@ -652,16 +651,13 @@ mod tests { #[cfg(feature = "serde")] #[test] fn deserialize_with_parity() { - let raw_signature_with_y_parity = serde_json::json!( - { - "r":"0xc569c92f176a3be1a6352dd5005bfc751dcb32f57623dd2a23693e64bf4447b0", - "s":"0x1a891b566d369e79b7a66eecab1e008831e22daa15f91a0a0cf4f9f28f47ee05", - "v":"0x1", + let raw_signature_with_y_parity = serde_json::json!({ + "r": "0xc569c92f176a3be1a6352dd5005bfc751dcb32f57623dd2a23693e64bf4447b0", + "s": "0x1a891b566d369e79b7a66eecab1e008831e22daa15f91a0a0cf4f9f28f47ee05", + "v": "0x1", "yParity": "0x1" - } - ); + }); - println!("{raw_signature_with_y_parity}"); let signature: Signature = serde_json::from_value(raw_signature_with_y_parity).unwrap(); let expected = Signature::from_rs_and_parity( diff --git a/crates/primitives/src/utils/mod.rs b/crates/primitives/src/utils/mod.rs index bf915c37b..fc10fe380 100644 --- a/crates/primitives/src/utils/mod.rs +++ b/crates/primitives/src/utils/mod.rs @@ -302,6 +302,7 @@ impl Keccak256 { #[cfg(test)] mod tests { use super::*; + use alloc::string::ToString; // test vector taken from: // https://web3js.readthedocs.io/en/v1.10.0/web3-eth-accounts.html#hashmessage diff --git a/crates/sol-type-parser/src/state_mutability.rs b/crates/sol-type-parser/src/state_mutability.rs index 3dc34f495..d4086a4f9 100644 --- a/crates/sol-type-parser/src/state_mutability.rs +++ b/crates/sol-type-parser/src/state_mutability.rs @@ -157,11 +157,9 @@ pub mod serde_state_mutability_compat { } } -#[cfg(test)] +#[cfg(all(test, feature = "serde"))] mod tests { use super::*; - - #[cfg(not(feature = "std"))] use alloc::string::ToString; #[derive(Debug, Serialize, Deserialize)] diff --git a/crates/syn-solidity/tests/contracts.rs b/crates/syn-solidity/tests/contracts.rs index f9af4fc87..c9fb26ea0 100644 --- a/crates/syn-solidity/tests/contracts.rs +++ b/crates/syn-solidity/tests/contracts.rs @@ -1,6 +1,6 @@ -#![cfg(feature = "visit")] #![allow(clippy::missing_const_for_fn)] #![allow(missing_docs)] +#![cfg(feature = "visit")] use std::{ fs::{self, DirEntry}, From b207c1815041881d09f3f47b0d2ea61a7a5e9eb8 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 20 Sep 2024 20:16:30 +0200 Subject: [PATCH 05/11] perf: override write_usize on stable --- crates/primitives/src/map/fixed.rs | 102 ++++++++++++++++++----------- 1 file changed, 62 insertions(+), 40 deletions(-) diff --git a/crates/primitives/src/map/fixed.rs b/crates/primitives/src/map/fixed.rs index ed3fc0abc..adc9e761e 100644 --- a/crates/primitives/src/map/fixed.rs +++ b/crates/primitives/src/map/fixed.rs @@ -47,16 +47,29 @@ macro_rules! fb_alias_maps { fb_alias_maps!(Selector<4>, Address<20>, B256<32>); +#[allow(unused_macros)] macro_rules! assert_unchecked { - ($e:expr) => { + ($e:expr) => { assert_unchecked!($e,); }; + ($e:expr, $($t:tt)*) => { if cfg!(debug_assertions) { - assert!($e); + assert!($e, $($t)*); } else if !$e { unsafe { core::hint::unreachable_unchecked() } } }; } +macro_rules! assert_eq_unchecked { + ($a:expr, $b:expr) => { assert_eq_unchecked!($a, $b,); }; + ($a:expr, $b:expr, $($t:tt)*) => { + if cfg!(debug_assertions) { + assert_eq!($a, $b, $($t)*); + } else if $a != $b { + unsafe { core::hint::unreachable_unchecked() } + } + }; +} + /// [`BuildHasher`] optimized for hashing [fixed-size byte arrays](FixedBytes). /// /// Works best with `fxhash`, enabled by default with the "map-fxhash" feature. @@ -95,57 +108,66 @@ impl Hasher for FbHasher { } #[inline] - fn write(&mut self, mut bytes: &[u8]) { - assert_unchecked!(bytes.len() == N); - - while let Some((chunk, rest)) = bytes.split_first_chunk() { - self.inner.write_usize(usize::from_ne_bytes(*chunk)); - bytes = rest; - } - if usize::BITS > 64 { - if let Some((chunk, rest)) = bytes.split_first_chunk() { - self.inner.write_u64(u64::from_ne_bytes(*chunk)); - bytes = rest; - } - } - if usize::BITS > 32 { - if let Some((chunk, rest)) = bytes.split_first_chunk() { - self.inner.write_u32(u32::from_ne_bytes(*chunk)); - bytes = rest; - } - } - if usize::BITS > 16 { - if let Some((chunk, rest)) = bytes.split_first_chunk() { - self.inner.write_u16(u16::from_ne_bytes(*chunk)); - bytes = rest; - } - } - if usize::BITS > 8 { - if let Some((chunk, rest)) = bytes.split_first_chunk() { - self.inner.write_u8(u8::from_ne_bytes(*chunk)); - bytes = rest; - } - } + fn write(&mut self, bytes: &[u8]) { + assert_eq_unchecked!(bytes.len(), N); + write_bytes(&mut self.inner, bytes); + } - debug_assert!(bytes.is_empty()); + // `write_length_prefix` calls `write_usize` by default. + #[cfg(not(feature = "nightly"))] + #[inline] + fn write_usize(&mut self, len: usize) { + assert_eq_unchecked!(len, N); } #[cfg(feature = "nightly")] #[inline] fn write_length_prefix(&mut self, len: usize) { - assert_unchecked!(len == N); + assert_eq_unchecked!(len, N); // We can just skip hashing the length prefix entirely since we know it's always `N`. } } +#[inline(always)] +fn write_bytes(hasher: &mut impl Hasher, mut bytes: &[u8]) { + while let Some((chunk, rest)) = bytes.split_first_chunk() { + hasher.write_usize(usize::from_ne_bytes(*chunk)); + bytes = rest; + } + if usize::BITS > 64 { + if let Some((chunk, rest)) = bytes.split_first_chunk() { + hasher.write_u64(u64::from_ne_bytes(*chunk)); + bytes = rest; + } + } + if usize::BITS > 32 { + if let Some((chunk, rest)) = bytes.split_first_chunk() { + hasher.write_u32(u32::from_ne_bytes(*chunk)); + bytes = rest; + } + } + if usize::BITS > 16 { + if let Some((chunk, rest)) = bytes.split_first_chunk() { + hasher.write_u16(u16::from_ne_bytes(*chunk)); + bytes = rest; + } + } + if usize::BITS > 8 { + if let Some((chunk, rest)) = bytes.split_first_chunk() { + hasher.write_u8(u8::from_ne_bytes(*chunk)); + bytes = rest; + } + } + + debug_assert!(bytes.is_empty()); +} + #[cfg(all(test, any(feature = "std", feature = "map-fxhash")))] mod tests { use super::*; - macro_rules! hash_zero { - ($n:expr) => { - BuildFbHasher::<$n>::default().hash_one(&FixedBytes::<$n>::ZERO) - }; + fn hash_zero() -> u64 { + BuildFbHasher::::default().hash_one(&FixedBytes::::ZERO) } #[test] @@ -154,7 +176,7 @@ mod tests { ruint::const_for!(N in [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47] { - assert_ne!(hash_zero!(N), 0); + assert_ne!(hash_zero::(), 0); }); } } From dc06282974d664829fff937a9c38a109210677f2 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Fri, 20 Sep 2024 21:30:01 +0200 Subject: [PATCH 06/11] chore: updates --- crates/core/Cargo.toml | 2 +- crates/primitives/Cargo.toml | 2 +- crates/primitives/src/map/fixed.rs | 38 +++++++++++++++++------------- 3 files changed, 24 insertions(+), 18 deletions(-) diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index f018253b8..431873a3a 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -30,7 +30,7 @@ alloy-sol-types = { workspace = true, optional = true } alloy-rlp = { workspace = true, optional = true } [features] -default = ["std", "map-fxhash", "rand"] +default = ["std", "map-fxhash"] std = [ "alloy-primitives/std", "alloy-json-abi?/std", diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index f8e49000d..a7a65f53a 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -90,7 +90,7 @@ criterion.workspace = true serde_json.workspace = true [features] -default = ["std", "map-fxhash", "rand"] +default = ["std", "map-fxhash"] std = [ "bytes/std", "hex/std", diff --git a/crates/primitives/src/map/fixed.rs b/crates/primitives/src/map/fixed.rs index adc9e761e..ebded0ebb 100644 --- a/crates/primitives/src/map/fixed.rs +++ b/crates/primitives/src/map/fixed.rs @@ -5,19 +5,19 @@ use core::hash::{BuildHasher, Hasher}; /// [`HashMap`] optimized for hashing [fixed-size byte arrays](FixedBytes). pub type FbHashMap = - HashMap, V, BuildFbHasher>; + HashMap, V, FbBuildHasher>; /// [`HashSet`] optimized for hashing [fixed-size byte arrays](FixedBytes). pub type FbHashSet = - HashSet, BuildFbHasher>; + HashSet, FbBuildHasher>; cfg_if! { if #[cfg(feature = "map-indexmap")] { /// [`IndexMap`] optimized for hashing [fixed-size byte arrays](FixedBytes). pub type FbIndexMap = - indexmap::IndexMap, V, BuildFbHasher>; + indexmap::IndexMap, V, FbBuildHasher>; /// [`IndexSet`] optimized for hashing [fixed-size byte arrays](FixedBytes). pub type FbIndexSet = - indexmap::IndexSet, BuildFbHasher>; + indexmap::IndexSet, FbBuildHasher>; } } @@ -26,19 +26,19 @@ macro_rules! fb_alias_maps { $( #[doc = concat!("[`HashMap`] optimized for hashing [`", stringify!($ty), "`].")] pub type [<$ty HashMap>] = - HashMap<$ty, V, BuildFbHasher<$n, S>>; + HashMap<$ty, V, FbBuildHasher<$n, S>>; #[doc = concat!("[`HashSet`] optimized for hashing [`", stringify!($ty), "`].")] pub type [<$ty HashSet>] = - HashSet<$ty, BuildFbHasher<$n, S>>; + HashSet<$ty, FbBuildHasher<$n, S>>; cfg_if! { if #[cfg(feature = "map-indexmap")] { #[doc = concat!("[`IndexMap`] optimized for hashing [`", stringify!($ty), "`].")] pub type [<$ty IndexMap>] = - IndexMap<$ty, V, BuildFbHasher<$n, S>>; + IndexMap<$ty, V, FbBuildHasher<$n, S>>; #[doc = concat!("[`IndexSet`] optimized for hashing [`", stringify!($ty), "`].")] pub type [<$ty IndexSet>] = - IndexSet<$ty, BuildFbHasher<$n, S>>; + IndexSet<$ty, FbBuildHasher<$n, S>>; } } )* @@ -76,12 +76,12 @@ macro_rules! assert_eq_unchecked { /// /// **NOTE:** this hasher accepts only `N`-length byte arrays! It is UB to hash anything else. #[derive(Clone, Debug, Default)] -pub struct BuildFbHasher { +pub struct FbBuildHasher { inner: S, _marker: core::marker::PhantomData<[(); N]>, } -impl BuildHasher for BuildFbHasher { +impl BuildHasher for FbBuildHasher { type Hasher = FbHasher; #[inline] @@ -110,21 +110,27 @@ impl Hasher for FbHasher { #[inline] fn write(&mut self, bytes: &[u8]) { assert_eq_unchecked!(bytes.len(), N); - write_bytes(&mut self.inner, bytes); + // Threshold decided by some basic micro-benchmarks with fxhash. + if N > 32 { + self.inner.write(bytes); + } else { + write_bytes(&mut self.inner, bytes); + } } + // We can just skip hashing the length prefix entirely since we know it's always `N`. + // `write_length_prefix` calls `write_usize` by default. #[cfg(not(feature = "nightly"))] #[inline] - fn write_usize(&mut self, len: usize) { - assert_eq_unchecked!(len, N); + fn write_usize(&mut self, i: usize) { + assert_eq_unchecked!(i, N); } #[cfg(feature = "nightly")] #[inline] fn write_length_prefix(&mut self, len: usize) { assert_eq_unchecked!(len, N); - // We can just skip hashing the length prefix entirely since we know it's always `N`. } } @@ -167,7 +173,7 @@ mod tests { use super::*; fn hash_zero() -> u64 { - BuildFbHasher::::default().hash_one(&FixedBytes::::ZERO) + FbBuildHasher::::default().hash_one(&FixedBytes::::ZERO) } #[test] @@ -175,7 +181,7 @@ mod tests { // Just by running it once we test that it compiles and that debug assertions are correct. ruint::const_for!(N in [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, - 32, 33, 34, 35, 36, 37, 38, 39, 40, 41, 42, 43, 44, 45, 46, 47] { + 32, 47, 48, 49, 63, 64, 127, 128, 256, 512, 1024, 2048, 4096] { assert_ne!(hash_zero::(), 0); }); } From f0d11e8dddcce3106b208dfad8c8d9dabed2abb9 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Sat, 21 Sep 2024 00:24:02 +0200 Subject: [PATCH 07/11] fix --- crates/primitives/src/map/fixed.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/primitives/src/map/fixed.rs b/crates/primitives/src/map/fixed.rs index ebded0ebb..6d67d5dae 100644 --- a/crates/primitives/src/map/fixed.rs +++ b/crates/primitives/src/map/fixed.rs @@ -182,7 +182,7 @@ mod tests { ruint::const_for!(N in [ 0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 26, 27, 28, 29, 30, 31, 32, 47, 48, 49, 63, 64, 127, 128, 256, 512, 1024, 2048, 4096] { - assert_ne!(hash_zero::(), 0); + let _ = hash_zero::(); }); } } From df640c9ad79ac4d1cd51f9f501c3568cf27b7a3c Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 24 Sep 2024 17:40:44 +0200 Subject: [PATCH 08/11] feat: docs, disable hashbrown by default --- crates/core/Cargo.toml | 10 ++++- crates/primitives/Cargo.toml | 9 +++-- crates/primitives/src/map/fixed.rs | 20 +++++++--- crates/primitives/src/map/mod.rs | 61 +++++++++++++++++++++++------- 4 files changed, 76 insertions(+), 24 deletions(-) diff --git a/crates/core/Cargo.toml b/crates/core/Cargo.toml index 431873a3a..74288adf1 100644 --- a/crates/core/Cargo.toml +++ b/crates/core/Cargo.toml @@ -30,12 +30,20 @@ alloy-sol-types = { workspace = true, optional = true } alloy-rlp = { workspace = true, optional = true } [features] -default = ["std", "map-fxhash"] +default = [ + "std", + "alloy-primitives/default", + "alloy-dyn-abi?/default", + "alloy-json-abi?/default", + "alloy-sol-types?/default", + "alloy-rlp?/default", +] std = [ "alloy-primitives/std", "alloy-json-abi?/std", "alloy-dyn-abi?/std", "alloy-sol-types?/std", + "alloy-rlp?/std", ] nightly = ["alloy-primitives/nightly"] diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index a7a65f53a..7b0f90ddf 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -67,7 +67,10 @@ rand = { workspace = true, optional = true, features = ["getrandom"] } k256 = { workspace = true, optional = true, features = ["ecdsa"] } # map -hashbrown = { workspace = true, optional = true, features = ["inline-more"] } +hashbrown = { workspace = true, optional = true, features = [ + "ahash", + "inline-more", +] } indexmap = { workspace = true, optional = true } rustc-hash = { workspace = true, optional = true } @@ -117,8 +120,8 @@ native-keccak = [] sha3-keccak = ["dep:sha3"] tiny-keccak = [] -map = ["dep:hashbrown"] -map-hashbrown = ["map"] +map = [] +map-hashbrown = ["map", "dep:hashbrown"] map-indexmap = ["map", "dep:indexmap"] map-fxhash = ["map", "dep:rustc-hash"] diff --git a/crates/primitives/src/map/fixed.rs b/crates/primitives/src/map/fixed.rs index 6d67d5dae..7f1c52454 100644 --- a/crates/primitives/src/map/fixed.rs +++ b/crates/primitives/src/map/fixed.rs @@ -74,7 +74,7 @@ macro_rules! assert_eq_unchecked { /// /// Works best with `fxhash`, enabled by default with the "map-fxhash" feature. /// -/// **NOTE:** this hasher accepts only `N`-length byte arrays! It is UB to hash anything else. +/// **NOTE:** this hasher accepts only `N`-length byte arrays! It is invalid to hash anything else. #[derive(Clone, Debug, Default)] pub struct FbBuildHasher { inner: S, @@ -94,7 +94,7 @@ impl BuildHasher for FbBuildHasher { /// /// Works best with `fxhash`, enabled by default with the "map-fxhash" feature. /// -/// **NOTE:** this hasher accepts only `N`-length byte arrays! It is UB to hash anything else. +/// **NOTE:** this hasher accepts only `N`-length byte arrays! It is invalid to hash anything else. #[derive(Clone, Debug, Default)] pub struct FbHasher { inner: H, @@ -114,7 +114,7 @@ impl Hasher for FbHasher { if N > 32 { self.inner.write(bytes); } else { - write_bytes(&mut self.inner, bytes); + write_bytes_unrolled(&mut self.inner, bytes); } } @@ -124,18 +124,18 @@ impl Hasher for FbHasher { #[cfg(not(feature = "nightly"))] #[inline] fn write_usize(&mut self, i: usize) { - assert_eq_unchecked!(i, N); + debug_assert_eq!(i, N); } #[cfg(feature = "nightly")] #[inline] fn write_length_prefix(&mut self, len: usize) { - assert_eq_unchecked!(len, N); + debug_assert_eq!(len, N); } } #[inline(always)] -fn write_bytes(hasher: &mut impl Hasher, mut bytes: &[u8]) { +fn write_bytes_unrolled(hasher: &mut impl Hasher, mut bytes: &[u8]) { while let Some((chunk, rest)) = bytes.split_first_chunk() { hasher.write_usize(usize::from_ne_bytes(*chunk)); bytes = rest; @@ -185,4 +185,12 @@ mod tests { let _ = hash_zero::(); }); } + + #[test] + fn map() { + let mut map = AddressHashMap::::default(); + map.insert(Address::ZERO, true); + assert_eq!(map.get(&Address::ZERO), Some(&true)); + assert_eq!(map.get(&Address::with_last_byte(1)), None); + } } diff --git a/crates/primitives/src/map/mod.rs b/crates/primitives/src/map/mod.rs index 016d4ec77..a2e3dcd42 100644 --- a/crates/primitives/src/map/mod.rs +++ b/crates/primitives/src/map/mod.rs @@ -1,18 +1,45 @@ //! Re-exports of map types and utilities. +//! +//! This module exports the following types: +//! - [`HashMap`] and [`HashSet`] from the standard library or `hashbrown` crate. The +//! "map-hashbrown" feature can be used to force the use of `hashbrown`, and is required in +//! `no_std` environments. +//! - [`IndexMap`] and [`IndexSet`] from the `indexmap` crate, if the "map-indexmap" feature is +//! enabled. +//! - The previously-listed hash map types prefixed with `Fx` if the "map-fxhash" feature is +//! enabled. These are type aliases with [`FxBuildHasher`] as the hasher builder. +//! - The previously-listed hash map types prefixed with `Fb`. These are type aliases with +//! [`FixedBytes`][fb] as the key, and [`FbBuildHasher`] as the hasher builder. This hasher is +//! optimized for hashing fixed-size byte arrays, and wraps around the default hasher builder. It +//! performs best when the hasher is `fxhash`, which is enabled by default with the "map-fxhash" +//! feature. +//! - The previously-listed hash map types prefixed with [`Selector`], [`Address`], and [`B256`]. +//! These use [`FbBuildHasher`] with the respective fixed-size byte array as the key. See the +//! previous point for more information. +//! +//! Unless specified otherwise, the default hasher builder used by these types is +//! [`DefaultHashBuilder`]. This hasher prioritizes speed over security. Users who require HashDoS +//! resistance should enable the "rand" feature so that the hasher is initialized using a random +//! seed. +//! +//! [fb]: crate::FixedBytes +//! [`Selector`]: crate::Selector +//! [`Address`]: crate::Address +//! [`B256`]: crate::B256 use cfg_if::cfg_if; mod fixed; pub use fixed::*; -use hashbrown as _; - -// Use `hashbrown` if requested with "map-hashbrown" or required by `no_std`. +// The `HashMap` type implementation. cfg_if! { - if #[cfg(any(feature = "map-hashbrown", not(feature = "std")))] { + if #[cfg(feature = "map-hashbrown")] { use hashbrown as imp; - } else { + } else if #[cfg(feature = "std")] { use std::collections as imp; + } else { + compile_error!("The `map-hashbrown` feature is required in `no_std` environments."); } } @@ -36,14 +63,19 @@ cfg_if! { cfg_if! { if #[cfg(all(feature = "std", feature = "rand"))] { - #[doc(no_inline)] - pub use rustc_hash::FxRandomState as FxBuildHasher; + use rustc_hash::FxRandomState as FxBuildHasherInner; } else { - #[doc(no_inline)] - pub use rustc_hash::FxBuildHasher; + use rustc_hash::FxBuildHasher as FxBuildHasherInner; } } + /// The [`FxHasher`] hasher builder. + /// + /// This is [`rustc_hash::FxBuildHasher`], unless both the "std" and "rand" features are + /// enabled, in which case it will be [`rustc_hash::FxRandomState`] for better security at + /// very little cost. If this is not preferred, consider using the `Fx*` aliases directly. + pub type FxBuildHasher = FxBuildHasherInner; + /// A [`HashMap`] using [`FxHasher`] as its hasher. pub type FxHashMap = HashMap; /// A [`HashSet`] using [`FxHasher`] as its hasher. @@ -55,21 +87,22 @@ cfg_if! { cfg_if! { if #[cfg(feature = "map-fxhash")] { type DefaultHashBuilderInner = FxBuildHasher; + } else if #[cfg(feature = "map-hashbrown")] { + type DefaultHashBuilderInner = hashbrown::hash_map::DefaultHashBuilder; } else if #[cfg(feature = "std")] { type DefaultHashBuilderInner = std::collections::hash_map::RandomState; } else { - type DefaultHashBuilderInner = hashbrown::hash_map::DefaultHashBuilder; + // An error has already been emitted in the `imp` block above. + type DefaultHashBuilderInner = (); } } /// The default [`BuildHasher`](core::hash::BuildHasher) used by [`HashMap`] and [`HashSet`]. /// -/// This hasher prioritizes speed over security, even if it is still secure enough for most -/// applications thanks to the use of a random seed. +/// See [the module documentation](self) for more information on the default hasher. pub type DefaultHashBuilder = DefaultHashBuilderInner; /// The default [`Hasher`](core::hash::Hasher) used by [`HashMap`] and [`HashSet`]. /// -/// This hasher prioritizes speed over security, even if it is still secure enough for most -/// applications thanks to the use of a random seed. +/// See [the module documentation](self) for more information on the default hasher. pub type DefaultHasher = ::Hasher; // `indexmap` re-exports. From 324947182e40fddd60e826cb4b8abab75d5dc73b Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 24 Sep 2024 18:12:39 +0200 Subject: [PATCH 09/11] ci: add more flags to cargo hack --- .github/workflows/ci.yml | 26 ++++++++++++-------------- scripts/check_features.sh | 7 +++++++ 2 files changed, 19 insertions(+), 14 deletions(-) create mode 100755 scripts/check_features.sh diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 6f00bae7a..57457dfb8 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -94,30 +94,28 @@ jobs: run: cargo check --workspace --target wasm32-unknown-unknown feature-checks: + name: features ${{ matrix.rust }} ${{ matrix.flags }} runs-on: ubuntu-latest timeout-minutes: 30 + strategy: + fail-fast: false + matrix: + rust: ["stable", "nightly"] + flags: ["", "--all-targets"] steps: - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable - - uses: taiki-e/install-action@cargo-hack - - uses: Swatinem/rust-cache@v2 + - uses: dtolnay/rust-toolchain@master with: - cache-on-failure: true - - name: cargo hack - run: cargo hack check --each-feature --skip nightly - - feature-checks-all-targets: - runs-on: ubuntu-latest - timeout-minutes: 30 - steps: - - uses: actions/checkout@v4 - - uses: dtolnay/rust-toolchain@stable + toolchain: ${{ matrix.rust }} - uses: taiki-e/install-action@cargo-hack - uses: Swatinem/rust-cache@v2 with: cache-on-failure: true - name: cargo hack - run: cargo hack check --each-feature --skip nightly --all-targets + run: | + args=(${{ matrix.flags }}) + [ ${{ matrix.rust }} == "stable" ] && args+=(--skip nightly) + ./scripts/check_features.sh "${args[@]}" check-no-std: name: check no_std ${{ matrix.features }} diff --git a/scripts/check_features.sh b/scripts/check_features.sh new file mode 100755 index 000000000..257dcd5e0 --- /dev/null +++ b/scripts/check_features.sh @@ -0,0 +1,7 @@ +#!/usr/bin/env bash +set -eo pipefail + +cargo hack check --feature-powerset --depth 1 \ + --group-features std,map --group-features std,map-fxhash --group-features std,map-indexmap \ + --ignore-unknown-features \ + "${@}" From 874c935d636fb5e3577fb4d7c161d08a2bd4ca4f Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 24 Sep 2024 22:11:34 +0200 Subject: [PATCH 10/11] feat: remove S generic from Fb* types Easier to change the implementation later on. --- crates/primitives/src/map/fixed.rs | 61 +++++++++++++++++------------- 1 file changed, 35 insertions(+), 26 deletions(-) diff --git a/crates/primitives/src/map/fixed.rs b/crates/primitives/src/map/fixed.rs index 7f1c52454..f59a86cf3 100644 --- a/crates/primitives/src/map/fixed.rs +++ b/crates/primitives/src/map/fixed.rs @@ -1,23 +1,24 @@ use super::*; use crate::{Address, FixedBytes, Selector, B256}; use cfg_if::cfg_if; -use core::hash::{BuildHasher, Hasher}; +use core::{ + fmt, + hash::{BuildHasher, Hasher}, +}; /// [`HashMap`] optimized for hashing [fixed-size byte arrays](FixedBytes). -pub type FbHashMap = - HashMap, V, FbBuildHasher>; +pub type FbHashMap = HashMap, V, FbBuildHasher>; /// [`HashSet`] optimized for hashing [fixed-size byte arrays](FixedBytes). -pub type FbHashSet = - HashSet, FbBuildHasher>; +pub type FbHashSet = HashSet, FbBuildHasher>; cfg_if! { if #[cfg(feature = "map-indexmap")] { /// [`IndexMap`] optimized for hashing [fixed-size byte arrays](FixedBytes). - pub type FbIndexMap = - indexmap::IndexMap, V, FbBuildHasher>; + pub type FbIndexMap = + indexmap::IndexMap, V, FbBuildHasher>; /// [`IndexSet`] optimized for hashing [fixed-size byte arrays](FixedBytes). - pub type FbIndexSet = - indexmap::IndexSet, FbBuildHasher>; + pub type FbIndexSet = + indexmap::IndexSet, FbBuildHasher>; } } @@ -25,20 +26,16 @@ macro_rules! fb_alias_maps { ($($ty:ident < $n:literal >),* $(,)?) => { paste::paste! { $( #[doc = concat!("[`HashMap`] optimized for hashing [`", stringify!($ty), "`].")] - pub type [<$ty HashMap>] = - HashMap<$ty, V, FbBuildHasher<$n, S>>; + pub type [<$ty HashMap>] = HashMap<$ty, V, FbBuildHasher<$n>>; #[doc = concat!("[`HashSet`] optimized for hashing [`", stringify!($ty), "`].")] - pub type [<$ty HashSet>] = - HashSet<$ty, FbBuildHasher<$n, S>>; + pub type [<$ty HashSet>] = HashSet<$ty, FbBuildHasher<$n>>; cfg_if! { if #[cfg(feature = "map-indexmap")] { #[doc = concat!("[`IndexMap`] optimized for hashing [`", stringify!($ty), "`].")] - pub type [<$ty IndexMap>] = - IndexMap<$ty, V, FbBuildHasher<$n, S>>; + pub type [<$ty IndexMap>] = IndexMap<$ty, V, FbBuildHasher<$n>>; #[doc = concat!("[`IndexSet`] optimized for hashing [`", stringify!($ty), "`].")] - pub type [<$ty IndexSet>] = - IndexSet<$ty, FbBuildHasher<$n, S>>; + pub type [<$ty IndexSet>] = IndexSet<$ty, FbBuildHasher<$n>>; } } )* @@ -75,14 +72,20 @@ macro_rules! assert_eq_unchecked { /// Works best with `fxhash`, enabled by default with the "map-fxhash" feature. /// /// **NOTE:** this hasher accepts only `N`-length byte arrays! It is invalid to hash anything else. -#[derive(Clone, Debug, Default)] -pub struct FbBuildHasher { - inner: S, +#[derive(Default)] +pub struct FbBuildHasher { + inner: DefaultHashBuilder, _marker: core::marker::PhantomData<[(); N]>, } -impl BuildHasher for FbBuildHasher { - type Hasher = FbHasher; +impl fmt::Debug for FbBuildHasher { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("FbBuildHasher").finish_non_exhaustive() + } +} + +impl BuildHasher for FbBuildHasher { + type Hasher = FbHasher; #[inline] fn build_hasher(&self) -> Self::Hasher { @@ -95,13 +98,19 @@ impl BuildHasher for FbBuildHasher { /// Works best with `fxhash`, enabled by default with the "map-fxhash" feature. /// /// **NOTE:** this hasher accepts only `N`-length byte arrays! It is invalid to hash anything else. -#[derive(Clone, Debug, Default)] -pub struct FbHasher { - inner: H, +#[derive(Default)] +pub struct FbHasher { + inner: DefaultHasher, _marker: core::marker::PhantomData<[(); N]>, } -impl Hasher for FbHasher { +impl fmt::Debug for FbHasher { + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("FbHasher").finish_non_exhaustive() + } +} + +impl Hasher for FbHasher { #[inline] fn finish(&self) -> u64 { self.inner.finish() From 2b5462c3a50882334f76d4d7c14a0c530e3330e1 Mon Sep 17 00:00:00 2001 From: DaniPopes <57450786+DaniPopes@users.noreply.github.com> Date: Tue, 24 Sep 2024 22:36:01 +0200 Subject: [PATCH 11/11] fix: keep hashbrown by default for no_std Ideally this is only a depedencny if `not(feature = "std")` but this is currently not something you can have with cargo features. --- crates/primitives/Cargo.toml | 4 ++-- crates/primitives/src/map/mod.rs | 17 +++++++---------- tests/core-sol/Cargo.toml | 2 +- tests/core-sol/src/lib.rs | 8 ++++++++ 4 files changed, 18 insertions(+), 13 deletions(-) diff --git a/crates/primitives/Cargo.toml b/crates/primitives/Cargo.toml index 7b0f90ddf..ee8786acb 100644 --- a/crates/primitives/Cargo.toml +++ b/crates/primitives/Cargo.toml @@ -120,8 +120,8 @@ native-keccak = [] sha3-keccak = ["dep:sha3"] tiny-keccak = [] -map = [] -map-hashbrown = ["map", "dep:hashbrown"] +map = ["dep:hashbrown"] +map-hashbrown = ["map"] map-indexmap = ["map", "dep:indexmap"] map-fxhash = ["map", "dep:rustc-hash"] diff --git a/crates/primitives/src/map/mod.rs b/crates/primitives/src/map/mod.rs index a2e3dcd42..7cdb5a786 100644 --- a/crates/primitives/src/map/mod.rs +++ b/crates/primitives/src/map/mod.rs @@ -32,14 +32,14 @@ use cfg_if::cfg_if; mod fixed; pub use fixed::*; -// The `HashMap` type implementation. +// The `HashMap` implementation. +// Use `hashbrown` if requested with "map-hashbrown" or required by `no_std`. cfg_if! { - if #[cfg(feature = "map-hashbrown")] { + if #[cfg(any(feature = "map-hashbrown", not(feature = "std")))] { use hashbrown as imp; - } else if #[cfg(feature = "std")] { - use std::collections as imp; } else { - compile_error!("The `map-hashbrown` feature is required in `no_std` environments."); + use hashbrown as _; + use std::collections as imp; } } @@ -87,13 +87,10 @@ cfg_if! { cfg_if! { if #[cfg(feature = "map-fxhash")] { type DefaultHashBuilderInner = FxBuildHasher; - } else if #[cfg(feature = "map-hashbrown")] { + } else if #[cfg(any(feature = "map-hashbrown", not(feature = "std")))] { type DefaultHashBuilderInner = hashbrown::hash_map::DefaultHashBuilder; - } else if #[cfg(feature = "std")] { - type DefaultHashBuilderInner = std::collections::hash_map::RandomState; } else { - // An error has already been emitted in the `imp` block above. - type DefaultHashBuilderInner = (); + type DefaultHashBuilderInner = std::collections::hash_map::RandomState; } } /// The default [`BuildHasher`](core::hash::BuildHasher) used by [`HashMap`] and [`HashSet`]. diff --git a/tests/core-sol/Cargo.toml b/tests/core-sol/Cargo.toml index 7f9b7d03e..b7c83830d 100644 --- a/tests/core-sol/Cargo.toml +++ b/tests/core-sol/Cargo.toml @@ -11,4 +11,4 @@ repository.workspace = true exclude.workspace = true [dependencies] -alloy-core = { workspace = true, features = ["sol-types", "json"] } +alloy-core = { workspace = true, features = ["sol-types", "json", "map"] } diff --git a/tests/core-sol/src/lib.rs b/tests/core-sol/src/lib.rs index 511e6381e..a5029c1d1 100644 --- a/tests/core-sol/src/lib.rs +++ b/tests/core-sol/src/lib.rs @@ -16,6 +16,7 @@ sol! { A, B } + #[derive(Default, PartialEq, Eq, Hash)] struct MyStruct { uint32 a; uint64 b; @@ -62,3 +63,10 @@ sol! { type MyOtherType is uint32; } } + +#[test] +fn do_stuff() { + let mut set = alloy_core::primitives::map::HashSet::::default(); + set.insert(Default::default()); + assert_eq!(set.len(), 1); +}