68faa87881f5334b2528db4adc72ec19d94316a3 test: use f-strings in mining_*.py tests (fanquake)
c2a5d560df2824df5731100c2584e8ad7a3d7bc2 test: use f-strings in interface_*.py tests (fanquake)
86d958262dff43002820d58ccb8958e2dbfb9d5b test: use f-strings in feature_proxy.py (fanquake)
31bdb33dcb8345df1bb94b28e811252a918d7dcb test: use f-strings in feature_segwit.py (fanquake)
b166d54c3cbb0c028210cee977b3dcde5ac5474f test: use f-strings in feature_versionbits_warning.py (fanquake)
cf6d66bf941d946600047d712c7cd15d7605322e test: use f-strings in feature_settings.py (fanquake)
6651d77f22862716f5bd7d0b31cfbd3937ab7b1d test: use f-strings in feature_pruning.py (fanquake)
961f5813ba65b6a601081912c4ece96c2679794d test: use f-strings in feature_notifications.py (fanquake)
1a546e6f6ca95772f0d7dbc2792477becbb8ea63 test: use f-strings in feature_minchainwork.py (fanquake)
6679eceacc915a8ea7cd7063f103ffc5eb9da884 test: use f-strings in feature_logging.py (fanquake)
fb633933ab570e945d2a366f37eeff39f516c613 test: use f-strings in feature_loadblock.py (fanquake)
e9ca8b254d4b9567831c0e113ce1c0a2b4795a95 test: use f-strings in feature_help.py (fanquake)
ff7e3309995a8960ac371741b2b00c6da40f7490 test: use f-strings in feature_filelock.py (fanquake)
d5a6adc5e478fa5c6e562377eea873dc38e66578 test: use f-strings in feature_fee_estimation.py (fanquake)
a2de33cbdc79202bccddb4beadfde88266ac979f test: use f-strings in feature_dersig.py (fanquake)
a2502cc63fd308be8af840962da9c53339433fa6 test: use f-strings in feature_dbcrash.py (fanquake)
3e2f84e7a96cb4b97b609ac853f78edd0ed43f82 test: use f-strings in feature_csv_activation.py (fanquake)
e2f1fd8ee92fa421b6d293169044d6ddd5a9b8df test: use f-strings in feature_config_args.py (fanquake)
36d33d32b1b498b61f56d552f6e2c1d064f978c3 test: use f-strings in feature_cltv.py (fanquake)
dca173cc044270b30782b1e3355e9dcb8c534295 test: use f-strings in feature_blocksdir.py (fanquake)
5453e8706278918ac51a725e81599cfa18c8cdbc test: use f-strings in feature_backwards_compatibility.py (fanquake)
6f3d5ad67ac8e7b50abae1a2949898d858e38106 test: use f-strings in feature_asmap.py (fanquake)
Pull request description:
Rather than using 3 different ways to build/format strings (sometimes all in the same test, i.e [`feature_config_args.py`](https://github.com/bitcoin/bitcoin/blob/master/test/functional/feature_config_args.py)), consolidate to using [f-strings (3.6+)](https://docs.python.org/3/reference/lexical_analysis.html#f-strings), which are generally more concise / readable, as well as more performant than existing methods.
This deals with the `feature_*.py`, `interface_*.py` and `mining_*.py` tests.
See also: [PEP 498](https://www.python.org/dev/peps/pep-0498/)
ACKs for top commit:
mjdietzx:
reACK 68faa87881f5334b2528db4adc72ec19d94316a3
Zero-1729:
crACK 68faa87881f5334b2528db4adc72ec19d94316a3
Tree-SHA512: d4e1a42e07d96d2c552387a46da1534223c4ce408703d7568ad2ef580797dd68d9695b8d19666b567af37f44de6e430e8be5db5d5404ba8fcecf9f5b026a6efb
fa6e6a3f03a38f8b431bf694268ed344d1815b3b doc: Remove confusing assert linter (MarcoFalke)
Pull request description:
The `assert()` documentation and linter are redundant and confusing:
* The source code already refuses to compile with `assert()` disabled.
* They violate the assumptions about `Assert()`, which *requires* side effects.
* The existing linter doesn't enforce the guideline, only checking for `++` and `--` side effects.
Fix all issues by removing the docs and the linter. See also https://github.com/bitcoin/bitcoin/pull/26684#discussion_r1287370102
Going forward everyone is free to use whatever code in this regard they think is the easiest to read. Also, everyone is still free to share style-nits, if they think it is a good use of their time and of the pull request author. Finally, the author is still free to dismiss or ignore this style-nit, or any other style-nit.
ACKs for top commit:
hebasto:
ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b, I have reviewed the code and it looks OK.
theStack:
ACK fa6e6a3f03a38f8b431bf694268ed344d1815b3b
Tree-SHA512: 686738d71e1316cc95e5d3f71869b55a02bfb137c795cc0875057f4410e564bc8eff03c985a2087b007fb08fc84551c7da1e8b30c7a9c3f2b14e5e44a5970236
1c36bafc5f7db268546dcc86c793071a7e9d35e0 wallet: have prune error take precedence over assumedvalid (James O'Beirne)
Pull request description:
Fixes https://github.com/bitcoin/bitcoin/pull/23997#discussion_r891412739.
From Russ Yanofsky:
> Agree with all of Marco's points here and think this should be updated
>
> If havePrune and hasAssumedValidChain are both true, better to show havePrune error message. Assumed-valid error message is vague and not very actionable. Would suggest "Error loading wallet. Wallet requires blocks to be downloaded, and software does not currently support loading wallets while blocks are being downloaded out of order though assumeutxo snapshots. Wallet should be able to load successfully after node sync reaches height {block_height}"
ACKs for top commit:
MarcoFalke:
ACK 1c36bafc5f7db268546dcc86c793071a7e9d35e0
aureleoules:
ACK 1c36bafc5f7db268546dcc86c793071a7e9d35e0
Tree-SHA512: bfb0024bb962525cbbd392ade3c0331a8b0525e7f2f2ab52b2dbb9b6dd6311070d85ecb762a7689db84a30991971865698ab6fec187206e6a92133790c5a91dc
817326a828d6148dc63d9ef08f641b9c0c522411 wallet: avoid rescans if under the snapshot (James O'Beirne)
Pull request description:
This is part of the [assumeutxo project](https://github.com/bitcoin/bitcoin/projects/11) (parent PR: #15606)
---
Refuse to load a wallet if it requires a rescan lower than the height of assumed-valid blocks.
Of course in live code right now, `BLOCK_ASSUMED_VALID` block index entries don't exist since they're a unique flag introduced by the use of UTXO snapshots, so this is prophylactic code exercised only by unittests.
ACKs for top commit:
achow101:
ACK 817326a828d6148dc63d9ef08f641b9c0c522411
ryanofsky:
Code review ACK 817326a828d6148dc63d9ef08f641b9c0c522411. This seems like the simplest change we can make to avoid wallet problems when an assumeutxo snapshot is loaded.
Tree-SHA512: cfa44b2eb33d1818d30df45210d0dde1e9b78cc9b7c88cb985054dc28427bba9e0905debe4196065d1d3a5ce7bca7e605e629d5ce5f0225b25395746e6d3d596
4de4221ab4b645ff77503c777c9b195a962e9fa1 build: Check for std::atomic::exchange rather than std::atomic_exchange (Andrew Chow)
Pull request description:
Our usage of std::atomic is with it's own exchange function, not std::atomic_exchange. So we should be looking specifically for that function.
This removes the need for -latomic for riscv builds, which resolves a guix cross architecture reproducibility issue.
ACKs for top commit:
hebasto:
ACK 4de4221ab4b645ff77503c777c9b195a962e9fa1
fanquake:
ACK 4de4221ab4b645ff77503c777c9b195a962e9fa1
Tree-SHA512: dd8225fc9c6a335601f611700003d0249b9ef941efa502db39306129677929d013048e9221be1d6d7f0ea2d90313d4b87de239f441be21b25bea40a6c19a031e
f665c6ecda854adaaa629e961b1912847b42fd34 test: fix failing test interface_usdt_utxocache.py (Sebastian Falbesoner)
Pull request description:
The `from_node` argument doesn't exist anymore for `MiniWallet.create_self_transfer` since PR #25435 (commit fa8421bc5bcd483a9501257073db17ff2e76eb46), leading to an error on master:
```
$ sudo ./test/functional/interface_usdt_utxocache.py
2022-06-27T17:45:35.585000Z TestFramework (INFO): Initializing test directory /tmp/bitcoin_func_test_7s1djjo1
2022-06-27T17:45:36.515000Z TestFramework (INFO): testing the utxocache:uncache tracepoint API
2022-06-27T17:45:36.517000Z TestFramework (ERROR): Unexpected exception caught during testing
Traceback (most recent call last):
File "/home/honeybadger/bitcoin/test/functional/test_framework/test_framework.py", line 133, in main
self.run_test()
File "/home/honeybadger/bitcoin/./test/functional/interface_usdt_utxocache.py", line 149, in run_test
self.test_uncache()
File "/home/honeybadger/bitcoin/./test/functional/interface_usdt_utxocache.py", line 172, in test_uncache
invalid_tx = self.wallet.create_self_transfer(
TypeError: create_self_transfer() got an unexpected keyword argument 'from_node'
2022-06-27T17:45:36.568000Z TestFramework (INFO): Stopping nodes
[...]
```
Fix this by removing the argument. (Unfortunately, the USDT tests don't seem to run on any CI target, I guess that's due to missing permissions to hook into the kernel.)
ACKs for top commit:
MarcoFalke:
cr ACK f665c6ecda854adaaa629e961b1912847b42fd34
Tree-SHA512: 74f8e398739a25ab5518ff71b998d03d4e529a786ba5b424509de81a511ad3e2e1cd38a5b7bb9f1f5a21340391d6807f4951ff39fa3a2ad65a3b11b989eebea6
05ef059a333479e553382c2ae6ef6fde668ce3cb doc: update windows -fstack-clash-protection doc (fanquake)
Pull request description:
Now that changes have been made in GCC, to fix the build failures.
See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=90458.
ACKs for top commit:
TheCharlatan:
ACK 05ef059a333479e553382c2ae6ef6fde668ce3cb
hebasto:
ACK 05ef059a333479e553382c2ae6ef6fde668ce3cb, I've verified that the fix commit is present in all branches starting from `gcc-11`.
Tree-SHA512: 96b79d65b46e6b9d939c8e6079e984da86987503210106d5155dbe5a6fd82d56d9983694656e27156b01bab795c766b85fc60c799813bc676bba5f3b73f9be22
4da243ba023f2987e97fc62886c6ebc70d6ee50a qt: show own outputs on PSBT signing window (Hernan Marino)
Pull request description:
This fixes https://github.com/bitcoin-core/gui/issues/732 .
It allows you to identify your own addresses in the outputs of a transaction in the PSBT signing window. This enables easy identification of change outputs, and prevents certain attacks where someone (co-signers of a multisig, or others ) might trick you into signing a transaction while they are stealing the change, since prior to this modification there was no easy way of knowing this.
The identification of the output is similar to the way this is done in the transaction details window.
A sample output is :
ACKs for top commit:
achow101:
ACK 4da243ba023f2987e97fc62886c6ebc70d6ee50a
jarolrod:
ACK 4da243ba023f2987e97fc62886c6ebc70d6ee50a
Tree-SHA512: fa9901d2acc84472c11afcd0a59a859db598cdf5cea755b492178d3e7434b70d9bd8f554928938a2ff9920c8f397fef814ce14b416556c30fba0c3c1f62cd722
89ba8905f5c68ae29412f9c4010314c5a113c234 test: indexes, fix on error infinite loop (furszy)
Pull request description:
Coming from https://github.com/bitcoin/bitcoin/pull/28036#issuecomment-1623813703, I thought that we were going to fix it there but seems that got merged without it for some reason.
As index sync failures trigger a shutdown request without notifying `BaseIndex::BlockUntilSyncedToCurrentChain` in any way, we also need to check whether a shutdown was requested or not inside 'IndexWaitSynced'.
Otherwise, any error inside the index sync process will hang the test forever.
ACKs for top commit:
MarcoFalke:
lgtm ACK 89ba8905f5c68ae29412f9c4010314c5a113c234
jamesob:
ACK 89ba890
ryanofsky:
Code review ACK 89ba8905f5c68ae29412f9c4010314c5a113c234. Just comment update since last review
Tree-SHA512: 1f6daf34e51d3fbc802799bfa4ac0ef0d8f774db5f9e2f5d35df18a77679778475c94efc3da1fb723ebaf3583e4075e4a5cbe4a5104ad0c50e2b32076e247b29
fabed7eb796637c02e3677ebbe183d90b258ba69 test: Restore unlimited timeout in IndexWaitSynced (MarcoFalke)
Pull request description:
The timeout was unlimited before, so just restore that value for now: https://github.com/bitcoin/bitcoin/pull/27988#issuecomment-1619218007 .
(Strictly speaking, this is a behavior change for the blockfilterindex and txindex tests, because it only restores the coinstatsindex behavior.)
ACKs for top commit:
ajtowns:
utACK fabed7eb796637c02e3677ebbe183d90b258ba69
mzumsande:
ACK fabed7eb796637c02e3677ebbe183d90b258ba69
furszy:
ACK fabed7eb
Tree-SHA512: 66a878be58bbe53ad8e0c23f05569dd42df688be747551fbd202ada22d20a8285714e58fa2a71664deadb070ddf86cfad88c01042ff95ed26f6b40e4a10cec0a
5fc4939e17509534eb36727b27ac0afb941e44f7 Added static_assert to check that base_blob is using whole bytes. (Brotcrunsher)
Pull request description:
Prior to this commit it was possible to create base_blobs with any arbitrary amount of bits, like base_blob<9>. One could assume that this would be a valid way to create a bit field that guarantees to have at least 9 bits. However, in such a case, base_blob would not behave as expected because the WIDTH is rounded down to the closest whole byte (simple integer division by 8). This commit makes sure that this oddity is detected and blocked by the compiler.
ACKs for top commit:
MarcoFalke:
lgtm ACK 5fc4939e17509534eb36727b27ac0afb941e44f7
theStack:
ACK 5fc4939e17509534eb36727b27ac0afb941e44f7
stickies-v:
ACK 5fc4939e17509534eb36727b27ac0afb941e44f7
Tree-SHA512: 6a06760f09d4a9e6f0b9338d4dddd4091f2ac59a843a443d9302959936d72c55f7cccd55a51ec3a5a799921f68be1b87968ef3c9c11d3389cbd369b5045bb50a
11d650060aed25273d860baa4e03168a778832bb feerate: For GetFeePerK() return nSatoshisPerK instead of round trip through GetFee (Andrew Chow)
Pull request description:
Returning the sats/kvb does not need to round trip through GetFee(1000) since the feerate is already stored as sats/kvb.
Fixes#27913, although this does bring up a larger question of how we should handle such large feerates in fuzzing.
ACKs for top commit:
furszy:
Code ACK 11d65006
Tree-SHA512: bec1a0d4b572a0c810cf7eb4e97d729d67e96835c2d576a909f755b053a9707c2f1b3df9adb8f08a9c4d310cdbb8b1e1b42b9c004bd1ade02a07d8ce9e902138
14405e8d4d259c18a21fc006d0a27550be3171f8 doc: test: update TestShell instructions (ismaelsadeeq)
Pull request description:
Fixes #27904
From #27904 and IRC.
Update [Testshell instructions ](https://github.com/bitcoin/bitcoin/blob/master/test/functional/test-shell.md#2-importing-testshell-from-the-bitcoin-core-repository)
E.g `TestShell.setup()` throws
```
AttributeError: type object 'TestShell' has no attribute 'setup'
```
Parentheses are missing, it should be `TestShell().setup()`
ACKs for top commit:
Sjors:
utACK 14405e8d4d259c18a21fc006d0a27550be3171f8
brunoerg:
crACK 14405e8d4d259c18a21fc006d0a27550be3171f8
hernanmarino:
utACK 14405e8d4d259c18a21fc006d0a27550be3171f8
Tree-SHA512: ffe5fa1103a3b00ef0ee99879adae967b0da07cb8f8451c4c261b0a70b3b666af7aeaacd6f46f85a84ee5e9c7c7ed49700209b5b1f124d7a76efc420ad5c9cd9
fc6c17b83887ef193f2b97264b1843c94dcb915d build: make sure we can overwrite config.{guess,sub} (0xb10c)
Pull request description:
Since ea7b8528 (#26422), `autogen.sh` overwrites the `build-aux/config.{guess, sub}` files (installed there by `autoreconf`) with the `depends/config.{guess, sub}` files if these are newer.
The `autoreconf` tool copies them from it's `share/autoconf/build-aux/` directory. Specifically on NixOS, the `share/autoconf/build-aux/` files are located in the nix-store and are read-only. `autoreconf` preserves the read-only permissions when copying. Overwriting them with our `depends/config.{guess, sub}` files subsequently fails.
To make sure we can overwrite the files, set write permissions to the current user and group before overwriting. This fixes the problem on NixOS.
fixes#27873
ACKs for top commit:
dergoegge:
tACK fc6c17b83887ef193f2b97264b1843c94dcb915d
fanquake:
ACK fc6c17b83887ef193f2b97264b1843c94dcb915d
Tree-SHA512: e8a31f739d5b598b2fe9fe6fc3d02303c117a6adccc49b8d0fea4980027a64f915a0e1e00e4788dce6113ef1b9ec9acf9e4164486f6e4904bad405f20b6746a0
65e3abcbf2b9e818f3b9f1ba35f3cfe7df5e3811 doc: document json rpc endpoints (willcl-ark)
Pull request description:
fixes#20246
This documents the two JSON-RPC endpoints available, details when they are active, specifies when they can or must be used, and outlines some known behaviour quirks.
ACKs for top commit:
fanquake:
ACK 65e3abcbf2b9e818f3b9f1ba35f3cfe7df5e3811 - Seems fine. Can be improved if need be.
Tree-SHA512: d557c2caf000a1bdd7b46c9da38afe63dc28446ba4a961526f1af3cec81d994a9da663e02c81ebdc4f609b794585349cfca77a582dc1e788c120de1d3b4c7db6
a7b46a1feae60e38fe4bdcacf5034f44cae49222 test: added coverage to mining_basic.py (kevkevin)
Pull request description:
Included a test that checks if we call submitblock with block.vtx.empty() then it throws an rpc deserialization error, currently we only test if !block.vtx->IsCoinBase() throws an rpc deserialization error
I've tested to make sure this actually doing what I intended by breaking up this if block into two if blocks with different error messages and running the functional test
322ec63b01/src/rpc/mining.cpp (L963)
This change should increase the test coverage for the `submitblock()` rpc in `./src/rpc/mining.cpp`
ACKs for top commit:
theStack:
ACK a7b46a1feae60e38fe4bdcacf5034f44cae49222
Tree-SHA512: 4078cb1fa879cc9e34438319f73085b521b90a5a95348b23e494cf8e5ac792ec426bc0e1a63e949645e16afebe54c5f35a194f02e20b7273871163d89a5c44e6
4fe5f3c4675263ea106e7ac6d336ec769392ebc3 depends: remove redundant stdlib option (Cory Fields)
Pull request description:
Like #27628, this is another dependency of #21778, though it doesn't become obvious until used with a newer clang.
This should be a no-op.
Use of -stdlib++-isystem gets rid of any system c++ header include paths and negates the need for this option. In newer versions of clangs the combo produces an annoying warning that actually causes problems during configure.
ACKs for top commit:
fanquake:
ACK 4fe5f3c4675263ea106e7ac6d336ec769392ebc3
Tree-SHA512: 904072f2b13dffbbeab2bc9ff20a74969888502fa1ea35d9030784dd397c6751e31233e6ec7dc34e9fd42574950be733b25d6653c2a93e214cc5e4eef2e0133a
c44f3f231988dc05c4c7a8a96bc2e7b1a54da277 test: Explicitly specify directory where to search tests for (Hennadii Stepanov)
Pull request description:
For out-of-source builds, the `test/functional/test_runner.py` is supposed to be run from the build directory which allows it to pick the `test/config.ini` file generated by the build system. Currently, it works accidently for the following reasons:
- on POSIX systems, when running a created by Autoconf symlink to the `test/functional/test_runner.py` in the source directory, it actually has the source directory location in the `sys.path`.
- on Windows (the `build_msvc` directory) VS project puts and copies every build artifact into the source tree (which is wrong and ugly).
This PR makes `test/functional/test_runner.py` work from a build directory in any form (a symbolic link, a hard link, a copy) on _all_ supported platforms, which is highly desirable in the upcoming [CMake-based build system](https://github.com/bitcoin/bitcoin/pull/25797).
For the current master branch, this PR has no behaviour change.
Required for https://github.com/hebasto/bitcoin/pull/15.
---
**Steps to reproduce the issue**
While the issue is mostly specific to Windows and CMake builds, it is still possible to reproduce it on the current master branch.
1. Make an out-of-source build:
```
$ ./autogen.sh
$ mkdir ../build && cd ../build
$ ../bitcoin/configure
$ make
```
2. Note that Autoconf created a symbolic link `test/functional/test_runner.py` in the `../build` directory:
```
$ ls -l test/functional/test_runner.py
lrwxrwxrwx 1 hebasto hebasto 47 May 5 17:40 test/functional/test_runner.py -> ../../../bitcoin/test/functional/test_runner.py
```
which works flawlessly.
3. However, replacing this symbolic link with a hard link or a copy of `test/functional/test_runner.py` from the source tree will cause the following error:
```
$ cp ../bitcoin/test/functional/test_runner.py test/functional/test_runner.py
$ ls -l test/functional/test_runner.py
$ ./test/functional/test_runner.py
Temporary test directory at /tmp/test_runner_₿_🏃_20230505_175104
Running Unit Tests for Test Framework Modules
E
======================================================================
ERROR: test_framework (unittest.loader._FailedTest)
----------------------------------------------------------------------
ImportError: Failed to import test module: test_framework
Traceback (most recent call last):
File "/usr/lib/python3.10/unittest/loader.py", line 154, in loadTestsFromName
module = __import__(module_name)
ModuleNotFoundError: No module named 'test_framework'
----------------------------------------------------------------------
Ran 1 test in 0.000s
FAILED (errors=1)
Early exiting after failure in TestFramework unit tests
```
ACKs for top commit:
stickies-v:
re-ACK c44f3f2319
MarcoFalke:
lgtm ACK c44f3f231988dc05c4c7a8a96bc2e7b1a54da277 💸
Tree-SHA512: 622ff629080a55f76dd4c1dab6699de0e9f06b75da3315cd3b31b972ef4bde746458bf3e8a95e879b3c6a63be2368af70005a83f6a3c85c4f1ba5be51e91a61d
fe434a469534766f18d7560d968deed37193835f bench: Update nanobench to 4.3.11 (TheCharlatan)
Pull request description:
The newest version fixes the false positive `* Turbo is enabled, CPU frequency will fluctuate` warning on AMD CPUs. The file was directly taken from the release page: https://github.com/martinus/nanobench/releases/tag/v4.3.11.
Other changes from the release notes:
* Check for failures in parseFile(), perf events tweaks by tommi-cujo in https://github.com/martinus/nanobench/pull/84
* Workaround missing noexcept for std::string move assignment by tommi-cujo in https://github.com/martinus/nanobench/pull/87
* removed the link by martinus in https://github.com/martinus/nanobench/pull/89
* Lots of minor cleanups by martinus in https://github.com/martinus/nanobench/pull/85
* Add linter for version & clang-format. Updated version by martinus in https://github.com/martinus/nanobench/pull/90
ACKs for top commit:
fanquake:
ACK fe434a469534766f18d7560d968deed37193835f - have not tested.
Tree-SHA512: a8f15e1db1d993673e4b295a3bab22e67ee3c9f3c0bcbef28974fe9ff37dbb741967a526088d5b148c8d25c9d57cd3b844238100c17b23038638787461805678
fabb419a3caafc00fbbc533fee14fab7a5d2a2c6 doc: Clarify that -fstack-reuse=all bugs exist on all versions of GCC (MarcoFalke)
Pull request description:
This is a follow-up to commit 7b850bc2a1cd8547a2dbb5a18173f53439601220. While the test case no longer reproduces, the general class of `-fstack-reuse` bugs still exists in all versions of GCC. The workaround can never be removed, unless the whole class of bugs is fixed.
ACKs for top commit:
fanquake:
ACK fabb419a3caafc00fbbc533fee14fab7a5d2a2c6
Tree-SHA512: 566e14fe82d13dda4f7b8cca90c6de75006d14828906b936780716d5b5b31de9b36a904aa7cfc9820ccdfb4d3224a8437f502f25f7230da5abe87c927123f0c8
ab4efad51b9ba276ffeb6871931e13772493f7cc test: fix immediate tx relay in wallet_groups.py (Sebastian Falbesoner)
Pull request description:
In the functional test wallet_groups.py we whitelist peers on all nodes (`-whitelist=noban@127.0.0.1`) to enable immediate tx relay for fast mempool synchronization. However, considering that this setting only applies to inbound peers and the default test topology looks like this:
```
node0 <--- node1 <---- node2 <--- ... <-- nodeN
```
txs propagate fast only from lower- to higher-numbered nodes (i.e. "left to right" in the above diagram) and take long from higher- to lower-numbered nodes ("right to left") since in the latter direction we only have outbound peers, where the trickle relay is still active. As a consequence, if a tx is submitted from any node other than node0, the mempool synchronization can take quite long.
This PR fixes this by simply adding another connection from node0 to the last node, leading to a ~2-3x speedup (5 runs measured via `time ./test/functional/wallet_groups.py` are shown):
```
master:
0m53.31s real 0m08.22s user 0m05.60s system
0m32.85s real 0m07.44s user 0m04.08s system
0m46.40s real 0m09.18s user 0m04.23s system
0m46.96s real 0m11.10s user 0m05.74s system
0m57.23s real 0m10.53s user 0m05.59s system
PR:
0m19.64s real 0m09.58s user 0m05.50s system
0m18.05s real 0m07.77s user 0m04.03s system
0m18.99s real 0m07.90s user 0m04.25s system
0m17.49s real 0m07.56s user 0m03.92s system
0m18.11s real 0m07.74s user 0m03.88s system
```
Note that in most tests this is not a problem since txs very often originate from node0.
ACKs for top commit:
brunoerg:
utACK ab4efad51b9ba276ffeb6871931e13772493f7cc
Tree-SHA512: 12675357e6eb5a18383f2bfe719a184c0790863b37a98749d8e757dd5dc3a36212e16a81f0a192340c11b793eda00db359c7011f46f7c27e3a093af4f5b62147
fa83005a2618abc18f9cd6af96f19fc28aba68d8 doc: Document affected gcc versions for -fstack-reuse=none workaround (MarcoFalke)
Pull request description:
gcc version(s) 11 and prior won't be fixed, looking at the activity in the bug report. So it seems best to just document gcc 12.1+ as fixed, so that in the future the workaround can be removed once the minimum compiler is gcc12.1.
ACKs for top commit:
fanquake:
ACK fa83005a2618abc18f9cd6af96f19fc28aba68d8
hebasto:
re-ACK fa83005a2618abc18f9cd6af96f19fc28aba68d8
Tree-SHA512: a19723457eb1925196828a5fafd4e7f75a04f86ffae63cb86679d732c662fd1a22e17fe3c69195a97438ff189ba3ff681be3650cf99aa195d7a3e89cd8ee137c
b49e19ccd9a50053c69cd42bae1b44df07890cfd doc: use arch agnostic clang path in fuzzing doc (macOS) (fanquake)
Pull request description:
The current path will only work for clang installed via brew on x86_64 macOS.
ACKs for top commit:
hebasto:
ACK b49e19ccd9a50053c69cd42bae1b44df07890cfd, similar to 702836530ffa351e863b1b1300fd2e559a14ef23.
Tree-SHA512: 8ae4845e1953d5a7178f2b422e2241af1057d8cce1ab79da65df0cd068456dbf85da3489355f81fc4ee09ba602a4b53e989e2dc02476b4abf6c5b3bc3e96473b
82f895d7b540ae421f80305a4f7cbb42905fb2c6 Update nanobench to version v4.3.10 (Martin Leitner-Ankerl)
Pull request description:
Nothing has changed that would affect Bitcoin's usage of nanobench.
Here is a detailed list of the changes
* Plenty of clang-tidy updates
* documentation updates
* faster Rng::shuffle
* Enable perf counters on older kernels
* Raise default minimum epoch time to 1ms (doesn't effect bitcoin's usage)
* Add support for custom information per benchmark
ACKs for top commit:
hebasto:
ACK 82f895d7b540ae421f80305a4f7cbb42905fb2c6, I've reviewed the code, all related changes from #26642 have been implemented.
Tree-SHA512: 942518398809a2794617a347ab8182b784a8e822e84de5af078b2531eabb438412d687cac22a21936585e60e07138a89b41c28c9750744c05a3d1053f55cad01
c497a198db6f417d2612078a9fbc101e259fab33 Fix comment about how wallet txs are sorted (John Moffett)
Pull request description:
The wallet transactions in the node are not sorted by txid (or any hash) since https://github.com/bitcoin/bitcoin/pull/24699.
This is how they're stored in memory now:
835212cd1d/src/wallet/wallet.h (L397-L399)
ACKs for top commit:
achow101:
ACK c497a198db6f417d2612078a9fbc101e259fab33
jarolrod:
ACK c497a198db6f417d2612078a9fbc101e259fab33
Tree-SHA512: e72559991688452ef254474d4235dc75fac655bce04909c3a0eece907360f4c6f57707db9b4373a4bd2271b23c57e863684c33e0728adf48e477c5499cdfdad7
fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d test: Use std::unique_ptr over manual delete in coins_tests (MarcoFalke)
Pull request description:
Makes the code smaller and easier to read
ACKs for top commit:
stickies-v:
ACK fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d
john-moffett:
ACK fab9f7d1bd48198d3e0d3c3a08e404ea73a2bc8d
Tree-SHA512: 30d2d2097906e61fdef47a52fc6a0c5ce2417bc41c3c82dafc1b216c655f31dabf9c1c13759575a696f61bbdfdba3f442be032d5e5145b7a54fae2a927824621
87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd refactor: use `Hash` helper for double-SHA256 calculations (Sebastian Falbesoner)
Pull request description:
We have two helper templates `Hash(const T& in1)` and `Hash(const T& in1, const T& in2)` available for calculating the double-SHA256 hash of one object or two concatenated objects, respectively:
b5868f4b1f/src/hash.h (L74-L89)
This PR uses them in order to increase readability and simplify the code. As in #15294 (which inspired this PR, doing the same for RIPEMD160), the helper is not utilized in validation.cpp and script/interpreter.cpp to avoid touching consensus-relevant code.
ACKs for top commit:
john-moffett:
ACK 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd
stickies-v:
ACK 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd
MarcoFalke:
review ACK 87f11ef47fea31d51bcc3f5df68f78fb28e3d8dd 😬
Tree-SHA512: 11d7e3d00c89685107784010fbffb33ccafb4d1b6a76c4dceb937b29bb234ef4d54581b16bd0737c8d2994a90cf4fe10a9738c7cc5b6d085c6a819f06176dab9
978852aad8e295c26801e17f2ee9e58edd4a5703 build: Fix depends build system when working with subtargets (Hennadii Stepanov)
Pull request description:
On master (f3e0ace8ecd84009a23da6b0de47f01d79c45772), the depends build system does _not_ guarantee that dependencies packages are available for `$(package)_built` target because these dependencies being prepared in `$(host_prefix)` at `$(package)_configured` target can be wiped out during building other package.
Please consider:
```
$ cd depends
$ make clean
$ make fontconfig_configured
$ make
...
CC fcdir.lo
In file included from fcftint.h:26,
from fcdir.c:26:
../fontconfig/fcfreetype.h:27:10: fatal error: ft2build.h: No such file or directory
27 | #include <ft2build.h>
| ^~~~~~~~~~~~
compilation terminated.
make[4]: *** [Makefile:642: fcdir.lo] Error 1
make[4]: *** Waiting for unfinished jobs....
make[4]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94/src'
make[3]: *** [Makefile:503: all] Error 2
make[3]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94/src'
make[2]: *** [Makefile:581: all-recursive] Error 1
make[2]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94'
make[1]: *** [Makefile:465: all] Error 2
make[1]: Leaving directory '/home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94'
make: *** [funcs.mk:288: /home/hebasto/GitHub/bitcoin/depends/work/build/x86_64-pc-linux-gnu/fontconfig/2.12.6-7daa5620c94/./.stamp_built] Error 2
```
The following commands:
```
$ cd depends
$ make clean
$ make qt_configured
$ make
```
also fail.
The similar issue was reported earlier: #21381.
This PR guarantees that dependencies packages are available for `$(package)_built` target.
Guix builds:
```
accef9ffccfe280fec1114c0440092ed5d792e9c53d95abc7fe65435aa136656 guix-build-978852aad8e2/output/aarch64-linux-gnu/SHA256SUMS.part
a75f1250975525a21d2e213e23f1f0dab516d2b28d0c7d747de292fe5c906013 guix-build-978852aad8e2/output/aarch64-linux-gnu/bitcoin-978852aad8e2-aarch64-linux-gnu-debug.tar.gz
d20787af2e7a14a3b7b1d21e0d8784aa6ebad1e916f02aebfa25afe9229ba43c guix-build-978852aad8e2/output/aarch64-linux-gnu/bitcoin-978852aad8e2-aarch64-linux-gnu.tar.gz
11c94a39c084763858c6de31b221868e52554f5500c0dc5589def429bb6b54c8 guix-build-978852aad8e2/output/arm-linux-gnueabihf/SHA256SUMS.part
3935b0e14d78800977dc813a3824215797096b6fb29c84b5996f48338908a65f guix-build-978852aad8e2/output/arm-linux-gnueabihf/bitcoin-978852aad8e2-arm-linux-gnueabihf-debug.tar.gz
c828c3f87a453db443a385a266331661f623f8f4684d88eb006290c83bfa8150 guix-build-978852aad8e2/output/arm-linux-gnueabihf/bitcoin-978852aad8e2-arm-linux-gnueabihf.tar.gz
b6ff14e1cc36e568fc726b50300f77498560322b3582738eb70e7144784f7e63 guix-build-978852aad8e2/output/arm64-apple-darwin/SHA256SUMS.part
2a3e6ba5843eaf9562e9dcfdb4595024a71738079cea00e391558feca4d5bde1 guix-build-978852aad8e2/output/arm64-apple-darwin/bitcoin-978852aad8e2-arm64-apple-darwin-unsigned.dmg
ffd2ad39e8b9f95dd714513ba1e1c77666b0f3cc4b67be4ab763ebd431fe9276 guix-build-978852aad8e2/output/arm64-apple-darwin/bitcoin-978852aad8e2-arm64-apple-darwin-unsigned.tar.gz
5c9f8acd1777effc1e860b64143ba9d06ba5e3d0330e7341529eeae5cc6b3c5e guix-build-978852aad8e2/output/arm64-apple-darwin/bitcoin-978852aad8e2-arm64-apple-darwin.tar.gz
e34c693ecef6159c57fdedabff9dc3d69ec20387966083b828532c58e1e6e30b guix-build-978852aad8e2/output/dist-archive/bitcoin-978852aad8e2.tar.gz
8de4681114d96425bf9b0ccc47d49f696293ead514faa3fa83ddcccfdca2eeb2 guix-build-978852aad8e2/output/powerpc64-linux-gnu/SHA256SUMS.part
d780330a105931eb4c66a536c332eb09e4b6d8c288832bb5a74b931c4600c0b3 guix-build-978852aad8e2/output/powerpc64-linux-gnu/bitcoin-978852aad8e2-powerpc64-linux-gnu-debug.tar.gz
af036a6d714ef362a2facfe4e5c63deaa9dfa391d20542ead9ffb4a43b2b7ca2 guix-build-978852aad8e2/output/powerpc64-linux-gnu/bitcoin-978852aad8e2-powerpc64-linux-gnu.tar.gz
dc6b4365632e7de386ead512b1cdcdd3c985623f63cff8b62974bd9ed8c05d5d guix-build-978852aad8e2/output/powerpc64le-linux-gnu/SHA256SUMS.part
5543a6dec58515186b71139a4a5c603d85638672f205e7c9fabb281c98018461 guix-build-978852aad8e2/output/powerpc64le-linux-gnu/bitcoin-978852aad8e2-powerpc64le-linux-gnu-debug.tar.gz
dd791be2e61ce6cbd3e14c165ce2f8c2d22881992e0df72bd338423d092cc467 guix-build-978852aad8e2/output/powerpc64le-linux-gnu/bitcoin-978852aad8e2-powerpc64le-linux-gnu.tar.gz
2b740db8e5b9c435be3e7b186c3b4a40885302243326ec990e24fe4ba4f777da guix-build-978852aad8e2/output/riscv64-linux-gnu/SHA256SUMS.part
de899c89874d4f34afeca179a6c7c8f49b0a975983ab2b31afd9f2d365674578 guix-build-978852aad8e2/output/riscv64-linux-gnu/bitcoin-978852aad8e2-riscv64-linux-gnu-debug.tar.gz
9052b1db22c56692d99a61c3783b36c6f76537d9aec14f17d87a155beac82532 guix-build-978852aad8e2/output/riscv64-linux-gnu/bitcoin-978852aad8e2-riscv64-linux-gnu.tar.gz
5e79ddf57a94c5978ad819896786107f735d5742bbd042c2c64ae2d0681ce53a guix-build-978852aad8e2/output/x86_64-apple-darwin/SHA256SUMS.part
96443ad839f87c723db1c0a96d8ead0afc69e9d96ad45b5814344866da2dae73 guix-build-978852aad8e2/output/x86_64-apple-darwin/bitcoin-978852aad8e2-x86_64-apple-darwin-unsigned.dmg
14b0a3081772e81a463398a2702aca039d2f276e301dee9f5a0ccffbb09e2749 guix-build-978852aad8e2/output/x86_64-apple-darwin/bitcoin-978852aad8e2-x86_64-apple-darwin-unsigned.tar.gz
6bfb8252524202028308267f5e96bc30f284052f5feaa58ed3697dde27a3130f guix-build-978852aad8e2/output/x86_64-apple-darwin/bitcoin-978852aad8e2-x86_64-apple-darwin.tar.gz
5f8ea6297e246b08ffd806913897cc863feeec6522fcfb4456a59c5f154e0c2d guix-build-978852aad8e2/output/x86_64-linux-gnu/SHA256SUMS.part
40d1bcf491660d54fe20b2db24828ebf61be848eefdc38fba09ed43f6bdba4b1 guix-build-978852aad8e2/output/x86_64-linux-gnu/bitcoin-978852aad8e2-x86_64-linux-gnu-debug.tar.gz
3200e67a4dea115e8e341b4d71d84dc5e8bd2ae35e550cde6aef88d120c65eae guix-build-978852aad8e2/output/x86_64-linux-gnu/bitcoin-978852aad8e2-x86_64-linux-gnu.tar.gz
0b0bf7effc493ecc68398f23fc81647f64fdee115e8ccd7ae91e7881804ec328 guix-build-978852aad8e2/output/x86_64-w64-mingw32/SHA256SUMS.part
e2064c9ddeb4af18468f37ba8cf70004062c31e1387b4cc0fe4b445fae518e8d guix-build-978852aad8e2/output/x86_64-w64-mingw32/bitcoin-978852aad8e2-win64-debug.zip
be347a901b896e0a1dc2f0f5a7f84614075805cccf1f2af8ec8df678d086fdbc guix-build-978852aad8e2/output/x86_64-w64-mingw32/bitcoin-978852aad8e2-win64-setup-unsigned.exe
bab8700e9e266970e8c7cad494902058ad12d1f2a6462e0039daa637b1a0ce0d guix-build-978852aad8e2/output/x86_64-w64-mingw32/bitcoin-978852aad8e2-win64-unsigned.tar.gz
c8e55e64b248fd7c9056fe811a1eba992bbb92e44857204e3024416d9ba6307d guix-build-978852aad8e2/output/x86_64-w64-mingw32/bitcoin-978852aad8e2-win64.zip
```
ACKs for top commit:
TheCharlatan:
tACK 978852aad8e295c26801e17f2ee9e58edd4a5703
Tree-SHA512: c195484274433039e327d44b1949afa296e09e7470a2b138b7a8476c8bf9c1302bc21284cd5436f09aa97824aae9f362b7932ff2937b78f79df0b43e50f3dfaa
9ab62d71fb1a54430ff5071bdb1120a414061288 [fuzz] Actually use mocked mempool in tx_pool target (dergoegge)
Pull request description:
The current tx_pool target uses the default mempool, making the target non-deterministic. This PR replaces the active chainstate's mempool (i.e. the node's default mempool) with the already present mocked mempool in the target.
ACKs for top commit:
fanquake:
ACK 9ab62d71fb1a54430ff5071bdb1120a414061288
Tree-SHA512: fe9af3dbdd13cb569fdc2ddbb4290b5ce94206ae83d94267c6365ed0ee9bbe072fcfe7fd632a1a8522dce44608e89aba2f398c1e20bd250484bbadb78143320c
376e01b382679b49d5c8464c869d14eca23ab4b9 doc: add databases/py-sqlite3 to FreeBSD test suite deps (fanquake)
Pull request description:
Adds missing documentation. See also https://cirrus-ci.com/task/5639240319500288.
ACKs for top commit:
john-moffett:
ACK 376e01b382679b49d5c8464c869d14eca23ab4b9
Tree-SHA512: a7b8d0bae00c3538934d23eae207b7927a64e748eb228ac6c5754aee0e9b6796c0f39066c7d81cc009bf442c8b1a0c31739adde87d1ebc3445aed54dda47c9ce
6d0ab07e817725369df699791b9c5b2fae204990 refactor: use convenience fn to auto parse non-string parameters (stickies-v)
Pull request description:
Minimizes code duplication and improves function naming by having a single (overloaded) convenience function `ParseIfNonString` that both checks if the parameter is a non-string parameter and automatically parses the value if so.
ACKs for top commit:
aureleoules:
ACK 6d0ab07e817725369df699791b9c5b2fae204990
Tree-SHA512: 8cbf68a17cfbdce1e31a19916f447a2965c139fdef00c19e32a9b679f4a4015dfe69ceea0bbe1723711e1c5033ea8d4005d1f4485dfbeea22226140f8cbe8aa3
c6119f478876f245ca5c65dd05da4cdc5be0e91f tests: Use unique port for ZMQ tests (Andrew Chow)
Pull request description:
The ZMQ interface tests should use unique ports as we do for the p2p and rpc ports so that multiple instances of the test can be run at the same time.
Without this, the test may hang until killed, or fail.
ACKs for top commit:
MarcoFalke:
ACK c6119f478876f245ca5c65dd05da4cdc5be0e91f
Tree-SHA512: 2ca3ed2f35e5a83d7ab83740674fed362a8d146dc751156cfe100133a591347cd1ac9d164046f1744d65451a57c52cb22d3bb2161105f421f8f655c4a2512c59
affbf58a1e52a8e60c830be6a9e0347e0ff4c28e build: Move environment variables into `$(package)_config_env` (Hennadii Stepanov)
d44fcd3c976572883bbf7f386bc88e2610dc1a58 build: Make $(package)_*_env available to all $(package)_*_cmds (Hennadii Stepanov)
Pull request description:
On master (1e7564eca8a688f39c75540877ec3bdfdde766b1) the depends build system, which is based on pure GNU Make, works, but it lacks robustness, and in some corner cases it fails. For example, see bitcoin/bitcoin#22552.
Another [bug](https://github.com/bitcoin/bitcoin/issues/22719) in the depends build system has already become a problem at least two times in the past (https://github.com/bitcoin/bitcoin/pull/16883#issuecomment-683817472 and https://github.com/bitcoin/bitcoin/pull/24134). Each time the problem was solved with other means.
The initial [solution](https://github.com/bitcoin/bitcoin/pull/19882) had some discussion. Also it was discussed on the IRC meeting in #bitcoin-core-builds channel. This PR, actually, is a resurrection of it, as the bug silently struck pretty [recently](https://github.com/bitcoin/bitcoin/pull/24134).
The bug is well described in bitcoin/bitcoin#22719.
Here is another, a bit simpler description, which requires only basic shell (bash, dash etc) experience.
After creating targets by this code:1e7564eca8/depends/funcs.mk (L280) a "draft" line of recipe like `$($(1)_config_env) $(call $(1)_config_cmds, $(1))` becomes a shell command sequence `VAR1=foo VAR2=bar command1 && command2` which is supposed to be executed in a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution).
Please note that `VAR1=foo VAR2=bar` part is visible for the first `command1` only (for details see shell docs). Example:
```sh
$ VAR1="foo" VAR2="bar" echo "begin" && printenv VAR1 && printenv VAR2 && echo "end"
begin
$ echo $?
1
```
Using the `export` command is a trivial solution:
```sh
$ export VAR1="foo" VAR2="bar"; echo "begin" && printenv VAR1 && printenv VAR2 && echo "end"
begin
foo
bar
end
$ echo $?
0
```
As a [new sub-shell](https://www.gnu.org/software/make/manual/html_node/Execution.html#Execution) is invoked for each line of the recipe, there are no side effects of using `export`. It means this solution should not be considered invasive.
Fixesbitcoin/bitcoin#22719.
---
Also this PR removes no longer needed crutch from `qt.mk`.
ACKs for top commit:
fanquake:
ACK affbf58a1e52a8e60c830be6a9e0347e0ff4c28e
Tree-SHA512: 0ce2cf82870a7774bdf1592fac50857126ae47da902e349f1092d50109223be9d6a8efd5e92ec08c2ca775b17516482aabaf232378950ade36484a883acc177b
31d0067f8bccd6b090bf88bad8472bb5cb1f20a4 doc: test: update/fix TestShell example instructions (Sebastian Falbesoner)
Pull request description:
This PR tackles two issues in the TestShell documentation:
- add missing instruction for creating a wallet prior to the `getnewaddress` call (needed as there is no default wallet created anymore since v0.21)
- fix `generatetoaddress` call syntax (the scripted-diff in commit fa0b916971e5bc23ad6396831940a2899ca05402 only worked for tests using `BitcoinTestFramework`)
ACKs for top commit:
fanquake:
ACK 31d0067f8bccd6b090bf88bad8472bb5cb1f20a4 - current instructions don't work. These do.
Tree-SHA512: d2b7808a06892ad16728cb2b6d4a72b255ad711d27fe98b1de562f80444e7bb25d73296abdde4308162fe3be702864e2f7b7dbbbb000fe54c709951c09e6c730
fa3ea81c3e7d962715788ab5525958a532c51414 refactor: Add LIFETIMEBOUND / -Wdangling-gsl to Assert() (MacroFake)
Pull request description:
Currently compiles clean, but I think it may still be useful.
Can be tested by adding an `&`:
```diff
diff --git a/src/test/util_tests.cpp b/src/test/util_tests.cpp
index 5766fff92d..300c1ec60f 100644
--- a/src/test/util_tests.cpp
+++ b/src/test/util_tests.cpp
@@ -125,7 +125,7 @@ BOOST_AUTO_TEST_CASE(util_check)
// Check -Wdangling-gsl does not trigger when copying the int. (It would
// trigger on "const int&")
- const int nine{*Assert(std::optional<int>{9})};
+ const int& nine{*Assert(std::optional<int>{9})};
BOOST_CHECK_EQUAL(9, nine);
}
```
Output:
```
test/util_tests.cpp:128:29: warning: object backing the pointer will be destroyed at the end of the full-expression [-Wdangling-gsl]
const int& nine{*Assert(std::optional<int>{9})};
^~~~~~~~~~~~~~~~~~~~~
./util/check.h:75:50: note: expanded from macro 'Assert'
#define Assert(val) inline_assertion_check<true>(val, __FILE__, __LINE__, __func__, #val)
^~~
1 warning generated.
ACKs for top commit:
jonatack:
ACK fa3ea81c3e7d962715788ab5525958a532c51414
theuni:
ACK fa3ea81c3e7d962715788ab5525958a532c51414
Tree-SHA512: 17dea4d75f2ee2bf6e1b6a6f6d8f439711c777df0390574e8d8edb6ac9ee807a135341e4439050bd6a15ecc4097a1ba9a7ab15d27541ebf70a4e081fa6871877
5c9a27a46f4b07f872f850f7e918c10a33595cfd test: Use proper Boost macros instead of assertions (Hennadii Stepanov)
Pull request description:
On the master branch:
```
$ src/test/test_bitcoin -l test_suite -t banman_tests
Running 1 test case...
...
Test case banman_tests/file did not check any assertions
...
```
This PR suggests to use proper Boost [macros](https://www.boost.org/doc/libs/1_80_0/libs/test/doc/html/boost_test/utf_reference/testing_tool_ref.html).
Top commit has no ACKs.
Tree-SHA512: e0c8e5e6371acd0e0a80070fffdf1445f264c62499f8d9811822994c89735a913c18c8ed730495578400abdd93d2d500345504f2a9246401d53fb2f9f71be8c5
0dbafcee46 Merge bitcoin/bitcoin#27289: Refactor: Remove unused FlatFilePos::SetNull (fanquake)
dbe2e04d62 Merge bitcoin/bitcoin#27212: test: Make the unlikely race in p2p_invalid_messages impossible (fanquake)
6f6b718f78 Merge bitcoin/bitcoin#27236: util: fix argsman dupe key error (fanquake)
74c6e38530 Merge bitcoin/bitcoin#27205: doc: Show how less noisy clang-tidy output can be achieved (fanquake)
9e552f0293 Merge bitcoin/bitcoin#27232: Use string interpolation for default value of -listen (fanquake)
2a39b93233 Merge bitcoin/bitcoin#27226: test: Use self.wait_until over wait_until_helper (fanquake)
be2e16f33a Merge bitcoin/bitcoin#27192: util: add missing include and fix function signature (fanquake)
176a4a60d2 Merge bitcoin/bitcoin#27173: valgrind: remove libsecp256k1 suppression (fanquake)
d2fc8be331 Merge bitcoin/bitcoin#27154: doc: mention sanitizer suppressions in developer docs (glozow)
f5b4cc7e32 Merge bitcoin/bitcoin#16195: util: Use void* throughout support/lockedpool.h (Andrew Chow)
c66c0fdbf8 Merge bitcoin/bitcoin#27137: test: Raise PRNG seed log to INFO (fanquake)
bba215031b Merge bitcoin/bitcoin#25950: test: fix test abort for high timeout values (and `--timeout-factor 0`) (fanquake)
6751add2ea Merge bitcoin/bitcoin#27107: doc: remove mention of "proper signing key" (merge-script)
34c895a542 Merge bitcoin/bitcoin#26997: psbt: s/transcation/transaction/ (fanquake)
befdbeddf9 Merge bitcoin/bitcoin#27097: descriptors: fix docstring (param [in] vs [out]) (fanquake)
c98dd824b6 Merge bitcoin/bitcoin#27080: Wallet: Zero out wallet master key upon locking so it doesn't persist in memory (Andrew Chow)
Pull request description:
## Issue being fixed or feature implemented
Batch of trivial backports
## What was done?
See commits
## How Has This Been Tested?
built locally; large combined merge passed tests locally
## Breaking Changes
Should be none
## Checklist:
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have added or updated relevant unit/integration/functional/e2e tests
- [ ] I have made corresponding changes to the documentation
- [x] I have assigned this pull request to a milestone _(for repository code-owners and collaborators only)_
ACKs for top commit:
knst:
utACK 0dbafcee46
UdjinM6:
utACK 0dbafcee46
Tree-SHA512: e93a1136e848aa6c6f3d9fb0567b3e284975d35e82bbc1d9a8cd908067df4bf1257c939882abcaca6820360a4982b991f505a1165d95e1a8b52c9b181b7026b7
BACKPORT NOTE:
Missing changes in src/fs.cpp are removed in bitcoin/bitcoin#20744 which is already backported
b367745cfe19f6de3f44b3adc90fa08e36e44bb6 ci: Make Cirrus CI Windows build with --enable-werror (Hennadii Stepanov)
c713bb2b243881a771ab288340ffeb623c82d7f6 Fix Windows build with --enable-werror on Ubuntu Focal (Hennadii Stepanov)
Pull request description:
This PR makes possible to cross-compile Windows build with `--enable-werror --enable-suppress-external-warnings`.
Some problems are fixed, others are silenced.
Also `--enable-werror` is enabled for Cirrus CI Windows build (the last one on Cirrus CI without `--enable-werror`).
ACKs for top commit:
practicalswift:
cr ACK b367745cfe19f6de3f44b3adc90fa08e36e44bb6: patch looks correct
laanwj:
Code review ACK b367745cfe19f6de3f44b3adc90fa08e36e44bb6
vasild:
ACK b367745cfe19f6de3f44b3adc90fa08e36e44bb6
jarolrod:
ACK b367745cfe19f6de3f44b3adc90fa08e36e44bb6
Tree-SHA512: 64f5c99b7dad4c0efce80cd45d7074f275bd8411235dc9e0841287bdab64b812c6f8f9d632c35531d0b8210148531f53aaaac77be7699b29d2d6aaae304dbee0
62b125fd197879f112322a1f67a318d6ab22e67a qt, refactor: Fix indentation (Prateek Sancheti)
ad28b66e98c9bb3bc7af2545654842544a798601 qt: Add SubFeeFromAmount option (Prateek Sancheti)
Pull request description:
This PR adds **_SubFeeFromAmount_** option which lets the user select their preferred setting of whether fee for a transaction is to be subtracted from the amount or not for future transactions. The setting chosen by the user is remembered even when the GUI mode is turned off.
**_Functionality and Usage:_**
- Go to `Settings > Options > Wallet` on _Windows/Linux_ or `bitcoin-qt > Preferences > Wallet` on _macOS_.
- The checkbox **Subtract Fee From Amount** corresponds to the added option **SubFeeFromAmount**.
- The preferred setting intended to be the default for all future send transactions should be selected by the user.
- Click on **OK**.
- Go to the **Send** tab in the wallet.
- You shall notice, any new Send transaction created will have the preferred setting as chosen by the user.<br> (Try clicking on Add recipient or even restarting the Node in GUI)
Attaching ScreenRecordings to explain the added feature.
> Master.mov: Master Branch
https://user-images.githubusercontent.com/54016434/127763378-be91837d-d0ab-4ae5-87c0-d303fa70a336.mov
> PR.mov: PullRequest
https://user-images.githubusercontent.com/54016434/127763404-05b834c1-4082-4fbd-9b05-1528ac898a21.movClose#386
ACKs for top commit:
Talkless:
tACK 62b125fd197879f112322a1f67a318d6ab22e67a, tested on Debian Sid with 5.15.2 and it works as described.
hebasto:
re-ACK 62b125fd197879f112322a1f67a318d6ab22e67a, only removed the unused `SubFeeFromAmountChanged` signal since my [previous](https://github.com/bitcoin-core/gui/pull/390#pullrequestreview-726531766) review.
meshcollider:
utACK 62b125fd197879f112322a1f67a318d6ab22e67a
Tree-SHA512: 932ca89ae578a1e1c426561400d87cf005c231944feaf0f662ff8d88f32bdd65a927a090ea41510a15f8ec0ebcd5529672e9917720eb5ea85f413f081e45d5bb
8169fc4e73a87331e02502fc24e293831765c8b1 qt, refactor: Fix code styling of moved InitExecutor class (Hennadii Stepanov)
c82165a55701fe4ff604d7f30163051cd47c2363 qt, refactor: Move InitExecutor class into its own module (Hennadii Stepanov)
dbcf56b6c6e939923673b3f07bed7bb3632dbeb1 scripted-diff: Rename BitcoinCore class to InitExecutor (Hennadii Stepanov)
19a1d008310f250b69b7aa764a9f26384d5a4a85 qt: Add BitcoinCore::m_thread member (Hennadii Stepanov)
Pull request description:
This PR makes the `BitcoinCore` class reusable, i.e., it can be used by the widget-based GUI or by the [QML-based](https://github.com/bitcoin-core/gui-qml/tree/main/src/qml) one, and it makes the divergence between these two repos minimal.
The small benefit to the current branch is more structured code.
Actually, this PR is ported from https://github.com/bitcoin-core/gui-qml/pull/10.
The example of the re-using of the `BitcoinCore` class is https://github.com/bitcoin-core/gui-qml/pull/11.
ACKs for top commit:
laanwj:
ACK 8169fc4e73a87331e02502fc24e293831765c8b1
ryanofsky:
Code review ACK 8169fc4e73a87331e02502fc24e293831765c8b1. Only change is switching from `m_executor` from pointer to optional type (thanks for update!)
Tree-SHA512: a0552c32d26d9acf42921eb12bcdf68f02d52f7183c688c43257b1a58679f64e45f193ee2d316850c7f0f516561e17abe989fe545bfa05e158ad3f4c66d19bca
4c43b7d41d11072f382f938379d21cd2e0bcbb47 contrib: use hkps://keys.openpgp.org to retrieve builder keys (fanquake)
Pull request description:
`hkps://hkps.pool.sks-keyservers.net` is essentially no-longer functional,
and a number of distributions and GPG tools have since switched to using
the `keys.openpgp.org` key server as their default.
See this Debian patch for additional context:
https://salsa.debian.org/debian/gnupg2/-/blob/debian/main/debian/patches/Use-hkps-keys.openpgp.org-as-the-default-keyserver.patch
Switch to using keys.openpgp.org in the CI as well.
ACKs for top commit:
MarcoFalke:
cr ACK 4c43b7d41d11072f382f938379d21cd2e0bcbb47
Zero-1729:
ACK 4c43b7d41d11072f382f938379d21cd2e0bcbb47
Tree-SHA512: e6c72b67778b76f81c659eee0e4195fea9e579587c64921affd35b9d46a077d4e8754b7fb85ca90a9a4bbc5cd5a47b0c6e4c9dbf9a335418a12f774d665e5a19
fabb72b contrib: Remove xpired 522739F6 key (MarcoFalke)
faeab66 contrib: Replace developer keys with list of pgp fingerprints (MarcoFalke)
Pull request description:
Having to host a copy of the keys in this repo was a common source of discussion and distraction, caused by problems such as:
* Outdated keys. Unclear whether and when to replace by fresh copies.
* Unclear when to add a key of a new developer or Gitian builder.
The problems are solved by
* Having no keys but only the fingerprints
* Adding a rule of thumb, when to add a new key
<strike>Moving the keys to a different repo solves none of these issues, but since the keys are not bound to releases or git branches of Bitcoin Core, they should live somewhere else.
Obviously, all keys are hosted and distributed on key servers, but were added to the repo solely for convenience and redundancy.
Moving the mirror of those keys to a different repo makes it less distracting to update them -- let's say -- prior to every major release.
I updated our `doc/release-process.md` to reflect the new location.
DEPENDS_ON https://github.com/bitcoin-core/gitian.sigs/pull/621
</strike>
Tree-SHA512: c00795a07603190e26dc4526f6ce11e492fb048dc7ef54b38f859b77dcde25f58ec4449f5cf3f85a5e9c2dd2743bde53f7ff03c8eccf0d75d51784a6b164e47d
bb822a7af86897a9b6a5d616f193c258e8e76729 wallet, rpc: add listdescriptors private option (S3RK)
Pull request description:
Rationale: make it possible to backup your wallet with `listdescriptors` command
* The default behaviour is still to show public version
* For private version only the root xprv is returned
Example use-case:
```
> bitcoin-cli -regtest -named createwallet wallet_name=old descriptors=true
> bitcoin-cli -regtest -rpcwallet=old listdescriptors true | jq '.descriptors' > descriptors.txt
> bitcoin-cli -regtest -named createwallet wallet_name=new descriptors=true blank=true
> bitcoin-cli -regtest -rpcwallet=new importdescriptors "$(cat descriptors.txt)"
```
In case of watch-only wallet without private keys there will be following output:
```
error code: -4
error message:
Can't get descriptor string.
```
ACKs for top commit:
achow101:
re-ACK bb822a7af86897a9b6a5d616f193c258e8e76729
Rspigler:
tACK bb822a7af86897a9b6a5d616f193c258e8e76729
jonatack:
ACK bb822a7af86897a9b6a5d616f193c258e8e76729 per `git diff 2854ddc bb822a7`
prayank23:
tACK bb822a7af8
meshcollider:
Code review ACK bb822a7af86897a9b6a5d616f193c258e8e76729
Tree-SHA512: f6dddc72a74e5667071ccd77f8dce578382e8e29e7ed6a0834ac2e114a6d3918b59c2f194f4079b3259e13d9ba3b4f405619940c3ecb7a1a0344615aed47c43d
607076d01bf23c69ac21950c17b01fb4e1130774 test: remove confusing `MAX_BLOCK_BASE_SIZE` (Sebastian Falbesoner)
4af97c74edcda56cd15523bf3a335adea2bad14a test: introduce `get_weight()` helper for CBlock (Sebastian Falbesoner)
a084ebe1330bcec15715e08b0f65319142927ad1 test: introduce `get_weight()` helper for CTransaction (Sebastian Falbesoner)
Pull request description:
This is a very late follow-up PR to #10618, which removed the constant `MAX_BLOCK_BASE_SIZE` from the core implementation about four years ago (see also #10608 in why it was considered confusing and superfluous).
Since there is also no point in still keeping it in the functional test framework, the PR switches to weight-based accounting on the relevant test code parts and use `MAX_BLOCK_WEIGHT` instead for the block limit checks. To prepare that, the first two commits introduce `get_weight()` helpers for the classes CTransaction and CBlock, respectively.
ACKs for top commit:
MarcoFalke:
review ACK 607076d01bf23c69ac21950c17b01fb4e1130774 🚴
Tree-SHA512: d59aa0b6b3dfd0a849b8063e66de275d252f705f99e25cd3bf6daec028b47d946777ee5b42a060f5283cb18e917ac073119c2c0e11bbc21211f69ef0a6ed335a
