Age | Commit message (Collapse) | Author |
|
serialization
fa626af3edbe8d98b2de91dd71729ceef90389fb Remove unused legacy CHashVerifier (MarcoFalke)
fafa3fc5a62702da72991497e3270034eb9159c0 test: add tests that exercise WithParams() (MarcoFalke)
fac81affb527132945773a5315bd27fec61ec52f Use serialization parameters for CAddress serialization (MarcoFalke)
faec591d64e40ba7ec7656cbfdda1a05953bde13 Support for serialization parameters (MarcoFalke)
fac42e9d35f6ba046999b2e3a757ab720c51b6bb Rename CSerAction* to Action* (MarcoFalke)
aaaa3fa9477eef9ea72e4a501d130c57b47b470a Replace READWRITEAS macro with AsBase wrapping function (MarcoFalke)
Pull request description:
It seems confusing that picking a wrong value for `ADDRV2_FORMAT` could have effects on consensus. (See the docstring of `ADDRV2_FORMAT`).
Fix this by implementing https://github.com/bitcoin/bitcoin/issues/19477#issuecomment-1147421608 .
This may also help with libbitcoinkernel, see https://github.com/bitcoin/bitcoin/pull/28327
ACKs for top commit:
TheCharlatan:
ACK fa626af3edbe8d98b2de91dd71729ceef90389fb
ajtowns:
ACK fa626af3edbe8d98b2de91dd71729ceef90389fb
Tree-SHA512: 229d379da27308890de212b1fd2b85dac13f3f768413cb56a4b0c2da708f28344d04356ffd75bfcbaa4cabf0b6cc363c4f812a8f1648cff9e436811498278318
|
|
using descriptors
10546a569c6c96a5ec1b9708abf9ff5c8644f669 wallet: accurately account for the size of the witness stack (Antoine Poinsot)
9b7ec393b82ca9d7ada77d06e0835df0386a8b85 wallet: use descriptor satisfaction size to estimate inputs size (Antoine Poinsot)
8d870a98731e8db5ecc614bb5f7c064cbf30c7f4 script/signingprovider: introduce a MultiSigningProvider (Antoine Poinsot)
fa7c46b503f0b69630f55dc43021d2099e3515ba descriptor: introduce a method to get the satisfaction size (Antoine Poinsot)
bdba7667d2d65f31484760a8e8420c488fc5f801 miniscript: introduce a helper to get the maximum witness size (Antoine Poinsot)
4ab382c2cdb09fb4056711b4336807845cbe1ad5 miniscript: make GetStackSize independent of P2WSH context (Antoine Poinsot)
Pull request description:
The wallet currently estimates the size of a signed input by doing a dry run of the signing logic. This is unnecessary since all outputs we can sign for can be represented by a descriptor, and we can derive the size of a satisfaction ("signature") directly from the descriptor itself.
In addition, the current approach does not generalize well: dry runs of the signing logic are only possible for the most basic scripts. See for instance the discussion in #24149 around that.
This introduces a method to get the maximum size of a satisfaction from a descriptor, and makes the wallet use that instead of the dry-run.
ACKs for top commit:
sipa:
utACK 10546a569c6c96a5ec1b9708abf9ff5c8644f669
achow101:
re-ACK 10546a569c6c96a5ec1b9708abf9ff5c8644f669
Tree-SHA512: 43ed1529fbd30af709d903c8c5063235e8c6a03b500bc8f144273d6184e23a53edf0fea9ef898ed57d8a40d73208b5d935cc73b94a24fad3ad3c63b3b2027174
|
|
Co-authored-by: Pieter Wuille <pieter@wuille.net>
|
|
When estimating the maximum size of an input, we were assuming the
number of elements on the witness stack could be encode in a single
byte. This is a valid approximation for all the descriptors we support
(including P2WSH Miniscript ones), but may not hold anymore once we
support Miniscript within Taproot descriptors (since the max standard
witness stack size of 100 gets lifted).
It's a low-hanging fruit to account for it correctly, so just do it now.
|
|
It is sometimes useful to interface with multiple signing providers at
once. For instance when inferring a descriptor with solving information
being provided from multiple sources (see next commit).
Instead of inneficiently copying the information from one provider into
the other, introduce a new signing provider that takes a list of
pointers to existing providers.
|
|
In the wallet code, we are currently estimating the size of a signed
input by doing a dry run of the signing logic. This is unnecessary as
all outputs we are able to sign for can be represented by a descriptor,
and we can derive the size of a satisfaction ("signature") from the
descriptor itself directly.
In addition, this approach does not scale: getting the size of a
satisfaction through a dry run of the signing logic is only possible for
the most basic scripts.
This commit introduces the computation of the size of satisfaction per
descriptor. It's a bit intricate for 2 main reasons:
- We want to conserve the behaviour of the current dry-run logic used by
the wallet that sometimes assumes ECDSA signatures will be low-r,
sometimes not (when we don't create them).
- We need to account for the witness discount. A single descriptor may
sometimes benefit of it, sometimes not (for instance `pk()` if used as
top-level versus if used inside `wsh()`).
|
|
Similarly to how we compute the maximum stack size.
Also note how it would be quite expensive to recompute it recursively
by accounting for different ECDSA signature sizes. So we just assume
high-R everywhere. It's only a trivial difference anyways.
|
|
It was taking into account the P2WSH script push in the number of stack
elements.
|
|
|
|
Since script/standard only contains things that are used by the Solver
and its callers, rename the files to script/solver.
|
|
|
|
Remove standard.h from files that don't use anything in it, and include
it in files that do.
|
|
|
|
CTxDestination is really our internal representation of an address and
doesn't really have anything to do with standard script types, so move
them to their own file.
|
|
TaprootSpendData and TaprootBuilder are used in signing in
SigningProvider contexts, so they should live near that.
|
|
CScriptID should be next to CScript just as CKeyID is next to CPubKey
|
|
Replaces the constructor in CScriptID that converts a ScriptHash with a
function ToScriptID that does the same. This prepares for a move of
CScriptID to avoid a circular dependency.
|
|
|
|
as sub descriptors
dd9633b516d6936ac4e23a40f9b0bea120117d35 test: wallet, add coverage for watch-only raw sh script migration (furszy)
cc781a21800a6ce13875feefd0cb14ab0a84524c descriptor: InferScript, do not return top-level only func as sub descriptor (furszy)
286e0c7d5e9538198b28b792c5168b8fafa1534f wallet: loading, log descriptor parsing error details (furszy)
Pull request description:
Linked to #28057.
Currently, the `InferScript` function returns an invalid descriptor when it tries to infer a p2sh-p2pkh script whose pubkey is not known by the wallet.
This behavior occurs because the inference process bypasses the `pkh` subscript when the pubkey is not contained by the wallet (no pubkey provider), interpreting it as a `sh(addr(ADDR))` descriptor. Then, the failure arises because the `addr()` function is restricted to being used only at the top level.
For reviewers, would recommend to start by examining the functional test to understand the context and the circumstances on which this can result in a fatal error (e.g. during the migration process).
ACKs for top commit:
achow101:
ACK dd9633b516d6936ac4e23a40f9b0bea120117d35
darosior:
utACK dd9633b516d6936ac4e23a40f9b0bea120117d35
Tree-SHA512: 61e763206c604c372019d2c36e31684f3dddf81f8b154eb9aba5cd66d8d61bda457ed4e591613eb6ce6c76cf7c3f11764abc6cd727a7c2b6414f1065783be032
|
|
e.g. sh(addr(ADDR)) or sh(raw(HEX)) are invalid descriptors.
Making sh and wsh top level functions to return addr/raw descriptors when
the subscript inference fails.
|
|
descriptors
c7db88af71b3204171f33399aa4f33b40a4f7cd9 descriptor: assert we never parse a sane miniscript with no pubkey (Antoine Poinsot)
a49402a9ec7431c286139b76f8759719a99a8551 qa: make sure we don't let unspendable Miniscript descriptors be imported (Antoine Poinsot)
639e3b6c9759a7a582c5c86fdbfa5ea99cb7bb16 descriptor: refuse to parse unspendable miniscript descriptors (Antoine Poinsot)
e3280eae1b53006d74d11f3cf9d7a9dc7ff2c39e miniscript: make GetStackSize() and GetOps() return optionals (Antoine Poinsot)
Pull request description:
`IsSane()` in Miniscript does not ensure a Script is actually spendable. This is an issue as we would accept any sane Miniscript when parsing a descriptor. Fix this by explicitly checking a Miniscript descriptor is both sane and spendable when parsing it.
This bug was exposed due to a check added in #22838 (https://github.com/bitcoin/bitcoin/pull/22838#discussion_r1226859880) that triggered a fuzz crash (https://github.com/bitcoin/bitcoin/pull/22838#issuecomment-1612510057).
ACKs for top commit:
sipa:
utACK c7db88af71b3204171f33399aa4f33b40a4f7cd9
achow101:
ACK c7db88af71b3204171f33399aa4f33b40a4f7cd9
Tree-SHA512: e79bc9f7842e98a4e8f358f05811fca51b15b4b80a171c0d2b17cf4bb1f578a18e4397bc2ece9817d392e0de0196ee6a054b7318441fd3566dd22e1f03eb64a5
|
|
|
|
It's possible for some unsatisfiable miniscripts to be considered sane.
Make sure we refuse to import those, as they would be unspendable.
|
|
The value is only set for satisfiable nodes, so it was undefined for
non-satisfiable nodes. Make it clear in the interface by returning
std::nullopt if the node isn't satisfiable instead of an undefined
value.
|
|
As we update the descriptor's db record every time that
the wallet is loaded (at `TopUp` time), if the spkm ID differs
from the one in db, the wallet will enter in an unrecoverable
corruption state, and no soft version will be able to open
it anymore.
Because we cannot change the past, to stay compatible between
releases, we need to always use the apostrophe version for the
spkm IDs.
|
|
This allows us to verify the descriptor ID on the descriptors
unit tests in different software versions without requiring to
use the entire DescriptorScriptPubKeyMan machinery.
Note:
The unit test changes are introduced after the bugfix commit
but this commit + the unit test commit can be cherry-picked
on top of the v25 branch to verify IDs correctness. IDs must
be the same for v25 and after the bugfix commit.
|
|
Since the kernel library no longer depends on the system file, move it
to the common library instead in accordance to the diagram in
doc/design/libraries.md.
|
|
fe49f06c0e91b96feb8d8f1bd478c3173f14782c doc: clarify PR 26076 release note (Sjors Provoost)
bd13dc2f46ea10302a928fcf0f53b7aed77ad260 Switch hardened derivation marker to h in descriptors (Sjors Provoost)
Pull request description:
This makes it easier to handle descriptor strings manually, especially when importing from another Bitcoin Core wallet.
For example the `importdescriptors` RPC call is easiest to use `h` as the marker: `'["desc": ".../0h/..."]'`, avoiding the need for escape characters. With this change `listdescriptors` will use `h`, so you can copy-paste the result, without having to add escape characters or switch `'` to 'h' manually.
Both markers can still be parsed.
The `hdkeypath` field in `getaddressinfo` is also impacted by this change, except for legacy wallets. The latter is to prevent accidentally breaking ancient software that uses our legacy wallet.
See discussion in #15740
ACKs for top commit:
achow101:
ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
darosior:
re-ACK fe49f06c0e91b96feb8d8f1bd478c3173f14782c
Tree-SHA512: f78bc873b24a6f7a2bf38f5dd58f2b723e35e6b10e4d65c36ec300e2d362d475eeca6e5afa04b3037ab4bee0bf8ebc93ea5fc18102a2111d3d88fc873c08dc89
|
|
This is an extraction of ArgsManager related functions from util/system
into their own common file.
Config file related functions are moved to common/config.cpp.
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager. The ArgsManager belongs
into the common library, since the kernel library should not depend on
it. See doc/design/libraries.md for more information on this rationale.
|
|
This makes it easier to handle descriptor strings manually. E.g. an RPC call that takes an array of descriptors can now use '["desc": ".../0h/..."]'.
Both markers can still be parsed. The default for new descriptors is changed to h. In normalized form h is also used. For private keys the chosen marker is preserved in a round trip.
The hdkeypath field in getaddressinfo is also impacted by this change.
|
|
|
|
https://clang.llvm.org/extra/clang-tidy/checks/performance/inefficient-vector-operation.html
|
|
error is a low-level function with a sole dependency on LogPrintf, which
is defined in logging.h
The background of this commit is an ongoing effort to decouple the
libbitcoinkernel library from the ArgsManager defined in system.h.
Moving the function out of system.h allows including it from a separate
source file without including the ArgsManager definitions from system.h.
|
|
|
|
Keep track of the total number of ops the constructed script will have
during miniscript_stable and miniscript_smart fuzzers' GenNode, so it
can abort early if the 201 ops limit would be exceeded.
Also add a self-check that the final constructed node has the predicted
ops size limit, so we know the fuzzer's logic for keeping track of this
is correct.
|
|
|
|
6c7a17a8e0eec377f83ed1399f003ae70b898270 psbt: support externally provided preimages for Miniscript satisfaction (Antoine Poinsot)
840a396029316896beda46600aec3c1af09a899c qa: add a "smart" Miniscript fuzz target (Antoine Poinsot)
17e3547241d593bc92c5c6b36c54284d9d9f3feb qa: add a fuzz target generating random nodes from a binary encoding (Antoine Poinsot)
611e12502a5887ffb751bb92fadaa334d484824b qa: functional test Miniscript signing with key and timelocks (Antoine Poinsot)
d57b7f2021d2369f6e88cdf0f562aab27c51beaf refactor: make descriptors in Miniscript functional test more readable (Antoine Poinsot)
0a8fc9e200b5018c1efd6f9126eb405ca0beeea3 wallet: check solvability using descriptor in AvailableCoins (Antoine Poinsot)
560e62b1e221832ae99ff8684559a7b8f9df84a7 script/sign: signing support for Miniscripts with hash preimage challenges (Antoine Poinsot)
a2f81b6a8f1ff3b0750711409c7538812a52ef40 script/sign: signing support for Miniscript with timelocks (Antoine Poinsot)
61c6d1a8440db09c44d7fd367a6f2c641ea93d40 script/sign: basic signing support for Miniscript descriptors (Antoine Poinsot)
4242c1c52127df3a24be0c15b88d4fc463af04fc Align 'e' property of or_d and andor with website spec (Pieter Wuille)
f5deb417804b9f267830bd40177677987df4526d Various additional explanations of the satisfaction logic from Pieter (Pieter Wuille)
22c5b00345063bdeb8b6d3da8b5692d18f92bfb7 miniscript: satisfaction support (Antoine Poinsot)
Pull request description:
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.
ACKs for top commit:
achow101:
ACK 6c7a17a8e0eec377f83ed1399f003ae70b898270
sipa:
utACK 6c7a17a8e0eec377f83ed1399f003ae70b898270 (to the extent that it's not my own code).
Tree-SHA512: a71ec002aaf66bd429012caa338fc58384067bcd2f453a46e21d381ed1bacc8e57afb9db57c0fb4bf40de43b30808815e9ebc0ae1fbd9e61df0e7b91a17771cc
|
|
As in title, these docstrings look incorrect.
|
|
Preimages must be externally provided (typically, via a PSBT).
|
|
|
|
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.
|
|
|
|
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
|
|
This introduces the logic to "sign for" a Miniscript.
Co-Authored-By: Pieter Wuille <pieter.wuille@gmail.com>
|
|
|
|
6879be691bf636a53208ef058f2ebe18bfa8017c refactor: Extract RIPEMD160 (Ben Woosley)
Pull request description:
To directly return a CRIPEMD160 hash from data.
Simplifies the call sites.
ACKs for top commit:
achow101:
ACK 6879be691bf636a53208ef058f2ebe18bfa8017c
theStack:
re-ACK 6879be691bf636a53208ef058f2ebe18bfa8017c
MarcoFalke:
review ACK 6879be691bf636a53208ef058f2ebe18bfa8017c 🏔
Tree-SHA512: 6ead85d8060c2ac6afd43ec716ff5a82d6754c4132fe7df3b898541fa19f1dfd8b301b2b66ae7cb7594b1b1a8c7f68bce3790a8c610d4a1164e995d89bc5ae34
|
|
To directly return a CRIPEMD160 hash from data.
Incidentally, decoding this acronym:
* RIPEMD -> RIPE Message Digest
* RIPE -> RACE Integrity Primitives Evaluation
* RACE -> Research and Development in Advanced Communications Technologies in Europe
|
|
in headers
1308b837dc3499896ca73eafa51ac69b455cef00 clang-tidy: Fix `performance-no-automatic-move` in headers (Hennadii Stepanov)
0a5dc030b92a78147787f158d6a5de234ffa8ba4 clang-tidy: Fix `performance-move-const-arg` in headers (Hennadii Stepanov)
Pull request description:
Split from bitcoin/bitcoin#26705 as was requested in https://github.com/bitcoin/bitcoin/pull/26705#issuecomment-1353293405.
To test this PR, consider applying a diff as follows:
```diff
--- a/src/.clang-tidy
+++ b/src/.clang-tidy
@@ -1,16 +1,7 @@
Checks: '
-*,
-bugprone-argument-comment,
-bugprone-use-after-move,
-misc-unused-using-decls,
-modernize-use-default-member-init,
-modernize-use-nullptr,
-performance-for-range-copy,
performance-move-const-arg,
performance-no-automatic-move,
-performance-unnecessary-copy-initialization,
-readability-redundant-declaration,
-readability-redundant-string-init,
'
WarningsAsErrors: '
bugprone-argument-comment,
@@ -28,4 +19,4 @@ readability-redundant-string-init,
CheckOptions:
- key: performance-move-const-arg.CheckTriviallyCopyableMove
value: false
-HeaderFilterRegex: './qt'
+HeaderFilterRegex: '.'
```
ACKs for top commit:
fanquake:
ACK 1308b837dc3499896ca73eafa51ac69b455cef00
Tree-SHA512: b7ef9a3e789846130ab4c3fd6fbe8d887bdbcd438e4cbc78e2b1ac01f819ae13d7f69c2a25f480bd36e3e7f58886a7d5a8609a3c3275c315e0697cd4010474bd
|
|
scripts until the tapleaf version is known
dee89438b82e94474ebaa31367035f98b4636dac Abstract out ComputeTapbranchHash (Russell O'Connor)
8e3fc9942729716e95907008fcf36eee758c3a6a Do not use CScript for tapleaf scripts until the tapleaf version is known (Russell O'Connor)
Pull request description:
While BIP-341 calls the contents of tapleaf a "script", only in the case that the tapleaf version is `0xc0` is this script known to be a tapscript. Otherwise the tapleaf "script" is simply an uninterpreted string of bytes.
This PR corrects the issue where the type `CScript` is used prior to the tapleaf version being known to be a tapscript. This prevents `CScript` methods from erroneously being called on non-tapscript data.
A second commit abstracts out the TapBranch hash computation in the same manner that the TapLeaf computation is already abstracted. These two abstractions ensure that the TapLeaf and TapBranch tagged hashes are always constructed properly.
ACKs for top commit:
ajtowns:
ACK dee89438b82e94474ebaa31367035f98b4636dac
instagibbs:
ACK dee89438b82e94474ebaa31367035f98b4636dac
achow101:
ACK dee89438b82e94474ebaa31367035f98b4636dac
sipa:
ACK dee89438b82e94474ebaa31367035f98b4636dac
aureleoules:
reACK dee89438b82e94474ebaa31367035f98b4636dac - I verified that there is no behavior change.
Tree-SHA512: 4a1d37f3e9a1890e7f5eadcf65562688cc451389581fe6e2da0feb2368708edacdd95392578d8afff05270d88fc61dce732d83d1063d84d12cf47b5f4633ec7e
|
|
See https://clang.llvm.org/extra/clang-tidy/checks/performance/no-automatic-move.html
|