8e9699cb10db9cce9f03675c4a5b625ff2f34fd0 Update doc to match new default wallet type (Bitcoin Hodler)
Pull request description:
#23002 changed the default wallet type to descriptors, so this doc was out of date.
ACKs for top commit:
achow101:
ACK 8e9699cb10db9cce9f03675c4a5b625ff2f34fd0
Tree-SHA512: 2f69b23c153163bf2a091dbf728b713d28f795cc81e031bf201160882d2456494e94955ff6385634615fdcfece11542749ad1c982e2994e64ed69011380a2353
fae3f178238df96554dc2495e040f5580b55408a fuzz: Split script formatting from script fuzz target (MarcoFalke)
Pull request description:
This is a follow-up to commit 9237bdaac196951a437accaefa65638149b25978.
The target was improved a bit, but is still taking enormously long. See for example 4096 seconds in https://cirrus-ci.com/task/5153886888525824?logs=ci#L4451.
Most of the time is spent formatting the script. See the flamegraph: 
Thus, I suggest to split up the formatting into a new target. This will:
* Allow more fuzz cycles in the `script` target when exploring the search space with the fuzz engine
* Hopefully allow to reduce the fuzz inputs in `qa-assets` without losing coverage
ACKs for top commit:
fanquake:
ACK fae3f178238df96554dc2495e040f5580b55408a
Tree-SHA512: f86154b23019b7721e5dd10f54d11f4f7603d280471a396cb5256f4c460f48333318a60efe8b77fa8749a4abc67ad2631211b766fde5da70ded9fab8f904747b
b223c3c21e89f6af76b5401413880923f7c444d6 test: Add functional test for symlinked blocks directory (laanwj)
ddb75c2e87a60ed24065bdf0c3bfabf4e058cef1 test: Add fs_tests/create_directories unit test (Hennadii Stepanov)
1f46b6e46e1454b91ff7ceb31853bc440952f8eb util: Work around libstdc++ create_directories issue (laanwj)
Pull request description:
Work around libstdc++ issue [PR101510] with create_directories where the leaf already exists as a symlink. Fixes#24257, introduced by the switch to `std::filesystem`. It is meant to be more thorough than #24266, which worked around one instance of the problem.
The issue was [fixed upstream](https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=124eaa50e0a34f5f89572c1aa812c50979da58fc), but unfortunately we'll have to carry a fix for it for a while.
This introduces a function `fs::create_directories` which wraps
`std::filesystem::create_directories`. This allows easiliy reverting the
workaround when it is no longer necessary.
ACKs for top commit:
jonatack:
re-ACK b223c3c21e89f6af76b5401413880923f7c444d6 per `git range-diff df08250 67019cd b223c3c`
hebasto:
re-ACK b223c3c21e89f6af76b5401413880923f7c444d6
w0xlt:
re-ACK b223c3c
vasild:
ACK b223c3c21e89f6af76b5401413880923f7c444d6
Tree-SHA512: 028321717c8b10d16185c3711b35da6b05fb7aa31cee1c8c7e754e92bf5a0b02719a3785cd0f6f8bf052b3bd759f644af212320672baabc9e44e0b93ba464abc
eb8b22d5176d7abc6f93b4473df446105ca595e6 block_connected: re-use previous GetTimeMicros (William Casarin)
80e1c55687aae61767f1ade0826746cda00d6a24 block_connected: don't serialize block hash twice (William Casarin)
Pull request description:
In the validation:block_connected tracepoint, we call block->GetHash(), which
ends up calling CBlockHeader::GetHash(), executing around 8000 serialization
instructions. We don't need to do this extra work, because block->GetHash() is
already called further up in the function. Let's save that value as a local
variable and re-use it in our tracepoint so there is no unnecessary tracepoint
overhead.
Shave off an extra 100 or so instructions from the validation:block_connected
tracepoint by reusing a nearby GetTimeMicros(). This brings the tracepoint down
to 54 instructions. Still high, but much better than the previous ~154 and
8000 instructions which it was originally.
Signed-off-by: William Casarin <jb55@jb55.com>
ACKs for top commit:
0xB10C:
ACK eb8b22d5176d7abc6f93b4473df446105ca595e6
laanwj:
Code review ACK eb8b22d5176d7abc6f93b4473df446105ca595e6
theStack:
re-ACK eb8b22d5176d7abc6f93b4473df446105ca595e6
Tree-SHA512: 92ae585e487554e0f73042a8abaa239f630502c1d198e010bd7c1de252d882bccb627bbf0e4faec09c1253e782b145bcf153f9fee78cdb8456188044a96f8267
Work around libstdc++ issue [PR101510] with create_directories where the
leaf already exists as a symlink. Fixes#24257, introduced by the switch
to `std::filesystem`. It is meant to be more thorough than #24266, which
only worked around one instance of the problem.
The issue was fixed upstream in
https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=124eaa50e0a34f5f89572c1aa812c50979da58fc,
but unfortunately we'll have to carry a fix for it for a while.
This introduces a function `fs::create_directories` which wraps
`std::filesystem::create_directories`. This allows easiliy reverting the
workaround when it is no longer necessary.
dc01cbc538765f64326bca30952c83e3862d0d54 test: Add fs_tests/rename unit test (Hennadii Stepanov)
d4999d40b9bd04dc20111aaaa6ed2d3db1a5caf9 util: Revert back MoveFileExW call for MinGW-w64 (Hennadii Stepanov)
Pull request description:
Unfortunately, bitcoin/bitcoin#24308 introduced a [regression](https://github.com/bitcoin/bitcoin/pull/24308#issuecomment-1037259386) for mingw builds.
The root of the problem is a broken implementation of [`std::filesystem::rename`](https://en.cppreference.com/w/cpp/filesystem/rename). In particular, the expected behavior
> If `old_p` is a non-directory file, then `new_p` must be ... existing non-directory file: `new_p` _is first deleted_...
fails with the "File exists" error.
This PR reverts back the `MoveFileExW` call, and adds the [suggested](https://github.com/bitcoin/bitcoin/pull/24308#pullrequestreview-878832906) unit test.
ACKs for top commit:
vasild:
ACK dc01cbc538765f64326bca30952c83e3862d0d54
Tree-SHA512: c8e5a98844cfa32bec0ad67a1aaa58fe2efd0c5474d3e83490211985b110f83245758a742dcaa0a933a192ab66a7f11807e0c53ae69260b7dd02fc99f6d03849
f485a0745455b46390f1c14260643ad69c8fe2ad Add missing thread safety lock assertions in validation.h (Jon Atack)
37af8a20cf39ed8ee4b3ba4e1d8d55178eaacb78 Add missing thread safety lock assertions in validation.cpp (Jon Atack)
Pull request description:
A number of functions in validation.{h,cpp} have a thread safety lock annotation in the declaration but are missing the corresponding run-time lock assertion in the definition.
ACKs for top commit:
hebasto:
re-ACK f485a0745455b46390f1c14260643ad69c8fe2ad, only suggested change since my [previous](https://github.com/bitcoin/bitcoin/pull/24177#pullrequestreview-877810465) review.
vasild:
ACK f485a0745455b46390f1c14260643ad69c8fe2ad
Tree-SHA512: c86c0c0e8fe6ec7ae9ed9890f1dd7d042aa482ecf99feb6679a670aa004f6e9a99f7bc047205a34968fab7f1f841898c59b48c3ed6245c166e3b5abbf0867445
bfcd60f5d505334230013de4115483b22a7898ee test: activate all index types in feature_init.py (Martin Zumsande)
0243907faee0aa6af09974131d9a46a7f9c3ef38 index: Don't commit without valid m_best_block_index (Martin Zumsande)
Pull request description:
When an index thread receives an interrupt during init before it got to index anything (so `m_best_block_index == nullptr` still), it will still try to commit previous "work" before stopping the thread. That means that `BaseIndex::CommitInternal()` calls `GetLocator(nullptr)`, which returns an locator to the tip ([code](06b6369766/src/chain.cpp (L31-L32))), and saves it to the index DB.
On the next startup, this locator will be read and it will be assumed that we have successfully synced the index to the tip, when in reality we have indexed nothing.
In the case of coinstatsindex, this would lead to a shutdown of bitcoind without any indication what went wrong. For the other indexes, there would be no immediate shutdown, but the index would be corrupt.
This PR fixes this by not committing when `m_best_block_index==nullptr`, and it also adds an error log message to the silent coinstatsindex shutdown path.
This is another small bug found by `feature_init.py` - the second commit enables blockfilterindex and coinstatsindex for this test, enabling coinstatsindex without the first commit would have led to frequent failures.
ACKs for top commit:
fjahr:
reACK bfcd60f5d505334230013de4115483b22a7898ee
shaavan:
reACK bfcd60f5d505334230013de4115483b22a7898ee
Tree-SHA512: 8e2bac0fc40cde209518a9e59b597ae0a5a875a2a90898673987c91733718d40e528dada942bf552b58bc021bf46e59da2d0cc5a61045f48f9bae2b1baf6033b
f11dad22a506e10fbbfbcb6ccf32754bf8e72b72 test: refactor: remove unneeded bytes<->hex conversions in `byte_to_base58` (Sebastian Falbesoner)
Pull request description:
It seems like the only reason for using hex strings in this method was to have a convenient way to convert to an integer from the input data interpreted as big-endian. In Python3 we have `int.from_bytes(..., 'big')` for that purpose, hence there is no need for that anymore and we can simply operate on bytes only.
ACKs for top commit:
laanwj:
Code review ACK f11dad22a506e10fbbfbcb6ccf32754bf8e72b72
Tree-SHA512: 9b1563010066ca74d85139c3b9259e9a5bb49e1f141c30b6506a0445afddb2bde7fd421fdd917dc516956e66f93610e2c21d720817640daee8f57f803be76ee4
faef344f84a566436ffe2a277dec2cd6c7e1664b Print enable_fuzz_binary in configure (MarcoFalke)
Pull request description:
A *disabled* `enable_fuzz` on current master does *not* mean the the fuzz binary is not compiled. This is confusion, so fix it.
* `enable_fuzz` toggles compilation flags for fuzzing and disables all other target. There is no need to print this in the configure result, because the compilation flags are already printed. Also, all other targets are already printed as `no`.
* `enable_fuzz_binary` does what it says it does and is currently not printed. So print it.
ACKs for top commit:
hebasto:
ACK faef344f84a566436ffe2a277dec2cd6c7e1664b, tested on Linux Mint 20.2 (x86_64):
Tree-SHA512: 9b02b05c4b9c5fc92cf3487497392690303c36eace5e217f18b4349f059b5a23a7c0e0d030fb6fa7bbad83e927576a5e81c00099164f9ed8e185c0969dc17689
fa455975e52db3296a36fde827755cf91321fc2f util: Add missing unlinkat to syscall sandbox (MarcoFalke)
Pull request description:
This will be needed for g++-12 (after libstdc++6 12-20220206).
Steps to reproduce:
```
gdb --args ./src/bitcoind -sandbox=log-and-abort -regtest
./src/bitcoin-cli -regtest -named createwallet wallet_name=a descriptors=false
./src/bitcoin-cli -regtest stop
```
BT:
```
Thread 1 "b-shutoff" received signal SIGSYS, Bad system call.
0x00007ffff79564f7 in unlinkat () from /lib/x86_64-linux-gnu/libc.so.6
(gdb) bt
#0 0x00007ffff79564f7 in unlinkat () from /lib/x86_64-linux-gnu/libc.so.6
#1 0x00007ffff7cc7335 in ?? () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#2 0x00007ffff7cc94e3 in std::filesystem::remove_all(std::filesystem::__cxx11::path const&) () from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#3 0x00005555559d4918 in wallet::BerkeleyEnvironment::Flush (this=0x7fffc4005160, fShutdown=<optimized out>) at /usr/include/c++/12/bits/fs_path.h:595
#4 0x000055555592c058 in wallet::StopWallets (context=...) at /usr/include/c++/12/bits/shared_ptr_base.h:1665
#5 0x00005555556617ca in Shutdown (node=...) at ./src/init.cpp:293
#6 0x000055555563ada6 in AppInit (argv=<optimized out>, argc=<optimized out>, node=...) at ./src/bitcoind.cpp:249
#7 main (argc=<optimized out>, argv=<optimized out>) at ./src/bitcoind.cpp:273
ACKs for top commit:
laanwj:
Code review ACK fa455975e52db3296a36fde827755cf91321fc2f
Tree-SHA512: e80a38828f8656040954c9befa2d1c9d5170e204dc09c61031633349897f51ccd85cc5c99a089c4726d7f5237875cd9ed3fa8ef864cd6c1c8a2b8250b392d57f
64645fa3e045ec30a57dbd416fcf66d4755f37c6 Release process: fix broken link to Guix building docs (Jeremy Rand)
Pull request description:
Not 100% sure whether this link was always broken or if the Guix docs renamed the heading at some point. Either way, seems good to fix it.
ACKs for top commit:
fanquake:
ACK 64645fa3e045ec30a57dbd416fcf66d4755f37c6
Tree-SHA512: 4932059fe583c0d27c70febf8f4dd8cffd3e15567359c5429d2691e221afc6da319bf43ebcd264ae0f98302e1eeb67ffd763d3d7d06ab1633913555ee7461643
460fa8e0d9c1d08b5b5bca3fa02ba2763337aaf1 test: remove `import socket` in test_ipv6_local (brunoerg)
Pull request description:
Since this module (`socket`) is imported at the top of file, there is no need to import it again within the function.
ACKs for top commit:
MarcoFalke:
cr ACK 460fa8e0d9c1d08b5b5bca3fa02ba2763337aaf1
Tree-SHA512: 031c17a776dedaa21b3ec6458ca822304e76a5a3f4494406e6b7b04f08cc2abefcfe742c462b60c9b3e2fee3cd110a69ed5ad413357886dc7b823abc916ea40e
f730bd7d580502ae3c3b5953ada3724b59f5cd9b scripted-diff: Rename ShowModalDialogAndDeleteOnClose (Hennadii Stepanov)
5d7666b15164a16aaf3af49af8f73ff4bd392f6a qt: Revert 7fa91e831227e556bd8a7ae3da64bd59d4f30d5f partially (Hennadii Stepanov)
89c277a6fca1149f10f8b55874c702c341679765 qt: Delay shutdown while a modal dialog is active (Hennadii Stepanov)
8c0eb80f41bca7b08c94de0f08692fac23e3e9f0 qt: Disable tray icon menu when a modal dialog is active (Hennadii Stepanov)
92427354dd5f80cb5c042e7afbcd386968161be4 qt, refactor: Use local QAction instances for the tray icon menu (Hennadii Stepanov)
58e16035c1fc513fce0b09e02c7d863c63ec990d qt, refactor: Drop BitcoinGUI::{send,receive}CoinsMenuAction members (Hennadii Stepanov)
fd667e73cd109bbfc14011f8c2c08556648b4c50 qt: Make show_hide_action dependent on the main window actual state (Hennadii Stepanov)
ee151d032789fa03daa44ab44dd0fd70e51b145c qt: Drop BitcoinGUI::toggleHideAction member (Hennadii Stepanov)
78189daac8d13870ab6ed8421b9ed067cf7ebac5 qt, refactor: Fill up trayIconMenu before connections (Hennadii Stepanov)
66afa286e519deda2fcfd580f190b7af13407e72 qt, refactor: Replace BitcoinGUI::trayIconActivated with a lambda (Hennadii Stepanov)
c3ca8364b2a8de8c73ecc0c7eed9608bd30e7e02 qt, refactor: Replace BitcoinGUI::macosDockIconActivated with a lambda (Hennadii Stepanov)
Pull request description:
As pointed in bitcoin/bitcoin#23790 a regression in wallet unlock was introduced in bitcoin-core/gui#336 when a synchronous `AskPassphraseDialog` has been replaced with an asynchronous one.
This PR reverts a call back to a synchronous mode.
To make synchronous dialogs behave nice during shutdown some additional changes were made.
Please note that disabling the tray icon menu when a modal dialog is active is useful itself as on master (4ad59042b359f473d5888ecee0c9288dcf98f1c9) it is possible to switch to the "Receive" tab while the GUI is waiting for a password for the "Send" tab:
This is confusing and must be avoided.
Fixesbitcoin/bitcoin#23790.
ACKs for top commit:
prayank23:
tACK f730bd7d58
Tree-SHA512: 2b68275754190e4a9831b96e882d3c5b005e03909aeb6f2c5846da07199bb3efbb74ce87a9d25bb139f643c43d377a2051b221d553281fa5aefdd3181a58077f
This comment described how the constructor of CFeeRate was previously
indirectly used to parse fee rate arguments from RPCs. The command line
input was actually in sat/vB but due to the use of AmountFromValue() it
got converted to BTC/vB. In the constructor this could be rectified by
creating a CFeeRate from that given value (in BTC/vB) and COIN as the
transaction size, turning the input effectively to sat/vB. Since this
usage pattern was removed from the codebase some months ago, the comment
is now obsolete.
Also:
• Use vsize and vbyte instead of size and byte
abc057c6030b2a0ddab46835a7801054da677781 build: Add Boost.Process usage check (Hennadii Stepanov)
Pull request description:
This PR adds a check that Boost.Process can be used without linking any libraries (header-only).
Disable the functionality if that is not the case.
Fixesbitcoin/bitcoin#24314.
ACKs for top commit:
fanquake:
ACK abc057c6030b2a0ddab46835a7801054da677781
Tree-SHA512: ed2a32b1f751ec6f88cc7220766edd4cdab93c1d0515c892aa3094ee8d5b13ef569830d6e7a7a00c0197b117585dc526d00d943cc99a1f8c8a66ac4e20fe2061
bcd36e14f0a2b89445f1436094de40696d6ad76c build: correct depends FreeBSD C{XX}FLAGS (fanquake)
7b06ffce9c50110b475c722918c55a14402346a5 build: add NetBSD support to depends (fanquake)
Pull request description:
Similar to #23948. Doesn't build the Qt package; I haven't looked at doing that yet, but have an assumption that it's going to fail out of the box similar to the FreeBSD build.
Guix Build:
```bash
```
ACKs for top commit:
theuni:
ACK bcd36e14f0a2b89445f1436094de40696d6ad76c
Tree-SHA512: 9a0946cefbcb9a92dd730b885463f3213e304c8d4b39fea8d831fc013a73d2ef998ca84e384bf45a01fa1449cf5a35eaffaa5b57a9062c2cdda34312d33ec3fc
aaa1d03d3acebeb44fdd40a302f086aad3d329ce Add optimized sha256d64_arm_shani::Transform_2way (Pieter Wuille)
fe0629852aaf3a26f291bfa535e7e455fe7bea06 Implement sha256_arm_shani::Transform (Pavol Rusnak)
48a72fa81f80c8a3c7c6de8339b5feb361dece1c Add sha256_arm_shani to build system (Pavol Rusnak)
c2b79342506e24e9b7100fb7a6025dc870375ef6 Rename SHANI to X86_SHANI to allow future implementation of ARM_SHANI (Pavol Rusnak)
Pull request description:
This PR adds support for ARMv8 SHA2 Intrinsics.
Fixes https://github.com/bitcoin/bitcoin/issues/13401 and https://github.com/bitcoin/bitcoin/issues/17414
* Integration part was done by me.
* The original SHA2 NI code comes from https://github.com/noloader/SHA-Intrinsics/blob/master/sha256-arm.c
* Minor optimizations from https://github.com/rollmeister/bitcoin-armv8/blob/master/src/crypto/sha256.cpp are applied too.
* The 2-way transform added by @sipa
ACKs for top commit:
laanwj:
Code review and lightly tested ACK aaa1d03d3acebeb44fdd40a302f086aad3d329ce
Tree-SHA512: 9689d6390c004269cb1ee79ed05430d7d35a6efef2554a2b6732f7258a11e7e959b3306c04b4e8637a9623fb4c12d1c1b3592da0ff0dc6d737932db302509669
...to make sure that the linker errors that arise from coupling
regressions are caught by CI.
Adding to the "no wallet" ci job as suggested by MarcoFalke.
The bitcoin-chainstate executable serves to surface the dependencies
required by a program wishing to use Bitcoin Core's consensus engine as
it is right now.
More broadly, the _SOURCES list serves as a guiding "North Star" for the
libbitcoinkernel project: as we decouple more and more modules of the
codebase from our consensus engine, this _SOURCES list will grow shorter
and shorter. One day, only what is critical to our consensus engine will
remain. Right now, it's "the minimal list of files to link in to even
use our consensus engine".
[META] In a future commit the libbitcoinkernel library will be extracted
from bitcoin-chainstate, and the libbitcoinkernel library's
_SOURCES list will be the list that we aim to shrink.
