From b9765ba1d67d7b74c17f9ce70cad5487715208a0 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 15 Dec 2018 04:20:46 +0000 Subject: [PATCH 1/5] 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 26144ba197e..4b48b124d47 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) { From f3fbe99fcf90daec79d49fd5d868102dc99feb23 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 15 Dec 2018 19:52:03 +0000 Subject: [PATCH 2/5] GUI: TransactionRecord: Refactor to turn send-to-self into send+receive pairs --- src/interfaces/wallet.h | 1 + src/qt/transactionrecord.cpp | 138 +++++++++++++++-------------------- src/wallet/interfaces.cpp | 1 + 3 files changed, 59 insertions(+), 81 deletions(-) diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h index f26ac866dcf..b5d98e36fe6 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 4b48b124d47..8a373451eb6 100644 --- a/src/qt/transactionrecord.cpp +++ b/src/qt/transactionrecord.cpp @@ -54,16 +54,63 @@ QList TransactionRecord::decomposeTransaction(const interface } } - if (!any_from_me) { - // - // Credit - // + if (fAllFromMe || !any_from_me) { + for (const isminetype mine : wtx.txout_is_mine) + { + if(mine & ISMINE_WATCH_ONLY) involvesWatchAddress = true; + } + + CAmount nTxFee = nDebit - wtx.tx->GetValueOut(); + for(unsigned int i = 0; i < wtx.tx->vout.size(); i++) { const CTxOut& txout = wtx.tx->vout[i]; + + if (wtx.txout_is_change[i]) { + continue; + } + + if (fAllFromMe) { + // + // Debit + // + + TransactionRecord sub(hash, nTime); + sub.idx = i; + sub.involvesWatchAddress = involvesWatchAddress; + + if (!std::get_if(&wtx.txout_address[i])) + { + // Sent to Bitcoin Address + sub.type = TransactionRecord::SendToAddress; + sub.address = EncodeDestination(wtx.txout_address[i]); + } + else + { + // Sent to IP, or other non-address transaction like OP_EVAL + sub.type = TransactionRecord::SendToOther; + sub.address = mapValue["to"]; + } + + CAmount nValue = txout.nValue; + /* Add fee to first output */ + if (nTxFee > 0) + { + nValue += nTxFee; + nTxFee = 0; + } + sub.debit = -nValue; + + 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; @@ -89,83 +136,12 @@ QList TransactionRecord::decomposeTransaction(const interface parts.append(sub); } } - } - else - { - isminetype fAllToMe = ISMINE_SPENDABLE; - for (const isminetype mine : wtx.txout_is_mine) - { - if(mine & ISMINE_WATCH_ONLY) involvesWatchAddress = true; - if(fAllToMe > mine) fAllToMe = mine; - } - - if (fAllFromMe && fAllToMe) - { - // 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); - } - - 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(); - - 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.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])) - { - // Sent to Bitcoin Address - sub.type = TransactionRecord::SendToAddress; - sub.address = EncodeDestination(wtx.txout_address[nOut]); - } - else - { - // Sent to IP, or other non-address transaction like OP_EVAL - sub.type = TransactionRecord::SendToOther; - sub.address = mapValue["to"]; - } - - CAmount nValue = txout.nValue; - /* Add fee to first output */ - if (nTxFee > 0) - { - nValue += nTxFee; - nTxFee = 0; - } - sub.debit = -nValue; - - 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 9083c304b2c..c027ff12401 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()) : From 71fbdb7f403e673877be94a79cd4c6b13b0bbcd6 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sun, 6 Jan 2019 12:45:52 +0000 Subject: [PATCH 3/5] 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 dd34656d5fd..3ebcac8ab10 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 44b4fee2e79..c551456bda3 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 778ef04b773..4e771258eb8 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)); From 2d182f77cd8100395cf47a721bd01dc8620c9718 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Mon, 7 Jan 2019 09:37:43 +0000 Subject: [PATCH 4/5] 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 8a373451eb6..85b7e701874 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 // From 099dbe4224e0e896604e7f6901d0fc302b0bd3a0 Mon Sep 17 00:00:00 2001 From: Luke Dashjr Date: Sat, 15 Dec 2018 22:49:26 +0000 Subject: [PATCH 5/5] 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 85b7e701874..f08c0d17c37 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 c551456bda3..8ae0a3c3abe 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: