From d337a6071e3f3975474cd8184eb4052db9b55e25 Mon Sep 17 00:00:00 2001 From: Niven Date: Mon, 2 Oct 2023 11:10:37 +0800 Subject: [PATCH 01/16] Fix tx receipt for system txs --- lib/ain-evm/src/core.rs | 2 +- lib/ain-evm/src/executor.rs | 21 ++++++++++++++------- 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/lib/ain-evm/src/core.rs b/lib/ain-evm/src/core.rs index e0ca6f8fcf..a7364a3f99 100644 --- a/lib/ain-evm/src/core.rs +++ b/lib/ain-evm/src/core.rs @@ -466,7 +466,7 @@ impl EVMCoreService { // Execute tx let mut executor = AinExecutor::new(&mut backend); let (tx_response, ..) = - executor.exec(&signed_tx, signed_tx.gas_limit(), prepay_fee, block_fee)?; + executor.exec(&signed_tx, signed_tx.gas_limit(), prepay_fee, block_fee, false)?; // Validate total gas usage in queued txs exceeds block size debug!("[validate_raw_tx] used_gas: {:#?}", tx_response.used_gas); diff --git a/lib/ain-evm/src/executor.rs b/lib/ain-evm/src/executor.rs index 29032aa1c8..e29dc74fc4 100644 --- a/lib/ain-evm/src/executor.rs +++ b/lib/ain-evm/src/executor.rs @@ -131,6 +131,7 @@ impl<'backend> AinExecutor<'backend> { gas_limit: U256, prepay_gas: U256, base_fee: U256, + system_tx: bool, ) -> Result<(TxResponse, ReceiptV3)> { self.backend.update_vicinity_from_tx(signed_tx, base_fee); trace!( @@ -146,8 +147,10 @@ impl<'backend> AinExecutor<'backend> { access_list: signed_tx.access_list(), }; - self.backend - .deduct_prepay_gas(signed_tx.sender, prepay_gas)?; + if prepay_gas != U256::zero() && !system_tx { + self.backend + .deduct_prepay_gas(signed_tx.sender, prepay_gas)?; + } let metadata = StackSubstateMetadata::new(ctx.gas_limit, &Self::CONFIG); let state = MemoryStackState::new(metadata, self.backend); @@ -177,14 +180,18 @@ impl<'backend> AinExecutor<'backend> { ), }; - let used_gas = executor.used_gas(); + let used_gas = if system_tx { + 0u64 + } else { + executor.used_gas() + }; let (values, logs) = executor.into_state().deconstruct(); let logs = logs.into_iter().collect::>(); ApplyBackend::apply(self.backend, values, logs.clone(), true); self.backend.commit(); - if prepay_gas != U256::zero() { + if prepay_gas != U256::zero() && !system_tx { self.backend .refund_unused_gas(signed_tx, U256::from(used_gas), base_fee)?; } @@ -227,7 +234,7 @@ impl<'backend> AinExecutor<'backend> { let prepay_gas = calculate_prepay_gas_fee(&signed_tx, base_fee)?; let (tx_response, receipt) = - self.exec(&signed_tx, signed_tx.gas_limit(), prepay_gas, base_fee)?; + self.exec(&signed_tx, signed_tx.gas_limit(), prepay_gas, base_fee, false)?; debug!( "[apply_queue_tx]receipt : {:?}, exit_reason {:#?} for signed_tx : {:#x}", receipt, @@ -292,7 +299,7 @@ impl<'backend> AinExecutor<'backend> { } let (tx_response, receipt) = - self.exec(&signed_tx, U256::MAX, U256::zero(), U256::zero())?; + self.exec(&signed_tx, U256::MAX, U256::zero(), U256::zero(), true)?; if !tx_response.exit_reason.is_succeed() { return Err(format_err!( "[apply_queue_tx] Transfer domain failed VM execution {:?}", @@ -349,7 +356,7 @@ impl<'backend> AinExecutor<'backend> { self.commit(); let (tx_response, receipt) = - self.exec(&signed_tx, U256::MAX, U256::zero(), U256::zero())?; + self.exec(&signed_tx, U256::MAX, U256::zero(), U256::zero(), true)?; if !tx_response.exit_reason.is_succeed() { debug!( "[apply_queue_tx] DST20 bridge failed VM execution {:?}, data {}", From edda1c2b43df8a43ff0e52f0db3c4d1925ba61cc Mon Sep 17 00:00:00 2001 From: Niven Date: Mon, 2 Oct 2023 11:14:02 +0800 Subject: [PATCH 02/16] Fix rust lint --- lib/ain-evm/src/core.rs | 9 +++++++-- lib/ain-evm/src/executor.rs | 15 ++++++++------- 2 files changed, 15 insertions(+), 9 deletions(-) diff --git a/lib/ain-evm/src/core.rs b/lib/ain-evm/src/core.rs index a7364a3f99..71f18c6549 100644 --- a/lib/ain-evm/src/core.rs +++ b/lib/ain-evm/src/core.rs @@ -465,8 +465,13 @@ impl EVMCoreService { // Execute tx let mut executor = AinExecutor::new(&mut backend); - let (tx_response, ..) = - executor.exec(&signed_tx, signed_tx.gas_limit(), prepay_fee, block_fee, false)?; + let (tx_response, ..) = executor.exec( + &signed_tx, + signed_tx.gas_limit(), + prepay_fee, + block_fee, + false, + )?; // Validate total gas usage in queued txs exceeds block size debug!("[validate_raw_tx] used_gas: {:#?}", tx_response.used_gas); diff --git a/lib/ain-evm/src/executor.rs b/lib/ain-evm/src/executor.rs index e29dc74fc4..7dcb348c74 100644 --- a/lib/ain-evm/src/executor.rs +++ b/lib/ain-evm/src/executor.rs @@ -180,11 +180,7 @@ impl<'backend> AinExecutor<'backend> { ), }; - let used_gas = if system_tx { - 0u64 - } else { - executor.used_gas() - }; + let used_gas = if system_tx { 0u64 } else { executor.used_gas() }; let (values, logs) = executor.into_state().deconstruct(); let logs = logs.into_iter().collect::>(); @@ -233,8 +229,13 @@ impl<'backend> AinExecutor<'backend> { } let prepay_gas = calculate_prepay_gas_fee(&signed_tx, base_fee)?; - let (tx_response, receipt) = - self.exec(&signed_tx, signed_tx.gas_limit(), prepay_gas, base_fee, false)?; + let (tx_response, receipt) = self.exec( + &signed_tx, + signed_tx.gas_limit(), + prepay_gas, + base_fee, + false, + )?; debug!( "[apply_queue_tx]receipt : {:?}, exit_reason {:#?} for signed_tx : {:#x}", receipt, From f35fa1a2a23ba0bbbd8208133f7a87d72fc8e839 Mon Sep 17 00:00:00 2001 From: Niven Date: Mon, 2 Oct 2023 17:25:10 +0800 Subject: [PATCH 03/16] Housekeeping --- lib/ain-evm/src/backend.rs | 10 +++++----- lib/ain-evm/src/executor.rs | 14 +++++++------- 2 files changed, 12 insertions(+), 12 deletions(-) diff --git a/lib/ain-evm/src/backend.rs b/lib/ain-evm/src/backend.rs index c403af331d..62fbcc57e4 100644 --- a/lib/ain-evm/src/backend.rs +++ b/lib/ain-evm/src/backend.rs @@ -151,11 +151,11 @@ impl EVMBackend { self.state.ro_handle(root) } - pub fn deduct_prepay_gas(&mut self, sender: H160, prepay_gas: U256) -> Result<()> { - debug!(target: "backend", "[deduct_prepay_gas] Deducting {:#x} from {:#x}", prepay_gas, sender); + pub fn deduct_prepay_gas_fee(&mut self, sender: H160, prepay_fee: U256) -> Result<()> { + debug!(target: "backend", "[deduct_prepay_gas_fee] Deducting {:#x} from {:#x}", prepay_fee, sender); let basic = self.basic(sender); - let balance = basic.balance.checked_sub(prepay_gas).ok_or_else(|| { + let balance = basic.balance.checked_sub(prepay_fee).ok_or_else(|| { BackendError::DeductPrepayGasFailed(String::from( "failed checked sub prepay gas with account balance", )) @@ -169,7 +169,7 @@ impl EVMBackend { Ok(()) } - pub fn refund_unused_gas( + pub fn refund_unused_gas_fee( &mut self, signed_tx: &SignedTx, used_gas: U256, @@ -182,7 +182,7 @@ impl EVMBackend { })?; let refund_amount = calculate_gas_fee(signed_tx, refund_gas, base_fee)?; - debug!(target: "backend", "[refund_unused_gas] Refunding {:#x} to {:#x}", refund_amount, signed_tx.sender); + debug!(target: "backend", "[refund_unused_gas_fee] Refunding {:#x} to {:#x}", refund_amount, signed_tx.sender); let basic = self.basic(signed_tx.sender); let balance = basic.balance.checked_add(refund_amount).ok_or_else(|| { diff --git a/lib/ain-evm/src/executor.rs b/lib/ain-evm/src/executor.rs index 7dcb348c74..2fc7b5f2ac 100644 --- a/lib/ain-evm/src/executor.rs +++ b/lib/ain-evm/src/executor.rs @@ -129,7 +129,7 @@ impl<'backend> AinExecutor<'backend> { &mut self, signed_tx: &SignedTx, gas_limit: U256, - prepay_gas: U256, + prepay_fee: U256, base_fee: U256, system_tx: bool, ) -> Result<(TxResponse, ReceiptV3)> { @@ -147,9 +147,9 @@ impl<'backend> AinExecutor<'backend> { access_list: signed_tx.access_list(), }; - if prepay_gas != U256::zero() && !system_tx { + if prepay_fee != U256::zero() && !system_tx { self.backend - .deduct_prepay_gas(signed_tx.sender, prepay_gas)?; + .deduct_prepay_gas_fee(signed_tx.sender, prepay_fee)?; } let metadata = StackSubstateMetadata::new(ctx.gas_limit, &Self::CONFIG); @@ -187,9 +187,9 @@ impl<'backend> AinExecutor<'backend> { ApplyBackend::apply(self.backend, values, logs.clone(), true); self.backend.commit(); - if prepay_gas != U256::zero() && !system_tx { + if prepay_fee != U256::zero() && !system_tx { self.backend - .refund_unused_gas(signed_tx, U256::from(used_gas), base_fee)?; + .refund_unused_gas_fee(signed_tx, U256::from(used_gas), base_fee)?; } let receipt = ReceiptV3::EIP1559(EIP658ReceiptData { @@ -228,11 +228,11 @@ impl<'backend> AinExecutor<'backend> { .into()); } - let prepay_gas = calculate_prepay_gas_fee(&signed_tx, base_fee)?; + let prepay_fee = calculate_prepay_gas_fee(&signed_tx, base_fee)?; let (tx_response, receipt) = self.exec( &signed_tx, signed_tx.gas_limit(), - prepay_gas, + prepay_fee, base_fee, false, )?; From 6ffab928885b21f098631e15afba0720d696368a Mon Sep 17 00:00:00 2001 From: Niven Date: Mon, 2 Oct 2023 23:24:59 +0800 Subject: [PATCH 04/16] Add test for td tx receipt for zero gas used --- test/functional/feature_evm_logs.py | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/test/functional/feature_evm_logs.py b/test/functional/feature_evm_logs.py index d00857f8e5..800afa36a7 100755 --- a/test/functional/feature_evm_logs.py +++ b/test/functional/feature_evm_logs.py @@ -5,7 +5,10 @@ # file LICENSE or http://www.opensource.org/licenses/mit-license.php. """Test EVM contract""" -from test_framework.util import assert_equal +from test_framework.util import ( + assert_equal, + int_to_eth_u256, +) from test_framework.test_framework import DefiTestFramework from test_framework.evm_contract import EVMContract from test_framework.evm_key_pair import EvmKeyPair @@ -140,6 +143,24 @@ def should_contract_store_and_emit_logs(self): event_data = get_event_data(node.w3.codec, self.event_abi, logs[0]) assert_equal(event_data["event"], "NumberStored") + def transferdomain_receipt_gas_used(self): + dvmHash = self.nodes[0].transferdomain( + [ + { + "src": {"address": self.address, "amount": "50@DFI", "domain": 2}, + "dst": { + "address": self.evm_key_pair.address, + "amount": "50@DFI", + "domain": 3, + }, + } + ] + ) + self.nodes[0].generate(1) + evmHash = self.nodes[0].vmmap(dvmHash, 5)["output"] + receipt = self.nodes[0].eth_getTransactionReceipt(evmHash) + assert_equal(receipt["gasUsed"], int_to_eth_u256(0)) + def run_test(self): self.setup() @@ -147,6 +168,8 @@ def run_test(self): self.should_contract_store_and_emit_logs() + self.transferdomain_receipt_gas_used() + if __name__ == "__main__": EVMTestLogs().main() From 3493c9581a443dc0594aa215e9715db4b5477143 Mon Sep 17 00:00:00 2001 From: jouzo Date: Mon, 2 Oct 2023 18:52:17 +0100 Subject: [PATCH 05/16] Set correct timestamp in queue backend --- lib/ain-evm/src/core.rs | 13 ++++++++---- lib/ain-evm/src/evm.rs | 37 ++++++++++++++++++++++++++++++++--- lib/ain-evm/src/txqueue.rs | 33 ++++++++++++++++++++++++++----- lib/ain-rs-exports/src/evm.rs | 4 ++-- lib/ain-rs-exports/src/lib.rs | 2 +- src/masternodes/mn_checks.cpp | 2 +- src/miner.cpp | 2 +- src/validation.cpp | 37 ++++++++++++----------------------- src/validation.h | 2 +- 9 files changed, 90 insertions(+), 42 deletions(-) diff --git a/lib/ain-evm/src/core.rs b/lib/ain-evm/src/core.rs index e0ca6f8fcf..3a1be66919 100644 --- a/lib/ain-evm/src/core.rs +++ b/lib/ain-evm/src/core.rs @@ -11,8 +11,10 @@ use ain_contracts::{ FixedContract, }; use anyhow::format_err; -use ethereum::EnvelopedEncodable; -use ethereum::{AccessList, Account, Block, Log, PartialHeader, TransactionAction, TransactionV2}; +use ethereum::{ + AccessList, Account, Block, EnvelopedEncodable, Log, PartialHeader, TransactionAction, + TransactionV2, +}; use ethereum_types::{Bloom, BloomInput, H160, H256, U256}; use log::{debug, trace}; use lru::LruCache; @@ -805,12 +807,15 @@ impl EVMCoreService { /// Result cannot be used safety unless `cs_main` lock is taken on C++ side /// across all usages. Note: To be replaced with a proper lock flow later. /// - pub unsafe fn create_queue(&self) -> Result { + pub unsafe fn create_queue(&self, timestamp: u64) -> Result { let (target_block, initial_state_root) = match self.storage.get_latest_block()? { None => (U256::zero(), GENESIS_STATE_ROOT), // Genesis queue Some(block) => (block.header.number + 1, block.header.state_root), }; - let queue_id = self.tx_queues.create(target_block, initial_state_root); + + let queue_id = self + .tx_queues + .create(target_block, initial_state_root, timestamp); Ok(queue_id) } diff --git a/lib/ain-evm/src/evm.rs b/lib/ain-evm/src/evm.rs index 1bdc7d87a7..1cb66f5d6e 100644 --- a/lib/ain-evm/src/evm.rs +++ b/lib/ain-evm/src/evm.rs @@ -404,14 +404,35 @@ impl EVMServices { queue_id: u64, tx: QueueTx, ) -> Result<(SignedTx, U256, H256)> { - let state_root = self.core.tx_queues.get_latest_state_root_in(queue_id)?; + let (target_block, state_root, timestamp, is_first_tx) = { + let queue = self.core.tx_queues.get(queue_id)?; + + let state_root = queue.get_latest_state_root(); + let data = queue.data.lock().unwrap(); + + ( + data.target_block, + state_root, + data.timestamp, + data.transactions.is_empty(), + ) + }; debug!( "[update_queue_state_from_tx] state_root : {:#?}", state_root ); - // Has to be mutable to obtain new state root - let mut backend = self.core.get_backend(state_root)?; + let mut backend = EVMBackend::from_root( + state_root, + Arc::clone(&self.core.trie_store), + Arc::clone(&self.storage), + Vicinity { + timestamp: U256::from(timestamp), + block_number: target_block, + ..Vicinity::default() + }, + )?; + let mut executor = AinExecutor::new(&mut backend); let (parent_hash, _) = self @@ -420,6 +441,16 @@ impl EVMServices { .unwrap_or_default(); // Safe since calculate_base_fee will default to INITIAL_BASE_FEE let base_fee = self.block.calculate_base_fee(parent_hash)?; + // Update instrinsics contract to reproduce construct_block behaviour + if is_first_tx { + let (current_native_height, _) = ain_cpp_imports::get_sync_status().unwrap(); + let DeployContractInfo { + address, storage, .. + } = dfi_intrinsics_v1_deploy_info((current_native_height + 1) as u64, target_block)?; + executor.update_storage(address, storage)?; + executor.commit(); + } + let apply_tx = executor.apply_queue_tx(tx, base_fee)?; Ok(( diff --git a/lib/ain-evm/src/txqueue.rs b/lib/ain-evm/src/txqueue.rs index a8ac5756c5..bd561e82b6 100644 --- a/lib/ain-evm/src/txqueue.rs +++ b/lib/ain-evm/src/txqueue.rs @@ -44,7 +44,7 @@ impl TransactionQueueMap { /// Result cannot be used safety unless `cs_main` lock is taken on C++ side /// across all usages. Note: To be replaced with a proper lock flow later. /// - pub unsafe fn create(&self, target_block: U256, state_root: H256) -> u64 { + pub unsafe fn create(&self, target_block: U256, state_root: H256, timestamp: u64) -> u64 { let mut rng = rand::thread_rng(); loop { let queue_id = rng.gen(); @@ -55,7 +55,11 @@ impl TransactionQueueMap { let mut write_guard = self.queues.write().unwrap(); if let std::collections::hash_map::Entry::Vacant(e) = write_guard.entry(queue_id) { - e.insert(Arc::new(TransactionQueue::new(target_block, state_root))); + e.insert(Arc::new(TransactionQueue::new( + target_block, + state_root, + timestamp, + ))); return queue_id; } } @@ -176,6 +180,15 @@ impl TransactionQueueMap { self.with_transaction_queue(queue_id, TransactionQueue::get_target_block) } + /// # Safety + /// + /// Result cannot be used safety unless `cs_main` lock is taken on C++ side + /// across all usages. Note: To be replaced with a proper lock flow later. + /// + pub unsafe fn get_timestamp_in(&self, queue_id: u64) -> Result { + self.with_transaction_queue(queue_id, TransactionQueue::get_timestamp) + } + /// # Safety /// /// Result cannot be used safety unless `cs_main` lock is taken on C++ side @@ -235,16 +248,18 @@ pub struct TransactionQueueData { pub total_gas_used: U256, pub target_block: U256, pub initial_state_root: H256, + pub timestamp: u64, } impl TransactionQueueData { - pub fn new(target_block: U256, state_root: H256) -> Self { + pub fn new(target_block: U256, state_root: H256, timestamp: u64) -> Self { Self { transactions: Vec::new(), total_gas_used: U256::zero(), block_data: None, target_block, initial_state_root: state_root, + timestamp, } } } @@ -255,9 +270,13 @@ pub struct TransactionQueue { } impl TransactionQueue { - fn new(target_block: U256, state_root: H256) -> Self { + fn new(target_block: U256, state_root: H256, timestamp: u64) -> Self { Self { - data: Mutex::new(TransactionQueueData::new(target_block, state_root)), + data: Mutex::new(TransactionQueueData::new( + target_block, + state_root, + timestamp, + )), } } @@ -317,6 +336,10 @@ impl TransactionQueue { self.data.lock().unwrap().target_block } + pub fn get_timestamp(&self) -> u64 { + self.data.lock().unwrap().timestamp + } + pub fn get_state_root_from_native_hash(&self, hash: XHash) -> Option { self.data .lock() diff --git a/lib/ain-rs-exports/src/evm.rs b/lib/ain-rs-exports/src/evm.rs index cda25ef754..b88bfffc31 100644 --- a/lib/ain-rs-exports/src/evm.rs +++ b/lib/ain-rs-exports/src/evm.rs @@ -474,8 +474,8 @@ fn unsafe_validate_transferdomain_tx_in_q( /// /// Returns the EVM queue ID as a `u64`. #[ffi_fallible] -fn unsafe_create_queue() -> Result { - unsafe { SERVICES.evm.core.create_queue() } +fn unsafe_create_queue(timestamp: u64) -> Result { + unsafe { SERVICES.evm.core.create_queue(timestamp) } } /// /// Discards an EVM queue. diff --git a/lib/ain-rs-exports/src/lib.rs b/lib/ain-rs-exports/src/lib.rs index 9d2704f6ab..1e96a4a3be 100644 --- a/lib/ain-rs-exports/src/lib.rs +++ b/lib/ain-rs-exports/src/lib.rs @@ -141,7 +141,7 @@ pub mod ffi { // If they are fallible, it's a TODO to changed and move later // so errors are propogated up properly. fn evm_try_get_balance(result: &mut CrossBoundaryResult, address: &str) -> u64; - fn evm_try_unsafe_create_queue(result: &mut CrossBoundaryResult) -> u64; + fn evm_try_unsafe_create_queue(result: &mut CrossBoundaryResult, timestamp: u64) -> u64; fn evm_try_unsafe_remove_queue(result: &mut CrossBoundaryResult, queue_id: u64); fn evm_try_disconnect_latest_block(result: &mut CrossBoundaryResult); diff --git a/src/masternodes/mn_checks.cpp b/src/masternodes/mn_checks.cpp index 002987ab3e..dd84bab073 100644 --- a/src/masternodes/mn_checks.cpp +++ b/src/masternodes/mn_checks.cpp @@ -456,7 +456,7 @@ Res CustomTxVisit(CCustomCSView &mnview, bool wipeQueue{}; if (q == 0) { wipeQueue = true; - auto r = XResultValue(evm_try_unsafe_create_queue(result)); + auto r = XResultValue(evm_try_unsafe_create_queue(result, time)); if (r) { q = *r; } else { return r; } } diff --git a/src/miner.cpp b/src/miner.cpp index de718d0c52..661110f830 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -254,7 +254,7 @@ ResVal> BlockAssembler::CreateNewBlock(const CSc uint64_t evmQueueId{}; if (isEvmEnabledForBlock) { - auto r = XResultValueLogged(evm_try_unsafe_create_queue(result)); + auto r = XResultValueLogged(evm_try_unsafe_create_queue(result, blockTime)); if (!r) return Res::Err("Failed to create queue"); evmQueueId = *r; } diff --git a/src/validation.cpp b/src/validation.cpp index 4c5fab4745..cb26c3de5d 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2369,7 +2369,7 @@ static void LogApplyCustomTx(const CTransaction &tx, const int64_t start) { * Validity checks that depend on the UTXO set are also done; ConnectBlock () * can fail if those validity checks fail (among other reasons). */ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, - CCoinsViewCache& view, CCustomCSView& mnview, const CChainParams& chainparams, bool & rewardedAnchors, uint64_t evmQueueId, bool fJustCheck) + CCoinsViewCache& view, CCustomCSView& mnview, const CChainParams& chainparams, bool & rewardedAnchors, bool fJustCheck) { AssertLockHeld(cs_main); assert(pindex); @@ -2445,7 +2445,6 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl AddCoins(view, *block.vtx[i], 0); } } - XResultThrowOnErr(evm_try_unsafe_remove_queue(result, evmQueueId)); return true; } @@ -2650,6 +2649,13 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl const auto consensus = chainparams.GetConsensus(); auto isEvmEnabledForBlock = IsEVMEnabled(pindex->nHeight, mnview, consensus); + uint64_t evmQueueId{}; + if (isEvmEnabledForBlock) { + auto r = XResultValueLogged(evm_try_unsafe_create_queue(result, pindex->GetBlockTime())); + if (!r) return Res::Err("Failed to create queue"); + evmQueueId = *r; + } + // Execute TXs for (unsigned int i = 0; i < block.vtx.size(); i++) { @@ -3376,8 +3382,7 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp CCoinsViewCache view(&CoinsTip()); CCustomCSView mnview(*pcustomcsview, paccountHistoryDB.get(), pburnHistoryDB.get(), pvaultHistoryDB.get()); bool rewardedAnchors{}; - auto invalidStateReturn = [&](CValidationState& s, CBlockIndex* p, CCustomCSView& m, uint64_t q) { - XResultThrowOnErr(evm_try_unsafe_remove_queue(result, q)); + auto invalidStateReturn = [&](CValidationState& s, CBlockIndex* p, CCustomCSView& m) { if (s.IsInvalid()) { InvalidBlockFound(p, s); } @@ -3385,13 +3390,9 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp return error("ConnectBlock %s failed, %s", p->GetBlockHash().ToString(), FormatStateMessage(s)); }; - auto r = XResultValue(evm_try_unsafe_create_queue(result)); - if (!r) { return invalidStateReturn(state, pindexNew, mnview, 0); } - uint64_t evmQueueId = *r; - - bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, mnview, chainparams, rewardedAnchors, evmQueueId, false); + bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, mnview, chainparams, rewardedAnchors, false); GetMainSignals().BlockChecked(blockConnecting, state); - if (!rv) { return invalidStateReturn(state, pindexNew, mnview, evmQueueId); } + if (!rv) { return invalidStateReturn(state, pindexNew, mnview); } nTime3 = GetTimeMicros(); nTimeConnectTotal += nTime3 - nTime2; LogPrint(BCLog::BENCH, " - Connect total: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime3 - nTime2) * MILLI, nTimeConnectTotal * MICRO, nTimeConnectTotal * MILLI / nBlocksTotal); @@ -4927,31 +4928,23 @@ bool TestBlockValidity(CValidationState& state, const CChainParams& chainparams, indexDummy.phashBlock = &block_hash; CheckContextState ctxState; - auto r = XResultValue(evm_try_unsafe_create_queue(result)); - if (!r) { return error("%s: Consensus::ContextualCheckBlockHeader: error creating EVM queue", __func__); } - uint64_t evmQueueId = *r; - // NOTE: ContextualCheckProofOfStake is called by CheckBlock auto res = ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime()); if (!res) { - XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, FormatStateMessage(state)); } res = CheckBlock(block, state, chainparams.GetConsensus(), ctxState, false, indexDummy.nHeight, fCheckMerkleRoot); if (!res) { - XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state)); } res = ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindexPrev); if (!res) { - XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); return error("%s: Consensus::ContextualCheckBlock: %s", __func__, FormatStateMessage(state)); } - res = ::ChainstateActive().ConnectBlock(block, state, &indexDummy, viewNew, mnview, chainparams, dummyRewardedAnchors, evmQueueId, true); - XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); + res = ::ChainstateActive().ConnectBlock(block, state, &indexDummy, viewNew, mnview, chainparams, dummyRewardedAnchors, true); if (!res) return false; assert(state.IsValid()); @@ -5418,11 +5411,7 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); bool dummyRewardedAnchors{}; - auto r = XResultValue(evm_try_unsafe_create_queue(result)); - if (!r) { return error("VerifyDB(): *** evm_try_unsafe_create_queue failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); } - uint64_t evmQueueId = *r; - auto res = ::ChainstateActive().ConnectBlock(block, state, pindex, coins, mnview, chainparams, dummyRewardedAnchors, evmQueueId); - XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); + auto res = ::ChainstateActive().ConnectBlock(block, state, pindex, coins, mnview, chainparams, dummyRewardedAnchors); if (!res) return error("VerifyDB(): *** found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), FormatStateMessage(state)); if (ShutdownRequested()) return true; diff --git a/src/validation.h b/src/validation.h index e01aa310cf..7c6be98d64 100644 --- a/src/validation.h +++ b/src/validation.h @@ -740,7 +740,7 @@ class CChainState { // Block (dis)connection on a given view: DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view, CCustomCSView& cache, std::vector & disconnectedAnchorConfirms); bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, - CCoinsViewCache& view, CCustomCSView& cache, const CChainParams& chainparams, bool & rewardedAnchors, uint64_t evmQueueId, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CCoinsViewCache& view, CCustomCSView& cache, const CChainParams& chainparams, bool & rewardedAnchors, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Apply the effects of a block disconnection on the UTXO set. bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs); From ff4f997de350a97b5c4d48f4d8d3a436520d91ad Mon Sep 17 00:00:00 2001 From: Niven Date: Tue, 3 Oct 2023 15:55:36 +0800 Subject: [PATCH 06/16] Update mutexes --- lib/ain-evm/src/evm.rs | 2 +- lib/ain-evm/src/txqueue.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ain-evm/src/evm.rs b/lib/ain-evm/src/evm.rs index ca6f31cf17..73c770404e 100644 --- a/lib/ain-evm/src/evm.rs +++ b/lib/ain-evm/src/evm.rs @@ -408,7 +408,7 @@ impl EVMServices { let queue = self.core.tx_queues.get(queue_id)?; let state_root = queue.get_latest_state_root(); - let data = queue.data.lock().unwrap(); + let data = queue.data.lock(); ( data.target_block, diff --git a/lib/ain-evm/src/txqueue.rs b/lib/ain-evm/src/txqueue.rs index a7ba57984c..983d985df5 100644 --- a/lib/ain-evm/src/txqueue.rs +++ b/lib/ain-evm/src/txqueue.rs @@ -335,7 +335,7 @@ impl TransactionQueue { } pub fn get_timestamp(&self) -> u64 { - self.data.lock().unwrap().timestamp + self.data.lock().timestamp } pub fn get_state_root_from_native_hash(&self, hash: XHash) -> Option { From 6a8b94d9d2690164b6a679aac30ea89cf2da9dfa Mon Sep 17 00:00:00 2001 From: Niven Date: Tue, 3 Oct 2023 17:04:58 +0800 Subject: [PATCH 07/16] Shift block gas limit check into executor, check in construct block --- lib/ain-evm/src/backend.rs | 9 ++++++++ lib/ain-evm/src/core.rs | 27 ++++++++++++------------ lib/ain-evm/src/evm.rs | 20 +++++++++++++++++- lib/ain-evm/src/executor.rs | 10 +++++++++ lib/ain-evm/src/lib.rs | 2 ++ lib/ain-rs-exports/src/evm.rs | 6 ++++++ lib/ain-rs-exports/src/lib.rs | 1 + src/masternodes/validation.cpp | 3 +++ src/miner.cpp | 38 ++++++++++++++++++++++++++++++---- 9 files changed, 97 insertions(+), 19 deletions(-) diff --git a/lib/ain-evm/src/backend.rs b/lib/ain-evm/src/backend.rs index c403af331d..c82eaf38fe 100644 --- a/lib/ain-evm/src/backend.rs +++ b/lib/ain-evm/src/backend.rs @@ -31,6 +31,8 @@ pub struct Vicinity { pub block_number: U256, pub timestamp: U256, pub gas_limit: U256, + pub total_gas_used: U256, + pub block_gas_limit: U256, pub block_base_fee_per_gas: U256, pub block_randomness: Option, } @@ -145,6 +147,13 @@ impl EVMBackend { }; } + pub fn update_vicinity_with_gas_used(&mut self, gas_used: U256) { + self.vicinity = Vicinity { + total_gas_used: gas_used, + ..self.vicinity + }; + } + // Read-only handle pub fn ro_handle(&self) -> MptRo { let root = self.state.root(); diff --git a/lib/ain-evm/src/core.rs b/lib/ain-evm/src/core.rs index 2133b8db98..69e7af4115 100644 --- a/lib/ain-evm/src/core.rs +++ b/lib/ain-evm/src/core.rs @@ -274,6 +274,8 @@ impl EVMCoreService { block_number, origin: caller.unwrap_or_default(), gas_limit: U256::from(gas_limit), + total_gas_used: U256::zero(), + block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), gas_price: if transaction_type == Some(U256::from(2)) { max_fee_per_gas.unwrap_or_default() } else { @@ -467,19 +469,10 @@ impl EVMCoreService { .into()); } - // Execute tx + // Execute tx and validate total gas usage in queued txs do not exceed block size let mut executor = AinExecutor::new(&mut backend); - let (tx_response, ..) = - executor.exec(&signed_tx, signed_tx.gas_limit(), prepay_fee, block_fee)?; - - // Validate total gas usage in queued txs exceeds block size - debug!("[validate_raw_tx] used_gas: {:#?}", tx_response.used_gas); - let block_gas_limit = self.storage.get_attributes_or_default()?.block_gas_limit; - if total_current_gas_used + U256::from(tx_response.used_gas) - > U256::from(block_gas_limit) - { - return Err(format_err!("Tx can't make it in block. Block size limit {}, pending block gas used : {:x?}, tx used gas : {:x?}, total : {:x?}", block_gas_limit, total_current_gas_used, U256::from(tx_response.used_gas), total_current_gas_used + U256::from(tx_response.used_gas)).into()); - } + executor.update_total_gas_used(total_current_gas_used); + executor.exec(&signed_tx, signed_tx.gas_limit(), prepay_fee, block_fee)?; } Ok(self.tx_validation_cache.set( @@ -1002,7 +995,10 @@ impl EVMCoreService { state_root, Arc::clone(&self.trie_store), Arc::clone(&self.storage), - Vicinity::default(), + Vicinity { + block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), + ..Vicinity::default() + } ) } @@ -1012,7 +1008,10 @@ impl EVMCoreService { state_root, Arc::clone(&self.trie_store), Arc::clone(&self.storage), - Vicinity::default(), + Vicinity { + block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), + ..Vicinity::default() + } ) } diff --git a/lib/ain-evm/src/evm.rs b/lib/ain-evm/src/evm.rs index 73c770404e..00b4dacf26 100644 --- a/lib/ain-evm/src/evm.rs +++ b/lib/ain-evm/src/evm.rs @@ -19,6 +19,7 @@ use crate::{ transfer_domain_v1_contract_deploy_info, DeployContractInfo, }, core::{EVMCoreService, XHash}, + EVMError, executor::AinExecutor, filters::FilterService, log::LogService, @@ -44,6 +45,7 @@ pub struct EVMServices { pub struct FinalizedBlockInfo { pub block_hash: XHash, + pub failed_transaction: Option, pub total_burnt_fees: U256, pub total_priority_fees: U256, pub block_number: U256, @@ -127,6 +129,7 @@ impl EVMServices { let queue_txs_len = queue.transactions.len(); let mut all_transactions = Vec::with_capacity(queue_txs_len); + let mut failed_transaction = None; let mut receipts_v3: Vec = Vec::with_capacity(queue_txs_len); let mut total_gas_used = 0u64; @@ -147,6 +150,7 @@ impl EVMServices { beneficiary, timestamp: U256::from(timestamp), block_number: U256::zero(), + block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), ..Vicinity::default() }, H256::zero(), @@ -157,6 +161,7 @@ impl EVMServices { beneficiary, timestamp: U256::from(timestamp), block_number: number + 1, + block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), ..Vicinity::default() }, hash, @@ -291,7 +296,18 @@ impl EVMServices { ); for queue_item in queue.transactions.clone() { - let apply_result = executor.apply_queue_tx(queue_item.tx, base_fee)?; + executor.update_total_gas_used(U256::from(total_gas_used)); + let apply_result = match executor.apply_queue_tx(queue_item.tx, base_fee) { + Ok(result) => result, + Err(EVMError::BlockSizeLimit(message)) => { + failed_transaction = Some(queue_item.tx_hash); + debug!("[construct_block] {}", message); + break; + } + Err(e) => { + return Err(e); + } + }; all_transactions.push(apply_result.tx); EVMCoreService::logs_bloom(apply_result.logs, &mut logs_bloom); @@ -356,6 +372,7 @@ impl EVMServices { Ok(FinalizedBlockInfo { block_hash, + failed_transaction, total_burnt_fees, total_priority_fees, block_number: current_block_number, @@ -429,6 +446,7 @@ impl EVMServices { Vicinity { timestamp: U256::from(timestamp), block_number: target_block, + block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), ..Vicinity::default() }, )?; diff --git a/lib/ain-evm/src/executor.rs b/lib/ain-evm/src/executor.rs index 0d6176301d..55886eed86 100644 --- a/lib/ain-evm/src/executor.rs +++ b/lib/ain-evm/src/executor.rs @@ -18,6 +18,7 @@ use crate::{ }, core::EVMCoreService, evm::ReceiptAndOptionalContractAddress, + EVMError, fee::{calculate_current_prepay_gas_fee, calculate_gas_fee}, precompiles::MetachainPrecompiles, transaction::{ @@ -68,6 +69,10 @@ impl<'backend> AinExecutor<'backend> { self.backend.update_storage(&address, storage) } + pub fn update_total_gas_used(&mut self, gas_used: U256) { + self.backend.update_vicinity_with_gas_used(gas_used) + } + pub fn commit(&mut self) -> H256 { self.backend.commit() } @@ -178,6 +183,11 @@ impl<'backend> AinExecutor<'backend> { }; let used_gas = executor.used_gas(); + let total_gas_used = self.backend.vicinity.total_gas_used; + let block_gas_limit = self.backend.vicinity.block_gas_limit; + if total_gas_used + U256::from(used_gas) > block_gas_limit { + return Err(EVMError::BlockSizeLimit("Block size limit exceeded, tx cannot make it into the block".to_string())); + } let (values, logs) = executor.into_state().deconstruct(); let logs = logs.into_iter().collect::>(); diff --git a/lib/ain-evm/src/lib.rs b/lib/ain-evm/src/lib.rs index b0501408f9..ae062821b5 100644 --- a/lib/ain-evm/src/lib.rs +++ b/lib/ain-evm/src/lib.rs @@ -38,6 +38,8 @@ pub enum EVMError { QueueError(#[from] QueueError), #[error("EVM: Queue invalid nonce error {0:?}")] QueueInvalidNonce((Box, ethereum_types::U256)), + #[error("EVM: Exceed block size limit")] + BlockSizeLimit(String), #[error("EVM: IO error")] IoError(#[from] std::io::Error), #[error("EVM: Hex error")] diff --git a/lib/ain-rs-exports/src/evm.rs b/lib/ain-rs-exports/src/evm.rs index ff5641abd5..6045ee032d 100644 --- a/lib/ain-rs-exports/src/evm.rs +++ b/lib/ain-rs-exports/src/evm.rs @@ -549,6 +549,7 @@ fn unsafe_construct_block_in_q( unsafe { let FinalizedBlockInfo { block_hash, + failed_transaction, total_burnt_fees, total_priority_fees, block_number, @@ -562,9 +563,14 @@ fn unsafe_construct_block_in_q( )?; let total_burnt_fees = u64::try_from(WeiAmount(total_burnt_fees).to_satoshi())?; let total_priority_fees = u64::try_from(WeiAmount(total_priority_fees).to_satoshi())?; + let failed_transaction = match failed_transaction { + Some(hash) => hash, + None => String::new(), + }; Ok(ffi::FinalizeBlockCompletion { block_hash, + failed_transaction, total_burnt_fees, total_priority_fees, block_number: block_number.as_u64(), diff --git a/lib/ain-rs-exports/src/lib.rs b/lib/ain-rs-exports/src/lib.rs index 137c1deb29..53b9ee373b 100644 --- a/lib/ain-rs-exports/src/lib.rs +++ b/lib/ain-rs-exports/src/lib.rs @@ -122,6 +122,7 @@ pub mod ffi { #[derive(Default)] pub struct FinalizeBlockCompletion { pub block_hash: String, + pub failed_transaction: String, pub total_burnt_fees: u64, pub total_priority_fees: u64, pub block_number: u64, diff --git a/src/masternodes/validation.cpp b/src/masternodes/validation.cpp index ee64366453..28517a94ba 100644 --- a/src/masternodes/validation.cpp +++ b/src/masternodes/validation.cpp @@ -2414,6 +2414,9 @@ static Res ProcessEVMQueue(const CBlock &block, const CBlockIndex *pindex, CCust if (!result.ok) { return Res::Err(result.reason.c_str()); } + if (!blockResult.failed_transaction.empty()) { + return Res::Err("Failed EVM transaction, block size limit exceeded"); + } if (block.vtx[0]->vout.size() < 2) { return Res::Err("Not enough outputs in coinbase TX"); } diff --git a/src/miner.cpp b/src/miner.cpp index d5cfe630e6..873cd7e9bd 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -270,13 +270,43 @@ ResVal> BlockAssembler::CreateNewBlock(const CSc XVM xvm{}; if (isEvmEnabledForBlock) { auto res = XResultValueLogged(evm_try_unsafe_construct_block_in_q(result, evmQueueId, pos::GetNextWorkRequired(pindexPrev, pblock->nTime, consensus), evmBeneficiary, blockTime, nHeight, static_cast(reinterpret_cast(&mnview)))); - - auto r = XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); - if (!r) return Res::Err("Failed to remove queue"); - if (!res) return Res::Err("Failed to construct block"); auto blockResult = *res; + if (!blockResult.failed_transaction.empty()) { + LogPrintf("%s: Construct block exceeded block size limit\n", __func__); + auto failedTxHash = std::string(blockResult.failed_transaction.data(), blockResult.failed_transaction.length()); + auto res = XResultValueLogged(evm_try_unsafe_remove_txs_above_hash_in_q(result, evmQueueId, failedTxHash)); + if (!res.ok) { + LogPrintf("%s: Unable to get txs to be removed from queue. Will result in a block hash mismatch.\n", __func__); + } + else { + auto failedTxHashes = *res; + std::set evmTxsToUndo; + for (const auto &txStr : failedTxHashes) { + auto txHash = std::string(txStr.data(), txStr.length()); + evmTxsToUndo.insert(uint256S(txHash)); + } + // Get all EVM Txs + CTxMemPool::setEntries failedEVMTxs; + for (const auto& iter : inBlock) { + auto tx = iter->GetTx(); + if (!evmTxsToUndo.count(tx.GetHash())) + continue; + std::vector metadata; + const auto txType = GuessCustomTxType(tx, metadata, false); + if (txType == CustomTxType::EvmTx) { + failedEVMTxs.insert(iter); + }else { + LogPrintf("%s: Unable to remove from block, not EVM tx. Will result in a block hash mismatch.\n", __func__); + } + } + RemoveFromBlock(failedEVMTxs, true); + } + } + + auto r = XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); + if (!r) return Res::Err("Failed to remove queue"); xvm = XVM{0, {0, std::string(blockResult.block_hash.data(), blockResult.block_hash.length()).substr(2), blockResult.total_burnt_fees, blockResult.total_priority_fees, evmBeneficiary}}; } From 808e000a2e5735df8b276999f2a9b9e4fa38bd04 Mon Sep 17 00:00:00 2001 From: Niven Date: Tue, 3 Oct 2023 17:14:06 +0800 Subject: [PATCH 08/16] Fix rust lint --- lib/ain-evm/src/backend.rs | 2 +- lib/ain-evm/src/core.rs | 12 ++++++++---- lib/ain-evm/src/evm.rs | 15 ++++++++++----- lib/ain-evm/src/executor.rs | 7 ++++--- 4 files changed, 23 insertions(+), 13 deletions(-) diff --git a/lib/ain-evm/src/backend.rs b/lib/ain-evm/src/backend.rs index c82eaf38fe..16befc7bc9 100644 --- a/lib/ain-evm/src/backend.rs +++ b/lib/ain-evm/src/backend.rs @@ -309,7 +309,7 @@ impl Backend for EVMBackend { } fn block_gas_limit(&self) -> U256 { - self.vicinity.gas_limit + self.vicinity.block_gas_limit } fn block_base_fee_per_gas(&self) -> U256 { diff --git a/lib/ain-evm/src/core.rs b/lib/ain-evm/src/core.rs index 69e7af4115..7258b9597e 100644 --- a/lib/ain-evm/src/core.rs +++ b/lib/ain-evm/src/core.rs @@ -996,9 +996,11 @@ impl EVMCoreService { Arc::clone(&self.trie_store), Arc::clone(&self.storage), Vicinity { - block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), + block_gas_limit: U256::from( + self.storage.get_attributes_or_default()?.block_gas_limit, + ), ..Vicinity::default() - } + }, ) } @@ -1009,9 +1011,11 @@ impl EVMCoreService { Arc::clone(&self.trie_store), Arc::clone(&self.storage), Vicinity { - block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), + block_gas_limit: U256::from( + self.storage.get_attributes_or_default()?.block_gas_limit, + ), ..Vicinity::default() - } + }, ) } diff --git a/lib/ain-evm/src/evm.rs b/lib/ain-evm/src/evm.rs index 00b4dacf26..a229f5a0db 100644 --- a/lib/ain-evm/src/evm.rs +++ b/lib/ain-evm/src/evm.rs @@ -19,7 +19,6 @@ use crate::{ transfer_domain_v1_contract_deploy_info, DeployContractInfo, }, core::{EVMCoreService, XHash}, - EVMError, executor::AinExecutor, filters::FilterService, log::LogService, @@ -31,7 +30,7 @@ use crate::{ }, trie::GENESIS_STATE_ROOT, txqueue::{BlockData, QueueTx}, - Result, + EVMError, Result, }; pub struct EVMServices { @@ -150,7 +149,9 @@ impl EVMServices { beneficiary, timestamp: U256::from(timestamp), block_number: U256::zero(), - block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), + block_gas_limit: U256::from( + self.storage.get_attributes_or_default()?.block_gas_limit, + ), ..Vicinity::default() }, H256::zero(), @@ -161,7 +162,9 @@ impl EVMServices { beneficiary, timestamp: U256::from(timestamp), block_number: number + 1, - block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), + block_gas_limit: U256::from( + self.storage.get_attributes_or_default()?.block_gas_limit, + ), ..Vicinity::default() }, hash, @@ -446,7 +449,9 @@ impl EVMServices { Vicinity { timestamp: U256::from(timestamp), block_number: target_block, - block_gas_limit: U256::from(self.storage.get_attributes_or_default()?.block_gas_limit), + block_gas_limit: U256::from( + self.storage.get_attributes_or_default()?.block_gas_limit, + ), ..Vicinity::default() }, )?; diff --git a/lib/ain-evm/src/executor.rs b/lib/ain-evm/src/executor.rs index 55886eed86..02c1c4fb93 100644 --- a/lib/ain-evm/src/executor.rs +++ b/lib/ain-evm/src/executor.rs @@ -18,7 +18,6 @@ use crate::{ }, core::EVMCoreService, evm::ReceiptAndOptionalContractAddress, - EVMError, fee::{calculate_current_prepay_gas_fee, calculate_gas_fee}, precompiles::MetachainPrecompiles, transaction::{ @@ -26,7 +25,7 @@ use crate::{ SignedTx, }, txqueue::QueueTx, - Result, + EVMError, Result, }; #[derive(Debug)] @@ -186,7 +185,9 @@ impl<'backend> AinExecutor<'backend> { let total_gas_used = self.backend.vicinity.total_gas_used; let block_gas_limit = self.backend.vicinity.block_gas_limit; if total_gas_used + U256::from(used_gas) > block_gas_limit { - return Err(EVMError::BlockSizeLimit("Block size limit exceeded, tx cannot make it into the block".to_string())); + return Err(EVMError::BlockSizeLimit( + "Block size limit exceeded, tx cannot make it into the block".to_string(), + )); } let (values, logs) = executor.into_state().deconstruct(); let logs = logs.into_iter().collect::>(); From 8ae2c35359f577aa7733cbe622ae557ba946a5d9 Mon Sep 17 00:00:00 2001 From: Niven Date: Tue, 3 Oct 2023 19:48:16 +0800 Subject: [PATCH 09/16] Fix naming --- lib/ain-evm/src/executor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ain-evm/src/executor.rs b/lib/ain-evm/src/executor.rs index b2e6946445..d557b395ae 100644 --- a/lib/ain-evm/src/executor.rs +++ b/lib/ain-evm/src/executor.rs @@ -228,11 +228,11 @@ impl<'backend> AinExecutor<'backend> { .into()); } - let prepay_gas = calculate_current_prepay_gas_fee(&signed_tx, base_fee)?; + let prepay_fee = calculate_current_prepay_gas_fee(&signed_tx, base_fee)?; let (tx_response, receipt) = self.exec( &signed_tx, signed_tx.gas_limit(), - prepay_gas, + prepay_fee, base_fee, false, )?; From 23f391ed0b38b87a12d5f497a145c5deda8c769b Mon Sep 17 00:00:00 2001 From: Niven Date: Tue, 3 Oct 2023 21:58:41 +0800 Subject: [PATCH 10/16] Fix rust lint --- lib/ain-evm/src/core.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/ain-evm/src/core.rs b/lib/ain-evm/src/core.rs index 8528320c26..48c6780c2e 100644 --- a/lib/ain-evm/src/core.rs +++ b/lib/ain-evm/src/core.rs @@ -472,7 +472,13 @@ impl EVMCoreService { // Execute tx and validate total gas usage in queued txs do not exceed block size let mut executor = AinExecutor::new(&mut backend); executor.update_total_gas_used(total_current_gas_used); - executor.exec(&signed_tx, signed_tx.gas_limit(), prepay_fee, block_fee, false)?; + executor.exec( + &signed_tx, + signed_tx.gas_limit(), + prepay_fee, + block_fee, + false, + )?; } Ok(self.tx_validation_cache.set( From 225e302906b5058fe02cbf5b822eb34268cef0c9 Mon Sep 17 00:00:00 2001 From: Niven Date: Tue, 3 Oct 2023 22:00:15 +0800 Subject: [PATCH 11/16] Restrict block gas limit check to only signed txs --- lib/ain-evm/src/executor.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ain-evm/src/executor.rs b/lib/ain-evm/src/executor.rs index 3ef3c1e2c0..8bb1ec11e2 100644 --- a/lib/ain-evm/src/executor.rs +++ b/lib/ain-evm/src/executor.rs @@ -187,7 +187,7 @@ impl<'backend> AinExecutor<'backend> { let used_gas = if system_tx { 0u64 } else { executor.used_gas() }; let total_gas_used = self.backend.vicinity.total_gas_used; let block_gas_limit = self.backend.vicinity.block_gas_limit; - if total_gas_used + U256::from(used_gas) > block_gas_limit { + if !system_tx && total_gas_used + U256::from(used_gas) > block_gas_limit { return Err(EVMError::BlockSizeLimit( "Block size limit exceeded, tx cannot make it into the block".to_string(), )); From 1bee0a43914a93d925ff30dc8cf0a8eece4dc94c Mon Sep 17 00:00:00 2001 From: Niven Date: Tue, 3 Oct 2023 22:01:23 +0800 Subject: [PATCH 12/16] Better refactor --- lib/ain-evm/src/executor.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ain-evm/src/executor.rs b/lib/ain-evm/src/executor.rs index d557b395ae..97e01a87cf 100644 --- a/lib/ain-evm/src/executor.rs +++ b/lib/ain-evm/src/executor.rs @@ -147,7 +147,7 @@ impl<'backend> AinExecutor<'backend> { access_list: signed_tx.access_list(), }; - if prepay_fee != U256::zero() && !system_tx { + if !system_tx && prepay_fee != U256::zero() { self.backend .deduct_prepay_gas_fee(signed_tx.sender, prepay_fee)?; } @@ -187,7 +187,7 @@ impl<'backend> AinExecutor<'backend> { ApplyBackend::apply(self.backend, values, logs.clone(), true); self.backend.commit(); - if prepay_fee != U256::zero() && !system_tx { + if !system_tx && prepay_fee != U256::zero() { self.backend .refund_unused_gas_fee(signed_tx, U256::from(used_gas), base_fee)?; } From f04a9145bc68cdda0a58d7e7a1e3d8ebd13ae83a Mon Sep 17 00:00:00 2001 From: Niven Date: Tue, 3 Oct 2023 22:09:37 +0800 Subject: [PATCH 13/16] Use txtype --- src/miner.cpp | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/miner.cpp b/src/miner.cpp index 873cd7e9bd..0aa9958908 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -290,14 +290,12 @@ ResVal> BlockAssembler::CreateNewBlock(const CSc // Get all EVM Txs CTxMemPool::setEntries failedEVMTxs; for (const auto& iter : inBlock) { - auto tx = iter->GetTx(); - if (!evmTxsToUndo.count(tx.GetHash())) + if (!evmTxsToUndo.count(iter->GetTx().GetHash())) continue; - std::vector metadata; - const auto txType = GuessCustomTxType(tx, metadata, false); + const auto txType = iter->GetCustomTxType(); if (txType == CustomTxType::EvmTx) { failedEVMTxs.insert(iter); - }else { + } else { LogPrintf("%s: Unable to remove from block, not EVM tx. Will result in a block hash mismatch.\n", __func__); } } From 09fd806b3d1029c06a8adfb348f1fa3d1eb31541 Mon Sep 17 00:00:00 2001 From: Niven Date: Wed, 4 Oct 2023 00:57:33 +0800 Subject: [PATCH 14/16] Fix construct block to return vector of failed tx hashes --- lib/ain-evm/src/evm.rs | 25 ++++++++++++++++----- lib/ain-rs-exports/src/evm.rs | 8 ++----- lib/ain-rs-exports/src/lib.rs | 2 +- src/masternodes/validation.cpp | 2 +- src/miner.cpp | 41 ++++++++++++++-------------------- 5 files changed, 41 insertions(+), 37 deletions(-) diff --git a/lib/ain-evm/src/evm.rs b/lib/ain-evm/src/evm.rs index a229f5a0db..0b221eb64e 100644 --- a/lib/ain-evm/src/evm.rs +++ b/lib/ain-evm/src/evm.rs @@ -44,7 +44,7 @@ pub struct EVMServices { pub struct FinalizedBlockInfo { pub block_hash: XHash, - pub failed_transaction: Option, + pub failed_transactions: Vec, pub total_burnt_fees: U256, pub total_priority_fees: U256, pub block_number: U256, @@ -128,7 +128,7 @@ impl EVMServices { let queue_txs_len = queue.transactions.len(); let mut all_transactions = Vec::with_capacity(queue_txs_len); - let mut failed_transaction = None; + let mut failed_transactions = Vec::with_capacity(queue_txs_len); let mut receipts_v3: Vec = Vec::with_capacity(queue_txs_len); let mut total_gas_used = 0u64; @@ -303,9 +303,24 @@ impl EVMServices { let apply_result = match executor.apply_queue_tx(queue_item.tx, base_fee) { Ok(result) => result, Err(EVMError::BlockSizeLimit(message)) => { - failed_transaction = Some(queue_item.tx_hash); debug!("[construct_block] {}", message); - break; + if let Some(index) = queue + .transactions + .iter() + .position(|item| item.tx_hash == queue_item.tx_hash) + { + failed_transactions = queue + .transactions + .drain(index..) + .map(|item| item.tx_hash) + .collect(); + break; + } else { + return Err(format_err!( + "exceed block size limit but unable to get failed transaction from queue" + ) + .into()); + } } Err(e) => { return Err(e); @@ -375,7 +390,7 @@ impl EVMServices { Ok(FinalizedBlockInfo { block_hash, - failed_transaction, + failed_transactions, total_burnt_fees, total_priority_fees, block_number: current_block_number, diff --git a/lib/ain-rs-exports/src/evm.rs b/lib/ain-rs-exports/src/evm.rs index 6045ee032d..fc03c9e539 100644 --- a/lib/ain-rs-exports/src/evm.rs +++ b/lib/ain-rs-exports/src/evm.rs @@ -549,7 +549,7 @@ fn unsafe_construct_block_in_q( unsafe { let FinalizedBlockInfo { block_hash, - failed_transaction, + failed_transactions, total_burnt_fees, total_priority_fees, block_number, @@ -563,14 +563,10 @@ fn unsafe_construct_block_in_q( )?; let total_burnt_fees = u64::try_from(WeiAmount(total_burnt_fees).to_satoshi())?; let total_priority_fees = u64::try_from(WeiAmount(total_priority_fees).to_satoshi())?; - let failed_transaction = match failed_transaction { - Some(hash) => hash, - None => String::new(), - }; Ok(ffi::FinalizeBlockCompletion { block_hash, - failed_transaction, + failed_transactions, total_burnt_fees, total_priority_fees, block_number: block_number.as_u64(), diff --git a/lib/ain-rs-exports/src/lib.rs b/lib/ain-rs-exports/src/lib.rs index 53b9ee373b..308d565e9d 100644 --- a/lib/ain-rs-exports/src/lib.rs +++ b/lib/ain-rs-exports/src/lib.rs @@ -122,7 +122,7 @@ pub mod ffi { #[derive(Default)] pub struct FinalizeBlockCompletion { pub block_hash: String, - pub failed_transaction: String, + pub failed_transactions: Vec, pub total_burnt_fees: u64, pub total_priority_fees: u64, pub block_number: u64, diff --git a/src/masternodes/validation.cpp b/src/masternodes/validation.cpp index 28517a94ba..2c39257fde 100644 --- a/src/masternodes/validation.cpp +++ b/src/masternodes/validation.cpp @@ -2414,7 +2414,7 @@ static Res ProcessEVMQueue(const CBlock &block, const CBlockIndex *pindex, CCust if (!result.ok) { return Res::Err(result.reason.c_str()); } - if (!blockResult.failed_transaction.empty()) { + if (!blockResult.failed_transactions.empty()) { return Res::Err("Failed EVM transaction, block size limit exceeded"); } if (block.vtx[0]->vout.size() < 2) { diff --git a/src/miner.cpp b/src/miner.cpp index 0aa9958908..da139cccf0 100644 --- a/src/miner.cpp +++ b/src/miner.cpp @@ -272,35 +272,28 @@ ResVal> BlockAssembler::CreateNewBlock(const CSc auto res = XResultValueLogged(evm_try_unsafe_construct_block_in_q(result, evmQueueId, pos::GetNextWorkRequired(pindexPrev, pblock->nTime, consensus), evmBeneficiary, blockTime, nHeight, static_cast(reinterpret_cast(&mnview)))); if (!res) return Res::Err("Failed to construct block"); auto blockResult = *res; - if (!blockResult.failed_transaction.empty()) { + if (!blockResult.failed_transactions.empty()) { LogPrintf("%s: Construct block exceeded block size limit\n", __func__); - auto failedTxHash = std::string(blockResult.failed_transaction.data(), blockResult.failed_transaction.length()); - auto res = XResultValueLogged(evm_try_unsafe_remove_txs_above_hash_in_q(result, evmQueueId, failedTxHash)); - if (!res.ok) { - LogPrintf("%s: Unable to get txs to be removed from queue. Will result in a block hash mismatch.\n", __func__); + auto failedTxHashes = *res; + std::set evmTxsToUndo; + for (const auto &txStr : blockResult.failed_transactions) { + auto txHash = std::string(txStr.data(), txStr.length()); + evmTxsToUndo.insert(uint256S(txHash)); } - else { - auto failedTxHashes = *res; - std::set evmTxsToUndo; - for (const auto &txStr : failedTxHashes) { - auto txHash = std::string(txStr.data(), txStr.length()); - evmTxsToUndo.insert(uint256S(txHash)); - } - // Get all EVM Txs - CTxMemPool::setEntries failedEVMTxs; - for (const auto& iter : inBlock) { - if (!evmTxsToUndo.count(iter->GetTx().GetHash())) - continue; - const auto txType = iter->GetCustomTxType(); - if (txType == CustomTxType::EvmTx) { - failedEVMTxs.insert(iter); - } else { - LogPrintf("%s: Unable to remove from block, not EVM tx. Will result in a block hash mismatch.\n", __func__); - } + // Get all EVM Txs + CTxMemPool::setEntries failedEVMTxs; + for (const auto& iter : inBlock) { + if (!evmTxsToUndo.count(iter->GetTx().GetHash())) + continue; + const auto txType = iter->GetCustomTxType(); + if (txType == CustomTxType::EvmTx) { + failedEVMTxs.insert(iter); + } else { + LogPrintf("%s: Unable to remove from block, not EVM tx. Will result in a block hash mismatch.\n", __func__); } - RemoveFromBlock(failedEVMTxs, true); } + RemoveFromBlock(failedEVMTxs, true); } auto r = XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); From 7890272beed0583b617ea1d67d30c5eb4123185e Mon Sep 17 00:00:00 2001 From: Niven Date: Wed, 4 Oct 2023 01:06:00 +0800 Subject: [PATCH 15/16] Fix error msg --- src/masternodes/validation.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/masternodes/validation.cpp b/src/masternodes/validation.cpp index 2c39257fde..a4571d7811 100644 --- a/src/masternodes/validation.cpp +++ b/src/masternodes/validation.cpp @@ -2415,7 +2415,7 @@ static Res ProcessEVMQueue(const CBlock &block, const CBlockIndex *pindex, CCust return Res::Err(result.reason.c_str()); } if (!blockResult.failed_transactions.empty()) { - return Res::Err("Failed EVM transaction, block size limit exceeded"); + return Res::Err("Failed EVM transactions, block size limit exceeded"); } if (block.vtx[0]->vout.size() < 2) { return Res::Err("Not enough outputs in coinbase TX"); From 037d5134c79a7e683b01bcefba1cdba65dbc8234 Mon Sep 17 00:00:00 2001 From: Niven Date: Wed, 4 Oct 2023 12:44:17 +0800 Subject: [PATCH 16/16] Revert changes in connect block --- src/validation.cpp | 37 ++++++++++++++++++++++++------------- src/validation.h | 2 +- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/src/validation.cpp b/src/validation.cpp index 71f20e436b..7ccc8b7201 100644 --- a/src/validation.cpp +++ b/src/validation.cpp @@ -2369,7 +2369,7 @@ static void LogApplyCustomTx(const CTransaction &tx, const int64_t start) { * Validity checks that depend on the UTXO set are also done; ConnectBlock () * can fail if those validity checks fail (among other reasons). */ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, - CCoinsViewCache& view, CCustomCSView& mnview, const CChainParams& chainparams, bool & rewardedAnchors, bool fJustCheck) + CCoinsViewCache& view, CCustomCSView& mnview, const CChainParams& chainparams, bool & rewardedAnchors, uint64_t evmQueueId, bool fJustCheck) { AssertLockHeld(cs_main); assert(pindex); @@ -2445,6 +2445,7 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl AddCoins(view, *block.vtx[i], 0); } } + XResultThrowOnErr(evm_try_unsafe_remove_queue(result, evmQueueId)); return true; } @@ -2649,13 +2650,6 @@ bool CChainState::ConnectBlock(const CBlock& block, CValidationState& state, CBl const auto consensus = chainparams.GetConsensus(); auto isEvmEnabledForBlock = IsEVMEnabled(pindex->nHeight, mnview, consensus); - uint64_t evmQueueId{}; - if (isEvmEnabledForBlock) { - auto r = XResultValueLogged(evm_try_unsafe_create_queue(result, pindex->GetBlockTime())); - if (!r) return Res::Err("Failed to create queue"); - evmQueueId = *r; - } - // Execute TXs for (unsigned int i = 0; i < block.vtx.size(); i++) { @@ -3382,7 +3376,8 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp CCoinsViewCache view(&CoinsTip()); CCustomCSView mnview(*pcustomcsview, paccountHistoryDB.get(), pburnHistoryDB.get(), pvaultHistoryDB.get()); bool rewardedAnchors{}; - auto invalidStateReturn = [&](CValidationState& s, CBlockIndex* p, CCustomCSView& m) { + auto invalidStateReturn = [&](CValidationState& s, CBlockIndex* p, CCustomCSView& m, uint64_t q) { + XResultThrowOnErr(evm_try_unsafe_remove_queue(result, q)); if (s.IsInvalid()) { InvalidBlockFound(p, s); } @@ -3390,9 +3385,13 @@ bool CChainState::ConnectTip(CValidationState& state, const CChainParams& chainp return error("ConnectBlock %s failed, %s", p->GetBlockHash().ToString(), FormatStateMessage(s)); }; - bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, mnview, chainparams, rewardedAnchors, false); + auto r = XResultValue(evm_try_unsafe_create_queue(result, pindexNew->GetBlockTime())); + if (!r) { return invalidStateReturn(state, pindexNew, mnview, 0); } + uint64_t evmQueueId = *r; + + bool rv = ConnectBlock(blockConnecting, state, pindexNew, view, mnview, chainparams, rewardedAnchors, evmQueueId, false); GetMainSignals().BlockChecked(blockConnecting, state); - if (!rv) { return invalidStateReturn(state, pindexNew, mnview); } + if (!rv) { return invalidStateReturn(state, pindexNew, mnview, evmQueueId); } nTime3 = GetTimeMicros(); nTimeConnectTotal += nTime3 - nTime2; LogPrint(BCLog::BENCH, " - Connect total: %.2fms [%.2fs (%.2fms/blk)]\n", (nTime3 - nTime2) * MILLI, nTimeConnectTotal * MICRO, nTimeConnectTotal * MILLI / nBlocksTotal); @@ -4928,23 +4927,31 @@ bool TestBlockValidity(CValidationState& state, const CChainParams& chainparams, indexDummy.phashBlock = &block_hash; CheckContextState ctxState; + auto r = XResultValue(evm_try_unsafe_create_queue(result, indexDummy.GetBlockTime())); + if (!r) { return error("%s: Consensus::ContextualCheckBlockHeader: error creating EVM queue", __func__); } + uint64_t evmQueueId = *r; + // NOTE: ContextualCheckProofOfStake is called by CheckBlock auto res = ContextualCheckBlockHeader(block, state, chainparams, pindexPrev, GetAdjustedTime()); if (!res) { + XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); return error("%s: Consensus::ContextualCheckBlockHeader: %s", __func__, FormatStateMessage(state)); } res = CheckBlock(block, state, chainparams.GetConsensus(), ctxState, false, indexDummy.nHeight, fCheckMerkleRoot); if (!res) { + XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); return error("%s: Consensus::CheckBlock: %s", __func__, FormatStateMessage(state)); } res = ContextualCheckBlock(block, state, chainparams.GetConsensus(), pindexPrev); if (!res) { + XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); return error("%s: Consensus::ContextualCheckBlock: %s", __func__, FormatStateMessage(state)); } - res = ::ChainstateActive().ConnectBlock(block, state, &indexDummy, viewNew, mnview, chainparams, dummyRewardedAnchors, true); + res = ::ChainstateActive().ConnectBlock(block, state, &indexDummy, viewNew, mnview, chainparams, dummyRewardedAnchors, evmQueueId, true); + XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); if (!res) return false; assert(state.IsValid()); @@ -5411,7 +5418,11 @@ bool CVerifyDB::VerifyDB(const CChainParams& chainparams, CCoinsView *coinsview, return error("VerifyDB(): *** ReadBlockFromDisk failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); bool dummyRewardedAnchors{}; - auto res = ::ChainstateActive().ConnectBlock(block, state, pindex, coins, mnview, chainparams, dummyRewardedAnchors); + auto r = XResultValue(evm_try_unsafe_create_queue(result, pindex->GetBlockTime())); + if (!r) { return error("VerifyDB(): *** evm_try_unsafe_create_queue failed at %d, hash=%s", pindex->nHeight, pindex->GetBlockHash().ToString()); } + uint64_t evmQueueId = *r; + auto res = ::ChainstateActive().ConnectBlock(block, state, pindex, coins, mnview, chainparams, dummyRewardedAnchors, evmQueueId); + XResultStatusLogged(evm_try_unsafe_remove_queue(result, evmQueueId)); if (!res) return error("VerifyDB(): *** found unconnectable block at %d, hash=%s (%s)", pindex->nHeight, pindex->GetBlockHash().ToString(), FormatStateMessage(state)); if (ShutdownRequested()) return true; diff --git a/src/validation.h b/src/validation.h index 7c6be98d64..e01aa310cf 100644 --- a/src/validation.h +++ b/src/validation.h @@ -740,7 +740,7 @@ class CChainState { // Block (dis)connection on a given view: DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view, CCustomCSView& cache, std::vector & disconnectedAnchorConfirms); bool ConnectBlock(const CBlock& block, CValidationState& state, CBlockIndex* pindex, - CCoinsViewCache& view, CCustomCSView& cache, const CChainParams& chainparams, bool & rewardedAnchors, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); + CCoinsViewCache& view, CCustomCSView& cache, const CChainParams& chainparams, bool & rewardedAnchors, uint64_t evmQueueId, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main); // Apply the effects of a block disconnection on the UTXO set. bool DisconnectTip(CValidationState& state, const CChainParams& chainparams, DisconnectedBlockTransactions* disconnectpool) EXCLUSIVE_LOCKS_REQUIRED(cs_main, ::mempool.cs);