📝 hodlinator opened a pull request: "net: Disconnect message follow-ups to #28521"
(https://github.com/bitcoin/bitcoin/pull/31633)
- Add missing calls to `DisconnectMsg()` - https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1890824361
- Specify context when stopping nodes - https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1890780754
- Bring back log message when resetting socket in case new entrypoints are added - https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1890795074
- Use `DisconnectMsg()` in `CConnman` as well - https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1791797716
(https://github.com/bitcoin/bitcoin/pull/31633)
- Add missing calls to `DisconnectMsg()` - https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1890824361
- Specify context when stopping nodes - https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1890780754
- Bring back log message when resetting socket in case new entrypoints are added - https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1890795074
- Use `DisconnectMsg()` in `CConnman` as well - https://github.com/bitcoin/bitcoin/pull/28521#discussion_r1791797716
💬 Sjors commented on pull request "mining: bugfix: Fix duplicate coinbase tx weight reservation":
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2582401336)
re-ACK dba211a9ceafc57a953efc757adfe09c0f3fdb14
(https://github.com/bitcoin/bitcoin/pull/31384#issuecomment-2582401336)
re-ACK dba211a9ceafc57a953efc757adfe09c0f3fdb14
👍 danielabrozzoni approved a pull request: "init,log: Unify block index log line"
(https://github.com/bitcoin/bitcoin/pull/31616#pullrequestreview-2542141582)
tACK e04be3731f4921cd51d25b1d6210eace7600fea4
(https://github.com/bitcoin/bitcoin/pull/31616#pullrequestreview-2542141582)
tACK e04be3731f4921cd51d25b1d6210eace7600fea4
🚀 fanquake merged a pull request: "doc: upgrade license to 2025."
(https://github.com/bitcoin/bitcoin/pull/31611)
(https://github.com/bitcoin/bitcoin/pull/31611)
💬 maflcko commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#issuecomment-2582444004)
review ACK 286de9749612565fd8d42f08f66c8426faf8a1f7 📋
<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 286de9749612
...
(https://github.com/bitcoin/bitcoin/pull/31633#issuecomment-2582444004)
review ACK 286de9749612565fd8d42f08f66c8426faf8a1f7 📋
<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 286de9749612
...
👍 danielabrozzoni approved a pull request: "test: importdescriptors is not available for non-descriptor wallets"
(https://github.com/bitcoin/bitcoin/pull/31609#pullrequestreview-2542174643)
My understanding of the legacy removal PRs is limited, but it appears that the importdescriptor function will still include the check for legacy wallets.
utACK f9dc45680fa958bc89335fa50f46052e6bb29b37
(https://github.com/bitcoin/bitcoin/pull/31609#pullrequestreview-2542174643)
My understanding of the legacy removal PRs is limited, but it appears that the importdescriptor function will still include the check for legacy wallets.
utACK f9dc45680fa958bc89335fa50f46052e6bb29b37
💬 stickies-v commented on pull request "kernel: Move kernel-related cache constants to kernel cache":
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1910229692)
> Are there places where this would be re-used?
I can't quite find one - it's also a bit difficult to search for given how much `<<` is used for streams too. I agree there's no need to generalize too early, but given how similar the functionality is I still have a preference for the more generic `CheckedLeftShift()` and/or `SaturatingLeftShift()` as suggested by @ryanofsky. But, I'm okay with either approach.
> I think I would drop the MiBToBytes function and just write the constants as by
...
(https://github.com/bitcoin/bitcoin/pull/31483#discussion_r1910229692)
> Are there places where this would be re-used?
I can't quite find one - it's also a bit difficult to search for given how much `<<` is used for streams too. I agree there's no need to generalize too early, but given how similar the functionality is I still have a preference for the more generic `CheckedLeftShift()` and/or `SaturatingLeftShift()` as suggested by @ryanofsky. But, I'm okay with either approach.
> I think I would drop the MiBToBytes function and just write the constants as by
...
🚀 fanquake merged a pull request: "depends: Fix spacing issue"
(https://github.com/bitcoin/bitcoin/pull/31627)
(https://github.com/bitcoin/bitcoin/pull/31627)
👍 BrandonOdiwuor approved a pull request: "init,log: Unify block index log line"
(https://github.com/bitcoin/bitcoin/pull/31616#pullrequestreview-2542199335)
Code Review ACK e04be3731f4921cd51d25b1d6210eace7600fea4
(https://github.com/bitcoin/bitcoin/pull/31616#pullrequestreview-2542199335)
Code Review ACK e04be3731f4921cd51d25b1d6210eace7600fea4
🚀 fanquake merged a pull request: "tracing: Rename the `MIN` macro to `_TRACEPOINT_TEST_MIN` in log_raw_p2p_msgs"
(https://github.com/bitcoin/bitcoin/pull/31623)
(https://github.com/bitcoin/bitcoin/pull/31623)
🚀 fanquake merged a pull request: "init,log: Unify block index log line"
(https://github.com/bitcoin/bitcoin/pull/31616)
(https://github.com/bitcoin/bitcoin/pull/31616)
👍 l0rinc approved a pull request: "net: Disconnect message follow-ups to #28521"
(https://github.com/bitcoin/bitcoin/pull/31633#pullrequestreview-2542183397)
utACK 286de9749612565fd8d42f08f66c8426faf8a1f7
(https://github.com/bitcoin/bitcoin/pull/31633#pullrequestreview-2542183397)
utACK 286de9749612565fd8d42f08f66c8426faf8a1f7
💬 l0rinc commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910228378)
I found the `%d%s` format weird at first, but I see it is an existing format, e.g. https://github.com/bitcoin/bitcoin/blob/master/src/net.cpp#L705
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910228378)
I found the `%d%s` format weird at first, but I see it is an existing format, e.g. https://github.com/bitcoin/bitcoin/blob/master/src/net.cpp#L705
💬 l0rinc commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910230992)
super-nit: above we're using a simple `,` before the `DisconnectMsg`, not a `;`.
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910230992)
super-nit: above we're using a simple `,` before the `DisconnectMsg`, not a `;`.
💬 l0rinc commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910231692)
nit: many other similar logs here were capitalized
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910231692)
nit: many other similar logs here were capitalized
💬 l0rinc commented on pull request "net: Disconnect message follow-ups to #28521":
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910245234)
unrelated: do we really need this level of detail for a timeout (i.e. ms precision expressed in seconds)?
(https://github.com/bitcoin/bitcoin/pull/31633#discussion_r1910245234)
unrelated: do we really need this level of detail for a timeout (i.e. ms precision expressed in seconds)?
💬 hebasto commented on issue "depends: llvm-ranlib (etc): "No such file or directory" on Intel macOS 15.0":
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849)
From my understanding, the issue arises because the `make` provided by Xcode does not set the `.SHELLFLAGS` variable properly. Here's an example of the output:
```
% whereis make
make: /usr/bin/make /Applications/Xcode.app/Contents/Developer/usr/share/man/man1/make.1
% make --version
GNU Make 3.81
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR
...
(https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849)
From my understanding, the issue arises because the `make` provided by Xcode does not set the `.SHELLFLAGS` variable properly. Here's an example of the output:
```
% whereis make
make: /usr/bin/make /Applications/Xcode.app/Contents/Developer/usr/share/man/man1/make.1
% make --version
GNU Make 3.81
Copyright (C) 2006 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.
There is NO warranty; not even for MERCHANTABILITY or FITNESS FOR A
PARTICULAR
...
💬 hebasto commented on pull request "depends: fix for llvm-ranlib (etc): 'No such file or directory' macOS 15.0":
(https://github.com/bitcoin/bitcoin/pull/30994#issuecomment-2582537242)
> > Not sure about this issue, or making this change. Looks like it needs more investigation. See my comment here: [#30978 (comment)](https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388086723).
>
> Yes, switched to draft until we have a better understanding of the issue.
See https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849.
(https://github.com/bitcoin/bitcoin/pull/30994#issuecomment-2582537242)
> > Not sure about this issue, or making this change. Looks like it needs more investigation. See my comment here: [#30978 (comment)](https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2388086723).
>
> Yes, switched to draft until we have a better understanding of the issue.
See https://github.com/bitcoin/bitcoin/issues/30978#issuecomment-2582535849.
💬 fanquake commented on pull request "build: Improve `ccache` performance for different build directories":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2582556693)
What is the status of this?
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2582556693)
What is the status of this?
💬 maflcko commented on pull request "test: importdescriptors is not available for non-descriptor wallets":
(https://github.com/bitcoin/bitcoin/pull/31609#issuecomment-2582574580)
Huh? Now you are adding a test for a previous release, which does not increase the test coverage, see also https://corecheck.dev/bitcoin/bitcoin/pulls/31609 .
If you want to test a previous release, it would be better to submit the test against the previous release branch. But I am not sure if this is worth it.
Also, I don't understand why the check should be kept after the bdb removal. Why would one want to guard against bdb at runtime when it is impossible to load bdb?
(https://github.com/bitcoin/bitcoin/pull/31609#issuecomment-2582574580)
Huh? Now you are adding a test for a previous release, which does not increase the test coverage, see also https://corecheck.dev/bitcoin/bitcoin/pulls/31609 .
If you want to test a previous release, it would be better to submit the test against the previous release branch. But I am not sure if this is worth it.
Also, I don't understand why the check should be kept after the bdb removal. Why would one want to guard against bdb at runtime when it is impossible to load bdb?