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

💬 RandyMcMillan commented on pull request "ci: Turn CentOS config into Alpine musl config":
(https://github.com/bitcoin/bitcoin/pull/33480#issuecomment-3342049302)
Concept ACK

👍 l0rinc approved a pull request: "test: set par=2 in default config for functional test framework"
(https://github.com/bitcoin/bitcoin/pull/33485#pullrequestreview-3275207076)
Code review ACK dda5228e02ca6a839bf87ae7dbd133547563816a

Q: does this affect the `"-par=1"` cases in https://github.com/bitcoin/bitcoin/blob/9a04635432183c437829339dbf10e7d702581010/test/functional/p2p_segwit.py#L221 and https://github.com/bitcoin/bitcoin/blob/6980852416040bdddf111df3cea3ec50639f010a/test/functional/feature_proxy.py#L385?

💬 151henry151 commented on pull request "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script":
(https://github.com/bitcoin/bitcoin/pull/33470#discussion_r2384416750)
Got it, made that change.

🤔 katesalazar reviewed a pull request: "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing"
(https://github.com/bitcoin/bitcoin/pull/33486#pullrequestreview-3275898775)
I don't like how dummy file is now explained much before created.

💬 katesalazar commented on pull request "ci: check if file or directory already exists for ${HOME}/.bitcoin instead of failing":
(https://github.com/bitcoin/bitcoin/pull/33486#discussion_r2385070435)
The dummy file explanation is much early.