Vote serialization compatibility fix (#984)

quorum_vote_t's were serialized as blob data, which is highly
non-portable (probably isn't the same on non-amd64 arches) and broke
between 5.x and 6.x because `signature` is aligned now (which changed
its offset and thus broke 5.x <-> 6.x vote transmission).

This adds a hack to write votes into a block of memory compatible with
AMD64 5.x nodes up until HF14, then switches to a new command that fully
serializes starting at the hard fork (after which we can remove the
backwards compatibility stuff added here).

contrib/epee/include/serialization/keyvalue_serialization.h

@@ -119,6 +119,14 @@ do { \
 #define KV_SERIALIZE_OPT(variable,default_value)          KV_SERIALIZE_OPT_N(variable, #variable, default_value)
 /// same as KV_SERIALIZE_OPT, but will set `explicitly_set` to true if non-default value found
 #define KV_SERIALIZE_OPT2(variable,default_value)          KV_SERIALIZE_OPT_N2(variable, #variable, default_value)
+#define KV_SERIALIZE_ENUM(enum_) do { \
+  using enum_t = std::remove_const_t<decltype(this_ref.enum_)>; \
+  using int_t = std::underlying_type_t<enum_t>; \
+  int_t int_value = is_store ? static_cast<int_t>(this_ref.enum_) : 0; \
+  epee::serialization::selector<is_store>::serialize(int_value, stg, hparent_section, #enum_); \
+  if (!is_store) \
+    const_cast<enum_t&>(this_ref.enum_) = static_cast<enum_t>(int_value); \
+} while(0);
 
 }
 

src/cryptonote_core/cryptonote_core.cpp

@@ -1670,10 +1670,20 @@ namespace cryptonote
 
     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);
+      if (hf_version >= cryptonote::network_version_14_blink_lns)
+      {
+        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);
+      }
+      else
+      {
+        NOTIFY_NEW_SERVICE_NODE_VOTE_OLD::request req{};
+        req.votes = std::move(p2p_votes);
+        cryptonote_connection_context fake_context{};
+        get_protocol()->relay_service_node_votes(req, fake_context);
+      }
     }
 
     return true;

src/cryptonote_core/service_node_voting.cpp

@@ -623,5 +623,61 @@ namespace service_nodes
 
     return false;
   }
+
+  void vote_to_blob(const quorum_vote_t& vote, unsigned char blob[])
+  {
+    blob[0] = vote.version;
+    blob[1] = static_cast<uint8_t>(vote.type);
+    for (size_t i = 2; i < 8; i++)
+      blob[i] = 0; // padding
+    {
+      uint64_t height = boost::endian::native_to_little(vote.block_height);
+      std::memcpy(&blob[8], &height, 8);
+    }
+    blob[16] = static_cast<uint8_t>(vote.group);
+    blob[17] = 0; // padding
+    {
+      uint16_t iig = boost::endian::native_to_little(vote.index_in_group);
+      std::memcpy(&blob[18], &iig, 2);
+    }
+    std::memcpy(&blob[20], &vote.signature, 64);
+    for (size_t i = 84; i < 88; i++)
+      blob[i] = 0; // padding
+    if (vote.type == quorum_type::checkpointing)
+    {
+      std::memcpy(&blob[84], &vote.checkpoint, 32);
+      for (size_t i = 116; i < 120; i++)
+        blob[i] = 0; // padding
+    }
+    else
+    {
+      uint16_t wi = boost::endian::native_to_little(vote.state_change.worker_index);
+      uint16_t st = boost::endian::native_to_little(static_cast<uint16_t>(vote.state_change.state));
+      std::memcpy(&blob[84], &wi, 2);
+      std::memcpy(&blob[86], &st, 2);
+      for (size_t i = 88; i < 120; i++)
+        blob[i] = 0;
+    }
+  }
+
+  void blob_to_vote(const unsigned char blob[], quorum_vote_t& vote)
+  {
+    vote.version = blob[0];
+    vote.type = static_cast<quorum_type>(blob[1]);
+    std::memcpy(&vote.block_height, &blob[8], 8); boost::endian::little_to_native_inplace(vote.block_height);
+    vote.group = static_cast<quorum_group>(blob[16]);
+    std::memcpy(&vote.index_in_group, &blob[18], 2); boost::endian::little_to_native_inplace(vote.index_in_group);
+    std::memcpy(&vote.signature, &blob[20], 64);
+    if (vote.type == quorum_type::checkpointing)
+    {
+      std::memcpy(&vote.checkpoint, &blob[84], 32);
+    }
+    else
+    {
+      std::memcpy(&vote.state_change.worker_index, &blob[84], 2); boost::endian::little_to_native_inplace(vote.state_change.worker_index);
+      uint16_t st;
+      std::memcpy(&st, &blob[86], 2); vote.state_change.state = static_cast<new_state>(boost::endian::little_to_native(st));
+    }
+  }
 }; // namespace service_nodes
 

src/cryptonote_core/service_node_voting.h

@@ -78,6 +78,11 @@ namespace service_nodes
   enum struct quorum_group : uint8_t { invalid, validator, worker, _count };
   struct quorum_vote_t
   {
+    // Note: This type has various padding and alignment and was mistakingly serialized as a blob
+    // (padding and all, and not portable).  To remain compatible, we have to reproduce the blob
+    // data byte-for-byte as expected in the loki 5.x struct memory layout on AMD64, via the
+    // vote_to_blob functions below.
+
     uint8_t           version = 0;
     quorum_type       type;
     uint64_t          block_height;
@@ -91,6 +96,24 @@ namespace service_nodes
       checkpoint_vote   checkpoint;
     };
 
+    BEGIN_KV_SERIALIZE_MAP()
+      KV_SERIALIZE(version)
+      KV_SERIALIZE_ENUM(type)
+      KV_SERIALIZE(block_height)
+      KV_SERIALIZE_ENUM(group)
+      KV_SERIALIZE(index_in_group)
+      KV_SERIALIZE_VAL_POD_AS_BLOB(signature)
+      if (this_ref.type == quorum_type::checkpointing)
+      {
+        KV_SERIALIZE_VAL_POD_AS_BLOB_N(checkpoint.block_hash, "checkpoint")
+      }
+      else
+      {
+        KV_SERIALIZE(state_change.worker_index)
+        KV_SERIALIZE_ENUM(state_change.state)
+      }
+    END_KV_SERIALIZE_MAP()
+
    // TODO(loki): idk exactly if I want to implement this, but need for core tests to compile. Not sure I care about serializing for core tests at all.
    private:
     friend class boost::serialization::access;
@@ -98,6 +121,9 @@ namespace service_nodes
     void serialize(Archive &ar, const unsigned int /*version*/) { }
   };
 
+  void vote_to_blob(const quorum_vote_t& vote, unsigned char blob[]);
+  void blob_to_vote(const unsigned char blob[], quorum_vote_t& vote);
+
   struct voter_to_signature
   {
     voter_to_signature() = default;

src/cryptonote_protocol/cryptonote_protocol_defs.h

@@ -42,6 +42,8 @@ namespace service_nodes
 {
   struct legacy_deregister_vote;
   struct quorum_vote_t;
+  void vote_to_blob(const quorum_vote_t& vote, unsigned char blob[]);
+  void blob_to_vote(const unsigned char blob[], quorum_vote_t& vote);
 };
 
 namespace cryptonote
@@ -348,14 +350,38 @@ namespace cryptonote
   /************************************************************************/
   /*                                                                      */
   /************************************************************************/
-  struct NOTIFY_NEW_SERVICE_NODE_VOTE
+  // TODO(loki) - remove this after HF14
+  struct NOTIFY_NEW_SERVICE_NODE_VOTE_OLD
   {
+  private:
+    struct alignas(8) blob { unsigned char data[120]; };
+  public:
     const static int ID = BC_COMMANDS_POOL_BASE + 12;
     struct request
     {
       std::vector<service_nodes::quorum_vote_t> votes;
+
       BEGIN_KV_SERIALIZE_MAP()
-        KV_SERIALIZE_CONTAINER_POD_AS_BLOB(votes)
+        auto &votes = this_ref.votes;
+
+        std::vector<blob> vote_blobs;
+        if (is_store) // means: "serializing".  Hate epee 2x: this is both badly named, and gross template parameter misuse.
+        {
+          vote_blobs.resize(votes.size());
+          for (size_t i = 0; i < votes.size(); i++)
+            vote_to_blob(votes[i], vote_blobs[i].data);
+        }
+        // Hate epee some more: this time for deficient serialization macros:
+        //KV_SERIALIZE_CONTAINER_POD_AS_BLOB_N(vote_blobs, "votes");
+        epee::serialization::selector<is_store>::serialize_stl_container_pod_val_as_blob(vote_blobs, stg, hparent_section, "votes");
+        if (!is_store)
+        {
+          // Hate epee even more:
+          auto &v = const_cast<std::vector<service_nodes::quorum_vote_t>&>(votes);
+          v.resize(vote_blobs.size());
+          for (size_t i = 0; i < v.size(); i++)
+            blob_to_vote(vote_blobs[i].data, v[i]);
+        }
       END_KV_SERIALIZE_MAP()
     };
   };
@@ -403,4 +429,17 @@ namespace cryptonote
       END_KV_SERIALIZE_MAP()
     };
   };
+
+  struct NOTIFY_NEW_SERVICE_NODE_VOTE
+  {
+    const static int ID = BC_COMMANDS_POOL_BASE + 16;
+    struct request
+    {
+      std::vector<service_nodes::quorum_vote_t> votes;
+
+      BEGIN_KV_SERIALIZE_MAP()
+        KV_SERIALIZE(votes)
+      END_KV_SERIALIZE_MAP()
+    };
+  };
 }

src/cryptonote_protocol/cryptonote_protocol_handler.h

@@ -80,6 +80,7 @@ namespace cryptonote
       HANDLE_NOTIFY_T2(NOTIFY_REQUEST_FLUFFY_MISSING_TX, &cryptonote_protocol_handler::handle_request_fluffy_missing_tx)
       HANDLE_NOTIFY_T2(NOTIFY_UPTIME_PROOF, &cryptonote_protocol_handler::handle_uptime_proof)
       HANDLE_NOTIFY_T2(NOTIFY_NEW_SERVICE_NODE_VOTE, &cryptonote_protocol_handler::handle_notify_new_service_node_vote)
+      HANDLE_NOTIFY_T2(NOTIFY_NEW_SERVICE_NODE_VOTE_OLD, &cryptonote_protocol_handler::handle_notify_new_service_node_vote_old)
       HANDLE_NOTIFY_T2(NOTIFY_REQUEST_BLOCK_BLINKS, &cryptonote_protocol_handler::handle_request_block_blinks)
       HANDLE_NOTIFY_T2(NOTIFY_RESPONSE_BLOCK_BLINKS, &cryptonote_protocol_handler::handle_response_block_blinks)
     END_INVOKE_MAP2()
@@ -118,6 +119,7 @@ namespace cryptonote
     int handle_request_fluffy_missing_tx(int command, NOTIFY_REQUEST_FLUFFY_MISSING_TX::request& arg, cryptonote_connection_context& context);
     int handle_uptime_proof(int command, NOTIFY_UPTIME_PROOF::request& arg, cryptonote_connection_context& context);
     int handle_notify_new_service_node_vote(int command, NOTIFY_NEW_SERVICE_NODE_VOTE::request& arg, cryptonote_connection_context& context);
+    int handle_notify_new_service_node_vote_old(int command, NOTIFY_NEW_SERVICE_NODE_VOTE_OLD::request& arg, cryptonote_connection_context& context);
     int handle_request_block_blinks(int command, NOTIFY_REQUEST_BLOCK_BLINKS::request& arg, cryptonote_connection_context& context);
     int handle_response_block_blinks(int command, NOTIFY_RESPONSE_BLOCK_BLINKS::request& arg, cryptonote_connection_context& context);
 
@@ -152,6 +154,7 @@ namespace cryptonote
     virtual bool relay_transactions(NOTIFY_NEW_TRANSACTIONS::request& arg, cryptonote_connection_context& exclude_context);
     virtual bool relay_uptime_proof(NOTIFY_UPTIME_PROOF::request& arg, cryptonote_connection_context& exclude_context);
     virtual bool relay_service_node_votes(NOTIFY_NEW_SERVICE_NODE_VOTE::request& arg, cryptonote_connection_context& exclude_context);
+    virtual bool relay_service_node_votes(NOTIFY_NEW_SERVICE_NODE_VOTE_OLD::request& arg, cryptonote_connection_context& exclude_context);
     //----------------------------------------------------------------------------------
     //bool get_payload_sync_data(HANDSHAKE_DATA::request& hshd, cryptonote_connection_context& context);
     bool should_drop_connection(cryptonote_connection_context& context, uint32_t next_stripe);

src/cryptonote_protocol/cryptonote_protocol_handler.inl

@@ -869,6 +869,7 @@ namespace cryptonote
     }
     return 1;
   }
+
   //------------------------------------------------------------------------------------------------------------------------  
   template<class t_core>
   int t_cryptonote_protocol_handler<t_core>::handle_notify_new_service_node_vote(int command, NOTIFY_NEW_SERVICE_NODE_VOTE::request& arg, cryptonote_connection_context& context)
@@ -911,6 +912,51 @@ namespace cryptonote
 
     return 1;
   }
+
+  //------------------------------------------------------------------------------------------------------------------------  
+  // TODO: delete this after HF14
+  template<class t_core>
+  int t_cryptonote_protocol_handler<t_core>::handle_notify_new_service_node_vote_old(int command, NOTIFY_NEW_SERVICE_NODE_VOTE_OLD::request& arg, cryptonote_connection_context& context)
+  {
+    MLOG_P2P_MESSAGE("Received NOTIFY_NEW_SERVICE_NODE_VOTE_OLD (" << arg.votes.size() << " txes)");
+
+    if(context.m_state != cryptonote_connection_context::state_normal)
+      return 1;
+
+    if(!is_synchronized())
+    {
+      LOG_DEBUG_CC(context, "Received new service node vote while syncing, ignored");
+      return 1;
+    }
+
+    for(auto it = arg.votes.begin(); it != arg.votes.end();)
+    {
+      cryptonote::vote_verification_context vvc = {};
+      m_core.add_service_node_vote(*it, vvc);
+
+      if (vvc.m_verification_failed)
+      {
+        LOG_PRINT_CCONTEXT_L1("Vote type: " << it->type << ", verification failed, dropping connection");
+        drop_connection(context, false /*add_fail*/, false /*flush_all_spans i.e. delete cached block data from this peer*/);
+        return 1;
+      }
+
+      if (vvc.m_added_to_pool)
+      {
+        it++;
+      }
+      else
+      {
+        it = arg.votes.erase(it);
+      }
+    }
+
+    if (arg.votes.size())
+      relay_service_node_votes(arg, context);
+
+    return 1;
+  }
+
   //------------------------------------------------------------------------------------------------------------------------  
   template<class t_core>
   int t_cryptonote_protocol_handler<t_core>::handle_request_fluffy_missing_tx(int command, NOTIFY_REQUEST_FLUFFY_MISSING_TX::request& arg, cryptonote_connection_context& context)
@@ -2427,6 +2473,14 @@ skip:
       m_core.set_service_node_votes_relayed(arg.votes);
     return result;
   }
+  template<class t_core>
+  bool t_cryptonote_protocol_handler<t_core>::relay_service_node_votes(NOTIFY_NEW_SERVICE_NODE_VOTE_OLD::request& arg, cryptonote_connection_context& exclude_context)
+  {
+    bool result = relay_to_synchronized_peers<NOTIFY_NEW_SERVICE_NODE_VOTE_OLD>(arg, exclude_context);
+    if (result)
+      m_core.set_service_node_votes_relayed(arg.votes);
+    return result;
+  }
   //------------------------------------------------------------------------------------------------------------------------
   template<class t_core>
   bool t_cryptonote_protocol_handler<t_core>::relay_transactions(NOTIFY_NEW_TRANSACTIONS::request& arg, cryptonote_connection_context& exclude_context)

src/cryptonote_protocol/cryptonote_protocol_handler_common.h

@@ -45,6 +45,7 @@ namespace cryptonote
     virtual bool relay_uptime_proof(NOTIFY_UPTIME_PROOF::request& arg, cryptonote_connection_context& exclude_context)=0;
     //virtual bool request_objects(NOTIFY_REQUEST_GET_OBJECTS::request& arg, cryptonote_connection_context& context)=0;
     virtual bool relay_service_node_votes(NOTIFY_NEW_SERVICE_NODE_VOTE::request& arg, cryptonote_connection_context& exclude_context)=0;
+    virtual bool relay_service_node_votes(NOTIFY_NEW_SERVICE_NODE_VOTE_OLD::request& arg, cryptonote_connection_context& exclude_context)=0;
   };
 
   /************************************************************************/
@@ -64,6 +65,10 @@ namespace cryptonote
     {
       return false;
     }
+    virtual bool relay_service_node_votes(NOTIFY_NEW_SERVICE_NODE_VOTE_OLD::request& arg, cryptonote_connection_context& exclude_context)
+    {
+      return false;
+    }
     virtual bool relay_uptime_proof(NOTIFY_UPTIME_PROOF::request& arg, cryptonote_connection_context& exclude_context)
     {
       return false;