💬 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.

💬 maflcko commented on pull request "rpc, cli: add getbalances#total, and use it for -getinfo":
(https://github.com/bitcoin/bitcoin/pull/31353#discussion_r1855464360)
Do descriptor wallets even allow watchonly and signable in the same wallet? If not, it seems clearer to keep the nesting?

In any case, given that this includes untrusted, it seems better to keep the `untrusted` in the name?

✅ maflcko closed a pull request: "Update addresstype.cpp"
(https://github.com/bitcoin/bitcoin/pull/31354)

💬 fjahr commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2496062468)
> Now, testnet4 coins are actively traded

@garlonicon Can you provide a link? Otherwise I don't think I can follow your ideas how they can be used for a open test network without creating other issues. I am happy to review a proposal if you write it up in more detail.

💬 fjahr commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2496062880)
> In [7864871](https://github.com/bitcoin/bitcoin/commit/786487133e45052d389ec472c40176ae04640105). If I fire a `getblocktemplate` **before** sync is done, I get a `segfault`.
>
> Valgrind stack trace

Thanks @Davidson-Souza , good find! I pushed an update that prevents the rollback during IBD and that should fix it.

📝 hebasto opened a pull request: "cmake, qt: Use absolute paths for includes in MOC-generated files"
(https://github.com/bitcoin/bitcoin/pull/31361)
Fixes https://github.com/bitcoin/bitcoin/issues/31145.

From the `moc --help` output:
```
-p <path> Path prefix for included file.
```

💬 hebasto commented on pull request "cmake, qt: Use absolute paths for includes in MOC-generated files":
(https://github.com/bitcoin/bitcoin/pull/31361#issuecomment-2496063177)
cc @laanwj

💬 fjahr commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2496066634)
@orangesurf Thanks for the insight, I have seen some your comments on Twitter too, just pretty busy at the moment :)

It seems now the latest block with a real difficulty is [55100](https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2493677462) so from time to time real miners still get luck but it seems to be much less frequently than in the beginning. This might be due to honest miners pointing their hashrate elsewhere or just shutting them down. At least that's the only explanation
...

📝 l0rinc opened a pull request: "kernel, refactor: Replace `goto` with RAII in `bitcoin-chainstate`"
(https://github.com/bitcoin/bitcoin/pull/31362)
This PR replaces the [`goto` statement](https://github.com/bitcoin/bitcoin/pull/24304/files#diff-04e685224f1ac5bfd91d47d8d7528a2e44f94fab5535d4b6b5af79b5a13aeb93R91) in the `bitcoin-chainstate` helper with a Resource-Acquisition-Is-Initialization-based cleanup mechanism using a dedicated Epilogue class.
I've bumped into it while reviewing [#25722](https://github.com/bitcoin/bitcoin/pull/25722/files#diff-04e685224f1ac5bfd91d47d8d7528a2e44f94fab5535d4b6b5af79b5a13aeb93L132).

Key Changes:
* En
...

📝 sipa opened a pull request: "cluster mempool: introduce TxGraph"
(https://github.com/bitcoin/bitcoin/pull/31363)
Part of cluster mempool: #30286.

This introduces the `TxGraph` class, which encapsulates knowledge about the fees, sizes, and dependencies between all mempool transactions, but nothing else. In particular, it lacks knowledge about `CTransaction`, inputs, outputs, txids, wtxids, prioritization, validatity, policy rules, and a lot more. Being restricted to just those aspects of the mempool makes the behavior very easy to fully specify, and write simulation-based tests for (which are included in
...

💬 andremralves commented on pull request "Add and use `satToBtc` and `btcToSat` util functions":
(https://github.com/bitcoin/bitcoin/pull/31356#discussion_r1855488436)
Yes, I think this might be better. I've updated the PR.

📝 l0rinc opened a pull request: "refactor: Fix remaining clang-tidy performance-unnecessary-copy-initialization errors"
(https://github.com/bitcoin/bitcoin/pull/31364)
PR inspired by https://github.com/bitcoin/bitcoin/pull/31306 - a follow-up to https://github.com/bitcoin/bitcoin/pull/31305.

The `clang-tidy` check can be run via:
```bash
cmake -B build -DCMAKE_C_COMPILER=clang -DCMAKE_CXX_COMPILER=clang++ -DCMAKE_EXPORT_COMPILE_COMMANDS=ON -DBUILD_BENCH=ON -DBUILD_FUZZ_BINARY=ON -DBUILD_FOR_FUZZING=ON && cmake --build build -j$(nproc)

run-clang-tidy -quiet -p build -j $(nproc) -checks='-*,performance-unnecessary-copy-initialization' | grep -v 'clang-ti
...