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.
This commit is contained in:
moneromooo-monero 2016-11-02 23:11:30 +00:00
parent d51f1af75f
commit b5d6faada3
No known key found for this signature in database
GPG Key ID: 686F07454D6CEFC3
4 changed files with 39 additions and 5 deletions

View File

@ -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) { 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; 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 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; std::string dests;
for (const auto &d: pd.m_dests) { for (const auto &d: pd.m_dests) {
if (!dests.empty()) 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) { 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; const tools::wallet2::unconfirmed_transfer_details &pd = i->second;
uint64_t amount = pd.m_amount_in; 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); 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) if (payment_id.substr(16).find_first_not_of('0') == std::string::npos)
payment_id = payment_id.substr(0,16); payment_id = payment_id.substr(0,16);

View File

@ -157,7 +157,7 @@ void TransactionHistoryImpl::refresh()
const tools::wallet2::confirmed_transfer_details &pd = i->second; 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 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); std::string payment_id = string_tools::pod_to_hex(i->second.m_payment_id);

View File

@ -704,6 +704,17 @@ void wallet2::process_outgoing(const cryptonote::transaction &tx, uint64_t heigh
else else
entry.first->second.m_amount_out = spent - tx.rct_signatures.txnFee; entry.first->second.m_amount_out = spent - tx.rct_signatures.txnFee;
entry.first->second.m_change = received; 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_block_height = height;
entry.first->second.m_timestamp = ts; 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; utd.m_amount_out = 0;
for (const auto &d: dests) for (const auto &d: dests)
utd.m_amount_out += d.amount; utd.m_amount_out += d.amount;
utd.m_amount_out += change_amount;
utd.m_change = change_amount; utd.m_change = change_amount;
utd.m_sent_time = time(NULL); utd.m_sent_time = time(NULL);
utd.m_tx = (const cryptonote::transaction_prefix&)tx; utd.m_tx = (const cryptonote::transaction_prefix&)tx;

View File

@ -570,8 +570,8 @@ namespace tools
BOOST_CLASS_VERSION(tools::wallet2, 14) BOOST_CLASS_VERSION(tools::wallet2, 14)
BOOST_CLASS_VERSION(tools::wallet2::transfer_details, 4) BOOST_CLASS_VERSION(tools::wallet2::transfer_details, 4)
BOOST_CLASS_VERSION(tools::wallet2::payment_details, 1) BOOST_CLASS_VERSION(tools::wallet2::payment_details, 1)
BOOST_CLASS_VERSION(tools::wallet2::unconfirmed_transfer_details, 5) BOOST_CLASS_VERSION(tools::wallet2::unconfirmed_transfer_details, 6)
BOOST_CLASS_VERSION(tools::wallet2::confirmed_transfer_details, 2) BOOST_CLASS_VERSION(tools::wallet2::confirmed_transfer_details, 3)
namespace boost namespace boost
{ {
@ -675,6 +675,14 @@ namespace boost
return; return;
a & x.m_amount_in; a & x.m_amount_in;
a & x.m_amount_out; 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> template <class Archive>
@ -691,6 +699,20 @@ namespace boost
if (ver < 2) if (ver < 2)
return; return;
a & x.m_timestamp; 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> template <class Archive>