💬 maflcko commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2397286103)
(I still want to benchmark the ramdisk difference, to see how much can be squeezed out before switching, but if people think a switch should be done sooner, that is fine, too)

💬 maflcko commented on pull request "test: remove unused code from `script_tests`":
(https://github.com/bitcoin/bitcoin/pull/31051#issuecomment-2397292949)
review ACK e0287bc4b2d83cefb7af48aef457153d0729bcd4

💬 theuni commented on issue "Increasing self-hosted runner raw performance":
(https://github.com/bitcoin/bitcoin/issues/30852#issuecomment-2397293278)
I'm pretty sure the potential hardware is 24core/48threads, so it doesn't quite meet those requirements. There's another available, though I'm currently using it, but I guess sacrificing that one is an option as well :)

💬 hodlinator commented on pull request "Windows bitcoind stall debugging [NOMERGE, DRAFT]":
(https://github.com/bitcoin/bitcoin/pull/30956#discussion_r1790490375)
Thanks! Leftover WIP change. Removed filter in latest push here and to my personal master.

💬 maflcko commented on pull request "rpc: add getorphantxs":
(https://github.com/bitcoin/bitcoin/pull/30793#discussion_r1790492621)
Indeed, for new code, it would be good to not use the ambiguous bool fallback. Especially, given that it is undocumented?

👍 TheCharlatan approved a pull request: "refactor: Replace g_genesis_wait_cv with m_tip_block_cv"
(https://github.com/bitcoin/bitcoin/pull/30967#pullrequestreview-2352371969)
ACK fa22e5c430acaef9713d9a4b4b97bb3f4876f816

Nice improvements to the shutdown handling since I last reviewed. The nits can be ignored.

💬 TheCharlatan commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1790519737)
Nit: This `if` statement is a bit tricky to read, because of the open braces on the next line. How about adding whitespace, or adding braces to the if statement?

💬 TheCharlatan commented on pull request "refactor: Replace g_genesis_wait_cv with m_tip_block_cv":
(https://github.com/bitcoin/bitcoin/pull/30967#discussion_r1790497695)
Nit: The other arguments are using curly braces, is this using parentheses on purpose?

💬 marcofleon commented on pull request "fuzz: Add fuzz-only build mode option for targets":
(https://github.com/bitcoin/bitcoin/pull/31028#discussion_r1790584206)
Agreed, that's a better way to do it. Let me know what you think about d470af336a3f66230e9c68ffc924caad0ad84980. Another possibility I was thinking of was something like:
```
#ifdef FUZZING_BUILD_MODE_UNSAFE_FOR_PRODUCTION
#define CONDITIONAL_REGISTER(name, target, opts) FuzzFrameworkRegisterTarget(name, target, opts)
#else
#define CONDITIONAL_REGISTER(name, target, opts)
do {
...

💬 marcofleon commented on pull request "fuzz: Add fuzz-only build mode option for targets":
(https://github.com/bitcoin/bitcoin/pull/31028#discussion_r1790590334)
Dealing with the logic in `DETAIL_FUZZ` seems better.

💬 brunoerg commented on pull request "net: fuzz: bypass network magic and checksum validation":
(https://github.com/bitcoin/bitcoin/pull/31012#issuecomment-2397513425)
> -v2transport=0 should be added to the bitcoind args used for netdriver fuzzing. Otherwise, it doesn't seem like honggfuzz ever fuzzes anything besides the v2 handshake.

Well noticed, just added it.

💬 Sjors commented on pull request "build: Bump minimum supported macOS to 12.0":
(https://github.com/bitcoin/bitcoin/pull/31048#issuecomment-2397513781)
Trying to test if this still works on macOS 13.7 I got stuck. Most likely unrelated to the changes here, but see #31050.

💬 marcofleon commented on issue "build: RFC Coverage build type":
(https://github.com/bitcoin/bitcoin/issues/31047#issuecomment-2397522603)
I don't have a strong preference for which one is supported by the build system. I happened to learn about [llvm-cov](https://clang.llvm.org/docs/SourceBasedCodeCoverage.html) before coming upon the [coverage](https://github.com/bitcoin/bitcoin/blob/master/doc/developer-notes.md#compiling-for-test-coverage) section in the docs, so that's what I've been using on both macOS and Linux. It's been working well for me so far.

🤔 pablomartin4btc reviewed a pull request: "depends: Print ready-to-use `--toolchain` option for CMake invocation"
(https://github.com/bitcoin/bitcoin/pull/31008#pullrequestreview-2352574245)
ACK 605926da0ab4ac7ae4e9aed55a059f37c31c15b5

💬 Sjors commented on pull request "ci: Add missing -DWERROR=ON to test-each-commit":
(https://github.com/bitcoin/bitcoin/pull/31045#issuecomment-2397533799)
> by assuming `nproc` exists on macos as well

It doesn't, but you can do `alias nproc="sysctl -n hw.physicalcpu"` instead of installing all of `coreutils`. Though on CI that's fine by me too.

https://gist.github.com/oleg-andreyev/053b90ef33d2c29446ef466a2817d01c

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1790645774)
I have incorporated this change.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1790649176)
Done.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1790648786)
Done, with a few changes.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1790648128)
I wouldn't call a Span a container; I have changed it to just saying "A Span such that ...".

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1790649002)
Done.

💬 sipa commented on pull request "cluster mempool: extend DepGraph functionality":
(https://github.com/bitcoin/bitcoin/pull/30857#discussion_r1790647190)
I have phrased it as "this information has to be inferred from the ancestor sets" (and similar for `GetReducedChildren`).