From bb5fb729e1387bee0a426eb2eec3b8ef9ae19bcb Mon Sep 17 00:00:00 2001 From: Antoine Poinsot Date: Thu, 16 Feb 2023 12:17:00 +0100 Subject: [PATCH] Update the Miniscript fuzz tests from Bitcoin Core master --- bitcoin/test/fuzz/miniscript.cpp | 70 ++++++++++++++++++++------------ 1 file changed, 45 insertions(+), 25 deletions(-) diff --git a/bitcoin/test/fuzz/miniscript.cpp b/bitcoin/test/fuzz/miniscript.cpp index 254d28c..1b791fc 100644 --- a/bitcoin/test/fuzz/miniscript.cpp +++ b/bitcoin/test/fuzz/miniscript.cpp @@ -1,4 +1,4 @@ -// Copyright (c) 2021 The Bitcoin Core developers +// Copyright (c) 2021-2022 The Bitcoin Core developers // Distributed under the MIT software license, see the accompanying // file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -14,7 +14,7 @@ namespace { -//! Some pre-computed data to simulate challenges. +//! Some pre-computed data for more efficient string roundtrips and to simulate challenges. struct TestData { typedef CPubKey Key; @@ -72,10 +72,18 @@ struct TestData { } } TEST_DATA; -//! Context to parse a Miniscript node to and from Script or text representation. +/** + * Context to parse a Miniscript node to and from Script or text representation. + * Uses an integer (an index in the dummy keys array from the test data) as keys in order + * to focus on fuzzing the Miniscript nodes' test representation, not the key representation. + */ struct ParserContext { typedef CPubKey Key; + bool KeyCompare(const Key& a, const Key& b) const { + return a < b; + } + std::optional ToString(const Key& key) const { auto it = TEST_DATA.dummy_key_idx_map.find(key); @@ -84,12 +92,12 @@ struct ParserContext { return HexStr(Span{&idx, 1}); } - const std::vector ToPKBytes(const Key& key) const + std::vector ToPKBytes(const Key& key) const { return {key.begin(), key.end()}; } - const std::vector ToPKHBytes(const Key& key) const + std::vector ToPKHBytes(const Key& key) const { const auto h = Hash160(key); return {h.begin(), h.end()}; @@ -104,7 +112,7 @@ struct ParserContext { } template - std::optional FromPKBytes(I first, I last) const { + std::optional FromPKBytes(I first, I last) const { CPubKey key; key.Set(first, last); if (!key.IsValid()) return {}; @@ -112,7 +120,7 @@ struct ParserContext { } template - std::optional FromPKHBytes(I first, I last) const { + std::optional FromPKHBytes(I first, I last) const { assert(last - first == 20); CKeyID keyid; std::copy(first, last, keyid.begin()); @@ -124,20 +132,23 @@ struct ParserContext { //! Context that implements naive conversion from/to script only, for roundtrip testing. struct ScriptParserContext { + //! For Script roundtrip we never need the key from a key hash. struct Key { bool is_hash; std::vector data; - - bool operator<(Key k) const { return data < k.data; } }; + bool KeyCompare(const Key& a, const Key& b) const { + return a.data < b.data; + } + const std::vector& ToPKBytes(const Key& key) const { assert(!key.is_hash); return key.data; } - const std::vector ToPKHBytes(const Key& key) const + std::vector ToPKHBytes(const Key& key) const { if (key.is_hash) return key.data; const auto h = Hash160(key.data); @@ -223,6 +234,13 @@ struct CheckerContext: BaseSignatureChecker { bool CheckSequence(const CScriptNum& nSequence) const override { return nSequence.GetInt64() & 1; } } CHECKER_CTX; +//! Context to check for duplicates when instancing a Node. +struct KeyComparator { + bool KeyCompare(const CPubKey& a, const CPubKey& b) const { + return a < b; + } +} KEY_COMP; + // A dummy scriptsig to pass to VerifyScript (we always use Segwit v0). const CScript DUMMY_SCRIPTSIG; @@ -230,10 +248,12 @@ using Fragment = miniscript::Fragment; using NodeRef = miniscript::NodeRef; using Node = miniscript::Node; using Type = miniscript::Type; +// https://github.com/llvm/llvm-project/issues/53444 +// NOLINTNEXTLINE(misc-unused-using-decls) using miniscript::operator"" _mst; //! Construct a miniscript node as a shared_ptr. -template NodeRef MakeNodeRef(Args&&... args) { return miniscript::MakeNodeRef(std::forward(args)...); } +template NodeRef MakeNodeRef(Args&&... args) { return miniscript::MakeNodeRef(KEY_COMP, std::forward(args)...); } /** Information about a yet to be constructed Miniscript node. */ struct NodeInfo { @@ -577,7 +597,7 @@ struct SmartInfo // Remove all recipes which involve non-constructible types. type_it->second.erase(std::remove_if(type_it->second.begin(), type_it->second.end(), [&](const recipe& rec) { - return !std::all_of(rec.second.begin(), rec.second.end(), known_constructible); + return !std::all_of(rec.second.begin(), rec.second.end(), known_constructible); }), type_it->second.end()); // Delete types entirely which have no recipes left. if (type_it->second.empty()) { @@ -700,7 +720,7 @@ NodeRef GenNode(F ConsumeNode, Type root_type = ""_mst, bool strict_valid = fals // Fragment/children have not been decided yet. Decide them. auto node_info = ConsumeNode(type_needed); if (!node_info) return {}; - auto subtypes = std::move(node_info)->subtypes; + auto subtypes = node_info->subtypes; todo.back().second = std::move(node_info); todo.reserve(todo.size() + subtypes.size()); // As elements on the todo stack are processed back to front, construct @@ -713,7 +733,7 @@ NodeRef GenNode(F ConsumeNode, Type root_type = ""_mst, bool strict_valid = fals // those children have been constructed at the back of stack. Pop // that entry off todo, and use it to construct a new NodeRef on // stack. - const NodeInfo& info = *todo.back().second; + NodeInfo& info = *todo.back().second; // Gather children from the back of stack. std::vector sub; sub.reserve(info.n_subs); @@ -751,9 +771,9 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) if (!node) return; // Check that it roundtrips to text representation - std::string str; - assert(node->ToString(PARSER_CTX, str)); - auto parsed = miniscript::FromString(str, PARSER_CTX); + std::optional str{node->ToString(PARSER_CTX)}; + assert(str); + auto parsed = miniscript::FromString(*str, PARSER_CTX); assert(parsed); assert(*parsed == *node); @@ -780,7 +800,7 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) assert(decoded->ToScript(PARSER_CTX) == script); assert(decoded->GetType() == node->GetType()); - if (provider.ConsumeBool()) { + if (provider.ConsumeBool() && node->GetOps() < MAX_OPS_PER_SCRIPT && node->ScriptSize() < MAX_STANDARD_P2WSH_SCRIPT_SIZE) { // Optionally pad the script with OP_NOPs to max op the ops limit of the constructed script. // This makes the script obviously not actually miniscript-compatible anymore, but the // signatures constructed in this test don't commit to the script anyway, so the same @@ -835,13 +855,13 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) assert(res || serror == ScriptError::SCRIPT_ERR_OP_COUNT || serror == ScriptError::SCRIPT_ERR_STACK_SIZE); } - if (node->IsSaneTopLevel()) { + if (node->IsSane()) { // For sane nodes, the two algorithms behave identically. assert(mal_success == nonmal_success); } // Verify that if a node is policy-satisfiable, the malleable satisfaction - // algorithm succeeds. Given that under IsSaneTopLevel() both satisfactions + // algorithm succeeds. Given that under IsSane() both satisfactions // are identical, this implies that for such nodes, the non-malleable // satisfaction will also match the expected policy. bool satisfiable = node->IsSatisfiable([](const Node& node) -> bool { @@ -880,6 +900,8 @@ void TestNode(const NodeRef& node, FuzzedDataProvider& provider) assert(mal_success == satisfiable); } +} // namespace + void FuzzInit() { ECC_Start(); @@ -892,8 +914,6 @@ void FuzzInitSmart() SMARTINFO.Init(); } -} // namespace - /** Fuzz target that runs TestNode on nodes generated using ConsumeNodeStable. */ FUZZ_TARGET_INIT(miniscript_stable, FuzzInit) { @@ -923,9 +943,9 @@ FUZZ_TARGET_INIT(miniscript_string, FuzzInit) auto parsed = miniscript::FromString(str, PARSER_CTX); if (!parsed) return; - std::string str2; - assert(parsed->ToString(PARSER_CTX, str2)); - auto parsed2 = miniscript::FromString(str2, PARSER_CTX); + const auto str2 = parsed->ToString(PARSER_CTX); + assert(str2); + auto parsed2 = miniscript::FromString(*str2, PARSER_CTX); assert(parsed2); assert(*parsed == *parsed2); }