Consensus::CheckTxInputs currently makes two UTXO lookups per input: first through a HaveInputs() pre-pass (which internally calls HaveCoin() for every input), and then through AccessCoin() on the same outpoints in the main validation loop.
HaveInputs() has only a single caller (CheckTxInputs), and AccessCoin() already returns a reference to a spent sentinel Coin for any missing outpoint. This means the presence check can be folded into the existing loop without changing behavior.
This PR removes the initial HaveInputs() pre-pass and instead checks coin.IsSpent() immediately after AccessCoin() returns. As a result, CheckTxInputs performs one lookup per input instead of two (2N → N, where N is the number of inputs) in the validation path shared by both ConnectBlock and mempool acceptance.
HaveInputs() itself is not removed. It is still used by the coins-view fuzz harness, although this change leaves it with no callers in the validation hot path. Removing it would be a trivial follow-up.
The only observable behavioral difference is which validation error is reported if one input is missing while another input in the same transaction fails for some unrelated reason (for example, coinbase immaturity).
- In
ConnectBlock, all such failures are ultimately reported asBLOCK_CONSENSUS, so the specific failure reason or ordering has no consensus impact. - In the mempool (
MemPoolAccept::PreChecks), every input has already been checked withm_view.HaveCoin()beforeCheckTxInputs()is called. Missing inputs are handled there as orphan transactions, making the "missing input" path insideCheckTxInputs()unreachable during mempool validation.
<details> <summary><strong>Benchmark</strong> (<code>bench_bitcoin -filter=CheckTxInputs</code>)</summary>
The benchmark below is intentionally designed to measure the cost of the redundant lookup itself, rather than end-to-end block validation.
A fresh CCoinsViewCache wrapping CoinsTip is constructed for each measured batch (coins are added to CoinsTip itself, not to the benchmarked view), so each pass actually crosses the view/base boundary instead of hitting an already-warmed local cache. This closely matches how a fresh per-block view behaves in production.
BEFORE="d31c26fd1a53300a704bccbf12aae5b050c67ec3"
AFTER="5021cd02fa503f0ea0dc741f7200eba8275a885d"
RESULTS_FILE="$HOME/checktxinputs_bench_results.txt"
: > "$RESULTS_FILE"
echo "machine | $(hostname) | $(uname -m) | $(lscpu | grep 'Model name' | head -1 | cut -d: -f2 | xargs) | $(nproc) cores | $(free -h | awk '/^Mem:/{print $2}') RAM" | tee -a "$RESULTS_FILE"
ORIG="$(git rev-parse --abbrev-ref HEAD)"
for hash in $BEFORE $AFTER; do
echo ">>> $hash"
git checkout "$hash" >/dev/null 2>&1 &&
cmake --build build -t bench_bitcoin &&
(
echo "--- ${hash:0:7} ---"
./build/bin/bench_bitcoin -filter=CheckTxInputs -min-time=3000
) 2>&1 | tee -a "$RESULTS_FILE"
done
git switch "$ORIG"
</details>
| ns/tx | tx/s | err% | |
|---|---|---|---|
| before | 1,154.31 | 866,316 | 0.4% |
| after | 985.84 | 1,014,367 | 0.5% |
~14.6% faster. This is smaller than a naive 2N → N lookup-count argument might suggest, because both lookups are FetchCoin calls against the same already-resolved cache entry after the first one resolves it. The second lookup is just a cheap lookup in an already-warm cache, not a second trip to the backing store. The real saving is exactly one redundant cache lookup per input, which is what this benchmark isolates.
As a sanity check, I also compared IBD over the same block range before and after this change and observed no measurable difference in synchronization time, which is expected: CheckTxInputs accounts for only a small fraction of ConnectBlock, where signature verification dominates.
The optimization is therefore real but modest. Its value comes from removing a genuinely redundant lookup without changing consensus behavior, rather than from any meaningful IBD speedup.
Historical context
This pre-pass originally served a real purpose.
Before April 2017, CheckTxInputs() used a per-txid CCoins cache where AccessCoins(txid) returned a raw pointer that could be NULL, and a single CCoins entry could contain both spent and unspent outputs. In that design, HaveInputs() prevented both null-pointer dereferences and spending an already-spent output within an otherwise live entry.
On 2017-04-25, two commits changed this:
0003911326("Introduce new per-txout CCoinsViewCache functions") introducedAccessCoin(), which returns a reference to a sentinel empty coin for missing outpoints.f68cdfe92b("Switch from per-tx to per-txout CCoinsViewCache methods in some places") updatedCheckTxInputs()to use the new API.
From that point onward, missing and already-spent outputs were both represented by coin.IsSpent() on the object returned by AccessCoin(), making the HaveInputs() pre-pass redundant. The validation loop has effectively remained unchanged since then, aside from the later scripted rename from IsPruned() to IsSpent() (589827975f).
This PR simply removes that leftover redundancy.
Edit: :When I opened this PR, I hadn't reviewed PR #35187, which modifies the same lines. While the primary goal of that PR is different, it also avoids the redundant lookup as a side effect.
Although that PR was submitted earlier, it is significantly larger and includes more critical and potentially controversial changes (it has already received one concept NACK), so it may take longer to be merged.
This PR is smaller, focused on a specific issue, and can be evaluated independently. It also doesn't block or conflict with the other PR, since #35187 can still be merged later on top of these changes without any issues.
