From b8fb4f35cb1a5f3c2b5028d796204dd7aa4e4c33 Mon Sep 17 00:00:00 2001 From: mfrankovi Date: Mon, 16 Dec 2024 09:28:09 +0100 Subject: [PATCH] fix: handle transactions with no nonce increase --- .../core_libs/consensus/src/pbft/pbft_manager.cpp | 12 +++++++++++- .../src/transaction/transaction_manager.cpp | 7 ++----- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp index 178c3428b0..cb9a976d32 100644 --- a/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp +++ b/libraries/core_libs/consensus/src/pbft/pbft_manager.cpp @@ -1939,15 +1939,25 @@ std::optional>>> Pbf } auto non_finalized_transactions = trx_mgr_->excludeFinalizedTransactions(transactions_to_query); - if (non_finalized_transactions.size() != period_data.transactions.size()) { + 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(); + sync_queue_.clear(); 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())) { + const auto account = + final_chain_->getAccount(period_data.transactions[i]->getSender()).value_or(taraxa::state_api::ZeroAccount); + + // Check for transaction already included but without an increase in nonce + if (account.nonce < period_data.transactions[i]->getNonce()) { + LOG(log_nf_) << "Skipping duplicate"; + continue; + } + LOG(log_er_) << "Synced PBFT block " << pbft_block_hash << " has incorrect transaction " << period_data.transactions[i]->getHash(); 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 e5fd6efa6f..e2a8a409fe 100644 --- a/libraries/core_libs/consensus/src/transaction/transaction_manager.cpp +++ b/libraries/core_libs/consensus/src/transaction/transaction_manager.cpp @@ -196,13 +196,10 @@ void TransactionManager::saveTransactionsFromDagBlock(SharedTransactions const & std::unique_lock transactions_lock(transactions_mutex_); for (auto t : trxs) { - const auto account = final_chain_->getAccount(t->getSender()).value_or(taraxa::state_api::ZeroAccount); const auto tx_hash = t->getHash(); - // Checking nonce in cheaper than checking db, verify with nonce if possible - bool trx_not_executed = account.nonce < t->getNonce() || !db_->transactionFinalized(tx_hash); - - if (trx_not_executed) { + if (!recently_finalized_transactions_.contains(tx_hash) && !nonfinalized_transactions_in_dag_.contains(tx_hash) && + !db_->transactionFinalized(tx_hash)) { if (!recently_finalized_transactions_.contains(tx_hash) && !nonfinalized_transactions_in_dag_.contains(tx_hash)) { db_->addTransactionToBatch(*t, write_batch);