Skip to content

Commit bddcade

Browse files
committed
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.
1 parent a5ead12 commit bddcade

12 files changed

+100
-25
lines changed

src/script/interpreter.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2164,7 +2164,7 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
21642164
}
21652165

21662166
#define FLAG_NAME(flag) {std::string(#flag), SCRIPT_VERIFY_##flag}
2167-
const std::map<std::string, uint32_t> g_verify_flag_names{
2167+
const std::map<std::string, script_verify_flag_name> g_verify_flag_names{
21682168
FLAG_NAME(P2SH),
21692169
FLAG_NAME(STRICTENC),
21702170
FLAG_NAME(DERSIG),
@@ -2203,7 +2203,7 @@ std::vector<std::string> GetScriptFlagNames(script_verify_flags flags)
22032203
}
22042204
}
22052205
if (leftover != 0) {
2206-
res.push_back(strprintf("0x%08x", leftover));
2206+
res.push_back(strprintf("0x%08x", leftover.as_int()));
22072207
}
22082208
return res;
22092209
}

src/script/interpreter.h

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <span.h>
1515
#include <uint256.h>
1616

17+
#include <bit>
1718
#include <cstddef>
1819
#include <cstdint>
1920
#include <optional>
@@ -43,9 +44,10 @@ enum
4344
* All flags are intended to be soft forks: the set of acceptable scripts under
4445
* flags (A | B) is a subset of the acceptable scripts under flag (A).
4546
*/
46-
enum : uint32_t {
47-
SCRIPT_VERIFY_NONE = 0,
4847

48+
static constexpr script_verify_flags SCRIPT_VERIFY_NONE{0};
49+
50+
enum class script_verify_flag_name : uint32_t {
4951
// Evaluate P2SH subscripts (BIP16).
5052
SCRIPT_VERIFY_P2SH = (1U << 0),
5153

@@ -148,6 +150,13 @@ enum : uint32_t {
148150
//
149151
SCRIPT_VERIFY_END_MARKER
150152
};
153+
using enum script_verify_flag_name;
154+
155+
// assert there is still a spare bit
156+
static_assert(static_cast<script_verify_flags::value_type>(SCRIPT_VERIFY_END_MARKER) < (1u << 31));
157+
158+
static constexpr script_verify_flags::value_type MAX_SCRIPT_VERIFY_FLAGS = ((static_cast<script_verify_flags::value_type>(SCRIPT_VERIFY_END_MARKER) - 1) << 1) - 1;
159+
static constexpr int MAX_SCRIPT_VERIFY_FLAGS_BITS = std::bit_width(MAX_SCRIPT_VERIFY_FLAGS);
151160

152161
bool CheckSignatureEncoding(const std::vector<unsigned char> &vchSig, script_verify_flags flags, ScriptError* serror);
153162

@@ -372,7 +381,7 @@ size_t CountWitnessSigOps(const CScript& scriptSig, const CScript& scriptPubKey,
372381

373382
int FindAndDelete(CScript& script, const CScript& b);
374383

375-
extern const std::map<std::string, uint32_t> g_verify_flag_names;
384+
extern const std::map<std::string, script_verify_flag_name> g_verify_flag_names;
376385

377386
std::vector<std::string> GetScriptFlagNames(script_verify_flags flags);
378387

src/script/verify_flags.h

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,66 @@
66
#ifndef BITCOIN_SCRIPT_VERIFY_FLAGS_H
77
#define BITCOIN_SCRIPT_VERIFY_FLAGS_H
88

9+
#include <compare>
910
#include <cstdint>
1011

11-
using script_verify_flags = uint32_t;
12+
enum class script_verify_flag_name : uint32_t;
13+
14+
class script_verify_flags
15+
{
16+
public:
17+
using value_type = uint32_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; }
23+
24+
// implicit construction from a hard-coded SCRIPT_VERIFY_* constant is also okay
25+
constexpr explicit(false) script_verify_flags(script_verify_flag_name f) : m_value{static_cast<value_type>(f)} { }
26+
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;
33+
34+
// integer conversion needs to be very explicit
35+
static constexpr script_verify_flags from_int(value_type f) { script_verify_flags r; r.m_value = f; return r; }
36+
constexpr value_type as_int() const { return m_value; }
37+
38+
// bitwise operations
39+
constexpr script_verify_flags operator~() const { return from_int(~m_value); }
40+
friend constexpr script_verify_flags operator|(script_verify_flags a, script_verify_flags b) { return from_int(a.m_value | b.m_value); }
41+
friend constexpr script_verify_flags operator&(script_verify_flags a, script_verify_flags b) { return from_int(a.m_value & b.m_value); }
42+
43+
// in-place bitwise operations
44+
constexpr script_verify_flags& operator|=(script_verify_flags vf) { m_value |= vf.m_value; return *this; }
45+
constexpr script_verify_flags& operator&=(script_verify_flags vf) { m_value &= vf.m_value; return *this; }
46+
47+
// tests
48+
constexpr explicit operator bool() const { return m_value != 0; }
49+
constexpr bool operator==(script_verify_flags other) const { return m_value == other.m_value; }
50+
51+
/** Compare two script_verify_flags. <, >, <=, and >= are auto-generated from this. */
52+
friend constexpr std::strong_ordering operator<=>(const script_verify_flags& a, const script_verify_flags& b) noexcept
53+
{
54+
return a.m_value <=> b.m_value;
55+
}
56+
57+
private:
58+
value_type m_value{0}; // default value is SCRIPT_VERIFY_NONE
59+
};
60+
61+
inline constexpr script_verify_flags operator~(script_verify_flag_name f)
62+
{
63+
return ~script_verify_flags{f};
64+
}
65+
66+
inline constexpr script_verify_flags operator|(script_verify_flag_name f1, script_verify_flag_name f2)
67+
{
68+
return script_verify_flags{f1} | f2;
69+
}
1270

1371
#endif // BITCOIN_SCRIPT_VERIFY_FLAGS_H

src/test/fuzz/coins_view.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -288,7 +288,7 @@ void TestCoinsView(FuzzedDataProvider& fuzzed_data_provider, CCoinsView& backend
288288
// consensus/tx_verify.cpp:130: unsigned int GetP2SHSigOpCount(const CTransaction &, const CCoinsViewCache &): Assertion `!coin.IsSpent()' failed.
289289
return;
290290
}
291-
const auto flags{fuzzed_data_provider.ConsumeIntegral<uint32_t>()};
291+
const auto flags = script_verify_flags::from_int(fuzzed_data_provider.ConsumeIntegral<script_verify_flags::value_type>());
292292
if (!transaction.vin.empty() && (flags & SCRIPT_VERIFY_WITNESS) != 0 && (flags & SCRIPT_VERIFY_P2SH) == 0) {
293293
// Avoid:
294294
// script/interpreter.cpp:1705: size_t CountWitnessSigOps(const CScript &, const CScript &, const CScriptWitness *, unsigned int): Assertion `(flags & SCRIPT_VERIFY_P2SH) != 0' failed.

src/test/fuzz/eval_script.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212
FUZZ_TARGET(eval_script)
1313
{
1414
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
15-
const script_verify_flags flags = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
15+
const auto flags = script_verify_flags::from_int(fuzzed_data_provider.ConsumeIntegral<script_verify_flags::value_type>());
1616
const std::vector<uint8_t> script_bytes = [&] {
1717
if (fuzzed_data_provider.remaining_bytes() != 0) {
1818
return fuzzed_data_provider.ConsumeRemainingBytes<uint8_t>();

src/test/fuzz/script.cpp

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -118,8 +118,8 @@ FUZZ_TARGET(script, .init = initialize_script)
118118
(void)FindAndDelete(script_mut, *other_script);
119119
}
120120
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};
121+
const auto flags_rand{fuzzed_data_provider.ConsumeIntegral<script_verify_flags::value_type>()};
122+
const auto flags = script_verify_flags::from_int(flags_rand) | SCRIPT_VERIFY_P2SH;
123123
{
124124
CScriptWitness wit;
125125
for (const auto& s : random_string_vector) {

src/test/fuzz/script_assets_test_minimizer.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -90,7 +90,7 @@ CScriptWitness ScriptWitnessFromJSON(const UniValue& univalue)
9090
return scriptwitness;
9191
}
9292

93-
const std::map<std::string, uint32_t> FLAG_NAMES = {
93+
const std::map<std::string, script_verify_flag_name> FLAG_NAMES = {
9494
{std::string("P2SH"), SCRIPT_VERIFY_P2SH},
9595
{std::string("DERSIG"), SCRIPT_VERIFY_DERSIG},
9696
{std::string("NULLDUMMY"), SCRIPT_VERIFY_NULLDUMMY},

src/test/fuzz/script_flags.cpp

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@
1515
#include <utility>
1616
#include <vector>
1717

18+
static DataStream& operator>>(DataStream& ds, script_verify_flags& f)
19+
{
20+
script_verify_flags::value_type n{0};
21+
ds >> n;
22+
f = script_verify_flags::from_int(n);
23+
return ds;
24+
}
25+
1826
FUZZ_TARGET(script_flags)
1927
{
2028
if (buffer.size() > 100'000) return;

src/test/fuzz/signature_checker.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ class FuzzedSignatureChecker : public BaseSignatureChecker
5151
FUZZ_TARGET(signature_checker)
5252
{
5353
FuzzedDataProvider fuzzed_data_provider(buffer.data(), buffer.size());
54-
const script_verify_flags flags = fuzzed_data_provider.ConsumeIntegral<unsigned int>();
54+
const auto flags = script_verify_flags::from_int(fuzzed_data_provider.ConsumeIntegral<script_verify_flags::value_type>());
5555
const SigVersion sig_version = fuzzed_data_provider.PickValueInArray({SigVersion::BASE, SigVersion::WITNESS_V0});
5656
const auto script_1{ConsumeScript(fuzzed_data_provider)};
5757
const auto script_2{ConsumeScript(fuzzed_data_provider)};

src/test/script_tests.cpp

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,7 @@ static ScriptErrorDesc script_errors[]={
9696
{SCRIPT_ERR_SIG_FINDANDDELETE, "SIG_FINDANDDELETE"},
9797
};
9898

99-
static std::string FormatScriptFlags(uint32_t flags)
99+
static std::string FormatScriptFlags(script_verify_flags flags)
100100
{
101101
return util::Join(GetScriptFlagNames(flags), ",");
102102
}
@@ -134,13 +134,13 @@ void DoTest(const CScript& scriptPubKey, const CScript& scriptSig, const CScript
134134
BOOST_CHECK_MESSAGE(err == scriptError, FormatScriptError(err) + " where " + FormatScriptError((ScriptError_t)scriptError) + " expected: " + message);
135135

136136
// Verify that removing flags from a passing test or adding flags to a failing test does not change the result.
137-
for (int i = 0; i < 16; ++i) {
138-
uint32_t extra_flags(m_rng.randbits(16));
139-
uint32_t combined_flags{expect ? (flags & ~extra_flags) : (flags | extra_flags)};
137+
for (int i = 0; i < 256; ++i) {
138+
script_verify_flags extra_flags = script_verify_flags::from_int(m_rng.randbits(MAX_SCRIPT_VERIFY_FLAGS_BITS));
139+
script_verify_flags combined_flags{expect ? (flags & ~extra_flags) : (flags | extra_flags)};
140140
// Weed out some invalid flag combinations.
141141
if (combined_flags & SCRIPT_VERIFY_CLEANSTACK && ~combined_flags & (SCRIPT_VERIFY_P2SH | SCRIPT_VERIFY_WITNESS)) continue;
142142
if (combined_flags & SCRIPT_VERIFY_WITNESS && ~combined_flags & SCRIPT_VERIFY_P2SH) continue;
143-
BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, combined_flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err) == expect, message + strprintf(" (with flags %x)", combined_flags));
143+
BOOST_CHECK_MESSAGE(VerifyScript(scriptSig, scriptPubKey, &scriptWitness, combined_flags, MutableTransactionSignatureChecker(&tx, 0, txCredit.vout[0].nValue, MissingDataBehavior::ASSERT_FAIL), &err) == expect, message + strprintf(" (with flags %x)", combined_flags.as_int()));
144144
}
145145
}
146146
}; // struct ScriptTest
@@ -1716,9 +1716,9 @@ BOOST_AUTO_TEST_CASE(formatscriptflags)
17161716
{
17171717
// quick check that FormatScriptFlags reports any unknown/unexpected bits
17181718
BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH), "P2SH");
1719-
BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH | (1u<<31)), "P2SH,0x80000000");
1720-
BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_TAPROOT | (1u<<27)), "TAPROOT,0x08000000");
1721-
BOOST_CHECK_EQUAL(FormatScriptFlags(1u<<26), "0x04000000");
1719+
BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_P2SH | script_verify_flags::from_int(1u<<31)), "P2SH,0x80000000");
1720+
BOOST_CHECK_EQUAL(FormatScriptFlags(SCRIPT_VERIFY_TAPROOT | script_verify_flags::from_int(1u<<27)), "TAPROOT,0x08000000");
1721+
BOOST_CHECK_EQUAL(FormatScriptFlags(script_verify_flags::from_int(1u<<26)), "0x04000000");
17221722
}
17231723

17241724
BOOST_AUTO_TEST_SUITE_END()

0 commit comments

Comments
 (0)