💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2182544165)
I added this recursive `LogPrintStr_` because I think it's better to have separate log statements than inserting into the existing string, and added the `should_ratelimit=false` which makes it safe for infinite recursion. I forgot to add the clang-tidy suppression though, [causing this CI failure](https://github.com/bitcoin/bitcoin/pull/32604/checks?check_run_id=45248600899). It seems like we can only suppress this by marking all of `LogPrintStr_` as `NOLINT`, instead of just this single source,
...

💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2182461594)
This is causing a [TSan issue](https://cirrus-ci.com/task/6014464291504128?logs=ci#L2383):

<details>
<summary> logs </summary>

```
[18:53:46.472] WARNING: ThreadSanitizer: data race (pid=9715)
[18:53:46.472] Read of size 8 at 0x723c00000068 by thread T1 (mutexes: write M0):
[18:53:46.472] #0 std::__1::unique_ptr<BCLog::LogRateLimiter, std::__1::default_delete<BCLog::LogRateLimiter>>::operator bool[abi:ne200100]() const /usr/lib/llvm-20/bin/../include/c++/v1/__memory/unique_ptr.h:
...

💬 l0rinc commented on pull request "test: check P2SH sigop count for coinbase tx":
(https://github.com/bitcoin/bitcoin/pull/32850#issuecomment-3031930388)
post-merge crACK

💬 stickies-v commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2182549098)
Sorry, please disregard. I think I misread this as `std::string, threadname` and probably didn't even try compiling the suggestion.

💬 kilavvy commented on pull request "fix typos and improve clarity in documentation":
(https://github.com/bitcoin/bitcoin/pull/32864#issuecomment-3031934039)
@l0rinc I can revert the change if needed, no problem. I also fixed a typo in the function name — it's now correctly spelled as `expensive_calculation`. I do believe that documentation and naming consistency are important, especially in a project like Bitcoin, where clarity helps both users and developers avoid confusion.

💬 maflcko commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#discussion_r2182569619)
should this also be enabled in `.github/ci-test-each-commit-exec.py`, `.github/workflows/ci.yml`, and `contrib/guix/libexec/build.sh`? Otherwise, it is possible that warnings are missed in those contexts?

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2182576679)
Will implement. I should have caught this as well -- I am glad we have TSAN in CI. I will test the branch locally under TSAN.

💬 fanquake commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#issuecomment-3031968877)
cc @purpleKarrot

💬 hebasto commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#discussion_r2182601204)
All warnings were intentionally printed just after the summary to draw the user's attention to them.

📝 rkrux opened a pull request: "doc: add note for watch-only wallet migration"
(https://github.com/bitcoin/bitcoin/pull/32866)
This was suggested in a previous PR #31423.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experience
significantly:

*
...

💬 rkrux commented on pull request "wallet: migration, avoid creating spendable wallet from a watch-only legacy wallet":
(https://github.com/bitcoin/bitcoin/pull/31423#issuecomment-3031999844)
I did the TODO mentioned in the PR description in PR #32866.

💬 instagibbs commented on issue "Notes on Compact Block getdata fallback responses":
(https://github.com/bitcoin/bitcoin/issues/13370#issuecomment-3032029389)
Is it a protocol violation, or otherwise dangerous, to just serve the block you advertised even though you later found it was invalid?

📝 rkrux opened a pull request: "doc: mention key removal in rpc interface modification"
(https://github.com/bitcoin/bitcoin/pull/32867)
A discussion in a previous PR 32618 prompted me to add this note.

<!--
*** Please remove the following help text before submitting: ***

Pull requests without a rationale and clear improvement may be closed
immediately.

GUI-related pull requests should be opened against
https://github.com/bitcoin-core/gui
first. See CONTRIBUTING.md
-->

<!--
Please provide clear motivation for your patch and explain how it improves
Bitcoin Core user experience or Bitcoin Core developer experienc
...

💬 rkrux commented on pull request "doc: mention key removal in rpc interface modification":
(https://github.com/bitcoin/bitcoin/pull/32867#issuecomment-3032050281)
I want to mention about how long is the deprecation/grace period generally but I don't know right now. Suggestions welcome.

📝 theStack opened a pull request: "test: refactor: overhaul block hash determination for `CBlock{,Header}` objects"
(https://github.com/bitcoin/bitcoin/pull/32868)
Similar to what #32421 did for `CTransaction` instances, this PR aims to improve the block hash determination of `CBlockHeader`/`CBlock` (the latter is a subclass of the former) instances by removing the block header caching mechanism and introducing consistent naming. Without the statefulness, sneaky testing bugs like #32742 and #32823 are less likely to happen in the future. Note that performance is even less of an issue here compared to `CTransaction`, as we only need to hash 80 bytes, which
...

🤔 hebasto reviewed a pull request: "ci: update pwsh to use custom shell that fails-fast"
(https://github.com/bitcoin/bitcoin/pull/32672#pullrequestreview-2983019444)
Left a couple of notes regarding the current implementation:

1. The output in the "Get tool information" step has changed:
```diff
--- old 2025-07-03 13:37:57.062194420 +0100
+++ new 2025-07-03 13:35:57.800242845 +0100
@@ -1,3 +1,15 @@
+
+Name Value
+---- -----
+PSVersion 7.4.10
+PSEdition Core
+GitCommitId 7.4.10
+OS Microsoft Windows 10.0.20
...

💬 fanquake commented on pull request "cmake: Use `AUTHOR_WARNING` for warnings":
(https://github.com/bitcoin/bitcoin/pull/32865#discussion_r2182696708)
I've added `.github/ci-test-each-commit-exec.py` & `.github/workflows/ci.yml`. I'll add a commit for Guix once the build passes.

💬 Sjors commented on pull request "wallet: allow skipping script paths":
(https://github.com/bitcoin/bitcoin/pull/32857#issuecomment-3032145576)
The `walletprocesspsbt` RPC now also has this option. Added test coverage by expanding `wallet_taproot.py`.

💬 Crypt-iQ commented on pull request "log: Mitigate disk filling attacks by rate limiting LogPrintf, LogInfo, LogWarning, LogError":
(https://github.com/bitcoin/bitcoin/pull/32604#discussion_r2182726209)
Unfortunate that it can only be resolved by marking the function as `NOLINT`. Addressed.

💬 maflcko commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#discussion_r2182756040)
Just for additional reference, the check is already sometimes failing on current master: https://github.com/bitcoin/bitcoin/issues/30797#issue-2503043392