EVM: Clean up construct block validation (#2526)

* Fix tx receipt for system txs * Fix rust lint * Housekeeping * Add test for td tx receipt for zero gas used * Set correct timestamp in queue backend * Update mutexes * Shift block gas limit check into executor, check in construct block * Fix rust lint * Fix naming * Fix rust lint * Restrict block gas limit check to only signed txs * Better refactor * Use txtype * Fix construct block to return vector of failed tx hashes * Fix error msg * Revert changes in connect block --------- Co-authored-by: jouzo <[email protected]> Co-authored-by: Prasanna Loganathar <[email protected]>
DeFiCh · Oct 4, 2023 · 507dd16 · 507dd16
1 parent 5dc6a2f
commit 507dd16
Show file tree

Hide file tree

Showing 9 changed files with 112 additions and 21 deletions.
diff --git 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<H256>,
 }
@@ -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();
@@ -300,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
@@ -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,24 +469,16 @@ 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(
+            executor.update_total_gas_used(total_current_gas_used);
+            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);
-            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());
-            }
         }
 
         Ok(self.tx_validation_cache.set(
@@ -1007,7 +1001,12 @@ 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()
+            },
         )
     }
 
@@ -1017,7 +1016,12 @@ 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
@@ -30,7 +30,7 @@ use crate::{
     },
     trie::GENESIS_STATE_ROOT,
     txqueue::{BlockData, QueueTx},
-    Result,
+    EVMError, Result,
 };
 
 pub struct EVMServices {
@@ -44,6 +44,7 @@ pub struct EVMServices {
 
 pub struct FinalizedBlockInfo {
     pub block_hash: XHash,
+    pub failed_transactions: Vec<XHash>,
     pub total_burnt_fees: U256,
     pub total_priority_fees: U256,
     pub block_number: U256,
@@ -127,6 +128,7 @@ impl EVMServices {
 
         let queue_txs_len = queue.transactions.len();
         let mut all_transactions = Vec::with_capacity(queue_txs_len);
+        let mut failed_transactions = Vec::with_capacity(queue_txs_len);
         let mut receipts_v3: Vec<ReceiptAndOptionalContractAddress> =
             Vec::with_capacity(queue_txs_len);
         let mut total_gas_used = 0u64;
@@ -147,6 +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,
+                    ),
                     ..Vicinity::default()
                 },
                 H256::zero(),
@@ -157,6 +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,
+                    ),
                     ..Vicinity::default()
                 },
                 hash,
@@ -291,7 +299,33 @@ 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)) => {
+                    debug!("[construct_block] {}", message);
+                    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);
+                }
+            };
 
             all_transactions.push(apply_result.tx);
             EVMCoreService::logs_bloom(apply_result.logs, &mut logs_bloom);
@@ -356,6 +390,7 @@ impl EVMServices {
 
         Ok(FinalizedBlockInfo {
             block_hash,
+            failed_transactions,
             total_burnt_fees,
             total_priority_fees,
             block_number: current_block_number,
@@ -429,6 +464,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,
+                ),
                 ..Vicinity::default()
             },
         )?;

diff --git a/lib/ain-evm/src/executor.rs b/lib/ain-evm/src/executor.rs
@@ -25,7 +25,7 @@ use crate::{
         SignedTx,
     },
     txqueue::QueueTx,
-    Result,
+    EVMError, Result,
 };
 
 #[derive(Debug)]
@@ -68,6 +68,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()
     }
@@ -181,6 +185,13 @@ 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 !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(),
+            ));
+        }
         let (values, logs) = executor.into_state().deconstruct();
         let logs = logs.into_iter().collect::<Vec<_>>();
 

diff --git 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<transaction::SignedTx>, 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
@@ -549,6 +549,7 @@ fn unsafe_construct_block_in_q(
     unsafe {
         let FinalizedBlockInfo {
             block_hash,
+            failed_transactions,
             total_burnt_fees,
             total_priority_fees,
             block_number,
@@ -565,6 +566,7 @@ fn unsafe_construct_block_in_q(
 
         Ok(ffi::FinalizeBlockCompletion {
             block_hash,
+            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
@@ -122,6 +122,7 @@ pub mod ffi {
     #[derive(Default)]
     pub struct FinalizeBlockCompletion {
         pub block_hash: String,
+        pub failed_transactions: Vec<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
@@ -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_transactions.empty()) {
+        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");
     }

diff --git a/src/miner.cpp b/src/miner.cpp
@@ -270,13 +270,34 @@ ResVal<std::unique_ptr<CBlockTemplate>> 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<std::size_t>(reinterpret_cast<uintptr_t>(&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_transactions.empty()) {
+            LogPrintf("%s: Construct block exceeded block size limit\n", __func__);
+            auto failedTxHashes = *res;
+            std::set<uint256> evmTxsToUndo;
+            for (const auto &txStr : blockResult.failed_transactions) {
+                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__);
+                }
+            }
+            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}};
     }