coins: remove redundant and confusing `CCoinsViewDB::HaveCoin`

l0rinc commented at 4:17 pm on January 16, 2026: contributor

Problem

We have two ways to check if 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 CCoinsViewDB implementation of HaveCoin is redundant.

Fix

Remove CCoinsViewDB::HaveCoin (falls back to the base CCoinsView::HaveCoin which delegates to GetCoin) and simplify CCoinsViewCache::HaveCoin to use AccessCoin.

Testing

A unit test was added to confirm that HaveInputs calls the database’s ::GetCoin (not ::HaveCoin) on cache miss and checks cache first. Also added a benchmark to track throughput before/after changes to ensure no regression.

Note: the latest version of the change keeps most HaveCoin calls for convenience, a previous version removed all of them.

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

DrahtBot commented at 4:17 pm on January 16, 2026: contributor

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:

#34132 (coins: fail fast on database read deserialization errors by l0rinc)
#34124 (refactor: make CCoinsView a purely virtual abstract base class by l0rinc)
#32317 (kernel: Separate UTXO set access from validation functions by sedited)
#31132 (validation: fetch block inputs on parallel threads 3x faster IBD by andrewtoth)
#30342 (kernel, logging: Pass Logger instances to kernel objects by ryanofsky)

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.

maflcko commented at 3:22 pm on January 20, 2026: member

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.

Are there any flamegraphs or benchmarks, to see how this affects performance?

l0rinc commented at 3:43 pm on January 20, 2026: contributor

Are there any flamegraphs or benchmarks, to see how this affects performance?

I’m not sure what to measure exactly because the current code was already delegating to GetCoin in most places. The pruned IBD and -reindex-chainstate and AssumeUTXO loading benchmarks I started haven’t finished yet - but I don’t expect any change there. If you can describe a scenario that you would find helpful, please let me know.

l0rinc commented at 8:50 pm on January 22, 2026: contributor

Are there any flamegraphs or benchmarks, to see how this affects performance?

Added assumeutxo load, reindex-chainstate and pruned IBD measurements to the PR description

andrewtoth commented at 8:56 pm on January 24, 2026: contributor

I’m not sure what to measure exactly because the current code was already delegating to GetCoin in most places.

I’m not sure about this. CCoinsViewCache::HaveCoin seems to be the implementation that is called in all non-test places outside of coins.cpp, and only delegates to GetCoin if there’s a cache miss. In CCoinsViewCache::HaveCoin we avoid copying Coin:

0bool CCoinsViewCache::HaveCoin(const COutPoint &outpoint) const {
1    CCoinsMap::const_iterator it = FetchCoin(outpoint);
2    return (it != cacheCoins.end() && !it->second.coin.IsSpent());
3}

vs CCoinsViewCache::GetCoin where we return a copy of Coin:

0std::optional<Coin> CCoinsViewCache::GetCoin(const COutPoint& outpoint) const
1{
2    if (auto it{FetchCoin(outpoint)}; it != cacheCoins.end() && !it->second.coin.IsSpent()) return it->second.coin;
3    return std::nullopt;
4}

andrewtoth commented at 9:38 pm on January 24, 2026: contributor

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.

I see 4 uses of HaveCoin

In ConnectBlock, we ensure we do not already have outputs of a block for BIP30 checks.
In ApplyTxInUndo, we ensure we do not already have the coin we are about to roll back to unspent.
In MemPoolAccept::PreChecks, where we check that we have all inputs of a tx we are accepting to the mempool.
In TxVerify, where we call HaveInputs.

For 1 and 2, we are checking for the non-existence of the coin so we won’t need the full value. In this case not caching the value makes sense. For 3, it is explicitly relying on the fact that the value is cached. The backend is thrown away after all input prevouts are added to the top level cache. For 4, it is a peformance improvement that the value is cached.

I think 1 and 2 would benefit from CCoinsViewCache::HaveCoin delegating to the base’s HaveCoin and not going through FetchCoin. For 3 this seems like an abuse of the CCoinsView API. But, calling GetCoin instead there would result in unnecessary copying of the Coin. I’m not sure what the best solution is here. For 4 the current behavior makes sense.

l0rinc commented at 3:28 pm on January 25, 2026: contributor

only delegates to GetCoin if there’s a cache miss. In CCoinsViewCache::HaveCoin we avoid copying Coin

Disassembly with separate translation units indicates GetCoin() does a new + memcpy of the Coin and the caller immediately deletes it, even when only checking .has_value(). HaveCoin() does indeed avoid it, but the extra copy only happens on the unexpectedly-present coin path, since if it’s expected, we usually want to cache it. If we want to avoid the extra copy, we can use AccessCoin, but we would duplicate the spentness check.

In this case not caching the value makes sense.

But there’s nothing to cache since these will return an empty optional. For BIP30 and ApplyTxInUndo the missing-coin path dominates, so GetCoin() short-circuits on Read() failure without deserialization, so I/O is the same as Exists(), no coin-copy happens (and when it does we crash or reorg anyway). But given that validation until bip30-end takes less than 3 minutes on my benchmarking server (see #33817), I’d say it’s irrelevant either way.

For 3 this seems like an abuse of the CCoinsView API. But, calling GetCoin instead there would result in unnecessary copying of the Coin. I’m not sure what the best solution is here.

This is a good point, the disassembly indicates there’s no devirtualization and no inlining in CCoinsViewCache::HaveInputs and MemPoolAccept::PreChecks, so the copy is real - though a lot faster than disk access, so it’s not actually measurable. We can use AccessCoin here and with explicit IsSpent call instead which would avoid the call (according to the assembly):

0if (AccessCoin(tx.vin[i].prevout).IsSpent()) {

and

0if (m_view.AccessCoin(txin.prevout).IsSpent()) {

I have added a new benchmark as a first commit to check the performance of on-disk HaveInputs to make sure the performance is retained.

Benchmark baseline:

ns/tx	tx/s	err%	total	benchmark
998.95	1,001,046.69	0.8%	1.09	`HaveInputsOnDisk`

Benchmark with !GetCoin() instead of HaveCoin:

ns/tx	tx/s	err%	total	benchmark
985.57	1,014,640.63	0.8%	1.10	`HaveInputsOnDisk`

Replace remaining HaveCoin calls with GetCoin()/AccessCoin and remove the former

ns/tx	tx/s	err%	total	benchmark
983.17	1,017,118.55	0.3%	1.10	`HaveInputsOnDisk`

(ran it 5 times for each commit and copied the fastest (with smallest noise). The small difference is reproducible, but not very important as long as it’s not measurably slower)

0for commit in 3544b98927050956f68cb183bf2686f5c65e5ebb 8c568500442144fbcd457eccda5a1956aa98ca81; do \                                        
1    git fetch origin $commit >/dev/null 2>&1 && git checkout $commit >/dev/null 2>&1 && echo "" && git log -1 --pretty='%h %s' && \
2    rm -rf build >/dev/null 2>&1 && cmake -B build -DBUILD_BENCH=ON -DCMAKE_BUILD_TYPE=Release >/dev/null 2>&1 && \
3    cmake --build build -j$(nproc) >/dev/null 2>&1 && \
4    for _ in $(seq 5); do \
5      sleep 5; \
6      ./build/bin/bench_bitcoin -filter='HaveInputsOnDisk' -min-time=1000; \
7    done; \
8done

resulting in

3544b98927 bench: add on-disk HaveInputs benchmark

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,137.20	467,902.92	0.0%	16,874.42	7,673.54	2.199	2,895.01	0.8%	1.09	`HaveInputsOnDisk`

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,153.41	464,380.56	0.0%	16,873.94	7,729.93	2.183	2,894.99	0.8%	1.10	`HaveInputsOnDisk`

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,148.36	465,471.81	0.1%	16,873.92	7,709.74	2.189	2,894.98	0.8%	1.10	`HaveInputsOnDisk`

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,150.10	465,095.33	0.1%	16,873.85	7,718.44	2.186	2,894.99	0.8%	1.10	`HaveInputsOnDisk`

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,135.95	468,175.11	0.0%	16,874.21	7,669.12	2.200	2,894.99	0.8%	1.10	`HaveInputsOnDisk`

8c56850044 coins: replace remaining HaveCoin calls with GetCoin()/AccessCoin and remove the former

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,120.50	471,586.34	0.0%	16,874.75	7,611.58	2.217	2,897.27	0.8%	1.10	`HaveInputsOnDisk`

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,178.86	458,955.50	0.5%	16,875.34	7,821.05	2.158	2,897.29	1.0%	1.08	`HaveInputsOnDisk`

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,153.54	464,352.06	0.0%	16,875.12	7,731.71	2.183	2,897.26	0.8%	1.10	`HaveInputsOnDisk`

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,137.40	467,857.52	0.0%	16,875.16	7,673.32	2.199	2,897.26	0.8%	1.10	`HaveInputsOnDisk`

ns/tx	tx/s	err%	ins/tx	cyc/tx	IPC	bra/tx	miss%	total	benchmark
2,135.08	468,365.96	0.0%	16,875.23	7,663.44	2.202	2,897.27	0.8%	1.11	`HaveInputsOnDisk`

l0rinc force-pushed on Jan 25, 2026

andrewtoth commented at 2:37 am on January 29, 2026: contributor

If we want to avoid the extra copy, we can use AccessCoin, but we would duplicate the spentness check.

This tells us that HaveCoin is the right level of abstraction then, and is a useful method, no?

In this case not caching the value makes sense.

But there’s nothing to cache since these will return an empty optional

Same as above.

We can use AccessCoin here and with explicit IsSpent call instead which would avoid the call (according to the assembly)

I think everywhere we use AccessCoin we assert the unspentness. This would now break that pattern and use AccessCoin to check existence via spentness. I think HaveCoin conveys intent better here.

Can we instead just remove HaveCoin on the db then, and the default implementation would just forward to !!GetCoin for it?

l0rinc renamed this:
~~coins: replace remaining `HaveCoin` calls with `GetCoin`~~
coins: remove redundant and confusing `CCoinsViewDB::HaveCoin`
on Feb 1, 2026

l0rinc commented at 1:38 pm on February 1, 2026: contributor

Thanks @andrewtoth, I didn’t like the suggestion at first since we keep polluting the interface, but after applying your suggestion I’m fine with the results, it’s simpler and better separated from the other similar PRs.

l0rinc force-pushed on Feb 1, 2026

DrahtBot added the label CI failed on Feb 1, 2026

l0rinc force-pushed on Feb 1, 2026

DrahtBot removed the label CI failed on Feb 1, 2026

in src/bench/coins_haveinputs.cpp:22 in 267af2c925 outdated

17+#include <ranges>
18+#include <system_error>
19+
20+static void HaveInputsOnDisk(benchmark::Bench& bench)
21+{
22+    const fs::path path{fs::current_path() / strprintf("bench-%s-%lld", bench.name(), GetTime())};

andrewtoth commented at 6:08 pm on February 1, 2026:

We should use std::filesystem::temp_directory_path() instead of current path, and then we don’t have to cleanup.

l0rinc commented at 9:59 pm on February 1, 2026:

Wouldn’t that store the blocks in memory in tmpfs? Though in reality after the first read it will likely be in cache already…

andrewtoth commented at 10:10 pm on February 1, 2026:

No, it will be on disk. In /tmp on Linux.

l0rinc commented at 10:13 pm on February 1, 2026:

yes, isn’t that using tmpfs?

Many Unix distributions enable and use tmpfs by default for the /tmp branch of the file system or for shared memory.

and

tmpfs (short for temporary file system) is a temporary file storage paradigm implemented in many Unix-like operating systems. It is intended to appear as a mounted file system, but data is stored in volatile memory rather than on a persistent storage device.

l0rinc commented at 5:57 pm on February 3, 2026:

On Mac this will create something like: /var/folders/5t/04gq0pqj5yv4t8cxw51q0s2m0000gn/T/bench-HaveInputsOnDisk-1770141170 - changed.

in src/bench/coins_haveinputs.cpp:40 in 267af2c925 outdated

35+    cache.Flush();
36+
37+    bench.batch(block.vtx.size() - 1).unit("tx").run([&] {
38+        CCoinsViewCache view{&db}; // Recreate to avoid caching
39+        for (auto& tx : block.vtx | std::views::drop(1)) {
40+            if (view.GetCacheSize() % 10 == 0) (void)view.HaveInputs(*tx); // Exercise the cache-hit path as well.

andrewtoth commented at 6:13 pm on February 1, 2026:

Why only every 10th iteration? Why not just do it every time?

l0rinc commented at 10:00 pm on February 1, 2026:

Isn’t it more realistic if some of the data comes from cache and some from disk?

andrewtoth commented at 10:42 pm on February 1, 2026:

Yeah, so if we call twice for every tx then half are coming from cache and half not?

l0rinc commented at 10:55 pm on February 1, 2026:

Not sure what you mean, if you feel strongly about doing both in every step, I don’t mind, just let me know if you think that’s more representative

l0rinc commented at 5:57 pm on February 3, 2026:

Changed it to:

0            assert(view.HaveInputs(*tx));
1            assert(view.HaveInputs(*tx)); // Exercise the cache-hit path as well.

in src/test/coins_tests.cpp:1082 in 6f533a0f3c

1077+    {
1078+    public:
1079+        const COutPoint expected;
1080+        mutable size_t havecoin_calls{0}, getcoin_calls{0};
1081+
1082+        explicit CCoinsViewSpy(CCoinsView* view, COutPoint out) : CCoinsViewBacked(view), expected{std::move(out)} {}

andrewtoth commented at 6:16 pm on February 1, 2026:

There’s nothing to move for a COutPoint. Some code does that erroneously, but we should not.

l0rinc commented at 10:19 pm on February 1, 2026:

Because it’s trivially copyable and we’re in a test? Ok, made it const COutPoint& out like we have in GetCoin: https://github.com/bitcoin/bitcoin/compare/4166ee8f0175a440f461320165ce8cea9207c22e..23dd085872bc0f01309260e00cce239a9512b45e

andrewtoth commented at 10:41 pm on February 1, 2026:

Because there’s nothing to move, so using std::move with it doesn’t make sense.

l0rinc commented at 10:53 pm on February 1, 2026:

Can you please expand on that? It’s not a primitive type, but it is trivially copyable which should likely be similar in speed - is that what you mean?

in src/test/coins_tests.cpp:1083 in 6f533a0f3c outdated

1082+        explicit CCoinsViewSpy(CCoinsView* view, COutPoint out) : CCoinsViewBacked(view), expected{std::move(out)} {}
1083+
1084+        std::optional<Coin> GetCoin(const COutPoint& out) const override
1085+        {
1086+            ++getcoin_calls;
1087+            BOOST_CHECK(out == expected);

andrewtoth commented at 6:17 pm on February 1, 2026:

nit: BOOST_CHECK_EQUAL(out, expected);

l0rinc commented at 10:10 pm on February 1, 2026:

I thought of that, but didn’t want to add a new std::ostream& for COutPoint:

 0diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
 1--- a/src/test/coins_tests.cpp	(revision 4166ee8f0175a440f461320165ce8cea9207c22e)
 2+++ b/src/test/coins_tests.cpp	(date 1769983694317)
 3@@ -105,6 +105,11 @@
 4 
 5 } // namespace
 6 
 7+std::ostream& operator<<(std::ostream& os, const COutPoint& outpoint)
 8+{
 9+    return os << outpoint.ToString();
10+}
11+
12 static const unsigned int NUM_SIMULATION_ITERATIONS = 40000;
13 
14 struct CacheTest : BasicTestingSetup {
15@@ -1084,14 +1089,14 @@
16         std::optional<Coin> GetCoin(const COutPoint& out) const override
17         {
18             ++getcoin_calls;
19-            BOOST_CHECK(out == expected);
20+            BOOST_CHECK_EQUAL(out, expected);
21             return CCoinsViewBacked::GetCoin(out);
22         }
23 
24         bool HaveCoin(const COutPoint& out) const override
25         {
26             ++havecoin_calls;
27-            BOOST_CHECK(out == expected);
28+            BOOST_CHECK_EQUAL(out, expected);
29             return CCoinsViewBacked::HaveCoin(out);
30         }
31     };

in src/test/coins_tests.cpp:1090 in 6f533a0f3c outdated

1089+        }
1090+
1091+        bool HaveCoin(const COutPoint& out) const override
1092+        {
1093+            ++havecoin_calls;
1094+            BOOST_CHECK(out == expected);

andrewtoth commented at 6:17 pm on February 1, 2026:

Same as above.

in src/coins.cpp:172 in 4166ee8f01 outdated

166@@ -167,12 +167,13 @@ const Coin& CCoinsViewCache::AccessCoin(const COutPoint &outpoint) const {
167     }
168 }
169 
170-bool CCoinsViewCache::HaveCoin(const COutPoint &outpoint) const {
171-    CCoinsMap::const_iterator it = FetchCoin(outpoint);
172-    return (it != cacheCoins.end() && !it->second.coin.IsSpent());
173+bool CCoinsViewCache::HaveCoin(const COutPoint& outpoint) const
174+{
175+    return !AccessCoin(outpoint).IsSpent();

andrewtoth commented at 6:24 pm on February 1, 2026:

I’m not sure why we are touching this. This does an extra IsSpent() check now on an empty coin, instead of just short circuiting at it != cacheCoins.end().

l0rinc commented at 10:16 pm on February 1, 2026:

The simplification deduplicates the other FetchCoin + iterator, why wouldn’t we reuse code here? And IsSpent() call on coinEmpty is just a null check, do you think that would matter anywhere?

l0rinc commented at 5:56 pm on February 3, 2026:

Reverted, kept the formatting change.

l0rinc force-pushed on Feb 1, 2026

l0rinc force-pushed on Feb 3, 2026

DrahtBot added the label CI failed on Feb 3, 2026

DrahtBot commented at 6:55 pm on February 3, 2026: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/actions/runs/21641577498/job/62382439018 LLM reason (✨ experimental): Lint check failed: direct use of std::filesystem was flagged by the std_filesystem lint rule.

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:

Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
An intermittent issue.

Leave a comment here, if you need help tracking down a confusing failure.

l0rinc force-pushed on Feb 3, 2026

DrahtBot removed the label CI failed on Feb 3, 2026

in src/bench/coins_haveinputs.cpp:46 in d957b36ab5

41+            assert(view.HaveInputs(*tx)); // Exercise the cache-hit path as well.
42+        }
43+    });
44+
45+    std::error_code ec;
46+    fs::remove_all(path, ec); // Remove the temporary directory we just created

andrewtoth commented at 1:06 am on February 4, 2026:

No need to do this now that we’re using the temp directory. The OS will take care of this.

l0rinc commented at 8:25 am on February 4, 2026:

Fixed, thanks

in src/bench/coins_haveinputs.cpp:22 in d957b36ab5

17+#include <ranges>
18+#include <system_error>
19+
20+static void HaveInputsOnDisk(benchmark::Bench& bench)
21+{
22+    const auto path{fs::temp_directory_path() / strprintf("bench-%s-%lld", bench.name(), GetTime())};

maflcko commented at 7:18 am on February 4, 2026:

this looks like it is re-inventing the wheel in a brittle and confusing way. You can use BasicTestingSetup to get a m_path_root, which you can join with bench.name() to get your temp coinsview db path. The current code seems not ideal for several reasons:

It is re-inventing the wheel for no given reason, which is confusing
It is using the deprecated GetTime helper
It is brittle, because it does create a temp dir, bug ignores the -testdatadir arg.
Manual cleanup code is added below, when a context manager could take care of it instead.

l0rinc commented at 8:25 am on February 4, 2026:

re-inventing the wheel

I didn’t want to use a test setup in a non-test, but if you think that’s better, I don’t mind.

It is using the deprecated GetTime helper

Fair, changed.

It is brittle, because it does create a temp dir, bug ignores the -testdatadir arg.

Good point, done.

Manual cleanup code is added below, when a context manager could take care of it instead.

Indeed, leftover that @andrewtoth also reported.

maflcko commented at 9:38 am on February 4, 2026:

re-inventing the wheel

I didn’t want to use a test setup in a non-test, but if you think that’s better, I don’t mind.

I don’t understand this. How is a benchmark not a test? The goal is to test the performance.

l0rinc commented at 9:54 am on February 4, 2026:

The distinction I have is that a test assumes a hypothesis of precise behavior, we assert against expected values. https://en.wikipedia.org/wiki/Software_testing writes:

Software testing can determine the correctness of software for specific scenarios

Strictly speaking, benchmarks are for measuring change, not testing expectations or correctness, it’s mostly meant for catching regressions. I wouldn’t call the compiler or linter or static analysis tests (even though they are definitely checking for correctness), but I don’t mind if others do, I was simply explaining why:

0const auto path{fs::temp_directory_path() / strprintf("bench-%s-%lld", bench.name(), GetTime())};

seemed simpler to me than

0const auto testing_setup{MakeNoLogFileContext<const BasicTestingSetup>(ChainType::REGTEST)};
1const auto path{testing_setup->m_path_root / fs::PathFromString(bench.name())};

But I have already applied your suggestions, thanks for the reviews! :)

maflcko commented at 10:13 am on February 4, 2026:

seeed simpler to me than

Right, it seems superficially simpler. However, if every test was brewing their own setup code, all behavior risks to be inconsistent and brittle. For example, I haven’t mentioned it, but the prior version also silently conflicted with the effort from #34439. Also, without the cleanup, it may introduce an out-of-storage issue (if the bench is run in a loop, similar to the symptoms of #34445)

I think generally it is best to come up with common re-usable concepts and then apply them consistently in the whole codebase.

If you think the docs imply that the common test utils are not applicable to bench tests, the docs can be improved. But the intent is certainly that the common test utils are to be used for all QA efforts.

l0rinc force-pushed on Feb 4, 2026

bench: add on-disk `HaveInputs` benchmark

`CCoinsViewCache::HaveInputs()` is used by `Consensus::CheckTxInputs()` and currently exercises the `HaveCoin()` path in the UTXO cache.
Performance could be affected due to `Coin` copying and different cache warming behavior.

Add a benchmark to track `HaveInputs()` throughput before and after the `HaveCoin()` changes in this series to make sure the performance is retained.
The benchmark populates an on-disk `CCoinsViewDB` with dummy unspent coins for all block inputs and then calls `HaveInputs()` for each non-coinbase transaction using a fresh `CCoinsViewCache` (to avoid cross-iteration caching).
It calls `HaveInputs()` twice to exercise the cache-hit path as well.

Benchmark baseline:
|               ns/tx |                tx/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|            1,004.58 |          995,437.07 |    0.7% |      1.10 | `HaveInputsOnDisk`

45b1b5baab

test: add `HaveInputs` call-path unit tests

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()`.

a355265538

dbwrapper: have `Read` and `Exists` reuse `ReadRaw`

`ExistsImpl` was removed since it duplicates `CDBWrapper::ReadImpl` (except that it copies the resulting string on success, but that will be needed for caching anyway).
`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.

d1470d44a8

coins: simplify `HaveCoin` usages

Direct `CCoinsViewDB::HaveCoin` calls aren't called in production anyway, so let's just revert to delegating to `::GetCoin`.

Benchmark at this commit:
|               ns/tx |                tx/s |    err% |     total | benchmark
|--------------------:|--------------------:|--------:|----------:|:----------
|              995.62 |        1,004,398.82 |    0.9% |      1.10 | `HaveInputsOnDisk`

d1a8a1bc56

l0rinc force-pushed on Feb 4, 2026

DrahtBot added the label CI failed on Feb 4, 2026

DrahtBot removed the label CI failed on Feb 4, 2026

coins: remove redundant and confusing CCoinsViewDB::HaveCoin #34320

Problem

Fix

Testing

Code Coverage & Benchmarks

Reviews

Conflicts

coins: remove redundant and confusing `CCoinsViewDB::HaveCoin` #34320