Merge pull request #901 from loki-project/dev

Merge v5.1.2 to master

src/cryptonote_core/blockchain.cpp

@@ -1712,20 +1712,39 @@ bool Blockchain::build_alt_chain(const crypto::hash &prev_id, std::list<block_ex
     // main chain -- that is, if we're adding on to an alternate chain
     if(!alt_chain.empty())
     {
+      bool failed = false;
       // make sure alt chain doesn't somehow start past the end of the main chain
-      CHECK_AND_ASSERT_MES(m_db->height() > alt_chain.front().height, false, "main blockchain wrong height");
+      if (m_db->height() < alt_chain.front().height)
+      {
+        LOG_PRINT_L1("main blockchain wrong height: " << m_db->height() << ", alt_chain: " << alt_chain.front().height);
+        failed = true;
+      }
 
       // make sure that the blockchain contains the block that should connect
       // this alternate chain with it.
-      if (!m_db->block_exists(alt_chain.front().bl.prev_id))
+      if (!failed && !m_db->block_exists(alt_chain.front().bl.prev_id))
       {
-        MERROR("alternate chain does not appear to connect to main chain...");
-        return false;
+        LOG_PRINT_L1("alternate chain does not appear to connect to main chain...: " << alt_chain.front().bl.prev_id);
+        failed = true;
       }
 
       // make sure block connects correctly to the main chain
       auto h = m_db->get_block_hash_from_height(alt_chain.front().height - 1);
-      CHECK_AND_ASSERT_MES(h == alt_chain.front().bl.prev_id, false, "alternative chain has wrong connection to main chain");
+      if (!failed && h != alt_chain.front().bl.prev_id)
+      {
+        LOG_PRINT_L1("alternative chain has wrong connection to main chain: " << h << ", mismatched with: " << alt_chain.front().bl.prev_id);
+        failed = true;
+      }
+
+      if (failed)
+      {
+        // Cleanup alt chain, it's invalid
+        for (auto const &bei : alt_chain)
+          m_db->remove_alt_block(cryptonote::get_block_hash(bei.bl));
+
+        return false;
+      }
+
       complete_timestamps_vector(m_db->get_block_height(alt_chain.front().bl.prev_id), timestamps);
     }
     // if block not associated with known alternate chain
@@ -5107,7 +5126,7 @@ void Blockchain::cancel()
 }
 
 #if defined(PER_BLOCK_CHECKPOINT)
-static const char expected_block_hashes_hash[] = "63b6445540c13f74d73fd753906e80bb84328c57b5a5a90c73353ed8405e7043";
+static const char expected_block_hashes_hash[] = "8754309c4501f4b1c547c5c14d41dca30e0836a2942b09584ccc43b287040d07";
 void Blockchain::load_compiled_in_block_hashes(const GetCheckpointsCallback& get_checkpoints)
 {
   if (get_checkpoints == nullptr || !m_fast_sync)

src/cryptonote_core/service_node_list.cpp

@@ -110,8 +110,9 @@ namespace service_nodes
     std::vector<std::pair<cryptonote::blobdata, cryptonote::block>> blocks;
     std::vector<cryptonote::transaction> txs;
     std::vector<crypto::hash> missed_txs;
-    auto work_start = std::chrono::high_resolution_clock::now();
-    for (uint64_t i = 0; m_state.height < current_height; i++)
+    auto work_start       = std::chrono::high_resolution_clock::now();
+    uint64_t start_height = m_state.height;
+    for (uint64_t i = 0; m_state.height < current_height; i++, start_height = m_state.height)
     {
       if (i > 0 && i % 10 == 0)
       {
@@ -145,6 +146,12 @@ namespace service_nodes
 
         process_block(block, txs);
       }
+
+      if (start_height == m_state.height)
+      {
+        MERROR("Unexpected state height did not change after processing blocks, height is: " << start_height);
+        return;
+      }
     }
 
     auto scan_end = std::chrono::high_resolution_clock::now();
@@ -153,14 +160,8 @@ namespace service_nodes
     if (store_to_disk) store();
   }
 
-  // TODO(loki): Temporary HF13 code, remove when we hit HF13 because we delete all HF12 checkpoints and don't need conditionals for HF12/HF13 checkpointing code
-  static uint64_t hf13_height;
-
   void service_node_list::init()
   {
-    // TODO(loki): Temporary HF13 code, remove when we hit HF13 because we delete all HF12 checkpoints and don't need conditionals for HF12/HF13 checkpointing code
-    hf13_height = m_blockchain.get_earliest_ideal_height_for_version(cryptonote::network_version_13_enforce_checkpoints);
-
     std::lock_guard<boost::recursive_mutex> lock(m_sn_mutex);
     if (m_blockchain.get_current_hard_fork_version() < 9)
     {
@@ -1448,7 +1449,7 @@ namespace service_nodes
     //
     // Cull alt state history
     //
-    if (hf_version >= cryptonote::network_version_12_checkpointing)
+    if (hf_version >= cryptonote::network_version_12_checkpointing && m_alt_state.size())
     {
       cryptonote::checkpoint_t immutable_checkpoint;
       if (m_db->get_immutable_checkpoint(&immutable_checkpoint, block_height))

src/cryptonote_core/tx_pool.cpp

@@ -155,7 +155,7 @@ namespace cryptonote
         tx_extra_service_node_state_change pool_tx_state_change;
         if (!get_service_node_state_change_from_tx_extra(pool_tx.extra, pool_tx_state_change, hard_fork_version))
         {
-          MERROR("Could not get service node state change from tx: " << get_transaction_hash(pool_tx) << ", possibly corrupt tx in the pool");
+          LOG_PRINT_L1("Could not get service node state change from tx: " << get_transaction_hash(pool_tx) << ", possibly corrupt tx in the pool");
           continue;
         }
 
@@ -201,7 +201,7 @@ namespace cryptonote
         tx_extra_tx_key_image_unlock pool_unlock;
         if (!cryptonote::get_tx_key_image_unlock_from_tx_extra(pool_tx.extra, pool_unlock))
         {
-          MERROR("Could not get key image unlock from tx: " << get_transaction_hash(tx) << ", possibly corrupt tx in the pool");
+          LOG_PRINT_L1("Could not get key image unlock from tx: " << get_transaction_hash(tx) << ", possibly corrupt tx in the pool");
           return true;
         }
 

src/daemon/rpc_command_executor.cpp

@@ -2446,7 +2446,7 @@ static std::string to_string_rounded(double d, int precision) {
   return ss.str();
 }
 
-static void append_printable_service_node_list_entry(cryptonote::network_type nettype, bool detailed_view, uint64_t curr_height, uint64_t entry_index, cryptonote::COMMAND_RPC_GET_SERVICE_NODES::response::entry const &entry, std::string &buffer)
+static void append_printable_service_node_list_entry(cryptonote::network_type nettype, bool detailed_view, uint64_t blockchain_height, uint64_t entry_index, cryptonote::COMMAND_RPC_GET_SERVICE_NODES::response::entry const &entry, std::string &buffer)
 {
   const char indent1[] = "    ";
   const char indent2[] = "        ";
@@ -2493,9 +2493,8 @@ static void append_printable_service_node_list_entry(cryptonote::network_type ne
     }
     else
     {
-      uint64_t delta_height      = expiry_height - curr_height;
+      uint64_t delta_height      = (blockchain_height >= expiry_height) ? 0 : expiry_height - blockchain_height;
       uint64_t expiry_epoch_time = now + (delta_height * DIFFICULTY_TARGET_V2);
-
       stream << expiry_height << " (in " << delta_height << ") blocks\n";
       stream << indent2 << "Expiry Date (Est. UTC): " << get_date_time(expiry_epoch_time) << " (" << get_human_time_ago(expiry_epoch_time, now) << ")\n";
     }

src/version.cpp.in

@@ -1,6 +1,6 @@
 #define DEF_LOKI_VERSION_MAJOR 5
 #define DEF_LOKI_VERSION_MINOR 1
-#define DEF_LOKI_VERSION_PATCH 1
+#define DEF_LOKI_VERSION_PATCH 2
 
 #define LOKI_STRINGIFY2(val) #val
 #define LOKI_STRINGIFY(val) LOKI_STRINGIFY2(val)

tests/core_tests/chaingen_main.cpp

@@ -107,6 +107,7 @@ int main(int argc, char* argv[])
     list_tests = command_line::get_arg(vm, arg_list_tests);
 
     // NOTE: Loki Tests
+    GENERATE_AND_PLAY(loki_checkpointing_alt_chain_handle_alt_blocks_at_tip);
     GENERATE_AND_PLAY(loki_checkpointing_alt_chain_more_service_node_checkpoints_less_pow_overtakes);
     GENERATE_AND_PLAY(loki_checkpointing_alt_chain_receive_checkpoint_votes_should_reorg_back);
     GENERATE_AND_PLAY(loki_checkpointing_alt_chain_with_increasing_service_node_checkpoints);

tests/core_tests/loki_tests.cpp

@@ -77,6 +77,82 @@ loki_generate_sequential_hard_fork_table(uint8_t max_hf_version)
   return result;
 }
 
+// Suppose we have checkpoint and alt block at height 40 and the main chain is at height 40 with a differing block.
+// Main chain receives checkpoints for height 40 on the alt chain via votes and reorgs back to height 39.
+// Now main chain has an alt block sitting in its DB for height 40 which actually starts beyond the chain.
+
+// In Monero land this is NOT ok because of the check in build_alt_chain
+// CHECK_AND_ASSERT_MES(m_db->height() > alt_chain.front().height, false, "main blockchain wrong height");
+// Where (m_db->height() == 40 and alt_chain.front().height == 40)
+
+// So, we change the > to a >= because it appears the code handles it fine and
+// it saves us from having to delete our alt_blocks and have to re-receive the
+// block over P2P again "just so that it can go through the normal block added
+// code path" again
+bool loki_checkpointing_alt_chain_handle_alt_blocks_at_tip::generate(std::vector<test_event_entry>& events)
+{
+  std::vector<std::pair<uint8_t, uint64_t>> hard_forks = loki_generate_sequential_hard_fork_table();
+  loki_chain_generator gen(events, hard_forks);
+
+  gen.add_blocks_until_version(hard_forks.back().first);
+  gen.add_n_blocks(40);
+  gen.add_mined_money_unlock_blocks();
+
+  int constexpr NUM_SERVICE_NODES = service_nodes::CHECKPOINT_QUORUM_SIZE;
+  std::vector<cryptonote::transaction> registration_txs(NUM_SERVICE_NODES);
+  for (auto i = 0u; i < NUM_SERVICE_NODES; ++i)
+    registration_txs[i] = gen.create_and_add_registration_tx(gen.first_miner());
+  gen.create_and_add_next_block(registration_txs);
+
+  // NOTE: Add blocks until we get to the first height that has a checkpointing quorum AND there are service nodes in the quorum.
+  int const MAX_TRIES = 16;
+  int tries           = 0;
+  for (; tries < MAX_TRIES; tries++)
+  {
+    gen.add_blocks_until_next_checkpointable_height();
+    std::shared_ptr<const service_nodes::testing_quorum> quorum = gen.get_testing_quorum(service_nodes::quorum_type::checkpointing, gen.height());
+    if (quorum && quorum->validators.size()) break;
+  }
+  assert(tries != MAX_TRIES);
+
+  for (size_t i = 0; i < service_nodes::CHECKPOINT_INTERVAL - 1; i++)
+    gen.create_and_add_next_block();
+
+  // NOTE: Create next block on checkpoint boundary and add checkpoiont
+  loki_chain_generator fork = gen;
+  gen.create_and_add_next_block();
+  fork.create_and_add_next_block();
+  fork.add_service_node_checkpoint(fork.height(), service_nodes::CHECKPOINT_MIN_VOTES);
+
+  // NOTE: Checkpoint should cause a reorg back to checkpoint height - 1. The
+  // alt block is still in the alt db because we don't trigger a chain switch
+  // until we receive a 2nd block that confirms the alt block.
+  loki_register_callback(events, "check_alt_block_count", [&events](cryptonote::core &c, size_t ev_index)
+  {
+    DEFINE_TESTS_ERROR_CONTEXT("check_alt_block_count");
+    CHECK_EQ(c.get_blockchain_storage().get_alternative_blocks_count(), 1);
+    return true;
+  });
+
+  // NOTE: We add a new block ontop that causes the alt block code path to run
+  // again, and calculate that this alt chain now has 2 blocks on it with
+  // a greater cumulative difficulty, causing a chain switch at this point.
+  fork.create_and_add_next_block();
+  crypto::hash expected_top_hash = cryptonote::get_block_hash(fork.top().block);
+  loki_register_callback(events, "check_chain_reorged", [&events, expected_top_hash](cryptonote::core &c, size_t ev_index)
+  {
+    DEFINE_TESTS_ERROR_CONTEXT("check_chain_reorged");
+    CHECK_EQ(c.get_blockchain_storage().get_alternative_blocks_count(), 0);
+    uint64_t top_height;
+    crypto::hash top_hash;
+    c.get_blockchain_top(top_height, top_hash);
+    CHECK_EQ(expected_top_hash, top_hash);
+    return true;
+  });
+  return true;
+}
+
+
 // NOTE: - Checks that a chain with a checkpoint but less PoW is preferred over a chain that is longer with more PoW but no checkpoints
 bool loki_checkpointing_alt_chain_more_service_node_checkpoints_less_pow_overtakes::generate(std::vector<test_event_entry>& events)
 {

tests/core_tests/loki_tests.h

@@ -38,6 +38,7 @@
 void                                      loki_register_callback                  (std::vector<test_event_entry> &events, std::string const &callback_name, loki_callback callback);
 std::vector<std::pair<uint8_t, uint64_t>> loki_generate_sequential_hard_fork_table(uint8_t max_hf_version = cryptonote::network_version_count - 1);
 
+struct loki_checkpointing_alt_chain_handle_alt_blocks_at_tip                         : public test_chain_unit_base { bool generate(std::vector<test_event_entry>& events); };
 struct loki_checkpointing_alt_chain_more_service_node_checkpoints_less_pow_overtakes : public test_chain_unit_base { bool generate(std::vector<test_event_entry>& events); };
 struct loki_checkpointing_alt_chain_receive_checkpoint_votes_should_reorg_back       : public test_chain_unit_base { bool generate(std::vector<test_event_entry>& events); };
 struct loki_checkpointing_alt_chain_with_increasing_service_node_checkpoints         : public test_chain_unit_base { bool generate(std::vector<test_event_entry>& events); };