Revert "Merge pull request #749"

This reverts commit 7fa63a82a1c3a0243f6757c1689855ed3ca61695, reversing
changes made to cb6be986c36b78eddb4b7f16e9ad440af8567dc4.

1 files changed, 10 insertions, 39 deletions

diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp
index 2f42e1db5..131f56a4d 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -49,7 +49,7 @@
 #include "common/boost_serialization_helper.h"
 #include "warnings.h"
 #include "crypto/hash.h"
-#include "cryptonote_core/checkpoints.h"
+#include "cryptonote_core/checkpoints_create.h"
 #include "cryptonote_core/cryptonote_core.h"
 #if defined(PER_BLOCK_CHECKPOINT)
 #include "blocks/blocks.h"
@@ -840,7 +840,7 @@ bool Blockchain::switch_to_alternative_blockchain(std::list<blocks_ext_by_hash::
     }
   }
 
-  //removing alt_chain entries from alternative chains container
+  //removing alt_chain entries from alternative chain
   for (auto ch_ent: alt_chain)
   {
     m_alternative_chains.erase(ch_ent);
@@ -991,7 +991,8 @@ bool Blockchain::validate_miner_transaction(const block& b, size_t cumulative_bl
   return true;
 }
 //------------------------------------------------------------------
-// get the block sizes of the last <count> blocks, and return by reference <sz>.
+// get the block sizes of the last <count> blocks, starting at <from_height>
+// and return by reference <sz>.
 void Blockchain::get_last_n_blocks_sizes(std::vector<size_t>& sz, size_t count) const
 {
   LOG_PRINT_L3("Blockchain::" << __func__);
@@ -1407,10 +1408,6 @@ bool Blockchain::get_blocks(uint64_t start_offset, size_t count, std::list<block
 //TODO: This function *looks* like it won't need to be rewritten
 //      to use BlockchainDB, as it calls other functions that were,
 //      but it warrants some looking into later.
-//
-//FIXME: This function appears to want to return false if any transactions
-//       that belong with blocks are missing, but not if blocks themselves
-//       are missing.
 bool Blockchain::handle_get_objects(NOTIFY_REQUEST_GET_OBJECTS::request& arg, NOTIFY_RESPONSE_GET_OBJECTS::request& rsp)
 {
   LOG_PRINT_L3("Blockchain::" << __func__);
@@ -1424,9 +1421,6 @@ bool Blockchain::handle_get_objects(NOTIFY_REQUEST_GET_OBJECTS::request& arg, NO
   {
     std::list<crypto::hash> missed_tx_ids;
     std::list<transaction> txs;
-
-    // FIXME: s/rsp.missed_ids/missed_tx_id/ ?  Seems like rsp.missed_ids
-    //        is for missed blocks, not missed transactions as well.
     get_transactions(bl.tx_hashes, txs, missed_tx_ids);
 
     if (missed_tx_ids.size() != 0)
@@ -1674,8 +1668,6 @@ uint64_t Blockchain::block_difficulty(uint64_t i) const
   return 0;
 }
 //------------------------------------------------------------------
-//TODO: return type should be void, throw on exception
-//       alternatively, return true only if no blocks missed
 template<class t_ids_container, class t_blocks_container, class t_missed_container>
 bool Blockchain::get_blocks(const t_ids_container& block_ids, t_blocks_container& blocks, t_missed_container& missed_bs) const
 {
@@ -1700,8 +1692,6 @@ bool Blockchain::get_blocks(const t_ids_container& block_ids, t_blocks_container
   return true;
 }
 //------------------------------------------------------------------
-//TODO: return type should be void, throw on exception
-//       alternatively, return true only if no transactions missed
 template<class t_ids_container, class t_tx_container, class t_missed_container>
 bool Blockchain::get_transactions(const t_ids_container& txs_ids, t_tx_container& txs, t_missed_container& missed_txs) const
 {
@@ -1718,6 +1708,7 @@ bool Blockchain::get_transactions(const t_ids_container& txs_ids, t_tx_container
     {
       missed_txs.push_back(tx_hash);
     }
+    //FIXME: is this the correct way to handle this?
     catch (const std::exception& e)
     {
       return false;
@@ -1974,11 +1965,6 @@ bool Blockchain::get_tx_outputs_gindexs(const crypto::hash& tx_id, std::vector<u
   return true;
 }
 //------------------------------------------------------------------
-//FIXME: it seems this function is meant to be merely a wrapper around
-//       another function of the same name, this one adding one bit of
-//       functionality.  Should probably move anything more than that
-//       (getting the hash of the block at height max_used_block_id)
-//       to the other function to keep everything in one place.
 // 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.
@@ -1989,7 +1975,6 @@ bool Blockchain::check_tx_inputs(const transaction& tx, uint64_t& max_used_block
 
 #if defined(PER_BLOCK_CHECKPOINT)
   // check if we're doing per-block checkpointing
-  // FIXME: investigate why this block returns
   if (m_db->height() < m_blocks_hash_check.size() && kept_by_block)
   {
     TIME_MEASURE_START(a);
@@ -2053,10 +2038,6 @@ bool Blockchain::have_tx_keyimges_as_spent(const transaction &tx) const
 }
 //------------------------------------------------------------------
 // This function validates transaction inputs and their keys.
-// FIXME: consider moving functionality specific to one input into
-//        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)
 {
   LOG_PRINT_L3("Blockchain::" << __func__);
@@ -2339,7 +2320,7 @@ bool Blockchain::check_tx_input(const txin_to_key& txin, const crypto::hash& tx_
 
   output_keys.clear();
 
-  // collect output keys
+  //check ring signature
   outputs_visitor vi(output_keys, *this);
   if (!scan_outputkeys_for_indexes(txin, vi, tx_prefix_hash, pmax_related_block_height))
   {
@@ -2636,11 +2617,6 @@ leave:
     txs.push_back(tx);
     TIME_MEASURE_START(dd);
 
-    // FIXME: the storage should not be responsible for validation.
-    //        If it does any, it is merely a sanity check.
-    //        Validation is the purview of the Blockchain class
-    //        - TW
-    //
     // ND: this is not needed, db->add_block() checks for duplicate k_images and fails accordingly.
     // if (!check_for_double_spend(tx, keys))
     // {
@@ -2821,8 +2797,6 @@ bool Blockchain::add_new_block(const block& bl_, block_verification_context& bvc
   return handle_block_to_main_chain(bl, id, bvc);
 }
 //------------------------------------------------------------------
-//TODO: Refactor, consider returning a failure height and letting
-//      caller decide course of action.
 void Blockchain::check_against_checkpoints(const checkpoints& points, bool enforce)
 {
   const auto& pts = points.get_points();
@@ -2860,16 +2834,16 @@ void Blockchain::check_against_checkpoints(const checkpoints& points, bool enfor
 // with an existing checkpoint.
 bool Blockchain::update_checkpoints(const std::string& file_path, bool check_dns)
 {
-  if (!m_checkpoints.load_checkpoints_from_json(file_path))
+  if (!cryptonote::load_checkpoints_from_json(m_checkpoints, file_path))
   {
-      return false;
+    return false;
   }
 
   // if we're checking both dns and json, load checkpoints from dns.
   // if we're not hard-enforcing dns checkpoints, handle accordingly
   if (m_enforce_dns_checkpoints && check_dns)
   {
-    if (!m_checkpoints.load_checkpoints_from_dns())
+    if (!cryptonote::load_checkpoints_from_dns(m_checkpoints))
     {
       return false;
     }
@@ -2877,7 +2851,7 @@ bool Blockchain::update_checkpoints(const std::string& file_path, bool check_dns
   else if (check_dns)
   {
     checkpoints dns_points;
-    dns_points.load_checkpoints_from_dns();
+    cryptonote::load_checkpoints_from_dns(dns_points);
     if (m_checkpoints.check_for_conflicts(dns_points))
     {
       check_against_checkpoints(dns_points, false);
@@ -2904,8 +2878,6 @@ void Blockchain::block_longhash_worker(const uint64_t height, const std::vector<
   TIME_MEASURE_START(t);
   slow_hash_allocate_state();
 
-  //FIXME: height should be changing here, as get_block_longhash expects
-  //       the height of the block passed to it
   for (const auto & block : blocks)
   {
     crypto::hash id = get_block_hash(block);
@@ -2961,7 +2933,6 @@ bool Blockchain::cleanup_handle_incoming_blocks(bool force_sync)
 }
 
 //------------------------------------------------------------------
-//FIXME: unused parameter txs
 void Blockchain::output_scan_worker(const uint64_t amount, const std::vector<uint64_t> &offsets, std::vector<output_data_t> &outputs, std::unordered_map<crypto::hash, cryptonote::transaction> &txs) const
 {
   try