From b9765ba1d67d7b74c17f9ce70cad5487715208a0 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 15 Dec 2018 04:20:46 +0000 Subject: GUI: TransactionRecord: Use "any from me" as the criteria for deciding whether a transaction is a send or receive This changes behaviour (IMO for the better) in the case where some but not all inputs are from us, and the net amount is positive. --- src/qt/transactionrecord.cpp | 26 ++++++++++++++++---------- 1 file changed, 16 insertions(+), 10 deletions(-) diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 26144ba197..4b48b124d4 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -13,6 +13,7 @@ #include +using wallet::ISMINE_NO; using wallet::ISMINE_SPENDABLE; using wallet::ISMINE_WATCH_ONLY; using wallet::isminetype; @@ -39,8 +40,21 @@ QList TransactionRecord::decomposeTransaction(const interface uint256 hash = wtx.tx->GetHash(); std::map mapValue = wtx.value_map; - if (nNet > 0 || wtx.is_coinbase) - { + bool involvesWatchAddress = false; + isminetype fAllFromMe = ISMINE_SPENDABLE; + bool any_from_me = false; + if (wtx.is_coinbase) { + fAllFromMe = ISMINE_NO; + } else { + for (const isminetype mine : wtx.txin_is_mine) + { + if(mine & ISMINE_WATCH_ONLY) involvesWatchAddress = true; + if(fAllFromMe > mine) fAllFromMe = mine; + if (mine) any_from_me = true; + } + } + + if (!any_from_me) { // // Credit // @@ -78,14 +92,6 @@ QList TransactionRecord::decomposeTransaction(const interface } else { - bool involvesWatchAddress = false; - isminetype fAllFromMe = ISMINE_SPENDABLE; - for (const isminetype mine : wtx.txin_is_mine) - { - if(mine & ISMINE_WATCH_ONLY) involvesWatchAddress = true; - if(fAllFromMe > mine) fAllFromMe = mine; - } - isminetype fAllToMe = ISMINE_SPENDABLE; for (const isminetype mine : wtx.txout_is_mine) { -- cgit v1.2.3 From f3fbe99fcf90daec79d49fd5d868102dc99feb23 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 15 Dec 2018 19:52:03 +0000 Subject: GUI: TransactionRecord: Refactor to turn send-to-self into send+receive pairs --- src/interfaces/wallet.h | 1 + src/qt/transactionrecord.cpp | 130 ++++++++++++++++++------------------------- src/wallet/interfaces.cpp | 1 + 3 files changed, 55 insertions(+), 77 deletions(-) diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index f26ac866dc..b5d98e36fe 100644 --- a/src/interfaces/wallet.h +++ b/src/interfaces/wallet.h @@ -387,6 +387,7 @@ struct WalletTx CTransactionRef tx; std::vector txin_is_mine; std::vector txout_is_mine; + std::vector txout_is_change; std::vector txout_address; std::vector txout_address_is_mine; CAmount credit; diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 4b48b124d4..8a373451eb 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -54,90 +54,36 @@ QList TransactionRecord::decomposeTransaction(const interface } } - if (!any_from_me) { - // - // Credit - // - for(unsigned int i = 0; i < wtx.tx->vout.size(); i++) - { - const CTxOut& txout = wtx.tx->vout[i]; - isminetype mine = wtx.txout_is_mine[i]; - if(mine) - { - TransactionRecord sub(hash, nTime); - sub.idx = i; // vout index - sub.credit = txout.nValue; - sub.involvesWatchAddress = mine & ISMINE_WATCH_ONLY; - if (wtx.txout_address_is_mine[i]) - { - // Received by Bitcoin Address - sub.type = TransactionRecord::RecvWithAddress; - sub.address = EncodeDestination(wtx.txout_address[i]); - } - else - { - // Received by IP connection (deprecated features), or a multisignature or other non-simple transaction - sub.type = TransactionRecord::RecvFromOther; - sub.address = mapValue["from"]; - } - if (wtx.is_coinbase) - { - // Generated - sub.type = TransactionRecord::Generated; - } - - parts.append(sub); - } - } - } - else - { - isminetype fAllToMe = ISMINE_SPENDABLE; + if (fAllFromMe || !any_from_me) { for (const isminetype mine : wtx.txout_is_mine) { if(mine & ISMINE_WATCH_ONLY) involvesWatchAddress = true; - if(fAllToMe > mine) fAllToMe = mine; } - if (fAllFromMe && fAllToMe) + CAmount nTxFee = nDebit - wtx.tx->GetValueOut(); + + for(unsigned int i = 0; i < wtx.tx->vout.size(); i++) { - // Payment to self - std::string address; - for (auto it = wtx.txout_address.begin(); it != wtx.txout_address.end(); ++it) { - if (it != wtx.txout_address.begin()) address += ", "; - address += EncodeDestination(*it); + const CTxOut& txout = wtx.tx->vout[i]; + + if (wtx.txout_is_change[i]) { + continue; } - CAmount nChange = wtx.change; - parts.append(TransactionRecord(hash, nTime, TransactionRecord::SendToSelf, address, -(nDebit - nChange), nCredit - nChange)); - parts.last().involvesWatchAddress = involvesWatchAddress; // maybe pass to TransactionRecord as constructor argument - } - else if (fAllFromMe) - { - // - // Debit - // - CAmount nTxFee = nDebit - wtx.tx->GetValueOut(); + if (fAllFromMe) { + // + // Debit + // - for (unsigned int nOut = 0; nOut < wtx.tx->vout.size(); nOut++) - { - const CTxOut& txout = wtx.tx->vout[nOut]; TransactionRecord sub(hash, nTime); - sub.idx = nOut; + sub.idx = i; sub.involvesWatchAddress = involvesWatchAddress; - if(wtx.txout_is_mine[nOut]) - { - // Ignore parts sent to self, as this is usually the change - // from a transaction sent back to our own address. - continue; - } - - if (!std::get_if(&wtx.txout_address[nOut])) + if (!std::get_if(&wtx.txout_address[i])) { // Sent to Bitcoin Address sub.type = TransactionRecord::SendToAddress; - sub.address = EncodeDestination(wtx.txout_address[nOut]); + sub.address = EncodeDestination(wtx.txout_address[i]); } else { @@ -157,15 +103,45 @@ QList TransactionRecord::decomposeTransaction(const interface parts.append(sub); } + + isminetype mine = wtx.txout_is_mine[i]; + if(mine) + { + // + // Credit + // + + TransactionRecord sub(hash, nTime); + sub.idx = i; // vout index + sub.credit = txout.nValue; + sub.involvesWatchAddress = mine & ISMINE_WATCH_ONLY; + if (wtx.txout_address_is_mine[i]) + { + // Received by Bitcoin Address + sub.type = TransactionRecord::RecvWithAddress; + sub.address = EncodeDestination(wtx.txout_address[i]); + } + else + { + // Received by IP connection (deprecated features), or a multisignature or other non-simple transaction + sub.type = TransactionRecord::RecvFromOther; + sub.address = mapValue["from"]; + } + if (wtx.is_coinbase) + { + // Generated + sub.type = TransactionRecord::Generated; + } + + parts.append(sub); + } } - else - { - // - // Mixed debit transaction, can't break down payees - // - parts.append(TransactionRecord(hash, nTime, TransactionRecord::Other, "", nNet, 0)); - parts.last().involvesWatchAddress = involvesWatchAddress; - } + } else { + // + // Mixed debit transaction, can't break down payees + // + parts.append(TransactionRecord(hash, nTime, TransactionRecord::Other, "", nNet, 0)); + parts.last().involvesWatchAddress = involvesWatchAddress; } return parts; diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp index 9083c304b2..c027ff1240 100644 --- a/src/wallet/interfaces.cpp +++ b/src/wallet/interfaces.cpp @@ -64,6 +64,7 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx) result.txout_address_is_mine.reserve(wtx.tx->vout.size()); for (const auto& txout : wtx.tx->vout) { result.txout_is_mine.emplace_back(wallet.IsMine(txout)); + result.txout_is_change.push_back(OutputIsChange(wallet, txout)); result.txout_address.emplace_back(); result.txout_address_is_mine.emplace_back(ExtractDestination(txout.scriptPubKey, result.txout_address.back()) ? wallet.IsMine(result.txout_address.back()) : -- cgit v1.2.3 From 71fbdb7f403e673877be94a79cd4c6b13b0bbcd6 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 6 Jan 2019 12:45:52 +0000 Subject: GUI: Remove SendToSelf TransactionRecord type --- src/qt/transactionrecord.h | 1 - src/qt/transactiontablemodel.cpp | 6 ------ src/qt/transactionview.cpp | 1 - 3 files changed, 8 deletions(-) diff --git a/src/qt/transactionrecord.h b/src/qt/transactionrecord.h index dd34656d5f..3ebcac8ab1 100644 --- a/src/qt/transactionrecord.h +++ b/src/qt/transactionrecord.h @@ -78,7 +78,6 @@ public: SendToOther, RecvWithAddress, RecvFromOther, - SendToSelf }; /** Number of confirmation recommended for accepting a transaction */ diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index 44b4fee2e7..c551456bda 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -380,8 +380,6 @@ QString TransactionTableModel::formatTxType(const TransactionRecord *wtx) const case TransactionRecord::SendToAddress: case TransactionRecord::SendToOther: return tr("Sent to"); - case TransactionRecord::SendToSelf: - return tr("Payment to yourself"); case TransactionRecord::Generated: return tr("Mined"); default: @@ -424,8 +422,6 @@ QString TransactionTableModel::formatTxToAddress(const TransactionRecord *wtx, b return lookupAddress(wtx->address, tooltip) + watchAddress; case TransactionRecord::SendToOther: return QString::fromStdString(wtx->address) + watchAddress; - case TransactionRecord::SendToSelf: - return lookupAddress(wtx->address, tooltip) + watchAddress; default: return tr("(n/a)") + watchAddress; } @@ -444,8 +440,6 @@ QVariant TransactionTableModel::addressColor(const TransactionRecord *wtx) const if(label.isEmpty()) return COLOR_BAREADDRESS; } break; - case TransactionRecord::SendToSelf: - return COLOR_BAREADDRESS; default: break; } diff --git a/src/qt/transactionview.cpp b/src/qt/transactionview.cpp index 778ef04b77..4e771258eb 100644 --- a/src/qt/transactionview.cpp +++ b/src/qt/transactionview.cpp @@ -91,7 +91,6 @@ TransactionView::TransactionView(const PlatformStyle *platformStyle, QWidget *pa TransactionFilterProxy::TYPE(TransactionRecord::RecvFromOther)); typeWidget->addItem(tr("Sent to"), TransactionFilterProxy::TYPE(TransactionRecord::SendToAddress) | TransactionFilterProxy::TYPE(TransactionRecord::SendToOther)); - typeWidget->addItem(tr("To yourself"), TransactionFilterProxy::TYPE(TransactionRecord::SendToSelf)); typeWidget->addItem(tr("Mined"), TransactionFilterProxy::TYPE(TransactionRecord::Generated)); typeWidget->addItem(tr("Other"), TransactionFilterProxy::TYPE(TransactionRecord::Other)); -- cgit v1.2.3 From 2d182f77cd8100395cf47a721bd01dc8620c9718 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 7 Jan 2019 09:37:43 +0000 Subject: Bugfix: Ignore ischange flag when we're not the sender If we didn't send it, it can't possibly be change, even if that's the key's purpose --- src/qt/transactionrecord.cpp | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 8a373451eb..85b7e70187 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -66,11 +66,13 @@ QList TransactionRecord::decomposeTransaction(const interface { const CTxOut& txout = wtx.tx->vout[i]; - if (wtx.txout_is_change[i]) { - continue; - } - if (fAllFromMe) { + // Change is only really possible if we're the sender + // Otherwise, someone just sent bitcoins to a change address, which should be shown + if (wtx.txout_is_change[i]) { + continue; + } + // // Debit // -- cgit v1.2.3 From 099dbe4224e0e896604e7f6901d0fc302b0bd3a0 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 15 Dec 2018 22:49:26 +0000 Subject: GUI: TransactionRecord: When time/index/etc match, sort send before receive --- src/qt/transactionrecord.cpp | 14 ++++++++++++-- src/qt/transactiontablemodel.cpp | 3 ++- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/src/qt/transactionrecord.cpp b/src/qt/transactionrecord.cpp index 85b7e70187..f08c0d17c3 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -154,11 +154,21 @@ void TransactionRecord::updateStatus(const interfaces::WalletTxStatus& wtx, cons // Determine transaction status // Sort order, unrecorded transactions sort to the top - status.sortKey = strprintf("%010d-%01d-%010u-%03d", + int typesort; + switch (type) { + case SendToAddress: case SendToOther: + typesort = 2; break; + case RecvWithAddress: case RecvFromOther: + typesort = 3; break; + default: + typesort = 9; + } + status.sortKey = strprintf("%010d-%01d-%010u-%03d-%d", wtx.block_height, wtx.is_coinbase ? 1 : 0, wtx.time_received, - idx); + idx, + typesort); status.countsForBalance = wtx.is_trusted && !(wtx.blocks_to_maturity > 0); status.depth = wtx.depth_in_main_chain; status.m_cur_block_hash = block_hash; diff --git a/src/qt/transactiontablemodel.cpp b/src/qt/transactiontablemodel.cpp index c551456bda..8ae0a3c3ab 100644 --- a/src/qt/transactiontablemodel.cpp +++ b/src/qt/transactiontablemodel.cpp @@ -16,6 +16,7 @@ #include #include +#include #include #include @@ -557,7 +558,7 @@ QVariant TransactionTableModel::data(const QModelIndex &index, int role) const case Status: return QString::fromStdString(rec->status.sortKey); case Date: - return rec->time; + return QString::fromStdString(strprintf("%020s-%s", rec->time, rec->status.sortKey)); case Type: return formatTxType(rec); case Watchonly: -- cgit v1.2.3