-
Notifications
You must be signed in to change notification settings - Fork 13
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
add verkle support to evm t8n for test framework #432
Conversation
95eb3c9
to
e23e237
Compare
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, some things I'd have to look in more detail to be entirely convinced since I'm not used to how this tool is usually called from the testing infra -- but at some point we'll figure out soon if there's a problem.
Left some comments for your consideration.
core/state/database.go
Outdated
@@ -415,7 +400,7 @@ func (db *cachingDB) OpenStorageTrie(stateRoot common.Hash, address common.Addre | |||
} | |||
} | |||
if db.InTransition() { | |||
fmt.Printf("OpenStorageTrie during transition, state root=%x root=%x\n", stateRoot, root) | |||
// fmt.Printf("OpenStorageTrie during transition, state root=%x root=%x\n", stateRoot, root) |
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.
Nit: wanna keep or remove?
err := vkt.UpdateAccount(addr, account) | ||
if err != nil { | ||
return fmt.Errorf("error inserting account: %w", err) | ||
} |
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.
Is the code missing chunkifying acc.Code
and inserting that?
chunks := trie.ChunkifyCode(acc.Code)
vk.UpdateContractCode(...)
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.
yup, but let's see if this is still in use first
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.
it is
* t8n: add verkle-code-chunk-key cmd Signed-off-by: Ignacio Hagopian <[email protected]> * t8n: add verkle-chunkify-code cmd Signed-off-by: Ignacio Hagopian <[email protected]> * t8n: create separate verkle parent command Signed-off-by: Ignacio Hagopian <[email protected]> --------- Signed-off-by: Ignacio Hagopian <[email protected]>
For PR readers, extra commands where added via #435 |
* fix: add the storage processed flag to t8n. * chore: cancun -> prague typo.
d5e3191
to
122366d
Compare
@@ -1772,7 +1772,7 @@ func (bc *BlockChain) insertChain(chain types.Blocks, setHead bool) (int, error) | |||
// If the verkle activation time hasn't started, declare it as "not started". | |||
// This is so that if the miner activates the conversion, the insertion happens | |||
// in the correct mode. | |||
bc.stateCache.InitTransitionStatus(false, false) | |||
bc.stateCache.InitTransitionStatus(false, false, common.Hash{}) |
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.
note to self: this is probably what kills the shadowfork
parent := p.bc.GetBlockByHash(block.ParentHash()) | ||
if !p.config.IsPrague(parent.Number(), parent.Time()) { | ||
InsertBlockHashHistoryAtEip2935Fork(statedb, block.NumberU64()-1, block.ParentHash(), p.bc) | ||
} else { | ||
ProcessParentBlockHash(statedb, block.NumberU64()-1, block.ParentHash()) | ||
} |
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 was one of the issues: worker.go
also need to be updated not to insert the 256 blocks
This PR is good for merging, but before we can do that, we need to:
|
Rebased in another PR, closing this one |
Rebase of #406
TODO
kaustinen-with-shapella
Not part of this PR: enable verkle for blocktests