diff options
| author | MarcoFalke <falke.marco@gmail.com> | 2021-07-20 15:04:01 +0200 |
|---|---|---|
| committer | MarcoFalke <falke.marco@gmail.com> | 2021-07-20 15:04:07 +0200 |
| commit | 539023ab4155c94c6e52dd6b2dd504619ae45e08 (patch) | |
| tree | cbea5d9c9e12d63d3010dad0c1860f861a02ebde /test | |
| parent | 8ed8164e6f1e85dce38ceb4f4766c21718691046 (diff) | |
| parent | 9b85a5e2f7e003ca8621feaac9bdd304d19081b4 (diff) | |
Merge bitcoin/bitcoin#22492: wallet: Reorder locks in dumpwallet to avoid lock order assertion
9b85a5e2f7e003ca8621feaac9bdd304d19081b4 tests: Test for dumpwallet lock order issue (Andrew Chow)
25d99e6511d8c43b2025a89bcd8295de755346a7 Reorder dumpwallet so that cs_main functions go first (Andrew Chow)
Pull request description:
When a wallet is loaded which has an unconfirmed transaction in the mempool, it will end up establishing the lock order of cs_wallet -> cs_main -> cs_KeyStore. If `dumpwallet` is used on this wallet, then a lock order of cs_wallet -> cs_KeyStore -> cs_main will be used, which causes a lock order assertion. This PR fixes this by reordering `dumpwallet` and `GetKeyBirthTimes` (only used by `dumpwallet`). Specifically, in both functions, the function calls which lock cs_main are done prior to locking cs_KeyStore. This avoids the lock order issue.
Additionally, I have added a test case to `wallet_dump.py`. Of course testing this requires `--enable-debug`.
Fixes #22489
ACKs for top commit:
MarcoFalke:
review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4 🎰
ryanofsky:
Code review ACK 9b85a5e2f7e003ca8621feaac9bdd304d19081b4. Nice to reduce lock scope, and good test!
prayank23:
tACK https://github.com/bitcoin/bitcoin/pull/22492/commits/9b85a5e2f7e003ca8621feaac9bdd304d19081b4
lsilva01:
Tested ACK https://github.com/bitcoin/bitcoin/pull/22492/commits/9b85a5e2f7e003ca8621feaac9bdd304d19081b4 under the same conditions reported in issue #22489 and the `dumpwallet` command completed successfully.
Tree-SHA512: d370a8f415ad64ee6a538ff419155837bcdbb167e3831b06572562289239028c6b46d80b23d227286afe875d9351f3377574ed831549ea426fb926af0e19c755
Diffstat (limited to 'test')
| -rwxr-xr-x | test/functional/wallet_dump.py | 9 |
1 files changed, 9 insertions, 0 deletions
diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index eb54da99f5..91d6121679 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -209,6 +209,15 @@ class WalletDumpTest(BitcoinTestFramework): with self.nodes[0].assert_debug_log(['Flushing wallet.dat'], timeout=20): self.nodes[0].getnewaddress() + # Make sure that dumpwallet doesn't have a lock order issue when there is an unconfirmed tx and it is reloaded + # See https://github.com/bitcoin/bitcoin/issues/22489 + self.nodes[0].createwallet("w3") + w3 = self.nodes[0].get_wallet_rpc("w3") + w3.importprivkey(privkey=self.nodes[0].get_deterministic_priv_key().key, label="coinbase_import") + w3.sendtoaddress(w3.getnewaddress(), 10) + w3.unloadwallet() + self.nodes[0].loadwallet("w3") + w3.dumpwallet(os.path.join(self.nodes[0].datadir, "w3.dump")) if __name__ == '__main__': WalletDumpTest().main() |