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.
86c2889518df0234b8b28112a9771b85c577eefd ci: Make log verbose in error case only (Hennadii Stepanov)
7f650883b71b67cd33026cf49af0b16f01e45541 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/5275741788045312
Closes #16368.
ACKs for top commit:
laanwj:
Tested ACK 86c2889518df0234b8b28112a9771b85c577eefd
Tree-SHA512: 497f2146fd2e38c952124aecfd80ebb42be22bbc5dc59521491545f4465fc38f23da7787a0caea5686b7c30aa862f2b0c02092ae3fe863e80a5ddd14b3d324b9
038d2a607f93a31be76f3a8210a5385996012783 test: add test for signet miner script (Sebastian Falbesoner)
449b96ed97f8b95cae91858dba605b326de1fd45 test: add `is_bitcoin_util_compiled` helper (Sebastian Falbesoner)
dde33eca63347801097e39b8dbed5b9ebbd97278 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 038d2a607f93a31be76f3a8210a5385996012783
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.
c71117fcb04fc2e45b5e76fe96b077a07b0c0f82 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 c71117fcb04fc2e45b5e76fe96b077a07b0c0f82.
lsilva01:
Code review ACK c71117fcb0
vasild:
ACK c71117fcb04fc2e45b5e76fe96b077a07b0c0f82
Tree-SHA512: feebfcfa75d997460a0ba42ffe1e0c25a7e0bfcad12510ad73ea4942cc1c653f9ad429adbbb00b9288fe319009552906fcb635a14dfd7dcbde3853baab6be065
fa32cc0682a0aa3420e6a11031721fcb6c50fa44 doc: Remove fee delta TODO from txmempool.cpp (MarcoFalke)
Pull request description:
This refactor request was added in commit eb306664e786ae43d539fde66f0fbe2a3e89d910, 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 fa32cc0682a0aa3420e6a11031721fcb6c50fa44
Tree-SHA512: 6629fef543e815136c82c38aa8ba2c4de68a5fe94c6954f2559e468f7e59052e02dd7c221d3b159be0314eaf0dbb18f74814297c58f76e2289c47e8d4f49be4e
457148a803cee02897b7428fa7b3eb93eed71e4c 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 457148a803cee02897b7428fa7b3eb93eed71e4c
laanwj:
Concept and code review ACK 457148a803cee02897b7428fa7b3eb93eed71e4c
gruve-p:
ACK 457148a803
hebasto:
ACK 457148a803cee02897b7428fa7b3eb93eed71e4c, tested `bitcoin-457148a803ce-win64.zip` on Windows 11 Pro 21H2. Confirming that bitcoin-core/gui#582 is fixed.
jarolrod:
ACK 457148a803cee02897b7428fa7b3eb93eed71e4c
Tree-SHA512: dfb832ce93d72827009458cebbbdd408175b90b98d3eb546f9bbd21efe7bdd4ceca6ed13f5f6ce8e8e15d1c5d613f3a10399847a3589e4e7cc113ac0196d4010
a4f4f89815c5aadff51a7a11e0d63caf5212345a 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 a4f4f89815c5aadff51a7a11e0d63caf5212345a
Tree-SHA512: 92a930fb7ddec5a5565deae2386f7d2d84645f9e8532f8d0c0178367ae081019b32eedcb59cc11028bac0cb15d9883228e016a466b1ee8fc3c6377b4df1d4180
e644591426fb4ee1bba5c4bfdde85eb378258272 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 e644591426fb4ee1bba5c4bfdde85eb378258272
shaavan:
Code review ACK e644591426fb4ee1bba5c4bfdde85eb378258272
Tree-SHA512: 8481fa0dc5bbf7dd6a180f7fae5a2ccc07f85b50c7a966bceb2d7e010e07e5f211ee3f74f8ac79bc5acfde5f0764264d599d959ff3ebb8511b1b4a33f79509bd
0cea7b10f1180e9993c14473e1a3b6525ef6ba01 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 0cea7b10f1180e9993c14473e1a3b6525ef6ba01
laanwj:
Tested ACK 0cea7b10f1180e9993c14473e1a3b6525ef6ba01
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).
63125752a902de725f8aad1ee5bdaf5cb0c70f91 qt: Update deprecated enum value (Hennadii Stepanov)
c7add881a674d927e58f11dac0dca223c7322cb9 qt: Use `|` instead of `+` for key modifiers (Hennadii Stepanov)
6f1e162fe135d051d16859be57ef44d191e095cc 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 63125752a902de725f8aad1ee5bdaf5cb0c70f91
jarolrod:
tACK 63125752a9
Tree-SHA512: ceee983192ddf62f09c1305458af3447ff0e3bd90311fa6328b139673bcaed3407dc0ce0b275028d4e0ca251d6b54dad40b48049211aeb251f65cbb4f5330834
d025d7f0251e26b7ab5cf48c236b6b5e46fafe26 gui, refactor: rename fInvalid to num_test_failures in test_main.cpp (Jon Atack)
2489b6fe9cd1b669362d459e93185ca0bd9f8714 gui: count test failures in test runner summary (Jon Atack)
ba44aae7683f412626baa8bf51708ddf25f51ba8 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 d025d7f0251e26b7ab5cf48c236b6b5e46fafe26
jarolrod:
tACK d025d7f025
Tree-SHA512: 981c5daa13db127d38167bcf78b296b1a7e5b2d12e65f364ec6382b24f1008a223521d3b6c56e920bcd037479da5414e43758794688019d09e9aa696f3964746
51708c4516cb9d52e84dc8850d93f556dda1a75b gui: peersWidget - ResizeToContents Age and IP/Netmask columns (randymcmillan)
209301a442512579d57f79c82417dc7c496248b6 gui: add Age column to peers tab (randymcmillan)
127de22c5fb396e1670d2a911faf7a9adc9241e2 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 51708c4516cb9d52e84dc8850d93f556dda1a75b
Jamewood:
> re-ACK 51708c4
shaavan:
reACK 51708c4516cb9d52e84dc8850d93f556dda1a75b
hebasto:
ACK 51708c4516cb9d52e84dc8850d93f556dda1a75b, 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
baf05075fae2cc2625a2a74b35cc66902f3cbfa3 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.
- LIBLEVELDB_SSE42_INT was defined, but never referenced anywhere
- LIBLEVELDB_SSE42 is referenced, but never defined anywhere
Apparently leveldb used to have platform-specific crc32 code before it
got split off into a separate lib.
This was used to, in effect, manually emulate --start-group/--end-group.
However, we can just order the libraries correctly and avoid specifying
libraries multiple times on the link line.
Note: lld (not ld.bfd) knows how to resolve out-of-order references and
doesn't seem to need the reodering
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>
The path is stored in `self.options.bitcoinutil`, points to
`src/bitcoin-util` by default and can be overrided with the
`BITCOINUTIL` environment variable.
c9c4e6cadda85783a005af773a683d44a208bf67 build: Do not define `PROVIDE_FUZZ_MAIN_FUNCTION` macro unconditionally (Hennadii Stepanov)
Pull request description:
No need to define the `PROVIDE_FUZZ_MAIN_FUNCTION` macro when the build system has been configured with the `--disable-fuzz-binary` option.
See https://github.com/bitcoin/bitcoin/pull/24336#pullrequestreview-881368272.
ACKs for top commit:
MarcoFalke:
Approach ACK c9c4e6cadda85783a005af773a683d44a208bf67 did not review or test 🐤
fanquake:
ACK c9c4e6cadda85783a005af773a683d44a208bf67 Checked that `PROVIDE_FUZZ_MAIN_FUNCTION` isn't defined when configuring with `--disable-fuzz-binary`.
Tree-SHA512: 54fbf02ba9f5ecc61b176b8ea7d05e308788d4de3f97ed40913e731300d9dc0edfdfcbf8e0a6e74cf1b2e2ae63f6208a34e03b9c8d203d070c457c4a7d9b5f2c
e8e48fa82bdce3f0c1da0693148867befa221de7 Converted lint-python-mutable-default-parameters.sh to python (TakeshiMusgrave)
Pull request description:
This converts one of the linter scripts to Python. Reference issue: https://github.com/bitcoin/bitcoin/issues/24783
The approach is to just call git grep using subprocess.run.
Alternative approaches could be to use Python instead of git grep (I'm not sure how) or use ```pylint --disable=all --enable=W0102```, though that requires installation of pylint.
ACKs for top commit:
MarcoFalke:
review ACK e8e48fa82bdce3f0c1da0693148867befa221de7
Tree-SHA512: 7f6f4887dee02c9751b225a6a131fb705868859c4a9af25bb3485cda2358650486b110f17adf89d96a20f212d7d94899922a07aab12c8dc11984cfd5feb7a076
494455f8a56c93cef4162a0cedc792901577314e test: use MiniWallet for feature_fee_estimation.py (Sebastian Falbesoner)
Pull request description:
This PR enables one more of the non-wallet functional tests (feature_fee_estimation.py) to be run even with the Bitcoin Core wallet disabled by using the MiniWallet instead, as proposed in https://github.com/bitcoin/bitcoin/issues/20078. It takes use of the recently introduced methods `{create,send}_self_transfer_multi` (#24637) which allows to specify multiple UTXOs to be spent rather than only one. Very likely the test can still be simplified (e.g. coin selection in `small_txpuzzle_randfee`), but this is a first step.
ACKs for top commit:
ayush933:
tACK 494455f8 . The test runs successfully with the wallet disabled.
vincenzopalazzo:
tACK 494455f8a5
Tree-SHA512: 89789fc34a4374c79c4b90acd926ac69153aad655dab50450ed796f03c770bd675ad872e906f516f90e8d4cb40b83b55f3c78a94b13bfb8fe8f5e27624937748
0f7dc893ea1776515173dcd0bfe6826e963c90f3 test: compare `/chaininfo` response with `getblockchaininfo` RPC (brunoerg)
Pull request description:
The `/chaininfo` REST endpoint gets its infos from `getblockchaininfo` RPC, so this PR adds an `assert_equal` (in `interface_rest`) to ensure both responses are the same. Obs: other endpoints do the same for their respective RPC.
ACKs for top commit:
0xB10C:
Concept and Code Review ACK 0f7dc893ea1776515173dcd0bfe6826e963c90f3. Belts-and-spenders.
Tree-SHA512: 51cbcf988090272e406a47dc869710740b74e2222af29c05ddcbf53bd49765cdc59efb525e970867f091b3d2efec4fb13371a342d9e484e51144b760265bc5b8
