💬 l0rinc commented on pull request "refactor, index: Remove member variables in coinstatsindex":
(https://github.com/bitcoin/bitcoin/pull/33134#discussion_r2252380027)
it's not immediately obvious how these changes are related to inlining of fields (and why we don't need to keep the accounting at all) - could you please extract it to a separate commit which explains what's happening here?
Bundling low-risk changes in the same commit as high-risk changes makes review harder.

💬 0xB10C commented on pull request "rpc: fix getpeerinfo ping duration unit docs":
(https://github.com/bitcoin/bitcoin/pull/33133#issuecomment-3152330339)
- removed the `(s)` unit from the docs: https://github.com/bitcoin/bitcoin/pull/33133#discussion_r2251809033
- removed the `getpeerinfo` field documentation from the `ping` RPC: https://github.com/bitcoin/bitcoin/pull/33133#issuecomment-3152076777
- added a bit more details to the commit message: https://github.com/bitcoin/bitcoin/pull/33133#pullrequestreview-3085116958

💬 jonatack commented on pull request "init: make `-blockmaxweight` startup option debug only":
(https://github.com/bitcoin/bitcoin/pull/32654#discussion_r2252548042)
It seems this change ought to have a release note to explain why the option is no longer available by default and how to enable it.

🤔 jonatack reviewed a pull request: "rpc: fix getpeerinfo ping duration unit docs"
(https://github.com/bitcoin/bitcoin/pull/33133#pullrequestreview-3085715524)
ACK 1252eeb997df2eb12c33d92eb1a5c9d6643a67ff

💬 jonatack commented on pull request "rpc: fix getpeerinfo ping duration unit docs":
(https://github.com/bitcoin/bitcoin/pull/33133#discussion_r2252556648)
pico-nit, if you have to retouch

```suggestion
"Results are provided in RPC getpeerinfo.\n"
```

👍 theStack approved a pull request: "rpc: fix getpeerinfo ping duration unit docs"
(https://github.com/bitcoin/bitcoin/pull/33133#pullrequestreview-3085748775)
ACK 1252eeb997df2eb12c33d92eb1a5c9d6643a67ff

💬 achow101 commented on pull request "allocators: Apply manual ASan poisoning to `PoolResource`":
(https://github.com/bitcoin/bitcoin/pull/32581#issuecomment-3152410690)
ACK ad132761fc49c38769c09653a265fdbc3b93eda5

💬 luke-jr commented on pull request "Adding alert for failure to prevent dead-end user crash":
(https://github.com/bitcoin/bitcoin/pull/33127#discussion_r2252594171)
There already is, in src/init.cpp ("Do you want to rebuild the databases now?")

(But I'm not sure we can detect this condition that early)

🤔 jonatack reviewed a pull request: "test: Fix 'getdescriptoractivity' RPCHelpMan, add test to verify fix"
(https://github.com/bitcoin/bitcoin/pull/33119#pullrequestreview-3085779061)
ACK 3543bfdfec345cf2c952143c31674ef02de2a64b

Suggest updating the PR title prefix from "test" to "rpc"

💬 jonatack commented on pull request "wallet: Add `exportwatchonlywallet` RPC to export a watchonly version of a wallet":
(https://github.com/bitcoin/bitcoin/pull/32489#issuecomment-3152447889)
> For anyone who's interested, I'm hosting a review club meeting about this PR on Wednesday: [bitcoincore.reviews/32489](https://bitcoincore.reviews/32489)

There is `Review Club` label in this repo that can be added, too.

💬 luke-jr commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2252606990)
```suggestion
if any(' bitcoin-util utility version ' in s for s in binary.strings):
```

💬 luke-jr commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#discussion_r2252612225)
```suggestion
mv build/dist/*.zip "${OUTDIR}/${DISTNAME}-${HOST}-unsigned.zip"
```

💬 luke-jr commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3152466438)
and ftr, Concept ACK. Core has always had a policy of using CLIENT_NAME correctly and not breaking needlessly when it's changed. PR title could be updated to appease naysayers.

💬 jonatack commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-3152474568)
Rebased to re-run the CI, no other change: `git range-diff d1b5831 caea5f0 721a051`

💬 luke-jr commented on pull request "Adding alert for failure to prevent dead-end user crash":
(https://github.com/bitcoin/bitcoin/pull/33127#issuecomment-3152486579)
Looking at the relevant code, this feels like a symptom of a problem we could/should detect earlier somehow (which may also be early enough to follow the reindex GUI path)

💬 l0rinc commented on pull request "cli: return local services in -netinfo":
(https://github.com/bitcoin/bitcoin/pull/31886#issuecomment-3152490929)
Redid the rebase, reran the test, reACK 721a051320f2c10d2e9c89c985f180da81d64dca

💬 jonatack commented on pull request "test: headers sync timeout":
(https://github.com/bitcoin/bitcoin/pull/32677#issuecomment-3152491438)
Post-merge ACK, thanks @stringintech!

💬 ismaelsadeeq commented on pull request "init: make `-blockmaxweight` startup option debug only":
(https://github.com/bitcoin/bitcoin/pull/32654#discussion_r2252692457)
The option is available by default and enabled.
It is just hidden in the help text.

💬 jonatack commented on pull request "init: make `-blockmaxweight` startup option debug only":
(https://github.com/bitcoin/bitcoin/pull/32654#discussion_r2252703592)
Indeed, yes -- but still ought to have a release note that it is now a hidden -help-debug option.

💬 BitcoinMechanic commented on pull request "Allowing multi client support in guix-build":
(https://github.com/bitcoin/bitcoin/pull/33126#issuecomment-3152622710)
> > to better support forked clients
>
> Concept N-A-C-K
>
> This is not something Bitcoin Core needs to support.

No I guess it isn't, but hopefully that means the spirit of FOSS that is invoked as an excuse when Bitcoin Core does something a large % of its users dislike is something we can stop pretending applies here.

Making life harder for no good reason for people who fork this repo is lamentable behaviour.

Otherwise, ACK.