From 6714276d72244c2e2dbe9617f1341ba7fc06c0cc Mon Sep 17 00:00:00 2001 From: brunoerg Date: Fri, 19 Jul 2024 13:34:50 -0300 Subject: [PATCH] miniscript: Use `ToIntegral` instead of `ParseInt64` --- src/script/miniscript.h | 31 +++++++++++++++---------------- src/test/miniscript_tests.cpp | 6 ++++++ 2 files changed, 21 insertions(+), 16 deletions(-) diff --git a/src/script/miniscript.h b/src/script/miniscript.h index 97912906d13..58f24434f06 100644 --- a/src/script/miniscript.h +++ b/src/script/miniscript.h @@ -1793,8 +1793,9 @@ inline NodeRef Parse(Span 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(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 keys; @@ -1948,21 +1949,19 @@ inline NodeRef Parse(Span 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(internal::NoDupCheck{}, ctx.MsContext(), Fragment::AFTER, num)); + const auto num{ToIntegral(std::string_view(in.begin(), arg_size))}; + if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {}; + constructed.push_back(MakeNodeRef(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(internal::NoDupCheck{}, ctx.MsContext(), Fragment::OLDER, num)); + const auto num{ToIntegral(std::string_view(in.begin(), arg_size))}; + if (!num.has_value() || *num < 1 || *num >= 0x80000000L) return {}; + constructed.push_back(MakeNodeRef(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 Parse(Span 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(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); diff --git a/src/test/miniscript_tests.cpp b/src/test/miniscript_tests.cpp index c99a4594cee..271318955ec 100644 --- a/src/test/miniscript_tests.cpp +++ b/src/test/miniscript_tests.cpp @@ -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