Skip to content

Commit edd20c1

Browse files
committed
partial Merge bitcoin#24148: Miniscript support in Output Descriptors
ffc79b8 qa: functional test Miniscript watchonly support (Antoine Poinsot) bfb0367 Miniscript support in output descriptors (Antoine Poinsot) 4a08288 qa: better error reporting on descriptor parsing error (Antoine Poinsot) d25d58b miniscript: add a helper to find the first insane sub with no child (Antoine Poinsot) c38c7c5 miniscript: don't check for top level validity at parsing time (Antoine Poinsot) Pull request description: This adds Miniscript support for Output Descriptors without any signing logic (yet). See the OP of bitcoin#24147 for a description of Miniscript and a rationale of having it in Bitcoin Core. On its own, this PR adds "watchonly" support for Miniscript descriptors in the descriptor wallet. A follow-up adds signing support. A minified corpus of Miniscript Descriptors for the `descriptor_parse` fuzz target is available at bitcoin-core/qa-assets#92. The Miniscript descriptors used in the unit tests here and in bitcoin#24149 were cross-tested against the Rust implementation at https://github.com/rust-bitcoin/rust-miniscript. This PR contains code and insights from Pieter Wuille. ACKs for top commit: Sjors: re-utACK ffc79b8 achow101: ACK ffc79b8 w0xlt: reACK bitcoin@ffc79b8 Tree-SHA512: 02d919d38bb626d3c557eca3680ce71117739fa161b7a92cfdb6c9c432ed88870b1ed127ba24248574c40c7428217d7e9bdd986fd8cd7c51fae8c776e1271fb9 Dash adaptations: - src/script/descriptor.cpp: Dash has no segwit, so `ParseScriptContext::P2WSH` does not exist. Miniscript is wired to Dash's only hash-wrapped-script context, `ParseScriptContext::P2SH`, at all five upstream P2WSH sites: `KeyParser::FromString`, `KeyParser::FromPKBytes`, `KeyParser::FromPKHBytes`, the miniscript gate in `ParseScript`, and the miniscript gate in `InferScript`. Miniscript expressions therefore live inside `sh()` instead of `wsh()`. - src/script/descriptor.cpp: the user-visible error string was written as "Miniscript expressions can only be used in sh" instead of upstream's "... can only be used in wsh". - src/script/descriptor.cpp: upstream's new `InferPubkey` body uses `ConstPubkeyProvider(0, pubkey, /*xonly=*/false)` and a 3-arg `OriginPubkeyProvider`. Dash's `ConstPubkeyProvider` takes 2 args (no xonly flag) and `OriginPubkeyProvider` takes a trailing `apostrophe` flag, so the moved-up `InferPubkey` was written as `ConstPubkeyProvider(0, pubkey)` / `OriginPubkeyProvider(0, std::move(info), std::move(key_provider), /*apostrophe=*/false)`. - src/script/descriptor.cpp: `InferXOnlyPubkey` and upstream's `InferMultiA` were dropped rather than added — Dash has no `XOnlyPubKey` inference, no `MatchMultiA`, and no `MultiADescriptor`. - src/script/descriptor.cpp: the `DescriptorImpl` conflict was resolved keeping Dash's existing comment wording on `m_pubkey_args` while taking upstream's move of the `protected:` label above that member (so `MiniscriptDescriptor` can reach it). - src/test/descriptor_tests.cpp: the DoCheck hunks were resolved onto Dash's `Parse(prv, keys_priv, error)` / `UseHInsteadOfApostrophe(pub)` flow, keeping Dash's apostrophe-vs-h handling and taking upstream's `BOOST_CHECK_MESSAGE(EqualDescriptor(...))` upgrades. - src/test/descriptor_tests.cpp: upstream's negative Miniscript tests were rewritten from `wsh(...)` to `sh(...)`, and the expected error for the context check is "Miniscript expressions can only be used in sh". Upstream's private descriptors use Bitcoin mainnet WIF keys, which do not decode under Dash's base58 prefixes and could not be re-encoded in this environment, so the hex public keys are used for both the private and the public descriptor in those `CheckUnparsable` calls; a comment in the file records this. - test/functional/wallet_miniscript.py: every descriptor is wrapped in `sh(...)` instead of `wsh(...)` (the four `MINISCRIPTS` policies via `descsum_create(f"sh({ms})")`, and the insane-descriptor sanity check). All four policies compile to redeemScripts well under the 520-byte `MAX_SCRIPT_ELEMENT_SIZE` push limit. - test/functional/wallet_miniscript.py: added `add_options` calling `self.add_wallet_options(parser, legacy=False)`. Dash's test framework requires this declaration, otherwise `self.options.descriptors` is `None` and the node is started with `-disablewallet`. - test/functional/test_runner.py: the new entry is `'wallet_miniscript.py --descriptors'` — the explicit flag is needed because Dash's framework runs with `REQUIRE_WALLET_TYPE_SET`. Upstream's hunk also carried the context line `'feature_maxtipage.py'`; it was dropped because Dash already lists that test earlier in the file and re-adding it would duplicate the entry. Omitted (partial backport): - src/test/descriptor_tests.cpp: They assert exact witness-program scriptPubKeys and use `OutputType::BECH32` / `OutputType::P2SH_SEGWIT` / `MIXED_PUBKEYS`, none of which exist in Dash. The equivalent `sh()` scriptPubKeys cannot be re-derived without executing the code. The positive path is covered by test/functional/wallet_miniscript.py instead. - src/test/descriptor_tests.cpp: It needs a valid checksum for the corresponding `sh()` descriptor, which could not be computed in this environment (no script execution available), and inventing one would produce a test asserting the wrong error. - src/test/descriptor_tests.cpp: Dash has no taproot: no `tr()` descriptor, no `TRDescriptor`, no `ParseScriptContext::P2TR`.
1 parent 3a0c938 commit edd20c1

6 files changed

Lines changed: 388 additions & 20 deletions

File tree

‎src/script/descriptor.cpp‎

Lines changed: 222 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
#include <key_io.h>
88
#include <pubkey.h>
9+
#include <script/miniscript.h>
910
#include <script/script.h>
1011
#include <script/standard.h>
1112

@@ -161,6 +162,20 @@ struct PubkeyProvider
161162

162163
virtual ~PubkeyProvider() = default;
163164

165+
/** Compare two public keys represented by this provider.
166+
* Used by the Miniscript descriptors to check for duplicate keys in the script.
167+
*/
168+
bool operator<(PubkeyProvider& other) const {
169+
CPubKey a, b;
170+
SigningProvider dummy;
171+
KeyOriginInfo dummy_info;
172+
173+
GetPubKey(0, dummy, a, dummy_info);
174+
other.GetPubKey(0, dummy, b, dummy_info);
175+
176+
return a < b;
177+
}
178+
164179
/** Derive a public key.
165180
* read_cache is the cache to read keys from (if not nullptr)
166181
* write_cache is the cache to write keys to (if not nullptr)
@@ -516,12 +531,12 @@ class BIP32PubkeyProvider final : public PubkeyProvider
516531
/** Base class for all Descriptor implementations. */
517532
class DescriptorImpl : public Descriptor
518533
{
534+
protected:
519535
//! Public key arguments for this descriptor (size 1 for PK, PKH; any size for Multisig).
520536
const std::vector<std::unique_ptr<PubkeyProvider>> m_pubkey_args;
521537
//! The string name of the descriptor function.
522538
const std::string m_name;
523539

524-
protected:
525540
//! The sub-descriptor arguments (empty for everything but SH and WSH).
526541
//! In doc/descriptors.m this is referred to as SCRIPT expressions sh(SCRIPT)
527542
//! and wsh(SCRIPT), and distinct from KEY expressions and ADDR expressions.
@@ -585,7 +600,7 @@ class DescriptorImpl : public Descriptor
585600
return true;
586601
}
587602

588-
bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, const DescriptorCache* cache = nullptr) const
603+
virtual bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type, const DescriptorCache* cache = nullptr) const
589604
{
590605
std::string extra = ToStringExtra();
591606
size_t pos = extra.size() > 0 ? 1 : 0;
@@ -854,6 +869,89 @@ class ComboDescriptor final : public DescriptorImpl
854869
bool IsSingleType() const final { return false; }
855870
};
856871

872+
/* We instantiate Miniscript here with a simple integer as key type.
873+
* The value of these key integers are an index in the
874+
* DescriptorImpl::m_pubkey_args vector.
875+
*/
876+
877+
/**
878+
* The context for converting a Miniscript descriptor into a Script.
879+
*/
880+
class ScriptMaker {
881+
//! Keys contained in the Miniscript (the evaluation of DescriptorImpl::m_pubkey_args).
882+
const std::vector<CPubKey>& m_keys;
883+
884+
public:
885+
ScriptMaker(const std::vector<CPubKey>& keys LIFETIMEBOUND) : m_keys(keys) {}
886+
887+
std::vector<unsigned char> ToPKBytes(uint32_t key) const {
888+
return {m_keys[key].begin(), m_keys[key].end()};
889+
}
890+
891+
std::vector<unsigned char> ToPKHBytes(uint32_t key) const {
892+
auto id = m_keys[key].GetID();
893+
return {id.begin(), id.end()};
894+
}
895+
};
896+
897+
/**
898+
* The context for converting a Miniscript descriptor to its textual form.
899+
*/
900+
class StringMaker {
901+
//! To convert private keys for private descriptors.
902+
const SigningProvider* m_arg;
903+
//! Keys contained in the Miniscript (a reference to DescriptorImpl::m_pubkey_args).
904+
const std::vector<std::unique_ptr<PubkeyProvider>>& m_pubkeys;
905+
//! Whether to serialize keys as private or public.
906+
bool m_private;
907+
908+
public:
909+
StringMaker(const SigningProvider* arg LIFETIMEBOUND, const std::vector<std::unique_ptr<PubkeyProvider>>& pubkeys LIFETIMEBOUND, bool priv)
910+
: m_arg(arg), m_pubkeys(pubkeys), m_private(priv) {}
911+
912+
std::optional<std::string> ToString(uint32_t key) const
913+
{
914+
std::string ret;
915+
if (m_private) {
916+
if (!m_pubkeys[key]->ToPrivateString(*m_arg, ret)) return {};
917+
} else {
918+
ret = m_pubkeys[key]->ToString();
919+
}
920+
return ret;
921+
}
922+
};
923+
924+
class MiniscriptDescriptor final : public DescriptorImpl
925+
{
926+
private:
927+
miniscript::NodeRef<uint32_t> m_node;
928+
929+
protected:
930+
std::vector<CScript> MakeScripts(const std::vector<CPubKey>& keys, Span<const CScript> scripts,
931+
FlatSigningProvider& provider) const override
932+
{
933+
for (const auto& key : keys) provider.pubkeys.emplace(key.GetID(), key);
934+
return Vector(m_node->ToScript(ScriptMaker(keys)));
935+
}
936+
937+
public:
938+
MiniscriptDescriptor(std::vector<std::unique_ptr<PubkeyProvider>> providers, miniscript::NodeRef<uint32_t> node)
939+
: DescriptorImpl(std::move(providers), "?"), m_node(std::move(node)) {}
940+
941+
bool ToStringHelper(const SigningProvider* arg, std::string& out, const StringType type,
942+
const DescriptorCache* cache = nullptr) const override
943+
{
944+
if (const auto res = m_node->ToString(StringMaker(arg, m_pubkey_args, type == StringType::PRIVATE))) {
945+
out = *res;
946+
return true;
947+
}
948+
return false;
949+
}
950+
951+
bool IsSolvable() const override { return false; } // For now, mark these descriptors as non-solvable (as we don't have signing logic for them).
952+
bool IsSingleType() const final { return true; }
953+
};
954+
857955
////////////////////////////////////////////////////////////////////////////
858956
// Parser //
859957
////////////////////////////////////////////////////////////////////////////
@@ -1003,6 +1101,81 @@ std::unique_ptr<PubkeyProvider> ParsePubkey(uint32_t key_exp_index, const Span<c
10031101
return std::make_unique<OriginPubkeyProvider>(key_exp_index, std::move(info), std::move(provider), apostrophe);
10041102
}
10051103

1104+
std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptContext, const SigningProvider& provider)
1105+
{
1106+
std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey);
1107+
KeyOriginInfo info;
1108+
if (provider.GetKeyOrigin(pubkey.GetID(), info)) {
1109+
return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider), /*apostrophe=*/false);
1110+
}
1111+
return key_provider;
1112+
}
1113+
1114+
/**
1115+
* The context for parsing a Miniscript descriptor (either from Script or from its textual representation).
1116+
*/
1117+
struct KeyParser {
1118+
//! The Key type is an index in DescriptorImpl::m_pubkey_args
1119+
using Key = uint32_t;
1120+
//! Must not be nullptr if parsing from string.
1121+
FlatSigningProvider* m_out;
1122+
//! Must not be nullptr if parsing from Script.
1123+
const SigningProvider* m_in;
1124+
//! List of keys contained in the Miniscript.
1125+
mutable std::vector<std::unique_ptr<PubkeyProvider>> m_keys;
1126+
//! Used to detect key parsing errors within a Miniscript.
1127+
mutable std::string m_key_parsing_error;
1128+
1129+
KeyParser(FlatSigningProvider* out LIFETIMEBOUND, const SigningProvider* in LIFETIMEBOUND) : m_out(out), m_in(in) {}
1130+
1131+
bool KeyCompare(const Key& a, const Key& b) const {
1132+
return *m_keys.at(a) < *m_keys.at(b);
1133+
}
1134+
1135+
template<typename I> std::optional<Key> FromString(I begin, I end) const
1136+
{
1137+
assert(m_out);
1138+
Key key = m_keys.size();
1139+
auto pk = ParsePubkey(key, {&*begin, &*end}, ParseScriptContext::P2SH, *m_out, m_key_parsing_error);
1140+
if (!pk) return {};
1141+
m_keys.push_back(std::move(pk));
1142+
return key;
1143+
}
1144+
1145+
std::optional<std::string> ToString(const Key& key) const
1146+
{
1147+
return m_keys.at(key)->ToString();
1148+
}
1149+
1150+
template<typename I> std::optional<Key> FromPKBytes(I begin, I end) const
1151+
{
1152+
assert(m_in);
1153+
CPubKey pubkey(begin, end);
1154+
if (pubkey.IsValid()) {
1155+
Key key = m_keys.size();
1156+
m_keys.push_back(InferPubkey(pubkey, ParseScriptContext::P2SH, *m_in));
1157+
return key;
1158+
}
1159+
return {};
1160+
}
1161+
1162+
template<typename I> std::optional<Key> FromPKHBytes(I begin, I end) const
1163+
{
1164+
assert(end - begin == 20);
1165+
assert(m_in);
1166+
uint160 hash;
1167+
std::copy(begin, end, hash.begin());
1168+
CKeyID keyid(hash);
1169+
CPubKey pubkey;
1170+
if (m_in->GetPubKey(keyid, pubkey)) {
1171+
Key key = m_keys.size();
1172+
m_keys.push_back(InferPubkey(pubkey, ParseScriptContext::P2SH, *m_in));
1173+
return key;
1174+
}
1175+
return {};
1176+
}
1177+
};
1178+
10061179
/** Parse a script in a particular context. */
10071180
std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const char>& sp, ParseScriptContext ctx, FlatSigningProvider& out, std::string& error)
10081181
{
@@ -1120,6 +1293,45 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
11201293
error = "Can only have raw() at top level";
11211294
return nullptr;
11221295
}
1296+
// Process miniscript expressions.
1297+
{
1298+
KeyParser parser(&out, nullptr);
1299+
auto node = miniscript::FromString(std::string(expr.begin(), expr.end()), parser);
1300+
if (node) {
1301+
if (ctx != ParseScriptContext::P2SH) {
1302+
error = "Miniscript expressions can only be used in sh";
1303+
return nullptr;
1304+
}
1305+
if (parser.m_key_parsing_error != "") {
1306+
error = std::move(parser.m_key_parsing_error);
1307+
return nullptr;
1308+
}
1309+
if (!node->IsSane()) {
1310+
// Try to find the first insane sub for better error reporting.
1311+
auto insane_node = node.get();
1312+
if (const auto sub = node->FindInsaneSub()) insane_node = sub;
1313+
if (const auto str = insane_node->ToString(parser)) error = *str;
1314+
if (!insane_node->IsValid()) {
1315+
error += " is invalid";
1316+
} else {
1317+
error += " is not sane";
1318+
if (!insane_node->IsNonMalleable()) {
1319+
error += ": malleable witnesses exist";
1320+
} else if (insane_node == node.get() && !insane_node->NeedsSignature()) {
1321+
error += ": witnesses without signature exist";
1322+
} else if (!insane_node->CheckTimeLocksMix()) {
1323+
error += ": contains mixes of timelocks expressed in blocks and seconds";
1324+
} else if (!insane_node->CheckDuplicateKey()) {
1325+
error += ": contains duplicate public keys";
1326+
} else if (!insane_node->ValidSatisfactions()) {
1327+
error += ": needs witnesses that may exceed resource limits";
1328+
}
1329+
}
1330+
return nullptr;
1331+
}
1332+
return std::make_unique<MiniscriptDescriptor>(std::move(parser.m_keys), std::move(node));
1333+
}
1334+
}
11231335
if (ctx == ParseScriptContext::P2SH) {
11241336
error = "A function is needed within P2SH";
11251337
return nullptr;
@@ -1128,16 +1340,6 @@ std::unique_ptr<DescriptorImpl> ParseScript(uint32_t& key_exp_index, Span<const
11281340
return nullptr;
11291341
}
11301342

1131-
std::unique_ptr<PubkeyProvider> InferPubkey(const CPubKey& pubkey, ParseScriptContext, const SigningProvider& provider)
1132-
{
1133-
std::unique_ptr<PubkeyProvider> key_provider = std::make_unique<ConstPubkeyProvider>(0, pubkey);
1134-
KeyOriginInfo info;
1135-
if (provider.GetKeyOrigin(pubkey.GetID(), info)) {
1136-
return std::make_unique<OriginPubkeyProvider>(0, std::move(info), std::move(key_provider), /*apostrophe=*/false);
1137-
}
1138-
return key_provider;
1139-
}
1140-
11411343
std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptContext ctx, const SigningProvider& provider)
11421344
{
11431345
std::vector<std::vector<unsigned char>> data;
@@ -1175,6 +1377,14 @@ std::unique_ptr<DescriptorImpl> InferScript(const CScript& script, ParseScriptCo
11751377
}
11761378
}
11771379

1380+
if (ctx == ParseScriptContext::P2SH) {
1381+
KeyParser parser(nullptr, &provider);
1382+
auto node = miniscript::FromScript(script, parser);
1383+
if (node && node->IsSane()) {
1384+
return std::make_unique<MiniscriptDescriptor>(std::move(parser.m_keys), std::move(node));
1385+
}
1386+
}
1387+
11781388
CTxDestination dest;
11791389
if (ExtractDestination(script, dest)) {
11801390
if (GetScriptForDestination(dest) == script) {

‎src/script/miniscript.h‎

Lines changed: 31 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,21 @@ struct Node {
429429
));
430430
}
431431

432+
/** Like TreeEval, but without downfn or State type.
433+
* upfn takes (const Node&, Span<Result>) and returns Result. */
434+
template<typename Result, typename UpFn>
435+
Result TreeEval(UpFn upfn) const
436+
{
437+
struct DummyState {};
438+
return std::move(*TreeEvalMaybe<Result>(DummyState{},
439+
[](DummyState, const Node&, size_t) { return DummyState{}; },
440+
[&upfn](DummyState, const Node& node, Span<Result> subs) {
441+
Result res{upfn(node, subs)};
442+
return std::optional<Result>(std::move(res));
443+
}
444+
));
445+
}
446+
432447
/** Compare two miniscript subtrees, using a non-recursive algorithm. */
433448
friend int Compare(const Node<Key>& node1, const Node<Key>& node2)
434449
{
@@ -815,6 +830,15 @@ struct Node {
815830
//! Return the expression type.
816831
Type GetType() const { return typ; }
817832

833+
//! Find an insane subnode which has no insane children. Nullptr if there is none.
834+
const Node* FindInsaneSub() const {
835+
return TreeEval<const Node*>([](const Node& node, Span<const Node*> subs) -> const Node* {
836+
for (auto& sub: subs) if (sub) return sub;
837+
if (!node.IsSaneSubexpression()) return &node;
838+
return nullptr;
839+
});
840+
}
841+
818842
//! Check whether this node is valid at all.
819843
bool IsValid() const { return !(GetType() == ""_mst); }
820844

@@ -950,7 +974,11 @@ void BuildBack(const Ctx& ctx, Fragment nt, std::vector<NodeRef<Key>>& construct
950974
}
951975
}
952976

953-
//! Parse a miniscript from its textual descriptor form.
977+
/**
978+
* Parse a miniscript from its textual descriptor form.
979+
* This does not check whether the script is valid, let alone sane. The caller is expected to use
980+
* the `IsValidTopLevel()` and `IsSaneTopLevel()` to check for these properties on the node.
981+
*/
954982
template<typename Key, typename Ctx>
955983
inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
956984
{
@@ -1252,9 +1280,7 @@ inline NodeRef<Key> Parse(Span<const char> in, const Ctx& ctx)
12521280
// Sanity checks on the produced miniscript
12531281
assert(constructed.size() == 1);
12541282
if (in.size() > 0) return {};
1255-
const NodeRef<Key> tl_node = std::move(constructed.front());
1256-
if (!tl_node->IsValidTopLevel()) return {};
1257-
return tl_node;
1283+
return std::move(constructed.front());
12581284
}
12591285

12601286
/** Decode a script into opcode/push pairs.
@@ -1687,7 +1713,7 @@ inline NodeRef<Key> DecodeScript(I& in, I last, const Ctx& ctx)
16871713
}
16881714
}
16891715
if (constructed.size() != 1) return {};
1690-
const NodeRef<Key> tl_node = std::move(constructed.front());
1716+
NodeRef<Key> tl_node = std::move(constructed.front());
16911717
// Note that due to how ComputeType works (only assign the type to the node if the
16921718
// subs' types are valid) this would fail if any node of tree is badly typed.
16931719
if (!tl_node->IsValidTopLevel()) return {};

0 commit comments

Comments
 (0)