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

💬 fjahr commented on pull request "validation: fix m_best_header tracking and BLOCK_FAILED_CHILD assignment":
(https://github.com/bitcoin/bitcoin/pull/30666#issuecomment-2356970307)
reACK 0bd53d913c1c2ffd2d0779f01bc51c81537b6992

💬 mzumsande commented on pull request "p2p: Increase inbound capacity for block-relay only connections":
(https://github.com/bitcoin/bitcoin/pull/28463#discussion_r1764098741)
What is meant with is that half of the slots can only be accessed by block-relay-only peers, and the other half is accessible to both:
"with 50% of inbound slots accessible for tx-relaying peers" and "half of
which can **only** be taken up by low-traffic block-relay-only peers" are therefore both saying the same thing:

If we'd have 0 tx-relay inobund peers, in theory all inbound slots could be filled up with block-relay-only peers, whereas if we had 0 block-relay-only inbound peers, still
...

💬 mzumsande commented on pull request "p2p: Increase inbound capacity for block-relay only connections":
(https://github.com/bitcoin/bitcoin/pull/28463#issuecomment-2357051015)
Rebased for cmake, and fixed silent conflicts with #30750.

💬 mzumsande commented on pull request "p2p: When close to the tip, download blocks in parallel from additional peers to prevent stalling":
(https://github.com/bitcoin/bitcoin/pull/29664#issuecomment-2357062649)
[824c125 ](https://github.com/bitcoin/bitcoin/commit/824c1254ed4ae248ff44ca536e82c1238b86b158)to [1bcfcf2](https://github.com/bitcoin/bitcoin/commit/1bcfcf27c134c6b78a53447bc91a78001c6181f9): Rebased