DRAFT: Custom Datadir Wiring #390

D33r-Gee · 2024-03-08T16:08:55Z

This pull request introduces the functionality for users to specify a custom data directory (datadir) during the onboarding process.

Ubuntu Screenshots

Prerequisites

For testing this pull request, ensure you have the following Qt modules installed:

On Ubuntu 22.04:

sudo apt-get install qtdeclarative5-dev
sudo apt-get install qtquickcontrols2-5-dev
sudo apt-get install qml-module-qtquick-controls2
sudo apt install qml-module-qtquick-dialogs

For Android:

Make sure you delete the prior depends folder (i.e. depends/aarch64-linux-android) and rebuild them.

Implementation Details

This enhancement leverages FileDialog class for a user-friendly selection experience. Additionally, necessary code updates and adjustments were made to accommodate the delayed node creation until onboarding completion.

QML Backend
- Introduced a new class within qml/bitcoin.h and cpp to manage custom data directory settings.
- Integrated pull request Only write options after onboarding #284 by Johny9.
- Added onboardingmodel files to oversee the onboarding process, specifically custom data directory configuration and placeholders for optionsModel.
QML Frontend
- Updated StorageLocations.qml to incorporate wiring for custom data directory selection and resetting to default.
- Commented out settings code in OnboardingCover.qml due to the postponed node initialization until onboarding completion.
- Modified Onboarding qml files to utilize onboardingModel instead of optionsModel. Additionally, arguments referencing chainModel were removed as node initialization now occurs at the end of the onboarding process.
The Make process was extended to include the onboardingmodel files.

TODO: will implement a way to display a read only path to the user's custom datadir, most likely tin the StorageOptions file.

johnny9

Left comments for some obvious code areas that can be cleaned up.

During testing, I was unable to start the app after onboarding. The inital onboarding flow worked, however, It appeared to not find the custom data directory I set on the second time loading the app.

Error: Cannot write to data directory '/home/johnny/.bitcoin/signet'; check permissions.
qrc:/qml/pages/node/NodeRunner.qml:25: ReferenceError: nodeModel is not defined
qrc:/qml/components/BlockClock.qml:31: TypeError: Cannot read property 'text' of undefined
qrc:/qml/components/BlockClock.qml:28: ReferenceError: nodeModel is not defined
qrc:/qml/components/BlockClock.qml:27: ReferenceError: nodeModel is not defined
qrc:/qml/components/BlockClock.qml:53: ReferenceError: nodeModel is not defined
qrc:/qml/components/BlockClock.qml:26: ReferenceError: nodeModel is not defined
qrc:/qml/components/BlockClock.qml:50: ReferenceError: nodeModel is not defined
qrc:/qml/components/BlockClock.qml:49: ReferenceError: chainModel is not defined
qrc:/qml/components/BlockClock.qml:32: TypeError: Cannot read property 'estimating' of undefined
qrc:/qml/components/BlockClock.qml:30: ReferenceError: nodeModel is not defined
qrc:/qml/components/BlockClock.qml:141: ReferenceError: nodeModel is not defined
qrc:/qml/components/BlockClock.qml:140: ReferenceError: nodeModel is not defined
qrc:/qml/components/NetworkIndicator.qml:20: ReferenceError: chainModel is not defined

src/qml/bitcoin.cpp

src/qml/components/StorageOptions.qml

src/qml/models/onboardingmodel.h

src/qml/pages/main.qml

src/qml/pages/onboarding/OnboardingStorageLocation.qml

src/qml/pages/onboarding/OnboardingCover.qml

D33r-Gee · 2024-03-12T16:53:47Z

Thanks @johnny9 for commenting and reviewing!

During testing, I was unable to start the app after onboarding. The initial onboarding flow worked, however, It appeared to not find the custom data directory I set on the second time loading the app.

Was able to reproduce, it seems that if the custom datadir exist but hasn't been fully initialized, meaning the onboarding process didn't finished, it will trigger the error you encountered. Will look into how to mitigate that...

Ok found the problem, I had commented out the gArgs.ClearPathCache(); in qml/bitcoin.cpp, it works now... going to update the commits

D33r-Gee · 2024-03-13T22:53:24Z

updated the commits from 87b4d75 to 76f83f2

src/qml/bitcoin.cpp

MarnixCroes · 2024-03-14T09:03:53Z

CI is failing, please fix

pablomartin4btc

tACK 22fb833

Tested on Ubuntu 22.04.

1. Building Issues (dynamic)/ Errors while running bitcoin-qt (Prerequisites section).

I wanted to try first if I needed any of the listed packages in the Prerequisites section in the description or if I have them already in my system.

./src/qt/bitcoin-qt -signet
QQmlApplicationEngine failed to load component
qrc:/qml/pages/main.qml:65:13: Type OnboardingStorageLocation unavailable
qrc:/qml/pages/onboarding/OnboardingStorageLocation.qml:25:17: Type StorageLocations unavailable
qrc:/qml/components/StorageLocations.qml:8:1: module "QtQuick.Dialogs" is not installed

I've then ran the commands provided by the author of the PR to install them, but it seems I already have them and the same error persisted when I re-try to execute bitcoin-qt.

qtquickcontrols2-5-dev is already the newest version (5.15.3+dfsg-1).
0 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.
qml-module-qtquick-controls2 is already the newest version (5.15.3+dfsg-1).
0 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.
qtdeclarative5-dev is already the newest version (5.15.3+dfsg-1).
0 upgraded, 0 newly installed, 0 to remove and 11 not upgraded.

At the end I found I needed also this one qml-module-qtquick-dialogs, so please update the section accordingly if you think this is correct.

2. On-boarding/ Settings defaults behaviour against #284 and main have changed.

Before testing the new feature wanted to check previous (#284/ main) behaviour and I noticed the default values of the settings have changed.

Before, main and #284 branches screenshot:

This PR's branch D33r-Gee:qml-intro-custom-datadir screenshot:

Not only that, dbcache setting value, introduced by #284 (not merged into main yet), changes every time it's persisted after I delete the default datadir:

(e.g. 3rd. or 4th. time)

{
    "dbcache": 1930848896,
    "prune": 1907
}

(e.g. After...)

{
    "dbcache": 154710288,
    "prune": 1907
}

dbcache is calculated, there's no (at the moment) field where a user can change its value.

Note: the rest of settings are not persisted because all false or 0 (zero) values are not persisted after #284 changes.

It seems there is an issue integrating #284 or basing your code on top of it. Please keep in mind that it would be easier for reviewers to check you haven't missed any changes and that these are in the correct order.

Perhaps it's due to this removed line in 1st commit 5cc8273 (?):

3. Testing Custom Datadir Wiring feature.

First time I set the new custom data dir I get these errors:

./src/qt/bitcoin-qt -signet
Warning: Ignoring XDG_SESSION_TYPE=wayland on Gnome. Use QT_QPA_PLATFORM=wayland to run on Wayland anyway.
Model size of -23 is less than 0

(bitcoin-qt:1000052): Gtk-CRITICAL **: 19:11:15.772: gtk_image_set_from_gicon: assertion 'GTK_IS_IMAGE (image)' failed
Running initialization in thread
Running Shutdown in thread
Shutdown finished

I checked that Model size of -23 is less than 0 happens once the FileDialog opens.
I tried to get the Gtk-CRITICAL again but only happened to me the very first time I tested the app and never again.

Once I've created a custom datadir and it works fine, everything gets saved there (except for the issue with the correct value of settings mentioned in 2.).

Now, second time I run the app the settings displayed and saved are totally messed up:

{
    "dbcache": -752209344,
    "listen": true,
    "natpmp": false,
    "prune": 0,
    "server": false,
    "upnp": false
}

Another thing I noticed is that once I close the app if I delete the test chain subdirectory of the custom datadir (eg signet), which contains the settings.json file, when I start the app again still believes I'm on-boarded, don't get asked about the settings and that shouldn't be the case I think.

As part of this PR, shouldn't we show the custom datadir that has been selected by the user somewhere? Perhaps as a non editable field in the Storage settings page?

D33r-Gee · 2024-03-18T22:58:44Z

At the end I found I needed also this one qml-module-qtquick-dialogs, so please update the section accordingly if you think this is correct.

Great catch @pablomartin4btc! Updated the description

D33r-Gee · 2024-03-18T23:06:30Z

Another thing I noticed is that once I close the app if I delete the test chain subdirectory of the custom datadir (eg signet), which contains the settings.json file, when I start the app again still believes I'm on-boarded, don't get asked about the settings and that shouldn't be the case I think.

In this case the the custom directory needs to be deleted as well, since the code cached the top level directory and just the chainstate one (i.e. signet). In other words if you'd like to test onboarding again you'd have rm -r custom

D33r-Gee · 2024-03-18T23:08:31Z

As part of this PR, shouldn't we show the custom datadir that has been selected by the user somewhere? Perhaps as a non editable field in the Storage settings page?

that's a good point... will update that along with @johnny9 android patches

jarolrod

Let's make this draft as it still needs a good bit of work. It's a requirement that changes need to work with all builds (dynamic + static)

src/qml/models/onboardingmodel.h

src/qml/bitcoin.h

D33r-Gee · 2024-03-20T23:09:39Z

Let's make this draft as it still needs a good bit of work. It's a requirement that changes need to work with all builds (dynamic + static)

Great... I'll make it a draft...

BTW, @johnny9's patches for android to include the FileDialog functionality also work with the x86_64-pc-linux-gnu static build.

pablomartin4btc

Regarding the commits, please consider the contributing guidelines about the commit titles (~50 chars max).

2nd commit name/ title should be the same as the original from @johnny9, also should not be at the top?

Perhaps would be easier to make this change independent from #284, making it just the control and the component to work (file dialog, libraries, etc) and later making it persistent as a follow up while #284 gets approved? Just thinking loud about it.

…adir

…core#284

MarnixCroes

agree with

As part of this PR, shouldn't we show the custom datadir that has been selected by the user somewhere? Perhaps as a non editable field in the Storage settings page?

#390 (review)

src/qml/models/onboardingmodel.cpp

MarnixCroes · 2024-03-25T18:35:52Z

it crashed once when clicking Custom at Storage Location. 9513874

terminal output

Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Bail out! Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Aborted (core dumped)

haven't been able to repro

D33r-Gee · 2024-03-25T21:00:04Z

it crashed once when clicking Custom at Storage Location. 9513874

terminal output

Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Bail out! Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Aborted (core dumped)

haven't been able to repro

@MarnixCroes, thanks for testing and reviewing

Does this mean it the crashing is persisting or that it only happened once?

Also are you testing on Ubuntu? if so what version?

johnny9

Some minor things still to clean up.

johnny9 · 2024-03-24T00:54:43Z

src/qml/bitcoin.cpp

+
+    QObject::connect(m_onboarding_model, &OnboardingModel::onboardingFinished, this, &BitcoinQmlApplication::startNodeAndTransitionSlot);
+
+    #ifdef __ANDROID__


src/qml/bitcoin.cpp

johnny9 · 2024-03-25T02:42:31Z

src/qml/bitcoin.cpp

+
+    createNode(*m_engine, argc, argv, gArgs);
+
+    #ifdef __ANDROID__


src/qml/models/onboardingmodel.h

MarnixCroes · 2024-03-26T07:35:38Z

it crashed once when clicking Custom at Storage Location. 9513874
terminal output
Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Bail out! Gtk:ERROR:../../../../gtk/gtkpathbar.c:1342:set_button_image_get_info_cb: assertion failed: (cancellable == data->button_data->cancellable)
Aborted (core dumped)
haven't been able to repro
@MarnixCroes, thanks for testing and reviewing

Does this mean it the crashing is persisting or that it only happened once?

Also are you testing on Ubuntu? if so what version?

it only happened once. can't reproduce yet
This was indeed on ubuntu, 22.04.4 LTS

…ecially custom datadir setting and placeholders for optionsModel

…ing to default

…re now waiting for onboarding to finish before node init

…el. Also moved chainModel args to onboardingModel

D33r-Gee · 2024-03-27T18:38:41Z

Based on @pablomartin4btc and @johnny9 feedback:

Converted to draft for now and will post follow up PR with only the UI elements for more straight forward reviewing and merging.

@johnny9

649c2a5 doc: updated the qml/README.md with qml-module-qtquick-dialogs (D33r-Gee) 8131e0f qml: UI only. Added initial custom datadir functionality without wiring (D33r-Gee) ffd5201 qml: UI only. added setcustomdatadir() method into the options_model files (D33r-Gee) 0b59fee qml: statically link QtQuick2Dialog and FolderListModel plugins (johnny9) 893348e qml: fix file location path in Controls plugin for Android (johnny9) Pull request description: This pull request is a focused iteration of #390, intended to isolate and test the UI frontend elements. Backend functionality has been intentionally excluded to streamline the testing, review, and merge process. <details> <summary>Ubuntu Screenshots</summary> ![Screenshot 2024-03-14 124000](https://github.com/bitcoin-core/gui-qml/assets/111142327/a8d8ebbc-c66c-4626-ba7a-630f19c889ae) ![Screenshot 2024-01-29 074759](https://github.com/bitcoin-core/gui-qml/assets/111142327/fea1b72d-2760-436c-aba5-2094b5826fa4) ![Screenshot 2024-01-29 074734](https://github.com/bitcoin-core/gui-qml/assets/111142327/768f93a0-4f9c-46cc-9b31-7d9fcabaaf1e) </details> **Prerequisites** For testing this pull request, ensure you have the following Qt modules installed: * On Ubuntu 22.04: ```bash sudo apt-get install qtdeclarative5-dev sudo apt-get install qtquickcontrols2-5-dev sudo apt-get install qml-module-qtquick-controls2 sudo apt install qml-module-qtquick-dialogs ``` * For Android: * Make sure you delete the prior depends folder (i.e. `depends/aarch64-linux-android`) and rebuild them. **Implementation Details** This introduces `FileDialog` class for a user-friendly selection experience. * **QML Backend** * Updated `options_model` files to account for custom data directory configuration and placeholders for custom `datadir` wiring. * Added @johnny9 depends patches to allow static building * **QML Frontend** * Updated `StorageLocations.qml` to allow for custom data directory selection. Follow up PR will add display of the custom `datadir` in StorageSettings.qml ACKs for top commit: johnny9: ACK 649c2a5 pablomartin4btc: tACK 649c2a5 Tree-SHA512: f480b40aeb28df1515dd191d0fb5e8df94e80cf79c3c468d78b153ebf907b5ad34a97aa0ef6463ac29c6ddebe259b2e3fc1c25c55396c377a9900ce0c68c3edf

johnny9 suggested changes Mar 12, 2024

View reviewed changes

D33r-Gee force-pushed the qml-intro-custom-datadir branch 2 times, most recently from 87b4d75 to 76f83f2 Compare March 12, 2024 18:32

MarnixCroes reviewed Mar 14, 2024

View reviewed changes

src/qml/bitcoin.cpp Outdated Show resolved Hide resolved

D33r-Gee force-pushed the qml-intro-custom-datadir branch from 76f83f2 to b9f2cd5 Compare March 14, 2024 15:16

D33r-Gee changed the title ~~qml: Custom Datadir Wiring~~ Custom Datadir Wiring Mar 14, 2024

D33r-Gee force-pushed the qml-intro-custom-datadir branch from b9f2cd5 to 22fb833 Compare March 14, 2024 18:53

pablomartin4btc reviewed Mar 15, 2024

View reviewed changes

jarolrod reviewed Mar 20, 2024

View reviewed changes

src/qml/models/onboardingmodel.h Show resolved Hide resolved

src/qml/bitcoin.h Show resolved Hide resolved

D33r-Gee marked this pull request as draft March 21, 2024 16:42

D33r-Gee force-pushed the qml-intro-custom-datadir branch from 2c494e9 to 4246ace Compare March 21, 2024 17:15

pablomartin4btc reviewed Mar 22, 2024

View reviewed changes

D33r-Gee force-pushed the qml-intro-custom-datadir branch from 4246ace to 7d4a331 Compare March 22, 2024 18:09

D33r-Gee and others added 2 commits March 22, 2024 14:23

qml: bitcoin.h and cpp, added new class to address setting custom dat…

eea9829

…adir

qml: integrated Johny9's Only write options after onboarding bitcoin-…

590d0c2

…core#284

D33r-Gee force-pushed the qml-intro-custom-datadir branch from 7d4a331 to 9513874 Compare March 22, 2024 21:31

D33r-Gee marked this pull request as ready for review March 22, 2024 21:35

D33r-Gee requested review from johnny9, jarolrod and pablomartin4btc March 22, 2024 21:35

MarnixCroes reviewed Mar 25, 2024

View reviewed changes

src/qml/models/onboardingmodel.cpp Outdated Show resolved Hide resolved

This comment was marked as duplicate.

Sign in to view

johnny9 suggested changes Mar 26, 2024

View reviewed changes

D33r-Gee and others added 9 commits March 26, 2024 08:08

qml: added onboardingmodel files to manage the onboarding process esp…

1925378

…ecially custom datadir setting and placeholders for optionsModel

qml: StorageLocations.qml, added wiring for custom datadir and resett…

be30a30

…ing to default

qml: OnboardingCover.qml, commented out the settings code since we a…

a61a3f6

…re now waiting for onboarding to finish before node init

make: added onboardingmodel files to the Make process

e9cd5ca

qml: updated these files to use onboardingModel instead of optionsMod…

be62516

…el. Also moved chainModel args to onboardingModel

qml: fix file location path in Controls plugin for Android

6b176c4

qml: statically link QtQuick2Dialog and FolderListModel plugins

2e3f2b7

qml: fix AndroidNotifier construction

555b212

android: added permissions checks to allow custom datadir read/write

830ae0b

D33r-Gee force-pushed the qml-intro-custom-datadir branch from 9513874 to 830ae0b Compare March 26, 2024 15:09

pablomartin4btc mentioned this pull request Mar 27, 2024

DRAFT: Custom data directory support #273

Draft

D33r-Gee marked this pull request as draft March 27, 2024 18:35

D33r-Gee mentioned this pull request Mar 27, 2024

UI Only Custom Datadir #392

Merged

pablomartin4btc mentioned this pull request Apr 14, 2024

UI Only Custom Datadir Display #397

Open

D33r-Gee changed the title ~~Custom Datadir Wiring~~ DRAFT: Custom Datadir Wiring May 1, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DRAFT: Custom Datadir Wiring #390

DRAFT: Custom Datadir Wiring #390

D33r-Gee commented Mar 8, 2024 •

edited

johnny9 left a comment •

edited

D33r-Gee commented Mar 12, 2024 •

edited

D33r-Gee commented Mar 13, 2024

MarnixCroes commented Mar 14, 2024

pablomartin4btc left a comment

D33r-Gee commented Mar 18, 2024

D33r-Gee commented Mar 18, 2024

D33r-Gee commented Mar 18, 2024

jarolrod left a comment •

edited

D33r-Gee commented Mar 20, 2024

pablomartin4btc left a comment

MarnixCroes left a comment

This comment was marked as duplicate.

MarnixCroes commented Mar 25, 2024

D33r-Gee commented Mar 25, 2024

johnny9 left a comment

johnny9 Mar 24, 2024

johnny9 Mar 25, 2024

MarnixCroes commented Mar 26, 2024

D33r-Gee commented Mar 27, 2024


		QObject::connect(m_onboarding_model, &OnboardingModel::onboardingFinished, this, &BitcoinQmlApplication::startNodeAndTransitionSlot);

		#ifdef __ANDROID__

DRAFT: Custom Datadir Wiring #390

Are you sure you want to change the base?

DRAFT: Custom Datadir Wiring #390

Conversation

D33r-Gee commented Mar 8, 2024 • edited

johnny9 left a comment • edited

Choose a reason for hiding this comment

D33r-Gee commented Mar 12, 2024 • edited

D33r-Gee commented Mar 13, 2024

MarnixCroes commented Mar 14, 2024

pablomartin4btc left a comment

Choose a reason for hiding this comment

D33r-Gee commented Mar 18, 2024

D33r-Gee commented Mar 18, 2024

D33r-Gee commented Mar 18, 2024

jarolrod left a comment • edited

Choose a reason for hiding this comment

D33r-Gee commented Mar 20, 2024

pablomartin4btc left a comment

Choose a reason for hiding this comment

MarnixCroes left a comment

Choose a reason for hiding this comment

This comment was marked as duplicate.

MarnixCroes commented Mar 25, 2024

D33r-Gee commented Mar 25, 2024

johnny9 left a comment

Choose a reason for hiding this comment

johnny9 Mar 24, 2024

Choose a reason for hiding this comment

johnny9 Mar 25, 2024

Choose a reason for hiding this comment

MarnixCroes commented Mar 26, 2024

D33r-Gee commented Mar 27, 2024

D33r-Gee commented Mar 8, 2024 •

edited

johnny9 left a comment •

edited

D33r-Gee commented Mar 12, 2024 •

edited

jarolrod left a comment •

edited