💬 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

💬 achow101 commented on pull request "Doc: add a comment referencing past vulnerability next to where it was fixed":
(https://github.com/bitcoin/bitcoin/pull/30538#issuecomment-2357146939)
~0

I'm not sure that having a comment that references code that no longer exists is all that useful.

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#discussion_r1764183486)
txrequest and the bloom filters take uint256s, so this seemed easiest. I don't feel strongly. `RelayTransaction` isn't called here...?

💬 glozow commented on pull request "refactor: TxDownloadManager + fuzzing":
(https://github.com/bitcoin/bitcoin/pull/30110#issuecomment-2357211750)
> Ran txdownloadman_impl and it crashed on this assertion:

> I think it'd make sense for ReceivedTx to abort trying to find a package if the parent is in reject filter, even if also in recent reject filter.

Thanks @marcofleon @instagibbs! Was able to reproduce the crash.

I hadn't noticed that quirk about the nested `if` conditions before - we were kind of expecting that a tx wouldn't have multiple things wrong with it. I agree it makes more sense to quit on anything that's non-reconside
...

💬 davidgumberg commented on pull request "net, net_processing: additional and consistent disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1764221454)
nit:
```suggestion
LogInfo("Peer is stalling block download, %s\n", pto->DisconnectMsg(fLogIPs));
```

💬 glozow commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1764225430)
Is the hope to be able to reconstruct direct parents of a transaction from the txgraph? Even if a direct parent is also an indirect ancestor?

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764228793)
thank you! Addressed this in [bc4a929](https://github.com/bitcoin/bitcoin/pull/30875/commits/bc4a929cd716cd2b412c70754749d4fda0ca2a10)

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1764229116)
No, that's impossible (A->B->C is indistinguishable from (A->B->C,A->C)); the goal is just computing a set of dependencies whose corresponding ancestry matches the DepGraph.

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764229313)
thank you! Addressed this in [bc4a929](https://github.com/bitcoin/bitcoin/pull/30875/commits/bc4a929cd716cd2b412c70754749d4fda0ca2a10)

I will take a look at the backtick nit, to see if I can find already existing instances of `cmake` with no backticks

💬 kevkevinpal commented on pull request "doc: fixed inconsistencies in documentation between autotools to cmake change":
(https://github.com/bitcoin/bitcoin/pull/30875#discussion_r1764229340)
thank you! Addressed this in [bc4a929](https://github.com/bitcoin/bitcoin/pull/30875/commits/bc4a929cd716cd2b412c70754749d4fda0ca2a10)