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.

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

[DRAFT] WIP QML Load Snapshot Signet #424

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

D33r-Gee
Copy link
Contributor

@D33r-Gee D33r-Gee commented Oct 11, 2024

Based on #421. For evaluation purposes only!

This adds wiring to connect the QML GUI to loading a signet utxo snapshot (160000 block height) via the connections settings.

Here a magnet link for the snapshot:

  • signet: magnet:?xt=urn:btih:9da986cb27b3980ea7fd06b21e199b148d486880&dn=utxo-signet-160000.dat&tr=udp%3A%2F%2Ftracker.bitcoin.sprovoost.nl%3A6969

This version currently does NOT work with Onboarding.

Testing:

  1. Start the node
  2. Complete onboarding
  3. Navigate to connection settings
  4. Load snapshot from provided interface (see screenshots).

If successful the snapshot progress sould fill and then the "Snapshot Loaded" portion should show and the snapshot details be accurate. Also the block clock should display 50% (see updated screenshots below)

If an error occurs check the debug.log file in the datadir (i.e. ~/.bitcoin) and search for [loadsnapshot] or [snapshot] to find the error.

This is a work in progress, do not merge!

Ubuntu 22.04 Screenshots

Screenshot 2024-10-11 100308
Screenshot 2024-10-11 100319
Load_UTXO_Snapshots_FileDialog
Screenshot 2024-11-13 144609
Screenshot 2024-11-13 144853
Screenshot 2024-11-13 144917
Screenshot 2024-11-13 145350

@D33r-Gee D33r-Gee marked this pull request as draft October 11, 2024 17:11
@D33r-Gee
Copy link
Contributor Author

With cb409e3 rebased and got rid off the extra options_mode settings which are not relevant for this evaluation.

Also deleted the extra loadsnapshot.cpp file and instead added the load snapshot function in src/node/interfaces.cpp

@yashrajd
Copy link

Should I be getting a binary specific for this issue? I tried loading a signet snapshot into the one for #421 but was unable to...

@D33r-Gee
Copy link
Contributor Author

Should I be getting a binary specific for this issue?

Yes please download the artifact for your system here

I tried loading a signet snapshot into the one for #421 but was unable to...

That's correct behaviour, 421 is UI only and doesn't not have backend wiring

@D33r-Gee
Copy link
Contributor Author

D33r-Gee commented Oct 16, 2024

With b77f7c1 minor tweaks to the src/node/interfaces.cpp snapshotload() function.

It mimics the rpc call loadtxoutset from bitcoin/bitcoin repository

Next will connect the loading to the progress bar

@D33r-Gee
Copy link
Contributor Author

D33r-Gee commented Oct 22, 2024

with c59e557 Integrated the UI with the C++ backend, implementing a progress callback function to dynamically update the loading progress based on the number of coins processed.

Touched src/validation.h and src/validation.cpp to extract the coin loading progress.

Next: will update the "Loaded Snapshot" stack with the real info from the snapshot

@D33r-Gee
Copy link
Contributor Author

With e3f8c70 moved the load snapshot functionality from nodeModel to optionsModel since it make it easier to store the snapshot info and reduces redundant variable declarations.

Copy link
Member

@jarolrod jarolrod left a comment

Choose a reason for hiding this comment

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

With e3f8c70 moved the load snapshot functionality from nodeModel to optionsModel since it make it easier to store the snapshot info and reduces redundant variable declarations.

e3f8c70 is a regression because optionsmodel is not an appropriate place for this, and we are now violating the role of settings.json to store the state of a process.

NodeModel and AppMode are more appropriate places for this information. But the entire approach here is still a concept NACK as we are performing unecessary changes to validation and chainparams.

src/qml/models/options_model.cpp Outdated Show resolved Hide resolved
if (new_snapshot_load_completed != m_snapshot_load_completed) {
m_snapshot_load_completed = new_snapshot_load_completed;
if (m_onboarded) {
m_node.updateRwSetting("snapshotloadcompleted", new_snapshot_load_completed);
Copy link
Member

Choose a reason for hiding this comment

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

This would be polluting settings.json that stores read/write configuration options for core -> with a state of a process. We cannot do this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, although I have a question for when we would want to allow the user to load a snapshot during onboarding, could the snapshot file path be stored in the settings.json?

@yashrajd
Copy link

yashrajd commented Oct 29, 2024

Ok I tested artefact#235 and functionality & UI seems to work.

Didn't see block clock so couldn't verify easily. Adding some UI feedback below:

assumeutxo-424-sc1 copy

  • fix list item width

assumeutxo-424-sc2 copy

assumeutxo-424-sc4 copy

  • fix list item width
  • fix icon size

assumeutxo-424-sc3 copy

  • blockHeight is accurate
  • the date in the copy above it seems right
  • didn't check hash accuracy
  • hash value is overflowing as you can see, I'd probably wrap it over 2-3 lines, perhaps add copy functionality

@D33r-Gee
Copy link
Contributor Author

D33r-Gee commented Oct 31, 2024

with 90dc0dd rebased over main and addressed @jarolrod concerns by reverting back to just a minimal integration of the loading snapshot functionality.

@yashrajd I will address the UI issues next

As a follow up will investigate how to access the snapshot verification progress

Also looking into how to make the snapshot data persist and be available for display (i.e. snapshot block height, date, and merkle hash)

@D33r-Gee
Copy link
Contributor Author

But the entire approach here is still a concept NACK as we are performing unecessary changes to validation and chainparams.

For the chainparams the changes there are only temporary for evaluating and testing within this repo. I checked upstream and the signet snapshot has already been hardcoded so once our repo is synced the changes in chainparams.cpp can be discarded.

This introduce the UI flow to load a AssumeUTXO snapshot
into the Bitcoin Core App. It modifies the connection seetings
and adds a SnapshotSettings file, Icon, and modified profress
bar.

It has been rebased
@D33r-Gee
Copy link
Contributor Author

D33r-Gee commented Nov 1, 2024

with 85592f5 made the snapshot activation persist on node restart by making use of the interface with chain->hasAssumedValidChain

Also changed the green-check icon size to 30 pix

Next looking into how to extract the progress from the ActivateSnapshot process

@D33r-Gee
Copy link
Contributor Author

D33r-Gee commented Nov 4, 2024

with 955c65d modified the snapshotLoad() function in src/node/interfaces.cpp and the initializeSnapshot() function in nodemodel.cpp to extract progress information using regular expressions.

This design decision aimed to keep snapshot loading mechanisms self-contained.

However, it looks like the use of std::stod is triggering some lint failures... will look into how to address that.

Perhaps the way to go is to use the handler in src/node/interfaces_ui ... will explore that path then update

@D33r-Gee
Copy link
Contributor Author

D33r-Gee commented Nov 8, 2024

with 0eba513 reverted back to minimal snapshot load functionality.

Snapshot loading progress is still not were I want it to be so in the meantime replaced the progress bar with placeholder text.

Also updated the snapshot info display. once successfully loaded, and I'm using the hardcoded info in chainparams since if the snapshot has loaded successfully it means the info there has been validated.
This is make the info persistent and not rely on the snapshot.dat file (in case it gets deleted)

@D33r-Gee
Copy link
Contributor Author

With cd2ec69 added a section to keep track of snapshot loading, it is not ideal so will look for better ways to achieve it.

Since the core functionality is there will ask for "pre-reviews" before un-drafting

id: settingsStack
currentIndex: onboarding ? 0 : snapshotVerified ? 2 : 0

function startLoading() {
Copy link
Member

Choose a reason for hiding this comment

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

We can't introduce any javascript functions

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok going to use signals and connections instead

Copy link
Contributor Author

Choose a reason for hiding this comment

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

using nodeModel member variable so that snapshotLoading persist while navigating away from settings page.

anchors.horizontalCenter: parent.horizontalCenter

// TODO: Remove this once the verification progress is available
Timer {
Copy link
Member

Choose a reason for hiding this comment

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

Can you write documentation on what we test in this PR? How we know a snapshot loaded, how we know that anything is working?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep will do

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok added documentation commnents for testing

@D33r-Gee D33r-Gee force-pushed the qml-load-snapshot-signet branch 2 times, most recently from 5d0756c to 262bb03 Compare November 13, 2024 21:23
@D33r-Gee
Copy link
Contributor Author

with 5d0756c updated SnapshotSettings.qml with comments documenting the loading process.

with 262bb03 updated nodemodel by adding the m_snapshot_loading bool so that the "Snapshot Loading" page persist if the user navigates away from it.

Next looking into how to finesse the snapshot loading progress...

        Adds minimal wiring to connect QML GUI to loading a signet UTXO snapshot via
        the connection settings. Uses SnapshotSettings.qml to allow user interaction.
	Modifies src/interfaces/node.h, src/node/interfaces.cpp
        and chainparams.cpp (temporarily for signet snapshot testing)
        to implement snapshot loading functionality through the node model.

        Current limitations:
        - Not integrated with onboarding process
        - Requires manual navigation to connection settings after initial startup
        - Snapshot verification progress is working, could be improved

        Testing:
        1. Start the node
        2. Complete onboarding
        3. Navigate to connection settings
        4. Load snapshot from provided interface
@D33r-Gee
Copy link
Contributor Author

with a6bede5 removed comments from SnapshotSettings.qml

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants