💬 laanwj commented on pull request "rpc: increase the defaults for -rpcthreads and -rpcworkqueue":
(https://github.com/bitcoin/bitcoin/pull/31215#issuecomment-2697832530)
@dergoegge Done in #31985

💬 fjahr commented on pull request "fuzz: Extend mini_miner fuzz coverage to max block weight":
(https://github.com/bitcoin/bitcoin/pull/31803#discussion_r1979583535)
Thanks @marcofleon , that the code doesn't run seems to be because we don't add enough transactions to hit the limit, see `LIMITED_WHILE(fuzzed_data_provider.ConsumeBool(), 100)` and in the coverage you linked the `break` on line 165 is never hit as well. This requires a bit of additional conceptual review but for now I have increased the loop limit to 5000 and I hope we should see some hits with that. Will the link get updated with the new numbers when I visit it again tomorrow?

💬 fjahr commented on pull request "fuzz: Extend mini_miner fuzz coverage to max block weight":
(https://github.com/bitcoin/bitcoin/pull/31803#discussion_r1979591922)
Thinking about it a bit more, this was probably an optimization to make the test faster. Not sure if increasing the number that much is seen as a downside. I could also add something so that we usually have 0-100 but then with a 10% chance or so I increase the number dramatically so that we hit the number max block limit with some regularity as well. There my understanding performance requirements on fuzz tests is limited so not sure which approach would be preferred.

👍 dergoegge approved a pull request: "doc: Bring reduce-memory.md up to date"
(https://github.com/bitcoin/bitcoin/pull/31985#pullrequestreview-2657838684)
ACK 4bb2eae061c9ed0c20e2a435145bf77c9b3601cc - 16 matches the `DEFAULT_HTTP_THREADS` constant.

👍 brunoerg approved a pull request: "doc: Bring reduce-memory.md up to date"
(https://github.com/bitcoin/bitcoin/pull/31985#pullrequestreview-2657863389)
ACK fff4f93dff8ba67689e43929615e3c63c67015e4

👍 TheCharlatan approved a pull request: "doc: Bring reduce-memory.md up to date"
(https://github.com/bitcoin/bitcoin/pull/31985#pullrequestreview-2657872493)
ACK fff4f93dff8ba67689e43929615e3c63c67015e4

🤔 rkrux reviewed a pull request: "test: Use rpc_deprecated only for testing deprecation"
(https://github.com/bitcoin/bitcoin/pull/31977#pullrequestreview-2657788839)
Concept ACK c5e384de6c0b92692021272a7464cd26a54d298c

New to this test, suggested some verbiage change. PLMK if it doesn't fit.

💬 rkrux commented on pull request "test: Use rpc_deprecated only for testing deprecation":
(https://github.com/bitcoin/bitcoin/pull/31977#discussion_r1979608022)
This is the final comment after this change:

> This test should be used to verify correct behaviour of deprecated RPC methods without the -deprecatedrpc flag.

I think it can be reworded to be more explicit in the intention. I'm assuming the deprecated RPCs will just throw errors, please correct me if I'm wrong.

```
This test should be used to verify the errors of the currently deprecated RPC methods (without the -deprecatedrpc flag) until such RPCs are fully removed.
```

Suggesting
...

💬 rkrux commented on pull request "test: Use rpc_deprecated only for testing deprecation":
(https://github.com/bitcoin/bitcoin/pull/31977#discussion_r1979568355)
Couple typos
`depgrecatedrpc`
`anther`

💬 rkrux commented on pull request "test: Use rpc_deprecated only for testing deprecation":
(https://github.com/bitcoin/bitcoin/pull/31977#discussion_r1979609096)
Nit:
```
self.log.info("No tested currently deprecated RPC methods")
```

💬 fanquake commented on pull request "doc: Bring reduce-memory.md up to date":
(https://github.com/bitcoin/bitcoin/pull/31985#issuecomment-2697910206)
>Let me know if you notice other mismatches with current defaults.

In this file "The minimum value for `-dbcache` is 4." However it's entirely possible to run bitcoind with `-dbcache=1`. Maybe that line can just be dropped?

💬 darosior commented on issue "test: intermittent issue in p2p_1p1c_network.py":
(https://github.com/bitcoin/bitcoin/issues/31946#issuecomment-2697912165)
Another instance occurred in #31603 https://github.com/bitcoin/bitcoin/actions/runs/13498602529/job/37711361328.

💬 fanquake commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979618887)
Yea, `700` or even more here, seems fine here.

💬 laanwj commented on pull request "doc: Bring reduce-memory.md up to date":
(https://github.com/bitcoin/bitcoin/pull/31985#issuecomment-2697915217)
> In this file "The minimum value for -dbcache is 4." However it's entirely possible to run bitcoind with -dbcache=1. Maybe that line can just be dropped?

Could be removed if it's not longer correct, but to be clear: 4 here was meant here as the minimum effective value, not the minimum value that would be accepted as command line argument.

💬 glozow commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979622063)
fwiw I also have 12GB for chainstate, but I didn't think it made much sense to decrease the number.

👍 TheCharlatan approved a pull request: "cmake: Set top-level target output locations"
(https://github.com/bitcoin/bitcoin/pull/31161#pullrequestreview-2657886194)
Re-ACK 568fcdddaec2cc8decba5a098257f31729cc1caa

👍 willcl-ark approved a pull request: "test, tracing: don't use problematic `bpf_usdt_readarg_p()`"
(https://github.com/bitcoin/bitcoin/pull/31848#pullrequestreview-2657887971)
tACK 943bf844a5635213e61c954c7c30791471143366

I also re-ran CI on this PR [10 times](https://github.com/willcl-ark/bitcoin/actions/runs/13653156736), of which all passed.

The code changes all look correct to me too, following the principle of the pointers being null initialized, having addressed assigned to them, and then the value being copied.

Hopefully this can finally end the intermittent failures 🤞🏼

💬 glozow commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979623957)
It is a big jump, but `du .bitcoin/testnet3 -h` gives me 181G. Let's see what other people have?

💬 fanquake commented on pull request "build: don't show ccache summary with MSVC":
(https://github.com/bitcoin/bitcoin/pull/31983#discussion_r1979624733)
Reworked this.

👍 hebasto approved a pull request: "build: don't show ccache summary with MSVC"
(https://github.com/bitcoin/bitcoin/pull/31983#pullrequestreview-2657896362)
ACK c718bffc361a1227de9deb823c35dd11c8570ddd, I have reviewed the code and it looks OK.