Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

gui: Add Open Wallet menu #15153

Merged
merged 8 commits into from Feb 12, 2019
Merged

gui: Add Open Wallet menu #15153

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
17abc0f
wallet: Factor out LoadWallet
promag Jan 12, 2019
ab288b4
interfaces: Add loadWallet to Node
promag Jan 12, 2019
32a8c6a
gui: Add openWallet and getWalletsAvailableToOpen to WalletController
promag Jan 12, 2019
6c49a55
gui: Add Open Wallet menu
promag Jan 12, 2019
be82dea
gui: Add thread to run background activity in WalletController
promag Jan 21, 2019
4c8982a
interfaces: Avoid interface instance if wallet is null
promag Jan 22, 2019
8847cda
gui: Add OpenWalletActivity
promag Jan 21, 2019
1951ea4
gui: Show indeterminate progress dialog while opening walllet
promag Jan 22, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/qt/bitcoin.cpp
View file
Open in desktop
Expand Up @@ -456,7 +456,7 @@ int GuiMain(int argc, char* argv[])
// IMPORTANT if it is no longer a typedef use the normal variant above
qRegisterMetaType< CAmount >("CAmount");
qRegisterMetaType< std::function<void()> >("std::function<void()>");

qRegisterMetaType<QMessageBox::Icon>("QMessageBox::Icon");
/// 2. Parse command-line options. We do this after qt in order to show an error if there are problems parsing these
// Command-line options take precedence:
node->setupServerArgs();
Expand Down
4 changes: 3 additions & 1 deletion src/qt/bitcoingui.cpp
View file
Open in desktop
Expand Up @@ -371,7 +371,9 @@ void BitcoinGUI::createActions()
QString name = path.empty() ? QString("["+tr("default wallet")+"]") : QString::fromStdString(path);
QAction* action = m_open_wallet_action->menu()->addAction(name);
connect(action, &QAction::triggered, [this, path] {
setCurrentWallet(m_wallet_controller->openWallet(path));
OpenWalletActivity* activity = m_wallet_controller->openWallet(path);
connect(activity, &OpenWalletActivity::opened, this, &BitcoinGUI::setCurrentWallet);
connect(activity, &OpenWalletActivity::finished, activity, &QObject::deleteLater);
});
}
});
Expand Down
36 changes: 26 additions & 10 deletions src/qt/walletcontroller.cpp
View file
Open in desktop
Expand Up @@ -55,17 +55,12 @@ std::vector<std::string> WalletController::getWalletsAvailableToOpen() const
return wallets;
}

WalletModel* WalletController::openWallet(const std::string& name, QWidget* parent)
OpenWalletActivity* WalletController::openWallet(const std::string& name, QWidget* parent)
{
std::string error, warning;
WalletModel* wallet_model = getOrCreateWallet(m_node.loadWallet(name, error, warning));
if (!wallet_model) {
QMessageBox::warning(parent, tr("Open Wallet"), QString::fromStdString(error));
}
if (!warning.empty()) {
QMessageBox::information(parent, tr("Open Wallet"), QString::fromStdString(warning));
}
return wallet_model;
OpenWalletActivity* activity = new OpenWalletActivity(this, name);
activity->moveToThread(&m_activity_thread);
QMetaObject::invokeMethod(activity, "open", Qt::QueuedConnection);
return activity;
}

WalletModel* WalletController::getOrCreateWallet(std::unique_ptr<interfaces::Wallet> wallet)
Expand Down Expand Up @@ -124,3 +119,24 @@ void WalletController::removeAndDeleteWallet(WalletModel* wallet_model)
// CWallet shared pointer.
delete wallet_model;
}


OpenWalletActivity::OpenWalletActivity(WalletController* wallet_controller, const std::string& name)
: m_wallet_controller(wallet_controller)
, m_name(name)
{}

void OpenWalletActivity::open()
{
std::string error, warning;
std::unique_ptr<interfaces::Wallet> wallet = m_wallet_controller->m_node.loadWallet(m_name, error, warning);
if (!warning.empty()) {
Q_EMIT message(QMessageBox::Warning, QString::fromStdString(warning));
}
if (wallet) {
Q_EMIT opened(m_wallet_controller->getOrCreateWallet(std::move(wallet)));
Copy link
Contributor

@ryanofsky ryanofsky Feb 13, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In commit "gui: Add OpenWalletActivity" (8847cda)

@promag, would there there any downsides to deleting this line? It seems redundant and as far as I can tell. Everything seems to works fine without it due to the load wallet event.

I'm trying to implement my suggestion from #15153 (comment) (so wallets will be loaded in the bitcoin-wallet processes not inside the bitcoin-node process), and I'd prefer just to have one way to attach a Wallet to the gui, not two different ways.

Copy link
Member Author

@promag promag Feb 13, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that the signal opened is connected to the setCurrentlyWallet but there is currently a problem there because the wallet view is created after, which needs to be fixed - I was already aware of this.

I don't think that's a good idea (only use the notification) - suppose you have 2 windows, you open a wallet on each (because the loading is async and doesn't block the GUI) you should see each wallet on the expected window.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, a multi window case is helpful to think about. I know you are working on fixing sync stuff in the gui, but what I would expect when loading a slow wallet is for the window to show "Loading wallet X..." right after the user selects the wallet, so the user has some immediate feedback. Then when the loadWallet notification is triggered, the GUI would use the Wallet handle passed with the notification to display actual wallet information. There would be no need for the call which the GUI makes to request loading of the wallet to return a wallet handle.

It sounds like this a concern for the future, though. Thanks for answering my immediate question.

Copy link
Member Author

@promag promag Feb 13, 2019 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But how would it know if the notified wallet handle belongs to the wallet opened? Wallet path?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I was just thinking wallet name.

} else {
Q_EMIT message(QMessageBox::Critical, QString::fromStdString(error));
}
Q_EMIT finished();
}
27 changes: 26 additions & 1 deletion src/qt/walletcontroller.h
View file
Open in desktop
Expand Up @@ -12,6 +12,7 @@
#include <memory>
#include <vector>

#include <QMessageBox>
#include <QMutex>
#include <QThread>

Expand All @@ -23,6 +24,8 @@ class Handler;
class Node;
} // namespace interfaces

class OpenWalletActivity;

/**
* Controller between interfaces::Node, WalletModel instances and the GUI.
*/
Expand All @@ -40,7 +43,7 @@ class WalletController : public QObject
std::vector<WalletModel*> getWallets() const;
std::vector<std::string> getWalletsAvailableToOpen() const;

WalletModel* openWallet(const std::string& name, QWidget* parent = nullptr);
OpenWalletActivity* openWallet(const std::string& name, QWidget* parent = nullptr);

private Q_SLOTS:
void addWallet(WalletModel* wallet_model);
Expand All @@ -59,6 +62,28 @@ private Q_SLOTS:
mutable QMutex m_mutex;
std::vector<WalletModel*> m_wallets;
std::unique_ptr<interfaces::Handler> m_handler_load_wallet;

friend class OpenWalletActivity;
};

class OpenWalletActivity : public QObject
{
Q_OBJECT

public:
OpenWalletActivity(WalletController* wallet_controller, const std::string& name);

public Q_SLOTS:
void open();

Q_SIGNALS:
void message(QMessageBox::Icon icon, const QString text);
void finished();
void opened(WalletModel* wallet_model);

private:
WalletController* const m_wallet_controller;
std::string const m_name;
};

#endif // BITCOIN_QT_WALLETCONTROLLER_H