From b5d6faada3d5604214de436a9acbb1687ca35587 Mon Sep 17 00:00:00 2001 From: moneromooo-monero Date: Wed, 2 Nov 2016 23:11:30 +0000 Subject: [PATCH] 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. --- src/simplewallet/simplewallet.cpp | 4 ++-- src/wallet/api/transaction_history.cpp | 2 +- src/wallet/wallet2.cpp | 12 ++++++++++++ src/wallet/wallet2.h | 26 ++++++++++++++++++++++++-- 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/src/simplewallet/simplewallet.cpp b/src/simplewallet/simplewallet.cpp index b1049265..12a04ee8 100644 --- a/src/simplewallet/simplewallet.cpp +++ b/src/simplewallet/simplewallet.cpp @@ -3688,7 +3688,7 @@ bool simple_wallet::show_transfers(const std::vector &args_) for (std::list>::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 &args_) for (std::list>::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); diff --git a/src/wallet/api/transaction_history.cpp b/src/wallet/api/transaction_history.cpp index 2ba5f362..63c4ea3c 100644 --- a/src/wallet/api/transaction_history.cpp +++ b/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); diff --git a/src/wallet/wallet2.cpp b/src/wallet/wallet2.cpp index 8ea60537..f57a8d2c 100644 --- a/src/wallet/wallet2.cpp +++ b/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_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; diff --git a/src/wallet/wallet2.h b/src/wallet/wallet2.h index 6cd288ac..b34db0b6 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 @@ -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