From 00ee28cb2886f98bf1b6d27004de847129a9b684 Mon Sep 17 00:00:00 2001
From: mfrankovi <miho.frankovic@gmail.com>
Date: Thu, 30 Jan 2025 09:01:29 +0100
Subject: [PATCH] chore: transaction handling optimization

---
 .../consensus/src/pbft/pbft_manager.cpp       | 23 +++++++++++--------
 .../src/transaction/transaction_manager.cpp   | 18 +++++++++++----
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp
index 0def4241c4..389b29090d 100644
--- a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp
+++ b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp
@@ -1939,6 +1939,20 @@ std::optional<std::pair<PeriodData, std::vector<std::shared_ptr<PbftVote>>>> Pbf
   }
   auto non_finalized_transactions = trx_mgr_->excludeFinalizedTransactions(transactions_to_query);
 
+  for (uint32_t i = 0; i < period_data.transactions.size(); i++) {
+    if (!non_finalized_transactions.contains(period_data.transactions[i]->getHash())) {
+      if(db_->transactionFinalized(period_data.transactions[i]->getHash())) {
+        non_finalized_transactions.erase(period_data.transactions[i]->getHash());
+      } else {
+        LOG(log_er_) << "Synced PBFT block " << pbft_block_hash << " has incorrect transaction "
+                  << period_data.transactions[i]->getHash();
+        sync_queue_.clear();
+        net->handleMaliciousSyncPeer(node_id);
+        return std::nullopt;
+      }
+    }
+  }
+
   if (non_finalized_transactions.size() != period_data.transactions.size()) {
     LOG(log_er_) << "Synced PBFT block " << pbft_block_hash << " transactions count " << period_data.transactions.size()
                  << " incorrect, expected: " << non_finalized_transactions.size();
@@ -1946,15 +1960,6 @@ std::optional<std::pair<PeriodData, std::vector<std::shared_ptr<PbftVote>>>> Pbf
     net->handleMaliciousSyncPeer(node_id);
     return std::nullopt;
   }
-  for (uint32_t i = 0; i < period_data.transactions.size(); i++) {
-    if (!non_finalized_transactions.contains(period_data.transactions[i]->getHash())) {
-      LOG(log_er_) << "Synced PBFT block " << pbft_block_hash << " has incorrect transaction "
-                   << period_data.transactions[i]->getHash();
-      sync_queue_.clear();
-      net->handleMaliciousSyncPeer(node_id);
-      return std::nullopt;
-    }
-  }
 
   if (!validatePillarDataInPeriodData(period_data)) {
     sync_queue_.clear();
diff --git a/libraries/core_libs/consensus/src/transaction/transaction_manager.cpp b/libraries/core_libs/consensus/src/transaction/transaction_manager.cpp
index fc4b78b9d7..d8138f21a8 100644
--- a/libraries/core_libs/consensus/src/transaction/transaction_manager.cpp
+++ b/libraries/core_libs/consensus/src/transaction/transaction_manager.cpp
@@ -205,8 +205,20 @@ void TransactionManager::saveTransactionsFromDagBlock(SharedTransactions const &
     for (auto t : trxs) {
       const auto tx_hash = t->getHash();
 
-      if (!recently_finalized_transactions_.contains(tx_hash) && !nonfinalized_transactions_in_dag_.contains(tx_hash) &&
-          !db_->transactionFinalized(tx_hash)) {
+      bool transaction_in_dag_or_finalized = nonfinalized_transactions_in_dag_.contains(tx_hash) || recently_finalized_transactions_.contains(tx_hash);
+      if(transaction_in_dag_or_finalized) {
+        continue;
+      }
+
+      // Checking nonce in cheaper than checking db, verify with nonce if possible
+      const auto account = final_chain_->getAccount(t->getSender()).value_or(taraxa::state_api::ZeroAccount);
+      if(account.nonce >= t->getNonce()) {
+        //This is a very rare scenario but it can happen:
+        //The check against database is needed because there is a possibility that transaction was executed within last 100 period (dag proposal period) but it might not be part of recently_finalized_transactions_
+        transaction_in_dag_or_finalized = db_->transactionFinalized(tx_hash);
+      }
+
+      if (!transaction_in_dag_or_finalized) {
         db_->addTransactionToBatch(*t, write_batch);
         nonfinalized_transactions_in_dag_.emplace(tx_hash, t);
         if (transactions_pool_.erase(tx_hash)) {
@@ -290,9 +302,7 @@ std::unordered_set<trx_hash_t> TransactionManager::excludeFinalizedTransactions(
   std::shared_lock transactions_lock(transactions_mutex_);
   for (const auto &hash : hashes) {
     if (!recently_finalized_transactions_.contains(hash)) {
-      if (!db_->transactionFinalized(hash)) {
         ret.insert(hash);
-      }
     }
   }
   return ret;