| Age | Commit message (Collapse) | Author |
|---|
|
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
|
|
version 0.2.0
202291722300b86f36e97de7960d40a32544c2d1 Add secp256k1_selftest call (Pieter Wuille)
3bfca788b0dae879bfc745cc52c2cb6edc49fd70 Remove explicit enabling of default modules (Pieter Wuille)
4462cb04986d77eddcfc6e8f75e04dc278a8147a Adapt to libsecp256k1 API changes (Pieter Wuille)
9d47e7b71b2805430e8c7b43816efd225a6ccd8c Squashed 'src/secp256k1/' changes from 44c2452fd3..21ffe4b22a (Pieter Wuille)
Pull request description:
Now that libsecp256k1 has a release (https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2022-December/021271.html), update the subtree to match it.
The changes themselves are not very impactful for Bitcoin Core, but include:
* It's no longer needed to specify whether contexts are for signing or verification or both (all contexts support everything), so make use of that in this PR.
* Verification operations can use the static context now, removing the need for some infrastructure in pubkey.cpp to make sure a context exists.
* Most modules are now enabled by default, so we can drop explicit enabling for them.
* CI improvements (in particular, MSVC and more recent MacOS)
* Introduction of an internal int128 type, which has no effect for GCC/Clang builds, but enables 128-bit multiplication in MSVC, giving a ~20% speedup there (but still slower than GCC/Clang).
* Release process changes (process documentation, changelog, ...).
ACKs for top commit:
Sjors:
ACK 202291722300b86f36e97de7960d40a32544c2d1, but 4462cb04986d77eddcfc6e8f75e04dc278a8147a could use more eyes on it.
achow101:
ACK 202291722300b86f36e97de7960d40a32544c2d1
jonasnick:
utACK 202291722300b86f36e97de7960d40a32544c2d1
Tree-SHA512: 8a9fe28852abe74abd6f96fef16a94d5a427b1d99bff4caab1699014d24698aab9b966a5364a46ed1001c07a7c1d825154ed4e6557c7decce952b77330a8616b
|
|
|
|
-BEGIN VERIFY SCRIPT-
./contrib/devtools/copyright_header.py update ./
-END VERIFY SCRIPT-
Commits of previous years:
- 2021: f47dda2c58b5d8d623e0e7ff4e74bc352dfa83d7
- 2020: fa0074e2d82928016a43ca408717154a1c70a4db
- 2019: aaaaad6ac95b402fe18d019d67897ced6b316ee0
|
|
* Use SECP256K1_CONTEXT_NONE when creating signing context, as
SECP256K1_CONTEXT_SIGN is deprecated and unnecessary.
* Use secp256k1_static_context where applicable.
|
|
|
