💬 Raimo33 commented on pull request "coinstats: avoid unnecessary Coin copy in ApplyHash":
(https://github.com/bitcoin/bitcoin/pull/33410#issuecomment-3303956055)
Concept ACK, I want to expand on l0rinc suggestion by saying that, given the above warning, I think this should only be merged if it results in a significant performance improvement. @sashass1315 are you able to run some before/after benchmarks?

🤔 mzumsande reviewed a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33344#pullrequestreview-3235527352)
utACK f2bd79f80c74a2b77f14954ac65679417697a332

I mostly looked at the p2p backports (#32646, #33296, #33395), the rest looked correct too but I didn't check very deeply.

💬 mstampfer commented on pull request "contrib: Add zsh completion scripts":
(https://github.com/bitcoin/bitcoin/pull/33402#issuecomment-3303976362)
I have opened an [Issue requesting zsh completion support](https://github.com/bitcoin/bitcoin/issues/33404). Please reopen this PR or please comment with any concerns why this feature should not be merged.

💬 achow101 commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2356282381)
>1 TB of RAM is entirely feasible on high end workstations and servers, I think we could add a couple more 0's to this check.

💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2356303156)
I wanted this to be a lower-end maximum - otherwise we can just delete it. But I don't mind adding another 0 at the end.

👍 hebasto approved a pull request: "[30.0] rc2 backports"
(https://github.com/bitcoin/bitcoin/pull/33356#pullrequestreview-3235615679)
ACK c9f751090cb638ad8fff600133349446bf426e15, I applied all backports locally without conflicts and obtained a zero diff with this PR branch.

https://github.com/bitcoin/bitcoin/pull/33395 is missing from the PR description.

🚀 glozow merged a pull request: "[29.x] Backports"
(https://github.com/bitcoin/bitcoin/pull/33344)

🚀 glozow merged a pull request: "[30.0] rc2 backports"
(https://github.com/bitcoin/bitcoin/pull/33356)

🤔 enirox001 reviewed a pull request: "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`"
(https://github.com/bitcoin/bitcoin/pull/32636#pullrequestreview-3235533295)
Concept ACK, overall the refactor is a good attempt, and would modularize the code better. Here are review comments on a commit by commit basis. The commits are self contained and atomic, which I commend, makes review very efficient.

f92031e3b0: Good rename from LoadWallet() to PopulateWalletFromDB() with proper scripted-diff.
8b6a8a04f9: Good separation of error handling logic into PopulateWalletFromDB() with overloaded method for detailed error reporting.
f6c4454b87: Good conversion from
...

💬 enirox001 commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2356250657)
The current error handling only checks for `LOAD_OK` and ignores the error string provided by the new overload. This may result in losing useful error details. It might be worth updating the handling to make use of the message

💬 enirox001 commented on pull request "Split `CWallet::Create()` into `CreateNew` and `LoadExisting`":
(https://github.com/bitcoin/bitcoin/pull/32636#discussion_r2356299627)
In 8b6a8a0 it seems the `WriteVersion` method uses `CLIENT_VERSION` instead of the `client_version` parameter

💬 hodlinator commented on pull request "build: Remove bitness suffix from Windows installer":
(https://github.com/bitcoin/bitcoin/pull/32132#issuecomment-3304068622)
> When upgrading to v30.0, it appears necessary to uninstall first in order to remove the "... (64-bit)" Start Menu entries. Otherwise, they remain lingering.

Can confirm, I ended up having to remove the "... (64-bit)"-entry in the list of installed programs manually through Regedit - not ideal. :\

💬 achow101 commented on pull request "[28.x] More backports":
(https://github.com/bitcoin/bitcoin/pull/33415#issuecomment-3304092285)
Should we also backport #33106 if we're going to be doing a 28.3 anyways?

💬 m3dwards commented on pull request "ci: Skip Centos job on GHA runners":
(https://github.com/bitcoin/bitcoin/pull/33365#issuecomment-3304121898)
Looks fine. I don't have strong feelings, and wouldn't NACK this, but my preference leans more towards having things still run on forks on GHA runners as it does for the main org albeit slower. Also has the benefit of having a bit less vendor lock in.

💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2356399465)
Added comment and updated commit message

💬 l0rinc commented on pull request "coins: warn on oversized `-dbcache`":
(https://github.com/bitcoin/bitcoin/pull/33333#discussion_r2356399704)
Done

💬 achow101 commented on pull request "wallet: reduce unconditional logging during load":
(https://github.com/bitcoin/bitcoin/pull/33299#discussion_r2356408987)
There's already a similar `count == 1` check above for first time initialization that this log line can be moved into.

📝 ismaelsadeeq opened a pull request: "node: add `BlockTemplateCache` "
(https://github.com/bitcoin/bitcoin/pull/33421)
This PR implements the first step of #33389.

Main motivation and design rationale are discussed in #33389.

It introduces a `BlockTemplateCache` exposed via the node interfaces.

Block template requests from other node components should go through the cache. Each request specifies the **maximum age** of the template in seconds. If a cached template exists with matching configuration and is still fresh, it is returned; otherwise, a new template with the requested configuration is generated
...

💬 ismaelsadeeq commented on pull request "cluster mempool: control/optimize TxGraph memory usage":
(https://github.com/bitcoin/bitcoin/pull/33157#discussion_r2356456729)
Thanks for the context.
For anyone that wants to dig in, the `testmempoolaccept` third level was discussed here https://github.com/bitcoin/bitcoin/issues/32160

💬 m3dwards commented on pull request "ci: disable cirrus cache in 32bit arm job":
(https://github.com/bitcoin/bitcoin/pull/33302#issuecomment-3304201962)
ACK 00c253d494176b31dc4aaba24dc7e61aecb20be2