fcmp++: store {output pubkey, commitment} in db, pre-torsion clear

- We must use the output pubkey to calculate key image generator I
- Since torsion cleared outputs can be spent via ring sig today,
if we torsion clear outputs **before** calculating I, then the key
image of torsioned outputs will be different when constructing
fcmp's, effectively enabling a double spend of torsioned outputs
via ring sig before fcmp's and again via fcmp.
- Storing {output pubkey, commitment} instead of {O.x,I.x,C.x} to
save 32 bytes per output.

src/blockchain_db/lmdb/db_lmdb.cpp

@@ -199,8 +199,8 @@ namespace
  *
  * spent_keys       input hash   -
  *
- * locked_leaves    block ID     [{output ID, leaf tuple}...]
+ * locked_outputs   block ID     [{output ID, output pubkey, commitment}...]
- * leaves           leaf_idx     leaf tuple
+ * leaves           leaf_idx     {output pubkey, commitment}
  * layers           layer_idx    [{child_chunk_idx, child_chunk_hash}...]
  *
  * txpool_meta      txn hash     txn metadata
@@ -233,7 +233,7 @@ const char* const LMDB_OUTPUT_AMOUNTS = "output_amounts";
 const char* const LMDB_SPENT_KEYS = "spent_keys";
 
 // Curve trees merkle tree tables
-const char* const LMDB_LOCKED_LEAVES = "locked_leaves";
+const char* const LMDB_LOCKED_OUTPUTS = "locked_outputs";
 const char* const LMDB_LEAVES = "leaves";
 const char* const LMDB_LAYERS = "layers";
 
@@ -827,11 +827,11 @@ void BlockchainLMDB::add_block(const block& blk, size_t block_weight, uint64_t l
   }
 
   // Grow the tree with outputs that unlock at this block height
-  auto unlocked_leaves = this->get_leaf_tuples_at_unlock_block_id(m_height);
+  auto unlocked_outputs = this->get_outs_at_unlock_block_id(m_height);
-  this->grow_tree(std::move(unlocked_leaves));
+  this->grow_tree(std::move(unlocked_outputs));
 
-  // Now that we've used the unlocked leaves to grow the tree, we can delete them from the locked leaves table
+  // Now that we've used the unlocked leaves to grow the tree, we can delete them from the locked outputs table
-  this->del_locked_leaf_tuples_at_block_id(m_height);
+  this->del_locked_outs_at_block_id(m_height);
 
   int result = 0;
 
@@ -878,17 +878,17 @@ void BlockchainLMDB::add_block(const block& blk, size_t block_weight, uint64_t l
   if (result)
     throw0(DB_ERROR(lmdb_error("Failed to add block height by hash to db transaction: ", result).c_str()));
 
-  CURSOR(locked_leaves)
+  CURSOR(locked_outputs)
 
-  // Add the locked leaf tuples from this block to the locked leaves table
+  // Add the locked outputs from this block to the locked outputs table
-  for (const auto &locked_tuple : leaf_tuples_by_unlock_block)
+  for (const auto &locked_output : leaf_tuples_by_unlock_block)
   {
-    MDB_val_set(k_block_id, locked_tuple.first);
+    MDB_val_set(k_block_id, locked_output.first);
-    MDB_val_set(v_tuple, locked_tuple.second);
+    MDB_val_set(v_output, locked_output.second);
 
     // MDB_NODUPDATA because no benefit to having duplicate outputs in the tree, only 1 can be spent
     // Can't use MDB_APPENDDUP because outputs aren't inserted in order sorted by unlock height
-    result = mdb_cursor_put(m_cur_locked_leaves, &k_block_id, &v_tuple, MDB_NODUPDATA);
+    result = mdb_cursor_put(m_cur_locked_outputs, &k_block_id, &v_output, MDB_NODUPDATA);
     if (result != MDB_SUCCESS && result != MDB_KEYEXIST)
       throw0(DB_ERROR(lmdb_error("Failed to add locked output: ", result).c_str()));
   }
@@ -1387,7 +1387,6 @@ void BlockchainLMDB::grow_tree(std::vector<fcmp_pp::curve_trees::LeafTupleContex
   // TODO: grow_layers
   const auto &c2_extensions = tree_extension.c2_layer_extensions;
   const auto &c1_extensions = tree_extension.c1_layer_extensions;
-  CHECK_AND_ASSERT_THROW_MES(!c2_extensions.empty(), "empty c2 extensions");
 
   bool use_c2 = true;
   uint64_t c2_idx = 0;
@@ -1400,7 +1399,7 @@ void BlockchainLMDB::grow_tree(std::vector<fcmp_pp::curve_trees::LeafTupleContex
     if (use_c2)
     {
       if (layer_idx % 2 != 0)
-        throw0(DB_ERROR(("Growing odd c2 layer, expected even layer idx for c1: "
+        throw0(DB_ERROR(("Growing odd c2 layer, expected even layer idx for c2: "
           + std::to_string(layer_idx)).c_str()));
 
       this->grow_layer<fcmp_pp::curve_trees::Selene>(m_curve_trees->m_c2,
@@ -1413,7 +1412,7 @@ void BlockchainLMDB::grow_tree(std::vector<fcmp_pp::curve_trees::LeafTupleContex
     else
     {
       if (layer_idx % 2 == 0)
-        throw0(DB_ERROR(("Growing even c1 layer, expected odd layer idx for c2: "
+        throw0(DB_ERROR(("Growing even c1 layer, expected odd layer idx for c1: "
           + std::to_string(layer_idx)).c_str()));
 
       this->grow_layer<fcmp_pp::curve_trees::Helios>(m_curve_trees->m_c1,
@@ -1817,10 +1816,10 @@ fcmp_pp::curve_trees::CurveTreesV1::LastChunkChildrenToTrim BlockchainLMDB::get_
         if (result != MDB_SUCCESS)
           throw0(DB_ERROR(lmdb_error("Failed to get leaf: ", result).c_str()));
 
-        const auto preprocessed_leaf_tuple = *(fcmp_pp::curve_trees::PreprocessedLeafTuple *)v.mv_data;
+        const auto output_pair = *(fcmp_pp::curve_trees::OutputPair *)v.mv_data;
 
         // TODO: parallelize calls to this function
-        auto leaf = m_curve_trees->leaf_tuple(preprocessed_leaf_tuple);
+        auto leaf = m_curve_trees->leaf_tuple(output_pair);
 
         leaves_to_trim.emplace_back(std::move(leaf.O_x));
         leaves_to_trim.emplace_back(std::move(leaf.I_x));
@@ -1993,8 +1992,8 @@ bool BlockchainLMDB::audit_tree(const uint64_t expected_n_leaf_tuples) const
       if (result != MDB_SUCCESS)
         throw0(DB_ERROR(lmdb_error("Failed to add leaf: ", result).c_str()));
 
-      const auto preprocessed_leaf_tuple = *(fcmp_pp::curve_trees::PreprocessedLeafTuple *)v.mv_data;
+      const auto output_pair = *(fcmp_pp::curve_trees::OutputPair *)v.mv_data;
-      auto leaf = m_curve_trees->leaf_tuple(preprocessed_leaf_tuple);
+      auto leaf = m_curve_trees->leaf_tuple(output_pair);
 
       leaf_tuples_chunk.emplace_back(std::move(leaf));
 
@@ -2226,17 +2225,17 @@ bool BlockchainLMDB::audit_layer(const std::unique_ptr<C_CHILD> &c_child,
   return audit_complete;
 }
 
-std::vector<fcmp_pp::curve_trees::LeafTupleContext> BlockchainLMDB::get_leaf_tuples_at_unlock_block_id(
+std::vector<fcmp_pp::curve_trees::LeafTupleContext> BlockchainLMDB::get_outs_at_unlock_block_id(
   uint64_t block_id)
 {
   LOG_PRINT_L3("BlockchainLMDB::" << __func__);
   check_open();
 
   TXN_PREFIX_RDONLY();
-  RCURSOR(locked_leaves)
+  RCURSOR(locked_outputs)
 
   MDB_val_set(k_block_id, block_id);
-  MDB_val v_tuple;
+  MDB_val v_output;
 
   // Get all the locked outputs at the provided block id
   std::vector<fcmp_pp::curve_trees::LeafTupleContext> leaf_tuples;
@@ -2244,7 +2243,7 @@ std::vector<fcmp_pp::curve_trees::LeafTupleContext> BlockchainLMDB::get_leaf_tup
   MDB_cursor_op op = MDB_SET;
   while (1)
   {
-    int result = mdb_cursor_get(m_cur_locked_leaves, &k_block_id, &v_tuple, op);
+    int result = mdb_cursor_get(m_cur_locked_outputs, &k_block_id, &v_output, op);
     if (result == MDB_NOTFOUND)
       break;
     if (result != MDB_SUCCESS)
@@ -2255,8 +2254,8 @@ std::vector<fcmp_pp::curve_trees::LeafTupleContext> BlockchainLMDB::get_leaf_tup
     if (blk_id != block_id)
       throw0(DB_ERROR(("Blk id " + std::to_string(blk_id) + " not the expected" + std::to_string(block_id)).c_str()));
 
-    const auto range_begin = ((const fcmp_pp::curve_trees::LeafTupleContext*)v_tuple.mv_data);
+    const auto range_begin = ((const fcmp_pp::curve_trees::LeafTupleContext*)v_output.mv_data);
-    const auto range_end = range_begin + v_tuple.mv_size / sizeof(fcmp_pp::curve_trees::LeafTupleContext);
+    const auto range_end = range_begin + v_output.mv_size / sizeof(fcmp_pp::curve_trees::LeafTupleContext);
 
     auto it = range_begin;
 
@@ -2276,25 +2275,25 @@ std::vector<fcmp_pp::curve_trees::LeafTupleContext> BlockchainLMDB::get_leaf_tup
   return leaf_tuples;
 }
 
-void BlockchainLMDB::del_locked_leaf_tuples_at_block_id(uint64_t block_id)
+void BlockchainLMDB::del_locked_outs_at_block_id(uint64_t block_id)
 {
   LOG_PRINT_L3("BlockchainLMDB::" << __func__);
   check_open();
   mdb_txn_cursors *m_cursors = &m_wcursors;
 
-  CURSOR(locked_leaves)
+  CURSOR(locked_outputs)
 
   MDB_val_set(k_block_id, block_id);
 
-  int result = mdb_cursor_get(m_cur_locked_leaves, &k_block_id, NULL, MDB_SET);
+  int result = mdb_cursor_get(m_cur_locked_outputs, &k_block_id, NULL, MDB_SET);
   if (result == MDB_NOTFOUND)
     return;
   if (result != MDB_SUCCESS)
-    throw1(DB_ERROR(lmdb_error("Error finding locked leaf tuples to remove: ", result).c_str()));
+    throw1(DB_ERROR(lmdb_error("Error finding locked outputs to remove: ", result).c_str()));
 
-  result = mdb_cursor_del(m_cur_locked_leaves, MDB_NODUPDATA);
+  result = mdb_cursor_del(m_cur_locked_outputs, MDB_NODUPDATA);
   if (result)
-    throw1(DB_ERROR(lmdb_error("Error removing locked leaf tuples: ", result).c_str()));
+    throw1(DB_ERROR(lmdb_error("Error removing locked outputs: ", result).c_str()));
 }
 
 BlockchainLMDB::~BlockchainLMDB()
@@ -2451,7 +2450,7 @@ void BlockchainLMDB::open(const std::string& filename, const int db_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_LOCKED_LEAVES, MDB_INTEGERKEY | MDB_DUPSORT | MDB_DUPFIXED | MDB_CREATE, m_locked_leaves, "Failed to open db handle for m_locked_leaves");
+  lmdb_db_open(txn, LMDB_LOCKED_OUTPUTS, MDB_INTEGERKEY | MDB_DUPSORT | MDB_DUPFIXED | MDB_CREATE, m_locked_outputs, "Failed to open db handle for m_locked_outputs");
   lmdb_db_open(txn, LMDB_LEAVES, MDB_INTEGERKEY | MDB_CREATE, m_leaves, "Failed to open db handle for m_leaves");
   lmdb_db_open(txn, LMDB_LAYERS, MDB_INTEGERKEY | MDB_DUPSORT | MDB_DUPFIXED | MDB_CREATE, m_layers, "Failed to open db handle for m_layers");
 
@@ -2474,7 +2473,7 @@ void BlockchainLMDB::open(const std::string& filename, const int db_flags)
   mdb_set_dupsort(txn, m_block_heights, compare_hash32);
   mdb_set_dupsort(txn, m_tx_indices, compare_hash32);
   mdb_set_dupsort(txn, m_output_amounts, compare_uint64);
-  mdb_set_dupsort(txn, m_locked_leaves, compare_uint64);
+  mdb_set_dupsort(txn, m_locked_outputs, compare_uint64);
   mdb_set_dupsort(txn, m_leaves, compare_uint64);
   mdb_set_dupsort(txn, m_layers, compare_uint64);
   mdb_set_dupsort(txn, m_output_txs, compare_uint64);
@@ -2654,8 +2653,8 @@ 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_locked_leaves, 0))
+  if (auto result = mdb_drop(txn, m_locked_outputs, 0))
-    throw0(DB_ERROR(lmdb_error("Failed to drop m_locked_leaves: ", result).c_str()));
+    throw0(DB_ERROR(lmdb_error("Failed to drop m_locked_outputs: ", result).c_str()));
   if (auto result = mdb_drop(txn, m_leaves, 0))
     throw0(DB_ERROR(lmdb_error("Failed to drop m_leaves: ", result).c_str()));
   if (auto result = mdb_drop(txn, m_layers, 0))
@@ -6719,7 +6718,7 @@ void BlockchainLMDB::migrate_5_6()
   do
   {
     // 1. Prepare all valid outputs to be inserted into the merkle tree and
-    //    place them in a locked leaves table. The key to this new table is the
+    //    place them in a locked outputs table. The key to this new table is the
     //    block id in which the outputs unlock.
     {
       MINFO("Setting up a locked outputs table (step 1/2 of full-chain membership proof migration)");
@@ -6727,8 +6726,8 @@ void BlockchainLMDB::migrate_5_6()
       result = mdb_txn_begin(m_env, NULL, 0, txn);
       if (result)
         throw0(DB_ERROR(lmdb_error("Failed to create a transaction for the db: ", result).c_str()));
-      lmdb_db_open(txn, LMDB_LOCKED_LEAVES, MDB_INTEGERKEY | MDB_DUPSORT | MDB_DUPFIXED | MDB_CREATE, m_locked_leaves, "Failed to open db handle for m_locked_leaves");
+      lmdb_db_open(txn, LMDB_LOCKED_OUTPUTS, MDB_INTEGERKEY | MDB_DUPSORT | MDB_DUPFIXED | MDB_CREATE, m_locked_outputs, "Failed to open db handle for m_locked_outputs");
-      mdb_set_dupsort(txn, m_locked_leaves, compare_uint64);
+      mdb_set_dupsort(txn, m_locked_outputs, compare_uint64);
       lmdb_db_open(txn, "tmp_last_output", MDB_INTEGERKEY | MDB_CREATE, m_tmp_last_output, "Failed to open db handle for m_tmp_last_output");
       txn.commit();
 
@@ -6742,7 +6741,7 @@ void BlockchainLMDB::migrate_5_6()
       struct tmp_output_cache { uint64_t n_outputs_read; uint64_t amount; outkey ok; };
       tmp_output_cache last_output;
 
-      MDB_cursor *c_output_amounts, *c_locked_leaves, *c_tmp_last_output;
+      MDB_cursor *c_output_amounts, *c_locked_outputs, *c_tmp_last_output;
       MDB_val k, v;
 
       i = 0;
@@ -6780,7 +6779,7 @@ void BlockchainLMDB::migrate_5_6()
           result = mdb_cursor_open(txn, m_output_amounts, &c_output_amounts);
           if (result)
             throw0(DB_ERROR(lmdb_error("Failed to open a cursor for output amounts: ", result).c_str()));
-          result = mdb_cursor_open(txn, m_locked_leaves, &c_locked_leaves);
+          result = mdb_cursor_open(txn, m_locked_outputs, &c_locked_outputs);
           if (result)
             throw0(DB_ERROR(lmdb_error("Failed to open a cursor for locked outputs: ", result).c_str()));
           result = mdb_cursor_open(txn, m_tmp_last_output, &c_tmp_last_output);
@@ -6867,35 +6866,25 @@ void BlockchainLMDB::migrate_5_6()
           last_output.n_outputs_read = i;
         }
 
-        // Convert the output into a leaf tuple context
+        // Prepare the output for insertion to the tree
-        fcmp_pp::curve_trees::LeafTupleContext tuple_context;
+        const auto tuple_context = m_curve_trees->output_to_leaf_context(output_id,
-        try
+          std::move(output_data.pubkey),
-        {
+          std::move(output_data.commitment));
-          tuple_context = m_curve_trees->output_to_leaf_context(
-            output_id,
-            output_data.pubkey,
-            output_data.commitment);
-        }
-        catch(...)
-        {
-          // Invalid outputs can't be added to the tree
-          continue;
-        }
 
         // Get the block in which the output will unlock
         const uint64_t unlock_block = cryptonote::get_unlock_block_index(output_data.unlock_time, output_data.height);
 
-        // Now add the leaf tuple to the locked leaves table
+        // Now add the output to the locked outputs table
         MDB_val_set(k_block_id, unlock_block);
-        MDB_val_set(v_tuple, tuple_context);
+        MDB_val_set(v_output, tuple_context);
 
         // MDB_NODUPDATA because no benefit to having duplicate outputs in the tree, only 1 can be spent
         // Can't use MDB_APPENDDUP because outputs aren't inserted in order sorted by unlock height
-        result = mdb_cursor_put(c_locked_leaves, &k_block_id, &v_tuple, MDB_NODUPDATA);
+        result = mdb_cursor_put(c_locked_outputs, &k_block_id, &v_output, MDB_NODUPDATA);
         if (result != MDB_SUCCESS && result != MDB_KEYEXIST)
           throw0(DB_ERROR(lmdb_error("Failed to add locked output: ", result).c_str()));
         if (result == MDB_KEYEXIST)
-          MDEBUG("Duplicate output pub key encountered: " << output_data.pubkey << " , output_id: " << output_id);
+          MDEBUG("Dup output pubkey: " << tuple_context.output_pair.output_pubkey << " , output_id: " << output_id);
       }
     }
 
@@ -6918,7 +6907,7 @@ void BlockchainLMDB::migrate_5_6()
       lmdb_db_open(txn, "block_infn", MDB_INTEGERKEY | MDB_CREATE | MDB_DUPSORT | MDB_DUPFIXED, m_block_info, "Failed to open db handle for block_infn");
       mdb_set_dupsort(txn, m_block_info, compare_uint64);
 
-      MDB_cursor *c_locked_leaves, *c_new_block_info, *c_old_block_info;
+      MDB_cursor *c_locked_outputs, *c_new_block_info, *c_old_block_info;
       MDB_val k_blk, v_blk;
 
       i = 0;
@@ -6945,7 +6934,7 @@ void BlockchainLMDB::migrate_5_6()
           }
 
           // Open all cursors
-          result = mdb_cursor_open(txn, m_locked_leaves, &c_locked_leaves);
+          result = mdb_cursor_open(txn, m_locked_outputs, &c_locked_outputs);
           if (result)
             throw0(DB_ERROR(lmdb_error("Failed to open a cursor for locked outputs: ", result).c_str()));
           result = mdb_cursor_open(txn, m_block_info, &c_new_block_info);
@@ -6969,11 +6958,11 @@ void BlockchainLMDB::migrate_5_6()
         }
 
         // Get the leaf tuples that unlock at the given block
-        auto unlocked_leaves = this->get_leaf_tuples_at_unlock_block_id(i);
+        auto unlocked_outputs = this->get_outs_at_unlock_block_id(i);
-        this->grow_tree(std::move(unlocked_leaves));
+        this->grow_tree(std::move(unlocked_outputs));
 
-        // Now that we've used the unlocked leaves to grow the tree, we delete them from the locked leaves table
+        // Now that we've used the unlocked leaves to grow the tree, we delete them from the locked outputs table
-        this->del_locked_leaf_tuples_at_block_id(i);
+        this->del_locked_outs_at_block_id(i);
 
         // Get old block_info and use it to set the new one with new values
         result = mdb_cursor_get(c_old_block_info, &k_blk, &v_blk, MDB_NEXT);

src/blockchain_db/lmdb/db_lmdb.h

@@ -65,7 +65,7 @@ typedef struct mdb_txn_cursors
 
   MDB_cursor *m_txc_spent_keys;
 
-  MDB_cursor *m_txc_locked_leaves;
+  MDB_cursor *m_txc_locked_outputs;
   MDB_cursor *m_txc_leaves;
   MDB_cursor *m_txc_layers;
 
@@ -92,7 +92,7 @@ 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_locked_leaves	m_cursors->m_txc_locked_leaves
+#define m_cur_locked_outputs	m_cursors->m_txc_locked_outputs
 #define m_cur_leaves		m_cursors->m_txc_leaves
 #define m_cur_layers		m_cursors->m_txc_layers
 #define m_cur_txpool_meta	m_cursors->m_txc_txpool_meta
@@ -117,7 +117,7 @@ typedef struct mdb_rflags
   bool m_rf_tx_indices;
   bool m_rf_tx_outputs;
   bool m_rf_spent_keys;
-  bool m_rf_locked_leaves;
+  bool m_rf_locked_outputs;
   bool m_rf_leaves;
   bool m_rf_layers;
   bool m_rf_txpool_meta;
@@ -444,12 +444,12 @@ private:
   template<typename C_CHILD, typename C_PARENT>
   bool audit_layer(const std::unique_ptr<C_CHILD> &c_child,
     const std::unique_ptr<C_PARENT> &c_parent,
-    const uint64_t layer_idx,
+    const uint64_t child_layer_idx,
     const uint64_t chunk_width) const;
 
-  std::vector<fcmp_pp::curve_trees::LeafTupleContext> get_leaf_tuples_at_unlock_block_id(uint64_t block_id);
+  std::vector<fcmp_pp::curve_trees::LeafTupleContext> get_outs_at_unlock_block_id(uint64_t block_id);
 
-  void del_locked_leaf_tuples_at_block_id(uint64_t block_id);
+  void del_locked_outs_at_block_id(uint64_t block_id);
 
   uint64_t num_outputs() const;
 
@@ -518,7 +518,7 @@ private:
 
   MDB_dbi m_spent_keys;
 
-  MDB_dbi m_locked_leaves;
+  MDB_dbi m_locked_outputs;
   MDB_dbi m_leaves;
   MDB_dbi m_layers;
 

src/fcmp_pp/curve_trees.cpp

@@ -643,31 +643,18 @@ static typename fcmp_pp::curve_trees::LayerReduction<C_PARENT> get_next_layer_re
 // CurveTrees public member functions
 //----------------------------------------------------------------------------------------------------------------------
 template<>
-LeafTupleContext CurveTrees<Helios, Selene>::output_to_leaf_context(
+LeafTupleContext CurveTrees<Helios, Selene>::output_to_leaf_context(const std::uint64_t output_id,
-    const std::uint64_t output_id,
+    crypto::public_key &&output_pubkey,
-    const crypto::public_key &output_pubkey,
+    rct::key &&commitment) const
-    const rct::key &commitment) const
 {
-    rct::key O, C;
+    auto output_pair = OutputPair{
-
+            .output_pubkey = std::move(output_pubkey),
-    if (!rct::clear_torsion(rct::pk2rct(output_pubkey), O))
+            .commitment    = std::move(commitment)
-        throw std::runtime_error("output pub key is invalid");
-    if (!rct::clear_torsion(commitment, C))
-        throw std::runtime_error("commitment is invalid");
-
-    if (O == rct::I)
-        throw std::runtime_error("O cannot equal identity");
-    if (C == rct::I)
-        throw std::runtime_error("C cannot equal identity");
-
-    PreprocessedLeafTuple o_c{
-            .O = std::move(O),
-            .C = std::move(C)
         };
 
     return LeafTupleContext{
-            .output_id               = output_id,
+            .output_id   = output_id,
-            .preprocessed_leaf_tuple = std::move(o_c)
+            .output_pair = std::move(output_pair)
         };
 };
 //----------------------------------------------------------------------------------------------------------------------
@@ -698,40 +685,43 @@ void CurveTrees<Helios, Selene>::tx_outs_to_leaf_tuple_contexts(const cryptonote
         if (!miner_tx && tx.version == 2)
             CHECK_AND_ASSERT_THROW_MES(tx.rct_signatures.outPk.size() > i, "unexpected size of outPk");
 
-        const rct::key commitment = (miner_tx || tx.version != 2)
+        rct::key commitment = (miner_tx || tx.version != 2)
             ? rct::zeroCommit(out.amount)
             : tx.rct_signatures.outPk[i].mask;
 
-        LeafTupleContext leaf_tuple_context;
+        auto tuple_context = output_to_leaf_context(output_ids[i],
-        try
+            std::move(output_public_key),
-        {
+            std::move(commitment));
-            // Convert output to leaf tuple context; throws if output is invalid
-            leaf_tuple_context = output_to_leaf_context(output_ids[i],
-                output_public_key,
-                commitment);
-        }
-        catch (...)
-        {
-            // We don't want leaf tuples from invalid outputs in the tree
-            continue;
-        };
 
-        leaf_tuples_by_unlock_block_inout.emplace(unlock_block, std::move(leaf_tuple_context));
+        leaf_tuples_by_unlock_block_inout.emplace(unlock_block, std::move(tuple_context));
     }
 }
 //----------------------------------------------------------------------------------------------------------------------
 template<>
 CurveTrees<Helios, Selene>::LeafTuple CurveTrees<Helios, Selene>::leaf_tuple(
-    const PreprocessedLeafTuple &preprocessed_leaf_tuple) const
+    const OutputPair &output_pair) const
 {
-    const rct::key &O = preprocessed_leaf_tuple.O;
+    const crypto::public_key &output_pubkey = output_pair.output_pubkey;
-    const rct::key &C = preprocessed_leaf_tuple.C;
+    const rct::key &commitment              = output_pair.commitment;
 
+    rct::key O, C;
+    if (!rct::clear_torsion(rct::pk2rct(output_pubkey), O))
+        throw std::runtime_error("output pubkey is invalid");
+    if (!rct::clear_torsion(commitment, C))
+        throw std::runtime_error("commitment is invalid");
+
+    if (O == rct::I)
+        throw std::runtime_error("O cannot equal identity");
+    if (C == rct::I)
+        throw std::runtime_error("C cannot equal identity");
+
+    // Must use the original output pubkey to derive I to prevent double spends, since torsioned outputs yield a
+    // a distinct I and key image from their respective torsion cleared output (and torsioned outputs are spendable
+    // before fcmp++)
     crypto::ec_point I;
-    crypto::derive_key_image_generator(rct::rct2pk(O), I);
+    crypto::derive_key_image_generator(output_pubkey, I);
 
     rct::key O_x, I_x, C_x;
-
     if (!rct::point_to_wei_x(O, O_x))
         throw std::runtime_error("failed to get wei x scalar from O");
     if (!rct::point_to_wei_x(rct::pt2rct(I), I_x))
@@ -773,43 +763,50 @@ typename CurveTrees<C1, C2>::TreeExtension CurveTrees<C1, C2>::get_tree_extensio
     std::vector<LeafTupleContext> &&new_leaf_tuples) const
 {
     TreeExtension tree_extension;
+    tree_extension.leaves.start_leaf_tuple_idx = old_n_leaf_tuples;
 
     if (new_leaf_tuples.empty())
         return tree_extension;
 
-    auto grow_layer_instructions = get_leaf_layer_grow_instructions(
-        old_n_leaf_tuples,
-        new_leaf_tuples.size(),
-        LEAF_TUPLE_SIZE,
-        m_leaf_layer_chunk_width);
-
-    tree_extension.leaves.start_leaf_tuple_idx = grow_layer_instructions.old_total_children / LEAF_TUPLE_SIZE;
-
     // Sort the leaves by order they appear in the chain
     const auto sort_fn = [](const LeafTupleContext &a, const LeafTupleContext &b) { return a.output_id < b.output_id; };
     std::sort(new_leaf_tuples.begin(), new_leaf_tuples.end(), sort_fn);
 
-    // Convert sorted pre-processed tuples into leaf tuples, place each element of each leaf tuple in a flat vector to
+    // Convert sorted outputs into leaf tuples, place each element of each leaf tuple in a flat vector to be hashed,
-    // be hashed, and place the pre-processed tuples in tree extension struct for insertion into the db
+    // and place the outputs in a tree extension struct for insertion into the db. We ignore invalid outputs, since
+    // they cannot be inserted to the tree.
     std::vector<typename C2::Scalar> flattened_leaves;
     flattened_leaves.reserve(new_leaf_tuples.size() * LEAF_TUPLE_SIZE);
     tree_extension.leaves.tuples.reserve(new_leaf_tuples.size());
     for (auto &l : new_leaf_tuples)
     {
         // TODO: this loop can be parallelized
-        auto leaf = leaf_tuple(l.preprocessed_leaf_tuple);
+        LeafTuple leaf;
+        try { leaf = leaf_tuple(l.output_pair); }
+        catch(...)
+        {
+          // Invalid outputs can't be added to the tree
+          continue;
+        }
 
+        // We use O.x, I.x, C.x to grow the tree
         flattened_leaves.emplace_back(std::move(leaf.O_x));
         flattened_leaves.emplace_back(std::move(leaf.I_x));
         flattened_leaves.emplace_back(std::move(leaf.C_x));
 
-        // We only need to store O and C in the db, the leaf tuple can be derived from O and C
+        // We can derive {O.x,I.x,C.x} from output pairs, so we store just the output pair in the db to save 32 bytes
-        tree_extension.leaves.tuples.emplace_back(PreprocessedLeafTuple{
+        tree_extension.leaves.tuples.emplace_back(std::move(l.output_pair));
-                .O = std::move(l.preprocessed_leaf_tuple.O),
-                .C = std::move(l.preprocessed_leaf_tuple.C)
-            });
     }
 
+    if (flattened_leaves.empty())
+        return tree_extension;
+
+    auto grow_layer_instructions = get_leaf_layer_grow_instructions(
+        old_n_leaf_tuples,
+        tree_extension.leaves.tuples.size(),
+        LEAF_TUPLE_SIZE,
+        m_leaf_layer_chunk_width);
+
     if (grow_layer_instructions.need_old_last_parent)
         CHECK_AND_ASSERT_THROW_MES(!existing_last_hashes.c2_last_hashes.empty(), "missing last c2 parent");
 

src/fcmp_pp/curve_trees.h

@@ -129,22 +129,24 @@ struct TrimLayerInstructions final
     uint64_t end_trim_idx;
 };
 
-// Output pub key and commitment, ready to be converted into a leaf tuple (from {O,C} -> {O.x, I.x, C.x})
+// Output pub key and commitment, ready to be converted to a leaf tuple
-struct PreprocessedLeafTuple final
+// - From {output_pubkey,commitment} -> {O,C} -> {O.x,I.x,C.x}
+// - Output pairs do NOT necessarily have torsion cleared. We need the output pubkey as it exists in the chain in order
+//   to derive the correct I (when deriving {O.x, I.x, C.x}). Torsion clearing O before deriving I from O would enable
+//   spending a torsioned output once before the fcmp++ fork and again with a different key image via fcmp++.
+struct OutputPair final
 {
-    // Output pubkey that has been checked valid and torsion cleared
+    crypto::public_key output_pubkey;
-    rct::key O;
+    rct::key           commitment;
-    // Commitment that has been checked valid and torsion cleared
-    rct::key C;
 };
-static_assert(sizeof(PreprocessedLeafTuple) == (32+32), "db expects 64 bytes for pre-processed leaf tuples");
+static_assert(sizeof(OutputPair) == (32+32), "db expects 64 bytes for output pairs");
 
-// Contextual wrapper for a pre-processed leaf tuple
+// Contextual wrapper for output pairs, ready to be conerted into leaf tuples
 struct LeafTupleContext final
 {
     // Global output ID useful to order the leaf tuple for insertion into the tree
-    uint64_t              output_id;
+    uint64_t   output_id;
-    PreprocessedLeafTuple preprocessed_leaf_tuple;
+    OutputPair output_pair;
 };
 
 //----------------------------------------------------------------------------------------------------------------------
@@ -186,9 +188,9 @@ public:
     struct Leaves final
     {
         // Starting leaf tuple index in the leaf layer
-        uint64_t                           start_leaf_tuple_idx{0};
+        uint64_t                start_leaf_tuple_idx{0};
         // Contiguous leaves in a tree that start at the start_idx
-        std::vector<PreprocessedLeafTuple> tuples;
+        std::vector<OutputPair> tuples;
     };
 
     // A struct useful to extend an existing tree
@@ -231,10 +233,10 @@ public:
 
 //member functions
 public:
-    // Convert cryptonote output pub key and commitment to a pre-processed leaf tuple ready for insertion to the tree
+    // Prepare output pubkey and commitment for insertion into the tree
     LeafTupleContext output_to_leaf_context(const std::uint64_t output_id,
-        const crypto::public_key &output_pubkey,
+        crypto::public_key &&output_pubkey,
-        const rct::key &C) const;
+        rct::key &&commitment) const;
 
     // Convert cryptonote tx outs to contexts ready to be converted to leaf tuples, grouped by unlock height
     void tx_outs_to_leaf_tuple_contexts(const cryptonote::transaction &tx,
@@ -243,8 +245,8 @@ public:
         const bool miner_tx,
         std::multimap<uint64_t, LeafTupleContext> &leaf_tuples_by_unlock_block_inout) const;
 
-    // Derive a leaf tuple from a pre-processed leaf tuple {O,C} -> {O.x,I.x,C.x}
+    // Convert output pairs into leaf tuples, from {output pubkey,commitment} -> {O,C} -> {O.x,I.x,C.x}
-    LeafTuple leaf_tuple(const PreprocessedLeafTuple &preprocessed_leaf_tuple) const;
+    LeafTuple leaf_tuple(const OutputPair &outpout_pair) const;
 
     // Flatten leaves [(O.x, I.x, C.x),(O.x, I.x, C.x),...] -> [O.x, I.x, C.x, O.x, I.x, C.x...]
     std::vector<typename C2::Scalar> flatten_leaves(std::vector<LeafTuple> &&leaves) const;

tests/unit_tests/curve_trees.cpp

@@ -168,9 +168,9 @@ void CurveTreesGlobalTree::extend_tree(const CurveTreesV1::TreeExtension &tree_e
         "unexpected leaf start idx");
 
     m_tree.leaves.reserve(m_tree.leaves.size() + tree_extension.leaves.tuples.size());
-    for (const auto &preprocessed_leaf_tuple : tree_extension.leaves.tuples)
+    for (const auto &output_pair : tree_extension.leaves.tuples)
     {
-        auto leaf = m_curve_trees.leaf_tuple(preprocessed_leaf_tuple);
+        auto leaf = m_curve_trees.leaf_tuple(output_pair);
 
         m_tree.leaves.emplace_back(CurveTreesV1::LeafTuple{
             .O_x = std::move(leaf.O_x),
@@ -640,8 +640,8 @@ void CurveTreesGlobalTree::log_tree_extension(const CurveTreesV1::TreeExtension
     MDEBUG("Leaf start idx: " << tree_extension.leaves.start_leaf_tuple_idx);
     for (std::size_t i = 0; i < tree_extension.leaves.tuples.size(); ++i)
     {
-        const auto &preprocessed_leaf_tuple = tree_extension.leaves.tuples[i];
+        const auto &output_pair = tree_extension.leaves.tuples[i];
-        const auto leaf = m_curve_trees.leaf_tuple(preprocessed_leaf_tuple);
+        const auto leaf = m_curve_trees.leaf_tuple(output_pair);
 
         const auto O_x = m_curve_trees.m_c2->to_string(leaf.O_x);
         const auto I_x = m_curve_trees.m_c2->to_string(leaf.I_x);
@@ -760,7 +760,8 @@ static const std::vector<fcmp_pp::curve_trees::LeafTupleContext> generate_random
         crypto::generate_keys(O, o, o, false);
         crypto::generate_keys(C, c, c, false);
 
-        auto tuple_context = curve_trees.output_to_leaf_context(output_id, O, rct::pk2rct(C));
+        rct::key C_key = rct::pk2rct(C);
+        auto tuple_context = curve_trees.output_to_leaf_context(output_id, std::move(O), std::move(C_key));
 
         tuples.emplace_back(std::move(tuple_context));
     }