Skip to content

Commit 8994145

Browse files
ryanofskyryanofsky
committed
Remove wallet.dat path handling from wallet.cpp, rpcwallet.cpp
Get rid of confusing and unnecessary path handling and verification in wallet application code and move it down to database layer. There is no change in behavior except for some error messages. Motivation for this change is making code more understandable, but also to prepare for adding SQLite support in #19077 so loading SQLite paths can also be handled at the database layer and wallet loading and creation code does not need to become more complicated than it already is.
1 parent 1fa4c87 commit 8994145

File tree

19 files changed

+212
-189
lines changed

19 files changed

+212
-189
lines changed

‎src/dummywallet.cpp‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,8 @@
77
#include <support/allocators/secure.h>
88

99
class CWallet;
10-
enum class WalletCreationStatus;
10+
enum class DatabaseStatus;
11+
struct DatabaseOptions;
1112
struct bilingual_str;
1213

1314
namespace interfaces {
@@ -73,12 +74,12 @@ std::vector<std::shared_ptr<CWallet>> GetWallets()
7374
throw std::logic_error("Wallet function called in non-wallet build.");
7475
}
7576

76-
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings)
77+
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings)
7778
{
7879
throw std::logic_error("Wallet function called in non-wallet build.");
7980
}
8081

81-
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result)
82+
std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings)
8283
{
8384
throw std::logic_error("Wallet function called in non-wallet build.");
8485
}

‎src/interfaces/node.cpp‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
#include <util/system.h>
3232
#include <util/translation.h>
3333
#include <validation.h>
34+
#include <wallet/db.h>
3435
#include <warnings.h>
3536

3637
#if defined(HAVE_CONFIG_H)
@@ -45,8 +46,8 @@ class CWallet;
4546
fs::path GetWalletDir();
4647
std::vector<fs::path> ListWalletDir();
4748
std::vector<std::shared_ptr<CWallet>> GetWallets();
48-
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings);
49-
WalletCreationStatus CreateWallet(interfaces::Chain& chain, const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, std::shared_ptr<CWallet>& result);
49+
std::shared_ptr<CWallet> LoadWallet(interfaces::Chain& chain, const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings);
50+
std::shared_ptr<CWallet> CreateWallet(interfaces::Chain& chain, const std::string& name, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error, std::vector<bilingual_str>& warnings);
5051
std::unique_ptr<interfaces::Handler> HandleLoadWallet(interfaces::Node::LoadWalletFn load_wallet);
5152

5253
namespace interfaces {
@@ -277,13 +278,19 @@ class NodeImpl : public Node
277278
}
278279
std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override
279280
{
280-
return MakeWallet(LoadWallet(*m_context.chain, name, error, warnings));
281+
DatabaseOptions options;
282+
DatabaseStatus status;
283+
options.require_existing = true;
284+
return MakeWallet(LoadWallet(*m_context.chain, name, options, status, error, warnings));
281285
}
282-
std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) override
286+
std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override
283287
{
284-
std::shared_ptr<CWallet> wallet;
285-
status = CreateWallet(*m_context.chain, passphrase, wallet_creation_flags, name, error, warnings, wallet);
286-
return MakeWallet(wallet);
288+
DatabaseOptions options;
289+
DatabaseStatus status;
290+
options.require_create = true;
291+
options.create_flags = wallet_creation_flags;
292+
options.create_passphrase = passphrase;
293+
return MakeWallet(CreateWallet(*m_context.chain, name, options, status, error, warnings));
287294
}
288295
std::unique_ptr<Handler> handleInitMessage(InitMessageFn fn) override
289296
{

‎src/interfaces/node.h‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,6 @@ class RPCTimerInterface;
2929
class UniValue;
3030
class proxyType;
3131
enum class SynchronizationState;
32-
enum class WalletCreationStatus;
3332
struct CNodeStateStats;
3433
struct NodeContext;
3534
struct bilingual_str;
@@ -216,7 +215,7 @@ class Node
216215
virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
217216

218217
//! Create a wallet from file
219-
virtual std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings, WalletCreationStatus& status) = 0;
218+
virtual std::unique_ptr<Wallet> createWallet(const SecureString& passphrase, uint64_t wallet_creation_flags, const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
220219

221220
//! Register handler for init messages.
222221
using InitMessageFn = std::function<void(const std::string& message)>;

‎src/qt/walletcontroller.cpp‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -249,10 +249,9 @@ void CreateWalletActivity::createWallet()
249249
}
250250

251251
QTimer::singleShot(500, worker(), [this, name, flags] {
252-
WalletCreationStatus status;
253-
std::unique_ptr<interfaces::Wallet> wallet = node().createWallet(m_passphrase, flags, name, m_error_message, m_warning_message, status);
252+
std::unique_ptr<interfaces::Wallet> wallet = node().createWallet(m_passphrase, flags, name, m_error_message, m_warning_message);
254253

255-
if (status == WalletCreationStatus::SUCCESS) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
254+
if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
256255

257256
QTimer::singleShot(500, this, &CreateWalletActivity::finish);
258257
});

‎src/wallet/bdb.cpp‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -856,3 +856,51 @@ std::unique_ptr<DatabaseBatch> BerkeleyDatabase::MakeBatch(const char* mode, boo
856856
{
857857
return MakeUnique<BerkeleyBatch>(*this, mode, flush_on_close);
858858
}
859+
860+
/** Return object for accessing database at specified path. */
861+
std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error)
862+
{
863+
status = DatabaseStatus::SUCCESS;
864+
865+
fs::path env_directory;
866+
std::string data_filename;
867+
try {
868+
SplitWalletPath(path, env_directory, data_filename);
869+
} catch (const fs::filesystem_error& e) {
870+
error = Untranslated(strprintf("Failed to access database path '%s': %s", path.string(), fsbridge::get_filesystem_error_message(e)));
871+
status = DatabaseStatus::FAILED_BAD_PATH;
872+
return nullptr;
873+
}
874+
875+
fs::path data_path = env_directory / data_filename;
876+
bool exists = fs::symlink_status(data_path).type() != fs::file_not_found;
877+
878+
if (exists && options.require_create) {
879+
error = Untranslated(strprintf("Failed to create database. Data file '%s' already exists.", data_path.string()));
880+
status = DatabaseStatus::FAILED_ALREADY_EXISTS;
881+
return nullptr;
882+
}
883+
884+
if (!exists && options.require_existing) {
885+
error = Untranslated(strprintf("Failed to load database. Data file '%s' does not exist.", data_path.string()));
886+
status = DatabaseStatus::FAILED_NOT_FOUND;
887+
return nullptr;
888+
}
889+
890+
LOCK(cs_db);
891+
std::shared_ptr<BerkeleyEnvironment> env = GetWalletEnv(path, data_filename);
892+
if (env->m_databases.count(data_filename)) {
893+
error = Untranslated(strprintf("Refusing to load database. Data file '%s' is already loaded.", data_path.string()));
894+
status = DatabaseStatus::FAILED_ALREADY_LOADED;
895+
return nullptr;
896+
}
897+
898+
auto db = MakeUnique<BerkeleyDatabase>(std::move(env), std::move(data_filename));
899+
900+
if (options.verify && !db->Verify(error)) {
901+
status = DatabaseStatus::FAILED_VERIFY;
902+
return nullptr;
903+
}
904+
905+
return db;
906+
}

‎src/wallet/bdb.h‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,10 @@ class BerkeleyDatabase : public WalletDatabase
150150
void ReloadDbEnv() override;
151151

152152
/** Verifies the environment and database file */
153-
bool Verify(bilingual_str& error) override;
153+
bool Verify(bilingual_str& error);
154+
155+
/** Return path to main database filename */
156+
std::string Filename() override { return (env->Directory() / strFile).string(); }
154157

155158
/**
156159
* Pointer to shared database environment.
@@ -238,4 +241,6 @@ class BerkeleyBatch : public DatabaseBatch
238241

239242
std::string BerkeleyDatabaseVersion();
240243

244+
std::unique_ptr<BerkeleyDatabase> MakeBerkeleyDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);
245+
241246
#endif // BITCOIN_WALLET_BDB_H

‎src/wallet/db.cpp‎

Lines changed: 0 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,3 @@ void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::
2323
database_filename = "wallet.dat";
2424
}
2525
}
26-
27-
fs::path WalletDataFilePath(const fs::path& wallet_path)
28-
{
29-
fs::path env_directory;
30-
std::string database_filename;
31-
SplitWalletPath(wallet_path, env_directory, database_filename);
32-
return env_directory / database_filename;
33-
}

‎src/wallet/db.h‎

Lines changed: 26 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,14 @@
99
#include <clientversion.h>
1010
#include <fs.h>
1111
#include <streams.h>
12+
#include <support/allocators/secure.h>
1213

1314
#include <atomic>
1415
#include <memory>
1516
#include <string>
1617

1718
struct bilingual_str;
1819

19-
/** Given a wallet directory path or legacy file path, return path to main data file in the wallet database. */
20-
fs::path WalletDataFilePath(const fs::path& wallet_path);
2120
void SplitWalletPath(const fs::path& wallet_path, fs::path& env_directory, std::string& database_filename);
2221

2322
/** RAII class that provides access to a WalletDatabase */
@@ -140,18 +139,40 @@ class WalletDatabase
140139

141140
virtual void ReloadDbEnv() = 0;
142141

142+
/** Return path to main database file for logs and error messages. */
143+
virtual std::string Filename() = 0;
144+
143145
std::atomic<unsigned int> nUpdateCounter;
144146
unsigned int nLastSeen;
145147
unsigned int nLastFlushed;
146148
int64_t nLastWalletUpdate;
147149

148-
/** Verifies the environment and database file */
149-
virtual bool Verify(bilingual_str& error) = 0;
150-
151150
std::string m_file_path;
152151

153152
/** Make a DatabaseBatch connected to this database */
154153
virtual std::unique_ptr<DatabaseBatch> MakeBatch(const char* mode = "r+", bool flush_on_close = true) = 0;
155154
};
156155

156+
struct DatabaseOptions {
157+
bool require_existing = false;
158+
bool require_create = false;
159+
uint64_t create_flags = 0;
160+
SecureString create_passphrase;
161+
bool verify = true;
162+
};
163+
164+
enum class DatabaseStatus {
165+
SUCCESS,
166+
FAILED_BAD_PATH,
167+
FAILED_ALREADY_LOADED,
168+
FAILED_ALREADY_EXISTS,
169+
FAILED_NOT_FOUND,
170+
FAILED_CREATE,
171+
FAILED_VERIFY,
172+
FAILED_ENCRYPT,
173+
FAILED_LOAD
174+
};
175+
176+
std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path& path, const DatabaseOptions& options, DatabaseStatus& status, bilingual_str& error);
177+
157178
#endif // BITCOIN_WALLET_DB_H

‎src/wallet/load.cpp‎

Lines changed: 10 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -49,11 +49,11 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal
4949
return false;
5050
}
5151

52+
DatabaseOptions options;
53+
DatabaseStatus status;
54+
options.verify = true;
5255
bilingual_str error_string;
53-
std::vector<bilingual_str> warnings;
54-
bool verify_success = CWallet::Verify(chain, wallet_file, error_string, warnings);
55-
if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n")));
56-
if (!verify_success) {
56+
if (!MakeWalletDatabase(wallet_file, options, status, error_string)) {
5757
chain.initError(error_string);
5858
return false;
5959
}
@@ -65,10 +65,14 @@ bool VerifyWallets(interfaces::Chain& chain, const std::vector<std::string>& wal
6565
bool LoadWallets(interfaces::Chain& chain, const std::vector<std::string>& wallet_files)
6666
{
6767
try {
68-
for (const std::string& walletFile : wallet_files) {
68+
for (const std::string& name : wallet_files) {
69+
DatabaseOptions options;
70+
DatabaseStatus status;
71+
options.verify = false; // No need to verify, assuming verified earlier in VerifyWallets()
6972
bilingual_str error;
7073
std::vector<bilingual_str> warnings;
71-
std::shared_ptr<CWallet> pwallet = CWallet::CreateWalletFromFile(chain, walletFile, error, warnings);
74+
std::unique_ptr<WalletDatabase> database = MakeWalletDatabase(name, options, status, error);
75+
std::shared_ptr<CWallet> pwallet = database ? CWallet::Create(chain, name, std::move(database), options.create_flags, error, warnings) : nullptr;
7276
if (!warnings.empty()) chain.initWarning(Join(warnings, Untranslated("\n")));
7377
if (!pwallet) {
7478
chain.initError(error);

‎src/wallet/rpcwallet.cpp‎

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -2499,22 +2499,17 @@ static UniValue loadwallet(const JSONRPCRequest& request)
24992499

25002500
WalletContext& context = EnsureWalletContext(request.context);
25012501
std::string name(request.params[0].get_str());
2502-
fs::path path(fs::absolute(name, GetWalletDir()));
2503-
2504-
if (fs::symlink_status(path).type() == fs::file_not_found) {
2505-
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Wallet " + name + " not found.");
2506-
} else if (fs::is_directory(path)) {
2507-
// The given filename is a directory. Check that there's a wallet.dat file.
2508-
fs::path wallet_dat_file = path / "wallet.dat";
2509-
if (fs::symlink_status(wallet_dat_file).type() == fs::file_not_found) {
2510-
throw JSONRPCError(RPC_WALLET_NOT_FOUND, "Directory " + name + " does not contain a wallet.dat file.");
2511-
}
2512-
}
25132502

2503+
DatabaseOptions options;
2504+
DatabaseStatus status;
2505+
options.require_existing = true;
25142506
bilingual_str error;
25152507
std::vector<bilingual_str> warnings;
2516-
std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, name, error, warnings);
2517-
if (!wallet) throw JSONRPCError(RPC_WALLET_ERROR, error.original);
2508+
std::shared_ptr<CWallet> const wallet = LoadWallet(*context.chain, name, options, status, error, warnings);
2509+
if (!wallet) {
2510+
RPCErrorCode code = status == DatabaseStatus::FAILED_NOT_FOUND ? RPC_WALLET_NOT_FOUND : RPC_WALLET_ERROR;
2511+
throw JSONRPCError(code, error.original);
2512+
}
25182513

25192514
UniValue obj(UniValue::VOBJ);
25202515
obj.pushKV("name", wallet->GetName());
@@ -2642,17 +2637,16 @@ static UniValue createwallet(const JSONRPCRequest& request)
26422637
warnings.emplace_back(Untranslated("Wallet is an experimental descriptor wallet"));
26432638
}
26442639

2640+
DatabaseOptions options;
2641+
DatabaseStatus status;
2642+
options.require_create = true;
2643+
options.create_flags = flags;
2644+
options.create_passphrase = passphrase;
26452645
bilingual_str error;
2646-
std::shared_ptr<CWallet> wallet;
2647-
WalletCreationStatus status = CreateWallet(*context.chain, passphrase, flags, request.params[0].get_str(), error, warnings, wallet);
2648-
switch (status) {
2649-
case WalletCreationStatus::CREATION_FAILED:
2650-
throw JSONRPCError(RPC_WALLET_ERROR, error.original);
2651-
case WalletCreationStatus::ENCRYPTION_FAILED:
2652-
throw JSONRPCError(RPC_WALLET_ENCRYPTION_FAILED, error.original);
2653-
case WalletCreationStatus::SUCCESS:
2654-
break;
2655-
// no default case, so the compiler can warn about missing cases
2646+
std::shared_ptr<CWallet> wallet = CreateWallet(*context.chain, request.params[0].get_str(), options, status, error, warnings);
2647+
if (!wallet) {
2648+
RPCErrorCode code = status == DatabaseStatus::FAILED_ENCRYPT ? RPC_WALLET_ENCRYPTION_FAILED : RPC_WALLET_ERROR;
2649+
throw JSONRPCError(code, error.original);
26562650
}
26572651

26582652
UniValue obj(UniValue::VOBJ);

0 commit comments

Comments
 (0)