62cb8d98d2 qt: Drop BitcoinGUI* WalletFrame data member (Hennadii Stepanov)
f73e5c972a qt: Move CreateWalletActivity connection from WalletFrame to BitcoinGUI (Hennadii Stepanov)
20e2e24e90 qt: Move WalletView connections from WalletFrame to BitcoinGUI (Hennadii Stepanov)
Pull request description:
This PR:
- implements an idea from https://github.com/bitcoin/bitcoin/pull/17937#issuecomment-575991765
- simplifies `WalletFrame` class interface
- as a side effect, removes `bitcoingui` -> `walletframe` -> `bitcoingui` circular dependency
- is an alternative to https://github.com/bitcoin/bitcoin/pull/17500
ACKs for top commit:
promag:
Tested ACK 62cb8d98d2 on macos 11.2.3 with depends build.
jarolrod:
ACK 62cb8d98d2
Tree-SHA512: 633b526a8499ba9ab4b16928daf4de4f6d610284bb9fa51891cad35300a03bde740df3466a71b46e87a62121330fcc9e606eac7666ea5e45fa6d5785b60dcbbd
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
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
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
* Rename RewindBlockIndex -> NeedsRedownload (follow-up to commit
d831e711ca)
* Fix typos
* Inline comments about faking chain data to avoid duplicating them
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
6c3fcd5591 test: remove BasicTestingSetup from util_threadnames unit tests (fanquake)
b53d3c1b1f test: remove BasicTestingSetup from uint256 unit tests (fanquake)
c0497a4928 test: remove BasicTestingSetup from torcontrol unit tests (fanquake)
ef8bb0473b test: remove BasicTestingSetup from sync unit tests (fanquake)
1aee83421f test: remove BasicTestingSetup from reverse_lock unit tests (fanquake)
57ba949ef5 test: remove BasicTestingSetup from policy_fee unit tests (fanquake)
3974c962b6 test: remove BasicTestingSetup from merkleblock tests (fanquake)
cd5bc4b470 test: remove BasicTestingSetup from hash unit tests (fanquake)
39cec22935 test: remove BasicTestingSetup from compilerbug unit tests (fanquake)
6d3b78c0e2 test: remove BasicTestingSetup from bswap unit tests (fanquake)
a13dc24831 test: remove BasicTestingSetup from bech32 unit tests (fanquake)
f4dcbe4498 test: remove BasicTestingSetup from base64 unit tests (fanquake)
fd144f6426 test: remove BasicTestingSetup from base32 unit tests (fanquake)
4c389ba04b test: remove BasicTestingSetup from arith_uint256 unit tests (fanquake)
05590651a0 test: remove BasicTestingSetup from amount unit tests (fanquake)
883a5c7d02 test: remove BasicTestingSetup from allocator unit tests (fanquake)
Pull request description:
* Less setup/overhead for tests that don't need it. Some naive bench-marking would suggest that a full `test_bitcoin` run is a few % faster after this change.
* Tests which don't need the BasicTestingSetup can't accidentally end up depending on it somehow.
* Already the case in at least the scheduler and block_filter tests.
This adds missing includes, but more significant is the removal of `setup_common.h` from tests where it isn't needed. This saves recompiling those tests when changes are made in the header.
ACKs for top commit:
practicalswift:
cr ACK 6c3fcd5591: patch looks correct
laanwj:
ACK 6c3fcd5591
Tree-SHA512: 69b891e2b4740402d62b86a4fc98c329a432d125971342a6f97334e166b3537ed3d4cdbb2531fa05c1feae32339c9fcb2dceda9afeeaed4edc70e8caa0962161
0680460041 qt: Translations update (Hennadii Stepanov)
Pull request description:
This is a pre-translation-string-freeze update. See [Release schedule for 22.0](https://github.com/bitcoin/bitcoin/issues/20851).
ACKs for top commit:
laanwj:
ACK 0680460041
Tree-SHA512: 65f11cc20e82459ca3484d4f377ff38be5ba31ba906abcb58d3e5ea56ee0eefd5e74c1ef1ea387833812bb2b9a02995dbeed7b45707e1d6db3f49e3b8055af6f
44d05d0a69 test: remove sanitizer suppression for nanobench (Martin Ankerl)
e3c866e3ca test: update nanobench from release 4.0.0 to 4.3.4 (Martin Ankerl)
Pull request description:
This updates the third-party library nanobench with the latest release. It contains mostly minor bugfixes, a new pyperf output format, ability to suppress warnings with environment variable `NANOBENCH_SUPPRESS_WARNINGS`. Full changelog:
v4.0.2
* Changed `doNotOptimizeAway` to what google benchmark is doing. The old code did not work on some machines.
* fix: display correct "total" value
* minor Documentation updates
v4.1.0
* Updated link to new pyperf home
* Adds ability to configure console output time unit
* Add support for environment variable `NANOBENCH_SUPPRESS_WARNINGS`
* Nanobench is now usable with CMake's FetchContent (see documentation: https://nanobench.ankerl.com/tutorial.html#cmake-integration)
v4.2.0
* Ability to store and later compare results added, through `pyperf`.
* See https://nanobench.ankerl.com/tutorial.html#pyperf-python-pyperf-module-output
* Added lots of build targets to travis, similar to bitcoin's build.
* Some minor API & documentation improvements
v4.3.0
* `ankerl::nanobench::Rng` can now return the state with `std::vector<uint64_t> Rng::state()`, and this can also be used to initialize the Rng.
v4.3.1
* Minor cmake improvements when integrationg as a third-party library: add alias `nanobench::nanobench`, default to C++17
v4.3.2
* Fixed a MSVC 2015 build problem
* updates license to 2021.
* build should now work with very old linux headers
* Also disable UBSAN (bitcoin needed to add a suppression)
v4.3.3
* Do not use locale-dependent `std::to_string`
v4.3.4
* Add missing sanitizer suppression to `rotl`
ACKs for top commit:
MarcoFalke:
review ACK 44d05d0a69
Tree-SHA512: 3291c85057720cfc84a44bfaa305a7d0df4dc35779169d20de73d32e40d4cdbf3f005bf343f79710eca517441de2459e8118c195c5f5136f99d1f50ebd5dfd08
e12f287498 net: cleanup newly added PeerManagerImpl::ProcessNewBlock (fanquake)
610151f5b0 validation: change ProcessNewBlock() to take a CBlock reference (fanquake)
Pull request description:
Addresses some [post-merge comments](https://github.com/bitcoin/bitcoin/pull/21713#pullrequestreview-638777410) from #21713. Also makes `ChainstateManager::ProcessNewBlock` take a const reference argument, as it [was asked](https://github.com/bitcoin/bitcoin/pull/21713#discussion_r615229548) why it was not the case in that PR.
ACKs for top commit:
jnewbery:
Code review ACK e12f287498
MarcoFalke:
review ACK e12f287498🚚
Tree-SHA512: 9c3e7353240c862d50bce2a0f58741c109dd628040b56ed46250103f8ebe9009238b131da710486791e28e3a83c985057b7be0a32aed1a929269b43097c7425b
7a799c9c2b index: refactor-only: Reuse CChain ref (Carl Dong)
db33cde80f index: Add chainstate member to BaseIndex (Carl Dong)
f4a47a1feb bench: Use existing chainman in AssembleBlock (Carl Dong)
91226eb917 bench: Use existing NodeContext in DuplicateInputs (Carl Dong)
e6b4aa6eb5 miner: Pass in chainman to RegenerateCommitments (Carl Dong)
9ecade1425 rest: Add GetChainman function and use it (Carl Dong)
fc1c282845 rpc/blockchain: Use existing blockman in gettxoutsetinfo (Carl Dong)
Pull request description:
Overall PR: #20158 (tree-wide: De-globalize ChainstateManager)
The first 2 commits are fixups addressing review for the last bundle: #21391
NEW note:
1. I have opened #21766 which keeps track of potential improvements where the flaws already existed before the de-globalization work, please post on that issue about these improvements, thanks!
Note to reviewers:
1. This bundle may _apparently_ introduce usage of `g_chainman` or `::Chain(state|)Active()` globals, but these are resolved later on in the overall PR. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
2. There may be seemingly obvious local references to `ChainstateManager` or other validation objects which are not being used in callers of the current function in question, this is done intentionally to **_keep each commit centered around one function/method_** to ease review and to make the overall change systematic. We don't assume anything about our callers. Rest assured that once we are considering that particular caller in later commits, we will use the obvious local references. [Commits of overall PR](https://github.com/bitcoin/bitcoin/pull/20158/commits)
3. When changing a function/method that has many callers (e.g. `LookupBlockIndex` with 55 callers), it is sometimes easier (and less error-prone) to use a scripted-diff. When doing so, there will be 3 commits in sequence so that every commit compiles like so:
1. Add `new_function`, make `old_function` a wrapper of `new_function`, divert all calls to `old_function` to `new_function` **in the local module only**
2. Scripted-diff to divert all calls to `old_function` to `new_function` **in the rest of the codebase**
3. Remove `old_function`
ACKs for top commit:
jarolrod:
ACK 7a799c9
ariard:
Code Review ACK 7a799c9
fjahr:
re-ACK 7a799c9c2b
MarcoFalke:
review ACK 7a799c9c2b🌠
ryanofsky:
Code review ACK 7a799c9c2b. Basically no change since last review except fixed rebase conflicts and a new comment about REST Ensure()
jamesob:
conditional ACK 7a799c9c2b ([`jamesob/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai`](https://github.com/jamesob/bitcoin/tree/ackr/21767.1.dongcarl.bundle_6_n_prune_g_chai))
Tree-SHA512: 531c00ddcb318817457db2812d9a9d930bc664e58e6f7f1c746350732b031dd624270bfa6b9f49d8056aeb6321d973f0e38e4ff914acd6768edd8602c017d10e
This updates the third-party library nanobench with the latest release. It contains mostly minor bugfixes, a new pyperf output format, ability to suppress warnings with environment variable `NANOBENCH_SUPPRESS_WARNINGS`. Full changelog:
v4.0.2
* Changed `doNotOptimizeAway` to what google benchmark is doing. The old code did not work on some machines.
* fix: display correct "total" value
* minor Documentation updates
v4.1.0
* Updated link to new pyperf home
* Adds ability to configure console output time unit
* Add support for environment variable `NANOBENCH_SUPPRESS_WARNINGS`
* Nanobench is now usable with CMake's FetchContent (see documentation: https://nanobench.ankerl.com/tutorial.html#cmake-integration)
v4.2.0
* Ability to store and later compare results added, through `pyperf`.
* See https://nanobench.ankerl.com/tutorial.html#pyperf-python-pyperf-module-output
* Added lots of build targets to travis, similar to bitcoin's build.
* Some minor API & documentation improvements
v4.3.0
* `ankerl::nanobench::Rng` can now return the state with `std::vector<uint64_t> Rng::state()`, and this can also be used to initialize the Rng.
v4.3.1
* Minor cmake improvements when integrationg as a third-party library: add alias `nanobench::nanobench`, default to C++17
v4.3.2
* Fixed a MSVC 2015 build problem
* updates license to 2021.
* build should now work with very old linux headers
* Also disable UBSAN (bitcoin needed to add a suppression)
v4.3.3
* Do not use locale-dependent `std::to_string`
v4.3.4
* Add missing sanitizer suppression to `rotl`
ffff0d0442 refactor: Switch serialize to uint8_t (1/n) (MarcoFalke)
Pull request description:
Replace `char` -> `uint8_t` in serialization where a sign doesn't make sense (char might be signed/unsigned).
ACKs for top commit:
practicalswift:
cr ACK ffff0d0442: patch looks correct and commit hash is ffffresh (was bbbbadass)
kristapsk:
ACK ffff0d0442
Tree-SHA512: cda682280c21d37cc3a6abd62569732079b31d18df3f157aa28bed80bd6f9f29a7db5c133b1f57b3a8f8d5ba181a76e473763c6e26a2df6d9244813f56f893ee
feb72e5432 scripted-diff: rename GetSystemTimeInSeconds to GetTimeSeconds (fanquake)
Pull request description:
This PR simply renames `GetSystemTimeInSeconds` to `GetTimeSeconds`, for uniformity amongst our time handling functions (`GetTimeMillis`, `GetTimeMicros`). I have a branch that does a chunk of `GetTime()` -> `GetSystemTimeInSeconds` (`GetTimeSeconds`) / `GetTime<T>` migration, so we can eventually remove the (2 year) deprecated `GetTime()`.
However, splitting this off and doing the renaming first while the number of `GetSystemTimeInSeconds` instances is small seems worthwhile.
ACKs for top commit:
practicalswift:
cr ACK feb72e5432: patch looks correct
promag:
Code review ACK feb72e5432.
Tree-SHA512: e2ac30be9cbcd77b70c9f74bef820b558945d0fcc6f3dc59fde68a18d08a7d36f42088b804ffe7c03478c8db048615b4c4aa65a3d8d9f5d717d59b58c99f1c54
38eb37c0bd qt, rpc: Do not accept command while executing another one (Hennadii Stepanov)
0c32b9c527 qt, rpc: Accept stop RPC even another command is executing (Hennadii Stepanov)
ccf790287c qt, rpc, refactor: Return early in RPCConsole::on_lineEdit_returnPressed (Hennadii Stepanov)
5b9c8c9cdd qt, rpc: Add "Executing…" message (Hennadii Stepanov)
Pull request description:
On master (3f512f3d56) it is possible to enter another command while the current command is still being executed. That makes a mess in the output.
With this PR:
Some previous context: https://github.com/bitcoin-core/gui/pull/59#issuecomment-715275185
---
It is still possible to enter and execute the `stop` command any time.
ACKs for top commit:
jarolrod:
ACK 38eb37c
promag:
Tested ACK 38eb37c0bd.
Tree-SHA512: 2b37a4b6838bf586b1b5c878192106721f713caeb6252514a6540356aab898986396e0777e73891d331b1be797a4926c20d3f9f38ba2c984ea90d55b0c34f664
0f3d955a38 qt: Make RPC console welcome message translation-friendly (Hennadii Stepanov)
Pull request description:
The best practice is do not split a translatable multi-line message into single lines. This helps translators to follow the context.
ACKs for top commit:
jarolrod:
re-ACK 0f3d955a38
Tree-SHA512: 30911ff3a972a7787804bb8b27d0b77bfff15939bb478c199261866bfb55d9acd12ab4d44b8b9fc1d4898222cabc4007cc897f9b65728924d121f31e914c44ac
d29ea72393 gui: Add access to the Peers tab from the network icon (Hennadii Stepanov)
Pull request description:
This PR add a small context menu to the network activity icon that provides an access to the Peers tab:
Closes#93.
ACKs for top commit:
Sjors:
re-ACK d29ea72393
kristapsk:
re-ACK d29ea72393
promag:
Code review ACK d29ea72393.
Tree-SHA512: dd871415fe514a19c6a22100d58f31954d9e55b80585d5a3f26e17a8d51dadf912441786fc0d23beabd812f1b501658fec1dbe345cd41beae5832a8eda890f77
c7bd5842e4 MOVEONLY: CWallet transaction code out of wallet.cpp/.h (Russell Yanofsky)
Pull request description:
This commit just moves function without making any changes. It can be reviewed with `git log -p -n1 --color-moved=dimmed_zebra`
Motivation for this change is to make `wallet.cpp/h` less monolithic and start to make wallet transaction state tracking comprehensible so bugs in https://github.com/bitcoin-core/bitcoin-devwiki/wiki/Wallet-Transaction-Conflict-Tracking can be fixed safely without introducing new problems.
This moves wallet classes and methods that deal with transactions out of `wallet.cpp/.h` into better organized files:
- `transaction.cpp/.h` - CWalletTx and CMerkleTx class definitions
- `receive.cpp/.h` - functions checking received transactions and computing balances
- `spend.cpp/.h` - functions creating transactions and finding spendable coins
After #20773, when loading is separated from syncing it will also be possible to move more `wallet.cpp/.h` functions to:
- `sync.cpp/.h` - functions handling chain notifications and rescanning
This commit arranges `receive.cpp` and `spend.cpp` functions in dependency order so it's possible to skim `receive.cpp` and get an idea of how computing balances works, and skim `spend.cpp` and get an idea of how transactions are created, without having to jump all over `wallet.cpp` where functions are not in order and there is a lot of unrelated code.
Followup commit "refactor: Detach wallet transaction methods" in https://github.com/bitcoin/bitcoin/pull/21206 follows up this PR and tweaks function names and arguments to reflect new locations. The two commits are split into separate PRs because this commit is more work to maintain and less work to review, while the other commit is less work to maintain and more work to review, so hopefully this commit can be merged earlier.
ACKs for top commit:
Sjors:
re-utACK c7bd5842e4
fjahr:
utACK c7bd5842e4
promag:
Code review ACK c7bd5842e4, verified move only claim.
meshcollider:
Dimmed-zebra-check and functional test run ACK c7bd5842e4
Tree-SHA512: 4981de6911cb1196774db375494355cc9af59b52456129c002d264a77cd9ed6175f8ecbb6b2f492a59a4d5a0def21a39d96fa79c9f4d99be0992985f553be32f
4935ac583b qt: Improve GUI responsiveness (Hennadii Stepanov)
75850106ae qt, macos: Fix GUIUtil::PolishProgressDialog bug (Hennadii Stepanov)
Pull request description:
[`QProgressDialog`](https://doc.qt.io/qt-5/qprogressdialog.html) estimates the time the operation will take (based on time for steps), and only shows itself if that estimate is beyond [`minimumDuration`](https://doc.qt.io/qt-5/qprogressdialog.html#minimumDuration-prop).
The default `minimumDuration` value is [4 seconds](https://doc.qt.io/qt-5/qprogressdialog.html#details), and it could make users think that the GUI is frozen.
This PR sets `minimumDuration` to zero for all progress dialogs, that affects ones in the `WalletControllerActivity` class.
ACKs for top commit:
ryanofsky:
Code review ACK 4935ac583b. I'm not very familiar with this API but all the changes and explanations make sense and are very clear, and this seems like it should be an improvement.
promag:
Code review ACK 4935ac583b.
jarolrod:
ACK 4935ac583b
Tree-SHA512: 2ddd74e7fd87894d341d2439dbaa544d031a350f7f57d4c7e9fbba977dc24080fe60fd7a80a542b1647f1de9091d7fd04a36eab695088d4d75fb836548e99b5f
