refactor: Construct g_verify_flag_names on first use #33600

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2510-cofu changing 3 files +30 −26
  1. maflcko commented at 3:05 pm on October 10, 2025: member

    The current usage of the g_verify_flag_names map seems fine and I can not see a static initialization order fiasco here.

    However, it seems brittle to hope this remains the case in the future. Also, it triggers a msan false-positive in the fuzz CI task. (C.f https://github.com/bitcoin-core/qa-assets/actions/runs/18352815555/job/52413137315?pr=241#step:7:5245)

    So just apply the “Construct on first use” idiom.

  2. refactor: Construct g_verify_flag_names on first use faa9d10c84
  3. DrahtBot added the label Refactoring on Oct 10, 2025
  4. DrahtBot commented at 3:05 pm on October 10, 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/33600.

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK kevkevinpal, janb84, stickies-v, ajtowns

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

  5. maflcko commented at 7:08 pm on October 10, 2025: member

    master:

     0...
 1+ /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/test/fuzz/test_runner.py -j16 -l DEBUG /ci_container_base/ci/scratch/qa-assets/fuzz_corpora/ --empty_min_time=60
 2==11639==WARNING: MemorySanitizer: use-of-uninitialized-value
 3    [#0](/bitcoin-bitcoin/0/) 0xaaaad25c2408 in SetArgs(int, char**) /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/src/test/fuzz/util/./test/fuzz/fuzz.cpp:52:5
 4    [#1](/bitcoin-bitcoin/1/) 0xaaaad25c2408 in LLVMFuzzerInitialize /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/src/test/fuzz/util/./test/fuzz/fuzz.cpp:223:5
 5    [#2](/bitcoin-bitcoin/2/) 0xaaaad193a1dc in fuzzer::FuzzerDriver(int*, char***, int (*)(unsigned char const*, unsigned long)) (/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/bin/fuzz+0xe4a1dc) (BuildId: d44700e52b2315b458e122acf4a7dd36326c741c)
 6    [#3](/bitcoin-bitcoin/3/) 0xaaaad19660f4 in main (/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/bin/fuzz+0xe760f4) (BuildId: d44700e52b2315b458e122acf4a7dd36326c741c)
 7    [#4](/bitcoin-bitcoin/4/) 0xffff87fb84c0  (/lib/aarch64-linux-gnu/libc.so.6+0x284c0) (BuildId: d6c205bda1b6e91815f8fef45bdf56bc2239c37e)
 8    [#5](/bitcoin-bitcoin/5/) 0xffff87fb8594 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x28594) (BuildId: d6c205bda1b6e91815f8fef45bdf56bc2239c37e)
 9    [#6](/bitcoin-bitcoin/6/) 0xaaaad1931aec in _start (/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/bin/fuzz+0xe41aec) (BuildId: d44700e52b2315b458e122acf4a7dd36326c741c)
10
11  Member fields were destroyed
12    [#0](/bitcoin-bitcoin/0/) 0xaaaad19a4b34 in __sanitizer_dtor_callback_fields (/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/bin/fuzz+0xeb4b34) (BuildId: d44700e52b2315b458e122acf4a7dd36326c741c)
13    [#1](/bitcoin-bitcoin/1/) 0xaaaad192b7ec in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>::~basic_string() /cxx_build/include/c++/v1/string:903:3
14    [#2](/bitcoin-bitcoin/2/) 0xaaaad192b7ec in std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>>::~basic_string() /cxx_build/include/c++/v1/string:1208:3
15    [#3](/bitcoin-bitcoin/3/) 0xaaaad192b7ec in std::__1::pair<std::__1::basic_string<char, std::__1::char_traits<char>, std::__1::allocator<char>> const, script_verify_flag_name>::~pair() /cxx_build/include/c++/v1/__utility/pair.h:90:8
16    [#4](/bitcoin-bitcoin/4/) 0xaaaad192b7ec in __cxx_global_var_init.18 /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/src/./script/interpreter.cpp:2167:54
17    [#5](/bitcoin-bitcoin/5/) 0xaaaad192b7ec in _GLOBAL__sub_I_interpreter.cpp /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/src/./script/interpreter.cpp
18    [#6](/bitcoin-bitcoin/6/) 0xffff87fb8610 in __libc_start_main (/lib/aarch64-linux-gnu/libc.so.6+0x28610) (BuildId: d6c205bda1b6e91815f8fef45bdf56bc2239c37e)
19    [#7](/bitcoin-bitcoin/7/) 0xaaaad1931aec in _start (/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/bin/fuzz+0xe41aec) (BuildId: d44700e52b2315b458e122acf4a7dd36326c741c)
20
21SUMMARY: MemorySanitizer: use-of-uninitialized-value /ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/src/test/fuzz/util/./test/fuzz/fuzz.cpp:52:5 in SetArgs(int, char**)
22Exiting
23Traceback (most recent call last):
24  File "/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/test/fuzz/test_runner.py", line 404, in <module>
25    main()
26  File "/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/test/fuzz/test_runner.py", line 111, in main
27    test_list_all = parse_test_list(
28                    ^^^^^^^^^^^^^^^^
29  File "/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/test/fuzz/test_runner.py", line 390, in parse_test_list
30    test_list_all = subprocess.run(
31                    ^^^^^^^^^^^^^^^
32  File "/usr/lib/python3.12/subprocess.py", line 571, in run
33    raise CalledProcessError(retcode, process.args,
34subprocess.CalledProcessError: Command '/ci_container_base/ci/scratch/build-aarch64-unknown-linux-gnu/bin/fuzz' returned non-zero exit status 1.
35Command '['./ci/test/02_run_container.sh']' returned non-zero exit status 1.

    This pull:

    (all good)

  6. in src/test/transaction_tests.cpp:53 in faa9d10c84 
    49@@ -50,7 +50,7 @@ typedef std::vector<unsigned char> valtype;
50 static CFeeRate g_dust{DUST_RELAY_TX_FEE};
51 static bool g_bare_multi{DEFAULT_PERMIT_BAREMULTISIG};
52 
53-static const std::map<std::string, script_verify_flag_name>& mapFlagNames = g_verify_flag_names;
54+static const std::map<std::string, script_verify_flag_name>& mapFlagNames = ScriptFlagNamesToEnum();
    ajtowns commented at 7:06 am on October 12, 2025:
    static const auto& mFN = SFNTE(); ?
    maflcko commented at 2:22 pm on October 15, 2025:
    thx, may use auto& when I have to re-touch.
  7. kevkevinpal commented at 4:29 pm on October 12, 2025: contributor

    ACK faa9d10

    Looks good to me, makes sense to follow the construct on first use idiom.

    I also did a quick grep to see if g_verify_flag_names was used anywhere else, and it was not

  8. janb84 commented at 12:01 pm on October 13, 2025: contributor

    lgtm ACK faa9d10c84bc6b465cbca266468990cc716b4300

    PR refactors g_verify_flag_names init to use the “Construct on first use” Idiom to prevent a “static initialization order fiasco” error. This currently gives some false positives, imho a valid change to remove the false positives and to prevent a “static initialization order fiasco” type of error in the future.

    • code review ✅
    • build en tests run ✅
  9. in src/script/interpreter.cpp:2170 in faa9d10c84 
    2188-    FLAG_NAME(DISCOURAGE_UPGRADABLE_PUBKEYTYPE),
2189-    FLAG_NAME(DISCOURAGE_OP_SUCCESS),
2190-    FLAG_NAME(DISCOURAGE_UPGRADABLE_TAPROOT_VERSION),
2191-};
2192+    static const std::map<std::string, script_verify_flag_name> g_names_to_enum{
2193+        FLAG_NAME(P2SH),
    stickies-v commented at 6:32 pm on October 13, 2025:

    An alternative approach would be to use a constexpr array and make the whole thing compile-time? As a nice benefit, removes the FLAG_NAME macro which imo doesn’t simplify things enough to be worth it.

      0diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
  1index abd99fc365..731c0a070f 100644
  2--- a/src/script/interpreter.cpp
  3+++ b/src/script/interpreter.cpp
  4@@ -2163,36 +2163,6 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
  5     return 0;
  6 }
  7 
  8-const std::map<std::string, script_verify_flag_name>& ScriptFlagNamesToEnum()
  9-{
 10-#define FLAG_NAME(flag) {std::string(#flag), SCRIPT_VERIFY_##flag}
 11-    static const std::map<std::string, script_verify_flag_name> g_names_to_enum{
 12-        FLAG_NAME(P2SH),
 13-        FLAG_NAME(STRICTENC),
 14-        FLAG_NAME(DERSIG),
 15-        FLAG_NAME(LOW_S),
 16-        FLAG_NAME(SIGPUSHONLY),
 17-        FLAG_NAME(MINIMALDATA),
 18-        FLAG_NAME(NULLDUMMY),
 19-        FLAG_NAME(DISCOURAGE_UPGRADABLE_NOPS),
 20-        FLAG_NAME(CLEANSTACK),
 21-        FLAG_NAME(MINIMALIF),
 22-        FLAG_NAME(NULLFAIL),
 23-        FLAG_NAME(CHECKLOCKTIMEVERIFY),
 24-        FLAG_NAME(CHECKSEQUENCEVERIFY),
 25-        FLAG_NAME(WITNESS),
 26-        FLAG_NAME(DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM),
 27-        FLAG_NAME(WITNESS_PUBKEYTYPE),
 28-        FLAG_NAME(CONST_SCRIPTCODE),
 29-        FLAG_NAME(TAPROOT),
 30-        FLAG_NAME(DISCOURAGE_UPGRADABLE_PUBKEYTYPE),
 31-        FLAG_NAME(DISCOURAGE_OP_SUCCESS),
 32-        FLAG_NAME(DISCOURAGE_UPGRADABLE_TAPROOT_VERSION),
 33-    };
 34-#undef FLAG_NAME
 35-    return g_names_to_enum;
 36-}
 37-
 38 std::vector<std::string> GetScriptFlagNames(script_verify_flags flags)
 39 {
 40     std::vector<std::string> res;
 41@@ -2200,9 +2170,9 @@ std::vector<std::string> GetScriptFlagNames(script_verify_flags flags)
 42         return res;
 43     }
 44     script_verify_flags leftover = flags;
 45-    for (const auto& [name, flag] : ScriptFlagNamesToEnum()) {
 46+    for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
 47         if ((flags & flag) != 0) {
 48-            res.push_back(name);
 49+            res.emplace_back(name);
 50             leftover &= ~flag;
 51         }
 52     }
 53diff --git a/src/script/interpreter.h b/src/script/interpreter.h
 54index 7f8a1f7a41..04611bc5d6 100644
 55--- a/src/script/interpreter.h
 56+++ b/src/script/interpreter.h
 57@@ -381,7 +381,29 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
 58 
 59 int FindAndDelete(CScript& script, const CScript& b);
 60 
 61-const std::map<std::string, script_verify_flag_name>& ScriptFlagNamesToEnum();
 62+inline constexpr std::array SCRIPT_FLAG_NAMES{std::to_array<std::pair<std::string_view, script_verify_flag_name>>({
 63+    {"CHECKLOCKTIMEVERIFY", SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY},
 64+    {"CHECKSEQUENCEVERIFY", SCRIPT_VERIFY_CHECKSEQUENCEVERIFY},
 65+    {"CLEANSTACK", SCRIPT_VERIFY_CLEANSTACK},
 66+    {"CONST_SCRIPTCODE", SCRIPT_VERIFY_CONST_SCRIPTCODE},
 67+    {"DERSIG", SCRIPT_VERIFY_DERSIG},
 68+    {"DISCOURAGE_OP_SUCCESS", SCRIPT_VERIFY_DISCOURAGE_OP_SUCCESS},
 69+    {"DISCOURAGE_UPGRADABLE_NOPS", SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS},
 70+    {"DISCOURAGE_UPGRADABLE_PUBKEYTYPE", SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_PUBKEYTYPE},
 71+    {"DISCOURAGE_UPGRADABLE_TAPROOT_VERSION", SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_TAPROOT_VERSION},
 72+    {"DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM", SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM},
 73+    {"LOW_S", SCRIPT_VERIFY_LOW_S},
 74+    {"MINIMALDATA", SCRIPT_VERIFY_MINIMALDATA},
 75+    {"MINIMALIF", SCRIPT_VERIFY_MINIMALIF},
 76+    {"NULLDUMMY", SCRIPT_VERIFY_NULLDUMMY},
 77+    {"NULLFAIL", SCRIPT_VERIFY_NULLFAIL},
 78+    {"P2SH", SCRIPT_VERIFY_P2SH},
 79+    {"SIGPUSHONLY", SCRIPT_VERIFY_SIGPUSHONLY},
 80+    {"STRICTENC", SCRIPT_VERIFY_STRICTENC},
 81+    {"TAPROOT", SCRIPT_VERIFY_TAPROOT},
 82+    {"WITNESS", SCRIPT_VERIFY_WITNESS},
 83+    {"WITNESS_PUBKEYTYPE", SCRIPT_VERIFY_WITNESS_PUBKEYTYPE},
 84+})};
 85 
 86 std::vector<std::string> GetScriptFlagNames(script_verify_flags flags);
 87 
 88diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
 89index 0ab9fbb479..e234153825 100644
 90--- a/src/test/transaction_tests.cpp
 91+++ b/src/test/transaction_tests.cpp
 92@@ -50,7 +50,6 @@ typedef std::vector<unsigned char> valtype;
 93 static CFeeRate g_dust{DUST_RELAY_TX_FEE};
 94 static bool g_bare_multi{DEFAULT_PERMIT_BAREMULTISIG};
 95 
 96-static const std::map<std::string, script_verify_flag_name>& mapFlagNames = ScriptFlagNamesToEnum();
 97 
 98 script_verify_flags ParseScriptFlags(std::string strFlags)
 99 {
100@@ -60,20 +59,21 @@ script_verify_flags ParseScriptFlags(std::string strFlags)
101     std::vector<std::string> words = SplitString(strFlags, ',');
102     for (const std::string& word : words)
103     {
104-        if (!mapFlagNames.count(word)) {
105+        auto it = std::ranges::find(SCRIPT_FLAG_NAMES, word, &decltype(SCRIPT_FLAG_NAMES)::value_type::first);
106+        if (it == SCRIPT_FLAG_NAMES.end()) {
107             BOOST_ERROR("Bad test: unknown verification flag '" << word << "'");
108             continue;
109         }
110-        flags |= mapFlagNames.at(word);
111+        flags |= it->second;
112     }
113     return flags;
114 }
115 
116-// Check that all flags in STANDARD_SCRIPT_VERIFY_FLAGS are present in mapFlagNames.
117-bool CheckMapFlagNames()
118+// Check that all flags in STANDARD_SCRIPT_VERIFY_FLAGS are present in SCRIPT_FLAG_NAMES.
119+bool CheckScriptFlagNames()
120 {
121     script_verify_flags standard_flags_missing{STANDARD_SCRIPT_VERIFY_FLAGS};
122-    for (const auto& pair : mapFlagNames) {
123+    for (const auto& pair : SCRIPT_FLAG_NAMES) {
124         standard_flags_missing &= ~(pair.second);
125     }
126     return standard_flags_missing == 0;
127@@ -142,11 +142,11 @@ script_verify_flags FillFlags(script_verify_flags flags)
128 // Exclude each possible script verify flag from flags. Returns a set of these flag combinations
129 // that are valid and without duplicates. For example: if flags=1111 and the 4 possible flags are
130 // 0001, 0010, 0100, and 1000, this should return the set {0111, 1011, 1101, 1110}.
131-// Assumes that mapFlagNames contains all script verify flags.
132+// Assumes that SCRIPT_FLAG_NAMES contains all script verify flags.
133 std::set<script_verify_flags> ExcludeIndividualFlags(script_verify_flags flags)
134 {
135     std::set<script_verify_flags> flags_combos;
136-    for (const auto& pair : mapFlagNames) {
137+    for (const auto& pair : SCRIPT_FLAG_NAMES) {
138         script_verify_flags flags_excluding_one = TrimFlags(flags & ~(pair.second));
139         if (flags != flags_excluding_one) {
140             flags_combos.insert(flags_excluding_one);
141@@ -159,7 +159,7 @@ BOOST_FIXTURE_TEST_SUITE(transaction_tests, BasicTestingSetup)
142 
143 BOOST_AUTO_TEST_CASE(tx_valid)
144 {
145-    BOOST_CHECK_MESSAGE(CheckMapFlagNames(), "mapFlagNames is missing a script verification flag");
146+    BOOST_CHECK_MESSAGE(CheckScriptFlagNames(), "SCRIPT_FLAG_NAMES is missing a script verification flag");
147     // Read tests from test/data/tx_valid.json
148     UniValue tests = read_json(json_tests::tx_valid);
149 
150@@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
151                                 "Tx unexpectedly failed: " << strTest);
152 
153             // Backwards compatibility of script verification flags: Removing any flag(s) should not invalidate a valid transaction
154-            for (const auto& [name, flag] : mapFlagNames) {
155+            for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
156                 // Removing individual flags
157                 script_verify_flags flags = TrimFlags(~(verify_flags | flag));
158                 if (!CheckTxScripts(tx, mapprevOutScriptPubKeys, mapprevOutValues, flags, txdata, strTest, /*expect_valid=*/true)) {
159@@ -314,7 +314,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
160                                 "Tx unexpectedly passed: " << strTest);
161 
162             // Backwards compatibility of script verification flags: Adding any flag(s) should not validate an invalid transaction
163-            for (const auto& [name, flag] : mapFlagNames) {
164+            for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
165                 script_verify_flags flags = FillFlags(verify_flags | flag);
166                 // Adding individual flags
167                 if (!CheckTxScripts(tx, mapprevOutScriptPubKeys, mapprevOutValues, flags, txdata, strTest, /*expect_valid=*/false)) {
    ajtowns commented at 8:35 pm on October 13, 2025:
    The value of the FLAG_NAME macro is that it makes any typos in the strings that would cause a mismatch with the enum names an immediate compile time error. It also makes it easier to type and easier to review. Massive -1 on dropping it. C++26 reflection would be better, when it’s available, of course.
    ajtowns commented at 8:56 pm on October 13, 2025:

    I had a look at this approach too. With C++20, I believe you can ensure the array is sorted at compile time with:

     0template<typename V, size_t N>
 1consteval auto SortedArray(const std::array<V,N>& pairs)
 2{
 3    std::array<V,N> result = pairs;
 4    std::sort(result.begin(), result.end());
 5    return result;
 6}
 7#define FLAG_NAME(flag) std::pair<std::string_view, script_verify_flag_name>{#flag, SCRIPT_VERIFY_##flag}
 8constexpr auto g_verify_flag_names = SortedArray(std::array{
 9    FLAG_NAME(P2SH),
10    ...
11});

    which lets the compiler avoid one source of bugs. static_assert(std::ranges::is_sorted(g_verify_flag_names)) might be equally effective though. std::ranges:binary_search then lets you do lookups essentially the same way map does, though I suppose with only up to ~60 entries, that doesn’t matter that much.

    Having the initialization be private to interpreter.cpp and exposing functions that (a) converts flags to a string, (b) looks up a flag from a string, (c) return a span over the array seem like it would cover all the uses for the info.

    maflcko commented at 2:05 pm on October 15, 2025:

    std::ranges:binary_search then lets you do lookups essentially the same way map does, though I suppose with only up to ~60 entries, that doesn’t matter that much.

    I think binary_search returns a bool, not an iterator, so it can not be used for lookup. I think you just meant to say “Binary search operations” (https://en.cppreference.com/w/cpp/algorithm/ranges.html#Binary_search_operations_.28on_sorted_ranges.29)

    I think I’ll keep this as-is for now. I can push the array approach (keeping the macro, but I don’t think it matters much.

    Also has a std::ranges::sort, but I’ll probably drop that too, if I end up pushing this.

      0diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
  1index abd99fc365..731c0a070f 100644
  2--- a/src/script/interpreter.cpp
  3+++ b/src/script/interpreter.cpp
  4@@ -2163,36 +2163,6 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
  5     return 0;
  6 }
  7 
  8-const std::map<std::string, script_verify_flag_name>& ScriptFlagNamesToEnum()
  9-{
 10-#define FLAG_NAME(flag) {std::string(#flag), SCRIPT_VERIFY_##flag}
 11-    static const std::map<std::string, script_verify_flag_name> g_names_to_enum{
 12-        FLAG_NAME(P2SH),
 13-        FLAG_NAME(STRICTENC),
 14-        FLAG_NAME(DERSIG),
 15-        FLAG_NAME(LOW_S),
 16-        FLAG_NAME(SIGPUSHONLY),
 17-        FLAG_NAME(MINIMALDATA),
 18-        FLAG_NAME(NULLDUMMY),
 19-        FLAG_NAME(DISCOURAGE_UPGRADABLE_NOPS),
 20-        FLAG_NAME(CLEANSTACK),
 21-        FLAG_NAME(MINIMALIF),
 22-        FLAG_NAME(NULLFAIL),
 23-        FLAG_NAME(CHECKLOCKTIMEVERIFY),
 24-        FLAG_NAME(CHECKSEQUENCEVERIFY),
 25-        FLAG_NAME(WITNESS),
 26-        FLAG_NAME(DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM),
 27-        FLAG_NAME(WITNESS_PUBKEYTYPE),
 28-        FLAG_NAME(CONST_SCRIPTCODE),
 29-        FLAG_NAME(TAPROOT),
 30-        FLAG_NAME(DISCOURAGE_UPGRADABLE_PUBKEYTYPE),
 31-        FLAG_NAME(DISCOURAGE_OP_SUCCESS),
 32-        FLAG_NAME(DISCOURAGE_UPGRADABLE_TAPROOT_VERSION),
 33-    };
 34-#undef FLAG_NAME
 35-    return g_names_to_enum;
 36-}
 37-
 38 std::vector<std::string> GetScriptFlagNames(script_verify_flags flags)
 39 {
 40     std::vector<std::string> res;
 41@@ -2200,9 +2170,9 @@ std::vector<std::string> GetScriptFlagNames(script_verify_flags flags)
 42         return res;
 43     }
 44     script_verify_flags leftover = flags;
 45-    for (const auto& [name, flag] : ScriptFlagNamesToEnum()) {
 46+    for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
 47         if ((flags & flag) != 0) {
 48-            res.push_back(name);
 49+            res.emplace_back(name);
 50             leftover &= ~flag;
 51         }
 52     }
 53diff --git a/src/script/interpreter.h b/src/script/interpreter.h
 54index 7f8a1f7a41..54d7d2e933 100644
 55--- a/src/script/interpreter.h
 56+++ b/src/script/interpreter.h
 57@@ -381,7 +381,35 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
 58 
 59 int FindAndDelete(CScript& script, const CScript& b);
 60 
 61-const std::map<std::string, script_verify_flag_name>& ScriptFlagNamesToEnum();
 62+inline constexpr std::array SCRIPT_FLAG_NAMES{[]() {
 63+#define FLAG_NAME(flag) std::pair<std::string_view, script_verify_flag_name>{#flag, SCRIPT_VERIFY_##flag}
 64+    std::array result{
 65+        FLAG_NAME(P2SH),
 66+        FLAG_NAME(STRICTENC),
 67+        FLAG_NAME(DERSIG),
 68+        FLAG_NAME(LOW_S),
 69+        FLAG_NAME(SIGPUSHONLY),
 70+        FLAG_NAME(MINIMALDATA),
 71+        FLAG_NAME(NULLDUMMY),
 72+        FLAG_NAME(DISCOURAGE_UPGRADABLE_NOPS),
 73+        FLAG_NAME(CLEANSTACK),
 74+        FLAG_NAME(MINIMALIF),
 75+        FLAG_NAME(NULLFAIL),
 76+        FLAG_NAME(CHECKLOCKTIMEVERIFY),
 77+        FLAG_NAME(CHECKSEQUENCEVERIFY),
 78+        FLAG_NAME(WITNESS),
 79+        FLAG_NAME(DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM),
 80+        FLAG_NAME(WITNESS_PUBKEYTYPE),
 81+        FLAG_NAME(CONST_SCRIPTCODE),
 82+        FLAG_NAME(TAPROOT),
 83+        FLAG_NAME(DISCOURAGE_UPGRADABLE_PUBKEYTYPE),
 84+        FLAG_NAME(DISCOURAGE_OP_SUCCESS),
 85+        FLAG_NAME(DISCOURAGE_UPGRADABLE_TAPROOT_VERSION),
 86+    };
 87+    std::ranges::sort(result);
 88+    return result;
 89+#undef FLAG_NAME
 90+}()};
 91 
 92 std::vector<std::string> GetScriptFlagNames(script_verify_flags flags);
 93 
 94diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
 95index 0ab9fbb479..e234153825 100644
 96--- a/src/test/transaction_tests.cpp
 97+++ b/src/test/transaction_tests.cpp
 98@@ -50,7 +50,6 @@ typedef std::vector<unsigned char> valtype;
 99 static CFeeRate g_dust{DUST_RELAY_TX_FEE};
100 static bool g_bare_multi{DEFAULT_PERMIT_BAREMULTISIG};
101 
102-static const std::map<std::string, script_verify_flag_name>& mapFlagNames = ScriptFlagNamesToEnum();
103 
104 script_verify_flags ParseScriptFlags(std::string strFlags)
105 {
106@@ -60,20 +59,21 @@ script_verify_flags ParseScriptFlags(std::string strFlags)
107     std::vector<std::string> words = SplitString(strFlags, ',');
108     for (const std::string& word : words)
109     {
110-        if (!mapFlagNames.count(word)) {
111+        auto it = std::ranges::find(SCRIPT_FLAG_NAMES, word, &decltype(SCRIPT_FLAG_NAMES)::value_type::first);
112+        if (it == SCRIPT_FLAG_NAMES.end()) {
113             BOOST_ERROR("Bad test: unknown verification flag '" << word << "'");
114             continue;
115         }
116-        flags |= mapFlagNames.at(word);
117+        flags |= it->second;
118     }
119     return flags;
120 }
121 
122-// Check that all flags in STANDARD_SCRIPT_VERIFY_FLAGS are present in mapFlagNames.
123-bool CheckMapFlagNames()
124+// Check that all flags in STANDARD_SCRIPT_VERIFY_FLAGS are present in SCRIPT_FLAG_NAMES.
125+bool CheckScriptFlagNames()
126 {
127     script_verify_flags standard_flags_missing{STANDARD_SCRIPT_VERIFY_FLAGS};
128-    for (const auto& pair : mapFlagNames) {
129+    for (const auto& pair : SCRIPT_FLAG_NAMES) {
130         standard_flags_missing &= ~(pair.second);
131     }
132     return standard_flags_missing == 0;
133@@ -142,11 +142,11 @@ script_verify_flags FillFlags(script_verify_flags flags)
134 // Exclude each possible script verify flag from flags. Returns a set of these flag combinations
135 // that are valid and without duplicates. For example: if flags=1111 and the 4 possible flags are
136 // 0001, 0010, 0100, and 1000, this should return the set {0111, 1011, 1101, 1110}.
137-// Assumes that mapFlagNames contains all script verify flags.
138+// Assumes that SCRIPT_FLAG_NAMES contains all script verify flags.
139 std::set<script_verify_flags> ExcludeIndividualFlags(script_verify_flags flags)
140 {
141     std::set<script_verify_flags> flags_combos;
142-    for (const auto& pair : mapFlagNames) {
143+    for (const auto& pair : SCRIPT_FLAG_NAMES) {
144         script_verify_flags flags_excluding_one = TrimFlags(flags & ~(pair.second));
145         if (flags != flags_excluding_one) {
146             flags_combos.insert(flags_excluding_one);
147@@ -159,7 +159,7 @@ BOOST_FIXTURE_TEST_SUITE(transaction_tests, BasicTestingSetup)
148 
149 BOOST_AUTO_TEST_CASE(tx_valid)
150 {
151-    BOOST_CHECK_MESSAGE(CheckMapFlagNames(), "mapFlagNames is missing a script verification flag");
152+    BOOST_CHECK_MESSAGE(CheckScriptFlagNames(), "SCRIPT_FLAG_NAMES is missing a script verification flag");
153     // Read tests from test/data/tx_valid.json
154     UniValue tests = read_json(json_tests::tx_valid);
155 
156@@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
157                                 "Tx unexpectedly failed: " << strTest);
158 
159             // Backwards compatibility of script verification flags: Removing any flag(s) should not invalidate a valid transaction
160-            for (const auto& [name, flag] : mapFlagNames) {
161+            for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
162                 // Removing individual flags
163                 script_verify_flags flags = TrimFlags(~(verify_flags | flag));
164                 if (!CheckTxScripts(tx, mapprevOutScriptPubKeys, mapprevOutValues, flags, txdata, strTest, /*expect_valid=*/true)) {
165@@ -314,7 +314,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
166                                 "Tx unexpectedly passed: " << strTest);
167 
168             // Backwards compatibility of script verification flags: Adding any flag(s) should not validate an invalid transaction
169-            for (const auto& [name, flag] : mapFlagNames) {
170+            for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
171                 script_verify_flags flags = FillFlags(verify_flags | flag);
172                 // Adding individual flags
173                 if (!CheckTxScripts(tx, mapprevOutScriptPubKeys, mapprevOutValues, flags, txdata, strTest, /*expect_valid=*/false)) {
  10. stickies-v approved
  11. stickies-v commented at 6:44 pm on October 13, 2025: contributor

    ACK faa9d10c84bc6b465cbca266468990cc716b4300

    I would prefer the constexpr approach, but either approach is acceptable to me.

  12. ajtowns commented at 2:44 pm on October 20, 2025: contributor

    ACK faa9d10c84bc6b465cbca266468990cc716b4300

    Didn’t realise this wasn’t already merged. This adds a trivial implicit mutex when this variable is accessed which is a little lame, but it’s not accessed in performance critical places, and even if it were, it’s fast anyway.

  13. maflcko commented at 9:49 am on October 21, 2025: member
    For reference, the msan false-positive can also be worked around via #33666. However, I still think this pull makes sense, because it is already reviewed and still comes with the other mentioned benefits at basically no cost. (If there were a cost, we could go with the compile-time stuff, but that diff is a bit larger, see above).
  14. glozow merged this on Oct 23, 2025
  15. glozow closed this on Oct 23, 2025
  16. maflcko deleted the branch on Oct 23, 2025


maflcko DrahtBot ajtowns kevkevinpal janb84 stickies-v

Labels
Refactoring

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2025-11-02 12:13 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me