0.17: Backport 15297

promag commented at 11:34 pm on March 10, 2019: member

This PR backports the following pull requests:

#12493 [wallet] Reopen CDBEnv after encryption instead of shutting down
#14350 Add WalletLocation class
#14320 [bugfix] wallet: Fix duplicate fileid detection
#14552 wallet: detecting duplicate wallet by comparing the db filename.
#11911 Free BerkeleyEnvironment instances when not in use
#15297 wallet: Releases dangling files on BerkeleyEnvironment::Close

fanquake added the label Wallet on Mar 10, 2019

fanquake added the label Backport on Mar 10, 2019

Add function to close all Db's and reload the databae environment

Adds a ReloadDbEnv function to BerkeleyEnvironment in order to close all Db
instances, closes the environment, resets it, and then reopens
the BerkeleyEnvironment.

Also adds a ReloadDbEnv function to BerkeleyDatabase that calls
BerkeleyEnvironment's ReloadDbEnv.

Github-Pull: #12493
Rebased-From: 5d296ac

f455979eb1

After encrypting the wallet, reload the database environment

Calls ReloadDbEnv after encrypting the wallet so that the database
environment is flushed, closed, and reopened to prevent unencrypted
keys from being saved on disk.

Github-Pull: #12493
Rebased-From: d7637c5

048fda2a66

Move BerkeleyEnvironment deletion from internal method to callsite

Instead of having the object destroy itself, having the caller
destroy it.

Github-Pull: #12493
Rebased-From: a769461

435df68c62

No longer shutdown after encrypting the wallet

Since the database environment is flushed, closed, and reopened during
EncryptWallet, there is no need to shut down the software anymore.

Github-Pull: #12493
Rebased-From: c1dde3a

1c98a758d0

promag force-pushed on Mar 11, 2019

in src/wallet/walletutil.cpp:50 in b35a71827b outdated

45+    //  - big endian systems - 00 05 31 62
46+    //  - little endian systems - 62 31 05 00
47+    return data == 0x00053162 || data == 0x62310500;
48+}
49+
50+std::vector<fs::path> ListWalletDir()

ryanofsky commented at 8:40 pm on March 11, 2019:

In commit “wallet: Add WalletLocation utility class” (b35a71827bddf534816f1c5fda83c8e33a18eb63)

This does not appear to have been cherry-picked from 01a4c095c87500650663341533f000c6b613e9da correctly. The original commit is not adding any ListWalletDir function, so it is unclear where this is coming from. Also the function is being added without being called in this or later commits.

promag commented at 11:17 pm on March 11, 2019:

Fixed.

ryanofsky commented at 8:42 pm on March 11, 2019: member

It looks like something went wrong with the #14350 cherry-pick.

wallet: Add WalletLocation utility class

Github-Pull: #14350
Rebased-From: 01a4c09

21693ff0b7

wallet: Refactor to use WalletLocation

Github-Pull: #14350
Rebased-From: 65f3672

16e5759455

wallet: Fix duplicate fileid

Github-Pull: #14320
Rebased-From: 2d796fa

8965b6ab47

tests: add test case for loading copied wallet twice

Github-Pull: #14320
Rebased-From: 4ea7732

34da2b7c76

wallet: Add trailing wallet.dat when detecting duplicate wallet if it's a directory.

Github-Pull: #14552
Rebased-From: 15c93f0

caf1146b13

Refactor: Move m_db pointers into BerkeleyDatabase

This is a refactoring change that doesn't affect behavior. The motivation
behind the change is give BerkeleyEnvironment objects access to
BerkeleyDatabase objects so it will be possible to simplify the duplicate
wallet check and more reliably avoid opening the same databases twice.

Github-Pull: #14552
Rebased-From: c456fbd

7751ea37b6

wallet: Create IsDatabaseLoaded function

Github-Pull: #14552
Rebased-From: 5912031

0a9af2d4cb

promag force-pushed on Mar 11, 2019

ryanofsky commented at 6:50 pm on March 12, 2019: member

There seems to be a problem with 940109dabfb5d657ed73e46de451af017b0d2312. I verified with script from #11592#pullrequestreview-73922693 that backport commits looked good (are present in master and have conflicts resolved correctly). But wallet_multiwallet.py is failing on travis and I also see it fail locally.

promag commented at 6:54 pm on March 12, 2019: member

@ryanofsky yup me too, actually 662f8f8 is failing. I’m checking if I’m missing other PR.

Free BerkeleyEnvironment instances when not in use

Instead of adding BerkeleyEnvironment objects permanently to the g_dbenvs map,
use reference counted shared pointers and remove map entries when the last
BerkeleyEnvironment reference goes out of scope.

This change was requested by Matt Corallo <git@bluematt.me> and makes code that
sets up mock databases cleaner. The mock database environment will now go out
of scope and be reset on destruction so there is no need to call
BerkeleyEnvironment::Reset() during wallet construction to clear out prior
state.

This change does affect bitcoin behavior slightly. On startup, instead of same
wallet environments staying open throughout VerifyWallets() and OpenWallets()
calls, VerifyWallets() will open and close an environment once for each wallet,
and OpenWallets() will create its own environment(s) later.

Github-Pull: #11911
Rebased-From: f1f4bb7

f22d02f537

wallet: Close wallet env lock file

Close .walletlock file when a BerkeleyEnvironment is deleted.

Github-Pull: #15297
Rebased-From: 2f8b8f4

2e9e904a5d

qa: Test .walletlock file is closed

Github-Pull: #15297
Rebased-From: d3bf3b9

fe95f84542

Trivial: add doxygen-compatible comments relating to BerkeleyEnvironment

Github-Pull: #11911
Rebased-From: 14bc2a1

85c6263ddb

Tests: add unit tests for GetWalletEnv

Github-Pull: #11911
Rebased-From: 88b1d95

f20513bd71

wallet: Close dbenv error file db.log

The error file db.log is opened by BerkeleyEnvironment instance and
should be closed after dbenv is closed.

Github-Pull: #15297
Rebased-From: 8602a1e

22cdb6cf59

promag force-pushed on Mar 12, 2019

promag commented at 11:44 pm on March 12, 2019: member

@ryanofsky included #11911.

fanquake added this to the milestone 0.17.2 on Mar 12, 2019

ryanofsky approved

ryanofsky commented at 3:24 pm on March 13, 2019: member

utACK fe95f84542f81862e9759503416d9da9f67d191b. Verified with script from #11592#pullrequestreview-73922693 that backport commits looked good (are present in master and have conflicts resolved correctly).

promag commented at 4:43 pm on March 13, 2019: member

Thanks for the review @ryanofsky.

laanwj merged this on Mar 20, 2019

laanwj closed this on Mar 20, 2019

laanwj referenced this in commit 6cf81b01b4 on Mar 20, 2019

promag deleted the branch on Mar 20, 2019

sidhujag referenced this in commit 3ee64a0aa7 on Mar 28, 2019

Rishabh42 referenced this in commit ce3547fae6 on Apr 22, 2019

DrahtBot locked this on Dec 16, 2021

0.17: Backport 15297 #15575