🤔 jonatack reviewed a pull request: "ci: Use `ninja` to build in macOS native CI job"
(https://github.com/bitcoin/bitcoin/pull/30915#pullrequestreview-2310534703)
LGTM but am new to CMake builds.

ACK d01b85bfecbcf16ea16f90e2ade7537bf582269f

💬 davidgumberg commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763724027)
Could be scripted diff:

```bash
sed -i -E 's/CheckWriteCoins\(([^,]+), ([^,]+), ([^,]+), ([^,]+), ([^,]+), ([^,]+)\);/CheckWriteCoins({\1, \4}, {\2, \5}, {\3, \6});/' src/test/coins_tests.cpp
```

👍 hebasto approved a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2310572569)
re-ACK 807a429a33cd9b1986f1ebb8fd761887f556afd8, rebased only since my recent [review](https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2300357639).

My Guix build:
```
aarch64
f6a91156bc9ac2ec9367d75a433d903db4fe5c4ff0c5680289882f1ca58b9759 guix-build-807a429a33cd/output/aarch64-linux-gnu/SHA256SUMS.part
2704a2fbc76888fb9073631513ea1fc2203edb344814dbe5db02fd51c6d9949d guix-build-807a429a33cd/output/aarch64-linux-gnu/bitcoin-807a429a33cd-aarch64-linux-gnu-debug.tar.gz
a5c
...

💬 davidgumberg commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763730500)
Although I guess this section would be ugly to do as a scripted diff

💬 hebasto commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#discussion_r1763730646)
@theuni

You might be interesting in reviewing this PR, as this comment was added at your request while working on the CMake staging branch, if I recall correctly :)

💬 davidgumberg commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#discussion_r1763731654)
Nit: Could be made into a loop as below

🤔 instagibbs reviewed a pull request: "cluster mempool: extend DepGraph functionality"
(https://github.com/bitcoin/bitcoin/pull/30857#pullrequestreview-2310464388)
it's likely the wrong place for the coverage, but I added some coverage for iterative additional and removal of transactions. Take what you think is useful if any:

```
diff --git a/src/test/fuzz/cluster_linearize.cpp b/src/test/fuzz/cluster_linearize.cpp
index be06898a6e..62b5ec2343 100644
--- a/src/test/fuzz/cluster_linearize.cpp
+++ b/src/test/fuzz/cluster_linearize.cpp
@@ -1,19 +1,20 @@
// Copyright (c) The Bitcoin Core developers
// Distributed under the MIT software license, see
...

💬 instagibbs commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1763665495)
Was reconstructing graphs incorrectly leading to inconsistent sets. Better to fail here early.

```Suggestion
{
// We can't add relationships to holes
Assume(parents.IsSubsetOf(m_used));
```
a check like this for `AddDependency` would also be good:

`Assume(SetType::Singleton(parent).IsSubsetOf(m_used));`

💬 davidgumberg commented on pull request "refactor: prohibit direct flags access in CCoinsCacheEntry and remove invalid tests":
(https://github.com/bitcoin/bitcoin/pull/30906#issuecomment-2356666728)
Concept ACK on dropping the bitfield interface for `CCoinsCacheEntry` flags

🤔 theuni reviewed a pull request: "build: drop obj/ subdirectory for generated build.h"
(https://github.com/bitcoin/bitcoin/pull/30856#pullrequestreview-2310630029)
Concept ACK.

Though, `obj/` was acting as a bit of a namespace here (IIRC, that was another reason for it to be in a subdir). Especially since this is included with `<>` as opposed to `""`, while we're at it, I think we should rename this file. `build.h` seems generic enough to collide with something else. `bitcoin-build.h` maybe?

💬 pablomartin4btc commented on pull request "cli: Improve error message on multiwallet cli-side commands":
(https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2356746568)
Updates:
- Removed validation commit which I'll added in a separate PR as [suggested](https://github.com/bitcoin/bitcoin/pull/26990#issuecomment-2354091658) by @jonatack.
- Updated description and title accordingly with above.
- Added test checking -rpcwallet=<filename> is not doable as [suggested](https://github.com/bitcoin/bitcoin/pull/26990#discussion_r1762109275) by @jonatack.
- Replaced `-rpcwallet=<filename>` which is incorrect with `-rpcwallet=<walletname>` as suggested by @mzumsande
...

💬 maflcko commented on pull request "ci: Use clang-19 from apt.llvm.org":
(https://github.com/bitcoin/bitcoin/pull/30634#issuecomment-2356758336)
testing tsan on aarch gives me:

<details><summary>tsan error</summary>

```
src/wallet/test/wallet_tests.cpp(75): Leaving test case "scan_for_wallet_transactions"; testing time: 2316025us
src/wallet/test/wallet_tests.cpp(921): Entering test case "CreateWalletWithoutChain"
==================
WARNING: ThreadSanitizer: lock-order-inversion (potential deadlock) (pid=62275)
Cycle in lock order graph: M0 (0xffff74604608) => M1 (0xffff7460cb58) => M2 (0xffff7460cd08) => M0

Mutex M1 acq
...

💬 theuni commented on pull request "build: drop obj/ subdirectory for generated build.h":
(https://github.com/bitcoin/bitcoin/pull/30856#issuecomment-2356804690)
Likewise for `config/bitcoin-config.h` btw. IIRC `config/` part is an old artifact of the switch _to_ autotools. I don't see any reason why bitcoin-config.h shouldn't just live in `build/src/` now.

Err.. I guess that would mean touching a _lot_ of files though :)

💬 maflcko commented on pull request "ci: Use clang-19 from apt.llvm.org":
(https://github.com/bitcoin/bitcoin/pull/30634#issuecomment-2356842989)
Testing on aarch64, Asan and Fuzz pass. The Tsan issue on aarch64 is pre-existing and unrelated to this pull request, so can be ignored.

💬 hodlinator commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2356850948)
Nice work @rkrux!

Counter intuitive behavior for the TRUC novice towards the end of [V3 Transactions TRUC](https://github.com/bitcoin-core/bitcoin-devwiki/wiki/28.0-Release-Candidate-Testing-Guide#2-v3-transactions-truc):

* `testmempoolaccept` shows `siblingTx2` as being in `TRUC-violation` and not allowed, but its child `txWithUncle` has no issues listed.
* `submitpackage` then accepts `siblingTx2` but rejects `txWithUncle`. (Wouldn't it be nice if `submitpackage` was atomic, all or noth
...

💬 maflcko commented on pull request "ci: Use clang-19 in msan tasks":
(https://github.com/bitcoin/bitcoin/pull/30639#issuecomment-2356852422)
Tested on aarch64 and both passed:

* `MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_native_msan.sh" ./ci/test_run_all.sh`
* `MAKEJOBS="-j$(nproc)" FILE_ENV="./ci/test/00_setup_env_native_fuzz_with_msan.sh" ./ci/test_run_all.sh`

🤔 furszy reviewed a pull request: "cli: Improve error message on multiwallet cli-side commands"
(https://github.com/bitcoin/bitcoin/pull/26990#pullrequestreview-2311022398)
utACK 54227e681a4

💬 hodlinator commented on issue "28.0 RC Testing Guide Feedback":
(https://github.com/bitcoin/bitcoin/issues/30854#issuecomment-2356951108)
## General feedback

Including the working directory is not necessary as the aliases operate on absolute paths. It also leaves land mines when copy-pasting from the guide:
`➜ 27-test bcli27 getblockchaininfo` -> `bcli27 getblockchaininfo` (or `➜ bcli27 getblockchaininfo`), etc
`rctesting bcli28` -> `bcli28`
`test28rc bcli28` -> `bcli28` (`test28rc` is named `28-rc-test` at the beginning of the guide, assuming both are directory names).


## [From Source](https://github.com/bitcoin-core
...

👍 jarolrod approved a pull request: "ci: Use `ninja` to build in macOS native CI job"
(https://github.com/bitcoin/bitcoin/pull/30915#pullrequestreview-2311089315)
ACK d01b85bfecbcf16ea16f90e2ade7537bf582269f

👍 hodlinator approved a pull request: "log: Use ConstevalFormatString"
(https://github.com/bitcoin/bitcoin/pull/30889#pullrequestreview-2311103729)
re-ACK facbcd4cef8890ae18976fb53b67ea56b3c04454

`git range-diff master fabd78e facbcd4`: Undid `Args` -> `Params` rename based on nit.