Merge pull request #767

24b3e90 Convey tx verification failure reasons to the RPC client (moneromooo-monero)

5 files changed, 39 insertions, 11 deletions

diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp
index da14d7575..34810d983 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -1991,7 +1991,7 @@ bool Blockchain::get_tx_outputs_gindexs(const crypto::hash& tx_id, std::vector<u
 // This function overloads its sister function with
 // an extra value (hash of highest block that holds an output used as input)
 // as a return-by-reference.
-bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t& max_used_block_height, crypto::hash& max_used_block_id, bool kept_by_block)
+bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t& max_used_block_height, crypto::hash& max_used_block_id, tx_verification_context &tvc, bool kept_by_block)
 {
   LOG_PRINT_L3("Blockchain::" << __func__);
   CRITICAL_REGION_LOCAL(m_blockchain_lock);
@@ -2013,7 +2013,7 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t& max_used_block
 #endif
 
   TIME_MEASURE_START(a);
-  bool res = check_tx_inputs(tx, &max_used_block_height);
+  bool res = check_tx_inputs(tx, tvc, &max_used_block_height);
   TIME_MEASURE_FINISH(a);
   crypto::hash tx_prefix_hash = get_transaction_prefix_hash(tx);
   if(m_show_time_stats)
@@ -2032,7 +2032,7 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t& max_used_block
   return true;
 }
 //------------------------------------------------------------------
-bool Blockchain::check_tx_outputs(const transaction& tx)
+bool Blockchain::check_tx_outputs(const transaction& tx, tx_verification_context &tvc)
 {
   LOG_PRINT_L3("Blockchain::" << __func__);
   CRITICAL_REGION_LOCAL(m_blockchain_lock);
@@ -2041,6 +2041,7 @@ bool Blockchain::check_tx_outputs(const transaction& tx)
   if (m_hardfork->get_current_version() >= 2) {
     for (auto &o: tx.vout) {
       if (!is_valid_decomposed_amount(o.amount)) {
+        tvc.m_invalid_output = true;
         return false;
       }
     }
@@ -2066,7 +2067,7 @@ bool Blockchain::have_tx_keyimges_as_spent(const transaction &tx) const
 //        check_tx_input() rather than here, and use this function simply
 //        to iterate the inputs as necessary (splitting the task
 //        using threads, etc.)
-bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_block_height)
+bool Blockchain::check_tx_inputs(const transaction& tx, tx_verification_context &tvc, uint64_t* pmax_used_block_height)
 {
   LOG_PRINT_L3("Blockchain::" << __func__);
   size_t sig_index = 0;
@@ -2113,11 +2114,13 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc
       if (n_unmixable == 0)
       {
         LOG_PRINT_L1("Tx " << get_transaction_hash(tx) << " has too low mixin (" << mixin << "), and no unmixable inputs");
+        tvc.m_low_mixin = true;
         return false;
       }
       if (n_mixable > 1)
       {
         LOG_PRINT_L1("Tx " << get_transaction_hash(tx) << " has too low mixin (" << mixin << "), and more than one mixable input with unmixable inputs");
+        tvc.m_low_mixin = true;
         return false;
       }
     }
@@ -2176,6 +2179,7 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t* pmax_used_bloc
     if(have_tx_keyimg_as_spent(in_to_key.k_image))
     {
       LOG_PRINT_L1("Key image already spent in blockchain: " << epee::string_tools::pod_to_hex(in_to_key.k_image));
+      tvc.m_double_spend = true;
       return false;
     }
 
@@ -2667,7 +2671,8 @@ leave:
 #endif
     {
       // validate that transaction inputs and the keys spending them are correct.
-      if(!check_tx_inputs(tx))
+      tx_verification_context tvc;
+      if(!check_tx_inputs(tx, tvc))
       {
         LOG_PRINT_L1("Block with id: " << id  << " has at least one transaction (id: " << tx_id << ") with wrong inputs.");
 
diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h
index 6bae0364d..a62487d1e 100644
--- a/src/cryptonote_core/blockchain.h
+++ b/src/cryptonote_core/blockchain.h
@@ -472,11 +472,12 @@ namespace cryptonote
      * @param tx the transaction to validate
      * @param pmax_used_block_height return-by-reference block height of most recent input
      * @param max_used_block_id return-by-reference block hash of most recent input
+     * @param tvc returned information about tx verification
      * @param kept_by_block whether or not the transaction is from a previously-verified block
      *
      * @return false if any input is invalid, otherwise true
      */
-    bool check_tx_inputs(const transaction& tx, uint64_t& pmax_used_block_height, crypto::hash& max_used_block_id, bool kept_by_block = false);
+    bool check_tx_inputs(const transaction& tx, uint64_t& pmax_used_block_height, crypto::hash& max_used_block_id, tx_verification_context &tvc, bool kept_by_block = false);
 
     /**
      * @brief check that a transaction's outputs conform to current standards
@@ -486,10 +487,11 @@ namespace cryptonote
      * written out would have only one non-zero digit in base 10).
      *
      * @param tx the transaction to check the outputs of
+     * @param tvc returned info about tx verification
      *
      * @return false if any outputs do not conform, otherwise true
      */
-    bool check_tx_outputs(const transaction& tx);
+    bool check_tx_outputs(const transaction& tx, tx_verification_context &tvc);
 
     /**
      * @brief gets the blocksize limit based on recent blocks
@@ -883,11 +885,12 @@ namespace cryptonote
      * transaction.
      *
      * @param tx the transaction to validate
+     * @param tvc returned information about tx verification
      * @param pmax_related_block_height return-by-pointer the height of the most recent block in the input set
      *
      * @return false if any validation step fails, otherwise true
      */
-    bool check_tx_inputs(const transaction& tx, uint64_t* pmax_used_block_height = NULL);
+    bool check_tx_inputs(const transaction& tx, tx_verification_context &tvc, uint64_t* pmax_used_block_height = NULL);
 
     /**
      * @brief performs a blockchain reorganization according to the longest chain rule
diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp
index c31be5acf..20b9f0b0b 100644
--- a/src/cryptonote_core/cryptonote_core.cpp
+++ b/src/cryptonote_core/cryptonote_core.cpp
@@ -489,6 +489,7 @@ namespace cryptonote
     {
       LOG_PRINT_L1("WRONG TRANSACTION BLOB, too big size " << tx_blob.size() << ", rejected");
       tvc.m_verifivation_failed = true;
+      tvc.m_too_big = true;
       return false;
     }
 
diff --git a/src/cryptonote_core/tx_pool.cpp b/src/cryptonote_core/tx_pool.cpp
index 49fa4aa57..a06826163 100644
--- a/src/cryptonote_core/tx_pool.cpp
+++ b/src/cryptonote_core/tx_pool.cpp
@@ -102,6 +102,7 @@ namespace cryptonote
     if(!check_inputs_types_supported(tx))
     {
       tvc.m_verifivation_failed = true;
+      tvc.m_invalid_input = true;
       return false;
     }
 
@@ -118,6 +119,7 @@ namespace cryptonote
     {
       LOG_PRINT_L1("transaction use more money then it has: use " << print_money(outputs_amount) << ", have " << print_money(inputs_amount));
       tvc.m_verifivation_failed = true;
+      tvc.m_overspend = true;
       return false;
     }
 
@@ -130,6 +132,7 @@ namespace cryptonote
     {
       LOG_PRINT_L1("transaction fee is not enough: " << print_money(fee) << ", minimum fee: " << print_money(needed_fee));
       tvc.m_verifivation_failed = true;
+      tvc.m_fee_too_low = true;
       return false;
     }
 
@@ -138,6 +141,7 @@ namespace cryptonote
     {
       LOG_PRINT_L1("transaction is too big: " << blob_size << " bytes, maximum size: " << tx_size_limit);
       tvc.m_verifivation_failed = true;
+      tvc.m_too_big = true;
       return false;
     }
 
@@ -150,21 +154,23 @@ namespace cryptonote
       {
         LOG_PRINT_L1("Transaction with id= "<< id << " used already spent key images");
         tvc.m_verifivation_failed = true;
+        tvc.m_double_spend = true;
         return false;
       }
     }
 
-    if (!m_blockchain.check_tx_outputs(tx))
+    if (!m_blockchain.check_tx_outputs(tx, tvc))
     {
       LOG_PRINT_L1("Transaction with id= "<< id << " has at least one invalid outout");
       tvc.m_verifivation_failed = true;
+      tvc.m_invalid_output = true;
       return false;
     }
 
     crypto::hash max_used_block_id = null_hash;
     uint64_t max_used_block_height = 0;
 #if BLOCKCHAIN_DB == DB_LMDB
-    bool ch_inp_res = m_blockchain.check_tx_inputs(tx, max_used_block_height, max_used_block_id, kept_by_block);
+    bool ch_inp_res = m_blockchain.check_tx_inputs(tx, max_used_block_height, max_used_block_id, tvc, kept_by_block);
 #else
     bool ch_inp_res = m_blockchain.check_tx_inputs(tx, max_used_block_height, max_used_block_id);
 #endif
@@ -495,7 +501,8 @@ namespace cryptonote
       if(txd.last_failed_id != null_hash && m_blockchain.get_current_blockchain_height() > txd.last_failed_height && txd.last_failed_id == m_blockchain.get_block_id_by_height(txd.last_failed_height))
         return false;//we already sure that this tx is broken for this height
 
-      if(!m_blockchain.check_tx_inputs(txd.tx, txd.max_used_block_height, txd.max_used_block_id))
+      tx_verification_context tvc;
+      if(!m_blockchain.check_tx_inputs(txd.tx, txd.max_used_block_height, txd.max_used_block_id, tvc))
       {
         txd.last_failed_height = m_blockchain.get_current_blockchain_height()-1;
         txd.last_failed_id = m_blockchain.get_block_id_by_height(txd.last_failed_height);
@@ -511,7 +518,12 @@ namespace cryptonote
         if(txd.last_failed_id == m_blockchain.get_block_id_by_height(txd.last_failed_height))
           return false;
         //check ring signature again, it is possible (with very small chance) that this transaction become again valid
+#if BLOCKCHAIN_DB == DB_LMDB
+        tx_verification_context tvc;
+        if(!m_blockchain.check_tx_inputs(txd.tx, txd.max_used_block_height, txd.max_used_block_id, tvc))
+#else
         if(!m_blockchain.check_tx_inputs(txd.tx, txd.max_used_block_height, txd.max_used_block_id))
+#endif
         {
           txd.last_failed_height = m_blockchain.get_current_blockchain_height()-1;
           txd.last_failed_id = m_blockchain.get_block_id_by_height(txd.last_failed_height);
diff --git a/src/cryptonote_core/verification_context.h b/src/cryptonote_core/verification_context.h
index fcfd2a3e2..e58291ea9 100644
--- a/src/cryptonote_core/verification_context.h
+++ b/src/cryptonote_core/verification_context.h
@@ -40,6 +40,13 @@ namespace cryptonote
     bool m_verifivation_failed; //bad tx, should drop connection
     bool m_verifivation_impossible; //the transaction is related with an alternative blockchain
     bool m_added_to_pool; 
+    bool m_low_mixin;
+    bool m_double_spend;
+    bool m_invalid_input;
+    bool m_invalid_output;
+    bool m_too_big;
+    bool m_overspend;
+    bool m_fee_too_low;
   };
 
   struct block_verification_context