Move mapFlagNames and FormatScriptFlags logic to script/interpreter.h
What changed, and why it matters
This commit is a straightforward code cleanup: it moves a helper that converts script verification flags into human-readable names from test files into a shared library location, and adds a small test for it. There is no change to how Bitcoin validates transactions or scripts, and no security fix.
No security action required; this is a benign refactoring. Normal code-review approval is sufficient.
Security signals we found
No strong security signals were identified.
Evidence from the diff
The change relocates the mapFlagNames table and FormatScriptFlags formatting logic from src/test/transaction_tests.cpp into src/script/interpreter.cpp/h as g_verify_flag_names and GetScriptFlagNames(). It also adds a unit test verifying that unknown flag bits are reported as hex. The only functional tweak is in ParseScriptFlags: when an unknown flag string is encountered, the test now logs an error and continues instead of aborting via BOOST_ERROR (which still logs but does not stop execution) and uses .at() rather than operator[]. This is a refactoring/test-hardening change with no consensus or networking impact.
Changed components
src/script/interpreter.cppsrc/script/interpreter.hsrc/test/script_tests.cppsrc/test/transaction_tests.cppsrc/deploymentinfo.hInspect captured patch +74 / −42
diff --git a/src/deploymentinfo.h b/src/deploymentinfo.h
index 10029b0e..3178ea74 100644
--- a/src/deploymentinfo.h
+++ b/src/deploymentinfo.h
@@ -8,8 +8,10 @@
#include <consensus/params.h>
#include <array>
+#include <cassert>
#include <optional>
#include <string>
+#include <string_view>
struct VBDeploymentInfo {
/** Deployment name */
diff --git a/src/script/interpreter.cpp b/src/script/interpreter.cpp
index 07fda48c..85cb7687 100644
--- a/src/script/interpreter.cpp
+++ b/src/script/interpreter.cpp
@@ -10,6 +10,7 @@
#include <crypto/sha256.h>
#include <pubkey.h>
#include <script/script.h>
+#include <tinyformat.h>
#include <uint256.h>
typedef std::vector<unsigned char> valtype;
@@ -2161,3 +2162,48 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
return 0;
}
+
+#define FLAG_NAME(flag) {std::string(#flag), SCRIPT_VERIFY_##flag}
+const std::map<std::string, uint32_t> g_verify_flag_names{
+ 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
+
+std::vector<std::string> GetScriptFlagNames(uint32_t flags)
+{
+ std::vector<std::string> res;
+ if (flags == SCRIPT_VERIFY_NONE) {
+ return res;
+ }
+ uint32_t leftover = flags;
+ for (const auto& [name, flag] : g_verify_flag_names) {
+ if ((flags & flag) != 0) {
+ res.push_back(name);
+ leftover &= ~flag;
+ }
+ }
+ if (leftover != 0) {
+ res.push_back(strprintf("0x%08x", leftover));
+ }
+ return res;
+}
diff --git a/src/script/interpreter.h b/src/script/interpreter.h
index 2108e399..1ce528ac 100644
--- a/src/script/interpreter.h
+++ b/src/script/interpreter.h
@@ -371,4 +371,8 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
int FindAndDelete(CScript& script, const CScript& b);
+extern const std::map<std::string, uint32_t> g_verify_flag_names;
+
+std::vector<std::string> GetScriptFlagNames(uint32_t flags);
+
#endif // BITCOIN_SCRIPT_INTERPRETER_H
diff --git a/src/test/script_tests.cpp b/src/test/script_tests.cpp
index 5d759302..ed68dc49 100644
--- a/src/test/script_tests.cpp
+++ b/src/test/script_tests.cpp
@@ -9,6 +9,7 @@
#include <core_io.h>
#include <key.h>
#include <rpc/util.h>
+#include <script/interpreter.h>
#include <script/script.h>
#include <script/script_error.h>
#include <script/sigcache.h>
@@ -22,6 +23,7 @@
#include <test/util/transaction_utils.h>
#include <util/fs.h>
#include <util/strencodings.h>
+#include <util/string.h>
#include <cstdint>
#include <fstream>
@@ -41,7 +43,6 @@ using namespace util::hex_literals;
static const unsigned int gFlags = SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_STRICTENC;
unsigned int ParseScriptFlags(std::string strFlags);
-std::string FormatScriptFlags(unsigned int flags);
struct ScriptErrorDesc
{
@@ -95,6 +96,11 @@ static ScriptErrorDesc script_errors[]={
{SCRIPT_ERR_SIG_FINDANDDELETE, "SIG_FINDANDDELETE"},
};
+static std::string FormatScriptFlags(uint32_t flags)
+{
+ return util::Join(GetScriptFlagNames(flags), ",");
+}
+
static std::string FormatScriptError(ScriptError_t err)
{
for (const auto& se : script_errors)
@@ -1706,4 +1712,13 @@ BOOST_AUTO_TEST_CASE(compute_tapleaf)
BOOST_CHECK_EQUAL(ComputeTapleafHash(0xc2, std::span(script)), tlc2);
}
+BOOST_AUTO_TEST_CASE(formatscriptflags)
+{
+ // quick check that FormatScriptFlags reports any unknown/unexpected bits
+ BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH), "P2SH");
+ BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH | (1u<<31)), "P2SH,0x80000000");
+ BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_TAPROOT | (1u<<27)), "TAPROOT,0x08000000");
+ BOOST_CHECK_EQUAL(FormatScriptFlags(1u<<26), "0x04000000");
+}
+
BOOST_AUTO_TEST_SUITE_END()
diff --git a/src/test/transaction_tests.cpp b/src/test/transaction_tests.cpp
index c0443f3a..e7b3ee7f 100644
--- a/src/test/transaction_tests.cpp
+++ b/src/test/transaction_tests.cpp
@@ -17,6 +17,7 @@
#include <policy/policy.h>
#include <policy/settings.h>
#include <primitives/transaction_identifier.h>
+#include <script/interpreter.h>
#include <script/script.h>
#include <script/script_error.h>
#include <script/sigcache.h>
@@ -49,29 +50,7 @@ typedef std::vector<unsigned char> valtype;
static CFeeRate g_dust{DUST_RELAY_TX_FEE};
static bool g_bare_multi{DEFAULT_PERMIT_BAREMULTISIG};
-static std::map<std::string, unsigned int> mapFlagNames = {
- {std::string("P2SH"), (unsigned int)SCRIPT_VERIFY_P2SH},
- {std::string("STRICTENC"), (unsigned int)SCRIPT_VERIFY_STRICTENC},
- {std::string("DERSIG"), (unsigned int)SCRIPT_VERIFY_DERSIG},
- {std::string("LOW_S"), (unsigned int)SCRIPT_VERIFY_LOW_S},
- {std::string("SIGPUSHONLY"), (unsigned int)SCRIPT_VERIFY_SIGPUSHONLY},
- {std::string("MINIMALDATA"), (unsigned int)SCRIPT_VERIFY_MINIMALDATA},
- {std::string("NULLDUMMY"), (unsigned int)SCRIPT_VERIFY_NULLDUMMY},
- {std::string("DISCOURAGE_UPGRADABLE_NOPS"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_NOPS},
- {std::string("CLEANSTACK"), (unsigned int)SCRIPT_VERIFY_CLEANSTACK},
- {std::string("MINIMALIF"), (unsigned int)SCRIPT_VERIFY_MINIMALIF},
- {std::string("NULLFAIL"), (unsigned int)SCRIPT_VERIFY_NULLFAIL},
- {std::string("CHECKLOCKTIMEVERIFY"), (unsigned int)SCRIPT_VERIFY_CHECKLOCKTIMEVERIFY},
- {std::string("CHECKSEQUENCEVERIFY"), (unsigned int)SCRIPT_VERIFY_CHECKSEQUENCEVERIFY},
- {std::string("WITNESS"), (unsigned int)SCRIPT_VERIFY_WITNESS},
- {std::string("DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_WITNESS_PROGRAM},
- {std::string("WITNESS_PUBKEYTYPE"), (unsigned int)SCRIPT_VERIFY_WITNESS_PUBKEYTYPE},
- {std::string("CONST_SCRIPTCODE"), (unsigned int)SCRIPT_VERIFY_CONST_SCRIPTCODE},
- {std::string("TAPROOT"), (unsigned int)SCRIPT_VERIFY_TAPROOT},
- {std::string("DISCOURAGE_UPGRADABLE_PUBKEYTYPE"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_PUBKEYTYPE},
- {std::string("DISCOURAGE_OP_SUCCESS"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_OP_SUCCESS},
- {std::string("DISCOURAGE_UPGRADABLE_TAPROOT_VERSION"), (unsigned int)SCRIPT_VERIFY_DISCOURAGE_UPGRADABLE_TAPROOT_VERSION},
-};
+static const std::map<std::string, unsigned int>& mapFlagNames = g_verify_flag_names;
unsigned int ParseScriptFlags(std::string strFlags)
{
@@ -81,9 +60,11 @@ unsigned int ParseScriptFlags(std::string strFlags)
std::vector<std::string> words = SplitString(strFlags, ',');
for (const std::string& word : words)
{
- if (!mapFlagNames.count(word))
+ if (!mapFlagNames.count(word)) {
BOOST_ERROR("Bad test: unknown verification flag '" << word << "'");
- flags |= mapFlagNames[word];
+ continue;
+ }
+ flags |= mapFlagNames.at(word);
}
return flags;
}
@@ -98,22 +79,6 @@ bool CheckMapFlagNames()
return standard_flags_missing == 0;
}
-std::string FormatScriptFlags(unsigned int flags)
-{
- if (flags == SCRIPT_VERIFY_NONE) {
- return "";
- }
- std::string ret;
- std::map<std::string, unsigned int>::const_iterator it = mapFlagNames.begin();
- while (it != mapFlagNames.end()) {
- if (flags & it->second) {
- ret += it->first + ",";
- }
- it++;
- }
- return ret.substr(0, ret.size() - 1);
-}
-
/*
* Check that the input scripts of a transaction are valid/invalid as expected.
*/
Why this scored 15/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.