Page MenuHomePhabricator
Files F13711192

D17958.id53607.diff
No OneTemporary
Actions

D17958.id53607.diff
View Options

diff --git a/src/interfaces/wallet.h b/src/interfaces/wallet.h
--- a/src/interfaces/wallet.h
+++ b/src/interfaces/wallet.h
@@ -95,7 +95,7 @@
// Get a new address.
virtual util::Result<CTxDestination>
- getNewDestination(const OutputType type, const std::string label) = 0;
+ getNewDestination(const OutputType type, const std::string &label) = 0;
//! Get public key.
virtual bool getPubKey(const CScript &script, const CKeyID &address,
@@ -303,19 +303,24 @@
class WalletClient : public ChainClient {
public:
//! Create new wallet.
- virtual std::unique_ptr<Wallet>
+ virtual util::Result<std::unique_ptr<Wallet>>
createWallet(const std::string &name, const SecureString &passphrase,
- uint64_t wallet_creation_flags, bilingual_str &error,
+ uint64_t wallet_creation_flags,
std::vector<bilingual_str> &warnings) = 0;
//! Load existing wallet.
- virtual std::unique_ptr<Wallet>
- loadWallet(const std::string &name, bilingual_str &error,
+ virtual util::Result<std::unique_ptr<Wallet>>
+ loadWallet(const std::string &name,
std::vector<bilingual_str> &warnings) = 0;
//! Return default wallet directory.
virtual std::string getWalletDir() = 0;
+ //! Restore backup wallet
+ virtual util::Result<std::unique_ptr<Wallet>>
+ restoreWallet(const fs::path &backup_file, const std::string &wallet_name,
+ std::vector<bilingual_str> &warnings) = 0;
+
//! Return available wallets in wallet directory.
virtual std::vector<std::string> listWalletDir() = 0;
diff --git a/src/qt/walletcontroller.cpp b/src/qt/walletcontroller.cpp
--- a/src/qt/walletcontroller.cpp
+++ b/src/qt/walletcontroller.cpp
@@ -254,14 +254,15 @@
flags |= WALLET_FLAG_DESCRIPTORS;
}
- QTimer::singleShot(500, worker(), [this, name, flags] {
- std::unique_ptr<interfaces::Wallet> wallet =
- node().walletClient().createWallet(
- name, m_passphrase, flags, m_error_message, m_warning_message);
+ QTimer::singleShot(500ms, worker(), [this, name, flags] {
+ auto wallet{node().walletClient().createWallet(
+ name, m_passphrase, flags, m_warning_message)};
if (wallet) {
m_wallet_model =
- m_wallet_controller->getOrCreateWallet(std::move(wallet));
+ m_wallet_controller->getOrCreateWallet(std::move(*wallet));
+ } else {
+ m_error_message = util::ErrorString(wallet);
}
QTimer::singleShot(500, this, &CreateWalletActivity::finish);
@@ -340,13 +341,13 @@
tr("Opening Wallet <b>%1</b>...").arg(name.toHtmlEscaped()));
QTimer::singleShot(0, worker(), [this, path] {
- std::unique_ptr<interfaces::Wallet> wallet =
- node().walletClient().loadWallet(path, m_error_message,
- m_warning_message);
+ auto wallet{node().walletClient().loadWallet(path, m_warning_message)};
if (wallet) {
m_wallet_model =
- m_wallet_controller->getOrCreateWallet(std::move(wallet));
+ m_wallet_controller->getOrCreateWallet(std::move(*wallet));
+ } else {
+ m_error_message = util::ErrorString(wallet);
}
QTimer::singleShot(0, this, &OpenWalletActivity::finish);
diff --git a/src/rpc/protocol.h b/src/rpc/protocol.h
--- a/src/rpc/protocol.h
+++ b/src/rpc/protocol.h
@@ -111,9 +111,10 @@
RPC_WALLET_NOT_SPECIFIED = -19,
//! This same wallet is already loaded
RPC_WALLET_ALREADY_LOADED = -35,
+ //! There is already a wallet with the same name
+ RPC_WALLET_ALREADY_EXISTS = -36,
//! Backwards compatible aliases
RPC_WALLET_INVALID_ACCOUNT_NAME = RPC_WALLET_INVALID_LABEL_NAME,
-
//! Unused reserved codes, kept around for backwards compatibility. Do not
//! reuse.
//! Server is in safe mode, and command is not allowed in safe mode
diff --git a/src/wallet/db.h b/src/wallet/db.h
--- a/src/wallet/db.h
+++ b/src/wallet/db.h
@@ -237,6 +237,7 @@
FAILED_LOAD,
FAILED_VERIFY,
FAILED_ENCRYPT,
+ FAILED_INVALID_BACKUP_FILE,
};
std::unique_ptr<WalletDatabase> MakeDatabase(const fs::path &path,
diff --git a/src/wallet/interfaces.cpp b/src/wallet/interfaces.cpp
--- a/src/wallet/interfaces.cpp
+++ b/src/wallet/interfaces.cpp
@@ -138,7 +138,7 @@
std::string getWalletName() override { return m_wallet->GetName(); }
util::Result<CTxDestination>
getNewDestination(const OutputType type,
- const std::string label) override {
+ const std::string &label) override {
LOCK(m_wallet->cs_wallet);
return m_wallet->GetNewDestination(type, label);
}
@@ -522,31 +522,53 @@
void setMockTime(int64_t time) override { return SetMockTime(time); }
//! WalletClient methods
- std::unique_ptr<Wallet>
+ util::Result<std::unique_ptr<Wallet>>
createWallet(const std::string &name, const SecureString &passphrase,
- uint64_t wallet_creation_flags, bilingual_str &error,
+ uint64_t wallet_creation_flags,
std::vector<bilingual_str> &warnings) override {
- std::shared_ptr<CWallet> wallet;
DatabaseOptions options;
DatabaseStatus status;
options.require_create = true;
options.create_flags = wallet_creation_flags;
options.create_passphrase = passphrase;
-
- return MakeWallet(m_context,
- CreateWallet(m_context, name,
- true /* load_on_start */, options,
- status, error, warnings));
+ bilingual_str error;
+ std::unique_ptr<Wallet> wallet{MakeWallet(
+ m_context, CreateWallet(m_context, name, /*load_on_start=*/true,
+ options, status, error, warnings))};
+ if (wallet) {
+ return wallet;
+ }
+ return util::Error{error};
}
- std::unique_ptr<Wallet>
- loadWallet(const std::string &name, bilingual_str &error,
+ util::Result<std::unique_ptr<Wallet>>
+ loadWallet(const std::string &name,
std::vector<bilingual_str> &warnings) override {
DatabaseOptions options;
DatabaseStatus status;
options.require_existing = true;
- return MakeWallet(
- m_context, LoadWallet(m_context, name, true /* load_on_start */,
- options, status, error, warnings));
+ bilingual_str error;
+ std::unique_ptr<Wallet> wallet{MakeWallet(
+ m_context, CreateWallet(m_context, name, /*load_on_start=*/true,
+ options, status, error, warnings))};
+ if (wallet) {
+ return wallet;
+ }
+ return util::Error{error};
+ }
+ util::Result<std::unique_ptr<Wallet>>
+ restoreWallet(const fs::path &backup_file,
+ const std::string &wallet_name,
+ std::vector<bilingual_str> &warnings) override {
+ DatabaseStatus status;
+ bilingual_str error;
+ std::unique_ptr<Wallet> wallet{MakeWallet(
+ m_context, RestoreWallet(m_context, backup_file, wallet_name,
+ /*load_on_start=*/true, status, error,
+ warnings))};
+ if (wallet) {
+ return wallet;
+ }
+ return util::Error{error};
}
std::string getWalletDir() override {
return fs::PathToString(GetWalletDir());
diff --git a/src/wallet/rpc/backup.cpp b/src/wallet/rpc/backup.cpp
--- a/src/wallet/rpc/backup.cpp
+++ b/src/wallet/rpc/backup.cpp
@@ -2475,34 +2475,22 @@
fs::path backup_file =
fs::PathFromString(request.params[1].get_str());
- if (!fs::exists(backup_file)) {
- throw JSONRPCError(RPC_INVALID_PARAMETER,
- "Backup file does not exist");
- }
-
std::string wallet_name = request.params[0].get_str();
- const fs::path wallet_path = fsbridge::AbsPathJoin(
- GetWalletDir(), fs::PathFromString(wallet_name));
-
- if (fs::exists(wallet_path)) {
- throw JSONRPCError(RPC_INVALID_PARAMETER,
- "Wallet name already exists.");
- }
-
- if (!TryCreateDirectories(wallet_path)) {
- throw JSONRPCError(RPC_WALLET_ERROR,
- strprintf("Failed to create database path "
- "'%s'. Database already exists.",
- fs::PathToString(wallet_path)));
- }
+ std::optional<bool> load_on_start =
+ request.params[2].isNull()
+ ? std::nullopt
+ : std::optional<bool>(request.params[2].get_bool());
- auto wallet_file = wallet_path / "wallet.dat";
+ DatabaseStatus status;
+ bilingual_str error;
+ std::vector<bilingual_str> warnings;
- fs::copy_file(backup_file, wallet_file, fs::copy_options::none);
+ const std::shared_ptr<CWallet> wallet =
+ RestoreWallet(context, backup_file, wallet_name, load_on_start,
+ status, error, warnings);
- auto [wallet, warnings] =
- LoadWalletHelper(context, request.params[2], wallet_name);
+ HandleWalletError(wallet, status, error);
UniValue obj(UniValue::VOBJ);
obj.pushKV("name", wallet->GetName());
diff --git a/src/wallet/rpc/util.h b/src/wallet/rpc/util.h
--- a/src/wallet/rpc/util.h
+++ b/src/wallet/rpc/util.h
@@ -12,6 +12,7 @@
struct bilingual_str;
class CWallet;
+enum class DatabaseStatus;
class JSONRPCRequest;
class LegacyScriptPubKeyMan;
class UniValue;
@@ -40,8 +41,7 @@
const CWallet &wallet);
std::string LabelFromValue(const UniValue &value);
-std::tuple<std::shared_ptr<CWallet>, std::vector<bilingual_str>>
-LoadWalletHelper(WalletContext &context, UniValue load_on_start_param,
- const std::string wallet_name);
+void HandleWalletError(const std::shared_ptr<CWallet> wallet,
+ DatabaseStatus &status, bilingual_str &error);
#endif // BITCOIN_WALLET_RPC_UTIL_H
diff --git a/src/wallet/rpc/util.cpp b/src/wallet/rpc/util.cpp
--- a/src/wallet/rpc/util.cpp
+++ b/src/wallet/rpc/util.cpp
@@ -132,20 +132,8 @@
return label;
}
-std::tuple<std::shared_ptr<CWallet>, std::vector<bilingual_str>>
-LoadWalletHelper(WalletContext &context, UniValue load_on_start_param,
- const std::string wallet_name) {
- DatabaseOptions options;
- DatabaseStatus status;
- options.require_existing = true;
- bilingual_str error;
- std::vector<bilingual_str> warnings;
- std::optional<bool> load_on_start =
- load_on_start_param.isNull()
- ? std::nullopt
- : std::make_optional<bool>(load_on_start_param.get_bool());
- std::shared_ptr<CWallet> const wallet = LoadWallet(
- context, wallet_name, load_on_start, options, status, error, warnings);
+void HandleWalletError(const std::shared_ptr<CWallet> wallet,
+ DatabaseStatus &status, bilingual_str &error) {
if (!wallet) {
// Map bad format to not found, since bad format is returned
// when the wallet directory exists, but doesn't contain a data
@@ -159,12 +147,16 @@
case DatabaseStatus::FAILED_ALREADY_LOADED:
code = RPC_WALLET_ALREADY_LOADED;
break;
+ case DatabaseStatus::FAILED_ALREADY_EXISTS:
+ code = RPC_WALLET_ALREADY_EXISTS;
+ break;
+ case DatabaseStatus::FAILED_INVALID_BACKUP_FILE:
+ code = RPC_INVALID_PARAMETER;
+ break;
default:
// RPC_WALLET_ERROR is returned for all other cases.
break;
}
throw JSONRPCError(code, error.original);
}
-
- return {wallet, warnings};
}
diff --git a/src/wallet/rpcwallet.cpp b/src/wallet/rpcwallet.cpp
--- a/src/wallet/rpcwallet.cpp
+++ b/src/wallet/rpcwallet.cpp
@@ -2683,8 +2683,19 @@
WalletContext &context = EnsureWalletContext(request.context);
const std::string name(request.params[0].get_str());
- auto [wallet, warnings] =
- LoadWalletHelper(context, request.params[1], name);
+ DatabaseOptions options;
+ DatabaseStatus status;
+ options.require_existing = true;
+ bilingual_str error;
+ std::vector<bilingual_str> warnings;
+ std::optional<bool> load_on_start =
+ request.params[1].isNull()
+ ? std::nullopt
+ : std::optional<bool>(request.params[1].get_bool());
+ std::shared_ptr<CWallet> const wallet = LoadWallet(
+ context, name, load_on_start, options, status, error, warnings);
+
+ HandleWalletError(wallet, status, error);
UniValue obj(UniValue::VOBJ);
obj.pushKV("name", wallet->GetName());
diff --git a/src/wallet/wallet.h b/src/wallet/wallet.h
--- a/src/wallet/wallet.h
+++ b/src/wallet/wallet.h
@@ -81,6 +81,11 @@
std::optional<bool> load_on_start, DatabaseOptions &options,
DatabaseStatus &status, bilingual_str &error,
std::vector<bilingual_str> &warnings);
+std::shared_ptr<CWallet>
+RestoreWallet(WalletContext &context, const fs::path &backup_file,
+ const std::string &wallet_name, std::optional<bool> load_on_start,
+ DatabaseStatus &status, bilingual_str &error,
+ std::vector<bilingual_str> &warnings);
std::unique_ptr<interfaces::Handler> HandleLoadWallet(WalletContext &context,
LoadWalletFn load_wallet);
std::unique_ptr<WalletDatabase>
@@ -785,7 +790,7 @@
EXCLUSIVE_LOCKS_REQUIRED(cs_wallet);
util::Result<CTxDestination> GetNewDestination(const OutputType type,
- const std::string label);
+ const std::string &label);
util::Result<CTxDestination> GetNewChangeDestination(const OutputType type);
isminetype IsMine(const CTxDestination &dest) const
diff --git a/src/wallet/wallet.cpp b/src/wallet/wallet.cpp
--- a/src/wallet/wallet.cpp
+++ b/src/wallet/wallet.cpp
@@ -381,6 +381,52 @@
return wallet;
}
+std::shared_ptr<CWallet>
+RestoreWallet(WalletContext &context, const fs::path &backup_file,
+ const std::string &wallet_name, std::optional<bool> load_on_start,
+ DatabaseStatus &status, bilingual_str &error,
+ std::vector<bilingual_str> &warnings) {
+ DatabaseOptions options;
+ options.require_existing = true;
+
+ const fs::path wallet_path =
+ fsbridge::AbsPathJoin(GetWalletDir(), fs::u8path(wallet_name));
+ auto wallet_file = wallet_path / "wallet.dat";
+ std::shared_ptr<CWallet> wallet;
+
+ try {
+ if (!fs::exists(backup_file)) {
+ error = Untranslated("Backup file does not exist");
+ status = DatabaseStatus::FAILED_INVALID_BACKUP_FILE;
+ return nullptr;
+ }
+
+ if (fs::exists(wallet_path) || !TryCreateDirectories(wallet_path)) {
+ error = Untranslated(strprintf(
+ "Failed to create database path '%s'. Database already exists.",
+ fs::PathToString(wallet_path)));
+ status = DatabaseStatus::FAILED_ALREADY_EXISTS;
+ return nullptr;
+ }
+
+ fs::copy_file(backup_file, wallet_file, fs::copy_options::none);
+
+ wallet = LoadWallet(context, wallet_name, load_on_start, options,
+ status, error, warnings);
+ } catch (const std::exception &e) {
+ assert(!wallet);
+ if (!error.empty()) {
+ error += Untranslated("\n");
+ }
+ error += strprintf(Untranslated("Unexpected exception: %s"), e.what());
+ }
+ if (!wallet) {
+ fs::remove_all(wallet_path);
+ }
+
+ return wallet;
+}
+
/** @defgroup mapWallet
*
* @{
@@ -2335,7 +2381,7 @@
}
util::Result<CTxDestination>
-CWallet::GetNewDestination(const OutputType type, const std::string label) {
+CWallet::GetNewDestination(const OutputType type, const std::string &label) {
LOCK(cs_wallet);
auto spk_man = GetScriptPubKeyMan(type, /*internal=*/false);
if (!spk_man) {
diff --git a/test/functional/wallet_backup.py b/test/functional/wallet_backup.py
--- a/test/functional/wallet_backup.py
+++ b/test/functional/wallet_backup.py
@@ -130,6 +130,24 @@
)
)
+ def restore_invalid_wallet(self):
+ node = self.nodes[3]
+ invalid_wallet_file = os.path.join(
+ self.nodes[0].datadir, "invalid_wallet_file.bak"
+ )
+ open(invalid_wallet_file, "a", encoding="utf8").write("invald wallet")
+ wallet_name = "res0"
+ not_created_wallet_file = os.path.join(
+ node.datadir, self.chain, "wallets", wallet_name
+ )
+ error_message = "Wallet file verification failed. Failed to load database path '{}'. Data is not in recognized format.".format(
+ not_created_wallet_file
+ )
+ assert_raises_rpc_error(
+ -18, error_message, node.restorewallet, wallet_name, invalid_wallet_file
+ )
+ assert not os.path.exists(not_created_wallet_file)
+
def restore_nonexistent_wallet(self):
node = self.nodes[3]
nonexistent_wallet_file = os.path.join(
@@ -143,18 +161,23 @@
wallet_name,
nonexistent_wallet_file,
)
+ not_created_wallet_file = os.path.join(
+ node.datadir, self.chain, "wallets", wallet_name
+ )
+ assert not os.path.exists(not_created_wallet_file)
def restore_wallet_existent_name(self):
node = self.nodes[3]
- wallet_file = os.path.join(self.nodes[0].datadir, "wallet.bak")
+ backup_file = os.path.join(self.nodes[0].datadir, "wallet.bak")
wallet_name = "res0"
+ wallet_file = os.path.join(node.datadir, self.chain, "wallets", wallet_name)
+ error_message = (
+ f"Failed to create database path '{wallet_file}'. Database already exists."
+ )
assert_raises_rpc_error(
- -8,
- "Wallet name already exists.",
- node.restorewallet,
- wallet_name,
- wallet_file,
+ -36, error_message, node.restorewallet, wallet_name, backup_file
)
+ assert os.path.exists(wallet_file)
def test_pruned_wallet_backup(self):
self.log.info(
@@ -225,6 +248,7 @@
##
self.log.info("Restoring wallets on node 3 using backup files")
+ self.restore_invalid_wallet()
self.restore_nonexistent_wallet()
backup_file_0 = os.path.join(self.nodes[0].datadir, "wallet.bak")
@@ -235,6 +259,16 @@
self.nodes[3].restorewallet("res1", backup_file_1)
self.nodes[3].restorewallet("res2", backup_file_2)
+ assert os.path.exists(
+ os.path.join(self.nodes[3].datadir, self.chain, "wallets", "res0")
+ )
+ assert os.path.exists(
+ os.path.join(self.nodes[3].datadir, self.chain, "wallets", "res1")
+ )
+ assert os.path.exists(
+ os.path.join(self.nodes[3].datadir, self.chain, "wallets", "res2")
+ )
+
res0_rpc = self.nodes[3].get_wallet_rpc("res0")
res1_rpc = self.nodes[3].get_wallet_rpc("res1")
res2_rpc = self.nodes[3].get_wallet_rpc("res2")

File Metadata

Mime Type
text/plain
Expires
Sat, Apr 26, 10:49 (10 h, 35 m)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
5548205
Default Alt Text
D17958.id53607.diff (19 KB)

Event Timeline