From 2525200185e6b470c6e6cec3c3c8044a0eb0ecc0 Mon Sep 17 00:00:00 2001
From: jeffro256 <jeffro256@tutanota.com>
Date: Mon, 27 Nov 2023 19:28:08 -0600
Subject: [PATCH] serialization: remove container wrappers and serialize
 directly

Some downstream code (most notably PR https://github.com/UkoeHB/monero/pull/25) wants to use the src/serialization lib
for storing information persistently. When one builds classes/machines wishing to serialize containers, they must use
the `serializable_*` container classes. In this case, this makes the Seraphis library code unnecessarily tightly coupled
with the src/serialization code since one cannot swap out their type of storage format without major refactoring of class
field types. By serializing STL containers directly, we can abstract the serialization details away, making for much
cleaner design. Also small bonus side effect of this change is that STL containers with custom Comparators, Allocators,
and Hashers are serializable. `std::multimap` is added to the list of serializable containers.

Depends upon https://github.com/monero-project/monero/pull/9069.
---
 src/rpc/rpc_payment.h          |  8 +--
 src/serialization/container.h  | 29 ++++++++--
 src/serialization/containers.h | 99 ++++++----------------------------
 src/wallet/wallet2.cpp         | 10 ++--
 src/wallet/wallet2.h           | 60 ++++++++++-----------
 5 files changed, 81 insertions(+), 125 deletions(-)

diff --git a/src/rpc/rpc_payment.h b/src/rpc/rpc_payment.h
index 4ead5a344..a4cc6db57 100644
--- a/src/rpc/rpc_payment.h
+++ b/src/rpc/rpc_payment.h
@@ -148,8 +148,8 @@ namespace cryptonote
     template <class t_archive>
     inline void serialize(t_archive &a, const unsigned int ver)
     {
-      a & m_client_info.parent();
-      a & m_hashrate.parent();
+      a & m_client_info;
+      a & m_hashrate;
       a & m_credits_total;
       a & m_credits_used;
       a & m_nonces_good;
@@ -177,9 +177,9 @@ namespace cryptonote
     cryptonote::account_public_address m_address;
     uint64_t m_diff;
     uint64_t m_credits_per_hash_found;
-    serializable_unordered_map<crypto::public_key, client_info> m_client_info;
+    std::unordered_map<crypto::public_key, client_info> m_client_info;
     std::string m_directory;
-    serializable_map<uint64_t, uint64_t> m_hashrate;
+    std::map<uint64_t, uint64_t> m_hashrate;
     uint64_t m_credits_total;
     uint64_t m_credits_used;
     uint64_t m_nonces_good;
diff --git a/src/serialization/container.h b/src/serialization/container.h
index 393c66399..6f0c48069 100644
--- a/src/serialization/container.h
+++ b/src/serialization/container.h
@@ -57,8 +57,28 @@ namespace serialization
       return true;
     }
 
-    template <typename C>
-    void do_reserve(C &c, size_t N) {}
+    //! @brief Add an element to a container, inserting at the back if applicable.
+    template <class Container>
+    auto do_add(Container &c, typename Container::value_type &&e) -> decltype(c.emplace_back(e))
+    { return c.emplace_back(e); }
+    template <class Container>
+    auto do_add(Container &c, typename Container::value_type &&e) -> decltype(c.emplace(e))
+    { return c.emplace(e); }
+
+    //! @brief Reserve space for N elements if applicable for container.
+    template<typename... C>
+    void do_reserve(const C&...) {}
+    template<typename C>
+    auto do_reserve(C &c, std::size_t N) -> decltype(c.reserve(N)) { return c.reserve(N); }
+
+    // The value_type of STL map-like containers come in the form std::pair<const K, V>.
+    // Since we can't {de}serialize const types in this lib, we must convert this to std::pair<K, V>
+    template <class Container, typename = void>
+    struct serializable_value_type
+    { using type = typename Container::value_type; };
+    template <class Container>
+    struct serializable_value_type<Container, std::conditional_t<false, typename Container::mapped_type, void>>
+    { using type = std::pair<typename Container::key_type, typename Container::mapped_type>; };
   }
 }
 
@@ -82,7 +102,7 @@ bool do_serialize_container(Archive<false> &ar, C &v)
   for (size_t i = 0; i < cnt; i++) {
     if (i > 0)
       ar.delimit_array();
-    typename C::value_type e;
+    typename ::serialization::detail::serializable_value_type<C>::type e;
     if (!::serialization::detail::serialize_container_element(ar, e))
       return false;
     ::serialization::detail::do_add(v, std::move(e));
@@ -104,7 +124,8 @@ bool do_serialize_container(Archive<true> &ar, C &v)
       return false;
     if (i != v.begin())
       ar.delimit_array();
-    if(!::serialization::detail::serialize_container_element(ar, (typename C::value_type&)*i))
+    using serializable_value_type = typename ::serialization::detail::serializable_value_type<C>::type;
+    if(!::serialization::detail::serialize_container_element(ar, (serializable_value_type&)*i))
       return false;
     if (!ar.good())
       return false;
diff --git a/src/serialization/containers.h b/src/serialization/containers.h
index 65e6359b2..28eaa50fb 100644
--- a/src/serialization/containers.h
+++ b/src/serialization/containers.h
@@ -38,91 +38,26 @@
 #include <set>
 #include "serialization.h"
 
-template <template <bool> class Archive, class T> bool do_serialize(Archive<false> &ar, std::vector<T> &v);
-template <template <bool> class Archive, class T> bool do_serialize(Archive<true> &ar, std::vector<T> &v);
-
-template <template <bool> class Archive, class T> bool do_serialize(Archive<false> &ar, std::deque<T> &v);
-template <template <bool> class Archive, class T> bool do_serialize(Archive<true> &ar, std::deque<T> &v);
-
-template<typename K, typename V>
-class serializable_unordered_map: public std::unordered_map<K, V>
-{
-public:
-  typedef typename std::pair<K, V> value_type;
-  typename std::unordered_map<K, V> &parent() { return *this; }
-};
-
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<false> &ar, serializable_unordered_map<K, V> &v);
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<true> &ar, serializable_unordered_map<K, V> &v);
-
-template<typename K, typename V>
-class serializable_map: public std::map<K, V>
-{
-public:
-  typedef typename std::pair<K, V> value_type;
-  typename std::map<K, V> &parent() { return *this; }
-};
-
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<false> &ar, serializable_map<K, V> &v);
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<true> &ar, serializable_map<K, V> &v);
-
-template<typename K, typename V>
-class serializable_unordered_multimap: public std::unordered_multimap<K, V>
-{
-public:
-  typedef typename std::pair<K, V> value_type;
-  typename std::unordered_multimap<K, V> &parent() { return *this; }
-};
-
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<false> &ar, serializable_unordered_multimap<K, V> &v);
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<true> &ar, serializable_unordered_multimap<K, V> &v);
-
-template <template <bool> class Archive, class T> bool do_serialize(Archive<false> &ar, std::unordered_set<T> &v);
-template <template <bool> class Archive, class T> bool do_serialize(Archive<true> &ar, std::unordered_set<T> &v);
-
-template <template <bool> class Archive, class T> bool do_serialize(Archive<false> &ar, std::set<T> &v);
-template <template <bool> class Archive, class T> bool do_serialize(Archive<true> &ar, std::set<T> &v);
-
 namespace serialization
 {
-  namespace detail
-  {
-    template <typename T> void do_reserve(std::vector<T> &c, size_t N) { c.reserve(N); }
-    template <typename T> void do_add(std::vector<T> &c, T &&e) { c.emplace_back(std::forward<T>(e)); }
-
-    template <typename T> void do_add(std::deque<T> &c, T &&e) { c.emplace_back(std::forward<T>(e)); }
-
-    template <typename K, typename V> void do_add(serializable_unordered_map<K, V> &c, std::pair<K, V> &&e) { c.insert(std::forward<std::pair<K, V>>(e)); }
-
-    template <typename K, typename V> void do_add(serializable_map<K, V> &c, std::pair<K, V> &&e) { c.insert(std::forward<std::pair<K, V>>(e)); }
-
-    template <typename K, typename V> void do_add(serializable_unordered_multimap<K, V> &c, std::pair<K, V> &&e) { c.insert(std::forward<std::pair<K, V>>(e)); }
-
-    template <typename T> void do_add(std::unordered_set<T> &c, T &&e) { c.insert(std::forward<T>(e)); }
-
-    template <typename T> void do_add(std::set<T> &c, T &&e) { c.insert(std::forward<T>(e)); }
-  }
+    //! @brief Is this type a STL-like container?
+    //! To add a new container to be serialized, partially specialize the template is_container like so:
+    template <typename T>     struct is_container:                                 std::false_type {};
+    template <typename... TA> struct is_container<std::deque<TA...>>:              std::true_type {};
+    template <typename... TA> struct is_container<std::map<TA...>>:                std::true_type {};
+    template <typename... TA> struct is_container<std::multimap<TA...>>:           std::true_type {};
+    template <typename... TA> struct is_container<std::set<TA...>>:                std::true_type {};
+    template <typename... TA> struct is_container<std::unordered_map<TA...>>:      std::true_type {};
+    template <typename... TA> struct is_container<std::unordered_multimap<TA...>>: std::true_type {};
+    template <typename... TA> struct is_container<std::unordered_set<TA...>>:      std::true_type {};
+    template <typename... TA> struct is_container<std::vector<TA...>>:             std::true_type {};
 }
 
 #include "container.h"
 
-template <template <bool> class Archive, class T> bool do_serialize(Archive<false> &ar, std::vector<T> &v) { return do_serialize_container(ar, v); }
-template <template <bool> class Archive, class T> bool do_serialize(Archive<true> &ar, std::vector<T> &v) { return do_serialize_container(ar, v); }
-
-template <template <bool> class Archive, class T> bool do_serialize(Archive<false> &ar, std::deque<T> &v) { return do_serialize_container(ar, v); }
-template <template <bool> class Archive, class T> bool do_serialize(Archive<true> &ar, std::deque<T> &v) { return do_serialize_container(ar, v); }
-
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<false> &ar, serializable_unordered_map<K, V> &v) { return do_serialize_container(ar, v); }
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<true> &ar, serializable_unordered_map<K, V> &v) { return do_serialize_container(ar, v); }
-
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<false> &ar, serializable_map<K, V> &v) { return do_serialize_container(ar, v); }
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<true> &ar, serializable_map<K, V> &v) { return do_serialize_container(ar, v); }
-
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<false> &ar, serializable_unordered_multimap<K, V> &v) { return do_serialize_container(ar, v); }
-template <template <bool> class Archive, typename K, typename V> bool do_serialize(Archive<true> &ar, serializable_unordered_multimap<K, V> &v) { return do_serialize_container(ar, v); }
-
-template <template <bool> class Archive, class T> bool do_serialize(Archive<false> &ar, std::unordered_set<T> &v) { return do_serialize_container(ar, v); }
-template <template <bool> class Archive, class T> bool do_serialize(Archive<true> &ar, std::unordered_set<T> &v) { return do_serialize_container(ar, v); }
-
-template <template <bool> class Archive, class T> bool do_serialize(Archive<false> &ar, std::set<T> &v) { return do_serialize_container(ar, v); }
-template <template <bool> class Archive, class T> bool do_serialize(Archive<true> &ar, std::set<T> &v) { return do_serialize_container(ar, v); }
+template <class Archive, class Container>
+std::enable_if_t<::serialization::is_container<Container>::value, bool>
+do_serialize(Archive &ar, Container &c)
+{
+    return ::do_serialize_container(ar, c);
+}
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index 743a49ccb..dedb0343f 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -1821,7 +1821,7 @@ void reattach_blockchain(hashchain &blockchain, wallet2::detached_blockchain_dat
 }
 //----------------------------------------------------------------------------------------------------
 bool has_nonrequested_tx_at_height_or_above_requested(uint64_t height, const std::unordered_set<crypto::hash> &requested_txids, const wallet2::transfer_container &transfers,
-    const wallet2::payment_container &payments, const serializable_unordered_map<crypto::hash, wallet2::confirmed_transfer_details> &confirmed_txs)
+    const wallet2::payment_container &payments, const std::unordered_map<crypto::hash, wallet2::confirmed_transfer_details> &confirmed_txs)
 {
   for (const auto &td : transfers)
     if (td.m_block_height >= height && requested_txids.find(td.m_txid) == requested_txids.end())
@@ -11985,7 +11985,7 @@ std::string wallet2::get_reserve_proof(const boost::optional<std::pair<uint32_t,
   }
 
   // collect all subaddress spend keys that received those outputs and generate their signatures
-  serializable_unordered_map<crypto::public_key, crypto::signature> subaddr_spendkeys;
+  std::unordered_map<crypto::public_key, crypto::signature> subaddr_spendkeys;
   for (const cryptonote::subaddress_index &index : subaddr_indices)
   {
     crypto::secret_key subaddr_spend_skey = m_account.get_keys().m_spend_secret_key;
@@ -12030,7 +12030,7 @@ bool wallet2::check_reserve_proof(const cryptonote::account_public_address &addr
 
   bool loaded = false;
   std::vector<reserve_proof_entry> proofs;
-  serializable_unordered_map<crypto::public_key, crypto::signature> subaddr_spendkeys;
+  std::unordered_map<crypto::public_key, crypto::signature> subaddr_spendkeys;
   try
   {
     binary_archive<false> ar{epee::strspan<std::uint8_t>(sig_decoded)};
@@ -12044,7 +12044,7 @@ bool wallet2::check_reserve_proof(const cryptonote::account_public_address &addr
   {
     std::istringstream iss(sig_decoded);
     boost::archive::portable_binary_iarchive ar(iss);
-    ar >> proofs >> subaddr_spendkeys.parent();
+    ar >> proofs >> subaddr_spendkeys;
   }
 
   THROW_WALLET_EXCEPTION_IF(subaddr_spendkeys.count(address.m_spend_public_key) == 0, error::wallet_internal_error,
@@ -12288,7 +12288,7 @@ std::string wallet2::get_description() const
   return "";
 }
 
-const std::pair<serializable_map<std::string, std::string>, std::vector<std::string>>& wallet2::get_account_tags()
+const std::pair<std::map<std::string, std::string>, std::vector<std::string>>& wallet2::get_account_tags()
 {
   // ensure consistency
   if (m_account_tags.second.size() != get_num_subaddress_accounts())
diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index d93b9b9fb..21ca48464 100644
--- a/src/wallet/wallet2.h
+++ b/src/wallet/wallet2.h
@@ -590,7 +590,7 @@ private:
     };
 
     typedef std::vector<transfer_details> transfer_container;
-    typedef serializable_unordered_multimap<crypto::hash, payment_details> payment_container;
+    typedef std::unordered_multimap<crypto::hash, payment_details> payment_container;
 
     struct multisig_sig
     {
@@ -701,7 +701,7 @@ private:
     {
       std::vector<pending_tx> ptx;
       std::vector<crypto::key_image> key_images;
-      serializable_unordered_map<crypto::public_key, crypto::key_image> tx_key_images;
+      std::unordered_map<crypto::public_key, crypto::key_image> tx_key_images;
 
       BEGIN_SERIALIZE_OBJECT()
         VERSION_FIELD(0)
@@ -1157,25 +1157,25 @@ private:
       }
       a & m_transfers;
       a & m_account_public_address;
-      a & m_key_images.parent();
+      a & m_key_images;
       if(ver < 6)
         return;
-      a & m_unconfirmed_txs.parent();
+      a & m_unconfirmed_txs;
       if(ver < 7)
         return;
-      a & m_payments.parent();
+      a & m_payments;
       if(ver < 8)
         return;
-      a & m_tx_keys.parent();
+      a & m_tx_keys;
       if(ver < 9)
         return;
-      a & m_confirmed_txs.parent();
+      a & m_confirmed_txs;
       if(ver < 11)
         return;
       a & dummy_refresh_height;
       if(ver < 12)
         return;
-      a & m_tx_notes.parent();
+      a & m_tx_notes;
       if(ver < 13)
         return;
       if (ver < 17)
@@ -1200,7 +1200,7 @@ private:
         }
         return;
       }
-      a & m_pub_keys.parent();
+      a & m_pub_keys;
       if(ver < 16)
         return;
       a & m_address_book;
@@ -1221,17 +1221,17 @@ private:
       a & m_scanned_pool_txs[1];
       if (ver < 20)
         return;
-      a & m_subaddresses.parent();
+      a & m_subaddresses;
       std::unordered_map<cryptonote::subaddress_index, crypto::public_key> dummy_subaddresses_inv;
       a & dummy_subaddresses_inv;
       a & m_subaddress_labels;
-      a & m_additional_tx_keys.parent();
+      a & m_additional_tx_keys;
       if(ver < 21)
         return;
-      a & m_attributes.parent();
+      a & m_attributes;
       if(ver < 22)
         return;
-      a & m_unconfirmed_payments.parent();
+      a & m_unconfirmed_payments;
       if(ver < 23)
         return;
       a & (std::pair<std::map<std::string, std::string>, std::vector<std::string>>&)m_account_tags;
@@ -1243,13 +1243,13 @@ private:
       a & m_last_block_reward;
       if(ver < 26)
         return;
-      a & m_tx_device.parent();
+      a & m_tx_device;
       if(ver < 27)
         return;
       a & m_device_last_key_image_sync;
       if(ver < 28)
         return;
-      a & m_cold_key_images.parent();
+      a & m_cold_key_images;
       if(ver < 29)
         return;
       crypto::secret_key dummy_rpc_client_secret_key; // Compatibility for old RPC payment system
@@ -1464,7 +1464,7 @@ private:
      * \brief  Get the list of registered account tags. 
      * \return first.Key=(tag's name), first.Value=(tag's label), second[i]=(i-th account's tag)
      */
-    const std::pair<serializable_map<std::string, std::string>, std::vector<std::string>>& get_account_tags();
+    const std::pair<std::map<std::string, std::string>, std::vector<std::string>>& get_account_tags();
     /*!
      * \brief  Set a tag to the given accounts.
      * \param  account_indices  Indices of accounts.
@@ -1776,28 +1776,28 @@ private:
     std::string m_mms_file;
     const std::unique_ptr<epee::net_utils::http::abstract_http_client> m_http_client;
     hashchain m_blockchain;
-    serializable_unordered_map<crypto::hash, unconfirmed_transfer_details> m_unconfirmed_txs;
-    serializable_unordered_map<crypto::hash, confirmed_transfer_details> m_confirmed_txs;
-    serializable_unordered_multimap<crypto::hash, pool_payment_details> m_unconfirmed_payments;
-    serializable_unordered_map<crypto::hash, crypto::secret_key> m_tx_keys;
+    std::unordered_map<crypto::hash, unconfirmed_transfer_details> m_unconfirmed_txs;
+    std::unordered_map<crypto::hash, confirmed_transfer_details> m_confirmed_txs;
+    std::unordered_multimap<crypto::hash, pool_payment_details> m_unconfirmed_payments;
+    std::unordered_map<crypto::hash, crypto::secret_key> m_tx_keys;
     cryptonote::checkpoints m_checkpoints;
-    serializable_unordered_map<crypto::hash, std::vector<crypto::secret_key>> m_additional_tx_keys;
+    std::unordered_map<crypto::hash, std::vector<crypto::secret_key>> m_additional_tx_keys;
 
     transfer_container m_transfers;
     payment_container m_payments;
-    serializable_unordered_map<crypto::key_image, size_t> m_key_images;
-    serializable_unordered_map<crypto::public_key, size_t> m_pub_keys;
+    std::unordered_map<crypto::key_image, size_t> m_key_images;
+    std::unordered_map<crypto::public_key, size_t> m_pub_keys;
     cryptonote::account_public_address m_account_public_address;
-    serializable_unordered_map<crypto::public_key, cryptonote::subaddress_index> m_subaddresses;
+    std::unordered_map<crypto::public_key, cryptonote::subaddress_index> m_subaddresses;
     std::vector<std::vector<std::string>> m_subaddress_labels;
-    serializable_unordered_map<crypto::hash, std::string> m_tx_notes;
-    serializable_unordered_map<std::string, std::string> m_attributes;
+    std::unordered_map<crypto::hash, std::string> m_tx_notes;
+    std::unordered_map<std::string, std::string> m_attributes;
     std::vector<tools::wallet2::address_book_row> m_address_book;
-    std::pair<serializable_map<std::string, std::string>, std::vector<std::string>> m_account_tags;
+    std::pair<std::map<std::string, std::string>, std::vector<std::string>> m_account_tags;
     uint64_t m_upper_transaction_weight_limit; //TODO: auto-calc this value or request from daemon, now use some fixed value
     const std::vector<std::vector<tools::wallet2::multisig_info>> *m_multisig_rescan_info;
     const std::vector<std::vector<rct::key>> *m_multisig_rescan_k;
-    serializable_unordered_map<crypto::public_key, crypto::key_image> m_cold_key_images;
+    std::unordered_map<crypto::public_key, crypto::key_image> m_cold_key_images;
 
     std::atomic<bool> m_run;
 
@@ -1868,7 +1868,7 @@ private:
     bool m_allow_mismatched_daemon_version;
 
     // Aux transaction data from device
-    serializable_unordered_map<crypto::hash, std::string> m_tx_device;
+    std::unordered_map<crypto::hash, std::string> m_tx_device;
 
     std::string m_ring_database;
     bool m_ring_history_saved;
@@ -2313,7 +2313,7 @@ namespace boost
       a & x.key_images;
       if (ver < 1)
         return;
-      a & x.tx_key_images.parent();
+      a & x.tx_key_images;
     }
 
     template <class Archive>