🤔 hebasto reviewed a pull request: "ci: Split out native fuzz jobs for macOS and windows (take 2)"
(https://github.com/bitcoin/bitcoin/pull/31221#pullrequestreview-2456407424)
Approach ACK c3354ea6dc34a3cadc60346c363478af22a5c0c2.

Please add [`strategy.fail-fast: false`](https://docs.github.com/en/actions/writing-workflows/workflow-syntax-for-github-actions#jobsjob_idstrategyfail-fast) everywhere.

💬 hebasto commented on pull request "ci: Split out native fuzz jobs for macOS and windows (take 2)":
(https://github.com/bitcoin/bitcoin/pull/31221#discussion_r1855194609)
This allows skipping the build of unneeded vcpkg packages:
```suggestion
generate-options: '-DVCPKG_MANIFEST_NO_DEFAULT_FEATURES=ON -DVCPKG_MANIFEST_FEATURES="sqlite" -DBUILD_GUI=OFF -DBUILD_FOR_FUZZING=ON -DWERROR=ON'
```

💬 brunoerg commented on pull request "Update addresstype.cpp":
(https://github.com/bitcoin/bitcoin/pull/31354#issuecomment-2495487280)
NACK

💬 furszy commented on pull request "wallet: remove BDB dependency from wallet migration benchmark":
(https://github.com/bitcoin/bitcoin/pull/31241#discussion_r1855213452)
> In [f7dbb30](https://github.com/bitcoin/bitcoin/commit/f7dbb300d7a4d18c9d85f80e5fdd7e5bcd21c6f0): Do we really need `TestLoadWallet` here? Couldn't we simply create the wallet directly (e.g. `std::unique_ptr<CWallet> wallet_ptr{std::make_unique<CWallet>(test_setup->m_node.chain.get()), "", CreateMockableWalletDatabase())};`)?

Not entirely, needed to also set the best locator manually. But done too. Thanks.

📝 hebasto opened a pull request: "cmake: Improve build script correctness"
(https://github.com/bitcoin/bitcoin/pull/31357)
When no build targets are specified, it is reasonable to expect the configuration step to succeed and produce a build system that does not build any targets.

This PR updates the code to ensure this behaviour:
```
$ cmake -B build -G "Ninja" -DBUILD_DAEMON=OFF -DBUILD_CLI=OFF -DBUILD_TX=OFF -DBUILD_UTIL=OFF -DENABLE_WALLET=OFF -DBUILD_TESTS=OFF
$ cmake --build build
ninja: no work to do.
```

💬 hebasto commented on pull request "cmake: Improve build script correctness":
(https://github.com/bitcoin/bitcoin/pull/31357#issuecomment-2495517355)
> When no build targets are specified, it is reasonable to expect the configuration step to succeed...

The issue was reported offline by @TheCharlatan.

👍 TheCharlatan approved a pull request: "cmake: Improve build script correctness"
(https://github.com/bitcoin/bitcoin/pull/31357#pullrequestreview-2456426859)
ACK ab5c63edccea7957e2c7b3b9d8b04655a7f82f22

💬 furszy commented on pull request "wallet: remove BDB dependency from wallet migration benchmark":
(https://github.com/bitcoin/bitcoin/pull/31241#issuecomment-2495531747)
> While this does remove the BDB requirement, it doesn't remove the reliance on legacy wallet specific code that will be removed. Would it be possible to change this to create a wallet with a LegacyDataSPKM without utilizing any of the legacy wallet functions?

Yes. Pushed the watch-only part. The spendable ones part is not that trivial, will finish it in the coming days.

📝 hebasto opened a pull request: "depends, refactor: Avoid hardcoding `host_prefix` in toolchain file"
(https://github.com/bitcoin/bitcoin/pull/31358)
This PR allows the entire `depends/<host_prefix>` directory to be relocatable.

Only `libevent` package configuration files are non-relocatable for the version `2.1.12-stable` we use now. However, this issue has been fixed upstream in https://github.com/libevent/libevent/commit/1f1593ff27bdf51c3e1c45b92cfb0ac931960298 and friends.

👍 tdb3 approved a pull request: "rpc: add getdescriptoractivity"
(https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2456458800)
code review and test re ACK 1ca5f7b998c14c44b6afd5e362fc4c007d8eebd3

Repeated test in https://github.com/bitcoin/bitcoin/pull/30708#pullrequestreview-2450158177

💬 furszy commented on pull request "Remove the legacy wallet and BDB dependency":
(https://github.com/bitcoin/bitcoin/pull/28710#discussion_r1855217304)
In order to continue testing this without manually writing the bdb file, we need to allow the 'mock' db format here:
```c++
const std::string& db_format{m_database->Format()};
Assert(db_format == "bdb_ro" || db_format == "mock");
```

💬 furszy commented on pull request "wallet: remove BDB dependency from wallet migration benchmark":
(https://github.com/bitcoin/bitcoin/pull/31241#issuecomment-2495680227)
Done, updated. The benchmark is no longer depending on wallet functions to fill-up the legacy wallet.
Tested that it works fine with #28710 (only need to tackle https://github.com/bitcoin/bitcoin/pull/28710#discussion_r1855217304).

💬 hodlinator commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1855301316)
Tests are running in a scratch directory - if part of the test fails, we don't worry about leaving some files behind.

👋 andremralves's pull request is ready for review: "Add and use `satToBtc` and `btcToSat` util functions"
(https://github.com/bitcoin/bitcoin/pull/31356)

💬 l0rinc commented on pull request "util: Improve documentation and negation of args":
(https://github.com/bitcoin/bitcoin/pull/31212#discussion_r1855390360)
Can we remove the cleanup in that case?

💬 hebasto commented on pull request "guix: use GCC 13 to build releases":
(https://github.com/bitcoin/bitcoin/pull/29881#issuecomment-2495868825)
My Guix build:
```
aarch64
588b6d6780720f5932e2e8b656ac729cb63a6ccbb6c5565d3e6bfd58b3217c3b guix-build-976b36f0eb12/output/aarch64-linux-gnu/SHA256SUMS.part
c3a1b0e7807cd280de074ee035b73df03509b902aef5ce7ac9c0efbb3bc0f45b guix-build-976b36f0eb12/output/aarch64-linux-gnu/bitcoin-976b36f0eb12-aarch64-linux-gnu-debug.tar.gz
ac5625327b6b677c4891b731f6a6dd232570411547a9f4a45dfb20137953e3b5 guix-build-976b36f0eb12/output/aarch64-linux-gnu/bitcoin-976b36f0eb12-aarch64-linux-gnu.tar.gz
ccd5c512
...

👍 hebasto approved a pull request: "guix: use GCC 13 to build releases"
(https://github.com/bitcoin/bitcoin/pull/29881#pullrequestreview-2456652810)
ACK 976b36f0eb1253da26dce27e39935588e75345c7.

📝 hebasto opened a pull request: "cmake: Add `CheckLinkerSupportsPIE` module"
(https://github.com/bitcoin/bitcoin/pull/31359)
This new module is a wrapper around CMake's `CheckPIESupported` module that:
1. Fixes an upstream bug. See: https://gitlab.kitware.com/cmake/cmake/-/issues/26463.
2. Enhances robustness by ensuring the linker is invoked, regardless of the `CMAKE_TRY_COMPILE_TARGET_TYPE` value at the call site.

Fixes https://github.com/bitcoin/bitcoin/issues/30771.

💬 hebasto commented on pull request "cmake: Add `CheckLinkerSupportsPIE` module":
(https://github.com/bitcoin/bitcoin/pull/31359#issuecomment-2495949546)
cc @vasild

💬 TheCharlatan commented on pull request "depends, refactor: Avoid hardcoding `host_prefix` in toolchain file":
(https://github.com/bitcoin/bitcoin/pull/31358#issuecomment-2495978639)
Concept ACK