Skip to content
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

[EVM] Adding account/slot/code iterators to the base storage #6555

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

ramtinms
Copy link
Member

@ramtinms ramtinms commented Oct 10, 2024

This PR adds some utility methods to the base storage, for iterating over accounts, codes and slots. This functionality is used later by an evm state exporter utility command (follow up PRs are coming).

@ramtinms ramtinms changed the title [EVM] Adding account/slots/codes iterators to the base storage [EVM] Adding account/slot/code iterators to the base storage Oct 10, 2024
@ramtinms ramtinms marked this pull request as ready for review October 10, 2024 19:49
@codecov-commenter
Copy link

codecov-commenter commented Oct 10, 2024

Codecov Report

Attention: Patch coverage is 23.63636% with 168 lines in your changes missing coverage. Please review.

Project coverage is 41.54%. Comparing base (0dde730) to head (9696e41).

Files with missing lines Patch % Lines
fvm/evm/emulator/state/exporter.go 0.00% 111 Missing ⚠️
fvm/evm/emulator/state/base.go 53.94% 24 Missing and 11 partials ⚠️
fvm/evm/emulator/state/code.go 0.00% 8 Missing ⚠️
fvm/evm/types/state.go 0.00% 8 Missing ⚠️
fvm/evm/emulator/state/collection.go 60.00% 4 Missing and 2 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #6555      +/-   ##
==========================================
+ Coverage   41.28%   41.54%   +0.25%     
==========================================
  Files        2033     2035       +2     
  Lines      145981   181915   +35934     
==========================================
+ Hits        60269    75568   +15299     
- Misses      79486   100107   +20621     
- Partials     6226     6240      +14     
Flag Coverage Δ
unittests 41.54% <23.63%> (+0.25%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@fxamacker fxamacker left a comment

Choose a reason for hiding this comment

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

Nice! I left some minor comments.

fvm/evm/emulator/state/base.go Outdated Show resolved Hide resolved
fvm/evm/emulator/state/base_test.go Show resolved Hide resolved
fvm/evm/emulator/state/base.go Outdated Show resolved Hide resolved
fvm/evm/emulator/state/base.go Outdated Show resolved Hide resolved
@@ -0,0 +1,163 @@
package state
Copy link
Contributor

Choose a reason for hiding this comment

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

should this perhaps be in util somewhere?

Comment on lines +342 to +344
require.Equal(t, nonces[acc.Address], acc.Nonce)
require.Equal(t, balances[acc.Address].Uint64(), acc.Balance.Uint64())
require.Equal(t, codeHashes[acc.Address], acc.CodeHash)
Copy link
Member

Choose a reason for hiding this comment

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

The deletion ensures we see all unique account. Otherwise, returning the same account would also pass the tests.

Suggested change
require.Equal(t, nonces[acc.Address], acc.Nonce)
require.Equal(t, balances[acc.Address].Uint64(), acc.Balance.Uint64())
require.Equal(t, codeHashes[acc.Address], acc.CodeHash)
require.Equal(t, nonces[acc.Address], acc.Nonce)
delete(nonces, acc.Address)
require.Equal(t, balances[acc.Address].Uint64(), acc.Balance.Uint64())
delete(balances, acc.Address)
require.Equal(t, codeHashes[acc.Address], acc.CodeHash)
delete(codeHashes, acc.Address)

Comment on lines +390 to +391
require.Equal(t, codeByCodeHash[cic.Hash], cic.Code)
require.Equal(t, refCountByCodeHash[cic.Hash], cic.RefCounts)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
require.Equal(t, codeByCodeHash[cic.Hash], cic.Code)
require.Equal(t, refCountByCodeHash[cic.Hash], cic.RefCounts)
require.Equal(t, codeByCodeHash[cic.Hash], cic.Code)
delete(codeByCodeHash, cic.Hash)
require.Equal(t, refCountByCodeHash[cic.Hash], cic.RefCounts)
delete(refCountByCodeHash, cic.Hash)

break
}
require.Equal(t, addr, slot.Address)
require.Equal(t, values[slot.Key], slot.Value)
Copy link
Member

Choose a reason for hiding this comment

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

same here

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants