✅ maflcko closed an issue: "descriptor: Tapscript-specific Miniscript key serialization / parsing leads to fuzz timeouts"
(https://github.com/bitcoin/bitcoin/issues/30168)

💬 maflcko commented on issue "descriptor: Tapscript-specific Miniscript key serialization / parsing leads to fuzz timeouts":
(https://github.com/bitcoin/bitcoin/issues/30168#issuecomment-2244797028)
This was fixed in commit 262260ce1e919613ba60194a5861b0b0a51dfe01

💬 paplorinc commented on pull request "rest: Reject truncated hex txid early in getutxos parsing":
(https://github.com/bitcoin/bitcoin/pull/30482#discussion_r1687798251)
You mean it's a "virtual property" - I can work with that, thanks, resolve it please.
(though it's weird that the getter seems to reverse the hex back to big-endian, weird, but I guess that's for another PR)

💬 hodlinator commented on pull request "fix: Make TxidFromString() respect string_view length":
(https://github.com/bitcoin/bitcoin/pull/30436#discussion_r1687805203)
Indeed.. saw it too. Seems like temporary code that somehow made it into the test. Guess I could remove if I retouch.

⚠️ maflcko opened an issue: "fuzz: crypter timeout"
(https://github.com/bitcoin/bitcoin/issues/30503)
Found by oss-fuzz

[clusterfuzz-testcase-crypter-4690510954954752.bin.not.txt](https://github.com/user-attachments/files/16347247/clusterfuzz-testcase-crypter-4690510954954752.bin.not.txt)


```
FUZZ=crypter perf record -g --call-graph dwarf ./src/test/fuzz/fuzz ./clusterfuzz-testcase-crypter-4690510954954752
... half a minute ...
hotspot ./perf.data
```



💬 maflcko commented on issue "fuzz: crypter timeout":
(https://github.com/bitcoin/bitcoin/issues/30503#issuecomment-2244828423)
My recommendation would be to limit the number of iterations to something reasonable, but still enough to cover all cases.

💬 theStack commented on pull request "crypto, refactor: add new KeyPair class":
(https://github.com/bitcoin/bitcoin/pull/30051#discussion_r1687820299)
Fwiw, we have a signing benchmark for taproot key-path spends, haven't verified yet if it covers the discussed code path though: https://github.com/bitcoin/bitcoin/blob/910d38b22f575cba9a3325de3f4c5ac667d4a487/src/bench/sign_transaction.cpp#L67

> In this case, I do think improving the clarity of the code and removing extra steps at the cost of performance seems worth it? I can also write a benchmark to quantify if performance here is indeed a concern.

No hard feelings on that topic from my
...

📝 vasild opened a pull request: "doc: use proper doxygen formatting for CTxMemPool::cs"
(https://github.com/bitcoin/bitcoin/pull/30504)
Having `@par title` followed by an empty line renders improperly in Doxygen - it results in a paragraph with a title but without a body.

https://www.doxygen.nl/manual/commands.html#cmdpar

This also results in a compiler warning (or error) with Clang 19:

```
./txmempool.h:368:34: error: empty paragraph passed to '@par' command [-Werror,-Wdocumentation]
368 | * @par Consistency guarantees
| ~~~~~~~~~~~~~~~~~~~~~~~~~~^
1 error generated.
```

<!--
*** Please rem
...

💬 fanquake commented on pull request "depends: build expat with CMake":
(https://github.com/bitcoin/bitcoin/pull/29878#issuecomment-2244849619)
Guix Build (aarch64):
```bash
4ed36603100265640e4e9e73fa8f04c2b10a581c0a68f37cb636f1cbff71469c guix-build-cf46c6b8f2ff/output/aarch64-linux-gnu/SHA256SUMS.part
70b0f40dec0646cc7378c17bb569c8aa292ceba93e9b7233e7a0067fde04a24c guix-build-cf46c6b8f2ff/output/aarch64-linux-gnu/bitcoin-cf46c6b8f2ff-aarch64-linux-gnu-debug.tar.gz
e778fdbf6dcf74a8cfeac8d3b353f877809f96a6f4759e497d939b001874a357 guix-build-cf46c6b8f2ff/output/aarch64-linux-gnu/bitcoin-cf46c6b8f2ff-aarch64-linux-gnu.tar.gz
c99a80
...

⚠️ maflcko opened an issue: "fuzz: crypto_fschacha20poly1305 timeout"
(https://github.com/bitcoin/bitcoin/issues/30505)
Found by oss-fuzz

[clusterfuzz-testcase-crypto_fschacha20poly1305-6583678709334016.txt](https://github.com/user-attachments/files/16347368/clusterfuzz-testcase-crypto_fschacha20poly1305-6583678709334016.txt)


```
FUZZ=crypto_fschacha20poly1305 perf record -g --call-graph dwarf ./src/test/fuzz/fuzz ./clusterfuzz-testcase-crypto_fschacha20poly1305-6583678709334016
...
hotspot ./perf.data
```

![Screenshot from 2024-07-23 12-34-02](https://github.com/user-attachments/assets/3111eab4-
...

💬 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