Make transaction_indentifier hex string constructor evaluated at comptime #34063

1. 
   rustaceanrob commented at 4:25 pm on December 12, 2025: contributor

   Suggested by l0rinc as a comment in #34004.

   There are tests that utilize FromHex that will only fail during runtime if malformed. Adds a compile time constructor that can be caught by LSPs.

2. 
   DrahtBot commented at 4:25 pm on December 12, 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/34063.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   ACK	l0rinc, maflcko, rkrux

   If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #33034 (wallet: Store transactions in a separate sqlite table by achow101)

   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.

3. DrahtBot added the label CI failed on Dec 12, 2025
4. 
   DrahtBot commented at 9:41 pm on December 12, 2025: contributor

   🚧 At least one of the CI tasks failed. Task Windows-cross to x86_64, ucrt: https://github.com/bitcoin/bitcoin/actions/runs/20173084809/job/57923462319 LLM reason (✨ experimental): Compilation failed due to -Werror treating unused-value warnings as errors in merkleblock_tests.cpp.

   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.

5. 
   l0rinc commented at 9:59 pm on December 12, 2025: contributor

   It seems you’ve hit https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117501 Created https://godbolt.org/z/xb5TMaPs6, we should be able to fix it with an explicit constexpr for the declarations.

   I have pushed an alternative to https://github.com/l0rinc/bitcoin/pull/63, please cherry-pick it if it passes CI. Note that I’ve also changed some constructors to use brace init and extended the commit message with the issue and the godbolt reproducer. Also added a missing #include <string_view>.

6. in src/primitives/transaction_identifier.h:37 in ce9f73fe1c outdated

   33@@ -34,6 +34,7 @@ class transaction_identifier
   34 
   35 public:
   36     transaction_identifier() : m_wrapped{} {}
   37+    consteval explicit transaction_identifier(std::string_view hex_str) : m_wrapped{uint256{hex_str}} {}
   

   ---

   ---

   

   ajtowns commented at 10:58 pm on December 12, 2025:

   Would it be better to provide an operator""_txid and _wtxid along the lines of hexliterals in util/strencodings.h? They could be made available only to test code that way.

   Adding contstexpr to the constructors in primitives/transaction_identifier.h and writing:

   0consteval auto operator""_txid(const char* str, size_t n) { return Txid::FromUint256(uint256{std::string_view{str, n}}); }
   1consteval auto operator""_wtxid(const char* str, size_t n) { return Wtxid::FromUint256(uint256{std::string_view{str, n}}); }
   2
   3Wtxid wtxid_1{"85cd1a31eb38f74ed5742ec9cb546712ab5aaf747de28a9168b53e846cbda17f"_wtxid};
   4Txid txid_1{"bd0f71c1d5e50589063e134fad22053cdae5ab2320db5bf5e540198b0b5a4e69"_txid};
   

   looks like it does the right thing, fwiw.

   ---

   

   l0rinc commented at 11:01 pm on December 12, 2025:

   Had a similar attempt in #31991 (review) - what do you think @maflcko?

   ---

   

   maflcko commented at 1:21 pm on December 13, 2025:

   Had a similar attempt in #31991 (comment) - what do you think @maflcko?

   Either should be fine. I just think there is no need to provide the exact same functionality via two ways. That seems confusing.

7. rustaceanrob force-pushed on Dec 13, 2025
8. refactor: Add compile-time-checked hex txid

   Suggested by @l0rinc in #34004
   
   Message by @l0rinc:
   
   This adds a consteval constructor to transaction_identifier (Txid/Wtxid) to allow parsing hex strings at compile-time.
   This replaces runtime FromHex checks in tests, ensuring that malformed hardcoded hashes cause build failures rather than runtime test failures.
   
   Test variables are explicitly marked constexpr. This is required to workaround a regression in GCC 14 (Bug 117501) where the compiler incorrectly flags consteval initialization of non-constexpr variables as "statements with no effect".
   
   GCC Bug: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=117501
   Reproducer: https://godbolt.org/z/xb5TMaPs6
   
   Co-authored-by: l0rinc <pap.lorinc@gmail.com>

   5ac3579520
9. rustaceanrob force-pushed on Dec 13, 2025
10. DrahtBot removed the label CI failed on Dec 13, 2025
11. 
    rustaceanrob commented at 8:14 pm on December 13, 2025: contributor

    Co-authorship attribution, rebased into single commit, brace initialization throughout the commit, and constexpr fix in 5ac35795206d252c9f464e967b84521ddaad38f1
12. 
    l0rinc commented at 8:29 pm on December 13, 2025: contributor

    ACK 5ac35795206d252c9f464e967b84521ddaad38f1

    I’m fine with this version and with @ajtowns’s suggestion as well - let’s see what others think.

13. 
    maflcko commented at 7:48 am on December 15, 2025: member

    review ACK 5ac35795206d252c9f464e967b84521ddaad38f1 🦎

    Signature:

    0untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
    1RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
    2trusted comment: review ACK 5ac35795206d252c9f464e967b84521ddaad38f1 🦎
    3sRW+9c4Xa164fJzsatQut371BfPuwGZUQxpppVTpFOQgXGpNPuQo4Opj5UF5Aj7N2DWpc2h/TNJrMWmGFrKmBA==
    

14. rkrux approved
15. 
    rkrux commented at 2:03 pm on December 15, 2025: contributor

    crACK 5ac35795206d252c9f464e967b84521ddaad38f1

    I prefer the style in the PR over the one suggested in #34063 (review), which is quite neat as well.

16. fanquake merged this on Dec 17, 2025
17. fanquake closed this on Dec 17, 2025

    ---

Contributors
rustaceanrob DrahtBot l0rinc ajtowns maflcko rkrux