miniscript: use optional instead of bool/outarg

Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
This commit is contained in:
Antoine Poinsot 2022-04-11 14:07:25 +02:00
parent 1ab8d89fd1
commit ed45ee3882
No known key found for this signature in database
GPG Key ID: E13FC145CD3F4304
4 changed files with 92 additions and 96 deletions

View File

@ -281,16 +281,15 @@ size_t ComputeScriptLen(Fragment fragment, Type sub0typ, size_t subsize, uint32_
return 0; return 0;
} }
bool DecomposeScript(const CScript& script, std::vector<std::pair<opcodetype, std::vector<unsigned char>>>& out) std::optional<std::vector<std::pair<opcodetype, std::vector<unsigned char>>>> DecomposeScript(const CScript& script)
{ {
out.clear(); std::vector<std::pair<opcodetype, std::vector<unsigned char>>> out;
CScript::const_iterator it = script.begin(), itend = script.end(); CScript::const_iterator it = script.begin(), itend = script.end();
while (it != itend) { while (it != itend) {
std::vector<unsigned char> push_data; std::vector<unsigned char> push_data;
opcodetype opcode; opcodetype opcode;
if (!script.GetOp(it, opcode, push_data)) { if (!script.GetOp(it, opcode, push_data)) {
out.clear(); return {};
return false;
} else if (opcode >= OP_1 && opcode <= OP_16) { } else if (opcode >= OP_1 && opcode <= OP_16) {
// Deal with OP_n (GetOp does not turn them into pushes). // Deal with OP_n (GetOp does not turn them into pushes).
push_data.assign(1, CScript::DecodeOP_N(opcode)); push_data.assign(1, CScript::DecodeOP_N(opcode));
@ -307,30 +306,28 @@ bool DecomposeScript(const CScript& script, std::vector<std::pair<opcodetype, st
out.emplace_back(OP_EQUAL, std::vector<unsigned char>()); out.emplace_back(OP_EQUAL, std::vector<unsigned char>());
opcode = OP_VERIFY; opcode = OP_VERIFY;
} else if (IsPushdataOp(opcode)) { } else if (IsPushdataOp(opcode)) {
if (!CheckMinimalPush(push_data, opcode)) return false; if (!CheckMinimalPush(push_data, opcode)) return {};
} else if (it != itend && (opcode == OP_CHECKSIG || opcode == OP_CHECKMULTISIG || opcode == OP_EQUAL) && (*it == OP_VERIFY)) { } else if (it != itend && (opcode == OP_CHECKSIG || opcode == OP_CHECKMULTISIG || opcode == OP_EQUAL) && (*it == OP_VERIFY)) {
// Rule out non minimal VERIFY sequences // Rule out non minimal VERIFY sequences
return false; return {};
} }
out.emplace_back(opcode, std::move(push_data)); out.emplace_back(opcode, std::move(push_data));
} }
std::reverse(out.begin(), out.end()); std::reverse(out.begin(), out.end());
return true; return out;
} }
bool ParseScriptNumber(const std::pair<opcodetype, std::vector<unsigned char>>& in, int64_t& k) { std::optional<int64_t> ParseScriptNumber(const std::pair<opcodetype, std::vector<unsigned char>>& in) {
if (in.first == OP_0) { if (in.first == OP_0) {
k = 0; return 0;
return true;
} }
if (!in.second.empty()) { if (!in.second.empty()) {
if (IsPushdataOp(in.first) && !CheckMinimalPush(in.second, in.first)) return false; if (IsPushdataOp(in.first) && !CheckMinimalPush(in.second, in.first)) return {};
try { try {
k = CScriptNum(in.second, true).GetInt64(); return CScriptNum(in.second, true).GetInt64();
return true;
} catch(const scriptnum_error&) {} } catch(const scriptnum_error&) {}
} }
return false; return {};
} }
int FindNextChar(Span<const char> sp, const char m) int FindNextChar(Span<const char> sp, const char m)

View File

@ -517,7 +517,7 @@ public:
} }
template<typename CTx> template<typename CTx>
bool ToString(const CTx& ctx, std::string& ret) const { std::optional<std::string> ToString(const CTx& ctx) const {
// To construct the std::string representation for a Miniscript object, we use // To construct the std::string representation for a Miniscript object, we use
// the TreeEvalMaybe algorithm. The State is a boolean: whether the parent node is a // the TreeEvalMaybe algorithm. The State is a boolean: whether the parent node is a
// wrapper. If so, non-wrapper expressions must be prefixed with a ":". // wrapper. If so, non-wrapper expressions must be prefixed with a ":".
@ -541,15 +541,15 @@ public:
case Fragment::WRAP_C: case Fragment::WRAP_C:
if (node.subs[0]->fragment == Fragment::PK_K) { if (node.subs[0]->fragment == Fragment::PK_K) {
// pk(K) is syntactic sugar for c:pk_k(K) // pk(K) is syntactic sugar for c:pk_k(K)
std::string key_str; auto key_str = ctx.ToString(node.subs[0]->keys[0]);
if (!ctx.ToString(node.subs[0]->keys[0], key_str)) return {}; if (!key_str) return {};
return std::move(ret) + "pk(" + std::move(key_str) + ")"; return std::move(ret) + "pk(" + std::move(*key_str) + ")";
} }
if (node.subs[0]->fragment == Fragment::PK_H) { if (node.subs[0]->fragment == Fragment::PK_H) {
// pkh(K) is syntactic sugar for c:pk_h(K) // pkh(K) is syntactic sugar for c:pk_h(K)
std::string key_str; auto key_str = ctx.ToString(node.subs[0]->keys[0]);
if (!ctx.ToString(node.subs[0]->keys[0], key_str)) return {}; if (!key_str) return {};
return std::move(ret) + "pkh(" + std::move(key_str) + ")"; return std::move(ret) + "pkh(" + std::move(*key_str) + ")";
} }
return "c" + std::move(subs[0]); return "c" + std::move(subs[0]);
case Fragment::WRAP_D: return "d" + std::move(subs[0]); case Fragment::WRAP_D: return "d" + std::move(subs[0]);
@ -568,14 +568,14 @@ public:
} }
switch (node.fragment) { switch (node.fragment) {
case Fragment::PK_K: { case Fragment::PK_K: {
std::string key_str; auto key_str = ctx.ToString(node.keys[0]);
if (!ctx.ToString(node.keys[0], key_str)) return {}; if (!key_str) return {};
return std::move(ret) + "pk_k(" + std::move(key_str) + ")"; return std::move(ret) + "pk_k(" + std::move(*key_str) + ")";
} }
case Fragment::PK_H: { case Fragment::PK_H: {
std::string key_str; auto key_str = ctx.ToString(node.keys[0]);
if (!ctx.ToString(node.keys[0], key_str)) return {}; if (!key_str) return {};
return std::move(ret) + "pk_h(" + std::move(key_str) + ")"; return std::move(ret) + "pk_h(" + std::move(*key_str) + ")";
} }
case Fragment::AFTER: return std::move(ret) + "after(" + ::ToString(node.k) + ")"; case Fragment::AFTER: return std::move(ret) + "after(" + ::ToString(node.k) + ")";
case Fragment::OLDER: return std::move(ret) + "older(" + ::ToString(node.k) + ")"; case Fragment::OLDER: return std::move(ret) + "older(" + ::ToString(node.k) + ")";
@ -598,9 +598,9 @@ public:
case Fragment::MULTI: { case Fragment::MULTI: {
auto str = std::move(ret) + "multi(" + ::ToString(node.k); auto str = std::move(ret) + "multi(" + ::ToString(node.k);
for (const auto& key : node.keys) { for (const auto& key : node.keys) {
std::string key_str; auto key_str = ctx.ToString(key);
if (!ctx.ToString(key, key_str)) return {}; if (!key_str) return {};
str += "," + std::move(key_str); str += "," + std::move(*key_str);
} }
return std::move(str) + ")"; return std::move(str) + ")";
} }
@ -616,9 +616,7 @@ public:
return ""; // Should never be reached. return ""; // Should never be reached.
}; };
auto res = TreeEvalMaybe<std::string>(false, downfn, upfn); return TreeEvalMaybe<std::string>(false, downfn, upfn);
if (res.has_value()) ret = std::move(*res);
return res.has_value();
} }
internal::Ops CalcOps() const { internal::Ops CalcOps() const {
@ -858,11 +856,11 @@ int FindNextChar(Span<const char> in, const char m);
template<typename Key, typename Ctx> template<typename Key, typename Ctx>
std::optional<std::pair<Key, int>> ParseKeyEnd(Span<const char> in, const Ctx& ctx) std::optional<std::pair<Key, int>> ParseKeyEnd(Span<const char> in, const Ctx& ctx)
{ {
Key key;
int key_size = FindNextChar(in, ')'); int key_size = FindNextChar(in, ')');
if (key_size < 1) return {}; if (key_size < 1) return {};
if (!ctx.FromString(in.begin(), in.begin() + key_size, key)) return {}; auto key = ctx.FromString(in.begin(), in.begin() + key_size);
return {{std::move(key), key_size}}; if (!key) return {};
return {{std::move(*key), key_size}};
} }
/** Parse a hex string ending at the end of the fragment's text representation. */ /** Parse a hex string ending at the end of the fragment's text representation. */
@ -1029,12 +1027,12 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
// Get keys // Get keys
std::vector<Key> keys; std::vector<Key> keys;
while (next_comma != -1) { while (next_comma != -1) {
Key key;
next_comma = FindNextChar(in, ','); next_comma = FindNextChar(in, ',');
int key_length = (next_comma == -1) ? FindNextChar(in, ')') : next_comma; int key_length = (next_comma == -1) ? FindNextChar(in, ')') : next_comma;
if (key_length < 1) return {}; if (key_length < 1) return {};
if (!ctx.FromString(in.begin(), in.begin() + key_length, key)) return {}; auto key = ctx.FromString(in.begin(), in.begin() + key_length);
keys.push_back(std::move(key)); if (!key) return {};
keys.push_back(std::move(*key));
in = in.subspan(key_length + 1); in = in.subspan(key_length + 1);
} }
if (keys.size() < 1 || keys.size() > 20) return {}; if (keys.size() < 1 || keys.size() > 20) return {};
@ -1207,10 +1205,10 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
* and OP_EQUALVERIFY are decomposed into OP_CHECKSIG, OP_CHECKMULTISIG, OP_EQUAL * and OP_EQUALVERIFY are decomposed into OP_CHECKSIG, OP_CHECKMULTISIG, OP_EQUAL
* respectively, plus OP_VERIFY. * respectively, plus OP_VERIFY.
*/ */
bool DecomposeScript(const CScript& script, std::vector<std::pair<opcodetype, std::vector<unsigned char>>>& out); std::optional<std::vector<std::pair<opcodetype, std::vector<unsigned char>>>> DecomposeScript(const CScript& script);
/** Determine whether the passed pair (created by DecomposeScript) is pushing a number. */ /** Determine whether the passed pair (created by DecomposeScript) is pushing a number. */
bool ParseScriptNumber(const std::pair<opcodetype, std::vector<unsigned char>>& in, int64_t& k); std::optional<int64_t> ParseScriptNumber(const std::pair<opcodetype, std::vector<unsigned char>>& in);
enum class DecodeContext { enum class DecodeContext {
/** A single expression of type B, K, or V. Specifically, this can't be an /** A single expression of type B, K, or V. Specifically, this can't be an
@ -1317,34 +1315,35 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx)
} }
// Public keys // Public keys
if (in[0].second.size() == 33) { if (in[0].second.size() == 33) {
Key key; auto key = ctx.FromPKBytes(in[0].second.begin(), in[0].second.end());
if (!ctx.FromPKBytes(in[0].second.begin(), in[0].second.end(), key)) return {}; if (!key) return {};
++in; ++in;
constructed.push_back(MakeNodeRef<Key>(Fragment::PK_K, Vector(std::move(key)))); constructed.push_back(MakeNodeRef<Key>(Fragment::PK_K, Vector(std::move(*key))));
break; break;
} }
if (last - in >= 5 && in[0].first == OP_VERIFY && in[1].first == OP_EQUAL && in[3].first == OP_HASH160 && in[4].first == OP_DUP && in[2].second.size() == 20) { if (last - in >= 5 && in[0].first == OP_VERIFY && in[1].first == OP_EQUAL && in[3].first == OP_HASH160 && in[4].first == OP_DUP && in[2].second.size() == 20) {
Key key; auto key = ctx.FromPKHBytes(in[2].second.begin(), in[2].second.end());
if (!ctx.FromPKHBytes(in[2].second.begin(), in[2].second.end(), key)) return {}; if (!key) return {};
in += 5; in += 5;
constructed.push_back(MakeNodeRef<Key>(Fragment::PK_H, Vector(std::move(key)))); constructed.push_back(MakeNodeRef<Key>(Fragment::PK_H, Vector(std::move(*key))));
break; break;
} }
// Time locks // Time locks
if (last - in >= 2 && in[0].first == OP_CHECKSEQUENCEVERIFY && ParseScriptNumber(in[1], k)) { std::optional<int64_t> num;
if (last - in >= 2 && in[0].first == OP_CHECKSEQUENCEVERIFY && (num = ParseScriptNumber(in[1]))) {
in += 2; in += 2;
if (k < 1 || k > 0x7FFFFFFFL) return {}; if (*num < 1 || *num > 0x7FFFFFFFL) return {};
constructed.push_back(MakeNodeRef<Key>(Fragment::OLDER, k)); constructed.push_back(MakeNodeRef<Key>(Fragment::OLDER, *num));
break; break;
} }
if (last - in >= 2 && in[0].first == OP_CHECKLOCKTIMEVERIFY && ParseScriptNumber(in[1], k)) { if (last - in >= 2 && in[0].first == OP_CHECKLOCKTIMEVERIFY && (num = ParseScriptNumber(in[1]))) {
in += 2; in += 2;
if (k < 1 || k > 0x7FFFFFFFL) return {}; if (num < 1 || num > 0x7FFFFFFFL) return {};
constructed.push_back(MakeNodeRef<Key>(Fragment::AFTER, k)); constructed.push_back(MakeNodeRef<Key>(Fragment::AFTER, *num));
break; break;
} }
// Hashes // Hashes
if (last - in >= 7 && in[0].first == OP_EQUAL && in[3].first == OP_VERIFY && in[4].first == OP_EQUAL && ParseScriptNumber(in[5], k) && k == 32 && in[6].first == OP_SIZE) { if (last - in >= 7 && in[0].first == OP_EQUAL && in[3].first == OP_VERIFY && in[4].first == OP_EQUAL && (num = ParseScriptNumber(in[5])) && num == 32 && in[6].first == OP_SIZE) {
if (in[2].first == OP_SHA256 && in[1].second.size() == 32) { if (in[2].first == OP_SHA256 && in[1].second.size() == 32) {
constructed.push_back(MakeNodeRef<Key>(Fragment::SHA256, in[1].second)); constructed.push_back(MakeNodeRef<Key>(Fragment::SHA256, in[1].second));
in += 7; in += 7;
@ -1366,20 +1365,20 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx)
// Multi // Multi
if (last - in >= 3 && in[0].first == OP_CHECKMULTISIG) { if (last - in >= 3 && in[0].first == OP_CHECKMULTISIG) {
std::vector<Key> keys; std::vector<Key> keys;
if (!ParseScriptNumber(in[1], n)) return {}; const auto n = ParseScriptNumber(in[1]);
if (last - in < 3 + n) return {}; if (!n || last - in < 3 + *n) return {};
if (n < 1 || n > 20) return {}; if (*n < 1 || *n > 20) return {};
for (int i = 0; i < n; ++i) { for (int i = 0; i < *n; ++i) {
Key key;
if (in[2 + i].second.size() != 33) return {}; if (in[2 + i].second.size() != 33) return {};
if (!ctx.FromPKBytes(in[2 + i].second.begin(), in[2 + i].second.end(), key)) return {}; auto key = ctx.FromPKBytes(in[2 + i].second.begin(), in[2 + i].second.end());
keys.push_back(std::move(key)); if (!key) return {};
keys.push_back(std::move(*key));
} }
if (!ParseScriptNumber(in[2 + n], k)) return {}; const auto k = ParseScriptNumber(in[2 + *n]);
if (k < 1 || k > n) return {}; if (!k || *k < 1 || *k > *n) return {};
in += 3 + n; in += 3 + *n;
std::reverse(keys.begin(), keys.end()); std::reverse(keys.begin(), keys.end());
constructed.push_back(MakeNodeRef<Key>(Fragment::MULTI, std::move(keys), k)); constructed.push_back(MakeNodeRef<Key>(Fragment::MULTI, std::move(keys), *k));
break; break;
} }
/** In the following wrappers, we only need to push SINGLE_BKV_EXPR rather /** In the following wrappers, we only need to push SINGLE_BKV_EXPR rather
@ -1407,10 +1406,10 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx)
break; break;
} }
// Thresh // Thresh
if (last - in >= 3 && in[0].first == OP_EQUAL && ParseScriptNumber(in[1], k)) { if (last - in >= 3 && in[0].first == OP_EQUAL && (num = ParseScriptNumber(in[1]))) {
if (k < 1) return {}; if (*num < 1) return {};
in += 2; in += 2;
to_parse.emplace_back(DecodeContext::THRESH_W, 0, k); to_parse.emplace_back(DecodeContext::THRESH_W, 0, *num);
break; break;
} }
// OP_ENDIF can be WRAP_J, WRAP_D, ANDOR, OR_C, OR_D, or OR_I // OP_ENDIF can be WRAP_J, WRAP_D, ANDOR, OR_C, OR_D, or OR_I
@ -1645,12 +1644,12 @@ inline NodeRef<typename Ctx::Key> FromString(const std::string& str, const Ctx&
template<typename Ctx> template<typename Ctx>
inline NodeRef<typename Ctx::Key> FromScript(const CScript& script, const Ctx& ctx) { inline NodeRef<typename Ctx::Key> FromScript(const CScript& script, const Ctx& ctx) {
using namespace internal; using namespace internal;
std::vector<std::pair<opcodetype, std::vector<unsigned char>>> decomposed; auto decomposed = DecomposeScript(script);
if (!DecomposeScript(script, decomposed)) return {}; if (!decomposed) return {};
auto it = decomposed.begin(); auto it = decomposed->begin();
auto ret = DecodeScript<typename Ctx::Key>(it, decomposed.end(), ctx); auto ret = DecodeScript<typename Ctx::Key>(it, decomposed->end(), ctx);
if (!ret) return {}; if (!ret) return {};
if (it != decomposed.end()) return {}; if (it != decomposed->end()) return {};
return ret; return ret;
} }

View File

@ -21,9 +21,8 @@ using miniscript::operator""_mst;
struct Converter { struct Converter {
typedef CPubKey Key; typedef CPubKey Key;
bool ToString(const Key& key, std::string& ret) const { std::optional<std::string> ToString(const Key& key) const {
ret = HexStr(key); return HexStr(key);
return true;
} }
const std::vector<unsigned char> ToPKBytes(const Key& key) const { const std::vector<unsigned char> ToPKBytes(const Key& key) const {
return {key.begin(), key.end()}; return {key.begin(), key.end()};
@ -34,20 +33,21 @@ struct Converter {
} }
template<typename I> template<typename I>
bool FromString(I first, I last, Key& key) const { std::optional<Key> FromString(I first, I last) const {
const auto bytes = ParseHex(std::string(first, last)); const auto bytes = ParseHex(std::string(first, last));
key.Set(bytes.begin(), bytes.end()); Key key{bytes.begin(), bytes.end()};
return key.IsValid(); if (key.IsValid()) return key;
return {};
} }
template<typename I> template<typename I>
bool FromPKBytes(I first, I last, CPubKey& key) const { std::optional<Key> FromPKBytes(I first, I last) const {
key.Set(first, last); Key key{first, last};
return key.IsValid(); if (key.IsValid()) return key;
return {};
} }
template<typename I> template<typename I>
bool FromPKHBytes(I first, I last, CPubKey& key) const { std::optional<Key> FromPKHBytes(I first, I last) const {
assert(last - first == 20); return {};
return false;
} }
}; };
@ -63,8 +63,7 @@ FUZZ_TARGET(miniscript_decode)
if (!ms) return; if (!ms) return;
// We can roundtrip it to its string representation. // We can roundtrip it to its string representation.
std::string ms_str; std::string ms_str = *ms->ToString(CONVERTER);
assert(ms->ToString(CONVERTER, ms_str));
assert(*miniscript::FromString(ms_str, CONVERTER) == *ms); assert(*miniscript::FromString(ms_str, CONVERTER) == *ms);
// The Script representation must roundtrip since we parsed it this way the first time. // The Script representation must roundtrip since we parsed it this way the first time.
const CScript ms_script = ms->ToScript(CONVERTER); const CScript ms_script = ms->ToScript(CONVERTER);

View File

@ -84,27 +84,28 @@ struct KeyConverter {
//! Parse a public key from a range of hex characters. //! Parse a public key from a range of hex characters.
template<typename I> template<typename I>
bool FromString(I first, I last, CPubKey& key) const { std::optional<Key> FromString(I first, I last) const {
auto bytes = ParseHex(std::string(first, last)); auto bytes = ParseHex(std::string(first, last));
key.Set(bytes.begin(), bytes.end()); Key key{bytes.begin(), bytes.end()};
return key.IsValid(); if (key.IsValid()) return key;
return {};
} }
template<typename I> template<typename I>
bool FromPKBytes(I first, I last, CPubKey& key) const { std::optional<Key> FromPKBytes(I first, I last) const {
key.Set(first, last); Key key{first, last};
return key.IsValid(); if (key.IsValid()) return key;
return {};
} }
template<typename I> template<typename I>
bool FromPKHBytes(I first, I last, CPubKey& key) const { std::optional<Key> FromPKHBytes(I first, I last) const {
assert(last - first == 20); assert(last - first == 20);
CKeyID keyid; CKeyID keyid;
std::copy(first, last, keyid.begin()); std::copy(first, last, keyid.begin());
auto it = g_testdata->pkmap.find(keyid); auto it = g_testdata->pkmap.find(keyid);
assert(it != g_testdata->pkmap.end()); assert(it != g_testdata->pkmap.end());
key = it->second; return it->second;
return true;
} }
}; };