Skip to content
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

fix: Fill storage_refunds / pubdata_costs data #2431

Merged
merged 17 commits into from
Jul 26, 2024
Merged
1 change: 1 addition & 0 deletions core/lib/multivm/src/interface/types/outputs/l2_block.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
use zksync_types::H256;

#[derive(Debug)]
pub struct L2Block {
pub number: u32,
pub timestamp: u64,
Expand Down
20 changes: 19 additions & 1 deletion core/lib/multivm/src/versions/shadow.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
use std::{collections::BTreeMap, fmt};
use std::{
collections::{BTreeMap, HashSet},
fmt,
};

use anyhow::Context as _;
use zksync_state::{ImmutableStorageView, ReadStorage, StoragePtr, StorageView};
Expand Down Expand Up @@ -276,6 +279,21 @@ impl DivergenceErrors {
&main.system_logs,
&shadow.system_logs,
);
self.check_match(
"final_state.storage_refunds",
&main.storage_refunds,
&shadow.storage_refunds,
);
self.check_match(
"final_state.pubdata_costs",
&main.pubdata_costs,
&shadow.pubdata_costs,
);
self.check_match(
"final_state.used_contract_hashes",
&main.used_contract_hashes.iter().collect::<HashSet<_>>(),
&shadow.used_contract_hashes.iter().collect::<HashSet<_>>(),
);

let main_deduplicated_logs = Self::gather_logs(&main.deduplicated_storage_logs);
let shadow_deduplicated_logs = Self::gather_logs(&shadow.deduplicated_storage_logs);
Expand Down
93 changes: 91 additions & 2 deletions core/lib/multivm/src/versions/vm_fast/tests/code_oracle.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
use ethabi::Token;
use zksync_types::{get_known_code_key, web3::keccak256, Address, Execute, U256};
use zksync_utils::{bytecode::hash_bytecode, u256_to_h256};
use zksync_types::{
get_known_code_key, web3::keccak256, Address, Execute, StorageLogWithPreviousValue, H256, U256,
};
use zksync_utils::{bytecode::hash_bytecode, h256_to_u256, u256_to_h256};

use crate::{
interface::{TxExecutionMode, VmExecutionMode, VmInterface},
Expand Down Expand Up @@ -91,6 +93,16 @@ fn test_code_oracle() {
assert!(!result.result.is_failed(), "Transaction wasn't successful");
}

fn find_code_oracle_cost_log(
precompiles_contract_address: Address,
logs: &[StorageLogWithPreviousValue],
) -> &StorageLogWithPreviousValue {
let log = logs.iter().find(|log| {
*log.log.key.address() == precompiles_contract_address && *log.log.key.key() == H256::zero()
slowli marked this conversation as resolved.
Show resolved Hide resolved
});
log.expect("no code oracle cost log")
}

#[test]
fn test_code_oracle_big_bytecode() {
let precompiles_contract_address = Address::random();
Expand Down Expand Up @@ -147,3 +159,80 @@ fn test_code_oracle_big_bytecode() {
let result = vm.vm.execute(VmExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");
}

#[test]
fn refunds_in_code_oracle() {
let precompiles_contract_address = Address::random();
let precompile_contract_bytecode = read_precompiles_contract();

let normal_zkevm_bytecode = read_test_contract();
let normal_zkevm_bytecode_hash = hash_bytecode(&normal_zkevm_bytecode);
let normal_zkevm_bytecode_keccak_hash = keccak256(&normal_zkevm_bytecode);
let mut storage = get_empty_storage();
storage.set_value(
get_known_code_key(&normal_zkevm_bytecode_hash),
u256_to_h256(U256::one()),
);

let precompile_contract = load_precompiles_contract();
let call_code_oracle_function = precompile_contract.function("callCodeOracle").unwrap();

// Execute code oracle twice with identical VM state that only differs in that the queried bytecode
// is already decommitted the second time. The second call must consume less gas (`decommit` doesn't charge additional gas
// for already decommitted codes).
let mut oracle_costs = vec![];
for decommit in [false, true] {
let mut vm = VmTesterBuilder::new()
.with_execution_mode(TxExecutionMode::VerifyExecute)
.with_random_rich_accounts(1)
.with_custom_contracts(vec![(
precompile_contract_bytecode.clone(),
precompiles_contract_address,
false,
)])
.with_storage(storage.clone())
.build();

vm.vm.insert_bytecodes([normal_zkevm_bytecode.as_slice()]);

let account = &mut vm.rich_accounts[0];
if decommit {
let (_, is_fresh) = vm
.vm
.inner
.world_diff
.decommit_opcode(&mut vm.vm.world, h256_to_u256(normal_zkevm_bytecode_hash));
assert!(is_fresh);
}

let tx = account.get_l2_tx_for_execute(
Execute {
contract_address: precompiles_contract_address,
calldata: call_code_oracle_function
.encode_input(&[
Token::FixedBytes(normal_zkevm_bytecode_hash.0.to_vec()),
Token::FixedBytes(normal_zkevm_bytecode_keccak_hash.to_vec()),
])
.unwrap(),
value: U256::zero(),
factory_deps: vec![],
},
None,
);

vm.vm.push_transaction(tx);
let result = vm.vm.execute(VmExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");
slowli marked this conversation as resolved.
Show resolved Hide resolved
let log =
find_code_oracle_cost_log(precompiles_contract_address, &result.logs.storage_logs);
oracle_costs.push(log.log.value);
}

// The refund is equal to `gasCost` parameter passed to the `decommit` opcode, which is defined as `4 * contract_length_in_words`
// in `CodeOracle.yul`.
let code_oracle_refund = h256_to_u256(oracle_costs[0]) - h256_to_u256(oracle_costs[1]);
assert_eq!(
code_oracle_refund,
(4 * (normal_zkevm_bytecode.len() / 32)).into()
);
}
21 changes: 12 additions & 9 deletions core/lib/multivm/src/versions/vm_fast/tests/default_aa.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,13 +8,11 @@ use zksync_utils::u256_to_h256;

use crate::{
interface::{TxExecutionMode, VmExecutionMode, VmInterface},
vm_fast::{
tests::{
tester::{DeployContractsTx, TxType, VmTesterBuilder},
utils::{get_balance, read_test_contract, verify_required_storage},
},
utils::fee::get_batch_base_fee,
vm_fast::tests::{
tester::{DeployContractsTx, TxType, VmTesterBuilder},
utils::{get_balance, read_test_contract, verify_required_storage},
},
vm_latest::utils::fee::get_batch_base_fee,
};

#[test]
Expand Down Expand Up @@ -54,21 +52,26 @@ fn test_default_aa_interaction() {
// The contract should be deployed successfully.
let account_code_key = get_code_key(&address);

let expected_slots = vec![
let expected_slots = [
(u256_to_h256(expected_nonce), account_nonce_key),
(u256_to_h256(U256::from(1u32)), known_codes_key),
(bytecode_hash, account_code_key),
];

verify_required_storage(&vm.vm.state, expected_slots);
verify_required_storage(
&expected_slots,
&mut vm.vm.world.storage,
vm.vm.inner.world_diff.get_storage_state(),
);

let expected_fee = maximal_fee
- U256::from(result.refunds.gas_refunded)
* U256::from(get_batch_base_fee(&vm.vm.batch_env));
let operator_balance = get_balance(
AccountTreeId::new(L2_BASE_TOKEN_ADDRESS),
&vm.fee_account,
vm.vm.state.storage.storage.get_ptr(),
&mut vm.vm.world.storage,
vm.vm.inner.world_diff.get_storage_state(),
);

assert_eq!(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,12 +38,12 @@ fn test_get_used_contracts() {

assert!(vm
.vm
.get_decommitted_hashes()
.decommitted_hashes()
.contains(&h256_to_u256(tx.bytecode_hash)));

// Note: `Default_AA` will be in the list of used contracts if L2 tx is used
assert_eq!(
vm.vm.get_decommitted_hashes().collect::<HashSet<U256>>(),
vm.vm.decommitted_hashes().collect::<HashSet<U256>>(),
known_bytecodes_without_aa_code(&vm.vm)
);

Expand Down Expand Up @@ -78,7 +78,7 @@ fn test_get_used_contracts() {
let hash = hash_bytecode(&factory_dep);
let hash_to_u256 = h256_to_u256(hash);
assert!(known_bytecodes_without_aa_code(&vm.vm).contains(&hash_to_u256));
assert!(!vm.vm.get_decommitted_hashes().contains(&hash_to_u256));
assert!(!vm.vm.decommitted_hashes().contains(&hash_to_u256));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use crate::{
},
};

#[ignore] // FIXME: fails on `assert_eq!(res.initial_storage_writes, basic_initial_writes)`
#[test]
fn test_l1_tx_execution() {
// In this test, we try to execute a contract deployment from L1
Expand Down
18 changes: 8 additions & 10 deletions core/lib/multivm/src/versions/vm_fast/tests/mod.rs
Original file line number Diff line number Diff line change
@@ -1,20 +1,18 @@
mod bootloader;
//mod default_aa;
// TODO - fix this test
// `mod invalid_bytecode;`
//mod block_tip;
mod default_aa;
//mod block_tip; FIXME: requires vm metrics
mod bytecode_publishing;
// mod call_tracer;
// mod circuits;
// mod call_tracer; FIXME: requires tracers
// mod circuits; FIXME: requires tracers / circuit stats
mod code_oracle;
mod gas_limit;
mod get_used_contracts;
mod is_write_initial;
mod l1_tx_execution;
mod l2_blocks;
//mod nonce_holder;
// mod precompiles;
// mod prestate_tracer;
mod nonce_holder;
// mod precompiles; FIXME: requires tracers / circuit stats
// mod prestate_tracer; FIXME: is pre-state tracer still relevant?
mod refunds;
mod require_eip712;
mod rollbacks;
Expand All @@ -24,5 +22,5 @@ mod storage;
mod tester;
mod tracing_execution_error;
mod transfer;
// mod upgrade;
mod upgrade;
mod utils;
50 changes: 21 additions & 29 deletions core/lib/multivm/src/versions/vm_fast/tests/nonce_holder.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,13 @@
use zksync_types::{Execute, Nonce};
use zksync_types::{Execute, ExecuteTransactionCommon, Nonce};

use crate::{
interface::{
ExecutionResult, Halt, TxExecutionMode, TxRevertReason, VmExecutionMode, VmInterface,
VmRevertReason,
},
vm_fast::{
tests::{
tester::{Account, VmTesterBuilder},
utils::read_nonce_holder_tester,
},
transaction_data::TransactionData,
vm_fast::tests::{
tester::{Account, VmTesterBuilder},
utils::read_nonce_holder_tester,
},
};

Expand Down Expand Up @@ -60,21 +57,21 @@ fn test_nonce_holder() {
// it will fail again and again. At the same time we have to keep the same storage, because we want to keep the nonce holder contract state.
// The easiest way in terms of lifetimes is to reuse `vm_builder` to achieve it.
vm.reset_state(true);
let mut transaction_data: TransactionData = account
.get_l2_tx_for_execute_with_nonce(
Execute {
contract_address: account.address,
calldata: vec![12],
value: Default::default(),
factory_deps: None,
},
None,
Nonce(nonce),
)
.into();

transaction_data.signature = vec![test_mode.into()];
vm.vm.push_raw_transaction(transaction_data, 0, 0, true);
let mut transaction = account.get_l2_tx_for_execute_with_nonce(
Execute {
contract_address: account.address,
calldata: vec![12],
value: Default::default(),
factory_deps: vec![],
},
None,
Nonce(nonce),
);
let ExecuteTransactionCommon::L2(tx_data) = &mut transaction.common_data else {
unreachable!();
};
tx_data.signature = vec![test_mode.into()];
vm.vm.push_transaction_inner(transaction, 0, true);
let result = vm.vm.execute(VmExecutionMode::OneTx);

if let Some(msg) = error_message {
Expand All @@ -86,14 +83,9 @@ fn test_nonce_holder() {
let ExecutionResult::Halt { reason } = result.result else {
panic!("Expected revert, got {:?}", result.result);
};
assert_eq!(
reason.to_string(),
expected_error.to_string(),
"{}",
comment
);
assert_eq!(reason.to_string(), expected_error.to_string(), "{comment}");
} else {
assert!(!result.result.is_failed(), "{}", comment);
assert!(!result.result.is_failed(), "{comment}: {result:?}");
}
};
// Test 1: trying to set value under non sequential nonce value.
Expand Down
14 changes: 10 additions & 4 deletions core/lib/multivm/src/versions/vm_fast/tests/require_eip712.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,15 +23,20 @@ impl VmTester {
AccountTreeId::new(L2_BASE_TOKEN_ADDRESS),
&address,
);
h256_to_u256(self.vm.world.storage.read_value(&key))
self.vm
.inner
.world_diff
.get_storage_state()
.get(&(L2_BASE_TOKEN_ADDRESS, h256_to_u256(*key.key())))
.copied()
.unwrap_or_else(|| h256_to_u256(self.vm.world.storage.read_value(&key)))
}
}

#[ignore] // FIXME: fails on `assert_eq!(vm.get_eth_balance(beneficiary.address), U256::from(888000088))`
#[tokio::test]
/// This test deploys 'buggy' account abstraction code, and then tries accessing it both with legacy
/// and EIP712 transactions.
/// Currently we support both, but in the future, we should allow only EIP712 transactions to access the AA accounts.
#[tokio::test]
async fn test_require_eip712() {
// Use 3 accounts:
// - `private_address` - EOA account, where we have the key
Expand Down Expand Up @@ -134,7 +139,8 @@ async fn test_require_eip712() {
Default::default(),
);

let transaction_request: TransactionRequest = tx_712.into();
let mut transaction_request: TransactionRequest = tx_712.into();
transaction_request.chain_id = Some(chain_id.into());

let domain = Eip712Domain::new(L2ChainId::from(chain_id));
let signature = private_account
Expand Down
Loading
Loading