💬 fanquake commented on pull request "contrib: simplify `test-security-check`":
(https://github.com/bitcoin/bitcoin/pull/30423#issuecomment-2244852254)
Guix Build (x86_64):
```bash
222a30ae2094e1e2536e5a1e8794d04f300fbf6ed0f67e0f6dcb4160e3d075a7 guix-build-1bc9f64bee91/output/aarch64-linux-gnu/SHA256SUMS.part
f7a7b2a6f7e85aa633d6cdb9907bf11cf05c3bdabdf32a4fefaf2bb63264583d guix-build-1bc9f64bee91/output/aarch64-linux-gnu/bitcoin-1bc9f64bee91-aarch64-linux-gnu-debug.tar.gz
8884a25e5a214b224d2c7d73e19786174ddb4eed37c2daa0dc57c23d1f49fc3c guix-build-1bc9f64bee91/output/aarch64-linux-gnu/bitcoin-1bc9f64bee91-aarch64-linux-gnu.tar.gz
3685576
...

💬 maflcko commented on issue "fuzz: crypto_fschacha20poly1305 timeout":
(https://github.com/bitcoin/bitcoin/issues/30505#issuecomment-2244853706)
My recommendation would be to reduce the number of iterations from 10'000 to something that still covers all edge cases.

💬 maflcko commented on pull request "Add fuzz test for FSChaCha20Poly1305, AEADChacha20Poly1305":
(https://github.com/bitcoin/bitcoin/pull/28263#issuecomment-2244855137)
Timeout in https://github.com/bitcoin/bitcoin/issues/30505

💬 maflcko commented on pull request "doc: use proper doxygen formatting for CTxMemPool::cs":
(https://github.com/bitcoin/bitcoin/pull/30504#issuecomment-2244874245)
review ACK 6a5e9e40e1dd3d397020703feb9aa0b6f4577c98

📝 hebasto opened a pull request: "depends: Cleanup postprocess commands after switching to CMake"
(https://github.com/bitcoin/bitcoin/pull/30506)
I overlooked this while reviewing https://github.com/bitcoin/bitcoin/pull/29723, https://github.com/bitcoin/bitcoin/pull/29835, and https://github.com/bitcoin/bitcoin/pull/29880.

👍 tdb3 approved a pull request: "doc: use proper doxygen formatting for CTxMemPool::cs"
(https://github.com/bitcoin/bitcoin/pull/30504#pullrequestreview-2193632828)
ACK 6a5e9e40e1dd3d397020703feb9aa0b6f4577c98
Looks to be rendered correctly now:


💬 josibake commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1687848841)
Ah, nice! I'll see if I can easily modify the benchmark to validate that the difference is indeed negligible (and quantify the difference in the event it is not negligible).

> Just wanted to point out that this could be a slight drawback.

It's a good observation! I have a slight preference for clarity over performance in this context, but good you mentioned it in case others feel differently.

💬 paplorinc commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1687854257)
it doesn't cover the path, but this one does:
```patch
Index: src/bench/sign_transaction.cpp
IDEA additional info:
Subsystem: com.intellij.openapi.diff.impl.patch.CharsetEP
<+>UTF-8
===================================================================
diff --git a/src/bench/sign_transaction.cpp b/src/bench/sign_transaction.cpp
--- a/src/bench/sign_transaction.cpp (revision 8d573611575c3fa66f08407aa9b02f91b29a94c3)
+++ b/src/bench/sign_transaction.cpp (date 1721731869277)
@@ -12,6 +12,7 @
...

💬 maflcko commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687788710)
```suggestion
connections. It was not possible to switch this off even if the node didn't
```

🤔 maflcko reviewed a pull request: "doc: add release notes for #22729"
(https://github.com/bitcoin/bitcoin/pull/30502#pullrequestreview-2193537102)
left some nits/questions

💬 maflcko commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687790716)
```suggestion
use Tor. Previously, a configuration `bind=addr:port` would result in
```

Is this an example? If yes, it would be good to list all breaking changes, instead of only one.

💬 maflcko commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687788247)
```suggestion
- Previously, if Bitcoin Core were listening for P2P connections, then
```

grammar nit.

💬 maflcko commented on pull request "guix: use GCC 13 to builds releases":
(https://github.com/bitcoin/bitcoin/pull/29881#discussion_r1687859398)
Maybe try https://packages.ubuntu.com/noble/g++-mingw-w64-x86-64-posix , or do you need https://packages.debian.org/trixie/mingw-w64-x86-64-dev ?

💬 Sjors commented on pull request "net: additional disconnect logging":
(https://github.com/bitcoin/bitcoin/pull/28521#issuecomment-2244921840)
Thanks @davidgumberg. I added a helper function `CNode::DisconnectMsg`, and it's now also used for:
* "receiving message bytes failed"
* "socket send error for"
* "socket recv error"
* "socket closed"

In order to get rid of the redundant generic "disconnecting peer" in `CNode::CloseSocketDisconnect()`, I added the log statement to the two remaining call sites.

Since it's a method on `CNode` I can use it in net_processing as well, for which I added a separate commit. It has the effect o
...

💬 fjahr commented on pull request "test: assumeutxo: add missing tests in wallet_assumeutxo.py":
(https://github.com/bitcoin/bitcoin/pull/30455#discussion_r1687867805)
> Thanks for the feedback. I added the fastprune option to my local copy and I'm working to understand how pruning operates.

In general, check `feature_index_prune.py`, there extensive tests using pruning and I think you can learn a bit more from that how much you have to mine for the fast pruning to work as you need it.

> However, after the background validation and sync to the tip ([here](https://github.com/bitcoin/bitcoin/pull/30455/files#diff-f763a7cf53e7a5183f66f2634baef84245919265a09
...

💬 vasild commented on issue "Fuzzing related configuration/build options can be improved":
(https://github.com/bitcoin/bitcoin/issues/30318#issuecomment-2244972146)
Opened https://github.com/hebasto/bitcoin/pull/276 to address this. I decided to preserve the behavior of the option controlling `ABORT_ON_FAILED_ASSUME` plus to set up the build targets because that will hopefully result in less friction with people that are already familiar with the existent workflow. Just rename the option.

As a fancy extension, just an idea, maybe detect if `-DSANITIZERS=fuzzer` has been used an enable the option automatically.

💬 vasild commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687896551)
Done.

💬 vasild commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687896685)
Done.

💬 vasild commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#discussion_r1687899396)
Applied the `s/For example/Previously/` change. Initially I wrote "for example `1.2.3.4:8333`" but changed it to "for example `addr:port`", now this covers all is not an example anymore.

💬 vasild commented on pull request "doc: add release notes for #22729":
(https://github.com/bitcoin/bitcoin/pull/30502#issuecomment-2244994924)
`620bbc77d9...307659733e`: address suggestions