Merge pull request #1295

b5d6faa wallet: fix bad amounts/fees again (moneromooo-monero)

src/simplewallet/simplewallet.cpp

@@ -3688,7 +3688,7 @@ bool simple_wallet::show_transfers(const std::vector<std::string> &args_)
     for (std::list<std::pair<crypto::hash, tools::wallet2::confirmed_transfer_details>>::const_iterator i = payments.begin(); i != payments.end(); ++i) {
       const tools::wallet2::confirmed_transfer_details &pd = i->second;
       uint64_t change = pd.m_change == (uint64_t)-1 ? 0 : pd.m_change; // change may not be known
-      uint64_t fee = pd.m_amount_in - pd.m_amount_out - change;
+      uint64_t fee = pd.m_amount_in - pd.m_amount_out;
       std::string dests;
       for (const auto &d: pd.m_dests) {
         if (!dests.empty())
@@ -3738,7 +3738,7 @@ bool simple_wallet::show_transfers(const std::vector<std::string> &args_)
     for (std::list<std::pair<crypto::hash, tools::wallet2::unconfirmed_transfer_details>>::const_iterator i = upayments.begin(); i != upayments.end(); ++i) {
       const tools::wallet2::unconfirmed_transfer_details &pd = i->second;
       uint64_t amount = pd.m_amount_in;
-      uint64_t fee = amount - pd.m_amount_out - pd.m_change;
+      uint64_t fee = amount - pd.m_amount_out;
       std::string payment_id = string_tools::pod_to_hex(i->second.m_payment_id);
       if (payment_id.substr(16).find_first_not_of('0') == std::string::npos)
         payment_id = payment_id.substr(0,16);

src/wallet/api/transaction_history.cpp

@@ -157,7 +157,7 @@ void TransactionHistoryImpl::refresh()
         const tools::wallet2::confirmed_transfer_details &pd = i->second;
         
         uint64_t change = pd.m_change == (uint64_t)-1 ? 0 : pd.m_change; // change may not be known
-        uint64_t fee = pd.m_amount_in - pd.m_amount_out - change;
+        uint64_t fee = pd.m_amount_in - pd.m_amount_out;
         
 
         std::string payment_id = string_tools::pod_to_hex(i->second.m_payment_id);

src/wallet/wallet2.cpp

@@ -704,6 +704,17 @@ void wallet2::process_outgoing(const cryptonote::transaction &tx, uint64_t heigh
     else
       entry.first->second.m_amount_out = spent - tx.rct_signatures.txnFee;
     entry.first->second.m_change = received;
+
+    std::vector<tx_extra_field> tx_extra_fields;
+    if(parse_tx_extra(tx.extra, tx_extra_fields))
+    {
+      tx_extra_nonce extra_nonce;
+      if (find_tx_extra_field_by_type(tx_extra_fields, extra_nonce))
+      {
+        // we do not care about failure here
+        get_payment_id_from_tx_extra_nonce(extra_nonce.nonce, entry.first->second.m_payment_id);
+      }
+    }
   }
   entry.first->second.m_block_height = height;
   entry.first->second.m_timestamp = ts;
@@ -2355,6 +2366,7 @@ void wallet2::add_unconfirmed_tx(const cryptonote::transaction& tx, uint64_t amo
   utd.m_amount_out = 0;
   for (const auto &d: dests)
     utd.m_amount_out += d.amount;
+  utd.m_amount_out += change_amount;
   utd.m_change = change_amount;
   utd.m_sent_time = time(NULL);
   utd.m_tx = (const cryptonote::transaction_prefix&)tx;

src/wallet/wallet2.h

@@ -570,8 +570,8 @@ namespace tools
 BOOST_CLASS_VERSION(tools::wallet2, 14)
 BOOST_CLASS_VERSION(tools::wallet2::transfer_details, 4)
 BOOST_CLASS_VERSION(tools::wallet2::payment_details, 1)
-BOOST_CLASS_VERSION(tools::wallet2::unconfirmed_transfer_details, 5)
-BOOST_CLASS_VERSION(tools::wallet2::confirmed_transfer_details, 2)
+BOOST_CLASS_VERSION(tools::wallet2::unconfirmed_transfer_details, 6)
+BOOST_CLASS_VERSION(tools::wallet2::confirmed_transfer_details, 3)
 
 namespace boost
 {
@@ -675,6 +675,14 @@ namespace boost
         return;
       a & x.m_amount_in;
       a & x.m_amount_out;
+      if (ver < 6)
+      {
+        // v<6 may not have change accumulated in m_amount_out, which is a pain,
+        // as it's readily understood to be sum of outputs.
+        // We convert it to include change from v6
+        if (!typename Archive::is_saving() && x.m_change != (uint64_t)-1)
+          x.m_amount_out += x.m_change;
+      }
     }
 
     template <class Archive>
@@ -691,6 +699,20 @@ namespace boost
       if (ver < 2)
         return;
       a & x.m_timestamp;
+      if (ver < 3)
+      {
+        // v<3 may not have change accumulated in m_amount_out, which is a pain,
+        // as it's readily understood to be sum of outputs. Whether it got added
+        // or not depends on whether it came from a unconfirmed_transfer_details
+        // (not included) or not (included). We can't reliably tell here, so we
+        // check whether either yields a "negative" fee, or use the other if so.
+        // We convert it to include change from v3
+        if (!typename Archive::is_saving() && x.m_change != (uint64_t)-1)
+        {
+          if (x.m_amount_in > (x.m_amount_out + x.m_change))
+            x.m_amount_out += x.m_change;
+        }
+      }
     }
 
     template <class Archive>