Skip to content

Commit

Permalink
Fix issues from code reviews
Browse files Browse the repository at this point in the history
  • Loading branch information
slowli committed Jul 26, 2024
1 parent b384c25 commit bb2c663
Show file tree
Hide file tree
Showing 3 changed files with 48 additions and 26 deletions.
31 changes: 22 additions & 9 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,6 @@
use ethabi::Token;
use zksync_types::{
get_known_code_key, web3::keccak256, Address, Execute, StorageLogWithPreviousValue, H256, U256,
get_known_code_key, web3::keccak256, Address, Execute, StorageLogWithPreviousValue, U256,
};
use zksync_utils::{bytecode::hash_bytecode, h256_to_u256, u256_to_h256};

Expand Down Expand Up @@ -70,7 +70,10 @@ fn test_code_oracle() {

vm.vm.push_transaction(tx1);
let result = vm.vm.execute(VmExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");
assert!(
!result.result.is_failed(),
"Transaction wasn't successful: {result:#?}"
);

// Now, we ask for the same bytecode. We use to partially check whether the memory page with
// the decommitted bytecode gets erased (it shouldn't).
Expand All @@ -90,17 +93,21 @@ fn test_code_oracle() {
);
vm.vm.push_transaction(tx2);
let result = vm.vm.execute(VmExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");
assert!(
!result.result.is_failed(),
"Transaction wasn't successful: {result:#?}"
);
}

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()
});
log.expect("no code oracle cost log")
logs.iter()
.find(|log| {
*log.log.key.address() == precompiles_contract_address && log.log.key.key().is_zero()
})
.expect("no code oracle cost log")
}

#[test]
Expand Down Expand Up @@ -157,7 +164,10 @@ fn test_code_oracle_big_bytecode() {

vm.vm.push_transaction(tx1);
let result = vm.vm.execute(VmExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");
assert!(
!result.result.is_failed(),
"Transaction wasn't successful: {result:#?}"
);
}

#[test]
Expand Down Expand Up @@ -222,7 +232,10 @@ fn refunds_in_code_oracle() {

vm.vm.push_transaction(tx);
let result = vm.vm.execute(VmExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");
assert!(
!result.result.is_failed(),
"Transaction wasn't successful: {result:#?}"
);
let log =
find_code_oracle_cost_log(precompiles_contract_address, &result.logs.storage_logs);
oracle_costs.push(log.log.value);
Expand Down
22 changes: 12 additions & 10 deletions core/lib/multivm/src/versions/vm_fast/vm.rs
Original file line number Diff line number Diff line change
Expand Up @@ -46,8 +46,9 @@ use crate::{
},
vm_latest::{
constants::{
get_vm_hook_params_start_position, get_vm_hook_position, OPERATOR_REFUNDS_OFFSET,
TX_GAS_LIMIT_OFFSET, VM_HOOK_PARAMS_COUNT,
get_used_bootloader_memory_bytes, get_vm_hook_params_start_position,
get_vm_hook_position, OPERATOR_REFUNDS_OFFSET, TX_GAS_LIMIT_OFFSET,
VM_HOOK_PARAMS_COUNT,
},
BootloaderMemory, CurrentExecutionState, ExecutionResult, FinishedL1Batch, L1BatchEnv,
L2BlockEnv, MultiVMSubversion, Refunds, SystemEnv, VmExecutionLogs, VmExecutionMode,
Expand Down Expand Up @@ -457,6 +458,7 @@ impl<S: ReadStorage> Vm<S> {
inner.state.current_frame.sp = 0;

// The bootloader writes results to high addresses in its heap, so it makes sense to preallocate it.
inner.state.heaps[vm2::FIRST_HEAP].reserve(get_used_bootloader_memory_bytes(VM_VERSION));
inner.state.current_frame.heap_size = u32::MAX;
inner.state.current_frame.aux_heap_size = u32::MAX;
inner.state.current_frame.exception_handler = INITIAL_FRAME_FORMAL_EH_LOCATION;
Expand Down Expand Up @@ -555,20 +557,20 @@ impl<S: ReadStorage> VmInterface for Vm<S> {
}
};

// FIXME: are statistics rolled back on halt as well?
let pubdata_after = self.inner.world_diff.pubdata();
VmExecutionResultAndLogs {
result,
logs,
// FIXME (PLA-936): Fill statistics; investigate whether they should be zeroed on `Halt`
statistics: VmExecutionStatistics {
contracts_used: 0, // TODO
cycles_used: 0, // TODO
gas_used: 0, // TODO
gas_remaining: 0, // TODO
computational_gas_used: 0, // TODO
total_log_queries: 0, // TODO
contracts_used: 0,
cycles_used: 0,
gas_used: 0,
gas_remaining: 0,
computational_gas_used: 0,
total_log_queries: 0,
pubdata_published: (pubdata_after - pubdata_before).max(0) as u32,
circuit_statistic: Default::default(), // TODO
circuit_statistic: Default::default(),
},
refunds,
}
Expand Down
21 changes: 14 additions & 7 deletions core/lib/multivm/src/versions/vm_latest/tests/code_oracle.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ use zk_evm_1_5_0::{
zkevm_opcode_defs::{ContractCodeSha256Format, VersionedHashLen32},
};
use zksync_types::{
get_known_code_key, web3::keccak256, Address, Execute, StorageLogWithPreviousValue, H256, U256,
get_known_code_key, web3::keccak256, Address, Execute, StorageLogWithPreviousValue, U256,
};
use zksync_utils::{bytecode::hash_bytecode, bytes_to_be_words, h256_to_u256, u256_to_h256};

Expand Down Expand Up @@ -84,7 +84,10 @@ fn test_code_oracle() {

vm.vm.push_transaction(tx1);
let result = vm.vm.execute(VmExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");
assert!(
!result.result.is_failed(),
"Transaction wasn't successful: {result:#?}"
);

// Now, we ask for the same bytecode. We use to partially check whether the memory page with
// the decommitted bytecode gets erased (it shouldn't).
Expand All @@ -104,17 +107,21 @@ fn test_code_oracle() {
);
vm.vm.push_transaction(tx2);
let result = vm.vm.execute(VmExecutionMode::OneTx);
assert!(!result.result.is_failed(), "Transaction wasn't successful");
assert!(
!result.result.is_failed(),
"Transaction wasn't successful: {result:#?}"
);
}

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()
});
log.expect("no code oracle cost log")
logs.iter()
.find(|log| {
*log.log.key.address() == precompiles_contract_address && log.log.key.key().is_zero()
})
.expect("no code oracle cost log")
}

#[test]
Expand Down

0 comments on commit bb2c663

Please sign in to comment.