Skip to content

Commit 591eea7

Browse files
committed
Merge bitcoin/bitcoin#33082: wallet, refactor: Remove Legacy check and error
d3c5e47 wallet, refactor: Remove Legacy check and error (pablomartin4btc) 30c6f64 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 bitcoin/bitcoin@d3c5e47 achow101: ACK d3c5e47 l0rinc: code review ACK d3c5e47 Tree-SHA512: ead37cf4061dfce59feb41ac50e807e6790e1a5e6b358e3b9c13e63d61a9cb82317a2e596cecb543f62f88a4338171788b651452425c1f40b5c1bec7fe78339e
2 parents c0894a0 + d3c5e47 commit 591eea7

File tree

4 files changed

+1
-6
lines changed

4 files changed

+1
-6
lines changed

src/wallet/test/coinselector_tests.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ inline std::vector<OutputGroup>& KnapsackGroupOutputs(const CoinsResult& availab
159159
static std::unique_ptr<CWallet> NewWallet(const node::NodeContext& m_node, const std::string& wallet_name = "")
160160
{
161161
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), wallet_name, CreateMockableWalletDatabase());
162-
BOOST_CHECK(wallet->LoadWallet() == DBErrors::LOAD_OK);
163162
LOCK(wallet->cs_wallet);
164163
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
165164
wallet->SetupDescriptorScriptPubKeyMans();

src/wallet/test/group_outputs_tests.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ static int nextLockTime = 0;
1919
static std::shared_ptr<CWallet> NewWallet(const node::NodeContext& m_node)
2020
{
2121
std::unique_ptr<CWallet> wallet = std::make_unique<CWallet>(m_node.chain.get(), "", CreateMockableWalletDatabase());
22-
wallet->LoadWallet();
2322
LOCK(wallet->cs_wallet);
2423
wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS);
2524
wallet->SetupDescriptorScriptPubKeyMans();

src/wallet/test/wallet_test_fixture.cpp

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ WalletTestingSetup::WalletTestingSetup(const ChainType chainType)
1414
m_wallet_loader{interfaces::MakeWalletLoader(*m_node.chain, *Assert(m_node.args))},
1515
m_wallet(m_node.chain.get(), "", CreateMockableWalletDatabase())
1616
{
17-
m_wallet.LoadWallet();
1817
m_chain_notifications_handler = m_node.chain->handleNotifications({ &m_wallet, [](CWallet*) {} });
1918
m_wallet_loader->registerRpcs();
2019
}

src/wallet/wallet.cpp

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3687,9 +3687,7 @@ util::Result<std::reference_wrapper<DescriptorScriptPubKeyMan>> CWallet::AddWall
36873687
{
36883688
AssertLockHeld(cs_wallet);
36893689

3690-
if (!IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS)) {
3691-
return util::Error{_("Cannot add WalletDescriptor to a non-descriptor wallet")};
3692-
}
3690+
Assert(IsWalletFlagSet(WALLET_FLAG_DESCRIPTORS));
36933691

36943692
auto spk_man = GetDescriptorScriptPubKeyMan(desc);
36953693
if (spk_man) {

0 commit comments

Comments
 (0)