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

💬 l0rinc commented on pull request "ci: Update Clang in "tidy" job":
(https://github.com/bitcoin/bitcoin/pull/31306#discussion_r1855510489)
Similarly to https://github.com/bitcoin/bitcoin/pull/31305, I've pushed a PR fixing this instead of disabling the check: https://github.com/bitcoin/bitcoin/pull/31364

💬 1440000bytes commented on pull request "miner: Reorg Testnet4 minimum difficulty blocks":
(https://github.com/bitcoin/bitcoin/pull/31117#issuecomment-2496167015)
> > 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.

He already shared the link above.

https://altquick.com/exchange/market/BitcoinTestnet4

👍 theStack approved a pull request: "policy: ephemeral dust followups"
(https://github.com/bitcoin/bitcoin/pull/31279#pullrequestreview-2456791069)
lgtm ACK 466e4df3fb83ef82b6add22e202e3a70dbf83a12

If you need to retouch, could also tackle https://github.com/bitcoin/bitcoin/pull/30239#discussion_r1828577696 (e.g. put it into c5c10fd317c6b4c033f3001757e6975b8b9a4942)

💬 theStack commented on pull request "policy: ephemeral dust followups":
(https://github.com/bitcoin/bitcoin/pull/31279#discussion_r1855531801)
nit: imho the name `GetDustIndexes` was fine and more expressive, but no strong feelings

🤔 TheCharlatan reviewed a pull request: "depends, refactor: Avoid hardcoding `host_prefix` in toolchain file"
(https://github.com/bitcoin/bitcoin/pull/31358#pullrequestreview-2456802495)
Guix build (aarch64):
```
1166ebc7f2e39568ced2e7851d93e9521bc867a7b5e61b6416ef9623cc8b82c1 guix-build-d9c8aacce38a/output/aarch64-linux-gnu/SHA256SUMS.part
ec8f92b7a8f3046dcff7d66e20c4e2d719b96e9d9b36ecbcba20c701c39e96a2 guix-build-d9c8aacce38a/output/aarch64-linux-gnu/bitcoin-d9c8aacce38a-aarch64-linux-gnu-debug.tar.gz
27ae1f55b45872b62e9946ff57d5a5cda7f0be4d79409cc7c8b8f0ff5bf6917c guix-build-d9c8aacce38a/output/aarch64-linux-gnu/bitcoin-d9c8aacce38a-aarch64-linux-gnu.tar.gz
ce07cdb3f7
...