various changes to runtime checkpoint updating

json checkpoints will be checked every 10 minutes, dns every 60.

json checkpoints always enforced, dns still with flag.

conflicting checkpoints is hard fail, but soft if dns enforce flag not
set and dns checkpoints are wonky.

7 files changed, 113 insertions, 23 deletions

diff --git a/src/cryptonote_core/blockchain_storage.cpp b/src/cryptonote_core/blockchain_storage.cpp
index 1165a9035..6f1b4121c 100644
--- a/src/cryptonote_core/blockchain_storage.cpp
+++ b/src/cryptonote_core/blockchain_storage.cpp
@@ -1782,16 +1782,11 @@ bool blockchain_storage::add_new_block(const block& bl_, block_verification_cont
   return handle_block_to_main_chain(bl, id, bvc);
 }
 //------------------------------------------------------------------
-bool blockchain_storage::update_checkpoints(const std::string& file_path)
+void blockchain_storage::check_against_checkpoints(checkpoints& points, bool enforce)
 {
-  if (!cryptonote::load_new_checkpoints(m_checkpoints, file_path))
-  {
-    return false;
-  }
-
-  const auto& points = m_checkpoints.get_points();
+  const auto& pts = points.get_points();
 
-  for (const auto& pt : points)
+  for (const auto& pt : pts)
   {
     // if the checkpoint is for a block we don't have yet, move on
     if (pt.first >= m_blocks.size())
@@ -1801,8 +1796,8 @@ bool blockchain_storage::update_checkpoints(const std::string& file_path)
 
     if (!m_checkpoints.check_block(pt.first, get_block_hash(m_blocks[pt.first].bl)))
     {
-      // if we're enforcing dns checkpoints, roll back to a couple of blocks before the checkpoint
-      if (m_enforce_dns_checkpoints)
+      // if asked to enforce checkpoints, roll back to a couple of blocks before the checkpoint
+      if (enforce)
       {
 	LOG_ERROR("Checkpoint failed when adding new checkpoints, rolling back!");
 	std::list<block> empty;
@@ -1814,8 +1809,46 @@ bool blockchain_storage::update_checkpoints(const std::string& file_path)
       }
     }
   }
+}
+//------------------------------------------------------------------
+// returns false if any of the checkpoints loading returns false.
+// That should happen only if a checkpoint is added that conflicts
+// with an existing checkpoint.
+bool blockchain_storage::update_checkpoints(const std::string& file_path, bool check_dns)
+{
+  if (!cryptonote::load_checkpoints_from_json(m_checkpoints, file_path))
+  {
+    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 (!cryptonote::load_checkpoints_from_dns(m_checkpoints))
+    {
+      return false;
+    }
+  }
+  else if (check_dns)
+  {
+    checkpoints dns_points;
+    cryptonote::load_checkpoints_from_dns(dns_points);
+    if (m_checkpoints.check_for_conflicts(dns_points))
+    {
+      check_against_checkpoints(dns_points, false);
+    }
+    else
+    {
+      LOG_PRINT_L0("One or more checkpoints fetched from DNS conflicted with existing checkpoints!");
+    }
+  }
+
+  check_against_checkpoints(m_checkpoints, true);
+
   return true;
 }
+//------------------------------------------------------------------
 void blockchain_storage::set_enforce_dns_checkpoints(bool enforce_checkpoints)
 {
   m_enforce_dns_checkpoints = enforce_checkpoints;
diff --git a/src/cryptonote_core/blockchain_storage.h b/src/cryptonote_core/blockchain_storage.h
index 1ab49a7bc..08c007a4a 100644
--- a/src/cryptonote_core/blockchain_storage.h
+++ b/src/cryptonote_core/blockchain_storage.h
@@ -180,7 +180,8 @@ namespace cryptonote
     void print_blockchain(uint64_t start_index, uint64_t end_index);
     void print_blockchain_index();
     void print_blockchain_outs(const std::string& file);
-    bool update_checkpoints(const std::string& file_path);
+    void check_against_checkpoints(checkpoints& points, bool enforce);
+    bool update_checkpoints(const std::string& file_path, bool check_dns);
     void set_enforce_dns_checkpoints(bool enforce_checkpoints);
 
   private:
diff --git a/src/cryptonote_core/checkpoints.cpp b/src/cryptonote_core/checkpoints.cpp
index 41b41fac5..25759792b 100644
--- a/src/cryptonote_core/checkpoints.cpp
+++ b/src/cryptonote_core/checkpoints.cpp
@@ -113,4 +113,15 @@ namespace cryptonote
     return m_points;
   }
 
+  bool checkpoints::check_for_conflicts(checkpoints& other)
+  {
+    for (auto& pt : other.get_points())
+    {
+      if (m_points.count(pt.first))
+      {
+        CHECK_AND_ASSERT_MES(pt.second == m_points[pt.first], false, "Checkpoint at given height already exists, and hash for new checkpoint was different!");
+      }
+    }
+    return true;
+  }
 }
diff --git a/src/cryptonote_core/checkpoints.h b/src/cryptonote_core/checkpoints.h
index 60147864d..132917228 100644
--- a/src/cryptonote_core/checkpoints.h
+++ b/src/cryptonote_core/checkpoints.h
@@ -47,6 +47,7 @@ namespace cryptonote
     bool is_alternative_block_allowed(uint64_t blockchain_height, uint64_t block_height) const;
     uint64_t get_max_height();
     const std::map<uint64_t, crypto::hash>& get_points();
+    bool check_for_conflicts(checkpoints& other);
   private:
     std::map<uint64_t, crypto::hash> m_points;
   };
diff --git a/src/cryptonote_core/checkpoints_create.cpp b/src/cryptonote_core/checkpoints_create.cpp
index b81353539..808bc46a7 100644
--- a/src/cryptonote_core/checkpoints_create.cpp
+++ b/src/cryptonote_core/checkpoints_create.cpp
@@ -32,6 +32,7 @@
 #include "common/dns_utils.h"
 #include "include_base_utils.h"
 #include <sstream>
+#include <random>
 #include "storages/portable_storage_template_helper.h" // epee json include
 
 namespace cryptonote
@@ -110,15 +111,47 @@ bool load_checkpoints_from_json(cryptonote::checkpoints& checkpoints, std::strin
   return true;
 }
 
-bool load_checkpoints_from_dns(cryptonote::checkpoints& checkpoints, const std::string& url)
+bool load_checkpoints_from_dns(cryptonote::checkpoints& checkpoints)
 {
+  static const std::vector<std::string> dns_urls = { "checkpoints.moneropulse.se"
+						   , "checkpoints.moneropulse.org"
+						   , "checkpoints.moneropulse.net"
+						   , "checkpoints.moneropulse.co"
+  };
   bool avail, valid;
-  auto records = tools::DNSResolver::instance().get_txt_record(url, avail, valid);
+  std::vector<std::string> records;
+
+  std::random_device rd;
+  std::mt19937 gen(rd());
+  std::uniform_int_distribution<int> dis(0, dns_urls.size() - 1);
+  int first_index = dis(gen);
+
+  int cur_index = first_index;
+  do
+  {
+    records = tools::DNSResolver::instance().get_txt_record(dns_urls[cur_index], avail, valid);
+    if (records.size() == 0 || (avail && !valid))
+    {
+      cur_index++;
+      if (cur_index == dns_urls.size())
+      {
+	cur_index = 0;
+      }
+      continue;
+    }
+    break;
+  } while (cur_index != first_index);
+
+  if (records.size() == 0)
+  {
+    LOG_PRINT_L1("Fetching checkpoints from DNS TXT records failed, no TXT records available.");
+    return true;
+  }
 
   if (avail && !valid)
   {
-    LOG_ERROR("DNSSEC present and failed validation for query to" << url);
-    return false;
+    LOG_PRINT_L0("DNSSEC present and failed validation for query last url, and all other urls either failed validation or returned no records");
+    return true;
   }
 
   for (auto& record : records)
@@ -154,7 +187,7 @@ bool load_checkpoints_from_dns(cryptonote::checkpoints& checkpoints, const std::
 bool load_new_checkpoints(cryptonote::checkpoints& checkpoints, std::string json_hashfile_fullpath)
 {
   // TODO: replace hard-coded url with const string or #define
-  return (load_checkpoints_from_json(checkpoints, json_hashfile_fullpath) && load_checkpoints_from_dns(checkpoints, "checkpoints.moneropulse.org"));
+  return (load_checkpoints_from_json(checkpoints, json_hashfile_fullpath) && load_checkpoints_from_dns(checkpoints));
 }
 
 }  // namespace cryptonote
diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp
index d14be252b..c83e6e0a0 100644
--- a/src/cryptonote_core/cryptonote_core.cpp
+++ b/src/cryptonote_core/cryptonote_core.cpp
@@ -56,7 +56,8 @@ namespace cryptonote
               m_starter_message_showed(false),
               m_target_blockchain_height(0),
               m_checkpoints_path(""),
-              m_last_checkpoints_update(0)
+              m_last_dns_checkpoints_update(0),
+              m_last_json_checkpoints_update(0)
   {
     set_cryptonote_protocol(pprotocol);
   }
@@ -86,10 +87,16 @@ namespace cryptonote
   bool core::update_checkpoints()
   {
     bool res = true;
-    if (time(NULL) - m_last_checkpoints_update >= 3600)
+    if (time(NULL) - m_last_dns_checkpoints_update >= 3600)
     {
-      res = m_blockchain_storage.update_checkpoints(m_checkpoints_path);
-      m_last_checkpoints_update = time(NULL);
+      res = m_blockchain_storage.update_checkpoints(m_checkpoints_path, true);
+      m_last_dns_checkpoints_update = time(NULL);
+      m_last_json_checkpoints_update = time(NULL);
+    }
+    else if (time(NULL) - m_last_json_checkpoints_update >= 600)
+    {
+      res = m_blockchain_storage.update_checkpoints(m_checkpoints_path, false);
+      m_last_json_checkpoints_update = time(NULL);
     }
     return res;
   }
@@ -152,7 +159,7 @@ namespace cryptonote
 
     // load json & DNS checkpoints, and verify them
     // with respect to what blocks we already have
-    update_checkpoints();
+    CHECK_AND_ASSERT_MES(update_checkpoints(), false, "One or more checkpoints loaded from json or dns conflicted with existing checkpoints.");
 
     r = m_miner.init(vm, testnet);
     CHECK_AND_ASSERT_MES(r, false, "Failed to initialize blockchain storage");
@@ -445,7 +452,10 @@ namespace cryptonote
   //-----------------------------------------------------------------------------------------------
   bool core::handle_incoming_block(const blobdata& block_blob, block_verification_context& bvc, bool update_miner_blocktemplate)
   {
-    update_checkpoints();
+    // load json & DNS checkpoints every 10min/hour respectively,
+    // and verify them with respect to what blocks we already have
+    CHECK_AND_ASSERT_MES(update_checkpoints(), false, "One or more checkpoints loaded from json or dns conflicted with existing checkpoints.");
+
     bvc = boost::value_initialized<block_verification_context>();
     if(block_blob.size() > get_max_block_size())
     {
diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h
index b3bfddff3..0c697df80 100644
--- a/src/cryptonote_core/cryptonote_core.h
+++ b/src/cryptonote_core/cryptonote_core.h
@@ -163,7 +163,8 @@ namespace cryptonote
      uint64_t m_target_blockchain_height;
 
      std::string m_checkpoints_path;
-     time_t m_last_checkpoints_update;
+     time_t m_last_dns_checkpoints_update;
+     time_t m_last_json_checkpoints_update;
    };
 }