@jeffro256 latest review comment

Introduced Compressed Tree Extension structs to cleanly separate
any curve trees-specific crypto logic from the db layer

Author: j-berman

src/blockchain_db/blockchain_db.cpp

@@ -390,8 +390,10 @@ void BlockchainDB::grow_tree(const uint64_t blk_idx, std::vector<fcmp_pp::Unifie
     return;
   }
 
-  const uint64_t new_n_leaf_tuples = tree_extension.leaves.tuples.size() + old_n_leaf_tuples;
-  const auto tree_edge = this->grow_with_tree_extension(tree_extension);
+  const auto compressed_tree_extension = m_curve_trees->compress_tree_extension(std::move(tree_extension));
+  const auto tree_edge = this->grow_with_tree_extension(compressed_tree_extension);
+
+  const uint64_t new_n_leaf_tuples = compressed_tree_extension.leaves.tuples.size() + old_n_leaf_tuples;
   this->save_tree_meta(blk_idx, new_n_leaf_tuples, tree_edge);
 }
 
@@ -452,7 +454,7 @@ void BlockchainDB::trim_tree(const uint64_t new_n_leaf_tuples, const uint64_t tr
   this->trim_layers(new_n_leaf_tuples, n_elems_per_layer, prev_tree_edge, expected_root_idx);
 }
 
-std::pair<uint64_t, fcmp_pp::PathBytes> BlockchainDB::get_last_path(const uint64_t block_idx) const
+std::pair<uint64_t, fcmp_pp::CompressedPath> BlockchainDB::get_last_path(const uint64_t block_idx) const
 {
   LOG_PRINT_L3("BlockchainDB::" << __func__);
 
@@ -475,8 +477,8 @@ std::pair<uint64_t, fcmp_pp::PathBytes> BlockchainDB::get_last_path(const uint64
   // Use tree edge at the provided block to set the last hash for each layer (so path state reflects old state)
   for (std::size_t i = 0; i < path.layer_chunks.size(); ++i)
   {
-    CHECK_AND_ASSERT_THROW_MES(path.layer_chunks[i].chunk_bytes.size(), "get_last_path: empty path");
-    path.layer_chunks[i].chunk_bytes.back() = tree_edge[i];
+    CHECK_AND_ASSERT_THROW_MES(path.layer_chunks[i].elems.size(), "get_last_path: empty path");
+    path.layer_chunks[i].elems.back() = tree_edge[i];
   }
 
   return { block_n_leaf_tuples, path };
@@ -485,7 +487,7 @@ std::pair<uint64_t, fcmp_pp::PathBytes> BlockchainDB::get_last_path(const uint64
 uint64_t BlockchainDB::get_path_by_unified_id(const std::vector<uint64_t> &unified_ids,
   const uint64_t as_of_n_blocks,
   std::vector<uint64_t> &leaf_idxs_out,
-  std::vector<fcmp_pp::PathBytes> &paths_out) const
+  std::vector<fcmp_pp::CompressedPath> &paths_out) const
 {
   LOG_PRINT_L3("BlockchainDB::" << __func__);
 
@@ -494,7 +496,7 @@ uint64_t BlockchainDB::get_path_by_unified_id(const std::vector<uint64_t> &unifi
   // Initialize result vectors with 0 values. If outptut is not in the tree,
   // result vectors kept as 0 values
   leaf_idxs_out = std::vector<uint64_t>(unified_ids.size(), 0);
-  paths_out = std::vector<fcmp_pp::PathBytes>(unified_ids.size(), fcmp_pp::PathBytes{});
+  paths_out = std::vector<fcmp_pp::CompressedPath>(unified_ids.size(), fcmp_pp::CompressedPath{});
 
   if (unified_ids.empty())
     return 0;
@@ -599,11 +601,11 @@ uint64_t BlockchainDB::get_path_by_unified_id(const std::vector<uint64_t> &unifi
       if (last_path_idxs.layers.at(i).second != path_idxs.layers.at(i).second)
         continue;
 
-      CHECK_AND_ASSERT_THROW_MES(path.layer_chunks.at(i).chunk_bytes.size(), "get_path_by_unified_id: empty layer in path");
-      CHECK_AND_ASSERT_THROW_MES(path.layer_chunks.at(i).chunk_bytes.size() == last_path.second.layer_chunks.at(i).chunk_bytes.size(),
+      CHECK_AND_ASSERT_THROW_MES(path.layer_chunks.at(i).elems.size(), "get_path_by_unified_id: empty layer in path");
+      CHECK_AND_ASSERT_THROW_MES(path.layer_chunks.at(i).elems.size() == last_path.second.layer_chunks.at(i).elems.size(),
         "get_path_by_unified_id: unexpected size of last path");
 
-      path.layer_chunks.at(i).chunk_bytes.back() = last_path.second.layer_chunks.at(i).chunk_bytes.back();
+      path.layer_chunks.at(i).elems.back() = last_path.second.layer_chunks.at(i).elems.back();
     }
 
     paths_out.at(i) = std::move(path);

src/blockchain_db/blockchain_db.h

@@ -586,9 +586,9 @@ class BlockchainDB
 
   virtual void del_tree_meta(const uint64_t block_idx) = 0;
 
-  virtual std::vector<crypto::ec_point> grow_with_tree_extension(const fcmp_pp::curve_trees::CurveTreesV1::TreeExtension &tree_extension) = 0;
+  virtual std::vector<crypto::ec_point> grow_with_tree_extension(const fcmp_pp::CompressedTreeExtension &tree_extension) = 0;
 
-  virtual fcmp_pp::PathBytes get_path(const fcmp_pp::PathIndexes &path_indexes) const = 0;
+  virtual fcmp_pp::CompressedPath get_path(const fcmp_pp::PathIndexes &path_indexes) const = 0;
 
   virtual uint64_t find_leaf_idx_by_unified_id_bounded_search(uint64_t unified_id, uint64_t leaf_idx_start, uint64_t leaf_idx_end) const = 0;
 
@@ -1869,12 +1869,12 @@ class BlockchainDB
 
   void trim_tree(const uint64_t new_n_leaf_tuples, const uint64_t trim_block_idx);
 
-  std::pair<uint64_t, fcmp_pp::PathBytes> get_last_path(const uint64_t block_idx) const;
+  std::pair<uint64_t, fcmp_pp::CompressedPath> get_last_path(const uint64_t block_idx) const;
 
   uint64_t get_path_by_unified_id(const std::vector<uint64_t> &unified_ids,
     const uint64_t as_of_n_blocks,
     std::vector<uint64_t> &leaf_idxs_out,
-    std::vector<fcmp_pp::PathBytes> &paths_out) const;
+    std::vector<fcmp_pp::CompressedPath> &paths_out) const;
 
   /**
    * @brief add outs to locked outputs tables

src/blockchain_db/lmdb/db_lmdb.cpp

@@ -372,6 +372,7 @@ typedef struct layer_val {
     uint64_t child_chunk_idx;
     crypto::ec_point child_chunk_hash;
 } layer_val;
+static_assert(sizeof(layer_val) == (8+32), "layer_val unexpected size");
 
 typedef struct mdb_tree_meta {
     uint64_t n_leaf_tuples;
@@ -1427,7 +1428,7 @@ void BlockchainLMDB::del_locked_outs_at_block_idx(uint64_t block_idx)
     throw1(DB_ERROR(lmdb_error("Error removing locked outputs: ", result).c_str()));
 }
 
-std::vector<crypto::ec_point> BlockchainLMDB::grow_with_tree_extension(const fcmp_pp::curve_trees::CurveTreesV1::TreeExtension &tree_extension)
+std::vector<crypto::ec_point> BlockchainLMDB::grow_with_tree_extension(const fcmp_pp::CompressedTreeExtension &tree_extension)
 {
   LOG_PRINT_L3("BlockchainLMDB::" << __func__);
   check_open();
@@ -1452,60 +1453,22 @@ std::vector<crypto::ec_point> BlockchainLMDB::grow_with_tree_extension(const fcm
   }
 
   // Grow the layers
-  const auto &c1_extensions = tree_extension.c1_layer_extensions;
-  const auto &c2_extensions = tree_extension.c2_layer_extensions;
-  const std::size_t n_layers = c1_extensions.size() + c2_extensions.size();
-  if (n_layers == 0)
+  const auto &layer_extensions = tree_extension.layer_extensions;
+  if (layer_extensions.empty())
     throw0(DB_ERROR("Unexpected 0 n layers"));
 
-  bool parent_is_c1 = true;
-  uint64_t c1_idx = 0;
-  uint64_t c2_idx = 0;
   std::vector<crypto::ec_point> tree_edge;
-  tree_edge.reserve(n_layers);
-  for (uint64_t i = 0; i < n_layers; ++i)
+  tree_edge.reserve(layer_extensions.size());
+  for (uint64_t layer_idx = 0; layer_idx < layer_extensions.size(); ++layer_idx)
   {
-    const uint64_t layer_idx = c1_idx + c2_idx;
     MTRACE("Growing layer " << layer_idx);
-
-    if (parent_is_c1)
-    {
-      if (layer_idx % 2 != 0)
-        throw0(DB_ERROR(("Growing odd c1 layer, expected even layer idx for c1: "
-          + std::to_string(layer_idx)).c_str()));
-
-      tree_edge.emplace_back(this->grow_layer<fcmp_pp::curve_trees::Selene>(
-          m_curve_trees->m_c1,
-          c1_extensions.at(c1_idx),
-          layer_idx
-        ));
-
-      ++c1_idx;
-    }
-    else
-    {
-      if (layer_idx % 2 == 0)
-        throw0(DB_ERROR(("Growing even c2 layer, expected odd layer idx for c2: "
-          + std::to_string(layer_idx)).c_str()));
-
-      tree_edge.emplace_back(this->grow_layer<fcmp_pp::curve_trees::Helios>(
-          m_curve_trees->m_c2,
-          c2_extensions.at(c2_idx),
-          layer_idx
-        ));
-
-      ++c2_idx;
-    }
-
-    parent_is_c1 = !parent_is_c1;
+    tree_edge.emplace_back(this->grow_layer(layer_extensions.at(layer_idx), layer_idx));
   }
 
   return tree_edge;
 }
 
-template<typename C>
-crypto::ec_point BlockchainLMDB::grow_layer(const std::unique_ptr<C> &curve,
-  const fcmp_pp::LayerExtension<C> &layer_extension,
+crypto::ec_point BlockchainLMDB::grow_layer(const fcmp_pp::CompressedLayerExtension &layer_extension,
   const uint64_t layer_idx)
 {
   LOG_PRINT_L3("BlockchainLMDB::" << __func__);
@@ -1525,7 +1488,7 @@ crypto::ec_point BlockchainLMDB::grow_layer(const std::unique_ptr<C> &curve,
   // 1. Update the existing last hash if necessary
   if (layer_extension.update_existing_last_hash)
   {
-    hashes.emplace_back(curve->to_bytes(layer_extension.hashes.front()));
+    hashes.emplace_back(layer_extension.hashes.front());
 
     // We updated the last hash, so update it
     layer_val lv;
@@ -1543,7 +1506,7 @@ crypto::ec_point BlockchainLMDB::grow_layer(const std::unique_ptr<C> &curve,
   // 2. Add all the new hashes found in the extension
   for (uint64_t i = layer_extension.update_existing_last_hash ? 1 : 0; i < layer_extension.hashes.size(); ++i)
   {
-    hashes.emplace_back(curve->to_bytes(layer_extension.hashes[i]));
+    hashes.emplace_back(layer_extension.hashes[i]);
 
     layer_val lv;
     lv.child_chunk_idx  = i + layer_extension.start_idx;
@@ -1652,7 +1615,7 @@ std::vector<crypto::ec_point> BlockchainLMDB::get_tree_edge(uint64_t block_id) c
   return res;
 }
 
-fcmp_pp::PathBytes BlockchainLMDB::get_path(const fcmp_pp::PathIndexes &path_indexes) const
+fcmp_pp::CompressedPath BlockchainLMDB::get_path(const fcmp_pp::PathIndexes &path_indexes) const
 {
   LOG_PRINT_L3("BlockchainLMDB::" << __func__);
   check_open();
@@ -1661,7 +1624,7 @@ fcmp_pp::PathBytes BlockchainLMDB::get_path(const fcmp_pp::PathIndexes &path_ind
   RCURSOR(leaves)
   RCURSOR(layers)
 
-  fcmp_pp::PathBytes path_bytes;
+  fcmp_pp::CompressedPath path_bytes;
 
   auto &leaves_out = path_bytes.leaves;
   auto &layer_chunks_out = path_bytes.layer_chunks;
@@ -1705,7 +1668,7 @@ fcmp_pp::PathBytes BlockchainLMDB::get_path(const fcmp_pp::PathIndexes &path_ind
   std::size_t layer_idx = 0;
   for (const auto &layer_idx_range : path_indexes.layers)
   {
-    fcmp_pp::ChunkBytes chunk;
+    fcmp_pp::CompressedChunk chunk;
 
     MDB_val_set(k, layer_idx);
     MDB_val_set(v, layer_idx_range.first);
@@ -1722,7 +1685,7 @@ fcmp_pp::PathBytes BlockchainLMDB::get_path(const fcmp_pp::PathIndexes &path_ind
         throw0(DB_ERROR(lmdb_error("Failed to get layer elem: ", result).c_str()));
 
       auto *lv = (layer_val *)v.mv_data;
-      chunk.chunk_bytes.emplace_back(std::move(lv->child_chunk_hash));
+      chunk.elems.emplace_back(std::move(lv->child_chunk_hash));
     }
 
     layer_chunks_out.emplace_back(std::move(chunk));

src/blockchain_db/lmdb/db_lmdb.h

@@ -436,12 +436,9 @@ class BlockchainLMDB final: public BlockchainDB
 
   void del_locked_outs_at_block_idx(uint64_t block_idx) override;
 
-  std::vector<crypto::ec_point> grow_with_tree_extension(const fcmp_pp::curve_trees::CurveTreesV1::TreeExtension &tree_extension) override;
+  std::vector<crypto::ec_point> grow_with_tree_extension(const fcmp_pp::CompressedTreeExtension &tree_extension) override;
 
-  template<typename C>
-  crypto::ec_point grow_layer(const std::unique_ptr<C> &curve,
-    const fcmp_pp::LayerExtension<C> &layer_extension,
-    const uint64_t layer_idx);
+  crypto::ec_point grow_layer(const fcmp_pp::CompressedLayerExtension &layer_extension, const uint64_t layer_idx);
 
   uint64_t trim_leaves(const uint64_t new_n_leaf_tuples, const uint64_t trim_block_idx) override;
 
@@ -464,7 +461,7 @@ class BlockchainLMDB final: public BlockchainDB
 
   std::vector<crypto::ec_point> get_tree_edge(uint64_t block_id) const override;
 
-  fcmp_pp::PathBytes get_path(const fcmp_pp::PathIndexes &path_indexes) const override;
+  fcmp_pp::CompressedPath get_path(const fcmp_pp::PathIndexes &path_indexes) const override;
 
   template<typename C_CHILD, typename C_PARENT>
   bool audit_layer(const std::unique_ptr<C_CHILD> &c_child,

src/blockchain_db/testdb.h

@@ -129,8 +129,8 @@ class BaseTestDB: public cryptonote::BlockchainDB {
   virtual std::vector<crypto::ec_point> get_tree_edge(uint64_t block_id) const override { return {}; };
   virtual void save_tree_meta(const uint64_t block_idx, const uint64_t n_leaf_tuples, const std::vector<crypto::ec_point> &tree_edge) override {};
   virtual void del_tree_meta(const uint64_t block_idx) override {};
-  virtual std::vector<crypto::ec_point> grow_with_tree_extension(const fcmp_pp::curve_trees::CurveTreesV1::TreeExtension &tree_extension) override { return std::vector<crypto::ec_point>{}; };
-  virtual fcmp_pp::PathBytes get_path(const fcmp_pp::PathIndexes &path_indexes) const override { return fcmp_pp::PathBytes{}; };
+  virtual std::vector<crypto::ec_point> grow_with_tree_extension(const fcmp_pp::CompressedTreeExtension &tree_extension) override { return std::vector<crypto::ec_point>{}; };
+  virtual fcmp_pp::CompressedPath get_path(const fcmp_pp::PathIndexes &path_indexes) const override { return fcmp_pp::CompressedPath{}; };
   virtual uint64_t find_leaf_idx_by_unified_id_bounded_search(uint64_t unified_id, uint64_t leaf_idx_start, uint64_t leaf_idx_end) const override { return 0; };
   virtual uint64_t trim_leaves(const uint64_t new_n_leaf_tuples, const uint64_t trim_block_idx) override { return 0; };
   virtual void trim_layers(const uint64_t new_n_leaf_tuples, const std::vector<uint64_t> &n_elems_per_layer, const std::vector<crypto::ec_point> &prev_tree_edge, const uint64_t expected_root_idx) override {};

src/crypto/crypto.h

@@ -62,7 +62,7 @@ namespace crypto {
 
   // x or y coordinate
   POD_CLASS ec_coord {
-    char data [32];
+    char data[32];
   };
 
   POD_CLASS public_key: ec_point {