RFC: Add operator""_uint256 compile-time user-defined literal #31991

1. 
   l0rinc commented at 9:02 pm on March 4, 2025: contributor

   Follow-up to Add mainnet assumeutxo param at height 880,000, looking to simplify specifying explicit hashes in the source code. It’s not a full implementation yet, I’m looking for concept ack/nack and full CI feedback for now - applying it to the whole codebase may follow based on that.

   ---

   This is also a follow-up to refactor: Replace ParseHex with consteval “"_hex literals, meant to simplify the usages (values beginning with the hexadecimal data, followed by the type conversion, similarly to sizeof(uint256) vs uint256::size()) and unifying hex parsing code. Similarly to ""_hex, the ""_uint256 has the same compile-time constraints as its constructor (properly sized with actual hexadecimal values), but uses detail::Hex parser (reversed) (used in most other places for hex parsing) instead of base_blob(std::string_view hex_str). The new implementation also makes it obvious that the values are reversed in this case.

   I thought of removing the consteval_ctor hack as well, now that consteval conversion is claimed to work in visual studio, but since https://godbolt.org/z/j837req5h still seems to fail for latest x64 msvc v19, I postponed doing that.

2. 
   DrahtBot commented at 9:02 pm on March 4, 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/31991.

   Reviews

   See the guideline for information on the review process.

   Type	Reviewers
   Concept NACK	maflcko

   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:

   • #31974 (Drop testnet3 by Sjors)
   • #31910 (qa: fix an off-by-one in utxo snapshot fuzz target and sanity check its snapshot data by darosior)
   • #31649 (consensus: Remove checkpoints (take 2) by marcofleon)
   • #26201 (Remove Taproot activation height by Sjors)

   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. in src/kernel/chainparams.cpp:199 in cf8fe00072 outdated

   195@@ -196,7 +196,7 @@ class CMainParams : public CChainParams {
   196                 .height = 880'000,
   197                 .hash_serialized = AssumeutxoHash{uint256{"dbd190983eaf433ef7c15f78a278ae42c00ef52e0fd2a54953782175fbadcea9"}},
   198                 .m_chain_tx_count = 1145604538,
   199-                .blockhash = consteval_ctor(uint256{"000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"}),
   200+                .blockhash = "000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"_hex,
   

   ---

   ---

   

   hodlinator commented at 8:56 am on March 5, 2025:

   Removing consteval_ctor should also involve bumping the minimum version in doc/build-windows-msvc.md, right? See: https://learn.microsoft.com/en-us/visualstudio/releases/2022/release-notes-v17.6

   ---

   

   l0rinc commented at 11:15 am on March 5, 2025:

   Thanks, bumped it from 17.6 to 17.6.18

   ---

   

   l0rinc commented at 4:42 pm on March 9, 2025:

   Removed the consteval_ctor change since godbolt and the MSVC docs have mixed signals about it
4. in src/uint256.h:214 in cf8fe00072 outdated

   210@@ -206,6 +211,8 @@ class uint256 : public base_blob<256> {
   211     consteval explicit uint256(std::string_view hex_str) : base_blob<256>(hex_str) {}
   212     constexpr explicit uint256(uint8_t v) : base_blob<256>(v) {}
   213     constexpr explicit uint256(Span<const unsigned char> vch) : base_blob<256>(vch) {}
   214+    constexpr uint256(const std::array<std::byte, 32>& hex) : base_blob(hex) {}
   

   ---

   ---

   

   hodlinator commented at 8:58 am on March 5, 2025:

   Possible alternative could be to introduce support for something like "000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"_uint256 which communicates what the final type is.

   ---

   

   l0rinc commented at 11:15 am on March 5, 2025:

   Yeah, I’d be fine with that as well, but where would we define that, next to _hex (and include uint256.h) or in uint256?

   ---

   

   hodlinator commented at 1:54 pm on March 5, 2025:

   I’d lean towards defining operator""_uint256 in uint256.h, but don’t have a strong preference. uint256.h currently has #include <util/strencodings.h>, so doing it the other way would introduce cyclic include issues.

   ---

   

   l0rinc commented at 2:25 pm on March 5, 2025:

   I’ll try that, it would likely clarify our byte ordering problem as well

   ---

   

   l0rinc commented at 3:59 pm on March 5, 2025:

   Done, thanks

   ---

   

   hodlinator commented at 8:07 pm on March 5, 2025:

   Thanks for trying that out! I like that the reversal is more directly tied to the literal than when you were using _hex.

   Hm… now that I compare:

   0"000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"_uint256
   

   and

   0uint256{"000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"}
   

   …does one character less really justify adding a literal?

   Could shorten it to _u256, but that has a different meaning than u8 in _hex_u8 (one single unsigned 256-bit value vs an array of unsigned 8-bit values).

   Maybe killing the consteval_ctor and updating the minimum VS version is enough for now?

   There are other loose ends like _hex_v_u8 being used for the signet challenge data instead of _hex_v if you want to go down that rabbit hole of using more std::byte.

   ---

   

   l0rinc commented at 8:15 pm on March 5, 2025:

   does one character less really justify adding a literal?

   I’m not counting chars, but that the noise is at the end now, it starts with the signal. I.e. that we see .blockhash = "000000000000000000010b17283c3c400507969a9c2afd1dcf2082ec5cca2880"... directly now.

5. l0rinc force-pushed on Mar 5, 2025
6. l0rinc renamed this:
   RFC: Add implicit constructor for `_hex` to `uint256` and remove `consteval_ctor`
   RFC: Add `operator""_uint256` compile-time user-defined literal
   on Mar 5, 2025
7. l0rinc force-pushed on Mar 9, 2025
8. DrahtBot added the label Needs rebase on Mar 11, 2025
9. l0rinc force-pushed on Mar 11, 2025
10. DrahtBot removed the label Needs rebase on Mar 11, 2025
11. DrahtBot added the label Needs rebase on Mar 14, 2025
12. Add `_uint256` user-defined literal similarly to `_hex`

    Co-authored-by: Hodlinator <172445034+hodlinator@users.noreply.github.com>

    8c27aaf1e9
13. l0rinc force-pushed on Mar 14, 2025
14. DrahtBot removed the label Needs rebase on Mar 14, 2025
15. 
    maflcko commented at 2:28 pm on March 14, 2025: member

    I thought of removing the consteval_ctor hack as well, now that consteval conversion is claimed to work in visual studio, but since https://godbolt.org/z/j837req5h still seems to fail for latest x64 msvc v19, I postponed doing that.

    I think the upstream bug link may be wrong. https://developercommunity.visualstudio.com/t/Can-not-initialize-array-with-consteval/10828473 looks closer

16. in src/kernel/chainparams.cpp:91 in 8c27aaf1e9

    87@@ -88,17 +88,17 @@ class CMainParams : public CChainParams {
    88         consensus.signet_challenge.clear();
    89         consensus.nSubsidyHalvingInterval = 210000;
    90         consensus.script_flag_exceptions.emplace( // BIP16 exception
    91-            uint256{"00000000000002dc756eebf4f49723ed8d30cc28a5f108eb94b1ba88ac4f9c22"}, SCRIPT_VERIFY_NONE);
    92+            "00000000000002dc756eebf4f49723ed8d30cc28a5f108eb94b1ba88ac4f9c22"_uint256, SCRIPT_VERIFY_NONE);
    

    ---

    ---

    

    maflcko commented at 2:31 pm on March 14, 2025:

    Not sure what the goal here is, or the improvement. Looks like extra code is added to do the same thing in two different ways?
17. 
    maflcko commented at 2:32 pm on March 14, 2025: member

    tend toward NACK for now, as I don’t see the goal or benefit

    Would be happy to ack a doc fix of the upstream msvc bug report

18. l0rinc closed this on Mar 14, 2025

    ---

19. 
    hodlinator commented at 2:53 pm on March 14, 2025: contributor

    Would be happy to ack a doc fix of the upstream msvc bug report

    Yes, it seems like our issue was closer this “sub-issue” in the currently linked issue: https://developercommunity.visualstudio.com/t/consteval-conversion-function-fails/1579014#T-N10550785 and they apparently only fixed the primary issue. :\

Contributors
l0rinc DrahtBot hodlinator maflcko