-
Notifications
You must be signed in to change notification settings - Fork 14
EIP-7748 test filling #534
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
base: kaustinen-with-shapella
Are you sure you want to change the base?
Conversation
Signed-off-by: Ignacio Hagopian <[email protected]>
Signed-off-by: Ignacio Hagopian <[email protected]>
Signed-off-by: Ignacio Hagopian <[email protected]>
if !bytes.Equal(acc.CodeHash, types.EmptyCodeHash[:]) { | ||
code := rawdb.ReadCode(statedb.Database().DiskDB(), common.BytesToHash(acc.CodeHash)) | ||
chunks := trie.ChunkifyCode(code) | ||
count += uint64(len(chunks)) / 32 |
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.
This is a required fix to the conversion code. While in EIP-7748 we always migrate all the code in one go, we still need to count the number of chunks as used stride quota.
For more info see the EIP python implementation and the corresponding rationale section.
// TODO(jsign): for fixture execution this isn't needed since it's configured (hardcoded) | ||
// in the forks.go file. But it's required for filling! Fix this eventually when the testing | ||
// framework passes flag. | ||
maxMovedCount = 7 |
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.
Fix the stride until the testing-framework supports configuring this from the test side.
Seven is a big enough number that allows to create most of the situations we're interested covering.
Started: true, | ||
// initialize so that the first storage-less accounts are processed | ||
StorageProcessed: true, | ||
StorageProcessed: false, |
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.
We already discussed this offline. The true
value conflicts with the first account to be converted being a contract. When re-running the Holesky shadow fork again, we can further confirm this is fine.
TerminalTotalDifficulty: big.NewInt(0), | ||
ShanghaiTime: u64(0), | ||
VerkleTime: u64(32), | ||
OverlayStride: 7, |
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.
This is where we'll wire the new flag to be added in the testing framework to make the stride dynamic (and remove the hardcoded value mentioned in a previous comment).
This PR contains required changes to support test filling for EIP-7748.
This is still work in progress.