💬 polespinasa commented on pull request "cli: Handle arguments that can be either JSON or string":
(https://github.com/bitcoin/bitcoin/pull/33230#issuecomment-3208206289)
> I wonder if an alternate solution might have the client treat hash_or_height as a string parameter instead of a JSON parameter, and update the server to accept height strings?

Even if there is, there are other use cases to taking lists and strings as parameters. See: https://github.com/bitcoin/bitcoin/pull/32468

💬 achow101 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2289397520)
In 3f10fe8763bf4a1bdb6b347204987584761b9dbf "rpc: Handle -named argument parsing where '=' character is used"

Please use the new class and variable naming style.

💬 achow101 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2289397926)
In 3f10fe8763bf4a1bdb6b347204987584761b9dbf "rpc: Handle -named argument parsing where '=' character is used"

Please use the new variable naming style.

💬 achow101 commented on pull request "rpc: Handle -named argument parsing where '=' character is used":
(https://github.com/bitcoin/bitcoin/pull/32821#discussion_r2289403854)
In 3f10fe8763bf4a1bdb6b347204987584761b9dbf "rpc: Handle -named argument parsing where '=' character is used"

We can use `contains` here rather than `count() > 0`

💬 fjahr commented on pull request "wallet: Remove isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3208236720)
reACK be776a1443fdf1a72e0d363c1566d71cb0cda8b5

💬 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_r2289424563)
Removed

🤔 murchandamus reviewed a pull request: "[29.x] 33106 backport and final changes for rc2"
(https://github.com/bitcoin/bitcoin/pull/33226#pullrequestreview-3138488628)
crACK 0034dcfba9dc599449e7569ed1b30e58d4f4434f

Had previously reviewed https://github.com/bitcoin/bitcoin/pull/33106, read the code changes here again, and compared to the original commits in master.

👍 brunoerg approved a pull request: "[29.x] 33106 backport and final changes for rc2"
(https://github.com/bitcoin/bitcoin/pull/33226#pullrequestreview-3138505376)
crACK 0034dcfba9dc599449e7569ed1b30e58d4f4434f

🤔 w0xlt reviewed a pull request: "cli: Handle arguments that can be either JSON or string"
(https://github.com/bitcoin/bitcoin/pull/33230#pullrequestreview-3138552075)
Approach ACK

📝 w0xlt opened a pull request: "net: Prevent node from binding to the same `CService`"
(https://github.com/bitcoin/bitcoin/pull/33231)
Currently, if the node starts with repeated `-bind` options (e.g. `./build/bin/bitcoind -listen -bind=0.0.0.0 -bind=0.0.0.0`), the user will receive a misleading message followed by the node shutdown:

```
[net:error] Unable to bind to 0.0.0.0:8333 on this computer. Bitcoin Core is probably already running.
[error] Unable to bind to 0.0.0.0:8333 on this computer. Bitcoin Core is probably already running.
```

And the user might spend some time looking for a `bitcoind` process or what appl
...

💬 davidgumberg commented on pull request "wallet: Remove isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2289457295)
> This is the current behavior anyways.

That's surprising!

Shouldn't we do something like:

```cpp
entry.pushKV("spendable", pwallet->IsWalletFlagSet(WALLET_FLAG_DISABLE_PRIVATE_KEYS));
```

instead?

Otherwise RPC docs should be more like @pablomartin4btc [below](https://github.com/bitcoin/bitcoin/pull/32523/commits/6a7aa015747e2634fe5a4b2f7fa0d104eb75c796#r2274128180) can be done in a follow-up.

💬 davidgumberg commented on pull request "wallet: Remove isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-3208497666)
crACK https://github.com/bitcoin/bitcoin/commit/be776a1443fdf1a72e0d363c1566d71cb0cda8b5

🤔 jonatack reviewed a pull request: "net: Prevent node from binding to the same `CService`"
(https://github.com/bitcoin/bitcoin/pull/33231#pullrequestreview-3138666437)
Concept ACK

> This PR proposes that repeated -bind options have no effect.

Only no effect for repeated *duplicate* -bind options, yes? Would it be preferable to give the user feedback instead?

Would be good to add test coverage.

💬 jonatack commented on pull request "net: Prevent node from binding to the same `CService`":
(https://github.com/bitcoin/bitcoin/pull/33231#discussion_r2289594951)
Would need to update the doxygen ("A vector...")

💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#issuecomment-3208833064)
recrACK 50e61f448eedfe0ccfa07f3124aeef9c7762a69b

I only left nits, I'm fine with merging it as is.

💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289730918)
nit: as mentioned this should be synchronized with the other PR to simplify the merge conflicts

💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289739420)
another super-nit: `CheckHeadersPow` -> `CheckHeadersPoW`. Only change if you have to change for any other reason.

💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289725133)
nit: if you touch again, you could explain in the commit message why the `LookupBlockIndex` cannot be eliminated as well

💬 l0rinc commented on pull request "refactor: Header sync optimisations & simplifications":
(https://github.com/bitcoin/bitcoin/pull/32740#discussion_r2289729638)
nit: if touching again, the commit message should probably mention the `explicit` as well. No that important, just noticed it.

✅ nervana21 closed an issue: "Crash on launch in PruneBlockIndexCandidates"
(https://github.com/bitcoin/bitcoin/issues/33129)