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

💬 pinheadmz commented on pull request "Support JSON-RPC 2.0 when requested by client":
(https://github.com/bitcoin/bitcoin/pull/27101#issuecomment-1476400553)
This PR is ready for code review if any of you fine handsome concept-ACKers have the time ❤️

📝 jessebarton reopened a pull request: "doc: FreeBSD DataDirectoryGroupReadable Setting"
(https://github.com/bitcoin/bitcoin/pull/26741)
Updating tor.md doc to include mention of FreeBSD requiring the DataDirectoryGroupReadable be set to 1.
Default per the FreeBSD man page is 0.


DataDirectoryGroupReadable 0|1
If this option is set to 0, don't allow the filesystem group to
read the DataDirectory. If the option is set to 1, make the
DataDirectory readable by the default GID. (Default: 0)

💬 virtu commented on pull request "mempool: Add mempool tracepoints":
(https://github.com/bitcoin/bitcoin/pull/26531#issuecomment-1476426500)
- [As requested here](https://github.com/bitcoin/bitcoin/pull/26531#issuecomment-1472375220), the`mempool:removed` tracepoint now includes the time the removed transaction originally entered the mempool so mempool residency times can be calculated: [4684aa3](https://github.com/bitcoin/bitcoin/commit/4684aa3d1721a772862f3087b4154f7c07854051)
- Addressed [feedback on demo script memory usage](https://github.com/bitcoin/bitcoin/pull/26531#issuecomment-1472375220) by introducing explicit counters f
...