From 8f9ad124558b26a2502affc0e181a51dd5ea492a Mon Sep 17 00:00:00 2001 From: DUQUEredes Date: Mon, 23 Feb 2026 11:42:30 +0100 Subject: [PATCH] tx_pool: fix early return in remove_transaction_keyimages Replace CHECK_AND_ASSERT_MES macros with explicit error checks that continue processing remaining inputs instead of returning early. The previous behavior could leave orphaned key images in m_spent_key_images when an error was encountered partway through the input list, since callers already deleted the transaction from the pool before calling this function. Track success via an all_found flag so the return value still indicates whether all key images were found and removed. Also clean up empty key_image sets instead of just failing on them. Resolves the FIXME at the top of the function that noted: "Should probably not return early, however." --- src/cryptonote_core/tx_pool.cpp | 49 +++++++++++++++++++++++---------- 1 file changed, 34 insertions(+), 15 deletions(-) diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp index d6824049c98..ab69c7c7a64 100644 --- a/src/cryptonote_core/tx_pool.cpp +++ b/src/cryptonote_core/tx_pool.cpp @@ -501,37 +501,56 @@ namespace cryptonote return true; } //--------------------------------------------------------------------------------- - //FIXME: Can return early before removal of all of the key images. - // At the least, need to make sure that a false return here - // is treated properly. Should probably not return early, however. bool tx_memory_pool::remove_transaction_keyimages(const transaction_prefix& tx, const crypto::hash &actual_hash) { CRITICAL_REGION_LOCAL(m_transactions_lock); CRITICAL_REGION_LOCAL1(m_blockchain); // ND: Speedup + bool all_found = true; for(const txin_v& vi: tx.vin) { - CHECKED_GET_SPECIFIC_VARIANT(vi, const txin_to_key, txin, false); - auto it = m_spent_key_images.find(txin.k_image); - CHECK_AND_ASSERT_MES(it != m_spent_key_images.end(), false, "failed to find transaction input in key images. img=" << txin.k_image << ENDL - << "transaction id = " << actual_hash); - std::unordered_set& key_image_set = it->second; - CHECK_AND_ASSERT_MES(key_image_set.size(), false, "empty key_image set, img=" << txin.k_image << ENDL - << "transaction id = " << actual_hash); + const txin_to_key *txin = boost::get(&vi); + if (nullptr == txin) + { + MERROR("unexpected input variant type in remove_transaction_keyimages, transaction id = " << actual_hash); + all_found = false; + continue; + } + auto it = m_spent_key_images.find(txin->k_image); + if(it == m_spent_key_images.end()) + { + MERROR("failed to find transaction input in key images. img=" << txin->k_image << ENDL + << "transaction id = " << actual_hash); + all_found = false; + continue; + } + std::unordered_set& key_image_set = it->second; + if(key_image_set.empty()) + { + MERROR("empty key_image set, img=" << txin->k_image << ENDL + << "transaction id = " << actual_hash); + m_spent_key_images.erase(it); + all_found = false; + continue; + } auto it_in_set = key_image_set.find(actual_hash); - CHECK_AND_ASSERT_MES(it_in_set != key_image_set.end(), false, "transaction id not found in key_image set, img=" << txin.k_image << ENDL - << "transaction id = " << actual_hash); + if(it_in_set == key_image_set.end()) + { + MERROR("transaction id not found in key_image set, img=" << txin->k_image << ENDL + << "transaction id = " << actual_hash); + all_found = false; + continue; + } key_image_set.erase(it_in_set); - if(!key_image_set.size()) + if(key_image_set.empty()) { //it is now empty hash container for this key_image m_spent_key_images.erase(it); } - } ++m_cookie; - return true; + return all_found; } //--------------------------------------------------------------------------------- bool tx_memory_pool::take_tx(const crypto::hash &id,