💬 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.
(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
...
(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
(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)
(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?
(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
...
(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
(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
(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!
(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
(https://github.com/bitcoin/bitcoin/pull/27028#issuecomment-1466450885)
Thanks, fixed
💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1466454098)
thank you @amitiuttarwar and @brunoerg! I've updated the PR to include your suggestions:
- added release notes
- test coverage for different networks
- hidden RPC check
(https://github.com/bitcoin/bitcoin/pull/26988#issuecomment-1466454098)
thank you @amitiuttarwar and @brunoerg! I've updated the PR to include your suggestions:
- added release notes
- test coverage for different networks
- hidden RPC check
💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134245231)
done.
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134245231)
done.
💬 stratospher commented on pull request "rpc: Add test-only RPC getaddrmaninfo for new/tried table address count":
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134253991)
adding addresses from different networks wouldn't cause these kind of collisions because [`GetGroup()`](https://github.com/bitcoin/bitcoin/blob/b5868f4b1f884e8d6612f34ca4005fe3a992053d/src/netgroup.cpp#L17) would differ right?
(https://github.com/bitcoin/bitcoin/pull/26988#discussion_r1134253991)
adding addresses from different networks wouldn't cause these kind of collisions because [`GetGroup()`](https://github.com/bitcoin/bitcoin/blob/b5868f4b1f884e8d6612f34ca4005fe3a992053d/src/netgroup.cpp#L17) would differ right?
💬 MarcoFalke commented on pull request "ci: A few fixes of `ccache` issues":
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1466466171)
lgtm. While it would be good to be able to point to docs, I don't think this is required. Happy to review if you move this out of draft.
(https://github.com/bitcoin/bitcoin/pull/27084#issuecomment-1466466171)
lgtm. While it would be good to be able to point to docs, I don't think this is required. Happy to review if you move this out of draft.
💬 TheCharlatan commented on pull request "refactor: Split logging utilities from system.h":
(https://github.com/bitcoin/bitcoin/pull/27238#issuecomment-1466482704)
Updated [4aa0ec6](https://github.com/bitcoin/bitcoin/pull/27238/commits/4aa0ec6a8617ac77a19e49d8cf081b7a7f6404ae) -> aaced5633b81b2f08b993106a527e2a0e1d663c1 ([splitSystemLogging_0](https://github.com/TheCharlatan/bitcoin/tree/splitSystemLogging_0) -> [splitSystemLogging_1](https://github.com/TheCharlatan/bitcoin/tree/splitSystemLogging_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemLogging_0..splitSystemLogging_1)) to fix header inclusion order addressing @MarcoFalke
...
(https://github.com/bitcoin/bitcoin/pull/27238#issuecomment-1466482704)
Updated [4aa0ec6](https://github.com/bitcoin/bitcoin/pull/27238/commits/4aa0ec6a8617ac77a19e49d8cf081b7a7f6404ae) -> aaced5633b81b2f08b993106a527e2a0e1d663c1 ([splitSystemLogging_0](https://github.com/TheCharlatan/bitcoin/tree/splitSystemLogging_0) -> [splitSystemLogging_1](https://github.com/TheCharlatan/bitcoin/tree/splitSystemLogging_1), [compare](https://github.com/TheCharlatan/bitcoin/compare/splitSystemLogging_0..splitSystemLogging_1)) to fix header inclusion order addressing @MarcoFalke
...
💬 instagibbs commented on pull request "[POLICY] Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1466487189)
Switched to OP_TRUE, as it was less confusing for at least a few people, and only requires light test editing. Had to rebase on top of master for trivial merge conflict.
(https://github.com/bitcoin/bitcoin/pull/26403#issuecomment-1466487189)
Switched to OP_TRUE, as it was less confusing for at least a few people, and only requires light test editing. Had to rebase on top of master for trivial merge conflict.
💬 MarcoFalke commented on pull request "refactor: Split logging utilities from system.h":
(https://github.com/bitcoin/bitcoin/pull/27238#issuecomment-1466487829)
re-ACK aaced5633b81b2f08b993106a527e2a0e1d663c1 🐍
<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: re-ACK aaced5633b81b2f08b99
...
(https://github.com/bitcoin/bitcoin/pull/27238#issuecomment-1466487829)
re-ACK aaced5633b81b2f08b993106a527e2a0e1d663c1 🐍
<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: re-ACK aaced5633b81b2f08b99
...
💬 instagibbs commented on pull request "[POLICY] Ephemeral anchors":
(https://github.com/bitcoin/bitcoin/pull/26403#discussion_r1134315587)
suggested in other PR: "switch the ordering? else if (whichType != ANCHOR && IsDust())"
(https://github.com/bitcoin/bitcoin/pull/26403#discussion_r1134315587)
suggested in other PR: "switch the ordering? else if (whichType != ANCHOR && IsDust())"
✅ glozow closed an issue: "Error messages for invalid address"
(https://github.com/bitcoin/bitcoin/issues/21741)
(https://github.com/bitcoin/bitcoin/issues/21741)
🚀 glozow merged a pull request: "Improve address decoding errors"
(https://github.com/bitcoin/bitcoin/pull/26514)
(https://github.com/bitcoin/bitcoin/pull/26514)