test: refactor: overhaul block hash determination for `CBlock{,Header}` objects

theStack commented at 12:39 pm on July 3, 2025: contributor

Similar to what #32421 did for CTransaction instances, this PR aims to improve the block hash determination of CBlockHeader/CBlock (the latter is a subclass of the former) instances by removing the block header caching mechanism and introducing consistent naming. Without the statefulness, sneaky testing bugs like #32742 and #32823 are less likely to happen in the future. Note that performance is even less of an issue here compared to CTransaction, as we only need to hash 80 bytes, which is less than typical standard transaction sizes [2]. The only instance where the testing logic was relying on caching (i.e. we want to return an outdated value) is tackled in the second commit, the rest should be straight-forward to review, especially for contributors who already reviewed #32421.

Summary table showing block hash determaination before/after this PR:

Task	master	PR
get block header hash (hex string)	`.hash`[1]	`.hash_hex`
get block header hash (integer)	`rehash()`, `.sha256`[1]	`.hash_int`

[1] = returned value might be None or out-of-date, if rehashing function wasn’t called after modification [2] = the only exception I could think of are transaction with pay-to-anchor (P2A) outputs

test: refactor: dedup `CBlockHeader` serialization

Note that we can't call `.serialize()` directly in
the `.calc_sha256()` method, as this could wrongly lead
to the serialization of the derived class (CBlock) if
called from an instance there.

b2d5cb6a42

test: avoid direct block header modification in feature_block.py

This is a preparatory commit for removing the header hash
caching in the CBlockHeader class. In order to not lose the
old block hash, necessary for updating the internal state of
the test (represented by `self.block_heights` and `self.blocks`),
we should only modify it within the `update_block` method.

daa3c45990

test: remove header hash caching in CBlockHeader class

Rather than block hashes (represented by the fields `.sha256` and
`.hash`) being stateful, simply compute them on-the-fly. This ensures
that the correct values are always returned and takes the burden of
rehashing from test writers, making the code shorter overall.  In a
first step, the fields are kept at the same name with @property
functions as drop-in replacements, for a minimal diff. In later commits,
the names are changed to be more descriptive and indicating the return
type of the block hash.

754eb2dcb1

test: remove bare CBlockHeader `.rehash()`/`.calc_sha256()` calls

Since the previous commit, CBlockHeader/CBlock object calls to the
methods `.rehash()` and `.calc_sha256()` are effectively no-ops
if the returned value is not used, so we can just remove them.

216ccaa093

test: rename CBlockHeader `.rehash()`/`.sha256` -> `.hash_int` for consistency

Note that we unfortunately can't use a scripted diff here, as the
`sha256` symbol is also used for other instances (e.g. as function
in hashlib, or in the `UTXO` class in p2p_segwit.py).

c14918cc1a

test: rename CBlockHeader `.hash` -> `.hash_hex` for consistency

Note that we unfortunately can't use a scripted diff here, as the
`.hash` symbol is also used for other instances (e.g. CInv).

5eb1e4d99f

DrahtBot added the label Tests on Jul 3, 2025

DrahtBot commented at 12:39 pm on July 3, 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/32868.

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:

#32587 (test: Fix reorg patterns in tests to use proper fork-based approach by yuvicc)
#32468 (rpc: generateblock to allow multiple outputs by polespinasa)
#32180 (p2p: Advance pindexLastCommonBlock early after connecting blocks by mzumsande)

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.

test: avoid unneeded block header hash -> integer conversions 50b41bcfbf

theStack force-pushed on Jul 3, 2025

test: refactor: overhaul block hash determination for CBlock{,Header} objects #32868

Code Coverage & Benchmarks

Reviews

Conflicts

test: refactor: overhaul block hash determination for `CBlock{,Header}` objects #32868