Merge pull request #5389

588e6700 simplewallet: fix output representation offset (moneromooo-monero)
35e0a968 wallet2: "output lineup" fake out selection (moneromooo-monero)

4 files changed, 190 insertions, 61 deletions

diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp
index fdff485d6..1640fa990 100644
--- a/src/simplewallet/simplewallet.cpp
+++ b/src/simplewallet/simplewallet.cpp
@@ -5579,7 +5579,7 @@ bool simple_wallet::print_ring_members(const std::vector<tools::wallet2::pending
         if (j == source.real_output)
           highlight_height = heights[j];
       }
-      std::pair<std::string, std::string> ring_str = show_outputs_line(heights, highlight_height);
+      std::pair<std::string, std::string> ring_str = show_outputs_line(heights, blockchain_height, highlight_height);
       ostr << ring_str.first << tr("\n|") << ring_str.second << tr("|\n");
     }
     // warn if rings contain keys originating from the same tx or temporally very close block heights
diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp
index a7da9395c..831166d5f 100644
--- a/src/wallet/wallet2.cpp
+++ b/src/wallet/wallet2.cpp
@@ -29,7 +29,6 @@
 // Parts of this file are originally copyright (c) 2012-2013 The Cryptonote developers
 
 #include <numeric>
-#include <random>
 #include <tuple>
 #include <boost/format.hpp>
 #include <boost/optional/optional.hpp>
@@ -129,7 +128,8 @@ using namespace cryptonote;
 
 #define FIRST_REFRESH_GRANULARITY     1024
 
-#define GAMMA_PICK_HALF_WINDOW 5
+#define GAMMA_SHAPE 19.28
+#define GAMMA_SCALE (1/1.61)
 
 static const std::string MULTISIG_SIGNATURE_MAGIC = "SigMultisigPkV1";
 static const std::string MULTISIG_EXTRA_INFO_MAGIC = "MultisigxV1";
@@ -958,6 +958,44 @@ const size_t MAX_SPLIT_ATTEMPTS = 30;
 constexpr const std::chrono::seconds wallet2::rpc_timeout;
 const char* wallet2::tr(const char* str) { return i18n_translate(str, "tools::wallet2"); }
 
+gamma_picker::gamma_picker(const std::vector<uint64_t> &rct_offsets, double shape, double scale):
+    rct_offsets(rct_offsets)
+{
+  gamma = std::gamma_distribution<double>(shape, scale);
+  THROW_WALLET_EXCEPTION_IF(rct_offsets.size() <= CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE, error::wallet_internal_error, "Bad offset calculation");
+  const size_t blocks_in_a_year = 86400 * 365 / DIFFICULTY_TARGET_V2;
+  const size_t blocks_to_consider = std::min<size_t>(rct_offsets.size(), blocks_in_a_year);
+  const size_t outputs_to_consider = rct_offsets.back() - (blocks_to_consider < rct_offsets.size() ? rct_offsets[rct_offsets.size() - blocks_to_consider - 1] : 0);
+  begin = rct_offsets.data();
+  end = rct_offsets.data() + rct_offsets.size() - CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE;
+  num_rct_outputs = *(end - 1);
+  THROW_WALLET_EXCEPTION_IF(num_rct_outputs == 0, error::wallet_internal_error, "No rct outputs");
+  average_output_time = DIFFICULTY_TARGET_V2 * blocks_to_consider / outputs_to_consider; // this assumes constant target over the whole rct range
+};
+
+gamma_picker::gamma_picker(const std::vector<uint64_t> &rct_offsets): gamma_picker(rct_offsets, GAMMA_SHAPE, GAMMA_SCALE) {}
+
+uint64_t gamma_picker::pick()
+{
+  double x = gamma(engine);
+  x = exp(x);
+  uint64_t output_index = x / average_output_time;
+  if (output_index >= num_rct_outputs)
+    return std::numeric_limits<uint64_t>::max(); // bad pick
+  output_index = num_rct_outputs - 1 - output_index;
+
+  const uint64_t *it = std::lower_bound(begin, end, output_index);
+  THROW_WALLET_EXCEPTION_IF(it == end, error::wallet_internal_error, "output_index not found");
+  uint64_t index = std::distance(begin, it);
+
+  const uint64_t first_rct = index == 0 ? 0 : rct_offsets[index - 1];
+  const uint64_t n_rct = rct_offsets[index] - first_rct;
+  if (n_rct == 0)
+    return std::numeric_limits<uint64_t>::max(); // bad pick
+  MDEBUG("Picking 1/" << n_rct << " in block " << index);
+  return first_rct + crypto::rand_idx(n_rct);
+};
+
 wallet_keys_unlocker::wallet_keys_unlocker(wallet2 &w, const boost::optional<tools::password_container> &password):
   w(w),
   locked(password != boost::none)
@@ -7562,61 +7600,9 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>
     COMMAND_RPC_GET_OUTPUTS_BIN::request req = AUTO_VAL_INIT(req);
     COMMAND_RPC_GET_OUTPUTS_BIN::response daemon_resp = AUTO_VAL_INIT(daemon_resp);
 
-    struct gamma_engine
-    {
-      typedef uint64_t result_type;
-      static constexpr result_type min() { return 0; }
-      static constexpr result_type max() { return std::numeric_limits<result_type>::max(); }
-      result_type operator()() { return crypto::rand<result_type>(); }
-    } engine;
-    static const double shape = 19.28/*16.94*/;
-    //static const double shape = m_testnet ? 17.02 : 17.28;
-    static const double scale = 1/1.61;
-    std::gamma_distribution<double> gamma(shape, scale);
-    THROW_WALLET_EXCEPTION_IF(rct_offsets.size() <= CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE, error::wallet_internal_error, "Bad offset calculation");
-    uint64_t last_usable_block = rct_offsets.size() - 1;
-    auto pick_gamma = [&]()
-    {
-      double x = gamma(engine);
-      x = exp(x);
-      uint64_t block_offset = x / DIFFICULTY_TARGET_V2; // this assumes constant target over the whole rct range
-      if (block_offset > last_usable_block - CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE)
-        return std::numeric_limits<uint64_t>::max(); // bad pick
-      block_offset = last_usable_block - block_offset;
-      THROW_WALLET_EXCEPTION_IF(block_offset > last_usable_block, error::wallet_internal_error, "Bad offset calculation");
-      THROW_WALLET_EXCEPTION_IF(block_offset > 0 && rct_offsets[block_offset] < rct_offsets[block_offset - 1],
-          error::get_output_distribution, "Decreasing offsets in rct distribution: " +
-          std::to_string(block_offset - 1) + ": " + std::to_string(rct_offsets[block_offset - 1]) + ", " +
-          std::to_string(block_offset) + ": " + std::to_string(rct_offsets[block_offset]));
-      uint64_t first_block_offset = block_offset, last_block_offset = block_offset;
-      for (size_t half_window = 0; half_window <= GAMMA_PICK_HALF_WINDOW; ++half_window)
-      {
-        // end when we have a non empty block
-        uint64_t cum0 = first_block_offset > 0 ? rct_offsets[first_block_offset] - rct_offsets[first_block_offset - 1] : rct_offsets[0];
-        if (cum0 > 1)
-          break;
-        uint64_t cum1 = last_block_offset > 0 ? rct_offsets[last_block_offset] - rct_offsets[last_block_offset - 1] : rct_offsets[0];
-        if (cum1 > 1)
-          break;
-        if (first_block_offset == 0 && last_block_offset >= last_usable_block)
-          break;
-        // expand up to bounds
-        if (first_block_offset > 0)
-          --first_block_offset;
-        else
-          return std::numeric_limits<uint64_t>::max(); // bad pick
-        if (last_block_offset < last_usable_block - CRYPTONOTE_DEFAULT_TX_SPENDABLE_AGE)
-          ++last_block_offset;
-        else
-          return std::numeric_limits<uint64_t>::max(); // bad pick
-      }
-      const uint64_t first_rct = first_block_offset == 0 ? 0 : rct_offsets[first_block_offset - 1];
-      const uint64_t n_rct = rct_offsets[last_block_offset] - first_rct;
-      if (n_rct == 0)
-        return rct_offsets[block_offset] ? rct_offsets[block_offset] - 1 : 0;
-      MDEBUG("Picking 1/" << n_rct << " in " << (last_block_offset - first_block_offset + 1) << " blocks centered around " << block_offset + rct_start_height);
-      return first_rct + crypto::rand_idx(n_rct);
-    };
+    std::unique_ptr<gamma_picker> gamma;
+    if (has_rct_distribution)
+      gamma.reset(new gamma_picker(rct_offsets));
 
     size_t num_selected_transfers = 0;
     for(size_t idx: selected_transfers)
@@ -7814,20 +7800,21 @@ void wallet2::get_outs(std::vector<std::vector<tools::wallet2::get_outs_entry>>
           const char *type = "";
           if (amount == 0 && has_rct_distribution)
           {
+            THROW_WALLET_EXCEPTION_IF(!gamma, error::wallet_internal_error, "No gamma picker");
             // gamma distribution
             if (num_found -1 < recent_outputs_count + pre_fork_outputs_count)
             {
-              do i = pick_gamma(); while (i >= segregation_limit[amount].first);
+              do i = gamma->pick(); while (i >= segregation_limit[amount].first);
               type = "pre-fork gamma";
             }
             else if (num_found -1 < recent_outputs_count + pre_fork_outputs_count + post_fork_outputs_count)
             {
-              do i = pick_gamma(); while (i < segregation_limit[amount].first || i >= num_outs);
+              do i = gamma->pick(); while (i < segregation_limit[amount].first || i >= num_outs);
               type = "post-fork gamma";
             }
             else
             {
-              do i = pick_gamma(); while (i >= num_outs);
+              do i = gamma->pick(); while (i >= num_outs);
               type = "gamma";
             }
           }
diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index 8561c42ba..d101e87f5 100644
--- a/src/wallet/wallet2.h
+++ b/src/wallet/wallet2.h
@@ -39,6 +39,7 @@
 #include <boost/serialization/deque.hpp>
 #include <boost/thread/lock_guard.hpp>
 #include <atomic>
+#include <random>
 
 #include "include_base_utils.h"
 #include "cryptonote_basic/account.h"
@@ -76,6 +77,30 @@ namespace tools
   class wallet2;
   class Notify;
 
+  class gamma_picker
+  {
+  public:
+    uint64_t pick();
+    gamma_picker(const std::vector<uint64_t> &rct_offsets);
+    gamma_picker(const std::vector<uint64_t> &rct_offsets, double shape, double scale);
+
+  private:
+    struct gamma_engine
+    {
+      typedef uint64_t result_type;
+      static constexpr result_type min() { return 0; }
+      static constexpr result_type max() { return std::numeric_limits<result_type>::max(); }
+      result_type operator()() { return crypto::rand<result_type>(); }
+    } engine;
+
+private:
+    std::gamma_distribution<double> gamma;
+    const std::vector<uint64_t> &rct_offsets;
+    const uint64_t *begin, *end;
+    uint64_t num_rct_outputs;
+    double average_output_time;
+  };
+
   class wallet_keys_unlocker
   {
   public:
diff --git a/tests/unit_tests/output_selection.cpp b/tests/unit_tests/output_selection.cpp
index a528679e4..0094fc765 100644
--- a/tests/unit_tests/output_selection.cpp
+++ b/tests/unit_tests/output_selection.cpp
@@ -101,3 +101,120 @@ TEST(select_outputs, order)
   PICK(1); // then the one that's on the same height
 }
 
+#define MKOFFSETS(N, n) \
+  offsets.resize(N); \
+  size_t n_outs = 0; \
+  for (auto &offset: offsets) \
+  { \
+    offset = n_outs += (n); \
+  }
+
+TEST(select_outputs, gamma)
+{
+  std::vector<uint64_t> offsets;
+
+  MKOFFSETS(300000, 1);
+  tools::gamma_picker picker(offsets);
+  std::vector<double> ages(100000);
+  double age_scale = 120. * (offsets.size() / (double)n_outs);
+  for (size_t i = 0; i < ages.size(); )
+  {
+    uint64_t o = picker.pick();
+    if (o >= n_outs)
+      continue;
+    ages[i] = (n_outs - 1 - o) * age_scale;
+    ASSERT_GE(ages[i], 0);
+    ASSERT_LE(ages[i], offsets.size() * 120);
+    ++i;
+  }
+  double median = epee::misc_utils::median(ages);
+  MDEBUG("median age: " << median / 86400. << " days");
+  ASSERT_GE(median, 1.3 * 86400);
+  ASSERT_LE(median, 1.4 * 86400);
+}
+
+TEST(select_outputs, density)
+{
+  static const size_t NPICKS = 1000000;
+  std::vector<uint64_t> offsets;
+
+  MKOFFSETS(300000, 1 + (rand() & 0x1f));
+  tools::gamma_picker picker(offsets);
+
+  std::vector<int> picks(/*n_outs*/offsets.size(), 0);
+  for (int i = 0; i < NPICKS; )
+  {
+    uint64_t o = picker.pick();
+    if (o >= n_outs)
+      continue;
+    auto it = std::lower_bound(offsets.begin(), offsets.end(), o);
+    auto idx = std::distance(offsets.begin(), it);
+    ASSERT_LT(idx, picks.size());
+    ++picks[idx];
+    ++i;
+  }
+
+  for (int d = 1; d < 0x20; ++d)
+  {
+    // count the number of times an output in a block of d outputs was selected
+    // count how many outputs are in a block of d outputs
+    size_t count_selected = 0, count_chain = 0;
+    for (size_t i = 0; i < offsets.size(); ++i)
+    {
+      size_t n_outputs = offsets[i] - (i == 0 ? 0 : offsets[i - 1]);
+      if (n_outputs == d)
+      {
+        count_selected += picks[i];
+        count_chain += d;
+      }
+    }
+    float selected_ratio = count_selected / (float)NPICKS;
+    float chain_ratio = count_chain / (float)n_outs;
+    MDEBUG(count_selected << "/" << NPICKS << " outputs selected in blocks of density " << d << ", " << 100.0f * selected_ratio << "%");
+    MDEBUG(count_chain << "/" << offsets.size() << " outputs in blocks of density " << d << ", " << 100.0f * chain_ratio << "%");
+    ASSERT_LT(fabsf(selected_ratio - chain_ratio), 0.02f);
+  }
+}
+
+TEST(select_outputs, same_distribution)
+{
+  static const size_t NPICKS = 1000000;
+  std::vector<uint64_t> offsets;
+
+  MKOFFSETS(300000, 1 + (rand() & 0x1f));
+  tools::gamma_picker picker(offsets);
+
+  std::vector<int> chain_picks(offsets.size(), 0);
+  std::vector<int> output_picks(n_outs, 0);
+  for (int i = 0; i < NPICKS; )
+  {
+    uint64_t o = picker.pick();
+    if (o >= n_outs)
+      continue;
+    auto it = std::lower_bound(offsets.begin(), offsets.end(), o);
+    auto idx = std::distance(offsets.begin(), it);
+    ASSERT_LT(idx, chain_picks.size());
+    ++chain_picks[idx];
+    ++output_picks[o];
+    ++i;
+  }
+
+  // scale them both to 0-100
+  std::vector<int> chain_norm(100, 0), output_norm(100, 0);
+  for (size_t i = 0; i < output_picks.size(); ++i)
+    output_norm[i * 100 / output_picks.size()] += output_picks[i];
+  for (size_t i = 0; i < chain_picks.size(); ++i)
+    chain_norm[i * 100 / chain_picks.size()] += chain_picks[i];
+
+  double max_dev = 0.0, avg_dev = 0.0;
+  for (size_t i = 0; i < 100; ++i)
+  {
+    const double diff = (double)output_norm[i] - (double)chain_norm[i];
+    double dev = fabs(2.0 * diff / (output_norm[i] + chain_norm[i]));
+    ASSERT_LT(dev, 0.1);
+    avg_dev += dev;
+  }
+  avg_dev /= 100;
+  MDEBUG("avg_dev: " << avg_dev);
+  ASSERT_LT(avg_dev, 0.015);
+}