TxGraph: Increase fuzz coverage #32177

pull instagibbs wants to merge 4 commits into bitcoin:master from instagibbs:2025-03-txgraph_coverage changing 2 files +11 −9

instagibbs commented at 5:17 PM on March 31, 2025: member
Was looking at my local coverage report, and noticed a few spots that will not or cannot be hit.

CountDistinctClusters, GetAncestorsUnion, and GetDescendantsUnion accept nullptrs, but the test harness never employs them. Disallow them.

We never call PullIn whenever there isn't staging, so just enforce that invariant via assertion.

Remaining places that are not covered:
1. Relinearize: Currently we seem to always start with a cold (not known to be optimal) cluster, and after one attempt at linearization result into something optimal. This means we never shortcircuit, nor run PostLinearization, nor store the quality as ACCEPTABLE. Reducing iterations causes these lines to be hit. sipa says he will take this on as varying the amount of iterations was meant to be done eventually anyways.
2. We never do a move assignment operator when the lvalue already has a m_graph (so we never call UnlinkRef) https://github.com/bitcoin/bitcoin/blob/3358b1d105196647230e6f828b8ec820426b96a0/src/txgraph.cpp#L2097
3. We never use the move constructor: https://github.com/bitcoin/bitcoin/blob/3358b1d105196647230e6f828b8ec820426b96a0/src/txgraph.cpp#L2108
TxGraphImpl::PullIn: only allowed when staging exists 2c5cf987e9
TxGraphImpl::Compact: m_main_clusterset.m_removed is always empty 8bca0d325a
CountDistinctClusters: nullptrs disallowed 57433502e6
Get*Union: disallow nulltpr Refs a40bd374aa
DrahtBot commented at 5:17 PM on March 31, 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/32177.



Reviews

See the guideline for information on the review process.

Type Reviewers

ACK sipa, glozow

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
sipa commented at 6:17 PM on March 31, 2025: member

utACK a40bd374aaf8e10116fa664fa7b480d85ee2fe59
fanquake requested review from glozow on Apr 1, 2025
in src/txgraph.cpp:1885 in a40bd374aa
```
1881 | @@ -1880,7 +1882,7 @@ TxGraph::GraphIndex TxGraphImpl::CountDistinctClusters(std::span<const Ref* cons
1882 |      std::vector<Cluster*> clusters;
1883 |      clusters.reserve(refs.size());
1884 |      for (const Ref* ref : refs) {
1885 | -        if (ref == nullptr) continue;
1886 | +        Assume(ref);
```
glozow commented at 4:23 PM on April 1, 2025:

Do you want to retain the safe exit if this isn't true? if (!Assume(ref)) continue; ?

instagibbs commented at 5:08 PM on April 1, 2025:

Current thinking and elsewhere in TxGraph we don't take the cost of a conditional if we're confident that the code cannot be reached by a valid invocation by the caller.

See #31363 (review) , #31363 (review) , and #32100 (comment) for more discussion
glozow commented at 6:48 PM on April 2, 2025: member

utACK a40bd374aaf8e10116fa664fa7b480d85ee2fe59
fanquake merged this on Apr 3, 2025
fanquake closed this on Apr 3, 2025
sedited referenced this in commit a9c46ce3c3 on Apr 24, 2025
stickies-v referenced this in commit 772a33e052 on May 23, 2025
yuvicc referenced this in commit 069643f094 on Jul 6, 2025
bug-castercv502 referenced this in commit 5b0af4b944 on Sep 28, 2025
Kino1994 referenced this in commit 7d0886e9e7 on Jun 28, 2026
BigcoinBGC referenced this in commit 2c013cc0d2 on Jun 30, 2026
bitcoin locked this on Jul 30, 2026

Type	Reviewers
ACK	sipa, glozow

Contributors

Linked (view graph)

#30289 Cluster mempool tracking issue

TxGraph: Increase fuzz coverage #32177

Code Coverage & Benchmarks

Reviews