Skip to content

Commit d1e4265

Browse files
achow101achow101
committed
Merge bitcoin/bitcoin#25543: wallet: cleanup cached amount and input mine check code
47ea70f wallet: clean AllInputsMine code, use InputIsMine internally (furszy) bf310b0 wallet: clean InputIsMine code, use GetWalletTx (furszy) 0cb1772 wallet: unify CachedTxGetImmatureCredit and CachedTxGetImmatureWatchOnlyCredit (furszy) 04c6423 wallet: remove always true 'fUseCache' arg from CachedTxGetAvailableCredit (furszy) 4f0ca9b wallet: remove always false 'recalculate' arg from GetCachableAmount (furszy) 47b1012 wallet: remove always true 'fUseCache' from CachedTxGetImmatureWatchOnlyCredit (furszy) da8f62d wallet: remove always true 'fUseCache' from CachedTxGetImmatureCredit (furszy) Pull request description: Another wallet's code garbage collector work. Part of the `mapWallet` encapsulation goal. Focused on the following points: 1) Remove always true `fUseCache` argument from `CachedTxGetImmatureCredit`, `CachedTxGetImmatureWatchOnly` and `CachedTxGetAvailableCredit`. 2) Remove always false `recalculate` argument from `GetCachableAmount`. 3) Merge `CachedTxGetImmatureCredit` and `CachedTxGetImmatureWatchOnlyCredit` as they do share the exact same code. 4) Clean `InputIsMine` method; use `GetWalletTx` instead of access the wallet's map directly. 5) Clean `AllInputsMine` method; use `InputIsMine` instead of duplicate the exact same code internally. ACKs for top commit: aureleoules: re-ACK 47ea70f achow101: ACK 47ea70f theStack: re-ACK 47ea70f Tree-SHA512: e9b64b57de7be6165c5e5552e28cd8a03d4736b0a3707d29d129e3a0a3db6a855c2abf47a24917236060835a297b564a97b66d4c8b178d6bdafb93a12a7c0b40
2 parents d67f89b + 47ea70f commit d1e4265

File tree

3 files changed

+20
-48
lines changed

3 files changed

+20
-48
lines changed

‎src/wallet/receive.cpp‎

Lines changed: 16 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -9,36 +9,21 @@
99
#include <wallet/wallet.h>
1010

1111
namespace wallet {
12-
isminetype InputIsMine(const CWallet& wallet, const CTxIn &txin)
12+
isminetype InputIsMine(const CWallet& wallet, const CTxIn& txin)
1313
{
1414
AssertLockHeld(wallet.cs_wallet);
15-
std::map<uint256, CWalletTx>::const_iterator mi = wallet.mapWallet.find(txin.prevout.hash);
16-
if (mi != wallet.mapWallet.end())
17-
{
18-
const CWalletTx& prev = (*mi).second;
19-
if (txin.prevout.n < prev.tx->vout.size())
20-
return wallet.IsMine(prev.tx->vout[txin.prevout.n]);
15+
const CWalletTx* prev = wallet.GetWalletTx(txin.prevout.hash);
16+
if (prev && txin.prevout.n < prev->tx->vout.size()) {
17+
return wallet.IsMine(prev->tx->vout[txin.prevout.n]);
2118
}
2219
return ISMINE_NO;
2320
}
2421

2522
bool AllInputsMine(const CWallet& wallet, const CTransaction& tx, const isminefilter& filter)
2623
{
2724
LOCK(wallet.cs_wallet);
28-
29-
for (const CTxIn& txin : tx.vin)
30-
{
31-
auto mi = wallet.mapWallet.find(txin.prevout.hash);
32-
if (mi == wallet.mapWallet.end())
33-
return false; // any unknown inputs can't be from us
34-
35-
const CWalletTx& prev = (*mi).second;
36-
37-
if (txin.prevout.n >= prev.tx->vout.size())
38-
return false; // invalid input!
39-
40-
if (!(wallet.IsMine(prev.tx->vout[txin.prevout.n]) & filter))
41-
return false;
25+
for (const CTxIn& txin : tx.vin) {
26+
if (!(InputIsMine(wallet, txin) & filter)) return false;
4227
}
4328
return true;
4429
}
@@ -111,10 +96,10 @@ CAmount TxGetChange(const CWallet& wallet, const CTransaction& tx)
11196
return nChange;
11297
}
11398

114-
static CAmount GetCachableAmount(const CWallet& wallet, const CWalletTx& wtx, CWalletTx::AmountType type, const isminefilter& filter, bool recalculate = false)
99+
static CAmount GetCachableAmount(const CWallet& wallet, const CWalletTx& wtx, CWalletTx::AmountType type, const isminefilter& filter)
115100
{
116101
auto& amount = wtx.m_amounts[type];
117-
if (recalculate || !amount.m_cached[filter]) {
102+
if (!amount.m_cached[filter]) {
118103
amount.Set(filter, type == CWalletTx::DEBIT ? wallet.GetDebit(*wtx.tx, filter) : TxGetCredit(wallet, *wtx.tx, filter));
119104
wtx.m_is_cache_empty = false;
120105
}
@@ -160,29 +145,18 @@ CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx)
160145
return wtx.nChangeCached;
161146
}
162147

163-
CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache)
164-
{
165-
AssertLockHeld(wallet.cs_wallet);
166-
167-
if (wallet.IsTxImmatureCoinBase(wtx) && wallet.IsTxInMainChain(wtx)) {
168-
return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, ISMINE_SPENDABLE, !fUseCache);
169-
}
170-
171-
return 0;
172-
}
173-
174-
CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache)
148+
CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
175149
{
176150
AssertLockHeld(wallet.cs_wallet);
177151

178152
if (wallet.IsTxImmatureCoinBase(wtx) && wallet.IsTxInMainChain(wtx)) {
179-
return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, ISMINE_WATCH_ONLY, !fUseCache);
153+
return GetCachableAmount(wallet, wtx, CWalletTx::IMMATURE_CREDIT, filter);
180154
}
181155

182156
return 0;
183157
}
184158

185-
CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache, const isminefilter& filter)
159+
CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
186160
{
187161
AssertLockHeld(wallet.cs_wallet);
188162

@@ -193,7 +167,7 @@ CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx,
193167
if (wallet.IsTxImmatureCoinBase(wtx))
194168
return 0;
195169

196-
if (fUseCache && allow_cache && wtx.m_amounts[CWalletTx::AVAILABLE_CREDIT].m_cached[filter]) {
170+
if (allow_cache && wtx.m_amounts[CWalletTx::AVAILABLE_CREDIT].m_cached[filter]) {
197171
return wtx.m_amounts[CWalletTx::AVAILABLE_CREDIT].m_value[filter];
198172
}
199173

@@ -328,8 +302,8 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse)
328302
const CWalletTx& wtx = entry.second;
329303
const bool is_trusted{CachedTxIsTrusted(wallet, wtx, trusted_parents)};
330304
const int tx_depth{wallet.GetTxDepthInMainChain(wtx)};
331-
const CAmount tx_credit_mine{CachedTxGetAvailableCredit(wallet, wtx, /*fUseCache=*/true, ISMINE_SPENDABLE | reuse_filter)};
332-
const CAmount tx_credit_watchonly{CachedTxGetAvailableCredit(wallet, wtx, /*fUseCache=*/true, ISMINE_WATCH_ONLY | reuse_filter)};
305+
const CAmount tx_credit_mine{CachedTxGetAvailableCredit(wallet, wtx, ISMINE_SPENDABLE | reuse_filter)};
306+
const CAmount tx_credit_watchonly{CachedTxGetAvailableCredit(wallet, wtx, ISMINE_WATCH_ONLY | reuse_filter)};
333307
if (is_trusted && tx_depth >= min_depth) {
334308
ret.m_mine_trusted += tx_credit_mine;
335309
ret.m_watchonly_trusted += tx_credit_watchonly;
@@ -338,8 +312,8 @@ Balance GetBalance(const CWallet& wallet, const int min_depth, bool avoid_reuse)
338312
ret.m_mine_untrusted_pending += tx_credit_mine;
339313
ret.m_watchonly_untrusted_pending += tx_credit_watchonly;
340314
}
341-
ret.m_mine_immature += CachedTxGetImmatureCredit(wallet, wtx);
342-
ret.m_watchonly_immature += CachedTxGetImmatureWatchOnlyCredit(wallet, wtx);
315+
ret.m_mine_immature += CachedTxGetImmatureCredit(wallet, wtx, ISMINE_SPENDABLE);
316+
ret.m_watchonly_immature += CachedTxGetImmatureCredit(wallet, wtx, ISMINE_WATCH_ONLY);
343317
}
344318
}
345319
return ret;

‎src/wallet/receive.h‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -29,11 +29,9 @@ CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const ism
2929
//! filter decides which addresses will count towards the debit
3030
CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter);
3131
CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx);
32-
CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true)
32+
CAmount CachedTxGetImmatureCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
3333
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
34-
CAmount CachedTxGetImmatureWatchOnlyCredit(const CWallet& wallet, const CWalletTx& wtx, const bool fUseCache = true)
35-
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
36-
CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, bool fUseCache = true, const isminefilter& filter = ISMINE_SPENDABLE)
34+
CAmount CachedTxGetAvailableCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter = ISMINE_SPENDABLE)
3735
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
3836
struct COutputEntry
3937
{

‎src/wallet/test/wallet_tests.cpp‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -360,13 +360,13 @@ BOOST_FIXTURE_TEST_CASE(coin_mark_dirty_immature_credit, TestChain100Setup)
360360

361361
// Call GetImmatureCredit() once before adding the key to the wallet to
362362
// cache the current immature credit amount, which is 0.
363-
BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx), 0);
363+
BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx, ISMINE_SPENDABLE), 0);
364364

365365
// Invalidate the cached value, add the key, and make sure a new immature
366366
// credit amount is calculated.
367367
wtx.MarkDirty();
368368
AddKey(wallet, coinbaseKey);
369-
BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx), 50*COIN);
369+
BOOST_CHECK_EQUAL(CachedTxGetImmatureCredit(wallet, wtx, ISMINE_SPENDABLE), 50*COIN);
370370
}
371371

372372
static int64_t AddTx(ChainstateManager& chainman, CWallet& wallet, uint32_t lockTime, int64_t mockTime, int64_t blockTime)

0 commit comments

Comments
 (0)