From 08abb670e1d8c2d459dcd49ca02785a8cebc7b8b Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 12 Aug 2017 10:59:27 +0100 Subject: [PATCH 1/2] protocol: fix reorgs while syncing --- src/cryptonote_basic/connection_context.h | 4 +- src/cryptonote_protocol/block_queue.cpp | 109 +++------- src/cryptonote_protocol/block_queue.h | 6 +- .../cryptonote_protocol_handler.inl | 30 ++- tests/unit_tests/block_queue.cpp | 188 ------------------ 5 files changed, 48 insertions(+), 289 deletions(-) diff --git a/src/cryptonote_basic/connection_context.h b/src/cryptonote_basic/connection_context.h index ff56b73d7..06f66120b 100644 --- a/src/cryptonote_basic/connection_context.h +++ b/src/cryptonote_basic/connection_context.h @@ -39,7 +39,8 @@ namespace cryptonote struct cryptonote_connection_context: public epee::net_utils::connection_context_base { - cryptonote_connection_context(): m_state(state_befor_handshake), m_remote_blockchain_height(0), m_last_response_height(0) {} + cryptonote_connection_context(): m_state(state_befor_handshake), m_remote_blockchain_height(0), m_last_response_height(0), + m_last_known_hash(cryptonote::null_hash) {} enum state { @@ -56,6 +57,7 @@ namespace cryptonote uint64_t m_last_response_height; boost::posix_time::ptime m_last_request_time; epee::copyable_atomic m_callback_request_count; //in debug purpose: problem with double callback rise + crypto::hash m_last_known_hash; //size_t m_score; TODO: add score calculations }; diff --git a/src/cryptonote_protocol/block_queue.cpp b/src/cryptonote_protocol/block_queue.cpp index 583c3abf4..610637637 100644 --- a/src/cryptonote_protocol/block_queue.cpp +++ b/src/cryptonote_protocol/block_queue.cpp @@ -118,25 +118,6 @@ void block_queue::remove_spans(const boost::uuids::uuid &connection_id, uint64_t } } -void block_queue::mark_last_block(uint64_t last_block_height) -{ - boost::unique_lock lock(mutex); - if (!blocks.empty() && is_blockchain_placeholder(*blocks.begin())) - blocks.erase(*blocks.begin()); - for (block_map::iterator i = blocks.begin(); i != blocks.end(); ) - { - block_map::iterator j = i++; - if (j->start_block_height + j->nblocks - 1 <= last_block_height) - { - blocks.erase(j); - } - } - - // mark the current state of the db (for a fresh db, it's just the genesis block) - add_blocks(0, last_block_height + 1, boost::uuids::nil_uuid()); - MDEBUG("last blocked marked at " << last_block_height); -} - uint64_t block_queue::get_max_block_height() const { boost::unique_lock lock(mutex); @@ -171,7 +152,19 @@ std::string block_queue::get_overview() const return s; } -std::pair block_queue::reserve_span(uint64_t first_block_height, uint64_t last_block_height, uint64_t max_blocks, const boost::uuids::uuid &connection_id, boost::posix_time::ptime time) +bool block_queue::requested(const crypto::hash &hash) const +{ + boost::unique_lock lock(mutex); + for (const auto &span: blocks) + { + for (const auto &h: span.hashes) + if (h == hash) + return true; + } + return false; +} + +std::pair block_queue::reserve_span(uint64_t first_block_height, uint64_t last_block_height, uint64_t max_blocks, const boost::uuids::uuid &connection_id, const std::list &block_hashes, boost::posix_time::ptime time) { boost::unique_lock lock(mutex); @@ -181,71 +174,25 @@ std::pair block_queue::reserve_span(uint64_t first_block_hei return std::make_pair(0, 0); } - uint64_t max_block_height = get_max_block_height(); - if (last_block_height > max_block_height) - max_block_height = last_block_height; - if (max_block_height == 0) + uint64_t span_start_height = last_block_height - block_hashes.size() + 1; + std::list::const_iterator i = block_hashes.begin(); + while (i != block_hashes.end() && requested(*i)) { - MDEBUG("reserve_span: max_block_height is 0"); - return std::make_pair(first_block_height, std::min(last_block_height - first_block_height + 1, max_blocks)); - } - - uint64_t base = 0, last_placeholder_block = 0; - bool has_placeholder = false; - block_map::const_iterator i = blocks.begin(); - if (i != blocks.end() && is_blockchain_placeholder(*i)) - { - base = i->start_block_height + i->nblocks; - last_placeholder_block = base - 1; - has_placeholder = true; ++i; - for (block_map::const_iterator j = i; j != blocks.end(); ++j) - { - if (j->start_block_height < base) - base = j->start_block_height; - } + ++span_start_height; } - if (base > first_block_height) - base = first_block_height; - - CHECK_AND_ASSERT_MES (base <= max_block_height + 1, std::make_pair(0, 0), "Blockchain placeholder larger than max block height"); - std::vector bitmap(max_block_height + 1 - base, 0); - MDEBUG("base " << base << ", last_placeholder_block " << (has_placeholder ? std::to_string(last_placeholder_block) : "none") << ", first_block_height " << first_block_height); - if (has_placeholder && last_placeholder_block >= base) - memset(bitmap.data(), 1, last_placeholder_block + 1 - base); - while (i != blocks.end()) + uint64_t span_length = 0; + std::list hashes; + while (i != block_hashes.end() && span_length < max_blocks) { - CHECK_AND_ASSERT_MES (i->start_block_height >= base, std::make_pair(0, 0), "Span starts before blochckain placeholder"); - memset(bitmap.data() + i->start_block_height - base, 1, i->nblocks); + hashes.push_back(*i); ++i; + ++span_length; } - - const uint8_t *ptr = (const uint8_t*)memchr(bitmap.data() + first_block_height - base, 0, bitmap.size() - (first_block_height - base)); - if (!ptr) - { - MDEBUG("reserve_span: 0 not found in bitmap: " << first_block_height << " " << bitmap.size()); - print(); - return std::make_pair(0, 0); - } - uint64_t start_block_height = ptr - bitmap.data() + base; - if (start_block_height > last_block_height) - { - MDEBUG("reserve_span: start_block_height > last_block_height: " << start_block_height << " < " << last_block_height); - return std::make_pair(0, 0); - } - if (start_block_height + max_blocks - 1 < first_block_height) - { - MDEBUG("reserve_span: start_block_height + max_blocks - 1 < first_block_height: " << start_block_height << " + " << max_blocks << " - 1 < " << first_block_height); - return std::make_pair(0, 0); - } - - uint64_t nblocks = 1; - while (start_block_height + nblocks <= last_block_height && nblocks < max_blocks && bitmap[start_block_height + nblocks - base] == 0) - ++nblocks; - - MDEBUG("Reserving span " << start_block_height << " - " << (start_block_height + nblocks - 1) << " for " << connection_id); - add_blocks(start_block_height, nblocks, connection_id, time); - return std::make_pair(start_block_height, nblocks); + MDEBUG("Reserving span " << span_start_height << " - " << (span_start_height + span_length - 1) << " for " << connection_id); + add_blocks(span_start_height, span_length, connection_id, time); + set_span_hashes(span_start_height, connection_id, hashes); + return std::make_pair(span_start_height, span_length); } bool block_queue::is_blockchain_placeholder(const span &span) const @@ -366,13 +313,15 @@ size_t block_queue::get_num_filled_spans() const return size; } -crypto::hash block_queue::get_last_known_hash() const +crypto::hash block_queue::get_last_known_hash(const boost::uuids::uuid &connection_id) const { boost::unique_lock lock(mutex); crypto::hash hash = cryptonote::null_hash; uint64_t highest_height = 0; for (const auto &span: blocks) { + if (span.connection_id != connection_id) + continue; uint64_t h = span.start_block_height + span.nblocks - 1; if (h > highest_height && span.hashes.size() == span.nblocks) { diff --git a/src/cryptonote_protocol/block_queue.h b/src/cryptonote_protocol/block_queue.h index c75ebc0b9..9a211ac47 100644 --- a/src/cryptonote_protocol/block_queue.h +++ b/src/cryptonote_protocol/block_queue.h @@ -73,11 +73,10 @@ namespace cryptonote void flush_stale_spans(const std::set &live_connections); void remove_span(uint64_t start_block_height); void remove_spans(const boost::uuids::uuid &connection_id, uint64_t start_block_height); - void mark_last_block(uint64_t last_block_height); uint64_t get_max_block_height() const; void print() const; std::string get_overview() const; - std::pair reserve_span(uint64_t first_block_height, uint64_t last_block_height, uint64_t max_blocks, const boost::uuids::uuid &connection_id, boost::posix_time::ptime time = boost::posix_time::microsec_clock::universal_time()); + std::pair reserve_span(uint64_t first_block_height, uint64_t last_block_height, uint64_t max_blocks, const boost::uuids::uuid &connection_id, const std::list &block_hashes, boost::posix_time::ptime time = boost::posix_time::microsec_clock::universal_time()); bool is_blockchain_placeholder(const span &span) const; std::pair get_start_gap_span() const; std::pair get_next_span_if_scheduled(std::list &hashes, boost::uuids::uuid &connection_id, boost::posix_time::ptime &time) const; @@ -86,9 +85,10 @@ namespace cryptonote size_t get_data_size() const; size_t get_num_filled_spans_prefix() const; size_t get_num_filled_spans() const; - crypto::hash get_last_known_hash() const; + crypto::hash get_last_known_hash(const boost::uuids::uuid &connection_id) const; float get_speed(const boost::uuids::uuid &connection_id) const; bool foreach(std::function f, bool include_blockchain_placeholder = false) const; + bool requested(const crypto::hash &hash) const; private: block_map blocks; diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index 3e3bb83d0..f9fe6fe70 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -947,7 +947,8 @@ namespace cryptonote } { - MLOG_YELLOW(el::Level::Debug, "Got NEW BLOCKS inside of " << __FUNCTION__ << ": size: " << arg.blocks.size()); + MLOG_YELLOW(el::Level::Debug, context << " Got NEW BLOCKS inside of " << __FUNCTION__ << ": size: " << arg.blocks.size() + << ", blocks: " << start_height << " - " << (start_height + arg.blocks.size() - 1)); // add that new span to the block queue const boost::posix_time::time_duration dt = now - context.m_last_request_time; @@ -978,7 +979,6 @@ namespace cryptonote uint64_t start_height; std::list blocks; boost::uuids::uuid span_connection_id; - m_block_queue.mark_last_block(previous_height - 1); if (!m_block_queue.get_next_span(start_height, blocks, span_connection_id)) { MDEBUG(context << " no next span found, going back to download"); @@ -986,11 +986,6 @@ namespace cryptonote } MDEBUG(context << " next span in the queue has blocks " << start_height << "-" << (start_height + blocks.size() - 1) << ", we need " << previous_height); - if (previous_height < start_height || previous_height >= start_height + blocks.size()) - { - MDEBUG(context << " this span is not what we need, going back to download"); - break; - } const boost::posix_time::ptime start = boost::posix_time::microsec_clock::universal_time(); @@ -1083,7 +1078,7 @@ namespace cryptonote m_core.cleanup_handle_incoming_blocks(); - m_block_queue.mark_last_block(m_core.get_current_blockchain_height() - 1); + m_block_queue.remove_spans(span_connection_id, start_height); if (m_core.get_current_blockchain_height() > previous_height) { @@ -1201,8 +1196,6 @@ skip: template bool t_cryptonote_protocol_handler::request_missing_objects(cryptonote_connection_context& context, bool check_having_blocks, bool force_next_span) { - m_block_queue.mark_last_block(m_core.get_current_blockchain_height() - 1); - // flush stale spans std::set live_connections; m_p2p->for_each_connection([&](cryptonote_connection_context& context, nodetool::peerid_type peer_id, uint32_t support_flags)->bool{ @@ -1308,8 +1301,13 @@ skip: context.m_last_response_height = 0; goto skip; } + // take out blocks we already have + while (!context.m_needed_objects.empty() && m_core.have_block(context.m_needed_objects.front())) + { + context.m_needed_objects.pop_front(); + } const uint64_t first_block_height = context.m_last_response_height - context.m_needed_objects.size() + 1; - span = m_block_queue.reserve_span(first_block_height, context.m_last_response_height, count_limit, context.m_connection_id); + span = m_block_queue.reserve_span(first_block_height, context.m_last_response_height, count_limit, context.m_connection_id, context.m_needed_objects); MDEBUG(context << " span from " << first_block_height << ": " << span.first << "/" << span.second); } if (span.second == 0 && !force_next_span) @@ -1360,8 +1358,6 @@ skip: auto j = it++; context.m_needed_objects.erase(j); } - - m_block_queue.set_span_hashes(span.first, context.m_connection_id, hashes); } context.m_last_request_time = boost::posix_time::microsec_clock::universal_time(); @@ -1384,10 +1380,9 @@ skip: if (!start_from_current_chain) { - // we'll want to start off from where we are on the download side, which may not be added yet - crypto::hash last_known_hash = m_block_queue.get_last_known_hash(); - if (last_known_hash != cryptonote::null_hash && r.block_ids.front() != last_known_hash) - r.block_ids.push_front(last_known_hash); + // we'll want to start off from where we are on that peer, which may not be added yet + if (context.m_last_known_hash != cryptonote::null_hash && r.block_ids.front() != context.m_last_known_hash) + r.block_ids.push_front(context.m_last_known_hash); } handler_request_blocks_history( r.block_ids ); // change the limit(?), sleep(?) @@ -1486,6 +1481,7 @@ skip: { context.m_needed_objects.push_back(bl_id); } + context.m_last_known_hash = context.m_needed_objects.back(); if (!request_missing_objects(context, false)) { diff --git a/tests/unit_tests/block_queue.cpp b/tests/unit_tests/block_queue.cpp index a52e221d6..f05d5487a 100644 --- a/tests/unit_tests/block_queue.cpp +++ b/tests/unit_tests/block_queue.cpp @@ -85,191 +85,3 @@ TEST(block_queue, flush_uuid) bq.add_blocks(0, 200, uuid1()); ASSERT_EQ(bq.get_max_block_height(), 399); } - -TEST(block_queue, reserve_overlaps_both) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(0, 100, uuid1()); - bq.add_blocks(200, 100, uuid1()); - ASSERT_EQ(bq.get_max_block_height(), 299); - - span = bq.reserve_span(50, 250, 250, uuid2()); - ASSERT_EQ(span.first, 100); - ASSERT_EQ(span.second, 100); -} - -TEST(block_queue, reserve_overlaps_none) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(0, 100, uuid1()); - bq.add_blocks(200, 100, uuid1()); - ASSERT_EQ(bq.get_max_block_height(), 299); - - span = bq.reserve_span(120, 180, 250, uuid2()); - ASSERT_EQ(span.first, 120); - ASSERT_EQ(span.second, 61); -} - -TEST(block_queue, reserve_overlaps_none_max_hit) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(0, 100, uuid1()); - bq.add_blocks(200, 100, uuid1()); - ASSERT_EQ(bq.get_max_block_height(), 299); - - span = bq.reserve_span(120, 500, 50, uuid2()); - ASSERT_EQ(span.first, 120); - ASSERT_EQ(span.second, 50); -} - -TEST(block_queue, reserve_overlaps_start) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(0, 100, uuid1()); - bq.add_blocks(200, 100, uuid1()); - ASSERT_EQ(bq.get_max_block_height(), 299); - - span = bq.reserve_span(50, 150, 250, uuid2()); - ASSERT_EQ(span.first, 100); - ASSERT_EQ(span.second, 51); -} - -TEST(block_queue, reserve_overlaps_start_max_hit) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(0, 100, uuid1()); - bq.add_blocks(200, 100, uuid1()); - ASSERT_EQ(bq.get_max_block_height(), 299); - - span = bq.reserve_span(50, 300, 75, uuid2()); - ASSERT_EQ(span.first, 100); - ASSERT_EQ(span.second, 75); -} - -TEST(block_queue, reserve_overlaps_stop) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(0, 100, uuid1()); - bq.add_blocks(200, 100, uuid1()); - ASSERT_EQ(bq.get_max_block_height(), 299); - - span = bq.reserve_span(150, 300, 250, uuid2()); - ASSERT_EQ(span.first, 150); - ASSERT_EQ(span.second, 50); -} - -TEST(block_queue, reserve_start_is_empty_after) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(100, 100, uuid1()); - span = bq.reserve_span(150, 250, 100, uuid1()); - ASSERT_EQ(span.first, 200); - ASSERT_EQ(span.second, 51); -} - -TEST(block_queue, reserve_start_is_empty_start_fits) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(100, 100, uuid1()); - span = bq.reserve_span(0, 250, 50, uuid1()); - ASSERT_EQ(span.first, 0); - ASSERT_EQ(span.second, 50); -} - -TEST(block_queue, reserve_start_is_empty_start_overflows) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(100, 100, uuid1()); - span = bq.reserve_span(0, 250, 150, uuid1()); - ASSERT_EQ(span.first, 0); - ASSERT_EQ(span.second, 100); -} - -TEST(block_queue, flush_spans) -{ - cryptonote::block_queue bq; - std::pair span; - - bq.add_blocks(100, 100, uuid2()); - bq.add_blocks(200, 100, uuid1()); - bq.add_blocks(300, 100, uuid2()); - ASSERT_EQ(bq.get_max_block_height(), 399); - bq.flush_spans(uuid2()); - ASSERT_EQ(bq.get_max_block_height(), 299); - span = bq.reserve_span(0, 500, 500, uuid1()); - ASSERT_EQ(span.first, 0); - ASSERT_EQ(span.second, 200); - bq.flush_spans(uuid1()); - ASSERT_EQ(bq.get_max_block_height(), 0); -} - -TEST(block_queue, get_next_span) -{ - cryptonote::block_queue bq; - std::pair span; - uint64_t height; - std::list blocks; - boost::uuids::uuid uuid; - - bq.add_blocks(100, std::list(100), uuid2(), 0, 0); - bq.add_blocks(200, std::list(101), uuid1(), 0, 0); - bq.add_blocks(300, std::list(102), uuid2(), 0, 0); - - ASSERT_TRUE(bq.get_next_span(height, blocks, uuid)); - ASSERT_EQ(height, 100); - ASSERT_EQ(blocks.size(), 100); - ASSERT_EQ(uuid, uuid2()); - bq.remove_span(height); - - ASSERT_TRUE(bq.get_next_span(height, blocks, uuid)); - ASSERT_EQ(height, 200); - ASSERT_EQ(blocks.size(), 101); - ASSERT_EQ(uuid, uuid1()); - bq.remove_span(height); - - ASSERT_TRUE(bq.get_next_span(height, blocks, uuid)); - ASSERT_EQ(height, 300); - ASSERT_EQ(blocks.size(), 102); - ASSERT_EQ(uuid, uuid2()); - bq.remove_span(height); - - ASSERT_FALSE(bq.get_next_span(height, blocks, uuid)); -} - -TEST(block_queue, get_next_span_if_scheduled) -{ - cryptonote::block_queue bq; - std::pair span; - uint64_t height; - std::list blocks; - boost::uuids::uuid uuid; - std::list hashes; - boost::posix_time::ptime time; - - bq.reserve_span(0, 100, 100, uuid1()); - span = bq.get_next_span_if_scheduled(hashes, uuid, time); - ASSERT_EQ(span.first, 0); - ASSERT_EQ(span.second, 100); - ASSERT_EQ(uuid, uuid1()); - bq.add_blocks(0, std::list(100), uuid1(), 0, 0); - span = bq.get_next_span_if_scheduled(hashes, uuid, time); - ASSERT_EQ(span.second, 0); -} From 2ec15a693165db96dd7e75ee060ea4622a47ab7a Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 12 Aug 2017 10:59:54 +0100 Subject: [PATCH 2/2] daemon: print peers' top height in sync_info --- src/cryptonote_protocol/cryptonote_protocol_defs.h | 3 +++ src/cryptonote_protocol/cryptonote_protocol_handler.inl | 5 ++++- src/daemon/rpc_command_executor.cpp | 2 +- 3 files changed, 8 insertions(+), 2 deletions(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_defs.h b/src/cryptonote_protocol/cryptonote_protocol_defs.h index 37b503436..6e6c83f04 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_defs.h +++ b/src/cryptonote_protocol/cryptonote_protocol_defs.h @@ -76,6 +76,8 @@ namespace cryptonote boost::uuids::uuid connection_id; + uint64_t height; + BEGIN_KV_SERIALIZE_MAP() KV_SERIALIZE(incoming) KV_SERIALIZE(localhost) @@ -97,6 +99,7 @@ namespace cryptonote KV_SERIALIZE(current_upload) KV_SERIALIZE(support_flags) KV_SERIALIZE_VAL_POD_AS_BLOB(connection_id) + KV_SERIALIZE(height) END_KV_SERIALIZE_MAP() }; diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index f9fe6fe70..b5a307208 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -239,6 +239,8 @@ namespace cryptonote cnx.connection_id = cntxt.m_connection_id; + cnx.height = cntxt.m_remote_blockchain_height; + connections.push_back(cnx); return true; @@ -264,6 +266,8 @@ namespace cryptonote return false; } + context.m_remote_blockchain_height = hshd.current_height; + uint64_t target = m_core.get_target_blockchain_height(); if (target == 0) target = m_core.get_current_blockchain_height(); @@ -293,7 +297,6 @@ namespace cryptonote } LOG_PRINT_L1("Remote blockchain height: " << hshd.current_height << ", id: " << hshd.top_id); context.m_state = cryptonote_connection_context::state_synchronizing; - context.m_remote_blockchain_height = hshd.current_height; //let the socket to send response to handshake, but request callback, to let send request data after response LOG_PRINT_CCONTEXT_L2("requesting callback"); ++context.m_callback_request_count; diff --git a/src/daemon/rpc_command_executor.cpp b/src/daemon/rpc_command_executor.cpp index 34d87d282..995fee484 100644 --- a/src/daemon/rpc_command_executor.cpp +++ b/src/daemon/rpc_command_executor.cpp @@ -1742,7 +1742,7 @@ bool t_rpc_command_executor::sync_info() for (const auto &s: res.spans) if (s.rate > 0.0f && s.connection_id == p.info.connection_id) nblocks += s.nblocks, size += s.size; - tools::success_msg_writer() << address << " " << p.info.peer_id << " " << p.info.current_download << " kB/s, " << nblocks << " blocks / " << size/1e6 << " MB queued"; + tools::success_msg_writer() << address << " " << p.info.peer_id << " " << p.info.height << " " << p.info.current_download << " kB/s, " << nblocks << " blocks / " << size/1e6 << " MB queued"; } uint64_t total_size = 0;