From cf8dc20fe2839f8d39d2fef497128d6f2449f55b Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 26 Jul 2024 18:16:51 +0300 Subject: [PATCH 1/2] Fix refund computation --- core/lib/multivm/src/versions/vm_fast/vm.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/lib/multivm/src/versions/vm_fast/vm.rs b/core/lib/multivm/src/versions/vm_fast/vm.rs index dd599d471b3b..de8efcecc115 100644 --- a/core/lib/multivm/src/versions/vm_fast/vm.rs +++ b/core/lib/multivm/src/versions/vm_fast/vm.rs @@ -153,7 +153,7 @@ impl Vm { gas_spent_on_pubdata.as_u64(), tx_gas_limit, gas_per_pubdata_byte.low_u32(), - pubdata_published - pubdata_before, + pubdata_published.saturating_sub(pubdata_before), self.bootloader_state .last_l2_block() .txs From a018fa8250a777c25b2ae42917ec4960259399e0 Mon Sep 17 00:00:00 2001 From: Alex Ostrovski Date: Fri, 26 Jul 2024 18:17:03 +0300 Subject: [PATCH 2/2] Test refund computation --- .../src/versions/vm_fast/tests/refunds.rs | 64 ++++++++++++++++++- .../src/versions/vm_fast/tests/utils.rs | 6 ++ .../src/versions/vm_latest/tests/refunds.rs | 64 ++++++++++++++++++- .../src/versions/vm_latest/tests/utils.rs | 6 ++ .../contracts/expensive/expensive.sol | 6 ++ 5 files changed, 144 insertions(+), 2 deletions(-) diff --git a/core/lib/multivm/src/versions/vm_fast/tests/refunds.rs b/core/lib/multivm/src/versions/vm_fast/tests/refunds.rs index 98085dd67ce0..21a3129a3a61 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/refunds.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/refunds.rs @@ -1,8 +1,11 @@ +use ethabi::Token; +use zksync_types::{Address, Execute, U256}; + use crate::{ interface::{TxExecutionMode, VmExecutionMode, VmInterface}, vm_fast::tests::{ tester::{DeployContractsTx, TxType, VmTesterBuilder}, - utils::read_test_contract, + utils::{read_expensive_contract, read_test_contract}, }, }; @@ -157,3 +160,62 @@ fn test_predetermined_refunded_gas() { current_state_without_predefined_refunds.used_contract_hashes ); } + +#[test] +fn negative_pubdata_for_transaction() { + let expensive_contract_address = Address::random(); + let (expensive_contract_bytecode, expensive_contract) = read_expensive_contract(); + let expensive_function = expensive_contract.function("expensive").unwrap(); + let cleanup_function = expensive_contract.function("cleanUp").unwrap(); + + let mut vm = VmTesterBuilder::new() + .with_empty_in_memory_storage() + .with_execution_mode(TxExecutionMode::VerifyExecute) + .with_random_rich_accounts(1) + .with_custom_contracts(vec![( + expensive_contract_bytecode, + expensive_contract_address, + false, + )]) + .build(); + + let expensive_tx = vm.rich_accounts[0].get_l2_tx_for_execute( + Execute { + contract_address: expensive_contract_address, + calldata: expensive_function + .encode_input(&[Token::Uint(10.into())]) + .unwrap(), + value: U256::zero(), + factory_deps: vec![], + }, + None, + ); + vm.vm.push_transaction(expensive_tx); + let result = vm.vm.execute(VmExecutionMode::OneTx); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); + + // This transaction cleans all initial writes in the contract, thus having negative `pubdata` impact. + let clean_up_tx = vm.rich_accounts[0].get_l2_tx_for_execute( + Execute { + contract_address: expensive_contract_address, + calldata: cleanup_function.encode_input(&[]).unwrap(), + value: U256::zero(), + factory_deps: vec![], + }, + None, + ); + vm.vm.push_transaction(clean_up_tx); + let result = vm.vm.execute(VmExecutionMode::OneTx); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); + assert!(result.refunds.operator_suggested_refund > 0); + assert_eq!( + result.refunds.gas_refunded, + result.refunds.operator_suggested_refund + ); +} diff --git a/core/lib/multivm/src/versions/vm_fast/tests/utils.rs b/core/lib/multivm/src/versions/vm_fast/tests/utils.rs index f5217a0e80ac..2cedafd0e68d 100644 --- a/core/lib/multivm/src/versions/vm_fast/tests/utils.rs +++ b/core/lib/multivm/src/versions/vm_fast/tests/utils.rs @@ -87,3 +87,9 @@ pub(crate) fn load_precompiles_contract() -> Contract { "etc/contracts-test-data/artifacts-zk/contracts/precompiles/precompiles.sol/Precompiles.json", ) } + +pub(crate) fn read_expensive_contract() -> (Vec, Contract) { + const PATH: &str = + "etc/contracts-test-data/artifacts-zk/contracts/expensive/expensive.sol/Expensive.json"; + (read_bytecode(PATH), load_contract(PATH)) +} diff --git a/core/lib/multivm/src/versions/vm_latest/tests/refunds.rs b/core/lib/multivm/src/versions/vm_latest/tests/refunds.rs index 72d2271f7158..52dbd6efb339 100644 --- a/core/lib/multivm/src/versions/vm_latest/tests/refunds.rs +++ b/core/lib/multivm/src/versions/vm_latest/tests/refunds.rs @@ -1,9 +1,12 @@ +use ethabi::Token; +use zksync_types::{Address, Execute, U256}; + use crate::{ interface::{TxExecutionMode, VmExecutionMode, VmInterface}, vm_latest::{ tests::{ tester::{DeployContractsTx, TxType, VmTesterBuilder}, - utils::read_test_contract, + utils::{read_expensive_contract, read_test_contract}, }, types::internals::TransactionData, HistoryEnabled, @@ -164,3 +167,62 @@ fn test_predetermined_refunded_gas() { current_state_without_predefined_refunds.used_contract_hashes ); } + +#[test] +fn negative_pubdata_for_transaction() { + let expensive_contract_address = Address::random(); + let (expensive_contract_bytecode, expensive_contract) = read_expensive_contract(); + let expensive_function = expensive_contract.function("expensive").unwrap(); + let cleanup_function = expensive_contract.function("cleanUp").unwrap(); + + let mut vm = VmTesterBuilder::new(HistoryEnabled) + .with_empty_in_memory_storage() + .with_execution_mode(TxExecutionMode::VerifyExecute) + .with_random_rich_accounts(1) + .with_custom_contracts(vec![( + expensive_contract_bytecode, + expensive_contract_address, + false, + )]) + .build(); + + let expensive_tx = vm.rich_accounts[0].get_l2_tx_for_execute( + Execute { + contract_address: expensive_contract_address, + calldata: expensive_function + .encode_input(&[Token::Uint(10.into())]) + .unwrap(), + value: U256::zero(), + factory_deps: vec![], + }, + None, + ); + vm.vm.push_transaction(expensive_tx); + let result = vm.vm.execute(VmExecutionMode::OneTx); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); + + // This transaction cleans all initial writes in the contract, thus having negative `pubdata` impact. + let clean_up_tx = vm.rich_accounts[0].get_l2_tx_for_execute( + Execute { + contract_address: expensive_contract_address, + calldata: cleanup_function.encode_input(&[]).unwrap(), + value: U256::zero(), + factory_deps: vec![], + }, + None, + ); + vm.vm.push_transaction(clean_up_tx); + let result = vm.vm.execute(VmExecutionMode::OneTx); + assert!( + !result.result.is_failed(), + "Transaction wasn't successful: {result:#?}" + ); + assert!(result.refunds.operator_suggested_refund > 0); + assert_eq!( + result.refunds.gas_refunded, + result.refunds.operator_suggested_refund + ); +} diff --git a/core/lib/multivm/src/versions/vm_latest/tests/utils.rs b/core/lib/multivm/src/versions/vm_latest/tests/utils.rs index 37bdd0cef8e0..2482df0d0e89 100644 --- a/core/lib/multivm/src/versions/vm_latest/tests/utils.rs +++ b/core/lib/multivm/src/versions/vm_latest/tests/utils.rs @@ -131,3 +131,9 @@ pub(crate) fn get_complex_upgrade_abi() -> Contract { "etc/contracts-test-data/artifacts-zk/contracts/complex-upgrade/complex-upgrade.sol/ComplexUpgrade.json" ) } + +pub(crate) fn read_expensive_contract() -> (Vec, Contract) { + const PATH: &str = + "etc/contracts-test-data/artifacts-zk/contracts/expensive/expensive.sol/Expensive.json"; + (read_bytecode(PATH), load_contract(PATH)) +} diff --git a/etc/contracts-test-data/contracts/expensive/expensive.sol b/etc/contracts-test-data/contracts/expensive/expensive.sol index c3b99df48923..27e18b6eb6cf 100644 --- a/etc/contracts-test-data/contracts/expensive/expensive.sol +++ b/etc/contracts-test-data/contracts/expensive/expensive.sol @@ -12,4 +12,10 @@ contract Expensive { } return keccak256(abi.encodePacked(array)); } + + function cleanUp() public { + for (uint i = 0; i < array.length; i++) { + array[i] = 0; + } + } }