kernel: Separate UTXO set access from validation functions

TheCharlatan commented at 1:15 pm on April 21, 2025: contributor

The current code makes it hard to call functions for checking the consensus-correctness of a block without having a full UTXO set at hand. This makes implementing features such as Utreexo and non-assumevalid swiftsync, where maintaining a full UTXO set defeats their purpose, through the kernel API difficult. Checks like the coinbase subsidy, or block sigops are only available through ConnectBlock, which requires a complete coins cache.

Solve this by moving accessing coins and the responsibility of spending them out of ConnectBlock and into a new separate function called SpendBlock. Pass a block’s spent coins through the existing CBlockUndo data structure to ConnectBlock.

While the change is largely a refactor, some smaller behaviour changes were unavoidable. These should be highlighted in the commit messages. The last commit contains the most important part of the coins logic move, the commits before attempt to prepare it piecemeal.

This pull request was benchmarked on benchcoin, which showed no performance regression. While discussing this pull request it was noted that pre-fetching and eliminating the need for some additional map lookups might improve performance a bit, but this does not make a measurable difference in the grand scheme of things and was not the motivation for this pull request.

In future changes, ConnectBlock could be made even more self contained, accessing the coins from the internal map could be further optimized by consolidating existence checking and spending, and the function eventually exposed in an external kernel API.

DrahtBot commented at 1:15 pm on April 21, 2025: 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/32317.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Concept ACK	w0xlt

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32379 (p2p: stop special-casing witness-stripped error for unconfirmed transactions by darosior)
#32080 (OP_CHECKCONTRACTVERIFY by bigspider)
#31981 (Add checkBlock() to Mining interface by Sjors)
#29641 (scripted-diff: Use LogInfo over LogPrintf [WIP, NOMERGE, DRAFT] by maflcko)
#29491 ([EXPERIMENTAL] Schnorr batch verification for blocks by fjahr)
#29060 (Policy: Report reason inputs are non standard by ismaelsadeeq)
#19463 (Prune locks by luke-jr)

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.

DrahtBot added the label Validation on Apr 21, 2025

TheCharlatan renamed this:
~~kernel: Seperate UTXO set access from validation functions~~
kernel: Separate UTXO set access from validation functions
on Apr 21, 2025

in src/validation.h:737 in 2be3c59151 outdated

708@@ -709,6 +709,9 @@ class Chainstate
709     bool ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex,
710                       CCoinsViewCache& view, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
711 
712+    bool SpendBlock(const CBlock& block, const CBlockIndex* pindex, const uint256& block_hash,

Sjors commented at 1:44 pm on April 21, 2025:

In 2be3c59151bcb840eb8cbd045a3dada6775ebc9e “validation: Add SpendBlock function”: this or the final commit would be a good time to document both SpendBlock and ConnectBlock.

IIUC SpendBlock:

Checks that all spent coins existed i) including coins created and spent within the block, in that order
Optimistically updates the UTXO set with newly created coins i) checks BIP30 (not in this order, but conceptually)
Populates Undo data

And then ConnectBlock does all the other consensus checks (like verifying the input scripts). It uses the Undo data populated by SpendBlock and does not need the UTXO set.

TheCharlatan commented at 4:15 pm on April 21, 2025:

Yes, that is the gist of it. Will add a commit at the end to add some documentation in the header.

TheCharlatan commented at 7:33 am on April 22, 2025:

Added a commit.

DrahtBot added the label CI failed on Apr 21, 2025

DrahtBot commented at 2:43 pm on April 21, 2025: contributor

🚧 At least one of the CI tasks failed. Debug: https://github.com/bitcoin/bitcoin/runs/40876609750

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.

consensus: Use Coin span in GetP2SHSigOpCount

Move the responsibility of retrieving coins from GetP2SHSigOpCount to
its caller.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

Define two explicit template specializations for both a span of
references to coins and a span of coins. This allows using it for both
Coin entries referenced from the CCoinsViewCache, and from contiguous
memory, like the vector in CBlockUndo.

a7e4132623

consensus: Use Coin span in GetTransactionSigOpCost

Move the responsibility of retrieving coins from GetTransactionSigOpCost
to its caller.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

Define two explicit template specializations for both a span of
references to coins and a span of coins. This allows using it for both
Coin entries referenced from the CCoinsViewCache, and from contiguous
memory, like the vector in CBlockUndo.

781668b6e8

TheCharlatan force-pushed on Apr 21, 2025

consensus: Use Coin span in CheckTxInputs

Move the responsibility of retrieving coins from CheckTxInputs
to its caller. The missing inputs check will be moved in an upcoming
commit.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

Define two explicit template specializations for both a span of
references to coins and a span of coins. This allows using it for both
Coin entries referenced from the CCoinsViewCache, and from contiguous
memory, like the vector in CBlockUndo.

c1285a81de

validation: Use vector of outputs instead of CCoinsViewCache in CheckInputScripts

Move the responsibility of retrieving coins from the CCoinsViewCache
in CheckInputScripts to its caller.

Add a helper method in CCoinsViewCache to collect all Coin's outputs
spent by a transaction's inputs.

Callers of CCoinsViewCache are updated to either pre-fetch the spent
outputs, or pass in an empty vector if the precomputed transaction data
has already been initialized with the required outputs.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

1eb575214d

validation: Add SpendBlock function

Move the BIP30 checks from ConnectBlock to a new SpendBlock method.
This is a move-only change, more content to SpendBlock is added in the
next commits. The goal is to move logic requiring access to the
CCoinsViewCache out of ConnectBlock and to the new SpendBlock method.
SpendBlock will in future handle all UTXO set interactions that
previously took place in ConnectBlock.

By returning early in case of a BIP30 validation failure, an
additional failure check for an invalid block reward in case of its
failure is left out.

Callers of ConnectBlock now also need to call SpendBlock before. This is
enforced in a future commit by adding a CBlockUndo argument that needs
to be passed to both.

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

008748c3ae

validation: Move SetBestBlock out of ConnectBlock

This is a part of a series of commits for removing access to the
CCoinsViewCache in consensus verification functions. The goal is to
allow calling verification functions with pre-fetched, or a user-defined
set of coins.

e1f88913b7

validation: Move coin existence and spend check to SpendBlock

Move the remaining UTXO-related operations from ConnectBlock to
SpendBlock. This includes moving the existence check, the UpdateCoins
call, and CBlockUndo generation.

ConnectBlock now takes a pre-populated CBlockUndo as an argument and no
longer accesses or manipulates the UTXO set.

8221e89ac4

doc: Add docstrings for ConnectBlock and SpendBlock 76a8f22c5c

in src/validation.h:710 in c1b36c4554 outdated

705@@ -706,8 +706,11 @@ class Chainstate
706     // Block (dis)connection on a given view:
707     DisconnectResult DisconnectBlock(const CBlock& block, const CBlockIndex* pindex, CCoinsViewCache& view)
708         EXCLUSIVE_LOCKS_REQUIRED(::cs_main);
709-    bool ConnectBlock(const CBlock& block, BlockValidationState& state, CBlockIndex* pindex,
710-                      CCoinsViewCache& view, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);
711+    bool ConnectBlock(const CBlock& block, const uint256& block_hash, BlockValidationState& state, CBlockIndex* pindex,
712+                      CBlockUndo& blockundo, bool fJustCheck = false) EXCLUSIVE_LOCKS_REQUIRED(cs_main);

darosior commented at 5:14 pm on April 21, 2025:

I think it would be clearer to take a vector of spent Coins directly? Or even a span?

TheCharlatan commented at 7:02 pm on April 21, 2025:

I might be missing something, but it would have to be a span of CTxUndo, and you’d have to move WriteBlockUndo to ConnectTip too, no? I wasn’t quite sure about doing that change here though. The implications of separately writing the undo data and raising the BlockIndex validity level are not quite clear to me yet. There is always a question where to draw the scope line with these refactors. While it might not be useful to write undo data if we ever get to the alternative kernel API client use cases, there might be other ways to handle that.

darosior commented at 7:37 pm on April 21, 2025:

Oh, i was just reading the diff and overlooked WriteBlockUndo! Yes that makes sense.

TheCharlatan force-pushed on Apr 21, 2025

DrahtBot removed the label CI failed on Apr 21, 2025

w0xlt commented at 6:32 pm on April 22, 2025: contributor

Concept ACK, as this PR increases the decoupling of components.

nit: The name SpendBlock may not be clear (unless this concept is already known and I’m missing the point). Maybe BIP30Validation, something like that, would be clearer.

kernel: Separate UTXO set access from validation functions #32317

Code Coverage & Benchmarks

Reviews

Conflicts