From 84f3939ece9f4901141b28fd2dd6e3899d01d66e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 11 Mar 2021 18:58:27 -0800 Subject: [PATCH 1/7] Remove support for subdescriptors expanding to multiple scripts --- src/script/descriptor.cpp | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 30399dca51..ed382f2a54 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -484,6 +484,7 @@ protected: //! The sub-descriptor argument (nullptr for everything but SH and WSH). //! In doc/descriptors.m this is referred to as SCRIPT expressions sh(SCRIPT) //! and wsh(SCRIPT), and distinct from KEY expressions and ADDR expressions. + //! Subdescriptors can only ever generate a single script. const std::unique_ptr m_subdescriptor_arg; //! Return a serialization of anything except pubkey and script arguments, to be prepended to those. @@ -491,8 +492,7 @@ protected: /** A helper function to construct the scripts for this descriptor. * - * This function is invoked once for every CScript produced by evaluating - * m_subdescriptor_arg, or just once in case m_subdescriptor_arg is nullptr. + * This function is invoked once by ExpandHelper. * @param pubkeys The evaluations of the m_pubkey_args field. * @param script The evaluation of m_subdescriptor_arg (or nullptr when m_subdescriptor_arg is nullptr). @@ -586,6 +586,7 @@ public: if (m_subdescriptor_arg) { FlatSigningProvider subprovider; if (!m_subdescriptor_arg->ExpandHelper(pos, arg, read_cache, subscripts, subprovider, write_cache)) return false; + assert(subscripts.size() == 1); out = Merge(out, subprovider); } @@ -596,13 +597,8 @@ public: out.origins.emplace(entry.first.GetID(), std::make_pair(CPubKey(entry.first), std::move(entry.second))); } if (m_subdescriptor_arg) { - for (const auto& subscript : subscripts) { - out.scripts.emplace(CScriptID(subscript), subscript); - std::vector addscripts = MakeScripts(pubkeys, &subscript, out); - for (auto& addscript : addscripts) { - output_scripts.push_back(std::move(addscript)); - } - } + out.scripts.emplace(CScriptID(subscripts[0]), subscripts[0]); + output_scripts = MakeScripts(pubkeys, &subscripts[0], out); } else { output_scripts = MakeScripts(pubkeys, nullptr, out); } From a917478db0788b244c0c799b98bf67a94df7035e Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 4 Feb 2021 16:11:05 -0800 Subject: [PATCH 2/7] refactor: move population of out.scripts from ExpandHelper to MakeScripts There are currently two DescriptorImpl subclasses that rely on the functionality that ExpandHelper automatically adds subscripts to the output SigningProvider. Taproot descriptors will have subscripts, but we don't want them in the SigningProvider's bare script field. To avoid them ending up there, move this functionality into the specific classes' MakeScripts implementation. --- src/script/descriptor.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index ed382f2a54..d15cba1fa8 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -497,7 +497,7 @@ protected: * @param pubkeys The evaluations of the m_pubkey_args field. * @param script The evaluation of m_subdescriptor_arg (or nullptr when m_subdescriptor_arg is nullptr). * @param out A FlatSigningProvider to put scripts or public keys in that are necessary to the solver. - * The script arguments to this function are automatically added, as is the origin info of the provided pubkeys. + * The origin info of the provided pubkeys is automatically added. * @return A vector with scriptPubKeys for this descriptor. */ virtual std::vector MakeScripts(const std::vector& pubkeys, const CScript* script, FlatSigningProvider& out) const = 0; @@ -597,7 +597,6 @@ public: out.origins.emplace(entry.first.GetID(), std::make_pair(CPubKey(entry.first), std::move(entry.second))); } if (m_subdescriptor_arg) { - out.scripts.emplace(CScriptID(subscripts[0]), subscripts[0]); output_scripts = MakeScripts(pubkeys, &subscripts[0], out); } else { output_scripts = MakeScripts(pubkeys, nullptr, out); @@ -776,7 +775,12 @@ public: class SHDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector&, const CScript* script, FlatSigningProvider&) const override { return Vector(GetScriptForDestination(ScriptHash(*script))); } + std::vector MakeScripts(const std::vector&, const CScript* script, FlatSigningProvider& out) const override + { + auto ret = Vector(GetScriptForDestination(ScriptHash(*script))); + if (ret.size()) out.scripts.emplace(CScriptID(*script), *script); + return ret; + } public: SHDescriptor(std::unique_ptr desc) : DescriptorImpl({}, std::move(desc), "sh") {} @@ -793,7 +797,12 @@ public: class WSHDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector&, const CScript* script, FlatSigningProvider&) const override { return Vector(GetScriptForDestination(WitnessV0ScriptHash(*script))); } + std::vector MakeScripts(const std::vector&, const CScript* script, FlatSigningProvider& out) const override + { + auto ret = Vector(GetScriptForDestination(WitnessV0ScriptHash(*script))); + if (ret.size()) out.scripts.emplace(CScriptID(*script), *script); + return ret; + } public: WSHDescriptor(std::unique_ptr desc) : DescriptorImpl({}, std::move(desc), "wsh") {} std::optional GetOutputType() const override { return OutputType::BECH32; } From 4441c6f3c046c0b28ce3f0ca6d938af71d797586 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Thu, 4 Feb 2021 15:58:33 -0800 Subject: [PATCH 3/7] Make DescriptorImpl support multiple subscripts So far, no descriptor exists that supports more than one sub-script descriptor. This will change with taproot, so prepare for this by changing the m_subdescriptor_arg from a unique_ptr to a vector of unique_ptr's. --- src/script/descriptor.cpp | 96 +++++++++++++++++++-------------------- 1 file changed, 47 insertions(+), 49 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index d15cba1fa8..5fbc740608 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -481,11 +481,11 @@ class DescriptorImpl : public Descriptor const std::string m_name; protected: - //! The sub-descriptor argument (nullptr for everything but SH and WSH). + //! The sub-descriptor arguments (empty for everything but SH and WSH). //! In doc/descriptors.m this is referred to as SCRIPT expressions sh(SCRIPT) //! and wsh(SCRIPT), and distinct from KEY expressions and ADDR expressions. //! Subdescriptors can only ever generate a single script. - const std::unique_ptr m_subdescriptor_arg; + const std::vector> m_subdescriptor_args; //! Return a serialization of anything except pubkey and script arguments, to be prepended to those. virtual std::string ToStringExtra() const { return ""; } @@ -493,22 +493,23 @@ protected: /** A helper function to construct the scripts for this descriptor. * * This function is invoked once by ExpandHelper. - + * * @param pubkeys The evaluations of the m_pubkey_args field. - * @param script The evaluation of m_subdescriptor_arg (or nullptr when m_subdescriptor_arg is nullptr). + * @param script The evaluations of m_subdescriptor_args (one for each m_subdescriptor_args element). * @param out A FlatSigningProvider to put scripts or public keys in that are necessary to the solver. * The origin info of the provided pubkeys is automatically added. * @return A vector with scriptPubKeys for this descriptor. */ - virtual std::vector MakeScripts(const std::vector& pubkeys, const CScript* script, FlatSigningProvider& out) const = 0; + virtual std::vector MakeScripts(const std::vector& pubkeys, Span scripts, FlatSigningProvider& out) const = 0; public: - DescriptorImpl(std::vector> pubkeys, std::unique_ptr script, const std::string& name) : m_pubkey_args(std::move(pubkeys)), m_name(name), m_subdescriptor_arg(std::move(script)) {} + DescriptorImpl(std::vector> pubkeys, const std::string& name) : m_pubkey_args(std::move(pubkeys)), m_name(name), m_subdescriptor_args() {} + DescriptorImpl(std::vector> pubkeys, std::unique_ptr script, const std::string& name) : m_pubkey_args(std::move(pubkeys)), m_name(name), m_subdescriptor_args(Vector(std::move(script))) {} bool IsSolvable() const override { - if (m_subdescriptor_arg) { - if (!m_subdescriptor_arg->IsSolvable()) return false; + for (const auto& arg : m_subdescriptor_args) { + if (!arg->IsSolvable()) return false; } return true; } @@ -518,8 +519,8 @@ public: for (const auto& pubkey : m_pubkey_args) { if (pubkey->IsRange()) return true; } - if (m_subdescriptor_arg) { - if (m_subdescriptor_arg->IsRange()) return true; + for (const auto& arg : m_subdescriptor_args) { + if (arg->IsRange()) return true; } return false; } @@ -541,10 +542,10 @@ public: } ret += std::move(tmp); } - if (m_subdescriptor_arg) { + for (const auto& scriptarg : m_subdescriptor_args) { if (pos++) ret += ","; std::string tmp; - if (!m_subdescriptor_arg->ToStringHelper(arg, tmp, priv, normalized)) return false; + if (!scriptarg->ToStringHelper(arg, tmp, priv, normalized)) return false; ret += std::move(tmp); } out = std::move(ret) + ")"; @@ -577,18 +578,20 @@ public: std::vector> entries; entries.reserve(m_pubkey_args.size()); - // Construct temporary data in `entries` and `subscripts`, to avoid producing output in case of failure. + // Construct temporary data in `entries`, `subscripts`, and `subprovider` to avoid producing output in case of failure. for (const auto& p : m_pubkey_args) { entries.emplace_back(); if (!p->GetPubKey(pos, arg, entries.back().first, entries.back().second, read_cache, write_cache)) return false; } std::vector subscripts; - if (m_subdescriptor_arg) { - FlatSigningProvider subprovider; - if (!m_subdescriptor_arg->ExpandHelper(pos, arg, read_cache, subscripts, subprovider, write_cache)) return false; - assert(subscripts.size() == 1); - out = Merge(out, subprovider); + FlatSigningProvider subprovider; + for (const auto& subarg : m_subdescriptor_args) { + std::vector outscripts; + if (!subarg->ExpandHelper(pos, arg, read_cache, outscripts, subprovider, write_cache)) return false; + assert(outscripts.size() == 1); + subscripts.emplace_back(std::move(outscripts[0])); } + out = Merge(std::move(out), std::move(subprovider)); std::vector pubkeys; pubkeys.reserve(entries.size()); @@ -596,11 +599,8 @@ public: pubkeys.push_back(entry.first); out.origins.emplace(entry.first.GetID(), std::make_pair(CPubKey(entry.first), std::move(entry.second))); } - if (m_subdescriptor_arg) { - output_scripts = MakeScripts(pubkeys, &subscripts[0], out); - } else { - output_scripts = MakeScripts(pubkeys, nullptr, out); - } + + output_scripts = MakeScripts(pubkeys, MakeSpan(subscripts), out); return true; } @@ -621,10 +621,8 @@ public: if (!p->GetPrivKey(pos, provider, key)) continue; out.keys.emplace(key.GetPubKey().GetID(), key); } - if (m_subdescriptor_arg) { - FlatSigningProvider subprovider; - m_subdescriptor_arg->ExpandPrivate(pos, provider, subprovider); - out = Merge(out, subprovider); + for (const auto& arg : m_subdescriptor_args) { + arg->ExpandPrivate(pos, provider, out); } } @@ -637,9 +635,9 @@ class AddressDescriptor final : public DescriptorImpl const CTxDestination m_destination; protected: std::string ToStringExtra() const override { return EncodeDestination(m_destination); } - std::vector MakeScripts(const std::vector&, const CScript*, FlatSigningProvider&) const override { return Vector(GetScriptForDestination(m_destination)); } + std::vector MakeScripts(const std::vector&, Span, FlatSigningProvider&) const override { return Vector(GetScriptForDestination(m_destination)); } public: - AddressDescriptor(CTxDestination destination) : DescriptorImpl({}, {}, "addr"), m_destination(std::move(destination)) {} + AddressDescriptor(CTxDestination destination) : DescriptorImpl({}, "addr"), m_destination(std::move(destination)) {} bool IsSolvable() const final { return false; } std::optional GetOutputType() const override @@ -663,9 +661,9 @@ class RawDescriptor final : public DescriptorImpl const CScript m_script; protected: std::string ToStringExtra() const override { return HexStr(m_script); } - std::vector MakeScripts(const std::vector&, const CScript*, FlatSigningProvider&) const override { return Vector(m_script); } + std::vector MakeScripts(const std::vector&, Span, FlatSigningProvider&) const override { return Vector(m_script); } public: - RawDescriptor(CScript script) : DescriptorImpl({}, {}, "raw"), m_script(std::move(script)) {} + RawDescriptor(CScript script) : DescriptorImpl({}, "raw"), m_script(std::move(script)) {} bool IsSolvable() const final { return false; } std::optional GetOutputType() const override @@ -689,9 +687,9 @@ public: class PKDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector& keys, const CScript*, FlatSigningProvider&) const override { return Vector(GetScriptForRawPubKey(keys[0])); } + std::vector MakeScripts(const std::vector& keys, Span, FlatSigningProvider&) const override { return Vector(GetScriptForRawPubKey(keys[0])); } public: - PKDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), {}, "pk") {} + PKDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "pk") {} bool IsSingleType() const final { return true; } }; @@ -699,14 +697,14 @@ public: class PKHDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector& keys, const CScript*, FlatSigningProvider& out) const override + std::vector MakeScripts(const std::vector& keys, Span, FlatSigningProvider& out) const override { CKeyID id = keys[0].GetID(); out.pubkeys.emplace(id, keys[0]); return Vector(GetScriptForDestination(PKHash(id))); } public: - PKHDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), {}, "pkh") {} + PKHDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "pkh") {} std::optional GetOutputType() const override { return OutputType::LEGACY; } bool IsSingleType() const final { return true; } }; @@ -715,14 +713,14 @@ public: class WPKHDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector& keys, const CScript*, FlatSigningProvider& out) const override + std::vector MakeScripts(const std::vector& keys, Span, FlatSigningProvider& out) const override { CKeyID id = keys[0].GetID(); out.pubkeys.emplace(id, keys[0]); return Vector(GetScriptForDestination(WitnessV0KeyHash(id))); } public: - WPKHDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), {}, "wpkh") {} + WPKHDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "wpkh") {} std::optional GetOutputType() const override { return OutputType::BECH32; } bool IsSingleType() const final { return true; } }; @@ -731,7 +729,7 @@ public: class ComboDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector& keys, const CScript*, FlatSigningProvider& out) const override + std::vector MakeScripts(const std::vector& keys, Span, FlatSigningProvider& out) const override { std::vector ret; CKeyID id = keys[0].GetID(); @@ -747,7 +745,7 @@ protected: return ret; } public: - ComboDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), {}, "combo") {} + ComboDescriptor(std::unique_ptr prov) : DescriptorImpl(Vector(std::move(prov)), "combo") {} bool IsSingleType() const final { return false; } }; @@ -758,7 +756,7 @@ class MultisigDescriptor final : public DescriptorImpl const bool m_sorted; protected: std::string ToStringExtra() const override { return strprintf("%i", m_threshold); } - std::vector MakeScripts(const std::vector& keys, const CScript*, FlatSigningProvider&) const override { + std::vector MakeScripts(const std::vector& keys, Span, FlatSigningProvider&) const override { if (m_sorted) { std::vector sorted_keys(keys); std::sort(sorted_keys.begin(), sorted_keys.end()); @@ -767,7 +765,7 @@ protected: return Vector(GetScriptForMultisig(m_threshold, keys)); } public: - MultisigDescriptor(int threshold, std::vector> providers, bool sorted = false) : DescriptorImpl(std::move(providers), {}, sorted ? "sortedmulti" : "multi"), m_threshold(threshold), m_sorted(sorted) {} + MultisigDescriptor(int threshold, std::vector> providers, bool sorted = false) : DescriptorImpl(std::move(providers), sorted ? "sortedmulti" : "multi"), m_threshold(threshold), m_sorted(sorted) {} bool IsSingleType() const final { return true; } }; @@ -775,10 +773,10 @@ public: class SHDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector&, const CScript* script, FlatSigningProvider& out) const override + std::vector MakeScripts(const std::vector&, Span scripts, FlatSigningProvider& out) const override { - auto ret = Vector(GetScriptForDestination(ScriptHash(*script))); - if (ret.size()) out.scripts.emplace(CScriptID(*script), *script); + auto ret = Vector(GetScriptForDestination(ScriptHash(scripts[0]))); + if (ret.size()) out.scripts.emplace(CScriptID(scripts[0]), scripts[0]); return ret; } public: @@ -786,8 +784,8 @@ public: std::optional GetOutputType() const override { - assert(m_subdescriptor_arg); - if (m_subdescriptor_arg->GetOutputType() == OutputType::BECH32) return OutputType::P2SH_SEGWIT; + assert(m_subdescriptor_args.size() == 1); + if (m_subdescriptor_args[0]->GetOutputType() == OutputType::BECH32) return OutputType::P2SH_SEGWIT; return OutputType::LEGACY; } bool IsSingleType() const final { return true; } @@ -797,10 +795,10 @@ public: class WSHDescriptor final : public DescriptorImpl { protected: - std::vector MakeScripts(const std::vector&, const CScript* script, FlatSigningProvider& out) const override + std::vector MakeScripts(const std::vector&, Span scripts, FlatSigningProvider& out) const override { - auto ret = Vector(GetScriptForDestination(WitnessV0ScriptHash(*script))); - if (ret.size()) out.scripts.emplace(CScriptID(*script), *script); + auto ret = Vector(GetScriptForDestination(WitnessV0ScriptHash(scripts[0]))); + if (ret.size()) out.scripts.emplace(CScriptID(scripts[0]), scripts[0]); return ret; } public: From 6ba5dda0c9de75196c6a427d9e59d39e5a41bff7 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sat, 6 Feb 2021 17:47:52 -0800 Subject: [PATCH 4/7] Account for key cache indices in subexpressions This has no effect for now, as the only fragments with sub-script expressions (sh, wsh) only allow one, and don't have key expressions in them. A future Taproot descriptor will however violate both, and we want the keys in different sub-scripts to be assigned non-overlapping cache indices. --- src/script/descriptor.cpp | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 5fbc740608..a1ef23ea1a 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -939,7 +939,7 @@ std::unique_ptr ParsePubkey(uint32_t key_exp_index, const Span ParseScript(uint32_t key_exp_index, Span& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error) +std::unique_ptr ParseScript(uint32_t& key_exp_index, Span& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error) { using namespace spanparsing; @@ -948,16 +948,19 @@ std::unique_ptr ParseScript(uint32_t key_exp_index, Span(std::move(pubkey)); } if (Func("pkh", expr)) { auto pubkey = ParsePubkey(key_exp_index, expr, ctx != ParseScriptContext::P2WSH, out, error); if (!pubkey) return nullptr; + ++key_exp_index; return std::make_unique(std::move(pubkey)); } if (ctx == ParseScriptContext::TOP && Func("combo", expr)) { auto pubkey = ParsePubkey(key_exp_index, expr, true, out, error); if (!pubkey) return nullptr; + ++key_exp_index; return std::make_unique(std::move(pubkey)); } else if (ctx != ParseScriptContext::TOP && Func("combo", expr)) { error = "Cannot have combo in non-top level"; @@ -1011,6 +1014,7 @@ std::unique_ptr ParseScript(uint32_t key_exp_index, Span(std::move(pubkey)); } else if (ctx == ParseScriptContext::P2WSH && Func("wpkh", expr)) { error = "Cannot have wpkh within wsh"; @@ -1177,7 +1181,8 @@ std::unique_ptr Parse(const std::string& descriptor, FlatSigningProv { Span sp{descriptor}; if (!CheckChecksum(sp, require_checksum, error)) return nullptr; - auto ret = ParseScript(0, sp, ParseScriptContext::TOP, out, error); + uint32_t key_exp_index = 0; + auto ret = ParseScript(key_exp_index, sp, ParseScriptContext::TOP, out, error); if (sp.size() == 0 && ret) return std::unique_ptr(std::move(ret)); return nullptr; } From 17e006ff8d5e42f22474c5191d1b745bbc97571f Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 7 Feb 2021 19:18:07 -0800 Subject: [PATCH 5/7] refactor: split off subscript logic from ToStringHelper This will allow subclasses to overwrite the serialization of subscript arguments without needing to reimplement all the rest of the ToString logic. --- src/script/descriptor.cpp | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index a1ef23ea1a..6253f1b2b0 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -525,6 +525,18 @@ public: return false; } + virtual bool ToStringSubScriptHelper(const SigningProvider* arg, std::string& ret, bool priv, bool normalized) const + { + size_t pos = 0; + for (const auto& scriptarg : m_subdescriptor_args) { + if (pos++) ret += ","; + std::string tmp; + if (!scriptarg->ToStringHelper(arg, tmp, priv, normalized)) return false; + ret += std::move(tmp); + } + return true; + } + bool ToStringHelper(const SigningProvider* arg, std::string& out, bool priv, bool normalized) const { std::string extra = ToStringExtra(); @@ -542,13 +554,10 @@ public: } ret += std::move(tmp); } - for (const auto& scriptarg : m_subdescriptor_args) { - if (pos++) ret += ","; - std::string tmp; - if (!scriptarg->ToStringHelper(arg, tmp, priv, normalized)) return false; - ret += std::move(tmp); - } - out = std::move(ret) + ")"; + std::string subscript; + if (!ToStringSubScriptHelper(arg, subscript, priv, normalized)) return false; + if (pos && subscript.size()) ret += ','; + out = std::move(ret) + std::move(subscript) + ")"; return true; } From 33275a96490445e293c322a29a3b146ccb91083c Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Wed, 17 Feb 2021 15:37:40 -0800 Subject: [PATCH 6/7] refactor: move uncompressed-permitted logic into ParsePubkey* This is a preparation for parsing xonly pubkeys, which will complicate this logic. It's cleaner to put the decision logic close to the public key parsing itself. --- src/script/descriptor.cpp | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 6253f1b2b0..59c3b2e0a6 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -821,9 +821,10 @@ public: //////////////////////////////////////////////////////////////////////////// enum class ParseScriptContext { - TOP, - P2SH, - P2WSH, + TOP, //!< Top-level context (script goes directly in scriptPubKey) + P2SH, //!< Inside sh() (script becomes P2SH redeemScript) + P2WPKH, //!< Inside wpkh() (no script, pubkey only) + P2WSH, //!< Inside wsh() (script becomes v0 witness script) }; /** Parse a key path, being passed a split list of elements (the first element is ignored). */ @@ -850,10 +851,11 @@ enum class ParseScriptContext { } /** Parse a public key that excludes origin information. */ -std::unique_ptr ParsePubkeyInner(uint32_t key_exp_index, const Span& sp, bool permit_uncompressed, FlatSigningProvider& out, std::string& error) +std::unique_ptr ParsePubkeyInner(uint32_t key_exp_index, const Span& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error) { using namespace spanparsing; + bool permit_uncompressed = ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH; auto split = Split(sp, '/'); std::string str(split[0].begin(), split[0].end()); if (str.size() == 0) { @@ -911,7 +913,7 @@ std::unique_ptr ParsePubkeyInner(uint32_t key_exp_index, const S } /** Parse a public key including origin information (if enabled). */ -std::unique_ptr ParsePubkey(uint32_t key_exp_index, const Span& sp, bool permit_uncompressed, FlatSigningProvider& out, std::string& error) +std::unique_ptr ParsePubkey(uint32_t key_exp_index, const Span& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error) { using namespace spanparsing; @@ -920,7 +922,7 @@ std::unique_ptr ParsePubkey(uint32_t key_exp_index, const Span ParsePubkey(uint32_t key_exp_index, const Span(key_exp_index, std::move(info), std::move(provider)); } @@ -955,19 +957,19 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, Span(std::move(pubkey)); } if (Func("pkh", expr)) { - auto pubkey = ParsePubkey(key_exp_index, expr, ctx != ParseScriptContext::P2WSH, out, error); + auto pubkey = ParsePubkey(key_exp_index, expr, ctx, out, error); if (!pubkey) return nullptr; ++key_exp_index; return std::make_unique(std::move(pubkey)); } if (ctx == ParseScriptContext::TOP && Func("combo", expr)) { - auto pubkey = ParsePubkey(key_exp_index, expr, true, out, error); + auto pubkey = ParsePubkey(key_exp_index, expr, ctx, out, error); if (!pubkey) return nullptr; ++key_exp_index; return std::make_unique(std::move(pubkey)); @@ -990,7 +992,7 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, SpanGetSize() + 1; providers.emplace_back(std::move(pk)); @@ -1021,7 +1023,7 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, Span(thres, std::move(providers), sorted_multi); } if (ctx != ParseScriptContext::P2WSH && Func("wpkh", expr)) { - auto pubkey = ParsePubkey(key_exp_index, expr, false, out, error); + auto pubkey = ParsePubkey(key_exp_index, expr, ParseScriptContext::P2WPKH, out, error); if (!pubkey) return nullptr; key_exp_index++; return std::make_unique(std::move(pubkey)); From 0b188b751f970027c52729e0c223cc9257669322 Mon Sep 17 00:00:00 2001 From: Pieter Wuille Date: Sun, 7 Feb 2021 19:31:46 -0800 Subject: [PATCH 7/7] Clean up context dependent checks in descriptor parsing This changes all context dependent checks in the parser to be disjunctions of equality checks, rather than also including inequalities. This makes sure that adding a new context enum in the future won't change semantics for existing checks. The error messages are also made a bit more consistent. --- src/script/descriptor.cpp | 26 ++++++++++++++++---------- src/test/descriptor_tests.cpp | 12 ++++++------ 2 files changed, 22 insertions(+), 16 deletions(-) diff --git a/src/script/descriptor.cpp b/src/script/descriptor.cpp index 59c3b2e0a6..5f085d233a 100644 --- a/src/script/descriptor.cpp +++ b/src/script/descriptor.cpp @@ -973,8 +973,8 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, Span(std::move(pubkey)); - } else if (ctx != ParseScriptContext::TOP && Func("combo", expr)) { - error = "Cannot have combo in non-top level"; + } else if (Func("combo", expr)) { + error = "Can only have combo() at top level"; return nullptr; } if ((sorted_multi = Func("sortedmulti", expr)) || Func("multi", expr)) { @@ -1022,29 +1022,29 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, Span(thres, std::move(providers), sorted_multi); } - if (ctx != ParseScriptContext::P2WSH && Func("wpkh", expr)) { + if ((ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH) && Func("wpkh", expr)) { auto pubkey = ParsePubkey(key_exp_index, expr, ParseScriptContext::P2WPKH, out, error); if (!pubkey) return nullptr; key_exp_index++; return std::make_unique(std::move(pubkey)); - } else if (ctx == ParseScriptContext::P2WSH && Func("wpkh", expr)) { - error = "Cannot have wpkh within wsh"; + } else if (Func("wpkh", expr)) { + error = "Can only have wpkh() at top level or inside sh()"; return nullptr; } if (ctx == ParseScriptContext::TOP && Func("sh", expr)) { auto desc = ParseScript(key_exp_index, expr, ParseScriptContext::P2SH, out, error); if (!desc || expr.size()) return nullptr; return std::make_unique(std::move(desc)); - } else if (ctx != ParseScriptContext::TOP && Func("sh", expr)) { - error = "Cannot have sh in non-top level"; + } else if (Func("sh", expr)) { + error = "Can only have sh() at top level"; return nullptr; } - if (ctx != ParseScriptContext::P2WSH && Func("wsh", expr)) { + if ((ctx == ParseScriptContext::TOP || ctx == ParseScriptContext::P2SH) && Func("wsh", expr)) { auto desc = ParseScript(key_exp_index, expr, ParseScriptContext::P2WSH, out, error); if (!desc || expr.size()) return nullptr; return std::make_unique(std::move(desc)); - } else if (ctx == ParseScriptContext::P2WSH && Func("wsh", expr)) { - error = "Cannot have wsh within wsh"; + } else if (Func("wsh", expr)) { + error = "Can only have wsh() at top level or inside sh()"; return nullptr; } if (ctx == ParseScriptContext::TOP && Func("addr", expr)) { @@ -1054,6 +1054,9 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, Span(std::move(dest)); + } else if (Func("addr", expr)) { + error = "Can only have addr() at top level"; + return nullptr; } if (ctx == ParseScriptContext::TOP && Func("raw", expr)) { std::string str(expr.begin(), expr.end()); @@ -1063,6 +1066,9 @@ std::unique_ptr ParseScript(uint32_t& key_exp_index, Span(CScript(bytes.begin(), bytes.end())); + } else if (Func("raw", expr)) { + error = "Can only have raw() at top level"; + return nullptr; } if (ctx == ParseScriptContext::P2SH) { error = "A function is needed within P2SH"; diff --git a/src/test/descriptor_tests.cpp b/src/test/descriptor_tests.cpp index aecf955fee..ea41a03728 100644 --- a/src/test/descriptor_tests.cpp +++ b/src/test/descriptor_tests.cpp @@ -24,7 +24,7 @@ void CheckUnparsable(const std::string& prv, const std::string& pub, const std:: auto parse_pub = Parse(pub, keys_pub, error); BOOST_CHECK_MESSAGE(!parse_priv, prv); BOOST_CHECK_MESSAGE(!parse_pub, pub); - BOOST_CHECK(error == expected_error); + BOOST_CHECK_EQUAL(error, expected_error); } constexpr int DEFAULT = 0; @@ -355,12 +355,12 @@ BOOST_AUTO_TEST_CASE(descriptor_test) // Check for invalid nesting of structures CheckUnparsable("sh(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)", "sh(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)", "A function is needed within P2SH"); // P2SH needs a script, not a key - CheckUnparsable("sh(combo(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1))", "sh(combo(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", "Cannot have combo in non-top level"); // Old must be top level + CheckUnparsable("sh(combo(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1))", "sh(combo(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", "Can only have combo() at top level"); // Old must be top level CheckUnparsable("wsh(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)", "wsh(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)", "A function is needed within P2WSH"); // P2WSH needs a script, not a key - CheckUnparsable("wsh(wpkh(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1))", "wsh(wpkh(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", "Cannot have wpkh within wsh"); // Cannot embed witness inside witness - CheckUnparsable("wsh(sh(pk(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)))", "wsh(sh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)))", "Cannot have sh in non-top level"); // Cannot embed P2SH inside P2WSH - CheckUnparsable("sh(sh(pk(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)))", "sh(sh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)))", "Cannot have sh in non-top level"); // Cannot embed P2SH inside P2SH - CheckUnparsable("wsh(wsh(pk(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)))", "wsh(wsh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)))", "Cannot have wsh within wsh"); // Cannot embed P2WSH inside P2WSH + CheckUnparsable("wsh(wpkh(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1))", "wsh(wpkh(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd))", "Can only have wpkh() at top level or inside sh()"); // Cannot embed witness inside witness + CheckUnparsable("wsh(sh(pk(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)))", "wsh(sh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)))", "Can only have sh() at top level"); // Cannot embed P2SH inside P2WSH + CheckUnparsable("sh(sh(pk(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)))", "sh(sh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)))", "Can only have sh() at top level"); // Cannot embed P2SH inside P2SH + CheckUnparsable("wsh(wsh(pk(L4rK1yDtCWekvXuE6oXD9jCYfFNV2cWRpVuPLBcCU2z8TrisoyY1)))", "wsh(wsh(pk(03a34b99f22c790c4e36b2b3c2c35a36db06226e41c692fc82b8b56ac1c540c5bd)))", "Can only have wsh() at top level or inside sh()"); // Cannot embed P2WSH inside P2WSH // Checksums Check("sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#ggrsrxfy", "sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#tjg09x5t", "sh(multi(2,[00000000/111'/222]xprvA1RpRA33e1JQ7ifknakTFpgNXPmW2YvmhqLQYMmrj4xJXXWYpDPS3xz7iAxn8L39njGVyuoseXzU6rcxFLJ8HFsTjSyQbLYnMpCqE2VbFWc,xprv9uPDJpEQgRQfDcW7BkF7eTya6RPxXeJCqCJGHuCJ4GiRVLzkTXBAJMu2qaMWPrS7AANYqdq6vcBcBUdJCVVFceUvJFjaPdGZ2y9WACViL4L/0))#ggrsrxfy", "sh(multi(2,[00000000/111'/222]xpub6ERApfZwUNrhLCkDtcHTcxd75RbzS1ed54G1LkBUHQVHQKqhMkhgbmJbZRkrgZw4koxb5JaHWkY4ALHY2grBGRjaDMzQLcgJvLJuZZvRcEL,xpub68NZiKmJWnxxS6aaHmn81bvJeTESw724CRDs6HbuccFQN9Ku14VQrADWgqbhhTHBaohPX4CjNLf9fq9MYo6oDaPPLPxSb7gwQN3ih19Zm4Y/0))#tjg09x5t", DEFAULT, {{"a91445a9a622a8b0a1269944be477640eedc447bbd8487"}}, OutputType::LEGACY, {{0x8000006FUL,222},{0}});