Skip to content

Commit fc51565

Browse files
committed
Merge #11039: Avoid second mapWallet lookup
8f2f1e0 wallet: Avoid second mapWallet lookup (João Barbosa) Pull request description: All calls to `mapWallet.count()` have the intent to detect if a `txid` exists and most are followed by a second lookup to retrieve the `CWalletTx`. This PR replaces all `mapWallet.count()` calls with `mapWallet.find()` to avoid the second lookup. Tree-SHA512: 96b7de7f5520ebf789a1aec1949a4e9c74e13683869cee012f717e5be8e51097d068e2347a36e89097c9a89f1ed1a1529db71760dac9b572e36a3e9ac1155f29
2 parents 9e00a62 + 8f2f1e0 commit fc51565

File tree

4 files changed

+44
-31
lines changed

4 files changed

+44
-31
lines changed

src/qt/walletmodel.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -577,10 +577,11 @@ void WalletModel::getOutputs(const std::vector<COutPoint>& vOutpoints, std::vect
577577
LOCK2(cs_main, wallet->cs_wallet);
578578
for (const COutPoint& outpoint : vOutpoints)
579579
{
580-
if (!wallet->mapWallet.count(outpoint.hash)) continue;
581-
int nDepth = wallet->mapWallet[outpoint.hash].GetDepthInMainChain();
580+
auto it = wallet->mapWallet.find(outpoint.hash);
581+
if (it == wallet->mapWallet.end()) continue;
582+
int nDepth = it->second.GetDepthInMainChain();
582583
if (nDepth < 0) continue;
583-
COutput out(&wallet->mapWallet[outpoint.hash], outpoint.n, nDepth, true /* spendable */, true /* solvable */, true /* safe */);
584+
COutput out(&it->second, outpoint.n, nDepth, true /* spendable */, true /* solvable */, true /* safe */);
584585
vOutputs.push_back(out);
585586
}
586587
}

src/wallet/feebumper.cpp

Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,12 @@ CFeeBumper::CFeeBumper(const CWallet *pWallet, const uint256 txidIn, const CCoin
7676
vErrors.clear();
7777
bumpedTxid.SetNull();
7878
AssertLockHeld(pWallet->cs_wallet);
79-
if (!pWallet->mapWallet.count(txid)) {
79+
auto it = pWallet->mapWallet.find(txid);
80+
if (it == pWallet->mapWallet.end()) {
8081
vErrors.push_back("Invalid or non-wallet transaction id");
8182
currentResult = BumpFeeResult::INVALID_ADDRESS_OR_KEY;
8283
return;
8384
}
84-
auto it = pWallet->mapWallet.find(txid);
8585
const CWalletTx& wtx = it->second;
8686

8787
if (!preconditionChecks(pWallet, wtx)) {
@@ -241,12 +241,13 @@ bool CFeeBumper::commit(CWallet *pWallet)
241241
if (!vErrors.empty() || currentResult != BumpFeeResult::OK) {
242242
return false;
243243
}
244-
if (txid.IsNull() || !pWallet->mapWallet.count(txid)) {
244+
auto it = txid.IsNull() ? pWallet->mapWallet.end() : pWallet->mapWallet.find(txid);
245+
if (it == pWallet->mapWallet.end()) {
245246
vErrors.push_back("Invalid or non-wallet transaction id");
246247
currentResult = BumpFeeResult::MISC_ERROR;
247248
return false;
248249
}
249-
CWalletTx& oldWtx = pWallet->mapWallet[txid];
250+
CWalletTx& oldWtx = it->second;
250251

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

src/wallet/rpcwallet.cpp

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1874,10 +1874,11 @@ UniValue listsinceblock(const JSONRPCRequest& request)
18741874
throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk");
18751875
}
18761876
for (const CTransactionRef& tx : block.vtx) {
1877-
if (pwallet->mapWallet.count(tx->GetHash()) > 0) {
1877+
auto it = pwallet->mapWallet.find(tx->GetHash());
1878+
if (it != pwallet->mapWallet.end()) {
18781879
// We want all transactions regardless of confirmation count to appear here,
18791880
// even negative confirmation ones, hence the big negative.
1880-
ListTransactions(pwallet, pwallet->mapWallet[tx->GetHash()], "*", -100000000, true, removed, filter);
1881+
ListTransactions(pwallet, it->second, "*", -100000000, true, removed, filter);
18811882
}
18821883
}
18831884
paltindex = paltindex->pprev;
@@ -1957,10 +1958,11 @@ UniValue gettransaction(const JSONRPCRequest& request)
19571958
filter = filter | ISMINE_WATCH_ONLY;
19581959

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

19651967
CAmount nCredit = wtx.GetCredit(filter);
19661968
CAmount nDebit = wtx.GetDebit(filter);

src/wallet/wallet.cpp

Lines changed: 29 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -623,8 +623,9 @@ void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid)
623623

624624
void CWallet::AddToSpends(const uint256& wtxid)
625625
{
626-
assert(mapWallet.count(wtxid));
627-
CWalletTx& thisTx = mapWallet[wtxid];
626+
auto it = mapWallet.find(wtxid);
627+
assert(it != mapWallet.end());
628+
CWalletTx& thisTx = it->second;
628629
if (thisTx.IsCoinBase()) // Coinbases don't spend anything!
629630
return;
630631

@@ -1007,8 +1008,9 @@ bool CWallet::LoadToWallet(const CWalletTx& wtxIn)
10071008
wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, nullptr)));
10081009
AddToSpends(hash);
10091010
for (const CTxIn& txin : wtx.tx->vin) {
1010-
if (mapWallet.count(txin.prevout.hash)) {
1011-
CWalletTx& prevtx = mapWallet[txin.prevout.hash];
1011+
auto it = mapWallet.find(txin.prevout.hash);
1012+
if (it != mapWallet.end()) {
1013+
CWalletTx& prevtx = it->second;
10121014
if (prevtx.nIndex == -1 && !prevtx.hashUnset()) {
10131015
MarkConflicted(prevtx.hashBlock, wtx.GetHash());
10141016
}
@@ -1107,8 +1109,9 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
11071109
std::set<uint256> done;
11081110

11091111
// Can't mark abandoned if confirmed or in mempool
1110-
assert(mapWallet.count(hashTx));
1111-
CWalletTx& origtx = mapWallet[hashTx];
1112+
auto it = mapWallet.find(hashTx);
1113+
assert(it != mapWallet.end());
1114+
CWalletTx& origtx = it->second;
11121115
if (origtx.GetDepthInMainChain() > 0 || origtx.InMempool()) {
11131116
return false;
11141117
}
@@ -1119,8 +1122,9 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
11191122
uint256 now = *todo.begin();
11201123
todo.erase(now);
11211124
done.insert(now);
1122-
assert(mapWallet.count(now));
1123-
CWalletTx& wtx = mapWallet[now];
1125+
auto it = mapWallet.find(now);
1126+
assert(it != mapWallet.end());
1127+
CWalletTx& wtx = it->second;
11241128
int currentconfirm = wtx.GetDepthInMainChain();
11251129
// If the orig tx was not in block, none of its spends can be
11261130
assert(currentconfirm <= 0);
@@ -1145,8 +1149,10 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
11451149
// available of the outputs it spends. So force those to be recomputed
11461150
for (const CTxIn& txin : wtx.tx->vin)
11471151
{
1148-
if (mapWallet.count(txin.prevout.hash))
1149-
mapWallet[txin.prevout.hash].MarkDirty();
1152+
auto it = mapWallet.find(txin.prevout.hash);
1153+
if (it != mapWallet.end()) {
1154+
it->second.MarkDirty();
1155+
}
11501156
}
11511157
}
11521158
}
@@ -1184,8 +1190,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
11841190
uint256 now = *todo.begin();
11851191
todo.erase(now);
11861192
done.insert(now);
1187-
assert(mapWallet.count(now));
1188-
CWalletTx& wtx = mapWallet[now];
1193+
auto it = mapWallet.find(now);
1194+
assert(it != mapWallet.end());
1195+
CWalletTx& wtx = it->second;
11891196
int currentconfirm = wtx.GetDepthInMainChain();
11901197
if (conflictconfirms < currentconfirm) {
11911198
// Block is 'more conflicted' than current confirm; update.
@@ -1204,10 +1211,11 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
12041211
}
12051212
// If a transaction changes 'conflicted' state, that changes the balance
12061213
// available of the outputs it spends. So force those to be recomputed
1207-
for (const CTxIn& txin : wtx.tx->vin)
1208-
{
1209-
if (mapWallet.count(txin.prevout.hash))
1210-
mapWallet[txin.prevout.hash].MarkDirty();
1214+
for (const CTxIn& txin : wtx.tx->vin) {
1215+
auto it = mapWallet.find(txin.prevout.hash);
1216+
if (it != mapWallet.end()) {
1217+
it->second.MarkDirty();
1218+
}
12111219
}
12121220
}
12131221
}
@@ -1222,10 +1230,11 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pin
12221230
// If a transaction changes 'conflicted' state, that changes the balance
12231231
// available of the outputs it spends. So force those to be
12241232
// recomputed, also:
1225-
for (const CTxIn& txin : tx.vin)
1226-
{
1227-
if (mapWallet.count(txin.prevout.hash))
1228-
mapWallet[txin.prevout.hash].MarkDirty();
1233+
for (const CTxIn& txin : tx.vin) {
1234+
auto it = mapWallet.find(txin.prevout.hash);
1235+
if (it != mapWallet.end()) {
1236+
it->second.MarkDirty();
1237+
}
12291238
}
12301239
}
12311240

0 commit comments

Comments
 (0)