highperfocused/bitcoin
1
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2026-06-02 09:15:04 +02:00
Code Issues Packages Projects Releases Wiki Activity

Merge bitcoin/bitcoin#33082: wallet, refactor: Remove Legacy check and error

Browse Source 
d3c5e47391 wallet, refactor: Remove Legacy check and error (pablomartin4btc)
30c6f64eed test: Remove unnecessary LoadWallet() calls (pablomartin4btc)

Pull request description:

  Remove dead code due to legacy wallet removal.

  Leftovers from previous #32481.

  ---

  **Note**:

  While attempting to remove the legacy check in `CWallet::UpgradeDescriptorCache()` (which is called from `DBErrors WalletBatch::LoadWallet(CWallet* pwallet))`, I once again ran into the fact that `LoadWallet()` is used in two distinct scenarios — something I was already aware of:
  - Wallet creation – the upgrade is ignored here because no wallet flags are yet set; attempting to set a flag (ie `WALLET_FLAG_LAST_HARDENED_XPUB_CACHED` at the end of the upgrade function, if the legacy check is removed) would produce a failure (`DBErrors CWallet::LoadWallet()` -> `Assert(m_wallet_flags == 0)`).
  - Wallet loading – the upgrade proceeds correctly and the flag `WALLET_FLAG_LAST_HARDENED_XPUB_CACHED` is set.

  While revisiting this, I also noticed that some `LoadWallet()` calls in the wallet tests are unnecessary and I've removed them in the first commit.

  The following change in `UpgradeDescriptorCache()` could be done in PR #32636 as part of the separation between wallet loading and creation responsibilities.

  ```diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp

   void CWallet::UpgradeDescriptorCache()
   {
  +    // Only descriptor wallets can upgrade descriptor cache
  +    Assert(IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
  +
  -    if (!IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS) || IsLocked() || IsWalletFlagSet(WALLET_FLAG_LAST_HARDENED_XPUB_CACHED)) {
  +    if (IsLocked() || IsWalletFlagSet(WALLET_FLAG_LAST_HARDENED_XPUB_CACHED)) {
           return;
       }
  ```

ACKs for top commit:
  davidgumberg:
    crACK d3c5e47391
  achow101:
    ACK d3c5e47391
  l0rinc:
    code review ACK d3c5e47391

Tree-SHA512: ead37cf4061dfce59feb41ac50e807e6790e1a5e6b358e3b9c13e63d61a9cb82317a2e596cecb543f62f88a4338171788b651452425c1f40b5c1bec7fe78339e
This commit is contained in:
Ava Chow
2025-09-09 14:36:56 -07:00
parent c0894a0a2b d3c5e47391
commit 591eea7b5a
4 changed files with 1 additions and 6 deletions
Download Patch File Download Diff File Expand all files Collapse all files

1
src/wallet/test/coinselector_tests.cpp
View File

@@ -159,7 +159,6 @@ inline std::vector<OutputGroup>& KnapsackGroupOutputs(const CoinsResult& availab
static std::unique_ptr<CWallet> NewWallet(const node::NodeContext& m_node, const std::string& wallet_name = "")
{
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), wallet_name, CreateMockableWalletDatabase());
BOOST_CHECK(wallet->LoadWallet() == DBErrors::LOAD_OK);
LOCK(wallet->cs_wallet);
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
wallet->SetupDescriptorScriptPubKeyMans();