refactor: Construct g_verify_flag_names on first use #33600

static const auto& mFN = SFNTE(); ?

thx, may use auto& when I have to re-touch.

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.

<details> <summary>git diff on faa9d10c84</summary>

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index abd99fc365..731c0a070f 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -2163,36 +2163,6 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
     return 0;
 }
 
-const std::map<std::string, script_verify_flag_name>& ScriptFlagNamesToEnum()
-{
-#define FLAG_NAME(flag) {std::string(#flag), SCRIPT_VERIFY_##flag}
-    static const std::map<std::string, script_verify_flag_name> g_names_to_enum{
-        FLAG_NAME(P2SH),
-        FLAG_NAME(STRICTENC),
-        FLAG_NAME(DERSIG),
-        FLAG_NAME(LOW_S),
-        FLAG_NAME(SIGPUSHONLY),
-        FLAG_NAME(MINIMALDATA),
-        FLAG_NAME(NULLDUMMY),
-        FLAG_NAME(DISCOURAGE_UPGRADABLE_NOPS),
-        FLAG_NAME(CLEANSTACK),
-        FLAG_NAME(MINIMALIF),
-        FLAG_NAME(NULLFAIL),
-        FLAG_NAME(CHECKLOCKTIMEVERIFY),
-        FLAG_NAME(CHECKSEQUENCEVERIFY),
-        FLAG_NAME(WITNESS),
-        FLAG_NAME(DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM),
-        FLAG_NAME(WITNESS_PUBKEYTYPE),
-        FLAG_NAME(CONST_SCRIPTCODE),
-        FLAG_NAME(TAPROOT),
-        FLAG_NAME(DISCOURAGE_UPGRADABLE_PUBKEYTYPE),
-        FLAG_NAME(DISCOURAGE_OP_SUCCESS),
-        FLAG_NAME(DISCOURAGE_UPGRADABLE_TAPROOT_VERSION),
-    };
-#undef FLAG_NAME
-    return g_names_to_enum;
-}
-
 std::vector<std::string> GetScriptFlagNames(script_verify_flags flags)
 {
     std::vector<std::string> res;
@@ -2200,9 +2170,9 @@ std::vector<std::string> GetScriptFlagNames(script_verify_flags flags)
         return res;
     }
     script_verify_flags leftover = flags;
-    for (const auto& [name, flag] : ScriptFlagNamesToEnum()) {
+    for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
         if ((flags & flag) != 0) {
-            res.push_back(name);
+            res.emplace_back(name);
             leftover &= ~flag;
         }
     }
diff --git a/src/script/interpreter.h b/src/script/interpreter.h
index 7f8a1f7a41..04611bc5d6 100644
--- a/src/script/interpreter.h
+++ b/src/script/interpreter.h
@@ -381,7 +381,29 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
 
 int FindAndDelete(CScript& script, const CScript& b);
 
-const std::map<std::string, script_verify_flag_name>& ScriptFlagNamesToEnum();
+inline constexpr std::array SCRIPT_FLAG_NAMES{std::to_array<std::pair<std::string_view, script_verify_flag_name>>({
+    {"CHECKLOCKTIMEVERIFY", SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY},
+    {"CHECKSEQUENCEVERIFY", SCRIPT_VERIFY_CHECKSEQUENCEVERIFY},
+    {"CLEANSTACK", SCRIPT_VERIFY_CLEANSTACK},
+    {"CONST_SCRIPTCODE", SCRIPT_VERIFY_CONST_SCRIPTCODE},
+    {"DERSIG", SCRIPT_VERIFY_DERSIG},
+    {"DISCOURAGE_OP_SUCCESS", SCRIPT_VERIFY_DISCOURAGE_OP_SUCCESS},
+    {"DISCOURAGE_UPGRADABLE_NOPS", SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS},
+    {"DISCOURAGE_UPGRADABLE_PUBKEYTYPE", SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_PUBKEYTYPE},
+    {"DISCOURAGE_UPGRADABLE_TAPROOT_VERSION", SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_TAPROOT_VERSION},
+    {"DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM", SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM},
+    {"LOW_S", SCRIPT_VERIFY_LOW_S},
+    {"MINIMALDATA", SCRIPT_VERIFY_MINIMALDATA},
+    {"MINIMALIF", SCRIPT_VERIFY_MINIMALIF},
+    {"NULLDUMMY", SCRIPT_VERIFY_NULLDUMMY},
+    {"NULLFAIL", SCRIPT_VERIFY_NULLFAIL},
+    {"P2SH", SCRIPT_VERIFY_P2SH},
+    {"SIGPUSHONLY", SCRIPT_VERIFY_SIGPUSHONLY},
+    {"STRICTENC", SCRIPT_VERIFY_STRICTENC},
+    {"TAPROOT", SCRIPT_VERIFY_TAPROOT},
+    {"WITNESS", SCRIPT_VERIFY_WITNESS},
+    {"WITNESS_PUBKEYTYPE", SCRIPT_VERIFY_WITNESS_PUBKEYTYPE},
+})};
 
 std::vector<std::string> GetScriptFlagNames(script_verify_flags flags);
 
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index 0ab9fbb479..e234153825 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -50,7 +50,6 @@ typedef std::vector<unsigned char> valtype;
 static CFeeRate g_dust{DUST_RELAY_TX_FEE};
 static bool g_bare_multi{DEFAULT_PERMIT_BAREMULTISIG};
 
-static const std::map<std::string, script_verify_flag_name>& mapFlagNames = ScriptFlagNamesToEnum();
 
 script_verify_flags ParseScriptFlags(std::string strFlags)
 {
@@ -60,20 +59,21 @@ script_verify_flags ParseScriptFlags(std::string strFlags)
     std::vector<std::string> words = SplitString(strFlags, ',');
     for (const std::string& word : words)
     {
-        if (!mapFlagNames.count(word)) {
+        auto it = std::ranges::find(SCRIPT_FLAG_NAMES, word, &decltype(SCRIPT_FLAG_NAMES)::value_type::first);
+        if (it == SCRIPT_FLAG_NAMES.end()) {
             BOOST_ERROR("Bad test: unknown verification flag '" << word << "'");
             continue;
         }
-        flags |= mapFlagNames.at(word);
+        flags |= it->second;
     }
     return flags;
 }
 
-// Check that all flags in STANDARD_SCRIPT_VERIFY_FLAGS are present in mapFlagNames.
-bool CheckMapFlagNames()
+// Check that all flags in STANDARD_SCRIPT_VERIFY_FLAGS are present in SCRIPT_FLAG_NAMES.
+bool CheckScriptFlagNames()
 {
     script_verify_flags standard_flags_missing{STANDARD_SCRIPT_VERIFY_FLAGS};
-    for (const auto& pair : mapFlagNames) {
+    for (const auto& pair : SCRIPT_FLAG_NAMES) {
         standard_flags_missing &= ~(pair.second);
     }
     return standard_flags_missing == 0;
@@ -142,11 +142,11 @@ script_verify_flags FillFlags(script_verify_flags flags)
 // Exclude each possible script verify flag from flags. Returns a set of these flag combinations
 // that are valid and without duplicates. For example: if flags=1111 and the 4 possible flags are
 // 0001, 0010, 0100, and 1000, this should return the set {0111, 1011, 1101, 1110}.
-// Assumes that mapFlagNames contains all script verify flags.
+// Assumes that SCRIPT_FLAG_NAMES contains all script verify flags.
 std::set<script_verify_flags> ExcludeIndividualFlags(script_verify_flags flags)
 {
     std::set<script_verify_flags> flags_combos;
-    for (const auto& pair : mapFlagNames) {
+    for (const auto& pair : SCRIPT_FLAG_NAMES) {
         script_verify_flags flags_excluding_one = TrimFlags(flags & ~(pair.second));
         if (flags != flags_excluding_one) {
             flags_combos.insert(flags_excluding_one);
@@ -159,7 +159,7 @@ BOOST_FIXTURE_TEST_SUITE(transaction_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(tx_valid)
 {
-    BOOST_CHECK_MESSAGE(CheckMapFlagNames(), "mapFlagNames is missing a script verification flag");
+    BOOST_CHECK_MESSAGE(CheckScriptFlagNames(), "SCRIPT_FLAG_NAMES is missing a script verification flag");
     // Read tests from test/data/tx_valid.json
     UniValue tests = read_json(json_tests::tx_valid);
 
@@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
                                 "Tx unexpectedly failed: " << strTest);
 
             // Backwards compatibility of script verification flags: Removing any flag(s) should not invalidate a valid transaction
-            for (const auto& [name, flag] : mapFlagNames) {
+            for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
                 // Removing individual flags
                 script_verify_flags flags = TrimFlags(~(verify_flags | flag));
                 if (!CheckTxScripts(tx, mapprevOutScriptPubKeys, mapprevOutValues, flags, txdata, strTest, /*expect_valid=*/true)) {
@@ -314,7 +314,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
                                 "Tx unexpectedly passed: " << strTest);
 
             // Backwards compatibility of script verification flags: Adding any flag(s) should not validate an invalid transaction
-            for (const auto& [name, flag] : mapFlagNames) {
+            for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
                 script_verify_flags flags = FillFlags(verify_flags | flag);
                 // Adding individual flags
                 if (!CheckTxScripts(tx, mapprevOutScriptPubKeys, mapprevOutValues, flags, txdata, strTest, /*expect_valid=*/false)) {

</details>

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.

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

template<typename V, size_t N>
consteval auto SortedArray(const std::array<V,N>& pairs)
{
    std::array<V,N> result = pairs;
    std::sort(result.begin(), result.end());
    return result;
}
#define FLAG_NAME(flag) std::pair<std::string_view, script_verify_flag_name>{#flag, SCRIPT_VERIFY_##flag}
constexpr auto g_verify_flag_names = SortedArray(std::array{
    FLAG_NAME(P2SH),
    ...
});

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.

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.

<details><summary>array approach (keeping the macro)</summary>

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

diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index abd99fc365..731c0a070f 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -2163,36 +2163,6 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
     return 0;
 }
 
-const std::map<std::string, script_verify_flag_name>& ScriptFlagNamesToEnum()
-{
-#define FLAG_NAME(flag) {std::string(#flag), SCRIPT_VERIFY_##flag}
-    static const std::map<std::string, script_verify_flag_name> g_names_to_enum{
-        FLAG_NAME(P2SH),
-        FLAG_NAME(STRICTENC),
-        FLAG_NAME(DERSIG),
-        FLAG_NAME(LOW_S),
-        FLAG_NAME(SIGPUSHONLY),
-        FLAG_NAME(MINIMALDATA),
-        FLAG_NAME(NULLDUMMY),
-        FLAG_NAME(DISCOURAGE_UPGRADABLE_NOPS),
-        FLAG_NAME(CLEANSTACK),
-        FLAG_NAME(MINIMALIF),
-        FLAG_NAME(NULLFAIL),
-        FLAG_NAME(CHECKLOCKTIMEVERIFY),
-        FLAG_NAME(CHECKSEQUENCEVERIFY),
-        FLAG_NAME(WITNESS),
-        FLAG_NAME(DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM),
-        FLAG_NAME(WITNESS_PUBKEYTYPE),
-        FLAG_NAME(CONST_SCRIPTCODE),
-        FLAG_NAME(TAPROOT),
-        FLAG_NAME(DISCOURAGE_UPGRADABLE_PUBKEYTYPE),
-        FLAG_NAME(DISCOURAGE_OP_SUCCESS),
-        FLAG_NAME(DISCOURAGE_UPGRADABLE_TAPROOT_VERSION),
-    };
-#undef FLAG_NAME
-    return g_names_to_enum;
-}
-
 std::vector<std::string> GetScriptFlagNames(script_verify_flags flags)
 {
     std::vector<std::string> res;
@@ -2200,9 +2170,9 @@ std::vector<std::string> GetScriptFlagNames(script_verify_flags flags)
         return res;
     }
     script_verify_flags leftover = flags;
-    for (const auto& [name, flag] : ScriptFlagNamesToEnum()) {
+    for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
         if ((flags & flag) != 0) {
-            res.push_back(name);
+            res.emplace_back(name);
             leftover &= ~flag;
         }
     }
diff --git a/src/script/interpreter.h b/src/script/interpreter.h
index 7f8a1f7a41..54d7d2e933 100644
--- a/src/script/interpreter.h
+++ b/src/script/interpreter.h
@@ -381,7 +381,35 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
 
 int FindAndDelete(CScript& script, const CScript& b);
 
-const std::map<std::string, script_verify_flag_name>& ScriptFlagNamesToEnum();
+inline constexpr std::array SCRIPT_FLAG_NAMES{[]() {
+#define FLAG_NAME(flag) std::pair<std::string_view, script_verify_flag_name>{#flag, SCRIPT_VERIFY_##flag}
+    std::array result{
+        FLAG_NAME(P2SH),
+        FLAG_NAME(STRICTENC),
+        FLAG_NAME(DERSIG),
+        FLAG_NAME(LOW_S),
+        FLAG_NAME(SIGPUSHONLY),
+        FLAG_NAME(MINIMALDATA),
+        FLAG_NAME(NULLDUMMY),
+        FLAG_NAME(DISCOURAGE_UPGRADABLE_NOPS),
+        FLAG_NAME(CLEANSTACK),
+        FLAG_NAME(MINIMALIF),
+        FLAG_NAME(NULLFAIL),
+        FLAG_NAME(CHECKLOCKTIMEVERIFY),
+        FLAG_NAME(CHECKSEQUENCEVERIFY),
+        FLAG_NAME(WITNESS),
+        FLAG_NAME(DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM),
+        FLAG_NAME(WITNESS_PUBKEYTYPE),
+        FLAG_NAME(CONST_SCRIPTCODE),
+        FLAG_NAME(TAPROOT),
+        FLAG_NAME(DISCOURAGE_UPGRADABLE_PUBKEYTYPE),
+        FLAG_NAME(DISCOURAGE_OP_SUCCESS),
+        FLAG_NAME(DISCOURAGE_UPGRADABLE_TAPROOT_VERSION),
+    };
+    std::ranges::sort(result);
+    return result;
+#undef FLAG_NAME
+}()};
 
 std::vector<std::string> GetScriptFlagNames(script_verify_flags flags);
 
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index 0ab9fbb479..e234153825 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -50,7 +50,6 @@ typedef std::vector<unsigned char> valtype;
 static CFeeRate g_dust{DUST_RELAY_TX_FEE};
 static bool g_bare_multi{DEFAULT_PERMIT_BAREMULTISIG};
 
-static const std::map<std::string, script_verify_flag_name>& mapFlagNames = ScriptFlagNamesToEnum();
 
 script_verify_flags ParseScriptFlags(std::string strFlags)
 {
@@ -60,20 +59,21 @@ script_verify_flags ParseScriptFlags(std::string strFlags)
     std::vector<std::string> words = SplitString(strFlags, ',');
     for (const std::string& word : words)
     {
-        if (!mapFlagNames.count(word)) {
+        auto it = std::ranges::find(SCRIPT_FLAG_NAMES, word, &decltype(SCRIPT_FLAG_NAMES)::value_type::first);
+        if (it == SCRIPT_FLAG_NAMES.end()) {
             BOOST_ERROR("Bad test: unknown verification flag '" << word << "'");
             continue;
         }
-        flags |= mapFlagNames.at(word);
+        flags |= it->second;
     }
     return flags;
 }
 
-// Check that all flags in STANDARD_SCRIPT_VERIFY_FLAGS are present in mapFlagNames.
-bool CheckMapFlagNames()
+// Check that all flags in STANDARD_SCRIPT_VERIFY_FLAGS are present in SCRIPT_FLAG_NAMES.
+bool CheckScriptFlagNames()
 {
     script_verify_flags standard_flags_missing{STANDARD_SCRIPT_VERIFY_FLAGS};
-    for (const auto& pair : mapFlagNames) {
+    for (const auto& pair : SCRIPT_FLAG_NAMES) {
         standard_flags_missing &= ~(pair.second);
     }
     return standard_flags_missing == 0;
@@ -142,11 +142,11 @@ script_verify_flags FillFlags(script_verify_flags flags)
 // Exclude each possible script verify flag from flags. Returns a set of these flag combinations
 // that are valid and without duplicates. For example: if flags=1111 and the 4 possible flags are
 // 0001, 0010, 0100, and 1000, this should return the set {0111, 1011, 1101, 1110}.
-// Assumes that mapFlagNames contains all script verify flags.
+// Assumes that SCRIPT_FLAG_NAMES contains all script verify flags.
 std::set<script_verify_flags> ExcludeIndividualFlags(script_verify_flags flags)
 {
     std::set<script_verify_flags> flags_combos;
-    for (const auto& pair : mapFlagNames) {
+    for (const auto& pair : SCRIPT_FLAG_NAMES) {
         script_verify_flags flags_excluding_one = TrimFlags(flags & ~(pair.second));
         if (flags != flags_excluding_one) {
             flags_combos.insert(flags_excluding_one);
@@ -159,7 +159,7 @@ BOOST_FIXTURE_TEST_SUITE(transaction_tests, BasicTestingSetup)
 
 BOOST_AUTO_TEST_CASE(tx_valid)
 {
-    BOOST_CHECK_MESSAGE(CheckMapFlagNames(), "mapFlagNames is missing a script verification flag");
+    BOOST_CHECK_MESSAGE(CheckScriptFlagNames(), "SCRIPT_FLAG_NAMES is missing a script verification flag");
     // Read tests from test/data/tx_valid.json
     UniValue tests = read_json(json_tests::tx_valid);
 
@@ -223,7 +223,7 @@ BOOST_AUTO_TEST_CASE(tx_valid)
                                 "Tx unexpectedly failed: " << strTest);
 
             // Backwards compatibility of script verification flags: Removing any flag(s) should not invalidate a valid transaction
-            for (const auto& [name, flag] : mapFlagNames) {
+            for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
                 // Removing individual flags
                 script_verify_flags flags = TrimFlags(~(verify_flags | flag));
                 if (!CheckTxScripts(tx, mapprevOutScriptPubKeys, mapprevOutValues, flags, txdata, strTest, /*expect_valid=*/true)) {
@@ -314,7 +314,7 @@ BOOST_AUTO_TEST_CASE(tx_invalid)
                                 "Tx unexpectedly passed: " << strTest);
 
             // Backwards compatibility of script verification flags: Adding any flag(s) should not validate an invalid transaction
-            for (const auto& [name, flag] : mapFlagNames) {
+            for (const auto& [name, flag] : SCRIPT_FLAG_NAMES) {
                 script_verify_flags flags = FillFlags(verify_flags | flag);
                 // Adding individual flags
                 if (!CheckTxScripts(tx, mapprevOutScriptPubKeys, mapprevOutValues, flags, txdata, strTest, /*expect_valid=*/false)) {

</details>