Skip to content

Commit 4c5cc3a

Browse files
authored
refactor(revme): separate bytecode from account in test state (#3803)
* fix(handler): derive 7702 delegated_before_tx from the code hash The EIP-8037 auth-refund bookkeeping read the authority's pre-transaction bytecode from the account's original info, which carries no code when the database serves bytecode separately from the account (code only via code_by_hash). A pre-existing delegation was then missed and the clearing refund under-counted. Derive the fact from the original code hash instead: an accepted authorization's code is guaranteed empty-or-delegation, so a non-empty hash is necessarily a delegation. * refactor(revme): serve test bytecode through code_by_hash Seed statetest and blockchaintest pre-state with the bytecode stored separately from the account (contracts map keyed by code hash) instead of inlined in AccountInfo. Execution now has to fetch code through Database::code_by_hash, the way a node's state provider serves it, so the fixtures exercise every code-loading path and catch loads that forget to fetch bytecode. Post-state code validation in blockchaintest falls back to code_by_hash when the account carries only a hash, and the debug pre-state keeps the code inlined for readable output.
1 parent a094a93 commit 4c5cc3a

3 files changed

Lines changed: 82 additions & 41 deletions

File tree

bins/revme/src/cmd/blockchaintest.rs

Lines changed: 62 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -337,15 +337,18 @@ fn validate_post_state(
337337
}
338338

339339
for (address, expected_account) in expected_post_state {
340-
// Load account from final state
341-
let actual_account = state
342-
.load_cache_account(*address)
343-
.map_err(|e| TestExecutionError::Database(format!("Account load failed: {e}")))?;
344-
let info = actual_account
345-
.account
346-
.as_ref()
347-
.map(|a| a.info.clone())
348-
.unwrap_or_default();
340+
// Load account from final state. Info and storage are cloned so the borrow
341+
// of `state` ends here and it can be queried again below (e.g. for code).
342+
let (info, actual_storage) = {
343+
let actual_account = state
344+
.load_cache_account(*address)
345+
.map_err(|e| TestExecutionError::Database(format!("Account load failed: {e}")))?;
346+
let account = actual_account.account.as_ref();
347+
(
348+
account.map(|a| a.info.clone()).unwrap_or_default(),
349+
account.map(|a| a.storage.clone()).unwrap_or_default(),
350+
)
351+
};
349352

350353
// Validate balance
351354
if info.balance != expected_account.balance {
@@ -376,9 +379,19 @@ fn validate_post_state(
376379
);
377380
}
378381

379-
// Validate code if present
382+
// Validate code if present. The account may carry only the code hash when the
383+
// code was never loaded during execution, so fall back to `code_by_hash`.
380384
if !expected_account.code.is_empty() {
381-
if let Some(actual_code) = &info.code {
385+
let actual_code = match info.code.clone() {
386+
Some(code) => Some(code),
387+
None if !info.is_empty_code_hash() => {
388+
Some(Database::code_by_hash(state, info.code_hash).map_err(|e| {
389+
TestExecutionError::Database(format!("Code load failed: {e}"))
390+
})?)
391+
}
392+
None => None,
393+
};
394+
if let Some(actual_code) = &actual_code {
382395
if actual_code.original_bytes() != expected_account.code {
383396
return make_failure(
384397
state,
@@ -405,23 +418,21 @@ fn validate_post_state(
405418
}
406419
}
407420

408-
// Check for unexpected storage entries. Avoid allocating a temporary HashMap when the account is None.
409-
if let Some(acc) = actual_account.account.as_ref() {
410-
for (slot, actual_value) in &acc.storage {
411-
let slot = *slot;
412-
let actual_value = *actual_value;
413-
if !expected_account.storage.contains_key(&slot) && !actual_value.is_zero() {
414-
return make_failure(
415-
state,
416-
debug_info,
417-
expected_post_state,
418-
print_env_on_error,
419-
*address,
420-
format!("storage_unexpected[{slot}]"),
421-
"0x0".to_string(),
422-
format!("{actual_value}"),
423-
);
424-
}
421+
// Check for unexpected storage entries.
422+
for (slot, actual_value) in &actual_storage {
423+
let slot = *slot;
424+
let actual_value = *actual_value;
425+
if !expected_account.storage.contains_key(&slot) && !actual_value.is_zero() {
426+
return make_failure(
427+
state,
428+
debug_info,
429+
expected_post_state,
430+
print_env_on_error,
431+
*address,
432+
format!("storage_unexpected[{slot}]"),
433+
"0x0".to_string(),
434+
format!("{actual_value}"),
435+
);
425436
}
426437
}
427438

@@ -669,20 +680,38 @@ fn execute_blockchain_test(
669680
// Capture pre-state for debug info
670681
let mut pre_state_debug = AddressMap::default();
671682

672-
// Insert genesis state into database
683+
// Insert genesis state into database. Bytecode is stored separately from the
684+
// account (in the contracts map, keyed by code hash) so that execution has to
685+
// fetch it through `Database::code_by_hash`, like a node's state provider would
686+
// serve it.
673687
let genesis_state = test_case.pre.clone().into_genesis_state();
674688
for (address, account) in genesis_state {
689+
let code_hash = revm::primitives::keccak256(&account.code);
690+
let bytecode = (!account.code.is_empty()).then(|| Bytecode::new_raw(account.code.clone()));
675691
let account_info = AccountInfo {
676692
balance: account.balance,
677693
nonce: account.nonce,
678-
code_hash: revm::primitives::keccak256(&account.code),
679-
code: Some(Bytecode::new_raw(account.code.clone())),
694+
code_hash,
695+
code: None,
680696
account_id: None,
681697
};
682698

683-
// Store for debug info
699+
if let Some(bytecode) = &bytecode {
700+
state.cache.contracts.insert(code_hash, bytecode.clone());
701+
}
702+
703+
// Store for debug info, with the code inlined so it shows up in the debug print.
684704
if print_env_on_error {
685-
pre_state_debug.insert(address, (account_info.clone(), account.storage.clone()));
705+
pre_state_debug.insert(
706+
address,
707+
(
708+
AccountInfo {
709+
code: bytecode,
710+
..account_info.clone()
711+
},
712+
account.storage.clone(),
713+
),
714+
);
686715
}
687716

688717
state.insert_account_with_storage(address, account_info, account.storage);

crates/handler/src/pre_execution.rs

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -311,18 +311,23 @@ pub fn apply_auth_list<
311311
// delegated_before_tx — its code at the start of the transaction was a
312312
// delegation (may differ from `delegated_now` when
313313
// an earlier authorization in this tx cleared it).
314+
// Derived from the code hash because the original
315+
// info carries no bytecode when the database serves
316+
// code separately from the account; a non-empty
317+
// hash is necessarily a delegation here, since code
318+
// only changes within a transaction through earlier
319+
// accepted authorizations, which keep it
320+
// empty-or-delegation.
314321
// clearing — this authorization clears the delegation.
315322
let existed = !(authority_acc_info.is_empty()
316323
&& authority_acc
317324
.account()
318325
.is_loaded_as_not_existing_not_touched());
319326
let delegated_now = !authority_acc_info.is_code_hash_empty_or_zero();
320-
let delegated_before_tx = authority_acc
327+
let delegated_before_tx = !authority_acc
321328
.account()
322329
.original_info()
323-
.code
324-
.as_ref()
325-
.is_some_and(Bytecode::is_eip7702);
330+
.is_code_hash_empty_or_zero();
326331
let clearing = authorization.address().is_zero();
327332

328333
// Existing authority: the worst-case `ACCOUNT_WRITE` regular gas and the

crates/statetest-types/src/test_unit.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -60,19 +60,26 @@ impl TestUnit {
6060
/// This function uses [`TestUnit::pre`] to prepare the pre-state from the test unit.
6161
/// It creates a new cache state and inserts the accounts from the test unit.
6262
///
63+
/// Bytecode is stored separately from the account (in the contracts map, keyed by
64+
/// code hash) so that execution has to fetch it through `Database::code_by_hash`,
65+
/// like a node's state provider would serve it.
66+
///
6367
/// # Returns
6468
///
65-
/// A [`CacheState`] object containing the pre-state accounts and storages.
69+
/// A [`CacheState`] object containing the pre-state accounts, storages and contracts.
6670
pub fn state(&self) -> CacheState {
6771
let mut cache_state = CacheState::new();
6872
for (address, info) in &self.pre {
6973
let code_hash = keccak256(&info.code);
70-
let bytecode = Bytecode::new_raw_checked(info.code.clone())
71-
.unwrap_or(Bytecode::new_legacy(info.code.clone()));
74+
if !info.code.is_empty() {
75+
let bytecode = Bytecode::new_raw_checked(info.code.clone())
76+
.unwrap_or(Bytecode::new_legacy(info.code.clone()));
77+
cache_state.contracts.insert(code_hash, bytecode);
78+
}
7279
let acc_info = state::AccountInfo {
7380
balance: info.balance,
7481
code_hash,
75-
code: Some(bytecode),
82+
code: None,
7683
nonce: info.nonce,
7784
..Default::default()
7885
};

0 commit comments

Comments
 (0)