From 4a9ae3eb8b9a6d7a2e64f360fda2d5790c8e3321 Mon Sep 17 00:00:00 2001 From: moneromooo-monero 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 output_entry; std::vector 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 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 : public binary_archive_base { - 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 : public binary_archive_base 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 : public binary_archive_base 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 - bool serialize_container_element(Archive& ar, T& e) + template + inline constexpr bool use_container_varint() noexcept { - return ::do_serialize(ar, e); + return std::is_integral::value && std::is_unsigned::value && sizeof(T) > 1; } - template - bool serialize_container_element(Archive& ar, uint32_t& e) + template + typename std::enable_if(), bool>::type + serialize_container_element(Archive& ar, T& e) { - ar.serialize_varint(e); - return true; + return ::do_serialize(ar, e); } - template - bool serialize_container_element(Archive& ar, uint64_t& e) + template + typename std::enable_if(), bool>::type + serialize_container_element(Archive& ar, T& e) { + static constexpr const bool previously_varint = std::is_same() || std::is_same(); + + 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 +#include #include "serialization.h" namespace serialization { namespace detail { + template + inline constexpr bool use_pair_varint() noexcept + { + return std::is_integral::value && std::is_unsigned::value && sizeof(T) > 1; + } + template - bool serialize_pair_element(Archive& ar, T& e) + typename std::enable_if(), bool>::type + serialize_pair_element(Archive& ar, T& e) { return ::do_serialize(ar, e); } - template - bool serialize_pair_element(Archive& ar, uint64_t& e) + template + typename std::enable_if(), bool>::type + serialize_pair_element(Archive& ar, T& e) { + static constexpr const bool previously_varint = std::is_same(); + + 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 a004c8855..759cb3249 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -5649,6 +5649,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 ar(iss); + ar.enable_varint_bug_backward_compatibility(); + if (::serialization::serialize(ar, *this)) + if (::serialization::check_stream_state(ar)) + loaded = true; + } } catch(...) { } @@ -12439,7 +12449,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>> ski = export_key_images(all); + std::pair>> 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; @@ -12466,7 +12476,7 @@ bool wallet2::export_key_images(const std::string &filename, bool all) const } //---------------------------------------------------------------------------------------------------- -std::pair>> wallet2::export_key_images(bool all) const +std::pair>> wallet2::export_key_images(bool all) const { PERF_TIMER(export_key_images_raw); std::vector> ski; @@ -12963,7 +12973,7 @@ void wallet2::import_blockchain(const std::tuple> wallet2::export_outputs(bool all) const +std::pair> wallet2::export_outputs(bool all) const { PERF_TIMER(export_outputs); std::vector outs; @@ -13003,7 +13013,7 @@ std::string wallet2::export_outputs_to_str(bool all) const return magic + ciphertext; } //---------------------------------------------------------------------------------------------------- -size_t wallet2::import_outputs(const std::pair> &outputs) +size_t wallet2::import_outputs(const std::pair> &outputs) { PERF_TIMER(import_outputs); @@ -13109,7 +13119,7 @@ size_t wallet2::import_outputs_from_str(const std::string &outputs_st) try { std::string body(data, headerlen); - std::pair> outputs; + std::pair> outputs; try { std::stringstream iss; diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index fed7d745c..322f3b104 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 m_multisig_k; @@ -1367,9 +1367,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> export_outputs(bool all = false) const; + std::pair> export_outputs(bool all = false) const; std::string export_outputs_to_str(bool all = false) const; - size_t import_outputs(const std::pair> &outputs); + size_t import_outputs(const std::pair> &outputs); size_t import_outputs_from_str(const std::string &outputs_st); payment_container export_payments() const; void import_payments(const payment_container &payments); @@ -1377,7 +1377,7 @@ private: std::tuple> export_blockchain() const; void import_blockchain(const std::tuple> &bc); bool export_key_images(const std::string &filename, bool all = false) const; - std::pair>> export_key_images(bool all = false) const; + std::pair>> export_key_images(bool all = false) const; uint64_t import_key_images(const std::vector> &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 key_images, size_t offset=0, boost::optional> selected_transfers=boost::none); diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 03db8b70f..db578ca63 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -2702,7 +2702,7 @@ namespace tools if (!m_wallet) return not_open(er); try { - std::pair>> ski = m_wallet->export_key_images(req.all); + std::pair>> 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> outputs; + std::pair> outputs; std::stringstream iss; iss << s; binary_archive ar(iss);