refactor: Use BundleBuilder instead of hashmaps #8264
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
quentinv72
commented
May 14, 2024
quentinv72
commented
May 14, 2024
quentinv72
requested review from
Rjected,
gakonst,
joshieDo,
mattsse,
onbjerg,
rakita and
shekhirin
as code owners
emhane
added
D-good-first-issue
emhane
requested changes
May 18, 2024
emhane
approved these changes
May 21, 2024
DaniPopes
added a commit
to paradigmxyz/revm-inspectors
that referenced
this pull request
Jun 16, 2024
Upgrade revm version to help unblock changes needed in paradigmxyz/reth#8264 (comment) --------- Co-authored-by: DaniPopes <[email protected]>
Merged
Contributor
Author
onbjerg
changed the title
rkrasiuk
requested changes
Jun 19, 2024
Comment on lines
+605
to
+608
| if let Some(old_info) = old_info { | ||
| bundle_builder = | ||
| bundle_builder.state_original_account_info(address, into_revm_acc(old_info)); | ||
| } |
Member
There was a problem hiding this comment.
previous behavior: if the entry is occuppied, overwrite the original account state even if old_info is None
current behavior: only overwrite the original account state if old_info is Some
if old_info is None, original info must still be overwritten
Comment on lines
+600
to
+602
| if let Some(new_info) = new_info { | ||
| bundle_builder = | ||
| bundle_builder.state_present_account_info(address, into_revm_acc(new_info)); |
Member
There was a problem hiding this comment.
same as the comment below, this leads to incorrect state for destroyed accounts
| entry.insert((present_info, present_info, HashMap::new())) | ||
| if !bundle_builder.get_states().contains(&address) { | ||
| let present_info = plain_accounts_cursor.seek_exact(address)?.map(|kv| kv.1); | ||
| if let Some(present_info) = present_info { |
Member
There was a problem hiding this comment.
there should be no Some check here
Comment on lines
+631
to
634
| let account_state = match bundle_builder.get_state_storage_mut().entry(address) { | ||
| hash_map::Entry::Occupied(entry) => entry.into_mut(), | ||
| hash_map::Entry::Vacant(entry) => entry.insert(HashMap::new()), | ||
| }; |
Member
There was a problem hiding this comment.
Suggested change
| let account_state = match bundle_builder.get_state_storage_mut().entry(address) { | |
| hash_map::Entry::Occupied(entry) => entry.into_mut(), | |
| hash_map::Entry::Vacant(entry) => entry.insert(HashMap::new()), | |
| }; | |
| let account_storage = bundle_builder.get_state_storage_mut().entry(address).or_default(); |
Member
|
addressing my comments probably requires some rework of |
Contributor
Author
|
Thank you all for reviewing. Unfortunately, I don't think that I'll be able to address the comments on this PR as I have some other commitments at the moment. If anyone else wants to pick this up or close the PR, then please do so. |
evadav-663
added a commit
to evadav-663/revminc
that referenced
this pull request
Oct 27, 2025
Upgrade revm version to help unblock changes needed in paradigmxyz/reth#8264 (comment) --------- Co-authored-by: DaniPopes <[email protected]>
arublee8052-9
added a commit
to arublee8052-9/revminc
that referenced
this pull request
Oct 27, 2025
Upgrade revm version to help unblock changes needed in paradigmxyz/reth#8264 (comment) --------- Co-authored-by: DaniPopes <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes #4614
insert_stateincrates/storage/db-common/src/init.rsto use BundleBuilderunwind_or_peek_stateincrates/storage/provider/src/providers/database/provider.rsto use BundleBuilderBundleStateInit,AccountRevertInit, andRevertInit.