💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650962320)
Taken in 7ad5349a2ec

💬 willcl-ark commented on pull request "init: Add option for rpccookie permissions (replace 26088)":
(https://github.com/bitcoin/bitcoin/pull/28167#discussion_r1650960724)
Thanks, now done in 7ad5349a2ec

💬 sipa commented on pull request "Several randomness improvements":
(https://github.com/bitcoin/bitcoin/pull/29625#discussion_r1650969447)
I have addressed this by making `StartScheduledTasks` not use `m_rng`, but just grab a new `FastRandomContext` instead; performance doesn't matter for this function.

👍 brunoerg approved a pull request: "contrib: asmap-tool - Compare ASMaps with respect to specific addresses"
(https://github.com/bitcoin/bitcoin/pull/30246#pullrequestreview-2135729296)
ACK 42eb7b144c09f0404487f2f5d17ca1a3e91c9365

🚀 fanquake merged a pull request: "ci: add option for running tests without volume"
(https://github.com/bitcoin/bitcoin/pull/30310)

👍 hebasto approved a pull request: "build: add `-Wundef`"
(https://github.com/bitcoin/bitcoin/pull/29876#pullrequestreview-2135788139)
ACK e3dc64f4990a15df3fd6147831f66fc2a31c71ad, I have reviewed the code and it looks OK.

💬 furszy commented on pull request "wallet: notify when preset + automatic inputs exceed max weight":
(https://github.com/bitcoin/bitcoin/pull/30309#discussion_r1651026133)
> Any specific reason to generate 1472 outputs when 1471 are picked?

All are picked. The extra output is to cover the fee.

📝 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)