84de8c93e7 ci: Add `deploy` target for native macOS CI job (Hennadii Stepanov)
fad57e9e0f build: Fix `macdeployqtplus` after switching to Qt 6 (Hennadii Stepanov)
938208d91a build: Resolve `@rpath` in `macdeployqtplus` (Hennadii Stepanov)
Pull request description:
Homebrew's Qt 6 package — namely `qt` or `qt@6` — introduces a few differences that must be properly handled by the `macdeployqtplus` script:
1. Use of `@rpath` references:
```
% objdump --macho --dylibs-used $(brew --prefix qt@5)/Frameworks/QtGui.framework/QtGui
/usr/local/opt/qt@5/Frameworks/QtGui.framework/QtGui:
/usr/local/opt/qt@5/lib/QtGui.framework/Versions/5/QtGui (compatibility version 5.15.0, current version 5.15.16)
/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit (compatibility version 45.0.0, current version 2575.30.19)
/System/Library/Frameworks/Metal.framework/Versions/A/Metal (compatibility version 1.0.0, current version 367.6.0)
/usr/local/Cellar/qt@5/5.15.16_1/lib/QtCore.framework/Versions/5/QtCore (compatibility version 5.15.0, current version 5.15.16)
/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
/System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics (compatibility version 64.0.0, current version 1889.2.7)
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 3208.0.0)
/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/AGL.framework/Versions/A/AGL (compatibility version 1.0.0, current version 1.0.0)
/usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 64.0.0, current version 64.0.0)
/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.12)
/System/Library/Frameworks/ApplicationServices.framework/Versions/A/ApplicationServices (compatibility version 1.0.0, current version 65.0.0)
/usr/local/opt/md4c/lib/libmd4c.0.dylib (compatibility version 0.0.0, current version 0.5.2)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1800.105.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3208.0.0)
/System/Library/Frameworks/CoreText.framework/Versions/A/CoreText (compatibility version 1.0.0, current version 844.2.0)
/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
% objdump --macho --dylibs-used $(brew --prefix qt@6)/Frameworks/QtGui.framework/QtGui
/usr/local/opt/qt/Frameworks/QtGui.framework/QtGui:
/usr/local/opt/qt/lib/QtGui.framework/Versions/A/QtGui (compatibility version 6.0.0, current version 6.9.0)
/System/Library/Frameworks/AppKit.framework/Versions/C/AppKit (compatibility version 45.0.0, current version 2575.30.19)
/System/Library/Frameworks/Carbon.framework/Versions/A/Carbon (compatibility version 2.0.0, current version 170.0.0)
/System/Library/Frameworks/OpenGL.framework/Versions/A/OpenGL (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/ImageIO.framework/Versions/A/ImageIO (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/CoreFoundation.framework/Versions/A/CoreFoundation (compatibility version 150.0.0, current version 3208.0.0)
/System/Library/Frameworks/CoreGraphics.framework/Versions/A/CoreGraphics (compatibility version 64.0.0, current version 1889.2.7)
/System/Library/Frameworks/CoreText.framework/Versions/A/CoreText (compatibility version 1.0.0, current version 844.2.0)
/System/Library/Frameworks/Foundation.framework/Versions/C/Foundation (compatibility version 300.0.0, current version 3208.0.0)
/System/Library/Frameworks/QuartzCore.framework/Versions/A/QuartzCore (compatibility version 1.2.0, current version 1.11.0)
/System/Library/Frameworks/Metal.framework/Versions/A/Metal (compatibility version 1.0.0, current version 367.6.0)
/usr/local/opt/glib/lib/libglib-2.0.0.dylib (compatibility version 8401.0.0, current version 8401.0.0)
@rpath/QtDBus.framework/Versions/A/QtDBus (compatibility version 6.0.0, current version 6.9.0)
/System/Library/Frameworks/AGL.framework/Versions/A/AGL (compatibility version 1.0.0, current version 1.0.0)
/usr/local/opt/libpng/lib/libpng16.16.dylib (compatibility version 64.0.0, current version 64.0.0)
/usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.12)
/usr/local/opt/harfbuzz/lib/libharfbuzz.0.dylib (compatibility version 61100.0.0, current version 61100.0.0)
/usr/local/opt/md4c/lib/libmd4c.0.dylib (compatibility version 0.0.0, current version 0.5.2)
/usr/local/opt/freetype/lib/libfreetype.6.dylib (compatibility version 27.0.0, current version 27.2.0)
/usr/local/opt/glib/lib/libgthread-2.0.0.dylib (compatibility version 8401.0.0, current version 8401.0.0)
@rpath/QtCore.framework/Versions/A/QtCore (compatibility version 6.0.0, current version 6.9.0)
/System/Library/Frameworks/IOKit.framework/Versions/A/IOKit (compatibility version 1.0.0, current version 275.0.0)
/System/Library/Frameworks/DiskArbitration.framework/Versions/A/DiskArbitration (compatibility version 1.0.0, current version 1.0.0)
/System/Library/Frameworks/UniformTypeIdentifiers.framework/Versions/A/UniformTypeIdentifiers (compatibility version 1.0.0, current version 709.0.0)
/usr/lib/libc++.1.dylib (compatibility version 1.0.0, current version 1800.105.0)
/usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1351.0.0)
/System/Library/Frameworks/CoreServices.framework/Versions/A/CoreServices (compatibility version 1.0.0, current version 1226.0.0)
/usr/lib/libobjc.A.dylib (compatibility version 1.0.0, current version 228.0.0)
```
2. Different directory layout:
```
% ls -l $(brew --prefix qt@5)/
total 544
drwxr-xr-x 79 hebasto admin 2528 13 Nov 06:22 Frameworks
-rw-r--r-- 1 hebasto admin 7533 16 Apr 09:09 INSTALL_RECEIPT.json
-rw-r--r-- 1 hebasto admin 22961 13 Nov 06:22 LICENSE.FDL
-rw-r--r-- 1 hebasto admin 36363 13 Nov 06:22 LICENSE.GPL3-EXCEPT
-rw-r--r-- 1 hebasto admin 15351 13 Nov 06:22 LICENSE.GPLv2
-rw-r--r-- 1 hebasto admin 35641 13 Nov 06:22 LICENSE.GPLv3
-rw-r--r-- 1 hebasto admin 26828 13 Nov 06:22 LICENSE.LGPLv21
-rw-r--r-- 1 hebasto admin 8174 13 Nov 06:22 LICENSE.LGPLv3
-rw-r--r-- 1 hebasto admin 106262 13 Nov 06:22 LICENSE.QT-LICENSE-AGREEMENT
-rw-r--r-- 1 hebasto admin 3842 13 Nov 06:22 README
drwxr-xr-x 57 hebasto admin 1824 16 Apr 09:09 bin
drwxr-xr-x 4 hebasto admin 128 13 Nov 06:22 doc
drwxr-xr-x 95 hebasto admin 3040 13 Nov 06:22 include
drwxr-xr-x 119 hebasto admin 3808 16 Apr 09:09 lib
drwxr-xr-x 8 hebasto admin 256 13 Nov 06:22 libexec
drwxr-xr-x 79 hebasto admin 2528 16 Apr 09:09 mkspecs
drwxr-xr-x 15 hebasto admin 480 13 Nov 06:22 phrasebooks
drwxr-xr-x 31 hebasto admin 992 13 Nov 06:22 plugins
drwxr-xr-x 28 hebasto admin 896 13 Nov 06:22 qml
-rw-r--r-- 1 hebasto admin 6952 16 Apr 09:09 sbom.spdx.json
drwxr-xr-x 3 hebasto admin 96 13 Nov 06:22 share
drwxr-xr-x 347 hebasto admin 11104 13 Nov 06:22 translations
% ls -l $(brew --prefix qt@6)/share/qt/
total 0
drwxr-xr-x 4 hebasto admin 128 30 Mar 09:49 doc
drwxr-xr-x 35 hebasto admin 1120 16 Apr 09:16 libexec
drwxr-xr-x 167 hebasto admin 5344 30 Mar 09:49 metatypes
drwxr-xr-x 70 hebasto admin 2240 16 Apr 09:16 mkspecs
drwxr-xr-x 178 hebasto admin 5696 30 Mar 09:49 modules
drwxr-xr-x 15 hebasto admin 480 30 Mar 09:49 phrasebooks
drwxr-xr-x 31 hebasto admin 992 30 Mar 09:49 plugins
drwxr-xr-x 34 hebasto admin 1088 30 Mar 09:49 qml
drwxr-xr-x 45 hebasto admin 1440 30 Mar 09:49 sbom
drwxr-xr-x 285 hebasto admin 9120 30 Mar 09:49 translations
```
This PR addresses both issues and additionally adds a `deploy` target to the native macOS CI job to prevent any similar recessions in the future.
Fixes https://github.com/bitcoin/bitcoin/issues/32267.
ACKs for top commit:
fanquake:
ACK 84de8c93e7
Tree-SHA512: 27a0eff3cd9317647529ff4571bd79c5dd8f007775b19415c8c27ca4912a60d85074c840cf0443be314d9a404f78bb015029d46dab18e292462249a5d90c6c47
22cff32319 doc: recommend gmake for FreeBSD (Sjors Provoost)
b645c52071 doc: recommend modern make for macOS depends (Sjors Provoost)
99e6490dc5 doc: shuffle depends instructions (Sjors Provoost)
Pull request description:
macOS ships with GNU Make 3.81 from 2006. This has caused
difficult to debug issues, e.g. #32070 and #30978.
Tell users / developers who use the depends system to install a modern version of `make`.
This PR does not change the non-depends build.
Although Homebrew allows overriding the system `make`, we instead just instruct users to build with `gmake`. This way there should be no impact on other projects they wish to compile.
To increase the likeliness of anyone actually seeing and following this instruction, the first commit moves things around in `depends/README.md`. It now starts with instructions for a local build and moves cross-compilation to the end. For each platform it shows what to install (`apt install`, `brew install`, etc) and what command to run (`make` or `gmake`).
There previously was no macOS specific section, so this is added. It points to the general `build-osx.md` for how to install the Xcode Command Line Tools and Homebrew Package Manager.
I didn't test on an empty system.
Preview: https://github.com/Sjors/bitcoin/tree/2025/03/mc-make/depends#depends-build
ACKs for top commit:
maflcko:
review ACK 22cff32319🏣
hebasto:
re-ACK 22cff32319.
willcl-ark:
ACK 22cff32319
Tree-SHA512: 11648ae73f3b70bc2df771e4eddca37221cd88b88bea4139a183e3f67f24a4c3e5aadf61a713ed73f3fc206511dfcf8670e4c4143c49dd4e56e501030be9c7ba
Native compilation is explained before cross-compilation. Move
install and (g)make steps up.
In the Configuring section, use Linux native compilation as the
example instead of Windows cross-compile.
In order to remove potential confusion, this commit adapts all script
error constant names in the functional tests (currently only in
feature_taproot.py) to the ones used in our C++ codebase. This also
makes checking whether we have test coverage for a certain script error
easier.
-BEGIN VERIFY SCRIPT-
ren() { sed -i "s|$1|$2|g" $( git grep -l "$1" -- "./test" ) ; }
ren ERR_SIG_SIZE ERR_SCHNORR_SIG_SIZE
ren ERR_SIG_HASHTYPE ERR_SCHNORR_SIG_HASHTYPE
ren ERR_SIG_SCHNORR ERR_SCHNORR_SIG
ren ERR_CONTROLBLOCK_SIZE ERR_TAPROOT_WRONG_CONTROL_SIZE
ren ERR_PUSH_LIMIT ERR_PUSH_SIZE
ren ERR_MINIMALIF ERR_TAPSCRIPT_MINIMALIF
ren ERR_UNKNOWN_PUBKEY ERR_PUBKEYTYPE
ren ERR_STACK_EMPTY ERR_INVALID_STACK_OPERATION
ren ERR_SIGOPS_RATIO ERR_TAPSCRIPT_VALIDATION_WEIGHT
ren ERR_UNDECODABLE ERR_BAD_OPCODE
ren ERR_NO_SUCCESS ERR_EVAL_FALSE
ren ERR_EMPTY_WITNESS ERR_WITNESS_PROGRAM_WITNESS_EMPTY
-END VERIFY SCRIPT-
3add6ab9ad test: remove Boost SIGCHLD workaround. (fanquake)
Pull request description:
The related code was removed from Boost in 2e3bd1025d.
ACKs for top commit:
achow101:
ACK 3add6ab9ad
laanwj:
ACK 3add6ab9ad
hebasto:
ACK 3add6ab9ad, I have reviewed the code and it looks OK.
mabu44:
ACK 3add6ab9ad
Tree-SHA512: a0db2bb4e6a476c920a97183bd807e800d935114ff28f8802373a08b5330df42a9be953e7ea6e3c09f2ed45175f60c26c33bb4e25010269e6e491f12867ba008
Add missing error check for fcntl(fd, F_GETFD, 0) in set_clo_on_exec.
Raise OSError on failure to align with existing FD_SETFD behavior.
This improves robustness in subprocess setup and error visibility.
Github-Pull: arun11299/cpp-subprocess#117
Rebased-From: 9974ff69cdd5fc1a2722cb63f006df9308628b35
This commit makes sure:
1. WaitForSingleObject returns with expected
code before proceeding.
2. Process handle is properly closed.
Github-Pull: arun11299/cpp-subprocess#116
Rebased-From: 625a8775791e62736f20f3fa3e6cc4f1b24aa89a
* refactor: Guard `util::quote_argument()` with `#ifdef __USING_WINDOWS__`
The `util::quote_argument()` function is specific to Windows and is used
in code already guarded by `#ifdef __USING_WINDOWS__`.
* Do not escape double quotes for command line arguments on Windows
This change fixes the handling of double quotes and aligns the behavior
with Python's `Popen` class. For example:
```
>py -3
>>> import subprocess
>>> p = subprocess.Popen("cmd.exe /c dir \"C:\\Program Files\"", stdout=subprocess.PIPE, text=True)
>>> print(f"Captured stdout:\n{stdout}")
```
Currently, the same command line processed by the `quote_argument()`
function looks like `cmd.exe /c dir "\"C:\Program" "Files\""`, which is
broken.
With this change, it looks correct: `cmd.exe /c dir "C:\Program Files"`.
Github-Pull: arun11299/cpp-subprocess#113
Rebased-From: ed313971c04ac10dc006104aba07d016ffc6542a
This suppresses the following warning caused by clang-20.
```
error: definition of implicit copy constructor for 'Streams' is deprecated because it has a user-declared copy assignment operator [-Werror,-Wdeprecated-copy]
```
Copy constructor or move constructor is called when std::vector re-allocates
memory. In this case, move constructor should be called, because copying
Streams instances breaks file-descriptor management.
Communication class is modified as well, since it's instance is a member of
Streams class.
Github-Pull: arun11299/cpp-subprocess#107
Rebased-From: 38d98d9d20be50c7187b98ac9bc9a6e66920f6ef
The commit a32c0f3df4b6bcd1d2e93f19e8f380bb890cd507 introduced code to
silence MSVC's "warning C4996: The POSIX name for this item is
deprecated."
However, it exhibits several issues:
1. The aliases may leak into code outside the `subprocess.hpp` header.
2. They are unnecessarily applied when using the MinGW-w64 toolchain.
3. The fix is incomplete: downstream projects still see C4996 warnings.
4. The implementation lacks documentation.
This change addresses all of the above shortcomings.
Github-Pull: arun11299/cpp-subprocess#112
Rebased-From: 778543b2f2ca7f5d1c4f0241b635bbb265d750dd
Co-authored-by: Luke Dashjr <luke-jr+git@utopios.org>
Currently, wait() returns 0 on windows regardless
of the actual return code of processes.
Github-Pull: arun11299/cpp-subprocess#109
Rebased-From: 04b015a8e52ead4d8bb5f0eb486419c77e418a17
When passing in a rvalue reference, compiler
considers it ambiguous between std::string and
std::string&&. Making one of them take a lvalue
reference makes compilers correctly pick the right
one depending on whether the passed in value binds
to a rvalue or lvalue reference.
Github-Pull: arun11299/cpp-subprocess#110
Rebased-From: 2d8a8eebb03e509840e2c3c755d1abf32d930f33
I encountered this issue while running my code with Valgrind today.
Below is part of the Valgrind error message:
```
==1578139== 472 bytes in 1 blocks are still reachable in loss record 1 of 1
==1578139== at 0x4848899: malloc (...)
==1578139== by 0x4B3AF62: fdopen@@GLIBC_2.2.5 (...)
==1578139== by 0x118B09: subprocess::Popen::execute_process() (...)
```
I noticed that a similar fix had been proposed by another contributor
previously. I did not mean to scoop their work, but merely hoping to fix
it sooner so other people don't get confused by it just as I did today.
Github-Pull: arun11299/cpp-subprocess#106
Rebased-From: 3afe581c1f22f106d59cf54b9b65251e6c554671
e976bd3045 validation: add randomness to periodic write interval (Andrew Toth)
2e2f410681 refactor: replace m_last_write with m_next_write (Andrew Toth)
b557fa7a17 refactor: rename fDoFullFlush to should_write (Andrew Toth)
d73bd9fbe4 validation: write chainstate to disk every hour (Andrew Toth)
0ad7d7abdb test: chainstate write test for periodic chainstate flush (Andrew Toth)
Pull request description:
Since #28233, periodically writing the chainstate to disk every 24 hours does not clear the dbcache. Since #28280, periodically writing the chainstate to disk is proportional only to the amount of dirty entries in the cache. Due to these changes, it is no longer beneficial to only write the chainstate to disk every 24 hours. The periodic flush interval was necessary because every write of the chainstate would clear the dbcache. Now, we can get rid of the periodic flush interval and simply write the chainstate along with blocks and block index at least every hour.
Three benefits of doing this:
1. For IBD or reindex-chainstate with a combination of large dbcache setting, slow CPU, slow internet speed/unreliable peers, it could be up to 24 hours until the chainstate is persisted to disk. A power outage or crash could potentially lose up to 24 hours of progress. If there is a very large amount of dirty cache entries, writing to disk when a flush finally does occur will take a very long time. Crashing during this window of writing can cause https://github.com/bitcoin/bitcoin/issues/11600. By syncing every hour in unison with the block index we avoid this problem. Only a maximum of one hour of progress can be lost, and the window for crashing during writing is much smaller. For IBD with lower dbcache settings, faster CPU, or better internet speed/reliable peers, chainstate writes are already triggered more often than every hour so this change will have no effect on IBD.
2. Based on discussion in #28280, writing only once every 24 hours during long running operation of a node causes IO spikes. Writing smaller chainstate changes every hour like we do with blocks and block index will reduce IO spikes.
3. Faster shutdown speeds. All dirty chainstate entries must be persisted to disk on shutdown. If we have a lot of dirty entries, such as when close to 24 hours or if we sync with a large dbcache, it can take a long time to shutdown. By keeping the chainstate clean we avoid this problem.
Inspired by [this comment](https://github.com/bitcoin/bitcoin/pull/28280#issuecomment-2121088705).
Resolves https://github.com/bitcoin/bitcoin/issues/11600
ACKs for top commit:
achow101:
ACK e976bd3045
davidgumberg:
utACK e976bd3045
sipa:
utACK e976bd3045
l0rinc:
ACK e976bd3045
Tree-SHA512: 5bccd8f1dea47f9820a3fd32fe3bb6841c0167b3d6870cc8f3f7e2368f124af1a914bca6acb06889cd7183638a8dbdbace54d3237c3683f2b567eb7355e015ee
6cbc28b8dd doc: Fix test_bitcoin path (monlovesmango)
Pull request description:
This commit fixes a couple command paths for interacting with the test_bitcoin binary within the Unit Test documentation.
If the commands are run as is a `command not found` error is returned.
```bash
❯ test_bitcoin --list_content
bash: test_bitcoin: command not found
```
```bash
❯ test_bitcoin --help
bash: test_bitcoin: command not found
```
ACKs for top commit:
davidgumberg:
ACK 6cbc28b8dd
Tree-SHA512: 0b10bc3aead360fa499beef7c9715f95a9acacdda44cbfac15566428594a7a8bdece24114a42618355959e20754bedc7a903bdddbf21b819c7b75375bdc80a93
We do not need to repeat the same test multiple times because BnB is
deterministic and will therefore always have the same outcome.
Additionally, this test was redundant because it repeats the "Smallest
combination too big" test.
Originally these tests verified that at a SelectCoins level that a
solution with fewer inputs gets preferred at high feerates, and a
solution with more inputs gets preferred at low feerates. This outcome
relies on the behavior of BnB, so we move these tests under the umbrella
of BnB tests.
Originally these tests relied on SFFO to work.
Recreates the tests in a new test suite coinselection_tests.cpp that is
based on UTXOs being created per their effective values rather than
nominal values and uses transactions with non-zero feerates.
97eaadc3bf util: Remove `fsbridge::get_filesystem_error_message()` (Hennadii Stepanov)
Pull request description:
The `fsbridge::get_filesystem_error_message()` function exhibits several drawbacks:
1. It was introduced in https://github.com/bitcoin/bitcoin/pull/14192 to account for platform-specific variations in
`boost::filesystem::filesystem_error::what()`. Since [migrating](https://github.com/bitcoin/bitcoin/pull/20744) to `std::filesystem`, those discrepancies no longer exist.
2. It fails to display UTF-8 paths correctly on Windows:
```
> build\bin\Release\bitcoind.exe -datadir="C:\Users\hebasto\dd_₿_🏃" -regtest
...
2025-04-30T00:17:48Z DeleteAuthCookie: Unable to remove random auth cookie file: remove: Access is denied.: "C:\Users\hebasto\dd_?_??\regtest\.cookie"
...
```
3. It relies on `std::wstring_convert`, which was deprecated in C++17 and removed in C++26 (also see https://github.com/bitcoin/bitcoin/issues/32361).
This PR removes the obsolete `fsbridge::get_filesystem_error_message()` function, thereby resolving all of the above issues.
ACKs for top commit:
maflcko:
lgtm re-ACK 97eaadc3bf
davidgumberg:
untested crACK 97eaadc3bf
achow101:
ACK 97eaadc3bf
laanwj:
Code review ACK 97eaadc3bf
Tree-SHA512: 3c7378a9b143ac2a71add967318a13c346ae3bccbec6e9879d7873083f3fa469b3eef529b2c9c142b2489ba9563e4e12f685745c09a8a219d58b384f7ecf1be1
The `fsbridge::get_filesystem_error_message()` function exhibits several
drawbacks:
1. It was introduced in https://github.com/bitcoin/bitcoin/pull/14192 to
account for platform-specific variations in
`boost::filesystem::filesystem_error::what()`. Since migrating to
`std::filesystem`, those discrepancies no longer exist.
2. It fails to display UTF-8 paths correctly on Windows.
3. It relies on `std::wstring_convert`, which was deprecated in C++17
and removed in C++26.
This change removes the `fsbridge::get_filesystem_error_message()`
function, thereby resolving all of the above issues.
Additionally, filesystem error messages now use the "Warning" log level.
a8333fc9ff scripted-diff: wallet: rename plain and encrypted master key variables (Sebastian Falbesoner)
5a92077fd5 wallet: refactor: dedup master key decryption (Sebastian Falbesoner)
846545947c wallet: refactor: dedup master key encryption / derivation rounds setting (Sebastian Falbesoner)
a6d9b415aa wallet: refactor: introduce `CMasterKey::DEFAULT_DERIVE_ITERATIONS` constant (Sebastian Falbesoner)
62c209f50d wallet: doc: remove mentions of unavailable scrypt derivation method (Sebastian Falbesoner)
Pull request description:
This PR contains various cleanups around the wallet's master key encryption logic. The default/minimum key derivation rounds magic number of 25000 is hoisted into a constant (member of `CMasterKey`) and two new functions `EncryptMasterKey`/`DecryptMasterKey` are introduced in order to deduplicate code for the derivation round determination and master key en/decryption. Also, mentions of the never-implemented derivation method `scrypt` are removed from the wallet crypter header and both plain and encrypted master key instances are renamed to adapt to moderning coding style (hopefully improving readability).
ACKs for top commit:
davidgumberg:
ACK a8333fc9ff
achow101:
ACK a8333fc9ff
Tree-SHA512: 5a66d3b26f481347d0b5b4f742dd237803a35aad6e3480ed15fd38b7fa3700650bd5f67f4c30ed88f5fad45d6cd4c893fe4f1657e36e563b4294fd3596187724
524f981bb8 Bugfix: Miner: Don't reuse block_reserved_weight for "block is full enough to give up" weight delta (Luke Dashjr)
Pull request description:
PR #30356 incorrectly changed a constant of `4000` to `m_options.coinbase_max_additional_weight` in the check for when to give up finding another transaction to fill the block:
```diff
if (nConsecutiveFailed > MAX_CONSECUTIVE_FAILURES && nBlockWeight >
- m_options.nBlockMaxWeight - 4000) {
+ m_options.nBlockMaxWeight - m_options.block_reserved_weight) {
// Give up if we're close to full and haven't succeeded in a while
break;
}
```
But this constant did not deal with the reserved weight at all. It was in fact simply checking if the block was close to full, and if so, giving up finding another transaction to pad it with after `MAX_CONSECUTIVE_FAILURES` failed attempts.
It doesn't seem very logical to reuse the reserve weight for this purpose, and it would be overcomplicated to add yet another setting, so this PR changes it to a new constexpr.
ACKs for top commit:
achow101:
ACK 524f981bb8
darosior:
utACK 524f981bb8
ismaelsadeeq:
ACK 524f981bb8
Tree-SHA512: c066debc34a021380424bd21b40444071b736325e41779a41590c2c8a6822ceeaf910fe067817c1dba108210b24c574977b0350b29520502e7af79d3b405928b
7e8ef959d0 refactor: Fix Sonar rule `cpp:S4998` - avoid unique_ptr const& as parameter (Lőrinc)
e400ac5352 refactor: simplify repeated comparisons in `FindChallenges` (Lőrinc)
f670836112 test: remove old recursive `FindChallenges_recursive` implementation (Lőrinc)
b80d0bdee4 test: avoid stack overflow in `FindChallenges` via manual iteration (Lőrinc)
Pull request description:
`FindChallenges` explores the `Miniscript` node tree by going deep into the first child's subtree, then the second, and so on - effectively performing a pre-order Traversal (Depth-First Search) recursively, using the call stack which can result in stack overflows on Windows debug builds.
This change replaces the recursive implementation with an iterative version using an explicit stack. The new implementation also performs a pre-order depth-first traversal, though it processes children in right-to-left order (rather than left-to-right) due to the LIFO nature of the stack. Since both versions store results in a `std::set`, which automatically sorts and deduplicates elements, the exact traversal order doesn't affect the final result.
It is an alternative to increasing the Windows stack size, as proposed in #32349, and addresses the issue raised in #32341 by avoiding deep recursion altogether.
The change is done in two commits:
* add a new iterative `FindChallenges` method and rename the old method to `*_recursive` (to simplify the next commit where we remove it), asserting that its result matches the original;
* remove the original recursive implementation.
This approach avoids ignoring the `misc-no-recursion` warning as well.
I tried modifying the new method to store results in a vector instead, but it demonstrated that the deduplication provided by `std::set` was necessary. One example showing the need for deduplication:
Recursive (using set):
```
(6, 9070746)
(6, 19532513)
(6, 3343376967)
```
Iterative (using vector attempt):
```
(6, 19532513)
(6, 9070746)
(6, 3343376967)
(6, 9070746) // Duplicate entry
```
The performance of the test is the same as before, with the recursive method.
Fixes https://github.com/bitcoin/bitcoin/issues/32341
ACKs for top commit:
achow101:
ACK 7e8ef959d0
sipa:
utACK 7e8ef959d0
hodlinator:
re-ACK 7e8ef959d0
Tree-SHA512: 9e52eff82a7d76f5d37e3b74c508f08e5fced5386dad504bed111b27ed2b529008a6dd12a5116f009609a94c7ee7ebe3e80a759dda55dd1cb3ae52078f65ec71
b9d4d5f66a net: Use GetAdaptersAddresses to get local addresses on Windows (laanwj)
Pull request description:
Instead of a `gethostname` hack, which is not guaranteed to return all addresses, use the official way of calling `GetAdaptersAddresses` to get local network addresses on Windows.
Do the same checks as the UNIX path: interface is up, interface is not loopback.
Suggested by Ava Chow.
Addiional changes:
- Cleanup: move out `FromSockAddr` in `netif.cpp` from MacOS and use it everywhere appropriate. This avoids code duplication.
ACKs for top commit:
davidgumberg:
utreACK b9d4d5f66a
achow101:
ACK b9d4d5f66a
Tree-SHA512: e9f0a7ec0c46f21c0377d5174e054a6569f858630727f94dac00c0cb7c241c56892d0b902706d6dd53880cc3b5ae1f2dba9caa1fec40e64cd4cf0d34493a49c1
