From f9b5b521e82b9dc08325bc65f25f8e4e446270fc Mon Sep 17 00:00:00 2001
From: moneromooo-monero <moneromooo-monero@users.noreply.github.com>
Date: Tue, 12 Jan 2021 18:24:55 +0000
Subject: [PATCH] fix serialization being different on mac

On Mac, size_t is a distinct type from uint64_t, and some
types (in wallet cache as well as cold/hot wallet transfer
data) use pairs/containers with size_t as fields. Mac would
save those as full size, while other platforms would save
them as varints. Might apply to other platforms where the
types are distinct.

There's a nasty hack for backward compatibility, which can
go after a couple forks.
---
 src/cryptonote_core/cryptonote_tx_utils.h |  4 ++--
 src/serialization/binary_archive.h        |  9 ++++++++-
 src/serialization/container.h             | 23 ++++++++++++++---------
 src/serialization/json_archive.h          |  2 ++
 src/serialization/pair.h                  | 19 ++++++++++++++++---
 src/wallet/wallet2.cpp                    | 20 +++++++++++++++-----
 src/wallet/wallet2.h                      | 10 +++++-----
 src/wallet/wallet_rpc_server.cpp          |  2 +-
 tests/fuzz/cold-outputs.cpp               |  2 +-
 9 files changed, 64 insertions(+), 27 deletions(-)

diff --git a/src/cryptonote_core/cryptonote_tx_utils.h b/src/cryptonote_core/cryptonote_tx_utils.h
index dbdf409b5..73cdd31cd 100644
--- a/src/cryptonote_core/cryptonote_tx_utils.h
+++ b/src/cryptonote_core/cryptonote_tx_utils.h
@@ -44,10 +44,10 @@ namespace cryptonote
     typedef std::pair<uint64_t, rct::ctkey> output_entry;
 
     std::vector<output_entry> outputs;  //index + key + optional ringct commitment
-    size_t real_output;                 //index in outputs vector of real output_entry
+    uint64_t real_output;               //index in outputs vector of real output_entry
     crypto::public_key real_out_tx_key; //incoming real tx public key
     std::vector<crypto::public_key> real_out_additional_tx_keys; //incoming real tx additional public keys
-    size_t real_output_in_tx_index;     //index in transaction outputs vector
+    uint64_t real_output_in_tx_index;   //index in transaction outputs vector
     uint64_t amount;                    //money
     bool rct;                           //true if the output is rct
     rct::key mask;                      //ringct amount mask
diff --git a/src/serialization/binary_archive.h b/src/serialization/binary_archive.h
index 80104c8f7..49ca8aa57 100644
--- a/src/serialization/binary_archive.h
+++ b/src/serialization/binary_archive.h
@@ -98,7 +98,7 @@ template <>
 struct binary_archive<false> : public binary_archive_base<std::istream, false>
 {
 
-  explicit binary_archive(stream_type &s) : base_type(s) {
+  explicit binary_archive(stream_type &s) : base_type(s), varint_bug_backward_compatibility_(false) {
     stream_type::pos_type pos = stream_.tellg();
     stream_.seekg(0, std::ios_base::end);
     eof_pos_ = stream_.tellg();
@@ -173,8 +173,13 @@ struct binary_archive<false> : public binary_archive_base<std::istream, false>
     assert(stream_.tellg() <= eof_pos_);
     return eof_pos_ - stream_.tellg();
   }
+
+  void enable_varint_bug_backward_compatibility() { varint_bug_backward_compatibility_ = true; }
+  bool varint_bug_backward_compatibility_enabled() const { return varint_bug_backward_compatibility_; }
+
 protected:
   std::streamoff eof_pos_;
+  bool varint_bug_backward_compatibility_;
 };
 
 template <>
@@ -227,6 +232,8 @@ struct binary_archive<true> : public binary_archive_base<std::ostream, true>
   void write_variant_tag(variant_tag_type t) {
     serialize_int(t);
   }
+
+  bool varint_bug_backward_compatibility_enabled() const { return false; }
 };
 
 POP_WARNINGS
diff --git a/src/serialization/container.h b/src/serialization/container.h
index d5e75bb4f..a4997c8ae 100644
--- a/src/serialization/container.h
+++ b/src/serialization/container.h
@@ -32,22 +32,27 @@ namespace serialization
 {
   namespace detail
   {
+    template<typename T>
+    inline constexpr bool use_container_varint() noexcept
+    {
+      return std::is_integral<T>::value && std::is_unsigned<T>::value && sizeof(T) > 1;
+    }
+
     template <typename Archive, class T>
-    bool serialize_container_element(Archive& ar, T& e)
+    typename std::enable_if<!use_container_varint<T>(), bool>::type
+    serialize_container_element(Archive& ar, T& e)
     {
       return ::do_serialize(ar, e);
     }
 
-    template <typename Archive>
-    bool serialize_container_element(Archive& ar, uint32_t& e)
+    template<typename Archive, typename T>
+    typename std::enable_if<use_container_varint<T>(), bool>::type
+    serialize_container_element(Archive& ar, T& e)
     {
-      ar.serialize_varint(e);
-      return true;
-    }
+      static constexpr const bool previously_varint = std::is_same<uint64_t, T>() || std::is_same<uint32_t, T>();
 
-    template <typename Archive>
-    bool serialize_container_element(Archive& ar, uint64_t& e)
-    {
+      if (!previously_varint && ar.varint_bug_backward_compatibility_enabled() && !typename Archive::is_saving())
+        return ::do_serialize(ar, e);
       ar.serialize_varint(e);
       return true;
     }
diff --git a/src/serialization/json_archive.h b/src/serialization/json_archive.h
index 50dd5bbd0..3f98b5101 100644
--- a/src/serialization/json_archive.h
+++ b/src/serialization/json_archive.h
@@ -84,6 +84,8 @@ struct json_archive_base
   void end_variant() { end_object(); }
   Stream &stream() { return stream_; }
 
+  bool varint_bug_backward_compatibility_enabled() const { return false; }
+
 protected:
   void make_indent()
   {
diff --git a/src/serialization/pair.h b/src/serialization/pair.h
index 18280d837..44aafa04d 100644
--- a/src/serialization/pair.h
+++ b/src/serialization/pair.h
@@ -30,21 +30,34 @@
 
 #pragma once
 #include <memory>
+#include <boost/type_traits/make_unsigned.hpp>
 #include "serialization.h"
 
 namespace serialization
 {
   namespace detail
   {
+    template<typename T>
+    inline constexpr bool use_pair_varint() noexcept
+    {
+      return std::is_integral<T>::value && std::is_unsigned<T>::value && sizeof(T) > 1;
+    }
+
     template <typename Archive, class T>
-    bool serialize_pair_element(Archive& ar, T& e)
+    typename std::enable_if<!use_pair_varint<T>(), bool>::type
+    serialize_pair_element(Archive& ar, T& e)
     {
       return ::do_serialize(ar, e);
     }
 
-    template <typename Archive>
-    bool serialize_pair_element(Archive& ar, uint64_t& e)
+    template<typename Archive, typename T>
+    typename std::enable_if<use_pair_varint<T>(), bool>::type
+    serialize_pair_element(Archive& ar, T& e)
     {
+      static constexpr const bool previously_varint = std::is_same<uint64_t, T>();
+
+      if (!previously_varint && ar.varint_bug_backward_compatibility_enabled() && !typename Archive::is_saving())
+        return ::do_serialize(ar, e);
       ar.serialize_varint(e);
       return true;
     }
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index e298eca53..206bafade 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -5706,6 +5706,16 @@ void wallet2::load(const std::string& wallet_, const epee::wipeable_string& pass
           if (::serialization::serialize(ar, *this))
             if (::serialization::check_stream_state(ar))
               loaded = true;
+          if (!loaded)
+          {
+            std::stringstream iss;
+            iss << cache_data;
+            binary_archive<false> ar(iss);
+            ar.enable_varint_bug_backward_compatibility();
+            if (::serialization::serialize(ar, *this))
+              if (::serialization::check_stream_state(ar))
+                loaded = true;
+          }
         }
         catch(...) { }
 
@@ -12528,7 +12538,7 @@ crypto::public_key wallet2::get_tx_pub_key_from_received_outs(const tools::walle
 bool wallet2::export_key_images(const std::string &filename, bool all) const
 {
   PERF_TIMER(export_key_images);
-  std::pair<size_t, std::vector<std::pair<crypto::key_image, crypto::signature>>> ski = export_key_images(all);
+  std::pair<uint64_t, std::vector<std::pair<crypto::key_image, crypto::signature>>> ski = export_key_images(all);
   std::string magic(KEY_IMAGE_EXPORT_FILE_MAGIC, strlen(KEY_IMAGE_EXPORT_FILE_MAGIC));
   const cryptonote::account_public_address &keys = get_account().get_keys().m_account_address;
   const uint32_t offset = ski.first;
@@ -12555,7 +12565,7 @@ bool wallet2::export_key_images(const std::string &filename, bool all) const
 }
 
 //----------------------------------------------------------------------------------------------------
-std::pair<size_t, std::vector<std::pair<crypto::key_image, crypto::signature>>> wallet2::export_key_images(bool all) const
+std::pair<uint64_t, std::vector<std::pair<crypto::key_image, crypto::signature>>> wallet2::export_key_images(bool all) const
 {
   PERF_TIMER(export_key_images_raw);
   std::vector<std::pair<crypto::key_image, crypto::signature>> ski;
@@ -13052,7 +13062,7 @@ void wallet2::import_blockchain(const std::tuple<size_t, crypto::hash, std::vect
   m_last_block_reward = cryptonote::get_outs_money_amount(genesis.miner_tx);
 }
 //----------------------------------------------------------------------------------------------------
-std::pair<size_t, std::vector<tools::wallet2::transfer_details>> wallet2::export_outputs(bool all) const
+std::pair<uint64_t, std::vector<tools::wallet2::transfer_details>> wallet2::export_outputs(bool all) const
 {
   PERF_TIMER(export_outputs);
   std::vector<tools::wallet2::transfer_details> outs;
@@ -13092,7 +13102,7 @@ std::string wallet2::export_outputs_to_str(bool all) const
   return magic + ciphertext;
 }
 //----------------------------------------------------------------------------------------------------
-size_t wallet2::import_outputs(const std::pair<size_t, std::vector<tools::wallet2::transfer_details>> &outputs)
+size_t wallet2::import_outputs(const std::pair<uint64_t, std::vector<tools::wallet2::transfer_details>> &outputs)
 {
   PERF_TIMER(import_outputs);
 
@@ -13198,7 +13208,7 @@ size_t wallet2::import_outputs_from_str(const std::string &outputs_st)
   try
   {
     std::string body(data, headerlen);
-    std::pair<size_t, std::vector<tools::wallet2::transfer_details>> outputs;
+    std::pair<uint64_t, std::vector<tools::wallet2::transfer_details>> outputs;
     try
     {
       std::stringstream iss;
diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index e5a5136a4..5f0303329 100644
--- a/src/wallet/wallet2.h
+++ b/src/wallet/wallet2.h
@@ -327,7 +327,7 @@ private:
       uint64_t m_block_height;
       cryptonote::transaction_prefix m_tx;
       crypto::hash m_txid;
-      size_t m_internal_output_index;
+      uint64_t m_internal_output_index;
       uint64_t m_global_output_index;
       bool m_spent;
       bool m_frozen;
@@ -338,7 +338,7 @@ private:
       bool m_rct;
       bool m_key_image_known;
       bool m_key_image_request; // view wallets: we want to request it; cold wallets: it was requested
-      size_t m_pk_index;
+      uint64_t m_pk_index;
       cryptonote::subaddress_index m_subaddr_index;
       bool m_key_image_partial;
       std::vector<rct::key> m_multisig_k;
@@ -1372,9 +1372,9 @@ private:
     bool verify_with_public_key(const std::string &data, const crypto::public_key &public_key, const std::string &signature) const;
 
     // Import/Export wallet data
-    std::pair<size_t, std::vector<tools::wallet2::transfer_details>> export_outputs(bool all = false) const;
+    std::pair<uint64_t, std::vector<tools::wallet2::transfer_details>> export_outputs(bool all = false) const;
     std::string export_outputs_to_str(bool all = false) const;
-    size_t import_outputs(const std::pair<size_t, std::vector<tools::wallet2::transfer_details>> &outputs);
+    size_t import_outputs(const std::pair<uint64_t, std::vector<tools::wallet2::transfer_details>> &outputs);
     size_t import_outputs_from_str(const std::string &outputs_st);
     payment_container export_payments() const;
     void import_payments(const payment_container &payments);
@@ -1382,7 +1382,7 @@ private:
     std::tuple<size_t, crypto::hash, std::vector<crypto::hash>> export_blockchain() const;
     void import_blockchain(const std::tuple<size_t, crypto::hash, std::vector<crypto::hash>> &bc);
     bool export_key_images(const std::string &filename, bool all = false) const;
-    std::pair<size_t, std::vector<std::pair<crypto::key_image, crypto::signature>>> export_key_images(bool all = false) const;
+    std::pair<uint64_t, std::vector<std::pair<crypto::key_image, crypto::signature>>> export_key_images(bool all = false) const;
     uint64_t import_key_images(const std::vector<std::pair<crypto::key_image, crypto::signature>> &signed_key_images, size_t offset, uint64_t &spent, uint64_t &unspent, bool check_spent = true);
     uint64_t import_key_images(const std::string &filename, uint64_t &spent, uint64_t &unspent);
     bool import_key_images(std::vector<crypto::key_image> key_images, size_t offset=0, boost::optional<std::unordered_set<size_t>> selected_transfers=boost::none);
diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp
index 327a189ca..30abd81df 100644
--- a/src/wallet/wallet_rpc_server.cpp
+++ b/src/wallet/wallet_rpc_server.cpp
@@ -2701,7 +2701,7 @@ namespace tools
     if (!m_wallet) return not_open(er);
     try
     {
-      std::pair<size_t, std::vector<std::pair<crypto::key_image, crypto::signature>>> ski = m_wallet->export_key_images(req.all);
+      std::pair<uint64_t, std::vector<std::pair<crypto::key_image, crypto::signature>>> ski = m_wallet->export_key_images(req.all);
       res.offset = ski.first;
       res.signed_key_images.resize(ski.second.size());
       for (size_t n = 0; n < ski.second.size(); ++n)
diff --git a/tests/fuzz/cold-outputs.cpp b/tests/fuzz/cold-outputs.cpp
index 2698a36ba..bd298eb71 100644
--- a/tests/fuzz/cold-outputs.cpp
+++ b/tests/fuzz/cold-outputs.cpp
@@ -51,7 +51,7 @@ END_INIT_SIMPLE_FUZZER()
 
 BEGIN_SIMPLE_FUZZER()
   std::string s((const char*)buf, len);
-  std::pair<size_t, std::vector<tools::wallet2::transfer_details>> outputs;
+  std::pair<uint64_t, std::vector<tools::wallet2::transfer_details>> outputs;
   std::stringstream iss;
   iss << s;
   binary_archive<false> ar(iss);