mirrors/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2024-11-19 18:09:47 +01:00
Code Issues Projects Releases Wiki Activity

Merge bitcoin/bitcoin#30577: miniscript: Use ToIntegral instead of ParseInt64

Browse Source 
6714276d72 miniscript: Use `ToIntegral` instead of `ParseInt64` (brunoerg)

Pull request description:

  Currently, miniscript code uses `ParseInt64` function for `after`, `older`, `multi` and `thresh` fragments. It means that a leading `+` or whitespace, among other things, are accepted into the fragments. However, these cases are not useful and cause Bitcoin Core to behave differently compared to other miniscript implementations (see https://github.com/brunoerg/bitcoinfuzz/issues/34). This PR fixes it.

ACKs for top commit:
  achow101:
    ACK 6714276d72
  tdb3:
    cr ACK 6714276d72
  danielabrozzoni:
    tACK 6714276d72
  darosior:
    utACK 6714276d72

Tree-SHA512: d9eeb93f380f346d636513eeaf26865285e7b0907b8ed258fe1e02153a9eb69d484c82180eb1c78b0ed77ad5f0e5b244be6672c2f890b1d9fddc9e844bee6dde
This commit is contained in:
Ava Chow 2024-08-06 20:11:38 -04:00
commit ce1c881ccc
No known key found for this signature in database
GPG Key ID: 17565732E08E5E41
2 changed files with 21 additions and 16 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

31
src/script/miniscript.h
View File

@ -1793,8 +1793,9 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
// Get threshold
int next_comma = FindNextChar(in, ',');
if (next_comma < 1) return false;
int64_t k;
if (!ParseInt64(std::string(in.begin(), in.begin() + next_comma), &k)) return false;
const auto k_to_integral{ToIntegral<int64_t>(std::string_view(in.begin(), next_comma))};
if (!k_to_integral.has_value()) return false;
const int64_t k{k_to_integral.value()};
in = in.subspan(next_comma + 1);
// Get keys. It is compatible for both compressed and x-only keys.
std::vector<Key> keys;
@ -1948,21 +1949,19 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
} else if (Const("after(", in)) {
int arg_size = FindNextChar(in, ')');
if (arg_size < 1) return {};
int64_t num;
if (!ParseInt64(std::string(in.begin(), in.begin() + arg_size), &num)) return {};
if (num < 1 || num >= 0x80000000L) return {};
constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AFTER, num));
const auto num{ToIntegral<int64_t>(std::string_view(in.begin(), arg_size))};
if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {};
constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AFTER, *num));
in = in.subspan(arg_size + 1);
script_size += 1 + (num > 16) + (num > 0x7f) + (num > 0x7fff) + (num > 0x7fffff);
script_size += 1 + (*num > 16) + (*num > 0x7f) + (*num > 0x7fff) + (*num > 0x7fffff);
} else if (Const("older(", in)) {
int arg_size = FindNextChar(in, ')');
if (arg_size < 1) return {};
int64_t num;
if (!ParseInt64(std::string(in.begin(), in.begin() + arg_size), &num)) return {};
if (num < 1 || num >= 0x80000000L) return {};
constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OLDER, num));
const auto num{ToIntegral<int64_t>(std::string_view(in.begin(), arg_size))};
if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {};
constructed.push_back(MakeNodeRef<Key>(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OLDER, *num));
in = in.subspan(arg_size + 1);
script_size += 1 + (num > 16) + (num > 0x7f) + (num > 0x7fff) + (num > 0x7fffff);
script_size += 1 + (*num > 16) + (*num > 0x7f) + (*num > 0x7fff) + (*num > 0x7fffff);
} else if (Const("multi(", in)) {
if (!parse_multi_exp(in, /* is_multi_a = */false)) return {};
} else if (Const("multi_a(", in)) {
@ -1970,13 +1969,13 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
} else if (Const("thresh(", in)) {
int next_comma = FindNextChar(in, ',');
if (next_comma < 1) return {};
if (!ParseInt64(std::string(in.begin(), in.begin() + next_comma), &k)) return {};
if (k < 1) return {};
const auto k{ToIntegral<int64_t>(std::string_view(in.begin(), next_comma))};
if (!k.has_value() || *k < 1) return {};
in = in.subspan(next_comma + 1);
// n = 1 here because we read the first WRAPPED_EXPR before reaching THRESH
to_parse.emplace_back(ParseContext::THRESH, 1, k);
to_parse.emplace_back(ParseContext::THRESH, 1, *k);
to_parse.emplace_back(ParseContext::WRAPPED_EXPR, -1, -1);
script_size += 2 + (k > 16) + (k > 0x7f) + (k > 0x7fff) + (k > 0x7fffff);
script_size += 2 + (*k > 16) + (*k > 0x7f) + (*k > 0x7fff) + (*k > 0x7fffff);
} else if (Const("andor(", in)) {
to_parse.emplace_back(ParseContext::ANDOR, -1, -1);
to_parse.emplace_back(ParseContext::CLOSE_BRACKET, -1, -1);

6
src/test/miniscript_tests.cpp
View File

@ -699,6 +699,12 @@ BOOST_AUTO_TEST_CASE(fixed_tests)
const auto insane_sub = ms_ins->FindInsaneSub();
BOOST_CHECK(insane_sub && *insane_sub->ToString(wsh_converter) == "and_b(after(1),a:after(1000000000))");
// Numbers can't be prefixed by a sign.
BOOST_CHECK(!miniscript::FromString("after(-1)", wsh_converter));
BOOST_CHECK(!miniscript::FromString("after(+1)", wsh_converter));
BOOST_CHECK(!miniscript::FromString("thresh(-1,pk(03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204))", wsh_converter));
BOOST_CHECK(!miniscript::FromString("multi(+1,03cdabb7f2dce7bfbd8a0b9570c6fd1e712e5d64045e9d6b517b3d5072251dc204)", wsh_converter));
// Timelock tests
Test("after(100)", "?", "?", TESTMODE_VALID | TESTMODE_NONMAL); // only heightlock
Test("after(1000000000)", "?", "?", TESTMODE_VALID | TESTMODE_NONMAL); // only timelock