wallet: fix, make 'total_effective_amount' optional actually optional
What the developer wrote
Authored by stratospher
wallet: fix, make 'total_effective_amount' optional actually optional
this is not needed for the remaining commits but good to fix
and came up in #25269 review.
Co-authored-by: furszy <matiasfurszyfer@protonmail.com>
What changed, and why it matters
This is a small code cleanup in Bitcoin Core's wallet coin-selection logic. The change makes a cached optional value behave as actually optional (unset by default) and only updated when it has a value. It fixes a logic inconsistency found during code review, but there is no direct evidence it prevents a real-world exploit or user-visible bug on its own.
Treat as a normal code-quality/defensive fix. No urgent security action is indicated by the commit alone. If auditing the broader PR #25269 area, verify that callers of GetEffectiveTotalAmount() correctly handle std::nullopt now that the default state changed.
Security signals we found
Logic correction in optional value handling
Defensive null-check before dereferencing optional
No explicit security claim in commit message or diff
No advisory, CVE, or exploit references present
Evidence from the diff
In src/wallet/spend.h, total_effective_amount is changed from an optional initialized to 0 to an optional left unset (std::nullopt). In src/wallet/spend.cpp, Erase() now checks total_effective_amount.has_value() before subtracting from it. Previously the optional always held a value, so the ‘optional’ semantics were not truly optional. The patch aligns behavior with the type’s intent and avoids subtracting effective values when the cache is not populated. The commit message says it is not needed for later commits but is a good fix that arose during PR #25269 review.
Changed components
Bitcoin Core walletsrc/wallet/spend.cppsrc/wallet/spend.hCoinsResult coin cache / coin selectionInspect captured patch +3 / −3
diff --git a/src/wallet/spend.cpp b/src/wallet/spend.cpp
index c7c41f5d..1e0ac8f0 100644
--- a/src/wallet/spend.cpp
+++ b/src/wallet/spend.cpp
@@ -223,7 +223,7 @@ void CoinsResult::Erase(const std::unordered_set<COutPoint, SaltedOutpointHasher
// update cached amounts
total_amount -= coin.txout.nValue;
- if (coin.HasEffectiveValue()) total_effective_amount = *total_effective_amount - coin.GetEffectiveValue();
+ if (coin.HasEffectiveValue() && total_effective_amount.has_value()) total_effective_amount = *total_effective_amount - coin.GetEffectiveValue();
return true;
});
vec.erase(remove_it, vec.end());
diff --git a/src/wallet/spend.h b/src/wallet/spend.h
index a22499f3..5a1a879a 100644
--- a/src/wallet/spend.h
+++ b/src/wallet/spend.h
@@ -59,13 +59,13 @@ struct CoinsResult {
void Add(OutputType type, const COutput& out);
CAmount GetTotalAmount() { return total_amount; }
- std::optional<CAmount> GetEffectiveTotalAmount() {return total_effective_amount; }
+ std::optional<CAmount> GetEffectiveTotalAmount() { return total_effective_amount; }
private:
/** Sum of all available coins raw value */
CAmount total_amount{0};
/** Sum of all available coins effective value (each output value minus fees required to spend it) */
- std::optional<CAmount> total_effective_amount{0};
+ std::optional<CAmount> total_effective_amount;
};
struct CoinFilterParams {
Why this scored 30/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.