Merge pull request #1261

069d688 simplewallet: log transactions to submit in submit_transfer (moneromooo-monero)
31abac4 wallet: fix pre-rct cold wallet signing not splitting change (moneromooo-monero)

src/simplewallet/simplewallet.cpp

@@ -3128,9 +3128,9 @@ bool simple_wallet::accept_loaded_tx(const tools::wallet2::unsigned_tx_set &txs)
       if (mixin < min_mixin)
         min_mixin = mixin;
     }
-    for (size_t d = 0; d < cd.destinations.size(); ++d)
+    for (size_t d = 0; d < cd.splitted_dsts.size(); ++d)
     {
-      const tx_destination_entry &entry = cd.destinations[d];
+      const tx_destination_entry &entry = cd.splitted_dsts[d];
       std::string address = get_account_address_as_str(m_wallet->testnet(), entry.addr);
       std::unordered_map<std::string,uint64_t>::iterator i = dests.find(address);
       if (i == dests.end())
@@ -3141,9 +3141,19 @@ bool simple_wallet::accept_loaded_tx(const tools::wallet2::unsigned_tx_set &txs)
     }
     if (cd.change_dts.amount > 0)
     {
-      dests.insert(std::make_pair(get_account_address_as_str(m_wallet->testnet(), cd.change_dts.addr), cd.change_dts.amount));
+      std::unordered_map<std::string, uint64_t>::iterator it = dests.find(get_account_address_as_str(m_wallet->testnet(), cd.change_dts.addr));
-      amount_to_dests += cd.change_dts.amount;
+      if (it == dests.end())
-      change += cd.change_dts.amount;
+      {
+        fail_msg_writer() << tr("Claimed change does not go to a paid address");
+        return false;
+      }
+      if (it->second < cd.change_dts.amount)
+      {
+        fail_msg_writer() << tr("Claimed change is larger than payment to the change address");
+        return false;
+      }
+      change = cd.change_dts.amount;
+      it->second -= cd.change_dts.amount;
     }
   }
   std::string dest_string;
@@ -3158,7 +3168,7 @@ bool simple_wallet::accept_loaded_tx(const tools::wallet2::unsigned_tx_set &txs)
     dest_string = tr("with no destinations");
 
   uint64_t fee = amount - amount_to_dests;
-  std::string prompt_str = (boost::format(tr("Loaded %lu transactions, for %s, fee %s, change %s, %s, with min mixin %lu (full details in log file). Is this okay? (Y/Yes/N/No)")) % (unsigned long)txs.txes.size() % print_money(amount) % print_money(fee) % print_money(change) % dest_string % (unsigned long)min_mixin).str();
+  std::string prompt_str = (boost::format(tr("Loaded %lu transactions, for %s, fee %s, change %s, %s, with min mixin %lu. Is this okay? (Y/Yes/N/No)")) % (unsigned long)txs.txes.size() % print_money(amount) % print_money(fee) % print_money(change) % dest_string % (unsigned long)min_mixin).str();
   std::string accepted = command_line::input_line(prompt_str);
   return is_it_true(accepted);
 }
@@ -3236,7 +3246,7 @@ bool simple_wallet::submit_transfer(const std::vector<std::string> &args_)
         if (dust_in_fee != 0) prompt << boost::format(tr(", of which %s is dust from change")) % print_money(dust_in_fee);
         if (dust_not_in_fee != 0)  prompt << tr(".") << ENDL << boost::format(tr("A total of %s from dust change will be sent to dust address"))
                                                    % print_money(dust_not_in_fee);
-        prompt << tr(".") << ENDL << tr("Is this okay?  (Y/Yes/N/No)");
+        prompt << tr(".") << ENDL << "Full transaction details are available in the log file" << ENDL << tr("Is this okay?  (Y/Yes/N/No)");
 
         std::string accepted = command_line::input_line(prompt.str());
         if (std::cin.eof())

src/wallet/wallet2.cpp

@@ -74,8 +74,8 @@ using namespace cryptonote;
 // arbitrary, used to generate different hashes from the same input
 #define CHACHA8_KEY_TAIL 0x8c
 
-#define UNSIGNED_TX_PREFIX "Monero unsigned tx set\001"
+#define UNSIGNED_TX_PREFIX "Monero unsigned tx set\002"
-#define SIGNED_TX_PREFIX "Monero signed tx set\001"
+#define SIGNED_TX_PREFIX "Monero signed tx set\002"
 
 #define RECENT_OUTPUT_RATIO (0.25) // 25% of outputs are from the recent zone
 #define RECENT_OUTPUT_ZONE (5 * 86400) // last 5 days are the recent zone
@@ -2630,11 +2630,8 @@ bool wallet2::sign_tx(const std::string &unsigned_filename, const std::string &s
     signed_txes.ptx.push_back(pending_tx());
     tools::wallet2::pending_tx &ptx = signed_txes.ptx.back();
     crypto::secret_key tx_key;
-    std::vector<cryptonote::tx_destination_entry> dests = sd.destinations;
+    bool r = cryptonote::construct_tx_and_get_tx_key(m_account.get_keys(), sd.sources, sd.splitted_dsts, sd.extra, ptx.tx, sd.unlock_time, tx_key, sd.use_rct);
-    if (sd.change_dts.amount > 0)
+    THROW_WALLET_EXCEPTION_IF(!r, error::tx_not_constructed, sd.sources, sd.splitted_dsts, sd.unlock_time, m_testnet);
-      dests.push_back(sd.change_dts);
-    bool r = cryptonote::construct_tx_and_get_tx_key(m_account.get_keys(), sd.sources, dests, sd.extra, ptx.tx, sd.unlock_time, tx_key, sd.use_rct);
-    THROW_WALLET_EXCEPTION_IF(!r, error::tx_not_constructed, sd.sources, sd.destinations, sd.unlock_time, m_testnet);
     // we don't test tx size, because we don't know the current limit, due to not having a blockchain,
     // and it's a bit pointless to fail there anyway, since it'd be a (good) guess only. We sign anyway,
     // and if we really go over limit, the daemon will reject when it gets submitted. Chances are it's
@@ -2661,13 +2658,13 @@ bool wallet2::sign_tx(const std::string &unsigned_filename, const std::string &s
     ptx.key_images = key_images;
     ptx.fee = 0;
     for (const auto &i: sd.sources) ptx.fee += i.amount;
-    for (const auto &i: dests) ptx.fee -= i.amount;
+    for (const auto &i: sd.splitted_dsts) ptx.fee -= i.amount;
     ptx.dust = 0;
     ptx.dust_added_to_fee = false;
     ptx.change_dts = sd.change_dts;
-//    ptx.selected_transfers = selected_transfers;
+    ptx.selected_transfers = sd.selected_transfers;
     ptx.tx_key = rct::rct2sk(rct::identity()); // don't send it back to the untrusted view wallet
-    ptx.dests = sd.destinations;
+    ptx.dests = sd.splitted_dsts;
     ptx.construction_data = sd;
   }
 
@@ -2709,7 +2706,8 @@ bool wallet2::load_tx(const std::string &signed_filename, std::vector<tools::wal
     LOG_PRINT_L0("Failed to parse data from " << signed_filename);
     return false;
   }
-  LOG_PRINT_L1("Loaded signed tx data from binary: " << signed_txs.ptx.size() << " transactions");
+  LOG_PRINT_L0("Loaded signed tx data from binary: " << signed_txs.ptx.size() << " transactions");
+  for (auto &ptx: signed_txs.ptx) LOG_PRINT_L0(cryptonote::obj_to_json_str(ptx.tx));
 
   ptx = signed_txs.ptx;
 
@@ -3188,8 +3186,9 @@ void wallet2::transfer_selected(const std::vector<cryptonote::tx_destination_ent
   ptx.tx_key = tx_key;
   ptx.dests = dsts;
   ptx.construction_data.sources = sources;
-  ptx.construction_data.destinations = dsts;
   ptx.construction_data.change_dts = change_dts;
+  ptx.construction_data.splitted_dsts = splitted_dsts;
+  ptx.construction_data.selected_transfers = selected_transfers;
   ptx.construction_data.extra = tx.extra;
   ptx.construction_data.unlock_time = unlock_time;
   ptx.construction_data.use_rct = false;
@@ -3307,8 +3306,9 @@ void wallet2::transfer_selected_rct(std::vector<cryptonote::tx_destination_entry
   ptx.tx_key = tx_key;
   ptx.dests = dsts;
   ptx.construction_data.sources = sources;
-  ptx.construction_data.destinations = dsts;
   ptx.construction_data.change_dts = change_dts;
+  ptx.construction_data.splitted_dsts = splitted_dsts;
+  ptx.construction_data.selected_transfers = selected_transfers;
   ptx.construction_data.extra = tx.extra;
   ptx.construction_data.unlock_time = unlock_time;
   ptx.construction_data.use_rct = true;
@@ -3531,7 +3531,7 @@ std::vector<wallet2::pending_tx> wallet2::create_transactions_2(std::vector<cryp
     if (!prefered_inputs.empty())
     {
       string s;
-      for (auto i: prefered_inputs) s += print_money(m_transfers[i].amount()) + " ";
+      for (auto i: prefered_inputs) s += boost::lexical_cast<std::string>(i) + "(" + print_money(m_transfers[i].amount()) + ") ";
       LOG_PRINT_L1("Found prefered rct inputs for rct tx: " << s);
     }
   }
@@ -3551,7 +3551,7 @@ std::vector<wallet2::pending_tx> wallet2::create_transactions_2(std::vector<cryp
     size_t idx = !prefered_inputs.empty() ? pop_back(prefered_inputs) : !unused_transfers_indices.empty() ? pop_best_value(unused_transfers_indices, tx.selected_transfers) : pop_best_value(unused_dust_indices, tx.selected_transfers);
 
     const transfer_details &td = m_transfers[idx];
-    LOG_PRINT_L2("Picking output " << idx << ", amount " << print_money(td.amount()));
+    LOG_PRINT_L2("Picking output " << idx << ", amount " << print_money(td.amount()) << ", ki " << td.m_key_image);
 
     // add this output to the list to spend
     tx.selected_transfers.push_back(idx);

src/wallet/wallet2.h

@@ -154,16 +154,18 @@ namespace tools
     struct tx_construction_data
     {
       std::vector<cryptonote::tx_source_entry> sources;
-      std::vector<cryptonote::tx_destination_entry> destinations;
       cryptonote::tx_destination_entry change_dts;
+      std::vector<cryptonote::tx_destination_entry> splitted_dsts;
+      std::list<size_t> selected_transfers;
       std::vector<uint8_t> extra;
       uint64_t unlock_time;
       bool use_rct;
 
       BEGIN_SERIALIZE_OBJECT()
         FIELD(sources)
-        FIELD(destinations)
         FIELD(change_dts)
+        FIELD(splitted_dsts)
+        FIELD(selected_transfers)
         FIELD(extra)
         VARINT_FIELD(unlock_time)
         FIELD(use_rct)
@@ -930,8 +932,9 @@ namespace tools
     ptx.tx_key = tx_key;
     ptx.dests = dsts;
     ptx.construction_data.sources = sources;
-    ptx.construction_data.destinations = dsts;
     ptx.construction_data.change_dts = change_dts;
+    ptx.construction_data.splitted_dsts = splitted_dsts;
+    ptx.construction_data.selected_transfers = selected_transfers;
     ptx.construction_data.extra = tx.extra;
     ptx.construction_data.unlock_time = unlock_time;
     ptx.construction_data.use_rct = false;