From 8c8f3d73b5f67df77739c3786fb924e62df8ea36 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 27 Dec 2020 14:13:26 +0000 Subject: protocol: fix false positives dropping peers it'd trigger on reorgs --- .../cryptonote_protocol_handler.inl | 44 +++++++++++----------- tests/core_proxy/core_proxy.h | 1 + tests/unit_tests/node_server.cpp | 1 + 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index 90f8298fb..c76e5628d 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/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: diff --git a/tests/core_proxy/core_proxy.h b/tests/core_proxy/core_proxy.h index ecfcc18ae..ebc3a89c2 100644 --- a/tests/core_proxy/core_proxy.h +++ b/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 &hashes, std::vector &txes) { return false; } bool get_pool_transaction_hashes(std::vector& txs, bool include_unrelayed_txes = true) const { return false; } + crypto::hash get_block_id_by_height(uint64_t height) const { return crypto::null_hash; } }; } diff --git a/tests/unit_tests/node_server.cpp b/tests/unit_tests/node_server.cpp index 0191e5aa4..0569d3748 100644 --- a/tests/unit_tests/node_server.cpp +++ b/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 &hashes, std::vector &txes) { return false; } bool get_pool_transaction_hashes(std::vector& 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() {} }; -- cgit v1.2.3