As stated on the website, duplicate keys make it hard to reason about
malleability as a single signature may unlock multiple paths.
We use a custom KeyCompare function instead of operator< to be explicit
about the requirement.
BIP 341 specifies constraints on the size of the control block _c_ used
to compute the taproot merkle root.
> The last stack element is called the control block _c_, and must have
> length _33 + 32m_, for a value of m that is an integer between 0 and
> 128, inclusive. Fail if it does not have such a length.
(See BIP-341 "Script Validation Rules" here: https://github.com/bitcoin/bips/blob/master/bip-0341.mediawiki#script-validation-rules)
Since the removal of NODISCARD in 81d5af42f4,
the only attributes def is LIFETIMEBOUND, and it's included in many more
places that it is used.
This removes all includes which do not have an associated use of LIFETIMEBOUND,
and adds it to the following files, due to their use of the same:
* src/validationinterface.h
* src/script/standard.h
This makes IsSane clearer. It is useful to differentiate between 'potential non-malleable satisfactions are valid' and 'such satisfactions exist' for testing.
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
The 'Fragment' type was previously named 'Nodetype'. For clarity, name
the variables the same.
-BEGIN VERIFY SCRIPT-
sed -i 's/nodetype/fragment/g' src/script/miniscript.*
-END VERIFY SCRIPT-
Co-authored-by: Pieter Wuille <pieter.wuille@gmail.com>
The value it leaves on the stack depends on the last element on the
stack. However, we can't make sure this element is OP_1 (which would
give us the 'u' property) without the MINIMALIF rule.
MINIMALIF is only policy for P2WSH, therefore giving 'd:' the 'u'
property breaks consensus soundness: it makes it possible (by consensus
but not policy) for instance to satisfy a thresh() without satisfying
at least k of its subs.
This bug was found and reported by Andrew Poelstra.
2da94a4c6f fuzz: add a fuzz target for Miniscript decoding from Script (Antoine Poinsot)
f8369996e7 Miniscript: ops limit and stack size computation (Pieter Wuille)
2e55e88f86 Miniscript: conversion from script (Pieter Wuille)
1ddaa66eae Miniscript: type system, script creation, text notation, tests (Pieter Wuille)
4fe29368c0 script: expose getter for CScriptNum, add a BuildScript helper (Antoine Poinsot)
f4e289f384 script: move CheckMinimalPush from interpreter to script.h (Antoine Poinsot)
31ec6ae92a script: make IsPushdataOp non-static (Antoine Poinsot)
Pull request description:
Miniscript is a language for writing (a subset of) Bitcoin Scripts in a structured way.
Miniscript permits:
- To safely extend the Output Descriptor language to many more scripting features thanks to the typing system (composition).
- Statical analysis of spending conditions, maximum spending cost of each branch, security properties, third-party malleability.
- General satisfaction of any correctly typed ("valid" [0]) Miniscript. The satisfaction itself is also analyzable.
- To extend the possibilities of external signers, because of all of the above and since it carries enough metadata.
Miniscript guarantees:
- That for any statically-analyzed as "safe" [0] Script, a witness can be constructed in the bounds of the consensus and standardness rules (standardness complete).
- That unless the conditions of the Miniscript are met, no witness can be created for the Script (consensus sound).
- Third-party malleability protection for the satisfaction of a sane Miniscript, which is too complex to summarize here.
For more details around Miniscript (including the specifications), please refer to the [website](https://bitcoin.sipa.be/miniscript/).
Miniscript was designed by Pieter Wuille, Andrew Poelstra and Sanket Kanjalkar.
This PR is an updated and rebased version of #16800. See [the commit history of the Miniscript repository](https://github.com/sipa/miniscript/commits/master) for details about the changes made since September 2019 (TL;DR: bugfixes, introduction of timelock conflicts in the type system, `pk()` and `pkh()` aliases, `thresh_m` renamed to `multi`, all recursive algorithms were made non-recursive).
This PR is also the first in a series of 3:
- The first one (here) integrates the backbone of Miniscript.
- The second one (#24148) introduces support for Miniscript in Output Descriptors, allowing for watch-only support of Miniscript Descriptors in the wallet.
- The third one (#24149) implements signing for these Miniscript Descriptors, using Miniscript's satisfaction algorithm.
Note to reviewers:
- Miniscript is currently defined only for P2WSH. No Taproot yet.
- Miniscript is different from the policy language (a high-level logical representation of a spending policy). A policy->Miniscript compiler is not included here.
- The fuzz target included here is more interestingly extended in the 3rd PR to check a script's satisfaction against `VerifyScript`. I think it could be further improved by having custom mutators as we now have for multisig (see https://github.com/bitcoin/bitcoin/issues/23105). A minified corpus of Miniscript Scripts is available at https://github.com/bitcoin-core/qa-assets/pull/85.
[0] We call "valid" any correctly-typed Miniscript. And "safe" any sane Miniscript, ie one whose satisfaction isn't malleable, which requires a key for any spending path, etc..
ACKs for top commit:
jb55:
ACK 2da94a4c6f
laanwj:
Light code review ACK 2da94a4c6f (mostly reviewed the changes to the existing code and build system)
Tree-SHA512: d3ef558436cfcc699a50ad13caf1e776f7d0addddb433ee28ef38f66ea5c3e581382d8c748ccac9b51768e4b95712ed7a6112b0e3281a6551e0f325331de9167
9b52672700 For descriptor pubkey parse errors, include context information (Ben Woosley)
Pull request description:
This adds readily-available context information to the error string, for further disambiguation.
This is a revival of #16123 which was largely addressed in #16542.
Note 'Multi:' is used rather than 'multi():' as it also encompasses 'sortedmulti():'
ACKs for top commit:
achow101:
ACK 9b52672700
theStack:
ACK 9b52672700
Tree-SHA512: 96533ea8c3ac7010f9b62e75b4bd20b65aff843030eb91c7a88312975acecaaf17909b7d1841f45edc86dbf7fa402d208adb85f0673bd79b857dbebacb8c9395
More information about Miniscript can be found at https://bitcoin.sipa.be/miniscript/ (the
website source is hosted at https://github.com/sipa/miniscript/).
This commit defines all fragments, their composition, parsing from
string representation and conversion to Script.
Co-Authored-By: Antoine Poinsot <darosior@protonmail.com>
Co-Authored-By: Sanket Kanjalkar <sanket1729@gmail.com>
Co-Authored-By: Samuel Dobson <dobsonsa68@gmail.com>
Some prep work for Miniscript. BuildScript is an efficient way to build
Scripts in a generic manner (by concatenating OPs, data, and other
Scripts).
Co-Authored-By: Pieter Wuille <pieter@wuille.net>
fae20e6b50 Revert "Avoid the use of P0083R3 std::set::merge" (MarcoFalke)
fab53b5fd4 ci/doc: Set minimum required clang/libc++ version to 8.0 (MarcoFalke)
Pull request description:
This is not for 23.0, but for 24.0. It comes with the following benefits:
* Can use C++17 P0083R3 std::set::merge from libc++ 8.0
* No longer need to provide support for clang-7, which already fails to compile on some architectures (https://github.com/bitcoin/bitcoin/issues/21294#issuecomment-998098483)
This should be fine, given that all supported operating systems ship with at least clang-10:
* CentOS 8: clang-12
* Stretch: https://packages.debian.org/stretch/clang-11
* Buster: https://packages.debian.org/buster-backports/clang-11
* Bionic: https://packages.ubuntu.com/bionic-updates/clang-10
* Focal: https://packages.ubuntu.com/focal/clang-10
ACKs for top commit:
fanquake:
ACK fae20e6b50 - I think this is fine to do. I would be surprised if in another 6 months time someone was stuck on a system we supported, needing to compile Core, and only had access to Clang 7 or older. As mentioned in the PR description, all systems we currently support, already support multiple newer versions of Clang.
hebasto:
ACK fae20e6b50, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 3b4c6c130ff40dd7e84934af076863415e5dd661d823c72e3e3832566c65be6e877a7ef9164bbcf394bcea4b897fc29a48db0f231c22ace0e2c9b5638659a628
4828d53ecc Add (sorted)multi_a descriptors to doc/descriptors.md (Pieter Wuille)
b5f33ac1f8 Simplify wallet_taproot.py functional test (Pieter Wuille)
eb0667ea96 Add tests for (sorted)multi_a derivation/signing (Pieter Wuille)
c17c6aa08d Add signing support for (sorted)multi_a scripts (Pieter Wuille)
3eed6fca57 Add multi_a descriptor inference (Pieter Wuille)
79728c4a3d Add (sorted)multi_a descriptor and script derivation (Pieter Wuille)
25e95f9ff8 Merge/generalize IsValidMultisigKeyCount/GetMultisigKeyCount (Pieter Wuille)
Pull request description:
This adds a new `multi_a(k,key_1,key_2,...,key_n)` (and corresponding `sortedmulti_a`) descriptor for k-of-n policies inside `tr()`. Semantically it is very similar to the existing `multi()` descriptor, but with the following changes:
* The corresponding script is `<key1> OP_CHECKSIG <key2> OP_CHECKSIGADD <key3> OP_CHECKSIGADD ... <key_n> OP_CHECKSIGADD <k> OP_NUMEQUAL`, rather than the traditional `OP_CHECKMULTISIG`-based script, making it usable inside the `tr()` descriptor.
* The keys can optionally be specified in x-only notation.
* Both the number of keys and the threshold can be as high as 999; this is the limit due to the consensus stacksize=1000 limit
I expect that this functionality will later be replaced with a miniscript-based implementation, but I don't think it's necessary to wait for that.
Limitations:
* The wallet code will for not estimate witness size incorrectly for script path spends, which may result in a (dramatic) fee underpayment with large multi_a scripts.
* The multi_a script construction is (slightly) suboptimal for n-of-n (where a `<key1> OP_CHECKSIGVERIFY ... <key_n-1> OP_CHECKSIGVERIFY <key_n> OP_CHECKSIG` would be better). Such a construction is not included here.
ACKs for top commit:
achow101:
ACK 4828d53ecc
gruve-p:
ACK 4828d53ecc
sanket1729:
code review ACK 4828d53ecc
darosior:
Code review ACK 4828d53ecc
Tree-SHA512: 5dcd434b79585f0ff830f7d501d27df5e346f5749f47a3109ec309ebf2cbbad0e1da541eec654026d911ab67fd7cf7793fab0f765628d68d81b96ef2a4d234ce
fa5d2e678c Remove unused char serialize (MarcoFalke)
fa24493d63 Use spans of std::byte in serialize (MarcoFalke)
fa65bbf217 span: Add BytePtr helper (MarcoFalke)
Pull request description:
This changes the serialize code (`.read()` and `.write()` functions) to take a `Span` instead of a pointer and size. This is a breaking change for the serialize interface, so at no additional cost we can also switch to `std::byte` (instead of using `char`).
The benefits of using `Span`:
* Less verbose and less fragile code when passing an already existing `Span`(-like) object to or from serialization
The benefits of using `std::byte`:
* `std::byte` can't accidentally be mistaken for an integer
The goal here is to only change serialize to use spans of `std::byte`. If needed, `AsBytes`, `MakeUCharSpan`, ... can be used (temporarily) to pass spans of the right type.
Other changes that are included here:
* [#22167](https://github.com/bitcoin/bitcoin/pull/22167) (refactor: Remove char serialize by MarcoFalke)
* [#21906](https://github.com/bitcoin/bitcoin/pull/21906) (Preserve const in cast on CTransactionSignatureSerializer by promag)
ACKs for top commit:
laanwj:
Concept and code review ACK fa5d2e678c
sipa:
re-utACK fa5d2e678c
Tree-SHA512: 08ee9eced5fb777cedae593b11e33660bed9a3e1711a7451a87b835089a96c99ce0632918bb4666a4e859c4d020f88fb50f2dd734216b0c3d1a9a704967ece6f
SerializeToVector, UnserializeFromVector, DeserializeHDKeypaths, and SerializeHDKeypaths
were in sign.h where PSBT was originally implemented. Since all of the PSBT serialization
has moved to its own file, these functions should follow.
4868c9f1b3 Extract Taproot internal keyid with GetKeyFromDestination (Andrew Chow)
d8abbe119c Mention bech32m in -addresstype and -changetype help (Andrew Chow)
8fb57845ee Create a tr() descriptor bech32m DescriptorScriptPubKeyMan by default (Andrew Chow)
54b3699862 Store pubkeys in TRDescriptor::MakeScripts (Andrew Chow)
Pull request description:
Make a `tr()` descriptor by default in descriptor wallets so that users will be able to make and use segwit v1 bech32m addresses.
ACKs for top commit:
MarcoFalke:
Concept ACK 4868c9f1b3
Sjors:
re-utACK 4868c9f1b3
gruve-p:
ACK 4868c9f1b3
meshcollider:
Concept + code review ACK 4868c9f1b3
Tree-SHA512: e5896e665b8d559f1d759b6582d1bb24f70d4698a57307684339d9fdcdac28ae9bc17bc946a7efec9cb35c130a95ffc36e3961a335124ec4535d77b8d00e9631
