Have -disablewallet remove all walletmodels and validate node-only mode with a functional test#697
Open
johnny9 wants to merge 3 commits into
Open
Have -disablewallet remove all walletmodels and validate node-only mode with a functional test#697johnny9 wants to merge 3 commits into
johnny9 wants to merge 3 commits into
Conversation
74c1a13 to
8281b21
Compare
jarolrod
reviewed
May 14, 2026
jarolrod
reviewed
May 18, 2026
|
|
||
| AppMode app_mode = SetupAppMode(); | ||
| #ifdef ENABLE_WALLET | ||
| const bool wallet_enabled = app_mode.walletEnabled(); |
Contributor
There was a problem hiding this comment.
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,
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
node-only mode using -disablewallet en espanol
Closes #506
Closes #691