Skip to content

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.

Sign up for GitHub

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

Draft
wants to merge 6 commits into
base: kaustinen-with-shapella
Choose a base branch
from
Draft

EIP-7748 test filling #534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9396540
hardcode stride for now
jsign Feb 27, 2025
a99be35
conversion: count code chunks
jsign Feb 27, 2025
e2d17b5
initialize StorageProcessed to false
jsign Mar 11, 2025
e2e30a6
fix code chunk counting
jsign Mar 11, 2025
2a332c6
use a stride of 7 for better test cases
jsign Mar 11, 2025
5475b2d
update stride to 7
jsign Mar 12, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions core/overlay/conversion.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,16 @@
}

// OverlayVerkleTransition contains the overlay conversion logic
func OverlayVerkleTransition(statedb *state.StateDB, root common.Hash, maxMovedCount uint64) error {

Check failure on line 217 in core/overlay/conversion.go

View workflow job for this annotation

GitHub Actions / lint 

SA4009: argument maxMovedCount is overwritten before first use (staticcheck)
migrdb := statedb.Database()
migrdb.LockCurrentTransitionState()
defer migrdb.UnLockCurrentTransitionState()

// 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

Check failure on line 225 in core/overlay/conversion.go

View workflow job for this annotation

GitHub Actions / lint 

SA4009(related information): assignment to maxMovedCount (staticcheck)
Comment on lines +222 to +225
Copy link
Collaborator Author

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.


// verkle transition: if the conversion process is in progress, move
// N values from the MPT into the verkle tree.
if migrdb.InTransition() {
Expand Down Expand Up @@ -373,6 +378,7 @@
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
Copy link
Collaborator Author

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.


mkv.addAccountCode(migrdb.GetCurrentAccountAddress().Bytes(), uint64(len(code)), chunks)
}
Expand Down
2 changes: 1 addition & 1 deletion core/state/database.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,7 @@ func (db *cachingDB) StartVerkleTransition(originalRoot, translatedRoot common.H
db.CurrentTransitionState = &TransitionState{
Started: true,
// initialize so that the first storage-less accounts are processed
StorageProcessed: true,
StorageProcessed: false,
Copy link
Collaborator Author

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.

}
// db.AddTranslation(originalRoot, translatedRoot)
db.baseRoot = originalRoot
Expand Down
1 change: 1 addition & 0 deletions tests/init.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -355,6 +355,7 @@ var Forks = map[string]*params.ChainConfig{
TerminalTotalDifficulty: big.NewInt(0),
ShanghaiTime: u64(0),
VerkleTime: u64(32),
OverlayStride: 7,
Copy link
Collaborator Author

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).

},
}

Expand Down
Loading