From f26651ab8a3ed7f39a13f258880ee57f7ab85b68 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 26 Mar 2016 11:44:43 +0000 Subject: [PATCH 1/7] wallet: factor fee calculation --- src/wallet/wallet2.cpp | 33 +++++++++++---------------------- 1 file changed, 11 insertions(+), 22 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 1c7187cf0..7c54ca5e9 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -92,6 +92,13 @@ void do_prepare_file_names(const std::string& file_path, std::string& keys_file, } } +uint64_t calculate_fee(const cryptonote::blobdata &blob) +{ + uint64_t bytes = blob.size(); + uint64_t kB = (bytes + 1023) / 1024; + return kB * FEE_PER_KB; +} + } //namespace namespace tools @@ -2030,13 +2037,7 @@ std::vector wallet2::create_transactions(std::vector wallet2::create_transactions_2(std::vector available_for_fee && dsts[0].amount > 0) @@ -2686,13 +2681,7 @@ std::vector wallet2::create_dust_sweep_transactions() { transfer_dust(num_outputs_per_tx, (uint64_t)0 /* unlock_time */, 0, detail::digit_split_strategy, dust_policy, extra, tx, ptx); auto txBlob = t_serializable_object_to_blob(ptx.tx); - uint64_t txSize = txBlob.size(); - uint64_t numKB = txSize / 1024; - if (txSize % 1024) - { - numKB++; - } - needed_fee = numKB * FEE_PER_KB; + needed_fee = calculate_fee(txBlob); // reroll the tx with the actual amount minus the fee // if there's not enough for the fee, it'll throw From f9a2fd2ff55654cdbc458378d0365189ffd1c46a Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 26 Mar 2016 11:51:58 +0000 Subject: [PATCH 2/7] wallet: handle rare case where fee adjustment can bump to the next kB It resulted in a tx being sent with too low a fee, and thus rejected. --- src/wallet/wallet2.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 7c54ca5e9..aa638143c 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -2677,7 +2677,7 @@ std::vector wallet2::create_dust_sweep_transactions() // loop until fee is met without increasing tx size to next KB boundary. uint64_t needed_fee = 0; - if (1) + do { transfer_dust(num_outputs_per_tx, (uint64_t)0 /* unlock_time */, 0, detail::digit_split_strategy, dust_policy, extra, tx, ptx); auto txBlob = t_serializable_object_to_blob(ptx.tx); @@ -2687,7 +2687,8 @@ std::vector wallet2::create_dust_sweep_transactions() // if there's not enough for the fee, it'll throw transfer_dust(num_outputs_per_tx, (uint64_t)0 /* unlock_time */, needed_fee, detail::digit_split_strategy, dust_policy, extra, tx, ptx); txBlob = t_serializable_object_to_blob(ptx.tx); - } + needed_fee = calculate_fee(txBlob); + } while (ptx.fee < needed_fee); ptx_vector.push_back(ptx); From 600a3cf0c0ca0a99dbdc91d32138db3c8aa4165c Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 26 Mar 2016 14:30:23 +0000 Subject: [PATCH 3/7] New RPC and daemon command to get output histogram This is a list of existing output amounts along with the number of outputs of that amount in the blockchain. The daemon command takes: - no parameters: all outputs with at least 3 instances - one parameter: all outputs with at least that many instances - two parameters: all outputs within that many instances The default starts at 3 to avoid massive spamming of all dust outputs in the blockchain, and is the current minimum mixin requirement. An optional vector of amounts may be passed, to request histogram only for those outputs. --- src/blockchain_db/berkeleydb/db_bdb.cpp | 6 +++ src/blockchain_db/berkeleydb/db_bdb.h | 9 ++++ src/blockchain_db/blockchain_db.h | 9 ++++ src/blockchain_db/lmdb/db_lmdb.cpp | 57 +++++++++++++++++++++++++ src/blockchain_db/lmdb/db_lmdb.h | 10 +++++ src/cryptonote_core/blockchain.cpp | 5 +++ src/cryptonote_core/blockchain.h | 9 ++++ src/daemon/command_parser_executor.cpp | 18 ++++++++ src/daemon/command_parser_executor.h | 2 + src/daemon/command_server.cpp | 5 +++ src/daemon/rpc_command_executor.cpp | 36 ++++++++++++++++ src/daemon/rpc_command_executor.h | 2 + src/rpc/core_rpc_server.cpp | 32 ++++++++++++++ src/rpc/core_rpc_server.h | 2 + src/rpc/core_rpc_server_commands_defs.h | 41 ++++++++++++++++++ tests/unit_tests/hardfork.cpp | 1 + 16 files changed, 244 insertions(+) diff --git a/src/blockchain_db/berkeleydb/db_bdb.cpp b/src/blockchain_db/berkeleydb/db_bdb.cpp index 1fef9e619..a7fa556bd 100644 --- a/src/blockchain_db/berkeleydb/db_bdb.cpp +++ b/src/blockchain_db/berkeleydb/db_bdb.cpp @@ -2180,6 +2180,12 @@ void BlockchainBDB::get_output_tx_and_index(const uint64_t& amount, const std::v LOG_PRINT_L3("db3: " << db3); } +std::map::BlockchainBDB::get_output_histogram(const std::vector &amounts) const +{ + LOG_PRINT_L3("BlockchainBDB::" << __func__); + throw1(DB_ERROR("Not implemented.")); +} + void BlockchainBDB::set_hard_fork_starting_height(uint8_t version, uint64_t height) { LOG_PRINT_L3("BlockchainBDB::" << __func__); diff --git a/src/blockchain_db/berkeleydb/db_bdb.h b/src/blockchain_db/berkeleydb/db_bdb.h index d7cbd24e7..5c6bda4eb 100644 --- a/src/blockchain_db/berkeleydb/db_bdb.h +++ b/src/blockchain_db/berkeleydb/db_bdb.h @@ -341,6 +341,15 @@ public: virtual bool can_thread_bulk_indices() const { return false; } #endif + /** + * @brief return a histogram of outputs on the blockchain + * + * @param amounts optional set of amounts to lookup + * + * @return a set of amount/instances + */ + std::map get_output_histogram(const std::vector &amounts) const; + private: virtual void add_block( const block& blk , const size_t& block_size diff --git a/src/blockchain_db/blockchain_db.h b/src/blockchain_db/blockchain_db.h index 3e0ca141e..3585bd061 100644 --- a/src/blockchain_db/blockchain_db.h +++ b/src/blockchain_db/blockchain_db.h @@ -1288,6 +1288,15 @@ public: */ virtual void drop_hard_fork_info() = 0; + /** + * @brief return a histogram of outputs on the blockchain + * + * @param amounts optional set of amounts to lookup + * + * @return a set of amount/instances + */ + virtual std::map get_output_histogram(const std::vector &amounts) const = 0; + /** * @brief is BlockchainDB in read-only mode? * diff --git a/src/blockchain_db/lmdb/db_lmdb.cpp b/src/blockchain_db/lmdb/db_lmdb.cpp index e928ab803..9b99520a1 100644 --- a/src/blockchain_db/lmdb/db_lmdb.cpp +++ b/src/blockchain_db/lmdb/db_lmdb.cpp @@ -2692,6 +2692,63 @@ void BlockchainLMDB::get_output_tx_and_index(const uint64_t& amount, const std:: LOG_PRINT_L3("db3: " << db3); } +std::map BlockchainLMDB::get_output_histogram(const std::vector &amounts) const +{ + LOG_PRINT_L3("BlockchainLMDB::" << __func__); + check_open(); + + TXN_PREFIX_RDONLY(); + RCURSOR(output_amounts); + + std::map histogram; + MDB_val k; + MDB_val v; + + if (amounts.empty()) + { + MDB_cursor_op op = MDB_FIRST; + while (1) + { + int ret = mdb_cursor_get(m_cur_output_amounts, &k, &v, op); + op = MDB_NEXT_NODUP; + if (ret == MDB_NOTFOUND) + break; + if (ret) + throw0(DB_ERROR(lmdb_error("Failed to enumerate outputs: ", ret).c_str())); + mdb_size_t num_elems = 0; + mdb_cursor_count(m_cur_output_amounts, &num_elems); + uint64_t amount = *(const uint64_t*)k.mv_data; + histogram[amount] = num_elems; + } + } + else + { + for (const auto &amount: amounts) + { + MDB_val_copy k(amount); + int ret = mdb_cursor_get(m_cur_output_amounts, &k, &v, MDB_SET); + if (ret == MDB_NOTFOUND) + { + histogram[amount] = 0; + } + else if (ret == MDB_SUCCESS) + { + mdb_size_t num_elems = 0; + mdb_cursor_count(m_cur_output_amounts, &num_elems); + histogram[amount] = num_elems; + } + else + { + throw0(DB_ERROR(lmdb_error("Failed to enumerate outputs: ", ret).c_str())); + } + } + } + + TXN_POSTFIX_RDONLY(); + + return histogram; +} + void BlockchainLMDB::check_hard_fork_info() { LOG_PRINT_L3("BlockchainLMDB::" << __func__); diff --git a/src/blockchain_db/lmdb/db_lmdb.h b/src/blockchain_db/lmdb/db_lmdb.h index a3f32ffaa..6cd3e0e8f 100644 --- a/src/blockchain_db/lmdb/db_lmdb.h +++ b/src/blockchain_db/lmdb/db_lmdb.h @@ -280,6 +280,16 @@ public: virtual void pop_block(block& blk, std::vector& txs); virtual bool can_thread_bulk_indices() const { return true; } + + /** + * @brief return a histogram of outputs on the blockchain + * + * @param amounts optional set of amounts to lookup + * + * @return a set of amount/instances + */ + std::map get_output_histogram(const std::vector &amounts) const; + private: void do_resize(uint64_t size_increase=0); diff --git a/src/cryptonote_core/blockchain.cpp b/src/cryptonote_core/blockchain.cpp index 2f42e1db5..035afc61e 100644 --- a/src/cryptonote_core/blockchain.cpp +++ b/src/cryptonote_core/blockchain.cpp @@ -3313,6 +3313,11 @@ bool Blockchain::get_hard_fork_voting_info(uint8_t version, uint32_t &window, ui return m_hardfork->get_voting_info(version, window, votes, threshold, earliest_height, voting); } +std::map Blockchain:: get_output_histogram(const std::vector &amounts) const +{ + return m_db->get_output_histogram(amounts); +} + void Blockchain::load_compiled_in_block_hashes() { if (m_fast_sync && get_blocks_dat_start(m_testnet) != nullptr) diff --git a/src/cryptonote_core/blockchain.h b/src/cryptonote_core/blockchain.h index 94def1aa9..36e0fedfc 100644 --- a/src/cryptonote_core/blockchain.h +++ b/src/cryptonote_core/blockchain.h @@ -682,6 +682,15 @@ namespace cryptonote */ bool flush_txes_from_pool(const std::list &txids); + /** + * @brief return a histogram of outputs on the blockchain + * + * @param amounts optional set of amounts to lookup + * + * @return a set of amount/instances + */ + std::map get_output_histogram(const std::vector &amounts) const; + /** * @brief perform a check on all key images in the blockchain * diff --git a/src/daemon/command_parser_executor.cpp b/src/daemon/command_parser_executor.cpp index 0b42257e7..166fe04ca 100644 --- a/src/daemon/command_parser_executor.cpp +++ b/src/daemon/command_parser_executor.cpp @@ -439,5 +439,23 @@ bool t_command_parser_executor::flush_txpool(const std::vector& arg return m_executor.flush_txpool(txid); } +bool t_command_parser_executor::output_histogram(const std::vector& args) +{ + if (args.size() > 2) return false; + + uint64_t min_count = 3; + uint64_t max_count = 0; + + if (args.size() >= 1) + { + min_count = boost::lexical_cast(args[0]); + } + if (args.size() >= 2) + { + max_count = boost::lexical_cast(args[1]); + } + return m_executor.output_histogram(min_count, max_count); +} + } // namespace daemonize diff --git a/src/daemon/command_parser_executor.h b/src/daemon/command_parser_executor.h index 51c55a8c0..11df92a5e 100644 --- a/src/daemon/command_parser_executor.h +++ b/src/daemon/command_parser_executor.h @@ -114,6 +114,8 @@ public: bool unban(const std::vector& args); bool flush_txpool(const std::vector& args); + + bool output_histogram(const std::vector& args); }; } // namespace daemonize diff --git a/src/daemon/command_server.cpp b/src/daemon/command_server.cpp index 206de9d4a..aabc2f09a 100644 --- a/src/daemon/command_server.cpp +++ b/src/daemon/command_server.cpp @@ -214,6 +214,11 @@ t_command_server::t_command_server( , std::bind(&t_command_parser_executor::flush_txpool, &m_parser, p::_1) , "Flush a transaction from the tx pool by its txid, or the whole tx pool" ); + m_command_lookup.set_handler( + "output_histogram" + , std::bind(&t_command_parser_executor::output_histogram, &m_parser, p::_1) + , "Print output histogram (amount, instances)" + ); } bool t_command_server::process_command_str(const std::string& cmd) diff --git a/src/daemon/rpc_command_executor.cpp b/src/daemon/rpc_command_executor.cpp index 15ddc081f..933c93ed7 100644 --- a/src/daemon/rpc_command_executor.cpp +++ b/src/daemon/rpc_command_executor.cpp @@ -1203,5 +1203,41 @@ bool t_rpc_command_executor::flush_txpool(const std::string &txid) return true; } +bool t_rpc_command_executor::output_histogram(uint64_t min_count, uint64_t max_count) +{ + cryptonote::COMMAND_RPC_GET_OUTPUT_HISTOGRAM::request req; + cryptonote::COMMAND_RPC_GET_OUTPUT_HISTOGRAM::response res; + std::string fail_message = "Unsuccessful"; + epee::json_rpc::error error_resp; + + req.min_count = min_count; + req.max_count = max_count; + + if (m_is_rpc) + { + if (!m_rpc_client->json_rpc_request(req, res, "get_output_histogram", fail_message.c_str())) + { + return true; + } + } + else + { + if (!m_rpc_server->on_get_output_histogram(req, res, error_resp)) + { + tools::fail_msg_writer() << fail_message.c_str(); + return true; + } + } + + std::sort(res.histogram.begin(), res.histogram.end(), + [](const cryptonote::COMMAND_RPC_GET_OUTPUT_HISTOGRAM::entry &e1, const cryptonote::COMMAND_RPC_GET_OUTPUT_HISTOGRAM::entry &e2)->bool { return e1.instances < e2.instances; }); + for (const auto &e: res.histogram) + { + tools::msg_writer() << e.instances << " " << cryptonote::print_money(e.amount); + } + + return true; +} + }// namespace daemonize diff --git a/src/daemon/rpc_command_executor.h b/src/daemon/rpc_command_executor.h index bc3f2d5c5..7e73e7faf 100644 --- a/src/daemon/rpc_command_executor.h +++ b/src/daemon/rpc_command_executor.h @@ -132,6 +132,8 @@ public: bool unban(const std::string &ip); bool flush_txpool(const std::string &txid); + + bool output_histogram(uint64_t min_count, uint64_t max_count); }; } // namespace daemonize diff --git a/src/rpc/core_rpc_server.cpp b/src/rpc/core_rpc_server.cpp index 6b625e77b..5350b6fe0 100644 --- a/src/rpc/core_rpc_server.cpp +++ b/src/rpc/core_rpc_server.cpp @@ -1041,6 +1041,38 @@ namespace cryptonote return true; } //------------------------------------------------------------------------------------------------------------------------------ + bool core_rpc_server::on_get_output_histogram(const COMMAND_RPC_GET_OUTPUT_HISTOGRAM::request& req, COMMAND_RPC_GET_OUTPUT_HISTOGRAM::response& res, epee::json_rpc::error& error_resp) + { + if(!check_core_busy()) + { + error_resp.code = CORE_RPC_ERROR_CODE_CORE_BUSY; + error_resp.message = "Core is busy."; + return false; + } + + std::map histogram; + try + { + histogram = m_core.get_blockchain_storage().get_output_histogram(req.amounts); + } + catch (const std::exception &e) + { + res.status = "Failed to get output histogram"; + return true; + } + + res.histogram.clear(); + res.histogram.reserve(histogram.size()); + for (const auto &i: histogram) + { + if (i.second >= req.min_count && (i.second <= req.max_count || req.max_count == 0)) + res.histogram.push_back(COMMAND_RPC_GET_OUTPUT_HISTOGRAM::entry(i.first, i.second)); + } + + res.status = CORE_RPC_STATUS_OK; + return true; + } + //------------------------------------------------------------------------------------------------------------------------------ bool core_rpc_server::on_fast_exit(const COMMAND_RPC_FAST_EXIT::request& req, COMMAND_RPC_FAST_EXIT::response& res) { cryptonote::core::set_fast_exit(); diff --git a/src/rpc/core_rpc_server.h b/src/rpc/core_rpc_server.h index f79087030..5c3707209 100644 --- a/src/rpc/core_rpc_server.h +++ b/src/rpc/core_rpc_server.h @@ -109,6 +109,7 @@ namespace cryptonote MAP_JON_RPC_WE("setbans", on_set_bans, COMMAND_RPC_SETBANS) MAP_JON_RPC_WE("getbans", on_get_bans, COMMAND_RPC_GETBANS) MAP_JON_RPC_WE("flush_txpool", on_flush_txpool, COMMAND_RPC_FLUSH_TRANSACTION_POOL) + MAP_JON_RPC_WE("get_output_histogram", on_get_output_histogram, COMMAND_RPC_GET_OUTPUT_HISTOGRAM) END_JSON_RPC_MAP() END_URI_MAP2() @@ -149,6 +150,7 @@ namespace cryptonote bool on_set_bans(const COMMAND_RPC_SETBANS::request& req, COMMAND_RPC_SETBANS::response& res, epee::json_rpc::error& error_resp); bool on_get_bans(const COMMAND_RPC_GETBANS::request& req, COMMAND_RPC_GETBANS::response& res, epee::json_rpc::error& error_resp); bool on_flush_txpool(const COMMAND_RPC_FLUSH_TRANSACTION_POOL::request& req, COMMAND_RPC_FLUSH_TRANSACTION_POOL::response& res, epee::json_rpc::error& error_resp); + bool on_get_output_histogram(const COMMAND_RPC_GET_OUTPUT_HISTOGRAM::request& req, COMMAND_RPC_GET_OUTPUT_HISTOGRAM::response& res, epee::json_rpc::error& error_resp); //----------------------- private: diff --git a/src/rpc/core_rpc_server_commands_defs.h b/src/rpc/core_rpc_server_commands_defs.h index 72e399ec4..6d4dd1252 100644 --- a/src/rpc/core_rpc_server_commands_defs.h +++ b/src/rpc/core_rpc_server_commands_defs.h @@ -986,5 +986,46 @@ namespace cryptonote END_KV_SERIALIZE_MAP() }; }; + + struct COMMAND_RPC_GET_OUTPUT_HISTOGRAM + { + struct request + { + std::vector amounts; + uint64_t min_count; + uint64_t max_count; + + BEGIN_KV_SERIALIZE_MAP() + KV_SERIALIZE(amounts); + KV_SERIALIZE(min_count); + KV_SERIALIZE(max_count); + END_KV_SERIALIZE_MAP() + }; + + struct entry + { + uint64_t amount; + uint64_t instances; + + BEGIN_KV_SERIALIZE_MAP() + KV_SERIALIZE(amount); + KV_SERIALIZE(instances); + END_KV_SERIALIZE_MAP() + + entry(uint64_t amount, uint64_t instances): amount(amount), instances(instances) {} + entry() {} + }; + + struct response + { + std::string status; + std::vector histogram; + + BEGIN_KV_SERIALIZE_MAP() + KV_SERIALIZE(status) + KV_SERIALIZE(histogram) + END_KV_SERIALIZE_MAP() + }; + }; } diff --git a/tests/unit_tests/hardfork.cpp b/tests/unit_tests/hardfork.cpp index 50e0e5ae8..c0ee5fff8 100644 --- a/tests/unit_tests/hardfork.cpp +++ b/tests/unit_tests/hardfork.cpp @@ -108,6 +108,7 @@ public: virtual bool for_all_transactions(std::function) const { return true; } virtual bool for_all_outputs(std::function f) const { return true; } virtual bool is_read_only() const { return false; } + virtual std::map get_output_histogram() const { return std::map(); } virtual void add_block( const block& blk , const size_t& block_size From 12146daeed66fc30319c481a378c8d317b49b4db Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 26 Mar 2016 21:15:47 +0000 Subject: [PATCH 4/7] wallet: change sweep_dust to sweep_unmixable With the change in mixin rules for v2, the "annoying" outputs are slightly changed. There is high correlation between dust and unmixable, but no equivalence. --- src/simplewallet/simplewallet.cpp | 35 ++++++---- src/simplewallet/simplewallet.h | 2 +- src/wallet/wallet2.cpp | 104 ++++++++++++++++++++++++++---- src/wallet/wallet2.h | 7 +- src/wallet/wallet_errors.h | 9 +++ src/wallet/wallet_rpc_server.cpp | 2 +- 6 files changed, 129 insertions(+), 30 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 1856118f7..dd166ede5 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -541,7 +541,7 @@ simple_wallet::simple_wallet() m_cmd_binder.set_handler("bc_height", boost::bind(&simple_wallet::show_blockchain_height, this, _1), tr("Show blockchain height")); m_cmd_binder.set_handler("transfer", boost::bind(&simple_wallet::transfer, this, _1), tr("transfer [] [ ... ] [payment_id] - Transfer ,... to ,... , respectively. is the number of extra inputs to include for untraceability (from 0 to maximum available)")); m_cmd_binder.set_handler("transfer_new", boost::bind(&simple_wallet::transfer_new, this, _1), tr("Same as transfer, but using a new transaction building algorithm")); - m_cmd_binder.set_handler("sweep_dust", boost::bind(&simple_wallet::sweep_dust, this, _1), tr("Send all dust outputs to yourself with mixin 0")); + m_cmd_binder.set_handler("sweep_unmixable", boost::bind(&simple_wallet::sweep_unmixable, this, _1), tr("Send all unmixable outputs to yourself with mixin 0")); m_cmd_binder.set_handler("set_log", boost::bind(&simple_wallet::set_log, this, _1), tr("set_log - Change current log detail level, <0-4>")); m_cmd_binder.set_handler("address", boost::bind(&simple_wallet::print_address, this, _1), tr("Show current wallet public address")); m_cmd_binder.set_handler("integrated_address", boost::bind(&simple_wallet::print_integrated_address, this, _1), tr("integrated_address [PID] - Encode a payment ID into an integrated address for the current wallet public address (no argument uses a random payment ID), or decode an integrated address to standard address and payment ID")); @@ -1722,8 +1722,7 @@ bool simple_wallet::refresh(const std::vector& args) bool simple_wallet::show_balance(const std::vector& args/* = std::vector()*/) { success_msg_writer() << tr("Balance: ") << print_money(m_wallet->balance()) << ", " - << tr("unlocked balance: ") << print_money(m_wallet->unlocked_balance()) << ", " - << tr("including unlocked dust: ") << print_money(m_wallet->unlocked_dust_balance(tools::tx_dust_policy(::config::DEFAULT_DUST_THRESHOLD))); + << tr("unlocked balance: ") << print_money(m_wallet->unlocked_balance()); return true; } //---------------------------------------------------------------------------------------------------- @@ -2208,7 +2207,7 @@ bool simple_wallet::transfer_new(const std::vector &args_) } //---------------------------------------------------------------------------------------------------- -bool simple_wallet::sweep_dust(const std::vector &args_) +bool simple_wallet::sweep_unmixable(const std::vector &args_) { if (!try_connect_to_daemon()) return true; @@ -2221,28 +2220,37 @@ bool simple_wallet::sweep_dust(const std::vector &args_) try { - uint64_t total_dust = m_wallet->unlocked_dust_balance(tools::tx_dust_policy(::config::DEFAULT_DUST_THRESHOLD)); - // figure out what tx will be necessary - auto ptx_vector = m_wallet->create_dust_sweep_transactions(); + auto ptx_vector = m_wallet->create_unmixable_sweep_transactions(); + + if (ptx_vector.empty()) + { + fail_msg_writer() << tr("No unmixable outputs found"); + return true; + } // give user total and fee, and prompt to confirm - uint64_t total_fee = 0; + uint64_t total_fee = 0, total_unmixable = 0; for (size_t n = 0; n < ptx_vector.size(); ++n) { total_fee += ptx_vector[n].fee; + for (const auto &vin: ptx_vector[n].tx.vin) + { + if (vin.type() == typeid(txin_to_key)) + total_unmixable += boost::get(vin).amount; + } } - std::string prompt_str = tr("Sweeping ") + print_money(total_dust); + std::string prompt_str = tr("Sweeping ") + print_money(total_unmixable); if (ptx_vector.size() > 1) { prompt_str = (boost::format(tr("Sweeping %s in %llu transactions for a total fee of %s. Is this okay? (Y/Yes/N/No)")) % - print_money(total_dust) % + print_money(total_unmixable) % ((unsigned long long)ptx_vector.size()) % print_money(total_fee)).str(); } else { prompt_str = (boost::format(tr("Sweeping %s for a total fee of %s. Is this okay? (Y/Yes/N/No)")) % - print_money(total_dust) % + print_money(total_unmixable) % print_money(total_fee)).str(); } std::string accepted = command_line::input_line(prompt_str); @@ -2285,11 +2293,12 @@ bool simple_wallet::sweep_dust(const std::vector &args_) } catch (const tools::error::not_enough_money& e) { - fail_msg_writer() << boost::format(tr("not enough money to transfer, available only %s, transaction amount %s = %s + %s (fee)")) % + fail_msg_writer() << boost::format(tr("not enough money to transfer, available only %s, transaction amount %s = %s + %s (fee).\n%s")) % print_money(e.available()) % print_money(e.tx_amount() + e.fee()) % print_money(e.tx_amount()) % - print_money(e.fee()); + print_money(e.fee()) % + tr("This is usually due to dust which is so small it cannot pay for itself in fees"); } catch (const tools::error::not_enough_outs_to_mix& e) { diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h index 7dadb8536..21bbfa566 100644 --- a/src/simplewallet/simplewallet.h +++ b/src/simplewallet/simplewallet.h @@ -121,7 +121,7 @@ namespace cryptonote bool transfer_main(bool new_algorithm, const std::vector &args); bool transfer(const std::vector &args); bool transfer_new(const std::vector &args); - bool sweep_dust(const std::vector &args); + bool sweep_unmixable(const std::vector &args); std::vector> split_amounts( std::vector dsts, size_t num_splits ); diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index aa638143c..7d308e615 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -2508,7 +2508,7 @@ uint64_t wallet2::unlocked_dust_balance(const tx_dust_policy &dust_policy) const } template -void wallet2::transfer_dust(size_t num_outputs, uint64_t unlock_time, uint64_t needed_fee, T destination_split_strategy, const tx_dust_policy& dust_policy, const std::vector &extra, cryptonote::transaction& tx, pending_tx &ptx) +void wallet2::transfer_from(const std::vector &outs, size_t num_outputs, uint64_t unlock_time, uint64_t needed_fee, T destination_split_strategy, const tx_dust_policy& dust_policy, const std::vector &extra, cryptonote::transaction& tx, pending_tx &ptx) { using namespace cryptonote; @@ -2518,6 +2518,19 @@ void wallet2::transfer_dust(size_t num_outputs, uint64_t unlock_time, uint64_t n // throw if there are none uint64_t money = 0; std::list selected_transfers; +#if 1 + for (size_t n = 0; n < outs.size(); ++n) + { + const transfer_details& td = m_transfers[outs[n]]; + if (!td.m_spent) + { + selected_transfers.push_back (m_transfers.begin() + outs[n]); + money += td.amount(); + if (selected_transfers.size() >= num_outputs) + break; + } + } +#else for (transfer_container::iterator i = m_transfers.begin(); i != m_transfers.end(); ++i) { const transfer_details& td = *i; @@ -2529,6 +2542,7 @@ void wallet2::transfer_dust(size_t num_outputs, uint64_t unlock_time, uint64_t n break; } } +#endif // we don't allow no output to self, easier, but one may want to burn the dust if = fee THROW_WALLET_EXCEPTION_IF(money <= needed_fee, error::not_enough_money, money, needed_fee, needed_fee); @@ -2622,8 +2636,8 @@ bool wallet2::use_fork_rules(uint8_t version) r = net_utils::invoke_http_json_remote_command2(m_daemon_address + "/json_rpc", req_t, resp_t, m_http_client); m_daemon_rpc_mutex.unlock(); CHECK_AND_ASSERT_MES(r, false, "Failed to connect to daemon"); - CHECK_AND_ASSERT_MES(res.status != CORE_RPC_STATUS_BUSY, false, "Failed to connect to daemon"); - CHECK_AND_ASSERT_MES(res.status == CORE_RPC_STATUS_OK, false, "Failed to get hard fork status"); + CHECK_AND_ASSERT_MES(resp_t.result.status != CORE_RPC_STATUS_BUSY, false, "Failed to connect to daemon"); + CHECK_AND_ASSERT_MES(resp_t.result.status == CORE_RPC_STATUS_OK, false, "Failed to get hard fork status"); bool close_enough = res.height >= resp_t.result.earliest_height - 10; // start using the rules a bit beforehand if (close_enough) @@ -2641,20 +2655,84 @@ uint64_t wallet2::get_upper_tranaction_size_limit() return ((full_reward_zone * 125) / 100) - CRYPTONOTE_COINBASE_BLOB_RESERVED_SIZE; } //---------------------------------------------------------------------------------------------------- -std::vector wallet2::create_dust_sweep_transactions() +std::vector wallet2::select_available_outputs(std::function f) +{ + std::vector outputs; + size_t n = 0; + for (transfer_container::const_iterator i = m_transfers.begin(); i != m_transfers.end(); ++i, ++n) + { + if (i->m_spent) + continue; + if (!is_transfer_unlocked(*i)) + continue; + if (f(*i)) + outputs.push_back(n); + } + return outputs; +} +//---------------------------------------------------------------------------------------------------- +std::vector wallet2::get_unspent_amounts_vector() +{ + std::set set; + for (const auto &td: m_transfers) + { + if (!td.m_spent) + set.insert(td.amount()); + } + std::vector vector; + vector.reserve(set.size()); + for (const auto &i: set) + { + vector.push_back(i); + } + return vector; +} +//---------------------------------------------------------------------------------------------------- +std::vector wallet2::select_available_unmixable_outputs() +{ + // request all outputs with at least 3 instances, so we can use mixin 2 with + epee::json_rpc::request req_t = AUTO_VAL_INIT(req_t); + epee::json_rpc::response resp_t = AUTO_VAL_INIT(resp_t); + m_daemon_rpc_mutex.lock(); + req_t.jsonrpc = "2.0"; + req_t.id = epee::serialization::storage_entry(0); + req_t.method = "get_output_histogram"; + req_t.params.amounts = get_unspent_amounts_vector(); + req_t.params.min_count = 3; + req_t.params.max_count = 0; + bool r = net_utils::invoke_http_json_remote_command2(m_daemon_address + "/json_rpc", req_t, resp_t, m_http_client); + m_daemon_rpc_mutex.unlock(); + THROW_WALLET_EXCEPTION_IF(!r, error::no_connection_to_daemon, "select_available_unmixable_outputs"); + THROW_WALLET_EXCEPTION_IF(resp_t.result.status == CORE_RPC_STATUS_BUSY, error::daemon_busy, "get_output_histogram"); + THROW_WALLET_EXCEPTION_IF(resp_t.result.status != CORE_RPC_STATUS_OK, error::get_histogram_error, resp_t.result.status); + + std::set mixable; + for (const auto &i: resp_t.result.histogram) + { + mixable.insert(i.amount); + } + + return select_available_outputs([mixable](const transfer_details &td) { + const uint64_t amount = td.amount(); + if (mixable.find(amount) == mixable.end()) + return true; + return false; + }); +} +//---------------------------------------------------------------------------------------------------- +std::vector wallet2::create_unmixable_sweep_transactions() { // From hard fork 1, we don't consider small amounts to be dust anymore const bool hf1_rules = use_fork_rules(2); // first hard fork has version 2 tx_dust_policy dust_policy(hf1_rules ? 0 : ::config::DEFAULT_DUST_THRESHOLD); - size_t num_dust_outputs = 0; - for (transfer_container::const_iterator i = m_transfers.begin(); i != m_transfers.end(); ++i) + // may throw + std::vector unmixable_outputs = select_available_unmixable_outputs(); + size_t num_dust_outputs = unmixable_outputs.size(); + + if (num_dust_outputs == 0) { - const transfer_details& td = *i; - if (!td.m_spent && (td.amount() < dust_policy.dust_threshold || !is_valid_decomposed_amount(td.amount())) && is_transfer_unlocked(td)) - { - num_dust_outputs++; - } + return std::vector(); } // failsafe split attempt counter @@ -2679,13 +2757,13 @@ std::vector wallet2::create_dust_sweep_transactions() uint64_t needed_fee = 0; do { - transfer_dust(num_outputs_per_tx, (uint64_t)0 /* unlock_time */, 0, detail::digit_split_strategy, dust_policy, extra, tx, ptx); + transfer_from(unmixable_outputs, num_outputs_per_tx, (uint64_t)0 /* unlock_time */, 0, detail::digit_split_strategy, dust_policy, extra, tx, ptx); auto txBlob = t_serializable_object_to_blob(ptx.tx); needed_fee = calculate_fee(txBlob); // reroll the tx with the actual amount minus the fee // if there's not enough for the fee, it'll throw - transfer_dust(num_outputs_per_tx, (uint64_t)0 /* unlock_time */, needed_fee, detail::digit_split_strategy, dust_policy, extra, tx, ptx); + transfer_from(unmixable_outputs, num_outputs_per_tx, (uint64_t)0 /* unlock_time */, needed_fee, detail::digit_split_strategy, dust_policy, extra, tx, ptx); txBlob = t_serializable_object_to_blob(ptx.tx); needed_fee = calculate_fee(txBlob); } while (ptx.fee < needed_fee); diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index e7b002925..2b6cdab92 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -280,7 +280,7 @@ namespace tools void transfer(const std::vector& dsts, size_t fake_outputs_count, uint64_t unlock_time, uint64_t fee, const std::vector& extra); void transfer(const std::vector& dsts, size_t fake_outputs_count, uint64_t unlock_time, uint64_t fee, const std::vector& extra, cryptonote::transaction& tx, pending_tx& ptx); template - void transfer_dust(size_t num_outputs, uint64_t unlock_time, uint64_t needed_fee, T destination_split_strategy, const tx_dust_policy& dust_policy, const std::vector& extra, cryptonote::transaction& tx, pending_tx &ptx); + void transfer_from(const std::vector &outs, size_t num_outputs, uint64_t unlock_time, uint64_t needed_fee, T destination_split_strategy, const tx_dust_policy& dust_policy, const std::vector& extra, cryptonote::transaction& tx, pending_tx &ptx); template void transfer_selected(const std::vector& dsts, const std::list selected_transfers, size_t fake_outputs_count, uint64_t unlock_time, uint64_t fee, const std::vector& extra, T destination_split_strategy, const tx_dust_policy& dust_policy, cryptonote::transaction& tx, pending_tx &ptx); @@ -289,7 +289,7 @@ namespace tools void commit_tx(std::vector& ptx_vector); std::vector create_transactions(std::vector dsts, const size_t fake_outs_count, const uint64_t unlock_time, const uint64_t fee, const std::vector extra); std::vector create_transactions_2(std::vector dsts, const size_t fake_outs_count, const uint64_t unlock_time, const uint64_t fee_UNUSED, const std::vector extra); - std::vector create_dust_sweep_transactions(); + std::vector create_unmixable_sweep_transactions(); bool check_connection(); void get_transfers(wallet2::transfer_container& incoming_transfers) const; void get_payments(const crypto::hash& payment_id, std::list& payments, uint64_t min_height = 0) const; @@ -402,6 +402,9 @@ namespace tools void parse_block_round(const cryptonote::blobdata &blob, cryptonote::block &bl, crypto::hash &bl_id, bool &error) const; uint64_t get_upper_tranaction_size_limit(); void check_pending_txes(); + std::vector get_unspent_amounts_vector(); + std::vector select_available_outputs(std::function f); + std::vector select_available_unmixable_outputs(); cryptonote::account_base m_account; std::string m_daemon_address; diff --git a/src/wallet/wallet_errors.h b/src/wallet/wallet_errors.h index 6074e0858..652b19499 100644 --- a/src/wallet/wallet_errors.h +++ b/src/wallet/wallet_errors.h @@ -76,6 +76,7 @@ namespace tools // daemon_busy // no_connection_to_daemon // is_key_image_spent_error + // get_histogram_error // wallet_files_doesnt_correspond // // * - class with protected ctor @@ -600,6 +601,14 @@ namespace tools } }; //---------------------------------------------------------------------------------------------------- + struct get_histogram_error : public wallet_rpc_error + { + explicit get_histogram_error(std::string&& loc, const std::string& request) + : wallet_rpc_error(std::move(loc), "failed to get output histogram", request) + { + } + }; + //---------------------------------------------------------------------------------------------------- struct wallet_files_doesnt_correspond : public wallet_logic_error { explicit wallet_files_doesnt_correspond(std::string&& loc, const std::string& keys_file, const std::string& wallet_file) diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 418de327c..83e1f7535 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -347,7 +347,7 @@ namespace tools try { - std::vector ptx_vector = m_wallet.create_dust_sweep_transactions(); + std::vector ptx_vector = m_wallet.create_unmixable_sweep_transactions(); m_wallet.commit_tx(ptx_vector); From 0be6e08dd0faf5f7e3492652f00b8904e7e8216d Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 26 Mar 2016 23:22:57 +0000 Subject: [PATCH 5/7] wallet: do not leak owned amounts to the daemon unless --trusted-daemon This will be slower, though more private. New trusted_daemon parameter to the matching RPC call, false by default. --- src/simplewallet/simplewallet.cpp | 2 +- src/wallet/wallet2.cpp | 9 +++++---- src/wallet/wallet2.h | 4 ++-- src/wallet/wallet_rpc_server.cpp | 2 +- src/wallet/wallet_rpc_server_commands_defs.h | 2 ++ 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index dd166ede5..04170df62 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -2221,7 +2221,7 @@ bool simple_wallet::sweep_unmixable(const std::vector &args_) try { // figure out what tx will be necessary - auto ptx_vector = m_wallet->create_unmixable_sweep_transactions(); + auto ptx_vector = m_wallet->create_unmixable_sweep_transactions(m_trusted_daemon); if (ptx_vector.empty()) { diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 7d308e615..3ec2265fa 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -2688,7 +2688,7 @@ std::vector wallet2::get_unspent_amounts_vector() return vector; } //---------------------------------------------------------------------------------------------------- -std::vector wallet2::select_available_unmixable_outputs() +std::vector wallet2::select_available_unmixable_outputs(bool trusted_daemon) { // request all outputs with at least 3 instances, so we can use mixin 2 with epee::json_rpc::request req_t = AUTO_VAL_INIT(req_t); @@ -2697,7 +2697,8 @@ std::vector wallet2::select_available_unmixable_outputs() req_t.jsonrpc = "2.0"; req_t.id = epee::serialization::storage_entry(0); req_t.method = "get_output_histogram"; - req_t.params.amounts = get_unspent_amounts_vector(); + if (trusted_daemon) + req_t.params.amounts = get_unspent_amounts_vector(); req_t.params.min_count = 3; req_t.params.max_count = 0; bool r = net_utils::invoke_http_json_remote_command2(m_daemon_address + "/json_rpc", req_t, resp_t, m_http_client); @@ -2720,14 +2721,14 @@ std::vector wallet2::select_available_unmixable_outputs() }); } //---------------------------------------------------------------------------------------------------- -std::vector wallet2::create_unmixable_sweep_transactions() +std::vector wallet2::create_unmixable_sweep_transactions(bool trusted_daemon) { // From hard fork 1, we don't consider small amounts to be dust anymore const bool hf1_rules = use_fork_rules(2); // first hard fork has version 2 tx_dust_policy dust_policy(hf1_rules ? 0 : ::config::DEFAULT_DUST_THRESHOLD); // may throw - std::vector unmixable_outputs = select_available_unmixable_outputs(); + std::vector unmixable_outputs = select_available_unmixable_outputs(trusted_daemon); size_t num_dust_outputs = unmixable_outputs.size(); if (num_dust_outputs == 0) diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 2b6cdab92..566be59c6 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -289,7 +289,7 @@ namespace tools void commit_tx(std::vector& ptx_vector); std::vector create_transactions(std::vector dsts, const size_t fake_outs_count, const uint64_t unlock_time, const uint64_t fee, const std::vector extra); std::vector create_transactions_2(std::vector dsts, const size_t fake_outs_count, const uint64_t unlock_time, const uint64_t fee_UNUSED, const std::vector extra); - std::vector create_unmixable_sweep_transactions(); + std::vector create_unmixable_sweep_transactions(bool trusted_daemon); bool check_connection(); void get_transfers(wallet2::transfer_container& incoming_transfers) const; void get_payments(const crypto::hash& payment_id, std::list& payments, uint64_t min_height = 0) const; @@ -404,7 +404,7 @@ namespace tools void check_pending_txes(); std::vector get_unspent_amounts_vector(); std::vector select_available_outputs(std::function f); - std::vector select_available_unmixable_outputs(); + std::vector select_available_unmixable_outputs(bool trusted_daemon); cryptonote::account_base m_account; std::string m_daemon_address; diff --git a/src/wallet/wallet_rpc_server.cpp b/src/wallet/wallet_rpc_server.cpp index 83e1f7535..d7d99c2ae 100644 --- a/src/wallet/wallet_rpc_server.cpp +++ b/src/wallet/wallet_rpc_server.cpp @@ -347,7 +347,7 @@ namespace tools try { - std::vector ptx_vector = m_wallet.create_unmixable_sweep_transactions(); + std::vector ptx_vector = m_wallet.create_unmixable_sweep_transactions(req.trusted_daemon); m_wallet.commit_tx(ptx_vector); diff --git a/src/wallet/wallet_rpc_server_commands_defs.h b/src/wallet/wallet_rpc_server_commands_defs.h index 40d6fd8f8..2c4e26406 100644 --- a/src/wallet/wallet_rpc_server_commands_defs.h +++ b/src/wallet/wallet_rpc_server_commands_defs.h @@ -178,9 +178,11 @@ namespace wallet_rpc struct request { bool get_tx_keys; + bool trusted_daemon; BEGIN_KV_SERIALIZE_MAP() KV_SERIALIZE(get_tx_keys) + KV_SERIALIZE(trusted_daemon) END_KV_SERIALIZE_MAP() }; From 25672d3f10e71f4db36d9497e2440f2ac26a6e01 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 26 Mar 2016 23:32:45 +0000 Subject: [PATCH 6/7] wallet: pass std::function by const ref, not value Because we can. --- src/wallet/wallet2.cpp | 2 +- src/wallet/wallet2.h | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 3ec2265fa..6fd77eead 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -2655,7 +2655,7 @@ uint64_t wallet2::get_upper_tranaction_size_limit() return ((full_reward_zone * 125) / 100) - CRYPTONOTE_COINBASE_BLOB_RESERVED_SIZE; } //---------------------------------------------------------------------------------------------------- -std::vector wallet2::select_available_outputs(std::function f) +std::vector wallet2::select_available_outputs(const std::function &f) { std::vector outputs; size_t n = 0; diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 566be59c6..fc700a3de 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -403,7 +403,7 @@ namespace tools uint64_t get_upper_tranaction_size_limit(); void check_pending_txes(); std::vector get_unspent_amounts_vector(); - std::vector select_available_outputs(std::function f); + std::vector select_available_outputs(const std::function &f); std::vector select_available_unmixable_outputs(bool trusted_daemon); cryptonote::account_base m_account; From d5d46e6d6da569ec07169a12f59cbc6d120c35f2 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Sat, 26 Mar 2016 23:44:04 +0000 Subject: [PATCH 7/7] tests: obligatory hardfork unit build fix after interface change --- tests/unit_tests/hardfork.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit_tests/hardfork.cpp b/tests/unit_tests/hardfork.cpp index c0ee5fff8..cc3eba8ea 100644 --- a/tests/unit_tests/hardfork.cpp +++ b/tests/unit_tests/hardfork.cpp @@ -108,7 +108,7 @@ public: virtual bool for_all_transactions(std::function) const { return true; } virtual bool for_all_outputs(std::function f) const { return true; } virtual bool is_read_only() const { return false; } - virtual std::map get_output_histogram() const { return std::map(); } + virtual std::map get_output_histogram(const std::vector &amounts) const { return std::map(); } virtual void add_block( const block& blk , const size_t& block_size