Skip to content

Commit 51b93a9

Browse files
ryanofskyryanofsky
committed
[wallet] Get rid of CWalletTx default constructor
No change in behavior. This makes CWalletTx construction more explicit and will now trigger compiler errors on mapWallet[hash] lookups that could inadvertently create dummy transaction entries.
1 parent 8bf612d commit 51b93a9

File tree

7 files changed

+32
-33
lines changed

7 files changed

+32
-33
lines changed

‎src/qt/walletmodel.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -578,9 +578,9 @@ void WalletModel::getOutputs(const std::vector<COutPoint>& vOutpoints, std::vect
578578
for (const COutPoint& outpoint : vOutpoints)
579579
{
580580
if (!wallet->mapWallet.count(outpoint.hash)) continue;
581-
int nDepth = wallet->mapWallet[outpoint.hash].GetDepthInMainChain();
581+
int nDepth = wallet->mapWallet.at(outpoint.hash).GetDepthInMainChain();
582582
if (nDepth < 0) continue;
583-
COutput out(&wallet->mapWallet[outpoint.hash], outpoint.n, nDepth, true /* spendable */, true /* solvable */, true /* safe */);
583+
COutput out(&wallet->mapWallet.at(outpoint.hash), outpoint.n, nDepth, true /* spendable */, true /* solvable */, true /* safe */);
584584
vOutputs.push_back(out);
585585
}
586586
}

‎src/wallet/feebumper.cpp‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -246,7 +246,7 @@ bool CFeeBumper::commit(CWallet *pWallet)
246246
currentResult = BumpFeeResult::MISC_ERROR;
247247
return false;
248248
}
249-
CWalletTx& oldWtx = pWallet->mapWallet[txid];
249+
CWalletTx& oldWtx = pWallet->mapWallet.at(txid);
250250

251251
// make sure the transaction still has no descendants and hasn't been mined in the meantime
252252
if (!preconditionChecks(pWallet, oldWtx)) {

‎src/wallet/rpcwallet.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1878,7 +1878,7 @@ UniValue listsinceblock(const JSONRPCRequest& request)
18781878
if (pwallet->mapWallet.count(tx->GetHash()) > 0) {
18791879
// We want all transactions regardless of confirmation count to appear here,
18801880
// even negative confirmation ones, hence the big negative.
1881-
ListTransactions(pwallet, pwallet->mapWallet[tx->GetHash()], "*", -100000000, true, removed, filter);
1881+
ListTransactions(pwallet, pwallet->mapWallet.at(tx->GetHash()), "*", -100000000, true, removed, filter);
18821882
}
18831883
}
18841884
paltindex = paltindex->pprev;
@@ -1958,10 +1958,11 @@ UniValue gettransaction(const JSONRPCRequest& request)
19581958
filter = filter | ISMINE_WATCH_ONLY;
19591959

19601960
UniValue entry(UniValue::VOBJ);
1961-
if (!pwallet->mapWallet.count(hash)) {
1961+
auto it = pwallet->mapWallet.find(hash);
1962+
if (it == pwallet->mapWallet.end()) {
19621963
throw JSONRPCError(RPC_INVALID_ADDRESS_OR_KEY, "Invalid or non-wallet transaction id");
19631964
}
1964-
const CWalletTx& wtx = pwallet->mapWallet[hash];
1965+
const CWalletTx& wtx = it->second;
19651966

19661967
CAmount nCredit = wtx.GetCredit(filter);
19671968
CAmount nDebit = wtx.GetDebit(filter);

‎src/wallet/test/accounting_tests.cpp‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ GetResults(std::map<CAmount, CAccountingEntry>& results)
3131
BOOST_AUTO_TEST_CASE(acc_orderupgrade)
3232
{
3333
std::vector<CWalletTx*> vpwtx;
34-
CWalletTx wtx;
34+
CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef());
3535
CAccountingEntry ae;
3636
std::map<CAmount, CAccountingEntry> results;
3737

@@ -46,7 +46,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade)
4646

4747
wtx.mapValue["comment"] = "z";
4848
pwalletMain->AddToWallet(wtx);
49-
vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]);
49+
vpwtx.push_back(&pwalletMain->mapWallet.at(wtx.GetHash()));
5050
vpwtx[0]->nTimeReceived = (unsigned int)1333333335;
5151
vpwtx[0]->nOrderPos = -1;
5252

@@ -88,7 +88,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade)
8888
wtx.SetTx(MakeTransactionRef(std::move(tx)));
8989
}
9090
pwalletMain->AddToWallet(wtx);
91-
vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]);
91+
vpwtx.push_back(&pwalletMain->mapWallet.at(wtx.GetHash()));
9292
vpwtx[1]->nTimeReceived = (unsigned int)1333333336;
9393

9494
wtx.mapValue["comment"] = "x";
@@ -98,7 +98,7 @@ BOOST_AUTO_TEST_CASE(acc_orderupgrade)
9898
wtx.SetTx(MakeTransactionRef(std::move(tx)));
9999
}
100100
pwalletMain->AddToWallet(wtx);
101-
vpwtx.push_back(&pwalletMain->mapWallet[wtx.GetHash()]);
101+
vpwtx.push_back(&pwalletMain->mapWallet.at(wtx.GetHash()));
102102
vpwtx[2]->nTimeReceived = (unsigned int)1333333329;
103103
vpwtx[2]->nOrderPos = -1;
104104

‎src/wallet/wallet.cpp‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -562,18 +562,18 @@ void CWallet::SyncMetaData(std::pair<TxSpends::iterator, TxSpends::iterator> ran
562562
for (TxSpends::iterator it = range.first; it != range.second; ++it)
563563
{
564564
const uint256& hash = it->second;
565-
int n = mapWallet[hash].nOrderPos;
565+
int n = mapWallet.at(hash).nOrderPos;
566566
if (n < nMinOrderPos)
567567
{
568568
nMinOrderPos = n;
569-
copyFrom = &mapWallet[hash];
569+
copyFrom = &mapWallet.at(hash);
570570
}
571571
}
572572
// Now copy data from copyFrom to rest:
573573
for (TxSpends::iterator it = range.first; it != range.second; ++it)
574574
{
575575
const uint256& hash = it->second;
576-
CWalletTx* copyTo = &mapWallet[hash];
576+
CWalletTx* copyTo = &mapWallet.at(hash);
577577
if (copyFrom == copyTo) continue;
578578
if (!copyFrom->IsEquivalentTo(*copyTo)) continue;
579579
copyTo->mapValue = copyFrom->mapValue;
@@ -624,7 +624,7 @@ void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid)
624624
void CWallet::AddToSpends(const uint256& wtxid)
625625
{
626626
assert(mapWallet.count(wtxid));
627-
CWalletTx& thisTx = mapWallet[wtxid];
627+
CWalletTx& thisTx = mapWallet.at(wtxid);
628628
if (thisTx.IsCoinBase()) // Coinbases don't spend anything!
629629
return;
630630

@@ -1001,14 +1001,17 @@ bool CWallet::LoadToWallet(const CWalletTx& wtxIn)
10011001
{
10021002
uint256 hash = wtxIn.GetHash();
10031003

1004-
mapWallet[hash] = wtxIn;
1005-
CWalletTx& wtx = mapWallet[hash];
1004+
auto inserted = mapWallet.emplace(hash, wtxIn);
1005+
CWalletTx& wtx = inserted.first->second;
1006+
if (!inserted.second) {
1007+
wtx = wtxIn;
1008+
}
10061009
wtx.BindWallet(this);
10071010
wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, (CAccountingEntry*)0)));
10081011
AddToSpends(hash);
10091012
for (const CTxIn& txin : wtx.tx->vin) {
10101013
if (mapWallet.count(txin.prevout.hash)) {
1011-
CWalletTx& prevtx = mapWallet[txin.prevout.hash];
1014+
CWalletTx& prevtx = mapWallet.at(txin.prevout.hash);
10121015
if (prevtx.nIndex == -1 && !prevtx.hashUnset()) {
10131016
MarkConflicted(prevtx.hashBlock, wtx.GetHash());
10141017
}
@@ -1108,7 +1111,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
11081111

11091112
// Can't mark abandoned if confirmed or in mempool
11101113
assert(mapWallet.count(hashTx));
1111-
CWalletTx& origtx = mapWallet[hashTx];
1114+
CWalletTx& origtx = mapWallet.at(hashTx);
11121115
if (origtx.GetDepthInMainChain() > 0 || origtx.InMempool()) {
11131116
return false;
11141117
}
@@ -1120,7 +1123,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
11201123
todo.erase(now);
11211124
done.insert(now);
11221125
assert(mapWallet.count(now));
1123-
CWalletTx& wtx = mapWallet[now];
1126+
CWalletTx& wtx = mapWallet.at(now);
11241127
int currentconfirm = wtx.GetDepthInMainChain();
11251128
// If the orig tx was not in block, none of its spends can be
11261129
assert(currentconfirm <= 0);
@@ -1146,7 +1149,7 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
11461149
for (const CTxIn& txin : wtx.tx->vin)
11471150
{
11481151
if (mapWallet.count(txin.prevout.hash))
1149-
mapWallet[txin.prevout.hash].MarkDirty();
1152+
mapWallet.at(txin.prevout.hash).MarkDirty();
11501153
}
11511154
}
11521155
}
@@ -1185,7 +1188,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
11851188
todo.erase(now);
11861189
done.insert(now);
11871190
assert(mapWallet.count(now));
1188-
CWalletTx& wtx = mapWallet[now];
1191+
CWalletTx& wtx = mapWallet.at(now);
11891192
int currentconfirm = wtx.GetDepthInMainChain();
11901193
if (conflictconfirms < currentconfirm) {
11911194
// Block is 'more conflicted' than current confirm; update.
@@ -1207,7 +1210,7 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
12071210
for (const CTxIn& txin : wtx.tx->vin)
12081211
{
12091212
if (mapWallet.count(txin.prevout.hash))
1210-
mapWallet[txin.prevout.hash].MarkDirty();
1213+
mapWallet.at(txin.prevout.hash).MarkDirty();
12111214
}
12121215
}
12131216
}
@@ -1225,7 +1228,7 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pin
12251228
for (const CTxIn& txin : tx.vin)
12261229
{
12271230
if (mapWallet.count(txin.prevout.hash))
1228-
mapWallet[txin.prevout.hash].MarkDirty();
1231+
mapWallet.at(txin.prevout.hash).MarkDirty();
12291232
}
12301233
}
12311234

@@ -2988,7 +2991,7 @@ bool CWallet::CommitTransaction(CTransactionRef tx, mapValue_t mapValue, std::ve
29882991
// Notify that old coins are spent
29892992
for (const CTxIn& txin : wtxNew.tx->vin)
29902993
{
2991-
CWalletTx &coin = mapWallet[txin.prevout.hash];
2994+
CWalletTx &coin = mapWallet.at(txin.prevout.hash);
29922995
coin.BindWallet(this);
29932996
NotifyTransactionChanged(this, coin.GetHash(), CT_UPDATED);
29942997
}
@@ -3523,7 +3526,7 @@ std::set< std::set<CTxDestination> > CWallet::GetAddressGroupings()
35233526
CTxDestination address;
35243527
if(!IsMine(txin)) /* If this input isn't mine, ignore it */
35253528
continue;
3526-
if(!ExtractDestination(mapWallet[txin.prevout.hash].tx->vout[txin.prevout.n].scriptPubKey, address))
3529+
if(!ExtractDestination(mapWallet.at(txin.prevout.hash).tx->vout[txin.prevout.n].scriptPubKey, address))
35273530
continue;
35283531
grouping.insert(address);
35293532
any_mine = true;

‎src/wallet/wallet.h‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -340,11 +340,6 @@ class CWalletTx : public CMerkleTx
340340
mutable CAmount nAvailableWatchCreditCached;
341341
mutable CAmount nChangeCached;
342342

343-
CWalletTx()
344-
{
345-
Init(nullptr);
346-
}
347-
348343
CWalletTx(const CWallet* pwalletIn, CTransactionRef arg) : CMerkleTx(std::move(arg))
349344
{
350345
Init(pwalletIn);

‎src/wallet/walletdb.cpp‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -270,7 +270,7 @@ ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue,
270270
{
271271
uint256 hash;
272272
ssKey >> hash;
273-
CWalletTx wtx;
273+
CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef());
274274
ssValue >> wtx;
275275
CValidationState state;
276276
if (!(CheckTransaction(wtx, state) && (wtx.GetHash() == hash) && state.IsValid()))
@@ -606,7 +606,7 @@ DBErrors CWalletDB::LoadWallet(CWallet* pwallet)
606606
pwallet->UpdateTimeFirstKey(1);
607607

608608
for (uint256 hash : wss.vWalletUpgrade)
609-
WriteTx(pwallet->mapWallet[hash]);
609+
WriteTx(pwallet->mapWallet.at(hash));
610610

611611
// Rewrite encrypted wallets of versions 0.4.0 and 0.5.0rc:
612612
if (wss.fIsEncrypted && (wss.nFileVersion == 40000 || wss.nFileVersion == 50000))
@@ -668,7 +668,7 @@ DBErrors CWalletDB::FindWalletTx(std::vector<uint256>& vTxHash, std::vector<CWal
668668
uint256 hash;
669669
ssKey >> hash;
670670

671-
CWalletTx wtx;
671+
CWalletTx wtx(nullptr /* pwallet */, MakeTransactionRef());
672672
ssValue >> wtx;
673673

674674
vTxHash.push_back(hash);

0 commit comments

Comments
 (0)