From 275894cdefeb80095b4b3463ec732c6e728f91b9 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 9 Aug 2015 18:07:44 +0100 Subject: [PATCH 1/2] blockchain: always select random outs using triangular distribution It was only used by the older blockchain_storage. We also move the code to the calling blockchain level, to avoid replicating the code in every DB implementation. This also makes the get_random_out method obsolete, and we delete it. --- src/blockchain_db/berkeleydb/db_bdb.cpp | 13 ------------- src/blockchain_db/berkeleydb/db_bdb.h | 2 -- src/blockchain_db/blockchain_db.h | 4 ---- src/blockchain_db/lmdb/db_lmdb.cpp | 13 ------------- src/blockchain_db/lmdb/db_lmdb.h | 2 -- src/cryptonote_core/blockchain.cpp | 10 +++++++++- 6 files changed, 9 insertions(+), 35 deletions(-) diff --git a/src/blockchain_db/berkeleydb/db_bdb.cpp b/src/blockchain_db/berkeleydb/db_bdb.cpp index ad3febbd..a990d7aa 100644 --- a/src/blockchain_db/berkeleydb/db_bdb.cpp +++ b/src/blockchain_db/berkeleydb/db_bdb.cpp @@ -1264,19 +1264,6 @@ uint64_t BlockchainBDB::get_tx_block_height(const crypto::hash& h) const return (uint64_t)result - 1; } -//FIXME: make sure the random method used here is appropriate -uint64_t BlockchainBDB::get_random_output(const uint64_t& amount) const -{ - LOG_PRINT_L3("BlockchainBDB::" << __func__); - check_open(); - - uint64_t num_outputs = get_num_outputs(amount); - if (num_outputs == 0) - throw1(OUTPUT_DNE("Attempting to get a random output for an amount, but none exist")); - - return crypto::rand() % num_outputs; -} - uint64_t BlockchainBDB::get_num_outputs(const uint64_t& amount) const { LOG_PRINT_L3("BlockchainBDB::" << __func__); diff --git a/src/blockchain_db/berkeleydb/db_bdb.h b/src/blockchain_db/berkeleydb/db_bdb.h index 41f4bcb7..f92bbef6 100644 --- a/src/blockchain_db/berkeleydb/db_bdb.h +++ b/src/blockchain_db/berkeleydb/db_bdb.h @@ -295,8 +295,6 @@ public: virtual uint64_t get_tx_block_height(const crypto::hash& h) const; - virtual uint64_t get_random_output(const uint64_t& amount) const; - virtual uint64_t get_num_outputs(const uint64_t& amount) const; virtual output_data_t get_output_key(const uint64_t& amount, const uint64_t& index); diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h index ff15109b..25a34fc0 100644 --- a/src/blockchain_db/blockchain_db.h +++ b/src/blockchain_db/blockchain_db.h @@ -104,7 +104,6 @@ * height get_tx_block_height(hash) * * Outputs: - * index get_random_output(amount) * uint64_t get_num_outputs(amount) * pub_key get_output_key(amount, index) * tx_out get_output(tx_hash, index) @@ -463,9 +462,6 @@ public: // returns height of block that contains transaction with hash virtual uint64_t get_tx_block_height(const crypto::hash& h) const = 0; - // return global output index of a random output of amount - virtual uint64_t get_random_output(const uint64_t& amount) const = 0; - // returns the total number of outputs of amount virtual uint64_t get_num_outputs(const uint64_t& amount) const = 0; diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index dd829f3b..d231322f 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -1620,19 +1620,6 @@ uint64_t BlockchainLMDB::get_tx_block_height(const crypto::hash& h) const return *(const uint64_t*)result.mv_data; } -//FIXME: make sure the random method used here is appropriate -uint64_t BlockchainLMDB::get_random_output(const uint64_t& amount) const -{ - LOG_PRINT_L3("BlockchainLMDB::" << __func__); - check_open(); - - uint64_t num_outputs = get_num_outputs(amount); - if (num_outputs == 0) - throw1(OUTPUT_DNE("Attempting to get a random output for an amount, but none exist")); - - return crypto::rand() % num_outputs; -} - uint64_t BlockchainLMDB::get_num_outputs(const uint64_t& amount) const { LOG_PRINT_L3("BlockchainLMDB::" << __func__); diff --git a/src/blockchain_db/lmdb/db_lmdb.h b/src/blockchain_db/lmdb/db_lmdb.h index 0facb871..b4c19780 100644 --- a/src/blockchain_db/lmdb/db_lmdb.h +++ b/src/blockchain_db/lmdb/db_lmdb.h @@ -157,8 +157,6 @@ public: virtual uint64_t get_tx_block_height(const crypto::hash& h) const; - virtual uint64_t get_random_output(const uint64_t& amount) const; - virtual uint64_t get_num_outputs(const uint64_t& amount) const; virtual output_data_t get_output_key(const uint64_t& amount, const uint64_t& index); diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index cc2d6f82..8a81268a 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -1511,7 +1511,15 @@ bool Blockchain::get_random_outs_for_amounts(const COMMAND_RPC_GET_RANDOM_OUTPUT // get a random output index from the DB. If we've already seen it, // return to the top of the loop and try again, otherwise add it to the // list of output indices we've seen. - uint64_t i = m_db->get_random_output(amount); + + // triangular distribution over [a,b) with a=0, mode c=b=up_index_limit + uint64_t r = crypto::rand() % ((uint64_t)1 << 53); + double frac = std::sqrt((double)r / ((uint64_t)1 << 53)); + uint64_t i = (uint64_t)(frac*num_outs); + // just in case rounding up to 1 occurs after sqrt + if (i == num_outs) + --i; + if (seen_indices.count(i)) { continue; From 4f19e6847609bf90d3a41fe98166f81a69fbae2f Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sun, 9 Aug 2015 18:14:30 +0100 Subject: [PATCH 2/2] blockchain: factor get_num_outpouts(amount) calls It has to stay constant as we get the blockchain lock for the entire function. Avoids some unnecessary DB accesses. --- src/cryptonote_core/blockchain.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 8a81268a..648b77b2 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -1473,6 +1473,7 @@ bool Blockchain::get_random_outs_for_amounts(const COMMAND_RPC_GET_RANDOM_OUTPUT // from BlockchainDB where is req.outs_count (number of mixins). for (uint64_t amount : req.amounts) { + auto num_outs = m_db->get_num_outputs(amount); // create outs_for_amount struct and populate amount field COMMAND_RPC_GET_RANDOM_OUTPUTS_FOR_AMOUNTS::outs_for_amount& result_outs = *res.outs.insert(res.outs.end(), COMMAND_RPC_GET_RANDOM_OUTPUTS_FOR_AMOUNTS::outs_for_amount()); result_outs.amount = amount; @@ -1481,9 +1482,9 @@ bool Blockchain::get_random_outs_for_amounts(const COMMAND_RPC_GET_RANDOM_OUTPUT // if there aren't enough outputs to mix with (or just enough), // use all of them. Eventually this should become impossible. - if (m_db->get_num_outputs(amount) <= req.outs_count) + if (num_outs <= req.outs_count) { - for (uint64_t i = 0; i < m_db->get_num_outputs(amount); i++) + for (uint64_t i = 0; i < num_outs; i++) { // get tx_hash, tx_out_index from DB tx_out_index toi = m_db->get_output_tx_and_index(amount, i); @@ -1499,7 +1500,6 @@ bool Blockchain::get_random_outs_for_amounts(const COMMAND_RPC_GET_RANDOM_OUTPUT else { // while we still need more mixins - auto num_outs = m_db->get_num_outputs(amount); while (result_outs.outs.size() < req.outs_count) { // if we've gone through every possible output, we've gotten all we can