fb1b1e0f3e qt: Save/restore column sizes of the tables in the Peers tab (Hennadii Stepanov)
Pull request description:
ACKs for top commit:
jonatack:
ACK fb1b1e0f3e code review, debug-built and tested
jarolrod:
ACK fb1b1e0f3e
Tree-SHA512: f93495ecd13e4202aba61b407fffbeec855f5b0c1cc027197c78edddd7d11c87ebdb0fcb1daac242f0407323b31f4e7e0313bd76113a5241e4c868a8829af20a
8b419b5163 qt: make console buttons look clickable (Jarol Rodriguez)
Pull request description:
On master, for macOS, the console buttons' hitboxes are quite small. This makes clicking on the button with your mouse a little more tedious than it should be. The Issue is related to recent versions of Qt (>5.9.8) not playing so nice on macOS when there are "incorrect" `width` and `height` values set for a `QPushButton` (here is another example: https://github.com/bitcoin-core/gui/pull/319#pullrequestreview-652907740).
This fixes this small hitbox issue by converting the buttons from `QPushButton` to `QToolButton`, which in turn makes the buttons look explicitly clickable. This approach was chosen as it helps us avoid having to play around with `width` and `height` values until we find values that play nice with macOS and look good on Linux & Windows. Also, `QToolButton` is an appropriate class for these buttons.
Per [Qt Docs](https://doc.qt.io/qt-5/qtoolbutton.html#details):
> A tool button is a special button that provides quick-access to specific commands or options. As opposed to a normal command button, a tool button usually doesn't show a text label, but shows an icon instead.
Since we are changing the type of the buttons, we need to change the respective actions connection logic in `rpcconsole`. Instead of plugging in `QToolButton`, we abstract it to the base class: `QAbstractButton`.
per [Qt Dev Notes](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Developer-Notes-for-Qt-Code#inherited-signals-and-slot)
> Use base class functions as this makes the code more general, e.g., use QAbstractButton::clicked instead of QPushButton::clicked.
While here, we also update the size of the icons to `22x22` to be consistent with other tool buttons.
**macOS: Master vs PR:**
| Master | PR |
| ----------- | ----------- |
|  |  |
**Linux: Master vs PR:**
| Master | PR |
| ----------- | ----------- |
|  |  |
ACKs for top commit:
hebasto:
ACK 8b419b5163, tested on Linux Mint 20.1 (Qt 5.12.8).
promag:
Tested ACK 8b419b5163 on macOS Big Sur M1, this drops only relevant usages to `flat` buttons.
Tree-SHA512: 3f3cdcbe83398136a1d1ee8fc2835be8681f2ed39e79db1e939cab6a00a779f528343d54992807a845cc84d9ef13591affb7a6dbca9e5753a2b8665b0af4d611
fa2b6c62cd test: Remove unused node from feature_nulldummy (MarcoFalke)
Pull request description:
This is confusing and might even slow down the test.
This reverts a change that was added a year ago in d438d60 and then the need for it was removed by 95d5d5e six months ago.
Top commit has no ACKs.
Tree-SHA512: 9a86792e9a634cf7bbd4e7a21b1acdfc3baba1b1962fe2b9b73848436d10351d2326dca01313c097ba2342dde7207add73e731d053c0bfa888a5d8f2b233a7cf
ca3a77068b test: Fix p2p_leak.py intermittent failure by lowering timeout (Martin Zumsande)
Pull request description:
Fixes#22085
Root cause: There was just 1 second between the wait (5 seconds) and the `-peertimeout=4`.
Since `ShouldRunInactivityChecks` in `net.cpp` measures the timeout in seconds, its result can only change once per second, even though it is called more often.
So in situations when the connection is established early in a given second like [here](https://bitcoinbuilds.org/index.php?ansilog=d7b3e075-683a-45cc-94d4-9645fc17e0b6.log#l3117) (2021-05-27T12:28:04.**001**913Z ), the 1 second leeway was not be sufficient, leading to the intermittent failures.
Fix this by lowering the timeout by one second.
ACKs for top commit:
MarcoFalke:
re-ACK ca3a77068b
Tree-SHA512: e7e22356d276c65a5b4f0a1b7ee5a9ad07d27691220746c7d02af3fad22cab1d53fd0ef59a938167ec80e4571c96649132d6922ad10667fc91baa47892f27a3e
e4356f6a6c Testcase for wallet issue with orphaned rewards. (Daniel Kraft)
Pull request description:
This adds a new test case demonstrating the wallet issue when block rewards are orphaned (#14148).
ACKs for top commit:
LarryRuane:
ACK e4356f6a6c
leonardojobim:
reACK e4356f6a6c .
Tree-SHA512: e9a2310ee1b3d52cfa302f431ed3d272bbc1b9195439ff318d9eb1006c0b28968dbe840e1600b6ff185e5d7ea57e4dcc837cef16051b5537445e10bc363b8c22
3d552b0d78 [doc] explain why CheckBlock() is called before AcceptBlock() (Sjors Provoost)
Pull request description:
Based on https://lists.linuxfoundation.org/pipermail/bitcoin-dev/2019-February/016697.html and its PDF attachment.
ACKs for top commit:
MarcoFalke:
cr ACK 3d552b0d78
Tree-SHA512: d1ef39855317853e0e7e051ec6015054d0d227fcdf20281c2c1921056537f1f79044aa1bdd35f46475edd17596fbcae79aeb338c4865b1269a01b158f6cb2ac4
2667366aaa tests: check derivation of P2TR (Pieter Wuille)
7cedafc541 Add tr() descriptor (derivation only, no signing) (Pieter Wuille)
90fcac365e Add TaprootBuilder class (Pieter Wuille)
5f6cc8daa8 Add XOnlyPubKey::CreateTapTweak (Pieter Wuille)
2fbfb1becb Make consensus checking of tweaks in pubkey.* Taproot-specific (Pieter Wuille)
a4bf84039c Separate WitnessV1Taproot variant in CTxDestination (Pieter Wuille)
41839bdb89 Avoid dependence on CTxDestination index order (Pieter Wuille)
31df02a070 Change Solver() output for WITNESS_V1_TAPROOT (Pieter Wuille)
4b1cc08f9f Make XOnlyPubKey act like byte container (Pieter Wuille)
Pull request description:
This is a subset of #21365, to aide review.
This adds support `tr(KEY)` or `tr(KEY,SCRIPT)` or `tr(KEY,{{S1,{{S2,S3},...}},...})` descriptors, describing Taproot outputs with specified internal key, and optionally any number of scripts, in nested groups of 2 inside `{`/`}` if there are more than one. While it permits importing `tr(KEY)`, anything beyond that is just laying foundations for more features later.
Missing:
* Signing support (see #21365)
* Support for more interesting scripts inside the tree (only `pk(KEY)` is supported for now). In particular, a multisig policy based on the new `OP_CHECKSIGADD` opcode would be very useful.
* Inferring `tr()` descriptors from outputs (given sufficient information).
* `getaddressinfo` support.
* MuSig support. Standardizing that is still an ongoing effort, and is generally kind of useless without corresponding PSBT support.
* Convenient ways of constructing descriptors without spendable internal key (especially ones that arent't trivially recognizable as such).
ACKs for top commit:
Sjors:
utACK 2667366 (based on https://github.com/bitcoin/bitcoin/pull/21365#issuecomment-846945215 review, plus the new functional test)
achow101:
Code Review ACK 2667366aaa
lsilva01:
Tested ACK 2667366aaa
meshcollider:
utACK 2667366aaa
Tree-SHA512: 61046fef22c561228338cb178422f0b782ef6587ec8208d3ce2bd07afcff29a664b54b35c6b01226eb70b6540b43f6dd245043d09aa6cb6db1381b6042667e75
5d82a57db4 contrib: remove torv2 seed nodes (Jon Atack)
5f7e086dac contrib: update generate-seeds.py to ignore torv2 addresses (Jon Atack)
8be56f0f8e p2p, refactor: extract OnionToString() from CNetAddr::ToStringIp() (Jon Atack)
5f9d3c09b4 p2p: remove torv2 from CNetAddr::ToStringIP() (Jon Atack)
3d39042144 p2p: remove torv2 in SetIP() and ADDR_TORV2_SIZE constant (Jon Atack)
cff5ec477a p2p: remove pre-addrv2 onions from SerializeV1Array() (Jon Atack)
4192a74413 p2p: ignore torv2-in-ipv6 addresses in SetLegacyIPv6() (Jon Atack)
1d631e956f p2p: remove BIP155Network::TORV2 from GetBIP155Network() (Jon Atack)
7d1769bc45 p2p: remove torv2 from SetNetFromBIP155Network() (Jon Atack)
eba9a94b9f fuzz: rename CNetAddr/CService deserialize targets (Jon Atack)
c56a1c9b18 p2p: drop onions from IsAddrV1Compatible(), no longer relay torv2 (Jon Atack)
f8e94002fc p2p: remove torv2/ADDR_TORV2_SIZE from SetTor() (Jon Atack)
0f1c58ae87 test: update feature_proxy to torv3 (Jon Atack)
Pull request description:
This patch removes support in Bitcoin Core for Tor v2 onions, which are already removed from the release of Tor 0.4.6.
- no longer serialize/deserialize and relay Tor v2 addresses
- ignore incoming Tor v2 addresses
- remove Tor v2 addresses from the addrman and peers.dat on node launch
- update generate-seeds.py to ignore Tor v2 addresses
- remove Tor v2 hard-coded seeds
Tested with tor-0.4.6.1-alpha (no v2 support) and 0.4.5.7 (v2 support). With the latest Tor (no v2 support), this removes all the warnings like those reported with current master in https://github.com/bitcoin/bitcoin/issues/21351
```
<bitcoind debug log>
Socks5() connect to […].onion:8333 failed: general failure
<tor log>
Invalid hostname [scrubbed]; rejecting
```
and the addrman no longer has Tor v2 addresses on launching bitcoind.
```rake
$ ./src/bitcoin-cli -addrinfo
{
"addresses_known": {
"ipv4": 44483,
"ipv6": 8467,
"torv2": 0,
"torv3": 2296,
"i2p": 6,
"total": 55252
}
}
```
After recompiling back to current master and restarting with either of the two Tor versions (0.4.5.7 or 0.4.6.1), -addrinfo initially returns 0 Tor v2 addresses and then begins finding them again.
Ran nodes on this patch over the past week on mainnet/testnet/signet/regtest after building with DEBUG_ADDRMAN.
Verified that this patch bootstraps an onlynet=onion node from the Tor v3 hardcoded fixed seeds on mainnet and testnet and connects to blocks and v3 onion peers: `rm ~/.bitcoin/testnet3/peers.dat ; ./src/bitcoind -testnet -dnsseed=0 -onlynet=onion`
Tested using `addnode`, `getaddednodeinfo`,`addpeeraddress`, `disconnectnode` and `-addrinfo` that a currently valid, connectable Tor v2 peer can no longer be added:
Thanks to Vasil Dimov, Carl Dong, and Wladimir J. van der Laan for their work on BIP155 and Tor v3 that got us here.
ACKs for top commit:
laanwj:
Code review ACK 5d82a57db4
Tree-SHA512: 590ff3d2f6ef682608596facb4b01f44fef69716d2ab3552ae1655aa225f4bf104f9ee08d6769abb9982a8031de93340df553279ce1f5023771f9f2b651178bb
f5ba424cd4 wallet: Add IsAddressUsed / SetAddressUsed methods (Russell Yanofsky)
62252c95e5 interfaces: Stop exposing wallet destdata to gui (Russell Yanofsky)
985430d9b2 test: Add gui test for wallet receive requests (Russell Yanofsky)
Pull request description:
Stop giving GUI access to destdata rows in database. Replace with narrow API just for saving and reading receive request information.
This simplifies code and should prevent the GUI from interfering with other destdata like address-used status. It also adds some more GUI test coverage.
There are no changes in behavior.
ACKs for top commit:
jarolrod:
tACK f5ba424cd4
laanwj:
Code review ACK f5ba424cd4
Tree-SHA512: 5423df4786e537a59013cb5bfb9e1bc29a7ca4b8835360c00cc2165a59f925fdc355907a4ceb8bca0285bb4946ba235bffa7645537a951ad03fd3b4cee17b6b0
a58868d201 build: Makes rcc output always deterministic (Hennadii Stepanov)
Pull request description:
The Qt Resource Compiler ([rcc](https://doc.qt.io/qt-5/rcc.html)) has a command-line option `--format-version` which has the [default value](https://code.qt.io/cgit/qt/qtbase.git/tree/src/tools/rcc/main.cpp?h=5.12.10#n172) 2.
The only difference from `--format-version 1` is adding a [last modified timestamp](https://code.qt.io/cgit/qt/qtbase.git/tree/src/tools/rcc/rcc.cpp?h=5.12.10#n207) to the output file ([credits](https://github.com/bitcoin/bitcoin/pull/21654#issuecomment-819198228) to **fanquake**). That, in turn, forces us to use `QT_RCC_SOURCE_DATE_OVERRIDE=1` to get deterministic builds (#13732).
This change makes rcc output always deterministic by using `--format-version 1` option that makes usage of the
`QT_RCC_SOURCE_DATE_OVERRIDE` needless.
---
Also it improves interaction with ccache:
On master (f6c44e999b):
```
$ make && make clean && ccache --zero-stats && make && ccache --show-stats
...
cache directory /home/hebasto/.ccache
primary config /home/hebasto/.ccache/ccache.conf
secondary config (readonly) /etc/ccache.conf
stats updated Sun Apr 11 15:45:43 2021
stats zeroed Sun Apr 11 15:45:05 2021
cache hit (direct) 638
cache hit (preprocessed) 0
cache miss 1
cache hit rate 99.84 %
called for link 10
cleanups performed 0
files in cache 20023
cache size 13.2 GB
max cache size 15.0 GB
```
The missed file is always `qt/libbitcoinqt_a-qrc_bitcoin_locale.o`.
With this PR:
```
$ make && make clean && ccache --zero-stats && make && ccache --show-stats
...
cache directory /home/hebasto/.ccache
primary config /home/hebasto/.ccache/ccache.conf
secondary config (readonly) /etc/ccache.conf
stats updated Sun Apr 11 15:28:46 2021
stats zeroed Sun Apr 11 15:28:21 2021
cache hit (direct) 639
cache hit (preprocessed) 0
cache miss 0
cache hit rate 100.00 %
called for link 10
cleanups performed 0
files in cache 20012
cache size 13.2 GB
max cache size 15.0 GB
```
ACKs for top commit:
fanquake:
ACK a58868d201
Tree-SHA512: 52f4a3267f41883d13025c0de79b6da22e92d60c729e01b986935c6812bbfe7fadc40b742bd715bfdf09df94af6838d4fbbe8208c6123f366108e38c8e1121c5
3b36395b96 depends: Fix qt.mk for mac arm64 (João Barbosa)
Pull request description:
With f16d4cd8c5 `depends/config.guess` gives `aarch64-apple-darwin20.3.0` where before would give `arm-apple-darwin20.3.0`. Fix `qt.mk` accordingly.
ACKs for top commit:
hebasto:
ACK 3b36395b96, I have reviewed the code and it looks OK, I agree it can be merged.
fanquake:
ACK 3b36395b96
Tree-SHA512: bd20402d0a6e9a5bb652198de189cf2b4f3f76fd03d0cba8c4d657c60b8a088cf3532efe6c1efbbedd94c00a155e6d180b77f1cd8bc24e0e35764839e8b77e30
ab86ac7739 build, qt: Make QWindowsVistaStylePlugin available again (regression) (Hennadii Stepanov)
Pull request description:
This PR is similar to 1be8e0f238, and, actually, it is a #21376 follow up.
Required as in Qt 5.12.x style plugins are separated.
Fixes#22132.
Fixes https://github.com/bitcoin-core/gui/issues/303.
**Note for reviewers**. Besides visual changes in the GUI, you could compare the first dozen of lines in the `debug.log` file.
---
#### Guix build:
```
$ HOSTS=x86_64-w64-mingw32 contrib/guix/guix-build
$ find guix-build-$(git rev-parse --short=12 HEAD)/output/ -type f -print0 | env LC_ALL=C sort -z | xargs -r0 sha256sum
e3b0c44298fc1c149afbf4c8996fb92427ae41e4649b934ca495991b7852b855 guix-build-ab86ac7739b2/output/dist-archive/SKIPATTEST.TAG
d3f05888c95d6cf6f4a0bac952d4c0bf1e271f49cb7ab92b08080b753957e5e5 guix-build-ab86ac7739b2/output/dist-archive/bitcoin-ab86ac7739b2.tar.gz
113bb86ffd2854e08d3c0c5cd3f9728f2e4fe6c7a1813bc1c4b73eb48ad6597f guix-build-ab86ac7739b2/output/x86_64-w64-mingw32/bitcoin-ab86ac7739b2-win-unsigned.tar.gz
3fdc99754d6f26d9655a308c8847076d8fd4db8d61f4f18d8ef545894a42c10d guix-build-ab86ac7739b2/output/x86_64-w64-mingw32/bitcoin-ab86ac7739b2-win64-debug.zip
932bb69e3eb1a617f9e337b83220c9d6a277bec421f4d78e32dffde5643eb00c guix-build-ab86ac7739b2/output/x86_64-w64-mingw32/bitcoin-ab86ac7739b2-win64-setup-unsigned.exe
852f4c03ad049fcd6bb2ce25b3bcf4e18b90d34f7232660024b341bd7c8dd710 guix-build-ab86ac7739b2/output/x86_64-w64-mingw32/bitcoin-ab86ac7739b2-win64.zip
c6a33a193a6b128e665198dde94758acab3a5ab7fefe0c3c6617eb98ad01693d guix-build-ab86ac7739b2/output/x86_64-w64-mingw32/inputs.SHA256SUMS
```
ACKs for top commit:
laanwj:
Concept and code review ACK ab86ac7739
fanquake:
ACK ab86ac7739
Tree-SHA512: 9f5498480379fad41de616da48331e123daf5b2294ca79c33ffea2b113b314634be9a2a70bf060cb5be1392a48e9d047891e1d22c129f408f1d76a0bc888441f
8050eb43bf script: fix spelling linter raising spuriously on "invokable" (Jon Atack)
Pull request description:
"invokable" is a valid word that means to be callable, but the linter is raising on it:
```
$ test/lint/lint-spelling.sh
contrib/guix/guix-attest:18: invokable ==> invocable
contrib/guix/guix-clean:18: invokable ==> invocable
contrib/guix/guix-verify:18: invokable ==> invocable
^ Warning: codespell identified likely spelling errors. Any false positives? Add them to the list of ignored words in test/lint/lint-spelling.ignore-words.txt
```
ACKs for top commit:
MarcoFalke:
cr ACK 8050eb43bf
Tree-SHA512: 43f8dc7b7adb00ae563ccfe04a64a7ceb50237f24ff87209062bf57b2564b4d38a409df80e0183aa4f40ab306b5e07d7a5fad1600d41705bd3c443ed66a6d1c1
* Rename RewindBlockIndex -> NeedsRedownload (follow-up to commit
d831e711ca)
* Fix typos
* Inline comments about faking chain data to avoid duplicating them
Since headers-first syncing, we only ever request a block if we've already validated its headers.
Therefore QueuedBlock.fValidatedHeaders is always set to true. Remove it.
37371268d1 Mark `CheckTxInputs` `[[nodiscard]]` (out-param `txfee` only set if call is successful). Avoid UUM in fuzzing harness `coins_view`. (practicalswift)
Pull request description:
Mark `CheckTxInputs` `[[nodiscard]]` (out-param `txfee` only set if call is successful).
Avoid use of uninitialised memory (UUM) in fuzzing harness `coins_view`.
ACKs for top commit:
MarcoFalke:
review ACK 37371268d1
Tree-SHA512: edada5b2e80ce9ad3bd57b4c445bedefffa0a2d1cc880957d6848e4b7d9fc1ce036cd17f8b18bc03a36fbf84fc29c166cd6ac3dfbfe03e69d6fdbda13697754d
5f23531926 CRegTestParams: Use `args` instead of `gArgs`. (Kiminuo)
Pull request description:
This PR is a very minor follow-up to #13311.
I believe that `gArgs` was just overlooked at the modified line.
ACKs for top commit:
MarcoFalke:
cr ACK 5f23531926
Tree-SHA512: f4e4ed6b23fca60e88825b502f20a1341ee2e4429bc8a2a7e419057adb643abda11be2061fe7ee076931657736e629aff88fd2c33737c84c330dc9d64f368c30
faa8dfd6a1 ci: Bump macOS image to big-sur-xcode-12.5 (MarcoFalke)
Pull request description:
Closes#22068
ACKs for top commit:
hebasto:
ACK faa8dfd6a1, I have reviewed the code and it looks OK, I agree it can be merged, and the Cirrus CI is green.
Tree-SHA512: e29f6290163f3727f3603a3d6b4cf47677f6b02fff370e8d9073962a42bd7ab1ae8d247306e4c41bcadf0a208784344a6229627fe1a883b1e5112df30ea88635
9a0969585f build: Add /opt/homebrew to path to look for boost libraries (Fu Yong Quah)
Pull request description:
Following the instruction in https://github.com/bitcoin/bitcoin/blob/master/doc/build-osx.md for building on the M1 Macs don't work out of the box, because homebrew now defaults to /opt/homebrew instead of /usr/local. This PR fixes that.
ACKs for top commit:
jonasschnelli:
utACK 9a0969585f
promag:
Tested ACK 9a0969585f.
Tree-SHA512: 472568b97fbd8623481fe6fd43b0509fa32fe7f1c1d8090321a6a6a5bdc7343d4ad4122c10dcc7c9c93068db8a3f009a73befaf1ba11e4af54a66afd2c2dbe14
dafab2b1b3 doc: improve note on choosing posix mingw32 (Jarol Rodriguez)
Pull request description:
The current [windows build doc](https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md) can lead someone to believe that the step where you must choose the posix mingw32 g++ compiler option is only for `Ubuntu 18.04`. It is only until you (or just me) go through the cross-compile process and realize that it's not building because you didn't set this option on > `Ubuntu 18.04`. Then you come back and read the footnotes and see: `Starting from Ubuntu Xenial 16.04...`.
This PR improves this portion of the doc by editing the text around this stage to state "this is now the next step". We could add a note saying `Ubuntu 18.04 and up`, but this is redundant as it's unlikely someone will be using < Ubuntu 18.04 since it's not officially supported by our build system. While here, some minor fixups and add some more guidance to completing this step.
**Master:** [render](https://github.com/bitcoin/bitcoin/blob/master/doc/build-windows.md#building-for-64-bit-windows)
**PR:** [render](dafab2b1b3/doc/build-windows.md (building-for-64-bit-windows))
ACKs for top commit:
laanwj:
ACK dafab2b1b3
hebasto:
ACK dafab2b1b3
Tree-SHA512: d8d50fa13c34be753700843183a1a4addad6447c813bfc039865ba11d5fa03c25206ed6f3782331df4c6a44448e3fec4138ae911b995b5cacc5b756bb5d74bb4
