From 4616cf2641c69e48ae19303099e701088ffe9045 Mon Sep 17 00:00:00 2001
From: Lee Clagett <code@leeclagett.com>
Date: Wed, 1 Aug 2018 22:10:09 -0400
Subject: [PATCH] Fixed ZMQ-RPC for transactions and GET_BLOCKS_FAST

---
 src/ringct/rctTypes.h                   |  10 +
 src/rpc/daemon_handler.cpp              |  54 ++--
 src/rpc/message_data_structs.h          |   2 +-
 src/serialization/json_object.cpp       | 332 +++++++++++++-----------
 src/serialization/json_object.h         |   6 -
 tests/unit_tests/CMakeLists.txt         |   2 +
 tests/unit_tests/json_serialization.cpp | 217 ++++++++++++++++
 7 files changed, 427 insertions(+), 196 deletions(-)
 create mode 100644 tests/unit_tests/json_serialization.cpp

diff --git a/src/ringct/rctTypes.h b/src/ringct/rctTypes.h
index 844291d0c..a3ccf2e85 100644
--- a/src/ringct/rctTypes.h
+++ b/src/ringct/rctTypes.h
@@ -403,6 +403,16 @@ namespace rct {
     };
     struct rctSig: public rctSigBase {
         rctSigPrunable p;
+
+        keyV& get_pseudo_outs()
+        {
+          return type == RCTTypeSimpleBulletproof ? p.pseudoOuts : pseudoOuts;
+        }
+
+        keyV const& get_pseudo_outs() const
+        {
+          return type == RCTTypeSimpleBulletproof ? p.pseudoOuts : pseudoOuts;
+        }
     };
 
     //other basepoint H = toPoint(cn_fast_hash(G)), G the basepoint
diff --git a/src/rpc/daemon_handler.cpp b/src/rpc/daemon_handler.cpp
index 55858cc2a..25abe4825 100644
--- a/src/rpc/daemon_handler.cpp
+++ b/src/rpc/daemon_handler.cpp
@@ -86,11 +86,27 @@ namespace rpc
           res.error_details = "incorrect number of transactions retrieved for block";
           return;
       }
-      std::vector<transaction> txs;
-      for (const auto& p : it->second)
+
+      cryptonote::rpc::block_output_indices& indices = res.output_indices[block_count];
+
+      // miner tx output indices
       {
-        txs.resize(txs.size() + 1);
-        if (!parse_and_validate_tx_from_blob(p.second, txs.back()))
+        cryptonote::rpc::tx_output_indices tx_indices;
+        if (!m_core.get_tx_outputs_gindexs(get_transaction_hash(bwt.block.miner_tx), tx_indices))
+        {
+          res.status = Message::STATUS_FAILED;
+          res.error_details = "core::get_tx_outputs_gindexs() returned false";
+          return;
+        }
+        indices.push_back(std::move(tx_indices));
+      }
+
+      auto hash_it = bwt.block.tx_hashes.begin();
+      bwt.transactions.reserve(it->second.size());
+      for (const auto& blob : it->second)
+      {
+        bwt.transactions.emplace_back();
+        if (!parse_and_validate_tx_from_blob(blob.second, bwt.transactions.back()))
         {
           res.blocks.clear();
           res.output_indices.clear();
@@ -98,41 +114,17 @@ namespace rpc
           res.error_details = "failed retrieving a requested transaction";
           return;
         }
-      }
-
-      cryptonote::rpc::block_output_indices& indices = res.output_indices[block_count];
-
-      // miner tx output indices
-      {
-        cryptonote::rpc::tx_output_indices tx_indices;
-        bool r = m_core.get_tx_outputs_gindexs(get_transaction_hash(bwt.block.miner_tx), tx_indices);
-        if (!r)
-        {
-          res.status = Message::STATUS_FAILED;
-          res.error_details = "core::get_tx_outputs_gindexs() returned false";
-          return;
-        }
-        indices.push_back(tx_indices);
-      }
-
-      // assume each block returned is returned with all its transactions
-      // in the correct order.
-      auto tx_it = txs.begin();
-      for (const crypto::hash& h : bwt.block.tx_hashes)
-      {
-        bwt.transactions.emplace(h, *tx_it);
-        tx_it++;
 
         cryptonote::rpc::tx_output_indices tx_indices;
-        bool r = m_core.get_tx_outputs_gindexs(h, tx_indices);
-        if (!r)
+        if (!m_core.get_tx_outputs_gindexs(*hash_it, tx_indices))
         {
           res.status = Message::STATUS_FAILED;
           res.error_details = "core::get_tx_outputs_gindexs() returned false";
           return;
         }
 
-        indices.push_back(tx_indices);
+        indices.push_back(std::move(tx_indices));
+        ++hash_it;
       }
 
       it++;
diff --git a/src/rpc/message_data_structs.h b/src/rpc/message_data_structs.h
index fc1b2329d..20390aee8 100644
--- a/src/rpc/message_data_structs.h
+++ b/src/rpc/message_data_structs.h
@@ -44,7 +44,7 @@ namespace rpc
   struct block_with_transactions
   {
     cryptonote::block block;
-    std::unordered_map<crypto::hash, cryptonote::transaction> transactions;
+    std::vector<cryptonote::transaction> transactions;
   };
 
   typedef std::vector<uint64_t> tx_output_indices;
diff --git a/src/serialization/json_object.cpp b/src/serialization/json_object.cpp
index c2467b863..67fe709dc 100644
--- a/src/serialization/json_object.cpp
+++ b/src/serialization/json_object.cpp
@@ -28,6 +28,8 @@
 
 #include "json_object.h"
 
+#include <boost/range/adaptor/transformed.hpp>
+#include <boost/variant/apply_visitor.hpp>
 #include <limits>
 #include <type_traits>
 #include "string_tools.h"
@@ -219,11 +221,11 @@ void toJsonValue(rapidjson::Document& doc, const cryptonote::transaction& tx, ra
 
   INSERT_INTO_JSON_OBJECT(val, doc, version, tx.version);
   INSERT_INTO_JSON_OBJECT(val, doc, unlock_time, tx.unlock_time);
-  INSERT_INTO_JSON_OBJECT(val, doc, vin, tx.vin);
-  INSERT_INTO_JSON_OBJECT(val, doc, vout, tx.vout);
+  INSERT_INTO_JSON_OBJECT(val, doc, inputs, tx.vin);
+  INSERT_INTO_JSON_OBJECT(val, doc, outputs, tx.vout);
   INSERT_INTO_JSON_OBJECT(val, doc, extra, tx.extra);
   INSERT_INTO_JSON_OBJECT(val, doc, signatures, tx.signatures);
-  INSERT_INTO_JSON_OBJECT(val, doc, rct_signatures, tx.rct_signatures);
+  INSERT_INTO_JSON_OBJECT(val, doc, ringct, tx.rct_signatures);
 }
 
 
@@ -236,11 +238,11 @@ void fromJsonValue(const rapidjson::Value& val, cryptonote::transaction& tx)
 
   GET_FROM_JSON_OBJECT(val, tx.version, version);
   GET_FROM_JSON_OBJECT(val, tx.unlock_time, unlock_time);
-  GET_FROM_JSON_OBJECT(val, tx.vin, vin);
-  GET_FROM_JSON_OBJECT(val, tx.vout, vout);
+  GET_FROM_JSON_OBJECT(val, tx.vin, inputs);
+  GET_FROM_JSON_OBJECT(val, tx.vout, outputs);
   GET_FROM_JSON_OBJECT(val, tx.extra, extra);
   GET_FROM_JSON_OBJECT(val, tx.signatures, signatures);
-  GET_FROM_JSON_OBJECT(val, tx.rct_signatures, rct_signatures);
+  GET_FROM_JSON_OBJECT(val, tx.rct_signatures, ringct);
 }
 
 void toJsonValue(rapidjson::Document& doc, const cryptonote::block& b, rapidjson::Value& val)
@@ -277,26 +279,31 @@ void toJsonValue(rapidjson::Document& doc, const cryptonote::txin_v& txin, rapid
 {
   val.SetObject();
 
-  if (txin.type() == typeid(cryptonote::txin_gen))
+  struct add_input
   {
-    val.AddMember("type", "txin_gen", doc.GetAllocator());
-    INSERT_INTO_JSON_OBJECT(val, doc, value, boost::get<cryptonote::txin_gen>(txin));
-  }
-  else if (txin.type() == typeid(cryptonote::txin_to_script))
-  {
-    val.AddMember("type", "txin_to_script", doc.GetAllocator());
-    INSERT_INTO_JSON_OBJECT(val, doc, value, boost::get<cryptonote::txin_to_script>(txin));
-  }
-  else if (txin.type() == typeid(cryptonote::txin_to_scripthash))
-  {
-    val.AddMember("type", "txin_to_scripthash", doc.GetAllocator());
-    INSERT_INTO_JSON_OBJECT(val, doc, value, boost::get<cryptonote::txin_to_scripthash>(txin));
-  }
-  else if (txin.type() == typeid(cryptonote::txin_to_key))
-  {
-    val.AddMember("type", "txin_to_key", doc.GetAllocator());
-    INSERT_INTO_JSON_OBJECT(val, doc, value, boost::get<cryptonote::txin_to_key>(txin));
-  }
+    using result_type = void;
+
+    rapidjson::Document& doc;
+    rapidjson::Value& val;
+
+    void operator()(cryptonote::txin_to_key const& input) const
+    {
+      INSERT_INTO_JSON_OBJECT(val, doc, to_key, input);
+    }
+    void operator()(cryptonote::txin_gen const& input) const
+    {
+      INSERT_INTO_JSON_OBJECT(val, doc, gen, input);
+    }
+    void operator()(cryptonote::txin_to_script const& input) const
+    {
+      INSERT_INTO_JSON_OBJECT(val, doc, to_script, input);
+    }
+    void operator()(cryptonote::txin_to_scripthash const& input) const
+    {
+      INSERT_INTO_JSON_OBJECT(val, doc, to_scripthash, input);
+    }
+  };
+  boost::apply_visitor(add_input{doc, val}, txin);
 }
 
 
@@ -307,31 +314,37 @@ void fromJsonValue(const rapidjson::Value& val, cryptonote::txin_v& txin)
     throw WRONG_TYPE("json object");
   }
 
-  OBJECT_HAS_MEMBER_OR_THROW(val, "type")
-  OBJECT_HAS_MEMBER_OR_THROW(val, "value")
-  if (val["type"]== "txin_gen")
+  if (val.MemberCount() != 1)
   {
-    cryptonote::txin_gen tmpVal;
-    fromJsonValue(val["value"], tmpVal);
-    txin = tmpVal;
+    throw MISSING_KEY("Invalid input object");
   }
-  else if (val["type"]== "txin_to_script")
+
+  for (auto const& elem : val.GetObject())
   {
-    cryptonote::txin_to_script tmpVal;
-    fromJsonValue(val["value"], tmpVal);
-    txin = tmpVal;
-  }
-  else if (val["type"] == "txin_to_scripthash")
-  {
-    cryptonote::txin_to_scripthash tmpVal;
-    fromJsonValue(val["value"], tmpVal);
-    txin = tmpVal;
-  }
-  else if (val["type"] == "txin_to_key")
-  {
-    cryptonote::txin_to_key tmpVal;
-    fromJsonValue(val["value"], tmpVal);
-    txin = tmpVal;
+    if (elem.name == "to_key")
+    {
+      cryptonote::txin_to_key tmpVal;
+      fromJsonValue(elem.value, tmpVal);
+      txin = std::move(tmpVal);
+    }
+    else if (elem.name == "gen")
+    {
+      cryptonote::txin_gen tmpVal;
+      fromJsonValue(elem.value, tmpVal);
+      txin = std::move(tmpVal);
+    }
+    else if (elem.name == "to_script")
+    {
+      cryptonote::txin_to_script tmpVal;
+      fromJsonValue(elem.value, tmpVal);
+      txin = std::move(tmpVal);
+    }
+    else if (elem.name == "to_scripthash")
+    {
+      cryptonote::txin_to_scripthash tmpVal;
+      fromJsonValue(elem.value, tmpVal);
+      txin = std::move(tmpVal);
+    }
   }
 }
 
@@ -405,7 +418,7 @@ void toJsonValue(rapidjson::Document& doc, const cryptonote::txin_to_key& txin,
 
   INSERT_INTO_JSON_OBJECT(val, doc, amount, txin.amount);
   INSERT_INTO_JSON_OBJECT(val, doc, key_offsets, txin.key_offsets);
-  INSERT_INTO_JSON_OBJECT(val, doc, k_image, txin.k_image);
+  INSERT_INTO_JSON_OBJECT(val, doc, key_image, txin.k_image);
 }
 
 
@@ -418,58 +431,7 @@ void fromJsonValue(const rapidjson::Value& val, cryptonote::txin_to_key& txin)
 
   GET_FROM_JSON_OBJECT(val, txin.amount, amount);
   GET_FROM_JSON_OBJECT(val, txin.key_offsets, key_offsets);
-  GET_FROM_JSON_OBJECT(val, txin.k_image, k_image);
-}
-
-void toJsonValue(rapidjson::Document& doc, const cryptonote::txout_target_v& txout, rapidjson::Value& val)
-{
-  val.SetObject();
-
-  if (txout.type() == typeid(cryptonote::txout_to_script))
-  {
-    val.AddMember("type", "txout_to_script", doc.GetAllocator());
-    INSERT_INTO_JSON_OBJECT(val, doc, value, boost::get<cryptonote::txout_to_script>(txout));
-  }
-  else if (txout.type() == typeid(cryptonote::txout_to_scripthash))
-  {
-    val.AddMember("type", "txout_to_scripthash", doc.GetAllocator());
-    INSERT_INTO_JSON_OBJECT(val, doc, value, boost::get<cryptonote::txout_to_scripthash>(txout));
-  }
-  else if (txout.type() == typeid(cryptonote::txout_to_key))
-  {
-    val.AddMember("type", "txout_to_key", doc.GetAllocator());
-    INSERT_INTO_JSON_OBJECT(val, doc, value, boost::get<cryptonote::txout_to_key>(txout));
-  }
-}
-
-
-void fromJsonValue(const rapidjson::Value& val, cryptonote::txout_target_v& txout)
-{
-  if (!val.IsObject())
-  {
-    throw WRONG_TYPE("json object");
-  }
-
-  OBJECT_HAS_MEMBER_OR_THROW(val, "type")
-  OBJECT_HAS_MEMBER_OR_THROW(val, "value")
-  if (val["type"]== "txout_to_script")
-  {
-    cryptonote::txout_to_script tmpVal;
-    fromJsonValue(val["value"], tmpVal);
-    txout = tmpVal;
-  }
-  else if (val["type"] == "txout_to_scripthash")
-  {
-    cryptonote::txout_to_scripthash tmpVal;
-    fromJsonValue(val["value"], tmpVal);
-    txout = tmpVal;
-  }
-  else if (val["type"] == "txout_to_key")
-  {
-    cryptonote::txout_to_key tmpVal;
-    fromJsonValue(val["value"], tmpVal);
-    txout = tmpVal;
-  }
+  GET_FROM_JSON_OBJECT(val, txin.k_image, key_image);
 }
 
 void toJsonValue(rapidjson::Document& doc, const cryptonote::txout_to_script& txout, rapidjson::Value& val)
@@ -533,7 +495,28 @@ void toJsonValue(rapidjson::Document& doc, const cryptonote::tx_out& txout, rapi
   val.SetObject();
 
   INSERT_INTO_JSON_OBJECT(val, doc, amount, txout.amount);
-  INSERT_INTO_JSON_OBJECT(val, doc, target, txout.target);
+
+  struct add_output
+  {
+    using result_type = void;
+
+    rapidjson::Document& doc;
+    rapidjson::Value& val;
+
+    void operator()(cryptonote::txout_to_key const& output) const
+    {
+      INSERT_INTO_JSON_OBJECT(val, doc, to_key, output);
+    }
+    void operator()(cryptonote::txout_to_script const& output) const
+    {
+      INSERT_INTO_JSON_OBJECT(val, doc, to_script, output);
+    }
+    void operator()(cryptonote::txout_to_scripthash const& output) const
+    {
+      INSERT_INTO_JSON_OBJECT(val, doc, to_scripthash, output);
+    }
+  };
+  boost::apply_visitor(add_output{doc, val}, txout.target);
 }
 
 void fromJsonValue(const rapidjson::Value& val, cryptonote::tx_out& txout)
@@ -543,8 +526,37 @@ void fromJsonValue(const rapidjson::Value& val, cryptonote::tx_out& txout)
     throw WRONG_TYPE("json object");
   }
 
-  GET_FROM_JSON_OBJECT(val, txout.amount, amount);
-  GET_FROM_JSON_OBJECT(val, txout.target, target);
+  if (val.MemberCount() != 2)
+  {
+    throw MISSING_KEY("Invalid input object");
+  }
+
+  for (auto const& elem : val.GetObject())
+  {
+    if (elem.name == "amount")
+    {
+      fromJsonValue(elem.value, txout.amount);
+    }
+
+    if (elem.name == "to_key")
+    {
+      cryptonote::txout_to_key tmpVal;
+      fromJsonValue(elem.value, tmpVal);
+      txout.target = std::move(tmpVal);
+    }
+    else if (elem.name == "to_script")
+    {
+      cryptonote::txout_to_script tmpVal;
+      fromJsonValue(elem.value, tmpVal);
+      txout.target = std::move(tmpVal);
+    }
+    else if (elem.name == "to_scripthash")
+    {
+      cryptonote::txout_to_scripthash tmpVal;
+      fromJsonValue(elem.value, tmpVal);
+      txout.target = std::move(tmpVal);
+    }
+  }
 }
 
 void toJsonValue(rapidjson::Document& doc, const cryptonote::connection_info& info, rapidjson::Value& val)
@@ -617,7 +629,7 @@ void toJsonValue(rapidjson::Document& doc, const cryptonote::block_complete_entr
   val.SetObject();
 
   INSERT_INTO_JSON_OBJECT(val, doc, block, blk.block);
-  INSERT_INTO_JSON_OBJECT(val, doc, txs, blk.txs);
+  INSERT_INTO_JSON_OBJECT(val, doc, transactions, blk.txs);
 }
 
 
@@ -629,7 +641,7 @@ void fromJsonValue(const rapidjson::Value& val, cryptonote::block_complete_entry
   }
 
   GET_FROM_JSON_OBJECT(val, blk.block, block);
-  GET_FROM_JSON_OBJECT(val, blk.txs, txs);
+  GET_FROM_JSON_OBJECT(val, blk.txs, transactions);
 }
 
 void toJsonValue(rapidjson::Document& doc, const cryptonote::rpc::block_with_transactions& blk, rapidjson::Value& val)
@@ -936,51 +948,70 @@ void fromJsonValue(const rapidjson::Value& val, cryptonote::rpc::BlockHeaderResp
 
 void toJsonValue(rapidjson::Document& doc, const rct::rctSig& sig, rapidjson::Value& val)
 {
+  using boost::adaptors::transform;
+
   val.SetObject();
 
+  const auto just_mask = [] (rct::ctkey const& key) -> rct::key const&
+  {
+    return key.mask;
+  };
+
   INSERT_INTO_JSON_OBJECT(val, doc, type, sig.type);
-  INSERT_INTO_JSON_OBJECT(val, doc, message, sig.message);
-  INSERT_INTO_JSON_OBJECT(val, doc, mixRing, sig.mixRing);
-  INSERT_INTO_JSON_OBJECT(val, doc, pseudoOuts, sig.pseudoOuts);
-  INSERT_INTO_JSON_OBJECT(val, doc, ecdhInfo, sig.ecdhInfo);
-  INSERT_INTO_JSON_OBJECT(val, doc, outPk, sig.outPk);
-  INSERT_INTO_JSON_OBJECT(val, doc, txnFee, sig.txnFee);
-  INSERT_INTO_JSON_OBJECT(val, doc, p, sig.p);
+  INSERT_INTO_JSON_OBJECT(val, doc, encrypted, sig.ecdhInfo);
+  INSERT_INTO_JSON_OBJECT(val, doc, commitments, transform(sig.outPk, just_mask));
+  INSERT_INTO_JSON_OBJECT(val, doc, fee, sig.txnFee);
+
+  // prunable
+  {
+    rapidjson::Value prunable;
+    prunable.SetObject();
+
+    INSERT_INTO_JSON_OBJECT(prunable, doc, range_proofs, sig.p.rangeSigs);
+    INSERT_INTO_JSON_OBJECT(prunable, doc, bulletproofs, sig.p.bulletproofs);
+    INSERT_INTO_JSON_OBJECT(prunable, doc, mlsags, sig.p.MGs);
+    INSERT_INTO_JSON_OBJECT(prunable, doc, pseudo_outs, sig.get_pseudo_outs());
+
+    val.AddMember("prunable", prunable, doc.GetAllocator());
+  }
 }
 
 void fromJsonValue(const rapidjson::Value& val, rct::rctSig& sig)
 {
+  using boost::adaptors::transform;
+
   if (!val.IsObject())
   {
     throw WRONG_TYPE("json object");
   }
 
+  std::vector<rct::key> commitments;
+
   GET_FROM_JSON_OBJECT(val, sig.type, type);
-  GET_FROM_JSON_OBJECT(val, sig.message, message);
-  GET_FROM_JSON_OBJECT(val, sig.mixRing, mixRing);
-  GET_FROM_JSON_OBJECT(val, sig.pseudoOuts, pseudoOuts);
-  GET_FROM_JSON_OBJECT(val, sig.ecdhInfo, ecdhInfo);
-  GET_FROM_JSON_OBJECT(val, sig.outPk, outPk);
-  GET_FROM_JSON_OBJECT(val, sig.txnFee, txnFee);
-  GET_FROM_JSON_OBJECT(val, sig.p, p);
-}
+  GET_FROM_JSON_OBJECT(val, sig.ecdhInfo, encrypted);
+  GET_FROM_JSON_OBJECT(val, commitments, commitments);
+  GET_FROM_JSON_OBJECT(val, sig.txnFee, fee);
 
-void toJsonValue(rapidjson::Document& doc, const rct::ctkey& key, rapidjson::Value& val)
-{
-  val.SetObject();
-
-  INSERT_INTO_JSON_OBJECT(val, doc, dest, key.dest);
-  INSERT_INTO_JSON_OBJECT(val, doc, mask, key.mask);
-}
-
-void fromJsonValue(const rapidjson::Value& val, rct::ctkey& key)
-{
-  if (!val.IsObject())
+  // prunable
   {
-    throw WRONG_TYPE("json object");
+    OBJECT_HAS_MEMBER_OR_THROW(val, "prunable");
+    const auto& prunable = val["prunable"];
+
+    rct::keyV pseudo_outs;
+
+    GET_FROM_JSON_OBJECT(prunable, sig.p.rangeSigs, range_proofs);
+    GET_FROM_JSON_OBJECT(prunable, sig.p.bulletproofs, bulletproofs);
+    GET_FROM_JSON_OBJECT(prunable, sig.p.MGs, mlsags);
+    GET_FROM_JSON_OBJECT(prunable, pseudo_outs, pseudo_outs);
+
+    sig.get_pseudo_outs() = std::move(pseudo_outs);
+  }
+
+  sig.outPk.reserve(commitments.size());
+  for (rct::key const& commitment : commitments)
+  {
+    sig.outPk.push_back({{}, commitment});
   }
-  GET_FROM_JSON_OBJECT(val, key.dest, dest);
-  GET_FROM_JSON_OBJECT(val, key.mask, mask);
 }
 
 void toJsonValue(rapidjson::Document& doc, const rct::ecdhTuple& tuple, rapidjson::Value& val)
@@ -1002,27 +1033,6 @@ void fromJsonValue(const rapidjson::Value& val, rct::ecdhTuple& tuple)
   GET_FROM_JSON_OBJECT(val, tuple.amount, amount);
 }
 
-void toJsonValue(rapidjson::Document& doc, const rct::rctSigPrunable& sig, rapidjson::Value& val)
-{
-  val.SetObject();
-
-  INSERT_INTO_JSON_OBJECT(val, doc, rangeSigs, sig.rangeSigs);
-  INSERT_INTO_JSON_OBJECT(val, doc, bulletproofs, sig.bulletproofs);
-  INSERT_INTO_JSON_OBJECT(val, doc, MGs, sig.MGs);
-}
-
-void fromJsonValue(const rapidjson::Value& val, rct::rctSigPrunable& sig)
-{
-  if (!val.IsObject())
-  {
-    throw WRONG_TYPE("json object");
-  }
-
-  GET_FROM_JSON_OBJECT(val, sig.rangeSigs, rangeSigs);
-  GET_FROM_JSON_OBJECT(val, sig.bulletproofs, bulletproofs);
-  GET_FROM_JSON_OBJECT(val, sig.MGs, MGs);
-}
-
 void toJsonValue(rapidjson::Document& doc, const rct::rangeSig& sig, rapidjson::Value& val)
 {
   val.SetObject();
@@ -1040,10 +1050,16 @@ void fromJsonValue(const rapidjson::Value& val, rct::rangeSig& sig)
     throw WRONG_TYPE("json object");
   }
 
+  const auto ci = val.FindMember("Ci");
+  if (ci == val.MemberEnd())
+  {
+    throw MISSING_KEY("Ci");
+  }
+
   GET_FROM_JSON_OBJECT(val, sig.asig, asig);
 
   std::vector<rct::key> keyVector;
-  cryptonote::json::fromJsonValue(val["Ci"], keyVector);
+  cryptonote::json::fromJsonValue(ci->value, keyVector);
   if (!(keyVector.size() == 64))
   {
     throw WRONG_TYPE("key64 (rct::key[64])");
@@ -1098,10 +1114,10 @@ void toJsonValue(rapidjson::Document& doc, const rct::boroSig& sig, rapidjson::V
   val.SetObject();
 
   std::vector<rct::key> keyVector(sig.s0, std::end(sig.s0));
-  INSERT_INTO_JSON_OBJECT(val, doc, s0, sig.s0);
+  INSERT_INTO_JSON_OBJECT(val, doc, s0, keyVector);
 
   keyVector.assign(sig.s1, std::end(sig.s1));
-  INSERT_INTO_JSON_OBJECT(val, doc, s1, sig.s1);
+  INSERT_INTO_JSON_OBJECT(val, doc, s1, keyVector);
 
   INSERT_INTO_JSON_OBJECT(val, doc, ee, sig.ee);
 }
diff --git a/src/serialization/json_object.h b/src/serialization/json_object.h
index de21ace66..da3351fe3 100644
--- a/src/serialization/json_object.h
+++ b/src/serialization/json_object.h
@@ -263,15 +263,9 @@ void fromJsonValue(const rapidjson::Value& val, cryptonote::rpc::BlockHeaderResp
 void toJsonValue(rapidjson::Document& doc, const rct::rctSig& i, rapidjson::Value& val);
 void fromJsonValue(const rapidjson::Value& i, rct::rctSig& sig);
 
-void toJsonValue(rapidjson::Document& doc, const rct::ctkey& key, rapidjson::Value& val);
-void fromJsonValue(const rapidjson::Value& val, rct::ctkey& key);
-
 void toJsonValue(rapidjson::Document& doc, const rct::ecdhTuple& tuple, rapidjson::Value& val);
 void fromJsonValue(const rapidjson::Value& val, rct::ecdhTuple& tuple);
 
-void toJsonValue(rapidjson::Document& doc, const rct::rctSigPrunable& sig, rapidjson::Value& val);
-void fromJsonValue(const rapidjson::Value& val, rct::rctSigPrunable& sig);
-
 void toJsonValue(rapidjson::Document& doc, const rct::rangeSig& sig, rapidjson::Value& val);
 void fromJsonValue(const rapidjson::Value& val, rct::rangeSig& sig);
 
diff --git a/tests/unit_tests/CMakeLists.txt b/tests/unit_tests/CMakeLists.txt
index ffa8065ab..0821751cb 100644
--- a/tests/unit_tests/CMakeLists.txt
+++ b/tests/unit_tests/CMakeLists.txt
@@ -47,6 +47,7 @@ set(unit_tests_sources
   epee_levin_protocol_handler_async.cpp
   epee_utils.cpp
   fee.cpp
+  json_serialization.cpp
   get_xtype_from_string.cpp
   hashchain.cpp
   http.cpp
@@ -86,6 +87,7 @@ target_link_libraries(unit_tests
     cryptonote_core
     blockchain_db
     rpc
+    serialization
     wallet
     p2p
     version
diff --git a/tests/unit_tests/json_serialization.cpp b/tests/unit_tests/json_serialization.cpp
new file mode 100644
index 000000000..ac6b60846
--- /dev/null
+++ b/tests/unit_tests/json_serialization.cpp
@@ -0,0 +1,217 @@
+
+#include <boost/optional/optional.hpp>
+#include <boost/range/adaptor/indexed.hpp>
+#include <gtest/gtest.h>
+#include <rapidjson/document.h>
+#include <vector>
+
+#include "crypto/hash.h"
+#include "cryptonote_basic/account.h"
+#include "cryptonote_basic/cryptonote_basic.h"
+#include "cryptonote_basic/cryptonote_format_utils.h"
+#include "cryptonote_core/cryptonote_tx_utils.h"
+#include "serialization/json_object.h"
+
+
+namespace
+{
+    cryptonote::transaction
+    make_miner_transaction(cryptonote::account_public_address const& to)
+    {
+        cryptonote::transaction tx{};
+        if (!cryptonote::construct_miner_tx(0, 0, 5000, 500, 500, to, tx))
+            throw std::runtime_error{"transaction construction error"};
+
+        crypto::hash id{0};
+        if (!cryptonote::get_transaction_hash(tx, id))
+            throw std::runtime_error{"could not get transaction hash"};
+
+        return tx;
+    }
+
+    cryptonote::transaction
+    make_transaction(
+        cryptonote::account_keys const& from,
+        std::vector<cryptonote::transaction> const& sources,
+        std::vector<cryptonote::account_public_address> const& destinations,
+        bool rct,
+        bool bulletproof)
+    {
+        std::uint64_t source_amount = 0;
+        std::vector<cryptonote::tx_source_entry> actual_sources;
+        for (auto const& source : sources)
+        {
+            std::vector<cryptonote::tx_extra_field> extra_fields;
+            if (!cryptonote::parse_tx_extra(source.extra, extra_fields))
+                throw std::runtime_error{"invalid transaction"};
+
+            cryptonote::tx_extra_pub_key key_field{};
+            if (!cryptonote::find_tx_extra_field_by_type(extra_fields, key_field))
+                throw std::runtime_error{"invalid transaction"};
+
+            for (auto const& input : boost::adaptors::index(source.vout))
+            {
+                source_amount += input.value().amount;
+                auto const& key = boost::get<cryptonote::txout_to_key>(input.value().target);
+
+                actual_sources.push_back(
+                    {{}, 0, key_field.pub_key, {}, std::size_t(input.index()), input.value().amount, rct, rct::identity()}
+                );
+
+                for (unsigned ring = 0; ring < 10; ++ring)
+                    actual_sources.back().push_output(input.index(), key.key, input.value().amount);
+            }
+        }
+
+        std::vector<cryptonote::tx_destination_entry> to;
+        for (auto const& destination : destinations)
+            to.push_back({(source_amount / destinations.size()), destination, false});
+
+        cryptonote::transaction tx{};
+
+        crypto::secret_key tx_key{};
+        std::vector<crypto::secret_key> extra_keys{};
+
+        std::unordered_map<crypto::public_key, cryptonote::subaddress_index> subaddresses;
+        subaddresses[from.m_account_address.m_spend_public_key] = {0,0};
+
+        if (!cryptonote::construct_tx_and_get_tx_key(from, subaddresses, actual_sources, to, boost::none, {}, tx, 0, tx_key, extra_keys, rct, bulletproof))
+            throw std::runtime_error{"transaction construction error"};
+
+        return tx;
+    }
+} // anonymous
+
+TEST(JsonSerialization, MinerTransaction)
+{
+    cryptonote::account_base acct;
+    acct.generate();
+    const auto miner_tx = make_miner_transaction(acct.get_keys().m_account_address);
+
+    crypto::hash tx_hash{};
+    ASSERT_TRUE(cryptonote::get_transaction_hash(miner_tx, tx_hash));
+
+    rapidjson::Document doc;
+    cryptonote::json::toJsonValue(doc, miner_tx, doc);
+
+    cryptonote::transaction miner_tx_copy;
+    cryptonote::json::fromJsonValue(doc, miner_tx_copy);
+
+    crypto::hash tx_copy_hash{};
+    ASSERT_TRUE(cryptonote::get_transaction_hash(miner_tx_copy, tx_copy_hash));
+    EXPECT_EQ(tx_hash, tx_copy_hash);
+
+    cryptonote::blobdata tx_bytes{};
+    cryptonote::blobdata tx_copy_bytes{};
+
+    ASSERT_TRUE(cryptonote::t_serializable_object_to_blob(miner_tx, tx_bytes));
+    ASSERT_TRUE(cryptonote::t_serializable_object_to_blob(miner_tx_copy, tx_copy_bytes));
+
+    EXPECT_EQ(tx_bytes, tx_copy_bytes);
+}
+
+TEST(JsonSerialization, RegularTransaction)
+{
+    cryptonote::account_base acct1;
+    acct1.generate();
+
+    cryptonote::account_base acct2;
+    acct2.generate();
+
+    const auto miner_tx = make_miner_transaction(acct1.get_keys().m_account_address);
+    const auto tx = make_transaction(
+        acct1.get_keys(), {miner_tx}, {acct2.get_keys().m_account_address}, false, false
+    );
+
+    crypto::hash tx_hash{};
+    ASSERT_TRUE(cryptonote::get_transaction_hash(tx, tx_hash));
+
+    rapidjson::Document doc;
+    cryptonote::json::toJsonValue(doc, tx, doc);
+
+    cryptonote::transaction tx_copy;
+    cryptonote::json::fromJsonValue(doc, tx_copy);
+
+    crypto::hash tx_copy_hash{};
+    ASSERT_TRUE(cryptonote::get_transaction_hash(tx_copy, tx_copy_hash));
+    EXPECT_EQ(tx_hash, tx_copy_hash);
+
+    cryptonote::blobdata tx_bytes{};
+    cryptonote::blobdata tx_copy_bytes{};
+
+    ASSERT_TRUE(cryptonote::t_serializable_object_to_blob(tx, tx_bytes));
+    ASSERT_TRUE(cryptonote::t_serializable_object_to_blob(tx_copy, tx_copy_bytes));
+
+    EXPECT_EQ(tx_bytes, tx_copy_bytes);
+}
+
+TEST(JsonSerialization, RingctTransaction)
+{
+    cryptonote::account_base acct1;
+    acct1.generate();
+
+    cryptonote::account_base acct2;
+    acct2.generate();
+
+    const auto miner_tx = make_miner_transaction(acct1.get_keys().m_account_address);
+    const auto tx = make_transaction(
+        acct1.get_keys(), {miner_tx}, {acct2.get_keys().m_account_address}, true, false
+    );
+
+    crypto::hash tx_hash{};
+    ASSERT_TRUE(cryptonote::get_transaction_hash(tx, tx_hash));
+
+    rapidjson::Document doc;
+    cryptonote::json::toJsonValue(doc, tx, doc);
+
+    cryptonote::transaction tx_copy;
+    cryptonote::json::fromJsonValue(doc, tx_copy);
+
+    crypto::hash tx_copy_hash{};
+    ASSERT_TRUE(cryptonote::get_transaction_hash(tx_copy, tx_copy_hash));
+    EXPECT_EQ(tx_hash, tx_copy_hash);
+
+    cryptonote::blobdata tx_bytes{};
+    cryptonote::blobdata tx_copy_bytes{};
+
+    ASSERT_TRUE(cryptonote::t_serializable_object_to_blob(tx, tx_bytes));
+    ASSERT_TRUE(cryptonote::t_serializable_object_to_blob(tx_copy, tx_copy_bytes));
+
+    EXPECT_EQ(tx_bytes, tx_copy_bytes);
+}
+
+TEST(JsonSerialization, BulletproofTransaction)
+{
+    cryptonote::account_base acct1;
+    acct1.generate();
+
+    cryptonote::account_base acct2;
+    acct2.generate();
+
+    const auto miner_tx = make_miner_transaction(acct1.get_keys().m_account_address);
+    const auto tx = make_transaction(
+        acct1.get_keys(), {miner_tx}, {acct2.get_keys().m_account_address}, true, true
+    );
+
+    crypto::hash tx_hash{};
+    ASSERT_TRUE(cryptonote::get_transaction_hash(tx, tx_hash));
+
+    rapidjson::Document doc;
+    cryptonote::json::toJsonValue(doc, tx, doc);
+
+    cryptonote::transaction tx_copy;
+    cryptonote::json::fromJsonValue(doc, tx_copy);
+
+    crypto::hash tx_copy_hash{};
+    ASSERT_TRUE(cryptonote::get_transaction_hash(tx_copy, tx_copy_hash));
+    EXPECT_EQ(tx_hash, tx_copy_hash);
+
+    cryptonote::blobdata tx_bytes{};
+    cryptonote::blobdata tx_copy_bytes{};
+
+    ASSERT_TRUE(cryptonote::t_serializable_object_to_blob(tx, tx_bytes));
+    ASSERT_TRUE(cryptonote::t_serializable_object_to_blob(tx_copy, tx_copy_bytes));
+
+    EXPECT_EQ(tx_bytes, tx_copy_bytes);
+}
+