From 0ad87db01f19eff22ab9c3998826b7aa943975cf Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Fri, 24 Mar 2017 20:58:02 +0000 Subject: [PATCH] wallet: try to save large outputs when using an unneeded second input When a single input is enough to satisfy a transfer, the code would previously try to add a second input, to match the "canonical" makeup of a transaction with two inputs and two outputs. This would cause wallets to slowly merge outputs till all the monero ends up in a single output, which causes trouble when making two transactions one after the other, since change is locked for 10 blocks, and an increasing portion of the remaining balance would end up locked on each transaction. There are two new settings (min-output-count and min-output-value) which can control when to stop adding such unneeded second outputs. The idea is that small "dust" outputs will still get added, but larger ones will not. Enable with, eg: set min-output-count 10 set min-output-value 30 to avoid using an unneeded second output of 30 monero or more, if there would be less than 10 such outputs left. This does not invalidate any other reason why such outputs would be used (ie, when they're really needed to satisfy a transfer, or when randomly picked in the normal course of selection). This may be improved in the future. --- src/simplewallet/simplewallet.cpp | 66 ++++++++++++++++++++++++++++++- src/simplewallet/simplewallet.h | 2 + src/wallet/wallet2.cpp | 31 ++++++++++++++- src/wallet/wallet2.h | 11 +++++- 4 files changed, 106 insertions(+), 4 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index 6cbdf1a1..9c58d71d 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -615,6 +615,42 @@ bool simple_wallet::set_unit(const std::vector &args/* = std::vecto return true; } +bool simple_wallet::set_min_output_count(const std::vector &args/* = std::vector()*/) +{ + uint32_t count; + if (!string_tools::get_xtype_from_string(count, args[1])) + { + fail_msg_writer() << tr("invalid count: must be an unsigned integer"); + return true; + } + + const auto pwd_container = get_and_verify_password(); + if (pwd_container) + { + m_wallet->set_min_output_count(count); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); + } + return true; +} + +bool simple_wallet::set_min_output_value(const std::vector &args/* = std::vector()*/) +{ + uint64_t value; + if (!cryptonote::parse_amount(value, args[1])) + { + fail_msg_writer() << tr("invalid value"); + return true; + } + + const auto pwd_container = get_and_verify_password(); + if (pwd_container) + { + m_wallet->set_min_output_value(value); + m_wallet->rewrite(m_wallet_file, pwd_container->password()); + } + return true; +} + bool simple_wallet::help(const std::vector &args/* = std::vector()*/) { success_msg_writer() << get_commands_str(); @@ -654,7 +690,7 @@ simple_wallet::simple_wallet() m_cmd_binder.set_handler("viewkey", boost::bind(&simple_wallet::viewkey, this, _1), tr("Display private view key")); m_cmd_binder.set_handler("spendkey", boost::bind(&simple_wallet::spendkey, this, _1), tr("Display private spend key")); m_cmd_binder.set_handler("seed", boost::bind(&simple_wallet::seed, this, _1), tr("Display Electrum-style mnemonic seed")); - m_cmd_binder.set_handler("set", boost::bind(&simple_wallet::set_variable, this, _1), tr("Available options: seed language - set wallet seed language; always-confirm-transfers <1|0> - whether to confirm unsplit txes; print-ring-members <1|0> - whether to print detailed information about ring members during confirmation; store-tx-info <1|0> - whether to store outgoing tx info (destination address, payment ID, tx secret key) for future reference; default-mixin - set default mixin (default is 4); auto-refresh <1|0> - whether to automatically sync new blocks from the daemon; refresh-type - set wallet refresh behaviour; priority [0|1|2|3|4] - default/unimportant/normal/elevated/priority fee; confirm-missing-payment-id <1|0>; ask-password <1|0>; unit - set default monero (sub-)unit")); + m_cmd_binder.set_handler("set", boost::bind(&simple_wallet::set_variable, this, _1), tr("Available options: seed language - set wallet seed language; always-confirm-transfers <1|0> - whether to confirm unsplit txes; print-ring-members <1|0> - whether to print detailed information about ring members during confirmation; store-tx-info <1|0> - whether to store outgoing tx info (destination address, payment ID, tx secret key) for future reference; default-mixin - set default mixin (default is 4); auto-refresh <1|0> - whether to automatically sync new blocks from the daemon; refresh-type - set wallet refresh behaviour; priority [0|1|2|3|4] - default/unimportant/normal/elevated/priority fee; confirm-missing-payment-id <1|0>; ask-password <1|0>; unit - set default monero (sub-)unit; min-output-count [n] - try to keep at least that many outputs of value at least min-output-value; min-output-value [n] - try to keep at least min-output-count outputs of at least that value")); m_cmd_binder.set_handler("rescan_spent", boost::bind(&simple_wallet::rescan_spent, this, _1), tr("Rescan blockchain for spent outputs")); m_cmd_binder.set_handler("get_tx_key", boost::bind(&simple_wallet::get_tx_key, this, _1), tr("Get transaction key (r) for a given ")); m_cmd_binder.set_handler("check_tx_key", boost::bind(&simple_wallet::check_tx_key, this, _1), tr("Check amount going to
in ")); @@ -690,6 +726,8 @@ bool simple_wallet::set_variable(const std::vector &args) success_msg_writer() << "confirm-missing-payment-id = " << m_wallet->confirm_missing_payment_id(); success_msg_writer() << "ask-password = " << m_wallet->ask_password(); success_msg_writer() << "unit = " << cryptonote::get_unit(m_wallet->get_default_decimal_point()); + success_msg_writer() << "min-outputs-count = " << m_wallet->get_min_output_count(); + success_msg_writer() << "min-outputs-value = " << cryptonote::print_money(m_wallet->get_min_output_value()); return true; } else @@ -838,6 +876,32 @@ bool simple_wallet::set_variable(const std::vector &args) return true; } } + else if (args[0] == "min-outputs-count") + { + if (args.size() <= 1) + { + fail_msg_writer() << tr("set min-outputs-count: needs an argument (unsigned integer)"); + return true; + } + else + { + set_min_output_count(args); + return true; + } + } + else if (args[0] == "min-outputs-value") + { + if (args.size() <= 1) + { + fail_msg_writer() << tr("set min-outputs-value: needs an argument (unsigned integer)"); + return true; + } + else + { + set_min_output_value(args); + return true; + } + } } fail_msg_writer() << tr("set: unrecognized argument(s)"); return true; diff --git a/src/simplewallet/simplewallet.h b/src/simplewallet/simplewallet.h index c7d65633..bb668b12 100644 --- a/src/simplewallet/simplewallet.h +++ b/src/simplewallet/simplewallet.h @@ -117,6 +117,8 @@ namespace cryptonote bool set_confirm_missing_payment_id(const std::vector &args = std::vector()); bool set_ask_password(const std::vector &args = std::vector()); bool set_unit(const std::vector &args = std::vector()); + bool set_min_output_count(const std::vector &args = std::vector()); + bool set_min_output_value(const std::vector &args = std::vector()); bool help(const std::vector &args = std::vector()); bool start_mining(const std::vector &args); bool stop_mining(const std::vector &args); diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index b3791b99..09df30e6 100644 --- a/src/wallet/wallet2.cpp +++ b/src/wallet/wallet2.cpp @@ -1918,6 +1918,12 @@ bool wallet2::store_keys(const std::string& keys_file_name, const std::string& p value2.SetInt(m_ask_password ? 1 :0); json.AddMember("ask_password", value2, json.GetAllocator()); + value2.SetUint(m_min_output_count); + json.AddMember("min_output_count", value2, json.GetAllocator()); + + value2.SetUint64(m_min_output_value); + json.AddMember("min_output_value", value2, json.GetAllocator()); + value2.SetInt(cryptonote::get_default_decimal_point()); json.AddMember("default_decimal_point", value2, json.GetAllocator()); @@ -1989,6 +1995,8 @@ bool wallet2::load_keys(const std::string& keys_file_name, const std::string& pa m_refresh_type = RefreshType::RefreshDefault; m_confirm_missing_payment_id = true; m_ask_password = true; + m_min_output_count = 0; + m_min_output_value = 0; } else { @@ -2053,6 +2061,10 @@ bool wallet2::load_keys(const std::string& keys_file_name, const std::string& pa m_ask_password = field_ask_password; GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, default_decimal_point, int, Int, false, CRYPTONOTE_DISPLAY_DECIMAL_POINT); cryptonote::set_default_decimal_point(field_default_decimal_point); + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, min_output_count, uint32_t, Uint, false, 0); + m_min_output_count = field_min_output_count; + GET_FIELD_FROM_JSON_RETURN_ON_ERROR(json, min_output_value, uint64_t, Uint64, false, 0); + m_min_output_value = field_min_output_value; } const cryptonote::account_keys& keys = m_account.get_keys(); @@ -4189,6 +4201,15 @@ std::vector wallet2::get_only_rct(const std::vector &unused_dust return indices; } +static uint32_t get_count_above(const std::vector &transfers, const std::vector &indices, uint64_t threshold) +{ + uint32_t count = 0; + for (size_t idx: indices) + if (transfers[idx].amount() >= threshold) + ++count; + return count; +} + // Another implementation of transaction creation that is hopefully better // While there is anything left to pay, it goes through random outputs and tries // to fill the next destination/amount. If it fully fills it, it will use the @@ -4339,12 +4360,20 @@ std::vector wallet2::create_transactions_2(std::vector indices = get_only_rct(unused_dust_indices, unused_transfers_indices); idx = pop_best_value(indices, tx.selected_transfers, true); + // we might not want to add it if it's a large output and we don't have many left + if (m_transfers[idx].amount() >= m_min_output_value) { + if (get_count_above(m_transfers, unused_transfers_indices, m_min_output_value) < m_min_output_count) { + LOG_PRINT_L2("Second output was not strictly needed, and we're running out of outputs above " << print_money(m_min_output_value) << ", not adding"); + break; + } + } + // since we're trying to add a second output which is not strictly needed, // we only add it if it's unrelated enough to the first one float relatedness = get_output_relatedness(m_transfers[idx], m_transfers[tx.selected_transfers.front()]); if (relatedness > SECOND_OUTPUT_RELATEDNESS_THRESHOLD) { - LOG_PRINT_L2("Second outout was not strictly needed, and relatedness " << relatedness << ", not adding"); + LOG_PRINT_L2("Second output was not strictly needed, and relatedness " << relatedness << ", not adding"); break; } pop_if_present(unused_transfers_indices, idx); diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 565cd987..d0d2cda3 100644 --- a/src/wallet/wallet2.h +++ b/src/wallet/wallet2.h @@ -105,7 +105,7 @@ namespace tools }; private: - wallet2(const wallet2&) : m_run(true), m_callback(0), m_testnet(false), m_always_confirm_transfers(true), m_print_ring_members(false), m_store_tx_info(true), m_default_mixin(0), m_default_priority(0), m_refresh_type(RefreshOptimizeCoinbase), m_auto_refresh(true), m_refresh_from_block_height(0), m_confirm_missing_payment_id(true), m_ask_password(true), m_node_rpc_proxy(m_http_client, m_daemon_rpc_mutex) {} + wallet2(const wallet2&) : m_run(true), m_callback(0), m_testnet(false), m_always_confirm_transfers(true), m_print_ring_members(false), m_store_tx_info(true), m_default_mixin(0), m_default_priority(0), m_refresh_type(RefreshOptimizeCoinbase), m_auto_refresh(true), m_refresh_from_block_height(0), m_confirm_missing_payment_id(true), m_ask_password(true), m_min_output_count(0), m_min_output_value(0), m_node_rpc_proxy(m_http_client, m_daemon_rpc_mutex) {} public: static const char* tr(const char* str); @@ -126,7 +126,8 @@ namespace tools //! Uses stdin and stdout. Returns a wallet2 and password for wallet with no file if no errors. static std::pair, password_container> make_new(const boost::program_options::variables_map& vm); - wallet2(bool testnet = false, bool restricted = false) : m_run(true), m_callback(0), m_testnet(testnet), m_always_confirm_transfers(true), m_print_ring_members(false), m_store_tx_info(true), m_default_mixin(0), m_default_priority(0), m_refresh_type(RefreshOptimizeCoinbase), m_auto_refresh(true), m_refresh_from_block_height(0), m_confirm_missing_payment_id(true), m_ask_password(true), m_restricted(restricted), is_old_file_format(false), m_node_rpc_proxy(m_http_client, m_daemon_rpc_mutex) {} + wallet2(bool testnet = false, bool restricted = false) : m_run(true), m_callback(0), m_testnet(testnet), m_always_confirm_transfers(true), m_print_ring_members(false), m_store_tx_info(true), m_default_mixin(0), m_default_priority(0), m_refresh_type(RefreshOptimizeCoinbase), m_auto_refresh(true), m_refresh_from_block_height(0), m_confirm_missing_payment_id(true), m_ask_password(true), m_min_output_count(0), m_min_output_value(0), m_restricted(restricted), is_old_file_format(false), m_node_rpc_proxy(m_http_client, m_daemon_rpc_mutex) {} + struct transfer_details { uint64_t m_block_height; @@ -526,6 +527,10 @@ namespace tools void ask_password(bool always) { m_ask_password = always; } void set_default_decimal_point(unsigned int decimal_point); unsigned int get_default_decimal_point() const; + void set_min_output_count(uint32_t count) { m_min_output_count = count; } + uint32_t get_min_output_count() const { return m_min_output_count; } + void set_min_output_value(uint64_t value) { m_min_output_value = value; } + uint64_t get_min_output_value() const { return m_min_output_value; } bool get_tx_key(const crypto::hash &txid, crypto::secret_key &tx_key) const; @@ -682,6 +687,8 @@ namespace tools uint64_t m_refresh_from_block_height; bool m_confirm_missing_payment_id; bool m_ask_password; + uint32_t m_min_output_count; + uint64_t m_min_output_value; NodeRPCProxy m_node_rpc_proxy; std::unordered_set m_scanned_pool_txs[2]; };