Bump SCRIPT_VERIFY flags to 64 bit

ajtowns commented at 10:58 pm on July 16, 2025: contributor

We currently use 21 of 32 possible bits for SCRIPT_VERIFY_* flags, with open PRs that may use 8 more (#29247, #31989, #32247, #32453). The mutinynet fork that has included many experimental soft fork features is already reusing bits here. Therefore, bump this to 64 bits.

In order to make it easier to update this logic in future, this PR also introduces a dedicated type for the script flags, and disables implicit conversion between that type and the underlying integer type. To make verifying that this change doesn’t cause flags to disappear, this PR also resurrects the changes from #28806 so that the script flags that are consensus enforced on each block can be queried via getdeploymentinfo.

DrahtBot commented at 10:58 pm on July 16, 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/32998.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	darosior, instagibbs, theStack, achow101
Stale ACK	sipa

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:

#32575 (refactor: Split multithreaded case out of CheckInputScripts by fjahr)
#32453 ([Policy] Discourage Unsigned Annexes by JeremyRubin)
#32317 (kernel: Separate UTXO set access from validation functions by TheCharlatan)
#31989 (BIP-119 (OP_CHECKTEMPLATEVERIFY) (regtest only) by jamesob)
#29843 (policy: Allow non-standard scripts with -acceptnonstdtxn=1 (test nets only) by ajtowns)
#29247 (CAT in Tapscript (BIP-347) by arminsabouri)

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.

ajtowns commented at 10:59 pm on July 16, 2025: contributor

The “introduce script_verify_flags typename” commit is probably best reviewed with --word-diff fwiw.

cc @instagibbs @darosior @benthecarman

ajtowns force-pushed on Jul 16, 2025

DrahtBot added the label CI failed on Jul 16, 2025

DrahtBot commented at 11:01 pm on July 16, 2025: contributor

🚧 At least one of the CI tasks failed. Task lint: https://github.com/bitcoin/bitcoin/runs/46136681380 LLM reason (✨ experimental): Lint failure caused by missing include guard in src/script/verify_flags.h.

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.

ajtowns force-pushed on Jul 17, 2025

ajtowns commented at 1:00 am on July 17, 2025: contributor

CI failure seems to be due to a bug in qt6 6.4:

Also for people coming here from Google: I had the same error message, but for me the problem was that I was using Qt 6.4.2 which apparently thinks that #if 202002L < 201103L is true, which causes c++config.h to not be included (and no #error is generated because moc doesn’t support the directive) . This has been fixed somewhere before Qt 6.7.3.

 0[20:04:17.396] Get:287 http://archive.ubuntu.com/ubuntu noble/universe amd64 qt6-base-dev-tools amd64 6.4.2+dfsg-21.1build5 [970 kB]
 1[20:04:17.414] Get:288 http://archive.ubuntu.com/ubuntu noble/universe amd64 qt6-qpa-plugins amd64 6.4.2+dfsg-21.1build5 [89.3 kB]
 2[20:04:17.417] Get:289 http://archive.ubuntu.com/ubuntu noble/universe amd64 qt6-base-dev amd64 6.4.2+dfsg-21.1build5 [1458 kB]
 3...
 4[20:12:02.187] Command
 5[20:12:02.187] -------
 6[20:12:02.187] /usr/lib/qt6/libexec/moc -DBOOST_MULTI_INDEX_DISABLE_SERIALIZATION -DBOOST_NO_CXX98_FUNCTION_BASE -DQT_CORE_LIB -DQT_DBUS_LIB -DQT_GUI_LIB -DQT_NETWORK_LIB -DQT_NO_DEBUG -DQT_NO_KEYWORDS -DQT_USE_QSTRINGBUILDER -DQT_WIDGETS_LIB -I/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src -I/ci_container_base/src -I/ci_container_base/src/univalue/include -I/ci_container_base/src/leveldb/include -I/usr/include/x86_64-linux-gnu/qt6/QtWidgets -I/usr/include/x86_64-linux-gnu/qt6 -I/usr/include/x86_64-linux-gnu/qt6/QtCore -I/usr/lib/x86_64-linux-gnu/qt6/mkspecs/linux-g++ -I/usr/include/x86_64-linux-gnu/qt6/QtGui -I/usr/include/x86_64-linux-gnu/qt6/QtNetwork -I/usr/include/x86_64-linux-gnu/qt6/QtDBus -I/usr/include -I/usr/include/c++/13 -I/usr/include/x86_64-linux-gnu/c++/13 -I/usr/include/c++/13/backward -I/usr/lib/llvm-20/lib/clang/20/include -I/usr/local/include -I/usr/include/x86_64-linux-gnu --include /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/qt/bitcoinqt_autogen/moc_predefs.h -p/ci_container_base/src/qt --output-dep-file -o /ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/src/qt/bitcoinqt_autogen/include/qt/intro.moc /ci_container_base/src/qt/intro.cpp
 7[20:12:02.187] 
 8[20:12:02.187] Output
 9[20:12:02.187] ------
10[20:12:02.187] usr/include/c++/13/concept:46:1: error: Parse error at "std"

The error output also seems to be cutting off the last letter of the filename (“concept” vs “concepts” here, and “stl_relops.” vs “stl_relops.h” in the original bug report linked above).

hebasto commented at 1:21 pm on July 17, 2025: member

CI failure seems to be due to a bug in qt6 6.4:

Although both failed CI job use Clang 20.1.7, the error can also be reproduced with GCC 13.3.

instagibbs commented at 1:31 pm on July 17, 2025: member

agreed, build issue also on my end with gcc 13.3.0

fanquake commented at 1:43 pm on July 17, 2025: member

Minified this is:

 0# podman run -it ubuntu:noble
 1
 2apt update && apt upgrade -y
 3apt install g++ qt6-base-dev qt6-tools-dev git
 4
 5git clone https://github.com/bitcoin/bitcoin
 6cd bitcoin
 7git fetch origin pull/32998/head:32998
 8git checkout 32998
 9
10/usr/lib/qt6/libexec/moc -I/usr/include/c++/13 --output-dep-file -o test.moc /bitcoin/src/qt/intro.cpp
11usr/include/c++/13/bits/cpp_type_traits.:69:1: error: Parse error at "std"

hebasto commented at 1:48 pm on July 17, 2025: member

CI failure seems to be due to a bug in qt6 6.4:

Although both failed CI job use Clang 20.1.7, the error can also be reproduced with GCC 13.3.

Refactoring helps.

in src/test/script_tests.cpp:1759 in 69c5e8141e outdated

1750@@ -1745,4 +1751,13 @@ BOOST_AUTO_TEST_CASE(compute_tapleaf)
1751     BOOST_CHECK_EQUAL(ComputeTapleafHash(0xc2, std::span(script)), tlc2);
1752 }
1753 
1754+BOOST_AUTO_TEST_CASE(formatscriptflags)
1755+{
1756+    // quick check that FormatScriptFlags reports any unknown/unexpected bits
1757+    BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH), "P2SH");
1758+    BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH | (1u<<31)), "P2SH,0x80000000");
1759+    BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_TAPROOT | (1u<<27)), "TAPROOT,0x08000000");

instagibbs commented at 3:03 pm on July 17, 2025:

bit of coverage of multiple unknown bits (didn’t validate)

0    BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_TAPROOT | (1u<<27)), "TAPROOT,0x08000000");
1    BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_TAPROOT | (1u<<27) | (1u<<31)), "TAPROOT,0x88000000");

ajtowns commented at 2:19 am on September 11, 2025:

Added something like this, though without the “88”

in src/test/script_tests.cpp:1718 in 69c5e8141e outdated

1750@@ -1745,4 +1751,13 @@ BOOST_AUTO_TEST_CASE(compute_tapleaf)
1751     BOOST_CHECK_EQUAL(ComputeTapleafHash(0xc2, std::span(script)), tlc2);
1752 }
1753 
1754+BOOST_AUTO_TEST_CASE(formatscriptflags)
1755+{
1756+    // quick check that FormatScriptFlags reports any unknown/unexpected bits
1757+    BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH), "P2SH");

instagibbs commented at 3:23 pm on July 17, 2025:

0    BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH), "P2SH");
1    BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_TAPROOT), "P2SH,TAPROOT");

ajtowns commented at 2:19 am on September 11, 2025:

Added

ajtowns commented at 5:37 am on July 19, 2025: contributor

Minified this is:

I don’t think you need the git fetch; git checkout here – you get the same error with that command (dropping the /bitcoin) on master afaics?

DrahtBot added the label Needs rebase on Jul 26, 2025

ajtowns force-pushed on Jul 26, 2025

fanquake commented at 9:44 am on July 26, 2025: member

@hebasto how can we move past the GUI build failures here? https://github.com/bitcoin/bitcoin/actions/runs/16538448423/job/46776353144?pr=32998#step:6:3769.

hebasto commented at 10:14 am on July 26, 2025: member

@hebasto how can we move past the GUI build failures here? https://github.com/bitcoin/bitcoin/actions/runs/16538448423/job/46776353144?pr=32998#step:6:3769.

Sure!

CI failure seems to be due to a bug in qt6 6.4:

Although both failed CI job use Clang 20.1.7, the error can also be reproduced with GCC 13.3.

Refactoring helps.

Implemented in https://github.com/bitcoin-core/gui/pull/881.

DrahtBot removed the label Needs rebase on Jul 26, 2025

hebasto referenced this in commit e6bfd95d50 on Jul 30, 2025

ajtowns force-pushed on Jul 31, 2025

in src/script/verify_flags.h:20 in 077797cb96 outdated

15+{
16+public:
17+    using value_type = uint64_t;
18+
19+    // default constructor is SCRIPT_VERIFY_NONE
20+    consteval script_verify_flags() : m_value{0} { }

DrahtBot commented at 6:13 am on July 31, 2025:

0[20:54:44.710] [676/676][43.3s] clang-tidy-20 -p=/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu -quiet -load=/tidy-build/libbitcoin-tidy.so /ci_container_base/src/qt/walletmodel.cpp
1[20:54:44.710] /ci_container_base/src/script/verify_flags.h:20:39: error: member initializer for 'm_value' is redundant [modernize-use-default-member-init,-warnings-as-errors]
2[20:54:44.710]    20 |     consteval script_verify_flags() : m_value{0} { }
3[20:54:44.710]       |                                       ^~~~~~~~~~

ajtowns force-pushed on Jul 31, 2025

DrahtBot removed the label CI failed on Jul 31, 2025

darosior commented at 1:53 pm on July 31, 2025: member

Concept ACK.

DrahtBot added the label Needs rebase on Aug 13, 2025

Move mapFlagNames and FormatScriptFlags logic to script/interpreter.h

Moves FormatScriptFlags logic into GetScriptFlagNames which returns a
vector of strings. For completeness, also has GetScriptFlagNames report
on any bits that do not match a known script flag.

5db8cd2d37

validation: export GetBlockScriptFlags() a3986935f0

rpc: have getdeploymentinfo report script verify flags 4577fb2b1e

script/interpreter: introduce script_verify_flags typename

Previously the SCRIPT_VERIFY_* flags were specified as either uint32_t,
unsigned int, or unsigned. This converts them to a common type alias in
preparation for changing the underlying type.

a5ead122fe

script/verify_flags: make script_verify_flags type safe

`using script_verify_flags = uint32_t` allows implicit conversion to
and from int, so replace it with a class to have the compiler ensure we
use the correct type. Provide from_int and as_int to allow for explicit
conversions when desired.

Introduces the type `script_verify_flag_name` for the individual flag
name enumeration.

bddcadee82

script/interpreter: make script_verify_flag_name an ordinary enum

Instead of having `SCRIPT_VERIFY_FOO = (1U << n)` just have it
be `n` directly, and do the bit shifting when converting it to
`script_verify_flags`.

3cbbcb66ef

script/verify_flags: extend script_verify_flags to 64 bits 417437eb01

ajtowns force-pushed on Aug 14, 2025

DrahtBot removed the label Needs rebase on Aug 14, 2025

ajtowns commented at 1:22 am on August 14, 2025: contributor

Rebased past header conflict with #33116

in src/test/fuzz/script.cpp:122 in bddcadee82 outdated

117@@ -118,8 +118,8 @@ FUZZ_TARGET(script, .init = initialize_script)
118             (void)FindAndDelete(script_mut, *other_script);
119         }
120         const std::vector<std::string> random_string_vector = ConsumeRandomLengthStringVector(fuzzed_data_provider);
121-        const uint32_t u32{fuzzed_data_provider.ConsumeIntegral<uint32_t>()};
122-        const script_verify_flags flags{u32 | SCRIPT_VERIFY_P2SH};
123+        const auto flags_rand{fuzzed_data_provider.ConsumeIntegral<script_verify_flags::value_type>()};
124+        const auto flags = script_verify_flags::from_int(flags_rand) | SCRIPT_VERIFY_P2SH;

instagibbs commented at 4:03 pm on September 9, 2025:

bddcadee82daf3ed1441820a0ffc4c5ef78f64f1

could be here or elsewhere, but round-simply round-tripping adds some coverage:

0assert(script_verify_flags::from_int(flags.as_int()) == flags);

ajtowns commented at 2:20 am on September 11, 2025:

Round-trip assertions in both directions added to fuzz/script_flags.cpp

sipa commented at 4:14 pm on September 9, 2025: member

utACK 417437eb01ac014c57aca47f44d7f8d3da351987

DrahtBot requested review from darosior on Sep 9, 2025

instagibbs approved

instagibbs commented at 4:40 pm on September 9, 2025: member

ACK 417437eb01ac014c57aca47f44d7f8d3da351987

non-blocking suggestions for coverage

test: additional test coverage for script_verify_flags 652424ad16

in src/script/verify_flags.h:22 in 652424ad16

17+    using value_type = uint64_t;
18+
19+    consteval script_verify_flags() = default;
20+
21+    // also allow construction with hard-coded 0 (but not other integers)
22+    consteval explicit(false) script_verify_flags(value_type f) : m_value{f} { if (f != 0) throw 0; }

darosior commented at 3:29 pm on September 11, 2025:

I was curious why this constructor from int with a runtime check was necessary. It’s because of all the if ((flags & SCRIPT_VERIFY_X) != 0) in the interpreter. This permits to implicitly construct a script_verify_flags from the 0 there instead of either having an implicit conversion to integer (the very thing this aims to avoid), or touching every single of those checks to make the conversion from the 0 integer explicit.

ajtowns commented at 0:29 am on September 12, 2025:

Note that it’s consteval, so the throw is a compile-time check, not a runtime one.

in src/script/verify_flags.h:32 in 652424ad16

27+    // rule of 5
28+    constexpr script_verify_flags(const script_verify_flags&) = default;
29+    constexpr script_verify_flags(script_verify_flags&&) = default;
30+    constexpr script_verify_flags& operator=(const script_verify_flags&) = default;
31+    constexpr script_verify_flags& operator=(script_verify_flags&&) = default;
32+    constexpr ~script_verify_flags() = default;

darosior commented at 5:10 pm on September 11, 2025:

How is the rule of 5 relevant for this class? It does not need a user-defined destructor.

From cppreference’s Rule of 5 section:

Because the presence of a user-defined (include = default or = delete declared) destructor, copy-constructor, or copy-assignment operator prevents implicit definition of the move constructor and the move assignment operator, any class for which move semantics are desirable, has to declare all five special member functions:

ajtowns commented at 0:28 am on September 12, 2025:

The rule of 5 just says if you define one, define/delete them all; I do it because that ensures I get a compile time error if there’s some problem that makes one of them not possible. Move semantics probably aren’t very interesting for a wrapper around an int though.

The C++ Core Guidelines version of the “rule of 0” says not to define any of them when possible, however.

darosior approved

darosior commented at 5:57 pm on September 11, 2025: member

ACK 652424ad162b63d73ecb6bd65bd26946e90c617f

I think it would also make sense, in a follow-up, to move the rest of the Script verification flags stuff to the new verify_flags.h.

DrahtBot requested review from sipa on Sep 11, 2025

DrahtBot requested review from instagibbs on Sep 11, 2025

instagibbs commented at 6:01 pm on September 11, 2025: member

reACK https://github.com/bitcoin/bitcoin/pull/32998/commits/652424ad162b63d73ecb6bd65bd26946e90c617f

in src/test/transaction_tests.cpp:63 in 5db8cd2d37 outdated

59@@ -81,9 +60,11 @@ unsigned int ParseScriptFlags(std::string strFlags)
60     std::vector<std::string> words = SplitString(strFlags, ',');
61     for (const std::string& word : words)
62     {
63-        if (!mapFlagNames.count(word))
64+        if (!mapFlagNames.count(word)) {

theStack commented at 4:05 pm on October 3, 2025:

nit: while touching, could switch to the more modern .contains

0        if (!mapFlagNames.contains(word)) {

in src/script/verify_flags.h:55 in bddcadee82 outdated

51+
52+    /** Compare two script_verify_flags. <, >, <=, and >= are auto-generated from this. */
53+    friend constexpr std::strong_ordering operator<=>(const script_verify_flags& a, const script_verify_flags& b) noexcept
54+    {
55+        return a.m_value <=> b.m_value;
56+    }

theStack commented at 5:46 pm on October 3, 2025:

nit: operator< would be sufficient (currently only needed in the transaction tests, where a std::set of script flag combinations is used), I doubt that the other ones would ever have a use-case

 0diff --git a/src/script/verify_flags.h b/src/script/verify_flags.h
 1index 95a55d2c79..e14a329ace 100644
 2--- a/src/script/verify_flags.h
 3+++ b/src/script/verify_flags.h
 4@@ -47,11 +47,9 @@ public:
 5     // tests
 6     constexpr explicit operator bool() const { return m_value != 0; }
 7     constexpr bool operator==(script_verify_flags other) const { return m_value == other.m_value; }
 8-
 9-    /** Compare two script_verify_flags. <, >, <=, and >= are auto-generated from this. */
10-    friend constexpr std::strong_ordering operator<=>(const script_verify_flags& a, const script_verify_flags& b) noexcept
11+    friend constexpr bool operator<(const script_verify_flags& a, const script_verify_flags& b) noexcept
12     {
13-        return a.m_value <=> b.m_value;
14+        return a.m_value < b.m_value;
15     }
16
17 private:

theStack approved

theStack commented at 5:48 pm on October 3, 2025: contributor

Code-review ACK 652424ad162b63d73ecb6bd65bd26946e90c617f :flags:

achow101 commented at 9:40 pm on October 7, 2025: member

ACK 652424ad162b63d73ecb6bd65bd26946e90c617f

achow101 merged this on Oct 7, 2025

achow101 closed this on Oct 7, 2025

Bump SCRIPT_VERIFY flags to 64 bit #32998

Code Coverage & Benchmarks

Reviews

Conflicts