7f3a6a9495 wallet: Add external-signer-support specific error message (Hennadii Stepanov)
Pull request description:
On master (5f44c5c428) an attempt to load an external signer wallet using Bitcoin Core compiled without external signer support fails with the following log messages:
```
2022-02-20T19:01:11Z [qt-walletctrl] Using SQLite Version 3.31.1
2022-02-20T19:01:11Z [qt-walletctrl] Using wallet /home/hebasto/.bitcoin/testnet3/wallets/coldcard-0220
2022-02-20T19:01:11Z [qt-walletctrl] init message: Loading wallet…
2022-02-20T19:01:11Z [qt-walletctrl] [coldcard-0220] Error: External signer wallet being loaded without external signer support compiled
2022-02-20T19:01:11Z [qt-walletctrl] [coldcard-0220] Releasing wallet
```
While log messages are good, a message in the GUI window is completely misleading:
This PR fixes this issue:
ACKs for top commit:
achow101:
ACK 7f3a6a9495
kristapsk:
ACK 7f3a6a9495
brunoerg:
crACK 7f3a6a9495
Tree-SHA512: a4842751c0ca8a37ccc3ea00503678f6b712a7f53d6cbdc07ce02dcb85ca8a94890d1c2da20307be043faa347747abeba29185c88ba12edd5253bfca56531585
1513727e2b build, qt: (Re-)sign package (Hennadii Stepanov)
c26a0a5af7 build, qt: Align frameworks with macOS codesign tool requirements (Hennadii Stepanov)
Pull request description:
Fixes#22403
This PR follows Apple [docs](https://developer.apple.com/documentation/macos-release-notes/macos-big-sur-11_0_1-universal-apps-release-notes):
> - New in macOS 11 on Macs with Apple silicon, and starting in macOS Big Sur 11 beta 6, the operating system enforces that any executable must be signed before it’s allowed to run. There isn’t a specific identity requirement for this signature: a simple ad-hoc signature is sufficient...
> - ... If you use a custom workflow involving tools that modify a binary after linking (e.g. `strip` or `install_name_tool`) you might need to manually call `codesign` as an additional build phase to properly ad-hoc sign your binary. These new signatures are not bound to the specific machine that was used to build the executable, they can be verified on any other system and will be sufficient to comply with the new default code signing requirement on Macs with Apple silicon...
When building with system Qt frameworks (i.e., without depends), a new string has been added to the `make deploy` log on M1-based macOS:
```
% make deploy
...
+ Generating .DS_Store +
dist/Bitcoin-Qt.app: replacing existing signature
+ Preparing .dmg disk image +
...
```
This PR does not change build system behavior:
- when building with depends
- on Intel-based macOS
ACKs for top commit:
jarolrod:
ACK 1513727e2b
fanquake:
ACK 1513727e2b - although didn't test on M1 hardware. Given the forced signing is scoped to only occur when running the deploy script on macOS, this doesn't interfere with our release signing.
Tree-SHA512: 3aa778fdd6ddb54f029f632f2fe52c2ae3bb197ba564cb776493aa5c3a655bd51d10ccbe6c007372d717e9b01fc4193dd5c29ea0bc7e069dcae7e991ae259f0c
fad7ddf9e3 test: Run symlink regression tests on Windows (MarcoFalke)
Pull request description:
Seems odd to add tests, but not run them on the platform that needs them most.
ACKs for top commit:
laanwj:
Code review ACK fad7ddf9e3
ryanofsky:
Code review ACK fad7ddf9e3, just removing new test. Would be nice if the test could be added later, of course.
Tree-SHA512: 64b235967a38c2eb90657e8d7a0447bcc8ce81d1b75a275b6c48bd42efd9ea7e7939257e484f297ee84598def3738eaeb289561aeba1dd6a99b258d389995139
80e78b6a04 build: pass win32-dll to LT_INIT() (fanquake)
Pull request description:
This is the recommended way to support building PE DLLs with modern mingw
toolchains and libtool. I made a similar change upstream in the secp256k1
repo: https://github.com/bitcoin-core/secp256k1/pull/1022. Note that we already
pass `-no-undefined` to our libtool LDFLAGS.
> This option should be used if the package has been ported to build clean
> dlls on win32 platforms.
> If this macro is not used, libtool will assume that the package libraries
> are not dll clean and will build only static libraries on win32 hosts.
See:
https://www.gnu.org/software/libtool/manual/libtool.html#LT_005fINIThttps://www.gnu.org/software/gnulib/manual/html_node/Libtool-and-Windows.htmlhttps://autotools.io/libtool/windows.htmlhttps://github.com/bitcoin-core/secp256k1/issues/923
Guix Build:
```bash
bash-5.1# find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
a504bac1c81818e2fa7e14d4b1e2ddf665b9b79683fff4390ec1d76335157012 guix-build-80e78b6a0479/output/aarch64-linux-gnu/SHA256SUMS.part
ef9193402c261adb993f6644de3f49858acb6d120002505e0def4827b8772294 guix-build-80e78b6a0479/output/aarch64-linux-gnu/bitcoin-80e78b6a0479-aarch64-linux-gnu-debug.tar.gz
030961da6966a14d3dd7322dc7559dfdc0bdeb9f39d042f379c41bab98303d28 guix-build-80e78b6a0479/output/aarch64-linux-gnu/bitcoin-80e78b6a0479-aarch64-linux-gnu.tar.gz
c485e456d325cdd64111eefe36b007e7bcb9e5eb61b1ab752e601023f5853e55 guix-build-80e78b6a0479/output/arm-linux-gnueabihf/SHA256SUMS.part
ff0481c57a3ab15c9651ead7b77990c37af9360bfbc1f5285ad5f7d0c66f5acc guix-build-80e78b6a0479/output/arm-linux-gnueabihf/bitcoin-80e78b6a0479-arm-linux-gnueabihf-debug.tar.gz
5f582e30bbdba9df175bf4c5d4aae64e2a1cf572086390ae6962d3ee9f0325a9 guix-build-80e78b6a0479/output/arm-linux-gnueabihf/bitcoin-80e78b6a0479-arm-linux-gnueabihf.tar.gz
e96c601af96e851a0351c6f8975feb47623a2dd5e3dd2c15bcdfe8435f845538 guix-build-80e78b6a0479/output/arm64-apple-darwin/SHA256SUMS.part
a50db7a8a9b6415842807644760110f2e01665b922b2762634d94e2b497cbd4a guix-build-80e78b6a0479/output/arm64-apple-darwin/bitcoin-80e78b6a0479-arm64-apple-darwin.tar.gz
0f3707a2423483f84be5edff91f8e657cf71ab097d2550f4369760ac8c6a1644 guix-build-80e78b6a0479/output/arm64-apple-darwin/bitcoin-80e78b6a0479-osx-unsigned.dmg
33252a9895c013cfbea06444d6372a23cc555831e4675705b4d7d6b065f06cff guix-build-80e78b6a0479/output/arm64-apple-darwin/bitcoin-80e78b6a0479-osx-unsigned.tar.gz
2ab70177c80c36e98018d07e2aece084c7d3d604e7dc12d2df2e1a077e06b983 guix-build-80e78b6a0479/output/dist-archive/bitcoin-80e78b6a0479.tar.gz
cc0237b05948472efa61f7d5a666d8e97b5abeb7f498f3f72d46ff69be38bcf4 guix-build-80e78b6a0479/output/powerpc64-linux-gnu/SHA256SUMS.part
b3778fd81bf4e432ad1590792673c91d09c8f8f43daef4cbe0852bca49e1ed57 guix-build-80e78b6a0479/output/powerpc64-linux-gnu/bitcoin-80e78b6a0479-powerpc64-linux-gnu-debug.tar.gz
bebe78f0e6a062d943c99470f12bfc520381acec40e0409915cc8d5dccbe5999 guix-build-80e78b6a0479/output/powerpc64-linux-gnu/bitcoin-80e78b6a0479-powerpc64-linux-gnu.tar.gz
350f7b22562d8b6642f37afb3e192d36dbcb360a361c8b834d0f7d50401667b8 guix-build-80e78b6a0479/output/powerpc64le-linux-gnu/SHA256SUMS.part
9a488fbd71c53092feda8dfccecc4ae7d10aa2efe48f99f150cb2322bb28c5e6 guix-build-80e78b6a0479/output/powerpc64le-linux-gnu/bitcoin-80e78b6a0479-powerpc64le-linux-gnu-debug.tar.gz
06d3c472171124d6ca92f95f7d5cb7fc4a523c25396dbbb9522cab920867d3db guix-build-80e78b6a0479/output/powerpc64le-linux-gnu/bitcoin-80e78b6a0479-powerpc64le-linux-gnu.tar.gz
67d591d5f15933d56046d0b8208970dc812ddd240c14a4c3b635cdc256ae5205 guix-build-80e78b6a0479/output/riscv64-linux-gnu/SHA256SUMS.part
f9a853d703ac153748f3d9f60d4a74a72c75966dc1d3711b688ebd003ff9389c guix-build-80e78b6a0479/output/riscv64-linux-gnu/bitcoin-80e78b6a0479-riscv64-linux-gnu-debug.tar.gz
07554223c5ab3b940f53f9483054023e639d4e9902810b3d5c1875fd390064ea guix-build-80e78b6a0479/output/riscv64-linux-gnu/bitcoin-80e78b6a0479-riscv64-linux-gnu.tar.gz
294dc1274391b17fb750eac7c76e59c18e972ed3fbf8bccd53ba514843fbc59f guix-build-80e78b6a0479/output/x86_64-apple-darwin/SHA256SUMS.part
cc5c1256ca57f80d5ecb93fe2ac477f90945206430545b0463813f7099804f47 guix-build-80e78b6a0479/output/x86_64-apple-darwin/bitcoin-80e78b6a0479-osx-unsigned.dmg
c5ff5cf7a8119981f8a1aa2306ff9e84c60e5c9845836eb2562941801495c7de guix-build-80e78b6a0479/output/x86_64-apple-darwin/bitcoin-80e78b6a0479-osx-unsigned.tar.gz
5951712d82391ba0471f253a7f87701b464dad1a90bb1d91866b0c7c51164a8f guix-build-80e78b6a0479/output/x86_64-apple-darwin/bitcoin-80e78b6a0479-osx64.tar.gz
28f84ed57769a642b1679acf90fdaad7ac489d59598d4a8a859021d39a32d878 guix-build-80e78b6a0479/output/x86_64-linux-gnu/SHA256SUMS.part
f7c37c47ffaec6fbca36c3c4897b2df2fa7dd5327cb860501eec9d9e987ea5c7 guix-build-80e78b6a0479/output/x86_64-linux-gnu/bitcoin-80e78b6a0479-x86_64-linux-gnu-debug.tar.gz
c7c6db897a604e5a85f37938b763538751a133bbab90e80904d4b7198b227d95 guix-build-80e78b6a0479/output/x86_64-linux-gnu/bitcoin-80e78b6a0479-x86_64-linux-gnu.tar.gz
80b1e0e249cefe8941ca0e1a563f479c5e4408da6f0aa02c127182a09310dd8c guix-build-80e78b6a0479/output/x86_64-w64-mingw32/SHA256SUMS.part
e7f8b2cd0f0f465e80d96338dcc398306b321a9c99556ca1d39f094752702a21 guix-build-80e78b6a0479/output/x86_64-w64-mingw32/bitcoin-80e78b6a0479-win-unsigned.tar.gz
5191f309c758135fd597df6bc9ef9e2c2746947abb74b38c32e5b6e073fa0995 guix-build-80e78b6a0479/output/x86_64-w64-mingw32/bitcoin-80e78b6a0479-win64-debug.zip
1569de943ca054841141c700f1d4fca2658228b85eee1f44d201b0c881218ef0 guix-build-80e78b6a0479/output/x86_64-w64-mingw32/bitcoin-80e78b6a0479-win64-setup-unsigned.exe
c8d78aeedeeaf7af4d325d38fb5e2307965b0080ce08a8cde802afaaa73f157d guix-build-80e78b6a0479/output/x86_64-w64-mingw32/bitcoin-80e78b6a0479-win64.zip
```
ACKs for top commit:
hebasto:
ACK 80e78b6a04
Tree-SHA512: fb4a6a443288723776491a9795429273b4a454cfd8230e75570d44fcd71037dc784a2061f6a979322ebc8f9b4131dfbb0494146ab3863f94829e72922be4ec07
9999f891d1 bench: Avoid deprecated use of volatile += (MarcoFalke)
Pull request description:
Deprecated in C++20 according to https://eel.is/c++draft/expr.ass#6 .
```
bench/examples.cpp:16:13: warning: compound assignment with ‘volatile’-qualified left operand is deprecated [-Wvolatile]
16 | sum += sin(d);
| ~~~~^~~~~~~~~
```
While C++20 is currently unsupported, I don't see any downside to a minor fixup to an example benchmark. This will also make a hypothetical C++20 patch smaller.
ACKs for top commit:
fanquake:
ACK 9999f891d1
Tree-SHA512: ca7d660fa8eba347a4648408a8b97a0ecb8263a825da7abd59129d783058102581e05b273667989f95480436a66d5384bd1e92d9ae79408f5b30e2178935cc38
fafc4eb363 test: Fix Wambiguous-reversed-operator compiler warnings (MarcoFalke)
Pull request description:
Add a missing const to avoid the C++20 clang **compiler warning**:
```
test/fuzz/addrman.cpp:325:22: error: ISO C++20 considers use of overloaded operator '==' (with operand types 'AddrManDeterministic' and 'AddrManDeterministic') to be ambiguous despite there being a unique best viable function [-Werror,-Wambiguous-reversed-operator]
assert(addr_man1 == addr_man2);
~~~~~~~~~ ^ ~~~~~~~~~
/usr/include/assert.h:93:27: note: expanded from macro 'assert'
(static_cast <bool> (expr) \
^~~~
test/fuzz/addrman.cpp:140:10: note: ambiguity is between a regular call to this operator and a call with the argument order reversed
bool operator==(const AddrManDeterministic& other)
^
1 error generated.
```
This patch also fixes the **compile error** if the first operand is `const`:
```
test/fuzz/addrman.cpp:326:23: error: invalid operands to binary expression ('const AddrManDeterministic' and 'AddrManDeterministic')
assert(addr_man_1 == addr_man2);
~~~~~~~~~~ ^ ~~~~~~~~~
/usr/include/assert.h:90:27: note: expanded from macro 'assert'
(static_cast <bool> (expr) \
^~~~
test/fuzz/addrman.cpp:140:10: note: candidate function not viable: 'this' argument has type 'const AddrManDeterministic', but method is not marked const
bool operator==(const AddrManDeterministic& other)
^
1 error generated.
ACKs for top commit:
hebasto:
ACK fafc4eb363, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 92cd62ae06ee1393a6dc2ea6f3f553595a8f8d66f51592d231b42122bfb71ed4801a016daafc85360040339c5ae59b76888265cec37449c4688d6c7768f4567e
03bc08e163 doc: Mention missing BIP157 in bips.md (laanwj)
e97e3ded69 doc: Update bips.md for 23.x (laanwj)
Pull request description:
As far as I know, there have been no new bips implemented in this major release. Update `bips.md` accordingly.
(if there are, please post below)
ACKs for top commit:
jonatack:
ACK 03bc08e163
prayank23:
ACK 03bc08e163
Tree-SHA512: d671c37d1aab9f700f9688dabec056acfd2503c83bd3e1612ed1cee4ba92b99db002e34b6f1100b98543fe60f8b757ea2bedcc2ff020243cf30e27b4dd73d04c
5a89bed410 contrib: address gen-manpages feedback from #24263 (fanquake)
2618fb8d15 Output license info when binaries are passed -version (fanquake)
4c3e3c5746 refactor: shift CopyrightHolders() and LicenseInfo() to clientversion.cpp (fanquake)
Pull request description:
Addresses a review comment from #24263, and addresses the [comment](https://github.com/bitcoin/bitcoin/pull/24263#issuecomment-1030582925) where it was pointed out that we are inconsistent with emitting our copyright. After this change, the copyright is always emitted with `-version`, rather than `-help`, i.e:
```bash
bitcoind -version
Bitcoin Core version v22.99.0-fc1f355913f6-dirty
Copyright (C) 2009-2022 The Bitcoin Core developers
Please contribute if you find Bitcoin Core useful. Visit
<https://bitcoincore.org/> for further information about the software.
The source code is available from <https://github.com/bitcoin/bitcoin>.
This is experimental software.
Distributed under the MIT software license, see the accompanying file COPYING
or <https://opensource.org/licenses/MIT>
```
The info is also added to binaries other than `bitcoind`/`bitcoin-qt`. This change also prevents duplicate copyright info appearing in the `bitcoind` man page.
ACKs for top commit:
laanwj:
Tested ACK 5a89bed410
Tree-SHA512: 0ac2a1adf9e9de0c3206f35837008e3f93eaf15b193736203d71609273f0887cca20b8a90972cb9f941ebd62b330d61a0cbb5fb1b7a7f2dbc715ed8a0c1569d9
5b8f2484ba lint: remove no-longer used exceptions from lint-format-strings.py (fanquake)
Pull request description:
ACKs for top commit:
laanwj:
ACK 5b8f2484ba if it passes CI
hebasto:
ACK 5b8f2484ba, I've verified that all of the remained false positive cases are valid.
Tree-SHA512: 25c40714d271c57fb09c963a3372b62c7b4f2e9367517cdf5c73ea82527a9c4c477f8b7857e37adc7eb9feea1f0a37435059798ddf2195dee3522bed3a6eea44
This primarily improves support for external signing, as it includes
multiple bugfixes for Boost Process. As well as various improvements to
the multi-index library.
Consolidate to outputting the licensing info when we pass -version to a binary,
i.e bitcoind -version:
```bash
itcoin Core version v22.99.0-fc1f355913f6-dirty
Copyright (C) 2009-2022 The Bitcoin Core developers
Please contribute if you find Bitcoin Core useful. Visit
<https://bitcoincore.org/> for further information about the software.
The source code is available from <https://github.com/bitcoin/bitcoin>.
This is experimental software.
Distributed under the MIT software license, see the accompanying file COPYING
or <https://opensource.org/licenses/MIT>
```
c5158290af qt: Update translation source file (Hennadii Stepanov)
Pull request description:
As the part of the v23.0 [release process](https://github.com/bitcoin/bitcoin/issues/22969) this PR updates translation source file, `src/qt/locale/bitcoin_en.xlf`.
The following changes are reflected in this PR:
- small string changes from bitcoin-core/gui#509
- internal technical details from bitcoin/bitcoin#22151
ACKs for top commit:
laanwj:
ACK c5158290af
Tree-SHA512: 2cf08f5b356dca25f99b0342645db5253eab0854796cf44fa52f8a6cf28f6d3f973e21589e0f9d3fef40a1b21b3f0aee00c9ca0897109a1967f9ef3320dd508f
48742693ac Replace "can not" with "cannot" in docs, user messages, and tests (Jon Atack)
e670edd434 User-facing content fixups from transifex translator feedback (Jon Atack)
Pull request description:
Closes#24366.
ACKs for top commit:
laanwj:
Code review re-ACK 48742693ac
hebasto:
re-ACK 48742693ac, only suggested change since my previous [review](https://github.com/bitcoin/bitcoin/pull/24367#pullrequestreview-885938219).
Tree-SHA512: 4dcdcb417251a413e65fab6070515e13a1267c8e0dbcf521386b842511391f24c84a0c2168fe13458c977682034466509bf2a3453719d4d94d3c568fd9f4adb4
e50a9be154 Remove outdated comment on CFeeRate (Murch)
Pull request description:
This comment described how the constructor of CFeeRate was previously indirectly used to parse fee rate arguments from RPCs. The command line input was actually in sat/vB but due to the use of AmountFromValue() it got converted to BTC/vB which then got rectified in the constructor by creating a CFeeRate from that given value and COIN as the transaction size. Since this usage pattern was removed from the codebase some months ago, the comment is now obsolete.
ACKs for top commit:
michaelfolkson:
ACK e50a9be154
jonatack:
ACK e50a9be154
Tree-SHA512: f17bf0baeeca85a5c7883edadd407da845f6e3af1c949e93116bd67c02e601682a5f7f1ab2497172472e3acf1c4e3c234b01161a77e7d7f028e3551da34777f0
e7fc50681e qt: Override BitcoinApplication::event() to handle QEvent::Quit (Hennadii Stepanov)
Pull request description:
bitcoin-core/gui#336 introduced a regression when termination requests from a platform are not handled properly.
This PR fixes this regression. On macOS shutdown after clicking "Quit" in Dock icon menu, and during logout works again.
Fixesbitcoin-core/gui#545.
ACKs for top commit:
RandyMcMillan:
tACK e7fc50681e
Sjors:
tACK e7fc50681e (rebased on master) indeed fixes the crash described in #545
promag:
Tested ACK e7fc50681e on macOS 10.15 with Qt 5.15.2.
Tree-SHA512: 236a483dc0828f22999469e133b8ac9f0b6267ec2a27004c3ebaa967689ddb972ea1fa90c1dd41f3bff3d17bf571a707babcef53bd79fd711fda98cfbf120131
ce690847b6 cli: describe quality/recency filtering in -addrinfo (Jon Atack)
7c975614c0 rpc: describe quality/recency filtering in getnodeaddresses (Jon Atack)
Pull request description:
Addresses #24278.
```
$ bitcoin-cli help getnodeaddresses
getnodeaddresses ( count "network" )
Return known addresses, after filtering for quality and recency.
These can potentially be used to find new peers in the network.
The total number of addresses known to the node may be higher.
```
```
$ bitcoin-cli -help | grep -A3 addrinfo
-addrinfo
Get the number of addresses known to the node, per network and total,
after filtering for quality and recency. The total number of
addresses known to the node may be higher.
```
ACKs for top commit:
mzumsande:
Thanks, Code Review ACK ce690847b6
prayank23:
reACK ce690847b6
Tree-SHA512: 82d23b15e64a99411eb8e70d7267a1b4f23182fabe072e824277569d9677e392b466be63f00e3d157d7db94bbe032d53f12ad4ab30b55b7b8a629c37d80d1d8c
87f54060ff doc: Swap gen-manpages and update RC steps in release process (laanwj)
42c202893b doc: Fix gen-manpages, rewrite in Python (laanwj)
Pull request description:
Rewrite the manual page generation script in Python.
This:
- solves '-' stripping issue (fixes#22681)
- makes that a copyright footer is generated correctly again
Also change the release process to swap gen-manpages and update RC steps, so that the pages will have the correct rc and/or final version.
ACKs for top commit:
dongcarl:
Code Review ACK 87f54060ff
fanquake:
ACK 87f54060ff - tested generating and opening the man pages locally, but didn't run through the release process. Will propose some changes to address consolidating the help / version output.
Tree-SHA512: 39254721ca84e4f223a321c554f2e08c36428b15019a0f9fa3eff408b4c6f1e1d74941143f4d2927427afa3ad7a7e6f999d6ec660132d817809b640a87ae9f7d
fa27745ccb ci: Bump fuzz tasks to jammy (MarcoFalke)
fab8cd5f87 Revert "ci: Run fuzzer task for the master branch only" (MarcoFalke)
Pull request description:
This reverts commit 5a9e255e5a.
I think we should attempt to maintain the fuzz tasks for release branches as well.
If it is too difficult for one branch, it could make sense to disable it for that branch, but not for all branches unconditionally.
Also, bump to jammy.
ACKs for top commit:
fanquake:
ACK fa27745ccb - we'll see how we go with the 23.x release branch.
Tree-SHA512: d6d08e7dce0884b556c51ff1896aebbbb5a805c22decd58af81a04192d19876978696017b489ec55886ddfd5c022963baaab5f11022369ae5291016826ff8017
c821ab8be8 Use `GetAllOutputTypes` in `getblock` RPC function (Kiminuo)
d970a85d33 Move `GetAllOutputTypes` function from `rpc/rawtransaction.cpp` to `rpc/util.{h|cpp}` (Kiminuo)
Pull request description:
This PR attempts to replicate 0ccf9b2e55/src/rpc/rawtransaction.cpp (L547) to one other place (at the moment) so that users have better idea what RPC methods can actually return.
I created this PR as a follow-up to the idea mentioned here https://github.com/bitcoin/bitcoin/pull/23320#discussion_r732458112 (resolved).
ACKs for top commit:
kristapsk:
re-ACK c821ab8be8
Tree-SHA512: 5ff66a41ad7c43ec769f4a99933d2d070feea7c617286d94b6f9bfa1a2547a42211915778210a89074ad4b14d99f34852cc6871efed5e6f1e2ffedd40d669386
fa30e62cc6 doc: Rework generate* doc (MarcoFalke)
Pull request description:
Hide the test-only calls and clarify the short description
ACKs for top commit:
0xB10C:
reACK fa30e62cc6. changes since fa3bb584dcc742a767b2141cd7324877e3cf5302 are: dropping the `immediately` + formatting the touched line and a rebase
Tree-SHA512: 07439f39660bbf144c2cc406b6010b64dcdd27150d78654fe04a36a982a519f837a0cf0f030c9f30af69c451ccf7a3b7287a275637aa81904c202029b9efc661
0683f377e1 Add tr() descriptor unit tests (Pieter Wuille)
4b2e31a7ae Bugfix: make ToPrivateString work with x-only keys (Pieter Wuille)
18ad54c3b2 Bugfix: set x-only flag when inferring pk() inside tr() (Pieter Wuille)
Pull request description:
This fixes two bugs in the current logic for `tr()` descriptors:
* ToPrivateString does not always work, because the provided private key may mismatch the parity of the x-only public key.
* The descriptors inferred for `pk()` inside `tr()` have the wrong x-only flag, leading to such descriptors generating the wrong scriptPubKey (roundtripping through ToString does fix it however, so this seems unobservable in the current code).
These were discovered while adding unit tests to descriptor_tests that cover various aspects of `tr()` descriptors, which are now also added here.
ACKs for top commit:
achow101:
ACK 0683f377e1
instagibbs:
ACK 0683f377e1
jonatack:
Code review ACK 0683f377e1
Tree-SHA512: fc0e11b45da53054a108effff2029d67b64e508b160a6e22e00c98b506c39ec12ccc95afd21ea68a6c691eb62930afc7af18908f2fa3a954d102afdc67bc355a
6981de4435 doc: fix wording of alertnotify (willcl-ark)
Pull request description:
The documentation of the `alertnotify` startup option no longer matches the implementation.
Currently the alert is only triggered by `DoWarning` (as part of `CChainstate::UpdateTip` when blocks containing unknown versionbits are detected on the network, indicating that there may be an upcoming softfork which you don't know about), but not when we see a "really long fork":
2825c41a61/src/validation.cpp (L2418-L2433)
I think it would be desirable in a follow-up PR to implement the logic to alert on a (really) long fork, but not to alert for "partition detection" (abnormally slow/fast blocks). `PartitionChecker` code was removed in ab8be98fdb
ACKs for top commit:
josibake:
ACK 6981de4435
achow101:
ACK 6981de4435
Tree-SHA512: ea124f53ca1db803ba93d649f4bc983484c47fb5fe7fa61a8eb32fcbc7425f67d8578e66a6ba70202e13868fe8add0103306dede3b1edd1d3261ffb9c1042b87
62cc138ecb Rename wallet-tool to bitcoin-wallet in code comment (Kristaps Kaupe)
0db3ad3ba4 Mention -signet in bitcoin-wallet help output (Kristaps Kaupe)
Pull request description:
* Mention `-signet` in sentence where there is already `-testnet/-signet` in help output.
* Rename `wallet-tool` to `bitcoin-wallet` in single remaining place in code comments (was already done in #17648 at other places).
ACKs for top commit:
RandyMcMillan:
tACK 62cc138ecb
Tree-SHA512: c5df7811b8200f61943908dcf3b2b788fe991bf00bef28f069ab8784924556ffd5d86fc0ba2ad0b3c3f9be2ba73a34bc67059d7c057bba646c1801ffa3cb2070
fa1b89a6bd scripted-diff: Rename nReadPos to m_read_pos in streams.h (MarcoFalke)
fa56c79df9 Make CDataStream work properly on 64-bit systems (MarcoFalke)
fab02f7991 streams: Fix read-past-the-end and integer overflows (MarcoFalke)
Pull request description:
This is a follow-up to commit e26b62093a with the following fixes:
* Fix unsigned integer overflow in `ignore()`, when `nReadPos` wraps.
* Fix unsigned integer overflow in `read()`, when `nReadPos` wraps.
* Fix read-past-the-end in `read()`, when `nReadPos` wraps.
This shouldn't be remote-exploitable, because it requires a stream of more than 1GB of size. However, it might be exploitable if the attacker controls the datadir (I haven't checked).
A unit test for the overflow in `ignore()` looks like following. It is left as an excercise to the reader to replace `foo.ignore(7)` with the appropriate call to `read()` to reproduce the overflow and read-error in `read()`.
```diff
diff --git a/src/test/coins_tests.cpp b/src/test/coins_tests.cpp
index 922fd8e513..ec6ea93919 100644
--- a/src/test/coins_tests.cpp
+++ b/src/test/coins_tests.cpp
@@ -534,6 +534,20 @@ BOOST_AUTO_TEST_CASE(ccoins_serialization)
} catch (const std::ios_base::failure&) {
}
+ CDataStream foo{0, 0};
+ auto size{std::numeric_limits<uint32_t>::max()};
+ foo.resize(size);
+ BOOST_CHECK_EQUAL(foo.size(), size);
+ foo.ignore(std::numeric_limits<int32_t>::max());
+ size -= std::numeric_limits<int32_t>::max();
+ BOOST_CHECK_EQUAL(foo.size(), size);
+ foo.ignore(std::numeric_limits<int32_t>::max());
+ size -= std::numeric_limits<int32_t>::max();
+ BOOST_CHECK_EQUAL(foo.size(), size);
+ BOOST_CHECK_EQUAL(foo.size(), 1);
+ foo.ignore(7); // Should overflow, as the size is only 1
+ BOOST_CHECK_EQUAL(foo.size(), uint32_t(1 - 7));
+
// Very large scriptPubKey (3*10^9 bytes) past the end of the stream
CDataStream tmp(SER_DISK, CLIENT_VERSION);
uint64_t x = 3000000000ULL;
```
ACKs for top commit:
klementtan:
Code Review ACK fa1b89a6bd:
Tree-SHA512: 67f0a1baafe88eaf1dc844ac55b638d5cf168a18c945e3bf7a2cb03c9a5976674a8e3af2487d8a2c3eae21e5c0e7a519c8b16ee7f104934442e2769d100660e9
