-
Notifications
You must be signed in to change notification settings - Fork 44
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 Generate Snapshot Signet #446
base: main
Are you sure you want to change the base?
Conversation
d44eb7f
to
abc189a
Compare
Great start! Thanks for continuing to do this man. I have not tested on my machine, responding to screenshots below.
Top-tier PR write-up, kudos! My main gripe still is that 'load/generate snapshot' makes no sense in Connection Settings. Otherwise looks good.
Isn't the snapshot generated a pre-decided block height only? not the current block height?
Suggested copy: "A snapshot captures the state of the bitcoin network up to a certain date in the recent past. It can be imported into other bitcoin nodes to speed up their initial sync. Perhaps the "Rewinding" text is placeholder. Should be something like: You can continue using the application while the process continues in the background.** Good to have would be to show some a % progress number The View File button is awesome! Consider adding a Done button too that will send the user back to home screen/block clock... |
in the figma design where would you idealy prefer these options would be placed under? |
Great suggestions will update with the next push |
Could you share some design ideas for this? |
Could you update the figma to reflect that? |
abc189a
to
34a6f54
Compare
@GBKS could you take a look at the current iteration of the design and user flow, and le me know your thoughts? |
Thanks for testing. Ok going to take a look I think I know what the issue is
Yep that's correct. That said, in terms of display if the user has loaded a snapshot my assumption is that we still want to display that, is that correct? (see screenshot below) |
Makes sense. On the flip side, if they *did not load a snapshot during IBD, the green check icon should not be present. (the Load Snapshot text has could be greyed out too). The description part could read "Cannot load a snapshot because sync is already completed". |
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
34a6f54
to
fd78f7e
Compare
with fd78f7e rebased and addressed @GBKS feedback (please see screenshots below) Next: will address the use case that @yashrajd mentioned...
|
Let's clarify the situation around having an imported snapshot. Is there any user benefit to showing that a snapshot was imported, after the import is done? Of course, during the import, the user wants to see progress and at the end they want to see a confirmation that it was completed successfully. But after that, is there any reason to keep this note around? |
Great questions, going to let @yashrajd respond first... then will share my thoughts |
I don't have a strong preference as I said earlier, but here's why I think it should be there:
|
Thanks @GBKS and @yashrajd for the feedback. So got a compromise in the form of 3 potential use cases:
Please let me know your thoughts? |
- Extend node interface with virtual functions for UTXO snapshot loading - Add signal mechanism to monitor snapshot loading progress - Include predefined signet UTXO dataset in chainparams for validation
This introduces a Rewind Progress notification so that it is available when generating an AssumeUTXO snapshot through the UI.
This commit adds the Snapshot Gen files to the Connection Settings page. It wires the generate snapshot functionality through the node model and expands the snapshot qml worker class.
fd78f7e
to
db364e3
Compare
@yashrajd we're not removing anything and it's not a marketing thing. There is a single item for this feature that is always visible and shows the appropriate state. If you can generate a snapshot, it shows that. If you can't, it shows that you can import a snapshot. It's one or the other, or am I missing something? |
TL;DR: IMO it's just a couple of list items that don't take up prominent or important space. They can serve multiple functions (as actionable elements, show state: enabled/disabled/progress) and we always display both, it even serves as marketing/education/communication. Happy to mockup different states if @D33r-Gee needs it.
my impression is that you are saying: the Load Snapshot list item should be removed after snapshot has been imported. sorry if I misunderstood you & don't have to read the stuff below .
|
Yes that would be helpful to visualize the different use cases/states. |
Based on #424. For evaluation purposes only!
GUI Integration for Generating UTXO Snapshots
Overview
This PR adds initial GUI support for generating a
signet
UTXO snapshot at height 160,000, building on Bitcoin Core'sassumeutxo
infrastructure.What This PR Does
Implementation Details
Core Components Modified
src/node/interfaces.cpp
)src/qml/models/snapshotqml.h
)src/qml/models/nodemodel.cpp
)src/qml/components/SnapshotGenSettings.qml
)Key Design Decisions
assumeutxo
changesTesting Instructions
signet
data directoryUbuntu 22.04 Screenshots
Expected Behavior
Future Work
assumeutxo
changestestnet
andmainnet
)Notes for Reviewers
Snapshot Compatibility
signet
m_assumeutxo_data
(the changes there can be discarded once synced since
m_assumeutxo_data
is already hardcoded)This is a work in progress - feedback welcome on the approach and implementation details.