💬 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.

💬 fanquake commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979628091)
I have atleast 154GB, and assume it's going to end up around the 180GB mark. Given that, it seems this should be something closer to 200?

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

💬 fanquake commented on pull request "doc: Bring reduce-memory.md up to date":
(https://github.com/bitcoin/bitcoin/pull/31985#issuecomment-2697936019)
> not the minimum value that would be accepted as command line argument.

Should we make this function like maxmempool, which will fail to accept a value lower than the minimim (5)? Seems we are somewhat inconsistent in our handling of these.

💬 glozow commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979644959)
Hm, I have 5.9G. IIRC I did a fresh run yesterday, so maybe I don't have as many reorged blocks?

But why do you expect 11 and not 9?

💬 glozow commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979646913)
Oops I shouldn't have just picked the tip. Now using 72600 which was mined 8 hours ago.

💬 glozow commented on pull request "kernel: pre-29.x chainparams and headerssync update":
(https://github.com/bitcoin/bitcoin/pull/31978#discussion_r1979647136)
fixed

💬 fjahr commented on pull request "fuzz: Extend mini_miner fuzz coverage to max block weight":
(https://github.com/bitcoin/bitcoin/pull/31803#discussion_r1979727414)
Ok, that hit an error further down below. It seems to me like the cluster limit of 500 tx was hit. Let me try again with a max of 500 tx and increasing the potential number of outputs instead.

💬 darosior commented on pull request "qa: clarify and document assumeutxo tests":
(https://github.com/bitcoin/bitcoin/pull/31907#issuecomment-2698108727)
Since it was tiny, [pretty specifc to my branch](https://github.com/bitcoin/bitcoin/pull/31907#issuecomment-2669777720) and [confused some reviewers](https://github.com/bitcoin/bitcoin/pull/31907#discussion_r1966033115), i dropped the first commit of this PR.

Since several reviewers were in favor, i included in this PR @theStack's commits to introduce varint and compression serialization primitives to the functional test framework. I adapted the commit to use these primitives instead of the d
...