Skip to content

Replace send-to-self with dual send+receive entries #119

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 22, 2023
Merged

Replace send-to-self with dual send+receive entries #119

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b9765ba
GUI: TransactionRecord: Use "any from me" as the criteria for decidin…
luke-jr Dec 15, 2018
f3fbe99
GUI: TransactionRecord: Refactor to turn send-to-self into send+recei…
luke-jr Dec 15, 2018
71fbdb7
GUI: Remove SendToSelf TransactionRecord type
luke-jr Jan 6, 2019
2d182f7
Bugfix: Ignore ischange flag when we're not the sender
luke-jr Jan 7, 2019
099dbe4
GUI: TransactionRecord: When time/index/etc match, sort send before r…
luke-jr Dec 15, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/interfaces/wallet.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,6 +387,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
Open in desktop
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)
Copy link
Member

@achow101 achow101 Jul 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In f3fbe99 "GUI: TransactionRecord: Refactor to turn send-to-self into send+receive pairs"

This for loop seems to be duplicated from the one above. I think this could be refactored so that we don't need to duplicate this work?

{
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

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

#include <algorithm>
Expand Down Expand Up @@ -380,8 +381,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 @@ -424,8 +423,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 @@ -444,8 +441,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 @@ -563,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:
Expand Down
1 change: 0 additions & 1 deletion src/qt/transactionview.cpp
View file
Open in desktop
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
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()) :
Expand Down