💬 Raimo33 commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432193066)
@l0rinc @cedwies the bench is not presented as evidence. it is presented as a transparent way of saying performance hasn't degraded. the PR body clearly states that it remained the same. I included it for the sake of transparency.

💬 waketraindev commented on pull request "addrman, net: filter during address selection via AddrPolicy to avoid underfill":
(https://github.com/bitcoin/bitcoin/pull/33663#issuecomment-3432260201)
* updated PR description and commit message to highlight P2p getaddr responses
* corrected formatting

💬 maflcko commented on pull request "ci, iwyu: Treat warnings as errors for specific directories":
(https://github.com/bitcoin/bitcoin/pull/31308#issuecomment-3432270963)
Looks like the CI re-run passed after one month, so it seems reasonable stable

💬 cedwies commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432352303)
> But the code seems more complicated than before - is there a way to retain the speedup without complicating the code (i.e. minimizing the diff)?

It is more complicated. I would:

Keep:
- change from index-based to range-based loops.
- In `AreInputsStandard(...)`: Move `vSolutions` outside the for loop (this brings the performance gain in `CCoinsCaching`)
- In `IsWitnessStandard(...)`: Keep the variable name change from `stack` to `script_stack`

Discard (to minimize diff):
- In `Ar
...

💬 furszy commented on pull request "net: introduce block tracker to retry to download blocks after failure":
(https://github.com/bitcoin/bitcoin/pull/27837#issuecomment-3432381897)
closing for now, will re-open after rebasing it.

✅ furszy closed a pull request: "net: introduce block tracker to retry to download blocks after failure"
(https://github.com/bitcoin/bitcoin/pull/27837)

✅ furszy closed a pull request: "wallet: fix crash during migration due to invalid multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31378)

💬 furszy commented on pull request "wallet: fix crash during migration due to invalid multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31378#issuecomment-3432386506)
closing for now. Not sure if I will ever back to work on this PR.

💬 furszy commented on pull request "descriptors: inference process, do not return unparsable multisig descriptors":
(https://github.com/bitcoin/bitcoin/pull/31404#issuecomment-3432388684)
Probably still useful but closing for now due to lack of availability.

✅ furszy closed a pull request: "descriptors: inference process, do not return unparsable multisig descriptors"
(https://github.com/bitcoin/bitcoin/pull/31404)

📝 furszy opened a pull request: "http: limit RPC server threads to available cores"
(https://github.com/bitcoin/bitcoin/pull/33678)
The HTTP server currently spawns 16 threads by default, regardless of the system’s available ones.
This change limits the number of threads in the RPC server to the number of available CPUs,
preventing excessive context switching and improving overall responsiveness on systems with
fewer cores. If `-rpcthreads` exceeds that limit, it is adjusted to num_cores - 1 and a warning is
logged.

📝 furszy opened a pull request: "test: set number of RPC server threads to 2"
(https://github.com/bitcoin/bitcoin/pull/33679)
The default `-rpcthreads` value spawns 16 HTTP server threads for each node.
Running the functional test suite with default `rpcthreads` can exhaust file
descriptors or hit other resource limits very easily (more when tests are run
in parallel).
Furthermore, having 16 threads is unnecessary since they are mostly idle. We
run RPC calls on a single RPC connection and wait for it result synchronously.
There is (almost) never two RPC calls occurring concurrently.
Because of this, the threads
...

💬 maflcko commented on pull request "http: limit RPC server threads to available cores":
(https://github.com/bitcoin/bitcoin/pull/33678#issuecomment-3432429726)
Not sure. This seems to be partially reverting e56fc7ce6a92eae7e80657d9f57a148cc002358d. See also https://github.com/bitcoin/bitcoin/pull/31215#discussion_r1828025349

💬 maflcko commented on pull request "test: set number of RPC server threads to 2":
(https://github.com/bitcoin/bitcoin/pull/33679#issuecomment-3432447263)
lgtm ACK e9cd45e3d3c7592265ebf67387090b3df1501df4

Any number larger than `1` should be fine, so that it is still possible to accidentally or intentionally cause races when several test threads call the rpc threads.

💬 achow101 commented on pull request "validation: do not wipe utxo cache for stats/scans/snapshots":
(https://github.com/bitcoin/bitcoin/pull/30610#issuecomment-3432564955)
Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.

✅ achow101 closed a pull request: "validation: do not wipe utxo cache for stats/scans/snapshots"
(https://github.com/bitcoin/bitcoin/pull/30610)

💬 Raimo33 commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432572647)
> * In AreInputsStandard(...): Don't move the stack outside the for loop

Agreed: it is not in the hot path (or the most likely path).

> * In IsWitnessStandard(...): Don't move stack and witnessprogram outside the for loop

Agreed for `stack`: it is only needed for P2SH which is unlikely
Disagree for `witnessprogram`: it is used in the hot path (P2WPKH is the most likely)

> * In `SpendsNonAnchorWitnessProg(...)`: Don't move `stack` outside the for loop.

Agreed: it is not in the hot
...

💬 l0rinc commented on pull request "refactor: optimize: reuse containers in transaction policy verification":
(https://github.com/bitcoin/bitcoin/pull/33645#issuecomment-3432572935)
> Since std::vector only allocates on first use

Actually it can reallocate on every size exhaustion, i.e. every resize.
I don't like the new reuse, that can actually be slower than before because now the code paths depend on each other.
Wouldn't reserving the vectors help here instead?

✅ achow101 closed a pull request: "RPC/Wallet: Convert walletprocesspsbt to use options parameter"
(https://github.com/bitcoin/bitcoin/pull/24963)

💬 achow101 commented on pull request "RPC/Wallet: Convert walletprocesspsbt to use options parameter":
(https://github.com/bitcoin/bitcoin/pull/24963#issuecomment-3432582603)
Closing this as it has not had any activity in a while. If you are interested in continuing work on this, please leave a comment so that it can be reopened.

Marking as up for grabs.