Merge #11687: External wallet files

be8ab7d08 Create new wallet databases as directories rather than files (Russell Yanofsky)
26c06f24e Allow wallet files not in -walletdir directory (Russell Yanofsky)
d8a99f65e Allow wallet files in multiple directories (Russell Yanofsky)

Pull request description:

  This change consists of three commits:

  * The first commit is a pure refactoring that removes the restriction that two wallets can only be opened at the same time if they are contained in the same directory.
  * The second commit removes the restriction that `-wallet` filenames can only refer to files in the `-walletdir` directory.
  * The third commit makes second commit a little safer by changing bitcoin to create wallet databases as directories rather than files, so they can be safely backed up.

  All three commits should be straightforward:

  *  The first commit adds around 20 lines of new code and then updates a bunch of function signatures (generally updating them to take plain fs::path parameters, instead of combinations of strings, fs::paths, and objects like CDBEnv and CWalletDBWrapper).
  * The second commit removes two `-wallet` filename checks and adds some test cases to the multiwallet unit test.
  * The third commit just changes the mapping from specified wallet paths to bdb environment & data paths.

    ---

  **Note:** For anybody looking at this PR for the first time, I think you can skip the comments before _20 Nov_ and start reading at https://github.com/bitcoin/bitcoin/pull/11687#issuecomment-345625565. Comments before  _20 Nov_ were about an earlier version of the PR that didn't include the third commit, and then confusion from not seeing the first commit.

Tree-SHA512: 00bbb120fe0df847cf57014f75f1f7f1f58b0b62fa0b3adab4560163ebdfe06ccdfff33b4231693f03c5dc23601cb41954a07bcea9a4919c8d42f7d62bcf6024

1 files changed, 38 insertions, 17 deletions

diff --git a/src/wallet/db.h b/src/wallet/db.h
index 787135e400..b1ce451534 100644
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -11,6 +11,7 @@
 #include <serialize.h>
 #include <streams.h>
 #include <sync.h>
+#include <util.h>
 #include <version.h>
 
 #include <atomic>
@@ -32,20 +33,19 @@ private:
     // shutdown problems/crashes caused by a static initialized internal pointer.
     std::string strPath;
 
-    void EnvShutdown();
-
 public:
-    mutable CCriticalSection cs_db;
     std::unique_ptr<DbEnv> dbenv;
     std::map<std::string, int> mapFileUseCount;
     std::map<std::string, Db*> mapDb;
 
-    CDBEnv();
+    CDBEnv(const fs::path& env_directory);
     ~CDBEnv();
     void Reset();
 
     void MakeMock();
     bool IsMock() const { return fMockDb; }
+    bool IsInitialized() const { return fDbEnvInit; }
+    fs::path Directory() const { return strPath; }
 
     /**
      * Verify that database file strFile is OK. If it is not,
@@ -56,7 +56,7 @@ public:
     enum VerifyResult { VERIFY_OK,
                         RECOVER_OK,
                         RECOVER_FAIL };
-    typedef bool (*recoverFunc_type)(const std::string& strFile, std::string& out_backup_filename);
+    typedef bool (*recoverFunc_type)(const fs::path& file_path, std::string& out_backup_filename);
     VerifyResult Verify(const std::string& strFile, recoverFunc_type recoverFunc, std::string& out_backup_filename);
     /**
      * Salvage data from a file that Verify says is bad.
@@ -68,7 +68,7 @@ public:
     typedef std::pair<std::vector<unsigned char>, std::vector<unsigned char> > KeyValPair;
     bool Salvage(const std::string& strFile, bool fAggressive, std::vector<KeyValPair>& vResult);
 
-    bool Open(const fs::path& path, bool retry = 0);
+    bool Open(bool retry);
     void Close();
     void Flush(bool fShutdown);
     void CheckpointLSN(const std::string& strFile);
@@ -85,7 +85,8 @@ public:
     }
 };
 
-extern CDBEnv bitdb;
+/** Get CDBEnv and database filename given a wallet path. */
+CDBEnv* GetWalletEnv(const fs::path& wallet_path, std::string& database_filename);
 
 /** An instance of this class represents one database.
  * For BerkeleyDB this is just a (env, strFile) tuple.
@@ -100,9 +101,33 @@ public:
     }
 
     /** Create DB handle to real database */
-    CWalletDBWrapper(CDBEnv *env_in, const std::string &strFile_in) :
-        nUpdateCounter(0), nLastSeen(0), nLastFlushed(0), nLastWalletUpdate(0), env(env_in), strFile(strFile_in)
+    CWalletDBWrapper(const fs::path& wallet_path, bool mock = false) :
+        nUpdateCounter(0), nLastSeen(0), nLastFlushed(0), nLastWalletUpdate(0)
     {
+        env = GetWalletEnv(wallet_path, strFile);
+        if (mock) {
+            env->Close();
+            env->Reset();
+            env->MakeMock();
+        }
+    }
+
+    /** Return object for accessing database at specified path. */
+    static std::unique_ptr<CWalletDBWrapper> Create(const fs::path& path)
+    {
+        return MakeUnique<CWalletDBWrapper>(path);
+    }
+
+    /** Return object for accessing dummy database with no read/write capabilities. */
+    static std::unique_ptr<CWalletDBWrapper> CreateDummy()
+    {
+        return MakeUnique<CWalletDBWrapper>();
+    }
+
+    /** Return object for accessing temporary in-memory database. */
+    static std::unique_ptr<CWalletDBWrapper> CreateMock()
+    {
+        return MakeUnique<CWalletDBWrapper>("", true /* mock */);
     }
 
     /** Rewrite the entire database on disk, with the exception of key pszSkip if non-zero
@@ -113,10 +138,6 @@ public:
      */
     bool Backup(const std::string& strDest);
 
-    /** Get a name for this database, for debugging etc.
-     */
-    std::string GetName() const { return strFile; }
-
     /** Make sure all changes are flushed to disk.
      */
     void Flush(bool shutdown);
@@ -161,15 +182,15 @@ public:
 
     void Flush();
     void Close();
-    static bool Recover(const std::string& filename, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename);
+    static bool Recover(const fs::path& file_path, void *callbackDataIn, bool (*recoverKVcallback)(void* callbackData, CDataStream ssKey, CDataStream ssValue), std::string& out_backup_filename);
 
     /* flush the wallet passively (TRY_LOCK)
        ideal to be called periodically */
     static bool PeriodicFlush(CWalletDBWrapper& dbw);
     /* verifies the database environment */
-    static bool VerifyEnvironment(const std::string& walletFile, const fs::path& walletDir, std::string& errorStr);
+    static bool VerifyEnvironment(const fs::path& file_path, std::string& errorStr);
     /* verifies the database file */
-    static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& walletDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc);
+    static bool VerifyDatabaseFile(const fs::path& file_path, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc);
 
 public:
     template <typename K, typename T>
@@ -329,7 +350,7 @@ public:
     {
         if (!pdb || activeTxn)
             return false;
-        DbTxn* ptxn = bitdb.TxnBegin();
+        DbTxn* ptxn = env->TxnBegin();
         if (!ptxn)
             return false;
         activeTxn = ptxn;