💬 fanquake commented on pull request "build: don't show ccache summary with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31983#issuecomment-2697305165)
> Why not use https://github.com/bitcoin/bitcoin/commit/c19a187c42fe867d61ca5dbd48ae18f15201839f instead?

From my read of #30861 nobody seems to agree that we should make this change? https://github.com/bitcoin/bitcoin/pull/30861#discussion_r1937227612

💬 Sjors commented on pull request "scripted-diff: rename libmultiprocess repository":
(https://github.com/bitcoin/bitcoin/pull/31982#issuecomment-2697315226)
Concept ACK

💬 eval-exec commented on pull request "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds":
(https://github.com/bitcoin/bitcoin/pull/31979#issuecomment-2697317938)
CI has passed, so I think this PR is ready for review.

💬 Sjors commented on pull request "guix: Notarize MacOS app bundle and codesign all MacOS and Windows binaries":
(https://github.com/bitcoin/bitcoin/pull/31407#issuecomment-2697341062)
Tested ACK e181bda061ca63021511be6e286fdf6a5818df49

I only tested on macOS (Intel and Apple Silicon), and only lightly reviewed the code.

💬 hebasto commented on pull request "build: don't show ccache summary with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31983#discussion_r1979315337)
The [`MSVC`](https://cmake.org/cmake/help/latest/variable/MSVC.html) variable is defined based on the compiler. However, the current ccache settings rely on the [`<LANG>_COMPILER_LAUNCHER`](https://cmake.org/cmake/help/latest/prop_tgt/LANG_COMPILER_LAUNCHER.html) properties, which are considered depending on the generator rather than the compiler.

💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979319485)
Changed to this

💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979319968)
Thanks, dropped.

💬 hebasto commented on pull request "build: Enhance Ccache performance across worktrees and build trees":
(https://github.com/bitcoin/bitcoin/pull/30861#issuecomment-2697383952)
Closing and marking "Up for grabs".

✅ hebasto closed a pull request: "build: Enhance Ccache performance across worktrees and build trees"
(https://github.com/bitcoin/bitcoin/pull/30861)

💬 laanwj commented on pull request "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds":
(https://github.com/bitcoin/bitcoin/pull/31979#discussion_r1979326582)
`timeout` should be a float, this a comparison on two float values returning a float.

💬 laanwj commented on pull request "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds":
(https://github.com/bitcoin/bitcoin/pull/31979#discussion_r1979328349)
imo it would be better to do the capping here:
```c++
reconnect_timeout = std::min(reconnect_timeout * RECONNECT_TIMEOUT_EXP, RECONNECT_TIMEOUT_MAX);
```

🤔 naiyoma reviewed a pull request: "wallet: fix crash on double block disconnection"
(https://github.com/bitcoin/bitcoin/pull/31757#pullrequestreview-2657287497)
TestedACK [https://github.com/bitcoin/bitcoin/pull/31757/commits/c80b105f77d8ee397c7dc07b013c37b7df63abd5](https://github.com/bitcoin/bitcoin/pull/31757/commits/c80b105f77d8ee397c7dc07b013c37b7df63abd5)

cherry-picked the test commit on master branch and verified that the test case fails on the second invalidation:

```
2025-03-04T12:18:39.077000Z TestFramework (ERROR): Unexpected exception caught during testing
raise RemoteDisconnected("Remote end closed connection without"
http.client.
...

⚠️ arejula27 opened an issue: "Request for Wiki Edit Permissions – Testing Guide: Bitcoin Core 29.0 RC"
(https://github.com/bitcoin/bitcoin/issues/31984)
Hello, maintainers,
We are currently working on the Testing Guide: Bitcoin Core 29.0 Release Candidate as part of the [BOSS program](https://learning.chaincode.com/#seminars). We are:
@musaHaruna
@arejula27
@janb84
@Prabhat1308
We would like to request permission to edit the Wiki so we can contribute and publish our guide.

Looking forward to your response.

Thank you!

💬 laanwj commented on pull request "torcontrol: Limit reconnect timeout to max seconds and log delay in whole seconds":
(https://github.com/bitcoin/bitcoin/pull/31979#issuecomment-2697457225)
Concept ACK.

It's debatable whether we need an exponential backoff here in the first place. After all Tor is generally a local service, there is no risk of creating a DoS. i added it back then, to reduce the amount of log spam in case Tor isn't running. However we have a better logging system now that takes both category and level into account, so that motivation is no longer relevant.

💬 l0rinc commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979362441)
Typo, maybe something like:
```suggestion
Using `lld` is required due to issues with Apple's `ld` and `LLVM`.
```

💬 l0rinc commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979383156)
Nice, finally we can get rid of that (tested, seems to be working without the warning, good find)!
Should we anything to https://github.com/llvm/llvm-project/issues/102630?

💬 m3dwards commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#discussion_r1979412430)
gah, taken!

Thanks for the spot.

💬 hebasto commented on pull request "scripted-diff: rename libmultiprocess repository":
(https://github.com/bitcoin/bitcoin/pull/31982#issuecomment-2697553252)
Concept ACK.

💬 l0rinc commented on pull request "doc: update fuzz instructions when on macOS":
(https://github.com/bitcoin/bitcoin/pull/31954#issuecomment-2697572586)
ACK 75486c8ed87a480b9f0c4dc7a10f3cd4eee87b12

Compared to [previous review](https://github.com/bitcoin/bitcoin/compare/725c0fd95b15157bd0ec141cc24d27818f536656..75486c8ed87a480b9f0c4dc7a10f3cd4eee87b12#diff-7ff2e1d1ed59fa96766a663236d71bfdd00b4af3321fde39f7dbf022bd968a0a), `APPEND_LDFLAGS` was removed and the description simplified to avoid repetition.

💬 laanwj commented on pull request "refactor: modernize outdated trait patterns using helper aliases (C++14/C++17)":
(https://github.com/bitcoin/bitcoin/pull/31904#issuecomment-2697629617)
Concept and code review ACK 4cd95a2921805f447a8bcecc6b448a365171eb93

Seems low-risk. As this is compile-time logic, i don't think this should actually affect the compiled code? (haven't verified, though)

🤔 vasild reviewed a pull request: "doc: Update documentation to include Clang/llvm based coverage report generation"
(https://github.com/bitcoin/bitcoin/pull/31933#pullrequestreview-2656773286)
Approach ACK 1245e05325b41101eddc76ba9214d910489e35b6

In the light of https://github.com/bitcoin/bitcoin/issues/31047 and https://github.com/bitcoin/bitcoin/pull/31394 I have been thinking that is would be useful to at least document the steps needed to gather coverage with clang. Thanks for the initiative!