From a9cd5d914e032f2ae6ce56e6b48f0f47d3a56ad7 Mon Sep 17 00:00:00 2001 From: xiphon Date: Fri, 20 Nov 2020 11:37:19 +0000 Subject: [PATCH] cryptonote_core: dandelion - use local height or median height if syncing --- src/cryptonote_core/cryptonote_core.cpp | 5 ++ src/cryptonote_core/cryptonote_core.h | 11 +++- src/cryptonote_core/i_core_events.h | 3 +- .../cryptonote_protocol_handler.h | 2 +- .../cryptonote_protocol_handler.inl | 6 +- .../cryptonote_protocol_handler_common.h | 5 ++ src/cryptonote_protocol/levin_notify.cpp | 58 ++++++++++++++++--- tests/core_proxy/core_proxy.h | 3 +- tests/unit_tests/levin.cpp | 7 ++- tests/unit_tests/node_server.cpp | 3 +- 10 files changed, 85 insertions(+), 18 deletions(-) diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp index fef411a0c..694f2cc34 100644 --- a/src/cryptonote_core/cryptonote_core.cpp +++ b/src/cryptonote_core/cryptonote_core.cpp @@ -1510,6 +1510,11 @@ namespace cryptonote return true; } //----------------------------------------------------------------------------------------------- + bool core::is_synchronized() const + { + return m_pprotocol != nullptr && m_pprotocol->is_synchronized(); + } + //----------------------------------------------------------------------------------------------- void core::on_synchronized() { m_miner.on_synchronized(); diff --git a/src/cryptonote_core/cryptonote_core.h b/src/cryptonote_core/cryptonote_core.h index 7c578ac51..1459d3bee 100644 --- a/src/cryptonote_core/cryptonote_core.h +++ b/src/cryptonote_core/cryptonote_core.h @@ -329,7 +329,7 @@ namespace cryptonote * * @note see Blockchain::get_current_blockchain_height() */ - uint64_t get_current_blockchain_height() const; + virtual uint64_t get_current_blockchain_height() const final; /** * @brief get the hash and height of the most recent block @@ -637,6 +637,13 @@ namespace cryptonote */ std::string print_pool(bool short_format) const; + /** + * @brief gets the core synchronization status + * + * @return core synchronization status + */ + virtual bool is_synchronized() const final; + /** * @copydoc miner::on_synchronized * @@ -663,7 +670,7 @@ namespace cryptonote * * @param target_blockchain_height the target height */ - virtual uint64_t get_target_blockchain_height() const override; + uint64_t get_target_blockchain_height() const; /** * @brief returns the newest hardfork version known to the blockchain diff --git a/src/cryptonote_core/i_core_events.h b/src/cryptonote_core/i_core_events.h index addb659ab..5d00858b5 100644 --- a/src/cryptonote_core/i_core_events.h +++ b/src/cryptonote_core/i_core_events.h @@ -39,7 +39,8 @@ namespace cryptonote virtual ~i_core_events() noexcept {} - virtual uint64_t get_target_blockchain_height() const = 0; + virtual uint64_t get_current_blockchain_height() const = 0; + virtual bool is_synchronized() const = 0; virtual void on_transactions_relayed(epee::span tx_blobs, relay_method tx_relay) = 0; }; } diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.h b/src/cryptonote_protocol/cryptonote_protocol_handler.h index 89860fe41..61aac6d81 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.h +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.h @@ -104,7 +104,7 @@ namespace cryptonote bool get_payload_sync_data(CORE_SYNC_DATA& hshd); bool on_callback(cryptonote_connection_context& context); t_core& get_core(){return m_core;} - bool is_synchronized(){return m_synchronized;} + virtual bool is_synchronized() const final { return !no_sync() && m_synchronized; } void log_connections(); std::list get_connections(); const block_queue &get_block_queue() const { return m_block_queue; } diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler.inl b/src/cryptonote_protocol/cryptonote_protocol_handler.inl index a8073e091..bb7b54b74 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler.inl +++ b/src/cryptonote_protocol/cryptonote_protocol_handler.inl @@ -430,7 +430,7 @@ namespace cryptonote MLOGIF_P2P_MESSAGE(crypto::hash hash; cryptonote::block b; bool ret = cryptonote::parse_and_validate_block_from_blob(arg.b.block, b, &hash);, ret, "Received NOTIFY_NEW_BLOCK " << hash << " (height " << arg.current_blockchain_height << ", " << arg.b.txs.size() << " txes)"); if(context.m_state != cryptonote_connection_context::state_normal) return 1; - if(!is_synchronized() || m_no_sync) // can happen if a peer connection goes to normal but another thread still hasn't finished adding queued blocks + if(!is_synchronized()) // can happen if a peer connection goes to normal but another thread still hasn't finished adding queued blocks { LOG_DEBUG_CC(context, "Received new block while syncing, ignored"); return 1; @@ -501,7 +501,7 @@ namespace cryptonote MLOGIF_P2P_MESSAGE(crypto::hash hash; cryptonote::block b; bool ret = cryptonote::parse_and_validate_block_from_blob(arg.b.block, b, &hash);, ret, "Received NOTIFY_NEW_FLUFFY_BLOCK " << hash << " (height " << arg.current_blockchain_height << ", " << arg.b.txs.size() << " txes)"); if(context.m_state != cryptonote_connection_context::state_normal) return 1; - if(!is_synchronized() || m_no_sync) // can happen if a peer connection goes to normal but another thread still hasn't finished adding queued blocks + if(!is_synchronized()) // can happen if a peer connection goes to normal but another thread still hasn't finished adding queued blocks { LOG_DEBUG_CC(context, "Received new block while syncing, ignored"); return 1; @@ -929,7 +929,7 @@ namespace cryptonote // while syncing, core will lock for a long time, so we ignore // those txes as they aren't really needed anyway, and avoid a // long block before replying - if(!is_synchronized() || m_no_sync) + if(!is_synchronized()) { LOG_DEBUG_CC(context, "Received new tx while syncing, ignored"); return 1; diff --git a/src/cryptonote_protocol/cryptonote_protocol_handler_common.h b/src/cryptonote_protocol/cryptonote_protocol_handler_common.h index 1c7635fd8..79c2edf1d 100644 --- a/src/cryptonote_protocol/cryptonote_protocol_handler_common.h +++ b/src/cryptonote_protocol/cryptonote_protocol_handler_common.h @@ -40,6 +40,7 @@ namespace cryptonote /************************************************************************/ struct i_cryptonote_protocol { + virtual bool is_synchronized() const = 0; virtual bool relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context)=0; virtual bool relay_transactions(NOTIFY_NEW_TRANSACTIONS::request& arg, const boost::uuids::uuid& source, epee::net_utils::zone zone, relay_method tx_relay)=0; //virtual bool request_objects(NOTIFY_REQUEST_GET_OBJECTS::request& arg, cryptonote_connection_context& context)=0; @@ -50,6 +51,10 @@ namespace cryptonote /************************************************************************/ struct cryptonote_protocol_stub: public i_cryptonote_protocol { + virtual bool is_synchronized() const final + { + return false; + } virtual bool relay_block(NOTIFY_NEW_BLOCK::request& arg, cryptonote_connection_context& exclude_context) { return false; diff --git a/src/cryptonote_protocol/levin_notify.cpp b/src/cryptonote_protocol/levin_notify.cpp index 634750cab..3f2835469 100644 --- a/src/cryptonote_protocol/levin_notify.cpp +++ b/src/cryptonote_protocol/levin_notify.cpp @@ -105,8 +105,44 @@ namespace levin return std::chrono::steady_clock::duration{crypto::rand_range(rep(0), range.count())}; } - //! \return Outgoing connections supporting fragments in `connections` filtered by remote blockchain height. - std::vector get_out_connections(connections& p2p, uint64_t min_blockchain_height) + uint64_t get_median_remote_height(connections& p2p) + { + std::vector remote_heights; + remote_heights.reserve(connection_id_reserve_size); + p2p.foreach_connection([&remote_heights] (detail::p2p_context& context) { + if (!context.m_is_income) + { + remote_heights.emplace_back(context.m_remote_blockchain_height); + } + return true; + }); + + if (remote_heights.empty()) + { + return 0; + } + + const size_t n = remote_heights.size() / 2; + std::sort(remote_heights.begin(), remote_heights.end()); + if (remote_heights.size() % 2 != 0) + { + return remote_heights[n]; + } + return remote_heights[n-1]; + } + + uint64_t get_blockchain_height(connections& p2p, const i_core_events* core) + { + const uint64_t local_blockchain_height = core->get_current_blockchain_height(); + if (core->is_synchronized()) + { + return local_blockchain_height; + } + return std::max(local_blockchain_height, get_median_remote_height(p2p)); + } + + //! \return Outgoing connections supporting fragments in `connections` filtered by blockchain height. + std::vector get_out_connections(connections& p2p, uint64_t blockchain_height) { std::vector outs; outs.reserve(connection_id_reserve_size); @@ -115,15 +151,21 @@ namespace levin the reserve call so a strand is not used. Investigate if there is lots of waiting in here. */ - p2p.foreach_connection([&outs, min_blockchain_height] (detail::p2p_context& context) { - if (!context.m_is_income && context.m_remote_blockchain_height >= min_blockchain_height) + p2p.foreach_connection([&outs, blockchain_height] (detail::p2p_context& context) { + if (!context.m_is_income && context.m_remote_blockchain_height >= blockchain_height) outs.emplace_back(context.m_connection_id); return true; }); + MDEBUG("Found " << outs.size() << " out connections having height >= " << blockchain_height); return outs; } + std::vector get_out_connections(connections& p2p, const i_core_events* core) + { + return get_out_connections(p2p, get_blockchain_height(p2p, core)); + } + std::string make_tx_payload(std::vector&& txs, const bool pad, const bool fluff) { NOTIFY_NEW_TRANSACTIONS::request request{}; @@ -545,7 +587,7 @@ namespace levin } // connection list may be outdated, try again - update_channels::run(zone_, get_out_connections(*zone_->p2p, core_->get_target_blockchain_height())); + update_channels::run(zone_, get_out_connections(*zone_->p2p, core_)); } MERROR("Unable to send transaction(s) via Dandelion++ stem"); @@ -646,7 +688,7 @@ namespace levin { channel.active = nullptr; channel.connection = boost::uuids::nil_uuid(); - auto height = core_->get_target_blockchain_height(); + auto height = get_blockchain_height(*zone_->p2p, core_); auto connections = get_out_connections(*zone_->p2p, height); if (connections.empty()) @@ -682,7 +724,7 @@ namespace levin const bool fluffing = crypto::rand_idx(unsigned(100)) < CRYPTONOTE_DANDELIONPP_FLUFF_PROBABILITY; const auto start = std::chrono::steady_clock::now(); - auto connections = get_out_connections(*(zone_->p2p), core_->get_target_blockchain_height()); + auto connections = get_out_connections(*(zone_->p2p), core_); zone_->strand.dispatch( change_channels{zone_, net::dandelionpp::connection_map{std::move(connections), count_}, fluffing} ); @@ -733,7 +775,7 @@ namespace levin return; zone_->strand.dispatch( - update_channels{zone_, get_out_connections(*(zone_->p2p), core_->get_target_blockchain_height())} + update_channels{zone_, get_out_connections(*(zone_->p2p), core_)} ); } diff --git a/tests/core_proxy/core_proxy.h b/tests/core_proxy/core_proxy.h index 4a41b5002..ecfcc18ae 100644 --- a/tests/core_proxy/core_proxy.h +++ b/tests/core_proxy/core_proxy.h @@ -66,9 +66,10 @@ namespace tests public: + virtual bool is_synchronized() const final { return true; } void on_synchronized(){} void safesyncmode(const bool){} - uint64_t get_current_blockchain_height(){return 1;} + virtual uint64_t get_current_blockchain_height() const final {return 1;} void set_target_blockchain_height(uint64_t) {} bool init(const boost::program_options::variables_map& vm); bool deinit(){return true;} diff --git a/tests/unit_tests/levin.cpp b/tests/unit_tests/levin.cpp index 9ff68d927..e7f9e126f 100644 --- a/tests/unit_tests/levin.cpp +++ b/tests/unit_tests/levin.cpp @@ -120,7 +120,12 @@ namespace { std::map> relayed_; - uint64_t get_target_blockchain_height() const override + virtual bool is_synchronized() const final + { + return false; + } + + virtual uint64_t get_current_blockchain_height() const final { return 0; } diff --git a/tests/unit_tests/node_server.cpp b/tests/unit_tests/node_server.cpp index 588e91d02..0191e5aa4 100644 --- a/tests/unit_tests/node_server.cpp +++ b/tests/unit_tests/node_server.cpp @@ -47,9 +47,10 @@ namespace cryptonote { class test_core : public cryptonote::i_core_events { public: + virtual bool is_synchronized() const final { return true; } void on_synchronized(){} void safesyncmode(const bool){} - uint64_t get_current_blockchain_height() const {return 1;} + virtual uint64_t get_current_blockchain_height() const final {return 1;} void set_target_blockchain_height(uint64_t) {} bool init(const boost::program_options::variables_map& vm) {return true ;} bool deinit(){return true;}