Merge pull request #7039

9d7f473 cryptonote_core: dandelion - use local height or median height if syncing (xiphon)

10 files changed, 85 insertions, 18 deletions

diff --git a/src/cryptonote_core/cryptonote_core.cpp b/src/cryptonote_core/cryptonote_core.cpp
index bd138038d..42b1c6c84 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 0e4318256..da7a21b27 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
@@ -638,6 +638,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
       *
       * @note see 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<const cryptonote::blobdata> 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<connection_info> 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 20bcd7f3b..a72b7db79 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 bbec2ba9b..69df22a92 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<boost::uuids::uuid> get_out_connections(connections& p2p, uint64_t min_blockchain_height)
+    uint64_t get_median_remote_height(connections& p2p)
+    {
+        std::vector<uint64_t> 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<boost::uuids::uuid> get_out_connections(connections& p2p, uint64_t blockchain_height)
     {
       std::vector<boost::uuids::uuid> 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<boost::uuids::uuid> 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<blobdata>&& txs, const bool pad, const bool fluff)
     {
       NOTIFY_NEW_TRANSACTIONS::request request{};
@@ -527,7 +569,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");
@@ -631,7 +673,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())
@@ -667,7 +709,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}
         );
@@ -718,7 +760,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 128d51fb3..22638942d 100644
--- a/tests/unit_tests/levin.cpp
+++ b/tests/unit_tests/levin.cpp
@@ -120,7 +120,12 @@ namespace
     {
         std::map<cryptonote::relay_method, std::vector<cryptonote::blobdata>> 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;}