-
Notifications
You must be signed in to change notification settings - Fork 236
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
Replace map types with alloy_primitives::map
#1365
Labels
Comments
DaniPopes
added
enhancement
New feature or request
good first issue
Good for newcomers
labels
Sep 25, 2024
DaniPopes
changed the title
[Feature] Replace map types with
Replace map types with Sep 25, 2024
alloy_primitives::map
alloy_primitives::map
This was referenced Sep 25, 2024
dhruvmalik007
added a commit
to dhruvmalik007/alloy
that referenced
this issue
Sep 25, 2024
- replacing the HashMap and HashSet from the std::collections to the alloy-primitives along with setting up the cargo file. - setting the dependencies in the cargo files
3 tasks
Hi I did some work on this and wanted your feedback , only the BTreemap types and those which the key type is |
nategraf
pushed a commit
to risc0/risc0-ethereum
that referenced
this issue
Sep 27, 2024
This fixes the current compilation errors. Previously, we used to use `revm::primitives::HashMap` to have the same hash in Steel and `revm`. However, with the latest (minor) revm version, this has been switched to using `alloy_primitves::map::HashMap`. Thus, in this PR we also replace all `{Hash,BTree}{Map,Set}` with their more performant and specific equivalent in `alloy_primitives::map`. ### Additional context - alloy-rs/alloy#1365
Wollac
added a commit
to risc0/risc0-ethereum
that referenced
this issue
Oct 1, 2024
This fixes the current compilation errors. Previously, we used to use `revm::primitives::HashMap` to have the same hash in Steel and `revm`. However, with the latest (minor) revm version, this has been switched to using `alloy_primitves::map::HashMap`. Thus, in this PR we also replace all `{Hash,BTree}{Map,Set}` with their more performant and specific equivalent in `alloy_primitives::map`. ### Additional context - alloy-rs/alloy#1365
Wollac
added a commit
to risc0/risc0-ethereum
that referenced
this issue
Oct 7, 2024
This fixes the current compilation errors. Previously, we used to use `revm::primitives::HashMap` to have the same hash in Steel and `revm`. However, with the latest (minor) revm version, this has been switched to using `alloy_primitves::map::HashMap`. Thus, in this PR we also replace all `{Hash,BTree}{Map,Set}` with their more performant and specific equivalent in `alloy_primitives::map`. ### Additional context - alloy-rs/alloy#1365
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Replace all
{Hash,BTree}{Map,Set}
with their equivalent inalloy_primitives::map
, as explained below. This both improves performance and is more compatible withno_std
, as the fallback will be usinghashbrown
when"std"
is not enabled.Migration guide:
std::collections::{HashMap, HashSet}
->alloy_primitives::map::{HashMap, HashSet}
; unless:Selector
,Address
, orB256
, then usealloy_primitives::map::{<Name>{HashMap,HashSet}}
, e.g.AddressHashMap
B*
orFixedBytes
, then usealloy_primitives::map::{FbHashMap, FbHashSet}
std::collections::{hash_map, hash_set}
->alloy_primitives::map::{hash_map, hash_set}
std::collections::{BTreeMap, BTreeSet}
-> refer to 1. aboveserde::Serialize
, BTree should be replaced with the Index* equivalents so as to have a consistent iteration order, rather than random; this requires the"map-index"
feature.split_off
, then there is nothing to do.The minimum
alloy-primitives
version should also be bumped to0.8.4
.Additional context
map
module core#743The text was updated successfully, but these errors were encountered: