💬 hebasto commented on pull request "ci: update pwsh to use custom shell that fails-fast":
(https://github.com/bitcoin/bitcoin/pull/32672#issuecomment-3031898351)
Concept ACK.

Defining a new custom `job.defaults.run.shell` for all Windows jobs allows us to revert f8619196ceb5c6a58125506d276d9515837f043a and use the same shell across all steps. This is especially convenient for reproducing failures locally.

📝 fanquake opened a pull request: "cmake: Use `AUTHOR_WARNING` for warnings"
(https://github.com/bitcoin/bitcoin/pull/32865)
Rather than create our own warning logging/handling, which isn't ideal (#31476), use CMakes [`AUTHOR_WARNING`](https://cmake.org/cmake/help/latest/command/message.html) (also pointed out by purpleKarrot here: https://github.com/bitcoin/bitcoin/pull/31665#issuecomment-2667723356), and turn on `-Werror=dev` in the CI.

Would result in failures like:
```bash
-- Performing Test LINKER_SUPPORTS__WL__Z_SEPARATE_CODE - Success
-- Could NOT find Python3 (missing: Python3_EXECUTABLE Interpreter) (Re
...

✅ fanquake closed a pull request: "fix typos and improve clarity in documentation"
(https://github.com/bitcoin/bitcoin/pull/32864)

💬 fanquake commented on pull request "fix typos and improve clarity in documentation":
(https://github.com/bitcoin/bitcoin/pull/32864#issuecomment-3031908342)
Thanks. I'll squash this and pull it into another branch.

💬 kilavvy commented on pull request "fix typos and improve clarity in documentation":
(https://github.com/bitcoin/bitcoin/pull/32864#issuecomment-3031914311)
> Thanks. I'll squash this and pull it into another branch.

@fanquake why not merged?

💬 l0rinc commented on pull request "fix typos and improve clarity in documentation":
(https://github.com/bitcoin/bitcoin/pull/32864#discussion_r2182542317)
https://www.collinsdictionary.com/dictionary/english/useable

Try to contribute something that provides more value

💬 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.