Skip to content

Commit 791e429

Browse files
AmirAbramsAmirAbrams
committed
[PS] Fix incorrect RPC output for mixing txes
- See Dash pull request: dashpay/dash#2434
1 parent c8ac7ee commit 791e429

File tree

4 files changed

+87
-20
lines changed

4 files changed

+87
-20
lines changed

‎src/wallet/rpcwallet.cpp

+13-7
Original file line numberDiff line numberDiff line change
@@ -1689,22 +1689,27 @@ void ListTransactions(const CWalletTx& wtx, const std::string& strAccount, int n
16891689
wtx.GetAmounts(listReceived, listSent, nFee, strSentAccount, filter);
16901690

16911691
bool fAllAccounts = (strAccount == std::string("*"));
1692-
bool involvesWatchonly = wtx.IsFromMe(ISMINE_WATCH_ONLY);
1692+
bool involvesWatchonly = wtx.IsRelevantToMe(ISMINE_WATCH_ONLY);
16931693

16941694
// Sent
16951695
if ((!listSent.empty() || nFee != 0) && (fAllAccounts || strAccount == strSentAccount)) {
1696-
BOOST_FOREACH (const COutputEntry& s, listSent) {
1696+
for (const COutputEntry& s : listSent) {
16971697
UniValue entry(UniValue::VOBJ);
16981698
if (involvesWatchonly || (::IsMine(*pwalletMain, s.destination) & ISMINE_WATCH_ONLY))
16991699
entry.push_back(Pair("involvesWatchonly", true));
17001700
entry.push_back(Pair("account", strSentAccount));
17011701
MaybePushAddress(entry, s.destination);
1702-
std::map<std::string, std::string>::const_iterator it = wtx.mapValue.find("PS");
1703-
entry.push_back(Pair("category", (it != wtx.mapValue.end() && it->second == "1") ? "privatesend" : "send"));
1702+
if (s.vout >= 0) {
1703+
std::map<std::string, std::string>::const_iterator it = wtx.mapValue.find("PS");
1704+
entry.push_back(Pair("category", (it != wtx.mapValue.end() && it->second == "1") ? "privatesend" : "send"));
1705+
} else {
1706+
entry.push_back(Pair("category", "mixedsend"));
1707+
}
17041708
entry.push_back(Pair("amount", ValueFromAmount(-s.amount)));
17051709
if (pwalletMain->mapAddressBook.count(s.destination))
17061710
entry.push_back(Pair("label", pwalletMain->mapAddressBook[s.destination].name));
1707-
entry.push_back(Pair("vout", s.vout));
1711+
if (s.vout >= 0)
1712+
entry.push_back(Pair("vout", s.vout));
17081713
entry.push_back(Pair("fee", ValueFromAmount(-nFee)));
17091714
if (fLong)
17101715
WalletTxToJSON(wtx, entry);
@@ -1715,7 +1720,7 @@ void ListTransactions(const CWalletTx& wtx, const std::string& strAccount, int n
17151720

17161721
// Received
17171722
if (listReceived.size() > 0 && ((wtx.GetDepthInMainChain() >= nMinDepth) || wtx.IsLockedByInstantSend())) {
1718-
BOOST_FOREACH (const COutputEntry& r, listReceived) {
1723+
for (const COutputEntry& r : listReceived) {
17191724
std::string account;
17201725
if (pwalletMain->mapAddressBook.count(r.destination))
17211726
account = pwalletMain->mapAddressBook[r.destination].name;
@@ -2139,7 +2144,8 @@ UniValue gettransaction(const JSONRPCRequest& request)
21392144
CAmount nCredit = wtx.GetCredit(filter);
21402145
CAmount nDebit = wtx.GetDebit(filter);
21412146
CAmount nNet = nCredit - nDebit;
2142-
CAmount nFee = (wtx.IsFromMe(filter) ? wtx.tx->GetValueOut() - nDebit : 0);
2147+
bool fFromMe = wtx.IsFromMe(filter);
2148+
CAmount nFee = (fFromMe ? wtx.tx->GetValueOut() - nDebit : 0);
21432149

21442150
entry.push_back(Pair("amount", ValueFromAmount(nNet - nFee)));
21452151
if (wtx.IsFromMe(filter))

‎src/wallet/test/wallet_tests.cpp

+4-2
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,17 @@ static void add_coin(const CAmount& nValue, int nAge = 6*24, bool fIsFromMe = fa
3737
tx.vout.resize(nInput+1);
3838
tx.vout[nInput].nValue = nValue;
3939
if (fIsFromMe) {
40-
// IsFromMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(),
41-
// so stop vin being empty, and cache a non-zero Debit to fake out IsFromMe()
40+
// IsRelevantToMe() returns (GetDebit() > 0), and GetDebit() is 0 if vin.empty(),
41+
// so stop vin being empty, and cache a non-zero Debit to fake out IsRelevantToMe()
4242
tx.vin.resize(1);
4343
}
4444
std::unique_ptr<CWalletTx> wtx(new CWalletTx(&wallet, MakeTransactionRef(std::move(tx))));
4545
if (fIsFromMe)
4646
{
4747
wtx->fDebitCached = true;
4848
wtx->nDebitCached = 1;
49+
wtx->fFromMeCached = true;
50+
wtx->fFromMeCachedValue = true;
4951
}
5052
COutput output(wtx.get(), nInput, nAge, true, true);
5153
vCoins.push_back(output);

‎src/wallet/wallet.cpp

+60-8
Original file line numberDiff line numberDiff line change
@@ -1387,7 +1387,7 @@ bool CWallet::AddToWalletIfInvolvingMe(const CTransaction& tx, const CBlockIndex
13871387
fIsMyStealth = ScanForOwnedOutputs(tx);
13881388
}
13891389

1390-
if (fExisted || IsMine(tx) || IsFromMe(tx) || fIsMyStealth) {
1390+
if (fExisted || IsMine(tx) || IsRelevantToMe(tx) || fIsMyStealth) {
13911391
const CTransactionRef ptx = MakeTransactionRef(tx);
13921392
if (tx.nVersion == BDAP_TX_VERSION) {
13931393
uint256 linkID;
@@ -1926,11 +1926,29 @@ bool CWallet::IsMine(const CTransaction& tx) const
19261926
return false;
19271927
}
19281928

1929-
bool CWallet::IsFromMe(const CTransaction& tx) const
1929+
bool CWallet::IsRelevantToMe(const CTransaction& tx) const
19301930
{
19311931
return (GetDebit(tx, ISMINE_ALL) > 0);
19321932
}
19331933

1934+
bool CWallet::IsFromMe(const CTransaction& tx, const isminefilter& filter) const
1935+
{
1936+
for (const CTxIn& txin : tx.vin)
1937+
{
1938+
LOCK(cs_wallet);
1939+
auto mi = mapWallet.find(txin.prevout.hash);
1940+
if (mi == mapWallet.end())
1941+
return false;
1942+
1943+
const CWalletTx& prev = (*mi).second;
1944+
if (txin.prevout.n < prev.tx->vout.size())
1945+
if (!(IsMine(prev.tx->vout[txin.prevout.n]) & filter))
1946+
return false;
1947+
}
1948+
1949+
return true;
1950+
}
1951+
19341952
CAmount CWallet::GetDebit(const CTransaction& tx, const isminefilter& filter) const
19351953
{
19361954
CAmount nDebit = 0;
@@ -2036,7 +2054,9 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
20362054

20372055
// Compute fee:
20382056
CAmount nDebit = GetDebit(filter);
2039-
if (nDebit > 0) // debit>0 means we signed/sent this transaction
2057+
CAmount nCredit = 0;
2058+
bool fFromMe = IsFromMe(filter);
2059+
if (fFromMe) // means we signed/sent this transaction and all inputs are from us
20402060
{
20412061
CAmount nValueOut = tx->GetValueOut();
20422062
nFee = nDebit - nValueOut;
@@ -2049,11 +2069,13 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
20492069
// Only need to handle txouts if AT LEAST one of these is true:
20502070
// 1) they debit from us (sent)
20512071
// 2) the output is to us (received)
2052-
if (nDebit > 0) {
2072+
if (fFromMe) {
20532073
// Don't report 'change' txouts
20542074
if (pwallet->IsChange(txout))
20552075
continue;
2056-
} else if (!(fIsMine & filter))
2076+
}
2077+
2078+
if (nDebit == 0 && !(fIsMine & filter))
20572079
continue;
20582080

20592081
// In either case, we need to get the destination address
@@ -2068,12 +2090,24 @@ void CWalletTx::GetAmounts(std::list<COutputEntry>& listReceived,
20682090
COutputEntry output = {address, txout.nValue, (int)i};
20692091

20702092
// If we are debited by the transaction, add the output as a "sent" entry
2071-
if (nDebit > 0)
2093+
if (fFromMe)
20722094
listSent.push_back(output);
20732095

20742096
// If we are receiving the output, add it as a "received" entry
2075-
if (fIsMine & filter)
2097+
if (fIsMine & filter) {
2098+
nCredit += txout.nValue;
20762099
listReceived.push_back(output);
2100+
}
2101+
}
2102+
2103+
if (!fFromMe && nDebit > 0) {
2104+
if (nCredit == nDebit) {
2105+
for(const auto& output: listReceived)
2106+
listSent.push_back(output);
2107+
} else {
2108+
COutputEntry output = {CNoDestination(), nDebit, -1};
2109+
listSent.push_back(output);
2110+
}
20772111
}
20782112
}
20792113

@@ -2269,6 +2303,16 @@ CAmount CWalletTx::GetDebit(const isminefilter& filter) const
22692303
return debit;
22702304
}
22712305

2306+
bool CWalletTx::IsFromMe(const isminefilter& filter) const
2307+
{
2308+
if (fFromMeCached)
2309+
return fFromMeCachedValue;
2310+
2311+
fFromMeCachedValue = pwallet->IsFromMe(*this, filter);
2312+
fFromMeCached = true;
2313+
return fFromMeCachedValue;
2314+
}
2315+
22722316
CAmount CWalletTx::GetCredit(const isminefilter& filter) const
22732317
{
22742318
// Must wait until coinbase is safely deep enough in the chain before valuing it
@@ -3087,7 +3131,15 @@ bool CWallet::SelectCoinsMinConf(const CAmount& nTargetValue, const int nConfMin
30873131
const CWalletTx *pcoin = output.tx;
30883132

30893133
// if (fDebug) LogPrint("selectcoins", "value %s confirms %d\n", FormatMoney(pcoin->vout[output.i].nValue), output.nDepth);
3090-
if (output.nDepth < (pcoin->IsFromMe(ISMINE_ALL) ? nConfMine : nConfTheirs))
3134+
int minDepth;
3135+
if (!pcoin->IsRelevantToMe(ISMINE_ALL))
3136+
minDepth = nConfTheirs;
3137+
else if (pcoin->IsFromMe(ISMINE_ALL))
3138+
minDepth = nConfMine;
3139+
else
3140+
minDepth = std::max(nConfMine, nConfTheirs);
3141+
3142+
if (output.nDepth < minDepth)
30913143
continue;
30923144

30933145
if (!mempool.TransactionWithinChainLimit(pcoin->GetHash(), nMaxAncestors))

‎src/wallet/wallet.h

+10-3
Original file line numberDiff line numberDiff line change
@@ -357,6 +357,7 @@ class CWalletTx : public CMerkleTx
357357
int64_t nOrderPos; //!< position in ordered transaction list
358358

359359
// memory only
360+
mutable bool fFromMeCached;
360361
mutable bool fDebitCached;
361362
mutable bool fCreditCached;
362363
mutable bool fImmatureCreditCached;
@@ -369,6 +370,8 @@ class CWalletTx : public CMerkleTx
369370
mutable bool fImmatureWatchCreditCached;
370371
mutable bool fAvailableWatchCreditCached;
371372
mutable bool fChangeCached;
373+
mutable bool fFromMeCachedValue;
374+
372375
mutable CAmount nDebitCached;
373376
mutable CAmount nCreditCached;
374377
mutable CAmount nImmatureCreditCached;
@@ -402,6 +405,7 @@ class CWalletTx : public CMerkleTx
402405
nTimeSmart = 0;
403406
fFromMe = false;
404407
strFromAccount.clear();
408+
fFromMeCached = false;
405409
fDebitCached = false;
406410
fCreditCached = false;
407411
fImmatureCreditCached = false;
@@ -514,11 +518,13 @@ class CWalletTx : public CMerkleTx
514518

515519
void GetAccountAmounts(const std::string& strAccount, CAmount& nReceived, CAmount& nSent, CAmount& nFee, const isminefilter& filter) const;
516520

517-
bool IsFromMe(const isminefilter& filter) const
521+
bool IsRelevantToMe(const isminefilter& filter) const
518522
{
519523
return (GetDebit(filter) > 0);
520524
}
521525

526+
bool IsFromMe(const isminefilter& filter) const;
527+
522528
// True if only scriptSigs are different
523529
bool IsEquivalentTo(const CWalletTx& tx) const;
524530

@@ -1195,8 +1201,9 @@ class CWallet : public CCryptoKeyStore, public CValidationInterface
11951201
CAmount GetChange(const CTxOut& txout) const;
11961202
bool IsMine(const CTransaction& tx) const;
11971203
void AutoLockDynodeCollaterals();
1198-
/** should probably be renamed to IsRelevantToMe */
1199-
bool IsFromMe(const CTransaction& tx) const;
1204+
1205+
bool IsRelevantToMe(const CTransaction& tx) const;
1206+
bool IsFromMe(const CTransaction& tx, const isminefilter& filter) const;
12001207
CAmount GetDebit(const CTransaction& tx, const isminefilter& filter) const;
12011208
/** Returns whether all of the inputs match the filter */
12021209
bool IsAllFromMe(const CTransaction& tx, const isminefilter& filter) const;

0 commit comments

Comments
 (0)