coins: replace remaining `HaveCoin` calls with `GetCoin` #34320

l0rinc · 2026-01-16T16:17:29Z

Problem

We have two ways to check a coin exists in the db. One tries to deserialize, the other just returns whether any value is associated with the key. But we usually need the full value we probed for, and HaveCoin cannot cache the returned value, so it can trigger an extra lookup if HaveCoin uses Exists and the caller later needs the actual value.
We're already delegating most HaveCoin calls to GetCoin anyway for the above reasons, so the current implementation of HaveCoin is redundant.

Fix

Replace HaveCoin calls with GetCoin, reuse ReadRaw in dbwrapper, stop ignoring deserialization failures, and readjust tests to this simpler state.

See

bitcoin/src/dbwrapper.cpp

Lines 304 to 331 in cd0959c

    
           std::optional<std::string> CDBWrapper::ReadImpl(std::span<const std::byte> key) const 
        
           { 
        
               leveldb::Slice slKey(CharCast(key.data()), key.size()); 
        
               std::string strValue; 
        
               leveldb::Status status = DBContext().pdb->Get(DBContext().readoptions, slKey, &strValue); 
        
               if (!status.ok()) { 
        
                   if (status.IsNotFound()) 
        
                       return std::nullopt; 
        
                   LogError("LevelDB read failure: %s", status.ToString()); 
        
                   HandleError(status); 
        
               } 
        
               return strValue; 
        
           } 
        
           bool CDBWrapper::ExistsImpl(std::span<const std::byte> key) const 
        
           { 
        
               leveldb::Slice slKey(CharCast(key.data()), key.size()); 
        
               std::string strValue; 
        
               leveldb::Status status = DBContext().pdb->Get(DBContext().readoptions, slKey, &strValue); 
        
               if (!status.ok()) { 
        
                   if (status.IsNotFound()) 
        
                       return false; 
        
                   LogError("LevelDB read failure: %s", status.ToString()); 
        
                   HandleError(status); 
        
               } 
        
               return true; 
        
           }

and https://github.com/bitcoin/bitcoin/blob/
a9b7f56/src/dbwrapper.h#L206-L241.

Notes

It seems Implement CCoinsViewErrorCatcher::HaveCoin and check disk space periodically #26331 did not fix Running out of disk space can leave bitcoin in a desynced state #26112.
After this PR deserialization failures will crash the app, see refactor: Let CCoinsViewCache::BatchWrite return void #33866 for similar cleanups. This is deliberate to fail early instead of trying to recover.

Add unit tests covering `CCoinsViewCache::HaveInputs()`. The tests document that `HaveInputs()` consults the cache first and that a cache miss pulls from the backing view via `GetCoin()`.

When `CDBWrapper::Read()` fails deserialization we silently return `false`, which makes decode failures indistinguishable from missing keys. Drop the catch so deserialization errors fail fast. This is in line with other recent changes where we're not trying to recover from database failures - and to make this explicit, see: bitcoin#33866

`CDBWrapper::Exists()` and `::ReadRaw()` both issue a LevelDB `::Get`. Introduce a small helper returning the raw value bytes (as `std::string`) for a given key. Move `Exists` closer to the read methods.

Simplify existence checks in coins-related tests. Separated this from the production code changes since the usages here are just for convenience, not correctness.

… the former `HaveCoin` duplicated `GetCoin()` semantics and added another confusing UTXO API surface.

DrahtBot · 2026-01-16T16:17:39Z

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34320.

Reviews

See the guideline for information on the review process.
A summary of reviews will appear here.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#34165 (coins: don't mutate main cache when connecting block by andrewtoth)
#34132 (refactor: inline CCoinsViewErrorCatcher into CCoinsViewDB by l0rinc)
#34124 (refactor: make CCoinsView a purely virtual abstract base class by l0rinc)
#32427 ((RFC) kernel: Replace leveldb-based BlockTreeDB with flat-file based store by sedited)
#32317 (kernel: Separate UTXO set access from validation functions by sedited)
#31132 (validation: fetch block inputs on parallel threads 3x faster IBD by andrewtoth)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

LLM Linter (✨ experimental)

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

AddCoin(outp, std::move(coin), false) in src/test/coins_tests.cpp
AddCoin(outp, std::move(coin), false) in src/test/coins_tests.cpp

^2026-01-16

l0rinc added 5 commits January 16, 2026 14:52

test: add HaveInputs call-path unit tests

0fbda20

Add unit tests covering `CCoinsViewCache::HaveInputs()`. The tests document that `HaveInputs()` consults the cache first and that a cache miss pulls from the backing view via `GetCoin()`.

dbwrapper: have Read and Exists reuse ReadRaw

7d8280a

`CDBWrapper::Exists()` and `::ReadRaw()` both issue a LevelDB `::Get`. Introduce a small helper returning the raw value bytes (as `std::string`) for a given key. Move `Exists` closer to the read methods.

test: use GetCoin in coins cache tests

fe8df84

Simplify existence checks in coins-related tests. Separated this from the production code changes since the usages here are just for convenience, not correctness.

coins: replace remaining HaveCoin calls with GetCoin() and remove…

9b4e044

… the former `HaveCoin` duplicated `GetCoin()` semantics and added another confusing UTXO API surface.

DrahtBot added the UTXO Db and Indexes label Jan 16, 2026

This was referenced Jan 16, 2026

refactor: inline CCoinsViewErrorCatcher into CCoinsViewDB #34132

Open

Coins Cache Cleanup checklist #34280

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

coins: replace remaining `HaveCoin` calls with `GetCoin` #34320

coins: replace remaining `HaveCoin` calls with `GetCoin` #34320

l0rinc commented Jan 16, 2026

Uh oh!

DrahtBot commented Jan 16, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	std::optional<std::string> CDBWrapper::ReadImpl(std::span<const std::byte> key) const
	{
	leveldb::Slice slKey(CharCast(key.data()), key.size());
	std::string strValue;
	leveldb::Status status = DBContext().pdb->Get(DBContext().readoptions, slKey, &strValue);
	if (!status.ok()) {
	if (status.IsNotFound())
	return std::nullopt;
	LogError("LevelDB read failure: %s", status.ToString());
	HandleError(status);
	}
	return strValue;
	}

	bool CDBWrapper::ExistsImpl(std::span<const std::byte> key) const
	{
	leveldb::Slice slKey(CharCast(key.data()), key.size());

	std::string strValue;
	leveldb::Status status = DBContext().pdb->Get(DBContext().readoptions, slKey, &strValue);
	if (!status.ok()) {
	if (status.IsNotFound())
	return false;
	LogError("LevelDB read failure: %s", status.ToString());
	HandleError(status);
	}
	return true;
	}

coins: replace remaining HaveCoin calls with GetCoin #34320

Are you sure you want to change the base?

coins: replace remaining HaveCoin calls with GetCoin #34320

Conversation

l0rinc commented Jan 16, 2026

Problem

Fix

Notes

Uh oh!

DrahtBot commented Jan 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

coins: replace remaining `HaveCoin` calls with `GetCoin` #34320

coins: replace remaining `HaveCoin` calls with `GetCoin` #34320

DrahtBot commented Jan 16, 2026 •

edited

Loading