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."

Author: DUQUEredes

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<crypto::hash>& 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<txin_to_key>(&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<crypto::hash>& 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,