protocol: fix false positives dropping peers

it'd trigger on reorgs

src/cryptonote_protocol/cryptonote_protocol_handler.inl

@@ -2064,8 +2064,14 @@ skip:
         // get rid of blocks we already requested, or already have
         if (skip_unneeded_hashes(context, true) && context.m_needed_objects.empty() && context.m_num_requested == 0)
         {
-          MERROR(context << "Nothing we can request from this peer, and we did not request anything previously");
-          return false;
+          if (context.m_remote_blockchain_height > m_block_queue.get_next_needed_height(bc_height))
+          {
+            MERROR(context << "Nothing we can request from this peer, and we did not request anything previously");
+            return false;
+          }
+          MDEBUG(context << "Nothing to get from this peer, and it's not ahead of us, all done");
+          context.m_state = cryptonote_connection_context::state_normal;
+          return true;
         }
         uint64_t next_needed_height = m_block_queue.get_next_needed_height(bc_height);
         uint64_t next_block_height;
@@ -2204,8 +2210,14 @@ skip:
         }
         if (skip_unneeded_hashes(context, false) && context.m_needed_objects.empty() && context.m_num_requested == 0)
         {
-          MERROR(context << "Nothing we can request from this peer, and we did not request anything previously");
-          return false;
+          if (context.m_remote_blockchain_height > m_block_queue.get_next_needed_height(m_core.get_current_blockchain_height()))
+          {
+            MERROR(context << "Nothing we can request from this peer, and we did not request anything previously");
+            return false;
+          }
+          MDEBUG(context << "Nothing to get from this peer, and it's not ahead of us, all done");
+          context.m_state = cryptonote_connection_context::state_normal;
+          return true;
         }
 
         const uint64_t first_block_height = context.m_last_response_height - context.m_needed_objects.size() + 1;
@@ -2309,30 +2321,16 @@ skip:
         return true;
       }
 
-      // if we're still around, we might be at a point where the peer is pruned, so we could either
-      // drop it to make space for other peers, or ask for a span further down the line
-      const uint32_t next_stripe = get_next_needed_pruning_stripe().first;
-      const uint32_t peer_stripe = tools::get_pruning_stripe(context.m_pruning_seed);
-      const uint32_t local_stripe = tools::get_pruning_stripe(m_core.get_blockchain_pruning_seed());
-      if (!(m_sync_pruned_blocks && peer_stripe == local_stripe) && next_stripe && peer_stripe && next_stripe != peer_stripe)
+      // we can do nothing, so drop this peer to make room for others unless we think we've downloaded all we need
+      const uint64_t blockchain_height = m_core.get_current_blockchain_height();
+      if (std::max(blockchain_height, m_block_queue.get_next_needed_height(blockchain_height)) >= m_core.get_target_blockchain_height())
       {
-        // at this point, we have to either close the connection, or start getting blocks past the
-        // current point, or become dormant
-        MDEBUG(context << "this peer is pruned at seed " << epee::string_tools::to_string_hex(context.m_pruning_seed) <<
-            ", next stripe needed is " << next_stripe);
-        if (!context.m_is_income)
-        {
-          if (should_drop_connection(context, next_stripe))
-          {
-            m_p2p->add_used_stripe_peer(context);
-            return false; // drop outgoing connections
-          }
-        }
-        // we'll get back stuck waiting for the go ahead
         context.m_state = cryptonote_connection_context::state_normal;
         MLOG_PEER_STATE("Nothing to do for now, switching to normal state");
         return true;
       }
+      MLOG_PEER_STATE("We can download nothing from this peer, dropping");
+      return false;
     }
 
 skip:

tests/core_proxy/core_proxy.h

@@ -112,5 +112,6 @@ namespace tests
     bool prune_blockchain(uint32_t pruning_seed) const { return true; }
     bool get_txpool_complement(const std::vector<crypto::hash> &hashes, std::vector<cryptonote::blobdata> &txes) { return false; }
     bool get_pool_transaction_hashes(std::vector<crypto::hash>& txs, bool include_unrelayed_txes = true) const { return false; }
+    crypto::hash get_block_id_by_height(uint64_t height) const { return crypto::null_hash; }
   };
 }

tests/unit_tests/node_server.cpp

@@ -93,6 +93,7 @@ public:
   bool has_block_weights(uint64_t height, uint64_t nblocks) const { return false; }
   bool get_txpool_complement(const std::vector<crypto::hash> &hashes, std::vector<cryptonote::blobdata> &txes) { return false; }
   bool get_pool_transaction_hashes(std::vector<crypto::hash>& txs, bool include_unrelayed_txes = true) const { return false; }
+  crypto::hash get_block_id_by_height(uint64_t height) const { return crypto::null_hash; }
   void stop() {}
 };