Skip to content

Commit 240765e

Browse files
kwvgkwvg
committed
merge bitcoin#25616: Return util::Result from WalletLoader methods
1 parent bd897fa commit 240765e

File tree

4 files changed

+50
-38
lines changed

4 files changed

+50
-38
lines changed

‎src/interfaces/wallet.h‎

Lines changed: 22 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,7 @@ class Wallet
112112
virtual std::string getWalletName() = 0;
113113

114114
// Get a new address.
115-
virtual util::Result<CTxDestination> getNewDestination(const std::string label) = 0;
115+
virtual util::Result<CTxDestination> getNewDestination(const std::string& label) = 0;
116116

117117
//! Get public key.
118118
virtual bool getPubKey(const CScript& script, const CKeyID& address, CPubKey& pub_key) = 0;
@@ -347,35 +347,35 @@ class Wallet
347347
class WalletLoader : public ChainClient
348348
{
349349
public:
350-
//! Register non-core wallet RPCs
351-
virtual void registerOtherRpcs(const Span<const CRPCCommand>& commands) = 0;
350+
//! Register non-core wallet RPCs
351+
virtual void registerOtherRpcs(const Span<const CRPCCommand>& commands) = 0;
352352

353-
//! Create new wallet.
354-
virtual std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
353+
//! Create new wallet.
354+
virtual util::Result<std::unique_ptr<Wallet>> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, std::vector<bilingual_str>& warnings) = 0;
355355

356-
//! Load existing wallet.
357-
virtual std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) = 0;
356+
//! Load existing wallet.
357+
virtual util::Result<std::unique_ptr<Wallet>> loadWallet(const std::string& name, std::vector<bilingual_str>& warnings) = 0;
358358

359-
//! Return default wallet directory.
360-
virtual std::string getWalletDir() = 0;
359+
//! Return default wallet directory.
360+
virtual std::string getWalletDir() = 0;
361361

362-
//! Restore backup wallet
363-
virtual util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0;
362+
//! Restore backup wallet
363+
virtual util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) = 0;
364364

365-
//! Return available wallets in wallet directory.
366-
virtual std::vector<std::string> listWalletDir() = 0;
365+
//! Return available wallets in wallet directory.
366+
virtual std::vector<std::string> listWalletDir() = 0;
367367

368-
//! Return interfaces for accessing wallets (if any).
369-
virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0;
368+
//! Return interfaces for accessing wallets (if any).
369+
virtual std::vector<std::unique_ptr<Wallet>> getWallets() = 0;
370370

371-
//! Register handler for load wallet messages. This callback is triggered by
372-
//! createWallet and loadWallet above, and also triggered when wallets are
373-
//! loaded at startup or by RPC.
374-
using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>;
375-
virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0;
371+
//! Register handler for load wallet messages. This callback is triggered by
372+
//! createWallet and loadWallet above, and also triggered when wallets are
373+
//! loaded at startup or by RPC.
374+
using LoadWalletFn = std::function<void(std::unique_ptr<Wallet> wallet)>;
375+
virtual std::unique_ptr<Handler> handleLoadWallet(LoadWalletFn fn) = 0;
376376

377-
//! Return pointer to internal context, useful for testing.
378-
virtual wallet::WalletContext* context() { return nullptr; }
377+
//! Return pointer to internal context, useful for testing.
378+
virtual wallet::WalletContext* context() { return nullptr; }
379379
};
380380

381381
//! Information about one wallet address.

‎src/qt/walletcontroller.cpp‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -258,9 +258,13 @@ void CreateWalletActivity::createWallet()
258258
}
259259

260260
QTimer::singleShot(500ms, worker(), [this, name, flags] {
261-
std::unique_ptr<interfaces::Wallet> wallet = node().walletLoader().createWallet(name, m_passphrase, flags, m_error_message, m_warning_message);
261+
auto wallet{node().walletLoader().createWallet(name, m_passphrase, flags, m_warning_message)};
262262

263-
if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
263+
if (wallet) {
264+
m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));
265+
} else {
266+
m_error_message = util::ErrorString(wallet);
267+
}
264268

265269
QTimer::singleShot(500ms, this, &CreateWalletActivity::finish);
266270
});
@@ -330,9 +334,13 @@ void OpenWalletActivity::open(const std::string& path)
330334
tr("Opening Wallet <b>%1</b>…").arg(name.toHtmlEscaped()));
331335

332336
QTimer::singleShot(0, worker(), [this, path] {
333-
std::unique_ptr<interfaces::Wallet> wallet = node().walletLoader().loadWallet(path, m_error_message, m_warning_message);
337+
auto wallet{node().walletLoader().loadWallet(path, m_warning_message)};
334338

335-
if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(wallet));
339+
if (wallet) {
340+
m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));
341+
} else {
342+
m_error_message = util::ErrorString(wallet);
343+
}
336344

337345
QTimer::singleShot(0, this, &OpenWalletActivity::finish);
338346
});
@@ -375,8 +383,11 @@ void RestoreWalletActivity::restore(const fs::path& backup_file, const std::stri
375383
QTimer::singleShot(0, worker(), [this, backup_file, wallet_name] {
376384
auto wallet{node().walletLoader().restoreWallet(backup_file, wallet_name, m_warning_message)};
377385

378-
m_error_message = util::ErrorString(wallet);
379-
if (wallet) m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));
386+
if (wallet) {
387+
m_wallet_model = m_wallet_controller->getOrCreateWallet(std::move(*wallet));
388+
} else {
389+
m_error_message = util::ErrorString(wallet);
390+
}
380391

381392
QTimer::singleShot(0, this, &RestoreWalletActivity::finish);
382393
});

‎src/wallet/interfaces.cpp‎

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ class WalletImpl : public Wallet
175175
}
176176
int64_t getKeysLeftSinceAutoBackup() override { return m_wallet->nKeysLeftSinceAutoBackup; }
177177
std::string getWalletName() override { return m_wallet->GetName(); }
178-
util::Result<CTxDestination> getNewDestination(const std::string label) override
178+
util::Result<CTxDestination> getNewDestination(const std::string& label) override
179179
{
180180
LOCK(m_wallet->cs_wallet);
181181
return m_wallet->GetNewDestination(label);
@@ -608,32 +608,34 @@ class WalletLoaderImpl : public WalletLoader
608608
{
609609
return RegisterRPCs(commands);
610610
}
611-
std::unique_ptr<Wallet> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, bilingual_str& error, std::vector<bilingual_str>& warnings) override
611+
util::Result<std::unique_ptr<Wallet>> createWallet(const std::string& name, const SecureString& passphrase, uint64_t wallet_creation_flags, std::vector<bilingual_str>& warnings) override
612612
{
613-
std::shared_ptr<CWallet> wallet;
614613
DatabaseOptions options;
615614
DatabaseStatus status;
616615
ReadDatabaseArgs(*m_context.args, options);
617616
options.require_create = true;
618617
options.create_flags = wallet_creation_flags;
619618
options.create_passphrase = passphrase;
620-
return MakeWallet(m_context, CreateWallet(m_context, name, true /* load_on_start */, options, status, error, warnings));
619+
bilingual_str error;
620+
util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, CreateWallet(m_context, name, /*load_on_start=*/true, options, status, error, warnings))};
621+
return wallet ? std::move(wallet) : util::Error{error};
621622
}
622-
std::unique_ptr<Wallet> loadWallet(const std::string& name, bilingual_str& error, std::vector<bilingual_str>& warnings) override
623+
util::Result<std::unique_ptr<Wallet>> loadWallet(const std::string& name, std::vector<bilingual_str>& warnings) override
623624
{
624625
DatabaseOptions options;
625626
DatabaseStatus status;
626627
ReadDatabaseArgs(*m_context.args, options);
627628
options.require_existing = true;
628-
return MakeWallet(m_context, LoadWallet(m_context, name, true /* load_on_start */, options, status, error, warnings));
629+
bilingual_str error;
630+
util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, LoadWallet(m_context, name, /*load_on_start=*/true, options, status, error, warnings))};
631+
return wallet ? std::move(wallet) : util::Error{error};
629632
}
630633
util::Result<std::unique_ptr<Wallet>> restoreWallet(const fs::path& backup_file, const std::string& wallet_name, std::vector<bilingual_str>& warnings) override
631634
{
632635
DatabaseStatus status;
633636
bilingual_str error;
634637
util::Result<std::unique_ptr<Wallet>> wallet{MakeWallet(m_context, RestoreWallet(m_context, backup_file, wallet_name, /*load_on_start=*/true, status, error, warnings))};
635-
if (!wallet) return util::Error{error};
636-
return wallet;
638+
return wallet ? std::move(wallet) : util::Error{error};
637639
}
638640
std::string getWalletDir() override
639641
{

‎src/wallet/test/wallet_tests.cpp‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1539,10 +1539,9 @@ BOOST_FIXTURE_TEST_CASE(wallet_sync_tx_invalid_state_test, TestChain100Setup)
15391539
// Add tx to wallet
15401540
const auto& op_dest = wallet.GetNewDestination("");
15411541
BOOST_ASSERT(op_dest);
1542-
const CTxDestination& dest = *op_dest;
15431542

15441543
CMutableTransaction mtx;
1545-
mtx.vout.push_back({COIN, GetScriptForDestination(dest)});
1544+
mtx.vout.push_back({COIN, GetScriptForDestination(*op_dest)});
15461545
mtx.vin.push_back(CTxIn(g_insecure_rand_ctx.rand256(), 0));
15471546
const auto& tx_id_to_spend = wallet.AddToWallet(MakeTransactionRef(mtx), TxStateInMempool{})->GetHash();
15481547

0 commit comments

Comments
 (0)