💬 MarcoFalke commented on issue "Confusing log order regarding DNS seeds":
(https://github.com/bitcoin/bitcoin/issues/18332#issuecomment-1466270405)
Closing as per the two previous comments that this is fixed.

🚀 fanquake merged a pull request: "[24.x] Bump version to v24.1rc1"
(https://github.com/bitcoin/bitcoin/pull/27247)

👍 fanquake approved a pull request: "test: Default timeout factor to 4 under --valgrind"
(https://github.com/bitcoin/bitcoin/pull/27221)
ACK fa27cf4cc7c24aa00a66dffabab849d4b3cb1c41 - I still see at least one actual issue when running the functional tests under `--valgrind` (outside the CI system), but will follow up separately with that. Increasing the timeout here seems fine.

🚀 fanquake merged a pull request: "test: Default timeout factor to 4 under --valgrind"
(https://github.com/bitcoin/bitcoin/pull/27221)

👍 john-moffett approved a pull request: "Avoid integer overflow in CheckDiskSpace"
(https://github.com/bitcoin/bitcoin/pull/27235)
ACK 05eeba2c5fb312e0e6a730b01eb7d1b422d75dbb

💬 MarcoFalke commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1134141994)
nit in d96c59cc5cd2f73f1f55c133c52208671fe75ef3:

Looks like the LOC can be reduced by moving all of this into the lambda instead of repeating the passed paths twice and the throw logic twice.

💬 MarcoFalke commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1134152481)
Same commit: At the risk of people running into this, I wonder if this should use `STR_INTERNAL_BUG()` so that more info is printed about the version/commit hash they were using.

💬 MarcoFalke commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1134153527)
Same. (`STR_INTERNAL_BUG(msg)`)

💬 MarcoFalke commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1134147272)
nit: Same about `__func__`

💬 MarcoFalke commented on pull request "assumeutxo: background validation completion":
(https://github.com/bitcoin/bitcoin/pull/25740#discussion_r1134114096)
nit in d96c59cc5cd2f73f1f55c133c52208671fe75ef3: No need to pass func

* This is already done by log internally
* This will print a useless `operator()`, twice: `[validation.cpp:5712] [operator()] operator(): error renaming file`

💬 kiminuo commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1466363604)
Thank you for the review.

> given how easy it would be to do it externally (which may or may not have been the case when people first asked for this?)

So what would be your approach to do it externally then? Especially if you want to get this information more often.

💬 willcl-ark commented on issue "A minor issue of SENDCMPCT msg in handshaking process":
(https://github.com/bitcoin/bitcoin/issues/18390#issuecomment-1466364121)
It appears that Bitcoin Core no longer sends multiple _versions_ of the `SENDCMPT` message:

```fish
$ rg NetMsgType::SENDCMPCT
src/net_processing.cpp
1220: m_connman.PushMessage(pnodeStop, CNetMsgMaker(pnodeStop->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/false, /*version=*/CMPCTBLOCKS_VERSION));
1227: m_connman.PushMessage(pfrom, CNetMsgMaker(pfrom->GetCommonVersion()).Make(NetMsgType::SENDCMPCT, /*high_bandwidth=*/true, /*version=*/CMPCTBLOC
...

💬 MarcoFalke commented on pull request "Improve address decoding errors":
(https://github.com/bitcoin/bitcoin/pull/26514#issuecomment-1466373851)
lgtm ACK 962a0930e699b74b3c4d019427df6e2b3af5c831

✅ MarcoFalke closed an issue: "A minor issue of SENDCMPCT msg in handshaking process"
(https://github.com/bitcoin/bitcoin/issues/18390)

💬 kouloumos commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1466377732)
> So what would be your approach to do it externally then? Especially if you want to get this information more often.

Could a tracepoints approach, similar to https://github.com/bitcoin/bitcoin/pull/26531, work?

👍 MarcoFalke approved a pull request: "refactor: Split logging utilities from system.h"
(https://github.com/bitcoin/bitcoin/pull/27238)
review ACK 4aa0ec6a8617ac77a19e49d8cf081b7a7f6404ae 🐸

<details><summary>Show signature</summary>

Signature:

```
untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 4aa0ec6a8617
...

💬 MarcoFalke commented on pull request "refactor: Split logging utilities from system.h":
(https://github.com/bitcoin/bitcoin/pull/27238#discussion_r1134181883)
should be on the first line and separate section to catch missing includes in the header

💬 davidgumberg commented on pull request "Improve address decoding errors":
(https://github.com/bitcoin/bitcoin/pull/26514#issuecomment-1466387824)
ACK https://github.com/bitcoin/bitcoin/pull/26514/commits/962a0930e699b74b3c4d019427df6e2b3af5c831

💬 kiminuo commented on pull request "Add feerate histogram to getmempoolinfo":
(https://github.com/bitcoin/bitcoin/pull/21422#issuecomment-1466387944)
> > So what would be your approach to do it externally then? Especially if you want to get this information more often.
>
> Could a tracepoints approach, similar to #26531, work?

I need to research that in detail but it looks promising. Thank you for sharing it here!

💬 MarcoFalke commented on pull request "ci: Cache more stuff in the ci images: msan, iwyu, pip, sdks":
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1466450885)
Thanks, fixed