💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383394089)
learning from https://github.com/bitcoin/bitcoin/pull/33435 we may want to add support for BSD as well (not sure anyone will use it, but at least we should be aware that it's an option)

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383440187)
nit: any reason for not sorting these?

💬 l0rinc commented on pull request "node: Add --debug_runs/-waitfordebugger + --debug_cmd":
(https://github.com/bitcoin/bitcoin/pull/31723#discussion_r2383468341)
I wonder if there's a better way than sleep. The [internets](https://www.oreilly.com/library/view/secure-programming-cookbook/0596003943/ch12s13.html) indicates that `raise(SIGSTOP)` + `SIGTRAP` would also pause and revive the process safely - I don't have Linux locally, but maybe we could give it a try

💬 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