Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/gui-functional-tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ jobs:
run: |
python3 test/functional/qml_test_bridge_sanity.py
python3 test/functional/qml_test_onboarding.py
python3 test/functional/qml_test_disablewallet_boot.py
python3 test/functional/qml_test_external_signer.py
python3 test/functional/qml_test_peers.py
python3 test/functional/qml_test_proxy.py
Expand Down
30 changes: 21 additions & 9 deletions qml/bitcoin.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -294,18 +294,26 @@ int QmlGuiMain(int argc, char* argv[])

handler_message_box.disconnect();

AppMode app_mode = SetupAppMode();
#ifdef ENABLE_WALLET
const bool wallet_enabled = app_mode.walletEnabled();
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

not a blocker here:

AppMode is kind of heavy right now. It does both the presentation mode (mobile vs desktop) and App features themselves.

Maybe we should split these? AppMode answers UI questions and a Features singleton to answer feature questions

This Features singleton could answer other questions as well, like do we have QR functionality built in etc etc

Whereas we currently have the following to check:

AppMode.walletEnabled && AppMode.isDesktop

We would then be checking the following

Features.walletEnabled && AppMode.isDesktop

It could look something like

Details
diff --git a/qml/appmode.h b/qml/appmode.h
index ba15865ed..c205cf2f1 100644
--- a/qml/appmode.h
+++ b/qml/appmode.h
@@ -12,7 +12,6 @@ class AppMode : public QObject
     Q_OBJECT
     Q_PROPERTY(bool isDesktop READ isDesktop NOTIFY modeChanged)
     Q_PROPERTY(bool isMobile READ isMobile NOTIFY modeChanged)
-    Q_PROPERTY(bool walletEnabled READ walletEnabled NOTIFY walletEnabledChanged)
     Q_PROPERTY(QString state READ state NOTIFY modeChanged)
 
 public:
@@ -21,15 +20,13 @@ public:
         MOBILE
     };
 
-    explicit AppMode(Mode mode, bool wallet_enabled)
+    explicit AppMode(Mode mode)
     : m_mode(mode)
-    , m_wallet_enabled(wallet_enabled)
     {
     }
 
     bool isMobile() { return m_mode == MOBILE; }
     bool isDesktop() { return m_mode == DESKTOP; }
-    bool walletEnabled() { return m_wallet_enabled; }
     QString state()
     {
         switch (m_mode) {
@@ -45,11 +42,9 @@ public:
 
 Q_SIGNALS:
     void modeChanged();
-    void walletEnabledChanged();
 
 private:
     const Mode m_mode;
-    const bool m_wallet_enabled;
 };
 
 #endif // BITCOIN_QML_APPMODE_H
diff --git a/qml/bitcoin.cpp b/qml/bitcoin.cpp
index 4f3f0c824..32c76f929 100644
--- a/qml/bitcoin.cpp
+++ b/qml/bitcoin.cpp
@@ -20,6 +20,7 @@
 #include <qml/bitcoinamount.h>
 #include <qml/buildinfo.h>
 #include <qml/clipboard.h>
+#include <qml/features.h>
 #ifdef __ANDROID__
 #include <qml/androidnotifier.h>
 #endif
@@ -105,7 +106,6 @@ void SetupUIArgs(ArgsManager& argsman)
 
 AppMode SetupAppMode()
 {
-    bool wallet_enabled;
     AppMode::Mode mode;
     #ifdef __ANDROID__
         mode = AppMode::MOBILE;
@@ -113,13 +113,17 @@ AppMode SetupAppMode()
         mode = AppMode::DESKTOP;
     #endif // __ANDROID__
 
-    #ifdef ENABLE_WALLET
-        wallet_enabled = !gArgs.GetBoolArg("-disablewallet", false);
-    #else
-        wallet_enabled = false;
-    #endif // ENABLE_WALLET
+    return AppMode(mode);
+}
+
+Features SetupFeatures()
+{
+    bool wallet_enabled{false};
+#ifdef ENABLE_WALLET
+    wallet_enabled = !gArgs.GetBoolArg("-disablewallet", false);
+#endif
 
-    return AppMode(mode, wallet_enabled);
+    return Features(wallet_enabled);
 }
 
 bool InitErrorMessageBox(
@@ -295,8 +299,9 @@ int QmlGuiMain(int argc, char* argv[])
     handler_message_box.disconnect();
 
     AppMode app_mode = SetupAppMode();
+    Features features = SetupFeatures();
 #ifdef ENABLE_WALLET
-    const bool wallet_enabled = app_mode.walletEnabled();
+    const bool wallet_enabled = features.walletEnabled();
 #endif
 
     NodeModel node_model{*node};
@@ -413,6 +418,7 @@ int QmlGuiMain(int argc, char* argv[])
     Clipboard clipboard;
 
     qmlRegisterSingletonInstance<AppMode>("org.bitcoincore.qt", 1, 0, "AppMode", &app_mode);
+    qmlRegisterSingletonInstance<Features>("org.bitcoincore.qt", 1, 0, "Features", &features);
     qmlRegisterSingletonInstance<BuildInfo>("org.bitcoincore.qt", 1, 0, "BuildInfo", &build_info);
     qmlRegisterSingletonInstance<Clipboard>("org.bitcoincore.qt", 1, 0, "Clipboard", &clipboard);
     qmlRegisterType<BlockClockDial>("org.bitcoincore.qt", 1, 0, "BlockClockDial");
diff --git a/qml/pages/main.qml b/qml/pages/main.qml
index ca5dc6d38..9a743fa9e 100644
--- a/qml/pages/main.qml
+++ b/qml/pages/main.qml
@@ -39,7 +39,7 @@ ApplicationWindow {
             if (needOnboarding) {
                 onboardingWizard
             } else {
-                if (AppMode.walletEnabled && AppMode.isDesktop) {
+                if (Features.walletEnabled && AppMode.isDesktop) {
                     desktopWallets
                 } else {
                     node
@@ -70,7 +70,7 @@ ApplicationWindow {
             onFinished: {
                 optionsModel.onboard()
                 nodeModel.startNodeInitializionThread()
-                if (AppMode.walletEnabled && AppMode.isDesktop) {
+                if (Features.walletEnabled && AppMode.isDesktop) {
                     main.push([
                         desktopWallets, {},
                         createWalletWizard, { "launchContext": CreateWalletWizard.Context.Onboarding }
diff --git a/qml/pages/node/NodeSettings.qml b/qml/pages/node/NodeSettings.qml
index 5466236cb..d96befd38 100644
--- a/qml/pages/node/NodeSettings.qml
+++ b/qml/pages/node/NodeSettings.qml
@@ -26,6 +26,9 @@ PageStack {
     }
 
     function openWalletSettings() {
+        if (!Features.walletEnabled) {
+            return
+        }
         while (root.depth > 1) {
             root.pop()
         }
@@ -97,14 +100,14 @@ PageStack {
                 Setting {
                     id: gotoWallet
                     objectName: "settingsWallet"
-                    visible: AppMode.walletEnabled
+                    visible: Features.walletEnabled
                     Layout.fillWidth: true
                     header: qsTr("External Signer")
                     actionItem: CaretRightIcon {
                         color: gotoWallet.stateColor
                     }
                     onClicked: {
-                        root.push(wallet_page)
+                        root.openWalletSettings()
                     }
                 }
                 Separator {
diff --git a/qml/walletqmlcontroller.cpp b/qml/walletqmlcontroller.cpp
index f824cba66..cde5d9004 100644
--- a/qml/walletqmlcontroller.cpp
+++ b/qml/walletqmlcontroller.cpp
@@ -580,7 +580,7 @@ void WalletQmlController::handleLoadWallet(std::unique_ptr<interfaces::Wallet> w
 
 void WalletQmlController::initialize()
 {
-    // wallet_loader is not set when -disablewallet is passed; bail out.
+    // wallet_loader is not constructed when -disablewallet is passed.
     if (gArgs.GetBoolArg("-disablewallet", false)) {
         return;
     }
diff --git a/test/functional/qml_test_debug_log.py b/test/functional/qml_test_debug_log.py
index 96cf3980b..abc8e8a31 100644
--- a/test/functional/qml_test_debug_log.py
+++ b/test/functional/qml_test_debug_log.py
@@ -54,7 +54,7 @@ class DebugLogHarness:
             f"-test-automation={self.socket_path}",
             # Intentionally omit -resetguisettings: the datadir already
             # exists so needOnboarding stays false and NodeRunner loads first.
-            # -disablewallet forces AppMode.walletEnabled=false so main.qml
+            # -disablewallet forces Features.walletEnabled=false so main.qml
             # routes to the node/NodeRunner stack instead of desktopWallets.
             "-disablewallet",
             "-logtimemicros",
@@ -91,7 +91,7 @@ class DebugLogHarness:
 def navigate_to_debug_log(gui):
     """From the NodeRunner main screen, navigate to the Debug Log page.
 
-    Requires -disablewallet so AppMode.walletEnabled is false and main.qml
+    Requires -disablewallet so Features.walletEnabled is false and main.qml
     routes to the node/NodeRunner stack rather than desktopWallets.
     """
     gui.wait_for_page("nodeRunner", timeout_ms=10000)
diff --git a/test/functional/qml_test_disablewallet_boot.py b/test/functional/qml_test_disablewallet_boot.py
index 7c271a858..19ff5d2a5 100755
--- a/test/functional/qml_test_disablewallet_boot.py
+++ b/test/functional/qml_test_disablewallet_boot.py
@@ -137,6 +137,15 @@ def assert_wallet_ui_absent(gui):
         f"got {wallet_settings_visible!r}"
     )
 
+    # The test bridge can invoke signals on hidden objects, so this verifies the
+    # route itself is guarded and the wallet settings page cannot be reached.
+    gui.click("settingsWallet")
+    gui.settle()
+    objects = {entry["objectName"] for entry in gui.list_objects()}
+    assert "settingsWalletBack" not in objects, (
+        "Wallet settings page should not be reachable in -disablewallet mode"
+    )
+
 
 def walk_about_settings(gui, checkpoints):
     print("  Opening About settings")
diff --git a/test/mocks/qml/org/bitcoincore/qt/qmldir b/test/mocks/qml/org/bitcoincore/qt/qmldir
index 56f01a045..589846049 100644
--- a/test/mocks/qml/org/bitcoincore/qt/qmldir
+++ b/test/mocks/qml/org/bitcoincore/qt/qmldir
@@ -1,2 +1,3 @@
 module org.bitcoincore.qt
 singleton AppMode 1.0 AppMode.qml
+singleton Features 1.0 Features.qml
diff --git a/test/qml/qml_tests_main.cpp b/test/qml/qml_tests_main.cpp
index b4fcc9426..2a8a003b9 100644
--- a/test/qml/qml_tests_main.cpp
+++ b/test/qml/qml_tests_main.cpp
@@ -21,12 +21,10 @@ class MockAppMode : public QObject
 {
     Q_OBJECT
     Q_PROPERTY(bool isDesktop READ isDesktop WRITE setIsDesktop NOTIFY isDesktopChanged)
-    Q_PROPERTY(bool walletEnabled READ walletEnabled WRITE setWalletEnabled NOTIFY walletEnabledChanged)
     Q_PROPERTY(QString state READ state WRITE setState NOTIFY stateChanged)
 
 public:
     bool isDesktop() const { return m_is_desktop; }
-    bool walletEnabled() const { return m_wallet_enabled; }
     QString state() const { return m_state; }
 
 public Q_SLOTS:
@@ -36,12 +34,6 @@ public Q_SLOTS:
         m_is_desktop = value;
         Q_EMIT isDesktopChanged();
     }
-    void setWalletEnabled(const bool value)
-    {
-        if (m_wallet_enabled == value) return;
-        m_wallet_enabled = value;
-        Q_EMIT walletEnabledChanged();
-    }
     void setState(const QString& value)
     {
         if (m_state == value) return;
@@ -51,15 +43,36 @@ public Q_SLOTS:
 
 Q_SIGNALS:
     void isDesktopChanged();
-    void walletEnabledChanged();
     void stateChanged();
 
 private:
     bool m_is_desktop{true};
-    bool m_wallet_enabled{true};
     QString m_state{QStringLiteral("desktop")};
 };
 
+class MockFeatures : public QObject
+{
+    Q_OBJECT
+    Q_PROPERTY(bool walletEnabled READ walletEnabled WRITE setWalletEnabled NOTIFY walletEnabledChanged)
+
+public:
+    bool walletEnabled() const { return m_wallet_enabled; }
+
+public Q_SLOTS:
+    void setWalletEnabled(const bool value)
+    {
+        if (m_wallet_enabled == value) return;
+        m_wallet_enabled = value;
+        Q_EMIT walletEnabledChanged();
+    }
+
+Q_SIGNALS:
+    void walletEnabledChanged();
+
+private:
+    bool m_wallet_enabled{true};
+};
+
 class MockPeerDetailsModel : public QObject
 {
     Q_OBJECT
@@ -1149,6 +1162,7 @@ public Q_SLOTS:
     {
         engine->addImportPath(QStringLiteral(BITCOINQML_QML_TEST_MOCKS_DIR));
         static MockAppMode app_mode;
+        static MockFeatures features;
         static MockOptionsModel options_model;
         static MockChainModel chain_model;
         static MockNodeModel node_model;
@@ -1175,6 +1189,7 @@ public Q_SLOTS:
         wallet_model.setCurrentPaymentRequest(&payment_request);
         wallet_controller.setSelectedWalletObject(&wallet_model);
         qmlRegisterSingletonInstance<MockAppMode>("org.bitcoincore.qt", 1, 0, "AppMode", &app_mode);
+        qmlRegisterSingletonInstance<MockFeatures>("org.bitcoincore.qt", 1, 0, "Features", &features);
         qmlRegisterUncreatableType<MockPeerDetailsModel>(
             "org.bitcoincore.qt",
             1,

#endif

NodeModel node_model{*node};
QmlInitExecutor init_executor{*node};
#ifdef ENABLE_WALLET
WalletQmlController wallet_controller(*node);
if (!gArgs.GetBoolArg("-disablewallet", false)) {
QObject::connect(&init_executor, &QmlInitExecutor::initializeResult, &wallet_controller, &WalletQmlController::initialize);
std::unique_ptr<WalletQmlController> wallet_controller;
if (wallet_enabled) {
wallet_controller = std::make_unique<WalletQmlController>(*node);
QObject::connect(&init_executor, &QmlInitExecutor::initializeResult, wallet_controller.get(), &WalletQmlController::initialize);
}
#endif
QObject::connect(&node_model, &NodeModel::requestedInitialize, &init_executor, &QmlInitExecutor::initialize);
QObject::connect(&node_model, &NodeModel::requestedShutdown, [&] {
#ifdef ENABLE_WALLET
wallet_controller.unloadWallets();
if (wallet_controller) {
wallet_controller->unloadWallets();
}
#endif
init_executor.shutdown();
});
Expand All @@ -329,7 +337,9 @@ int QmlGuiMain(int argc, char* argv[])
qGuiApp->setQuitOnLastWindowClosed(false);
QObject::connect(qGuiApp, &QGuiApplication::lastWindowClosed, [&] {
#ifdef ENABLE_WALLET
wallet_controller.unloadWallets();
if (wallet_controller) {
wallet_controller->unloadWallets();
}
#endif
node->startShutdown();
});
Expand Down Expand Up @@ -366,9 +376,12 @@ int QmlGuiMain(int argc, char* argv[])
engine.rootContext()->setContextProperty("debugLogModel", &debug_log_model);

#ifdef ENABLE_WALLET
WalletListModel wallet_list_model{*node, nullptr};
engine.rootContext()->setContextProperty("walletController", &wallet_controller);
engine.rootContext()->setContextProperty("walletListModel", &wallet_list_model);
std::unique_ptr<WalletListModel> wallet_list_model;
if (wallet_enabled) {
wallet_list_model = std::make_unique<WalletListModel>(*node, nullptr);
engine.rootContext()->setContextProperty("walletController", wallet_controller.get());
engine.rootContext()->setContextProperty("walletListModel", wallet_list_model.get());
}
#endif

OptionsQmlModel options_model(*node, !need_onboarding.toBool());
Expand Down Expand Up @@ -396,7 +409,6 @@ int QmlGuiMain(int argc, char* argv[])
engine.retranslate();
});

AppMode app_mode = SetupAppMode();
BuildInfo build_info;
Clipboard clipboard;

Expand Down
1 change: 1 addition & 0 deletions qml/pages/node/NetworkTraffic.qml
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ InformationPage {
property alias trafficGraphScale: root.trafficGraphScale
}
navLeftDetail: NavButton {
objectName: "networkTrafficBackButton"
iconSource: "image://images/caret-left"
text: qsTr("Back")
onClicked: root.back()
Expand Down
3 changes: 3 additions & 0 deletions qml/pages/node/NodeSettings.qml
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ PageStack {
}
rightItem: NavButton {
id: doneButton
objectName: "nodeSettingsDoneButton"
text: qsTr("Done")
onClicked: root.doneClicked()
}
Expand Down Expand Up @@ -82,6 +83,7 @@ PageStack {
Separator { Layout.fillWidth: true }
Setting {
id: gotoStorage
objectName: "gotoStorage"
Layout.fillWidth: true
header: qsTr("Storage")
actionItem: CaretRightIcon {
Expand Down Expand Up @@ -138,6 +140,7 @@ PageStack {
Separator { Layout.fillWidth: true }
Setting {
id: gotoNetworkTraffic
objectName: "settingsNetworkTraffic"
Layout.fillWidth: true
header: qsTr("Network Traffic")
actionItem: CaretRightIcon {
Expand Down
1 change: 1 addition & 0 deletions qml/pages/node/Peers.qml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Page {

header: NavigationBar2 {
leftItem: NavButton {
objectName: "peersBackButton"
iconSource: "image://images/caret-left"
text: qsTr("Back")
onClicked: root.back()
Expand Down
1 change: 1 addition & 0 deletions qml/pages/settings/SettingsAbout.qml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ InformationPage {
Component {
id: backButton
NavButton {
objectName: "settingsAboutBack"
iconSource: "image://images/caret-left"
text: qsTr("Back")
onClicked: root.back()
Expand Down
1 change: 1 addition & 0 deletions qml/pages/settings/SettingsConnection.qml
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ Page {
Component {
id: backButton
NavButton {
objectName: "settingsConnectionBack"
iconSource: "image://images/caret-left"
text: qsTr("Back")
onClicked: root.back()
Expand Down
1 change: 1 addition & 0 deletions qml/pages/settings/SettingsDeveloper.qml
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ InformationPage {
objectName: "settingsDeveloper"
property bool onboarding: false
navLeftDetail: NavButton {
objectName: "settingsDeveloperBack"
iconSource: "image://images/caret-left"
text: qsTr("Back")
onClicked: {
Expand Down
1 change: 1 addition & 0 deletions qml/pages/settings/SettingsDisplay.qml
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ Item {

header: NavigationBar2 {
leftItem: NavButton {
objectName: "settingsDisplayBack"
iconSource: "image://images/caret-left"
text: qsTr("Back")
onClicked: root.back()
Expand Down
1 change: 1 addition & 0 deletions qml/pages/settings/SettingsStorage.qml
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ InformationPage {
Component {
id: backButton
NavButton {
objectName: "settingsStorageBack"
iconSource: "image://images/caret-left"
text: qsTr("Back")
onClicked: root.back()
Expand Down
31 changes: 31 additions & 0 deletions qml/test/testbridge.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include <QPointer>
#include <QQuickItem>
#include <QQuickWindow>
#include <QQmlContext>
#include <QScopedValueRollback>
#include <QTimer>
#include <QVariant>
Expand Down Expand Up @@ -375,6 +376,8 @@ QByteArray TestBridge::processCommand(const QByteArray& json_cmd)

if (cmd == QLatin1String("get_current_page")) {
return cmdGetCurrentPage();
} else if (cmd == QLatin1String("get_context_property")) {
return cmdGetContextProperty(obj.value(QStringLiteral("name")).toString());
} else if (cmd == QLatin1String("get_property")) {
return cmdGetProperty(
obj.value(QStringLiteral("objectName")).toString(),
Expand Down Expand Up @@ -457,6 +460,34 @@ QByteArray TestBridge::cmdGetCurrentPage()
return errorResponse(QStringLiteral("Could not determine current page; missing mainPageStack/current page item"));
}

QByteArray TestBridge::cmdGetContextProperty(const QString& name)
{
if (name.isEmpty()) {
return errorResponse(QStringLiteral("name is required"));
}

QVariant value = m_engine->rootContext()->contextProperty(name);
const bool exists = value.isValid() && !value.isNull();

QJsonObject resp;
resp[QStringLiteral("exists")] = exists;
if (!exists) {
return QJsonDocument(resp).toJson(QJsonDocument::Compact);
}

if (QObject* object = value.value<QObject*>()) {
resp[QStringLiteral("isQObject")] = true;
resp[QStringLiteral("className")] = QString::fromLatin1(object->metaObject()->className());
resp[QStringLiteral("objectName")] = object->objectName();
} else {
resp[QStringLiteral("isQObject")] = false;
resp[QStringLiteral("typeName")] = QString::fromLatin1(value.typeName());
resp[QStringLiteral("value")] = QJsonValue::fromVariant(value);
}

return QJsonDocument(resp).toJson(QJsonDocument::Compact);
}

QByteArray TestBridge::cmdGetProperty(const QString& object_name, const QString& prop)
{
if (object_name.isEmpty() || prop.isEmpty()) {
Expand Down
2 changes: 2 additions & 0 deletions qml/test/testbridge.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
///
/// Supported commands (JSON over newline-delimited stream):
/// {"cmd": "get_current_page"}
/// {"cmd": "get_context_property", "name": "<contextPropertyName>"}
/// {"cmd": "get_property", "objectName": "<name>", "prop": "<property>"}
/// {"cmd": "click", "objectName": "<name>"}
/// {"cmd": "set_text", "objectName": "<name>", "text": "<value>"}
Expand Down Expand Up @@ -70,6 +71,7 @@ private Q_SLOTS:

/// Dispatch individual command handlers.
QByteArray cmdGetCurrentPage();
QByteArray cmdGetContextProperty(const QString& name);
QByteArray cmdGetProperty(const QString& object_name, const QString& prop);
QByteArray cmdClick(const QString& object_name);
QByteArray cmdSetText(const QString& object_name, const QString& text);
Expand Down
9 changes: 9 additions & 0 deletions test/functional/qml_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -198,6 +198,15 @@ def list_objects(self):
raise QmlDriverError(f"list_objects failed: {resp['error']}")
return resp["objects"]

def get_context_property(self, name):
"""Return metadata for a QQmlEngine root context property."""
resp = self._send({"cmd": "get_context_property", "name": name})
if "error" in resp:
raise QmlDriverError(
f"get_context_property({name!r}) failed: {resp['error']}"
)
return resp

def save_screenshot(self, path):
"""Save a screenshot of the current QML window to a PNG file.

Expand Down
Loading
Loading