Skip to content

Commit fec93bd

Browse files
xqftJereSalo
authored andcommitted
fix(l2): add test chain with a non-empty block and bug fixes (#2068)
**Motivation** The current `l2-loadtest.rlp` had empty blocks. After replacing with a chain that contains a non-empty block, a bug was discovered when creating a `ExecutionDB` from a `Store`: we would try to filter out new accounts using revm's `AccountStatus` but for some reason every account has the same status (`Touched`), meaning that we were not filtering at all. The solution was to not filter and assume that the `Store` is correct based on the success of the pre-execution (so if an account is missing from the parent state, then that account was created in the block to prove). **Description** - replaces `l2-loadtest.rlp` file with one that contains a non empty block - fixes the bug described above
1 parent 5bb9747 commit fec93bd

File tree

3 files changed

+25
-21
lines changed

3 files changed

+25
-21
lines changed

crates/l2/prover/tests/perf_zkvm.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ async fn setup() -> (ProgramInput, Block) {
8282
);
8383
add_block(block, &store).unwrap();
8484
}
85-
let block_to_prove = blocks.get(6).unwrap();
85+
let block_to_prove = blocks.get(3).unwrap();
8686

8787
let parent_block_header = store
8888
.get_block_header_by_hash(block_to_prove.header.parent_hash)

crates/vm/db.rs

Lines changed: 24 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -247,35 +247,37 @@ impl ToExecDB for StoreWrapper {
247247
});
248248

249249
// fetch all read/written values from store
250-
let already_existing_accounts = cache
251-
.accounts
252-
.iter()
253-
// filter out new accounts, we're only interested in already existing accounts.
254-
// new accounts are storage cleared, self-destructed accounts too but they're marked with "not
255-
// existing" status instead.
256-
.filter_map(|(address, account)| {
257-
if !account.account_state.is_storage_cleared() {
258-
Some((CoreAddress::from(address.0.as_ref()), account))
259-
} else {
260-
None
261-
}
262-
});
263-
let accounts = already_existing_accounts
250+
let cache_accounts = cache.accounts.iter().filter_map(|(address, account)| {
251+
let address = CoreAddress::from(address.0.as_ref());
252+
// filter new accounts (accounts that didn't exist before) assuming our store is
253+
// correct (based on the success of the pre-execution).
254+
if store_wrapper
255+
.store
256+
.get_account_info_by_hash(parent_hash, address)
257+
.is_ok_and(|account| account.is_some())
258+
{
259+
Some((address, account))
260+
} else {
261+
None
262+
}
263+
});
264+
let accounts = cache_accounts
264265
.clone()
265266
.map(|(address, _)| {
266267
// return error if account is missing
267268
let account = match store_wrapper
268269
.store
269270
.get_account_info_by_hash(parent_hash, address)
270271
{
271-
Ok(None) => Err(ExecutionDBError::NewMissingAccountInfo(address)),
272272
Ok(Some(some)) => Ok(some),
273273
Err(err) => Err(ExecutionDBError::Store(err)),
274+
Ok(None) => unreachable!(), // we are filtering out accounts that are not present
275+
// in the store
274276
};
275277
Ok((address, account?))
276278
})
277279
.collect::<Result<HashMap<_, _>, ExecutionDBError>>()?;
278-
let code = already_existing_accounts
280+
let code = cache_accounts
279281
.clone()
280282
.map(|(_, account)| {
281283
// return error if code is missing
@@ -289,7 +291,7 @@ impl ToExecDB for StoreWrapper {
289291
))
290292
})
291293
.collect::<Result<_, ExecutionDBError>>()?;
292-
let storage = already_existing_accounts
294+
let storage = cache_accounts
293295
.map(|(address, account)| {
294296
// return error if storage is missing
295297
Ok((
@@ -349,12 +351,14 @@ impl ToExecDB for StoreWrapper {
349351
let mut storage_proofs = HashMap::new();
350352
let mut final_storage_proofs = HashMap::new();
351353
for (address, storage_keys) in index {
354+
let Some(parent_storage_trie) = self.store.storage_trie(parent_hash, address)? else {
355+
// the storage of this account was empty or the account is newly created, either
356+
// way the storage trie was initially empty so there aren't any proofs to add.
357+
continue;
358+
};
352359
let storage_trie = self.store.storage_trie(block.hash(), address)?.ok_or(
353360
ExecutionDBError::NewMissingStorageTrie(block.hash(), address),
354361
)?;
355-
let parent_storage_trie = self.store.storage_trie(parent_hash, address)?.ok_or(
356-
ExecutionDBError::NewMissingStorageTrie(parent_hash, address),
357-
)?;
358362
let paths = storage_keys.iter().map(hash_key).collect::<Vec<_>>();
359363

360364
let initial_proofs = parent_storage_trie.get_proofs(&paths)?;

test_data/l2-loadtest.rlp

191 KB
Binary file not shown.

0 commit comments

Comments
 (0)