Skip to content

Commit 7bf6dfb

Browse files
ryanofskyryanofsky
committed
wallet: Remove path checking code from bitcoin-wallet tool
This commit does not change behavior except for error messages which now include more complete information.
1 parent 77d5bb7 commit 7bf6dfb

File tree

6 files changed

+29
-39
lines changed

6 files changed

+29
-39
lines changed

‎src/wallet/salvage.cpp‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,13 @@ static bool KeyFilter(const std::string& type)
2323

2424
bool RecoverDatabaseFile(const fs::path& file_path, bilingual_str& error, std::vector<bilingual_str>& warnings)
2525
{
26+
DatabaseOptions options;
27+
DatabaseStatus status;
28+
options.require_existing = true;
29+
options.verify = false;
30+
std::unique_ptr<WalletDatabase> database = MakeDatabase(file_path, options, status, error);
31+
if (!database) return false;
32+
2633
std::string filename;
2734
std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(file_path, filename);
2835

‎src/wallet/walletdb.cpp‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1032,13 +1032,6 @@ std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const Databas
10321032
return MakeBerkeleyDatabase(path, options, status, error);
10331033
}
10341034

1035-
/** Return object for accessing database at specified path. */
1036-
std::unique_ptr<WalletDatabase> CreateWalletDatabase(const fs::path& path)
1037-
{
1038-
std::string filename;
1039-
return MakeUnique<BerkeleyDatabase>(GetWalletEnv(path, filename), std::move(filename));
1040-
}
1041-
10421035
/** Return object for accessing dummy database with no read/write capabilities. */
10431036
std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase()
10441037
{

‎src/wallet/walletdb.h‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -285,9 +285,6 @@ using KeyFilterFn = std::function<bool(const std::string&)>;
285285
//! Unserialize a given Key-Value pair and load it into the wallet
286286
bool ReadKeyValue(CWallet* pwallet, CDataStream& ssKey, CDataStream& ssValue, std::string& strType, std::string& strErr, const KeyFilterFn& filter_fn = nullptr);
287287

288-
/** Return object for accessing database at specified path. */
289-
std::unique_ptr<WalletDatabase> CreateWalletDatabase(const fs::path& path);
290-
291288
/** Return object for accessing dummy database with no read/write capabilities. */
292289
std::unique_ptr<WalletDatabase> CreateDummyWalletDatabase();
293290

‎src/wallet/wallettool.cpp‎

Lines changed: 18 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -21,21 +21,9 @@ static void WalletToolReleaseWallet(CWallet* wallet)
2121
delete wallet;
2222
}
2323

24-
static std::shared_ptr<CWallet> CreateWallet(const std::string& name, const fs::path& path)
24+
static void WalletCreate(CWallet* wallet_instance)
2525
{
26-
if (fs::exists(path)) {
27-
tfm::format(std::cerr, "Error: File exists already\n");
28-
return nullptr;
29-
}
30-
// dummy chain interface
31-
std::shared_ptr<CWallet> wallet_instance(new CWallet(nullptr /* chain */, name, CreateWalletDatabase(path)), WalletToolReleaseWallet);
3226
LOCK(wallet_instance->cs_wallet);
33-
bool first_run = true;
34-
DBErrors load_wallet_ret = wallet_instance->LoadWallet(first_run);
35-
if (load_wallet_ret != DBErrors::LOAD_OK) {
36-
tfm::format(std::cerr, "Error creating %s", name);
37-
return nullptr;
38-
}
3927

4028
wallet_instance->SetMinVersion(FEATURE_HD_SPLIT);
4129

@@ -46,18 +34,26 @@ static std::shared_ptr<CWallet> CreateWallet(const std::string& name, const fs::
4634

4735
tfm::format(std::cout, "Topping up keypool...\n");
4836
wallet_instance->TopUpKeyPool();
49-
return wallet_instance;
5037
}
5138

52-
static std::shared_ptr<CWallet> LoadWallet(const std::string& name, const fs::path& path)
39+
static std::shared_ptr<CWallet> MakeWallet(const std::string& name, const fs::path& path, bool create)
5340
{
54-
if (!fs::exists(path)) {
55-
tfm::format(std::cerr, "Error: Wallet files does not exist\n");
41+
DatabaseOptions options;
42+
DatabaseStatus status;
43+
if (create) {
44+
options.require_create = true;
45+
} else {
46+
options.require_existing = true;
47+
}
48+
bilingual_str error;
49+
std::unique_ptr<WalletDatabase> database = MakeDatabase(path, options, status, error);
50+
if (!database) {
51+
tfm::format(std::cerr, "%s\n", error.original);
5652
return nullptr;
5753
}
5854

5955
// dummy chain interface
60-
std::shared_ptr<CWallet> wallet_instance(new CWallet(nullptr /* chain */, name, CreateWalletDatabase(path)), WalletToolReleaseWallet);
56+
std::shared_ptr<CWallet> wallet_instance{new CWallet(nullptr /* chain */, name, std::move(database)), WalletToolReleaseWallet};
6157
DBErrors load_wallet_ret;
6258
try {
6359
bool first_run;
@@ -89,6 +85,8 @@ static std::shared_ptr<CWallet> LoadWallet(const std::string& name, const fs::pa
8985
}
9086
}
9187

88+
if (create) WalletCreate(wallet_instance.get());
89+
9290
return wallet_instance;
9391
}
9492

@@ -109,19 +107,14 @@ bool ExecuteWalletToolFunc(const std::string& command, const std::string& name)
109107
fs::path path = fs::absolute(name, GetWalletDir());
110108

111109
if (command == "create") {
112-
std::shared_ptr<CWallet> wallet_instance = CreateWallet(name, path);
110+
std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, /* create= */ true);
113111
if (wallet_instance) {
114112
WalletShowInfo(wallet_instance.get());
115113
wallet_instance->Close();
116114
}
117115
} else if (command == "info" || command == "salvage") {
118-
if (!fs::exists(path)) {
119-
tfm::format(std::cerr, "Error: no wallet file at %s\n", name);
120-
return false;
121-
}
122-
123116
if (command == "info") {
124-
std::shared_ptr<CWallet> wallet_instance = LoadWallet(name, path);
117+
std::shared_ptr<CWallet> wallet_instance = MakeWallet(name, path, /* create= */ false);
125118
if (!wallet_instance) return false;
126119
WalletShowInfo(wallet_instance.get());
127120
wallet_instance->Close();

‎src/wallet/wallettool.h‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,6 @@
99

1010
namespace WalletTool {
1111

12-
std::shared_ptr<CWallet> CreateWallet(const std::string& name, const fs::path& path);
13-
std::shared_ptr<CWallet> LoadWallet(const std::string& name, const fs::path& path);
1412
void WalletShowInfo(CWallet* wallet_instance);
1513
bool ExecuteWalletToolFunc(const std::string& command, const std::string& file);
1614

‎test/functional/tool_wallet.py‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,14 @@ def test_invalid_tool_commands_and_args(self):
7070
self.assert_raises_tool_error('Invalid command: help', 'help')
7171
self.assert_raises_tool_error('Error: two methods provided (info and create). Only one method should be provided.', 'info', 'create')
7272
self.assert_raises_tool_error('Error parsing command line arguments: Invalid parameter -foo', '-foo')
73+
locked_dir = os.path.join(self.options.tmpdir, "node0", "regtest", "wallets")
7374
self.assert_raises_tool_error(
74-
'Error loading wallet.dat. Is wallet being used by another process?',
75+
'Error initializing wallet database environment "{}"!'.format(locked_dir),
7576
'-wallet=wallet.dat',
7677
'info',
7778
)
78-
self.assert_raises_tool_error('Error: no wallet file at nonexistent.dat', '-wallet=nonexistent.dat', 'info')
79+
path = os.path.join(self.options.tmpdir, "node0", "regtest", "wallets", "nonexistent.dat")
80+
self.assert_raises_tool_error("Failed to load database path '{}'. Path does not exist.".format(path), '-wallet=nonexistent.dat', 'info')
7981

8082
def test_tool_wallet_info(self):
8183
# Stop the node to close the wallet to call the info command.

0 commit comments

Comments
 (0)