-
Notifications
You must be signed in to change notification settings - Fork 110
refactor(levm): use AccountStatus
for managing status of accounts
#4833
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
Conversation
Lines of code reportTotal lines added: Detailed view
|
Benchmark Results ComparisonNo significant difference was registered for any benchmark run. Detailed ResultsBenchmark Results: BubbleSort
Benchmark Results: ERC20Approval
Benchmark Results: ERC20Mint
Benchmark Results: ERC20Transfer
Benchmark Results: Factorial
Benchmark Results: FactorialRecursive
Benchmark Results: Fibonacci
Benchmark Results: FibonacciRecursive
Benchmark Results: ManyHashes
Benchmark Results: MstoreBench
Benchmark Results: Push
Benchmark Results: SstoreBench_no_opt
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR refactors the LEVM to use the AccountStatus
enum for managing account states instead of maintaining a separate destroyed_accounts
HashMap. The changes improve account state tracking by consolidating status management into individual accounts and optimizing state transition processing.
- Remove the
destroyed_accounts
HashSet from GeneralizedDatabase and use AccountStatus enum instead - Add methods to mark accounts as destroyed or modified with proper status transitions
- Optimize state transition processing by skipping unmodified accounts
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
File | Description |
---|---|
crates/vm/levm/src/hooks/default_hook.rs | Replace destroyed_accounts HashMap usage with account.mark_destroyed() method |
crates/vm/levm/src/db/gen_db.rs | Remove destroyed_accounts field and logic, add account modification tracking, update state transition processing |
crates/vm/levm/src/account.rs | Add mark_destroyed() and mark_modified() methods, update AccountStatus enum with better documentation |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!
Motivation
AccountStatus
enum that we created some time ago in order to know the state of an account at a given moment.Description
destroyed_accounts
in the GeneralizedDatabase because each account will keep track of their state.DestroyedModified
category.Closes #3948 (issue related to the
destroyed_accounts
part)