Public commit record
58/100 · Thin
What the developer wrote
Authored by Ava Chow
wallet: Remove ISMINE_USED
This isminetype is not a real isminetype as it is never returned by
IsMine. This is only used for isminefilters in one function, which can
be better represented with a bool parameter avoid_reuse.
✓ Descriptive subject✓ Provides detailed explanatory context
What changed, and why it matters
This is a small internal cleanup in Bitcoin Core's wallet code. It removes a special flag called ISMINE_USED that was only used to decide whether to count already-used addresses when calculating balances. The same behavior is now controlled by a simple true/false parameter called avoid_reuse. There is no direct security bug being fixed here; it is a refactoring change that makes the code easier to understand and maintain.
Recommended action
No immediate action required. Treat as normal code maintenance. Reviewers may want to confirm that all former ISMINE_USED callers now pass the correct avoid_reuse value and that balance reporting remains unchanged, especially for wallets using avoid_reuse.
Security signals we found
01
No security-relevant signal in commit message or diff
02
Refactor only: enum flag removed and replaced with equivalent boolean parameter
03
No changes to cryptographic operations, network protocol, consensus, or transaction validation
04
No bug pattern such as buffer overflow, use-after-free, or missing authorization
Evidence from the diff
The commit refactors wallet balance caching. Previously, ISMINE_USED was overloaded as an isminefilter bitflag to represent the ‘avoid reuse’ balance mode, even though IsMine never returned it. The patch removes ISMINE_USED and ISMINE_ALL_USED from the isminetype enum and replaces the cache indexing by filter with two explicit optional fields: m_avoid_reuse_value and m_all_value. CachedTxGetCredit/Debit gain a bool avoid_reuse parameter. Call sites are updated: MakeWalletTx passes avoid_reuse=true; gettransaction RPC and CachedTxGetAmounts/CachedTxIsFromMe pass false. This is a behavior-preserving refactor with no evident vulnerability fix.
Changed components
src/wallet/types.hsrc/wallet/transaction.hsrc/wallet/receive.cppsrc/wallet/receive.hsrc/wallet/interfaces.cppsrc/wallet/rpc/transactions.cppInspect captured patch +41 / −29
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
index 42ccfae6..07500cc0 100644
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -76,8 +76,8 @@ WalletTx MakeWalletTx(CWallet& wallet, const CWalletTx& wtx)
wallet.IsMine(result.txout_address.back()) :
ISMINE_NO);
}
- result.credit = CachedTxGetCredit(wallet, wtx, ISMINE_ALL);
- result.debit = CachedTxGetDebit(wallet, wtx, ISMINE_ALL);
+ result.credit = CachedTxGetCredit(wallet, wtx, ISMINE_ALL, /*avoid_reuse=*/true);
+ result.debit = CachedTxGetDebit(wallet, wtx, ISMINE_ALL, /*avoid_reuse=*/true);
result.change = CachedTxGetChange(wallet, wtx);
result.time = wtx.GetTxTime();
result.value_map = wtx.mapValue;
diff --git a/src/wallet/receive.cpp b/src/wallet/receive.cpp
index 4c9ffc49..46e52b3f 100644
--- a/src/wallet/receive.cpp
+++ b/src/wallet/receive.cpp
@@ -97,17 +97,17 @@ CAmount TxGetChange(const CWallet& wallet, const CTransaction& tx)
return nChange;
}
-static CAmount GetCachableAmount(const CWallet& wallet, const CWalletTx& wtx, CWalletTx::AmountType type, const isminefilter& filter)
+static CAmount GetCachableAmount(const CWallet& wallet, const CWalletTx& wtx, CWalletTx::AmountType type, const isminefilter& filter, bool avoid_reuse)
{
auto& amount = wtx.m_amounts[type];
- if (!amount.m_cached[filter]) {
- amount.Set(filter, type == CWalletTx::DEBIT ? wallet.GetDebit(*wtx.tx, filter) : TxGetCredit(wallet, *wtx.tx, filter));
+ if (!amount.IsCached(avoid_reuse)) {
+ amount.Set(avoid_reuse, type == CWalletTx::DEBIT ? wallet.GetDebit(*wtx.tx, filter) : TxGetCredit(wallet, *wtx.tx, filter));
wtx.m_is_cache_empty = false;
}
- return amount.m_value[filter];
+ return amount.Get(avoid_reuse);
}
-CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
+CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter, bool avoid_reuse)
{
AssertLockHeld(wallet.cs_wallet);
@@ -119,12 +119,12 @@ CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const ism
const isminefilter get_amount_filter{filter & ISMINE_ALL};
if (get_amount_filter) {
// GetBalance can assume transactions in mapWallet won't change
- credit += GetCachableAmount(wallet, wtx, CWalletTx::CREDIT, get_amount_filter);
+ credit += GetCachableAmount(wallet, wtx, CWalletTx::CREDIT, get_amount_filter, avoid_reuse);
}
return credit;
}
-CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
+CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter, bool avoid_reuse)
{
if (wtx.tx->vin.empty())
return 0;
@@ -132,7 +132,7 @@ CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const ismi
CAmount debit = 0;
const isminefilter get_amount_filter{filter & ISMINE_ALL};
if (get_amount_filter) {
- debit += GetCachableAmount(wallet, wtx, CWalletTx::DEBIT, get_amount_filter);
+ debit += GetCachableAmount(wallet, wtx, CWalletTx::DEBIT, get_amount_filter, avoid_reuse);
}
return debit;
}
@@ -156,7 +156,7 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
listSent.clear();
// Compute fee:
- CAmount nDebit = CachedTxGetDebit(wallet, wtx, filter);
+ CAmount nDebit = CachedTxGetDebit(wallet, wtx, filter, /*avoid_reuse=*/false);
if (nDebit > 0) // debit>0 means we signed/sent this transaction
{
CAmount nValueOut = wtx.tx->GetValueOut();
@@ -205,7 +205,7 @@ void CachedTxGetAmounts(const CWallet& wallet, const CWalletTx& wtx,
bool CachedTxIsFromMe(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
{
- return (CachedTxGetDebit(wallet, wtx, filter) > 0);
+ return (CachedTxGetDebit(wallet, wtx, filter, /*avoid_reuse=*/false) > 0);
}
// NOLINTNEXTLINE(misc-no-recursion)
diff --git a/src/wallet/receive.h b/src/wallet/receive.h
index f129c195..271c1a96 100644
--- a/src/wallet/receive.h
+++ b/src/wallet/receive.h
@@ -25,10 +25,10 @@ bool OutputIsChange(const CWallet& wallet, const CTxOut& txout) EXCLUSIVE_LOCKS_
CAmount OutputGetChange(const CWallet& wallet, const CTxOut& txout) EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
CAmount TxGetChange(const CWallet& wallet, const CTransaction& tx);
-CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter)
+CAmount CachedTxGetCredit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter, bool avoid_reuse)
EXCLUSIVE_LOCKS_REQUIRED(wallet.cs_wallet);
//! filter decides which addresses will count towards the debit
-CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter);
+CAmount CachedTxGetDebit(const CWallet& wallet, const CWalletTx& wtx, const isminefilter& filter, bool avoid_reuse);
CAmount CachedTxGetChange(const CWallet& wallet, const CWalletTx& wtx);
struct COutputEntry
{
diff --git a/src/wallet/rpc/transactions.cpp b/src/wallet/rpc/transactions.cpp
index b58d3dad..8d2c7fb2 100644
--- a/src/wallet/rpc/transactions.cpp
+++ b/src/wallet/rpc/transactions.cpp
@@ -741,8 +741,8 @@ RPCHelpMan gettransaction()
}
const CWalletTx& wtx = it->second;
- CAmount nCredit = CachedTxGetCredit(*pwallet, wtx, filter);
- CAmount nDebit = CachedTxGetDebit(*pwallet, wtx, filter);
+ CAmount nCredit = CachedTxGetCredit(*pwallet, wtx, filter, /*avoid_reuse=*/false);
+ CAmount nDebit = CachedTxGetDebit(*pwallet, wtx, filter, /*avoid_reuse=*/false);
CAmount nNet = nCredit - nDebit;
CAmount nFee = (CachedTxIsFromMe(*pwallet, wtx, filter) ? wtx.tx->GetValueOut() - nDebit : 0);
diff --git a/src/wallet/transaction.h b/src/wallet/transaction.h
index 83c3c660..ec4fb99e 100644
--- a/src/wallet/transaction.h
+++ b/src/wallet/transaction.h
@@ -128,21 +128,38 @@ std::string TxStateString(const T& state)
}
/**
- * Cachable amount subdivided into watchonly and spendable parts.
+ * Cachable amount subdivided into avoid reuse and all balances
*/
struct CachableAmount
{
- // NO and ALL are never (supposed to be) cached
- std::bitset<ISMINE_ENUM_ELEMENTS> m_cached;
- CAmount m_value[ISMINE_ENUM_ELEMENTS];
+ std::optional<CAmount> m_avoid_reuse_value;
+ std::optional<CAmount> m_all_value;
inline void Reset()
{
- m_cached.reset();
+ m_avoid_reuse_value.reset();
+ m_all_value.reset();
}
- void Set(isminefilter filter, CAmount value)
+ void Set(bool avoid_reuse, CAmount value)
{
- m_cached.set(filter);
- m_value[filter] = value;
+ if (avoid_reuse) {
+ m_avoid_reuse_value = value;
+ } else {
+ m_all_value = value;
+ }
+ }
+ CAmount Get(bool avoid_reuse)
+ {
+ if (avoid_reuse) {
+ Assert(m_avoid_reuse_value.has_value());
+ return m_avoid_reuse_value.value();
+ }
+ Assert(m_all_value.has_value());
+ return m_all_value.value();
+ }
+ bool IsCached(bool avoid_reuse)
+ {
+ if (avoid_reuse) return m_avoid_reuse_value.has_value();
+ return m_all_value.has_value();
}
};
diff --git a/src/wallet/types.h b/src/wallet/types.h
index 8d1202dd..066a286a 100644
--- a/src/wallet/types.h
+++ b/src/wallet/types.h
@@ -26,23 +26,18 @@ namespace wallet {
* For LegacyScriptPubKeyMan,
* ISMINE_NO: the scriptPubKey is not in the wallet;
* ISMINE_SPENDABLE: the scriptPubKey corresponds to an address owned by the wallet user (can spend with the private key);
- * ISMINE_USED: the scriptPubKey corresponds to a used address owned by the wallet user;
* ISMINE_ALL: all ISMINE flags except for USED;
- * ISMINE_ALL_USED: all ISMINE flags including USED;
* ISMINE_ENUM_ELEMENTS: the number of isminetype enum elements.
*
* For DescriptorScriptPubKeyMan and future ScriptPubKeyMan,
* ISMINE_NO: the scriptPubKey is not in the wallet;
* ISMINE_SPENDABLE: the scriptPubKey matches a scriptPubKey in the wallet.
- * ISMINE_USED: the scriptPubKey corresponds to a used address owned by the wallet user.
*
*/
enum isminetype : unsigned int {
ISMINE_NO = 0,
ISMINE_SPENDABLE = 1 << 1,
- ISMINE_USED = 1 << 2,
ISMINE_ALL = ISMINE_SPENDABLE,
- ISMINE_ALL_USED = ISMINE_ALL | ISMINE_USED,
ISMINE_ENUM_ELEMENTS,
};
/** used for bitflags of isminetype */
Risk score
Our methodology →Why this scored 18/100
Human-validated context
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
No validated notes yet.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.