Skip to content

Commit

Permalink
Merge bitcoin#11039: Avoid second mapWallet lookup
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@laanwj @PastaPastaPasta
laanwj authored and PastaPastaPasta committed Jan 2, 2020
1 parent ca63ad3 commit a634264
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 27 deletions.
7 changes: 4 additions & 3 deletions src/qt/walletmodel.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -669,10 +669,11 @@ void WalletModel::getOutputs(const std::vector<COutPoint>& vOutpoints, std::vect
LOCK2(cs_main, wallet->cs_wallet);
for (const COutPoint& outpoint : vOutpoints)
{
if (!wallet->mapWallet.count(outpoint.hash)) continue;
int nDepth = wallet->mapWallet[outpoint.hash].GetDepthInMainChain();
auto it = wallet->mapWallet.find(outpoint.hash);
if (it == wallet->mapWallet.end()) continue;
int nDepth = it->second.GetDepthInMainChain();
if (nDepth < 0) continue;
COutput out(&wallet->mapWallet[outpoint.hash], outpoint.n, nDepth, true /* spendable */, true /* solvable */, true /* safe */);
COutput out(&it->second, outpoint.n, nDepth, true /* spendable */, true /* solvable */, true /* safe */);
vOutputs.push_back(out);
}
}
Expand Down
10 changes: 6 additions & 4 deletions src/wallet/rpcwallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1843,10 +1843,11 @@ UniValue listsinceblock(const JSONRPCRequest& request)
throw JSONRPCError(RPC_INTERNAL_ERROR, "Can't read block from disk");
}
for (const CTransactionRef& tx : block.vtx) {
if (pwallet->mapWallet.count(tx->GetHash()) > 0) {
auto it = pwallet->mapWallet.find(tx->GetHash());
if (it != pwallet->mapWallet.end()) {
// We want all transactions regardless of confirmation count to appear here,
// even negative confirmation ones, hence the big negative.
ListTransactions(pwallet, pwallet->mapWallet[tx->GetHash()], "*", -100000000, true, removed, filter);
ListTransactions(pwallet, it->second, "*", -100000000, true, removed, filter);
}
}
paltindex = paltindex->pprev;
Expand Down Expand Up @@ -1927,10 +1928,11 @@ UniValue gettransaction(const JSONRPCRequest& request)
filter = filter | ISMINE_WATCH_ONLY;

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

CAmount nCredit = wtx.GetCredit(filter);
CAmount nDebit = wtx.GetDebit(filter);
Expand Down
49 changes: 29 additions & 20 deletions src/wallet/wallet.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -791,8 +791,9 @@ void CWallet::AddToSpends(const COutPoint& outpoint, const uint256& wtxid)

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

Expand Down Expand Up @@ -1194,8 +1195,9 @@ bool CWallet::LoadToWallet(const CWalletTx& wtxIn)
wtxOrdered.insert(std::make_pair(wtx.nOrderPos, TxPair(&wtx, (CAccountingEntry*)0)));
AddToSpends(hash);
for (const CTxIn& txin : wtx.tx->vin) {
if (mapWallet.count(txin.prevout.hash)) {
CWalletTx& prevtx = mapWallet[txin.prevout.hash];
auto it = mapWallet.find(txin.prevout.hash);
if (it != mapWallet.end()) {
CWalletTx& prevtx = it->second;
if (prevtx.nIndex == -1 && !prevtx.hashUnset()) {
MarkConflicted(prevtx.hashBlock, wtx.GetHash());
}
Expand Down Expand Up @@ -1294,8 +1296,9 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
std::set<uint256> done;

// Can't mark abandoned if confirmed or in mempool
assert(mapWallet.count(hashTx));
CWalletTx& origtx = mapWallet[hashTx];
auto it = mapWallet.find(hashTx);
assert(it != mapWallet.end());
CWalletTx& origtx = it->second;
if (origtx.GetDepthInMainChain() > 0 || origtx.InMempool() || origtx.IsLockedByInstantSend()) {
return false;
}
Expand All @@ -1306,8 +1309,9 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
uint256 now = *todo.begin();
todo.erase(now);
done.insert(now);
assert(mapWallet.count(now));
CWalletTx& wtx = mapWallet[now];
auto it = mapWallet.find(now);
assert(it != mapWallet.end());
CWalletTx& wtx = it->second;
int currentconfirm = wtx.GetDepthInMainChain();
// If the orig tx was not in block, none of its spends can be
assert(currentconfirm <= 0);
Expand All @@ -1332,8 +1336,10 @@ bool CWallet::AbandonTransaction(const uint256& hashTx)
// available of the outputs it spends. So force those to be recomputed
for (const CTxIn& txin : wtx.tx->vin)
{
if (mapWallet.count(txin.prevout.hash))
mapWallet[txin.prevout.hash].MarkDirty();
auto it = mapWallet.find(txin.prevout.hash);
if (it != mapWallet.end()) {
it->second.MarkDirty();
}
}
}
}
Expand Down Expand Up @@ -1374,8 +1380,9 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
uint256 now = *todo.begin();
todo.erase(now);
done.insert(now);
assert(mapWallet.count(now));
CWalletTx& wtx = mapWallet[now];
auto it = mapWallet.find(now);
assert(it != mapWallet.end());
CWalletTx& wtx = it->second;
int currentconfirm = wtx.GetDepthInMainChain();
if (conflictconfirms < currentconfirm) {
// Block is 'more conflicted' than current confirm; update.
Expand All @@ -1394,10 +1401,11 @@ void CWallet::MarkConflicted(const uint256& hashBlock, const uint256& hashTx)
}
// If a transaction changes 'conflicted' state, that changes the balance
// available of the outputs it spends. So force those to be recomputed
for (const CTxIn& txin : wtx.tx->vin)
{
if (mapWallet.count(txin.prevout.hash))
mapWallet[txin.prevout.hash].MarkDirty();
for (const CTxIn& txin : wtx.tx->vin) {
auto it = mapWallet.find(txin.prevout.hash);
if (it != mapWallet.end()) {
it->second.MarkDirty();
}
}
}
}
Expand All @@ -1415,10 +1423,11 @@ void CWallet::SyncTransaction(const CTransactionRef& ptx, const CBlockIndex *pin
// If a transaction changes 'conflicted' state, that changes the balance
// available of the outputs it spends. So force those to be
// recomputed, also:
for (const CTxIn& txin : tx.vin)
{
if (mapWallet.count(txin.prevout.hash))
mapWallet[txin.prevout.hash].MarkDirty();
for (const CTxIn& txin : tx.vin) {
auto it = mapWallet.find(txin.prevout.hash);
if (it != mapWallet.end()) {
it->second.MarkDirty();
}
}

fAnonymizableTallyCached = false;
Expand Down

0 comments on commit a634264

Please sign in to comment.