💬 achow101 commented on pull request "cli: Handle arguments that can be either JSON or string":
(https://github.com/bitcoin/bitcoin/pull/33230#discussion_r2376667216)
I think there are a number of other type checking improvements that we can do at the same time, but I would prefer to do those separately.

💬 ryanofsky commented on issue "`bitcoin-node` is unkillable after mining IPC connection is established":
(https://github.com/bitcoin/bitcoin/issues/33463#issuecomment-3330113600)
> [@ryanofsky](https://github.com/ryanofsky) I'm on macOS, would a stack trace from LLDB be equally useful?

Yes a stack trace from all threads after the hang happens should make the cause of the problem clear. lldb command to print that should be `thread backtrace all`. It's also good to know the problem doesn't happen with waitNext, only waitTipChanged since we can look for differences between those methods.

💬 achow101 commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#discussion_r2376694679)
In 1b1f359fc43385cb4d465b15754dac84eef06873 "[doc] manpages for 28.3rc1"

I don't think this is supposed to be removed from the manage.

Same with `-upnp`.

💬 w0xlt commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376702343)
Is this copying `nOrderPosNext` from source wallet to the `watchonly` one ?
```suggestion
watchonly_batch.WriteOrderPosNext(this->nOrderPosNext);
```

🤔 mzumsande reviewed a pull request: "validation: remove BLOCK_FAILED_CHILD"
(https://github.com/bitcoin/bitcoin/pull/32950#pullrequestreview-3264142883)
Looks good - just some minor comments.

💬 mzumsande commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2376698894)
do we need to keep BLOCK_FAILED_MASK in `chain.h` after it's no longer used? After all, it's only used as a mask (unlike BLOCK_FAILED_CHILD).

💬 mzumsande commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2376682303)
commit message ca2421d52792b32754f1801492822b999839ec3a:
"since the previous commit removes BLOCK_FAILED_CHILD" - only the following commit removes it. Or do you mean "any special logic for BLOCK_FAILED_CHILD"?

💬 mzumsande commented on pull request "validation: remove BLOCK_FAILED_CHILD":
(https://github.com/bitcoin/bitcoin/pull/32950#discussion_r2376701606)
comment still mentions a "mask"

💬 w0xlt commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376728821)
Wouldn't it be safe to delete only the watchonly wallet file ?

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376734029)
Oops fixed

💬 achow101 commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376736505)
> Wouldn't it be safe to delete only the watchonly wallet file ?

I don't think we should be leaving behind the wallet directory, and possibly the `-journal` file either.

💬 w0xlt commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#discussion_r2376758113)
Can `desc.cache` be empty here ?

💬 glozow commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#discussion_r2376784754)
Is there a configure flag I need? I'm just running gen-manpages.py after building.

💬 instagibbs commented on pull request "Cluster mempool implementation":
(https://github.com/bitcoin/bitcoin/pull/28676#discussion_r2376795377)
pendantic: it's redundant *when an RBF is encountered*, but not otherwise and the result is cached so pretty cheap to throw around

📝 glozow opened a pull request: "[28.x] backports + 28.3rc1"
(https://github.com/bitcoin/bitcoin/pull/33476)
Built on top of #33415

Includes backports of
- #33106
- #30125
- #30948
- #30784

💬 glozow commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3330324239)
Opened #33476 to follow

💬 fanquake commented on pull request "build: Move CMAKE_SKIP_INSTALL_RPATH from CMake to Guix script":
(https://github.com/bitcoin/bitcoin/pull/33470#discussion_r2376849213)
Can this just be `CMAKE_SKIP_RPATH`? (also move to `CONFIGFLAGS`)

💬 fanquake commented on pull request "[28.x] backports + 28.3rc1":
(https://github.com/bitcoin/bitcoin/pull/33476#discussion_r2376868135)
Leftover from rebase?

💬 maflcko commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2376962309)
seems a bit odd to have the number of nodes in a test influence whether or not the test has to be edited to remove or add `-par=1` everywhere. Would it not be easier to just globally set `-par=2` for all funtional tests?


```diff
diff --git a/test/functional/test_framework/util.py b/test/functional/test_framework/util.py
index e5a5938f07..42bb213dd3 100644
--- a/test/functional/test_framework/util.py
+++ b/test/functional/test_framework/util.py
@@ -459,6 +459,7 @@ def write_config(conf
...

💬 andrewtoth commented on pull request "validation: fetch block inputs on parallel threads >10% faster IBD":
(https://github.com/bitcoin/bitcoin/pull/31132#discussion_r2377071137)
Yes, I wondered if that would be more invasive to other tests though.