From 35f6d1baed9ab0dcbd38a85eefeabb6d5c277837 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Fri, 30 Aug 2019 18:25:30 -0700 Subject: [PATCH] Make ToString fallible --- bitcoin/script/miniscript.h | 63 +++++++++++++++++++------------ bitcoin/test/miniscript_tests.cpp | 6 ++- compiler.h | 2 +- main.cpp | 4 +- 4 files changed, 47 insertions(+), 28 deletions(-) diff --git a/bitcoin/script/miniscript.h b/bitcoin/script/miniscript.h index cb35880..03ae199 100644 --- a/bitcoin/script/miniscript.h +++ b/bitcoin/script/miniscript.h @@ -385,22 +385,22 @@ private: //! Internal code for ToString. template - std::string MakeString(const Ctx& ctx, bool wrapped = false) const { + std::string MakeString(const Ctx& ctx, bool& success, bool wrapped = false) const { switch (nodetype) { - case NodeType::WRAP_A: return "a" + subs[0]->MakeString(ctx, true); - case NodeType::WRAP_S: return "s" + subs[0]->MakeString(ctx, true); - case NodeType::WRAP_C: return "c" + subs[0]->MakeString(ctx, true); - case NodeType::WRAP_D: return "d" + subs[0]->MakeString(ctx, true); - case NodeType::WRAP_V: return "v" + subs[0]->MakeString(ctx, true); - case NodeType::WRAP_J: return "j" + subs[0]->MakeString(ctx, true); - case NodeType::WRAP_N: return "n" + subs[0]->MakeString(ctx, true); + case NodeType::WRAP_A: return "a" + subs[0]->MakeString(ctx, success, true); + case NodeType::WRAP_S: return "s" + subs[0]->MakeString(ctx, success, true); + case NodeType::WRAP_C: return "c" + subs[0]->MakeString(ctx, success, true); + case NodeType::WRAP_D: return "d" + subs[0]->MakeString(ctx, success, true); + case NodeType::WRAP_V: return "v" + subs[0]->MakeString(ctx, success, true); + case NodeType::WRAP_J: return "j" + subs[0]->MakeString(ctx, success, true); + case NodeType::WRAP_N: return "n" + subs[0]->MakeString(ctx, success, true); case NodeType::AND_V: // t:X is syntactic sugar for and_v(X,1). - if (subs[1]->nodetype == NodeType::TRUE) return "t" + subs[0]->MakeString(ctx, true); + if (subs[1]->nodetype == NodeType::TRUE) return "t" + subs[0]->MakeString(ctx, success, true); break; case NodeType::OR_I: - if (subs[0]->nodetype == NodeType::FALSE) return "l" + subs[1]->MakeString(ctx, true); - if (subs[1]->nodetype == NodeType::FALSE) return "u" + subs[0]->MakeString(ctx, true); + if (subs[0]->nodetype == NodeType::FALSE) return "l" + subs[1]->MakeString(ctx, success, true); + if (subs[1]->nodetype == NodeType::FALSE) return "u" + subs[0]->MakeString(ctx, success, true); break; default: break; @@ -409,8 +409,16 @@ private: std::string ret = wrapped ? ":" : ""; switch (nodetype) { - case NodeType::PK: return std::move(ret) + "pk(" + ctx.ToString(keys[0]) + ")"; - case NodeType::PK_H: return std::move(ret) + "pk_h(" + ctx.ToString(keys[0]) + ")"; + case NodeType::PK: { + std::string key_str; + success = ctx.ToString(keys[0], key_str); + return std::move(ret) + "pk(" + std::move(key_str) + ")"; + } + case NodeType::PK_H: { + std::string key_str; + success = ctx.ToString(keys[0], key_str); + return std::move(ret) + "pk_h(" + std::move(key_str) + ")"; + } case NodeType::AFTER: return std::move(ret) + "after(" + std::to_string(k) + ")"; case NodeType::OLDER: return std::move(ret) + "older(" + std::to_string(k) + ")"; case NodeType::HASH256: return std::move(ret) + "hash256(" + HexStr(data.begin(), data.end()) + ")"; @@ -419,27 +427,29 @@ private: case NodeType::RIPEMD160: return std::move(ret) + "ripemd160(" + HexStr(data.begin(), data.end()) + ")"; case NodeType::TRUE: return std::move(ret) + "1"; case NodeType::FALSE: return std::move(ret) + "0"; - case NodeType::AND_V: return std::move(ret) + "and_v(" + subs[0]->MakeString(ctx) + "," + subs[1]->MakeString(ctx) + ")"; - case NodeType::AND_B: return std::move(ret) + "and_b(" + subs[0]->MakeString(ctx) + "," + subs[1]->MakeString(ctx) + ")"; - case NodeType::OR_B: return std::move(ret) + "or_b(" + subs[0]->MakeString(ctx) + "," + subs[1]->MakeString(ctx) + ")"; - case NodeType::OR_D: return std::move(ret) + "or_d(" + subs[0]->MakeString(ctx) + "," + subs[1]->MakeString(ctx) + ")"; - case NodeType::OR_C: return std::move(ret) + "or_c(" + subs[0]->MakeString(ctx) + "," + subs[1]->MakeString(ctx) + ")"; - case NodeType::OR_I: return std::move(ret) + "or_i(" + subs[0]->MakeString(ctx) + "," + subs[1]->MakeString(ctx) + ")"; + case NodeType::AND_V: return std::move(ret) + "and_v(" + subs[0]->MakeString(ctx, success) + "," + subs[1]->MakeString(ctx, success) + ")"; + case NodeType::AND_B: return std::move(ret) + "and_b(" + subs[0]->MakeString(ctx, success) + "," + subs[1]->MakeString(ctx, success) + ")"; + case NodeType::OR_B: return std::move(ret) + "or_b(" + subs[0]->MakeString(ctx, success) + "," + subs[1]->MakeString(ctx, success) + ")"; + case NodeType::OR_D: return std::move(ret) + "or_d(" + subs[0]->MakeString(ctx, success) + "," + subs[1]->MakeString(ctx, success) + ")"; + case NodeType::OR_C: return std::move(ret) + "or_c(" + subs[0]->MakeString(ctx, success) + "," + subs[1]->MakeString(ctx, success) + ")"; + case NodeType::OR_I: return std::move(ret) + "or_i(" + subs[0]->MakeString(ctx, success) + "," + subs[1]->MakeString(ctx, success) + ")"; case NodeType::ANDOR: // and_n(X,Y) is syntactic sugar for andor(X,Y,0). - if (subs[2]->nodetype == NodeType::FALSE) return std::move(ret) + "and_n(" + subs[0]->MakeString(ctx) + "," + subs[1]->MakeString(ctx) + ")"; - return std::move(ret) + "andor(" + subs[0]->MakeString(ctx) + "," + subs[1]->MakeString(ctx) + "," + subs[2]->MakeString(ctx) + ")"; + if (subs[2]->nodetype == NodeType::FALSE) return std::move(ret) + "and_n(" + subs[0]->MakeString(ctx, success) + "," + subs[1]->MakeString(ctx, success) + ")"; + return std::move(ret) + "andor(" + subs[0]->MakeString(ctx, success) + "," + subs[1]->MakeString(ctx, success) + "," + subs[2]->MakeString(ctx, success) + ")"; case NodeType::THRESH_M: { auto str = std::move(ret) + "thresh_m(" + std::to_string(k); for (const auto& key : keys) { - str += "," + ctx.ToString(key); + std::string key_str; + success &= ctx.ToString(key, key_str); + str += "," + std::move(key_str); } return std::move(str) + ")"; } case NodeType::THRESH: { auto str = std::move(ret) + "thresh(" + std::to_string(k); for (const auto& sub : subs) { - str += "," + sub->MakeString(ctx); + str += "," + sub->MakeString(ctx, success); } return std::move(str) + ")"; } @@ -760,7 +770,12 @@ public: //! Convert this miniscript to its textual descriptor notation. template - std::string ToString(const Ctx& ctx) const { return MakeString(ctx); } + bool ToString(const Ctx& ctx, std::string& out) const { + bool ret = true; + out = MakeString(ctx, ret); + if (!ret) out = ""; + return ret; + } template bool Satisfy(const Ctx& ctx, std::vector>& stack, bool nonmalleable = true) const { diff --git a/bitcoin/test/miniscript_tests.cpp b/bitcoin/test/miniscript_tests.cpp index f357b71..9943deb 100644 --- a/bitcoin/test/miniscript_tests.cpp +++ b/bitcoin/test/miniscript_tests.cpp @@ -225,7 +225,7 @@ struct TestContext { std::set supported; - std::string ToString(const TestKey& key) const { return {char('A' + key.c)}; } + bool ToString(const TestKey& key, std::string& ret) const { ret = char('A' + key.c); return true; } std::vector ToPKBytes(const TestKey& key) const { return PUBKEYS[key.c]; } std::vector ToPKHBytes(const TestKey& key) const { return PKHASHES[key.c]; } @@ -537,7 +537,9 @@ BOOST_AUTO_TEST_CASE(random_miniscript_tests) for (int i = 0; i < 100000; ++i) { auto typ = InsecureRandRange(100) ? "B"_mst : "Bms"_mst; // require 1% strong, non-malleable auto node = RandomNode(typ, 1 + InsecureRandRange(90)); - auto str = node->ToString(CTX); + std::string str; + bool str_ret = node->ToString(CTX, str); + BOOST_CHECK(str_ret); auto script = node->ToScript(CTX); // Check consistency between script size estimation and real size BOOST_CHECK(node->ScriptSize() == script.size()); diff --git a/compiler.h b/compiler.h index a813ac8..6ca91f8 100644 --- a/compiler.h +++ b/compiler.h @@ -12,7 +12,7 @@ struct CompilerContext { typedef std::string Key; - std::string ToString(const Key& key) const { return key; } + bool ToString(const Key& key, std::string& str) const { str = key; return true; } template bool FromString(I first, I last, Key& key) const { diff --git a/main.cpp b/main.cpp index 9d5482a..243c2ee 100644 --- a/main.cpp +++ b/main.cpp @@ -17,7 +17,9 @@ static bool run(std::string&& line, int64_t count) { miniscript::NodeRef ret; double avgcost = 0; if (Compile(Expand(line), ret, avgcost)) { - printf("X %17.10f %5i %s %s\n", ret->ScriptSize() + avgcost, (int)ret->ScriptSize(), Abbreviate(ret->ToString(COMPILER_CTX)).c_str(), line.c_str()); + std::string str; + ret->ToString(COMPILER_CTX, str); + printf("X %17.10f %5i %s %s\n", ret->ScriptSize() + avgcost, (int)ret->ScriptSize(), Abbreviate(std::move(str)).c_str(), line.c_str()); } else if ((ret = miniscript::FromString(Expand(line), COMPILER_CTX))) { printf("%7li scriptlen=%i maxops=%i type=%s safe=%s nonmal=%s dissat=%s input=%s output=%s miniscript=%s\n", (long)count, (int)ret->ScriptSize(), (int)ret->GetOps(), ret->GetType() << "B"_mst ? "B" : ret->GetType() << "V"_mst ? "V" : ret->GetType() << "W"_mst ? "W" : ret->GetType() << "K"_mst ? "K" : "(invalid)", ret->GetType() << "s"_mst ? "yes" : "no", ret->GetType() << "m"_mst ? "yes" : "no", ret->GetType() << "f"_mst ? "no" : ret->GetType() << "e"_mst ? "unique" : ret->GetType() << "d"_mst ? "yes" : "unknown", ret->GetType() << "z"_mst ? "0" : ret->GetType() << "o"_mst ? (ret->GetType() << "n"_mst ? "1n" : "1") : ret->GetType() << "n"_mst ? "n" : "-", ret->GetType() << "u"_mst ? "1" : "nonzero", line.c_str()); } else {