diff --git a/src/blockchain_db/blockchain_db.cpp b/src/blockchain_db/blockchain_db.cpp index 5a4ab8378..c3e58e4ee 100644 --- a/src/blockchain_db/blockchain_db.cpp +++ b/src/blockchain_db/blockchain_db.cpp @@ -74,17 +74,22 @@ static void get_outs_by_unlock_block(const cryptonote::transaction &tx, auto output_pair = fcmp_pp::curve_trees::OutputPair{ .output_pubkey = std::move(output_public_key), - .commitment = std::move(commitment) + .commitment = std::move(commitment) + }; + + auto output_context = fcmp_pp::curve_trees::OutputContext{ + .output_id = output_ids[i], + .output_pair = std::move(output_pair) }; if (outs_by_unlock_block_inout.find(unlock_block) == outs_by_unlock_block_inout.end()) { - auto new_vec = std::vector{std::move(output_pair)}; + auto new_vec = std::vector{std::move(output_context)}; outs_by_unlock_block_inout[unlock_block] = std::move(new_vec); } else { - outs_by_unlock_block_inout[unlock_block].emplace_back(std::move(output_pair)); + outs_by_unlock_block_inout[unlock_block].emplace_back(std::move(output_context)); } } } diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h index 6ff97809f..8b4fac9b9 100644 --- a/src/blockchain_db/blockchain_db.h +++ b/src/blockchain_db/blockchain_db.h @@ -1783,7 +1783,7 @@ public: virtual bool for_all_alt_blocks(std::function f, bool include_blob = false) const = 0; // TODO: description and make private - virtual void grow_tree(std::vector &&new_leaves) = 0; + virtual void grow_tree(std::vector &&new_leaves) = 0; virtual void trim_tree(const uint64_t trim_n_leaf_tuples) = 0; diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index f7637dad8..5efed60dd 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -199,7 +199,7 @@ namespace * * spent_keys input hash - * - * locked_outputs block ID [{output pubkey, commitment}...] + * locked_outputs block ID [{output ID, output pubkey, commitment}...] * leaves leaf_idx {output pubkey, commitment} * layers layer_idx [{child_chunk_idx, child_chunk_hash}...] * @@ -887,9 +887,8 @@ void BlockchainLMDB::add_block(const block& blk, size_t block_weight, uint64_t l MDB_val_set(k_block_id, locked_output.first); 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 - result = mdb_cursor_put(m_cur_locked_outputs, &k_block_id, &v_output, MDB_APPENDDUP | MDB_NODUPDATA); - if (result != MDB_SUCCESS && result != MDB_KEYEXIST) + result = mdb_cursor_put(m_cur_locked_outputs, &k_block_id, &v_output, MDB_APPENDDUP); + if (result != MDB_SUCCESS) throw0(DB_ERROR(lmdb_error("Failed to add locked output: ", result).c_str())); } @@ -1344,7 +1343,7 @@ void BlockchainLMDB::remove_spent_key(const crypto::key_image& k_image) } } -void BlockchainLMDB::grow_tree(std::vector &&new_leaves) +void BlockchainLMDB::grow_tree(std::vector &&new_leaves) { LOG_PRINT_L3("BlockchainLMDB::" << __func__); if (new_leaves.empty()) @@ -1375,10 +1374,7 @@ void BlockchainLMDB::grow_tree(std::vector &&n MDB_val_copy k(i + leaves.start_leaf_tuple_idx); MDB_val_set(v, leaves.tuples[i]); - // TODO: according to the docs, MDB_APPEND isn't supposed to perform any key comparisons to maximize efficiency. - // Adding MDB_NOOVERWRITE I assume re-introduces a key comparison. Benchmark NOOVERWRITE here - // MDB_NOOVERWRITE makes sure key doesn't already exist - int result = mdb_cursor_put(m_cur_leaves, &k, &v, MDB_APPEND | MDB_NOOVERWRITE); + int result = mdb_cursor_put(m_cur_leaves, &k, &v, MDB_APPEND); if (result != MDB_SUCCESS) throw0(DB_ERROR(lmdb_error("Failed to add leaf: ", result).c_str())); } @@ -1471,10 +1467,7 @@ void BlockchainLMDB::grow_layer(const std::unique_ptr &curve, lv.child_chunk_hash = curve->to_bytes(ext.hashes[i]); MDB_val_set(v, lv); - // TODO: according to the docs, MDB_APPENDDUP isn't supposed to perform any key comparisons to maximize efficiency. - // Adding MDB_NODUPDATA I assume re-introduces a key comparison. Benchmark MDB_NODUPDATA here - // MDB_NODUPDATA makes sure key/data pair doesn't already exist - int result = mdb_cursor_put(m_cur_layers, &k, &v, MDB_APPENDDUP | MDB_NODUPDATA); + int result = mdb_cursor_put(m_cur_layers, &k, &v, MDB_APPENDDUP); if (result != MDB_SUCCESS) throw0(DB_ERROR(lmdb_error("Failed to add hash: ", result).c_str())); } @@ -2225,7 +2218,7 @@ bool BlockchainLMDB::audit_layer(const std::unique_ptr &c_child, return audit_complete; } -std::vector BlockchainLMDB::get_outs_at_unlock_block_id( +std::vector BlockchainLMDB::get_outs_at_unlock_block_id( uint64_t block_id) { LOG_PRINT_L3("BlockchainLMDB::" << __func__); @@ -2238,7 +2231,7 @@ std::vector BlockchainLMDB::get_outs_at_unlock MDB_val v_output; // Get all the locked outputs at the provided block id - std::vector outs; + std::vector outs; MDB_cursor_op op = MDB_SET; while (1) @@ -2254,8 +2247,8 @@ std::vector BlockchainLMDB::get_outs_at_unlock 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::OutputPair*)v_output.mv_data); - const auto range_end = range_begin + v_output.mv_size / sizeof(fcmp_pp::curve_trees::OutputPair); + const auto range_begin = ((const fcmp_pp::curve_trees::OutputContext*)v_output.mv_data); + const auto range_end = range_begin + v_output.mv_size / sizeof(fcmp_pp::curve_trees::OutputContext); auto it = range_begin; @@ -2474,7 +2467,6 @@ void BlockchainLMDB::open(const std::string& filename, const int db_flags) mdb_set_dupsort(txn, m_tx_indices, compare_hash32); mdb_set_dupsort(txn, m_output_amounts, 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); mdb_set_dupsort(txn, m_block_info, compare_uint64); @@ -6867,9 +6859,14 @@ void BlockchainLMDB::migrate_5_6() } // Prepare the output for insertion to the tree - const auto output_pair = fcmp_pp::curve_trees::OutputPair{ + auto output_pair = fcmp_pp::curve_trees::OutputPair{ .output_pubkey = std::move(output_data.pubkey), - .commitment = std::move(output_data.commitment) + .commitment = std::move(output_data.commitment) + }; + + auto output_context = fcmp_pp::curve_trees::OutputContext{ + .output_id = output_id, + .output_pair = std::move(output_pair) }; // Get the block in which the output will unlock @@ -6877,15 +6874,12 @@ void BlockchainLMDB::migrate_5_6() // Now add the output to the locked outputs table MDB_val_set(k_block_id, unlock_block); - MDB_val_set(v_output, output_pair); + MDB_val_set(v_output, output_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_outputs, &k_block_id, &v_output, MDB_NODUPDATA); - if (result != MDB_SUCCESS && result != MDB_KEYEXIST) + result = mdb_cursor_put(c_locked_outputs, &k_block_id, &v_output, 0); + if (result != MDB_SUCCESS) throw0(DB_ERROR(lmdb_error("Failed to add locked output: ", result).c_str())); - if (result == MDB_KEYEXIST) - MDEBUG("Duplicate output pubkey: " << output_pair.output_pubkey << " , output_id: " << output_id); } } diff --git a/src/blockchain_db/lmdb/db_lmdb.h b/src/blockchain_db/lmdb/db_lmdb.h index 9fb19abd6..2e108d554 100644 --- a/src/blockchain_db/lmdb/db_lmdb.h +++ b/src/blockchain_db/lmdb/db_lmdb.h @@ -368,7 +368,7 @@ public: static int compare_string(const MDB_val *a, const MDB_val *b); // make private - virtual void grow_tree(std::vector &&new_leaves); + virtual void grow_tree(std::vector &&new_leaves); virtual void trim_tree(const uint64_t trim_n_leaf_tuples); @@ -447,7 +447,7 @@ private: const uint64_t child_layer_idx, const uint64_t chunk_width) const; - std::vector get_outs_at_unlock_block_id(uint64_t block_id); + std::vector get_outs_at_unlock_block_id(uint64_t block_id); void del_locked_outs_at_block_id(uint64_t block_id); diff --git a/src/blockchain_db/testdb.h b/src/blockchain_db/testdb.h index 98d4260b2..91ebf8a1f 100644 --- a/src/blockchain_db/testdb.h +++ b/src/blockchain_db/testdb.h @@ -116,7 +116,7 @@ public: virtual void add_tx_amount_output_indices(const uint64_t tx_index, const std::vector& amount_output_indices) override {} virtual void add_spent_key(const crypto::key_image& k_image) override {} virtual void remove_spent_key(const crypto::key_image& k_image) override {} - virtual void grow_tree(std::vector &&new_leaves) override {}; + virtual void grow_tree(std::vector &&new_leaves) override {}; virtual void trim_tree(const uint64_t trim_n_leaf_tuples) override {}; virtual bool audit_tree(const uint64_t expected_n_leaf_tuples) const override { return false; }; virtual std::array get_tree_root() const override { return {}; }; diff --git a/src/fcmp_pp/curve_trees.cpp b/src/fcmp_pp/curve_trees.cpp index c688c4e5a..e12df47ed 100644 --- a/src/fcmp_pp/curve_trees.cpp +++ b/src/fcmp_pp/curve_trees.cpp @@ -706,7 +706,7 @@ template typename CurveTrees::TreeExtension CurveTrees::get_tree_extension( const uint64_t old_n_leaf_tuples, const LastHashes &existing_last_hashes, - std::vector &&new_leaf_tuples) const + std::vector &&new_leaf_tuples) const { TreeExtension tree_extension; tree_extension.leaves.start_leaf_tuple_idx = old_n_leaf_tuples; @@ -714,9 +714,13 @@ typename CurveTrees::TreeExtension CurveTrees::get_tree_extensio if (new_leaf_tuples.empty()) return tree_extension; - // Convert outputs into leaf tuples, place each element of each leaf tuple in a flat vector to be hashed, 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. + // Sort the leaves by order they appear in the chain + const auto sort_fn = [](const OutputContext &a, const OutputContext &b) { return a.output_id < b.output_id; }; + std::sort(new_leaf_tuples.begin(), new_leaf_tuples.end(), sort_fn); + + // Convert sorted outputs into leaf tuples, place each element of each leaf tuple in a flat vector to be hashed, + // 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 flattened_leaves; flattened_leaves.reserve(new_leaf_tuples.size() * LEAF_TUPLE_SIZE); tree_extension.leaves.tuples.reserve(new_leaf_tuples.size()); @@ -724,7 +728,7 @@ typename CurveTrees::TreeExtension CurveTrees::get_tree_extensio { // TODO: this loop can be parallelized LeafTuple leaf; - try { leaf = leaf_tuple(o); } + try { leaf = leaf_tuple(o.output_pair); } catch(...) { // Invalid outputs can't be added to the tree @@ -737,7 +741,7 @@ typename CurveTrees::TreeExtension CurveTrees::get_tree_extensio flattened_leaves.emplace_back(std::move(leaf.C_x)); // 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(std::move(o)); + tree_extension.leaves.tuples.emplace_back(std::move(o.output_pair)); } if (flattened_leaves.empty()) @@ -802,7 +806,7 @@ typename CurveTrees::TreeExtension CurveTrees::get_tree_extensio template CurveTrees::TreeExtension CurveTrees::get_tree_extension( const uint64_t old_n_leaf_tuples, const LastHashes &existing_last_hashes, - std::vector &&new_leaf_tuples) const; + std::vector &&new_leaf_tuples) const; //---------------------------------------------------------------------------------------------------------------------- template std::vector CurveTrees::get_trim_instructions( diff --git a/src/fcmp_pp/curve_trees.h b/src/fcmp_pp/curve_trees.h index 6c7d2b450..4d6f48d86 100644 --- a/src/fcmp_pp/curve_trees.h +++ b/src/fcmp_pp/curve_trees.h @@ -134,14 +134,26 @@ struct TrimLayerInstructions final // - 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++. +#pragma pack(push, 1) struct OutputPair final { crypto::public_key output_pubkey; rct::key commitment; }; -static_assert(sizeof(OutputPair) == (32+32), "db expects 64 bytes for output pairs"); -using OutputsByUnlockBlock = std::map>; +// Contextual wrapper for the output +struct OutputContext final +{ + // Output's global id in the chain, used to insert the output in the tree in the order it entered the chain + uint64_t output_id; + OutputPair output_pair; +}; +#pragma pack(pop) + +static_assert(sizeof(OutputPair) == (32+32), "db expects 64 bytes for output pairs"); +static_assert(sizeof(OutputContext) == (8+32+32), "db expects 72 bytes for output context"); + +using OutputsByUnlockBlock = std::map>; //---------------------------------------------------------------------------------------------------------------------- //---------------------------------------------------------------------------------------------------------------------- @@ -237,7 +249,7 @@ public: // leaves to add to the tree, and return a tree extension struct that can be used to extend a tree TreeExtension get_tree_extension(const uint64_t old_n_leaf_tuples, const LastHashes &existing_last_hashes, - std::vector &&new_leaf_tuples) const; + std::vector &&new_leaf_tuples) const; // Get instructions useful for trimming all existing layers in the tree std::vector get_trim_instructions( diff --git a/tests/unit_tests/curve_trees.cpp b/tests/unit_tests/curve_trees.cpp index fa2045099..a16ffebc9 100644 --- a/tests/unit_tests/curve_trees.cpp +++ b/tests/unit_tests/curve_trees.cpp @@ -744,15 +744,17 @@ void CurveTreesGlobalTree::log_tree() //---------------------------------------------------------------------------------------------------------------------- // Test helpers //---------------------------------------------------------------------------------------------------------------------- -static const std::vector generate_random_leaves(const CurveTreesV1 &curve_trees, +static const std::vector generate_random_leaves(const CurveTreesV1 &curve_trees, const std::size_t old_n_leaf_tuples, const std::size_t new_n_leaf_tuples) { - std::vector output_pairs; - output_pairs.reserve(new_n_leaf_tuples); + std::vector outs; + outs.reserve(new_n_leaf_tuples); for (std::size_t i = 0; i < new_n_leaf_tuples; ++i) { + const std::uint64_t output_id = old_n_leaf_tuples + i; + // Generate random output tuple crypto::secret_key o,c; crypto::public_key O,C; @@ -765,10 +767,15 @@ static const std::vector generate_random_leave .commitment = std::move(C_key) }; - output_pairs.emplace_back(std::move(output_pair)); + auto output_context = fcmp_pp::curve_trees::OutputContext{ + .output_id = output_id, + .output_pair = std::move(output_pair) + }; + + outs.emplace_back(std::move(output_context)); } - return output_pairs; + return outs; } //---------------------------------------------------------------------------------------------------------------------- static const Selene::Scalar generate_random_selene_scalar()