Use raw string
Use re.search instead of grep in check_matching_test_names
Replaced bash commands in check_unique_test_names with python commands
Use set and sort output
Use set comprehension
Use .splitlines()
Call grep_boost_fixture_test_suite once
splitlines() once
Fixed copyright date
Use check_output() instead of run()
add encoding='utf8'
Use clearer code for getting duplicates
I have some qualms with maintaining a suspicious hosts list as part as
the repository. But also, it's stale and irrelevant. I've checked the
entire list and none of them is connectable. Only one still appars in
`nodes_main.txt` but with low uptime and an old subversion string so it
wouldn't be picked in the first place.
3eaf5dbfe0 qt: Remove `QApplication::globalStrut()` call (Hennadii Stepanov)
Pull request description:
This function has been deprecated in Qt 5.15.0, and has been [removed](033d01bd6e) in Qt 6.
ACKs for top commit:
jarolrod:
ACK 3eaf5dbfe0
luke-jr:
utACK 3eaf5dbfe0
Tree-SHA512: 71ee539b6ffa3755f7e6beaa72a8937886471e298830878def6dd9f48c601611d94d52c638bc1602f938df2ba84ff8b130ea8da8e6c08ae7146173fa613a5003
0e5dedbc9e qt/wallettests: sort includes (William Casarin)
0554251d66 qt: Skip displayUnitChanged signal if unit is not actually changed (Hennadii Stepanov)
ffbc2fe459 qt, refactor: Remove default cases for scoped enum (Hennadii Stepanov)
152d5bad50 qt, refactor: Remove BitcoinUnits::valid function (Hennadii Stepanov)
aa23960fdf qt, refactor: Make BitcoinUnits::Unit a scoped enum (Hennadii Stepanov)
75832fdc37 qt: Use QVariant instead of int for BitcoinUnit in QSettings (Hennadii Stepanov)
Pull request description:
This is a rebased version of #60
Since Qt 5.5 there are [means](https://doc.qt.io/qt-5/qobject.html#Q_ENUM) to register an enum type with the meta-object system (such enum still lacks an ability to interact with [QSettings::setValue()](https://doc.qt.io/qt-5/qsettings.html#setValue) and [QSettings::value()](https://doc.qt.io/qt-5/qsettings.html#value) without defined stream operators).
In order to reduce global namespace polluting and to force strong type checking, this PR makes BitcoinUnits::Unit a scoped enum (typedef BitcoinUnits::Unit BitcoinUnit;).
No behavior change.
ACKs for top commit:
jonatack:
ACK 0e5dedbc9e, review and debug build of each commit after rebase on current master, lightly tested running the GUI, changing units a few times, and verifying persistence after restarting
promag:
Code review ACK 0e5dedbc9e
Tree-SHA512: 39ec0d7e4f0b9b25be287888121a8db6b282339674e37ec3a3554da63a9e22d6fe079e8310ca289b2a0356a19b3c7e55afa17d09dd34e0f222177f603bb053a3
343f83d088 qt, refactor: Use member initializers in TransactionStatus (w0xlt)
66d58ad7a9 qt, refactor: remove unused field `qint64 TransactionStatus::open_for` (w0xlt)
ad6adedb46 qt, refactor: remove unused parameters in `TransactionDesc::FormatTxStatus()` (w0xlt)
045f8d0310 scripted-diff: rename nDepth -> depth (w0xlt)
b1bc1431db qt, refactor: remove redundant scope in `TransactionDesc::FormatTxStatus()` (w0xlt)
Pull request description:
This PR implements the changes suggested in https://github.com/bitcoin-core/gui/issues/538#issuecomment-1021913294 .
. remove redundant scope, rename `nDepth` -> `depth`, remove unused parameters and add translator comments in `TransactionDesc::FormatTxStatus()`
. Use member initializers and remove unused field `qint64 TransactionStatus::open_for` in `TransactionStatus`.
Closes https://github.com/bitcoin-core/gui/issues/538
ACKs for top commit:
hebasto:
ACK 343f83d088, I have reviewed the code and it looks OK, I agree it can be merged.
jarolrod:
Code Review ACK 343f83d088
Tree-SHA512: cc7333d85b7eb731aa8cdd2d8dfc707341532c93e1b5e3858e8341446cf055ba055b601f9662e8d4602726b1bedf13149c46256a60a0ce1a562f94c9986d945a
b2c3d1086b Revert "build: Specify `zeromq` port explicitly for MSVC builds" (Hennadii Stepanov)
ca40d6432e ci: Bump vcpkg to the latest version (Hennadii Stepanov)
5561f17e37 ci: Make Qt configure option change invalidate "msvc_qt_built_cache" (Hennadii Stepanov)
Pull request description:
Dependency changes in vcpkg [`2022.04.12`](https://github.com/microsoft/vcpkg/releases/tag/2022.04.12):
- zeromq 4.3.4#4 -> 4.3.4#5
This allows to revert our [workaround](20b6c87117) because of an upstream [patch](https://github.com/microsoft/vcpkg/pull/23435).
ACKs for top commit:
sipsorcery:
ACK b2c3d1086b.
Tree-SHA512: aa7e43b4b53e8b3c9900e4770360504371e62ff6a1765ac7eae612f2c2febc0430359e3d6032ece93b4f818cb6ab7141506ae0b90b44ed378d2eef63ca805178
c457fb144c improve clarity and up max ipv6 ASNs (Baas)
Pull request description:
This PR attempts to address some of the areas of improvement raised in #17020 . Concretely, my proposed change is fairly minor but addresses the following changes to [`makeseeds.py`](https://github.com/bitcoin/bitcoin/blob/master/contrib/seeds/makeseeds.py):
- Increase max seeds per ASN for IPv6 to 10 as recommended [here](https://github.com/bitcoin/bitcoin/pull/16999#issuecomment-536999544), while keeping max seeds per ASN for IPv4 at 2.
- Bump `MIN_BLOCKS` to 730000.
- Improved script clarity: added function types and more docs to functions, added progress indicator when performing ASN lookup, and change string formatting to better align with [bitcoin python style guidelines](https://github.com/bitcoin/bitcoin/blob/master/test/functional/README.md#style-guidelines)
With the different ASN limits for IPv4 and IPv6, and the new minimum block requirement, the current stats look look like:
```
IPv4 IPv6 Onion Pass
470689 73238 0 Initial
470689 73238 0 Skip entries with invalid address
470689 73238 0 After removing duplicates
470688 73238 0 Skip entries from suspicious hosts
6098 1676 0 Enforce minimal number of blocks
5252 1443 0 Require service bit 1
3812 898 0 Require minimum uptime
3738 877 0 Require a known and recent user agent
3715 869 0 Filter out hosts with multiple bitcoin ports
512 512 0 Look up ASNs and limit results per ASN and per net
```
The new ASN max seeds of 10 allows for 512 IPv6 addresses to be included, up from the ~150 that was filtered by the previous version.
While there is more to do for #17020 , these changes I think are fairly isolated from the rest and should make it a bit easier for others to get up to speed with what the functions in the script do.
ACKs for top commit:
laanwj:
Concept and code review ACK c457fb144c
Tree-SHA512: 3ed67868443cc50544e23b27e2341758c3a8866997b0dba47b137032d5e1a13428855daaeed682626ed471542b44435635178d54848a2cd6fe73777679428032
In the current code, the database is created before the last validation,
which checks that passphrase is set and private keys are disabled.
Therefore, if this validation fails, it will result in an empty database
and the user will not be able to recreate a wallet with the same name
and with the correct parameters.
86c2889518 ci: Make log verbose in error case only (Hennadii Stepanov)
7f650883b7 depends: Add file-based logging for individual packages (Hennadii Stepanov)
Pull request description:
This PR adds file-based logging for individual packages in depends. To use this feature one should provide `LOG=1`.
A log file is printed out automatically in case of a build error. After successful build log files are being moved along with package archives:
```
$ make -C depends HOST=x86_64-w64-mingw32 LOG=1
$ find ./depends/built/x86_64-w64-mingw32 -name '*.log' | sort
./depends/built/x86_64-w64-mingw32/bdb/bdb-4.8.30-5100a099801.log
./depends/built/x86_64-w64-mingw32/boost/boost-1_71_0-313f82dc7de.log
./depends/built/x86_64-w64-mingw32/libevent/libevent-2.1.12-stable-3fa27048d5e.log
./depends/built/x86_64-w64-mingw32/libnatpmp/libnatpmp-4536032ae32268a45c073a4d5e91bbab4534773a-9db4850dd32.log
./depends/built/x86_64-w64-mingw32/miniupnpc/miniupnpc-2.2.2-75d9a1807e0.log
./depends/built/x86_64-w64-mingw32/native_b2/native_b2-1_71_0-3bf253c19bf.log
./depends/built/x86_64-w64-mingw32/qrencode/qrencode-3.4.4-dfac87af599.log
./depends/built/x86_64-w64-mingw32/qt/qt-5.15.2-9304e03d3ac.log
./depends/built/x86_64-w64-mingw32/sqlite/sqlite-3320100-455acafa7be.log
./depends/built/x86_64-w64-mingw32/zeromq/zeromq-4.3.1-5ff627ec84a.log
```
An example of CI tasks with package build errors -- https://cirrus-ci.com/task/5275741788045312Closes#16368.
ACKs for top commit:
laanwj:
Tested ACK 86c2889518
Tree-SHA512: 497f2146fd2e38c952124aecfd80ebb42be22bbc5dc59521491545f4465fc38f23da7787a0caea5686b7c30aa862f2b0c02092ae3fe863e80a5ddd14b3d324b9
038d2a607f test: add test for signet miner script (Sebastian Falbesoner)
449b96ed97 test: add `is_bitcoin_util_compiled` helper (Sebastian Falbesoner)
dde33eca63 test: determine path to `bitcoin-util` in test framework (Sebastian Falbesoner)
Pull request description:
This PR adds a very basic test for the signet miner script (contrib/signet/miner). ~~It was based on #24553 (merged by now) which fixes a bug (and was also the motivation to write this test).~~
The test roughly follows the steps from https://en.bitcoin.it/wiki/Signet#Custom_Signet, except that the challenge key-pair is created solely with the test framework. Calibration is also skipped, the difficulty is simply set to the first mainnet target `0x1d00ffff` (see also https://bitcoin.stackexchange.com/a/57186).
ACKs for top commit:
laanwj:
re-ACK 038d2a607f
Tree-SHA512: 150698a3c0cda3679661b47688e3b932c9761e777fdd284776b867b485db6a8895960177bd02a53f838a4c9b9bbe6a9beea8d7a5b14825b38e4e43b3177821b3
When we use only manually specified inputs, we should still calculate
the waste so that if anything later on calls GetWaste (in order to log
it), there won't be an error.
c71117fcb0 net: remove non-blocking bool from interface (Bushstar)
Pull request description:
SetSocketNonBlocking was added in 0.11 in the PR below with a second argument to toggle non-blocking mode for the socket. That argument has always been set to true in all subsequent releases and I'm not sure why it is present.
https://github.com/bitcoin/bitcoin/pull/4491
ACKs for top commit:
promag:
Code review ACK c71117fcb0.
lsilva01:
Code review ACK c71117fcb0
vasild:
ACK c71117fcb0
Tree-SHA512: feebfcfa75d997460a0ba42ffe1e0c25a7e0bfcad12510ad73ea4942cc1c653f9ad429adbbb00b9288fe319009552906fcb635a14dfd7dcbde3853baab6be065
fa32cc0682 doc: Remove fee delta TODO from txmempool.cpp (MarcoFalke)
Pull request description:
This refactor request was added in commit eb306664e7, though it didn't explain why the refactor is needed and what the goal is. Given that this wasn't touched for more than 5 years, it doesn't seem critical. Generally, non-trivial `TODO`s make more sense as GitHub issues, so that they can be discussed and triaged more easily.
ACKs for top commit:
laanwj:
Code review ACK fa32cc0682
Tree-SHA512: 6629fef543e815136c82c38aa8ba2c4de68a5fe94c6954f2559e468f7e59052e02dd7c221d3b159be0314eaf0dbb18f74814297c58f76e2289c47e8d4f49be4e
457148a803 guix: fix GCC 10.3.0 + mingw-w64 setjmp/longjmp issues (fanquake)
Pull request description:
This commit backports [a patch](https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e8d1ca7d2c344a411779892616c423e157f4aea8) to the GCC 10.3.0 we build for Windows
cross-compilation in Guix. The commit has been [backported to the GCC
releases/gcc-10 branch](https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=e3abcc56d2604b9d2652b615ff9e68981cb7f79e), but hasn't yet made it into a 10.x release.
The patch corrects a regression from an earlier GCC commit, see:
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=357c4350680bf29f0c7a115424e3da11c53b5582
and
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=074226d5aa86cd3de517014acfe34c7f69a2ccc7,
related to the way newer versions of mingw-w64 implement setjmp/longjmp.
Ultimately this was causing a crash for us when Windows users were
viewing the network traffic tab inside the GUI. After some period, long
enough that a buffer would need reallocating, a call into FreeTypes
[`gray_record_cell()`](a18906091c/src/smooth/ftgrays.c (L526)) would result in a call to [`ft_longjmp` (longjmp)](a18906091c/src/smooth/ftgrays.c (L165)), which
would then trigger a crash.
Fixes: https://github.com/bitcoin-core/gui/issues/582.
See also:
https://bugreports.qt.io/browse/QTBUG-93476 - very similar issue reported to Qt.
Guix Build (on x86_64):
```bash
62172df3089e7bca3fd00f63acc9c8d3678a35bfb2bb5a0af905e61e9d8def52 guix-build-457148a803ce/output/dist-archive/bitcoin-457148a803ce.tar.gz
f8318d16d0418e0e790efd94527a5be374ac50f51df53e05a6d54cc8c08a8633 guix-build-457148a803ce/output/x86_64-w64-mingw32/SHA256SUMS.part
72076e6896297a36beec6c62065b3d8aeeeb87fed407df947261cefdc81cdb93 guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-debug.zip
c617d2347f50d2706bbdcc2b3b97f2ecaf59243747f4c81d7747a22e64cb9d76 guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-setup-unsigned.exe
8b1e7821e495121bea8a70f09ea6a0b703503b054d831b0dd86a0fe29cece457 guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64-unsigned.tar.gz
c8d2c0e68e3bf21ed7cfe08df64925bfa54ce6225c6d29bb710f9d9d4474caee guix-build-457148a803ce/output/x86_64-w64-mingw32/bitcoin-457148a803ce-win64.zip
```
ACKs for top commit:
jonatack:
Approach and review-only ACK 457148a803
laanwj:
Concept and code review ACK 457148a803
gruve-p:
ACK 457148a803
hebasto:
ACK 457148a803, tested `bitcoin-457148a803ce-win64.zip` on Windows 11 Pro 21H2. Confirming that bitcoin-core/gui#582 is fixed.
jarolrod:
ACK 457148a803
Tree-SHA512: dfb832ce93d72827009458cebbbdd408175b90b98d3eb546f9bbd21efe7bdd4ceca6ed13f5f6ce8e8e15d1c5d613f3a10399847a3589e4e7cc113ac0196d4010
a4f4f89815 Replace uint256 specific implementations of base_uint::GetHex() and base_uint::SetHex() with proper ones that don't depend on uint256 and replace template methods instantiations of base_uint with template class instantiation (Samer Afach)
Pull request description:
The current implementations of `SetHex()` and `GetHex()` in `base_uint` use `arith_uint256`'s implementations. Which means, any attempt to create anything other than `arith_uint256` (say `arith_uint512`) and using any of these functions (which is what I needed in my application) will just not work and will cause compilation errors (besides the immediate linking errors due to templates being in source files instantiated only for 256) because there's no viable conversion from `arith_uint256` and any of the other possible types. Besides that these function will yield wrong results even if the conversion is possible depending on the size. This is fixed in this PR.
ACKs for top commit:
laanwj:
re-ACK a4f4f89815
Tree-SHA512: 92a930fb7ddec5a5565deae2386f7d2d84645f9e8532f8d0c0178367ae081019b32eedcb59cc11028bac0cb15d9883228e016a466b1ee8fc3c6377b4df1d4180
e644591426 build, refactor: Drop useless `call` Make function (Hennadii Stepanov)
Pull request description:
Using the [`call`](https://www.gnu.org/software/make/manual/html_node/Call-Function.html) function with `$(package)_*_cmds` is effectively noop because the latter, which could be found in `<package>.mk` files, do not use temporary `$(1)` variable at all.
This PR removes useless calls of the `call` function, and makes code more readable and easier to reason about.
No change in resulted dependency binaries could be easy verified with bitcoin/bitcoin/#21995.
ACKs for top commit:
laanwj:
Code review ACK e644591426
shaavan:
Code review ACK e644591426
Tree-SHA512: 8481fa0dc5bbf7dd6a180f7fae5a2ccc07f85b50c7a966bceb2d7e010e07e5f211ee3f74f8ac79bc5acfde5f0764264d599d959ff3ebb8511b1b4a33f79509bd
0cea7b10f1 print `(none)` if no warnings in -getinfo (/dev/fd0)
Pull request description:
Adds `(none)` in warnings when no warnings returned by -getinfo
Reviewers can test this by making the following change in `/src/warnings.cpp`:
```diff
bilingual_str GetWarnings(bool verbose)
{
bilingual_str warnings_concise;
std::vector<bilingual_str> warnings_verbose;
LOCK(g_warnings_mutex);
// Pre-release build warning
if (!CLIENT_VERSION_IS_RELEASE) {
- warnings_concise = _("This is a pre-release test build - use at your own risk - do not use for mining or merchant applications");;
+ warnings_concise = _("");;
```
Before this pull request:
```
$ bitcoin-cli -getinfo
Chain: regtest
Blocks: 0
Headers: 0
Verification progress: 100.0000%
Difficulty: 4.656542373906925e-10
Network: in 0, out 0, total 0
Version: 239900
Time offset (s): 0
Proxies: n/a
Min tx relay fee rate (BTC/kvB): 0.00001000
Warnings:
```
After this pull request:
```diff
$ bitcoin-cli -getinfo
Chain: regtest
Blocks: 0
Headers: 0
Verification progress: 100.0000%
Difficulty: 4.656542373906925e-10
Network: in 0, out 0, total 0
Version: 239900
Time offset (s): 0
Proxies: n/a
Min tx relay fee rate (BTC/kvB): 0.00001000
Warnings: (none)
```
ACKs for top commit:
jonatack:
ACK 0cea7b10f1
laanwj:
Tested ACK 0cea7b10f1
Tree-SHA512: a12499d11ff84bc954db354f968eb1f5ee4999d8b80581fe0bdf604732b2e2f608cb5c35c4ca8cb5a430f3991954a6207f0758302618662e6b9505044cf2dc95
This change is preparation for Qt 6, and it fixes an experimental build
with Qt 6.2.4 as copying of `QEvent` has been disabled in Qt 6.0.0 (see
19f9b0d5f54379151eb71e98555b203ad6756276 upstream commit).
63125752a9 qt: Update deprecated enum value (Hennadii Stepanov)
c7add881a6 qt: Use `|` instead of `+` for key modifiers (Hennadii Stepanov)
6f1e162fe1 qt: Fix headers (Hennadii Stepanov)
Pull request description:
For Qt 5 all changes in this PR are refactoring. But for [Qt 6](https://github.com/bitcoin/bitcoin/pull/24798) they are real bugfixes :)
As I do not provide anyway way to build `bitcoin-qt` against Qt 6.2.4 fir now, suggesting to reviewers to verify changes for Qt 5 only.
ACKs for top commit:
shaavan:
ACK 63125752a9
jarolrod:
tACK 63125752a9
Tree-SHA512: ceee983192ddf62f09c1305458af3447ff0e3bd90311fa6328b139673bcaed3407dc0ce0b275028d4e0ca251d6b54dad40b48049211aeb251f65cbb4f5330834
d025d7f025 gui, refactor: rename fInvalid to num_test_failures in test_main.cpp (Jon Atack)
2489b6fe9c gui: count test failures in test runner summary (Jon Atack)
ba44aae768 gui: add test runner summary (Jon Atack)
Pull request description:
Append a one-line summary to the output of running `./src/qt/test/test_bitcoin-qt` indicating that all tests passed or showing the number of failing tests. It's currently a bit inconvenient to see this result by eyeballing all of the output.
ACKs for top commit:
shaavan:
ACK d025d7f025
jarolrod:
tACK d025d7f025
Tree-SHA512: 981c5daa13db127d38167bcf78b296b1a7e5b2d12e65f364ec6382b24f1008a223521d3b6c56e920bcd037479da5414e43758794688019d09e9aa696f3964746
51708c4516 gui: peersWidget - ResizeToContents Age and IP/Netmask columns (randymcmillan)
209301a442 gui: add Age column to peers tab (randymcmillan)
127de22c5f gui: add FormatPeerAge() utility helper (Jon Atack)
Pull request description:
This change adds an "Age" column to the peers table view,
which displays the duration of each peer's connection.
ACKs for top commit:
jonatack:
re-ACK 51708c4516
Jamewood:
> re-ACK 51708c4
shaavan:
reACK 51708c4516
hebasto:
ACK 51708c4516, I have reviewed the code and it looks OK, I agree it can be merged.
Tree-SHA512: 27323f7080ec0d3fcdbf1b190fba1cd2d7406840ab6607c221cf8af950db9134e22721cc5a88f4fc4f390d8b05e98bc4b7521661a31fadad9e2c6c6390e71788
Now BlockManager::LoadBlockIndex() will ACTUALLY only load BlockMan
members.
[META] In a later commit, pindexBestHeader will be moved to ChainMan as
a member
-----
Code Reviewer Notes
Call graph of relevant functions:
ChainstateManager::LoadBlockIndex() <-- Moved to
calls BlockManager::LoadBlockIndexDB()
which calls BlockManager::LoadBlockIndex() <-- Moved from
There is only one call to each of inner functions, meaning that no
behavior is changing.
Prevent -noproxy and -proxy=0 settings from interacting with -listen, -upnp,
and -natpmp settings.
These settings started being handled inconsistently in the `AppInitMain` and
`InitParameterInteraction` functions starting in commit
baf05075fa from #6272:
baf05075fa/src/init.cpp (L990-L991)baf05075fa/src/init.cpp (L687)
This commit changes both functions to handle proxy arguments the same way so
there are not side effects from specifying a proxy=0 setting.
This helper uses spanparsing::Split internally and enables to replace
all calls to boost::split where only a single separator is passed.
Co-authored-by: Martin Ankerl <Martin.Ankerl@gmail.com>
Co-authored-by: MarcoFalke <falke.marco@gmail.com>
