⚠️ dergoegge opened an issue: "Continuous benchmark tracking"
(https://github.com/bitcoin/bitcoin/issues/27284)
It would be beneficial to have continuous tracking of our benchmark tests, because regressions (or unexpected improvements) otherwise go undetected (at least for a while). Afaict currently, the only benefit of our benchmarking tests is to evaluate changes as they are being proposed but imo that only gives us ~50% of the benefit that benchmarks can provide.

I am imagining this to be a separate service (maybe integrated with @DrahtBot) that regularly runs the benchmarks in an environment config
...

💬 MarcoFalke commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1142170248)
nit in b8e35a2: You can use `std::min<size_t>(total, InsecureRandRange(.))` instead. Not that it matters, but this allows to check for unintentional unsigned integer wrap around, I think.

💬 MarcoFalke commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-1476301070)
I think @jamesob set something up at one point, but it had to be queried manually, as there were no notifications. Also, I am not sure if it is running at all. See https://codespeed.bitcoinperf.com/timeline/

💬 fanquake commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1476309129)
> [However, given that the functionality added is currently to just build a histogram based on the individual feerates, I'm a little less clear on its utility and why it is necessary to add and maintain this in Bitcoin Core given how easy it would be to do it externally](https://github.com/bitcoin/bitcoin/pull/21422#pullrequestreview-1336717945)

I agree. I don't think this is something that needs to exist in Core. The fact that you've had this PR open for a long time is unfortunate, but also
...

💬 hebasto commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#issuecomment-1476310112)
Updated e580afee614b93236cb0e37457a5a7c28f4a0b73 -> ebfc2a2dddedcbf384f435716d30d8418a77505b ([pr26749.12](https://github.com/hebasto/bitcoin/commits/pr26749.12) -> [pr26749.13](https://github.com/hebasto/bitcoin/commits/pr26749.13), [diff](https://github.com/hebasto/bitcoin/compare/pr26749.12..pr26749.13)):

- addressed @MarcoFalke's [comment](https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1142170248)

💬 hebasto commented on pull request "refactor: Use move semantics instead of custom swap functions":
(https://github.com/bitcoin/bitcoin/pull/26749#discussion_r1142185014)
Thanks! [Done](https://github.com/bitcoin/bitcoin/pull/26749#issuecomment-1476310112).

💬 jonatack commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-1476316638)
I proposed to @LarryRuane last week to check in with @jamesob about picking up https://bitcoinperf.com/ and checking with @0xB10C about potential cross-fertilization with tracepoints and their dashboards. Also https://github.com/bitcoin/bitcoin/pull/26957#issuecomment-1474955390.

💬 dergoegge commented on pull request "Log new headers":
(https://github.com/bitcoin/bitcoin/pull/27278#discussion_r1142194524)
I think in terms of DoS risk it is pretty similar as the compact block location. Basically: "Only log for BIP 130 announcements that told us about new headers meeting our DoS threshold".

From the logs you shared here: https://twitter.com/jamesob/status/1637191706691903488 it looks like logging for headers announcements would have been useful. Looking at `PeerManagerImpl::NewPoWValidBlock`, we only announce via compact block once per height, so a competing height will be announced via headers.
...

💬 jonatack commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-1476324797)
See also https://github.com/bitcoin/bitcoin/pull/26957#issuecomment-1474761651 by @martinus for one nice way, with an example, to create and share detailed benchmark results.

💬 jonatack commented on pull request "bench: update logging benchmarks":
(https://github.com/bitcoin/bitcoin/pull/26957#issuecomment-1476333698)
> code review & tested ACK, here are my benchmark results:

@martinus could you add the hash of the last commit to your ACK (like https://github.com/bitcoin/bitcoin/pull/26957#issuecomment-1458622474) so that it is counted by DrahtBot above in https://github.com/bitcoin/bitcoin/pull/26957#issuecomment-1400700700? (thanks!)

👍 alexsio27444 approved a pull request: "[24.x] Backports for 24.0.1"
(https://github.com/bitcoin/bitcoin/pull/26616)

💬 TheCharlatan commented on pull request "refactor: Extract util/fs from util/system":
(https://github.com/bitcoin/bitcoin/pull/27254#issuecomment-1476359052)
Thank you for the review @hebasto,

Updated cc662d09386e50eb92a7ccbaa1edff62fd8cdd44 -> 3ceb8dde48fc12f4f16372f661a4cccf7104e194 ([splitSystemFs_5](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_5) -> [splitSystemFs_6](https://github.com/TheCharlatan/bitcoin/commits/splitSystemFs_6), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemFs_5..splitSystemFs_6)) to fix sorting in the Makefile and remove `fs.cpp`, which was still supposed to refer to `fs_helpers.cpp`
...

💬 kiminuo commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1476361661)
Closing the PR.

> If the goal of this change can be achieved externally, with 20 lines of Python, then that's where it should be done.

I would say it's rather a proof that one can _inefficiently_ achieve the goal.

An efficient solution would be to mirror mempool using tracepoints (that did not exist when the PR was created), if[^1] it is possible.

[^1]: I have not found the time to test it yet but **if** it is not possible then that would be imho a very nice PR to allow people to e
...

✅ kiminuo closed a pull request: "Add feerate histogram to getmempoolinfo"
(https://github.com/bitcoin/bitcoin/pull/21422)

💬 glozow commented on pull request "validate package transactions with their in-package ancestor sets":
(https://github.com/bitcoin/bitcoin/pull/26711#discussion_r1142230795)
Ah thanks!

💬 Sjors commented on issue "Memory leak in wallet_tests with BDB using address sanitizer":
(https://github.com/bitcoin/bitcoin/issues/27283#issuecomment-1476364655)
I went back to the #18727 merge commit 0f204dd3f21b997334a0e99954c939db154b64ca where this test was introduced. In order to test that old commit, I used `contrib/install_db4.sh` as it existed at the time, but otherwise didn't use depends (can't get that old version of boost to build). I then recreated a narrowed down test:

```cpp
auto chain = interfaces::MakeChain(m_node);
auto wallet = TestLoadWallet(*chain);
TestUnloadWallet(std::move(wallet));
wallet = TestLoadWallet(*c
...

💬 TheCharlatan commented on pull request "refactor, kernel: Decouple ArgsManager from blockstorage":
(https://github.com/bitcoin/bitcoin/pull/27125#discussion_r1142232528)
> I feel this though, I'll try out the first and second option you laid out.

Both are branching out a bit too much in my taste for the refactor we seek to achieve here. If the `const BlockManager` is a non-starter, I can always revert back to https://github.com/bitcoin/bitcoin/commit/f87c39399823e22c553b797cc66fa4063462a32b

💬 dergoegge commented on issue "Continuous benchmark tracking":
(https://github.com/bitcoin/bitcoin/issues/27284#issuecomment-1476374459)
Honestly I think https://codespeed.bitcoinperf.com/ is pretty close to what we want here. It does seem like that hasn't been running for a while? But getting that running again and adding some kind of notification system is probably all we need.

💬 jessebarton commented on pull request "doc: FreeBSD DataDirectoryGroupReadable Setting":
(https://github.com/bitcoin/bitcoin/pull/26741#issuecomment-1476374767)
@vasild

✅ jessebarton closed a pull request: "doc: FreeBSD DataDirectoryGroupReadable Setting"
(https://github.com/bitcoin/bitcoin/pull/26741)