refactor: disable default std::hash for CTransactionRef #35101

pull Sjors wants to merge 2 commits into bitcoin:master from Sjors:2026/04/txref-safety changing 1 files +23 −0

Sjors commented at 7:48 PM on April 17, 2026: member
While working on #33922 I initially forgot to add CTransactionRefComp to the std::unordered_map<CTransactionRef defined there. This PR turns that into a compiler error.

This change triggers a false positive IWYU error, and an inconsistent one at that: our CI wants <variant>, while a manual build on Ubuntu (version 0.26 with clang version 22.1.1) wants <string_view>.

Various workarounds were discussed in:
The approach chosen here is:
- narrowly scoped to this file
- does add unneeded includes
- does not ignore the false positive <variant> / <string_view> requirement (as that could cause future bugs)
- works independent of the exact IWYU build
- is skipped for libc++, because that doesn't have the false positive and the workaround triggers an ambiguous specialization.
The main downside is that it's slightly verbose, though mostly from the comment.

refactor: disable default std::hash for CTransactionRef

The default std::hash for shared_ptr compares by pointer.

CTransactionRefSaltedHash or a custom hasher should be used instead.

e5d04fb46a

iwyu: redeclare hash primary template

IWYU incorrectly suggests <variant> or <string_view> for std::hash.

276da6236a

DrahtBot added the label Refactoring on Apr 17, 2026
DrahtBot commented at 7:48 PM on April 17, 2026: contributor



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Reviews

See the guideline for information on the review process. A summary of reviews will appear here.
in src/primitives/transaction.h:426 in 276da6236a
```
 421 | + *  comparing by pointer. Use CTransactionRefSaltedHash or provide a custom
 422 | + *  hasher. */
 423 | +template <>
 424 | +struct hash<CTransactionRef> {
 425 | +    size_t operator()(const CTransactionRef&) const = delete;
 426 | +};
```
hebasto commented at 5:54 PM on April 18, 2026:

e5d04fb46a68bd884d8eefae1014aaa3dd79fa6a:

This approach doesn't handle scenarios where a container is declared, but no member functions involving hashing are explicitly called. For example, this compiles.

I suggest a more defensive approach relying on the conditions that must be satisfied for the std::hash<CTransactionRef> specialization to be enabled:

template <> struct hash<CTransactionRef> { hash() = delete; };

Here are two examples on https://godbolt.org/:

A container with the default hasher: fails.

A container with the custom hasher: passes.
Sjors commented at 10:21 AM on April 19, 2026:

Thanks, marking draft while I study this suggestion.
in src/primitives/transaction.h:419 in 276da6236a
```
 414 | + * workaround.
 415 | + */
 416 | +#ifndef _LIBCPP_VERSION
 417 | +template <class T>
 418 | +struct hash; // IWYU pragma: keep
 419 | +#endif
```
hebasto commented at 6:00 PM on April 18, 2026:

276da6236af12bb7aee12f74f8138f2c9bb16176:

This changes follows this suggestion, excluding libc++, which is not used in the IWYU CI job.

Looks fine to me.

hebasto commented at 8:34 AM on April 20, 2026:

I can confirm that the bug has been fixed upstream in https://github.com/include-what-you-use/include-what-you-use/pull/2013.
Sjors marked this as a draft on Apr 19, 2026
Sjors commented at 10:55 AM on April 20, 2026: member

Since this isn't urgent, I think I'll wait for #35101 (review) to land in a release.

Contributors

Labels

Linked (view graph)

#33922 mining: add getMemoryLoad() and track template non-mempool memory footprint #35073 iwyu: map std::hash to <functional>

refactor: disable default std::hash for CTransactionRef #35101

Reviews