From 7a24a1317aff71e2c2550fc967cb95ed3811319f Mon Sep 17 00:00:00 2001
From: shawn-zil <shawn@zilliqa.com>
Date: Mon, 7 Oct 2024 18:04:09 +0800
Subject: [PATCH] Reduce time taken to collect VerifiedTransactions (#1563)

* Added timing probes.

* Reduced recover_signer() call when assembling the list of txns in execute_block().
Makes it up to 1,500x faster.

* Made it more idiomatic.

* Removed logging/probes.

* Fixed review comments.
---
 zilliqa/src/consensus.rs | 44 +++++++++++++++++++++++-----------------
 1 file changed, 25 insertions(+), 19 deletions(-)

diff --git a/zilliqa/src/consensus.rs b/zilliqa/src/consensus.rs
index 5a54cab26..1568fec8a 100644
--- a/zilliqa/src/consensus.rs
+++ b/zilliqa/src/consensus.rs
@@ -2627,29 +2627,36 @@ impl Consensus {
             trace!("applying {} transactions to state", transactions.len());
         }
 
-        let transactions: Result<Vec<_>> = transactions.into_iter().map(|tx| tx.verify()).collect();
-        let mut transactions = transactions?;
-
-        let transaction_hashes = transactions
-            .iter()
-            .map(|tx| format!("{:?}", tx.hash))
-            .join(",");
+        let mut verified_txns = Vec::new();
 
         // We re-inject any missing Intershard transactions (or really, any missing
-        // transactions) from our mempool. If any txs are unavailable either in the
+        // transactions) from our mempool. If any txs are unavailable in both the
         // message or locally, the proposal cannot be applied
         for (idx, tx_hash) in block.transactions.iter().enumerate() {
-            if transactions.get(idx).is_some_and(|tx| tx.hash == *tx_hash) {
-                // all good
-            } else {
-                let Some(local_tx) = self.transaction_pool.pop_transaction(*tx_hash) else {
-                    warn!("Proposal {} at view {} referenced a transaction {} that was neither included in the broadcast nor found locally - cannot apply block", block.hash(), block.view(), tx_hash);
-                    return Ok(());
-                };
-                transactions.insert(idx, local_tx);
-            }
+            // Prefer to insert verified txn from pool. This is faster.
+            let txn = match self.transaction_pool.pop_transaction(*tx_hash) {
+                Some(txn) => txn,
+                _ => match transactions
+                    .get(idx)
+                    .map(|sig_tx| sig_tx.clone().verify())
+                    .transpose()?
+                {
+                    // Otherwise, recover txn from proposal. This is slower.
+                    Some(txn) if txn.hash == *tx_hash => txn,
+                    _ => {
+                        warn!("Proposal {} at view {} referenced a transaction {} that was neither included in the broadcast nor found locally - cannot apply block", block.hash(), block.view(), tx_hash);
+                        return Ok(());
+                    }
+                },
+            };
+            verified_txns.push(txn);
         }
 
+        let transaction_hashes = verified_txns
+            .iter()
+            .map(|tx| format!("{:?}", tx.hash))
+            .join(",");
+
         self.apply_rewards(committee, &parent, block.view(), &block.header.qc.cosigned)?;
 
         // ZIP-9: Sink gas to zero account
@@ -2666,7 +2673,7 @@ impl Consensus {
         let mut receipts_trie = eth_trie::EthTrie::new(Arc::new(MemoryDB::new(true)));
         let mut transactions_trie = eth_trie::EthTrie::new(Arc::new(MemoryDB::new(true)));
 
-        for (tx_index, txn) in transactions.into_iter().enumerate() {
+        for (tx_index, txn) in verified_txns.into_iter().enumerate() {
             self.new_transaction(txn.clone())?;
             let tx_hash = txn.hash;
             let mut inspector = TouchedAddressInspector::default();
@@ -2770,7 +2777,6 @@ impl Consensus {
 
         // Tell the block store to request more blocks if it can.
         self.block_store.request_missing_blocks()?;
-
         Ok(())
     }