move the rct commitments to the output_amounts database

Since these are needed at the same time as the output pubkeys,
this is a whole lot faster, and takes less space. Only outputs
of 0 amount store the commitment. When reading other outputs,
a fake commitment is regenerated on the fly. This avoids having
to rewrite the database to add space for fake commitments for
existing outputs.

This code relies on two things:

- LMDB must support fixed size records per key, rather than
per database (ie, all records on key 0 are the same size, all
records for non 0 keys are same size, but records from key 0
and non 0 keys do have different sizes).

- the commitment must be directly after the rest of the data
in outkey and output_data_t.

8 files changed, 63 insertions, 195 deletions

diff --git a/src/blockchain_db/berkeleydb/db_bdb.cpp b/src/blockchain_db/berkeleydb/db_bdb.cpp
index 4d759d157..4ec284e38 100644
--- a/src/blockchain_db/berkeleydb/db_bdb.cpp
+++ b/src/blockchain_db/berkeleydb/db_bdb.cpp
@@ -361,7 +361,7 @@ void BlockchainBDB::remove_transaction_data(const crypto::hash& tx_hash, const t
         throw1(DB_ERROR("Failed to add removal of tx outputs to db transaction"));
 }
 
-void BlockchainBDB::add_output(const crypto::hash& tx_hash, const tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time)
+void BlockchainBDB::add_output(const crypto::hash& tx_hash, const tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time, const rct::key *commitment)
 {
     LOG_PRINT_L3("BlockchainBDB::" << __func__);
     check_open();
diff --git a/src/blockchain_db/berkeleydb/db_bdb.h b/src/blockchain_db/berkeleydb/db_bdb.h
index 6bc26ed3d..f8e9440bd 100644
--- a/src/blockchain_db/berkeleydb/db_bdb.h
+++ b/src/blockchain_db/berkeleydb/db_bdb.h
@@ -364,7 +364,7 @@ private:
 
   virtual void remove_transaction_data(const crypto::hash& tx_hash, const transaction& tx);
 
-  virtual void add_output(const crypto::hash& tx_hash, const tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time);
+  virtual void add_output(const crypto::hash& tx_hash, const tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time, const rct::key *commitment);
 
   virtual void remove_output(const tx_out& tx_output);
 
diff --git a/src/blockchain_db/blockchain_db.cpp b/src/blockchain_db/blockchain_db.cpp
index 86a117405..3719d9eb0 100644
--- a/src/blockchain_db/blockchain_db.cpp
+++ b/src/blockchain_db/blockchain_db.cpp
@@ -90,9 +90,8 @@ void BlockchainDB::add_transaction(const crypto::hash& blk_hash, const transacti
   // we need the index
   for (uint64_t i = 0; i < tx.vout.size(); ++i)
   {
-    amount_output_indices.push_back(add_output(tx_hash, tx.vout[i], i, tx.unlock_time));
-    if (tx.version > 1 && tx.vout[i].amount == 0)
-      add_rct_commitment(tx.rct_signatures.outPk[i].mask);
+    amount_output_indices.push_back(add_output(tx_hash, tx.vout[i], i, tx.unlock_time,
+      tx.version > 1 ? &tx.rct_signatures.outPk[i].mask : NULL));
   }
   add_tx_amount_output_indices(tx_id, amount_output_indices);
 }
diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h
index 5b759db4c..d26080a3b 100644
--- a/src/blockchain_db/blockchain_db.h
+++ b/src/blockchain_db/blockchain_db.h
@@ -110,6 +110,7 @@ struct output_data_t
   crypto::public_key pubkey;       //!< the output's public key (for spend verification)
   uint64_t           unlock_time;  //!< the output's unlock time (or height)
   uint64_t           height;       //!< the height of the block which created the output
+  rct::key           commitment;   //!< the output's amount commitment (for spend verification)
 };
 #pragma pack(pop)
 
@@ -407,9 +408,10 @@ private:
    * @param tx_output the output
    * @param local_index index of the output in its transaction
    * @param unlock_time unlock time/height of the output
+   * @param commitment the rct commitment to the output amount
    * @return amount output index
    */
-  virtual uint64_t add_output(const crypto::hash& tx_hash, const tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time) = 0;
+  virtual uint64_t add_output(const crypto::hash& tx_hash, const tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time, const rct::key *commitment) = 0;
 
   /**
    * @brief store amount output indices for a tx's outputs
@@ -1200,39 +1202,6 @@ public:
   virtual bool has_key_image(const crypto::key_image& img) const = 0;
 
   /**
-   * @brief returns the number of ringct outputs in the database
-   *
-   * @return the number of ringct outputs in the database
-   */
-  virtual uint64_t get_num_rct_outputs() const = 0;
-
-  /**
-   * @brief returns the commitment for a given ringct output
-   *
-   * Throws OUTPUT_DNE if the index is out of range
-   * Throws DB_ERROR on other error
-   *
-   * @return the commitment for the given index
-   */
-  virtual rct::key get_rct_commitment(uint64_t idx) const = 0;
-
-  /**
-   * @brief Adds a new ringct output with the given commitment
-   *
-   * Throws DB_ERROR if the addition fails
-   *
-   * @return the index of the newly added record
-   */
-  virtual uint64_t add_rct_commitment(const rct::key &commitment) = 0;
-
-  /**
-   * @brief Remove a ringct output with the given index
-   *
-   * Throws DB_ERROR if the removal fails
-   */
-  virtual void remove_rct_commitment(uint64_t idx) = 0;
-
-  /**
    * @brief runs a function over all key images stored
    *
    * The subclass should run the passed function for each key image it has
diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp
index 887740e06..a0a0a041d 100644
--- a/src/blockchain_db/lmdb/db_lmdb.cpp
+++ b/src/blockchain_db/lmdb/db_lmdb.cpp
@@ -51,6 +51,13 @@ using epee::string_tools::pod_to_hex;
 namespace
 {
 
+struct pre_rct_output_data_t
+{
+  crypto::public_key pubkey;       //!< the output's public key (for spend verification)
+  uint64_t           unlock_time;  //!< the output's unlock time (or height)
+  uint64_t           height;       //!< the height of the block which created the output
+};
+
 template <typename T>
 inline void throw0(const T &e)
 {
@@ -155,8 +162,6 @@ int compare_string(const MDB_val *a, const MDB_val *b)
  *
  * spent_keys       input hash   -
  *
- * rct_commitments  input ID     {commitment key}
- *
  * Note: where the data items are of uniform size, DUPFIXED tables have
  * been used to save space. In most of these cases, a dummy "zerokval"
  * key is used when accessing the table; the Key listed above will be
@@ -177,8 +182,6 @@ const char* const LMDB_OUTPUT_TXS = "output_txs";
 const char* const LMDB_OUTPUT_AMOUNTS = "output_amounts";
 const char* const LMDB_SPENT_KEYS = "spent_keys";
 
-const char* const LMDB_RCT_COMMITMENTS  = "rct_commitments";
-
 const char* const LMDB_HF_STARTING_HEIGHTS = "hf_starting_heights";
 const char* const LMDB_HF_VERSIONS = "hf_versions";
 
@@ -246,6 +249,12 @@ typedef struct txindex {
     tx_data_t data;
 } txindex;
 
+typedef struct pre_rct_outkey {
+    uint64_t amount_index;
+    uint64_t output_id;
+    pre_rct_output_data_t data;
+} pre_rct_outkey;
+
 typedef struct outkey {
     uint64_t amount_index;
     uint64_t output_id;
@@ -769,7 +778,8 @@ void BlockchainLMDB::remove_transaction_data(const crypto::hash& tx_hash, const
 uint64_t BlockchainLMDB::add_output(const crypto::hash& tx_hash,
     const tx_out& tx_output,
     const uint64_t& local_index,
-    const uint64_t unlock_time)
+    const uint64_t unlock_time,
+    const rct::key *commitment)
 {
   LOG_PRINT_L3("BlockchainLMDB::" << __func__);
   check_open();
@@ -782,6 +792,8 @@ uint64_t BlockchainLMDB::add_output(const crypto::hash& tx_hash,
 
   if (tx_output.target.type() != typeid(txout_to_key))
     throw0(DB_ERROR("Wrong output type: expected txout_to_key"));
+  if (tx_output.amount == 0 && !commitment)
+    throw0(DB_ERROR("RCT output without commitment"));
 
   outtx ot = {m_num_outputs, tx_hash, local_index};
   MDB_val_set(vot, ot);
@@ -810,8 +822,16 @@ uint64_t BlockchainLMDB::add_output(const crypto::hash& tx_hash,
   ok.data.pubkey = boost::get < txout_to_key > (tx_output.target).key;
   ok.data.unlock_time = unlock_time;
   ok.data.height = m_height;
+  if (tx_output.amount == 0)
+  {
+    ok.data.commitment = *commitment;
+    data.mv_size = sizeof(ok);
+  }
+  else
+  {
+    data.mv_size = sizeof(pre_rct_outkey);
+  }
   data.mv_data = &ok;
-  data.mv_size = sizeof(ok);
 
   if ((result = mdb_cursor_put(m_cur_output_amounts, &val_amount, &data, MDB_APPENDDUP)))
       throw0(DB_ERROR(lmdb_error("Failed to add output pubkey to db transaction: ", result).c_str()));
@@ -861,8 +881,6 @@ void BlockchainLMDB::remove_tx_outputs(const uint64_t tx_id, const transaction&
   {
     const tx_out tx_output = tx.vout[i-1];
     remove_output(tx_output.amount, amount_output_indices[i-1]);
-    if (tx_output.amount == 0)
-      remove_rct_commitment(amount_output_indices[i-1]);
   }
 }
 
@@ -1090,8 +1108,6 @@ void BlockchainLMDB::open(const std::string& filename, const int mdb_flags)
 
   lmdb_db_open(txn, LMDB_SPENT_KEYS, MDB_INTEGERKEY | MDB_CREATE | MDB_DUPSORT | MDB_DUPFIXED, m_spent_keys, "Failed to open db handle for m_spent_keys");
 
-  lmdb_db_open(txn, LMDB_RCT_COMMITMENTS, MDB_INTEGERKEY | MDB_CREATE | MDB_DUPSORT | MDB_DUPFIXED, m_rct_commitments, "Failed to open db handle for m_rct_commitments");
-
   // this subdb is dropped on sight, so it may not be present when we open the DB.
   // Since we use MDB_CREATE, we'll get an exception if we open read-only and it does not exist.
   // So we don't open for read-only, and also not drop below. It is not used elsewhere.
@@ -1261,8 +1277,6 @@ void BlockchainLMDB::reset()
     throw0(DB_ERROR(lmdb_error("Failed to drop m_output_amounts: ", result).c_str()));
   if (auto result = mdb_drop(txn, m_spent_keys, 0))
     throw0(DB_ERROR(lmdb_error("Failed to drop m_spent_keys: ", result).c_str()));
-  if (auto result = mdb_drop(txn, m_rct_commitments, 0))
-    throw0(DB_ERROR(lmdb_error("Failed to drop m_rct_commitments: ", result).c_str()));
   (void)mdb_drop(txn, m_hf_starting_heights, 0); // this one is dropped in new code
   if (auto result = mdb_drop(txn, m_hf_versions, 0))
     throw0(DB_ERROR(lmdb_error("Failed to drop m_hf_versions: ", result).c_str()));
@@ -1943,8 +1957,19 @@ output_data_t BlockchainLMDB::get_output_key(const uint64_t& amount, const uint6
     throw1(OUTPUT_DNE("Attempting to get output pubkey by index, but key does not exist"));
   else if (get_result)
     throw0(DB_ERROR("Error attempting to retrieve an output pubkey from the db"));
-  outkey *okp = (outkey *)v.mv_data;
-  output_data_t ret = okp->data;
+
+  output_data_t ret;
+  if (amount == 0)
+  {
+    const outkey *okp = (const outkey *)v.mv_data;
+    ret = okp->data;
+  }
+  else
+  {
+    const pre_rct_outkey *okp = (const pre_rct_outkey *)v.mv_data;
+    memcpy(&ret, &okp->data, sizeof(pre_rct_output_data_t));;
+    ret.commitment = rct::zeroCommit(amount);
+  }
   TXN_POSTFIX_RDONLY();
   return ret;
 }
@@ -2567,8 +2592,18 @@ void BlockchainLMDB::get_output_key(const uint64_t &amount, const std::vector<ui
     else if (get_result)
       throw0(DB_ERROR(lmdb_error("Error attempting to retrieve an output pubkey from the db", get_result).c_str()));
 
-    outkey *okp = (outkey *)v.mv_data;
-    output_data_t data = okp->data;
+    output_data_t data;
+    if (amount == 0)
+    {
+      const outkey *okp = (const outkey *)v.mv_data;
+      data = okp->data;
+    }
+    else
+    {
+      const pre_rct_outkey *okp = (const pre_rct_outkey *)v.mv_data;
+      memcpy(&data, &okp->data, sizeof(pre_rct_output_data_t));
+      data.commitment = rct::zeroCommit(amount);
+    }
     outputs.push_back(data);
   }
 
@@ -2742,106 +2777,6 @@ uint8_t BlockchainLMDB::get_hard_fork_version(uint64_t height) const
   return ret;
 }
 
-uint64_t BlockchainLMDB::get_num_rct_outputs() const
-{
-  LOG_PRINT_L3("BlockchainLMDB::" << __func__);
-  check_open();
-
-  TXN_PREFIX_RDONLY();
-  RCURSOR(rct_commitments);
-
-  mdb_size_t num_outputs = 0;
-  MDB_stat ms;
-  auto result = mdb_stat(m_txn, m_rct_commitments, &ms);
-  if (result == MDB_SUCCESS)
-  {
-    num_outputs = ms.ms_entries;
-  }
-  else if (result == MDB_NOTFOUND)
-  {
-    num_outputs = 0;
-  }
-  else
-  {
-    throw0(DB_ERROR("DB error attempting to get number of ringct outputs"));
-  }
-
-  TXN_POSTFIX_RDONLY();
-
-  return num_outputs;
-}
-
-rct::key BlockchainLMDB::get_rct_commitment(uint64_t idx) const
-{
-  LOG_PRINT_L3("BlockchainLMDB::" << __func__);
-  check_open();
-
-  TXN_PREFIX_RDONLY();
-  RCURSOR(rct_commitments);
-
-  MDB_val_set(val_key, idx);
-  MDB_val val_ret;
-  auto result = mdb_cursor_get(m_cur_rct_commitments, &val_key, &val_ret, MDB_SET);
-  if (result == MDB_NOTFOUND)
-    throw0(OUTPUT_DNE(lmdb_error("Error attempting to retrieve rct commitment for " + boost::lexical_cast<std::string>(idx) + " from the db: ", result).c_str()));
-  else if (result)
-    throw0(DB_ERROR(lmdb_error("Error attempting to retrieve rct commitment for " + boost::lexical_cast<std::string>(idx) + " from the db: ", result).c_str()));
-
-  rct::key commitment = *(const rct::key*)val_ret.mv_data;
-  TXN_POSTFIX_RDONLY();
-  return commitment;
-}
-
-void BlockchainLMDB::remove_rct_commitment(uint64_t idx)
-{
-  LOG_PRINT_L3("BlockchainLMDB::" << __func__);
-  check_open();
-
-  mdb_txn_cursors *m_cursors = &m_wcursors;
-  CURSOR(rct_commitments);
-
-  MDB_val_set(val_key, idx);
-  MDB_val val_ret;
-  auto result = mdb_cursor_get(m_cur_rct_commitments, &val_key, &val_ret, MDB_SET);
-  if (result == MDB_NOTFOUND)
-    throw0(OUTPUT_DNE(lmdb_error("Error attempting to retrieve rct commitment for " + boost::lexical_cast<std::string>(idx) + " from the db: ", result).c_str()));
-  else if (result)
-    throw0(DB_ERROR(lmdb_error("Error attempting to retrieve rct commitment for " + boost::lexical_cast<std::string>(idx) + " from the db: ", result).c_str()));
-  result = mdb_cursor_del(m_cur_rct_commitments, 0);
-  if (result)
-    throw0(DB_ERROR(lmdb_error("Error attempting to remove rct commitment for " + boost::lexical_cast<std::string>(idx) + " from the db: ", result).c_str()));
-}
-
-uint64_t BlockchainLMDB::add_rct_commitment(const rct::key &commitment)
-{
-  LOG_PRINT_L3("BlockchainLMDB::" << __func__);
-  check_open();
-
-  mdb_txn_cursors *m_cursors = &m_wcursors;
-
-  CURSOR(rct_commitments);
-
-  uint64_t num_outputs = 0;
-  MDB_stat ms;
-  auto result = mdb_stat(*m_write_txn, m_rct_commitments, &ms);
-  if (result == MDB_SUCCESS)
-    num_outputs = ms.ms_entries;
-  else if (result == MDB_NOTFOUND)
-    num_outputs = 0;
-  else
-    throw0(DB_ERROR("DB error attempting to get number of ringct outputs"));
-
-  MDB_val_set(val_key, num_outputs);
-  MDB_val val_value;
-  val_value.mv_size = sizeof(rct::key);
-  val_value.mv_data = (void*)&commitment;
-  result = mdb_cursor_put(m_cur_rct_commitments, (MDB_val *)&val_key, &val_value, 0);
-  if (result)
-    throw0(DB_ERROR(lmdb_error("Failed to add rct output to db transaction: ", result).c_str()));
-
-  return num_outputs;
-}
-
 bool BlockchainLMDB::is_read_only() const
 {
   unsigned int flags;
diff --git a/src/blockchain_db/lmdb/db_lmdb.h b/src/blockchain_db/lmdb/db_lmdb.h
index 9e92b0661..050e9e0ae 100644
--- a/src/blockchain_db/lmdb/db_lmdb.h
+++ b/src/blockchain_db/lmdb/db_lmdb.h
@@ -55,8 +55,6 @@ typedef struct mdb_txn_cursors
 
   MDB_cursor *m_txc_spent_keys;
 
-  MDB_cursor *m_txc_rct_commitments;
-
   MDB_cursor *m_txc_hf_versions;
 } mdb_txn_cursors;
 
@@ -69,7 +67,6 @@ typedef struct mdb_txn_cursors
 #define m_cur_tx_indices	m_cursors->m_txc_tx_indices
 #define m_cur_tx_outputs	m_cursors->m_txc_tx_outputs
 #define m_cur_spent_keys	m_cursors->m_txc_spent_keys
-#define m_cur_rct_commitments	m_cursors->m_txc_rct_commitments
 #define m_cur_hf_versions	m_cursors->m_txc_hf_versions
 
 typedef struct mdb_rflags
@@ -84,7 +81,6 @@ typedef struct mdb_rflags
   bool m_rf_tx_indices;
   bool m_rf_tx_outputs;
   bool m_rf_spent_keys;
-  bool m_rf_rct_commitments;
   bool m_rf_hf_versions;
 } mdb_rflags;
 
@@ -236,11 +232,6 @@ public:
 
   virtual bool has_key_image(const crypto::key_image& img) const;
 
-  virtual uint64_t get_num_rct_outputs() const;
-  virtual rct::key get_rct_commitment(uint64_t idx) const;
-  virtual uint64_t add_rct_commitment(const rct::key &commitment);
-  virtual void remove_rct_commitment(uint64_t idx);
-
   virtual bool for_all_key_images(std::function<bool(const crypto::key_image&)>) const;
   virtual bool for_all_blocks(std::function<bool(uint64_t, const crypto::hash&, const cryptonote::block&)>) const;
   virtual bool for_all_transactions(std::function<bool(const crypto::hash&, const cryptonote::transaction&)>) const;
@@ -302,7 +293,8 @@ private:
   virtual uint64_t add_output(const crypto::hash& tx_hash,
       const tx_out& tx_output,
       const uint64_t& local_index,
-      const uint64_t unlock_time
+      const uint64_t unlock_time,
+      const rct::key *commitment
       );
 
   virtual void add_tx_amount_output_indices(const uint64_t tx_id,
@@ -369,8 +361,6 @@ private:
 
   MDB_dbi m_spent_keys;
 
-  MDB_dbi m_rct_commitments;
-
   MDB_dbi m_hf_starting_heights;
   MDB_dbi m_hf_versions;
 
diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp
index 82bc6c0d4..f46de9ca8 100644
--- a/src/cryptonote_core/blockchain.cpp
+++ b/src/cryptonote_core/blockchain.cpp
@@ -207,21 +207,8 @@ bool Blockchain::scan_outputkeys_for_indexes(size_t tx_version, const txin_to_ke
         else
           output_index = m_db->get_output_key(tx_in_to_key.amount, i);
 
-        rct::key commitment;
-        if (tx_version > 1)
-        {
-          if (tx_in_to_key.amount == 0)
-            commitment = m_db->get_rct_commitment(i);
-          else
-            commitment = rct::zeroCommit(tx_in_to_key.amount);
-        }
-        else
-        {
-          rct::identity(commitment);
-        }
-
         // call to the passed boost visitor to grab the public key for the output
-        if (!vis.handle_output(output_index.unlock_time, output_index.pubkey, commitment))
+        if (!vis.handle_output(output_index.unlock_time, output_index.pubkey, output_index.commitment))
         {
           LOG_PRINT_L0("Failed to handle_output for output no = " << count << ", with absolute offset " << i);
           return false;
@@ -1633,15 +1620,7 @@ void Blockchain::add_out_to_get_rct_random_outs(std::list<COMMAND_RPC_GET_RANDOM
   oen.global_amount_index = i;
   output_data_t data = m_db->get_output_key(amount, i);
   oen.out_key = data.pubkey;
-  if (amount == 0)
-  {
-    oen.commitment = m_db->get_rct_commitment(i);
-  }
-  else
-  {
-    // not a rct output, make a fake commitment with zero key
-    oen.commitment = rct::zeroCommit(amount);
-  }
+  oen.commitment = data.commitment;
 }
 //------------------------------------------------------------------
 // This function takes an RPC request for mixins and creates an RPC response
diff --git a/tests/unit_tests/hardfork.cpp b/tests/unit_tests/hardfork.cpp
index 1eb73e156..a279b6c68 100644
--- a/tests/unit_tests/hardfork.cpp
+++ b/tests/unit_tests/hardfork.cpp
@@ -96,14 +96,10 @@ public:
   virtual void remove_block() { blocks.pop_back(); }
   virtual uint64_t add_transaction_data(const crypto::hash& blk_hash, const transaction& tx, const crypto::hash& tx_hash) {return 0;}
   virtual void remove_transaction_data(const crypto::hash& tx_hash, const transaction& tx) {}
-  virtual uint64_t add_output(const crypto::hash& tx_hash, const tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time) {return 0;}
+  virtual uint64_t add_output(const crypto::hash& tx_hash, const tx_out& tx_output, const uint64_t& local_index, const uint64_t unlock_time, const rct::key *commitment) {return 0;}
   virtual void add_tx_amount_output_indices(const uint64_t tx_index, const std::vector<uint64_t>& amount_output_indices) {}
   virtual void add_spent_key(const crypto::key_image& k_image) {}
   virtual void remove_spent_key(const crypto::key_image& k_image) {}
-  virtual uint64_t get_num_rct_outputs() const { return 0; }
-  virtual rct::key get_rct_commitment(uint64_t idx) const { return rct::key(); }
-  virtual uint64_t add_rct_commitment(const rct::key &commitment) { return 0; }
-  virtual void remove_rct_commitment(uint64_t idx) {}
 
   virtual bool for_all_key_images(std::function<bool(const crypto::key_image&)>) const { return true; }
   virtual bool for_all_blocks(std::function<bool(uint64_t, const crypto::hash&, const cryptonote::block&)>) const { return true; }