💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808706)
Done

💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#discussion_r2107808810)
Done

💬 achow101 commented on pull request "wallet: Remove watchonly behavior and isminetypes":
(https://github.com/bitcoin/bitcoin/pull/32523#issuecomment-2910550733)
> maybe the first 4 commits can be a separate PR that removes the watch only stuff separately

I've split those commits into #32618

💬 jonatack commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2107820363)
> Would it help if we included the two different hashes in the error?

Sure, why not, since we have them available.

💬 achow101 commented on pull request "wallet: init, don't error out when loading legacy wallets":
(https://github.com/bitcoin/bitcoin/pull/32449#issuecomment-2910573702)
ACK 86e1111239cdb39dd32cfb5178653c608fa30515

🤔 jonatack reviewed a pull request: "blocks: avoid recomputing block header hash in `ReadBlock`"
(https://github.com/bitcoin/bitcoin/pull/32487#pullrequestreview-2869118446)
LGTM, perhaps take some of the review suggestions if you're willing

💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2107822793)
I'm planning a follow-up to this, will do it there

💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2107822981)
Will do it in a follow up, thanks

💬 jonatack commented on pull request "log: print reason when writing chainstate":
(https://github.com/bitcoin/bitcoin/pull/32404#issuecomment-2910581812)
ACK 53e9b71b2fd59c18b75e45e3a24c39182c20a59e per `git range-diff 2df824 cfc8056 53e9b71`

📝 achow101 opened a pull request: "wallet, rpc, gui: List legacy wallets with a message about migration"
(https://github.com/bitcoin/bitcoin/pull/32619)
A new field `warnings` is added for each wallet in `listwalletdir`. If a legacy wallet is detected, the warning will contain a message that the wallet is a legacy wallet and will need to be migrated before it can be loaded.

In the GUI, the "Open Wallet" menu is changed to show legacy wallets greyed out with "(needs migration)" appended to their name to indicate to the user that the legacy wallet will need to be migrated.

💬 jonatack commented on pull request "wallet, rpc, gui: List legacy wallets with a message about migration":
(https://github.com/bitcoin/bitcoin/pull/32619#discussion_r2107860073)
I also needed to change this to make `wallet_multiwallet.py` pass:

```diff
--- a/test/functional/wallet_multiwallet.py
+++ b/test/functional/wallet_multiwallet.py
@@ -72,7 +72,7 @@ class MultiWalletTest(BitcoinTestFramework):
return wallet_dir(name, "wallet.dat")
return wallet_dir(name)

- assert_equal(self.nodes[0].listwalletdir(), {'wallets': [{'name': self.default_wallet_name}]})
+ assert_equal(self.nodes[0].listwalletdir(), {'wallets':
...

💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2107863221)
Decided to do it here instead:

> [ReadBlock] [error] GetHash() doesn't match index at FlatFilePos(nFile=0, nPos=8) while reading block (000000000019d6689c085ae165831e934ff763ae46a2a6c172b3f1b60a8ce26f != 0000000000000000000000000000000000000000000000000000000000000001)
test/blockmanager_tests.cpp:147: info: check !m_node.chainman->m_blockman.ReadBlock(dummy, *fake_index) has passed

💬 l0rinc commented on pull request "blocks: avoid recomputing block header hash in `ReadBlock`":
(https://github.com/bitcoin/bitcoin/pull/32487#discussion_r2107863310)
Did it here instead - thanks

📝 achow101 opened a pull request: "wallet: Fix wallet interface detection of encrypted wallets"
(https://github.com/bitcoin/bitcoin/pull/32620)
The GUI uses `WalletLoader::isEncrypted()` to detect whether a wallet file is encrypted so that it knows whether to prompt for a passphrase when migrating a legacy wallet. However, legacy wallets need to be opened with `options.require_format = BERKELEY_RO`. Since this wasn't being provided, following #28710, encrypted legacy wallets could not be migrated.

This fixes the issue by detecting when a wallet file is for a legacy wallet, and re-attempting with `options.require_format = BERKELEY_RO`
...

📝 achow101 converted_to_draft a pull request: "wallet: Fix wallet interface detection of encrypted wallets"
(https://github.com/bitcoin/bitcoin/pull/32620)
The GUI uses `WalletLoader::isEncrypted()` to detect whether a wallet file is encrypted so that it knows whether to prompt for a passphrase when migrating a legacy wallet. However, legacy wallets need to be opened with `options.require_format = BERKELEY_RO`. Since this wasn't being provided, following #28710, encrypted legacy wallets could not be migrated.

This fixes the issue by detecting when a wallet file is for a legacy wallet, and re-attempting with `options.require_format = BERKELEY_RO`
...

📝 achow101 opened a pull request: "gui: Add a menu action to restore then migrate a legacy wallet"
(https://github.com/bitcoin-core/gui/pull/877)
Some users will have a backup of their legacy wallet. These cannot be restored since the "Restore Wallet" action expects to be able to load the wallet after restoring, and this fails for legacy wallets now that they are deleted. Furthermore, the "Migrate Wallet" action only allows users to migrate wallets that are in the wallets directory, so such backups cannot be migrated from the GUI.

This PR resolves this issue by adding a menu item in the "Migrate Wallet" menu which allows users to selec
...

💬 achow101 commented on pull request "wallet, rpc, gui: List legacy wallets with a message about migration":
(https://github.com/bitcoin/bitcoin/pull/32619#discussion_r2107887370)
Indeed, done.

💬 adyshimony commented on pull request "wallet: Always set descriptor cache upgraded flag for new wallets":
(https://github.com/bitcoin/bitcoin/pull/32597#issuecomment-2910699910)
ACK

[47237cd](https://github.com/bitcoin/bitcoin/pull/32597/commits/47237cd1938058b29fdec242c3a37611e255fda0)


Manual tests:

```

./bitcoin-cli createwallet "test_32597_wallet" false false "" false true
{
"name": "test_32597_wallet",
"warnings": [
"Empty string given as passphrase, wallet will not be encrypted."
]
}



./bitcoin-cli -rpcwallet=test_32597_wallet getwalletinfo
{
"walletname": "test_32597_wallet",
"walletversion": 169900,
"format": "sqlite
...

📝 theStack opened a pull request: "contrib: utxo_to_sqlite.py: add option to store txid/spk as BLOBs"
(https://github.com/bitcoin/bitcoin/pull/32621)
This PR is a late follow-up to https://github.com/bitcoin/bitcoin/pull/27432, introducing an option for the utxo-to-sqlite script to store the txid/scriptPubKey columns as bytes (= `BLOB` storage class in sqlite, see e.g. https://www.sqlite.org/datatype3.html in sqlite) rather than hex strings. This was proposed in earlier reviews (https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1516857024, https://github.com/bitcoin/bitcoin/pull/27432#issuecomment-1653739351) and has the obvious adva
...

💬 Krellan 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-2910981771)
Good idea. I had this happen to me just recently. I encountered a Catch-22: couldn't restore an old wallet file because it wasn't in a currently recognized format (it told me to use the migrate wallet option instead), but I also couldn't access the wallet in the migrate menu because it hadn't successfully been loaded yet!

I tried manually copying the `wallet.dat` file into a new directory created in my Bitcoin datadir, but that just produced an error at startup saying the wallet was not in a
...