📝 hebasto opened a pull request: "build: Drop redundant `sys/sysctl.h` header check"
(https://github.com/bitcoin/bitcoin/pull/30327)
The `AC_CHECK_HEADERS` macro defines `HAVE_SYS_SYSCTL_H` if the `sys/sysctl.h` header is found. However, in the source code, this header is guarded by `HAVE_SYSCTL` and `HAVE_SYSCTL_ARND` macros, which have their own checks. Since `HAVE_SYS_SYSCTL_H` is not used, we can skip the `AC_CHECK_HEADERS(... sys/sysctl.h ...)` check.

💬 willcl-ark commented on issue "Porting bcc tools to libbpf":
(https://github.com/bitcoin/bitcoin/issues/30298#issuecomment-2186613731)
@rob-scheepens Thanks for opening this up for discussion again after the developments in `libbpf`.

Is this something that you plan on implementing yourself and wanted to brainstorm on first?

If not (and @0xB10C is not interested in implementing currently due to lack of review) then I'm not sure that keeping this issue open longer than the above discussion is useful and might recommend closing it again until developer interest in this feature picks up...

🤔 fjahr reviewed a pull request: "contrib: asmap-tool - Compare ASMaps with respect to specific addresses"
(https://github.com/bitcoin/bitcoin/pull/30246#pullrequestreview-2135834115)
Looks good to me, just some additional documentation on documentation on the addrs input file would be good to avoid people getting strange results.

💬 fjahr commented on pull request "contrib: asmap-tool - Compare ASMaps with respect to specific addresses":
(https://github.com/bitcoin/bitcoin/pull/30246#discussion_r1651050889)
nit: sys isn't in the right alphabetical order here. You could reorder it while you are at it.

💬 fjahr commented on pull request "contrib: asmap-tool - Compare ASMaps with respect to specific addresses":
(https://github.com/bitcoin/bitcoin/pull/30246#discussion_r1651055643)
You should name the specific settings to run getnodeaddresses with so that the result is as expected. I guess that would be 0 and all (default) right? I fear that if people do it wrong they could get a result like "nothing was reassigned" and then think: oh great, the internet is so stable nowadays ;)

💬 fjahr commented on pull request "contrib: asmap-tool - Compare ASMaps with respect to specific addresses":
(https://github.com/bitcoin/bitcoin/pull/30246#discussion_r1651063291)
Maybe add the count of addresses that were unassigned here specifically in the summary. Those may be among the most interesting to look at so I think it could be highlighted.

💬 jlopp commented on pull request "Drop -dbcache limit":
(https://github.com/bitcoin/bitcoin/pull/28358#issuecomment-2186630679)
Concept ACK.

For reference, I ran several sync tests with different dbcache sizes to see how much of an effect they have on performance. A good 25% improvement vs default when my machine abstained from flushing to disk. https://blog.lopp.net/effects-dbcache-size-bitcoin-node-sync-speed/

💬 fanquake commented on pull request "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin":
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2186671730)
> These changes result in a modest performance improvement:

I ran the benchmark on an aarch64 machine, and saw mostly worse performance when using the latest GCC, or Clang, with libstdc++. There was a small improvement when using Clang and libc++.

GCC 14.1.1 20240620 (Red Hat 14.1.1-6)

Master
```bash
| ns/op | op/s | err% | ins/op | cyc/op | IPC | bra/op | miss% | total | benchmark
|--------------------:|-------------
...

✅ fanquake closed an issue: "Check usages of `#if defined(...)`"
(https://github.com/bitcoin/bitcoin/issues/16419)

🚀 fanquake merged a pull request: "build: add `-Wundef`"
(https://github.com/bitcoin/bitcoin/pull/29876)

💬 fanquake commented on pull request "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core"":
(https://github.com/bitcoin/bitcoin/pull/30308#issuecomment-2186691312)
> Changing it would break that test.

Not in our repository though? In any case, this could be revisited if someone follows up with a CI change.

💬 paplorinc commented on pull request "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin":
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2186691697)
Thanks for checking @fanquake!
As the profiler showed, this is only 5% of `AssembleBlock`, and the benchmark varied a lot on my machine as well, it might be why you also see the discrepancy.
Do you think I should add a dedicated benchmark for this only?

🚀 fanquake merged a pull request: "QA: Expect PACKAGE_NAME rather than constant "Bitcoin Core""
(https://github.com/bitcoin/bitcoin/pull/30308)

💬 fanquake commented on pull request "optimization: Reduce cache lookups in CCoinsViewCache::FetchCoin":
(https://github.com/bitcoin/bitcoin/pull/30326#issuecomment-2186716560)
I think that rather than trying to micro-optimise these somewhat sensitive functions, if you're interested in benchmarking / performance related changes, your time might be better spent reviewing Pull Requests like #28280 (which your changes conflict with).

I'd also note that all of your benchmarking / profiling seems to be being done on an arm64 macOS machine, using (I assume) Apple Clang as the compiler, and libc++ as the standard library. So when you create changes like these, even if you
...

🤔 hodlinator requested changes to a pull request: "doc: clarify loadwallet path loading for wallets"
(https://github.com/bitcoin/bitcoin/pull/30302#pullrequestreview-2135964825)
Concept ACK a0dffe6318d32b7b49b7d0be6d45b59ee3e0f4ec

`git range-diff ee15876~1..ee15876 a0dffe6~1..a0dffe6`
Agree that "regular" wording wasn't needed.

💬 hodlinator commented on pull request "doc: clarify loadwallet path loading for wallets":
(https://github.com/bitcoin/bitcoin/pull/30302#discussion_r1651128863)
Would have written it closer to something like:
```suggestion
fs::path absolutePath;
#ifdef WIN32
if (const char* homedrive = std::getenv("HOMEDRIVE"), * homepath = std::getenv("HOMEPATH");
homedrive != nullptr && homedrive[0] != 0 && homepath != nullptr && homepath[0] != 0)
absolutePath = fs::path(homedrive) / fs::path(homepath);
else
absolutePath = fs::path("C:\\Users\\Satoshi");
#else
if (const char* home = std::getenv("HOME"); home != nullptr
...

💬 fanquake commented on issue "Check usages of `#if defined(...)`":
(https://github.com/bitcoin/bitcoin/issues/16419#issuecomment-2186722083)
Going to close this now that #29876 is merged.

🤔 furszy reviewed a pull request: "prune, rpc: Check undo data when finding pruneheight"
(https://github.com/bitcoin/bitcoin/pull/29668#pullrequestreview-2135821455)
Code review ACK 8789dc8f315a9d9ad7142d831bc9412f780248e7.
Left few nits.

💬 furszy commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1651079954)
little topic:
maybe changing the RPC docs to say that "pruneheight" returns "the first block unpruned, all previous blocks were pruned" is more useful than saying "height of the last block pruned, plus one" as is now.

💬 furszy commented on pull request "prune, rpc: Check undo data when finding pruneheight":
(https://github.com/bitcoin/bitcoin/pull/29668#discussion_r1651156361)
```suggestion
const CBlockIndex* first_unpruned{Assert(blockman.GetFirstBlock(*chain_tip, /*status_mask=*/BLOCK_HAVE_MASK, first_block))};
if (first_unpruned == first_block) {
```