Keep checkpoint votes over p2p

Checkpoint votes were only going over quorumnet, which was quite broken
as they need to go out universally: random nodes piece together the
individual checkpoints to create checkpoints on their own, and so need
to see all of the votes.  With the current code only service nodes that
participated in the specific quorum ever saw the checkpoint votes.

This commit returns checkpoint vote distribution to distribution over
p2p.

We could, in theory, do checkpoint vote collection over quorumnet and
then relay the set of votes as a pack to reduce p2p traffic a bit, but
this wouldn't be a huge optimization since we'd still have to distribute
all the votes over p2p at some point anyway, so leaving that as a future
optimization for now.

(Obligations votes, in contrast, don't need to be distributed at all --
if a vote results in a state change, the quorum members themselves can
produce and distribute the state change tx).

src/cryptonote_core/cryptonote_core.cpp

@@ -262,12 +262,12 @@ namespace cryptonote
   }
   void *(*quorumnet_new)(core &, const std::string &bind);
   void (*quorumnet_delete)(void *&self);
-  void (*quorumnet_relay_votes)(void *self, const std::vector<service_nodes::quorum_vote_t> &);
+  void (*quorumnet_relay_obligation_votes)(void *self, const std::vector<service_nodes::quorum_vote_t> &);
   std::future<std::pair<blink_result, std::string>> (*quorumnet_send_blink)(void *self, const std::string &tx_blob);
   static bool init_core_callback_stubs() {
     quorumnet_new = [](core &, const std::string &) -> void * { need_core_init(); };
     quorumnet_delete = [](void *&) { need_core_init(); };
-    quorumnet_relay_votes = [](void *, const std::vector<service_nodes::quorum_vote_t> &) { need_core_init(); };
+    quorumnet_relay_obligation_votes = [](void *, const std::vector<service_nodes::quorum_vote_t> &) { need_core_init(); };
     quorumnet_send_blink = [](void *, const std::string &) -> std::future<std::pair<blink_result, std::string>> { need_core_init(); };
     return false;
   }
@@ -1661,26 +1661,27 @@ namespace cryptonote
   bool core::relay_service_node_votes()
   {
     auto height = get_current_blockchain_height();
-    auto qnet_begins = get_earliest_ideal_height_for_version(network_version_14_blink_lns);
+    auto hf_version = get_hard_fork_version(height);
 
-    auto votes = m_quorum_cop.get_relayable_votes(height);
-    if (votes.empty())
-      return true;
+    auto quorum_votes = m_quorum_cop.get_relayable_votes(height, hf_version, true);
+    auto p2p_votes    = m_quorum_cop.get_relayable_votes(height, hf_version, false);
+    if (!quorum_votes.empty())
+      quorumnet_relay_obligation_votes(m_quorumnet_obj, quorum_votes);
 
-    if (height >= qnet_begins) {
-      quorumnet_relay_votes(m_quorumnet_obj, votes);
-      m_quorum_cop.set_votes_relayed(votes);
-      return true;
+    if (!p2p_votes.empty())
+    {
+      NOTIFY_NEW_SERVICE_NODE_VOTE::request req{};
+      req.votes = std::move(p2p_votes);
+      cryptonote_connection_context fake_context{};
+      get_protocol()->relay_service_node_votes(req, fake_context);
     }
 
-    NOTIFY_NEW_SERVICE_NODE_VOTE::request req{};
-    req.votes = std::move(votes);
-    cryptonote_connection_context fake_context{};
-    if (get_protocol()->relay_service_node_votes(req, fake_context))
-      m_quorum_cop.set_votes_relayed(req.votes);
-
     return true;
   }
+  void core::set_service_node_votes_relayed(const std::vector<service_nodes::quorum_vote_t> &votes)
+  {
+    m_quorum_cop.set_votes_relayed(votes);
+  }
   //-----------------------------------------------------------------------------------------------
   bool core::get_block_template(block& b, const account_public_address& adr, difficulty_type& diffic, uint64_t& height, uint64_t& expected_reward, const blobdata& ex_nonce)
   {

src/cryptonote_core/cryptonote_core.h

@@ -85,7 +85,7 @@ namespace cryptonote
   // Stops the quorumnet listener; is expected to delete the object and reset the pointer to nullptr.
   extern void (*quorumnet_delete)(void *&self);
   // Relays votes via quorumnet.
-  extern void (*quorumnet_relay_votes)(void *self, const std::vector<service_nodes::quorum_vote_t> &votes);
+  extern void (*quorumnet_relay_obligation_votes)(void *self, const std::vector<service_nodes::quorum_vote_t> &votes);
   // Sends a blink tx to the current blink quorum, returns a future that can be used to wait for the
   // result.
   extern std::future<std::pair<blink_result, std::string>> (*quorumnet_send_blink)(void *self, const std::string &tx_blob);
@@ -930,6 +930,12 @@ namespace cryptonote
       */
      bool relay_service_node_votes();
 
+     /**
+      * @brief sets the given votes to relayed; generally called automatically when
+      * relay_service_node_votes() is called.
+      */
+     void set_service_node_votes_relayed(const std::vector<service_nodes::quorum_vote_t> &votes);
+
      /**
       * @brief Record if the service node has checkpointed at this point in time
       */

src/cryptonote_core/service_node_quorum_cop.cpp

@@ -187,9 +187,9 @@ namespace service_nodes
     m_vote_pool.set_relayed(relayed_votes);
   }
 
-  std::vector<quorum_vote_t> quorum_cop::get_relayable_votes(uint64_t current_height)
+  std::vector<quorum_vote_t> quorum_cop::get_relayable_votes(uint64_t current_height, uint8_t hf_version, bool quorum_relay)
   {
-    return m_vote_pool.get_relayable_votes(current_height);
+    return m_vote_pool.get_relayable_votes(current_height, hf_version, quorum_relay);
   }
 
   int find_index_in_quorum_group(std::vector<crypto::public_key> const &group, crypto::public_key const &my_pubkey)
@@ -535,7 +535,7 @@ namespace service_nodes
     return true;
   }
 
-  static bool handle_checkpoint_vote(cryptonote::core &core, const quorum_vote_t& vote, const std::vector<pool_vote_entry>& votes, const quorum& quorum)
+  static bool handle_checkpoint_vote(cryptonote::core& core, const quorum_vote_t& vote, const std::vector<pool_vote_entry>& votes, const quorum& quorum)
   {
     if (votes.size() < CHECKPOINT_MIN_VOTES)
     {

src/cryptonote_core/service_node_quorum_cop.h

@@ -99,7 +99,7 @@ namespace service_nodes
     void blockchain_detached(uint64_t height, bool by_pop_blocks) override;
 
     void                       set_votes_relayed  (std::vector<quorum_vote_t> const &relayed_votes);
-    std::vector<quorum_vote_t> get_relayable_votes(uint64_t current_height);
+    std::vector<quorum_vote_t> get_relayable_votes(uint64_t current_height, uint8_t hf_version, bool quorum_relay);
     bool                       handle_vote        (quorum_vote_t const &vote, cryptonote::vote_verification_context &vvc);
 
     static int64_t calculate_decommission_credit(const service_node_info &info, uint64_t current_height);

src/cryptonote_core/service_node_voting.cpp

@@ -435,7 +435,9 @@ namespace service_nodes
       return result;
 
     result = crypto::check_signature(hash, key, vote.signature);
-    if (!result)
+    if (result)
+      MDEBUG("Signature accepted for " << vote.type << " voter " << vote.index_in_group << "/" << key << " voting for worker " << vote.state_change.worker_index << " at height " << vote.block_height);
+    else
       vvc.m_signature_not_valid = true;
 
     return result;
@@ -500,7 +502,7 @@ namespace service_nodes
           result.push_back(vote_entry.vote);
   }
 
-  std::vector<quorum_vote_t> voting_pool::get_relayable_votes(uint64_t height) const
+  std::vector<quorum_vote_t> voting_pool::get_relayable_votes(uint64_t height, uint8_t hf_version, bool quorum_relay) const
   {
     CRITICAL_REGION_LOCAL(m_lock);
 
@@ -515,8 +517,16 @@ namespace service_nodes
     const uint64_t min_height = height > VOTE_LIFETIME ? height - VOTE_LIFETIME : 0;
 
     std::vector<quorum_vote_t> result;
-    append_relayable_votes(result, m_obligations_pool, max_last_sent, min_height);
-    append_relayable_votes(result, m_checkpoint_pool,  max_last_sent, min_height);
+
+    if (quorum_relay && hf_version < cryptonote::network_version_14_blink_lns)
+      return result; // no quorum relaying before HF14
+
+    if (hf_version < cryptonote::network_version_14_blink_lns || quorum_relay)
+      append_relayable_votes(result, m_obligations_pool, max_last_sent, min_height);
+
+    if (hf_version < cryptonote::network_version_14_blink_lns || !quorum_relay)
+      append_relayable_votes(result, m_checkpoint_pool,  max_last_sent, min_height);
+
     return result;
   }
 

src/cryptonote_core/service_node_voting.h

@@ -150,7 +150,11 @@ namespace service_nodes
     void                         set_relayed         (const std::vector<quorum_vote_t>& votes);
     void                         remove_expired_votes(uint64_t height);
     void                         remove_used_votes   (std::vector<cryptonote::transaction> const &txs, uint8_t hard_fork_version);
-    std::vector<quorum_vote_t>   get_relayable_votes (uint64_t height) const;
+
+    /// Returns relayable votes for either p2p (quorum_relay=false) or quorumnet
+    /// (quorum_relay=true).  Before HF14 everything goes via p2p; starting in HF14 obligation votes
+    /// go via quorumnet, checkpoints go via p2p.
+    std::vector<quorum_vote_t>   get_relayable_votes (uint64_t height, uint8_t hf_version, bool quorum_relay) const;
     bool                         received_checkpoint_vote(uint64_t height, size_t index_in_quorum) const;
 
   private:

src/cryptonote_protocol/cryptonote_protocol_handler.inl

@@ -2423,6 +2423,8 @@ skip:
   bool t_cryptonote_protocol_handler<t_core>::relay_service_node_votes(NOTIFY_NEW_SERVICE_NODE_VOTE::request& arg, cryptonote_connection_context& exclude_context)
   {
     bool result = relay_to_synchronized_peers<NOTIFY_NEW_SERVICE_NODE_VOTE>(arg, exclude_context);
+    if (result)
+      m_core.set_service_node_votes_relayed(arg.votes);
     return result;
   }
   //------------------------------------------------------------------------------------------------------------------------

src/cryptonote_protocol/quorumnet.cpp

@@ -476,24 +476,25 @@ quorum_vote_t deserialize_vote(const bt_value &v) {
     return vote;
 }
 
-void relay_votes(void *obj, const std::vector<service_nodes::quorum_vote_t> &votes) {
+void relay_obligation_votes(void *obj, const std::vector<service_nodes::quorum_vote_t> &votes) {
     auto &snw = *reinterpret_cast<SNNWrapper *>(obj);
 
     auto my_keys_ptr = snw.core.get_service_node_keys();
     assert(my_keys_ptr);
     const auto &my_keys = *my_keys_ptr;
 
-    // Loop twice: the first time we build up the set of remotes we need for all votes; then we look
-    // up their proofs -- which requires a potentially expensive lock -- to get the x25519 pubkey
-    // and port from the proof; then we do the sending in the second loop.
-    std::unordered_set<crypto::public_key> need_remotes;
-    int votes_relayed = 0;
     MDEBUG("Starting relay of " << votes.size() << " votes");
+    std::vector<service_nodes::quorum_vote_t> relayed_votes;
+    relayed_votes.reserve(votes.size());
     for (auto &vote : votes) {
-        auto quorum = snw.core.get_service_node_list().get_quorum(vote.type, vote.block_height);
+        if (vote.type != quorum_type::obligations) {
+            MERROR("Internal logic error: quorumnet asked to relay a " << vote.type << " vote, but should only be called with obligations votes");
+            continue;
+        }
 
+        auto quorum = snw.core.get_service_node_list().get_quorum(vote.type, vote.block_height);
         if (!quorum) {
-            MWARNING("Unable to relay vote: no testing quorum vote for type " << vote.type << " @ height " << vote.block_height);
+            MWARNING("Unable to relay vote: no " << vote.type << " quorum available for height " << vote.block_height);
             continue;
         }
 
@@ -511,16 +512,17 @@ void relay_votes(void *obj, const std::vector<service_nodes::quorum_vote_t> &vot
             continue;
         }
 
-        pinfo.relay_to_peers("vote", serialize_vote(vote));
-        votes_relayed++;
+        pinfo.relay_to_peers("vote_ob", serialize_vote(vote));
+        relayed_votes.push_back(vote);
     }
-    MDEBUG("Relayed " << votes_relayed << " votes");
+    MDEBUG("Relayed " << relayed_votes.size() << " votes");
+    snw.core.set_service_node_votes_relayed(relayed_votes);
 }
 
-void handle_vote(SNNetwork::message &m, void *self) {
+void handle_obligation_vote(SNNetwork::message &m, void *self) {
     auto &snw = *reinterpret_cast<SNNWrapper *>(self);
 
-    MDEBUG("Received a relayed vote from " << as_hex(m.pubkey));
+    MDEBUG("Received a relayed obligation vote from " << as_hex(m.pubkey));
 
     if (m.data.size() != 1) {
         MINFO("Ignoring vote: expected 1 data part, not " << m.data.size());
@@ -528,14 +530,20 @@ void handle_vote(SNNetwork::message &m, void *self) {
     }
 
     try {
-        quorum_vote_t vote = deserialize_vote(m.data[0]);
+        std::vector<quorum_vote_t> vvote;
+        vvote.push_back(deserialize_vote(m.data[0]));
+        auto& vote = vvote.back();
 
+        if (vote.type != quorum_type::obligations) {
+            MWARNING("Received invalid non-obligations vote via quorumnet; ignoring");
+            return;
+        }
         if (vote.block_height > snw.core.get_current_blockchain_height()) {
             MDEBUG("Ignoring vote: block height " << vote.block_height << " is too high");
             return;
         }
 
-        cryptonote::vote_verification_context vvc = {};
+        cryptonote::vote_verification_context vvc{};
         snw.core.add_service_node_vote(vote, vvc);
         if (vvc.m_verification_failed)
         {
@@ -544,7 +552,7 @@ void handle_vote(SNNetwork::message &m, void *self) {
         }
 
         if (vvc.m_added_to_pool)
-            relay_votes(self, {{vote}});
+            relay_obligation_votes(self, std::move(vvote));
     }
     catch (const std::exception &e) {
         MWARNING("Deserialization of vote from " << as_hex(m.pubkey) << " failed: " << e.what());
@@ -1449,11 +1457,11 @@ void handle_blink_success(SNNetwork::message &m, void *self) {
 void init_core_callbacks() {
     cryptonote::quorumnet_new = new_snnwrapper;
     cryptonote::quorumnet_delete = delete_snnwrapper;
-    cryptonote::quorumnet_relay_votes = relay_votes;
+    cryptonote::quorumnet_relay_obligation_votes = relay_obligation_votes;
     cryptonote::quorumnet_send_blink = send_blink;
 
-    // Receives a vote
-    SNNetwork::register_quorum_command("vote", handle_vote);
+    // Receives an obligation vote
+    SNNetwork::register_quorum_command("vote_ob", handle_obligation_vote);
 
     // Receives a new blink tx submission from an external node, or forward from other quorum
     // members who received it from an external node.