[wallet] Clarify wallet initialization / destruction interface #10767

Open
wants to merge 12 commits into
from
View
@@ -165,6 +165,7 @@ BITCOIN_CORE_H = \
wallet/rpcwallet.h \
wallet/wallet.h \
wallet/walletdb.h \
+ wallet/walletinit.h \
warnings.h \
zmq/zmqabstractnotifier.h \
zmq/zmqconfig.h\
@@ -243,6 +244,7 @@ libbitcoin_wallet_a_SOURCES = \
wallet/rpcwallet.cpp \
wallet/wallet.cpp \
wallet/walletdb.cpp \
+ wallet/walletinit.cpp \
$(BITCOIN_CORE_H)
# crypto primitives library
View
@@ -43,7 +43,7 @@
#include "utilmoneystr.h"
#include "validationinterface.h"
#ifdef ENABLE_WALLET
-#include "wallet/wallet.h"
+#include "wallet/walletinit.h"
#endif
#include "warnings.h"
#include <stdint.h>
@@ -187,11 +187,6 @@ void Shutdown()
StopREST();
StopRPC();
StopHTTPServer();
-#ifdef ENABLE_WALLET
@promag

promag Jul 7, 2017

Contributor

Nothing replacing this code?

@jnewbery

jnewbery Jul 7, 2017

Member

Correct - I don't believe the early wallet flush is necessary (although I may be wrong). See PR description:

The most important one is that Flush() no longer gets called twice on shutdown. I think this is fine, and all the tests pass, but this probably requires some careful consideration from people who are more familiar with the wallet code.

- for (CWalletRef pwallet : vpwallets) {
- pwallet->Flush(false);
- }
-#endif
MapPort(false);
UnregisterValidationInterface(peerLogic.get());
peerLogic.reset();
@@ -230,9 +225,7 @@ void Shutdown()
pblocktree = NULL;
}
#ifdef ENABLE_WALLET
@promag

promag Jul 7, 2017

Contributor

What about moving these #ifdef ENABLE_WALLET to walletinit.cpp?

@jnewbery

jnewbery Jul 7, 2017

Member

Because walletinit.cpp is in libbitcoin_wallet, so is not linked if ENABLE_WALLET isn't defined. #10762 will remove the libbitcoin_server -> libbitcoin_wallet dependencies entirely from init.cpp.

- for (CWalletRef pwallet : vpwallets) {
- pwallet->Flush(true);
- }
+ ShutdownWallets();
#endif
#if ENABLE_ZMQ
@@ -252,10 +245,7 @@ void Shutdown()
#endif
UnregisterAllValidationInterfaces();
#ifdef ENABLE_WALLET
- for (CWalletRef pwallet : vpwallets) {
- delete pwallet;
- }
- vpwallets.clear();
+ DetachWallets();
#endif
globalVerifyHandle.reset();
ECC_Stop();
@@ -404,7 +394,7 @@ std::string HelpMessage(HelpMessageMode mode)
strUsage += HelpMessageOpt("-maxuploadtarget=<n>", strprintf(_("Tries to keep outbound traffic under the given target (in MiB per 24h), 0 = no limit (default: %d)"), DEFAULT_MAX_UPLOAD_TARGET));
#ifdef ENABLE_WALLET
- strUsage += CWallet::GetWalletHelpString(showDebug);
+ strUsage += GetWalletHelpString(showDebug);
#endif
#if ENABLE_ZMQ
@@ -1007,7 +997,7 @@ bool AppInitParameterInteraction()
RegisterAllCoreRPCCommands(tableRPC);
#ifdef ENABLE_WALLET
- RegisterWalletRPCCommands(tableRPC);
+ RegisterWalletRPC(tableRPC);
#endif
nConnectTimeout = GetArg("-timeout", DEFAULT_CONNECT_TIMEOUT);
@@ -1052,8 +1042,7 @@ bool AppInitParameterInteraction()
nBytesPerSigOp = GetArg("-bytespersigop", nBytesPerSigOp);
#ifdef ENABLE_WALLET
- if (!CWallet::ParameterInteraction())
- return false;
+ if (!WalletParameterInteraction()) return false;
#endif
fIsBareMultisigStd = GetBoolArg("-permitbaremultisig", DEFAULT_PERMIT_BAREMULTISIG);
@@ -1219,8 +1208,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
// ********************************************************* Step 5: verify wallet database integrity
#ifdef ENABLE_WALLET
- if (!CWallet::Verify())
- return false;
+ if (!VerifyWallets()) return false;
#endif
// ********************************************************* Step 6: network initialization
// Note that we absolutely cannot open any actual connections
@@ -1507,8 +1495,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
// ********************************************************* Step 8: load wallet
#ifdef ENABLE_WALLET
- if (!CWallet::InitLoadWallet())
- return false;
+ if (!AttachWallets()) return false;
#else
LogPrintf("No wallet support compiled in!\n");
#endif
@@ -1638,9 +1625,7 @@ bool AppInitMain(boost::thread_group& threadGroup, CScheduler& scheduler)
uiInterface.InitMessage(_("Done loading"));
#ifdef ENABLE_WALLET
- for (CWalletRef pwallet : vpwallets) {
- pwallet->postInitProcess(scheduler);
- }
+ WalletCompleteStartup(scheduler);
#endif
return !fRequestShutdown;
@@ -184,7 +184,7 @@ void TestSendCoins()
BumpFee(transactionView, txid2, false /* expect disabled */, {} /* expected error */, false /* cancel */);
BumpFee(transactionView, txid2, true /* expect disabled */, "already bumped" /* expected error */, false /* cancel */);
- bitdb.Flush(true);
+ bitdb.Flush("wallet_test.dat");
bitdb.Reset();
}
View
@@ -224,42 +224,9 @@ bool CDB::Recover(const std::string& filename, void *callbackDataIn, bool (*reco
return fSuccess;
}
-bool CDB::VerifyEnvironment(const std::string& walletFile, const fs::path& dataDir, std::string& errorStr)
-{
- LogPrintf("Using BerkeleyDB version %s\n", DbEnv::version(0, 0, 0));
- LogPrintf("Using wallet %s\n", walletFile);
-
- // Wallet file must be a plain filename without a directory
- if (walletFile != fs::basename(walletFile) + fs::extension(walletFile))
- {
- errorStr = strprintf(_("Wallet %s resides outside data directory %s"), walletFile, dataDir.string());
- return false;
- }
-
- if (!bitdb.Open(dataDir))
- {
- // try moving the database env out of the way
- fs::path pathDatabase = dataDir / "database";
- fs::path pathDatabaseBak = dataDir / strprintf("database.%d.bak", GetTime());
- try {
- fs::rename(pathDatabase, pathDatabaseBak);
- LogPrintf("Moved old %s to %s. Retrying.\n", pathDatabase.string(), pathDatabaseBak.string());
- } catch (const fs::filesystem_error&) {
- // failure is ok (well, not really, but it's not worse than what we started with)
- }
-
- // try again
- if (!bitdb.Open(dataDir)) {
- // if it still fails, it probably means we can't even create the database env
- errorStr = strprintf(_("Error initializing wallet database environment %s!"), GetDataDir());
- return false;
- }
- }
- return true;
-}
-
bool CDB::VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc)
{
+ LogPrintf("Using wallet %s\n", walletFile);
if (fs::exists(dataDir / walletFile))
{
std::string backup_filename;
@@ -557,22 +524,32 @@ bool CDB::Rewrite(CWalletDBWrapper& dbw, const char* pszSkip)
return false;
}
+void CDBEnv::Shutdown()
+{
+ char** listp;
+ if (mapFileUseCount.empty()) {
+ dbenv->log_archive(&listp, DB_ARCH_REMOVE);
+ Close();
+ if (!fMockDb)
+ fs::remove_all(fs::path(strPath) / "database");
+ }
+}
-void CDBEnv::Flush(bool fShutdown)
+void CDBEnv::Flush(std::string strFile)
{
int64_t nStart = GetTimeMillis();
// Flush log data to the actual data file on all files that are not in use
- LogPrint(BCLog::DB, "CDBEnv::Flush: Flush(%s)%s\n", fShutdown ? "true" : "false", fDbEnvInit ? "" : " database not started");
+ LogPrint(BCLog::DB, "CDBEnv::Flush: Flush%s\n", fDbEnvInit ? "" : " database not started");
if (!fDbEnvInit)
return;
{
LOCK(cs_db);
std::map<std::string, int>::iterator mi = mapFileUseCount.begin();
while (mi != mapFileUseCount.end()) {
- std::string strFile = (*mi).first;
+ std::string foundStrFile = (*mi).first;
int nRefCount = (*mi).second;
- LogPrint(BCLog::DB, "CDBEnv::Flush: Flushing %s (refcount = %d)...\n", strFile, nRefCount);
- if (nRefCount == 0) {
+ if (foundStrFile == strFile && nRefCount == 0) {
+ LogPrint(BCLog::DB, "CDBEnv::Flush: Flushing %s (refcount = %d)...\n", strFile, nRefCount);
// Move log data to the dat file
CloseDb(strFile);
LogPrint(BCLog::DB, "CDBEnv::Flush: %s checkpoint\n", strFile);
@@ -585,16 +562,7 @@ void CDBEnv::Flush(bool fShutdown)
} else
mi++;
}
- LogPrint(BCLog::DB, "CDBEnv::Flush: Flush(%s)%s took %15dms\n", fShutdown ? "true" : "false", fDbEnvInit ? "" : " database not started", GetTimeMillis() - nStart);
- if (fShutdown) {
- char** listp;
- if (mapFileUseCount.empty()) {
- dbenv->log_archive(&listp, DB_ARCH_REMOVE);
- Close();
- if (!fMockDb)
- fs::remove_all(fs::path(strPath) / "database");
- }
- }
+ LogPrint(BCLog::DB, "CDBEnv::Flush: Flush took %15dms\n", GetTimeMillis() - nStart);
}
}
@@ -683,9 +651,9 @@ bool CWalletDBWrapper::Backup(const std::string& strDest)
return false;
}
-void CWalletDBWrapper::Flush(bool shutdown)
+void CWalletDBWrapper::Flush()
{
if (!IsDummy()) {
- env->Flush(shutdown);
+ env->Flush(strFile);
}
}
View
@@ -70,7 +70,8 @@ class CDBEnv
bool Open(const fs::path& path);
void Close();
- void Flush(bool fShutdown);
+ void Flush(std::string strFile);
+ void Shutdown();
void CheckpointLSN(const std::string& strFile);
void CloseDb(const std::string& strFile);
@@ -119,7 +120,7 @@ class CWalletDBWrapper
/** Make sure all changes are flushed to disk.
*/
- void Flush(bool shutdown);
+ void Flush();
void IncrementUpdateCounter();
@@ -163,8 +164,6 @@ class CDB
/* 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& dataDir, std::string& errorStr);
/* verifies the database file */
static bool VerifyDatabaseFile(const std::string& walletFile, const fs::path& dataDir, std::string& warningStr, std::string& errorStr, CDBEnv::recoverFunc_type recoverFunc);
View
@@ -3038,9 +3038,8 @@ static const CRPCCommand commands[] =
void RegisterWalletRPCCommands(CRPCTable &t)
{
- if (GetBoolArg("-disablewallet", false))
- return;
-
- for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++)
+ for (unsigned int vcidx = 0; vcidx < ARRAYLEN(commands); vcidx++) {
t.appendCommand(commands[vcidx].name, &commands[vcidx]);
+ }
}
+
View
@@ -7,6 +7,7 @@
class CRPCTable;
class JSONRPCRequest;
+class CWallet;
void RegisterWalletRPCCommands(CRPCTable &t);
@@ -30,6 +30,7 @@ WalletTestingSetup::~WalletTestingSetup()
delete pwalletMain;
pwalletMain = NULL;
- bitdb.Flush(true);
+ bitdb.Flush("wallet_test.dat");
+ bitdb.Shutdown();
bitdb.Reset();
}
@@ -606,7 +606,8 @@ class ListCoinsTestingSetup : public TestChain100Setup
~ListCoinsTestingSetup()
{
wallet.reset();
- ::bitdb.Flush(true);
+ ::bitdb.Flush("wallet_test.dat");
+ ::bitdb.Shutdown();
::bitdb.Reset();
}
Oops, something went wrong.