diff options
author | fanquake <fanquake@gmail.com> | 2021-07-01 09:42:42 +0800 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2021-07-01 10:06:56 +0800 |
commit | 045bb06ebd55e826b77594f835c4b67f7dab2994 (patch) | |
tree | 75dcba7bb11e0da7f6d8e80498ae8c75abf8c15a /src/wallet/wallet.cpp | |
parent | 722776c0fd218cc41ccb741453c58190c71e64f9 (diff) | |
parent | 3efaf83c75cd8dc2fa084537b8ed6715fb58c04d (diff) | |
download | bitcoin-045bb06ebd55e826b77594f835c4b67f7dab2994.tar.xz |
Merge bitcoin/bitcoin#19651: wallet: importdescriptors update existing
3efaf83c75cd8dc2fa084537b8ed6715fb58c04d wallet: deactivate descriptor (S3RK)
6737d9655bcf527afbd85d610d805a2d0fd28c4f test: wallet importdescriptors update existing (S3RK)
586f1d53d60880ea2873d860f95e3390016620d1 wallet: maintain SPK consistency on internal flag change (S3RK)
f1b7db14748d9ee04735b4968366d33bc89aea23 wallet: don't mute exceptions in importdescriptors (S3RK)
bf68ebc1cd555f791103f81adc9111e0e55c8003 wallet: allow to import same descriptor twice (S3RK)
Pull request description:
Rationale: allow updating existing descriptors with `importdescriptors` command.
Currently if you run same `importdescriptors` command twice with a descriptor containing private key you will get very confusing error — `Missing required fields`. What happens is that Wallet tries to write imported private key to the disk, but it exists already so we get `DB_KEYEXIST (-30995)` from BerkelyDB. Please note, that we set `DB_NOOVERWRITE` (I guess not to lose some keys accidentally). The exception is caught in `catch (...)` in rpcdump.cpp with a generic error.
With this PR if a descriptor is already present than we will update its activeness, internalness, label, range and next_index.
For the range only expansion is allowed (range start can only decrease, range end increase).
ACKs for top commit:
achow101:
re-ACK 3efaf83c75cd8dc2fa084537b8ed6715fb58c04d
meshcollider:
Code review ACK 3efaf83c75cd8dc2fa084537b8ed6715fb58c04d
jonatack:
Light ACK 3efaf83c75cd8dc2fa084537b8ed6715fb58c04d per `git range-diff a000cb0 5d96704 3efaf83` and as a sanity check, re-debug-built on debian with gcc 10.2.1 and clang 11, ran wallet_importdescriptors.py
Tree-SHA512: 122c4b621d64ec8a3b625f3aed9f01a2b5cbaf2029ad0325b5ff38d67fff5cd35324335fabe2dd5169548b01b267c81be6ae0f5c834342f3d5f6eeed515c4843
Diffstat (limited to 'src/wallet/wallet.cpp')
-rw-r--r-- | src/wallet/wallet.cpp | 74 |
1 files changed, 39 insertions, 35 deletions
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 36807cbeeb..c506bc6255 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -3168,12 +3168,38 @@ void CWallet::AddActiveScriptPubKeyMan(uint256 id, OutputType type, bool interna void CWallet::LoadActiveScriptPubKeyMan(uint256 id, OutputType type, bool internal) { + // Activating ScriptPubKeyManager for a given output and change type is incompatible with legacy wallets. + // Legacy wallets have only one ScriptPubKeyManager and it's active for all output and change types. + Assert(IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)); + WalletLogPrintf("Setting spkMan to active: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast<int>(type), static_cast<int>(internal)); auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers; + auto& spk_mans_other = internal ? m_external_spk_managers : m_internal_spk_managers; auto spk_man = m_spk_managers.at(id).get(); spk_man->SetInternal(internal); spk_mans[type] = spk_man; + if (spk_mans_other[type] == spk_man) { + spk_mans_other[type] = nullptr; + } + + NotifyCanGetAddressesChanged(); +} + +void CWallet::DeactivateScriptPubKeyMan(uint256 id, OutputType type, bool internal) +{ + auto spk_man = GetScriptPubKeyMan(type, internal); + if (spk_man != nullptr && spk_man->GetID() == id) { + WalletLogPrintf("Deactivate spkMan: id = %s, type = %d, internal = %d\n", id.ToString(), static_cast<int>(type), static_cast<int>(internal)); + WalletBatch batch(GetDatabase()); + if (!batch.EraseActiveScriptPubKeyMan(static_cast<uint8_t>(type), internal)) { + throw std::runtime_error(std::string(__func__) + ": erasing active ScriptPubKeyMan id failed"); + } + + auto& spk_mans = internal ? m_internal_spk_managers : m_external_spk_managers; + spk_mans[type] = nullptr; + } + NotifyCanGetAddressesChanged(); } @@ -3207,44 +3233,26 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat } LOCK(cs_wallet); - auto new_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, desc)); - - // If we already have this descriptor, remove it from the maps but add the existing cache to desc - auto old_spk_man = GetDescriptorScriptPubKeyMan(desc); - if (old_spk_man) { + auto spk_man = GetDescriptorScriptPubKeyMan(desc); + if (spk_man) { WalletLogPrintf("Update existing descriptor: %s\n", desc.descriptor->ToString()); + spk_man->UpdateWalletDescriptor(desc); + } else { + auto new_spk_man = std::unique_ptr<DescriptorScriptPubKeyMan>(new DescriptorScriptPubKeyMan(*this, desc)); + spk_man = new_spk_man.get(); - { - LOCK(old_spk_man->cs_desc_man); - new_spk_man->SetCache(old_spk_man->GetWalletDescriptor().cache); - } - - // Remove from maps of active spkMans - auto old_spk_man_id = old_spk_man->GetID(); - for (bool internal : {false, true}) { - for (OutputType t : OUTPUT_TYPES) { - auto active_spk_man = GetScriptPubKeyMan(t, internal); - if (active_spk_man && active_spk_man->GetID() == old_spk_man_id) { - if (internal) { - m_internal_spk_managers.erase(t); - } else { - m_external_spk_managers.erase(t); - } - break; - } - } - } - m_spk_managers.erase(old_spk_man_id); + // Save the descriptor to memory + m_spk_managers[new_spk_man->GetID()] = std::move(new_spk_man); } // Add the private keys to the descriptor for (const auto& entry : signing_provider.keys) { const CKey& key = entry.second; - new_spk_man->AddDescriptorKey(key, key.GetPubKey()); + spk_man->AddDescriptorKey(key, key.GetPubKey()); } // Top up key pool, the manager will generate new scriptPubKeys internally - if (!new_spk_man->TopUp()) { + if (!spk_man->TopUp()) { WalletLogPrintf("Could not top up scriptPubKeys\n"); return nullptr; } @@ -3252,7 +3260,7 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat // Apply the label if necessary // Note: we disable labels for ranged descriptors if (!desc.descriptor->IsRange()) { - auto script_pub_keys = new_spk_man->GetScriptPubKeys(); + auto script_pub_keys = spk_man->GetScriptPubKeys(); if (script_pub_keys.empty()) { WalletLogPrintf("Could not generate scriptPubKeys (cache is empty)\n"); return nullptr; @@ -3264,12 +3272,8 @@ ScriptPubKeyMan* CWallet::AddWalletDescriptor(WalletDescriptor& desc, const Flat } } - // Save the descriptor to memory - auto ret = new_spk_man.get(); - m_spk_managers[new_spk_man->GetID()] = std::move(new_spk_man); - // Save the descriptor to DB - ret->WriteDescriptor(); + spk_man->WriteDescriptor(); - return ret; + return spk_man; } |