From 70b8c6d77a6aaf90a6e84f3ec6f25bea3b163c40 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 18 Aug 2017 20:14:23 +0100 Subject: [PATCH 01/10] cryptonote_protocol: misc fixes to the new sync algorithm Fix sync wedge corner case: It could happen if a connection went into standby mode, while it was the one which had requested the next span, and that span was still waiting for the data, and that peer is not on the main chain. Other peers can then start asking for that data again and again, but never get it as only that forked peer does. And various other fixes --- src/cryptonote_protocol/block_queue.cpp | 28 ++++- src/cryptonote_protocol/block_queue.h | 3 +- .../cryptonote_protocol_handler.h | 2 + .../cryptonote_protocol_handler.inl | 112 ++++++++++++------ src/p2p/net_node.inl | 2 + 5 files changed, 109 insertions(+), 38 deletions(-) diff --git a/src/cryptonote_protocol/block_queue.cpp b/src/cryptonote_protocol/block_queue.cpp index 94d31404..02a8e3ec 100644 --- a/src/cryptonote_protocol/block_queue.cpp +++ b/src/cryptonote_protocol/block_queue.cpp @@ -52,8 +52,11 @@ namespace cryptonote void block_queue::add_blocks(uint64_t height, std::list bcel, const boost::uuids::uuid &connection_id, float rate, size_t size) { boost::unique_lock lock(mutex); - remove_span(height); + std::list hashes; + bool has_hashes = remove_span(height, &hashes); blocks.insert(span(height, std::move(bcel), connection_id, rate, size)); + if (has_hashes) + set_span_hashes(height, connection_id, hashes); } void block_queue::add_blocks(uint64_t height, uint64_t nblocks, const boost::uuids::uuid &connection_id, boost::posix_time::ptime time) @@ -92,17 +95,20 @@ void block_queue::flush_stale_spans(const std::set &live_con } } -void block_queue::remove_span(uint64_t start_block_height) +bool block_queue::remove_span(uint64_t start_block_height, std::list *hashes) { boost::unique_lock lock(mutex); for (block_map::iterator i = blocks.begin(); i != blocks.end(); ++i) { if (i->start_block_height == start_block_height) { + if (hashes) + *hashes = std::move(i->hashes); blocks.erase(i); - return; + return true; } } + return false; } void block_queue::remove_spans(const boost::uuids::uuid &connection_id, uint64_t start_block_height) @@ -278,6 +284,22 @@ bool block_queue::get_next_span(uint64_t &height, std::list lock(mutex); + if (blocks.empty()) + return false; + block_map::const_iterator i = blocks.begin(); + if (is_blockchain_placeholder(*i)) + ++i; + if (i == blocks.end()) + return false; + if (i->connection_id != connection_id) + return false; + filled = !i->blocks.empty(); + return true; +} + size_t block_queue::get_data_size() const { boost::unique_lock lock(mutex); diff --git a/src/cryptonote_protocol/block_queue.h b/src/cryptonote_protocol/block_queue.h index fa1a0f21..13d4619b 100644 --- a/src/cryptonote_protocol/block_queue.h +++ b/src/cryptonote_protocol/block_queue.h @@ -71,7 +71,7 @@ namespace cryptonote void add_blocks(uint64_t height, uint64_t nblocks, const boost::uuids::uuid &connection_id, boost::posix_time::ptime time = boost::date_time::min_date_time); void flush_spans(const boost::uuids::uuid &connection_id, bool all = false); void flush_stale_spans(const std::set &live_connections); - void remove_span(uint64_t start_block_height); + bool remove_span(uint64_t start_block_height, std::list *hashes = NULL); void remove_spans(const boost::uuids::uuid &connection_id, uint64_t start_block_height); uint64_t get_max_block_height() const; void print() const; @@ -82,6 +82,7 @@ namespace cryptonote std::pair get_next_span_if_scheduled(std::list &hashes, boost::uuids::uuid &connection_id, boost::posix_time::ptime &time) const; void set_span_hashes(uint64_t start_height, const boost::uuids::uuid &connection_id, std::list hashes); bool get_next_span(uint64_t &height, std::list &bcel, boost::uuids::uuid &connection_id, bool filled = true) const; + bool has_next_span(const boost::uuids::uuid &connection_id, bool &filled) const; size_t get_data_size() const; size_t get_num_filled_spans_prefix() const; size_t get_num_filled_spans() const; diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.h b/src/cryptonote_protocol/cryptonote_protocol_handler.h index d9474776..d54687e6 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.h +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.h @@ -111,6 +111,7 @@ namespace cryptonote std::list get_connections(); const block_queue &get_block_queue() const { return m_block_queue; } void stop(); + void on_connection_close(cryptonote_connection_context &context); private: //----------------- commands handlers ---------------------------------------------- int handle_notify_new_block(int command, NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& context); @@ -133,6 +134,7 @@ namespace cryptonote bool should_download_next_span(cryptonote_connection_context& context) const; void drop_connection(cryptonote_connection_context &context, bool add_fail, bool flush_all_spans); bool kick_idle_peers(); + int try_add_next_blocks(cryptonote_connection_context &context); t_core& m_core; diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index daefe88b..87690f1f 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -106,6 +106,11 @@ namespace cryptonote LOG_PRINT_CCONTEXT_L2("-->>NOTIFY_REQUEST_CHAIN: m_block_ids.size()=" << r.block_ids.size() ); post_notify(r, context); } + else if(context.m_state == cryptonote_connection_context::state_standby) + { + context.m_state = cryptonote_connection_context::state_synchronizing; + try_add_next_blocks(context); + } return true; } @@ -819,8 +824,6 @@ namespace cryptonote { MLOG_P2P_MESSAGE("Received NOTIFY_RESPONSE_GET_OBJECTS (" << arg.blocks.size() << " blocks, " << arg.txs.size() << " txes)"); - bool force_next_span = false; - // calculate size of request size_t size = 0; for (const auto &element : arg.txs) size += element.size(); @@ -938,19 +941,34 @@ namespace cryptonote context.m_last_known_hash = cryptonote::get_blob_hash(arg.blocks.back().block); - if (m_core.get_test_drop_download() && m_core.get_test_drop_download_height()) { // DISCARD BLOCKS for testing + if (!m_core.get_test_drop_download() || !m_core.get_test_drop_download_height()) { // DISCARD BLOCKS for testing + return 1; + } + } - // We try to lock the sync lock. If we can, it means no other thread is - // currently adding blocks, so we do that for as long as we can from the - // block queue. Then, we go back to download. - const boost::unique_lock sync{m_sync_lock, boost::try_to_lock}; - if (!sync.owns_lock()) - { - MINFO("Failed to lock m_sync_lock, going back to download"); - goto skip; - } - MDEBUG(context << " lock m_sync_lock, adding blocks to chain..."); +skip: + try_add_next_blocks(context); + return 1; + } + template + int t_cryptonote_protocol_handler::try_add_next_blocks(cryptonote_connection_context& context) + { + bool force_next_span = false; + + { + // We try to lock the sync lock. If we can, it means no other thread is + // currently adding blocks, so we do that for as long as we can from the + // block queue. Then, we go back to download. + const boost::unique_lock sync{m_sync_lock, boost::try_to_lock}; + if (!sync.owns_lock()) + { + MINFO("Failed to lock m_sync_lock, going back to download"); + goto skip; + } + MDEBUG(context << " lock m_sync_lock, adding blocks to chain..."); + + { m_core.pause_mine(); epee::misc_utils::auto_scope_leave_caller scope_exit_handler = epee::misc_utils::create_scope_leave_handler( boost::bind(&t_core::resume_mine, &m_core)); @@ -984,21 +1002,15 @@ namespace cryptonote // - later in an alt chain // - orphan // if it was requested, then it'll be resolved later, otherwise it's an orphan - bool parent_requested = false; - m_p2p->for_each_connection([&](cryptonote_connection_context& context, nodetool::peerid_type peer_id, uint32_t support_flags)->bool{ - if (context.m_requested_objects.find(new_block.prev_id) != context.m_requested_objects.end()) - { - parent_requested = true; - return false; - } - return true; - }); + bool parent_requested = m_block_queue.requested(new_block.prev_id); if (!parent_requested) { - LOG_ERROR_CCONTEXT("Got block with unknown parent which was not requested - dropping connection"); - // in case the peer had dropped beforehand, remove the span anyway so other threads can wake up and get it - m_block_queue.remove_spans(span_connection_id, start_height); - return 1; + // this can happen if a connection was sicced onto a late span, if it did not have those blocks, + // since we don't know that at the sic time + LOG_ERROR_CCONTEXT("Got block with unknown parent which was not requested - querying block hashes"); + context.m_needed_objects.clear(); + context.m_last_response_height = 0; + goto skip; } // parent was requested, so we wait for it to be retrieved @@ -1007,6 +1019,7 @@ namespace cryptonote } const boost::posix_time::ptime start = boost::posix_time::microsec_clock::universal_time(); + context.m_last_request_time = start; m_core.prepare_handle_incoming_blocks(blocks); @@ -1108,7 +1121,7 @@ namespace cryptonote << timing_message); } } - } // if not DISCARD BLOCK + } if (should_download_next_span(context)) { @@ -1179,9 +1192,17 @@ skip: std::list hashes; boost::uuids::uuid span_connection_id; boost::posix_time::ptime request_time; - std::pair span = m_block_queue.get_next_span_if_scheduled(hashes, span_connection_id, request_time); + std::pair span; + + span = m_block_queue.get_start_gap_span(); + if (span.second > 0) + { + MDEBUG(context << " we should download it as there is a gap"); + return true; + } // if the next span is not scheduled (or there is none) + span = m_block_queue.get_next_span_if_scheduled(hashes, span_connection_id, request_time); if (span.second == 0) { // we might be in a weird case where there is a filled next span, @@ -1270,6 +1291,17 @@ skip: first = false; context.m_state = cryptonote_connection_context::state_standby; } + + // this needs doing after we went to standby, so the callback knows what to do + bool filled; + if (m_block_queue.has_next_span(context.m_connection_id, filled) && !filled) + { + MDEBUG(context << " we have the next span, and it is scheduled, resuming"); + ++context.m_callback_request_count; + m_p2p->request_callback(context); + return 1; + } + for (size_t n = 0; n < 50; ++n) { if (m_stopping) @@ -1289,9 +1321,8 @@ skip: size_t count = 0; const size_t count_limit = m_core.get_block_sync_size(m_core.get_current_blockchain_height()); std::pair span = std::make_pair(0, 0); - if (force_next_span) { - MDEBUG(context << " force_next_span is true, trying next span"); + MDEBUG(context << " checking for gap"); span = m_block_queue.get_start_gap_span(); if (span.second > 0) { @@ -1311,6 +1342,9 @@ skip: } MDEBUG(context << " we have the hashes for this gap"); } + } + if (force_next_span) + { if (span.second == 0) { std::list hashes; @@ -1360,7 +1394,12 @@ skip: for (const auto &hash: hashes) { req.blocks.push_back(hash); + ++count; context.m_requested_objects.insert(hash); + // that's atrocious O(n) wise, but this is rare + auto i = std::find(context.m_needed_objects.begin(), context.m_needed_objects.end(), hash); + if (i != context.m_needed_objects.end()) + context.m_needed_objects.erase(i); } } } @@ -1384,14 +1423,12 @@ skip: return false; } - std::list hashes; auto it = context.m_needed_objects.begin(); for (size_t n = 0; n < span.second; ++n) { req.blocks.push_back(*it); ++count; context.m_requested_objects.insert(*it); - hashes.push_back(*it); auto j = it++; context.m_needed_objects.erase(j); } @@ -1399,7 +1436,7 @@ skip: context.m_last_request_time = boost::posix_time::microsec_clock::universal_time(); LOG_PRINT_CCONTEXT_L1("-->>NOTIFY_REQUEST_GET_OBJECTS: blocks.size()=" << req.blocks.size() << ", txs.size()=" << req.txs.size() - << "requested blocks count=" << count << " / " << count_limit << " from " << span.first); + << "requested blocks count=" << count << " / " << count_limit << " from " << span.first << ", first hash " << req.blocks.front()); //epee::net_utils::network_throttle_manager::get_global_throttle_inreq().logger_handle_net("log/dr-monero/net/req-all.data", sec, get_avg_block_size()); post_notify(req, context); @@ -1582,8 +1619,15 @@ skip: { if (add_fail) m_p2p->add_host_fail(context.m_remote_address); + m_p2p->drop_connection(context); + m_block_queue.flush_spans(context.m_connection_id, flush_all_spans); + } + //------------------------------------------------------------------------------------------------------------------------ + template + void t_cryptonote_protocol_handler::on_connection_close(cryptonote_connection_context &context) + { uint64_t target = 0; m_p2p->for_each_connection([&](const connection_context& cntxt, nodetool::peerid_type peer_id, uint32_t support_flags) { if (cntxt.m_state >= cryptonote_connection_context::state_synchronizing && cntxt.m_connection_id != context.m_connection_id) @@ -1597,7 +1641,7 @@ skip: m_core.set_target_blockchain_height(target); } - m_block_queue.flush_spans(context.m_connection_id, flush_all_spans); + m_block_queue.flush_spans(context.m_connection_id, false); } //------------------------------------------------------------------------------------------------------------------------ diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index e179fc14..4dd7dbf8 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -1795,6 +1795,8 @@ namespace nodetool m_peerlist.remove_from_peer_anchor(na); } + m_payload_handler.on_connection_close(context); + MINFO("["<< epee::net_utils::print_connection_context(context) << "] CLOSE CONNECTION"); } From 8f8cc09ba19d66243701f2861e24b9018d6aa255 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 19 Aug 2017 14:35:47 +0100 Subject: [PATCH 02/10] contrib: add sync_info to rlwrap command set --- contrib/rlwrap/monerocommands_bitmonerod.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/contrib/rlwrap/monerocommands_bitmonerod.txt b/contrib/rlwrap/monerocommands_bitmonerod.txt index 9572e40f..c4f77b37 100644 --- a/contrib/rlwrap/monerocommands_bitmonerod.txt +++ b/contrib/rlwrap/monerocommands_bitmonerod.txt @@ -32,4 +32,5 @@ status stop_daemon stop_mining stop_save_graph +sync_info unban From 5524bc31510c8116e5e215635307ef973e6abe83 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 20 Aug 2017 21:15:53 +0100 Subject: [PATCH 03/10] print peer id in 0 padded hex for consistency --- contrib/epee/include/string_tools.h | 12 ++++++++++++ src/daemon/rpc_command_executor.cpp | 20 ++++---------------- src/p2p/net_node.inl | 12 ++++++------ src/p2p/p2p_protocol_defs.h | 7 +++++++ 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/contrib/epee/include/string_tools.h b/contrib/epee/include/string_tools.h index 258caa49..ce7b2fb8 100644 --- a/contrib/epee/include/string_tools.h +++ b/contrib/epee/include/string_tools.h @@ -314,6 +314,18 @@ POP_WARNINGS return str; } //---------------------------------------------------------------------------- + inline std::string pad_string(std::string s, size_t n, char c = ' ', bool prepend = false) + { + if (s.size() < n) + { + if (prepend) + s = std::string(n - s.size(), c) + s; + else + s.append(n - s.size(), c); + } + return s; + } + //---------------------------------------------------------------------------- template std::string pod_to_hex(const t_pod_type& s) { diff --git a/src/daemon/rpc_command_executor.cpp b/src/daemon/rpc_command_executor.cpp index bd2142ab..ca2403e3 100644 --- a/src/daemon/rpc_command_executor.cpp +++ b/src/daemon/rpc_command_executor.cpp @@ -113,18 +113,6 @@ namespace { return base; return base + " -- " + status; } - - std::string pad(std::string s, size_t n, char c = ' ', bool prepend = false) - { - if (s.size() < n) - { - if (prepend) - s = std::string(n - s.size(), c) + s; - else - s.append(n - s.size(), c); - } - return s; - } } t_rpc_command_executor::t_rpc_command_executor( @@ -497,7 +485,7 @@ bool t_rpc_command_executor::print_connections() { tools::msg_writer() //<< std::setw(30) << std::left << in_out << std::setw(30) << std::left << address - << std::setw(20) << pad(info.peer_id, 16, '0', true) + << std::setw(20) << epee::string_tools::pad_string(info.peer_id, 16, '0', true) << std::setw(20) << info.support_flags << std::setw(30) << std::to_string(info.recv_count) + "(" + std::to_string(info.recv_idle_time) + ")/" + std::to_string(info.send_count) + "(" + std::to_string(info.send_idle_time) + ")" << std::setw(25) << info.state @@ -1744,12 +1732,12 @@ bool t_rpc_command_executor::sync_info() tools::success_msg_writer() << std::to_string(res.peers.size()) << " peers"; for (const auto &p: res.peers) { - std::string address = pad(p.info.address, 24); + std::string address = epee::string_tools::pad_string(p.info.address, 24); uint64_t nblocks = 0, size = 0; 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 << " " << pad(p.info.peer_id, 16, '0', true) << " " << p.info.height << " " << p.info.current_download << " kB/s, " << nblocks << " blocks / " << size/1e6 << " MB queued"; + tools::success_msg_writer() << address << " " << epee::string_tools::pad_string(p.info.peer_id, 16, '0', true) << " " << p.info.height << " " << p.info.current_download << " kB/s, " << nblocks << " blocks / " << size/1e6 << " MB queued"; } uint64_t total_size = 0; @@ -1758,7 +1746,7 @@ bool t_rpc_command_executor::sync_info() tools::success_msg_writer() << std::to_string(res.spans.size()) << " spans, " << total_size/1e6 << " MB"; for (const auto &s: res.spans) { - std::string address = pad(s.remote_address, 24); + std::string address = epee::string_tools::pad_string(s.remote_address, 24); if (s.size == 0) { tools::success_msg_writer() << address << " " << s.nblocks << " (" << s.start_block_height << " - " << (s.start_block_height + s.nblocks - 1) << ") -"; diff --git a/src/p2p/net_node.inl b/src/p2p/net_node.inl index 4dd7dbf8..889cfaf9 100644 --- a/src/p2p/net_node.inl +++ b/src/p2p/net_node.inl @@ -1077,7 +1077,7 @@ namespace nodetool bool node_server::make_new_connection_from_anchor_peerlist(const std::vector& anchor_peerlist) { for (const auto& pe: anchor_peerlist) { - _note("Considering connecting (out) to peer: " << pe.id << " " << pe.adr.str()); + _note("Considering connecting (out) to peer: " << peerid_type(pe.id) << " " << pe.adr.str()); if(is_peer_used(pe)) { _note("Peer is used"); @@ -1092,7 +1092,7 @@ namespace nodetool continue; } - MDEBUG("Selected peer: " << pe.id << " " << pe.adr.str() + MDEBUG("Selected peer: " << peerid_to_string(pe.id) << " " << pe.adr.str() << "[peer_type=" << anchor << "] first_seen: " << epee::misc_utils::get_time_interval_string(time(NULL) - pe.first_seen)); @@ -1145,7 +1145,7 @@ namespace nodetool ++try_count; - _note("Considering connecting (out) to peer: " << pe.id << " " << pe.adr.str()); + _note("Considering connecting (out) to peer: " << peerid_to_string(pe.id) << " " << pe.adr.str()); if(is_peer_used(pe)) { _note("Peer is used"); @@ -1158,7 +1158,7 @@ namespace nodetool if(is_addr_recently_failed(pe.adr)) continue; - MDEBUG("Selected peer: " << pe.id << " " << pe.adr.str() + MDEBUG("Selected peer: " << peerid_to_string(pe.id) << " " << pe.adr.str() << "[peer_list=" << (use_white_list ? white : gray) << "] last_seen: " << (pe.last_seen ? epee::misc_utils::get_time_interval_string(time(NULL) - pe.last_seen) : "never")); @@ -1962,14 +1962,14 @@ namespace nodetool if (!success) { m_peerlist.remove_from_peer_gray(pe); - LOG_PRINT_L2("PEER EVICTED FROM GRAY PEER LIST IP address: " << pe.adr.host_str() << " Peer ID: " << std::hex << pe.id); + LOG_PRINT_L2("PEER EVICTED FROM GRAY PEER LIST IP address: " << pe.adr.host_str() << " Peer ID: " << peerid_type(pe.id)); return true; } m_peerlist.set_peer_just_seen(pe.id, pe.adr); - LOG_PRINT_L2("PEER PROMOTED TO WHITE PEER LIST IP address: " << pe.adr.host_str() << " Peer ID: " << std::hex << pe.id); + LOG_PRINT_L2("PEER PROMOTED TO WHITE PEER LIST IP address: " << pe.adr.host_str() << " Peer ID: " << peerid_type(pe.id)); return true; } diff --git a/src/p2p/p2p_protocol_defs.h b/src/p2p/p2p_protocol_defs.h index f38615de..f2b2cd1d 100644 --- a/src/p2p/p2p_protocol_defs.h +++ b/src/p2p/p2p_protocol_defs.h @@ -44,6 +44,13 @@ namespace nodetool typedef boost::uuids::uuid uuid; typedef uint64_t peerid_type; + static inline std::string peerid_to_string(peerid_type peer_id) + { + std::ostringstream s; + s << std::hex << peer_id; + return epee::string_tools::pad_string(s.str(), 16, '0', true); + } + #pragma pack (push, 1) struct network_address_old From 80794b311455a44ba9d47f116616170839f20f8a Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 20 Aug 2017 21:17:50 +0100 Subject: [PATCH 04/10] thread_group: set thread size to THREAD_STACK_SIZE --- src/common/thread_group.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/common/thread_group.cpp b/src/common/thread_group.cpp index 860d0b73..691a27a2 100644 --- a/src/common/thread_group.cpp +++ b/src/common/thread_group.cpp @@ -32,6 +32,7 @@ #include #include +#include "cryptonote_config.h" #include "common/util.h" namespace tools @@ -63,8 +64,10 @@ thread_group::data::data(std::size_t count) , has_work() , stop(false) { threads.reserve(count); + boost::thread::attributes attrs; + attrs.set_stack_size(THREAD_STACK_SIZE); while (count--) { - threads.push_back(boost::thread(&thread_group::data::run, this)); + threads.push_back(boost::thread(attrs, boost::bind(&thread_group::data::run, this))); } } From b5345ef4f08eee2b7e36853ca3efede44ab4a091 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 20 Aug 2017 21:19:29 +0100 Subject: [PATCH 05/10] crypto: use malloc instead of alloca --- src/crypto/crypto.cpp | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/src/crypto/crypto.cpp b/src/crypto/crypto.cpp index 1c7adff3..5fb670f8 100644 --- a/src/crypto/crypto.cpp +++ b/src/crypto/crypto.cpp @@ -36,18 +36,13 @@ #include #include #include +#include #include "common/varint.h" #include "warnings.h" #include "crypto.h" #include "hash.h" -#if !defined(__FreeBSD__) && !defined(__OpenBSD__) && !defined(__DragonFly__) - #include -#else - #include -#endif - namespace crypto { using std::abort; @@ -411,7 +406,9 @@ POP_WARNINGS ge_p3 image_unp; ge_dsmp image_pre; ec_scalar sum, k, h; - rs_comm *const buf = reinterpret_cast(alloca(rs_comm_size(pubs_count))); + boost::shared_ptr buf(reinterpret_cast(malloc(rs_comm_size(pubs_count))), free); + if (!buf) + abort(); assert(sec_index < pubs_count); #if !defined(NDEBUG) { @@ -459,7 +456,7 @@ POP_WARNINGS sc_add(&sum, &sum, &sig[i].c); } } - hash_to_scalar(buf, rs_comm_size(pubs_count), h); + hash_to_scalar(buf.get(), rs_comm_size(pubs_count), h); sc_sub(&sig[sec_index].c, &h, &sum); sc_mulsub(&sig[sec_index].r, &sig[sec_index].c, &sec, &k); } @@ -471,7 +468,9 @@ POP_WARNINGS ge_p3 image_unp; ge_dsmp image_pre; ec_scalar sum, h; - rs_comm *const buf = reinterpret_cast(alloca(rs_comm_size(pubs_count))); + boost::shared_ptr buf(reinterpret_cast(malloc(rs_comm_size(pubs_count))), free); + if (!buf) + return false; #if !defined(NDEBUG) for (i = 0; i < pubs_count; i++) { assert(check_key(*pubs[i])); @@ -499,7 +498,7 @@ POP_WARNINGS ge_tobytes(&buf->ab[i].b, &tmp2); sc_add(&sum, &sum, &sig[i].c); } - hash_to_scalar(buf, rs_comm_size(pubs_count), h); + hash_to_scalar(buf.get(), rs_comm_size(pubs_count), h); sc_sub(&h, &h, &sum); return sc_isnonzero(&h) == 0; } From 727e67cadaeea1a992326e1aec6de028bb791360 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Mon, 21 Aug 2017 21:55:10 +0100 Subject: [PATCH 06/10] cryptonote_protocol: print peer top height along with its version --- src/cryptonote_protocol/cryptonote_protocol_handler.inl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index 87690f1f..684c9484 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -268,7 +268,7 @@ namespace cryptonote const uint8_t version = m_core.get_ideal_hard_fork_version(hshd.current_height - 1); if (version >= 6 && version != hshd.top_version) { - LOG_DEBUG_CC(context, "Ignoring due to wrong top version " << (unsigned)hshd.top_version << ", expected " << (unsigned)version); + LOG_DEBUG_CC(context, "Ignoring due to wrong top version for block " << (hshd.current_height - 1) << ": " << (unsigned)hshd.top_version << ", expected " << (unsigned)version); return false; } From e2ad372b876c98528f33d49439003e81e88b84b4 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 22 Aug 2017 17:16:57 +0100 Subject: [PATCH 07/10] cryptonote_protocol: simplify and remove unnecessary casts --- .../cryptonote_protocol_handler.inl | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index 684c9484..f9becd7f 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -291,13 +291,14 @@ namespace cryptonote /* As I don't know if accessing hshd from core could be a good practice, I prefer pushing target height to the core at the same time it is pushed to the user. Nz. */ - m_core.set_target_blockchain_height(static_cast(hshd.current_height)); + m_core.set_target_blockchain_height((hshd.current_height)); int64_t diff = static_cast(hshd.current_height) - static_cast(m_core.get_current_blockchain_height()); - int64_t max_block_height = max(static_cast(hshd.current_height),static_cast(m_core.get_current_blockchain_height())); - int64_t last_block_v1 = m_core.get_testnet() ? 624633 : 1009826; - int64_t diff_v2 = max_block_height > last_block_v1 ? min(abs(diff), max_block_height - last_block_v1) : 0; + uint64_t abs_diff = std::abs(diff); + uint64_t max_block_height = max(hshd.current_height,m_core.get_current_blockchain_height()); + uint64_t last_block_v1 = m_core.get_testnet() ? 624633 : 1009826; + uint64_t diff_v2 = max_block_height > last_block_v1 ? min(abs_diff, max_block_height - last_block_v1) : 0; MCLOG(is_inital ? el::Level::Info : el::Level::Debug, "global", context << "Sync data returned a new top block candidate: " << m_core.get_current_blockchain_height() << " -> " << hshd.current_height - << " [Your node is " << std::abs(diff) << " blocks (" << ((abs(diff) - diff_v2) / (24 * 60 * 60 / DIFFICULTY_TARGET_V1)) + (diff_v2 / (24 * 60 * 60 / DIFFICULTY_TARGET_V2)) << " days) " + << " [Your node is " << abs_diff << " blocks (" << ((abs_diff - diff_v2) / (24 * 60 * 60 / DIFFICULTY_TARGET_V1)) + (diff_v2 / (24 * 60 * 60 / DIFFICULTY_TARGET_V2)) << " days) " << (0 <= diff ? std::string("behind") : std::string("ahead")) << "] " << ENDL << "SYNCHRONIZATION started"); } From cc81a3715560699adddba21fd7dd6d45be73454c Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Tue, 22 Aug 2017 17:17:19 +0100 Subject: [PATCH 08/10] cryptonote_protocol: update target height when syncing too --- src/cryptonote_protocol/cryptonote_protocol_handler.inl | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index f9becd7f..2cc1b0dd 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -1561,6 +1561,10 @@ skip: drop_connection(context, false, false); return 1; } + + if (arg.total_height > m_core.get_target_blockchain_height()) + m_core.set_target_blockchain_height(arg.total_height); + return 1; } //------------------------------------------------------------------------------------------------------------------------ From 317ab21a0317fdf38424d7d342e8bacc382c9a0a Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 23 Aug 2017 01:22:12 +0100 Subject: [PATCH 09/10] cryptonote_protocol: less strict check on top version on connect This allows peers who synced past a fork on the wrong height to reorg to the right chain after they updated their software to include the new version. --- src/cryptonote_protocol/cryptonote_protocol_handler.inl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index 2cc1b0dd..d4736fad 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -315,7 +315,7 @@ namespace cryptonote bool t_cryptonote_protocol_handler::get_payload_sync_data(CORE_SYNC_DATA& hshd) { m_core.get_blockchain_top(hshd.current_height, hshd.top_id); - hshd.top_version = m_core.get_hard_fork_version(hshd.current_height); + hshd.top_version = m_core.get_ideal_hard_fork_version(hshd.current_height); hshd.cumulative_difficulty = m_core.get_block_cumulative_difficulty(hshd.current_height); hshd.current_height +=1; return true; From df0cffede0d65debd8f486cfbfe820a9c6078f38 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 23 Aug 2017 12:31:56 +0100 Subject: [PATCH 10/10] cryptonote_protocol: warn if we see a higher top version we expect --- src/cryptonote_protocol/cryptonote_protocol_handler.inl | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index d4736fad..f27a5d7b 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -268,6 +268,8 @@ namespace cryptonote const uint8_t version = m_core.get_ideal_hard_fork_version(hshd.current_height - 1); if (version >= 6 && version != hshd.top_version) { + if (version < hshd.top_version) + MCLOG_RED(el::Level::Warning, "global", context << " peer claims higher version that we think - we may be forked from the network and a software upgrade may be needed"); LOG_DEBUG_CC(context, "Ignoring due to wrong top version for block " << (hshd.current_height - 1) << ": " << (unsigned)hshd.top_version << ", expected " << (unsigned)version); return false; }