This makes the Miniscript descriptors solvable.
Note this introduces signing support for much more complex scripts than the wallet was previously able to solve, and the whole tooling isn’t provided for a complete Miniscript integration in the wallet. Particularly, the PSBT<->Miniscript integration isn’t entirely covered in this PR.
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | sipa, achow101 |
| Stale ACK | instagibbs |
If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.
Reviewers, this pull request conflicts with the following ones:
GetBalance() logic with AvailableCoins() by w0xlt)If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
257+ //! Mark this input stack as having a signature.
258+ InputStack& WithSig();
259+ //! Mark this input stack as non-canonical (known to not be necessary in non-malleable satisfactions).
260+ InputStack& NonCanon();
261+ //! Mark this input stack as malleable.
262+ InputStack& Malleable(bool x = true);
In b2e3582323b1e3dfded7eb0a2597fd74ba52e4df “miniscript: satisfaction support”
I think these should be named with a Set prefix as otherwise they read like they are functions to check these properties rather than setting them.
327+ // If only one (or neither) is valid, pick the other one.
328+ if (a.available == Availability::NO) return b;
329+ if (b.available == Availability::NO) return a;
330+ // If both are valid, they must be distinct.
331+ if (nonmalleable) {
332+ // If both options are weak, any result is fine; it just needs the malleable marker.
In b2e3582323b1e3dfded7eb0a2597fd74ba52e4df “miniscript: satisfaction support”
Is there a doc somewhere that defines “weak” and “strong” in this context?
HASSIG. That a satisfaction is weak means that it isn’t protected by a signature, strong is the opposite. Note this property can be analyzed statically on the Witness Script: it is the ’s’ property (NeedsSignature(), a requirement to be IsSaneTopLevel()).
837+ auto& x = subres[0], &z = subres[1];
838+ return InputResult(INVALID, Choose(std::move(x.sat), z.sat + x.nsat, nonmal));
839+ }
840+ case NodeType::OR_D: {
841+ auto& x = subres[0], &z = subres[1];
842+ auto nsat = z.nsat + x.nsat, sat_l = x.sat, sat_r = z.sat + x.nsat;
In b2e3582323b1e3dfded7eb0a2597fd74ba52e4df “miniscript: satisfaction support”
This line appears to not be doing anything.
In 3bedfb5b13e863508cb9b0d44fa03cc991151927 “miniscript: satisfaction support”
This line is still here.
301@@ -297,29 +302,39 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string&
302 BOOST_CHECK_MESSAGE(left_paths.empty(), "Not all expected key paths found: " + prv);
303 }
304
305-void Check(const std::string& prv, const std::string& pub, const std::string& norm_prv, const std::string& norm_pub, int flags, const std::vector<std::vector<std::string>>& scripts, const std::optional<OutputType>& type, const std::set<std::vector<uint32_t>>& paths = ONLY_EMPTY)
306+void Check(const std::string& prv, const std::string& pub, const std::string& norm_prv, const std::string& norm_pub,
307+ int flags, const std::vector<std::vector<std::string>>& scripts, const std::optional<OutputType>& type,
308+ const std::set<std::vector<uint32_t>>& paths = ONLY_EMPTY, uint32_t spender_nlocktime=0, uint32_t spender_nsequence=0)
In 96e9de9304edf5feec87adf5ee56ad0f526459b8 “script/sign: signing support for Miniscript with timelocks”
In DoCheck, spender_nsequence is CTxIn::SEQUENCE_FINAL, should it be that here as well (instead of 0)?
CTxIn::SEQUENCE_FINAL.
Particularly, the PSBT<->Miniscript integration isn’t part of this PR.
I don’t think it would be particularly hard considering that you’re already using SignatureData for non-wallet data lookups. I think all that would really need to be done is to update FillSignatureData to include the preimages from the PSBT.
Edit: This diff does the trick
0diff --git a/src/psbt.cpp b/src/psbt.cpp
1index 8248609ba6..9e09ccf15d 100644
2--- a/src/psbt.cpp
3+++ b/src/psbt.cpp
4@@ -113,6 +113,18 @@ void PSBTInput::FillSignatureData(SignatureData& sigdata) const
5 for (const auto& key_pair : hd_keypaths) {
6 sigdata.misc_pubkeys.emplace(key_pair.first.GetID(), key_pair);
7 }
8+ for (const auto& [hash, preimage] : ripemd160_preimages) {
9+ sigdata.hash_preimages.emplace(std::vector<unsigned char>(hash.begin(), hash.end()), preimage);
10+ }
11+ for (const auto& [hash, preimage] : sha256_preimages) {
12+ sigdata.hash_preimages.emplace(std::vector<unsigned char>(hash.begin(), hash.end()), preimage);
13+ }
14+ for (const auto& [hash, preimage] : hash160_preimages) {
15+ sigdata.hash_preimages.emplace(std::vector<unsigned char>(hash.begin(), hash.end()), preimage);
16+ }
17+ for (const auto& [hash, preimage] : hash256_preimages) {
18+ sigdata.hash_preimages.emplace(std::vector<unsigned char>(hash.begin(), hash.end()), preimage);
19+ }
20 }
21
22 void PSBTInput::FromSignatureData(const SignatureData& sigdata)
Rebased, addressed Andrew’s comments, added some cleanups from @sipa, and added a miniscript_random fuzz target which generates a random Miniscript node based on a binary encoding. We might eventually add another one which tries to be smart, either here or in a follow-up.
I don’t think it would be particularly hard considering that you’re already using SignatureData for non-wallet data lookups. I think all that would really need to be done is to update FillSignatureData to include the preimages from the PSBT.
Yeah, but i figured it would be cleaner to have this as part of a PR which’d also update the various PSBT RPCs and test it using those.
Fixed the fuzzer crash on integer overflow, and pushed another bunch of updates to the satisfaction code:
Choose to operator|See https://github.com/sipa/miniscript/pull/102 for details.
I also pushed an initial corpus for miniscript_random fuzz target at https://github.com/bitcoin-core/qa-assets/pull/87
359 bool found_apostrophes_in_prv = false;
360 bool found_apostrophes_in_pub = false;
361
362 // Do not replace apostrophes with 'h' in prv and pub
363- DoCheck(prv, pub, norm_prv, norm_pub, flags, scripts, type, paths);
364+ DoCheck(prv, pub, norm_prv, norm_pub, flags, scripts, type, paths, /* replace_apostrophe_with_h_in_prv = */false,
0 DoCheck(prv, pub, norm_prv, norm_pub, flags, scripts, type, paths, /*replace_apostrophe_with_h_in_prv=*/false,
Anywhere you are adding / touching named args, please use the style from the developer docs.
miniscript_stable and miniscript_smart targets introduced in the last two commits, see the commit messages for details).
I don’t think it would be particularly hard considering that you’re already using SignatureData for non-wallet data lookups. I think all that would really need to be done is to update FillSignatureData to include the preimages from the PSBT.
Yeah, but i figured it would be cleaner to have this as part of a PR which’d also update the various PSBT RPCs and test it using those.
I created a watch-only wallet for testing miniscript, which relies on the private keys from other wallets. I might be nice to have a (draft) PSBT-enabled PR on top of this for easier testing.
I’m testing with something of the form wsh(or_d(pk(A),and_v(v:pkh(B),older(3)))). I’m able to create a PSBT in the watch-only wallet and sign it with wallet B. But then analyzepsbt still complains that signature A is missing (and finalizepsbt refuses to give me the hex transaction). Sounds like that’s expected behaviour for now.
954+ return InputResult((y.nsat + x.nsat) | (y.sat + x.nsat).SetNonCanon() | (y.nsat + x.sat).SetNonCanon(), y.sat + x.sat);
955+ }
956+ case Fragment::OR_B: {
957+ auto& x = subres[0], &z = subres[1];
958+ // The (sat(Z) sat(X)) solution is overcomplete (attacker can change either into dsat).
959+ return InputResult(z.nsat + x.nsat, (z.nsat + x.sat) | (z.sat + x.nsat) | (z.sat + x.sat).SetMalleable());
In 3bedfb5b13e863508cb9b0d44fa03cc991151927 “miniscript: satisfaction support”
The satisfactions table lists sat(z), sat(x) as non-canonical. Should that also have SetNonCanon()?
Indeed. This enables stricter testing. Also, it looks like we are missing a few SetMalleable() here, as per the specs:
The non-canonical options for and_b, or_b, and thresh are always overcomplete (reason 3), so instead use DONTUSE there as well (with HASSIG flag if the original non-canonical solution had one). @sipa does the following diff seem correct to you?
0diff --git a/src/script/miniscript.h b/src/script/miniscript.h
1index 5c04c40587..e94105d0f4 100644
2--- a/src/script/miniscript.h
3+++ b/src/script/miniscript.h
4@@ -951,12 +951,12 @@ public:
5 }
6 case Fragment::AND_B: {
7 auto& x = subres[0], &y = subres[1];
8- return InputResult((y.nsat + x.nsat) | (y.sat + x.nsat).SetNonCanon() | (y.nsat + x.sat).SetNonCanon(), y.sat + x.sat);
9+ return InputResult((y.nsat + x.nsat) | (y.sat + x.nsat).SetMalleable().SetNonCanon() | (y.nsat + x.sat).SetMalleable().SetNonCanon(), y.sat + x.sat);
10 }
11 case Fragment::OR_B: {
12 auto& x = subres[0], &z = subres[1];
13 // The (sat(Z) sat(X)) solution is overcomplete (attacker can change either into dsat).
14- return InputResult(z.nsat + x.nsat, (z.nsat + x.sat) | (z.sat + x.nsat) | (z.sat + x.sat).SetMalleable());
15+ return InputResult(z.nsat + x.nsat, (z.nsat + x.sat) | (z.sat + x.nsat) | (z.sat + x.sat).SetMalleable().SetNonCanon());
16 }
17 case Fragment::OR_C: {
18 auto& x = subres[0], &z = subres[1];
I’d follow @achow101’s suggestion but not @darosior’s.
The reason is that adding SetMalleable() is “weakening” the self-testing ability, while SetNonCanon() is “strengthening” it.
The malleability flag influences the signer’s choices, so it should only need to be added when we’re certain the signer logic can’t already figure it out on its own. In OR_B, having an alternative (z.nsat + x.nsat) choice should be sufficient, always available, and not have any signatures. That implies the (z.nsat + x.sat) and (z.sat + x.nsat) choices should be treated as malleable anyway.
On the other hand, the non-canonical flag is effectively an assertion: it doesn’t influence the signing logic, except for raising an error if a non-canonical choice ends up being actually used. These should be added wherever we believe, based on higher-level reasoning, that certain choices are off-limits.
1113+ return subs[0] || subs[1];
1114+ case Fragment::THRESH:
1115+ return std::count(subs.begin(), subs.end(), true) >= node.k;
1116+ default: // wrappers
1117+ assert(subs.size() == 1);
1118+ return !!subs[0];
In 3bedfb5b13e863508cb9b0d44fa03cc991151927 “miniscript: satisfaction support”
Why a double negation?
subs[0] is an int but we want a bool.
-> bool to the lambda definition, I think, if we care.
-> bool to the lambda definition as suggested.
684@@ -672,6 +685,7 @@ class DummySignatureChecker final : public BaseSignatureChecker
685 bool CheckSchnorrSignature(Span<const unsigned char> sig, Span<const unsigned char> pubkey, SigVersion sigversion, ScriptExecutionData& execdata, ScriptError* serror) const override { return sig.size() != 0; }
686 bool CheckLockTime(const CScriptNum& nLockTime) const override { return true; }
687 bool CheckSequence(const CScriptNum& nSequence) const override { return true; }
688+ bool CheckEqual(const std::vector<unsigned char>& vch1, const std::vector<unsigned char>& vch2) const override { return true; }
In 8f1b45bf256b7454d1b790038a00eb537b9bbb70 “script/sign: signing support for Miniscripts with hash preimage challenges”
I think this line should be in the previous commit.
CheckEqual, whereas this commit adds LookupPreimage. It’s not immediately obvious how these are related, and IMO it makes more sense to introduce the CheckEquals in the same commit even if unneeded immediately.
My rationale was that adding CheckEqual to the DummySignatureChecker is related to the descriptor: it makes a descriptor with timelocks reported as solvable (which in turn allows us to mark them as such in the unit tests). On the other hand the previous commit aims to be the isolated 3-lines-diff that touches consensus code, and only do that.
Not that i feel too strongly about it of course!
262@@ -263,6 +263,8 @@ class BaseSignatureChecker
263 return false;
264 }
265
266+ virtual bool CheckEqual(const std::vector<unsigned char>& vch1, const std::vector<unsigned char>& vch2) const { return vch1 == vch2; }
I vaguely recall having a discussion about this already, but I can’t remember the conclusion.
Is this function needed just to make the dummy signing pass verification in the signing logic? If so, it may be easier to only add a way to bypass that verification check for dummy signing in the first place.
I’m somewhat uncomfortable with making a change to consensus code like this just for that purpose.
Yes, it’s only to pass the IsSolvable check and, yes, it is unfortunate. Previous discussion logs here. TL;DR:
IsSolvable is overkill already, we could just drop the VerifyScript there and not need the interpreter patch here:
https://github.com/bitcoin/bitcoin/blob/0897b189e4242321cd03339e7dde5c4d963d0e48/src/script/sign.cpp#L640-L646But just getting rid of VerifyScript isn’t enough. We still need to keep ProduceSignature, which will in turn call VerifyScript again. And since this path is used at “real” signing time too, it gets pretty convoluted to special case IsSolvable without using the DummySignatureChecker.
I think a cleaner path forward would be to revise the implementation of IsSolvable altogether, detangling it from the signing code. It dates from back when there were no descriptors. Nowadays everything we can sign for (that “is solvable”) can be represented as a descriptor. So how about re-defining IsSolvable as “a descriptor can be inferred from this script, and this descriptor is solvable (ie not raw)”?
It’s probably a good cleanup of legacy code on its own too, so it could have its own PR and i would rebase this one on top of it and drop the interpreter patch.
EDIT: a branch implementing this on top of this PR: https://github.com/darosior/bitcoin/tree/miniscript_wallet_signing_new_is_solvable
IIRC, the DummySignatureCreator isn’t just used for testing solvability, but also for determining the size of produced signatures before actual keys are available. If so, we still have a dependence on ProduceSignature in that context.
That said, it seems possible to just remove both VerifyScript calls in script/sign.cpp’s ProduceSignature and IsSolvable. No tests break.
Sorry for the late reply.
Yes. I think it would be nice to get rid of it entirely but indeed only redefining IsSolvable doesn’t permit that, It’s sufficient for dropping the interpreter patch here though.
I don’t know about removing the belt-and-suspender VerifyScript check in ProduceSignature. I’d be more comfortable with just re-defining IsSolvable. I’ve done that in #25664.
Rebased. Dropped the commit modifying the interpreter, not needed after #25664. This PR does not touch consensus code anymore.
Note however that since we can’t mock the preimage check, we can’t estimate the input size when spending from a Miniscript descriptor with a hash preimage challenge in all spending paths. Ideally, i think we should estimate the input size for spending a coin using the descriptors (no need to run the signature producer, we have this information already).
Relatedly, i added a commit for determining solvability using an inferred descriptor in AvailableCoins, as the optimization introduced in 8a105ecd1aeff15f84c3883e2762bf71ad59d920 of using the result of CalculateMaximumSignedInputSize would not work for descriptors with hash preimage challenges.
I didn’t forget about the draft PR for PSBT support. :) But i’ll look first into using descriptors for determining input size.
EDIT: the switch to using descriptors for estimating input satisfaction size in the wallet is implemented in #26567.
61@@ -32,6 +62,9 @@ class BaseSignatureCreator {
62 /** Create a singular (non-script) signature. */
63 virtual bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const =0;
64 virtual bool CreateSchnorrSig(const SigningProvider& provider, std::vector<unsigned char>& sig, const XOnlyPubKey& pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion) const =0;
65+
66+ //! Fetch a preimage corresponding to this hash.
67+ virtual bool LookupPreimage(SignatureData& sig_data, const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const =0;
I find slightly ugly to expose SignatureData to the SignatureCreator interface. I’d say that SignatureData is the “upper” interface of the script/sign logic, which it exposes to its users. And SignatureCreator is part of the “lower” interface - it’s how one can provide simple individual signature logic to script/sign. So it feels awkward to make the SignatureCreator implementations (which should deal with simple low-level things like signatures) be exposed to this SignatureData class which deals with script signing. This isn’t more than a philosophical concern about code structure, but I’d like to offer an alternative.
Trying to draw an analogy with how keys/signatures are handled, there is the somewhat weird observation that hash preimages are both key material (which should really go in a SigningProvider, as it’s a kind of secret) and signature material (which rightfully belongs in SigningData, as something that will end up in the witness). I don’t think that’s a problem - maybe it belongs in both. So a possibility is:
SigningProvider to hold/lookup preimages.SignatureCreator that takes a const SigningProvider& as argument (like CreateSig etc). It’s so simple it could have a default implementation, looking things up in the provider. Dummy implementations could override it by return success with zero-byte ones.SignatureData too, accessed/controlled by a non-overridable ::CreatePreimage function (mimicking ::CreateSig in script/sign.cpp), which can invoke LookupPreimage on the SignatureCreator if needed, but can also access the “cached” version in SignatureData.This means that SignatureCreator implementations only need to know about SigningProvider, like before, but can still override. It also avoids the need to pass a SignatureData to SignSignature.
An alternative, if the above is too complex, is to introduce a simple PreimageData structure, make LookupPreimage take const PreimageData&, and add a PreimageData field to SignatureData.
Lastly, if none of the above are doable: can SignatureData& here and the argument to SignSignature be made a const reference?
I see. But since SignatureData is already largely exposed to the inner of ProduceSignature, what would you think of just making it a parameter in SignSignature (instead of creating a fresh one there)? Because actually it’s not necessary to expose it to SignatureCreator:
0diff --git a/src/script/sign.cpp b/src/script/sign.cpp
1index 4fe738dfd5..f337e8ba48 100644
2--- a/src/script/sign.cpp
3+++ b/src/script/sign.cpp
4@@ -89,13 +89,6 @@ bool MutableTransactionSignatureCreator::CreateSchnorrSig(const SigningProvider&
5 return true;
6 }
7
8-bool MutableTransactionSignatureCreator::LookupPreimage(SignatureData& sig_data, const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const {
9- auto it = sig_data.hash_preimages.find(hash);
10- if (it == sig_data.hash_preimages.end()) return false;
11- preimage = it->second;
12- return true;
13-}
14-
15 static bool GetCScript(const SigningProvider& provider, const SignatureData& sigdata, const CScriptID& scriptid, CScript& script)
16 {
17 if (provider.GetCScript(scriptid, script)) {
18@@ -390,6 +383,17 @@ static CScript PushAll(const std::vector<valtype>& values)
19 return result;
20 }
21
22+template<typename M, typename K, typename V>
23+bool LookupHelper(const M& map, const K& key, V& value)
24+{
25+ auto it = map.find(key);
26+ if (it != map.end()) {
27+ value = it->second;
28+ return true;
29+ }
30+ return false;
31+}
32+
33 /**
34 * Context for solving a Miniscript.
35 * If enough material (access to keys, hash preimages, ..) is given, produces a valid satisfaction.
36@@ -451,16 +455,16 @@ struct Satisfier {
37
38 //! Hash preimage satisfactions.
39 miniscript::Availability SatSHA256(const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const {
40- return m_creator.LookupPreimage(m_sig_data, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
41+ return LookupHelper(m_sig_data.hash_preimages, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
42 }
43 miniscript::Availability SatRIPEMD160(const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const {
44- return m_creator.LookupPreimage(m_sig_data, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
45+ return LookupHelper(m_sig_data.hash_preimages, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
46 }
47 miniscript::Availability SatHASH256(const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const {
48- return m_creator.LookupPreimage(m_sig_data, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
49+ return LookupHelper(m_sig_data.hash_preimages, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
50 }
51 miniscript::Availability SatHASH160(const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const {
52- return m_creator.LookupPreimage(m_sig_data, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
53+ return LookupHelper(m_sig_data.hash_preimages, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
54 }
55 };
56
57@@ -720,11 +724,6 @@ public:
58 sig.assign(64, '\000');
59 return true;
60 }
61-
62- bool LookupPreimage(SignatureData& sig_data, const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const override {
63- preimage.assign(32, '\000');
64- return true;
65- }
66 };
67
68 }
69diff --git a/src/script/sign.h b/src/script/sign.h
70index fdb3244d5d..8df1fbc98d 100644
71--- a/src/script/sign.h
72+++ b/src/script/sign.h
73@@ -62,9 +62,6 @@ public:
74 /** Create a singular (non-script) signature. */
75 virtual bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const =0;
76 virtual bool CreateSchnorrSig(const SigningProvider& provider, std::vector<unsigned char>& sig, const XOnlyPubKey& pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion) const =0;
77-
78- //! Fetch a preimage corresponding to this hash.
79- virtual bool LookupPreimage(SignatureData& sig_data, const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const =0;
80 };
81
82 /** A signature creator for transactions. */
83@@ -83,7 +80,6 @@ public:
84 const BaseSignatureChecker& Checker() const override { return checker; }
85 bool CreateSig(const SigningProvider& provider, std::vector<unsigned char>& vchSig, const CKeyID& keyid, const CScript& scriptCode, SigVersion sigversion) const override;
86 bool CreateSchnorrSig(const SigningProvider& provider, std::vector<unsigned char>& sig, const XOnlyPubKey& pubkey, const uint256* leaf_hash, const uint256* merkle_root, SigVersion sigversion) const override;
87- bool LookupPreimage(SignatureData& sig_data, const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const override;
88 };
89
90 /** A signature creator that just produces 71-byte empty signatures. */
can […] the argument to SignSignature be made a const reference?
It’s modified by SignSignature so it wouldn’t be possible to make it a const reference.
460+ }
461+ miniscript::Availability SatRIPEMD160(const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const {
462+ return m_creator.LookupPreimage(m_sig_data, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
463+ }
464+ miniscript::Availability SatHASH256(const std::vector<unsigned char>& hash, std::vector<unsigned char>& preimage) const {
465+ return m_creator.LookupPreimage(m_sig_data, hash, preimage) ? miniscript::Availability::YES : miniscript::Availability::NO;
h=SHA256(SHA256(x)), then both x and SHA256(x) can be correct preimages (under Hash256 and SHA256 respectively), and both might be used in a single script.
Rebased (quite verbose, following #25863) and removed needless commit 1af2145ef6 (“descriptor_tests: use clang-tidy verifiable syntax for argument comment”).
I changed the commit introducing hashlock satisfaction to use a mapping per hash type, since there might be collisions between hash types (thanks sipa!). I kept a single mapping in the unit tests that is copied in order to not introduce 4 new arguments to the function.
I also changed this commit to not expose SignatureData to the SignatureCreator anymore. I might change it soon to use a SigningProvider to hold the preimages instead, depending on the outcome of the discussion in #24149 (review).
Rebased.
There was a spurious (apparently build?) failure in the Windows CI, had to force-pushed to re-kick it.
392+ const SigningProvider& m_provider;
393+ SignatureData& m_sig_data;
394+ const BaseSignatureCreator& m_creator;
395+ const CScript& m_witness_script;
396+
397+ explicit Satisfier(const SigningProvider& provider, SignatureData& sig_data, const BaseSignatureCreator& creator,
LIFETIMEBOUND?
317@@ -317,6 +318,11 @@ void DoCheck(const std::string& prv, const std::string& pub, const std::string&
318 txdata.Init(spend, std::move(utxos), /*force=*/true);
319 MutableTransactionSignatureCreator creator{spend, 0, CAmount{0}, &txdata, SIGHASH_DEFAULT};
320 SignatureData sigdata;
321+ // We assume there is no collusion between the hashes (eg h1=SHA256(SHA256(x)) and h2=SHA256(x))
collision ?
212+ */
213+class TestSignatureChecker : public BaseSignatureChecker {
214+ const Satisfier *ctx;
215+
216+public:
217+ TestSignatureChecker(const Satisfier *in_ctx) : ctx(in_ctx) {}
LIFETIMEBOUND ?
275+ friend InputStack operator+(InputStack a, InputStack b);
276+ //! Choose between two potential input stacks.
277+ friend InputStack operator|(InputStack a, InputStack b);
278+};
279+
280+static const auto ZERO = InputStack(std::vector<unsigned char>());
Maybe worth adding some comments to these. I suggest:
0/** A stack consisting of a single zero-length element (interpreted as 0 by the script interpreter in numeric context). */
1static const auto ZERO = InputStack(std::vector<unsigned char>());
2/** A stack consisting of a single malleable 32-byte 0x0000...0000 element (for dissatisfying hash challenges). */
3static const auto ZERO32 = InputStack(std::vector<unsigned char>(32, 0)).SetMalleable();
4/** A stack consisting of a single 0x01 element (interpreted as 1 by the script interpreted in numeric context). */
5static const auto ONE = InputStack(Vector((unsigned char)1));
6/** The empty stack. */
7static const auto EMPTY = InputStack();
8/** A stack representing the lack of any (dis)satisfactions. */
9static const auto INVALID = InputStack().SetAvailable(Availability::NO);
838@@ -785,6 +839,188 @@ struct Node {
839 assert(false);
840 }
841
842+ template<typename Ctx>
843+ internal::InputResult ProduceInput(const Ctx& ctx) const {
844+ using namespace internal;
845+
846+ auto helper = [&ctx](const Node& node, Span<InputResult> subres) -> InputResult {
Internal function which is invoked for every tree node, constructing satisfaction/dissatisfactions given those of its subnodes..
398+ const CScript& witscript) : m_provider(provider),
399+ m_sig_data(sig_data),
400+ m_creator(creator),
401+ m_witness_script(witscript) {}
402+
403+ bool KeyCompare(const Key& a, const Key& b) const {
static (and non-const then).
490 solved = solved && SignStep(provider, creator, witnessscript, result, subType, SigVersion::WITNESS_V0, sigdata) && subType != TxoutType::SCRIPTHASH && subType != TxoutType::WITNESS_V0_SCRIPTHASH && subType != TxoutType::WITNESS_V0_KEYHASH;
491+
492+ // Don't try to satisfy using Miniscript if it was a multisig as it would have already been handled by
493+ // SignStep, which appends partial signatures to the witness stack and we must not temper with to keep
494+ // compatibility with the extractor relying on this behaviour to combine witnesses.
495+ if (!solved && subType != TxoutType::MULTISIG) {
if (!solved && result.empty()) { instead? From a cursory reading, it looks like only TxoutType::MULTISIG populates result without fully solving, so that should match the exact behavior we want, without needing to special case MULTISIG, and without needing a slightly-concerning result.clear().
result.clear() to be slightly-concerning though? It’s what we do at the beginning of every SignStep() (and the Miniscript satisfier is kind of a more modern SignStep()).
My intuitive reading was just that the old code effectively says “unless doing multisig, just wipe whatever we have as partial solving data and start over using miniscript logic”, which may make someone wonder if truly nothing except multisig will have some important output we shouldn’t delete.
The code I suggested instead conveys “only try miniscript if we don’t already have any signing output” which doesn’t require the reader to know that only multisig actually produces non-final partial solving data.
674 UpdateInput(txTo.vin.at(nIn), sigdata);
675 return ret;
676 }
677
678-bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType)
679+bool SignSignature(const SigningProvider &provider, const CTransaction& txFrom, CMutableTransaction& txTo, unsigned int nIn, int nHashType, SignatureData sigdata)
The sigdata argument here should probably be passed to the call to SignSignature() below?
Also, here too, does SignatureData sigdata need to be passed by value?
105+ * @param fromPubKey The script to produce a satisfaction for.
106+ * @param txTo The spending transaction.
107+ * @param nIn The index of the input in `txTo` refering the output being spent.
108+ * @param amount The value of the output being spent.
109+ * @param nHashType Signature hash type.
110+ * @param sig_data Additional data provided to solve a script. Filled with the resulting satisfying
661@@ -644,19 +662,18 @@ void SignatureData::MergeSignatureData(SignatureData sigdata)
662 signatures.insert(std::make_move_iterator(sigdata.signatures.begin()), std::make_move_iterator(sigdata.signatures.end()));
663 }
664
665-bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType)
666+bool SignSignature(const SigningProvider &provider, const CScript& fromPubKey, CMutableTransaction& txTo, unsigned int nIn, const CAmount& amount, int nHashType, SignatureData sigdata)
SignatureData sigdata by value, is that necessary?
319+ }
320+ return a;
321+}
322+
323+InputStack operator|(InputStack a, InputStack b) {
324+ // If only one (or neither) is valid, pick the other one.
suggestion: “If only one is invalid, pick the other one. If both are invalid, pick an arbitrary one.”
The current comment seems to say the opposite of what the code does.
light ACK 33d30d1edd85a0110cbd9e3e29cc832f1cb6dab2
The satisfaction code looks good to me, as do the fixed tests. I didn’t delve too deeply into the fuzz and random tests though.
25+ {
26+ "ms": "or_i(pk(tprv8ZgxMBicQKsPerQj6m35no46amfKQdjY7AhLnmatHYXs8S4MTgeZYkWAn4edSGwwL3vkSiiGqSZQrmy5D3P5gBoqgvYP2fCUpBwbKTMTAkL/*),pk(tpubD6NzVbkrYhZ4YPAbyf6urxqqnmJF79PzQtyERAmvkSVS9fweCTjxjDh22Z5St9fGb1a5DUCv8G27nYupKP1Ctr1pkamJossoetzws1moNRn/*))",
27+ "sequence": None,
28+ "locktime": None,
29+ },
30+ # A more complex policy, that can't be satisfied through the first branch (need for a preimage)
Initially i wanted to tackle all the PSBT integration in a followup (see #24149 (comment)). This comment made me (finally) have a look at what should be done. I think analyzepsbt, utxoupdatepsbt would need to be updated to support hash preimages. Maybe walletprocesspsbt too, but i’m not sure yet (for instance hash preimages could be imported separately and kept in a signing provider?). However i think we should first make those commands not rely on a dry run of the signer anymore (Similarly to what i do in #26567 with the input size estimation).
Given this followup would be blocked by preparatory work and since the the patch for supporting the preimages provided directly in the PSBT by external software is trivial (basically @achow101’s patch from above #24149#pullrequestreview-882103227), i added a commit that implements it and tests satisfying using a path with a preimage check.
19@@ -20,6 +20,39 @@
20 "or_i(and_b(pk(029ffbe722b147f3035c87cb1c60b9a5947dd49c774cc31e94773478711a929ac0),a:and_b(pk(025f05815e3a1a8a83bfbb03ce016c9a2ee31066b98f567f6227df1d76ec4bd143),a:and_b(pk(025625f41e4a065efc06d5019cbbd56fe8c07595af1231e7cbc03fafb87ebb71ec),a:and_b(pk(02a27c8b850a00f67da3499b60562673dcf5fdfb82b7e17652a7ac54416812aefd),s:pk(03e618ec5f384d6e19ca9ebdb8e2119e5bef978285076828ce054e55c4daf473e2))))),and_v(v:thresh(2,pkh(tpubD6NzVbkrYhZ4YK67cd5fDe4fBVmGB2waTDrAt1q4ey9HPq9veHjWkw3VpbaCHCcWozjkhgAkWpFrxuPMUrmXVrLHMfEJ9auoZA6AS1g3grC/*),a:pkh(033841045a531e1adf9910a6ec279589a90b3b8a904ee64ffd692bd08a8996c1aa),a:pkh(02aebf2d10b040eb936a6f02f44ee82f8b34f5c1ccb20ff3949c2b28206b7c1068)),older(4209713)))",
21 ]
22
23+MINISCRIPTS_PRIV = [
24+ # One of two keys, of which one private key is known
25+ {
26+ "ms": "or_i(pk(tprv8ZgxMBicQKsPerQj6m35no46amfKQdjY7AhLnmatHYXs8S4MTgeZYkWAn4edSGwwL3vkSiiGqSZQrmy5D3P5gBoqgvYP2fCUpBwbKTMTAkL/*),pk(tpubD6NzVbkrYhZ4YPAbyf6urxqqnmJF79PzQtyERAmvkSVS9fweCTjxjDh22Z5St9fGb1a5DUCv8G27nYupKP1Ctr1pkamJossoetzws1moNRn/*))",
Maybe have lists with all the tprv/tpub/pk and insert them directly into the strings? Could make these easier to read and easier to write more in the future with less copy/paste/accidental key publication
e.g. I wanted to add the liquid style backoff key examples
0tprv1 = "tprv8ZgxMBicQKsPerQj6m35no46amfKQdjY7AhLnmatHYXs8S4MTgeZYkWAn4edSGwwL3vkSiiGqSZQrmy5D3P5gBoqgvYP2fCUpBwbKTMTAkL/*"
1tprv2 = "tprv8ZgxMBicQKsPd3cbrKjE5GKKJLDEidhtzSSmPVtSPyoHQGL2LZw49yt9foZsN9BeiC5VqRaESUSDV2PS9w7zAVBSK6EQH3CZW9sMKxSKDwD/*"
2tpubs = ["tpubD6NzVbkrYhZ4YPAbyf6urxqqnmJF79PzQtyERAmvkSVS9fweCTjxjDh22Z5St9fGb1a5DUCv8G27nYupKP1Ctr1pkamJossoetzws1moNRn",
3 "tpubD6NzVbkrYhZ4YMQC15JS7QcrsAyfGrGiykweqMmPxTkEVScu7vCZLNpPXW1XphHwzsgmqdHWDQAfucbM72EEB1ZEyfgZxYvkZjYVXx1xS9p",
4 "tpubD6NzVbkrYhZ4YU9vM1s53UhD75UyJatx8EMzMZ3VUjR2FciNfLLkAw6a4pWACChzobTseNqdWk4G7ZdBqRDLtLSACKykTScmqibb1ZrCvJu",
5 "tpubD6NzVbkrYhZ4XRMcMFMMFvzVt6jaDAtjZhD7JLwdPdMm9xa76DnxYYP7w9TZGJDVFkek3ArwVsuacheqqPog8TH5iBCX1wuig8PLXim4n9a",
6 "tpubD6NzVbkrYhZ4WsqRzDmkL82SWcu42JzUvKWzrJHQ8EC2vEHRHkXj1De93sD3biLrKd8XGnamXURGjMbYavbszVDXpjXV2cGUERucLJkE6cy"]
7pk = "02aebf2d10b040eb936a6f02f44ee82f8b34f5c1ccb20ff3949c2b28206b7c1068"
8
9MINISCRIPTS_PRIV = [
10 # Liquid-like federated pegin with emergency recovery privkeys
11 {
12 "ms": f"or_i(and_b(pk({tpubs[0]}),a:and_b(pk({tpubs[1]}),a:and_b(pk({tpubs[2]}),a:and_b(pk({tpubs[3]}),s:pk({pk}))))),and_v(v:thresh(2,pkh({tprv1}),a:pkh({tprv2}),a:pkh({tpubs[4]})),older(420)))",
13 "sequence": 420,
14 "locktime": None,
15 },
I’d like to boost the tests a bit.
One general case I’d like covered is the one where the PSBT cannot be finalized. Also, I think it’d be useful to assert things like how many partial signatures you expect the wallet to make, which finalization paths it takes depending on nsequence value, things like that.
some examples I found illuminating
0 # We have one key on each branch; Core signs both (can't finalize)
1 {
2 "ms": f"c:andor(pk({tprv1}),pk_k({tpubs[0]}),and_v(v:pk({tprv2}),pk_k({tpubs[1]})))",
3 "sequence": None,
4 "locktime": None,
5 },
6 # We have all the keys, wallet selects the timeout path to sign since it's smaller and sequence is set
7 {
8 "ms": f"andor(pk({tprv1}),pk({tprv3}),and_v(v:pk({tprv2}),older(10)))",
9 "sequence": 10,
10 "locktime": None,
11 },
12 # We have all the keys, wallet selects the primary path to sign unconditionally since nsequence wasn't set to be valid for timeout path
13 {
14 "ms": f"andor(pk({tprv1}),pk({tprv3}),and_v(v:pk({tprv2}),older(10)))",
15 "sequence": None,
16 "locktime": None,
17 },
18 # Finalizes to the smallest valid witness, regardless of sequence
19 {
20 "ms": f"or_d(pk({tprv1}),and_v(v:pk({tprv2}),and_v(v:pk({tprv3}),older(10))))",
21 "sequence": None,
22 "locktime": None,
23 },
24 # Liquid-like federated pegin with emergency recovery privkeys
25 {
26 "ms": f"or_i(and_b(pk({tpubs[0]}),a:and_b(pk({tpubs[1]}),a:and_b(pk({tpubs[2]}),a:and_b(pk({tpubs[3]}),s:pk({pk}))))),and_v(v:thresh(2,pkh({tprv1}),a:pkh({tprv2}),a:pkh({tpubs[4]})),older(420)))",
27 "sequence": 420,
28 "locktime": None,
29 },
This feature will likely need to be explained in the various PSBT calls as well.
This introduces the logic to "sign for" a Miniscript.
Co-Authored-By: Pieter Wuille <pieter.wuille@gmail.com>
Cherry-picked and squashed from
https://github.com/sipa/bitcoin/commits/202302_miniscript_improve.
- Explain thresh() and multi() satisfaction algorithms
- Comment on and_v dissatisfaction
- Mark overcomplete thresh() dissats as malleable and explain
- Add comment on unnecessity of Malleable() in and_b dissat
Try to solve a script using the Miniscript satisfier if the legacy
solver fails under P2WSH context. Only solve public key and public key
hash challenges for now.
We don't entirely replace the raw solver and especially rule out trying to
solve CHECKMULTISIG-based multisigs with the Miniscript satisfier since
some features, such as the transaction input combiner, rely on the
specific behaviour of the former.
Preimages must be externally provided (typically, via a PSBT).
This is a workaround for Miniscript descriptors containing hash
challenges. For those we can't mock the signature creator without making
OP_EQUAL mockable in the interpreter, so CalculateMaximumInputSize will
always return -1 and outputs for these descriptors would appear
unsolvable while they actually are.
We'll add more of them in the next commit, let's keep it bearable.
We'll need a better integration of the hash preimages PSBT fields to
satisfy Miniscript with such challenges from the RPC.
Thanks to Greg Sanders for his examples and suggestions to improve this
test.
e property is related to signing, so i just cherry-picked them as well (squashed in one commit).
This is a "dumb" way of randomly generating a Miniscript node from
fuzzer input. It defines a strict binary encoding and will always generate
a node defined from the encoding without "helping" to create valid nodes.
It will cut through as soon as it encounters an invalid fragment so
hopefully the fuzzer can tend to learn the encoding and generate valid
nodes with a higher probability.
On a valid generated node a number of invariants are checked, especially
around the satisfactions and testing them against the Script
interpreter.
The node generation and testing is modular in order to later introduce
other ways to generate nodes from fuzzer inputs with minimal code.
Co-Authored-By: Pieter Wuille <pieter@wuille.net>
At the expense of more complexity, this target generates a valid
Miniscript node at every iteration.
This target will at first run populate a list of recipe (a map from
desired type to possible ways of creating such type) and curate it
(remove the unavailable or redundant recipes).
Then, at each iteration it will pick a type, choose a manner to create a
node of such type from the available recipes, and then
pseudo-recursively do the same for the type constraints of the picked
recipe.
For instance, if it is instructed based on the fuzzer output to create a
Miniscript node of type 'Bd', it could choose to create an 'or_i(subA, subB)'
nodes with type constraints 'B' for subA and 'Bd' for subB. It then
consults the recipes for creating subA and subB, etc...
Here is the list of all the existing recipes, by type constraint:
B: 0()
B: 1()
B: older()
B: after()
B: sha256()
B: hash256()
B: ripemd160()
B: hash160()
B: c:(K)
B: d:(Vz)
B: j:(Bn)
B: n:(B)
B: and_v(V,B)
B: and_b(B,W)
B: or_b(Bd,Wd)
B: or_d(Bdu,B)
B: or_i(B,B)
B: andor(Bdu,B,B)
B: thresh(Bdu)
B: thresh(Bdu,Wdu)
B: thresh(Bdu,Wdu,Wdu)
B: multi()
V: v:(B)
V: and_v(V,V)
V: or_c(Bdu,V)
V: or_i(V,V)
V: andor(Bdu,V,V)
K: pk_k()
K: pk_h()
K: and_v(V,K)
K: or_i(K,K)
K: andor(Bdu,K,K)
W: a:(B)
W: s:(Bo)
Bz: 0()
Bz: 1()
Bz: older()
Bz: after()
Bz: n:(Bz)
Bz: and_v(Vz,Bz)
Bz: or_d(Bzdu,Bz)
Bz: andor(Bzdu,Bz,Bz)
Bz: thresh(Bzdu)
Vz: v:(Bz)
Vz: and_v(Vz,Vz)
Vz: or_c(Bzdu,Vz)
Vz: andor(Bzdu,Vz,Vz)
Bo: sha256()
Bo: hash256()
Bo: ripemd160()
Bo: hash160()
Bo: c:(Ko)
Bo: d:(Vz)
Bo: j:(Bon)
Bo: n:(Bo)
Bo: and_v(Vz,Bo)
Bo: and_v(Vo,Bz)
Bo: or_d(Bodu,Bz)
Bo: or_i(Bz,Bz)
Bo: andor(Bzdu,Bo,Bo)
Bo: andor(Bodu,Bz,Bz)
Bo: thresh(Bodu)
Vo: v:(Bo)
Vo: and_v(Vz,Vo)
Vo: and_v(Vo,Vz)
Vo: or_c(Bodu,Vz)
Vo: or_i(Vz,Vz)
Vo: andor(Bzdu,Vo,Vo)
Vo: andor(Bodu,Vz,Vz)
Ko: pk_k()
Ko: and_v(Vz,Ko)
Ko: andor(Bzdu,Ko,Ko)
Bn: sha256()
Bn: hash256()
Bn: ripemd160()
Bn: hash160()
Bn: c:(Kn)
Bn: d:(Vz)
Bn: j:(Bn)
Bn: n:(Bn)
Bn: and_v(Vz,Bn)
Bn: and_v(Vn,B)
Bn: and_b(Bn,W)
Bn: multi()
Vn: v:(Bn)
Vn: and_v(Vz,Vn)
Vn: and_v(Vn,V)
Kn: pk_k()
Kn: pk_h()
Kn: and_v(Vz,Kn)
Kn: and_v(Vn,K)
Bon: sha256()
Bon: hash256()
Bon: ripemd160()
Bon: hash160()
Bon: c:(Kon)
Bon: d:(Vz)
Bon: j:(Bon)
Bon: n:(Bon)
Bon: and_v(Vz,Bon)
Bon: and_v(Von,Bz)
Von: v:(Bon)
Von: and_v(Vz,Von)
Von: and_v(Von,Vz)
Kon: pk_k()
Kon: and_v(Vz,Kon)
Bd: 0()
Bd: sha256()
Bd: hash256()
Bd: ripemd160()
Bd: hash160()
Bd: c:(Kd)
Bd: d:(Vz)
Bd: j:(Bn)
Bd: n:(Bd)
Bd: and_b(Bd,Wd)
Bd: or_b(Bd,Wd)
Bd: or_d(Bdu,Bd)
Bd: or_i(B,Bd)
Bd: or_i(Bd,B)
Bd: andor(Bdu,B,Bd)
Bd: thresh(Bdu)
Bd: thresh(Bdu,Wdu)
Bd: thresh(Bdu,Wdu,Wdu)
Bd: multi()
Kd: pk_k()
Kd: pk_h()
Kd: or_i(K,Kd)
Kd: or_i(Kd,K)
Kd: andor(Bdu,K,Kd)
Wd: a:(Bd)
Wd: s:(Bod)
Bzd: 0()
Bzd: n:(Bzd)
Bzd: or_d(Bzdu,Bzd)
Bzd: andor(Bzdu,Bz,Bzd)
Bzd: thresh(Bzdu)
Bod: sha256()
Bod: hash256()
Bod: ripemd160()
Bod: hash160()
Bod: c:(Kod)
Bod: d:(Vz)
Bod: j:(Bon)
Bod: n:(Bod)
Bod: or_d(Bodu,Bzd)
Bod: or_i(Bz,Bzd)
Bod: or_i(Bzd,Bz)
Bod: andor(Bzdu,Bo,Bod)
Bod: andor(Bodu,Bz,Bzd)
Bod: thresh(Bodu)
Kod: pk_k()
Kod: andor(Bzdu,Ko,Kod)
Bu: 0()
Bu: 1()
Bu: sha256()
Bu: hash256()
Bu: ripemd160()
Bu: hash160()
Bu: c:(K)
Bu: d:(Vz)
Bu: j:(Bnu)
Bu: n:(B)
Bu: and_v(V,Bu)
Bu: and_b(B,W)
Bu: or_b(Bd,Wd)
Bu: or_d(Bdu,Bu)
Bu: or_i(Bu,Bu)
Bu: andor(Bdu,Bu,Bu)
Bu: thresh(Bdu)
Bu: thresh(Bdu,Wdu)
Bu: thresh(Bdu,Wdu,Wdu)
Bu: multi()
Bzu: 0()
Bzu: 1()
Bzu: n:(Bz)
Bzu: and_v(Vz,Bzu)
Bzu: or_d(Bzdu,Bzu)
Bzu: andor(Bzdu,Bzu,Bzu)
Bzu: thresh(Bzdu)
Bou: sha256()
Bou: hash256()
Bou: ripemd160()
Bou: hash160()
Bou: c:(Ko)
Bou: d:(Vz)
Bou: j:(Bonu)
Bou: n:(Bo)
Bou: and_v(Vz,Bou)
Bou: and_v(Vo,Bzu)
Bou: or_d(Bodu,Bzu)
Bou: or_i(Bzu,Bzu)
Bou: andor(Bzdu,Bou,Bou)
Bou: andor(Bodu,Bzu,Bzu)
Bou: thresh(Bodu)
Bnu: sha256()
Bnu: hash256()
Bnu: ripemd160()
Bnu: hash160()
Bnu: c:(Kn)
Bnu: d:(Vz)
Bnu: j:(Bnu)
Bnu: n:(Bn)
Bnu: and_v(Vz,Bnu)
Bnu: and_v(Vn,Bu)
Bnu: and_b(Bn,W)
Bnu: multi()
Bonu: sha256()
Bonu: hash256()
Bonu: ripemd160()
Bonu: hash160()
Bonu: c:(Kon)
Bonu: d:(Vz)
Bonu: j:(Bonu)
Bonu: n:(Bon)
Bonu: and_v(Vz,Bonu)
Bonu: and_v(Von,Bzu)
Bdu: 0()
Bdu: sha256()
Bdu: hash256()
Bdu: ripemd160()
Bdu: hash160()
Bdu: c:(Kd)
Bdu: d:(Vz)
Bdu: j:(Bnu)
Bdu: n:(Bd)
Bdu: and_b(Bd,Wd)
Bdu: or_b(Bd,Wd)
Bdu: or_d(Bdu,Bdu)
Bdu: or_i(Bu,Bdu)
Bdu: or_i(Bdu,Bu)
Bdu: andor(Bdu,Bu,Bdu)
Bdu: thresh(Bdu)
Bdu: thresh(Bdu,Wdu)
Bdu: thresh(Bdu,Wdu,Wdu)
Bdu: multi()
Wdu: a:(Bdu)
Wdu: s:(Bodu)
Bzdu: 0()
Bzdu: n:(Bzd)
Bzdu: or_d(Bzdu,Bzdu)
Bzdu: andor(Bzdu,Bzu,Bzdu)
Bzdu: thresh(Bzdu)
Bodu: sha256()
Bodu: hash256()
Bodu: ripemd160()
Bodu: hash160()
Bodu: c:(Kod)
Bodu: d:(Vz)
Bodu: j:(Bonu)
Bodu: n:(Bod)
Bodu: or_d(Bodu,Bzdu)
Bodu: or_i(Bzu,Bzdu)
Bodu: or_i(Bzdu,Bzu)
Bodu: andor(Bzdu,Bou,Bodu)
Bodu: andor(Bodu,Bzu,Bzdu)
Bodu: thresh(Bodu)
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
285@@ -266,6 +286,7 @@ def run_test(self):
286 ms["locktime"],
287 ms["sigs_count"],
288 ms["stack_size"],
289+ ms.get("sha256_preimages"),
comment above should be updated I think:
“# Test we can sign most Miniscript (all but ones requiring preimages, for now)”
ACK https://github.com/bitcoin/bitcoin/pull/24149/commits/36478ac00ef0a82a82052032e68d513a92ea38b8
reviewed tests only
Co-Authored-By: Andrew Chow <github@achow101.com>
108@@ -108,7 +109,8 @@ BOOST_AUTO_TEST_CASE(DoS_mapOrphans)
109 tx.vin[j].prevout.n = j;
110 tx.vin[j].prevout.hash = txPrev->GetHash();
111 }
112- BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL));
113+ SignatureData empty;
114+ BOOST_CHECK(SignSignature(keystore, *txPrev, tx, 0, SIGHASH_ALL, empty));
SignatureData objects that need to be created in calls, would it make sense to add another overload of SignSignature that has no SignaturaData& argument?
ACK 6c7a17a8e0eec377f83ed1399f003ae70b898270
Reviewed code and non-fuzz tests.
251+// https://github.com/llvm/llvm-project/issues/53444
252+// NOLINTNEXTLINE(misc-unused-using-decls)
253+using miniscript::operator"" _mst;
254+
255+//! Construct a miniscript node as a shared_ptr.
256+template<typename... Args> NodeRef MakeNodeRef(Args&&... args) { return miniscript::MakeNodeRef<CPubKey>(KEY_COMP, std::forward<Args>(args)...); }
GenNode()?
Labels
Build system
Descriptors
Milestone
25.0