-
Notifications
You must be signed in to change notification settings - Fork 33
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
feat(l1): in-memory cache database and new EvmState
#888
Conversation
Co-authored-by: Federico Borello <156438142+fborello-lambda@users.noreply.github.com>
crates/vm/execution_db.rs
Outdated
.get_latest_block_number()? | ||
.ok_or(ExecutionDBError::NewMissingBlockNumber())?; | ||
|
||
let numbers = earliest_number..=latest_number; |
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.
Why is this needed? This doesn't scale well, since for every new block you will have to do a new db read.
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.
What you mean is that I'm reading the hashes of all blocks every time from the DB instead of retrieving them once and then adding each incoming block hash to the list, right?
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.
I guess so. Do you need yo know every single block hash since genesis every time to make the proof?
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.
In mainnet there are 21M blocks since genesis..
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.
Yes, it's not optimized, but it’s not intended for mainnet. We require all prior blocks strictly necessary
to execute any desired block. The goal is to create the smallest possible "database", that can be de/serialiazable, to facilitate the execute_block
operation for the prover. Since we're currently testing the L2, we have a few blocks, so this approach is feasible.
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.
i've created an issue to track this problem/optimization.
#1016
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.
Ok
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.
Turns out that for our current tests we don't need any block hashes (the same way we don't need any bytecode). We can leave this hashmap empty as well and modify it accordingly if we have to in the future.
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.
just applied that change.
Motivation
implement a memory cache database and expand the
EvmState
struct into an enum. This is to take advantage of the block execution and validation implementations of the L1 to use it for proving in L2 mode.Description
EvmState
enum containing two variants (store and cache)ExecutionDB
database for storing only the relevant data of the execution of a block (this is going to be passed into a zkVM for proving)EvmState
functions and adapts the codebase to those changes