diff options
| author | fanquake <fanquake@gmail.com> | 2023-12-12 11:31:30 +0000 |
|---|---|---|
| committer | fanquake <fanquake@gmail.com> | 2023-12-12 11:47:34 +0000 |
| commit | 60f677375e52832536a599afae3226f8e2155d88 (patch) | |
| tree | a62c7d2c6756564d2dc35ac69e28f18cab368830 | |
| parent | 7a283836eb78d76452fcc586275626df010f3cd9 (diff) | |
| parent | bd7f5d33e3b9e01cd600afe1682b7afa935f68a3 (diff) | |
Merge bitcoin/bitcoin#29055: tests, bench: Fix issue with CWallet::LoadWallet() being called in the wrong places
bd7f5d33e3b9e01cd600afe1682b7afa935f68a3 wallet: Assert that the wallet is not initialized in LoadWallet (Andrew Chow)
fb0b6ca4e5d981cf58bf23ae2993117f171608e8 tests, bench: Remove incorrect LoadWallet() calls (Andrew Chow)
Pull request description:
`CWallet::LoadWallet()` expects to be called after a `CWallet` is constructed, but before any of its member functions called. Doing so invalidates pointers which causes issues with some PRs and branches that I am working on. This was being used incorrectly in a few tests and benchmarks, resulting in segfaults.
As a precaution for this kind of issue in the future, I've also added a few asserts to `LoadWallet()` so that developers will notice when it is used incorrectly.
As similar issue was fixed in #27666
ACKs for top commit:
S3RK:
ACK bd7f5d33e3b9e01cd600afe1682b7afa935f68a3
furszy:
ACK bd7f5d33
Tree-SHA512: 7664f12b8452994e7fc4d7d4f77697fb5f75edb0dba95ba99a4a23ec03d5b8e0ecbdcb7635547a0e8b4f89f708f98dcb5d039df0559e24b1ae411ed630e16e14
| -rw-r--r-- | src/bench/wallet_create_tx.cpp | 3 | ||||
| -rw-r--r-- | src/wallet/test/util.cpp | 1 | ||||
| -rw-r--r-- | src/wallet/wallet.cpp | 2 |
3 files changed, 2 insertions, 4 deletions
diff --git a/src/bench/wallet_create_tx.cpp b/src/bench/wallet_create_tx.cpp index 632918c0ca..c0ca8f983d 100644 --- a/src/bench/wallet_create_tx.cpp +++ b/src/bench/wallet_create_tx.cpp @@ -16,7 +16,6 @@ using wallet::CWallet; using wallet::CreateMockableWalletDatabase; -using wallet::DBErrors; using wallet::WALLET_FLAG_DESCRIPTORS; struct TipBlock @@ -90,7 +89,6 @@ static void WalletCreateTx(benchmark::Bench& bench, const OutputType output_type LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetupDescriptorScriptPubKeyMans(); - if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false); } // Generate destinations @@ -146,7 +144,6 @@ static void AvailableCoins(benchmark::Bench& bench, const std::vector<OutputType LOCK(wallet.cs_wallet); wallet.SetWalletFlag(WALLET_FLAG_DESCRIPTORS); wallet.SetupDescriptorScriptPubKeyMans(); - if (wallet.LoadWallet() != DBErrors::LOAD_OK) assert(false); } // Generate destinations diff --git a/src/wallet/test/util.cpp b/src/wallet/test/util.cpp index ad8613d515..cbf3ccd1ec 100644 --- a/src/wallet/test/util.cpp +++ b/src/wallet/test/util.cpp @@ -24,7 +24,6 @@ std::unique_ptr<CWallet> CreateSyncedWallet(interfaces::Chain& chain, CChain& cc LOCK2(wallet->cs_wallet, ::cs_main); wallet->SetLastBlockProcessed(cchain.Height(), cchain.Tip()->GetBlockHash()); } - wallet->LoadWallet(); { LOCK(wallet->cs_wallet); wallet->SetWalletFlag(WALLET_FLAG_DESCRIPTORS); diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp index 4d4e23dd4c..892b9d51e4 100644 --- a/src/wallet/wallet.cpp +++ b/src/wallet/wallet.cpp @@ -2292,6 +2292,8 @@ DBErrors CWallet::LoadWallet() { LOCK(cs_wallet); + Assert(m_spk_managers.empty()); + Assert(m_wallet_flags == 0); DBErrors nLoadWalletRet = WalletBatch(GetDatabase()).LoadWallet(this); if (nLoadWalletRet == DBErrors::NEED_REWRITE) { |