fuzz: make `AddCoins` query view for overwrites
What the developer wrote
Authored by Lőrinc
fuzz: make `AddCoins` query view for overwrites
In validation, `AddCoins(check_for_overwrite=false)` is only used after BIP30 has already ensured the transaction does not overwrite any unspent outputs in the UTXO view.
The coins view fuzz target can call `AddCoins` with arbitrary txids, so using the `check_for_overwrite=false` fast path on non-coinbase transactions may violate the `AddCoin` caller contract and trigger logic errors.
Only use `check_for_overwrite=false` when we have first confirmed that none of the outputs are currently unspent.
Otherwise, fall back to `check_for_overwrite=true` so `AddCoins` determines overwrites via the view.
What changed, and why it matters
This is a fix inside a Bitcoin Core fuzz test, not in the live network code. The fuzz test randomly feeds data into coin-handling functions to find crashes. Previously, the test could call AddCoins in a way that breaks an internal rule (overwriting an unspent coin while telling the function not to check for overwrites), causing assertion failures or logic errors. The patch makes the fuzz test check the coin view first, so it only uses the fast 'no overwrite check' path when it is actually safe. It does not change how real Bitcoin nodes validate transactions.
No urgent action for node operators. This is a test-only improvement. Developers should ensure fuzz targets continue to cover both AddCoins code paths and that the new PeekCoin loop does not materially slow fuzzing. Consider whether the same precondition should be documented more explicitly in AddCoins/AddCoin callers.
Security signals we found
Fix is in a fuzz test harness, not production code
Removes expected logic_error/exception path from test
Adds view query (PeekCoin) before AddCoins to satisfy caller precondition
Caller-contract violation could previously trigger assertion or std::logic_error
No change to consensus validation, BIP30, or AddCoins implementation
Evidence from the diff
The change is confined to src/test/fuzz/coins_view.cpp. In validation, AddCoins(check_for_overwrite=false) is safe only after BIP30 has guaranteed no unspent outputs will be overwritten. The fuzz target can construct arbitrary txids, so it could violate that caller contract and hit the logic_error ‘Attempted to overwrite an unspent coin (when possible_overwrite is false)’ or an assertion. The patch replaces the previous try/catch/expect-failure logic with a proactive query: for non-coinbase transactions it scans the outputs via PeekCoin; if any output is currently unspent it forces check_for_overwrite=true, otherwise it may still use the fuzzer’s bool. Coinbase transactions keep the original fast path because their txids are unique enough to avoid this scenario. No consensus, P2P, wallet, or mempool code is modified.
Changed components
src/test/fuzz/coins_view.cppInspect captured patch +6 / −11
diff --git a/src/test/fuzz/coins_view.cpp b/src/test/fuzz/coins_view.cpp
index 47a0144f..d4bdb8a1 100644
--- a/src/test/fuzz/coins_view.cpp
+++ b/src/test/fuzz/coins_view.cpp
@@ -280,19 +280,14 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsViewCache& co
// coins.cpp:69: void CCoinsViewCache::AddCoin(const COutPoint &, Coin &&, bool): Assertion `!coin.IsSpent()' failed.
return;
}
- bool expected_code_path = false;
const int height{int(fuzzed_data_provider.ConsumeIntegral<uint32_t>() >> 1)};
- const bool possible_overwrite = fuzzed_data_provider.ConsumeBool();
- try {
- AddCoins(coins_view_cache, transaction, height, possible_overwrite);
- expected_code_path = true;
- } catch (const std::logic_error& e) {
- if (e.what() == std::string{"Attempted to overwrite an unspent coin (when possible_overwrite is false)"}) {
- assert(!possible_overwrite);
- expected_code_path = true;
+ const bool check_for_overwrite{transaction.IsCoinBase() || [&] {
+ for (uint32_t i{0}; i < transaction.vout.size(); ++i) {
+ if (coins_view_cache.PeekCoin(COutPoint{transaction.GetHash(), i})) return true;
}
- }
- assert(expected_code_path);
+ return fuzzed_data_provider.ConsumeBool();
+ }()}; // We can only skip the check if the current txid has no unspent outputs
+ AddCoins(coins_view_cache, transaction, height, check_for_overwrite);
},
[&] {
(void)AreInputsStandard(CTransaction{random_mutable_transaction}, coins_view_cache);
Why this scored 17/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.