💬 achow101 commented on pull request "wallet: Always set descriptor cache upgraded flag for new wallets":
(https://github.com/bitcoin/bitcoin/pull/32597#discussion_r2109902489)
There can be no DB entries when upgrade is run on a newly created wallet as the descriptors don't even exist yet.

💬 achow101 commented on pull request "wallet: Always set descriptor cache upgraded flag for new wallets":
(https://github.com/bitcoin/bitcoin/pull/32597#discussion_r2109907898)
The only behavior that is conditional on this flag is whether the upgrade needs to be run. It exists only as a startup optimization to ensure that we aren't re-running the same upgrade over and over. The `DescriptorScriptPubKeyMan` will always write the records for the upgraded cache, regardless of whether the flag exists. There is no need to upgrade the cache either as the upgraded cache is always used regardless of the flag.

💬 achow101 commented on pull request "gui: Add a menu action to restore then migrate a legacy wallet":
(https://github.com/bitcoin-core/gui/pull/877#issuecomment-2913572120)
> IMO this is bad UX. Users will be looking to restore the wallet, and the details of how it does that shouldn't require unintuitive steps.

While it is an extra click, I don't think it is unintuitive.

When the user goes to restore a legacy wallet, they will be informed that the restore failed and that they will need to migrate the wallet via the corresponding menu. When the user goes there, this option to restore and migrate will be presented to them.

💬 achow101 commented on pull request "wallet: Keep track of the wallet's own transaction outputs in memory":
(https://github.com/bitcoin/bitcoin/pull/27286#issuecomment-2913670535)
Updated the description with recent benchmarks.

💬 maflcko commented on pull request "rest: fetch spent transaction outputs by blockhash":
(https://github.com/bitcoin/bitcoin/pull/32540#discussion_r2110009543)
(If you want, you can also remove them in other places in this file. This way, the file doesn't have to be touched again for that reason.)

💬 achow101 commented on pull request "multiprocess: Add bitcoin wrapper executable":
(https://github.com/bitcoin/bitcoin/pull/31375#issuecomment-2913752433)
ACK a5ac43d98d1ad3ebed934f2c50208a85aae17e5e

🚀 achow101 merged a pull request: "multiprocess: Add bitcoin wrapper executable"
(https://github.com/bitcoin/bitcoin/pull/31375)

💬 achow101 commented on pull request "test: Add missing ipc subtree to lint":
(https://github.com/bitcoin/bitcoin/pull/32623#issuecomment-2913796673)
ACK fa4b8b16c37830bf7ee1419aae94e99f294f0e9f

💬 achow101 commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#issuecomment-2913804719)
ACK 09ee8b7f278627b917f0784adf23cbc76cae5fa0

🚀 achow101 merged a pull request: "test: Add missing ipc subtree to lint"
(https://github.com/bitcoin/bitcoin/pull/32623)

🤔 mzumsande reviewed a pull request: "p2p: Drop unsolicited CMPCTBLOCK from non-HB peer"
(https://github.com/bitcoin/bitcoin/pull/32606#pullrequestreview-2872405952)
Concept ACK

In the commit message:
"making it only possible for peers selected as high bandwidth to discover a node's
mempool" is a bit too strong, since mempools are discoverable, just not transactions that were added to them after the last inv to the peer.

🚀 achow101 merged a pull request: "blocks: avoid recomputing block header hash in `ReadBlock`"
(https://github.com/bitcoin/bitcoin/pull/32487)

💬 enirox001 commented on pull request "kernel: Separate UTXO set access from validation functions":
(https://github.com/bitcoin/bitcoin/pull/32317#issuecomment-2913994757)
Concept ACK. I'll be conducting a detailed commit-by-commit code review shortly.

🤔 janb84 reviewed a pull request: "wallet, rpc: Return normalized descriptor in parent_descs"
(https://github.com/bitcoin/bitcoin/pull/32594#pullrequestreview-2872621106)
re ACK fdabfc7b01ec2cff5f2356e1b13b5537d30dbaf8

Changes since last ACK:
- removal of erroneous / double comment
- rename of variable to explicit make clear it's a dummy

💬 instagibbs commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2110266152)
done

💬 instagibbs commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2110266257)
done

💬 instagibbs commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2110266338)
deleted

💬 instagibbs commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2110266413)
done

💬 instagibbs commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2110266477)
done

💬 instagibbs commented on pull request "test: add MAX_DISCONNECTED_TX_POOL_BYTES, chainlimits coverage":
(https://github.com/bitcoin/bitcoin/pull/32516#discussion_r2110266583)
did a proper reorg using blocktools to prep the fork.