Skip to content

Commit

Permalink
Merge #119: Replace send-to-self with dual send+receive entries
Browse files Browse the repository at this point in the history 
099dbe4 GUI: TransactionRecord: When time/index/etc match, sort send before receive (Luke Dashjr)
2d182f7 Bugfix: Ignore ischange flag when we're not the sender (Luke Dashjr)
71fbdb7 GUI: Remove SendToSelf TransactionRecord type (Luke Dashjr)
f3fbe99 GUI: TransactionRecord: Refactor to turn send-to-self into send+receive pairs (Luke Dashjr)
b9765ba GUI: TransactionRecord: Use "any from me" as the criteria for deciding whether a transaction is a send or receive (Luke Dashjr)

Pull request description:

  Makes the GUI transaction list more like the RPC, and IMO clearer in general.

  As a side effect, this also fixes the GUI entries when a transaction is a net profit to us, but some inputs were also from us.

  Originally bitcoin/bitcoin#15115

  Has Concept ACKs from @*Empact @*jonasschnelli

ACKs for top commit:
  hebasto:
    ACK 099dbe4.

Tree-SHA512: 7d581add2f59431aa019126d54232a1f15723def5147d7a1b672e9b6d525b6e5a944cc437701aa1bd5bd0fbe557a3d1f4b239337f42bdba4fe1d3960442d0e3b
  • Loading branch information
@hebasto
hebasto committed Sep 22, 2023
2 parents bce7b08 + 099dbe4 commit b000ed5
Show file tree
Hide file tree
Showing 6 changed files with 81 additions and 92 deletions.
1 change: 1 addition & 0 deletions src/interfaces/wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -393,6 +393,7 @@ struct WalletTx
CTransactionRef tx;
std::vector<wallet::isminetype> txin_is_mine;
std::vector<wallet::isminetype> txout_is_mine;
std::vector<bool> txout_is_change;
std::vector<CTxDestination> txout_address;
std::vector<wallet::isminetype> txout_address_is_mine;
CAmount credit;
Expand Down
160 changes: 77 additions & 83 deletions src/qt/transactionrecord.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

#include <QDateTime>

using wallet::ISMINE_NO;
using wallet::ISMINE_SPENDABLE;
using wallet::ISMINE_WATCH_ONLY;
using wallet::isminetype;
Expand All @@ -39,99 +40,52 @@ QList<TransactionRecord> TransactionRecord::decomposeTransaction(const interface
uint256 hash = wtx.tx->GetHash();
std::map<std::string, std::string> mapValue = wtx.value_map;

if (nNet > 0 || wtx.is_coinbase)
{
//
// 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
{
bool involvesWatchAddress = false;
isminetype fAllFromMe = ISMINE_SPENDABLE;
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;
}
}

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)
{
// 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 nTxFee = nDebit - wtx.tx->GetValueOut();

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)
for(unsigned int i = 0; i < wtx.tx->vout.size(); i++)
{
//
// 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;
const CTxOut& txout = wtx.tx->vout[i];

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.
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;
}

if (!std::get_if<CNoDestination>(&wtx.txout_address[nOut]))
//
// Debit
//

TransactionRecord sub(hash, nTime);
sub.idx = i;
sub.involvesWatchAddress = involvesWatchAddress;

if (!std::get_if<CNoDestination>(&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
{
Expand All @@ -151,15 +105,45 @@ QList<TransactionRecord> 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;
Expand All @@ -170,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;
Expand Down
1 change: 0 additions & 1 deletion src/qt/transactionrecord.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class TransactionRecord
SendToOther,
RecvWithAddress,
RecvFromOther,
SendToSelf
};

/** Number of confirmation recommended for accepting a transaction */
Expand Down
9 changes: 2 additions & 7 deletions src/qt/transactiontablemodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@

#include <core_io.h>
#include <interfaces/handler.h>
#include <tinyformat.h>
#include <uint256.h>

#include <algorithm>
Expand Down Expand Up @@ -377,8 +378,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:
Expand Down Expand Up @@ -421,8 +420,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;
}
Expand All @@ -441,8 +438,6 @@ QVariant TransactionTableModel::addressColor(const TransactionRecord *wtx) const
if(label.isEmpty())
return COLOR_BAREADDRESS;
} break;
case TransactionRecord::SendToSelf:
return COLOR_BAREADDRESS;
default:
break;
}
Expand Down Expand Up @@ -560,7 +555,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:
Expand Down
1 change: 0 additions & 1 deletion src/qt/transactionview.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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));

Expand Down
1 change: 1 addition & 0 deletions src/wallet/interfaces.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,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()) :
Expand Down

0 comments on commit b000ed5

Please sign in to comment.