💬 l0rinc commented on pull request "rest: rename `strURIPart` -> `uri_part`":
(https://github.com/bitcoin/bitcoin/pull/32825#issuecomment-3015210185)
reACK 856f4235b1ae56540e1d2279c27405d44a5c7b34

💬 pinheadmz commented on pull request "rpc: use CScheduler for HTTPRPCTimer":
(https://github.com/bitcoin/bitcoin/pull/32796#issuecomment-3015228080)
> drop HTTPRPCTimer altogether

Interesting, I can try that approach... for that matter, could we rip out `QtRPCTimerInterface` and the base class `RPCTimerInterface` as well? I don't think any of it is used outside of `walletpassphrase` and any future RPCs we add can just use the node/wallet scheduler as well...

💬 HowHsu commented on pull request "validation: fetch block inputs on parallel threads 10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2173389714)
> Is there anything prohibiting us from doing something like this to minimize synchronization and lock contention during the fetch phase? I understand some synchronization would still be needed during the merge, but this could help reduce global locks and unnecessary synchronization throughout the process.

As far as I know, the advantage of coroutines over threads is faster context switching, since it doesn’t go through the operating system kernel. This advantage only becomes apparent unde
...

💬 zaidmstrr commented on pull request "rpc: Handle -named argument parsing where Base64 encoding is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3015984834)
I generalised this change to any string parameter which might contain the `=` char in it. I also added the functional test for most of the RPCs I included in the new table. For any new RPC method which might need to be parsed this way with -named enabled, one can add the new entry of that RPC in the `vRPCStringParams` table.

💬 gmaxwell commented on pull request "test: disable secp256 tests by default":
(https://github.com/bitcoin/bitcoin/pull/32782#issuecomment-3016185525)
As an author of that library I am uncomfortable with any usage unguarded by correctness tests. The software is sensitive to compiler errors (which can be triggered by all sorts of random environmental nonsense) and ought to be tested with the exact compiler environment used. A single predictable bit error in signing is enough to leak private keys and can happen in ways that still result in valid signatures. (e.g. miscompilation that results in the first byte/bit of the nonce being set to any s
...

💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#issuecomment-3016379665)
Low-latency HTTP request benchmark improvement - compared with `main` branch with https://github.com/bitcoin/bitcoin/pull/32541#discussion_r2169518557:
```
$ ab -k -c 1 -n 100000 http://localhost:8332/rest/txfromblock/000000000000000000017bfd05b5fa367a424c4a565a4baf7950d9e8605df8ec-5000.bin

Document Path: /rest/txfromblock/000000000000000000017bfd05b5fa367a424c4a565a4baf7950d9e8605df8ec-5000.bin
Document Length: 234 bytes

Concurrency Level: 1
Time taken for tests:
...

👋 l0rinc's pull request is ready for review: "clang-format: modernize and realign clang-format configuration"
(https://github.com/bitcoin/bitcoin/pull/32813)

💬 romanz commented on pull request "Replace libevent with our own HTTP and socket-handling implementation":
(https://github.com/bitcoin/bitcoin/pull/32061#issuecomment-3016428956)
The latency of 404 response has improved too:
```
$ ab -k -c 1 -n 100000 http://localhost:8332/rest/
...
Time taken for tests: 2.876 seconds
Complete requests: 100000
Failed requests: 0
Non-2xx responses: 100000
Keep-Alive requests: 100000
Total transferred: 9500000 bytes
HTML transferred: 0 bytes
Requests per second: 34774.44 [#/sec] (mean)
Time per request: 0.029 [ms] (mean)
Transfer rate: 3226.14 [Kbytes/sec] received
```

`ma
...

🤔 romanz reviewed a pull request: "Replace libevent with our own HTTP and socket-handling implementation"
(https://github.com/bitcoin/bitcoin/pull/32061#pullrequestreview-2969148112)
Tested ACK e531a7cd2c

💬 l0rinc commented on pull request "policy: make pathological transactions packed with legacy sigops non-standard":
(https://github.com/bitcoin/bitcoin/pull/32521#discussion_r2173661408)
Doing a differential perf flame graph on b3bb4031 vs 1ba50d81 (master vs rebased `darosior:2503_nonstd_tx_sigops`) indicates the slowdown is indeed caused by both `AccessCoin` and the extra `CScript::GetSigOpCount` calls:
<img width="1000" alt="image" src="https://github.com/user-attachments/assets/24ef8b91-bd9c-4fc0-be1a-1c7564d2a418" />

<details>
<summary>Details</summary>

```bash
COMMITS="b3bb4031ab32a1306c610f8683b25b8459b21dbc 1ba50d81b251c705aae872a2a9b37778ffa50c62"; \
CC=gcc; C
...

💬 HowHsu commented on pull request "checkqueue: implement a new scriptcheck worker pool with atomic variables":
(https://github.com/bitcoin/bitcoin/pull/32791#issuecomment-3016695705)
> hmmm...Ok, I realized that this may only be suitable for when all the preout Coin are in CCoinsViewCache, otherwise the `Add()` is not fast due to I/O there.

With #31132 , this one makes sense again, need to test them together in all-cache-miss case

💬 HowHsu commented on pull request "checkqueue: set MAX_SCRIPTCHECK_THREADS to nCores - 1":
(https://github.com/bitcoin/bitcoin/pull/32692#issuecomment-3016699198)
I post a new scriptcheck pool implementation #32791 with atomic variables rather than mutex to reduce thread sync contention. That achieves very good performance improvement. But the idea is based on that `the adding period is very short compared to the script verification period`, so it has to be with PR #31132. @sipa and others, welcome to take a look.

📝 HowHsu reopened a pull request: "checkqueue: set MAX_SCRIPTCHECK_THREADS to nCores - 1"
(https://github.com/bitcoin/bitcoin/pull/32692)
A fixed MAX_SCRIPTCHECK_THREADS value is not flexible for users to leverage their cpu resources, and a value large than nCores - 1 doesn't make sense since it only adds some context switch overhead. Set it to nCores - 1. Assumption: A user who sets the number of script verification workers is aware of how this affects the system performance, otherwise he/she leaves it as default (which is 0)

💬 HowHsu commented on pull request "checkqueue: set MAX_SCRIPTCHECK_THREADS to nCores - 1":
(https://github.com/bitcoin/bitcoin/pull/32692#issuecomment-3016707658)
> Maybe this is something best solved by runtime benchmarking? (Or perhaps just at startup, since early blocks may not give us the parallelization we need for it)

I think we can do it so, binary search the best worker thread number with bencharking on some standard test data.

💬 yuvicc commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#issuecomment-3016746882)
Concept ACK

```
./build/bin/bitcoin-cli -named finalizepsbt cHNidP8BAJoCAAAAAqvNEjSrzRI0q80SNKvNEjSrzRI0q80SNKvNEjSrzRI0AAAAAAD9////NBLNqzQSzas0Es2rNBLNqzQSzas0Es2rNBLNqzQSzasBAAAAAP3///8CoIYBAAAAAAAWABQVQBGVs/sqFAmC8HZ8O=g1htqivkANAwAAAAAAFgAUir7MzgyzDnRMjdkVa7d+Dwr07jsAAAAAAAAAAAA=

error code: -8
error message:
Unknown named parameter cHNidP8BAJoCAAAAAqvNEjSrzRI0q80SNKvNEjSrzRI0q80SNKvNEjSrzRI0AAAAAAD9////NBLNqzQSzas0Es2rNBLNqzQSzas0Es2rNBLNqzQSzasBAAAAAP3///8CoIYBAAAAAAAWABQVQBGVs/
...

💬 yuvicc commented on pull request "bench: replace embedded raw block with configurable block generator":
(https://github.com/bitcoin/bitcoin/pull/32554#issuecomment-3016802388)
Concept ACK

This one is pretty useful as I can test with up-to date test_data.

💬 dergoegge commented on pull request "[WIP] Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#issuecomment-3016809091)
I fuzzed this branch (https://github.com/bitcoin/bitcoin/pull/28676/commits/3bfaedbd7d9d6e71e9df03fbbb51a33c6184cca6) with [fuzzamoto](https://github.com/dergoegge/fuzzamoto) and it found an assertion crash in `CTxMemPool::check`.

```
2025-06-29T15:20:42Z (mocktime: 2031-04-03T07:09:03Z) [bench] - Disconnect block: 0.72ms
2025-06-29T15:20:42Z (mocktime: 2031-04-03T07:09:03Z) [prune] basic block filter index prune lock moved back to 200
2025-06-29T15:20:42Z (mocktime: 2031-04-03T07:09:03Z)
...

🤔 BrandonOdiwuor reviewed a pull request: "rpc: Handle -named argument parsing where '=' character is used"
(https://github.com/bitcoin/bitcoin/pull/32821#pullrequestreview-2969336275)
Tested ACK e246741db1f2f056fac32d0f3967f55306e78b49

Confirmed that commit 59fcefcd1f490f8ae276150421cbd895a10f3d4a correctly handles named argument parsing for parameters containing = characters—an issue currently reproducible on master.

<commit: 59fcefcd1f490f8ae276150421cbd895a10f3d4a>
<img width="1512" alt="Screenshot 2025-06-29 at 19 49 25" src="https://github.com/user-attachments/assets/fc4e2230-453d-4691-9ff5-db980693bc85" />


<branch: master>
<img width="1512" alt="Screenshot
...

📝 jlopp opened a pull request: "add more bad p2p ports"
(https://github.com/bitcoin/bitcoin/pull/32826)
Add a few more ports used by extremely well adopted services that require authentication and really ought not be used by bitcoin nodes for p2p traffic.

📝 l0rinc opened a pull request: "mempool: Avoid needless vtx iteration in `removeForBlock` during IBD"
(https://github.com/bitcoin/bitcoin/pull/32827)
During Initial Block Download, the mempool is known to be empty, but `CTxMemPool::removeForBlock` is still called for every connected block where we:
* iterate over every transaction in the block even though none will be found in the empty `mapTx`, always leaving `txs_removed_for_block` empty...
* which is pre-allocated regardless with 40 bytes * vtx.size(), even though it will always remain empty.

Similarly to https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2140691354, this chang
...

💬 l0rinc commented on pull request "p2p: avoid traversing blocks (twice) during IBD":
(https://github.com/bitcoin/bitcoin/pull/32730#discussion_r2173852408)
Opened it separately in https://github.com/bitcoin/bitcoin/pull/32827, please resolve this comment