wallet: fix bad amounts/fees again

m_amount_out was sometimes getting initialized with the sum of
an transaction's outputs, and sometimes with the sum of outputs
that were not change. This caused confusion and bugs. We now
always set it to the sum of outputs. This reverts an earlier
fix for bad amounts as this used the other semantics. The wallet
data should be converted automatically in a percentage of cases
that I'm hesitant to estimate. In any case, restoring from seed
or keys or rebuilding the cache will get it right.

1 files changed, 24 insertions, 2 deletions

diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h
index 6cd288ac1..b34db0b6c 100644
--- a/src/wallet/wallet2.h
+++ b/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>