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.

2 files changed, 39 insertions, 3 deletions

diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index b3791b99c..09df30e66 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<size_t> wallet2::get_only_rct(const std::vector<size_t> &unused_dust
   return indices;
 }
 
+static uint32_t get_count_above(const std::vector<wallet2::transfer_details> &transfers, const std::vector<size_t> &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::pending_tx> wallet2::create_transactions_2(std::vector<cryp
       std::vector<size_t> 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 565cd9870..d0d2cda3c 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<std::unique_ptr<wallet2>, 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<crypto::hash> m_scanned_pool_txs[2];
   };