review notes

src/wallet3/daemon_comms.hpp

@@ -39,8 +39,11 @@ namespace wallet
     virtual std::pair<int64_t, int64_t>
     get_fee_parameters() = 0;
 
+    // Called by the wallet to fetch the necessary information to make a ring signature. The indexes are a global reference
+    // used by the daemon to uniquely identify outputs. In our distribution we find the earliest and latest indexes that are
+    // available and simply pick numbers between the indexes according to our distribution function.
     virtual std::future<std::vector<Decoy>>
-    fetch_decoys(std::vector<int64_t>& indexes) = 0;
+    fetch_decoys(const std::vector<int64_t>& indexes) = 0;
   };
 
 }  // namespace wallet

src/wallet3/decoy_selection/decoy_selection.cpp

@@ -2,7 +2,7 @@
 
 namespace wallet
 {
-  std::vector<Decoy>
+  std::vector<int64_t>
   DecoySelector::operator()(const Output& selected_output)
   {
     std::vector<Decoy> return_decoys;
@@ -12,15 +12,10 @@ namespace wallet
     std::random_device rd;
     std::default_random_engine rng(rd());
 
-    // TODO(sean): these should be built using the distribution
-    int64_t min_output_index = 100;
-    int64_t max_output_index = 100000;
     constexpr int ALPHA = 1;
     constexpr int BETA = 2;
-    std::gamma_distribution<double> distribution(ALPHA, BETA);
+    std::gamma_distribution<double> distribution{ALPHA, BETA};
 
-    // Build a distribution and apply a score to each element of available outputs depending
-    // on distance from the number chosen. Lower score is better.
     std::vector<int64_t> decoy_indexes;
     for (size_t i = 0; i < n_decoys; ++i)
     {
@@ -28,9 +23,11 @@ namespace wallet
       decoy_indexes.push_back(output_height_from_distribution);
     }
 
-    // TODO(sean): we need to also request the chosen output
-    auto decoy_promise = daemon->fetch_decoys(decoy_indexes);
-    decoy_promise.wait();
-    return decoy_promise.get();
+    // TODO(sean): uncomment this line and figure out how to remove the real index
+    // We need to request the chosen output to ensure the daemon cant guess which output is real by elimination
+    //decoy_indexes.push_back(selected_output.global_index);
+
+    return decoy_indexes;
+
   }
 }  // namespace wallet

src/wallet3/decoy_selection/decoy_selection.hpp

@@ -1,7 +1,6 @@
 #pragma once
 
 #include <vector>
-#include "../daemon_comms.hpp"
 #include "../output.hpp"
 #include "../decoy.hpp"
 
@@ -14,13 +13,12 @@ namespace wallet
   class DecoySelector
   {
    public:
-    std::vector<Decoy>
+    std::vector<int64_t>
     operator()(const Output& selected_output);
 
-    DecoySelector(std::shared_ptr<DaemonComms> dmn) : daemon(std::move(dmn)) {};
-
-   private:
-    std::shared_ptr<DaemonComms> daemon;
+    DecoySelector(int64_t min, int64_t max) : min_output_index(min), max_output_index(max) {};
 
+    int64_t min_output_index = 0;
+    int64_t max_output_index = 0;
   };
 }  // namespace wallet

src/wallet3/default_daemon_comms.cpp

@@ -262,7 +262,7 @@ namespace wallet
   }
 
   std::future<std::vector<Decoy>>
-  DefaultDaemonComms::fetch_decoys(std::vector<int64_t>& indexes)
+  DefaultDaemonComms::fetch_decoys(const std::vector<int64_t>& indexes)
   {
     auto p = std::make_shared<std::promise<std::vector<Decoy> > >();
     auto fut = p->get_future();

src/wallet3/default_daemon_comms.hpp

@@ -48,7 +48,7 @@ namespace wallet
     get_fee_parameters();
 
     std::future<std::vector<Decoy>>
-    fetch_decoys(std::vector<int64_t>& indexes);
+    fetch_decoys(const std::vector<int64_t>& indexes);
 
    private:
 

src/wallet3/transaction_constructor.cpp

@@ -7,6 +7,7 @@
 
 namespace wallet
 {
+  // create_transaction will create a vanilla spend transaction without any special features.
   PendingTransaction
   TransactionConstructor::create_transaction(
       const std::vector<TransactionRecipient>& recipients) const
@@ -55,6 +56,8 @@ namespace wallet
       transaction_total += additional_input * ptx.fee_per_byte;
 
     std::vector<Output> available_outputs{};
+
+    // Selects all outputs where the amount is greater than the estimated fee for an ADDITIONAL input.
     SQLite::Statement st{
         db->db,
         "SELECT amount, output_index, global_index, unlock_time, block_height, spending, "
@@ -70,15 +73,23 @@ namespace wallet
     ptx.update_change();
   }
 
-  // select_decoys will choose some available outputs from the database and allocate to the
-  // transaction to sign as part of the ring signature
+  // select_and_fetch_decoys will choose some available outputs from the database, fetch the
+  // details necessary for a ring signature from teh daemon and add them to the
+  // transaction ready to sign at a later point in time.
   void
-  TransactionConstructor::select_decoys(PendingTransaction& ptx) const
+  TransactionConstructor::select_and_fetch_decoys(PendingTransaction& ptx) const
   {
     ptx.decoys = {};
-    DecoySelector decoy_selection(daemon);
+    // TODO(sean): the min and max figures here for the decoys should be acquired somewhere better
+    DecoySelector decoy_selection(100, 100000);
+    std::vector<int64_t> indexes;
     for (const auto& output : ptx.chosen_outputs)
-      ptx.decoys.emplace_back(decoy_selection(output));
+    {
+      indexes = decoy_selection(output);
+      auto decoy_promise = daemon->fetch_decoys(indexes);
+      decoy_promise.wait();
+      ptx.decoys.emplace_back(decoy_promise.get());
+    }
   }
 
   void
@@ -91,6 +102,6 @@ namespace wallet
       else
         select_inputs(ptx);
     }
-    select_decoys(ptx);
+    select_and_fetch_decoys(ptx);
   }
 }  // namespace wallet

src/wallet3/transaction_constructor.hpp

@@ -34,7 +34,7 @@ namespace wallet
     select_inputs(PendingTransaction& ptx) const;
 
     void
-    select_decoys(PendingTransaction& ptx) const;
+    select_and_fetch_decoys(PendingTransaction& ptx) const;
 
     void
     select_inputs_and_finalise(PendingTransaction& ptx) const;

tests/wallet3_tests/mock_daemon_comms.hpp

@@ -21,7 +21,7 @@ class MockDaemonComms: public DefaultDaemonComms
     }
 
     std::future<std::vector<Decoy>>
-    fetch_decoys(std::vector<int64_t>& indexes) override {
+    fetch_decoys(const std::vector<int64_t>& indexes) override {
       auto p = std::promise<std::vector<Decoy>>();
       auto fut = p.get_future();