🤔 kevkevinpal reviewed a pull request: "Rollback for dumptxoutset without invalidating blocks"
(https://github.com/bitcoin/bitcoin/pull/33477#pullrequestreview-3274904950)
Concept ACK [6d409d5](https://github.com/bitcoin/bitcoin/pull/33477/commits/6d409d59704a026a56d18856ab2f9e90eea62186)

This approach makes more sense. I reviewed the code a bit and made some comments, but nothing blocking

I also added comments on possible functional tests for the new `JSONRPCError` but these can be done in a followup

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384138037)
It may be worth noting that "network activity will **_not_** be suspended during this process."

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384142999)
Maybe this text would make more sense

"For deep rollbacks, make sure to use no RPC timeout (bitcoin-cli -rpcclienttimeout=0) as it may take several minutes."

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384175011)
Might be able to add a functional test for this rpc error

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384175151)
Might be able to add a functional test for this rpc error

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384179539)
It might be useful to add a log here to inform the user the temp UTXO db was cleaned up since we mentioned in logs that we are creating a temp UTXO db

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384175091)
Might be able to add a functional test for this rpc error

💬 kevkevinpal commented on pull request "Rollback for dumptxoutset without invalidating blocks":
(https://github.com/bitcoin/bitcoin/pull/33477#discussion_r2384175185)
Might be able to add a functional test for this rpc error

📝 andrewtoth opened a pull request: "test: set par=2 in default config for functional test framework"
(https://github.com/bitcoin/bitcoin/pull/33485)
Depending on the host machine, a default `par` value can spawn up to 15 script verification threads for each node. Running the functional test suite with default `par` can exhaust file descriptors or hit other resource limits when many threads are spawned. These threads are mostly idle and the same code paths are executed with a value of `par=2`. Limit this to 2 for functional tests that do not override the default option.

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2384229679)
Makes sense. Done here https://github.com/bitcoin/bitcoin/pull/33485.

👍 andrewtoth approved a pull request: "net: support overriding the proxy selection in ConnectNode()"
(https://github.com/bitcoin/bitcoin/pull/33454#pullrequestreview-3275014878)
utACK ec7c21534505bb727ad0344c7f6881836b4e9ec5

Fairly simple change that is clearly unused here so will have no effect. Hopefully can make https://github.com/bitcoin/bitcoin/pull/29415 easier to review and get in for v31.

💬 instagibbs commented on pull request "test: Fix reorg patterns in tests to use proper fork-based approach":
(https://github.com/bitcoin/bitcoin/pull/32587#issuecomment-3341946462)
forgot about this, definitely worthwhile to get merged soon

📝 kevkevinpal opened a pull request: "ci: check if file or directory already exists for /home/kevkevin/.bitcoin instead of failing"
(https://github.com/bitcoin/bitcoin/pull/33486)
## Description
I was trying to run this script, but then it was giving me this error

```
/ci_container_base/ci/test/03_test_script.sh: line 90: /root/.bitcoin: Is a directory
Command '['./ci/test/02_run_container.sh']' returned non-zero exit status 1.
```
Since there is a directory there, we should skip this step.

## Solution
I created a check to see if a directory or a file already exists, otherwise create one

💬 kevkevinpal commented on pull request "cmake: Inherit WERROR setting for secp256k1 build":
(https://github.com/bitcoin/bitcoin/pull/33297#issuecomment-3341957155)
Concept ACK [18d3d67](https://github.com/bitcoin/bitcoin/pull/33297/commits/18d3d670168b1c79041a54fe3c0e682ae3ef2994)

I agree with luke-jr that even if we decided to drop this in favor of another solution that should be considered separately

💬 furszy commented on pull request "test: set par=2 in default config for functional test framework":
(https://github.com/bitcoin/bitcoin/pull/33485#discussion_r2384285506)
What about `min(2, os.cpu_count())` to avoid forcing a second thread when only one core is available.

💬 andrewtoth commented on pull request "test: set par=2 in default config for functional test framework":
(https://github.com/bitcoin/bitcoin/pull/33485#discussion_r2384286749)
Is there a realistic case where only one core is available?

💬 furszy commented on pull request "test: set par=2 in default config for functional test framework":
(https://github.com/bitcoin/bitcoin/pull/33485#discussion_r2384304328)
Only the rpi zero comes to mind. It seems like a simple change anyway. I wouldn’t overthink it too much.

💬 l0rinc commented on pull request "test: set par=2 in default config for functional test framework":
(https://github.com/bitcoin/bitcoin/pull/33485#discussion_r2384308993)
Or maybe a dockerized container

💬 andrewtoth commented on pull request "test: set par=2 in default config for functional test framework":
(https://github.com/bitcoin/bitcoin/pull/33485#discussion_r2384316530)
Done!

🤔 pablomartin4btc reviewed a pull request: "test: set par=2 in default config for functional test framework"
(https://github.com/bitcoin/bitcoin/pull/33485#pullrequestreview-3275123515)
ACK dda5228e02ca6a839bf87ae7dbd133547563816a

I thought some of the top longest tests would require a diff (>) `par=`, but it looks alright.