📝 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

📝 hebasto opened a pull request: "depends: Avoid using helper variables in toolchain file"
(https://github.com/bitcoin/bitcoin/pull/31360)
Using helper variables has two issues:
1. They contaminate the global namespace of the main build script.
2. They can be used as `set(var)`, effectively [exposing](https://cmake.org/cmake/help/latest/command/set.html) a cache variable `var`, which makes the toolchain file susceptible to the build environment.

The [`depends/Makefile`](https://github.com/bitcoin/bitcoin/blob/master/depends/Makefile) can generate values with "not-set" semantics as empty strings or strings containing only space
...

🤔 maflcko reviewed a pull request: "Add and use `satToBtc` and `btcToSat` util functions"
(https://github.com/bitcoin/bitcoin/pull/31356#pullrequestreview-2456721037)
Instead of repeating the diff in the pull request description (if someone was interested in the changes in detail, they could just look at the diff themselves), it would be better to mention the benefits/goals and how they are expected to be achieved for new code written after this was merged.

Possibly there is a case to have feerate conversion helpers, or a class to hold feerates. However, any "global" change here would need a solid motivation and long-term upside.

💬 maflcko commented on pull request "Add and use `satToBtc` and `btcToSat` util functions":
(https://github.com/bitcoin/bitcoin/pull/31356#discussion_r1855459528)
Not sure about this. I think writing `10 * COIN` with the meaning of "10 bitcoins" is self-explanatory and couldn't be clearer. `btcToSat(10)` just seems like an extra step and extra complexity without a benefit?

Also, the "new" style isn't enforced, so you'll end up with both styles being used in the future, increasing the confusing further.

Finally, I had the impression that python code is using `snake_case`, so `btcToSat` would be confusing in that regard as well.

💬 hebasto commented on issue "Cmake build system breaks with symbolic links":
(https://github.com/bitcoin/bitcoin/issues/31145#issuecomment-2496032817)
> This affects ... the build with multiprocess...

Unable to reproduce this part on Ubuntu 24.04.