💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383424565)
is it possible to add an assert somewhere that this isn't enabled when we're not in debug mode, just to be sure?

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383472403)
Most of this is hard to test, but maybe we can test this safely

nit: had to read the sentence a few times, maybe we can change it to:
```suggestion
return InitError(Untranslated("-waitfordebugger is unavailable in this build. Rebuild with -DWAIT_FOR_DEBUGGER=ON."));
```

💬 ajtowns commented on pull request "docs: Undeprecate datacarrier and datacarriersize configuration options":
(https://github.com/bitcoin/bitcoin/pull/33453#issuecomment-3341177765)
> No because the high latency statistically hurts small miners disproportionately relative to large, and that effect is made worse by selfish mining attacks which benefit large miners.

The difference between large miners and small miners here is that large miners are (a) more likely to get two blocks in a row, meaning they have ~0 latency, and (b) may have more resources to spend setting up their own custom FIBRE relay network or similar, to improve latency for themselves. But the effect of d
...

👍 rkrux approved a pull request: "doc: rpc: fix case typo in `finalizepsbt` help (final_scriptwitness)"
(https://github.com/bitcoin/bitcoin/pull/33484#pullrequestreview-3274671517)
Ah crACK ff05bebcc4262966b117082a67dc4c63a3f67d2d

There are two different kinds of spellings in the `decodepsbt` response.

💬 maflcko commented on pull request "multiprocess: Add bitcoin-wallet -ipcconnect option":
(https://github.com/bitcoin/bitcoin/pull/19460#issuecomment-3341361282)
I guess on the next rebase, this would have to revert https://github.com/bitcoin/bitcoin/pull/33459 ?

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