Skip to content

Commit

Permalink
Create walletdir if datadir doesn't exist and correct tests
Browse files Browse the repository at this point in the history 
Adaptation of btc@8263f6a5ac3f3af102a2819b7e179b00db7e0437

And cleanups for walletdir PR
Adaptation of btc@b67342906ced2353d378f4369c8d8a979d525fee
  • Loading branch information
@furszy
furszy committed Jul 21, 2021
1 parent 03db5c8 commit dcb43ea
Show file tree
Hide file tree
Showing 12 changed files with 48 additions and 46 deletions.
2 changes: 1 addition & 1 deletion doc/release-notes.md
View file
Expand Up @@ -304,7 +304,7 @@ Custom wallet directories
The ability to specify a directory other than the default data directory in which to store
wallets has been added. An existing directory can be specified using the `-walletdir=<dir>`
argument. Wallets loaded via `-wallet` arguments must be in this wallet directory. Care should be taken
when choosing a wallet directory location, as if it becomes unavailable during operation,
when choosing a wallet directory location, as if it becomes unavailable during operation,
funds may be lost.

Default wallet directory change
Expand Down
2 changes: 0 additions & 2 deletions src/init.cpp
View file
Expand Up @@ -64,7 +64,6 @@
#include "wallet/init.h"
#include "wallet/wallet.h"
#include "wallet/rpcwallet.h"
#include "wallet/walletutil.h"
#endif

#include <atomic>
Expand Down Expand Up @@ -1269,7 +1268,6 @@ bool AppInitMain()
LogPrintf("Startup time: %s\n", FormatISO8601DateTime(GetTime()));
LogPrintf("Default data directory %s\n", GetDefaultDataDir().string());
LogPrintf("Using data directory %s\n", GetDataDir().string());
LogPrintf("Using wallet directory %s\n", GetWalletDir().string());
LogPrintf("Using config file %s\n", GetConfigFile(gArgs.GetArg("-conf", PIVX_CONF_FILENAME)).string());
LogPrintf("Using at most %i connections (%i file descriptors available)\n", nMaxConnections, nFD);
std::ostringstream strErrors;
Expand Down
8 changes: 0 additions & 8 deletions src/pivxd.cpp
View file
Expand Up @@ -13,10 +13,6 @@
#include "noui.h"
#include "util/system.h"

#ifdef ENABLE_WALLET
#include "wallet/walletutil.h"
#endif

#include <stdio.h>

/* Introduction text for doxygen: */
Expand Down Expand Up @@ -84,10 +80,6 @@ bool AppInit(int argc, char* argv[])
fprintf(stderr, "Error: Specified data directory \"%s\" does not exist.\n", gArgs.GetArg("-datadir", "").c_str());
return false;
}
if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) {
fprintf(stderr, "Error: Specified wallet directory \"%s\" does not exist.\n", gArgs.GetArg("-walletdir", "").c_str());
return false;
}
try {
gArgs.ReadConfigFile(gArgs.GetArg("-conf", PIVX_CONF_FILENAME));
} catch (const std::exception& e) {
Expand Down
5 changes: 4 additions & 1 deletion src/qt/intro.cpp
View file
Expand Up @@ -193,7 +193,10 @@ bool Intro::pickDataDirectory()
}
dataDir = intro.getDataDirectory();
try {
TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir));
if (TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir))) {
// If a new data directory has been created, make wallets subdirectory too
TryCreateDirectories(GUIUtil::qstringToBoostPath(dataDir) / "wallets");
}
break;
} catch (const fs::filesystem_error& e) {
QMessageBox::critical(0, tr("PIVX Core"),
Expand Down
5 changes: 0 additions & 5 deletions src/qt/pivx.cpp
View file
Expand Up @@ -608,11 +608,6 @@ int main(int argc, char* argv[])
QObject::tr("Error: Specified data directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-datadir", ""))));
return 1;
}
if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) {
QMessageBox::critical(0, QObject::tr(PACKAGE_NAME),
QObject::tr("Error: Specified wallet directory \"%1\" does not exist.").arg(QString::fromStdString(gArgs.GetArg("-walletdir", ""))));
return EXIT_FAILURE;
}
try {
gArgs.ReadConfigFile(gArgs.GetArg("-conf", PIVX_CONF_FILENAME));
} catch (const std::exception& e) {
Expand Down
5 changes: 4 additions & 1 deletion src/util/system.cpp
View file
Expand Up @@ -697,7 +697,10 @@ const fs::path& GetDataDir(bool fNetSpecific)
if (fNetSpecific)
path /= BaseParams().DataDir();

fs::create_directories(path);
if (fs::create_directories(path)) {
// This is the first run, create wallets subdirectory too
fs::create_directories(path / "wallets");
}

return path;
}
Expand Down
9 changes: 9 additions & 0 deletions src/wallet/init.cpp
View file
Expand Up @@ -148,6 +148,15 @@ bool WalletVerify()
return true;
}

if (gArgs.IsArgSet("-walletdir") && !fs::is_directory(GetWalletDir())) {
if (fs::exists(fs::system_complete(gArgs.GetArg("-walletdir", "")))) {
return UIError(strprintf(_("Specified -walletdir \"%s\" is not a directory"), gArgs.GetArg("-walletdir", "").c_str()));
}
return UIError(strprintf(_("Specified -walletdir \"%s\" does not exist"), gArgs.GetArg("-walletdir", "").c_str()));
}

LogPrintf("Using wallet directory %s\n", GetWalletDir().string());

uiInterface.InitMessage(_("Verifying wallet(s)..."));

// Keep track of each wallet absolute path to detect duplicates.
Expand Down
3 changes: 2 additions & 1 deletion src/wallet/walletdb.cpp
View file
Expand Up @@ -18,6 +18,7 @@
#include "util/system.h"
#include "utiltime.h"
#include "wallet/wallet.h"
#include "wallet/walletutil.h"

#include <atomic>
#include <string>
Expand Down Expand Up @@ -974,7 +975,7 @@ bool BackupWallet(const CWallet& wallet, const fs::path& strDest)

// Copy wallet file
fs::path pathDest(strDest);
fs::path pathSrc = GetDataDir() / strFile;
fs::path pathSrc = GetWalletDir() / strFile;
if (is_directory(pathDest)) {
if(!exists(pathDest)) create_directory(pathDest);
pathDest /= strFile;
Expand Down
20 changes: 10 additions & 10 deletions test/functional/wallet_backup.py
View file
Expand Up @@ -98,9 +98,9 @@ def stop_three(self):
self.stop_node(2)

def erase_three(self):
os.remove(self.options.tmpdir + "/node0/regtest/wallet.dat")
os.remove(self.options.tmpdir + "/node1/regtest/wallet.dat")
os.remove(self.options.tmpdir + "/node2/regtest/wallet.dat")
os.remove(self.options.tmpdir + "/node0/regtest/wallets/wallet.dat")
os.remove(self.options.tmpdir + "/node1/regtest/wallets/wallet.dat")
os.remove(self.options.tmpdir + "/node2/regtest/wallets/wallet.dat")

def run_test(self):
self.log.info("Generating initial blockchain")
Expand Down Expand Up @@ -162,9 +162,9 @@ def run_test(self):
shutil.rmtree(self.options.tmpdir + "/node2/regtest/chainstate")

# Restore wallets from backup
shutil.copyfile(tmpdir + "/node0/wallet.bak", tmpdir + "/node0/regtest/wallet.dat")
shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallet.dat")
shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallet.dat")
shutil.copyfile(tmpdir + "/node0/wallet.bak", tmpdir + "/node0/regtest/wallets/wallet.dat")
shutil.copyfile(tmpdir + "/node1/wallet.bak", tmpdir + "/node1/regtest/wallets/wallet.dat")
shutil.copyfile(tmpdir + "/node2/wallet.bak", tmpdir + "/node2/regtest/wallets/wallet.dat")

self.log.info("Re-starting nodes")
self.start_three()
Expand Down Expand Up @@ -200,10 +200,10 @@ def run_test(self):

# Backup to source wallet file must fail
sourcePaths = [
tmpdir + "/node0/regtest/wallet.dat",
tmpdir + "/node0/./regtest/wallet.dat",
tmpdir + "/node0/regtest/",
tmpdir + "/node0/regtest"]
tmpdir + "/node0/regtest/wallets/wallet.dat",
tmpdir + "/node0/./regtest/wallets/wallet.dat",
tmpdir + "/node0/regtest/wallets/",
tmpdir + "/node0/regtest/wallets"]

for sourcePath in sourcePaths:
assert_raises_rpc_error(-4, "backup failed", self.nodes[0].backupwallet, sourcePath)
Expand Down
6 changes: 3 additions & 3 deletions test/functional/wallet_hd.py
View file
Expand Up @@ -131,7 +131,7 @@ def run_test(self):
# otherwise node1 would auto-recover all funds in flag the keypool keys as used
shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "blocks"))
shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "chainstate"))
shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallet.dat"))
shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat"))
self.start_node(1)

# Assert that derivation is deterministic
Expand All @@ -154,7 +154,7 @@ def run_test(self):
self.stop_node(1)
shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "blocks"))
shutil.rmtree(os.path.join(self.nodes[1].datadir, "regtest", "chainstate"))
shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallet.dat"))
shutil.copyfile(os.path.join(self.nodes[1].datadir, "hd.bak"), os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat"))
self.start_and_connect_node1()
# Wallet automatically scans blocks older than key on startup (but shielded addresses need to be regenerated)
assert_equal(self.nodes[1].getbalance(), NUM_HD_ADDS + 1)
Expand Down Expand Up @@ -213,7 +213,7 @@ def run_test(self):

# Delete wallet and recover from first seed
self.stop_node(1)
os.remove(os.path.join(self.nodes[1].datadir, "regtest", "wallet.dat"))
os.remove(os.path.join(self.nodes[1].datadir, "regtest", "wallets", "wallet.dat"))
self.start_node(1)
# Regenerate old shield addresses
self.log.info("Restore from seed ...")
Expand Down
4 changes: 2 additions & 2 deletions test/functional/wallet_keypool_topup.py
View file
Expand Up @@ -36,7 +36,7 @@ def run_test(self):

self.stop_node(1)

shutil.copyfile(self.tmpdir + "/node1/regtest/wallet.dat", self.tmpdir + "/wallet.bak")
shutil.copyfile(self.tmpdir + "/node1/regtest/wallets/wallet.dat", self.tmpdir + "/wallet.bak")
self.start_node(1, self.extra_args[1])
connect_nodes(self.nodes[0], 1)

Expand All @@ -59,7 +59,7 @@ def run_test(self):

self.stop_node(1)

shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallet.dat")
shutil.copyfile(self.tmpdir + "/wallet.bak", self.tmpdir + "/node1/regtest/wallets/wallet.dat")

self.log.info("Verify keypool is restored and balance is correct")

Expand Down
25 changes: 13 additions & 12 deletions test/functional/wallet_multiwallet.py
View file
Expand Up @@ -25,40 +25,41 @@ def set_test_params(self):
def run_test(self):
node = self.nodes[0]

# !TODO: backport bitcoin#12220
#data_dir = lambda *p: os.path.join(node.datadir, 'regtest', *p)
#wallet_dir = lambda *p: data_dir('wallets', *p)
data_dir = lambda *p: os.path.join(node.datadir, 'regtest', *p)
wallet_dir = lambda *p: data_dir('wallets', *p)
wallet = lambda name: node.get_wallet_rpc(name)

assert_equal(set(node.listwallets()), {"w1", "w2", "w3", "w"})

self.stop_nodes()

# !TODO: backport bitcoin#12220
#self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist')
self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" does not exist')
#self.assert_start_raises_init_error(0, ['-walletdir=wallets'], 'Error: Specified -walletdir "wallets" is a relative path', cwd=data_dir())
#self.assert_start_raises_init_error(0, ['-walletdir=debug.log'], 'Error: Specified -walletdir "debug.log" is not a directory', cwd=data_dir())

# should not initialize if there are duplicate wallets
self.assert_start_raises_init_error(0, ['-wallet=w1', '-wallet=w1'], 'Error loading wallet w1. Duplicate -wallet filename specified.')

# should not initialize if wallet file is a directory
os.mkdir(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w11'))
#os.mkdir(wallet_dir('w11'))
os.mkdir(wallet_dir('w11'))
self.assert_start_raises_init_error(0, ['-wallet=w11'], 'Error loading wallet w11. -wallet filename must be a regular file.')

#should not initialize if one wallet is a copy of another
#shutil.copyfile(wallet_dir('w2'), wallet_dir('w22')) # !TODO: backport bitcoin#11970
shutil.copyfile(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w2'),
os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w22'))
shutil.copyfile(wallet_dir('w2'), wallet_dir('w22')) # !TODO: backport bitcoin#11970
self.assert_start_raises_init_error(0, ['-wallet=w2', '-wallet=w22'], 'duplicates fileid')

# should not initialize if wallet file is a symlink
os.symlink(os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w1'),
os.path.join(self.options.tmpdir, 'node0', 'regtest', 'w12'))
# os.symlink(wallet_dir('w1'), wallet_dir('w12')) # !TODO: backport bitcoin#11970
os.symlink(wallet_dir('w1'), wallet_dir('w12')) # !TODO: backport bitcoin#11970
self.assert_start_raises_init_error(0, ['-wallet=w12'], 'Error loading wallet w12. -wallet filename must be a regular file.')

# should not initialize if the specified walletdir does not exist
self.assert_start_raises_init_error(0, ['-walletdir=bad'], 'Error: Specified -walletdir "bad" does not exist')
# should not initialize if the specified walletdir is not a directory
not_a_dir = wallet_dir('notadir')
open(not_a_dir, 'a', encoding="utf8").close()
self.assert_start_raises_init_error(0, ['-walletdir='+not_a_dir], 'Error: Specified -walletdir "' + not_a_dir + '" is not a directory')

self.log.info("Do not allow -zapwallettxes with multiwallet")
self.assert_start_raises_init_error(0, ['-zapwallettxes', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file")
self.assert_start_raises_init_error(0, ['-zapwallettxes=1', '-wallet=w1', '-wallet=w2'], "Error: -zapwallettxes is only allowed with a single wallet file")
Expand Down

0 comments on commit dcb43ea

Please sign in to comment.