6a371b70c87ad6b763c89384562fce8549f37434 gui: Avoid pathological QT text/markdown behavior... (David Gumberg)
Pull request description:
...during text selection by only setting plaintext mime data.
Fixes the OOM described in #887.
The issue is related to the construction of the [`text/markdown`](b617d11765/src/widgets/widgets/qwidgettextcontrol.cpp (L3539)) MIME data for the selection. Using the `heaptrack` utility, I observed that nearly all of the allocations when reproducing happen in [`QTextMarkdownWriter::writeFrame`](b617d11765/src/gui/text/qtextmarkdownwriter.cpp (L95)). I am not 100% sure what is causing this issue in QT's conversion of our HTML to markdown; I have tried changing the [HTML tags](689a321976/src/qt/rpcconsole.cpp (L916-L924)) (e.g. using `<p></p`> and `<ul><li></li></ul>` in place of tables) used in our `rpcconsole` messages, but the issue recurs.
The solution applied here is to override `createMimeDataFromSelection()` to avoid construction of the (likely never-used anyways) `text/markdown` mime data, and only set plaintext mime data in the clipboard.
ACKs for top commit:
hebasto:
ACK 6a371b70c87ad6b763c89384562fce8549f37434.
Tree-SHA512: 3edc4da47e6dbe939f27664d2265376938eed4f83ded3706e4b73677eac5c9a4ba8819f241428b45a08e8834982ee7759ee096afd090586db3b523d0ccbbbf73
be776a1443fdf1a72e0d363c1566d71cb0cda8b5 wallet: Remove isminetype (Ava Chow)
009a69a616cf30217f6583e45cc049895b9e10d2 wallet: Remove ISMINE_USED (Ava Chow)
6a7aa015747e2634fe5a4b2f7fa0d104eb75c796 wallet: Remove COutput::spendable and AvailableCoinsListUnspent (Ava Chow)
620abe985e5150c3151192d08746b7845a69dbbf interfaces, gui: Remove is_mine output parameter from getAddress (Ava Chow)
Pull request description:
The remaining isminetypes are `ISMINE_SPENDABLE` and `ISMINE_USED`.
`ISMINE_USED` is only used as a filter for caching balances and is never actually returned from `IsMine`. Since we do still want this behavior, This PR changes the caching to utilize bools and explicit members variables to account for the avoid_reuse case. This allows us to remove `ISMINE_USED`.
`ISMINE_SPENDABLE` and `ISMINE_NO` are the only things that are returned by `IsMine`. This is a bool, so it can be replaced as such.
After removing `ISMINE_USED` and `ISMINE_SPENDABLE`, we are able to remove isminetypes altogether.
ACKs for top commit:
murchandamus:
ACK be776a1443fdf1a72e0d363c1566d71cb0cda8b5
fjahr:
reACK be776a1443fdf1a72e0d363c1566d71cb0cda8b5
davidgumberg:
crACK be776a1443
enirox001:
re-ACK be776a1
jlest01:
reACK be776a1443
Tree-SHA512: 689759f6a6ba20a1ae988b0c3abacb15424844f29a1ec2fcb2d1ca9d87b44ae68313e8f61d6fd310281b681144f0ade67e90fcfab807e982b52ed99441d9c987
Since the only remaining isminetypes are ISMINE_NO and ISMINE_SPENDABLE,
this enum is now just a bool and can be removed. IsMine is changed to
return a bool and any usage of isminetypes and isminefilters are changed
to be the remaining ISMINE_SPENDABLE case.
This change avoids hardcoding the "debug.log" filename, ensuring
compatibility with custom filenames provided via the `-debuglogfile`
command-line option.
Using `file(GLOB)` in the generates step is discouraged because the
globbing result may be out of date when the target is built.
Performing the globbing in a script that is executed as the build
target means the result is always reproducable and the overhead
of globbing is only paid when used.
As a follow up, the dependency on `sed` may be removed by performing
the replacement with cmake. Also, the logic from extract_strings_qt.py
can be migrated to cmake.
de0675f9de5feae1f070840ad7218b1378fb880b refactor: Move `transaction_identifier.h` to primitives (marcofleon)
6f068f65de17951dc459bc8637e5de15b84ca445 Remove implicit uint256 conversion and comparison (marcofleon)
9c24cda72edb2085edfa75296d6b42fab34433d9 refactor: Convert remaining instances from uint256 to Txid (marcofleon)
d2ecd6815d89c9b089b55bc96fdf93b023be8dda policy, refactor: Convert uint256 to Txid (marcofleon)
f6c0d1d23128f742dfdda253752cba7db9bb0679 mempool, refactor: Convert uint256 to Txid (marcofleon)
aeb0f783305c923ee7667c46ca0ff7e1b96ed45c refactor: Convert `mini_miner` from uint256 to Txid (marcofleon)
326f24472487dc7f447839136db2ccf60833e9a2 refactor: Convert RPCs and `merkleblock` from uint256 to Txid (marcofleon)
49b3d3a92a7250e80c56ff8c351cf1670e32c1a2 Clean up `FindTxForGetData` (marcofleon)
Pull request description:
This is the final leg of the [type safety refactor](https://github.com/bitcoin/bitcoin/pull/32189).
All of these changes are straightforward `uint256` --> `Txid` along with any necessary explicit conversions. Also, `transaction_identifier.h` is moved to primitives in the last commit, as `Txid` and `Wtxid` become fundamental types after this PR.
ACKs for top commit:
stickies-v:
re-ACK de0675f9de5feae1f070840ad7218b1378fb880b, no changes since a20724d926d5844168c6a13fa8293df8c8927efe except address review nits.
janb84:
re ACK de0675f9de5feae1f070840ad7218b1378fb880b
dergoegge:
re-ACK de0675f9de5feae1f070840ad7218b1378fb880b
theStack:
Code-review ACK de0675f9de5feae1f070840ad7218b1378fb880b
Tree-SHA512: 2413160fca7ab146a8d79d18ce3afcf7384cacc73c513d41928904aa453b4dd7a350064cee71e9c5d015da5904c7c81ac17603e50a47441ebc5b0c653235dd08
Moves the file from `src/util` to `src/primitives`. Now that the
refactor is complete, Txid and Wtxid are fundamental types, so it
makes sense for them to reside in `src/primitives`.
656e16aa5e65731a61b77444ac3ab874f7635c50 qt: Update the `src/qt/locale/bitcoin_en.xlf` translation source file (Hennadii Stepanov)
a0eaa4492548800ba1b2cdd8232195ab5d5c49c7 Fix typos (Hennadii Stepanov)
8d4aaaec49c056e5f20dbc6756369ddcecbc325f Update Transifex slug for 30.x (Hennadii Stepanov)
Pull request description:
This PR follows our [Release Process](53a996f122/doc/release-process.md).
It is required to open Transifex translations for v30.0, as scheduled in https://github.com/bitcoin/bitcoin/issues/32275.
For reference, see the previous similar PR: https://github.com/bitcoin/bitcoin/pull/31809.
**Note for reviewers:**
To reproduce the diff in the last commit, run:
```
cmake --preset dev-mode
cmake --build build_dev_mode --target translate
```
ACKs for top commit:
laanwj:
Code review ACK 656e16aa5e65731a61b77444ac3ab874f7635c50
stickies-v:
ACK 656e16aa5e65731a61b77444ac3ab874f7635c50 , was able to reproduce
Tree-SHA512: 403b534329755079584fcdf98b696e3e75952dfc8d069f305843dbfa85de95f6816ee1d5dfc9b553c7c7f52cc296cb8d3cb03207051d26e0e76ff30d377f49e4
f49840dd902cd9b14b6aadb431b16a4aeb719c3f doc: Fix typo in files.md (Ryan Ofsky)
f5cf0b1ccc8fd426135809a8a4becdae2d797bb5 bitcoin wrapper: improve help output (Ryan Ofsky)
c810b168b89dc07017e9feaec1a8746a449a60b1 doc: Add description of installed files to files.md (Ryan Ofsky)
94ffd01a0294afbe045f1b17a77e4a3caf21e674 doc: Add release notes describing libexec/ binaries (Ryan Ofsky)
cd97905ebc564b8b095099a28d1d5437951927c4 cmake: Move internal binaries from bin/ to libexec/ (Ryan Ofsky)
Pull request description:
This change moves binaries that are not typically invoked directly by users from the `bin/` directory to the `libexec/` directory in CMake installs and binary releases. The goal of the PR is to introduce a distinction between internal and external binaries so starting with #31802, we can use IPC to implement features in new binaries without adding those binaries to the CLI. The change also helps reduce clutter in `bin/`, making it easier for users to identify useful tools to run. Summary of changes:
- For **source builds** (i.e. developer builds) — There are no changes.
- For **source installs** (i.e. `cmake --install` result) — `test_bitcoin`, `test_bitcoin-qt`, and `bench_bitcoin` are installed in `${CMAKE_PREFIX_PATH}/libexec` instead of `${CMAKE_PREFIX_PATH}/bin`, so they are no longer on the system `PATH`. However, they can still be invoked from the `libexec/` directory, or from the CLI as `bitcoin test`, `bitcoin test-gui`, and `bitcoin bench`, respectively.
- For **binary releases** — Since `test_bitcoin` is the only test binary enabled in releases, the only change is moving `test_bitcoin` from `bin/` to `libexec/`.
<details><summary>Details</summary>
<p>
The table below shows the install location of each binary after this change, and the availability of each binary.
| Binary | Location | Availability | Change |
|----------------------|--------------|----------------------|-------------------------------|
| `bitcoin` | `bin/` | 📦 Binary release (since #31375) | Unchanged |
| `bitcoin-cli` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoind` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-qt` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-tx` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-util` | `bin/` | 📦 Binary release | Unchanged |
| `bitcoin-wallet` | `bin/` | 📦 Binary release | Unchanged |
| `bench_bitcoin` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
| `bitcoin-chainstate` | `libexec/` | 🛠 Source build only | Newly installed (was built) |
| `bitcoin-gui` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `bitcoin-node` | `libexec/` | 🛠 Source build only (until #31802) | Moved from `bin/` |
| `test_bitcoin` | `libexec/` | 📦 Binary release | Moved from `bin/` |
| `test_bitcoin-qt` | `libexec/` | 🛠 Source build only | Moved from `bin/` |
</p>
</details>
---
This PR is part of the [process separation project](https://github.com/bitcoin/bitcoin/issues/28722).
ACKs for top commit:
l0rinc:
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
Sjors:
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
achow101:
ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
janb84:
re ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
BrandonOdiwuor:
Tested ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
hodlinator:
re-ACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
willcl-ark:
utACK f49840dd902cd9b14b6aadb431b16a4aeb719c3f
Tree-SHA512: 858a2e1a53db11ee3c5c759bfdeea566f242b9ce5e8a898fa435222e41662b8184577c0dc2c4c058294b4de41d8cb3ba3e5d24c748c280efa4a3f84e3ec4344d
The translations for the following languages, which appear to be the
result of a mistake or an act of vandalism, have been discarded:
- Greek (el)
- Vietnamese (vi)
This change offers a few advantages, such as:
- a more readable and cleaner `ts_files.cmake` (see the next commit);
- a scoped `ts_files` variable;
- improved code locality;
- no need to adjust the location of the resulting `*.qm` files.
94931656b52fddb7c03b29685844d85c008cf6ca cmake: Use newer signature of `qt6_add_lrelease` when available (Hennadii Stepanov)
Pull request description:
See Qt docs here: https://doc.qt.io/qt-6/qtlinguist-cmake-qt-add-lrelease.html.
Fixes https://github.com/bitcoin/bitcoin/issues/32710.
ACKs for top commit:
fanquake:
ACK 94931656b52fddb7c03b29685844d85c008cf6ca
Tree-SHA512: bf0320306967164374499dd0be122473799e830fdff5e070ef13f87af3c14a3b799d90afb423881edd7eea17c13d27af8ced381bbb3cd149353b31b3990dde67
For some reason, building x86_64-w64-mingw32 on x86_64 and aarch64
results in a single instruction difference which can be traced down to
prevector.h:174. The ultimate caller of this is the copy constructor for
a prevector that ends up being called by std::vector::push_back in
walletmodel.cpp:183. By replacing the push_back with an emplace_back,
somehow this non-determinism goes away.
fcfd3db563e89fd79820a4cdfa102d624d801de1 remove RPCTimerInterface and RPCRunLater (Matthew Zipkin)
8a1765795fd3bff79d790102ca7cefa8fd9b204c use WalletContext scheduler for walletpassphrase callback (Matthew Zipkin)
Pull request description:
This removes the dependency on libevent for events scheduled by RPC commands, like re-locking a wallet some time after decryption with walletpassphrase. Since walletpassphrase is currently the only RPC that does this, `RPCRunLater`, `RPCTimerInterface` and all related methods are left unused, and deleted in the second commit. Any future RPC that needs to execute a callback in the future can follow the pattern in this PR and just use a scheduler from node or wallet context.
This is an alternative approach to #32796, described in https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3014309449
ACKs for top commit:
fjahr:
Code Review ACK fcfd3db563e89fd79820a4cdfa102d624d801de1
achow101:
ACK fcfd3db563e89fd79820a4cdfa102d624d801de1
furszy:
ACK fcfd3db563e89fd79820a4cdfa102d624d801de1
Tree-SHA512: 04f5e9c3f73f598c3d41d6e35bb59c64c7b93b03ad9fce3c40901733147ce7764f41f475fef1527d44af18f722759996a31ca83b48cb52153795d5022fecfd14
67dc7523f3e103c8359b546d38f28c1feb2b9b34 cmake, test: Disable tests instead of ignoring them (Hennadii Stepanov)
bb9157db5d3920d971e365e416de4e23866c715a cmake, refactor: Switch to `Python3::Interpreter` imported target (Hennadii Stepanov)
Pull request description:
This PR:
1. Switches to a modern CMake approach by using the `Python3::Interpreter` imported target, which is more robust than using variables.
2. Disables the `util_rpcauth_test` test explicitly instead of silently ignoring it.
A build and test log for the case when Python is unavailable is provided below:
```
$ cmake -B build
$ cmake --build build -j 16
$ ctest --test-dir build -j $(nproc) -R "^util"
Internal ctest changing into directory: /bitcoin/build
Test project /bitcoin/build
Start 115: util_tests
Start 117: util_trace_tests
Start 114: util_string_tests
Start 116: util_threadnames_tests
Start 1: util_rpcauth_test
1/5 Test #1: util_rpcauth_test ................***Not Run (Disabled) 0.00 sec
2/5 Test #114: util_string_tests ................ Passed 0.11 sec
3/5 Test #117: util_trace_tests ................. Passed 0.11 sec
4/5 Test #116: util_threadnames_tests ........... Passed 0.11 sec
5/5 Test #115: util_tests ....................... Passed 0.13 sec
100% tests passed, 0 tests failed out of 4
Total Test time (real) = 0.13 sec
The following tests did not run:
1 - util_rpcauth_test (Disabled)
```
ACKs for top commit:
purpleKarrot:
ACK 67dc7523f3e103c8359b546d38f28c1feb2b9b34
janb84:
tACK 67dc7523f3e103c8359b546d38f28c1feb2b9b34
Tree-SHA512: 5fc7ebe31ac03f4b8a53ecfcfc1cace0f647a1d2c989651988edae96bdfbbe2dee171714e57cb028e65ead1bb40806a82d9821746451dbf005538601fd33ea88
b78990734621b8fe46c68a6e7edaf1fbd2f7d351 wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet (furszy)
e86d71b749c08bde6002b9aa2baee824975a518a wallet: refactor, dedup wallet re-loading code (furszy)
1de423e0a08bbc63eed36c8772e9ef8b48e80fb8 wallet: introduce method to return all db created files (furszy)
d04f6a97ba9a55aa9455e1a805feeed4d630f59a refactor: remove sqlite dir path back-and-forth conversion (furszy)
Pull request description:
Currently, the migration process creates a brand-new descriptor wallet with no
connection to the user's legacy wallet when the legacy wallet lacks key material
and contains only watch-only scripts. This behavior is not aligned with user
expectations. If the legacy wallet contains only watch-only scripts, the migration
process should only generate a watch-only wallet instead.
TODO List:
* Explain that `migratewallet` renames the watch-only after migration, and
also that the wallet will not have keys enabled.
ACKs for top commit:
achow101:
ACK b78990734621b8fe46c68a6e7edaf1fbd2f7d351
pablomartin4btc:
tACK b78990734621b8fe46c68a6e7edaf1fbd2f7d351
rkrux:
LGTM ACK b78990734621b8fe46c68a6e7edaf1fbd2f7d351
Tree-SHA512: 1d583ac4b206fb477e9727daf4b5ad9c3e18b12d40e1ab4a61e8565da44c3d0327c892b51cf47b4894405d122e414cefb6b6366c357e02a74a7ca96e06762d83
173394d9511ed091e73eb12cb28f819026c09576 depends: Build `qt` package for FreeBSD hosts (Hennadii Stepanov)
Pull request description:
This PR continues the work started in https://github.com/bitcoin/bitcoin/pull/23948.
Here is an excerpt from the log:
```
$ ./build/bin/bitcoin-qt -printtoconsole
2025-06-12T01:06:56Z Bitcoin Core version v29.99.0-15de25ba2a28 (release build)
2025-06-12T01:06:56Z Qt 6.7.3 (static), plugin=xcb
2025-06-12T01:06:56Z Static plugins:
2025-06-12T01:06:56Z QMinimalIntegrationPlugin, version 395008
2025-06-12T01:06:56Z QXcbIntegrationPlugin, version 395008
2025-06-12T01:06:56Z Style: fusion / QFusionStyle
2025-06-12T01:06:56Z System: FreeBSD 14.3-RELEASE, x86_64-little_endian-lp64
```
And here are the screenshots:
ACKs for top commit:
vasild:
ACK 173394d9511ed091e73eb12cb28f819026c09576
Tree-SHA512: 42a0bd11e4ef1a23efcfe6c4ab179dc667a076e65060891ce8358b3fe78de4e3ea33f975387d4236cc2ac620e2935b0a29c278065a47f038c66658106bf36755
fa9ca13f35be0a023aeed78775ad66f95717b28b refactor: Sort includes of touched source files (MarcoFalke)
facb152697b8d7b75a9e6108f8896f774b06b35f scripted-diff: Bump copyright headers after include changes (MarcoFalke)
fae71d30f7227594e2f59499cf7f7f9420284e04 clang-tidy: Apply modernize-deprecated-headers (MarcoFalke)
Pull request description:
Bitcoin Core is written in C++, so it is confusing to sometimes use the deprecated C headers (with the `.h` extension). For example, it is less clear whether `string.h` refers to the file in this repo or the cstring stdlib header (https://github.com/bitcoin/bitcoin/pull/31308#discussion_r2121492797).
The check is currently disabled for headers, to exclude subtree headers.
ACKs for top commit:
l0rinc:
ACK fa9ca13f35be0a023aeed78775ad66f95717b28b
achow101:
ACK fa9ca13f35be0a023aeed78775ad66f95717b28b
janb84:
ACK fa9ca13f35be0a023aeed78775ad66f95717b28b
stickies-v:
ACK fa9ca13f35be0a023aeed78775ad66f95717b28b
Tree-SHA512: 6639608308c598d612e24435aa519afe92d71b955874b87e527245291fb874b67f3ab95d3a0a5125c6adce5eb41c0d62f6ca488fbbfd60a94f2063d734173f4d
Historically, the headers have been bumped some time after a file has
been touched. Do it now to avoid having to touch them again in the
future for that reason.
-BEGIN VERIFY SCRIPT-
sed -i --regexp-extended 's;( 20[0-2][0-9])(-20[0-2][0-9])? The Bitcoin Core developers;\1-present The Bitcoin Core developers;g' $( git show --pretty="" --name-only HEAD~0 )
-END VERIFY SCRIPT-
This can be reproduced according to the developer notes with something
like
( cd ./src/ && ../contrib/devtools/run-clang-tidy.py -p ../bld-cmake -fix -j $(nproc) )
Also, the header related changes were done manually.
This change moves binaries that are not typically invoked directly by users
from the `bin/` directory to the `libexec/` directory in CMake installs and
binary releases. The goal is to simplify the contents of `bin/` for end users
while still making all binaries available when needed. After this change, the
binaries remaining in `bin/` are:
- bitcoin
- bitcoin-cli
- bitcoind
- bitcoin-qt
- bitcoin-tx
- bitcoin-util
- bitcoin-wallet
And the binaries that are moved to `libexec/` are:
- bench_bitcoin
- bitcoin-chainstate(*)
- bitcoin-gui(***)
- bitcoin-node(***)
- test_bitcoin(**)
- test_bitcoin-qt
(*) bitcoin-chainstate was previously missing an install rule and was actually
not installed even when it was enabled.
(**) test_bitcoin is the only libexec/ binary that is currently included in
bitcoin binary releases. The others are only installed when building from
source with relevant cmake options enabled.
(***) bitcoin-node and bitcoin-gui are not currently built by default or
included in binary releases but both of these changes are planned and
implemented in #31802
Currently, the migration process creates a brand-new descriptor wallet with no
connection to the user's legacy wallet when the legacy wallet lacks key material
and contains only watch-only scripts. This behavior is not aligned with user
expectations. If the legacy wallet contains only watch-only scripts, the migration
process should only generate a watch-only wallet instead.
Instead of allowing users to load a legacy wallet from the "Open Wallet"
menu, show the legacy wallet greyed out with a message that the wallet
needs to be migrated.
785e1407b0a39fef81a7b25554aab88d4cecd66b wallet: Use util::Error throughout AddWalletDescriptor (Ava Chow)
Pull request description:
#32023 changed `AddWalletDescriptor` to return `util::Error`, but did not change all of the failure cases to do so. This may result in some callers continuing when there was actually an error. Unify all of the failure cases to use `util::Error` so that all callers handle `AddWalletDescriptor` errors in the same way.
The encapsulated return type is changed from `ScriptPubKeyMan*` to `std::reference_wrapper<DescriptorScriptPubKeyMan>`. This avoids having a value that can be interpreted as a bool, and also removes the need to constantly dynamic_cast the returned value. The only kind of `ScriptPubKeyMan` that can come out of `AddWalletDescriptor` is a `DescriptorScriptPubKeyMan` anyways.
ACKs for top commit:
Sjors:
utACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
ryanofsky:
Code review ACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
furszy:
Code review ACK 785e1407b0a39fef81a7b25554aab88d4cecd66b
Tree-SHA512: 52a48263c8d4161a8c0419b7289c25b0986f8e3bcd10b639eeeb0b6862d08b6c5e70998d20070ab26b39ecd90ab83dc8b71c65d85f70626282cf8cc6abff50e7
e8661aac752eb08fee318eb8f56e599578d78f9f wallet: drop watch-only things from interface (Sjors Provoost)
e99188e7daa19dcb14f387931949edf581ae5f37 qt: drop unused watch-only functionality (Sjors Provoost)
Pull request description:
The watch-only functionality in the GUI was only used for legacy wallets. Watch-only descriptor wallets do not use this.
The only visible changes of this PR should be:
- dropped "Spendable:" label from the overview tab
- column width cache is reset
This PR also removes some unused variables from the interface.
ACKs for top commit:
davidgumberg:
Review ACK e8661aac75.
hebasto:
ACK e8661aac752eb08fee318eb8f56e599578d78f9f, I have reviewed the code and it looks OK. The `src/qt/forms/overviewpage.ui` form was reviewed in Qt Designer.
Tree-SHA512: d7edb0f167e0b934075398a76eddca69890bb36848a918c932b1c2cea85ee87285e876cbfdf1f6dec7adf26b9f405fb558c70bec0c84585c0a9df33c2af78393
ee045b61efc1479c1866b786661ef39a863677d0 rpc, psbt: Require sighashes match for descriptorprocesspsbt (Ava Chow)
2b7682c3729d4e054ac4260b344a75ad4b7239b3 psbt: use sighash type field to determine whether to remove non-witness utxos (Ava Chow)
28781b5f06709212934c521c513bb2e1a521a31f psbt: Add sighash types to PSBT when not DEFAULT or ALL (Ava Chow)
15ce1bd73f80e998f7402433572b695f589f7f42 psbt: Enforce sighash type of signatures matches psbt (Ava Chow)
1f71cd337ad75390a1f8810d6715f3634ed07e98 wallet: Remove sighash type enforcement from FillPSBT (Ava Chow)
4c7d767e49b2e709a2b00af92ca76e9f30e47aec psbt: Check sighash types in SignPSBTInput and take sighash as optional (Ava Chow)
a11825694856a2643e9600fa537182fbb597c107 script: Add IsPayToTaproot() (Ava Chow)
d6001dcd4ada5b64c8113450ed736a2581c97518 wallet: change FillPSBT to take sighash as optional (Ava Chow)
e58b680923b10f0690de9dcd34f17fbb8d6de5eb psbt: Return PSBTError from SignPSBTInput (Ava Chow)
2adfd815325713d64b9daa61c2f93061d27bd47d tests: Test PSBT sighash type mismatch (Ava Chow)
5a5d26d6123e0056656e406cd9f35aac6f71df4b psbt: Require ECDSA signatures to be validly encoded (Ava Chow)
Pull request description:
Currently, we do not add the sighash field to PSBTs at all, even when we have signed with a non-default sighash. This PR changes the behavior such that when we (attempt to) sign with a sighash other than DEFAULT or ALL, the sighash type field will be added to the PSBT to inform the later signers that a different sighash type was used by a signer. Notably, this is necessary for MuSig2 support as all signers must sign using the same sighash type, but the sighash is not provided in partial signatures.
Furthermore, because the sighash type can also be provided on the command line, we require that if both a command line sighash type and the sighash field is present, they must specify the same sighash type. However, this was being checked by the wallet, rather than the signing code, so the `descriptorprocesspsbt` RPC was not enforcing this restriction at all, and in fact ignored the sighash field entirely. This PR refactors the checking code so that the underlying PSBT signing function `SignPSBTInput` does the check.
ACKs for top commit:
theStack:
re-ACK ee045b61efc1479c1866b786661ef39a863677d0
rkrux:
re-ACK ee045b61efc1479c1866b786661ef39a863677d0
fjahr:
Code review ACK ee045b61efc1479c1866b786661ef39a863677d0
Tree-SHA512: 4ead5be1ef6756251b827f594beba868a145d75bf7f4ef6f15ad21f0ae4b8d71b38c83494e5a6b75f37fadd097178cddd93d614b962a2c72fc134f00ba2f74ae
7193245cd66791216d4e586ca09302b26d4b7528 doc: remove For ... comments (fanquake)
1b9cdc933f6c11cb8593b5ad9ae3f4eb2c726859 net: drop win32 ifdef (fanquake)
19ba499b1f3884ea69c5f833f3eb797f90372aa5 init: cerrno is used on all platforms (fanquake)
Pull request description:
We don't add or maintain these, and they are of little value, as
well as having the effect of polluting diffs, if changed.
They are also wrong, i.e `DEFAULT_SCRIPTCHECK_THREADS` is not in
`validation.h`.
ACKs for top commit:
stickies-v:
re-ACK 7193245cd66791216d4e586ca09302b26d4b7528
fjahr:
ACK 7193245cd66791216d4e586ca09302b26d4b7528
willcl-ark:
reACK 7193245cd66791216d4e586ca09302b26d4b7528
Tree-SHA512: 6b5f83cd1df699356e1cbb78949f8d456b13ce288f0064138118cfb45b4c77e2d1945babe91598dffe9823ab07dfae36f4c3b61c586cf98baf16890bdf322b08
32023 changed AddWalletDescriptor to return util::Error, but did not
change all of the failure cases to do so. This may result in some
callers continuing when there was actually an error. Unify all of the
failure cases to use util::Error so that all callers handle
AddWalletDescriptor errors in the same way.
The encapsulated return type is changed from ScriptPubKeyMan* to
std::reference_wrapper<DescriptorScriptPubKeyMan>. This avoids having a
value that can be interpreted as a bool, and also removes the need to
constantly dynamic_cast the returned value. The only kind of
ScriptPubKeyMan that can come out of AddWalletDescriptor is a
DescriptorScriptPubKeyMan anyways.
We don't add or maintain these, and they are of little value, as
well as having the effect of polluting diffs.
They are also wrong, i.e DEFAULT_SCRIPTCHECK_THREADS is not in
validation.h.
