Bitcoin Core Github
44 subscribers
121K links
Download Telegram
πŸ‘ stickies-v approved a pull request: "[24.x] qt: 24.1rc1 translations update"
(https://github.com/bitcoin/bitcoin/pull/27251)
I ran `bitcoin-maintainer-tools/update-translations.py` and verified that my diff matches with a2f8a839d9b071f9c90ed5e3db2d30f90993992d when discarding `bitcoin_nl.ts`.

I'm not familiar with the translations process and I couldn't verify that `bitcoin_nl.ts` is damaged, but if this constitutes sufficient review then ACK a2f8a839d9b071f9c90ed5e3db2d30f90993992d
πŸ’¬ willcl-ark commented on issue "Confusing log order regarding DNS seeds":
(https://github.com/bitcoin/bitcoin/issues/18332#issuecomment-1466253933)
I think this can be close now that the log messages are no longer returned in a confusing way since #16939:

```log
2023-03-13T14:30:03Z 0 block-relay-only anchors will be tried for connections.
2023-03-13T14:30:03Z init message: Starting network threads…
2023-03-13T14:30:03Z dnsseed thread start
2023-03-13T14:30:03Z init message: Done loading
2023-03-13T14:30:03Z opencon thread start
2023-03-13T14:30:03Z Loading addresses from DNS seed seed.bitcoin.sprovoost.nl.
2023-03-13T14:30:03Z ne
...
πŸš€ fanquake merged a pull request: "[24.x] qt: 24.1rc1 translations update"
(https://github.com/bitcoin/bitcoin/pull/27251)
βœ… MarcoFalke closed an issue: "Confusing log order regarding DNS seeds"
(https://github.com/bitcoin/bitcoin/issues/18332)
πŸ’¬ 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
...